From 93d10940bab8b4d09b48395fb4ec8dacfb905ea6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Sat, 29 Nov 2025 16:52:34 +0100 Subject: [PATCH 1/5] Add internal/digests.Options and internal/digests.Options.Choose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The goal is to allow building the digest-choice-dependent machinery from the bottom up without committing to an API while we don't understand the problem space; for now, we don't expose any way for users to actually make a choice. This code is never called at this point, so this should not change behavior. Signed-off-by: Miloslav Trmač --- image/internal/digests/digests.go | 146 ++++++++++++++++++ image/internal/digests/digests_test.go | 201 +++++++++++++++++++++++++ 2 files changed, 347 insertions(+) create mode 100644 image/internal/digests/digests.go create mode 100644 image/internal/digests/digests_test.go 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()) + } +} From b10f5a6ff0b35f78e8a08402f566ff8a338405c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 5 Dec 2025 23:12:57 +0100 Subject: [PATCH 2/5] Add image.copy.digestOptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit for now, purely to let us internally work on implementing digest choice / conversions; there is no way for a caller to set it. Should not change behavior, the field has no users. Signed-off-by: Miloslav Trmač --- image/copy/copy.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/image/copy/copy.go b/image/copy/copy.go index eed5f8d96d..cc165ff711 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,15 @@ 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 } // OptionCompressionVariant allows to supply information about @@ -200,6 +210,12 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, if options == nil { options = &Options{} } + // FIXME: Currently, digestsOptions is not implemented at all, and exists in the codebase + // only to allow gradually building the feature set. + // After c/image/copy consistently implements it, provide a public digest options API of some kind. + optionsCopy := *options + optionsCopy.digestOptions = digests.CanonicalDefault() + options = &optionsCopy if err := validateImageListSelection(options.ImageListSelection); err != nil { return nil, err From d6e3e04d6089fc09700851b99fbf229ad33c04cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Sat, 29 Nov 2025 17:12:21 +0100 Subject: [PATCH 3/5] Add DigestIfAlgorithmUnknown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- .../internal/putblobdigest/put_blob_digest.go | 19 +++++++++---- .../putblobdigest/put_blob_digest_test.go | 27 +++++++++++++++++++ 2 files changed, 41 insertions(+), 5 deletions(-) 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), + }, + }) +} From 5b980b7195fe879cca3ecd5205c67ff8f683c518 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 5 Dec 2025 23:14:02 +0100 Subject: [PATCH 4/5] WIP: Sketch PutBlob... digest choice in c/image/copy + transports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is: - UNUSABLE (no external API) - INCOMPLETE (manifest digests, TryReusingBlob, ...) - BROKEN (various code assumes that digest returned from upload matches input) - INCONSISTENT (PutBlobOptions.Digests is only implemented in one transport, and silently ignored elsewhere) Signed-off-by: Miloslav Trmač --- image/copy/blob.go | 5 +++++ image/directory/directory_dest.go | 7 ++++++- image/internal/imagedestination/impl/compat.go | 3 +++ image/internal/imagedestination/wrapper.go | 10 ++++++++++ image/internal/private/private.go | 7 +++++-- 5 files changed, 29 insertions(+), 3 deletions(-) diff --git a/image/copy/blob.go b/image/copy/blob.go index 9db6338d75..d7e7b5dadb 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 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/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. From 8a75ccc92e0dd06d019fe2b64424f3e751316e31 Mon Sep 17 00:00:00 2001 From: Lokesh Mandvekar Date: Tue, 9 Dec 2025 15:13:41 -0500 Subject: [PATCH 5/5] image/copy: Allow user to force digest algorithm in CLI ops like skopeo copy Changes to image/copy/blob.go: - Use digests.Options.Choose() to validate digest changes Changes to image/copy/copy.go: - Add SetForceDigestAlgorithm() helper to configure digest forcing Changes to image/copy/single.go: - Add updateManifestConfigDigest() helper that uses typed manifest structures (OCI1/Schema2) to update config digest, avoiding generic JSON manipulation - Modify copyConfig() to return the new config digest when changed - Update copyUpdatedConfigAndManifest() to handle config digest changes by calling updateManifestConfigDigest() when needed Signed-off-by: Lokesh Mandvekar --- image/copy/blob.go | 13 ++++++++++- image/copy/copy.go | 18 ++++++++++++++- image/copy/single.go | 55 +++++++++++++++++++++++++++++++++++++++----- 3 files changed, 78 insertions(+), 8 deletions(-) diff --git a/image/copy/blob.go b/image/copy/blob.go index d7e7b5dadb..dfbfc28324 100644 --- a/image/copy/blob.go +++ b/image/copy/blob.go @@ -7,6 +7,7 @@ import ( "io" "github.com/sirupsen/logrus" + "go.podman.io/image/v5/internal/digests" "go.podman.io/image/v5/internal/private" compressiontypes "go.podman.io/image/v5/pkg/compression/types" "go.podman.io/image/v5/types" @@ -138,7 +139,17 @@ 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) + expectedAlgo, err := ic.c.options.digestOptions.Choose(digests.Situation{ + Preexisting: stream.info.Digest, + CannotChangeAlgorithmReason: ic.cannotModifyManifestReason, + }) + if err != nil { + return types.BlobInfo{}, err + } + // If we're forcing a different algorithm and the uploaded digest uses that algorithm, it's acceptable + if uploadedInfo.Digest.Algorithm() != expectedAlgo { + 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 cc165ff711..7d6060a3f7 100644 --- a/image/copy/copy.go +++ b/image/copy/copy.go @@ -167,6 +167,19 @@ type Options struct { digestOptions digests.Options } +// SetForceDigestAlgorithm forces the use of a specific digest algorithm for this copy operation. +func (o *Options) SetForceDigestAlgorithm(algo digest.Algorithm) error { + 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 // selected compression algorithm and compression level by the // end-user. Refer to EnsureCompressionVariantsExist to know @@ -214,7 +227,10 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, // only to allow gradually building the feature set. // After c/image/copy consistently implements it, provide a public digest options API of some kind. optionsCopy := *options - optionsCopy.digestOptions = digests.CanonicalDefault() + // Only set default if not already configured + if optionsCopy.digestOptions.MustUseSet() == "" { + optionsCopy.digestOptions = digests.CanonicalDefault() + } options = &optionsCopy if err := validateImageListSelection(options.ImageListSelection); err != nil { diff --git a/image/copy/single.go b/image/copy/single.go index 5a5d5e3ddb..cd5b1c5812 100644 --- a/image/copy/single.go +++ b/image/copy/single.go @@ -592,10 +592,19 @@ 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 } + // Config digest changed due to forcing a different digest algorithm + 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) if err != nil { @@ -611,13 +620,41 @@ func (ic *imageCopier) copyUpdatedConfigAndManifest(ctx context.Context, instanc return man, manifestDigest, nil } +// updateManifestConfigDigest uses typed manifest structures instead of generic JSON manipulation. +// This leverages the existing manifest parsing and serialization infrastructure. +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) + } + + switch typedManifest := m.(type) { + case *manifest.OCI1: + typedManifest.Config.Digest = newConfigDigest + return typedManifest.Serialize() + case *manifest.Schema2: + typedManifest.ConfigDescriptor.Digest = newConfigDigest + return typedManifest.Serialize() + case *manifest.Schema1: + return nil, fmt.Errorf("cannot update config digest for schema1 manifest") + default: + return nil, fmt.Errorf("unsupported manifest type for config digest update: %T", m) + } +} + // 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 +682,19 @@ 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) + // Allow digest algorithm changes when forcing a specific digest algorithm + forcingDifferentAlgo := ic.c.options.digestOptions.MustUseSet() != "" && + destInfo.Digest.Algorithm() != srcInfo.Digest.Algorithm() + if !forcingDifferentAlgo { + 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.