Merge pull request #1598 from mtrmac/cosign-verify

Add Cosign verification support

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")
 		}

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`.
 
 <!-- ### `signedBaseLayer` -->
 
+
+### `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"
                     }
                 }
             ]

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 (

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)
 }

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 {

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)
+}

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")
+}

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
+}

signature/fixtures/cosign.pub

@@ -0,0 +1,4 @@
+-----BEGIN PUBLIC KEY-----
+MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEFNLqFhf4fiN6o/glAuYnq2jYUeL0
+vRuLu/z39pmbVwS9ff5AYnlwaP9sxREajdLY9ynM6G1sy6AAmb7Z63TsLg==
+-----END PUBLIC KEY-----

signature/fixtures/dir-img-cosign-manifest-digest-error/manifest.json

@@ -0,0 +1 @@
+../v2s1-invalid-signatures.manifest.json

signature/fixtures/dir-img-cosign-manifest-digest-error/signature-1

@@ -0,0 +1 @@
+../dir-img-cosign-valid/signature-1

signature/fixtures/dir-img-cosign-mixed/manifest.json

@@ -0,0 +1 @@
+../dir-img-cosign-valid/manifest.json

signature/fixtures/dir-img-cosign-mixed/signature-1

@@ -0,0 +1 @@
+../unknown-cosign-key.signature

signature/fixtures/dir-img-cosign-mixed/signature-2

@@ -0,0 +1 @@
+../dir-img-cosign-valid/signature-1

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"
+}

signature/fixtures/dir-img-cosign-modified-manifest/signature-1

@@ -0,0 +1 @@
+../dir-img-cosign-valid/signature-1

signature/fixtures/dir-img-cosign-no-manifest/signature-1

@@ -0,0 +1 @@
+../dir-img-cosign-valid/signature-1

signature/fixtures/dir-img-cosign-other-attachment/manifest.json

@@ -0,0 +1 @@
+../dir-img-cosign-valid/manifest.json

signature/fixtures/dir-img-cosign-valid-2/manifest.json

@@ -0,0 +1 @@
+../dir-img-cosign-valid/manifest.json

signature/fixtures/dir-img-cosign-valid-2/signature-1

@@ -0,0 +1 @@
+../dir-img-cosign-valid/signature-1

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"}]}

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"}]}

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"
+                    }
+                }
             ]
         }
     }

signature/internal/cosign_payload.go

@@ -1,17 +1,22 @@
 package internal
 
 import (
+	"bytes"
+	"crypto"
+	"encoding/base64"
 	"encoding/json"
 	"errors"
 	"fmt"
 	"time"
 
 	"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 &timestamp
-		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 &timestamp
+			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
+}