Skip to content

Commit

Permalink
Use only full named references for signing identity
Browse files Browse the repository at this point in the history
This ensures we never create an invalid signature.

Signed-off-by: James Hewitt <james.hewitt@uk.ibm.com>
  • Loading branch information
Jamstah committed Mar 30, 2022
1 parent 7dde4a3 commit 030d990
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 17 deletions.
8 changes: 4 additions & 4 deletions copy/copy.go
Expand Up @@ -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
Expand Down
16 changes: 10 additions & 6 deletions 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")
Expand All @@ -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")
}
Expand Down
22 changes: 15 additions & 7 deletions copy/sign_test.go
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Expand All @@ -65,25 +66,32 @@ 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)
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()
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)
Expand Down

0 comments on commit 030d990

Please sign in to comment.