From 43bd7941ec4747f895e149bb23ef6167969d7737 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 5 Jul 2022 18:17:53 +0200 Subject: [PATCH 1/8] Introduce ImageSourceInternalOnly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- internal/private/private.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/internal/private/private.go b/internal/private/private.go index efc44d9c7..48c85ca6b 100644 --- a/internal/private/private.go +++ b/internal/private/private.go @@ -9,16 +9,21 @@ import ( "github.com/containers/image/v5/types" ) -// ImageSource is an internal extension to the types.ImageSource interface. -type ImageSource interface { - types.ImageSource - +// ImageSourceInternalOnly is the part of private.ImageSource that is not +// a part of types.ImageSource. +type ImageSourceInternalOnly interface { // SupportsGetBlobAt() returns true if GetBlobAt (BlobChunkAccessor) is supported. SupportsGetBlobAt() bool // BlobChunkAccessor.GetBlobAt is available only if SupportsGetBlobAt(). BlobChunkAccessor } +// ImageSource is an internal extension to the types.ImageSource interface. +type ImageSource interface { + types.ImageSource + ImageSourceInternalOnly +} + // ImageDestinationInternalOnly is the part of private.ImageDestination that is not // a part of types.ImageDestination. type ImageDestinationInternalOnly interface { From 1b6a4bf0d0a01980f5fc8915c2e90ff31bec4075 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 5 Jul 2022 19:18:05 +0200 Subject: [PATCH 2/8] Rename local variables in storageImageSource.GetSignatures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We want the "signatures" package name to be visible. Signed-off-by: Miloslav Trmač --- storage/storage_src.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/storage/storage_src.go b/storage/storage_src.go index 2ef56b70b..8aee015cd 100644 --- a/storage/storage_src.go +++ b/storage/storage_src.go @@ -296,7 +296,7 @@ func buildLayerInfosForCopy(manifestInfos []manifest.LayerInfo, physicalInfos [] func (s *storageImageSource) GetSignatures(ctx context.Context, instanceDigest *digest.Digest) (signatures [][]byte, err error) { var offset int sigslice := [][]byte{} - signature := []byte{} + signatureBlobs := []byte{} signatureSizes := s.SignatureSizes key := "signatures" instance := "default instance" @@ -306,21 +306,21 @@ func (s *storageImageSource) GetSignatures(ctx context.Context, instanceDigest * instance = instanceDigest.Encoded() } if len(signatureSizes) > 0 { - signatureBlob, err := s.imageRef.transport.store.ImageBigData(s.image.ID, key) + data, err := s.imageRef.transport.store.ImageBigData(s.image.ID, key) if err != nil { return nil, perrors.Wrapf(err, "looking up signatures data for image %q (%s)", s.image.ID, instance) } - signature = signatureBlob + signatureBlobs = data } for _, length := range signatureSizes { - if offset+length > len(signature) { - return nil, perrors.Wrapf(err, "looking up signatures data for image %q (%s): expected at least %d bytes, only found %d", s.image.ID, instance, len(signature), offset+length) + if offset+length > len(signatureBlobs) { + return nil, perrors.Wrapf(err, "looking up signatures data for image %q (%s): expected at least %d bytes, only found %d", s.image.ID, instance, len(signatureBlobs), offset+length) } - sigslice = append(sigslice, signature[offset:offset+length]) + sigslice = append(sigslice, signatureBlobs[offset:offset+length]) offset += length } - if offset != len(signature) { - return nil, fmt.Errorf("signatures data (%s) contained %d extra bytes", instance, len(signatures)-offset) + if offset != len(signatureBlobs) { + return nil, fmt.Errorf("signatures data (%s) contained %d extra bytes", instance, len(signatureBlobs)-offset) } return sigslice, nil } From ba768be6eed077c4f4806d6981c6ada1cf1a37d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 5 Jul 2022 19:20:32 +0200 Subject: [PATCH 3/8] Fix error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit err is necessarily nil at that location, so this was returning no error indication. Signed-off-by: Miloslav Trmač --- storage/storage_src.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/storage_src.go b/storage/storage_src.go index 8aee015cd..5456e2115 100644 --- a/storage/storage_src.go +++ b/storage/storage_src.go @@ -314,7 +314,7 @@ func (s *storageImageSource) GetSignatures(ctx context.Context, instanceDigest * } for _, length := range signatureSizes { if offset+length > len(signatureBlobs) { - return nil, perrors.Wrapf(err, "looking up signatures data for image %q (%s): expected at least %d bytes, only found %d", s.image.ID, instance, len(signatureBlobs), offset+length) + return nil, fmt.Errorf("looking up signatures data for image %q (%s): expected at least %d bytes, only found %d", s.image.ID, instance, len(signatureBlobs), offset+length) } sigslice = append(sigslice, signatureBlobs[offset:offset+length]) offset += length From f1f6bbc1c51049b492b990ec5c7fb0e362404714 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 5 Jul 2022 19:24:36 +0200 Subject: [PATCH 4/8] Rename a variable to be consistent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- oci/layout/oci_src.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/oci/layout/oci_src.go b/oci/layout/oci_src.go index db627015c..604047872 100644 --- a/oci/layout/oci_src.go +++ b/oci/layout/oci_src.go @@ -56,7 +56,7 @@ func newImageSource(sys *types.SystemContext, ref ociReference) (private.ImageSo if err != nil { return nil, err } - d := &ociImageSource{ + s := &ociImageSource{ PropertyMethodsInitialize: impl.PropertyMethods(impl.Properties{ HasThreadSafeGetBlob: false, }), @@ -69,9 +69,9 @@ func newImageSource(sys *types.SystemContext, ref ociReference) (private.ImageSo } if sys != nil { // TODO(jonboulle): check dir existence? - d.sharedBlobDir = sys.OCISharedBlobDirPath + s.sharedBlobDir = sys.OCISharedBlobDirPath } - return d, nil + return s, nil } // Reference returns the reference used to set up this source. From fbf83e451fa9d2f3d6d14c8292c9c17307a21a86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 5 Jul 2022 18:50:26 +0200 Subject: [PATCH 5/8] Add an internal/signature package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We need something to wrap (format ID, actual data). And a shared helper for a file/blob representation for things like dir or c/storage. This looks quite immature. Tests of the non-simple signing will only be added with support for that format. Signed-off-by: Miloslav Trmač --- directory/directory_test.go | 9 +- internal/signature/signature.go | 111 +++++++++++++++++++ internal/signature/signature_test.go | 53 +++++++++ internal/signature/simple.go | 27 +++++ internal/signature/simple_test.go | 36 ++++++ internal/signature/testdata/simple.signature | 1 + 6 files changed, 233 insertions(+), 4 deletions(-) create mode 100644 internal/signature/signature.go create mode 100644 internal/signature/signature_test.go create mode 100644 internal/signature/simple.go create mode 100644 internal/signature/simple_test.go create mode 120000 internal/signature/testdata/simple.signature diff --git a/directory/directory_test.go b/directory/directory_test.go index c9154561b..8c80262ad 100644 --- a/directory/directory_test.go +++ b/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) diff --git a/internal/signature/signature.go b/internal/signature/signature.go new file mode 100644 index 000000000..1b1bee428 --- /dev/null +++ b/internal/signature/signature.go @@ -0,0 +1,111 @@ +package signature + +import ( + "bytes" + "errors" + "fmt" +) + +// FIXME FIXME: MIME type? Int? String? +// An interface with a name, parse methods? +type FormatID string + +const ( + SimpleSigningFormat FormatID = "simple-signing" + CosignFormat FormatID = "cosign-json" + // Update also UnsupportedFormatError below +) + +// Signature is an image signature of some kind. +type Signature interface { + FormatID() FormatID + // blobChunk returns a representation of signature as a []byte, suitable for long-term storage. + // Almost everyone should use signature.Blob() instead. + blobChunk() ([]byte, error) +} + +// BlobChunk returns a representation of sig as a []byte, suitable for long-term storage. +func Blob(sig Signature) ([]byte, error) { + chunk, err := sig.blobChunk() + if err != nil { + return nil, err + } + + format := sig.FormatID() + switch format { + case SimpleSigningFormat: + // For compatibility with old dir formats: + return chunk, nil + default: + res := []byte{0} // Start with a zero byte to clearly mark this is a binary format, and disambiguate from random text. + res = append(res, []byte(format)...) + res = append(res, '\n') + res = append(res, chunk...) + return res, nil + } +} + +// FromBlob returns a signature from parsing a blob created by signature.Blob. +func FromBlob(blob []byte) (Signature, error) { + if len(blob) == 0 { + return nil, errors.New("empty signature blob") + } + // Historically we’ve just been using GPG with no identification; try to auto-detect that. + switch blob[0] { + // OpenPGP "compressed data" wrapping the message + case 0xA0, 0xA1, 0xA2, 0xA3, // bit 7 = 1; bit 6 = 0 (old packet format); bits 5…2 = 8 (tag: compressed data packet); bits 1…0 = length-type (any) + 0xC8, // bit 7 = 1; bit 6 = 1 (new packet format); bits 5…0 = 8 (tag: compressed data packet) + // OpenPGP “one-pass signature” starting a signature + 0x90, 0x91, 0x92, 0x3d, // bit 7 = 1; bit 6 = 0 (old packet format); bits 5…2 = 4 (tag: one-pass signature packet); bits 1…0 = length-type (any) + 0xC4, // bit 7 = 1; bit 6 = 1 (new packet format); bits 5…0 = 4 (tag: one-pass signature packet) + // OpenPGP signature packet signing the following data + 0x88, 0x89, 0x8A, 0x8B, // bit 7 = 1; bit 6 = 0 (old packet format); bits 5…2 = 2 (tag: signature packet); bits 1…0 = length-type (any) + 0xC2: // bit 7 = 1; bit 6 = 1 (new packet format); bits 5…0 = 2 (tag: signature packet) + return SimpleSigningFromBlob(blob), nil + + // The newer format: binary 0, format name, newline, data + case 0x00: + blob = blob[1:] + newline := bytes.IndexByte(blob, '\n') + if newline == -1 { + return nil, fmt.Errorf("invalid signature format, missing newline") + } + formatBytes := blob[:newline] + for _, b := range formatBytes { + if b < 32 || b >= 0x7F { + return nil, fmt.Errorf("invalid signature format, non-ASCII byte %#x", b) + } + } + blobChunk := blob[newline+1:] + switch { + case bytes.Equal(formatBytes, []byte(SimpleSigningFormat)): + return SimpleSigningFromBlob(blobChunk), nil + case bytes.Equal(formatBytes, []byte(CosignFormat)): + fallthrough + default: + return nil, fmt.Errorf("unrecognized signature format %q", string(formatBytes)) + } + + default: + return nil, fmt.Errorf("unrecognized signature format, starting with binary %#x", blob[0]) + } + +} + +// UnsupportedFormatError returns an error complaining about sig having an unsupported format. +func UnsupportedFormatError(sig Signature) error { + formatID := sig.FormatID() + switch formatID { + case SimpleSigningFormat, CosignFormat: + return fmt.Errorf("unsupported signature format %s", string(formatID)) + default: + return fmt.Errorf("unsupported, and unrecognized, signature format %q", string(formatID)) + } +} + +// copyByteSlice returns a guaranteed-fresh copy of a byte slice +// Use this to make sure the underlying data is not shared and can’t be unexpectedly modified. +func copyByteSlice(s []byte) []byte { + res := []byte{} + return append(res, s...) +} diff --git a/internal/signature/signature_test.go b/internal/signature/signature_test.go new file mode 100644 index 000000000..df8e4ed52 --- /dev/null +++ b/internal/signature/signature_test.go @@ -0,0 +1,53 @@ +package signature + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestBlobSimpleSigning(t *testing.T) { + simpleSigData, err := os.ReadFile("testdata/simple.signature") + require.NoError(t, err) + simpleSig := SimpleSigningFromBlob(simpleSigData) + + simpleBlob, err := Blob(simpleSig) + require.NoError(t, err) + assert.Equal(t, simpleSigData, simpleBlob) + + fromBlob, err := FromBlob(simpleBlob) + require.NoError(t, err) + fromBlobSimple, ok := fromBlob.(SimpleSigning) + require.True(t, ok) + assert.Equal(t, simpleSigData, fromBlobSimple.UntrustedSignature()) +} + +// mockFormatSignature returns a specified format +type mockFormatSignature struct { + fmt FormatID +} + +func (ms mockFormatSignature) FormatID() FormatID { + return ms.fmt +} + +func (ms mockFormatSignature) blobChunk() ([]byte, error) { + panic("Unexpected call to a mock function") +} + +func TestUnsuportedFormatError(t *testing.T) { + // Warning: The exact text returned by the function is not an API commitment. + for _, c := range []struct { + input Signature + expected string + }{ + {SimpleSigningFromBlob(nil), "unsupported signature format simple-signing"}, + {mockFormatSignature{CosignFormat}, "unsupported signature format cosign-json"}, + {mockFormatSignature{FormatID("invalid")}, `unsupported, and unrecognized, signature format "invalid"`}, + } { + res := UnsupportedFormatError(c.input) + assert.Equal(t, c.expected, res.Error(), string(c.input.FormatID())) + } +} diff --git a/internal/signature/simple.go b/internal/signature/simple.go new file mode 100644 index 000000000..88b8adad0 --- /dev/null +++ b/internal/signature/simple.go @@ -0,0 +1,27 @@ +package signature + +// SimpleSigning is a “simple signing” signature. +type SimpleSigning struct { + untrustedSignature []byte +} + +// SimpleSigningFromBlob converts a “simple signing” signature into a SimpleSigning object. +func SimpleSigningFromBlob(blobChunk []byte) SimpleSigning { + return SimpleSigning{ + untrustedSignature: copyByteSlice(blobChunk), + } +} + +func (s SimpleSigning) FormatID() FormatID { + return SimpleSigningFormat +} + +// blobChunk returns a representation of signature as a []byte, suitable for long-term storage. +// Almost everyone should use signature.Blob() instead. +func (s SimpleSigning) blobChunk() ([]byte, error) { + return copyByteSlice(s.untrustedSignature), nil +} + +func (s SimpleSigning) UntrustedSignature() []byte { + return copyByteSlice(s.untrustedSignature) +} diff --git a/internal/signature/simple_test.go b/internal/signature/simple_test.go new file mode 100644 index 000000000..76537e21d --- /dev/null +++ b/internal/signature/simple_test.go @@ -0,0 +1,36 @@ +package signature + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSimpleSigningFromBlob(t *testing.T) { + var data = []byte("some contents") + + sig := SimpleSigningFromBlob(data) + assert.Equal(t, SimpleSigning{untrustedSignature: data}, sig) +} + +func TestSimpleSigningFormatID(t *testing.T) { + sig := SimpleSigningFromBlob([]byte("some contents")) + assert.Equal(t, SimpleSigningFormat, sig.FormatID()) +} + +func TestSimpleSigningBlobChunk(t *testing.T) { + var data = []byte("some contents") + + sig := SimpleSigningFromBlob(data) + chunk, err := sig.blobChunk() + require.NoError(t, err) + assert.Equal(t, data, chunk) +} + +func TestSimpleSigningUntrustedSignature(t *testing.T) { + var data = []byte("some contents") + + sig := SimpleSigningFromBlob(data) + assert.Equal(t, data, sig.UntrustedSignature()) +} diff --git a/internal/signature/testdata/simple.signature b/internal/signature/testdata/simple.signature new file mode 120000 index 000000000..dae8bd528 --- /dev/null +++ b/internal/signature/testdata/simple.signature @@ -0,0 +1 @@ +../../../signature/fixtures/image.signature \ No newline at end of file From a5ad6604a4b0762d5b45645aac53475052d015db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 5 Jul 2022 18:54:19 +0200 Subject: [PATCH 6/8] Introduce ImageDestination.PutSignaturesWithFormat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NOTE: This makes implementation choices: - dir, docker LOOKASIDE, ostree and storage are going to store the generic-format blobs, and can represent any future formats of signatures. - docker X-R-S-S and OpenShift only accept simple signatures, and fail when copying the other kind. Signed-off-by: Miloslav Trmač --- directory/directory_dest.go | 12 +++++-- docker/docker_image_dest.go | 36 ++++++++++++------- internal/imagedestination/impl/compat.go | 14 ++++++++ internal/imagedestination/stubs/signatures.go | 5 +-- internal/imagedestination/wrapper.go | 18 ++++++++++ internal/private/private.go | 8 +++++ oci/archive/oci_dest.go | 8 +++-- openshift/openshift_dest.go | 15 ++++++-- ostree/ostree_dest.go | 16 ++++++--- pkg/blobcache/dest.go | 9 +++-- storage/storage_dest.go | 14 ++++++-- 11 files changed, 123 insertions(+), 32 deletions(-) diff --git a/directory/directory_dest.go b/directory/directory_dest.go index 8215c9089..87029f887 100644 --- a/directory/directory_dest.go +++ b/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 } } diff --git a/docker/docker_image_dest.go b/docker/docker_image_dest.go index 2c55ac4fb..02a6b66cf 100644 --- a/docker/docker_image_dest.go +++ b/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,9 +575,9 @@ 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) @@ -583,7 +585,11 @@ func (d *dockerImageDestination) putOneSignature(url *url.URL, signature []byte) 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 diff --git a/internal/imagedestination/impl/compat.go b/internal/imagedestination/impl/compat.go index 2cc3c7aac..ee34ffdbd 100644 --- a/internal/imagedestination/impl/compat.go +++ b/internal/imagedestination/impl/compat.go @@ -6,7 +6,9 @@ import ( "github.com/containers/image/v5/internal/blobinfocache" "github.com/containers/image/v5/internal/private" + "github.com/containers/image/v5/internal/signature" "github.com/containers/image/v5/types" + "github.com/opencontainers/go-digest" ) // Compat implements the obsolete parts of types.ImageDestination @@ -61,3 +63,15 @@ func (c *Compat) TryReusingBlob(ctx context.Context, info types.BlobInfo, cache CanSubstitute: canSubstitute, }) } + +// PutSignatures 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 (c *Compat) PutSignatures(ctx context.Context, signatures [][]byte, instanceDigest *digest.Digest) error { + withFormat := []signature.Signature{} + for _, sig := range signatures { + withFormat = append(withFormat, signature.SimpleSigningFromBlob(sig)) + } + return c.dest.PutSignaturesWithFormat(ctx, withFormat, instanceDigest) +} diff --git a/internal/imagedestination/stubs/signatures.go b/internal/imagedestination/stubs/signatures.go index 2e3c2eadb..7015fd068 100644 --- a/internal/imagedestination/stubs/signatures.go +++ b/internal/imagedestination/stubs/signatures.go @@ -4,6 +4,7 @@ import ( "context" "errors" + "github.com/containers/image/v5/internal/signature" "github.com/opencontainers/go-digest" ) @@ -27,11 +28,11 @@ func (stub NoSignaturesInitialize) SupportsSignatures(ctx context.Context) error return errors.New(stub.message) } -// 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. // MUST be called after PutManifest (signatures may reference manifest contents). -func (stub NoSignaturesInitialize) PutSignatures(ctx context.Context, signatures [][]byte, instanceDigest *digest.Digest) error { +func (stub NoSignaturesInitialize) PutSignaturesWithFormat(ctx context.Context, signatures []signature.Signature, instanceDigest *digest.Digest) error { if len(signatures) != 0 { return errors.New(stub.message) } diff --git a/internal/imagedestination/wrapper.go b/internal/imagedestination/wrapper.go index 429f546ad..43575ede3 100644 --- a/internal/imagedestination/wrapper.go +++ b/internal/imagedestination/wrapper.go @@ -6,7 +6,9 @@ import ( "github.com/containers/image/v5/internal/imagedestination/stubs" "github.com/containers/image/v5/internal/private" + "github.com/containers/image/v5/internal/signature" "github.com/containers/image/v5/types" + "github.com/opencontainers/go-digest" ) // wrapped provides the private.ImageDestination operations @@ -59,3 +61,19 @@ func (w *wrapped) PutBlobWithOptions(ctx context.Context, stream io.Reader, inpu func (w *wrapped) TryReusingBlobWithOptions(ctx context.Context, info types.BlobInfo, options private.TryReusingBlobOptions) (bool, types.BlobInfo, error) { return w.TryReusingBlob(ctx, info, options.Cache, options.CanSubstitute) } + +// 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 (w *wrapped) PutSignaturesWithFormat(ctx context.Context, signatures []signature.Signature, instanceDigest *digest.Digest) error { + simpleSigs := [][]byte{} + for _, sig := range signatures { + simpleSig, ok := sig.(signature.SimpleSigning) + if !ok { + return signature.UnsupportedFormatError(sig) + } + simpleSigs = append(simpleSigs, simpleSig.UntrustedSignature()) + } + return w.PutSignatures(ctx, simpleSigs, instanceDigest) +} diff --git a/internal/private/private.go b/internal/private/private.go index 48c85ca6b..ed4ecf4db 100644 --- a/internal/private/private.go +++ b/internal/private/private.go @@ -6,7 +6,9 @@ import ( "github.com/containers/image/v5/docker/reference" "github.com/containers/image/v5/internal/blobinfocache" + "github.com/containers/image/v5/internal/signature" "github.com/containers/image/v5/types" + "github.com/opencontainers/go-digest" ) // ImageSourceInternalOnly is the part of private.ImageSource that is not @@ -54,6 +56,12 @@ type ImageDestinationInternalOnly interface { // reflected in the manifest that will be written. // If the transport can not reuse the requested blob, TryReusingBlob returns (false, {}, nil); it returns a non-nil error only on an unexpected failure. TryReusingBlobWithOptions(ctx context.Context, info types.BlobInfo, options TryReusingBlobOptions) (bool, types.BlobInfo, 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). + PutSignaturesWithFormat(ctx context.Context, signatures []signature.Signature, instanceDigest *digest.Digest) error } // ImageDestination is an internal extension to the types.ImageDestination diff --git a/oci/archive/oci_dest.go b/oci/archive/oci_dest.go index 3ba9f6d3b..23b89e8e2 100644 --- a/oci/archive/oci_dest.go +++ b/oci/archive/oci_dest.go @@ -9,6 +9,7 @@ import ( "github.com/containers/image/v5/internal/imagedestination" "github.com/containers/image/v5/internal/imagedestination/impl" "github.com/containers/image/v5/internal/private" + "github.com/containers/image/v5/internal/signature" "github.com/containers/image/v5/types" "github.com/containers/storage/pkg/archive" digest "github.com/opencontainers/go-digest" @@ -142,11 +143,12 @@ func (d *ociArchiveImageDestination) PutManifest(ctx context.Context, m []byte, return d.unpackedDest.PutManifest(ctx, m, instanceDigest) } -// 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 *ociArchiveImageDestination) PutSignatures(ctx context.Context, signatures [][]byte, instanceDigest *digest.Digest) error { - return d.unpackedDest.PutSignatures(ctx, signatures, instanceDigest) +// MUST be called after PutManifest (signatures may reference manifest contents). +func (d *ociArchiveImageDestination) PutSignaturesWithFormat(ctx context.Context, signatures []signature.Signature, instanceDigest *digest.Digest) error { + return d.unpackedDest.PutSignaturesWithFormat(ctx, signatures, instanceDigest) } // Commit marks the process of storing the image as successful and asks for the image to be persisted diff --git a/openshift/openshift_dest.go b/openshift/openshift_dest.go index db83e4e66..58f0eac70 100644 --- a/openshift/openshift_dest.go +++ b/openshift/openshift_dest.go @@ -17,6 +17,7 @@ import ( "github.com/containers/image/v5/internal/imagedestination/impl" "github.com/containers/image/v5/internal/imagedestination/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" @@ -154,7 +155,11 @@ func (d *openshiftImageDestination) PutManifest(ctx context.Context, m []byte, i return d.docker.PutManifest(ctx, m, instanceDigest) } -func (d *openshiftImageDestination) 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 *openshiftImageDestination) PutSignaturesWithFormat(ctx context.Context, signatures []signature.Signature, instanceDigest *digest.Digest) error { var imageStreamImageName string if instanceDigest == nil { if d.imageStreamImageName == "" { @@ -182,7 +187,13 @@ func (d *openshiftImageDestination) PutSignatures(ctx context.Context, signature } 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 image.Signatures { if existingSig.Type == imageSignatureTypeAtomic && bytes.Equal(existingSig.Content, newSig) { continue sigExists diff --git a/ostree/ostree_dest.go b/ostree/ostree_dest.go index ae8a398bd..98275da60 100644 --- a/ostree/ostree_dest.go +++ b/ostree/ostree_dest.go @@ -25,6 +25,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/manifest" "github.com/containers/image/v5/types" "github.com/containers/storage/pkg/archive" @@ -398,10 +399,11 @@ func (d *ostreeImageDestination) PutManifest(ctx context.Context, manifestBlob [ return os.WriteFile(manifestPath, manifestBlob, 0644) } -// PutSignatures writes signatures to the destination. -// The instanceDigest value is expected to always be nil, because this transport does not support manifest lists, so -// there can be no secondary manifests. -func (d *ostreeImageDestination) 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 *ostreeImageDestination) PutSignaturesWithFormat(ctx context.Context, signatures []signature.Signature, instanceDigest *digest.Digest) error { if instanceDigest != nil { return errors.New(`Manifest lists are not supported by "ostree:"`) } @@ -413,7 +415,11 @@ func (d *ostreeImageDestination) PutSignatures(ctx context.Context, signatures [ for i, sig := range signatures { signaturePath := filepath.Join(d.tmpDirPath, d.ref.signaturePath(i)) - if err := os.WriteFile(signaturePath, sig, 0644); err != nil { + blob, err := signature.Blob(sig) + if err != nil { + return err + } + if err := os.WriteFile(signaturePath, blob, 0644); err != nil { return err } } diff --git a/pkg/blobcache/dest.go b/pkg/blobcache/dest.go index 8bae8a24b..7ea7bbba2 100644 --- a/pkg/blobcache/dest.go +++ b/pkg/blobcache/dest.go @@ -12,6 +12,7 @@ import ( "github.com/containers/image/v5/internal/imagedestination" "github.com/containers/image/v5/internal/imagedestination/impl" "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/transports" "github.com/containers/image/v5/types" @@ -280,8 +281,12 @@ func (d *blobCacheDestination) PutManifest(ctx context.Context, manifestBytes [] return d.destination.PutManifest(ctx, manifestBytes, instanceDigest) } -func (d *blobCacheDestination) PutSignatures(ctx context.Context, signatures [][]byte, instanceDigest *digest.Digest) error { - return d.destination.PutSignatures(ctx, signatures, instanceDigest) +// 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 *blobCacheDestination) PutSignaturesWithFormat(ctx context.Context, signatures []signature.Signature, instanceDigest *digest.Digest) error { + return d.destination.PutSignaturesWithFormat(ctx, signatures, instanceDigest) } func (d *blobCacheDestination) Commit(ctx context.Context, unparsedToplevel types.UnparsedImage) error { diff --git a/storage/storage_dest.go b/storage/storage_dest.go index 943edab9d..4bc4cb5d5 100644 --- a/storage/storage_dest.go +++ b/storage/storage_dest.go @@ -21,6 +21,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/tmpdir" "github.com/containers/image/v5/manifest" "github.com/containers/image/v5/pkg/blobinfocache/none" @@ -890,11 +891,18 @@ func (s *storageImageDestination) PutManifest(ctx context.Context, manifestBlob return nil } -// PutSignatures records the image's signatures for committing as a single data blob. -func (s *storageImageDestination) 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 (s *storageImageDestination) PutSignaturesWithFormat(ctx context.Context, signatures []signature.Signature, instanceDigest *digest.Digest) error { sizes := []int{} sigblob := []byte{} - for _, sig := range signatures { + for _, sigWithFormat := range signatures { + sig, err := signature.Blob(sigWithFormat) + if err != nil { + return err + } sizes = append(sizes, len(sig)) newblob := make([]byte, len(sigblob)+len(sig)) copy(newblob, sigblob) From ad2c293d754955eec34e0c93ee1e86ad59de533a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 5 Jul 2022 19:34:12 +0200 Subject: [PATCH 7/8] Introduce ImageSource.GetSignaturesWithFormat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- directory/directory_src.go | 20 ++++++--- docker/docker_image_src.go | 41 ++++++++++++------- docker/internal/tarfile/src.go | 5 ++- internal/imagesource/impl/compat.go | 54 +++++++++++++++++++++++++ internal/imagesource/impl/signatures.go | 5 ++- internal/imagesource/wrapper.go | 20 +++++++++ internal/private/private.go | 6 +++ oci/archive/oci_src.go | 16 +++++--- oci/layout/oci_src.go | 2 + openshift/openshift_src.go | 16 +++++--- ostree/ostree_src.go | 28 ++++++++----- pkg/blobcache/src.go | 20 ++++++--- sif/src.go | 7 +++- storage/storage_src.go | 20 ++++++--- tarball/tarball_src.go | 2 + 15 files changed, 207 insertions(+), 55 deletions(-) create mode 100644 internal/imagesource/impl/compat.go diff --git a/directory/directory_src.go b/directory/directory_src.go index 4fc20b1ca..98efdedd7 100644 --- a/directory/directory_src.go +++ b/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,7 +27,7 @@ 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, }), @@ -32,6 +35,8 @@ func newImageSource(ref dirReference) private.ImageSource { 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 diff --git a/docker/docker_image_src.go b/docker/docker_image_src.go index 3cb477167..53f9cecd4 100644 --- a/docker/docker_image_src.go +++ b/docker/docker_image_src.go @@ -20,6 +20,7 @@ import ( "github.com/containers/image/v5/internal/imagesource/stubs" "github.com/containers/image/v5/internal/iolimits" "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/pkg/sysregistriesv2" "github.com/containers/image/v5/types" @@ -29,6 +30,7 @@ import ( ) type dockerImageSource struct { + impl.Compat impl.PropertyMethodsInitialize impl.DoesNotAffectLayerInfosForCopy stubs.ImplementsGetBlobAt @@ -140,6 +142,7 @@ func newImageSourceAttempt(ctx context.Context, sys *types.SystemContext, logica physicalRef: physicalRef, c: client, } + s.Compat = impl.AddCompat(s) if err := s.ensureManifestIsLoaded(ctx); err != nil { return nil, err @@ -466,11 +469,11 @@ func (s *dockerImageSource) GetBlob(ctx context.Context, info types.BlobInfo, ca return res.Body, getBlobSize(res), 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 *dockerImageSource) GetSignatures(ctx context.Context, instanceDigest *digest.Digest) ([][]byte, error) { +func (s *dockerImageSource) GetSignaturesWithFormat(ctx context.Context, instanceDigest *digest.Digest) ([]signature.Signature, error) { if err := s.c.detectProperties(ctx); err != nil { return nil, err } @@ -502,16 +505,16 @@ func (s *dockerImageSource) manifestDigest(ctx context.Context, instanceDigest * return manifest.Digest(s.cachedManifest) } -// getSignaturesFromLookaside implements GetSignatures() from the lookaside location configured in s.c.signatureBase, +// getSignaturesFromLookaside implements GetSignaturesWithFormat() from the lookaside location configured in s.c.signatureBase, // which is not nil. -func (s *dockerImageSource) getSignaturesFromLookaside(ctx context.Context, instanceDigest *digest.Digest) ([][]byte, error) { +func (s *dockerImageSource) getSignaturesFromLookaside(ctx context.Context, instanceDigest *digest.Digest) ([]signature.Signature, error) { manifestDigest, err := s.manifestDigest(ctx, instanceDigest) if err != nil { return nil, err } // NOTE: Keep this in sync with docs/signature-protocols.md! - signatures := [][]byte{} + signatures := []signature.Signature{} for i := 0; ; i++ { url := signatureStorageURL(s.c.signatureBase, manifestDigest, i) signature, missing, err := s.getOneSignature(ctx, url) @@ -526,20 +529,24 @@ func (s *dockerImageSource) getSignaturesFromLookaside(ctx context.Context, inst return signatures, nil } -// getOneSignature downloads one signature from url. -// If it successfully determines that the signature does not exist, returns with missing set to true and error set to nil. +// getOneSignature downloads one signature from url, and returns (signature, false, nil) +// If it successfully determines that the signature does not exist, returns (nil, true, nil). // NOTE: Keep this in sync with docs/signature-protocols.md! -func (s *dockerImageSource) getOneSignature(ctx context.Context, url *url.URL) (signature []byte, missing bool, err error) { +func (s *dockerImageSource) getOneSignature(ctx context.Context, url *url.URL) (signature.Signature, bool, error) { switch url.Scheme { case "file": logrus.Debugf("Reading %s", url.Path) - sig, err := os.ReadFile(url.Path) + sigBlob, err := os.ReadFile(url.Path) if err != nil { if os.IsNotExist(err) { return nil, true, nil } return nil, false, err } + sig, err := signature.FromBlob(sigBlob) + if err != nil { + return nil, false, fmt.Errorf("parsing signature %q: %w", url.Path, err) + } return sig, false, nil case "http", "https": @@ -556,12 +563,16 @@ func (s *dockerImageSource) getOneSignature(ctx context.Context, url *url.URL) ( if res.StatusCode == http.StatusNotFound { return nil, true, nil } else if res.StatusCode != http.StatusOK { - return nil, false, fmt.Errorf("Error reading signature from %s: status %d (%s)", url.Redacted(), res.StatusCode, http.StatusText(res.StatusCode)) + return nil, false, fmt.Errorf("reading signature from %s: status %d (%s)", url.Redacted(), res.StatusCode, http.StatusText(res.StatusCode)) } - sig, err := iolimits.ReadAtMost(res.Body, iolimits.MaxSignatureBodySize) + sigBlob, err := iolimits.ReadAtMost(res.Body, iolimits.MaxSignatureBodySize) if err != nil { return nil, false, err } + sig, err := signature.FromBlob(sigBlob) + if err != nil { + return nil, false, fmt.Errorf("parsing signature %s: %w", url.Redacted(), err) + } return sig, false, nil default: @@ -569,8 +580,8 @@ func (s *dockerImageSource) getOneSignature(ctx context.Context, url *url.URL) ( } } -// getSignaturesFromAPIExtension implements GetSignatures() using the X-Registry-Supports-Signatures API extension. -func (s *dockerImageSource) getSignaturesFromAPIExtension(ctx context.Context, instanceDigest *digest.Digest) ([][]byte, error) { +// getSignaturesFromAPIExtension implements GetSignaturesWithFormat() using the X-Registry-Supports-Signatures API extension. +func (s *dockerImageSource) getSignaturesFromAPIExtension(ctx context.Context, instanceDigest *digest.Digest) ([]signature.Signature, error) { manifestDigest, err := s.manifestDigest(ctx, instanceDigest) if err != nil { return nil, err @@ -581,10 +592,10 @@ func (s *dockerImageSource) getSignaturesFromAPIExtension(ctx context.Context, i return nil, err } - var sigs [][]byte + var sigs []signature.Signature for _, sig := range parsedBody.Signatures { if sig.Version == extensionSignatureSchemaVersion && sig.Type == extensionSignatureTypeAtomic { - sigs = append(sigs, sig.Content) + sigs = append(sigs, signature.SimpleSigningFromBlob(sig.Content)) } } return sigs, nil diff --git a/docker/internal/tarfile/src.go b/docker/internal/tarfile/src.go index f5cfefe84..8230d88c6 100644 --- a/docker/internal/tarfile/src.go +++ b/docker/internal/tarfile/src.go @@ -25,6 +25,7 @@ import ( // Source is a partial implementation of types.ImageSource for reading from tarPath. type Source struct { + impl.Compat impl.PropertyMethodsInitialize impl.NoSignatures impl.DoesNotAffectLayerInfosForCopy @@ -56,7 +57,7 @@ type layerInfo struct { // and sourceIndex (or the only image if they are (nil, -1)). // The archive will be closed if closeArchive func NewSource(archive *Reader, closeArchive bool, transportName string, ref reference.NamedTagged, sourceIndex int) *Source { - return &Source{ + s := &Source{ PropertyMethodsInitialize: impl.PropertyMethods(impl.Properties{ HasThreadSafeGetBlob: true, }), @@ -67,6 +68,8 @@ func NewSource(archive *Reader, closeArchive bool, transportName string, ref ref ref: ref, sourceIndex: sourceIndex, } + s.Compat = impl.AddCompat(s) + return s } // ensureCachedDataIsPresent loads data necessary for any of the public accessors. diff --git a/internal/imagesource/impl/compat.go b/internal/imagesource/impl/compat.go new file mode 100644 index 000000000..6f7932916 --- /dev/null +++ b/internal/imagesource/impl/compat.go @@ -0,0 +1,54 @@ +package impl + +import ( + "context" + + "github.com/containers/image/v5/internal/private" + "github.com/containers/image/v5/internal/signature" + "github.com/opencontainers/go-digest" +) + +// Compat implements the obsolete parts of types.ImageSource +// for implementations of private.ImageSource. +// See AddCompat below. +type Compat struct { + src private.ImageSourceInternalOnly +} + +// AddCompat initializes Compat to implement the obsolete parts of types.ImageSource +// for implementations of private.ImageSource. +// +// Use it like this: +// type yourSource struct { +// impl.Compat +// … +// } +// src := &yourSource{…} +// src.Compat = impl.AddCompat(src) +// +func AddCompat(src private.ImageSourceInternalOnly) Compat { + return Compat{src} +} + +// GetSignatures 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 (c *Compat) GetSignatures(ctx context.Context, instanceDigest *digest.Digest) ([][]byte, error) { + // Silently ignore signatures with other formats; the caller can’t handle them. + // Admittedly callers that want to sync all of the image might want to fail instead; this + // way an upgrade of c/image neither breaks them nor adds new functionality. + // Alternatively, we could possibly define the old GetSignatures to use the multi-format + // signature.Blob representation now, in general, but that could silently break them as well. + sigs, err := c.src.GetSignaturesWithFormat(ctx, instanceDigest) + if err != nil { + return nil, err + } + simpleSigs := [][]byte{} + for _, sig := range sigs { + if sig, ok := sig.(signature.SimpleSigning); ok { + simpleSigs = append(simpleSigs, sig.UntrustedSignature()) + } + } + return simpleSigs, nil +} diff --git a/internal/imagesource/impl/signatures.go b/internal/imagesource/impl/signatures.go index ee25152b5..b3a8c7e88 100644 --- a/internal/imagesource/impl/signatures.go +++ b/internal/imagesource/impl/signatures.go @@ -3,16 +3,17 @@ package impl import ( "context" + "github.com/containers/image/v5/internal/signature" "github.com/opencontainers/go-digest" ) // NoSignatures implements GetSignatures() that returns nothing. type NoSignatures struct{} -// 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 (stub NoSignatures) GetSignatures(ctx context.Context, instanceDigest *digest.Digest) ([][]byte, error) { +func (stub NoSignatures) GetSignaturesWithFormat(ctx context.Context, instanceDigest *digest.Digest) ([]signature.Signature, error) { return nil, nil } diff --git a/internal/imagesource/wrapper.go b/internal/imagesource/wrapper.go index deacc713d..886b4e833 100644 --- a/internal/imagesource/wrapper.go +++ b/internal/imagesource/wrapper.go @@ -1,9 +1,13 @@ package imagesource import ( + "context" + "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/types" + "github.com/opencontainers/go-digest" ) // wrapped provides the private.ImageSource operations @@ -34,3 +38,19 @@ func FromPublic(src types.ImageSource) private.ImageSource { ImageSource: src, } } + +// 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 (w *wrapped) GetSignaturesWithFormat(ctx context.Context, instanceDigest *digest.Digest) ([]signature.Signature, error) { + sigs, err := w.GetSignatures(ctx, instanceDigest) + 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/internal/private/private.go b/internal/private/private.go index ed4ecf4db..40189e043 100644 --- a/internal/private/private.go +++ b/internal/private/private.go @@ -18,6 +18,12 @@ type ImageSourceInternalOnly interface { SupportsGetBlobAt() bool // BlobChunkAccessor.GetBlobAt is available only if SupportsGetBlobAt(). BlobChunkAccessor + + // 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). + GetSignaturesWithFormat(ctx context.Context, instanceDigest *digest.Digest) ([]signature.Signature, error) } // ImageSource is an internal extension to the types.ImageSource interface. diff --git a/oci/archive/oci_src.go b/oci/archive/oci_src.go index d53f2ae15..b24fc77b2 100644 --- a/oci/archive/oci_src.go +++ b/oci/archive/oci_src.go @@ -6,7 +6,9 @@ import ( "io" "github.com/containers/image/v5/internal/imagesource" + "github.com/containers/image/v5/internal/imagesource/impl" "github.com/containers/image/v5/internal/private" + "github.com/containers/image/v5/internal/signature" ocilayout "github.com/containers/image/v5/oci/layout" "github.com/containers/image/v5/types" digest "github.com/opencontainers/go-digest" @@ -16,6 +18,8 @@ import ( ) type ociArchiveImageSource struct { + impl.Compat + ref ociArchiveReference unpackedSrc private.ImageSource tempDirRef tempDirOCIRef @@ -36,11 +40,13 @@ func newImageSource(ctx context.Context, sys *types.SystemContext, ref ociArchiv } return nil, err } - return &ociArchiveImageSource{ + s := &ociArchiveImageSource{ ref: ref, unpackedSrc: imagesource.FromPublic(unpackedSrc), tempDirRef: tempDirRef, - }, nil + } + s.Compat = impl.AddCompat(s) + return s, nil } // LoadManifestDescriptor loads the manifest @@ -120,12 +126,12 @@ func (s *ociArchiveImageSource) GetBlobAt(ctx context.Context, info types.BlobIn return s.unpackedSrc.GetBlobAt(ctx, info, chunks) } -// 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 *ociArchiveImageSource) GetSignatures(ctx context.Context, instanceDigest *digest.Digest) ([][]byte, error) { - return s.unpackedSrc.GetSignatures(ctx, instanceDigest) +func (s *ociArchiveImageSource) GetSignaturesWithFormat(ctx context.Context, instanceDigest *digest.Digest) ([]signature.Signature, error) { + return s.unpackedSrc.GetSignaturesWithFormat(ctx, instanceDigest) } // LayerInfosForCopy returns either nil (meaning the values in the manifest are fine), or updated values for the layer diff --git a/oci/layout/oci_src.go b/oci/layout/oci_src.go index 604047872..9641ad024 100644 --- a/oci/layout/oci_src.go +++ b/oci/layout/oci_src.go @@ -22,6 +22,7 @@ import ( ) type ociImageSource struct { + impl.Compat impl.PropertyMethodsInitialize impl.NoSignatures impl.DoesNotAffectLayerInfosForCopy @@ -71,6 +72,7 @@ func newImageSource(sys *types.SystemContext, ref ociReference) (private.ImageSo // TODO(jonboulle): check dir existence? s.sharedBlobDir = sys.OCISharedBlobDirPath } + s.Compat = impl.AddCompat(s) return s, nil } diff --git a/openshift/openshift_src.go b/openshift/openshift_src.go index 8555c05e0..93ba8d10e 100644 --- a/openshift/openshift_src.go +++ b/openshift/openshift_src.go @@ -12,12 +12,14 @@ import ( "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/types" "github.com/opencontainers/go-digest" "github.com/sirupsen/logrus" ) type openshiftImageSource struct { + impl.Compat impl.DoesNotAffectLayerInfosForCopy // This is slightly suboptimal. We could forward GetBlobAt(), but we need to call ensureImageIsResolved in SupportsGetBlobAt(), // and that method doesn’t provide a context for timing out. That could actually be fixed (SupportsGetBlobAt is private and we @@ -40,12 +42,14 @@ func newImageSource(sys *types.SystemContext, ref openshiftReference) (private.I return nil, err } - return &openshiftImageSource{ + s := &openshiftImageSource{ NoGetBlobAtInitialize: stubs.NoGetBlobAt(ref), client: client, sys: sys, - }, nil + } + s.Compat = impl.AddCompat(s) + return s, nil } // Reference returns the reference used to set up this source, _as specified by the user_ @@ -92,11 +96,11 @@ func (s *openshiftImageSource) GetBlob(ctx context.Context, info types.BlobInfo, return s.docker.GetBlob(ctx, info, cache) } -// 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 *openshiftImageSource) GetSignatures(ctx context.Context, instanceDigest *digest.Digest) ([][]byte, error) { +func (s *openshiftImageSource) GetSignaturesWithFormat(ctx context.Context, instanceDigest *digest.Digest) ([]signature.Signature, error) { var imageStreamImageName string if instanceDigest == nil { if err := s.ensureImageIsResolved(ctx); err != nil { @@ -110,10 +114,10 @@ func (s *openshiftImageSource) GetSignatures(ctx context.Context, instanceDigest if err != nil { return nil, err } - var sigs [][]byte + var sigs []signature.Signature for _, sig := range image.Signatures { if sig.Type == imageSignatureTypeAtomic { - sigs = append(sigs, sig.Content) + sigs = append(sigs, signature.SimpleSigningFromBlob(sig.Content)) } } return sigs, nil diff --git a/ostree/ostree_src.go b/ostree/ostree_src.go index 5c7dcdf2e..9983acc0a 100644 --- a/ostree/ostree_src.go +++ b/ostree/ostree_src.go @@ -17,6 +17,7 @@ import ( "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/containers/storage/pkg/ioutils" @@ -37,6 +38,7 @@ import ( import "C" type ostreeImageSource struct { + impl.Compat impl.PropertyMethodsInitialize stubs.NoGetBlobAtInitialize @@ -49,7 +51,7 @@ type ostreeImageSource struct { // newImageSource returns an ImageSource for reading from an existing directory. func newImageSource(tmpDir string, ref ostreeReference) (private.ImageSource, error) { - return &ostreeImageSource{ + s := &ostreeImageSource{ PropertyMethodsInitialize: impl.PropertyMethods(impl.Properties{ HasThreadSafeGetBlob: false, }), @@ -58,7 +60,9 @@ func newImageSource(tmpDir string, ref ostreeReference) (private.ImageSource, er ref: ref, tmpDir: tmpDir, compressed: nil, - }, nil + } + s.Compat = impl.AddCompat(s) + return s, nil } // Reference returns the reference used to set up this source. @@ -349,10 +353,11 @@ func (s *ostreeImageSource) GetBlob(ctx context.Context, info types.BlobInfo, ca return rc, layerSize, nil } -// GetSignatures returns the image's signatures. It may use a remote (= slow) service. -// This source implementation does not support manifest lists, so the passed-in instanceDigest should always be nil, -// as there can be no secondary manifests. -func (s *ostreeImageSource) GetSignatures(ctx context.Context, instanceDigest *digest.Digest) ([][]byte, error) { +// 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 *ostreeImageSource) GetSignaturesWithFormat(ctx context.Context, instanceDigest *digest.Digest) ([]signature.Signature, error) { if instanceDigest != nil { return nil, errors.New(`Manifest lists are not supported by "ostree:"`) } @@ -370,18 +375,23 @@ func (s *ostreeImageSource) GetSignatures(ctx context.Context, instanceDigest *d s.repo = repo } - signatures := [][]byte{} + signatures := []signature.Signature{} for i := int64(1); i <= lenSignatures; i++ { - sigReader, err := s.readSingleFile(branch, fmt.Sprintf("/signature-%d", i)) + path := fmt.Sprintf("/signature-%d", i) + sigReader, err := s.readSingleFile(branch, path) if err != nil { return nil, err } defer sigReader.Close() - sig, err := io.ReadAll(sigReader) + sigBlob, err := io.ReadAll(sigReader) if err != nil { return nil, err } + sig, err := signature.FromBlob(sigBlob) + if err != nil { + return nil, fmt.Errorf("parsing signature %q: %w", path, err) + } signatures = append(signatures, sig) } return signatures, nil diff --git a/pkg/blobcache/src.go b/pkg/blobcache/src.go index c2983e6a5..74ad46666 100644 --- a/pkg/blobcache/src.go +++ b/pkg/blobcache/src.go @@ -9,7 +9,9 @@ import ( "github.com/containers/image/v5/internal/image" "github.com/containers/image/v5/internal/imagesource" + "github.com/containers/image/v5/internal/imagesource/impl" "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/pkg/compression" "github.com/containers/image/v5/transports" @@ -21,6 +23,8 @@ import ( ) type blobCacheSource struct { + impl.Compat + reference *BlobCache source private.ImageSource sys types.SystemContext @@ -37,7 +41,9 @@ func (b *BlobCache) NewImageSource(ctx context.Context, sys *types.SystemContext return nil, perrors.Wrapf(err, "error creating new image source %q", transports.ImageName(b.reference)) } logrus.Debugf("starting to read from image %q using blob cache in %q (compression=%v)", transports.ImageName(b.reference), b.directory, b.compress) - return &blobCacheSource{reference: b, source: imagesource.FromPublic(src), sys: *sys}, nil + s := &blobCacheSource{reference: b, source: imagesource.FromPublic(src), sys: *sys} + s.Compat = impl.AddCompat(s) + return s, nil } func (s *blobCacheSource) Reference() types.ImageReference { @@ -100,16 +106,20 @@ func (s *blobCacheSource) GetBlob(ctx context.Context, blobinfo types.BlobInfo, return rc, size, nil } -func (s *blobCacheSource) GetSignatures(ctx context.Context, instanceDigest *digest.Digest) ([][]byte, error) { - return s.source.GetSignatures(ctx, instanceDigest) +// 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 *blobCacheSource) GetSignaturesWithFormat(ctx context.Context, instanceDigest *digest.Digest) ([]signature.Signature, error) { + return s.source.GetSignaturesWithFormat(ctx, instanceDigest) } func (s *blobCacheSource) LayerInfosForCopy(ctx context.Context, instanceDigest *digest.Digest) ([]types.BlobInfo, error) { - signatures, err := s.source.GetSignatures(ctx, instanceDigest) + signatures, err := s.source.GetSignaturesWithFormat(ctx, instanceDigest) if err != nil { return nil, perrors.Wrapf(err, "error checking if image %q has signatures", transports.ImageName(s.reference)) } - canReplaceBlobs := !(len(signatures) > 0 && len(signatures[0]) > 0) + canReplaceBlobs := len(signatures) == 0 infos, err := s.source.LayerInfosForCopy(ctx, instanceDigest) if err != nil { diff --git a/sif/src.go b/sif/src.go index a90d70fcd..b645f80dd 100644 --- a/sif/src.go +++ b/sif/src.go @@ -22,6 +22,7 @@ import ( ) type sifImageSource struct { + impl.Compat impl.PropertyMethodsInitialize impl.NoSignatures impl.DoesNotAffectLayerInfosForCopy @@ -144,7 +145,7 @@ func newImageSource(ctx context.Context, sys *types.SystemContext, ref sifRefere } succeeded = true - return &sifImageSource{ + s := &sifImageSource{ PropertyMethodsInitialize: impl.PropertyMethods(impl.Properties{ HasThreadSafeGetBlob: true, }), @@ -158,7 +159,9 @@ func newImageSource(ctx context.Context, sys *types.SystemContext, ref sifRefere config: configBytes, configDigest: configDigest, manifest: manifestBytes, - }, nil + } + s.Compat = impl.AddCompat(s) + return s, nil } // Reference returns the reference used to set up this source. diff --git a/storage/storage_src.go b/storage/storage_src.go index 5456e2115..cf255b34b 100644 --- a/storage/storage_src.go +++ b/storage/storage_src.go @@ -17,6 +17,7 @@ import ( "github.com/containers/image/v5/internal/image" "github.com/containers/image/v5/internal/imagesource/impl" "github.com/containers/image/v5/internal/imagesource/stubs" + "github.com/containers/image/v5/internal/signature" "github.com/containers/image/v5/internal/tmpdir" "github.com/containers/image/v5/manifest" "github.com/containers/image/v5/types" @@ -30,6 +31,7 @@ import ( ) type storageImageSource struct { + impl.Compat impl.PropertyMethodsInitialize stubs.NoGetBlobAtInitialize @@ -65,6 +67,7 @@ func newImageSource(ctx context.Context, sys *types.SystemContext, imageRef stor SignatureSizes: []int{}, SignaturesSizes: make(map[digest.Digest][]int), } + image.Compat = impl.AddCompat(image) if img.Metadata != "" { if err := json.Unmarshal([]byte(img.Metadata), image); err != nil { return nil, perrors.Wrap(err, "decoding metadata for source image") @@ -292,10 +295,12 @@ func buildLayerInfosForCopy(manifestInfos []manifest.LayerInfo, physicalInfos [] return res, nil } -// GetSignatures() parses the image's signatures blob into a slice of byte slices. -func (s *storageImageSource) GetSignatures(ctx context.Context, instanceDigest *digest.Digest) (signatures [][]byte, err error) { +// 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 *storageImageSource) GetSignaturesWithFormat(ctx context.Context, instanceDigest *digest.Digest) ([]signature.Signature, error) { var offset int - sigslice := [][]byte{} signatureBlobs := []byte{} signatureSizes := s.SignatureSizes key := "signatures" @@ -312,17 +317,22 @@ func (s *storageImageSource) GetSignatures(ctx context.Context, instanceDigest * } signatureBlobs = data } + res := []signature.Signature{} for _, length := range signatureSizes { if offset+length > len(signatureBlobs) { return nil, fmt.Errorf("looking up signatures data for image %q (%s): expected at least %d bytes, only found %d", s.image.ID, instance, len(signatureBlobs), offset+length) } - sigslice = append(sigslice, signatureBlobs[offset:offset+length]) + sig, err := signature.FromBlob(signatureBlobs[offset : offset+length]) + if err != nil { + return nil, fmt.Errorf("parsing signature at (%d, %d): %w", offset, length, err) + } + res = append(res, sig) offset += length } if offset != len(signatureBlobs) { return nil, fmt.Errorf("signatures data (%s) contained %d extra bytes", instance, len(signatureBlobs)-offset) } - return sigslice, nil + return res, nil } // getSize() adds up the sizes of the image's data blobs (which includes the configuration blob), the diff --git a/tarball/tarball_src.go b/tarball/tarball_src.go index 039df406c..1dc4c3ad9 100644 --- a/tarball/tarball_src.go +++ b/tarball/tarball_src.go @@ -21,6 +21,7 @@ import ( ) type tarballImageSource struct { + impl.Compat impl.PropertyMethodsInitialize impl.NoSignatures impl.DoesNotAffectLayerInfosForCopy @@ -209,6 +210,7 @@ func (r *tarballReference) NewImageSource(ctx context.Context, sys *types.System configSize: configSize, manifest: manifestBytes, } + src.Compat = impl.AddCompat(src) return src, nil } From 4b4a5be0413184a77f7036d2e65398979c1621ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 5 Jul 2022 19:54:02 +0200 Subject: [PATCH 8/8] Generalize copy.Image to be able to copy signatures with any format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- copy/copy.go | 17 ++++++++-------- copy/sign.go | 5 +++-- copy/sign_test.go | 9 ++++++-- internal/image/sourced.go | 6 ++++++ internal/image/unparsed.go | 26 ++++++++++++++++++++---- internal/private/private.go | 2 ++ signature/policy_reference_match_test.go | 6 +----- 7 files changed, 50 insertions(+), 21 deletions(-) diff --git a/copy/copy.go b/copy/copy.go index 78f555f18..e48b87da1 100644 --- a/copy/copy.go +++ b/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") } diff --git a/copy/sign.go b/copy/sign.go index 08e0c6c76..59ee5a70b 100644 --- a/copy/sign.go +++ b/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 } diff --git a/copy/sign_test.go b/copy/sign_test.go index 5fa2df326..668e517e1 100644 --- a/copy/sign_test.go +++ b/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) diff --git a/internal/image/sourced.go b/internal/image/sourced.go index dc09a9e04..ff7350113 100644 --- a/internal/image/sourced.go +++ b/internal/image/sourced.go @@ -6,6 +6,7 @@ package image import ( "context" + "github.com/containers/image/v5/internal/signature" "github.com/containers/image/v5/types" ) @@ -129,6 +130,11 @@ 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 deb2e0cd0..a72252e26 100644 --- a/internal/image/unparsed.go +++ b/internal/image/unparsed.go @@ -5,6 +5,9 @@ import ( "fmt" "github.com/containers/image/v5/docker/reference" + "github.com/containers/image/v5/internal/imagesource" + "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" @@ -16,13 +19,13 @@ import ( // // This is publicly visible as c/image/image.UnparsedImage. type UnparsedImage struct { - src types.ImageSource + src private.ImageSource instanceDigest *digest.Digest cachedManifest []byte // A private cache for Manifest(); nil if not yet known. // A private cache for Manifest(), may be the empty string if guessing failed. // Valid iff cachedManifest is not nil. cachedManifestMIMEType string - cachedSignatures [][]byte // A private cache for Signatures(); nil if not yet known. + cachedSignatures []signature.Signature // A private cache for Signatures(); nil if not yet known. } // UnparsedInstance returns a types.UnparsedImage implementation for (source, instanceDigest). @@ -33,7 +36,7 @@ type UnparsedImage struct { // This is publicly visible as c/image/image.UnparsedInstance. func UnparsedInstance(src types.ImageSource, instanceDigest *digest.Digest) *UnparsedImage { return &UnparsedImage{ - src: src, + src: imagesource.FromPublic(src), instanceDigest: instanceDigest, } } @@ -89,8 +92,23 @@ 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) + if err != nil { + return nil, err + } + simpleSigs := [][]byte{} + for _, sig := range sigs { + if sig, ok := sig.(signature.SimpleSigning); ok { + simpleSigs = append(simpleSigs, sig.UntrustedSignature()) + } + } + 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) { if i.cachedSignatures == nil { - sigs, err := i.src.GetSignatures(ctx, i.instanceDigest) + sigs, err := i.src.GetSignaturesWithFormat(ctx, i.instanceDigest) if err != nil { return nil, err } diff --git a/internal/private/private.go b/internal/private/private.go index 40189e043..613c7b136 100644 --- a/internal/private/private.go +++ b/internal/private/private.go @@ -37,6 +37,8 @@ type ImageSource interface { type ImageDestinationInternalOnly interface { // SupportsPutBlobPartial returns true if PutBlobPartial is supported. SupportsPutBlobPartial() bool + // FIXME: Add SupportsSignaturesWithFormat or something like that, to allow early failures + // on unsupported formats. // PutBlobWithOptions writes contents of stream and returns data representing the result. // inputInfo.Digest can be optionally provided if known; if provided, and stream is read to the end without error, the digest MUST match the stream contents. diff --git a/signature/policy_reference_match_test.go b/signature/policy_reference_match_test.go index 92fb227a6..746f90269 100644 --- a/signature/policy_reference_match_test.go +++ b/signature/policy_reference_match_test.go @@ -69,11 +69,7 @@ type refImageReferenceMock struct { } func (ref refImageReferenceMock) Transport() types.ImageTransport { - // We use this in error messages, so sady we must return something. But right now we do so only when DockerReference is nil, so restrict to that. - if ref.ref == nil { - return mocks.NameImageTransport("== Transport mock") - } - panic("unexpected call to a mock function") + return mocks.NameImageTransport("== Transport mock") } func (ref refImageReferenceMock) StringWithinTransport() string { // We use this in error messages, so sadly we must return something. But right now we do so only when DockerReference is nil, so restrict to that.