diff --git a/image/copy/blob.go b/image/copy/blob.go index 9db6338d75..508bb1320c 100644 --- a/image/copy/blob.go +++ b/image/copy/blob.go @@ -101,6 +101,11 @@ func (ic *imageCopier) copyBlobFromStream(ctx context.Context, srcReader io.Read Cache: ic.c.blobInfoCache, IsConfig: isConfig, EmptyLayer: emptyLayer, + Digests: ic.c.options.digestOptions, + // CannotChangeDigestReason requires stream.info.Digest to always be set, and it is: + // If ic.cannotModifyManifestReason, stream.info was not modified since its initialization at the top of this + // function, and the caller is required to provide a digest. + CannotChangeDigestReason: ic.cannotModifyManifestReason, } if !isConfig { options.LayerIndex = &layerIndex @@ -133,7 +138,10 @@ func (ic *imageCopier) copyBlobFromStream(ctx context.Context, srcReader io.Read return types.BlobInfo{}, fmt.Errorf("Internal error writing blob %s, digest verification failed but was ignored", srcInfo.Digest) } if stream.info.Digest != "" && uploadedInfo.Digest != stream.info.Digest { - return types.BlobInfo{}, fmt.Errorf("Internal error writing blob %s, blob with digest %s saved with digest %s", srcInfo.Digest, stream.info.Digest, uploadedInfo.Digest) + // If algorithms match, the whole digest values must match + if stream.info.Digest.Algorithm() == uploadedInfo.Digest.Algorithm() { + return types.BlobInfo{}, fmt.Errorf("Internal error writing blob %s, blob with digest %s saved with digest %s", srcInfo.Digest, stream.info.Digest, uploadedInfo.Digest) + } } if digestingReader.validationSucceeded { if err := compressionStep.recordValidatedDigestData(ic.c, uploadedInfo, srcInfo, encryptionStep, decryptionStep); err != nil { diff --git a/image/copy/copy.go b/image/copy/copy.go index eed5f8d96d..9c5e50c246 100644 --- a/image/copy/copy.go +++ b/image/copy/copy.go @@ -14,6 +14,7 @@ import ( "github.com/sirupsen/logrus" "go.podman.io/image/v5/docker/reference" internalblobinfocache "go.podman.io/image/v5/internal/blobinfocache" + "go.podman.io/image/v5/internal/digests" "go.podman.io/image/v5/internal/image" "go.podman.io/image/v5/internal/imagedestination" "go.podman.io/image/v5/internal/imagesource" @@ -155,6 +156,35 @@ type Options struct { // In oci-archive: destinations, this will set the create/mod/access timestamps in each tar entry // (but not a timestamp of the created archive file). DestinationTimestamp *time.Time + + // FIXME: + // - this reference to an internal type is unusable from the outside even if we made the field public + // - what is the actual semantics? Right now it is probably “choices to use when writing to the destination”, TBD + // - anyway do we want to expose _all_ of the digests.Options tunables, or fewer? + // - … do we want to expose _more_ granularity than that? + // - (“must have at least sha512 integrity when reading”, what does “at least” mean for random pairs of algorithms?) + // - should some of this be in config files, maybe ever per-registry? + digestOptions digests.Options +} + +// BrokenSetForceDestinationDigestAlgorithm forces the use of a specific digest algorithm when writing to the destination. +// +// UNSTABLE API: This API is incomplete and may be changed or removed at any time. +// It currently only enforces the digest algorithm for a subset of transports and operations. +// See https://github.com/containers/container-libs/pull/552 for implementation status. +func (o *Options) BrokenSetForceDestinationDigestAlgorithm(algo digest.Algorithm) error { + if o.digestOptions.MustUseSet() != "" { + return fmt.Errorf("digest options are already configured") + } + if !algo.Available() { + return fmt.Errorf("digest algorithm %q is not available", algo.String()) + } + digestOpts, err := digests.MustUse(algo) + if err != nil { + return fmt.Errorf("failed to set force-digest algorithm: %w", err) + } + o.digestOptions = digestOpts + return nil } // OptionCompressionVariant allows to supply information about @@ -200,6 +230,13 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, if options == nil { options = &Options{} } + // FIXME: digestsOptions exists to gradually build the feature. Provide public API once fully implemented. + // Set default only if not configured by BrokenSetForceDestinationDigestAlgorithm + if options.digestOptions.MustUseSet() == "" { + optionsCopy := *options + optionsCopy.digestOptions = digests.CanonicalDefault() + options = &optionsCopy + } if err := validateImageListSelection(options.ImageListSelection); err != nil { return nil, err diff --git a/image/copy/multiple.go b/image/copy/multiple.go index 9ab82f9bb0..196108f94f 100644 --- a/image/copy/multiple.go +++ b/image/copy/multiple.go @@ -208,6 +208,12 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte, cannotModifyManifestListReason = "Instructed to preserve digests" } + // FIXME: Multi-arch not supported yet; would need config digest updates for each instance. + // See https://github.com/containers/container-libs/pull/552#discussion_r2611627578 + if c.options.digestOptions.MustUseSet() != "" { + return nil, fmt.Errorf("forcing digest algorithm with multi-arch images is not yet implemented") + } + // Determine if we'll need to convert the manifest list to a different format. forceListMIMEType := c.options.ForceManifestMIMEType switch forceListMIMEType { diff --git a/image/copy/single.go b/image/copy/single.go index 5a5d5e3ddb..4bd874eb25 100644 --- a/image/copy/single.go +++ b/image/copy/single.go @@ -18,6 +18,7 @@ import ( "github.com/sirupsen/logrus" "github.com/vbauerster/mpb/v8" "go.podman.io/image/v5/docker/reference" + "go.podman.io/image/v5/internal/digests" "go.podman.io/image/v5/internal/image" "go.podman.io/image/v5/internal/pkg/platform" "go.podman.io/image/v5/internal/private" @@ -592,12 +593,27 @@ func (ic *imageCopier) copyUpdatedConfigAndManifest(ctx context.Context, instanc return nil, "", fmt.Errorf("reading manifest: %w", err) } - if err := ic.copyConfig(ctx, pendingImage); err != nil { + newConfigDigest, err := ic.copyConfig(ctx, pendingImage) + if err != nil { return nil, "", err } + // FIXME: Single image only; multi-arch needs per-instance config digest updates. + // See https://github.com/containers/container-libs/pull/552#discussion_r2611627578 + if newConfigDigest != nil { + man, err = ic.updateManifestConfigDigest(man, pendingImage, *newConfigDigest) + if err != nil { + return nil, "", fmt.Errorf("updating manifest config digest: %w", err) + } + } + ic.c.Printf("Writing manifest to image destination\n") - manifestDigest, err := manifest.Digest(man) + // Choose the digest algorithm based on digest options + manifestDigestAlgo, err := ic.c.options.digestOptions.Choose(digests.Situation{}) + if err != nil { + return nil, "", fmt.Errorf("choosing manifest digest algorithm: %w", err) + } + manifestDigest, err := manifest.DigestWithAlgorithm(man, manifestDigestAlgo) if err != nil { return nil, "", err } @@ -611,13 +627,33 @@ func (ic *imageCopier) copyUpdatedConfigAndManifest(ctx context.Context, instanc return man, manifestDigest, nil } +// updateManifestConfigDigest updates the config digest in the manifest using the manifest abstraction layer. +func (ic *imageCopier) updateManifestConfigDigest(manifestBlob []byte, src types.Image, newConfigDigest digest.Digest) ([]byte, error) { + _, mt, err := src.Manifest(context.Background()) + if err != nil { + return nil, fmt.Errorf("getting manifest type: %w", err) + } + + m, err := manifest.FromBlob(manifestBlob, mt) + if err != nil { + return nil, fmt.Errorf("parsing manifest: %w", err) + } + + if err := m.UpdateConfigDigest(newConfigDigest); err != nil { + return nil, err + } + + return m.Serialize() +} + // copyConfig copies config.json, if any, from src to dest. -func (ic *imageCopier) copyConfig(ctx context.Context, src types.Image) error { +// It returns the new config digest if it changed (due to digest algorithm forcing), or nil otherwise. +func (ic *imageCopier) copyConfig(ctx context.Context, src types.Image) (*digest.Digest, error) { srcInfo := src.ConfigInfo() if srcInfo.Digest != "" { if err := ic.c.concurrentBlobCopiesSemaphore.Acquire(ctx, 1); err != nil { // This can only fail with ctx.Err(), so no need to blame acquiring the semaphore. - return fmt.Errorf("copying config: %w", err) + return nil, fmt.Errorf("copying config: %w", err) } defer ic.c.concurrentBlobCopiesSemaphore.Release(1) @@ -645,13 +681,17 @@ func (ic *imageCopier) copyConfig(ctx context.Context, src types.Image) error { return destInfo, nil }() if err != nil { - return err + return nil, err } if destInfo.Digest != srcInfo.Digest { - return fmt.Errorf("Internal error: copying uncompressed config blob %s changed digest to %s", srcInfo.Digest, destInfo.Digest) + // If algorithms match, the whole digest values must match + if destInfo.Digest.Algorithm() == srcInfo.Digest.Algorithm() { + return nil, fmt.Errorf("Internal error: copying uncompressed config blob %s changed digest to %s", srcInfo.Digest, destInfo.Digest) + } + return &destInfo.Digest, nil } } - return nil + return nil, nil } // diffIDResult contains both a digest value and an error from diffIDComputationGoroutine. @@ -743,19 +783,34 @@ func (ic *imageCopier) copyLayer(ctx context.Context, srcInfo types.BlobInfo, to tocDigest = *d } - reused, reusedBlob, err := ic.c.dest.TryReusingBlobWithOptions(ctx, srcInfo, private.TryReusingBlobOptions{ - Cache: ic.c.blobInfoCache, - CanSubstitute: canSubstitute, - EmptyLayer: emptyLayer, - LayerIndex: &layerIndex, - SrcRef: srcRef, - PossibleManifestFormats: append([]string{ic.manifestConversionPlan.preferredMIMEType}, ic.manifestConversionPlan.otherMIMETypeCandidates...), - RequiredCompression: requiredCompression, - OriginalCompression: srcInfo.CompressionAlgorithm, - TOCDigest: tocDigest, - }) - if err != nil { - return types.BlobInfo{}, "", fmt.Errorf("trying to reuse blob %s at destination: %w", srcInfo.Digest, err) + // FIXME: Blob reuse disabled when forcing different digest algorithm. + canTryReuse := true + if forcedAlgo := ic.c.options.digestOptions.MustUseSet(); forcedAlgo != "" { + if srcInfo.Digest.Algorithm() != forcedAlgo { + logrus.Debugf("Skipping blob reuse for %s: digest algorithm %s doesn't match forced algorithm %s", + srcInfo.Digest, srcInfo.Digest.Algorithm(), forcedAlgo) + canTryReuse = false + } + } + + reused := false + var reusedBlob private.ReusedBlob + if canTryReuse { + var err error + reused, reusedBlob, err = ic.c.dest.TryReusingBlobWithOptions(ctx, srcInfo, private.TryReusingBlobOptions{ + Cache: ic.c.blobInfoCache, + CanSubstitute: canSubstitute, + EmptyLayer: emptyLayer, + LayerIndex: &layerIndex, + SrcRef: srcRef, + PossibleManifestFormats: append([]string{ic.manifestConversionPlan.preferredMIMEType}, ic.manifestConversionPlan.otherMIMETypeCandidates...), + RequiredCompression: requiredCompression, + OriginalCompression: srcInfo.CompressionAlgorithm, + TOCDigest: tocDigest, + }) + if err != nil { + return types.BlobInfo{}, "", fmt.Errorf("trying to reuse blob %s at destination: %w", srcInfo.Digest, err) + } } if reused { logrus.Debugf("Skipping blob %s (already present):", srcInfo.Digest) diff --git a/image/copy/single_test.go b/image/copy/single_test.go index 66daf901ce..0f8340d5b6 100644 --- a/image/copy/single_test.go +++ b/image/copy/single_test.go @@ -159,3 +159,22 @@ func TestComputeDiffID(t *testing.T) { _, err = computeDiffID(reader, nil) assert.Error(t, err) } + +func TestBrokenSetForceDestinationDigestAlgorithm(t *testing.T) { + opts := &Options{} + + // First call should succeed + err := opts.BrokenSetForceDestinationDigestAlgorithm(digest.SHA256) + require.NoError(t, err) + + // Second call should fail because digest options are already configured + err = opts.BrokenSetForceDestinationDigestAlgorithm(digest.SHA512) + assert.Error(t, err) + assert.Contains(t, err.Error(), "digest options are already configured") + + // Test with unavailable algorithm + opts2 := &Options{} + err = opts2.BrokenSetForceDestinationDigestAlgorithm("sha999") + assert.Error(t, err) + assert.Contains(t, err.Error(), "is not available") +} diff --git a/image/directory/directory_dest.go b/image/directory/directory_dest.go index c9a593d085..0d7b08a912 100644 --- a/image/directory/directory_dest.go +++ b/image/directory/directory_dest.go @@ -11,6 +11,7 @@ import ( "github.com/opencontainers/go-digest" "github.com/sirupsen/logrus" + "go.podman.io/image/v5/internal/digests" "go.podman.io/image/v5/internal/imagedestination/impl" "go.podman.io/image/v5/internal/imagedestination/stubs" "go.podman.io/image/v5/internal/private" @@ -150,7 +151,11 @@ func (d *dirImageDestination) PutBlobWithOptions(ctx context.Context, stream io. } }() - digester, stream := putblobdigest.DigestIfUnknown(stream, inputInfo) + algorithm, err := options.Digests.Choose(digests.Situation{Preexisting: inputInfo.Digest, CannotChangeAlgorithmReason: options.CannotChangeDigestReason}) + if err != nil { + return private.UploadedBlob{}, err + } + digester, stream := putblobdigest.DigestIfAlgorithmUnknown(stream, inputInfo, algorithm) // TODO: This can take quite some time, and should ideally be cancellable using ctx.Done(). size, err := io.Copy(blobFile, stream) diff --git a/image/internal/digests/digests.go b/image/internal/digests/digests.go new file mode 100644 index 0000000000..ca76ea3be7 --- /dev/null +++ b/image/internal/digests/digests.go @@ -0,0 +1,146 @@ +// Package digests provides an internal representation of users’ digest use preferences. +// +// Something like this _might_ be eventually made available as a public API: +// before doing so, carefully think whether the API should be modified before we commit to it. + +package digests + +import ( + "errors" + "fmt" + + "github.com/opencontainers/go-digest" +) + +// Options records users’ preferences for used digest algorithm usage. +// It is a value type and can be copied using ordinary assignment. +// +// It can only be created using one of the provided constructors. +type Options struct { + initialized bool // To prevent uses that don’t call a public constructor; this is necessary to enforce the .Available() promise. + + // If any of the fields below is set, it is guaranteed to be .Available(). + + mustUse digest.Algorithm // If not "", written digests must use this algorithm. + prefer digest.Algorithm // If not "", use this algorithm whenever possible. + defaultAlgo digest.Algorithm // If not "", use this algorithm if there is no reason to use anything else. +} + +// CanonicalDefault is Options which default to using digest.Canonical if there is no reason to use a different algorithm +// (e.g. when there is no pre-existing digest). +// +// The configuration can be customized using .WithPreferred() or .WithDefault(). +func CanonicalDefault() Options { + // This does not set .defaultAlgo so that .WithDefault() can be called (once). + return Options{ + initialized: true, + } +} + +// MustUse constructs Options which always use algo. +func MustUse(algo digest.Algorithm) (Options, error) { + // We don’t provide Options.WithMustUse because there is no other option that makes a difference + // once .mustUse is set. + if !algo.Available() { + return Options{}, fmt.Errorf("attempt to use an unavailable digest algorithm %q", algo.String()) + } + return Options{ + initialized: true, + mustUse: algo, + }, nil +} + +// WithPreferred returns a copy of o with a “preferred” algorithm set to algo. +// The preferred algorithm is used whenever possible (but if there is a strict requirement to use something else, it will be overridden). +func (o Options) WithPreferred(algo digest.Algorithm) (Options, error) { + if err := o.ensureInitialized(); err != nil { + return Options{}, err + } + if o.prefer != "" { + return Options{}, errors.New("digests.Options already have a 'prefer' algorithm configured") + } + + if !algo.Available() { + return Options{}, fmt.Errorf("attempt to use an unavailable digest algorithm %q", algo.String()) + } + o.prefer = algo + return o, nil +} + +// WithDefault returns a copy of o with a “default” algorithm set to algo. +// The default algorithm is used if there is no reason to use anything else (e.g. when there is no pre-existing digest). +func (o Options) WithDefault(algo digest.Algorithm) (Options, error) { + if err := o.ensureInitialized(); err != nil { + return Options{}, err + } + if o.defaultAlgo != "" { + return Options{}, errors.New("digests.Options already have a 'default' algorithm configured") + } + + if !algo.Available() { + return Options{}, fmt.Errorf("attempt to use an unavailable digest algorithm %q", algo.String()) + } + o.defaultAlgo = algo + return o, nil +} + +// ensureInitialized returns an error if o is not initialized. +func (o Options) ensureInitialized() error { + if !o.initialized { + return errors.New("internal error: use of uninitialized digests.Options") + } + return nil +} + +// Situation records the context in which a digest is being chosen. +type Situation struct { + Preexisting digest.Digest // If not "", a pre-existing digest value (frequently one which is cheaper to use than others) + CannotChangeAlgorithmReason string // The reason why we must use Preexisting, or "" if we can use other algorithms. +} + +// Choose chooses a digest algorithm based on the options and the situation. +func (o Options) Choose(s Situation) (digest.Algorithm, error) { + if err := o.ensureInitialized(); err != nil { + return "", err + } + + if s.CannotChangeAlgorithmReason != "" && s.Preexisting == "" { + return "", fmt.Errorf("internal error: digests.Situation.CannotChangeAlgorithmReason is set but Preexisting is empty") + } + + var choice digest.Algorithm // = what we want to use + switch { + case o.mustUse != "": + choice = o.mustUse + case s.CannotChangeAlgorithmReason != "": + choice = s.Preexisting.Algorithm() + if !choice.Available() { + return "", fmt.Errorf("existing digest uses unimplemented algorithm %s", choice) + } + case o.prefer != "": + choice = o.prefer + case s.Preexisting != "" && s.Preexisting.Algorithm().Available(): + choice = s.Preexisting.Algorithm() + case o.defaultAlgo != "": + choice = o.defaultAlgo + default: + choice = digest.Canonical // We assume digest.Canonical is always available. + } + + if s.CannotChangeAlgorithmReason != "" && choice != s.Preexisting.Algorithm() { + return "", fmt.Errorf("requested to always use digest algorithm %s but we cannot replace existing digest algorithm %s: %s", + choice, s.Preexisting.Algorithm(), s.CannotChangeAlgorithmReason) + } + + return choice, nil +} + +// MustUseSet returns an algorithm if o is set to always use a specific algorithm, "" if it is flexible. +func (o Options) MustUseSet() digest.Algorithm { + // We don’t do .ensureInitialized() because that would require an extra error value just for that. + // This should not be a part of any public API either way. + if o.mustUse != "" { + return o.mustUse + } + return "" +} diff --git a/image/internal/digests/digests_test.go b/image/internal/digests/digests_test.go new file mode 100644 index 0000000000..e5a98a851f --- /dev/null +++ b/image/internal/digests/digests_test.go @@ -0,0 +1,201 @@ +package digests + +import ( + "testing" + + "github.com/opencontainers/go-digest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCanonicalDefault(t *testing.T) { + o := CanonicalDefault() + assert.Equal(t, Options{initialized: true}, o) +} + +func TestMustUse(t *testing.T) { + o, err := MustUse(digest.SHA512) + require.NoError(t, err) + assert.Equal(t, Options{ + initialized: true, + mustUse: digest.SHA512, + }, o) + + _, err = MustUse(digest.Algorithm("this is not a known algorithm")) + require.Error(t, err) +} + +func TestOptionsWithPreferred(t *testing.T) { + preferSHA512, err := CanonicalDefault().WithPreferred(digest.SHA512) + require.NoError(t, err) + assert.Equal(t, Options{ + initialized: true, + prefer: digest.SHA512, + }, preferSHA512) + + for _, c := range []struct { + base Options + algo digest.Algorithm + }{ + { // Uninitialized Options + base: Options{}, + algo: digest.SHA256, + }, + { // Unavailable algorithm + base: CanonicalDefault(), + algo: digest.Algorithm("this is not a known algorithm"), + }, + { // WithPreferred already called + base: preferSHA512, + algo: digest.SHA512, + }, + } { + _, err := c.base.WithPreferred(c.algo) + assert.Error(t, err) + } +} + +func TestOptionsWithDefault(t *testing.T) { + defaultSHA512, err := CanonicalDefault().WithDefault(digest.SHA512) + require.NoError(t, err) + assert.Equal(t, Options{ + initialized: true, + defaultAlgo: digest.SHA512, + }, defaultSHA512) + + for _, c := range []struct { + base Options + algo digest.Algorithm + }{ + { // Uninitialized Options + base: Options{}, + algo: digest.SHA256, + }, + { // Unavailable algorithm + base: CanonicalDefault(), + algo: digest.Algorithm("this is not a known algorithm"), + }, + { // WithDefault already called + base: defaultSHA512, + algo: digest.SHA512, + }, + } { + _, err := c.base.WithDefault(c.algo) + assert.Error(t, err) + } +} + +func TestOptionsChoose(t *testing.T) { + sha512Digest := digest.Digest("sha512:cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e") + unknownDigest := digest.Digest("unknown:abcd1234") + + // The tests use sha512 = pre-existing if any; sha384 = primary configured; sha256 = supposedly irrelevant. + + mustSHA384, err := MustUse(digest.SHA384) + require.NoError(t, err) + mustSHA384, err = mustSHA384.WithPreferred(digest.SHA256) + require.NoError(t, err) + mustSHA384, err = mustSHA384.WithDefault(digest.SHA256) + require.NoError(t, err) + + preferSHA384, err := CanonicalDefault().WithPreferred(digest.SHA384) + require.NoError(t, err) + preferSHA384, err = preferSHA384.WithDefault(digest.SHA256) + require.NoError(t, err) + + defaultSHA384, err := CanonicalDefault().WithDefault(digest.SHA384) + require.NoError(t, err) + + cases := []struct { + opts Options + wantDefault digest.Algorithm + wantPreexisting digest.Algorithm // Pre-existing sha512 + wantCannotChange digest.Algorithm // Pre-existing sha512, CannotChange + wantUnavailable digest.Algorithm // Pre-existing unavailable + }{ + { + opts: Options{}, // uninitialized + wantDefault: "", + wantPreexisting: "", + wantCannotChange: "", + wantUnavailable: "", + }, + { + opts: mustSHA384, + wantDefault: digest.SHA384, + wantPreexisting: digest.SHA384, + wantCannotChange: "", + // Warning: We don’t generally _promise_ that unavailable digests are going to be silently ignored + // in these situations (e.g. we might still try to validate them when reading inputs). + wantUnavailable: digest.SHA384, + }, + { + opts: preferSHA384, + wantDefault: digest.SHA384, + wantPreexisting: digest.SHA384, + wantCannotChange: digest.SHA512, + wantUnavailable: digest.SHA384, + }, + { + opts: defaultSHA384, + wantDefault: digest.SHA384, + wantPreexisting: digest.SHA512, + wantCannotChange: digest.SHA512, + wantUnavailable: digest.SHA384, + }, + { + opts: CanonicalDefault(), + wantDefault: digest.SHA256, + wantPreexisting: digest.SHA512, + wantCannotChange: digest.SHA512, + wantUnavailable: digest.SHA256, + }, + } + for _, c := range cases { + run := func(s Situation, want digest.Algorithm) { + res, err := c.opts.Choose(s) + if want == "" { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, want, res) + } + } + + run(Situation{}, c.wantDefault) + run(Situation{Preexisting: sha512Digest}, c.wantPreexisting) + run(Situation{Preexisting: sha512Digest, CannotChangeAlgorithmReason: "test reason"}, c.wantCannotChange) + run(Situation{Preexisting: unknownDigest}, c.wantUnavailable) + + run(Situation{Preexisting: unknownDigest, CannotChangeAlgorithmReason: "test reason"}, "") + run(Situation{CannotChangeAlgorithmReason: "test reason"}, "") // CannotChangeAlgorithm with missing Preexisting + } +} + +func TestOptionsMustUseSet(t *testing.T) { + mustSHA512, err := MustUse(digest.SHA512) + require.NoError(t, err) + prefersSHA512, err := CanonicalDefault().WithPreferred(digest.SHA512) + require.NoError(t, err) + defaultSHA512, err := CanonicalDefault().WithDefault(digest.SHA512) + require.NoError(t, err) + for _, c := range []struct { + opts Options + want digest.Algorithm + }{ + { + opts: mustSHA512, + want: digest.SHA512, + }, + { + opts: prefersSHA512, + want: "", + }, + { + opts: defaultSHA512, + want: "", + }, + } { + assert.Equal(t, c.want, c.opts.MustUseSet()) + } +} diff --git a/image/internal/image/docker_schema1.go b/image/internal/image/docker_schema1.go index da7a943b3d..77b018e8db 100644 --- a/image/internal/image/docker_schema1.go +++ b/image/internal/image/docker_schema1.go @@ -255,3 +255,8 @@ func (m *manifestSchema1) SupportsEncryption(context.Context) bool { func (m *manifestSchema1) CanChangeLayerCompression(mimeType string) bool { return true // There are no MIME types in the manifest, so we must assume a valid image. } + +// UpdateConfigDigest updates the config descriptor's digest in the manifest. +func (m *manifestSchema1) UpdateConfigDigest(newDigest digest.Digest) error { + return fmt.Errorf("cannot update config digest for schema1 manifest") +} diff --git a/image/internal/image/docker_schema2.go b/image/internal/image/docker_schema2.go index b40f4fc71e..1d75771f32 100644 --- a/image/internal/image/docker_schema2.go +++ b/image/internal/image/docker_schema2.go @@ -412,3 +412,8 @@ func (m *manifestSchema2) SupportsEncryption(context.Context) bool { func (m *manifestSchema2) CanChangeLayerCompression(mimeType string) bool { return m.m.CanChangeLayerCompression(mimeType) } + +// UpdateConfigDigest updates the config descriptor's digest in the manifest. +func (m *manifestSchema2) UpdateConfigDigest(newDigest digest.Digest) error { + return m.m.UpdateConfigDigest(newDigest) +} diff --git a/image/internal/image/manifest.go b/image/internal/image/manifest.go index d6ae8b6fa5..0ffa143482 100644 --- a/image/internal/image/manifest.go +++ b/image/internal/image/manifest.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/opencontainers/go-digest" imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" "go.podman.io/image/v5/docker/reference" "go.podman.io/image/v5/manifest" @@ -59,6 +60,9 @@ type genericManifest interface { // algorithms depends not on the current format, but possibly on the target of a conversion (if UpdatedImage converts // to a different manifest format). CanChangeLayerCompression(mimeType string) bool + // UpdateConfigDigest updates the config descriptor's digest in the manifest. + // This returns an error if the manifest does not support config digest updates (e.g., schema1 manifests). + UpdateConfigDigest(newDigest digest.Digest) error } // manifestInstanceFromBlob returns a genericManifest implementation for (manblob, mt) in src. diff --git a/image/internal/image/oci.go b/image/internal/image/oci.go index 8ddb2875e0..ff0afcfc41 100644 --- a/image/internal/image/oci.go +++ b/image/internal/image/oci.go @@ -8,6 +8,7 @@ import ( "slices" ociencspec "github.com/containers/ocicrypt/spec" + "github.com/opencontainers/go-digest" imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" "go.podman.io/image/v5/docker/reference" "go.podman.io/image/v5/internal/iolimits" @@ -332,3 +333,8 @@ func (m *manifestOCI1) SupportsEncryption(context.Context) bool { func (m *manifestOCI1) CanChangeLayerCompression(mimeType string) bool { return m.m.CanChangeLayerCompression(mimeType) } + +// UpdateConfigDigest updates the config descriptor's digest in the manifest. +func (m *manifestOCI1) UpdateConfigDigest(newDigest digest.Digest) error { + return m.m.UpdateConfigDigest(newDigest) +} diff --git a/image/internal/imagedestination/impl/compat.go b/image/internal/imagedestination/impl/compat.go index 9a8d187138..f77914657f 100644 --- a/image/internal/imagedestination/impl/compat.go +++ b/image/internal/imagedestination/impl/compat.go @@ -6,6 +6,7 @@ import ( "github.com/opencontainers/go-digest" "go.podman.io/image/v5/internal/blobinfocache" + "go.podman.io/image/v5/internal/digests" "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/internal/signature" "go.podman.io/image/v5/types" @@ -46,6 +47,8 @@ func (c *Compat) PutBlob(ctx context.Context, stream io.Reader, inputInfo types. res, err := c.dest.PutBlobWithOptions(ctx, stream, inputInfo, private.PutBlobOptions{ Cache: blobinfocache.FromBlobInfoCache(cache), IsConfig: isConfig, + + Digests: digests.CanonicalDefault(), }) if err != nil { return types.BlobInfo{}, err diff --git a/image/internal/imagedestination/wrapper.go b/image/internal/imagedestination/wrapper.go index cbbb6b42a5..5496c900aa 100644 --- a/image/internal/imagedestination/wrapper.go +++ b/image/internal/imagedestination/wrapper.go @@ -5,6 +5,7 @@ import ( "io" "github.com/opencontainers/go-digest" + "go.podman.io/image/v5/internal/digests" "go.podman.io/image/v5/internal/imagedestination/stubs" "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/internal/signature" @@ -53,6 +54,15 @@ func (w *wrapped) PutBlobWithOptions(ctx context.Context, stream io.Reader, inpu if err != nil { return private.UploadedBlob{}, err } + // Check that the returned digest is compatible with options.Digests. If it isn’t, there’s nothing we can do, but at least the callers of PutBlobWithOptions + // won’t need to double-check. + if _, err := options.Digests.Choose(digests.Situation{ + Preexisting: res.Digest, + CannotChangeAlgorithmReason: "external transport API does not allow choosing a digest algorithm", + }); err != nil { + return private.UploadedBlob{}, err + } + return private.UploadedBlob{ Digest: res.Digest, Size: res.Size, diff --git a/image/internal/private/private.go b/image/internal/private/private.go index a5d2057aef..e9fab8060c 100644 --- a/image/internal/private/private.go +++ b/image/internal/private/private.go @@ -9,6 +9,7 @@ import ( imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" "go.podman.io/image/v5/docker/reference" "go.podman.io/image/v5/internal/blobinfocache" + "go.podman.io/image/v5/internal/digests" "go.podman.io/image/v5/internal/signature" compression "go.podman.io/image/v5/pkg/compression/types" "go.podman.io/image/v5/types" @@ -111,8 +112,10 @@ type PutBlobOptions struct { // if they use internal/imagedestination/impl.Compat; // in that case, they will all be consistently zero-valued. - EmptyLayer bool // True if the blob is an "empty"/"throwaway" layer, and may not necessarily be physically represented. - LayerIndex *int // If the blob is a layer, a zero-based index of the layer within the image; nil otherwise. + EmptyLayer bool // True if the blob is an "empty"/"throwaway" layer, and may not necessarily be physically represented. + LayerIndex *int // If the blob is a layer, a zero-based index of the layer within the image; nil otherwise. + Digests digests.Options // Unlike other private fields, this is always initialized, and mandatory requests are enforced by the compatibility wrapper. + CannotChangeDigestReason string // The reason why PutBlobWithOptions is provided with a digest and the destination must use precisely that one (in particular, use that algorithm). } // PutBlobPartialOptions are used in PutBlobPartial. diff --git a/image/internal/putblobdigest/put_blob_digest.go b/image/internal/putblobdigest/put_blob_digest.go index ce50542751..865ee8abcd 100644 --- a/image/internal/putblobdigest/put_blob_digest.go +++ b/image/internal/putblobdigest/put_blob_digest.go @@ -13,15 +13,15 @@ type Digester struct { digester digest.Digester // Or nil } -// newDigester initiates computation of a digest.Canonical digest of stream, +// newDigester initiates computation of a digest of stream using the specified algorithm, // if !validDigest; otherwise it just records knownDigest to be returned later. // The caller MUST use the returned stream instead of the original value. -func newDigester(stream io.Reader, knownDigest digest.Digest, validDigest bool) (Digester, io.Reader) { +func newDigester(stream io.Reader, knownDigest digest.Digest, validDigest bool, algorithm digest.Algorithm) (Digester, io.Reader) { if validDigest { return Digester{knownDigest: knownDigest}, stream } else { res := Digester{ - digester: digest.Canonical.Digester(), + digester: algorithm.Digester(), } stream = io.TeeReader(stream, res.digester.Hash()) return res, stream @@ -34,7 +34,7 @@ func newDigester(stream io.Reader, knownDigest digest.Digest, validDigest bool) // The caller MUST use the returned stream instead of the original value. func DigestIfUnknown(stream io.Reader, blobInfo types.BlobInfo) (Digester, io.Reader) { d := blobInfo.Digest - return newDigester(stream, d, d != "") + return newDigester(stream, d, d != "", digest.Canonical) } // DigestIfCanonicalUnknown initiates computation of a digest.Canonical digest of stream, @@ -43,7 +43,16 @@ func DigestIfUnknown(stream io.Reader, blobInfo types.BlobInfo) (Digester, io.Re // The caller MUST use the returned stream instead of the original value. func DigestIfCanonicalUnknown(stream io.Reader, blobInfo types.BlobInfo) (Digester, io.Reader) { d := blobInfo.Digest - return newDigester(stream, d, d != "" && d.Algorithm() == digest.Canonical) + return newDigester(stream, d, d != "" && d.Algorithm() == digest.Canonical, digest.Canonical) +} + +// DigestIfAlgorithmUnknown initiates computation of a digest of stream, +// if a digest of the specified algorithm is not supplied in the provided blobInfo; +// otherwise blobInfo.Digest will be used. +// The caller MUST use the returned stream instead of the original value. +func DigestIfAlgorithmUnknown(stream io.Reader, blobInfo types.BlobInfo, algorithm digest.Algorithm) (Digester, io.Reader) { + d := blobInfo.Digest + return newDigester(stream, d, d != "" && d.Algorithm() == algorithm, algorithm) } // Digest() returns a digest value possibly computed by Digester. diff --git a/image/internal/putblobdigest/put_blob_digest_test.go b/image/internal/putblobdigest/put_blob_digest_test.go index 8ca42954a8..c6a3e6343c 100644 --- a/image/internal/putblobdigest/put_blob_digest_test.go +++ b/image/internal/putblobdigest/put_blob_digest_test.go @@ -73,3 +73,30 @@ func TestDigestIfCanonicalUnknown(t *testing.T) { }, }) } + +func TestDigestIfAlgorithmUnknown(t *testing.T) { + testDigester(t, func(r io.Reader, bi types.BlobInfo) (Digester, io.Reader) { + return DigestIfAlgorithmUnknown(r, bi, digest.SHA512) + }, []testCase{ + { + inputDigest: digest.Digest("sha512:uninspected-value"), + computesDigest: false, + expectedDigest: digest.Digest("sha512:uninspected-value"), + }, + { + inputDigest: digest.Digest("sha256:uninspected-value"), + computesDigest: true, + expectedDigest: digest.SHA512.FromBytes(testData), + }, + { + inputDigest: digest.Digest("unknown-algorithm:uninspected-value"), + computesDigest: true, + expectedDigest: digest.SHA512.FromBytes(testData), + }, + { + inputDigest: "", + computesDigest: true, + expectedDigest: digest.SHA512.FromBytes(testData), + }, + }) +} diff --git a/image/manifest/docker_schema1.go b/image/manifest/docker_schema1.go index a6d5c67758..76a6cf3224 100644 --- a/image/manifest/docker_schema1.go +++ b/image/manifest/docker_schema1.go @@ -176,6 +176,11 @@ func (m *Schema1) UpdateLayerInfos(layerInfos []types.BlobInfo) error { return nil } +// UpdateConfigDigest updates the config descriptor's digest in the manifest. +func (m *Schema1) UpdateConfigDigest(newDigest digest.Digest) error { + return fmt.Errorf("cannot update config digest for schema1 manifest") +} + // Serialize returns the manifest in a blob format. // NOTE: Serialize() does not in general reproduce the original blob if this object was loaded from one, even if no modifications were made! func (m *Schema1) Serialize() ([]byte, error) { diff --git a/image/manifest/docker_schema1_test.go b/image/manifest/docker_schema1_test.go index 77775de33d..206c80f15c 100644 --- a/image/manifest/docker_schema1_test.go +++ b/image/manifest/docker_schema1_test.go @@ -367,3 +367,12 @@ func TestSchema1ImageID(t *testing.T) { // This is mostly a smoke-test; it’s fine to just update this value if that implementation changes. assert.Equal(t, "9ca4bda0a6b3727a6ffcc43e981cad0f24e2ec79d338f6ba325b4dfd0756fb8f", id) } + +func TestSchema1UpdateConfigDigestFails(t *testing.T) { + m := manifestSchema1FromFixture(t, "schema2-to-schema1-by-docker.json") + newDigest := digest.Digest("sha512:abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890ab") + + err := m.UpdateConfigDigest(newDigest) + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot update config digest for schema1 manifest") +} diff --git a/image/manifest/docker_schema2.go b/image/manifest/docker_schema2.go index b4255d8863..25583fe5c4 100644 --- a/image/manifest/docker_schema2.go +++ b/image/manifest/docker_schema2.go @@ -305,3 +305,10 @@ func (m *Schema2) ImageID([]digest.Digest) (string, error) { func (m *Schema2) CanChangeLayerCompression(mimeType string) bool { return compressionVariantsRecognizeMIMEType(schema2CompressionMIMETypeSets, mimeType) } + +// UpdateConfigDigest updates the config descriptor's digest in the manifest. +// This returns an error if the manifest does not support config digest updates. +func (m *Schema2) UpdateConfigDigest(newDigest digest.Digest) error { + m.ConfigDescriptor.Digest = newDigest + return nil +} diff --git a/image/manifest/docker_schema2_test.go b/image/manifest/docker_schema2_test.go index fcdf689809..43e2bca968 100644 --- a/image/manifest/docker_schema2_test.go +++ b/image/manifest/docker_schema2_test.go @@ -288,3 +288,19 @@ func TestSchema2CanChangeLayerCompression(t *testing.T) { // Some projects like to use squashfs and other unspecified formats for layers; don’t touch those. assert.False(t, m.CanChangeLayerCompression("a completely unknown and quite possibly invalid MIME type")) } + +func TestSchema2UpdateConfigDigest(t *testing.T) { + m := manifestSchema2FromFixture(t, "v2s2.manifest.json") + originalDigest := m.ConfigDescriptor.Digest + + newDigest := digest.Digest("sha512:abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890ab") + err := m.UpdateConfigDigest(newDigest) + require.NoError(t, err) + assert.Equal(t, newDigest, m.ConfigDescriptor.Digest) + assert.NotEqual(t, originalDigest, m.ConfigDescriptor.Digest) + + newDigest256 := digest.Digest("sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef") + err = m.UpdateConfigDigest(newDigest256) + require.NoError(t, err) + assert.Equal(t, newDigest256, m.ConfigDescriptor.Digest) +} diff --git a/image/manifest/manifest.go b/image/manifest/manifest.go index d54b395d31..22f963f653 100644 --- a/image/manifest/manifest.go +++ b/image/manifest/manifest.go @@ -90,6 +90,10 @@ type Manifest interface { // the underlying image format is expected to include a configuration blob. Inspect(configGetter func(types.BlobInfo) ([]byte, error)) (*types.ImageInspectInfo, error) + // UpdateConfigDigest updates the config descriptor's digest in the manifest. + // This returns an error if the manifest does not support config digest updates (e.g., schema1 manifests). + UpdateConfigDigest(newDigest digest.Digest) error + // Serialize returns the manifest in a blob format. // NOTE: Serialize() does not in general reproduce the original blob if this object was loaded from one, even if no modifications were made! Serialize() ([]byte, error) diff --git a/image/manifest/oci.go b/image/manifest/oci.go index 286d58c423..46d8f560b5 100644 --- a/image/manifest/oci.go +++ b/image/manifest/oci.go @@ -274,3 +274,10 @@ func (m *OCI1) CanChangeLayerCompression(mimeType string) bool { } return compressionVariantsRecognizeMIMEType(oci1CompressionMIMETypeSets, mimeType) } + +// UpdateConfigDigest updates the config descriptor's digest in the manifest. +// This returns an error if the manifest does not support config digest updates. +func (m *OCI1) UpdateConfigDigest(newDigest digest.Digest) error { + m.Config.Digest = newDigest + return nil +} diff --git a/image/manifest/oci_test.go b/image/manifest/oci_test.go index 50d2b463e0..b9ea2a8089 100644 --- a/image/manifest/oci_test.go +++ b/image/manifest/oci_test.go @@ -411,3 +411,21 @@ func TestOCI1CanChangeLayerCompression(t *testing.T) { artifact := manifestOCI1FromFixture(t, "ociv1.artifact.json") assert.False(t, artifact.CanChangeLayerCompression(imgspecv1.MediaTypeImageLayerGzip)) } + +func TestOCI1UpdateConfigDigest(t *testing.T) { + m := manifestOCI1FromFixture(t, "ociv1.manifest.json") + originalDigest := m.Config.Digest + + // Test updating to sha512 + newDigest := digest.Digest("sha512:abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890ab") + err := m.UpdateConfigDigest(newDigest) + require.NoError(t, err) + assert.Equal(t, newDigest, m.Config.Digest) + assert.NotEqual(t, originalDigest, m.Config.Digest) + + // Test updating back to sha256 + newDigest256 := digest.Digest("sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef") + err = m.UpdateConfigDigest(newDigest256) + require.NoError(t, err) + assert.Equal(t, newDigest256, m.Config.Digest) +}