From 8c2d0611b6e33b0c034d47dd38a0dc6563995755 Mon Sep 17 00:00:00 2001 From: CrazyMax <1951866+crazy-max@users.noreply.github.com> Date: Wed, 28 Oct 2026 14:57:30 +0100 Subject: [PATCH] source: preserve image cache across attestation changes Rebuilding an unchanged image can change its index digest through provenance attestations, invalidating downstream cache despite an unchanged platform manifest. Use the selected platform manifest as an alternate source cache key while retaining the root manifest key for existing caches. Capture the image manifest during the existing platform-filtered traversal, including nested indexes and embedded manifests. Preserve the first image manifest so malformed config or layer descriptors cannot replace its cache identity. Keep checksum validation and provenance pins tied to the root manifest. Both cache keys use verified manifest digests, without restoring the unverified DiffID fallback removed for GHSA-f2v9-hprr-32q3. Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com> --- client/client_cache_test.go | 247 ++++++++++++++++++++++++++++++++++ client/client_test.go | 2 + source/containerimage/pull.go | 35 +++-- util/pull/pull.go | 37 +++-- 4 files changed, 299 insertions(+), 22 deletions(-) diff --git a/client/client_cache_test.go b/client/client_cache_test.go index 79b83f60a..b60c91274 100644 --- a/client/client_cache_test.go +++ b/client/client_cache_test.go @@ -1,6 +1,8 @@ package client import ( + "archive/tar" + "bytes" "context" "encoding/base64" "encoding/json" @@ -27,6 +29,7 @@ import ( "github.com/containerd/containerd/v2/pkg/namespaces" contentlocal "github.com/containerd/containerd/v2/plugins/content/local" cerrdefs "github.com/containerd/errdefs" + "github.com/containerd/platforms" "github.com/gofrs/flock" cacheimporttypes "github.com/moby/buildkit/cache/remotecache/v1/types" "github.com/moby/buildkit/client/llb" @@ -36,10 +39,13 @@ import ( "github.com/moby/buildkit/session" sessioncontent "github.com/moby/buildkit/session/content" "github.com/moby/buildkit/util/compression" + "github.com/moby/buildkit/util/contentutil" "github.com/moby/buildkit/util/testutil" "github.com/moby/buildkit/util/testutil/helpers" "github.com/moby/buildkit/util/testutil/integration" "github.com/moby/buildkit/util/testutil/workers" + digest "github.com/opencontainers/go-digest" + "github.com/opencontainers/image-spec/specs-go" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" "github.com/stretchr/testify/require" @@ -573,6 +579,247 @@ func testAlternativeInputKeysCacheImport(t *testing.T, sb integration.Sandbox) { require.NoError(t, err) } +func testImageCacheAcrossAttestationChanges(t *testing.T, sb integration.Sandbox) { + requiresLinux(t) + workers.CheckFeatureCompat(t, sb, workers.FeatureDirectPush, workers.FeatureProvenance) + registry, err := sb.NewRegistry() + if errors.Is(err, integration.ErrRequirements) { + t.Skip(err.Error()) + } + require.NoError(t, err) + c, err := New(sb.Context(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "Dockerfile"), []byte("FROM busybox:latest\nRUN echo hello world\n"), 0600)) + fs, err := fsutil.NewFS(dir) + require.NoError(t, err) + target := registry + "/buildkit/attestation-cache:latest" + var previousIndex, previousManifest string + var previousOutput []byte + for _, tc := range []struct { + name string + nested bool + embedded bool + changed bool + }{ + { + name: "initial build", + }, + { + name: "attestation-only rebuild", + }, + { + name: "nested index", + nested: true, + }, + { + name: "nested index with embedded manifests", + nested: true, + embedded: true, + }, + { + name: "changed base layers", + changed: true, + }, + } { + t.Log(tc.name) + if tc.changed { + require.NoError(t, os.WriteFile(filepath.Join(dir, "Dockerfile"), []byte("FROM busybox:latest\nRUN echo changed > /marker\n"), 0600)) + } + resp, err := c.Solve(sb.Context(), nil, SolveOpt{ + Frontend: "dockerfile.v0", + FrontendAttrs: map[string]string{"attest:provenance": "mode=min"}, + LocalMounts: map[string]fsutil.FS{"context": fs, "dockerfile": fs}, + Exports: []ExportEntry{{ + Type: ExporterImage, + Attrs: map[string]string{"name": target, "push": "true"}, + }}, + }, nil) + require.NoError(t, err) + indexDigest := resp.ExporterResponse[exptypes.ExporterImageDigestKey] + desc, provider, err := contentutil.ProviderFromRef(sb.Context(), target+"@"+indexDigest) + require.NoError(t, err) + require.True(t, images.IsIndexType(desc.MediaType)) + dt, err := content.ReadBlob(sb.Context(), provider, desc) + require.NoError(t, err) + var idx ocispecs.Index + require.NoError(t, json.Unmarshal(dt, &idx)) + require.Len(t, idx.Manifests, 2) + manifestDigest := idx.Manifests[0].Digest.String() + if tc.nested { + ingester, err := contentutil.IngesterFromRef(sb.Context(), target) + require.NoError(t, err) + pushIndex := func(idx ocispecs.Index) ocispecs.Descriptor { + dt, err := json.Marshal(idx) + require.NoError(t, err) + desc := ocispecs.Descriptor{ + MediaType: ocispecs.MediaTypeImageIndex, + Digest: digest.FromBytes(dt), + Size: int64(len(dt)), + } + require.NoError(t, content.WriteBlob(sb.Context(), ingester, desc.Digest.String(), bytes.NewReader(dt), desc)) + if tc.embedded { + desc.Data = dt + } + return desc + } + if tc.embedded { + dt, err := content.ReadBlob(sb.Context(), provider, idx.Manifests[0]) + require.NoError(t, err) + idx.Manifests[0].Data = dt + desc = pushIndex(idx) + } + desc = pushIndex(ocispecs.Index{ + Versioned: specs.Versioned{SchemaVersion: 2}, + MediaType: ocispecs.MediaTypeImageIndex, + Manifests: []ocispecs.Descriptor{desc}, + }) + indexDigest = desc.Digest.String() + } + + // Pin each solve to its new index so resolver caching cannot hide the + // change. Random output proves the downstream RUN reused its result. + st := llb.Image(target+"@"+indexDigest). + Run(llb.Shlex("sh -c 'head -c 100 /dev/urandom > /out/unique'")). + AddMount("/out", llb.Scratch()) + def, err := st.Marshal(sb.Context()) + require.NoError(t, err) + out := t.TempDir() + _, err = c.Solve(sb.Context(), def, SolveOpt{ + Exports: []ExportEntry{{Type: ExporterLocal, OutputDir: out}}, + }, nil) + require.NoError(t, err) + output, err := os.ReadFile(filepath.Join(out, "unique")) + require.NoError(t, err) + require.Len(t, output, 100) + if previousIndex != "" { + require.NotEqual(t, previousIndex, indexDigest) + if !tc.changed { + require.Equal(t, previousManifest, manifestDigest) + require.Equal(t, previousOutput, output, "changes outside the image manifest must not invalidate the downstream RUN") + } else { + require.NotEqual(t, previousManifest, manifestDigest) + require.NotEqual(t, previousOutput, output, "changed base layers must invalidate the downstream RUN") + } + } + previousIndex, previousManifest, previousOutput = indexDigest, manifestDigest, output + } +} + +// A failed pull advertising another image's DiffIDs must not poison that +// image's source cache (GHSA-f2v9-hprr-32q3). +func testImageCacheRejectsUnverifiedDiffIDs(t *testing.T, sb integration.Sandbox) { + requiresLinux(t) + registry, err := sb.NewRegistry() + if errors.Is(err, integration.ErrRequirements) { + t.Skip(err.Error()) + } + require.NoError(t, err) + c, err := New(sb.Context(), sb.Address()) + require.NoError(t, err) + defer c.Close() + ctx := sb.Context() + target := registry + "/buildkit/diffid-cache:test" + ingester, err := contentutil.IngesterFromRef(ctx, target) + require.NoError(t, err) + push := func(mediaType string, dt []byte) ocispecs.Descriptor { + desc := ocispecs.Descriptor{ + MediaType: mediaType, + Digest: digest.FromBytes(dt), + Size: int64(len(dt)), + } + require.NoError(t, content.WriteBlob(ctx, ingester, desc.Digest.String(), bytes.NewReader(dt), desc)) + return desc + } + layer := func(value string) ocispecs.Descriptor { + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + require.NoError(t, tw.WriteHeader(&tar.Header{Name: "proof", Mode: 0644, Size: int64(len(value))})) + _, err := tw.Write([]byte(value)) + require.NoError(t, err) + require.NoError(t, tw.Close()) + return push(ocispecs.MediaTypeImageLayer, buf.Bytes()) + } + victimLayer := layer("VICTIM") + attackerLayer := layer("ATTACKER") + platform := platforms.DefaultSpec() + dt, err := json.Marshal(ocispecs.Image{ + Platform: platform, + RootFS: ocispecs.RootFS{ + Type: "layers", + DiffIDs: []digest.Digest{victimLayer.Digest}, + }, + }) + require.NoError(t, err) + config := push(ocispecs.MediaTypeImageConfig, dt) + image := func(layer, config ocispecs.Descriptor) (string, ocispecs.Descriptor) { + dt, err := json.Marshal(ocispecs.Manifest{ + Versioned: specs.Versioned{SchemaVersion: 2}, + MediaType: ocispecs.MediaTypeImageManifest, + Config: config, + Layers: []ocispecs.Descriptor{layer}, + }) + require.NoError(t, err) + manifest := push(ocispecs.MediaTypeImageManifest, dt) + manifest.Platform = &platform + dt, err = json.Marshal(ocispecs.Index{ + Versioned: specs.Versioned{SchemaVersion: 2}, + MediaType: ocispecs.MediaTypeImageIndex, + Manifests: []ocispecs.Descriptor{manifest}, + }) + require.NoError(t, err) + index := push(ocispecs.MediaTypeImageIndex, dt) + return target + "@" + index.Digest.String(), manifest + } + victim, victimManifest := image(victimLayer, config) + attacker, _ := image(attackerLayer, config) + read := func(ref string) ([]byte, error) { + st := llb.Scratch().File(llb.Copy(llb.Image(ref), "/proof", "/proof")) + def, err := st.Marshal(ctx) + require.NoError(t, err) + out := t.TempDir() + _, err = c.Solve(ctx, def, SolveOpt{ + Exports: []ExportEntry{{Type: ExporterLocal, OutputDir: out}}, + }, nil) + if err != nil { + return nil, err + } + return os.ReadFile(filepath.Join(out, "proof")) + } + _, err = read(attacker) + require.ErrorContains(t, err, "failed to verify layer") + + // Do not prune between builds: the failed attack and victim share a daemon + // and its source cache, which is the advisory's trust boundary. + got, err := read(victim) + require.NoError(t, err) + require.Equal(t, "VICTIM", string(got)) + + _, provider, err := contentutil.ProviderFromRef(ctx, victim) + require.NoError(t, err) + manifestData, err := content.ReadBlob(ctx, provider, victimManifest) + require.NoError(t, err) + // The registry requires layer references to exist in its blob store. + push(ocispecs.MediaTypeImageLayer, manifestData) + + // A manifest referenced as a layer must not replace the selected image + // manifest and borrow its cached source result. Use a different DiffID so + // the cache manager cannot reuse the victim's verified snapshot by ChainID. + dt, err = json.Marshal(ocispecs.Image{ + Platform: platform, + RootFS: ocispecs.RootFS{ + Type: "layers", + DiffIDs: []digest.Digest{attackerLayer.Digest}, + }, + }) + require.NoError(t, err) + malformed, _ := image(victimManifest, push(ocispecs.MediaTypeImageConfig, dt)) + _, err = read(malformed) + require.Error(t, err, "a manifest-typed layer must not reuse the victim's cached source") +} + func testBasicRegistryCacheImportExport(t *testing.T, sb integration.Sandbox) { workers.CheckFeatureCompat(t, sb, workers.FeatureCacheExport, diff --git a/client/client_test.go b/client/client_test.go index e652e0b3a..2e45a35ca 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -23,6 +23,8 @@ var allTests = []func(t *testing.T, sb integration.Sandbox){ testBasicLocalCacheImportExport, testAlternativeInputKeysCacheImport, testConcurrentLocalCacheExport, + testImageCacheAcrossAttestationChanges, + testImageCacheRejectsUnverifiedDiffIDs, testBasicRegistryCacheImportExport, testBasicS3CacheImportExport, testCacheExportCacheDeletedContent, diff --git a/source/containerimage/pull.go b/source/containerimage/pull.go index ca4e7b627..a89dddfd8 100644 --- a/source/containerimage/pull.go +++ b/source/containerimage/pull.go @@ -49,13 +49,14 @@ type puller struct { ResolverType store sourceresolver.ResolveImageConfigOptStore - g flightcontrol.Group[struct{}] - cacheKeyErr error - cacheKeyDone bool - releaseTmpLeases func(context.Context) error - descHandlers cache.DescHandlers - manifest *pull.PulledManifests - manifestKey string + g flightcontrol.Group[struct{}] + cacheKeyErr error + cacheKeyDone bool + releaseTmpLeases func(context.Context) error + descHandlers cache.DescHandlers + manifest *pull.PulledManifests + manifestKey string + platformManifestKey string *pull.Puller } @@ -132,8 +133,8 @@ func (p *puller) CacheKey(ctx context.Context, jobCtx solver.JobContext, index i return struct{}{}, err } - if p.checksum != "" && p.manifest.MainManifestDesc.Digest != p.checksum { - return struct{}{}, errors.Errorf("image digest %s for %s does not match expected checksum %s", p.manifest.MainManifestDesc.Digest, p.Ref, p.checksum) + if p.checksum != "" && p.manifest.RootManifestDesc.Digest != p.checksum { + return struct{}{}, errors.Errorf("image digest %s for %s does not match expected checksum %s", p.manifest.RootManifestDesc.Digest, p.Ref, p.checksum) } if ll := p.layerLimit; ll != nil { @@ -172,12 +173,17 @@ func (p *puller) CacheKey(ctx context.Context, jobCtx solver.JobContext, index i } } - desc := p.manifest.MainManifestDesc + desc := p.manifest.RootManifestDesc k, err := mainManifestKey(desc, p.Platform, p.layerLimit) if err != nil { return struct{}{}, err } p.manifestKey = k.String() + k, err = mainManifestKey(p.manifest.ImageManifestDesc, p.Platform, p.layerLimit) + if err != nil { + return struct{}{}, err + } + p.platformManifestKey = k.String() // Do not use RootFS DiffIDs from the image config as an alternate // cache key. They are verified only when layers are applied, and a // failed malicious pull must not poison another image source. @@ -193,7 +199,14 @@ func (p *puller) CacheKey(ctx context.Context, jobCtx solver.JobContext, index i cacheOpts[cache.DescHandlerKey(dgst)] = descHandler } - return p.manifestKey, p.manifest.MainManifestDesc.Digest.String(), cacheOpts, true, nil + // Keep the index key for existing caches, but also match the selected + // manifest so changes to attestations or other platforms do not invalidate + // this source. Both keys identify verified manifest content, not DiffIDs + // advertised by an unverified image config. + if index == 0 { + return p.manifestKey, p.manifest.RootManifestDesc.Digest.String(), cacheOpts, p.manifestKey == p.platformManifestKey, nil + } + return p.platformManifestKey, p.manifest.RootManifestDesc.Digest.String(), cacheOpts, true, nil } func (p *puller) Snapshot(ctx context.Context, jobCtx solver.JobContext) (ir cache.ImmutableRef, err error) { diff --git a/util/pull/pull.go b/util/pull/pull.go index 1f5926c8a..359ac4f0f 100644 --- a/util/pull/pull.go +++ b/util/pull/pull.go @@ -45,12 +45,13 @@ type Puller struct { var _ content.Provider = &provider{} type PulledManifests struct { - Ref string - MainManifestDesc ocispecs.Descriptor - ConfigDesc ocispecs.Descriptor - Nonlayers []ocispecs.Descriptor - Descriptors []ocispecs.Descriptor - Provider func(session.Group) content.Provider + Ref string + RootManifestDesc ocispecs.Descriptor + ImageManifestDesc ocispecs.Descriptor // Platform-specific manifest selected from RootManifestDesc. + ConfigDesc ocispecs.Descriptor + Nonlayers []ocispecs.Descriptor + Descriptors []ocispecs.Descriptor + Provider func(session.Group) content.Provider } func (p *Puller) resolve(ctx context.Context, resolver remotes.Resolver) error { @@ -144,6 +145,18 @@ func (p *Puller) PullManifests(ctx context.Context, getResolver SessionResolver) // Limit manifests pulled to the best match in an index childrenHandler = images.LimitManifests(childrenHandler, platform, 1) + var manifestDesc ocispecs.Descriptor + var manifestOnce sync.Once + manifestHandler := images.HandlerFunc(func(ctx context.Context, desc ocispecs.Descriptor) ([]ocispecs.Descriptor, error) { + if images.IsManifestType(desc.MediaType) { + // Each index yields one child, so the first manifest is the + // selected image, including through nested indexes. Do not let + // malformed config or layer descriptors replace it later. + manifestOnce.Do(func() { manifestDesc = desc }) + } + return nil, nil + }) + dslHandler, err := docker.AppendDistributionSourceLabel(p.ContentStore, p.ref) if err != nil { return nil, err @@ -152,6 +165,7 @@ func (p *Puller) PullManifests(ctx context.Context, getResolver SessionResolver) filterLayerBlobs(metadata, &mu), retryhandler.New(limited.FetchHandler(p.ContentStore, fetcher, p.ref), logs.LoggerFromContext(ctx)), childrenHandler, + manifestHandler, dslHandler, ) @@ -174,11 +188,12 @@ func (p *Puller) PullManifests(ctx context.Context, getResolver SessionResolver) } return &PulledManifests{ - Ref: p.ref, - MainManifestDesc: p.desc, - ConfigDesc: p.configDesc, - Nonlayers: p.nonlayers, - Descriptors: p.layers, + Ref: p.ref, + RootManifestDesc: p.desc, + ImageManifestDesc: manifestDesc, + ConfigDesc: p.configDesc, + Nonlayers: p.nonlayers, + Descriptors: p.layers, Provider: func(g session.Group) content.Provider { return &provider{puller: p, resolver: getResolver(g)} },