From 7dde4a36164181836071d8e81f54662011a8862f Mon Sep 17 00:00:00 2001 From: James Hewitt Date: Tue, 29 Mar 2022 21:00:54 +0100 Subject: [PATCH 1/2] Add option to specify the identity for signing This enables pushing to registries where the push and pull uris may be different, for example where pushed images are mirrored to a read only replica for distribution. Underpins implementation for https://github.com/containers/skopeo/issues/1588 Signed-off-by: James Hewitt --- copy/copy.go | 5 +++-- copy/sign.go | 13 ++++++++----- copy/sign_test.go | 19 +++++++++++++++---- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/copy/copy.go b/copy/copy.go index 0501fb3c1..9837972c5 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -127,6 +127,7 @@ type Options struct { RemoveSignatures bool // Remove any pre-existing signatures. SignBy will still add a new signature. SignBy string // If non-empty, asks for a signature to be added during the copy, and specifies a key ID, as accepted by signature.NewGPGSigningMechanism().SignDockerManifest(), SignPassphrase string // Passphare to use when signing with the key ID from `SignBy`. + SignIdentity string // Identify to use when signing, defaults to the docker reference of the destination ReportWriter io.Writer SourceCtx *types.SystemContext DestinationCtx *types.SystemContext @@ -574,7 +575,7 @@ func (c *copier) copyMultipleImages(ctx context.Context, policyContext *signatur // Sign the manifest list. if options.SignBy != "" { - newSig, err := c.createSignature(manifestList, options.SignBy, options.SignPassphrase) + newSig, err := c.createSignature(manifestList, options.SignBy, options.SignPassphrase, options.SignIdentity) if err != nil { return nil, err } @@ -796,7 +797,7 @@ func (c *copier) copyOneImage(ctx context.Context, policyContext *signature.Poli } if options.SignBy != "" { - newSig, err := c.createSignature(manifestBytes, options.SignBy, options.SignPassphrase) + newSig, err := c.createSignature(manifestBytes, options.SignBy, options.SignPassphrase, options.SignIdentity) if err != nil { return nil, "", "", err } diff --git a/copy/sign.go b/copy/sign.go index 21a3facd7..0c203c29f 100644 --- a/copy/sign.go +++ b/copy/sign.go @@ -7,7 +7,7 @@ import ( ) // createSignature creates a new signature of manifest using keyIdentity. -func (c *copier) createSignature(manifest []byte, keyIdentity string, passphrase string) ([]byte, error) { +func (c *copier) createSignature(manifest []byte, keyIdentity string, passphrase string, identity string) ([]byte, error) { mech, err := signature.NewGPGSigningMechanism() if err != nil { return nil, errors.Wrap(err, "initializing GPG") @@ -17,13 +17,16 @@ func (c *copier) createSignature(manifest []byte, keyIdentity string, passphrase return nil, errors.Wrap(err, "Signing not supported") } - dockerReference := c.dest.Reference().DockerReference() - if dockerReference == nil { - return nil, errors.Errorf("Cannot determine canonical Docker reference for destination %s", transports.ImageName(c.dest.Reference())) + if identity == "" { + dockerReference := c.dest.Reference().DockerReference() + if dockerReference == nil { + return nil, errors.Errorf("Cannot determine canonical Docker reference for destination %s", transports.ImageName(c.dest.Reference())) + } + identity = dockerReference.String() } c.Printf("Signing manifest\n") - newSig, err := signature.SignDockerManifestWithOptions(manifest, dockerReference.String(), mech, keyIdentity, &signature.SignOptions{Passphrase: passphrase}) + newSig, err := signature.SignDockerManifestWithOptions(manifest, identity, mech, keyIdentity, &signature.SignOptions{Passphrase: passphrase}) if err != nil { return nil, errors.Wrap(err, "creating signature") } diff --git a/copy/sign_test.go b/copy/sign_test.go index 0e74c7cde..5f44cad62 100644 --- a/copy/sign_test.go +++ b/copy/sign_test.go @@ -49,7 +49,7 @@ func TestCreateSignature(t *testing.T) { dest: imagedestination.FromPublic(dirDest), reportWriter: ioutil.Discard, } - _, err = c.createSignature(manifestBlob, testKeyFingerprint, "") + _, err = c.createSignature(manifestBlob, testKeyFingerprint, "", "") assert.Error(t, err) // Set up a docker: reference @@ -65,17 +65,28 @@ func TestCreateSignature(t *testing.T) { } // Signing with an unknown key fails - _, err = c.createSignature(manifestBlob, "this key does not exist", "") + _, err = c.createSignature(manifestBlob, "this key does not exist", "", "") assert.Error(t, err) - // Success + // Signing without overriding the identity uses the docker reference mech, err = signature.NewGPGSigningMechanism() require.NoError(t, err) defer mech.Close() - sig, err := c.createSignature(manifestBlob, testKeyFingerprint, "") + sig, err := c.createSignature(manifestBlob, testKeyFingerprint, "", "") require.NoError(t, err) verified, err := signature.VerifyDockerManifestSignature(sig, 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) + + // Can override the identity with own + mech, err = signature.NewGPGSigningMechanism() + require.NoError(t, err) + defer mech.Close() + sig, err = c.createSignature(manifestBlob, testKeyFingerprint, "", "myregistry.io/myrepo:mytag") + require.NoError(t, err) + verified, err = signature.VerifyDockerManifestSignature(sig, 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) } From 030d990f22213b138bb018692f0d004f297d98e0 Mon Sep 17 00:00:00 2001 From: James Hewitt Date: Wed, 30 Mar 2022 00:30:12 +0100 Subject: [PATCH 2/2] Use only full named references for signing identity This ensures we never create an invalid signature. Signed-off-by: James Hewitt --- copy/copy.go | 8 ++++---- copy/sign.go | 16 ++++++++++------ copy/sign_test.go | 22 +++++++++++++++------- 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/copy/copy.go b/copy/copy.go index 9837972c5..b616e566c 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -124,10 +124,10 @@ type ImageListSelection int // Options allows supplying non-default configuration modifying the behavior of CopyImage. type Options struct { - RemoveSignatures bool // Remove any pre-existing signatures. SignBy will still add a new signature. - SignBy string // If non-empty, asks for a signature to be added during the copy, and specifies a key ID, as accepted by signature.NewGPGSigningMechanism().SignDockerManifest(), - SignPassphrase string // Passphare to use when signing with the key ID from `SignBy`. - SignIdentity string // Identify to use when signing, defaults to the docker reference of the destination + RemoveSignatures bool // Remove any pre-existing signatures. SignBy will still add a new signature. + SignBy string // If non-empty, asks for a signature to be added during the copy, and specifies a key ID, as accepted by signature.NewGPGSigningMechanism().SignDockerManifest(), + SignPassphrase string // Passphare to use when signing with the key ID from `SignBy`. + SignIdentity reference.Named // Identify to use when signing, defaults to the docker reference of the destination ReportWriter io.Writer SourceCtx *types.SystemContext DestinationCtx *types.SystemContext diff --git a/copy/sign.go b/copy/sign.go index 0c203c29f..aa42674bc 100644 --- a/copy/sign.go +++ b/copy/sign.go @@ -1,13 +1,14 @@ package copy import ( + "github.com/containers/image/v5/docker/reference" "github.com/containers/image/v5/signature" "github.com/containers/image/v5/transports" "github.com/pkg/errors" ) // createSignature creates a new signature of manifest using keyIdentity. -func (c *copier) createSignature(manifest []byte, keyIdentity string, passphrase string, identity string) ([]byte, error) { +func (c *copier) createSignature(manifest []byte, keyIdentity string, passphrase string, identity reference.Named) ([]byte, error) { mech, err := signature.NewGPGSigningMechanism() if err != nil { return nil, errors.Wrap(err, "initializing GPG") @@ -17,16 +18,19 @@ func (c *copier) createSignature(manifest []byte, keyIdentity string, passphrase return nil, errors.Wrap(err, "Signing not supported") } - if identity == "" { - dockerReference := c.dest.Reference().DockerReference() - if dockerReference == nil { + if identity != nil { + if reference.IsNameOnly(identity) { + return nil, errors.Errorf("Sign identity must be a fully specified reference %s", identity) + } + } else { + identity = c.dest.Reference().DockerReference() + if identity == nil { return nil, errors.Errorf("Cannot determine canonical Docker reference for destination %s", transports.ImageName(c.dest.Reference())) } - identity = dockerReference.String() } c.Printf("Signing manifest\n") - newSig, err := signature.SignDockerManifestWithOptions(manifest, identity, mech, keyIdentity, &signature.SignOptions{Passphrase: passphrase}) + newSig, err := signature.SignDockerManifestWithOptions(manifest, identity.String(), mech, keyIdentity, &signature.SignOptions{Passphrase: passphrase}) if err != nil { return nil, errors.Wrap(err, "creating signature") } diff --git a/copy/sign_test.go b/copy/sign_test.go index 5f44cad62..1e9fc6738 100644 --- a/copy/sign_test.go +++ b/copy/sign_test.go @@ -12,6 +12,7 @@ import ( "github.com/containers/image/v5/manifest" "github.com/containers/image/v5/signature" "github.com/containers/image/v5/types" + "github.com/docker/distribution/reference" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -49,7 +50,7 @@ func TestCreateSignature(t *testing.T) { dest: imagedestination.FromPublic(dirDest), reportWriter: ioutil.Discard, } - _, err = c.createSignature(manifestBlob, testKeyFingerprint, "", "") + _, err = c.createSignature(manifestBlob, testKeyFingerprint, "", nil) assert.Error(t, err) // Set up a docker: reference @@ -65,14 +66,22 @@ func TestCreateSignature(t *testing.T) { } // Signing with an unknown key fails - _, err = c.createSignature(manifestBlob, "this key does not exist", "", "") + _, err = c.createSignature(manifestBlob, "this key does not exist", "", nil) assert.Error(t, err) - // Signing without overriding the identity uses the docker reference + // Can't sign without a full reference + ref, err := reference.ParseNamed("myregistry.io/myrepo") + require.NoError(t, err) + _, err = c.createSignature(manifestBlob, testKeyFingerprint, "", ref) + assert.Error(t, err) + + // Mechanism for verifying the signatures mech, err = signature.NewGPGSigningMechanism() require.NoError(t, err) defer mech.Close() - sig, err := c.createSignature(manifestBlob, testKeyFingerprint, "", "") + + // 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) require.NoError(t, err) @@ -80,10 +89,9 @@ func TestCreateSignature(t *testing.T) { assert.Equal(t, manifestDigest, verified.DockerManifestDigest) // Can override the identity with own - mech, err = signature.NewGPGSigningMechanism() + ref, err = reference.ParseNamed("myregistry.io/myrepo:mytag") require.NoError(t, err) - defer mech.Close() - sig, err = c.createSignature(manifestBlob, testKeyFingerprint, "", "myregistry.io/myrepo:mytag") + sig, err = c.createSignature(manifestBlob, testKeyFingerprint, "", ref) require.NoError(t, err) verified, err = signature.VerifyDockerManifestSignature(sig, manifestBlob, "myregistry.io/myrepo:mytag", mech, testKeyFingerprint) require.NoError(t, err)