Merge pull request #1593 from mtrmac/cosign-sigs

Generalize signature copy support

copy/copy.go

@@ -19,6 +19,7 @@ import (
 	"github.com/containers/image/v5/internal/imagesource"
 	"github.com/containers/image/v5/internal/pkg/platform"
 	"github.com/containers/image/v5/internal/private"
+	internalsig "github.com/containers/image/v5/internal/signature"
 	"github.com/containers/image/v5/manifest"
 	"github.com/containers/image/v5/pkg/blobinfocache"
 	"github.com/containers/image/v5/pkg/compression"
@@ -396,12 +397,12 @@ func (c *copier) copyMultipleImages(ctx context.Context, policyContext *signatur
 	updatedList := originalList.Clone()
 
 	// Read and/or clear the set of signatures for this list.
-	var sigs [][]byte
+	var sigs []internalsig.Signature
 	if options.RemoveSignatures {
-		sigs = [][]byte{}
+		sigs = []internalsig.Signature{}
 	} else {
 		c.Printf("Getting image list signatures\n")
-		s, err := c.rawSource.GetSignatures(ctx, nil)
+		s, err := c.rawSource.GetSignaturesWithFormat(ctx, nil)
 		if err != nil {
 			return nil, perrors.Wrap(err, "reading signatures")
 		}
@@ -576,7 +577,7 @@ func (c *copier) copyMultipleImages(ctx context.Context, policyContext *signatur
 	}
 
 	c.Printf("Storing list signatures\n")
-	if err := c.dest.PutSignatures(ctx, sigs, nil); err != nil {
+	if err := c.dest.PutSignaturesWithFormat(ctx, sigs, nil); err != nil {
 		return nil, perrors.Wrap(err, "writing signatures")
 	}
 
@@ -639,12 +640,12 @@ func (c *copier) copyOneImage(ctx context.Context, policyContext *signature.Poli
 		return nil, "", "", err
 	}
 
-	var sigs [][]byte
+	var sigs []internalsig.Signature
 	if options.RemoveSignatures {
-		sigs = [][]byte{}
+		sigs = []internalsig.Signature{}
 	} else {
 		c.Printf("Getting image source signatures\n")
-		s, err := src.Signatures(ctx)
+		s, err := src.SignaturesWithFormat(ctx)
 		if err != nil {
 			return nil, "", "", perrors.Wrap(err, "reading signatures")
 		}
@@ -807,7 +808,7 @@ func (c *copier) copyOneImage(ctx context.Context, policyContext *signature.Poli
 	}
 
 	c.Printf("Storing signatures\n")
-	if err := c.dest.PutSignatures(ctx, sigs, targetInstance); err != nil {
+	if err := c.dest.PutSignaturesWithFormat(ctx, sigs, targetInstance); err != nil {
 		return nil, "", "", perrors.Wrap(err, "writing signatures")
 	}
 

copy/sign.go

@@ -4,13 +4,14 @@ import (
 	"fmt"
 
 	"github.com/containers/image/v5/docker/reference"
+	internalsig "github.com/containers/image/v5/internal/signature"
 	"github.com/containers/image/v5/signature"
 	"github.com/containers/image/v5/transports"
 	perrors "github.com/pkg/errors"
 )
 
 // createSignature creates a new signature of manifest using keyIdentity.
-func (c *copier) createSignature(manifest []byte, keyIdentity string, passphrase string, identity reference.Named) ([]byte, error) {
+func (c *copier) createSignature(manifest []byte, keyIdentity string, passphrase string, identity reference.Named) (internalsig.Signature, error) {
 	mech, err := signature.NewGPGSigningMechanism()
 	if err != nil {
 		return nil, perrors.Wrap(err, "initializing GPG")
@@ -36,5 +37,5 @@ func (c *copier) createSignature(manifest []byte, keyIdentity string, passphrase
 	if err != nil {
 		return nil, perrors.Wrap(err, "creating signature")
 	}
-	return newSig, nil
+	return internalsig.SimpleSigningFromBlob(newSig), nil
 }

copy/sign_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/containers/image/v5/directory"
 	"github.com/containers/image/v5/docker"
 	"github.com/containers/image/v5/internal/imagedestination"
+	internalsig "github.com/containers/image/v5/internal/signature"
 	"github.com/containers/image/v5/manifest"
 	"github.com/containers/image/v5/signature"
 	"github.com/containers/image/v5/types"
@@ -81,7 +82,9 @@ func TestCreateSignature(t *testing.T) {
 	// Signing without overriding the identity uses the docker reference
 	sig, err := c.createSignature(manifestBlob, testKeyFingerprint, "", nil)
 	require.NoError(t, err)
-	verified, err := signature.VerifyDockerManifestSignature(sig, manifestBlob, "docker.io/library/busybox:latest", mech, testKeyFingerprint)
+	simpleSig, ok := sig.(internalsig.SimpleSigning)
+	require.True(t, ok)
+	verified, err := signature.VerifyDockerManifestSignature(simpleSig.UntrustedSignature(), manifestBlob, "docker.io/library/busybox:latest", mech, testKeyFingerprint)
 	require.NoError(t, err)
 	assert.Equal(t, "docker.io/library/busybox:latest", verified.DockerReference)
 	assert.Equal(t, manifestDigest, verified.DockerManifestDigest)
@@ -91,7 +94,9 @@ func TestCreateSignature(t *testing.T) {
 	require.NoError(t, err)
 	sig, err = c.createSignature(manifestBlob, testKeyFingerprint, "", ref)
 	require.NoError(t, err)
-	verified, err = signature.VerifyDockerManifestSignature(sig, manifestBlob, "myregistry.io/myrepo:mytag", mech, testKeyFingerprint)
+	simpleSig, ok = sig.(internalsig.SimpleSigning)
+	require.True(t, ok)
+	verified, err = signature.VerifyDockerManifestSignature(simpleSig.UntrustedSignature(), manifestBlob, "myregistry.io/myrepo:mytag", mech, testKeyFingerprint)
 	require.NoError(t, err)
 	assert.Equal(t, "myregistry.io/myrepo:mytag", verified.DockerReference)
 	assert.Equal(t, manifestDigest, verified.DockerManifestDigest)

directory/directory_dest.go

@@ -13,6 +13,7 @@ import (
 	"github.com/containers/image/v5/internal/imagedestination/stubs"
 	"github.com/containers/image/v5/internal/private"
 	"github.com/containers/image/v5/internal/putblobdigest"
+	"github.com/containers/image/v5/internal/signature"
 	"github.com/containers/image/v5/types"
 	"github.com/opencontainers/go-digest"
 	perrors "github.com/pkg/errors"
@@ -218,12 +219,17 @@ func (d *dirImageDestination) PutManifest(ctx context.Context, manifest []byte,
 	return os.WriteFile(d.ref.manifestPath(instanceDigest), manifest, 0644)
 }
 
-// PutSignatures writes a set of signatures to the destination.
+// PutSignaturesWithFormat writes a set of signatures to the destination.
 // If instanceDigest is not nil, it contains a digest of the specific manifest instance to write or overwrite the signatures for
 // (when the primary manifest is a manifest list); this should always be nil if the primary manifest is not a manifest list.
-func (d *dirImageDestination) PutSignatures(ctx context.Context, signatures [][]byte, instanceDigest *digest.Digest) error {
+// MUST be called after PutManifest (signatures may reference manifest contents).
+func (d *dirImageDestination) PutSignaturesWithFormat(ctx context.Context, signatures []signature.Signature, instanceDigest *digest.Digest) error {
 	for i, sig := range signatures {
-		if err := os.WriteFile(d.ref.signaturePath(i, instanceDigest), sig, 0644); err != nil {
+		blob, err := signature.Blob(sig)
+		if err != nil {
+			return err
+		}
+		if err := os.WriteFile(d.ref.signaturePath(i, instanceDigest), blob, 0644); err != nil {
 			return err
 		}
 	}

directory/directory_src.go

@@ -2,18 +2,21 @@ package directory
 
 import (
 	"context"
+	"fmt"
 	"io"
 	"os"
 
 	"github.com/containers/image/v5/internal/imagesource/impl"
 	"github.com/containers/image/v5/internal/imagesource/stubs"
 	"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/types"
 	"github.com/opencontainers/go-digest"
 )
 
 type dirImageSource struct {
+	impl.Compat
 	impl.PropertyMethodsInitialize
 	impl.DoesNotAffectLayerInfosForCopy
 	stubs.NoGetBlobAtInitialize
@@ -24,14 +27,16 @@ type dirImageSource struct {
 // newImageSource returns an ImageSource reading from an existing directory.
 // The caller must call .Close() on the returned ImageSource.
 func newImageSource(ref dirReference) private.ImageSource {
-	return &dirImageSource{
+	s := &dirImageSource{
 		PropertyMethodsInitialize: impl.PropertyMethods(impl.Properties{
 			HasThreadSafeGetBlob: false,
 		}),
 		NoGetBlobAtInitialize: stubs.NoGetBlobAt(ref),
 
 		ref: ref,
 	}
+	s.Compat = impl.AddCompat(s)
+	return s
 }
 
 // Reference returns the reference used to set up this source, _as specified by the user_
@@ -72,20 +77,25 @@ func (s *dirImageSource) GetBlob(ctx context.Context, info types.BlobInfo, cache
 	return r, fi.Size(), nil
 }
 
-// GetSignatures returns the image's signatures.  It may use a remote (= slow) service.
+// GetSignaturesWithFormat returns the image's signatures.  It may use a remote (= slow) service.
 // If instanceDigest is not nil, it contains a digest of the specific manifest instance to retrieve signatures for
 // (when the primary manifest is a manifest list); this never happens if the primary manifest is not a manifest list
 // (e.g. if the source never returns manifest lists).
-func (s *dirImageSource) GetSignatures(ctx context.Context, instanceDigest *digest.Digest) ([][]byte, error) {
-	signatures := [][]byte{}
+func (s *dirImageSource) GetSignaturesWithFormat(ctx context.Context, instanceDigest *digest.Digest) ([]signature.Signature, error) {
+	signatures := []signature.Signature{}
 	for i := 0; ; i++ {
-		signature, err := os.ReadFile(s.ref.signaturePath(i, instanceDigest))
+		path := s.ref.signaturePath(i, instanceDigest)
+		sigBlob, err := os.ReadFile(path)
 		if err != nil {
 			if os.IsNotExist(err) {
 				break
 			}
 			return nil, err
 		}
+		signature, err := signature.FromBlob(sigBlob)
+		if err != nil {
+			return nil, fmt.Errorf("parsing signature %q: %w", path, err)
+		}
 		signatures = append(signatures, signature)
 	}
 	return signatures, nil

directory/directory_test.go

@@ -158,13 +158,14 @@ func TestGetPutSignatures(t *testing.T) {
 	list := []byte("test-manifest-list")
 	md, err := manifest.Digest(man)
 	require.NoError(t, err)
+	// These signatures are completely invalid; start with 0xA3 just to be minimally plausible to signature.FromBlob.
 	signatures := [][]byte{
-		[]byte("sig1"),
-		[]byte("sig2"),
+		[]byte("\xA3sig1"),
+		[]byte("\xA3sig2"),
 	}
 	listSignatures := [][]byte{
-		[]byte("sig3"),
-		[]byte("sig4"),
+		[]byte("\xA3sig3"),
+		[]byte("\xA3sig4"),
 	}
 
 	dest, err := ref.NewImageDestination(context.Background(), nil)

docker/docker_image_dest.go

@@ -20,6 +20,7 @@ import (
 	"github.com/containers/image/v5/internal/imagedestination/stubs"
 	"github.com/containers/image/v5/internal/private"
 	"github.com/containers/image/v5/internal/putblobdigest"
+	"github.com/containers/image/v5/internal/signature"
 	"github.com/containers/image/v5/internal/streamdigest"
 	"github.com/containers/image/v5/internal/uploadreader"
 	"github.com/containers/image/v5/manifest"
@@ -506,10 +507,11 @@ func isManifestInvalidError(err error) bool {
 	}
 }
 
-// PutSignatures uploads a set of signatures to the relevant lookaside or API extension point.
-// If instanceDigest is not nil, it contains a digest of the specific manifest instance to upload the signatures for (when
-// the primary manifest is a manifest list); this should always be nil if the primary manifest is not a manifest list.
-func (d *dockerImageDestination) PutSignatures(ctx context.Context, signatures [][]byte, instanceDigest *digest.Digest) error {
+// PutSignaturesWithFormat writes a set of signatures to the destination.
+// If instanceDigest is not nil, it contains a digest of the specific manifest instance to write or overwrite the signatures for
+// (when the primary manifest is a manifest list); this should always be nil if the primary manifest is not a manifest list.
+// MUST be called after PutManifest (signatures may reference manifest contents).
+func (d *dockerImageDestination) PutSignaturesWithFormat(ctx context.Context, signatures []signature.Signature, instanceDigest *digest.Digest) error {
 	// Do not fail if we don’t really need to support signatures.
 	if len(signatures) == 0 {
 		return nil
@@ -535,9 +537,9 @@ func (d *dockerImageDestination) PutSignatures(ctx context.Context, signatures [
 	}
 }
 
-// putSignaturesToLookaside implements PutSignatures() from the lookaside location configured in s.c.signatureBase,
+// putSignaturesToLookaside implements PutSignaturesWithFormat() from the lookaside location configured in s.c.signatureBase,
 // which is not nil, for a manifest with manifestDigest.
-func (d *dockerImageDestination) putSignaturesToLookaside(signatures [][]byte, manifestDigest digest.Digest) error {
+func (d *dockerImageDestination) putSignaturesToLookaside(signatures []signature.Signature, manifestDigest digest.Digest) error {
 	// FIXME? This overwrites files one at a time, definitely not atomic.
 	// A failure when updating signatures with a reordered copy could lose some of them.
 
@@ -573,17 +575,21 @@ func (d *dockerImageDestination) putSignaturesToLookaside(signatures [][]byte, m
 	return nil
 }
 
-// putOneSignature stores one signature to url.
+// putOneSignature stores sig to url.
 // NOTE: Keep this in sync with docs/signature-protocols.md!
-func (d *dockerImageDestination) putOneSignature(url *url.URL, signature []byte) error {
+func (d *dockerImageDestination) putOneSignature(url *url.URL, sig signature.Signature) error {
 	switch url.Scheme {
 	case "file":
 		logrus.Debugf("Writing to %s", url.Path)
 		err := os.MkdirAll(filepath.Dir(url.Path), 0755)
 		if err != nil {
 			return err
 		}
-		err = os.WriteFile(url.Path, signature, 0644)
+		blob, err := signature.Blob(sig)
+		if err != nil {
+			return err
+		}
+		err = os.WriteFile(url.Path, blob, 0644)
 		if err != nil {
 			return err
 		}
@@ -616,9 +622,9 @@ func (c *dockerClient) deleteOneSignature(url *url.URL) (missing bool, err error
 	}
 }
 
-// putSignaturesToAPIExtension implements PutSignatures() using the X-Registry-Supports-Signatures API extension,
+// putSignaturesToAPIExtension implements PutSignaturesWithFormat() using the X-Registry-Supports-Signatures API extension,
 // for a manifest with manifestDigest.
-func (d *dockerImageDestination) putSignaturesToAPIExtension(ctx context.Context, signatures [][]byte, manifestDigest digest.Digest) error {
+func (d *dockerImageDestination) putSignaturesToAPIExtension(ctx context.Context, signatures []signature.Signature, manifestDigest digest.Digest) error {
 	// Skip dealing with the manifest digest, or reading the old state, if not necessary.
 	if len(signatures) == 0 {
 		return nil
@@ -638,7 +644,13 @@ func (d *dockerImageDestination) putSignaturesToAPIExtension(ctx context.Context
 	}
 
 sigExists:
-	for _, newSig := range signatures {
+	for _, newSigWithFormat := range signatures {
+		newSigSimple, ok := newSigWithFormat.(signature.SimpleSigning)
+		if !ok {
+			return signature.UnsupportedFormatError(newSigWithFormat)
+		}
+		newSig := newSigSimple.UntrustedSignature()
+
 		for _, existingSig := range existingSignatures.Signatures {
 			if existingSig.Version == extensionSignatureSchemaVersion && existingSig.Type == extensionSignatureTypeAtomic && bytes.Equal(existingSig.Content, newSig) {
 				continue sigExists