From 959b0f90e044f45c3051070c6daf02de71a49cb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 6 Jul 2022 05:33:55 +0200 Subject: [PATCH 1/5] Fix a long-standing incorrect comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- signature/policy_types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/signature/policy_types.go b/signature/policy_types.go index c6819929b..323e6157b 100644 --- a/signature/policy_types.go +++ b/signature/policy_types.go @@ -77,7 +77,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"` } From 69adefae4bfa1ededb0f2d033aaeefef32985f26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 7 Jul 2022 16:50:52 +0200 Subject: [PATCH 2/5] Fix JSON syntax in the policy.json(5) man page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- docs/containers-policy.json.5.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/containers-policy.json.5.md b/docs/containers-policy.json.5.md index 62f58550f..420c9bbc8 100644 --- a/docs/containers-policy.json.5.md +++ b/docs/containers-policy.json.5.md @@ -255,7 +255,7 @@ 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"}] /* Other docker: images use the global default policy and are rejected */ @@ -301,7 +301,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" } } ] From c8ba41095c6863f3d481232b5dc3d9930e22f898 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 7 Jul 2022 16:19:55 +0200 Subject: [PATCH 3/5] Correctly decode Cosign-generated payloads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... which can have "optional": null . Signed-off-by: Miloslav Trmač --- signature/internal/cosign_payload.go | 30 +++++++++++++---------- signature/internal/cosign_payload_test.go | 9 +++++++ 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/signature/internal/cosign_payload.go b/signature/internal/cosign_payload.go index 8027f9266..bfad9cd0b 100644 --- a/signature/internal/cosign_payload.go +++ b/signature/internal/cosign_payload.go @@ -1,6 +1,7 @@ package internal import ( + "bytes" "encoding/json" "errors" "fmt" @@ -96,20 +97,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 diff --git a/signature/internal/cosign_payload_test.go b/signature/internal/cosign_payload_test.go index 792b1aac3..65f21bd7f 100644 --- a/signature/internal/cosign_payload_test.go +++ b/signature/internal/cosign_payload_test.go @@ -131,6 +131,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 From bdb2613fe658654d87dacd1e9d61a37e741ade4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 6 Jul 2022 06:28:39 +0200 Subject: [PATCH 4/5] Add private.UnparsedImage, use it for signature handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- copy/copy.go | 2 +- internal/image/sourced.go | 6 ---- internal/image/unparsed.go | 8 +++-- internal/private/private.go | 7 +++++ internal/testing/mocks/unparsed_image.go | 6 ++++ internal/unparsedimage/wrapper.go | 38 ++++++++++++++++++++++++ signature/policy_eval.go | 19 +++++++----- signature/policy_eval_baselayer.go | 6 ++-- signature/policy_eval_signedby.go | 9 +++--- signature/policy_eval_signedby_test.go | 9 +++--- signature/policy_eval_simple.go | 10 +++---- signature/policy_eval_simple_test.go | 2 +- signature/policy_eval_test.go | 5 ++-- signature/policy_reference_match.go | 16 +++++----- signature/policy_reference_match_test.go | 2 +- 15 files changed, 100 insertions(+), 45 deletions(-) create mode 100644 internal/unparsedimage/wrapper.go 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/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/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_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..772955184 100644 --- a/signature/policy_eval_signedby_test.go +++ b/signature/policy_eval_signedby_test.go @@ -9,20 +9,21 @@ 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/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) 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..6a8cd85af 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 From 64756915ba0e05baa52d1b911c401e05f540b3bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 6 Jul 2022 06:42:37 +0200 Subject: [PATCH 5/5] Add Cosign verification support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit type: cosignSigned, with the usual keyData/keyPath. Fulcio/Rekor is not currently implemented. NOTE: This only allows a single public key, not a keyring, unlike simple signing. That seems problematic, there are known users of that. But we can fix that later by adding keyDirectory and the like. NOTE: Cosign interoperability requires use of signedIdentity: matchRepository. The fairly useful signedIdentity: remapIdentity has no repository-match functionality. NOTE: Multi-arch images need to be signed by cosign with --recursive to be accepted; c/image enforces signatures per platform. Signed-off-by: Miloslav Trmač --- docs/containers-policy.json.5.md | 37 ++- go.mod | 1 - signature/fixtures/cosign.pub | 4 + .../manifest.json | 1 + .../signature-1 | 1 + .../dir-img-cosign-mixed/manifest.json | 1 + .../fixtures/dir-img-cosign-mixed/signature-1 | 1 + .../fixtures/dir-img-cosign-mixed/signature-2 | 1 + .../manifest.json | 17 ++ .../signature-1 | 1 + .../dir-img-cosign-no-manifest/signature-1 | 1 + .../manifest.json | 1 + .../signature-1 | Bin 0 -> 276 bytes .../dir-img-cosign-valid-2/manifest.json | 1 + .../dir-img-cosign-valid-2/signature-1 | 1 + .../dir-img-cosign-valid-2/signature-2 | Bin 0 -> 589 bytes .../manifest.json | 1 + .../dir-img-cosign-valid-with-tag/signature-1 | Bin 0 -> 657 bytes .../dir-img-cosign-valid/manifest.json | 1 + .../fixtures/dir-img-cosign-valid/signature-1 | Bin 0 -> 589 bytes signature/fixtures/policy.json | 15 ++ .../fixtures/unknown-cosign-key.signature | Bin 0 -> 590 bytes signature/internal/cosign_payload.go | 50 +++- signature/internal/cosign_payload_test.go | 135 ++++++++++ signature/internal/fixtures_info_test.go | 5 + signature/internal/testdata/cosign.pub | 1 + signature/internal/testdata/valid.signature | 1 + signature/policy_config.go | 103 +++++++ signature/policy_config_test.go | 166 ++++++++++++ signature/policy_eval_cosign.go | 140 ++++++++++ signature/policy_eval_cosign_test.go | 252 ++++++++++++++++++ signature/policy_eval_signedby_test.go | 5 +- signature/policy_reference_match_test.go | 2 +- signature/policy_types.go | 19 ++ 34 files changed, 958 insertions(+), 7 deletions(-) create mode 100644 signature/fixtures/cosign.pub create mode 120000 signature/fixtures/dir-img-cosign-manifest-digest-error/manifest.json create mode 120000 signature/fixtures/dir-img-cosign-manifest-digest-error/signature-1 create mode 120000 signature/fixtures/dir-img-cosign-mixed/manifest.json create mode 120000 signature/fixtures/dir-img-cosign-mixed/signature-1 create mode 120000 signature/fixtures/dir-img-cosign-mixed/signature-2 create mode 100644 signature/fixtures/dir-img-cosign-modified-manifest/manifest.json create mode 120000 signature/fixtures/dir-img-cosign-modified-manifest/signature-1 create mode 120000 signature/fixtures/dir-img-cosign-no-manifest/signature-1 create mode 120000 signature/fixtures/dir-img-cosign-other-attachment/manifest.json create mode 100644 signature/fixtures/dir-img-cosign-other-attachment/signature-1 create mode 120000 signature/fixtures/dir-img-cosign-valid-2/manifest.json create mode 120000 signature/fixtures/dir-img-cosign-valid-2/signature-1 create mode 100644 signature/fixtures/dir-img-cosign-valid-2/signature-2 create mode 100644 signature/fixtures/dir-img-cosign-valid-with-tag/manifest.json create mode 100644 signature/fixtures/dir-img-cosign-valid-with-tag/signature-1 create mode 100644 signature/fixtures/dir-img-cosign-valid/manifest.json create mode 100644 signature/fixtures/dir-img-cosign-valid/signature-1 create mode 100644 signature/fixtures/unknown-cosign-key.signature create mode 120000 signature/internal/testdata/cosign.pub create mode 120000 signature/internal/testdata/valid.signature create mode 100644 signature/policy_eval_cosign.go create mode 100644 signature/policy_eval_cosign_test.go diff --git a/docs/containers-policy.json.5.md b/docs/containers-policy.json.5.md index 420c9bbc8..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 @@ -257,7 +275,22 @@ selectively allow individual transports and scopes as desired. form, with the explicit /library/, must be used. */ "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": { 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/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 0000000000000000000000000000000000000000..780a8b4918ac6812a1b3e3101ac9f578e67cf664 GIT binary patch literal 276 zcmXYrOK*Ze0EBz?XY92PDsEae=>aHEv>2e8cD*gI;gNm8E=4r{cOmUz4l|jVe0P~p zRly%?CHU+DSjEVGkOZuOCMRu3iYpEtfW!k8I3MHOFJ(5^le%;f7wF4A#)|KE2tS=xZzfXTOvK2MS z^uIGEK&-oVq6%H)1o1o5m^CP>!(ut-4Y5n9+lVR%v1PQcQUA%540am?!#*gj-8PX8 kG2PX{v9<5zkvJR6+DdWEU>CviK9K!(@4h%R0oUv71_zE`A^-pY literal 0 HcmV?d00001 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 0000000000000000000000000000000000000000..a84c1ba0f664c496755a9ed9012e2a4047591551 GIT binary patch literal 589 zcmYL`Ta%hV5QX!cpTVn%0?Mv#)jr5YgvhWU0yA&iRtIN>Sw#W2_P=K|N#&)hPFJ6< zKK*?wnls+BpY^$6-#;aaX%!8;l3t{YbApSk#SNQWSvfAN>-f*`_>3th)!m2LetcC% zcenII;@Lo&tQ6*IaO;8+IZMDHH=HM-IkMwRX%o5HKJ+nH!3m{KThN%I)Fn9u#HkzX zMaN-NTdHr&RA9^ozutA8xx7Mkam$kOvPl6)sz#8Z2(7W)-nE6gY74t34wKIgB`r)! zv5P-DG!(v;6j%{GjUyW0N<%q{J*1ch*vIZIv#kL{@CnqO45I_}ZZQT4>;VmHREGzc zSekNrtuD%Bs^ z{$+4%aLrkG5ciAQbe<op--}_eUaT%?hei9h_r}bz*U#8tfv8Sch1h22s*Vk_* Cf6D>@ literal 0 HcmV?d00001 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 0000000000000000000000000000000000000000..b8b8362a3468945ae95eab495f26fe0ce4fba750 GIT binary patch literal 657 zcmYjP%Z{5c6z#g7!8UCIPMC<1W`PnyOu^A4Jc4&%7_ei&4Fraa=HF}5R_(%;u6577 z=ix7;X=zb^XKhn|{ZJ}e<^CvgLl|VYw6As6zIxjD2(EF{i|~H^j`+ z5j)bDkrk>ii+yX9_sYBPF4!+GvPVTxpdI`INZbHxwT1OjC${869hNR9zN;bzhSa|k zX%&vp=3N|R9PS@l%4{Ew_~aPw#_7raC{m#R^-cO1mYHpglQ;j*AfqADXjkKtsksck z%hO4Z0i*+!K6jJuIMN1*dq*r-dh7xaGxfK#`Xa+h>bmJ3@wUqIhw?98acx*mx<6qr zrkIoNcAfr&X9XK*mJKw3SsZ3<#I8?h_R6g7t7Sw(p*6{NOA-fi(+z$exk)Y4m{C!t mi*>5|+Ao2U@3vG?#-%az@oOhE-;Pnbab2_t(OuYPo&tQ>Md9asX}4_AEJgG-HqgQe!_l z4J+d4fo15>GKPTcI^SO0kUqbsadBN8ph)y7O)O37DbL(po9oLqcY6xB+5kpd6c=)r zcm*t!zLw@#lf9`(*#2Hv+DY!A`DB9w?A=q>8DNBFkoYQ!D(2t!7{;iFEF=jDD-=6j zV1oh0A@N<%2N6;c1Czi`7}A~Imo>2cK5=(O;)Vp;dkUkI0h!bJmN#Eno?0HvXSRPC zfD3OK4=Z`Uyv-C@yyVH&eC9)kP|Mj4|Jrsqo0#&4^DBxw9I&aDx&2%|I_11>pJ*E8 z{ZsjmMqFxA=Ivil;|HR&yH23r-d+=qepT^zukPIJKQ-ZwpCV|sF#tZ&)= zWpEwPWmR|-_lw)sJd0l{v|Hcu!6a02w#Vh^}F zk)A%K|7iHRCS~6K1@&-6l(v`im8WlSP50G^E;!Y-pYO{hq)2x>xagwsG-OI=0v6q-|h1VO62lruvz0K>`PesPJ%-@mov@%PpS@_3De_J}+UL~vk`C0n 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_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_test.go b/signature/policy_eval_signedby_test.go index 772955184..e234617b7 100644 --- a/signature/policy_eval_signedby_test.go +++ b/signature/policy_eval_signedby_test.go @@ -9,6 +9,7 @@ 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" @@ -33,14 +34,14 @@ func dirImageMockWithRef(t *testing.T, dir string, ref types.ImageReference) pri 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_reference_match_test.go b/signature/policy_reference_match_test.go index 6a8cd85af..6309d286a 100644 --- a/signature/policy_reference_match_test.go +++ b/signature/policy_reference_match_test.go @@ -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 323e6157b..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: @@ -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.