diff --git a/copy/copy.go b/copy/copy.go index 1c3b343e9..5fdb41b06 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -654,7 +654,7 @@ func (c *copier) copyOneImage(ctx context.Context, policyContext *signature.Poli sigs = []internalsig.Signature{} } else { c.Printf("Getting image source signatures\n") - s, err := src.SignaturesWithFormat(ctx) + s, err := src.UntrustedSignatures(ctx) if err != nil { return nil, "", "", perrors.Wrap(err, "reading signatures") } diff --git a/docs/containers-policy.json.5.md b/docs/containers-policy.json.5.md index 62f58550f..e495da8bd 100644 --- a/docs/containers-policy.json.5.md +++ b/docs/containers-policy.json.5.md @@ -149,7 +149,7 @@ This requirement rejects every image, and every signature. ### `signedBy` -This requirement requires an image to be signed with an expected identity, or accepts a signature if it is using an expected identity and key. +This requirement requires an image to be signed using “simple signing” with an expected identity, or accepts a signature if it is using an expected identity and key. ```js { @@ -236,6 +236,24 @@ used with `exactReference` or `exactRepository`. + +### `cosignSigned` + +This requirement requires an image to be signed using a Cosign signature with an expected identity and key. + +```js +{ + "type": "cosignSigned", + "keyPath": "/path/to/local/keyring/file", + "keyData": "base64-encoded-keyring-data", + "signedIdentity": identity_requirement +} +``` +Exactly one of `keyPath` and `keyData` must be present, containing a Cosign public key. Only signatures made by this key is accepted. + +The `signedIdentity` field has the same semantics as in the `signedBy` requirement described above. +Note that `cosign`-created signatures only contain a repository, so only `matchRepository` and `exactRepository` can be used to accept them (and that does not protect against substitution of a signed image with an unexpected tag). + ## Examples It is *strongly* recommended to set the `default` policy to `reject`, and then @@ -255,9 +273,24 @@ selectively allow individual transports and scopes as desired. "docker.io/openshift": [{"type": "insecureAcceptAnything"}], /* Similarly, allow installing the “official” busybox images. Note how the fully expanded form, with the explicit /library/, must be used. */ - "docker.io/library/busybox": [{"type": "insecureAcceptAnything"}] + "docker.io/library/busybox": [{"type": "insecureAcceptAnything"}], /* Allow installing images from all subdomains */ - "*.temporary-project.example.com": [{"type": "insecureAcceptAnything"}] + "*.temporary-project.example.com": [{"type": "insecureAcceptAnything"}], + /* A Cosign-signed repository */ + "hostname:5000/myns/cosign-signed-with-full-references": [ + { + "type": "cosignSigned", + "keyPath": "/path/to/cosign-pubkey.key" + } + ], + /* A Cosign-signed repository, accepts signatures by /usr/bin/cosign */ + "hostname:5000/myns/cosign-signed-risky": [ + { + "type": "cosignSigned", + "keyPath": "/path/to/cosign-pubkey.key", + "signedIdentity": {"type": "matchRepository"} + } + ] /* Other docker: images use the global default policy and are rejected */ }, "dir": { @@ -301,7 +334,7 @@ selectively allow individual transports and scopes as desired. "signedIdentity": { "type": "remapIdentity", "prefix": "private-mirror:5000/vendor-mirror", - "signedPrefix": "vendor.example.com", + "signedPrefix": "vendor.example.com" } } ] diff --git a/go.mod b/go.mod index 067f0bbd0..65bca9199 100644 --- a/go.mod +++ b/go.mod @@ -37,7 +37,6 @@ require ( golang.org/x/net v0.0.0-20220624214902-1bab6f366d9e golang.org/x/sync v0.0.0-20220601150217-0de741cfad7f golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 - ) require ( diff --git a/internal/image/sourced.go b/internal/image/sourced.go index ff7350113..dc09a9e04 100644 --- a/internal/image/sourced.go +++ b/internal/image/sourced.go @@ -6,7 +6,6 @@ package image import ( "context" - "github.com/containers/image/v5/internal/signature" "github.com/containers/image/v5/types" ) @@ -130,11 +129,6 @@ func (i *SourcedImage) Manifest(ctx context.Context) ([]byte, string, error) { return i.ManifestBlob, i.ManifestMIMEType, nil } -// SignaturesWithFormat is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need. -func (i *SourcedImage) SignaturesWithFormat(ctx context.Context) ([]signature.Signature, error) { - return i.UnparsedImage.signaturesWithFormat(ctx) -} - func (i *SourcedImage) LayerInfosForCopy(ctx context.Context) ([]types.BlobInfo, error) { return i.UnparsedImage.src.LayerInfosForCopy(ctx, i.UnparsedImage.instanceDigest) } diff --git a/internal/image/unparsed.go b/internal/image/unparsed.go index a72252e26..25d49f5e4 100644 --- a/internal/image/unparsed.go +++ b/internal/image/unparsed.go @@ -92,7 +92,9 @@ func (i *UnparsedImage) expectedManifestDigest() (digest.Digest, bool) { // Signatures is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need. func (i *UnparsedImage) Signatures(ctx context.Context) ([][]byte, error) { - sigs, err := i.signaturesWithFormat(ctx) + // It would be consistent to make this an internal/unparsedimage/impl.Compat wrapper, + // but this is very likely to be the only implementation ever. + sigs, err := i.UntrustedSignatures(ctx) if err != nil { return nil, err } @@ -105,8 +107,8 @@ func (i *UnparsedImage) Signatures(ctx context.Context) ([][]byte, error) { return simpleSigs, nil } -// signaturesWithFormat is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need. -func (i *UnparsedImage) signaturesWithFormat(ctx context.Context) ([]signature.Signature, error) { +// UntrustedSignatures is like ImageSource.GetSignaturesWithFormat, but the result is cached; it is OK to call this however often you need. +func (i *UnparsedImage) UntrustedSignatures(ctx context.Context) ([]signature.Signature, error) { if i.cachedSignatures == nil { sigs, err := i.src.GetSignaturesWithFormat(ctx, i.instanceDigest) if err != nil { diff --git a/internal/private/private.go b/internal/private/private.go index 613c7b136..bfd6148ce 100644 --- a/internal/private/private.go +++ b/internal/private/private.go @@ -137,3 +137,10 @@ type BadPartialRequestError struct { func (e BadPartialRequestError) Error() string { return e.Status } + +// UnparsedImage is an internal extension to the types.UnparsedImage interface. +type UnparsedImage interface { + types.UnparsedImage + // UntrustedSignatures is like ImageSource.GetSignaturesWithFormat, but the result is cached; it is OK to call this however often you need. + UntrustedSignatures(ctx context.Context) ([]signature.Signature, error) +} diff --git a/internal/testing/mocks/unparsed_image.go b/internal/testing/mocks/unparsed_image.go index 69551ec57..a2e2f84cd 100644 --- a/internal/testing/mocks/unparsed_image.go +++ b/internal/testing/mocks/unparsed_image.go @@ -3,6 +3,7 @@ package mocks import ( "context" + "github.com/containers/image/v5/internal/signature" "github.com/containers/image/v5/types" ) @@ -23,3 +24,8 @@ func (ref ForbiddenUnparsedImage) Manifest(ctx context.Context) ([]byte, string, func (ref ForbiddenUnparsedImage) Signatures(context.Context) ([][]byte, error) { panic("unexpected call to a mock function") } + +// UntrustedSignatures is a mock that panics. +func (ref ForbiddenUnparsedImage) UntrustedSignatures(ctx context.Context) ([]signature.Signature, error) { + panic("unexpected call to a mock function") +} diff --git a/internal/unparsedimage/wrapper.go b/internal/unparsedimage/wrapper.go new file mode 100644 index 000000000..fe65b1a98 --- /dev/null +++ b/internal/unparsedimage/wrapper.go @@ -0,0 +1,38 @@ +package unparsedimage + +import ( + "context" + + "github.com/containers/image/v5/internal/private" + "github.com/containers/image/v5/internal/signature" + "github.com/containers/image/v5/types" +) + +// wrapped provides the private.UnparsedImage operations +// for an object that only implements types.UnparsedImage +type wrapped struct { + types.UnparsedImage +} + +// FromPublic(unparsed) returns an object that provides the private.UnparsedImage API +func FromPublic(unparsed types.UnparsedImage) private.UnparsedImage { + if unparsed2, ok := unparsed.(private.UnparsedImage); ok { + return unparsed2 + } + return &wrapped{ + UnparsedImage: unparsed, + } +} + +// UntrustedSignatures is like ImageSource.GetSignaturesWithFormat, but the result is cached; it is OK to call this however often you need. +func (w *wrapped) UntrustedSignatures(ctx context.Context) ([]signature.Signature, error) { + sigs, err := w.Signatures(ctx) + if err != nil { + return nil, err + } + res := []signature.Signature{} + for _, sig := range sigs { + res = append(res, signature.SimpleSigningFromBlob(sig)) + } + return res, nil +} diff --git a/signature/fixtures/cosign.pub b/signature/fixtures/cosign.pub new file mode 100644 index 000000000..8dae995e3 --- /dev/null +++ b/signature/fixtures/cosign.pub @@ -0,0 +1,4 @@ +-----BEGIN PUBLIC KEY----- +MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEFNLqFhf4fiN6o/glAuYnq2jYUeL0 +vRuLu/z39pmbVwS9ff5AYnlwaP9sxREajdLY9ynM6G1sy6AAmb7Z63TsLg== +-----END PUBLIC KEY----- diff --git a/signature/fixtures/dir-img-cosign-manifest-digest-error/manifest.json b/signature/fixtures/dir-img-cosign-manifest-digest-error/manifest.json new file mode 120000 index 000000000..3dee14b4a --- /dev/null +++ b/signature/fixtures/dir-img-cosign-manifest-digest-error/manifest.json @@ -0,0 +1 @@ +../v2s1-invalid-signatures.manifest.json \ No newline at end of file diff --git a/signature/fixtures/dir-img-cosign-manifest-digest-error/signature-1 b/signature/fixtures/dir-img-cosign-manifest-digest-error/signature-1 new file mode 120000 index 000000000..c5bec7e3c --- /dev/null +++ b/signature/fixtures/dir-img-cosign-manifest-digest-error/signature-1 @@ -0,0 +1 @@ +../dir-img-cosign-valid/signature-1 \ No newline at end of file diff --git a/signature/fixtures/dir-img-cosign-mixed/manifest.json b/signature/fixtures/dir-img-cosign-mixed/manifest.json new file mode 120000 index 000000000..3c1dbaed0 --- /dev/null +++ b/signature/fixtures/dir-img-cosign-mixed/manifest.json @@ -0,0 +1 @@ +../dir-img-cosign-valid/manifest.json \ No newline at end of file diff --git a/signature/fixtures/dir-img-cosign-mixed/signature-1 b/signature/fixtures/dir-img-cosign-mixed/signature-1 new file mode 120000 index 000000000..0aee1d563 --- /dev/null +++ b/signature/fixtures/dir-img-cosign-mixed/signature-1 @@ -0,0 +1 @@ +../unknown-cosign-key.signature \ No newline at end of file diff --git a/signature/fixtures/dir-img-cosign-mixed/signature-2 b/signature/fixtures/dir-img-cosign-mixed/signature-2 new file mode 120000 index 000000000..c5bec7e3c --- /dev/null +++ b/signature/fixtures/dir-img-cosign-mixed/signature-2 @@ -0,0 +1 @@ +../dir-img-cosign-valid/signature-1 \ No newline at end of file diff --git a/signature/fixtures/dir-img-cosign-modified-manifest/manifest.json b/signature/fixtures/dir-img-cosign-modified-manifest/manifest.json new file mode 100644 index 000000000..40a935bf0 --- /dev/null +++ b/signature/fixtures/dir-img-cosign-modified-manifest/manifest.json @@ -0,0 +1,17 @@ +{ + "schemaVersion": 2, + "mediaType": "application/vnd.docker.distribution.manifest.v2+json", + "config": { + "mediaType": "application/vnd.docker.container.image.v1+json", + "size": 1512, + "digest": "sha256:961769676411f082461f9ef46626dd7a2d1e2b2a38e6a44364bcbecf51e66dd4" + }, + "layers": [ + { + "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", + "size": 2896510, + "digest": "sha256:9d16cba9fb961d1aafec9542f2bf7cb64acfc55245f9e4eb5abecd4cdc38d749" + } + ], + "extra": "this manifest has been modified" +} diff --git a/signature/fixtures/dir-img-cosign-modified-manifest/signature-1 b/signature/fixtures/dir-img-cosign-modified-manifest/signature-1 new file mode 120000 index 000000000..c5bec7e3c --- /dev/null +++ b/signature/fixtures/dir-img-cosign-modified-manifest/signature-1 @@ -0,0 +1 @@ +../dir-img-cosign-valid/signature-1 \ No newline at end of file diff --git a/signature/fixtures/dir-img-cosign-no-manifest/signature-1 b/signature/fixtures/dir-img-cosign-no-manifest/signature-1 new file mode 120000 index 000000000..c5bec7e3c --- /dev/null +++ b/signature/fixtures/dir-img-cosign-no-manifest/signature-1 @@ -0,0 +1 @@ +../dir-img-cosign-valid/signature-1 \ No newline at end of file diff --git a/signature/fixtures/dir-img-cosign-other-attachment/manifest.json b/signature/fixtures/dir-img-cosign-other-attachment/manifest.json new file mode 120000 index 000000000..3c1dbaed0 --- /dev/null +++ b/signature/fixtures/dir-img-cosign-other-attachment/manifest.json @@ -0,0 +1 @@ +../dir-img-cosign-valid/manifest.json \ No newline at end of file diff --git a/signature/fixtures/dir-img-cosign-other-attachment/signature-1 b/signature/fixtures/dir-img-cosign-other-attachment/signature-1 new file mode 100644 index 000000000..780a8b491 Binary files /dev/null and b/signature/fixtures/dir-img-cosign-other-attachment/signature-1 differ diff --git a/signature/fixtures/dir-img-cosign-valid-2/manifest.json b/signature/fixtures/dir-img-cosign-valid-2/manifest.json new file mode 120000 index 000000000..3c1dbaed0 --- /dev/null +++ b/signature/fixtures/dir-img-cosign-valid-2/manifest.json @@ -0,0 +1 @@ +../dir-img-cosign-valid/manifest.json \ No newline at end of file diff --git a/signature/fixtures/dir-img-cosign-valid-2/signature-1 b/signature/fixtures/dir-img-cosign-valid-2/signature-1 new file mode 120000 index 000000000..c5bec7e3c --- /dev/null +++ b/signature/fixtures/dir-img-cosign-valid-2/signature-1 @@ -0,0 +1 @@ +../dir-img-cosign-valid/signature-1 \ No newline at end of file diff --git a/signature/fixtures/dir-img-cosign-valid-2/signature-2 b/signature/fixtures/dir-img-cosign-valid-2/signature-2 new file mode 100644 index 000000000..a84c1ba0f Binary files /dev/null and b/signature/fixtures/dir-img-cosign-valid-2/signature-2 differ diff --git a/signature/fixtures/dir-img-cosign-valid-with-tag/manifest.json b/signature/fixtures/dir-img-cosign-valid-with-tag/manifest.json new file mode 100644 index 000000000..72fb41f65 --- /dev/null +++ b/signature/fixtures/dir-img-cosign-valid-with-tag/manifest.json @@ -0,0 +1 @@ +{"schemaVersion":2,"mediaType":"application/vnd.docker.distribution.manifest.v2+json","config":{"mediaType":"application/vnd.docker.container.image.v1+json","size":1512,"digest":"sha256:961769676411f082461f9ef46626dd7a2d1e2b2a38e6a44364bcbecf51e66dd4"},"layers":[{"mediaType":"application/vnd.docker.image.rootfs.diff.tar.gzip","size":2896510,"digest":"sha256:9d16cba9fb961d1aafec9542f2bf7cb64acfc55245f9e4eb5abecd4cdc38d749"}]} \ No newline at end of file diff --git a/signature/fixtures/dir-img-cosign-valid-with-tag/signature-1 b/signature/fixtures/dir-img-cosign-valid-with-tag/signature-1 new file mode 100644 index 000000000..b8b8362a3 Binary files /dev/null and b/signature/fixtures/dir-img-cosign-valid-with-tag/signature-1 differ diff --git a/signature/fixtures/dir-img-cosign-valid/manifest.json b/signature/fixtures/dir-img-cosign-valid/manifest.json new file mode 100644 index 000000000..72fb41f65 --- /dev/null +++ b/signature/fixtures/dir-img-cosign-valid/manifest.json @@ -0,0 +1 @@ +{"schemaVersion":2,"mediaType":"application/vnd.docker.distribution.manifest.v2+json","config":{"mediaType":"application/vnd.docker.container.image.v1+json","size":1512,"digest":"sha256:961769676411f082461f9ef46626dd7a2d1e2b2a38e6a44364bcbecf51e66dd4"},"layers":[{"mediaType":"application/vnd.docker.image.rootfs.diff.tar.gzip","size":2896510,"digest":"sha256:9d16cba9fb961d1aafec9542f2bf7cb64acfc55245f9e4eb5abecd4cdc38d749"}]} \ No newline at end of file diff --git a/signature/fixtures/dir-img-cosign-valid/signature-1 b/signature/fixtures/dir-img-cosign-valid/signature-1 new file mode 100644 index 000000000..8d7302879 Binary files /dev/null and b/signature/fixtures/dir-img-cosign-valid/signature-1 differ diff --git a/signature/fixtures/policy.json b/signature/fixtures/policy.json index 893a6e8c4..da0f816e7 100644 --- a/signature/fixtures/policy.json +++ b/signature/fixtures/policy.json @@ -132,6 +132,21 @@ "dockerReference": "registry.access.redhat.com/rhel7/rhel:latest" } } + ], + "example.com/cosign/key-data-example": [ + { + "type": "cosignSigned", + "keyData": "bm9uc2Vuc2U=" + } + ], + "example.com/cosign/key-Path-example": [ + { + "type": "cosignSigned", + "keyPath": "/keys/public-key", + "signedIdentity": { + "type": "matchRepository" + } + } ] } } diff --git a/signature/fixtures/unknown-cosign-key.signature b/signature/fixtures/unknown-cosign-key.signature new file mode 100644 index 000000000..e650a1605 Binary files /dev/null and b/signature/fixtures/unknown-cosign-key.signature differ diff --git a/signature/internal/cosign_payload.go b/signature/internal/cosign_payload.go index 8027f9266..96ef0a10d 100644 --- a/signature/internal/cosign_payload.go +++ b/signature/internal/cosign_payload.go @@ -1,6 +1,9 @@ package internal import ( + "bytes" + "crypto" + "encoding/base64" "encoding/json" "errors" "fmt" @@ -8,10 +11,12 @@ import ( "github.com/containers/image/v5/version" digest "github.com/opencontainers/go-digest" + cosignSignature "github.com/sigstore/sigstore/pkg/signature" ) const ( - cosignSignatureType = "cosign container image signature" + cosignSignatureType = "cosign container image signature" + cosignHarcodedHashAlgorithm = crypto.SHA256 ) // UntrustedCosignPayload is a parsed content of a Cosign signature payload (not the full signature) @@ -96,20 +101,23 @@ func (s *UntrustedCosignPayload) strictUnmarshalJSON(data []byte) error { var creatorID string var timestamp float64 var gotCreatorID, gotTimestamp = false, false - if err := ParanoidUnmarshalJSONObject(optional, func(key string) interface{} { - switch key { - case "creator": - gotCreatorID = true - return &creatorID - case "timestamp": - gotTimestamp = true - return ×tamp - default: - var ignore interface{} - return &ignore + // Cosign generates "optional": null if there are no user-specified annotations. + if !bytes.Equal(optional, []byte("null")) { + if err := ParanoidUnmarshalJSONObject(optional, func(key string) interface{} { + switch key { + case "creator": + gotCreatorID = true + return &creatorID + case "timestamp": + gotTimestamp = true + return ×tamp + default: + var ignore interface{} + return &ignore + } + }); err != nil { + return err } - }); err != nil { - return err } if gotCreatorID { s.UntrustedCreatorID = &creatorID @@ -147,3 +155,47 @@ func (s *UntrustedCosignPayload) strictUnmarshalJSON(data []byte) error { "docker-reference": &s.UntrustedDockerReference, }) } + +// CosignPayloadAcceptanceRules specifies how to decide whether an untrusted payload is acceptable. +// We centralize the actual parsing and data extraction in VerifyCosignPayload; this supplies +// the policy. We use an object instead of supplying func parameters to verifyAndExtractSignature +// because the functions have the same or similar types, so there is a risk of exchanging the functions; +// named members of this struct are more explicit. +type CosignPayloadAcceptanceRules struct { + ValidateSignedDockerReference func(string) error + ValidateSignedDockerManifestDigest func(digest.Digest) error +} + +// VerifyCosignPayload verifies unverifiedBase64Signature of unverifiedPayload was correctly created by publicKey, and that its principal components +// match expected values, both as specified by rules, and returns it. +// We return an *UntrustedCosignPayload, although nothing actually uses it, +// just to double-check against stupid typos. +func VerifyCosignPayload(publicKey crypto.PublicKey, unverifiedPayload []byte, unverifiedBase64Signature string, rules CosignPayloadAcceptanceRules) (*UntrustedCosignPayload, error) { + verifier, err := cosignSignature.LoadVerifier(publicKey, cosignHarcodedHashAlgorithm) + if err != nil { + return nil, fmt.Errorf("creating verifier: %w", err) + } + + unverifiedSignature, err := base64.StdEncoding.DecodeString(unverifiedBase64Signature) + if err != nil { + return nil, NewInvalidSignatureError(fmt.Sprintf("base64 decoding: %v", err)) + } + // github.com/sigstore/cosign/pkg/cosign.verifyOCISignature uses signatureoptions.WithContext(), + // which seems to be not used by anything. So we don’t bother. + if err := verifier.VerifySignature(bytes.NewReader(unverifiedSignature), bytes.NewReader(unverifiedPayload)); err != nil { + return nil, NewInvalidSignatureError(fmt.Sprintf("cryptographic signature verification failed: %v", err)) + } + + var unmatchedPayload UntrustedCosignPayload + if err := json.Unmarshal(unverifiedPayload, &unmatchedPayload); err != nil { + return nil, NewInvalidSignatureError(err.Error()) + } + if err := rules.ValidateSignedDockerManifestDigest(unmatchedPayload.UntrustedDockerManifestDigest); err != nil { + return nil, err + } + if err := rules.ValidateSignedDockerReference(unmatchedPayload.UntrustedDockerReference); err != nil { + return nil, err + } + // CosignPayloadAcceptanceRules have accepted this value. + return &unmatchedPayload, nil +} diff --git a/signature/internal/cosign_payload_test.go b/signature/internal/cosign_payload_test.go index 792b1aac3..2e28a7aa8 100644 --- a/signature/internal/cosign_payload_test.go +++ b/signature/internal/cosign_payload_test.go @@ -1,11 +1,17 @@ package internal import ( + "encoding/base64" "encoding/json" + "errors" + "os" "testing" "time" + "github.com/containers/image/v5/internal/signature" "github.com/containers/image/v5/version" + digest "github.com/opencontainers/go-digest" + "github.com/sigstore/sigstore/pkg/cryptoutils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -131,6 +137,15 @@ func TestUntrustedCosignPayloadUnmarshalJSON(t *testing.T) { s = successfullyUnmarshalUntrustedCosignPayload(t, validJSON) assert.Equal(t, validSig, s) + // A Cosign-generated payload is handled correctly + s = successfullyUnmarshalUntrustedCosignPayload(t, []byte(`{"critical":{"identity":{"docker-reference":"192.168.64.2:5000/cosign-signed-multi"},"image":{"docker-manifest-digest":"sha256:43955d6857268cc948ae9b370b221091057de83c4962da0826f9a2bdc9bd6b44"},"type":"cosign container image signature"},"optional":null}`)) + assert.Equal(t, UntrustedCosignPayload{ + UntrustedDockerManifestDigest: "sha256:43955d6857268cc948ae9b370b221091057de83c4962da0826f9a2bdc9bd6b44", + UntrustedDockerReference: "192.168.64.2:5000/cosign-signed-multi", + UntrustedCreatorID: nil, + UntrustedTimestamp: nil, + }, s) + // Various ways to corrupt the JSON breakFns := []func(mSI){ // A top-level field is missing @@ -197,3 +212,132 @@ func TestUntrustedCosignPayloadUnmarshalJSON(t *testing.T) { s = successfullyUnmarshalUntrustedCosignPayload(t, validJSON) assert.Equal(t, validSig, s) } + +func TestVerifyCosignPayload(t *testing.T) { + publicKeyPEM, err := os.ReadFile("./testdata/cosign.pub") + require.NoError(t, err) + publicKey, err := cryptoutils.UnmarshalPEMToPublicKey(publicKeyPEM) + require.NoError(t, err) + + type acceptanceData struct { + signedDockerReference string + signedDockerManifestDigest digest.Digest + } + var wanted, recorded acceptanceData + // recordingRules are a plausible CosignPayloadAcceptanceRules implementations, but equally + // importantly record that we are passing the correct values to the rule callbacks. + recordingRules := CosignPayloadAcceptanceRules{ + ValidateSignedDockerReference: func(signedDockerReference string) error { + recorded.signedDockerReference = signedDockerReference + if signedDockerReference != wanted.signedDockerReference { + return errors.New("signedDockerReference mismatch") + } + return nil + }, + ValidateSignedDockerManifestDigest: func(signedDockerManifestDigest digest.Digest) error { + recorded.signedDockerManifestDigest = signedDockerManifestDigest + if signedDockerManifestDigest != wanted.signedDockerManifestDigest { + return errors.New("signedDockerManifestDigest mismatch") + } + return nil + }, + } + + sigBlob, err := os.ReadFile("./testdata/valid.signature") + require.NoError(t, err) + genericSig, err := signature.FromBlob(sigBlob) + require.NoError(t, err) + cosignSig, ok := genericSig.(signature.Cosign) + require.True(t, ok) + cryptoBase64Sig, ok := cosignSig.UntrustedAnnotations()[signature.CosignSignatureAnnotationKey] + require.True(t, ok) + signatureData := acceptanceData{ + signedDockerReference: TestCosignSignatureReference, + signedDockerManifestDigest: TestCosignManifestDigest, + } + + // Successful verification + wanted = signatureData + recorded = acceptanceData{} + res, err := VerifyCosignPayload(publicKey, cosignSig.UntrustedPayload(), cryptoBase64Sig, recordingRules) + require.NoError(t, err) + assert.Equal(t, res, &UntrustedCosignPayload{ + UntrustedDockerManifestDigest: TestCosignManifestDigest, + UntrustedDockerReference: TestCosignSignatureReference, + UntrustedCreatorID: nil, + UntrustedTimestamp: nil, + }) + assert.Equal(t, signatureData, recorded) + + // For extra paranoia, test that we return a nil signature object on error. + + // Invalid verifier + recorded = acceptanceData{} + invalidPublicKey := struct{}{} // crypto.PublicKey is, for some reason, just an interface{}, so this is acceptable. + res, err = VerifyCosignPayload(invalidPublicKey, cosignSig.UntrustedPayload(), cryptoBase64Sig, recordingRules) + assert.Error(t, err) + assert.Nil(t, res) + assert.Equal(t, acceptanceData{}, recorded) + + // Invalid base64 encoding + for _, invalidBase64Sig := range []string{ + "&", // Invalid base64 characters + cryptoBase64Sig + "=", // Extra padding + cryptoBase64Sig[:len(cryptoBase64Sig)-1], // Truncated base64 data + } { + recorded = acceptanceData{} + res, err = VerifyCosignPayload(publicKey, cosignSig.UntrustedPayload(), invalidBase64Sig, recordingRules) + assert.Error(t, err) + assert.Nil(t, res) + assert.Equal(t, acceptanceData{}, recorded) + } + + // Invalid signature + validSignatureBytes, err := base64.StdEncoding.DecodeString(cryptoBase64Sig) + require.NoError(t, err) + for _, invalidSig := range [][]byte{ + {}, // Empty signature + []byte("invalid signature"), + append(validSignatureBytes, validSignatureBytes...), + } { + recorded = acceptanceData{} + res, err = VerifyCosignPayload(publicKey, cosignSig.UntrustedPayload(), base64.StdEncoding.EncodeToString(invalidSig), recordingRules) + assert.Error(t, err) + assert.Nil(t, res) + assert.Equal(t, acceptanceData{}, recorded) + } + + // Valid signature of non-JSON + recorded = acceptanceData{} + res, err = VerifyCosignPayload(publicKey, []byte("&"), "MEUCIARnnxZQPALBfqkB4aNAYXad79Qs6VehcrgIeZ8p7I2FAiEAzq2HXwXlz1iJeh+ucUR3L0zpjynQk6Rk0+/gXYp49RU=", recordingRules) + assert.Error(t, err) + assert.Nil(t, res) + assert.Equal(t, acceptanceData{}, recorded) + + // Valid signature of an unacceptable JSON + recorded = acceptanceData{} + res, err = VerifyCosignPayload(publicKey, []byte("{}"), "MEUCIQDkySOBGxastVP0+koTA33NH5hXjwosFau4rxTPN6g48QIgb7eWKkGqfEpHMM3aT4xiqyP/170jEkdFuciuwN4mux4=", recordingRules) + assert.Error(t, err) + assert.Nil(t, res) + assert.Equal(t, acceptanceData{}, recorded) + + // Valid signature with a wrong manifest digest: asked for signedDockerManifestDigest + wanted = signatureData + wanted.signedDockerManifestDigest = "invalid digest" + recorded = acceptanceData{} + res, err = VerifyCosignPayload(publicKey, cosignSig.UntrustedPayload(), cryptoBase64Sig, recordingRules) + assert.Error(t, err) + assert.Nil(t, res) + assert.Equal(t, acceptanceData{ + signedDockerManifestDigest: signatureData.signedDockerManifestDigest, + }, recorded) + + // Valid signature with a wrong image reference + wanted = signatureData + wanted.signedDockerReference = "unexpected docker reference" + recorded = acceptanceData{} + res, err = VerifyCosignPayload(publicKey, cosignSig.UntrustedPayload(), cryptoBase64Sig, recordingRules) + assert.Error(t, err) + assert.Nil(t, res) + assert.Equal(t, signatureData, recorded) +} diff --git a/signature/internal/fixtures_info_test.go b/signature/internal/fixtures_info_test.go index 41fce493f..3d410e920 100644 --- a/signature/internal/fixtures_info_test.go +++ b/signature/internal/fixtures_info_test.go @@ -7,4 +7,9 @@ const ( TestImageManifestDigest = digest.Digest("sha256:20bf21ed457b390829cdbeec8795a7bea1626991fda603e0d01b4e7f60427e55") // TestImageSignatureReference is the Docker image reference signed in "image.signature" TestImageSignatureReference = "testing/manifest" + + // TestCosignManifestDigest is the manifest digest of "valid.signature" + TestCosignManifestDigest = digest.Digest("sha256:634a8f35b5f16dcf4aaa0822adc0b1964bb786fca12f6831de8ddc45e5986a00") + // TestCosignSignatureReference is the Docker reference signed in "valid.signature" + TestCosignSignatureReference = "192.168.64.2:5000/cosign-signed-single-sample" ) diff --git a/signature/internal/testdata/cosign.pub b/signature/internal/testdata/cosign.pub new file mode 120000 index 000000000..96b287993 --- /dev/null +++ b/signature/internal/testdata/cosign.pub @@ -0,0 +1 @@ +../../fixtures/cosign.pub \ No newline at end of file diff --git a/signature/internal/testdata/valid.signature b/signature/internal/testdata/valid.signature new file mode 120000 index 000000000..b720f8c4d --- /dev/null +++ b/signature/internal/testdata/valid.signature @@ -0,0 +1 @@ +../../fixtures/dir-img-cosign-valid/signature-1 \ No newline at end of file diff --git a/signature/policy_config.go b/signature/policy_config.go index 51d4da275..05c345f1e 100644 --- a/signature/policy_config.go +++ b/signature/policy_config.go @@ -244,6 +244,8 @@ func newPolicyRequirementFromJSON(data []byte) (PolicyRequirement, error) { res = &prSignedBy{} case prTypeSignedBaseLayer: res = &prSignedBaseLayer{} + case prTypeCosignSigned: + res = &prCosignSigned{} default: return nil, InvalidPolicyFormatError(fmt.Sprintf("Unknown policy requirement type \"%s\"", typeField.Type)) } @@ -493,6 +495,107 @@ func (pr *prSignedBaseLayer) UnmarshalJSON(data []byte) error { return nil } +// newPRCosignSigned returns a new prCosignSigned if parameters are valid. +func newPRCosignSigned(keyPath string, keyData []byte, signedIdentity PolicyReferenceMatch) (*prCosignSigned, error) { + if len(keyPath) > 0 && len(keyData) > 0 { + return nil, InvalidPolicyFormatError("keyType and keyData cannot be used simultaneously") + } + if signedIdentity == nil { + return nil, InvalidPolicyFormatError("signedIdentity not specified") + } + return &prCosignSigned{ + prCommon: prCommon{Type: prTypeCosignSigned}, + KeyPath: keyPath, + KeyData: keyData, + SignedIdentity: signedIdentity, + }, nil +} + +// newPRCosignSignedKeyPath is NewPRCosignSignedKeyPath, except it returns the private type. +func newPRCosignSignedKeyPath(keyPath string, signedIdentity PolicyReferenceMatch) (*prCosignSigned, error) { + return newPRCosignSigned(keyPath, nil, signedIdentity) +} + +// NewPRCosignSignedKeyPath returns a new "cosignSigned" PolicyRequirement using a KeyPath +func NewPRCosignSignedKeyPath(keyPath string, signedIdentity PolicyReferenceMatch) (PolicyRequirement, error) { + return newPRCosignSignedKeyPath(keyPath, signedIdentity) +} + +// newPRCosignSignedKeyData is NewPRCosignSignedKeyData, except it returns the private type. +func newPRCosignSignedKeyData(keyData []byte, signedIdentity PolicyReferenceMatch) (*prCosignSigned, error) { + return newPRCosignSigned("", keyData, signedIdentity) +} + +// NewPRCosignSignedKeyData returns a new "cosignSigned" PolicyRequirement using a KeyData +func NewPRCosignSignedKeyData(keyData []byte, signedIdentity PolicyReferenceMatch) (PolicyRequirement, error) { + return newPRCosignSignedKeyData(keyData, signedIdentity) +} + +// Compile-time check that prCosignSigned implements json.Unmarshaler. +var _ json.Unmarshaler = (*prCosignSigned)(nil) + +// UnmarshalJSON implements the json.Unmarshaler interface. +func (pr *prCosignSigned) UnmarshalJSON(data []byte) error { + *pr = prCosignSigned{} + var tmp prCosignSigned + var gotKeyPath, gotKeyData = false, false + var signedIdentity json.RawMessage + if err := internal.ParanoidUnmarshalJSONObject(data, func(key string) interface{} { + switch key { + case "type": + return &tmp.Type + case "keyPath": + gotKeyPath = true + return &tmp.KeyPath + case "keyData": + gotKeyData = true + return &tmp.KeyData + case "signedIdentity": + return &signedIdentity + default: + return nil + } + }); err != nil { + return err + } + + if tmp.Type != prTypeCosignSigned { + return InvalidPolicyFormatError(fmt.Sprintf("Unexpected policy requirement type \"%s\"", tmp.Type)) + } + if signedIdentity == nil { + tmp.SignedIdentity = NewPRMMatchRepoDigestOrExact() + } else { + si, err := newPolicyReferenceMatchFromJSON(signedIdentity) + if err != nil { + return err + } + tmp.SignedIdentity = si + } + + var res *prCosignSigned + var err error + switch { + case gotKeyPath && gotKeyData: + return InvalidPolicyFormatError("keyPath and keyData cannot be used simultaneously") + case gotKeyPath && !gotKeyData: + res, err = newPRCosignSignedKeyPath(tmp.KeyPath, tmp.SignedIdentity) + case !gotKeyPath && gotKeyData: + res, err = newPRCosignSignedKeyData(tmp.KeyData, tmp.SignedIdentity) + case !gotKeyPath && !gotKeyData: + return InvalidPolicyFormatError("At least one of keyPath and keyData mus be specified") + default: // Coverage: This should never happen + return fmt.Errorf("Impossible keyPath/keyData presence combination!?") + } + if err != nil { + // Coverage: This cannot currently happen, creating a prCosignSigned only fails + // if signedIdentity is nil, which we replace with a default above. + return err + } + *pr = *res + + return nil +} + // newPolicyReferenceMatchFromJSON parses JSON data into a PolicyReferenceMatch implementation. func newPolicyReferenceMatchFromJSON(data []byte) (PolicyReferenceMatch, error) { var typeField prmCommon diff --git a/signature/policy_config_test.go b/signature/policy_config_test.go index 7f82d4962..60c60d028 100644 --- a/signature/policy_config_test.go +++ b/signature/policy_config_test.go @@ -98,6 +98,14 @@ var policyFixtureContents = &Policy{ "bogus/signed-identity-example": { xNewPRSignedBaseLayer(xNewPRMExactReference("registry.access.redhat.com/rhel7/rhel:latest")), }, + "example.com/cosign/key-data-example": { + xNewPRCosignSignedKeyData([]byte("nonsense"), + NewPRMMatchRepoDigestOrExact()), + }, + "example.com/cosign/key-Path-example": { + xNewPRCosignSignedKeyPath("/keys/public-key", + NewPRMMatchRepository()), + }, }, }, } @@ -948,6 +956,164 @@ func TestPRSignedBaseLayerUnmarshalJSON(t *testing.T) { }.run(t) } +// xNewPRCosignSignedKeyPath is like NewPRCosignSignedKeyPath, except it must not fail. +func xNewPRCosignSignedKeyPath(keyPath string, signedIdentity PolicyReferenceMatch) PolicyRequirement { + pr, err := NewPRCosignSignedKeyPath(keyPath, signedIdentity) + if err != nil { + panic("xNewPRCosignSignedKeyPath failed") + } + return pr +} + +// xNewPRCosignSignedKeyData is like NewPRCosignSignedKeyData, except it must not fail. +func xNewPRCosignSignedKeyData(keyData []byte, signedIdentity PolicyReferenceMatch) PolicyRequirement { + pr, err := NewPRCosignSignedKeyData(keyData, signedIdentity) + if err != nil { + panic("xNewPRCosignSignedKeyData failed") + } + return pr +} + +func TestNewPRCosignSigned(t *testing.T) { + const testPath = "/foo/bar" + testData := []byte("abc") + testIdentity := NewPRMMatchRepoDigestOrExact() + + // Success + pr, err := newPRCosignSigned(testPath, nil, testIdentity) + require.NoError(t, err) + assert.Equal(t, &prCosignSigned{ + prCommon: prCommon{prTypeCosignSigned}, + KeyPath: testPath, + KeyData: nil, + SignedIdentity: testIdentity, + }, pr) + pr, err = newPRCosignSigned("", testData, testIdentity) + require.NoError(t, err) + assert.Equal(t, &prCosignSigned{ + prCommon: prCommon{prTypeCosignSigned}, + KeyPath: "", + KeyData: testData, + SignedIdentity: testIdentity, + }, pr) + + // Both keyPath and keyData specified + _, err = newPRCosignSigned(testPath, testData, testIdentity) + assert.Error(t, err) + + // Invalid signedIdentity + _, err = newPRCosignSigned(testPath, nil, nil) + assert.Error(t, err) +} + +func TestNewPRCosignSignedKeyPath(t *testing.T) { + const testPath = "/foo/bar" + _pr, err := NewPRCosignSignedKeyPath(testPath, NewPRMMatchRepoDigestOrExact()) + require.NoError(t, err) + pr, ok := _pr.(*prCosignSigned) + require.True(t, ok) + assert.Equal(t, testPath, pr.KeyPath) + // Failure cases tested in TestNewPRCosignSigned. +} + +func TestNewPRCosignSignedKeyData(t *testing.T) { + testData := []byte("abc") + _pr, err := NewPRCosignSignedKeyData(testData, NewPRMMatchRepoDigestOrExact()) + require.NoError(t, err) + pr, ok := _pr.(*prCosignSigned) + require.True(t, ok) + assert.Equal(t, testData, pr.KeyData) + // Failure cases tested in TestNewPRCosignSigned. +} + +// Return the result of modifying validJSON with fn and unmarshaling it into *pr +func tryUnmarshalModifiedCosignSigned(t *testing.T, pr *prCosignSigned, validJSON []byte, modifyFn func(mSI)) error { + var tmp mSI + err := json.Unmarshal(validJSON, &tmp) + require.NoError(t, err) + + modifyFn(tmp) + + *pr = prCosignSigned{} + return jsonUnmarshalFromObject(t, tmp, &pr) +} + +func TestPRCosignSignedUnmarshalJSON(t *testing.T) { + keyDataTests := policyJSONUmarshallerTests{ + newDest: func() json.Unmarshaler { return &prCosignSigned{} }, + newValidObject: func() (interface{}, error) { + return NewPRCosignSignedKeyData([]byte("abc"), NewPRMMatchRepoDigestOrExact()) + }, + otherJSONParser: func(validJSON []byte) (interface{}, error) { + return newPolicyRequirementFromJSON(validJSON) + }, + breakFns: []func(mSI){ + // The "type" field is missing + func(v mSI) { delete(v, "type") }, + // Wrong "type" field + func(v mSI) { v["type"] = 1 }, + func(v mSI) { v["type"] = "this is invalid" }, + // Extra top-level sub-object + func(v mSI) { v["unexpected"] = 1 }, + // Both "keyPath" and "keyData" is missing + func(v mSI) { delete(v, "keyData") }, + // Both "keyPath" and "keyData" is present + func(v mSI) { v["keyPath"] = "/foo/bar" }, + // Invalid "keyPath" field + func(v mSI) { delete(v, "keyData"); v["keyPath"] = 1 }, + func(v mSI) { v["type"] = "this is invalid" }, + // Invalid "keyData" field + func(v mSI) { v["keyData"] = 1 }, + func(v mSI) { v["keyData"] = "this is invalid base64" }, + // Invalid "signedIdentity" field + func(v mSI) { v["signedIdentity"] = "this is invalid" }, + // "signedIdentity" an explicit nil + func(v mSI) { v["signedIdentity"] = nil }, + }, + duplicateFields: []string{"type", "keyData", "signedIdentity"}, + } + keyDataTests.run(t) + // Test the keyPath-specific aspects + policyJSONUmarshallerTests{ + newDest: func() json.Unmarshaler { return &prCosignSigned{} }, + newValidObject: func() (interface{}, error) { + return NewPRCosignSignedKeyPath("/foo/bar", NewPRMMatchRepoDigestOrExact()) + }, + otherJSONParser: func(validJSON []byte) (interface{}, error) { + return newPolicyRequirementFromJSON(validJSON) + }, + duplicateFields: []string{"type", "keyPath", "signedIdentity"}, + }.run(t) + + var pr prCosignSigned + + // Start with a valid JSON. + _, validJSON := keyDataTests.validObjectAndJSON(t) + + // Various allowed modifications to the requirement + allowedModificationFns := []func(mSI){ + // Delete the signedIdentity field + func(v mSI) { delete(v, "signedIdentity") }, + } + for _, fn := range allowedModificationFns { + err := tryUnmarshalModifiedCosignSigned(t, &pr, validJSON, fn) + require.NoError(t, err) + } + + // Various ways to set signedIdentity to the default value + signedIdentityDefaultFns := []func(mSI){ + // Set signedIdentity to the default explicitly + func(v mSI) { v["signedIdentity"] = NewPRMMatchRepoDigestOrExact() }, + // Delete the signedIdentity field + func(v mSI) { delete(v, "signedIdentity") }, + } + for _, fn := range signedIdentityDefaultFns { + err := tryUnmarshalModifiedCosignSigned(t, &pr, validJSON, fn) + require.NoError(t, err) + assert.Equal(t, NewPRMMatchRepoDigestOrExact(), pr.SignedIdentity) + } +} + func TestNewPolicyReferenceMatchFromJSON(t *testing.T) { // Sample success. Others tested in the individual PolicyReferenceMatch.UnmarshalJSON implementations. validPRM := NewPRMMatchRepoDigestOrExact() diff --git a/signature/policy_eval.go b/signature/policy_eval.go index b49474764..2edf8397c 100644 --- a/signature/policy_eval.go +++ b/signature/policy_eval.go @@ -9,6 +9,8 @@ import ( "context" "fmt" + "github.com/containers/image/v5/internal/private" + "github.com/containers/image/v5/internal/unparsedimage" "github.com/containers/image/v5/types" "github.com/sirupsen/logrus" ) @@ -55,14 +57,14 @@ type PolicyRequirement interface { // a container based on this image; use IsRunningImageAllowed instead. // - Just because a signature is accepted does not automatically mean the contents of the // signature are authorized to run code as root, or to affect system or cluster configuration. - isSignatureAuthorAccepted(ctx context.Context, image types.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) + isSignatureAuthorAccepted(ctx context.Context, image private.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) // isRunningImageAllowed returns true if the requirement allows running an image. // If it returns false, err must be non-nil, and should be an PolicyRequirementError if evaluation // succeeded but the result was rejection. // WARNING: This validates signatures and the manifest, but does not download or validate the // layers. Users must validate that the layers match their expected digests. - isRunningImageAllowed(ctx context.Context, image types.UnparsedImage) (bool, error) + isRunningImageAllowed(ctx context.Context, image private.UnparsedImage) (bool, error) } // PolicyReferenceMatch specifies a set of image identities accepted in PolicyRequirement. @@ -71,7 +73,7 @@ type PolicyReferenceMatch interface { // matchesDockerReference decides whether a specific image identity is accepted for an image // (or, usually, for the image's Reference().DockerReference()). Note that // image.Reference().DockerReference() may be nil. - matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool + matchesDockerReference(image private.UnparsedImage, signatureDockerReference string) bool } // PolicyContext encapsulates a policy and possible cached state @@ -174,7 +176,7 @@ func (pc *PolicyContext) requirementsForImageRef(ref types.ImageReference) Polic // a container based on this image; use IsRunningImageAllowed instead. // - Just because a signature is accepted does not automatically mean the contents of the // signature are authorized to run code as root, or to affect system or cluster configuration. -func (pc *PolicyContext) GetSignaturesWithAcceptedAuthor(ctx context.Context, image types.UnparsedImage) (sigs []*Signature, finalErr error) { +func (pc *PolicyContext) GetSignaturesWithAcceptedAuthor(ctx context.Context, publicImage types.UnparsedImage) (sigs []*Signature, finalErr error) { if err := pc.changeState(pcReady, pcInUse); err != nil { return nil, err } @@ -185,11 +187,12 @@ func (pc *PolicyContext) GetSignaturesWithAcceptedAuthor(ctx context.Context, im } }() + image := unparsedimage.FromPublic(publicImage) + logrus.Debugf("GetSignaturesWithAcceptedAuthor for image %s", policyIdentityLogName(image.Reference())) reqs := pc.requirementsForImageRef(image.Reference()) - // FIXME: rename Signatures to UnverifiedSignatures - // FIXME: pass context.Context + // FIXME: Use image.UntrustedSignatures, use that to improve error messages (needs tests!) unverifiedSignatures, err := image.Signatures(ctx) if err != nil { return nil, err @@ -255,7 +258,7 @@ func (pc *PolicyContext) GetSignaturesWithAcceptedAuthor(ctx context.Context, im // succeeded but the result was rejection. // WARNING: This validates signatures and the manifest, but does not download or validate the // layers. Users must validate that the layers match their expected digests. -func (pc *PolicyContext) IsRunningImageAllowed(ctx context.Context, image types.UnparsedImage) (res bool, finalErr error) { +func (pc *PolicyContext) IsRunningImageAllowed(ctx context.Context, publicImage types.UnparsedImage) (res bool, finalErr error) { if err := pc.changeState(pcReady, pcInUse); err != nil { return false, err } @@ -266,6 +269,8 @@ func (pc *PolicyContext) IsRunningImageAllowed(ctx context.Context, image types. } }() + image := unparsedimage.FromPublic(publicImage) + logrus.Debugf("IsRunningImageAllowed for image %s", policyIdentityLogName(image.Reference())) reqs := pc.requirementsForImageRef(image.Reference()) diff --git a/signature/policy_eval_baselayer.go b/signature/policy_eval_baselayer.go index 55cdd3054..a8bc01301 100644 --- a/signature/policy_eval_baselayer.go +++ b/signature/policy_eval_baselayer.go @@ -5,15 +5,15 @@ package signature import ( "context" - "github.com/containers/image/v5/types" + "github.com/containers/image/v5/internal/private" "github.com/sirupsen/logrus" ) -func (pr *prSignedBaseLayer) isSignatureAuthorAccepted(ctx context.Context, image types.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) { +func (pr *prSignedBaseLayer) isSignatureAuthorAccepted(ctx context.Context, image private.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) { return sarUnknown, nil, nil } -func (pr *prSignedBaseLayer) isRunningImageAllowed(ctx context.Context, image types.UnparsedImage) (bool, error) { +func (pr *prSignedBaseLayer) isRunningImageAllowed(ctx context.Context, image private.UnparsedImage) (bool, error) { // FIXME? Reject this at policy parsing time already? logrus.Errorf("signedBaseLayer not implemented yet!") return false, PolicyRequirementError("signedBaseLayer not implemented yet!") diff --git a/signature/policy_eval_cosign.go b/signature/policy_eval_cosign.go new file mode 100644 index 000000000..377ccad9c --- /dev/null +++ b/signature/policy_eval_cosign.go @@ -0,0 +1,140 @@ +// Policy evaluation for prCosignSigned. + +package signature + +import ( + "context" + "errors" + "fmt" + "os" + "strings" + + "github.com/containers/image/v5/internal/private" + "github.com/containers/image/v5/internal/signature" + "github.com/containers/image/v5/manifest" + "github.com/containers/image/v5/signature/internal" + digest "github.com/opencontainers/go-digest" + "github.com/sigstore/sigstore/pkg/cryptoutils" +) + +func (pr *prCosignSigned) isSignatureAuthorAccepted(ctx context.Context, image private.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) { + // We don’t know of a single user of this API, and we might return unexpected values in Signature. + // For now, just punt. + return sarRejected, nil, errors.New("isSignatureAuthorAccepted is not implemented for Cosign") +} + +func (pr *prCosignSigned) isSignatureAccepted(ctx context.Context, image private.UnparsedImage, sig signature.Cosign) (signatureAcceptanceResult, error) { + if pr.KeyPath != "" && pr.KeyData != nil { + return sarRejected, errors.New(`Internal inconsistency: both "keyPath" and "keyData" specified`) + } + // FIXME: move this to per-context initialization + var publicKeyPEM []byte + if pr.KeyData != nil { + publicKeyPEM = pr.KeyData + } else { + d, err := os.ReadFile(pr.KeyPath) + if err != nil { + return sarRejected, err + } + publicKeyPEM = d + } + + publicKey, err := cryptoutils.UnmarshalPEMToPublicKey(publicKeyPEM) + if err != nil { + return sarRejected, fmt.Errorf("parsing public key: %w", err) + } + + untrustedAnnotations := sig.UntrustedAnnotations() + untrustedBase64Signature, ok := untrustedAnnotations[signature.CosignSignatureAnnotationKey] + if !ok { + return sarRejected, fmt.Errorf("missing %s annotation", signature.CosignSignatureAnnotationKey) + } + + signature, err := internal.VerifyCosignPayload(publicKey, sig.UntrustedPayload(), untrustedBase64Signature, internal.CosignPayloadAcceptanceRules{ + ValidateSignedDockerReference: func(ref string) error { + if !pr.SignedIdentity.matchesDockerReference(image, ref) { + return PolicyRequirementError(fmt.Sprintf("Signature for identity %s is not accepted", ref)) + } + return nil + }, + ValidateSignedDockerManifestDigest: func(digest digest.Digest) error { + m, _, err := image.Manifest(ctx) + if err != nil { + return err + } + digestMatches, err := manifest.MatchesDigest(m, digest) + if err != nil { + return err + } + if !digestMatches { + return PolicyRequirementError(fmt.Sprintf("Signature for digest %s does not match", digest)) + } + return nil + }, + }) + if err != nil { + return sarRejected, err + } + if signature == nil { // A paranoid sanity check that VerifyCosignPayload has returned consistent values + return sarRejected, errors.New("internal error: VerifyCosignPayload succeeded but returned no data") // Coverage: This should never happen. + } + + return sarAccepted, nil +} + +func (pr *prCosignSigned) isRunningImageAllowed(ctx context.Context, image private.UnparsedImage) (bool, error) { + sigs, err := image.UntrustedSignatures(ctx) + if err != nil { + return false, err + } + var rejections []error + foundNonCosignSignatures := 0 + foundCosignNonAttachments := 0 + for _, s := range sigs { + cosignSig, ok := s.(signature.Cosign) + if !ok { + foundNonCosignSignatures++ + continue + } + if cosignSig.UntrustedMIMEType() != signature.CosignSignatureMIMEType { + foundCosignNonAttachments++ + continue + } + + var reason error + switch res, err := pr.isSignatureAccepted(ctx, image, cosignSig); res { + case sarAccepted: + // One accepted signature is enough. + return true, nil + case sarRejected: + reason = err + case sarUnknown: + // Huh?! This should not happen at all; treat it as any other invalid value. + fallthrough + default: + reason = fmt.Errorf(`Internal error: Unexpected signature verification result "%s"`, string(res)) + } + rejections = append(rejections, reason) + } + var summary error + switch len(rejections) { + case 0: + if foundNonCosignSignatures == 0 && foundCosignNonAttachments == 0 { + // A nice message for the most common case. + summary = PolicyRequirementError("A signature was required, but no signature exists") + } else { + summary = PolicyRequirementError(fmt.Sprintf("A signature was required, but no signature exists (%d non-Cosign signatures, %d Cosign non-signature attachments)", + foundNonCosignSignatures, foundCosignNonAttachments)) + } + case 1: + summary = rejections[0] + default: + var msgs []string + for _, e := range rejections { + msgs = append(msgs, e.Error()) + } + summary = PolicyRequirementError(fmt.Sprintf("None of the signatures were accepted, reasons: %s", + strings.Join(msgs, "; "))) + } + return false, summary +} diff --git a/signature/policy_eval_cosign_test.go b/signature/policy_eval_cosign_test.go new file mode 100644 index 000000000..c525481c7 --- /dev/null +++ b/signature/policy_eval_cosign_test.go @@ -0,0 +1,252 @@ +// Policy evaluation for prCosignSigned. + +package signature + +import ( + "context" + "encoding/base64" + "os" + "testing" + + "github.com/containers/image/v5/internal/signature" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPRrCosignSignedIsSignatureAuthorAccepted(t *testing.T) { + // Currently, this fails even with a correctly signed image. + prm := NewPRMMatchRepository() // We prefer to test with a Cosign-created signature for interoperability, and that doesn’t work with matchExact. + testImage := dirImageMock(t, "fixtures/dir-img-cosign-valid", "192.168.64.2:5000/cosign-signed-single-sample") + testImageSigBlob, err := os.ReadFile("fixtures/dir-img-cosign-valid/signature-1") + require.NoError(t, err) + + // Successful validation, with KeyData and KeyPath + pr, err := newPRCosignSignedKeyPath("fixtures/cosign.pub", prm) + require.NoError(t, err) + sar, parsedSig, err := pr.isSignatureAuthorAccepted(context.Background(), testImage, testImageSigBlob) + assertSARRejected(t, sar, parsedSig, err) +} + +// cosignSignatureFromFile returns a signature.Cosign loaded from path. +func cosignSignatureFromFile(t *testing.T, path string) signature.Cosign { + blob, err := os.ReadFile(path) + require.NoError(t, err) + genericSig, err := signature.FromBlob(blob) + require.NoError(t, err) + sig, ok := genericSig.(signature.Cosign) + require.True(t, ok) + return sig +} + +func TestPRrCosignSignedIsSignatureAccepted(t *testing.T) { + assertAccepted := func(sar signatureAcceptanceResult, err error) { + assert.Equal(t, sarAccepted, sar) + assert.NoError(t, err) + } + assertRejected := func(sar signatureAcceptanceResult, err error) { + assert.Equal(t, sarRejected, sar) + assert.Error(t, err) + } + + prm := NewPRMMatchRepository() // We prefer to test with a Cosign-created signature to ensure interoperability, and that doesn’t work with matchExact. matchExact is tested later. + testImage := dirImageMock(t, "fixtures/dir-img-cosign-valid", "192.168.64.2:5000/cosign-signed-single-sample") + testImageSig := cosignSignatureFromFile(t, "fixtures/dir-img-cosign-valid/signature-1") + + // Successful validation, with KeyData and KeyPath + pr, err := newPRCosignSignedKeyPath("fixtures/cosign.pub", prm) + require.NoError(t, err) + sar, err := pr.isSignatureAccepted(context.Background(), testImage, testImageSig) + assertAccepted(sar, err) + + keyData, err := os.ReadFile("fixtures/cosign.pub") + require.NoError(t, err) + pr, err = newPRCosignSignedKeyData(keyData, prm) + require.NoError(t, err) + sar, err = pr.isSignatureAccepted(context.Background(), testImage, testImageSig) + assertAccepted(sar, err) + + // Both KeyPath and KeyData set. Do not use newPRCosignSigned*, because it would reject this. + pr = &prCosignSigned{ + KeyPath: "/foo/bar", + KeyData: []byte("abc"), + SignedIdentity: prm, + } + // Pass nil and empty data to, kind of, test that the return value does not depend on the image. + sar, err = pr.isSignatureAccepted(context.Background(), nil, testImageSig) + assertRejected(sar, err) + + // Invalid KeyPath + pr, err = newPRCosignSignedKeyPath("/this/does/not/exist", prm) + require.NoError(t, err) + // Pass nil and empty data to, kind of, test that the return value does not depend on the image. + sar, err = pr.isSignatureAccepted(context.Background(), nil, testImageSig) + assertRejected(sar, err) + + // KeyData doesn’t contain a public key. + pr, err = newPRCosignSignedKeyData([]byte{}, prm) + require.NoError(t, err) + // Pass nil and empty data to, kind of, test that the return value does not depend on the image. + sar, err = pr.isSignatureAccepted(context.Background(), nil, testImageSig) + assertRejected(sar, err) + + // Signature has no cryptographic signature + pr, err = newPRCosignSignedKeyPath("fixtures/cosign.pub", prm) + require.NoError(t, err) + // Pass a nil pointer to, kind of, test that the return value does not depend on the image. + sar, err = pr.isSignatureAccepted(context.Background(), nil, + signature.CosignFromComponents(testImageSig.UntrustedMIMEType(), testImageSig.UntrustedPayload(), nil)) + assertRejected(sar, err) + + // A signature which does not verify + pr, err = newPRCosignSignedKeyPath("fixtures/cosign.pub", prm) + require.NoError(t, err) + // Pass a nil pointer to, kind of, test that the return value does not depend on the image. + sar, err = pr.isSignatureAccepted(context.Background(), nil, + signature.CosignFromComponents(testImageSig.UntrustedMIMEType(), testImageSig.UntrustedPayload(), map[string]string{ + signature.CosignSignatureAnnotationKey: base64.StdEncoding.EncodeToString([]byte("invalid signature")), + })) + assertRejected(sar, err) + + // A valid signature using an unknown key. + pr, err = newPRCosignSignedKeyPath("fixtures/cosign.pub", prm) + require.NoError(t, err) + // Pass a nil pointer to, kind of, test that the return value does not depend on the image. + sar, err = pr.isSignatureAccepted(context.Background(), nil, cosignSignatureFromFile(t, "fixtures/unknown-cosign-key.signature")) + assertRejected(sar, err) + + // A valid signature with a rejected identity. + nonmatchingPRM, err := NewPRMExactReference("this/doesnt:match") + require.NoError(t, err) + pr, err = newPRCosignSignedKeyPath("fixtures/cosign.pub", nonmatchingPRM) + require.NoError(t, err) + sar, err = pr.isSignatureAccepted(context.Background(), testImage, testImageSig) + assertRejected(sar, err) + + // Error reading image manifest + image := dirImageMock(t, "fixtures/dir-img-cosign-no-manifest", "192.168.64.2:5000/cosign-signed-single-sample") + pr, err = newPRCosignSignedKeyPath("fixtures/cosign.pub", prm) + require.NoError(t, err) + sar, err = pr.isSignatureAccepted(context.Background(), image, cosignSignatureFromFile(t, "fixtures/dir-img-cosign-no-manifest/signature-1")) + assertRejected(sar, err) + + // Error computing manifest digest + image = dirImageMock(t, "fixtures/dir-img-cosign-manifest-digest-error", "192.168.64.2:5000/cosign-signed-single-sample") + pr, err = newPRCosignSignedKeyPath("fixtures/cosign.pub", prm) + require.NoError(t, err) + sar, err = pr.isSignatureAccepted(context.Background(), image, cosignSignatureFromFile(t, "fixtures/dir-img-cosign-manifest-digest-error/signature-1")) + assertRejected(sar, err) + + // A valid signature with a non-matching manifest + image = dirImageMock(t, "fixtures/dir-img-cosign-modified-manifest", "192.168.64.2:5000/cosign-signed-single-sample") + pr, err = newPRCosignSignedKeyPath("fixtures/cosign.pub", prm) + require.NoError(t, err) + sar, err = pr.isSignatureAccepted(context.Background(), image, cosignSignatureFromFile(t, "fixtures/dir-img-cosign-modified-manifest/signature-1")) + assertRejected(sar, err) + + // Minimally check that the prmMatchExact also works as expected: + // - Signatures with a matching tag work + image = dirImageMock(t, "fixtures/dir-img-cosign-valid-with-tag", "192.168.64.2:5000/skopeo-signed:tag") + pr, err = newPRCosignSignedKeyPath("fixtures/cosign.pub", NewPRMMatchExact()) + require.NoError(t, err) + sar, err = pr.isSignatureAccepted(context.Background(), image, cosignSignatureFromFile(t, "fixtures/dir-img-cosign-valid-with-tag/signature-1")) + assertAccepted(sar, err) + // - Signatures with a non-matching tag are rejected + image = dirImageMock(t, "fixtures/dir-img-cosign-valid-with-tag", "192.168.64.2:5000/skopeo-signed:othertag") + pr, err = newPRCosignSignedKeyPath("fixtures/cosign.pub", NewPRMMatchExact()) + require.NoError(t, err) + sar, err = pr.isSignatureAccepted(context.Background(), image, cosignSignatureFromFile(t, "fixtures/dir-img-cosign-valid-with-tag/signature-1")) + assertRejected(sar, err) + // - Cosign-created signatures are rejected + pr, err = newPRCosignSignedKeyPath("fixtures/cosign.pub", NewPRMMatchExact()) + require.NoError(t, err) + sar, err = pr.isSignatureAccepted(context.Background(), testImage, testImageSig) + assertRejected(sar, err) +} + +func TestPRCosignSignedIsRunningImageAllowed(t *testing.T) { + prm := NewPRMMatchRepository() // We prefer to test with a Cosign-created signature to ensure interoperability, and that doesn’t work with matchExact. matchExact is tested later. + + // A simple success case: single valid signature. + image := dirImageMock(t, "fixtures/dir-img-cosign-valid", "192.168.64.2:5000/cosign-signed-single-sample") + pr, err := NewPRCosignSignedKeyPath("fixtures/cosign.pub", prm) + require.NoError(t, err) + allowed, err := pr.isRunningImageAllowed(context.Background(), image) + assertRunningAllowed(t, allowed, err) + + // Error reading signatures + invalidSigDir := createInvalidSigDir(t) + image = dirImageMock(t, invalidSigDir, "192.168.64.2:5000/cosign-signed-single-sample") + pr, err = NewPRCosignSignedKeyPath("fixtures/cosign.pub", prm) + require.NoError(t, err) + allowed, err = pr.isRunningImageAllowed(context.Background(), image) + assertRunningRejected(t, allowed, err) + + // No signatures + image = dirImageMock(t, "fixtures/dir-img-unsigned", "testing/manifest:latest") + pr, err = NewPRCosignSignedKeyPath("fixtures/cosign.pub", prm) + require.NoError(t, err) + allowed, err = pr.isRunningImageAllowed(context.Background(), image) + assertRunningRejected(t, allowed, err) + + // Only non-Cosign signatures + image = dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest") + pr, err = NewPRCosignSignedKeyPath("fixtures/cosign.pub", prm) + require.NoError(t, err) + allowed, err = pr.isRunningImageAllowed(context.Background(), image) + assertRunningRejected(t, allowed, err) + + // Only non-signature Cosign attachments + image = dirImageMock(t, "fixtures/dir-img-cosign-other-attachment", "testing/manifest:latest") + pr, err = NewPRCosignSignedKeyPath("fixtures/cosign.pub", prm) + require.NoError(t, err) + allowed, err = pr.isRunningImageAllowed(context.Background(), image) + assertRunningRejected(t, allowed, err) + + // 1 invalid signature: use dir-img-valid, but a non-matching Docker reference + image = dirImageMock(t, "fixtures/dir-img-cosign-valid", "testing/manifest:notlatest") + pr, err = NewPRCosignSignedKeyPath("fixtures/cosign.pub", prm) + require.NoError(t, err) + allowed, err = pr.isRunningImageAllowed(context.Background(), image) + assertRunningRejectedPolicyRequirement(t, allowed, err) + + // 2 valid signatures + image = dirImageMock(t, "fixtures/dir-img-cosign-valid-2", "192.168.64.2:5000/cosign-signed-single-sample") + pr, err = NewPRCosignSignedKeyPath("fixtures/cosign.pub", prm) + require.NoError(t, err) + allowed, err = pr.isRunningImageAllowed(context.Background(), image) + assertRunningAllowed(t, allowed, err) + + // One invalid, one valid signature (in this order) + image = dirImageMock(t, "fixtures/dir-img-cosign-mixed", "192.168.64.2:5000/cosign-signed-single-sample") + pr, err = NewPRCosignSignedKeyPath("fixtures/cosign.pub", prm) + require.NoError(t, err) + allowed, err = pr.isRunningImageAllowed(context.Background(), image) + assertRunningAllowed(t, allowed, err) + + // 2 invalid signajtures: use dir-img-cosign-valid-2, but a non-matching Docker reference + image = dirImageMock(t, "fixtures/dir-img-cosign-valid-2", "this/doesnt:match") + pr, err = NewPRCosignSignedKeyPath("fixtures/cosign.pub", prm) + require.NoError(t, err) + allowed, err = pr.isRunningImageAllowed(context.Background(), image) + assertRunningRejectedPolicyRequirement(t, allowed, err) + + // Minimally check that the prmMatchExact also works as expected: + // - Signatures with a matching tag work + image = dirImageMock(t, "fixtures/dir-img-cosign-valid-with-tag", "192.168.64.2:5000/skopeo-signed:tag") + pr, err = NewPRCosignSignedKeyPath("fixtures/cosign.pub", NewPRMMatchExact()) + require.NoError(t, err) + allowed, err = pr.isRunningImageAllowed(context.Background(), image) + assertRunningAllowed(t, allowed, err) + // - Signatures with a non-matching tag are rejected + image = dirImageMock(t, "fixtures/dir-img-cosign-valid-with-tag", "192.168.64.2:5000/skopeo-signed:othertag") + pr, err = NewPRCosignSignedKeyPath("fixtures/cosign.pub", NewPRMMatchExact()) + require.NoError(t, err) + allowed, err = pr.isRunningImageAllowed(context.Background(), image) + assertRunningRejectedPolicyRequirement(t, allowed, err) + // - Cosign-created signatures are rejected + image = dirImageMock(t, "fixtures/dir-img-cosign-valid", "192.168.64.2:5000/cosign-signed-single-sample") + pr, err = NewPRCosignSignedKeyPath("fixtures/cosign.pub", NewPRMMatchExact()) + require.NoError(t, err) + allowed, err = pr.isRunningImageAllowed(context.Background(), image) + assertRunningRejectedPolicyRequirement(t, allowed, err) +} diff --git a/signature/policy_eval_signedby.go b/signature/policy_eval_signedby.go index cf435da97..b04e57382 100644 --- a/signature/policy_eval_signedby.go +++ b/signature/policy_eval_signedby.go @@ -9,12 +9,12 @@ import ( "os" "strings" + "github.com/containers/image/v5/internal/private" "github.com/containers/image/v5/manifest" - "github.com/containers/image/v5/types" digest "github.com/opencontainers/go-digest" ) -func (pr *prSignedBy) isSignatureAuthorAccepted(ctx context.Context, image types.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) { +func (pr *prSignedBy) isSignatureAuthorAccepted(ctx context.Context, image private.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) { switch pr.KeyType { case SBKeyTypeGPGKeys: case SBKeyTypeSignedByGPGKeys, SBKeyTypeX509Certificates, SBKeyTypeSignedByX509CAs: @@ -89,8 +89,9 @@ func (pr *prSignedBy) isSignatureAuthorAccepted(ctx context.Context, image types return sarAccepted, signature, nil } -func (pr *prSignedBy) isRunningImageAllowed(ctx context.Context, image types.UnparsedImage) (bool, error) { - // FIXME: pass context.Context +func (pr *prSignedBy) isRunningImageAllowed(ctx context.Context, image private.UnparsedImage) (bool, error) { + // FIXME: Use image.UntrustedSignatures, use that to improve error messages + // (needs tests!) sigs, err := image.Signatures(ctx) if err != nil { return false, err diff --git a/signature/policy_eval_signedby_test.go b/signature/policy_eval_signedby_test.go index f6efefda4..e234617b7 100644 --- a/signature/policy_eval_signedby_test.go +++ b/signature/policy_eval_signedby_test.go @@ -9,20 +9,22 @@ import ( "github.com/containers/image/v5/directory" "github.com/containers/image/v5/docker/reference" "github.com/containers/image/v5/internal/image" + "github.com/containers/image/v5/internal/imagesource" + "github.com/containers/image/v5/internal/private" "github.com/containers/image/v5/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// dirImageMock returns a types.UnparsedImage for a directory, claiming a specified dockerReference. -func dirImageMock(t *testing.T, dir, dockerReference string) types.UnparsedImage { +// dirImageMock returns a private.UnparsedImage for a directory, claiming a specified dockerReference. +func dirImageMock(t *testing.T, dir, dockerReference string) private.UnparsedImage { ref, err := reference.ParseNormalizedNamed(dockerReference) require.NoError(t, err) return dirImageMockWithRef(t, dir, refImageReferenceMock{ref: ref}) } -// dirImageMockWithRef returns a types.UnparsedImage for a directory, claiming a specified ref. -func dirImageMockWithRef(t *testing.T, dir string, ref types.ImageReference) types.UnparsedImage { +// dirImageMockWithRef returns a private.UnparsedImage for a directory, claiming a specified ref. +func dirImageMockWithRef(t *testing.T, dir string, ref types.ImageReference) private.UnparsedImage { srcRef, err := directory.NewReference(dir) require.NoError(t, err) src, err := srcRef.NewImageSource(context.Background(), nil) @@ -32,14 +34,14 @@ func dirImageMockWithRef(t *testing.T, dir string, ref types.ImageReference) typ require.NoError(t, err) }) return image.UnparsedInstance(&dirImageSourceMock{ - ImageSource: src, + ImageSource: imagesource.FromPublic(src), ref: ref, }, nil) } // dirImageSourceMock inherits dirImageSource, but overrides its Reference method. type dirImageSourceMock struct { - types.ImageSource + private.ImageSource ref types.ImageReference } diff --git a/signature/policy_eval_simple.go b/signature/policy_eval_simple.go index f949088b5..031866f0d 100644 --- a/signature/policy_eval_simple.go +++ b/signature/policy_eval_simple.go @@ -6,24 +6,24 @@ import ( "context" "fmt" + "github.com/containers/image/v5/internal/private" "github.com/containers/image/v5/transports" - "github.com/containers/image/v5/types" ) -func (pr *prInsecureAcceptAnything) isSignatureAuthorAccepted(ctx context.Context, image types.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) { +func (pr *prInsecureAcceptAnything) isSignatureAuthorAccepted(ctx context.Context, image private.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) { // prInsecureAcceptAnything semantics: Every image is allowed to run, // but this does not consider the signature as verified. return sarUnknown, nil, nil } -func (pr *prInsecureAcceptAnything) isRunningImageAllowed(ctx context.Context, image types.UnparsedImage) (bool, error) { +func (pr *prInsecureAcceptAnything) isRunningImageAllowed(ctx context.Context, image private.UnparsedImage) (bool, error) { return true, nil } -func (pr *prReject) isSignatureAuthorAccepted(ctx context.Context, image types.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) { +func (pr *prReject) isSignatureAuthorAccepted(ctx context.Context, image private.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) { return sarRejected, nil, PolicyRequirementError(fmt.Sprintf("Any signatures for image %s are rejected by policy.", transports.ImageName(image.Reference()))) } -func (pr *prReject) isRunningImageAllowed(ctx context.Context, image types.UnparsedImage) (bool, error) { +func (pr *prReject) isRunningImageAllowed(ctx context.Context, image private.UnparsedImage) (bool, error) { return false, PolicyRequirementError(fmt.Sprintf("Running image %s is rejected by policy.", transports.ImageName(image.Reference()))) } diff --git a/signature/policy_eval_simple_test.go b/signature/policy_eval_simple_test.go index e2bce36b9..3249090b5 100644 --- a/signature/policy_eval_simple_test.go +++ b/signature/policy_eval_simple_test.go @@ -8,7 +8,7 @@ import ( "github.com/containers/image/v5/types" ) -// nameOnlyImageMock is a mock of types.UnparsedImage which only allows transports.ImageName to work +// nameOnlyImageMock is a mock of private.UnparsedImage which only allows transports.ImageName to work type nameOnlyImageMock struct { mocks.ForbiddenUnparsedImage } diff --git a/signature/policy_eval_test.go b/signature/policy_eval_test.go index 981978471..af3c3e4ae 100644 --- a/signature/policy_eval_test.go +++ b/signature/policy_eval_test.go @@ -8,6 +8,7 @@ import ( "github.com/containers/image/v5/docker" "github.com/containers/image/v5/docker/policyconfiguration" "github.com/containers/image/v5/docker/reference" + "github.com/containers/image/v5/internal/private" "github.com/containers/image/v5/internal/testing/mocks" "github.com/containers/image/v5/transports" "github.com/containers/image/v5/types" @@ -202,8 +203,8 @@ func TestPolicyContextRequirementsForImageRef(t *testing.T) { } } -// pcImageMock returns a types.UnparsedImage for a directory, claiming a specified dockerReference and implementing PolicyConfigurationIdentity/PolicyConfigurationNamespaces. -func pcImageMock(t *testing.T, dir, dockerReference string) types.UnparsedImage { +// pcImageMock returns a private.UnparsedImage for a directory, claiming a specified dockerReference and implementing PolicyConfigurationIdentity/PolicyConfigurationNamespaces. +func pcImageMock(t *testing.T, dir, dockerReference string) private.UnparsedImage { ref, err := reference.ParseNormalizedNamed(dockerReference) require.NoError(t, err) return dirImageMockWithRef(t, dir, pcImageReferenceMock{transportName: "docker", ref: ref}) diff --git a/signature/policy_reference_match.go b/signature/policy_reference_match.go index 064866cf6..4e70c0f2e 100644 --- a/signature/policy_reference_match.go +++ b/signature/policy_reference_match.go @@ -7,12 +7,12 @@ import ( "strings" "github.com/containers/image/v5/docker/reference" + "github.com/containers/image/v5/internal/private" "github.com/containers/image/v5/transports" - "github.com/containers/image/v5/types" ) // parseImageAndDockerReference converts an image and a reference string into two parsed entities, failing on any error and handling unidentified images. -func parseImageAndDockerReference(image types.UnparsedImage, s2 string) (reference.Named, reference.Named, error) { +func parseImageAndDockerReference(image private.UnparsedImage, s2 string) (reference.Named, reference.Named, error) { r1 := image.Reference().DockerReference() if r1 == nil { return nil, nil, PolicyRequirementError(fmt.Sprintf("Docker reference match attempted on image %s with no known Docker reference identity", @@ -25,7 +25,7 @@ func parseImageAndDockerReference(image types.UnparsedImage, s2 string) (referen return r1, r2, nil } -func (prm *prmMatchExact) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool { +func (prm *prmMatchExact) matchesDockerReference(image private.UnparsedImage, signatureDockerReference string) bool { intended, signature, err := parseImageAndDockerReference(image, signatureDockerReference) if err != nil { return false @@ -56,7 +56,7 @@ func matchRepoDigestOrExactReferenceValues(intended, signature reference.Named) return false } } -func (prm *prmMatchRepoDigestOrExact) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool { +func (prm *prmMatchRepoDigestOrExact) matchesDockerReference(image private.UnparsedImage, signatureDockerReference string) bool { intended, signature, err := parseImageAndDockerReference(image, signatureDockerReference) if err != nil { return false @@ -64,7 +64,7 @@ func (prm *prmMatchRepoDigestOrExact) matchesDockerReference(image types.Unparse return matchRepoDigestOrExactReferenceValues(intended, signature) } -func (prm *prmMatchRepository) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool { +func (prm *prmMatchRepository) matchesDockerReference(image private.UnparsedImage, signatureDockerReference string) bool { intended, signature, err := parseImageAndDockerReference(image, signatureDockerReference) if err != nil { return false @@ -85,7 +85,7 @@ func parseDockerReferences(s1, s2 string) (reference.Named, reference.Named, err return r1, r2, nil } -func (prm *prmExactReference) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool { +func (prm *prmExactReference) matchesDockerReference(image private.UnparsedImage, signatureDockerReference string) bool { intended, signature, err := parseDockerReferences(prm.DockerReference, signatureDockerReference) if err != nil { return false @@ -97,7 +97,7 @@ func (prm *prmExactReference) matchesDockerReference(image types.UnparsedImage, return signature.String() == intended.String() } -func (prm *prmExactRepository) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool { +func (prm *prmExactRepository) matchesDockerReference(image private.UnparsedImage, signatureDockerReference string) bool { intended, signature, err := parseDockerReferences(prm.DockerRepository, signatureDockerReference) if err != nil { return false @@ -141,7 +141,7 @@ func (prm *prmRemapIdentity) remapReferencePrefix(ref reference.Named) (referenc return newParsedRef, nil } -func (prm *prmRemapIdentity) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool { +func (prm *prmRemapIdentity) matchesDockerReference(image private.UnparsedImage, signatureDockerReference string) bool { intended, signature, err := parseImageAndDockerReference(image, signatureDockerReference) if err != nil { return false diff --git a/signature/policy_reference_match_test.go b/signature/policy_reference_match_test.go index 746f90269..6309d286a 100644 --- a/signature/policy_reference_match_test.go +++ b/signature/policy_reference_match_test.go @@ -52,7 +52,7 @@ func TestParseImageAndDockerReference(t *testing.T) { } } -// refImageMock is a mock of types.UnparsedImage which returns itself in Reference().DockerReference. +// refImageMock is a mock of private.UnparsedImage which returns itself in Reference().DockerReference. type refImageMock struct { mocks.ForbiddenUnparsedImage ref reference.Named @@ -143,7 +143,7 @@ var prmRepositoryMatchTestTable = []prmSymmetricTableTest{ {"busybox:notlatest", "docker.io/library/busybox:latest", true}, {"busybox:latest", "busybox" + digestSuffix, true}, {"busybox" + digestSuffix, "busybox" + digestSuffixOther, true}, // Even this is accepted here. (This could more reasonably happen with two different digest algorithms.) - // The same as above, but with defaulted tags (should not actually happen) + // The same as above, but with defaulted tags (which can happen with Cosign) {"busybox", "busybox:notlatest", true}, {fullRHELRef, untaggedRHELRef, true}, {"busybox", "busybox" + digestSuffix, true}, diff --git a/signature/policy_types.go b/signature/policy_types.go index c6819929b..1126fbc8e 100644 --- a/signature/policy_types.go +++ b/signature/policy_types.go @@ -46,6 +46,7 @@ const ( prTypeReject prTypeIdentifier = "reject" prTypeSignedBy prTypeIdentifier = "signedBy" prTypeSignedBaseLayer prTypeIdentifier = "signedBaseLayer" + prTypeCosignSigned prTypeIdentifier = "cosignSigned" ) // prInsecureAcceptAnything is a PolicyRequirement with type = prTypeInsecureAcceptAnything: @@ -77,7 +78,7 @@ type prSignedBy struct { KeyData []byte `json:"keyData,omitempty"` // SignedIdentity specifies what image identity the signature must be claiming about the image. - // Defaults to "match-exact" if not specified. + // Defaults to "matchRepoDigestOrExact" if not specified. SignedIdentity PolicyReferenceMatch `json:"signedIdentity"` } @@ -104,6 +105,24 @@ type prSignedBaseLayer struct { BaseLayerIdentity PolicyReferenceMatch `json:"baseLayerIdentity"` } +// prCosignSigned is a PolicyRequirement with type = prTypeCosignSigned: the image is signed by trusted keys for a specified identity +type prCosignSigned struct { + prCommon + + // KeyPath is a pathname to a local file containing the trusted key. Exactly one of KeyPath and KeyData must be specified. + KeyPath string `json:"keyPath,omitempty"` + // KeyData contains the trusted key, base64-encoded. Exactly one of KeyPath and KeyData must be specified. + KeyData []byte `json:"keyData,omitempty"` + // FIXME: Multiple public keys? + + // FIXME: Support fulcio+rekor as an alternative. + + // SignedIdentity specifies what image identity the signature must be claiming about the image. + // Defaults to "matchRepoDigestOrExact" if not specified. + // Note that Cosign interoperability might require using repo-only matching. + SignedIdentity PolicyReferenceMatch `json:"signedIdentity"` +} + // PolicyReferenceMatch specifies a set of image identities accepted in PolicyRequirement. // The type is public, but its implementation is private.