From 464a4adde064e34fd5063ee182f7db76701f96b0 Mon Sep 17 00:00:00 2001 From: closer-claudio Date: Sun, 13 Sep 2026 12:10:21 -0700 Subject: [PATCH] image: a digest-pinned ref only trusts a record that hashes back to it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve built the Image for a cached ref@platform from the record's two fields, digest and config, and for a digest-pinned ref never checked either against the ref. Under the default pull policy that record is the whole story — no registry is consulted — so a record that was edited, or written for something else, redirected `debian@sha256:X` to another rootfs with another entrypoint, offline, with nothing left to notice. The digest the user pinned was the one thing the run did not verify. A plain digest comparison would be wrong: a multi-platform ref names an index, and the record legitimately holds the per-platform manifest's digest, not the index's. So the record now keeps the chain as raw bytes — the index the ref named (when it is one), the platform manifest, the config blob — and a digest-pinned ref is resolved from bytes, not fields: sha256(index) must equal the ref, the index must list sha256(manifest) for the platform (or, with no index, the manifest must hash to the ref), the manifest's config descriptor must equal sha256(config blob), and the digest and config the Image carries are re-derived from those bytes. A record that fails any link is refused with ErrResolve naming what disagrees. A tag's record is taken as is, as before: a tag can name anything. Records written before the chain was kept still serve tags. For a pinned ref they are re-resolved once under missing (the registry answer is recorded with its chain) and refused under never, naming --pull=always. What this does not do: the flattened rootfs blob is a derived artifact and is trusted from the cache as before; the chain reaches the manifest, not the layers. The record cannot be edited undetected any more; the store still can. That is the boundary the content cache has always had. Co-Authored-By: Claude Fable 5.1 Signed-off-by: closer-claudio --- pkg/image/image.go | 245 ++++++++++++++++++++++++++++++++++++---- pkg/image/image_test.go | 195 +++++++++++++++++++++++++++++++- 2 files changed, 420 insertions(+), 20 deletions(-) diff --git a/pkg/image/image.go b/pkg/image/image.go index 23527d9..30d6133 100644 --- a/pkg/image/image.go +++ b/pkg/image/image.go @@ -19,6 +19,7 @@ package image import ( + "bytes" "context" "crypto/sha256" "encoding/hex" @@ -237,15 +238,9 @@ func Resolve(ctx context.Context, store Cache, ref, platformStr, pull string) (* return nil, fmt.Errorf("%w: parse ref %q: %w", ErrResolve, ref, err) } - platform := hostPlatform() - - if platformStr != "" { - p, err := v1.ParsePlatform(platformStr) - if err != nil { - return nil, fmt.Errorf("%w: parse platform %q: %w", ErrResolve, platformStr, err) - } - - platform = *p + platform, err := requestedPlatform(platformStr) + if err != nil { + return nil, err } platKey := platform.String() @@ -254,28 +249,28 @@ func Resolve(ctx context.Context, store Cache, ref, platformStr, pull string) (* // lazy re-fetch via resolveSource stays cancellable too). Auth from Docker's // credential store (a `docker login` lifts the anonymous rate limit; falls // back to anonymous, so always safe to pass). - remoteImage := func() (v1.Image, error) { + remoteImage := func() (v1.Image, *remote.Descriptor, error) { desc, err := remote.Get(parsed, remote.WithContext(ctx), remote.WithPlatform(platform), remote.WithAuthFromKeychain(authn.DefaultKeychain), ) if err != nil { - return nil, fmt.Errorf("%w: resolve %s: %w", ErrResolve, ref, err) + return nil, nil, fmt.Errorf("%w: resolve %s: %w", ErrResolve, ref, err) } img, err := desc.Image() if err != nil { - return nil, fmt.Errorf("%w: image for %s (platform %s): %w", ErrResolve, ref, platKey, err) + return nil, nil, fmt.Errorf("%w: image for %s (platform %s): %w", ErrResolve, ref, platKey, err) } - return img, nil + return img, desc, nil } // online resolves via the registry now, records the resolution, and returns a // fully-bound Image (source set → flatten needs no further network). online := func() (*Image, error) { - img, err := remoteImage() + img, desc, err := remoteImage() if err != nil { return nil, err } @@ -291,9 +286,13 @@ func Resolve(ctx context.Context, store Cache, ref, platformStr, pull string) (* } // Record ref@platform → digest+config so a later missing/never run is - // offline. Best-effort: a cache-write failure must not fail the run. - if cachePath, cacheErr := resolveCacheFile(ref, platKey); cacheErr == nil { - storeResolution(cachePath, dgst.String(), cfg.Config) + // offline, with the manifest chain a digest-pinned ref is verified + // against (see verifyPinnedRecord). Best-effort: a cache-write failure + // must not fail the run. + if rec, recErr := chainedResolution(desc, img, dgst.String(), cfg.Config); recErr == nil { + if cachePath, cacheErr := resolveCacheFile(ref, platKey); cacheErr == nil { + storeResolution(cachePath, rec) + } } return &Image{ @@ -325,6 +324,17 @@ func Resolve(ctx context.Context, store Cache, ref, platformStr, pull string) (* return online() // missing: never seen — resolve now } + // A digest-pinned ref names its content itself; the record is only + // allowed to agree, and it proves that with bytes, not fields. + res, reResolve, err := trustedRecord(parsed, platform, pull, res) + if err != nil { + return nil, err + } + + if reResolve { + return online() + } + hash, err := v1.NewHash(res.Digest) if err != nil { return nil, fmt.Errorf("%w: cached digest %q for %s: %w", ErrResolve, res.Digest, ref, err) @@ -384,6 +394,203 @@ func pinnedFetcher(ctx context.Context, ref string, pinned name.Digest) func() ( type resolution struct { Digest string `json:"digest"` // full digest string, e.g. "sha256:…" Config v1.Config `json:"config"` + + // The chain from the ref to what runs, kept as the raw bytes so a + // digest-pinned ref can be checked offline without trusting the two + // fields above: Index is what the ref's digest names when that is a + // multi-platform index (empty when the ref names an image manifest + // directly), Manifest is the platform image manifest (Digest is its + // sha256), ConfigBlob is the blob Manifest's config descriptor names + // (Config is parsed from it). Absent on records written before the chain + // was kept; such a record still serves a tag, never a pinned ref. + Index []byte `json:"index,omitempty"` + Manifest []byte `json:"manifest,omitempty"` + ConfigBlob []byte `json:"configBlob,omitempty"` +} + +// requestedPlatform parses platformStr ("" → the host). +func requestedPlatform(platformStr string) (v1.Platform, error) { + if platformStr == "" { + return hostPlatform(), nil + } + + parsed, err := v1.ParsePlatform(platformStr) + if err != nil { + return v1.Platform{}, fmt.Errorf("%w: parse platform %q: %w", ErrResolve, platformStr, err) + } + + return *parsed, nil +} + +// trustedRecord is the record Resolve may build an Image from. A tag's +// record is taken as is (a tag can name anything). A digest-pinned ref's +// record must prove itself against the ref (verifyPinnedRecord): a record +// that cannot — older than the chain — means resolve once more under +// missing (reResolve) and a refusal under never; a record whose bytes do +// not add up was edited, and is refused under either. +func trustedRecord( + parsed name.Reference, + platform v1.Platform, + pull string, + res *resolution, +) (rec *resolution, reResolve bool, err error) { + pinned, ok := parsed.(name.Digest) + if !ok { + return res, false, nil + } + + verified, err := verifyPinnedRecord(pinned, platform, res) + + switch { + case err == nil: + return verified, false, nil + case errors.Is(err, errRecordUnchained) && pull == PullMissing: + return nil, true, nil + case errors.Is(err, errRecordUnchained): + return nil, false, fmt.Errorf("%w: %q was resolved before ossein kept the manifest chain; "+ + "re-pull once with --pull=always", ErrResolve, parsed) + default: + return nil, false, err + } +} + +// errRecordUnchained marks a record without the manifest chain: written by an +// ossein that did not keep it. Not an ErrResolve itself — the caller decides +// whether that is a re-resolve (missing) or a refusal (never). +var errRecordUnchained = errors.New("resolution record carries no manifest chain") + +// errIndexMissingManifest: the cached index does not list the cached manifest +// for the platform — the record was altered. +var errIndexMissingManifest = errors.New( + "index does not list the manifest for the platform; the record was altered, re-pull with --pull=always", +) + +// chainedResolution builds the record for what desc (the bytes the ref +// named) and img (the platform image chosen from it) resolved to. +func chainedResolution(desc *remote.Descriptor, img v1.Image, dgst string, cfg v1.Config) (resolution, error) { + manifest, err := img.RawManifest() + if err != nil { + return resolution{}, fmt.Errorf("raw manifest: %w", err) + } + + configBlob, err := img.RawConfigFile() + if err != nil { + return resolution{}, fmt.Errorf("raw config: %w", err) + } + + rec := resolution{Digest: dgst, Config: cfg, Manifest: manifest, ConfigBlob: configBlob} + + if desc.MediaType.IsIndex() { + rec.Index = desc.Manifest + } + + return rec, nil +} + +// bytesDigest is the "sha256:…" digest of raw bytes. +func bytesDigest(raw []byte) string { + sum := sha256.Sum256(raw) + + return "sha256:" + hex.EncodeToString(sum[:]) +} + +// verifyPinnedRecord checks a record against the digest a pinned ref names +// and returns the record with Digest and Config re-derived from the verified +// bytes. The chain: pinned == sha256(Index) and Index lists sha256(Manifest) +// for platform — or, with no Index, pinned == sha256(Manifest); then +// Manifest's config descriptor == sha256(ConfigBlob). A record without the +// bytes is errRecordUnchained; a record whose bytes do not add up is +// ErrResolve — someone edited it, and following it would run a rootfs the +// ref never named. +func verifyPinnedRecord(pinned name.Digest, platform v1.Platform, res *resolution) (*resolution, error) { + if len(res.Manifest) == 0 || len(res.ConfigBlob) == 0 { + return nil, errRecordUnchained + } + + want := pinned.DigestStr() + manifestDigest := bytesDigest(res.Manifest) + + switch { + case len(res.Index) > 0: + if got := bytesDigest(res.Index); got != want { + return nil, fmt.Errorf( + "%w: cached index for %s hashes to %s; the record was altered, re-pull with --pull=always", + ErrResolve, + pinned, + got, + ) + } + + if err := indexLists(res.Index, manifestDigest, platform); err != nil { + return nil, fmt.Errorf("%w: cached index for %s: %w", ErrResolve, pinned, err) + } + case manifestDigest != want: + return nil, fmt.Errorf( + "%w: cached manifest for %s hashes to %s; the record was altered, re-pull with --pull=always", + ErrResolve, + pinned, + manifestDigest, + ) + default: + // No index and the manifest hashes to the ref: the direct chain holds. + } + + if res.Digest != manifestDigest { + return nil, fmt.Errorf( + "%w: record for %s names %s but its manifest is %s; the record was altered, re-pull with --pull=always", + ErrResolve, + pinned, + res.Digest, + manifestDigest, + ) + } + + manifest, err := v1.ParseManifest(bytes.NewReader(res.Manifest)) + if err != nil { + return nil, fmt.Errorf("%w: cached manifest for %s: %w", ErrResolve, pinned, err) + } + + if got := bytesDigest(res.ConfigBlob); manifest.Config.Digest.String() != got { + return nil, fmt.Errorf( + "%w: cached config for %s hashes to %s, manifest names %s; the record was altered, re-pull with --pull=always", + ErrResolve, + pinned, + got, + manifest.Config.Digest, + ) + } + + cfg, err := v1.ParseConfigFile(bytes.NewReader(res.ConfigBlob)) + if err != nil { + return nil, fmt.Errorf("%w: cached config for %s: %w", ErrResolve, pinned, err) + } + + return &resolution{ + Digest: manifestDigest, Config: cfg.Config, + Index: res.Index, Manifest: res.Manifest, ConfigBlob: res.ConfigBlob, + }, nil +} + +// indexLists reports whether raw (an index) names manifestDigest for +// platform: the same OS and architecture, or an entry with no platform. +func indexLists(raw []byte, manifestDigest string, platform v1.Platform) error { + idx, err := v1.ParseIndexManifest(bytes.NewReader(raw)) + if err != nil { + return fmt.Errorf("parsing: %w", err) + } + + for _, entry := range idx.Manifests { + if entry.Digest.String() != manifestDigest { + continue + } + + if entry.Platform == nil || + (entry.Platform.OS == platform.OS && entry.Platform.Architecture == platform.Architecture) { + return nil + } + } + + return fmt.Errorf("%w (%s for %s/%s)", errIndexMissingManifest, manifestDigest, platform.OS, platform.Architecture) } // resolveCacheFile is the on-disk path for a ref@platform resolution — a sibling @@ -440,12 +647,12 @@ func loadResolution(path string) (*resolution, error) { // storeResolution records a resolution at path. Best-effort: failures are // swallowed — a warm-run optimization must never fail a run. -func storeResolution(path, dgst string, cfg v1.Config) { +func storeResolution(path string, res resolution) { if err := os.MkdirAll(filepath.Dir(path), cacheDirPerm); err != nil { return } - encoded, err := json.Marshal(resolution{Digest: dgst, Config: cfg}) + encoded, err := json.Marshal(res) if err != nil { return } diff --git a/pkg/image/image_test.go b/pkg/image/image_test.go index f267a9e..0294a1b 100644 --- a/pkg/image/image_test.go +++ b/pkg/image/image_test.go @@ -14,7 +14,10 @@ import ( goerofs "github.com/forkcloser/erofs" "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/google/go-containerregistry/pkg/v1/mutate" "github.com/google/go-containerregistry/pkg/v1/random" + "github.com/google/go-containerregistry/pkg/v1/types" blobcache "github.com/mycophonic/primordium/store/cache" "github.com/farcloser/ossein/internal/rootfsblob" @@ -159,7 +162,7 @@ func TestResolutionRoundTrip(t *testing.T) { path := filepath.Join(t.TempDir(), "res.json") cfg := v1.Config{Entrypoint: []string{"/bin/sh"}, Env: []string{"FOO=bar"}} - storeResolution(path, "sha256:deadbeef", cfg) + storeResolution(path, resolution{Digest: "sha256:deadbeef", Config: cfg}) res, err := loadResolution(path) if err != nil { @@ -217,6 +220,196 @@ func TestResolvePullNeverUncachedFails(t *testing.T) { } } +// pinnedFixture is a random image, an index that lists it for the host +// platform, and the chained record online() would have written for it. +type pinnedFixture struct { + img v1.Image + imgDigest string + idxDigest string + record resolution +} + +func newPinnedFixture(t *testing.T) pinnedFixture { + t.Helper() + + img, err := random.Image(512, 1) + if err != nil { + t.Fatal(err) + } + + // Listed for the platform the tests resolve — the host's, whichever + // runner this is — since that is what the index must vouch for. + host := hostPlatform() + + idx := mutate.AppendManifests(empty.Index, mutate.IndexAddendum{ + Add: img, + Descriptor: v1.Descriptor{ + MediaType: types.OCIManifestSchema1, + Platform: &host, + }, + }) + + rawIndex, err := idx.RawManifest() + if err != nil { + t.Fatal(err) + } + + rawManifest, err := img.RawManifest() + if err != nil { + t.Fatal(err) + } + + rawConfig, err := img.RawConfigFile() + if err != nil { + t.Fatal(err) + } + + cfg, err := img.ConfigFile() + if err != nil { + t.Fatal(err) + } + + imgDigest, _ := img.Digest() + idxDigest, _ := idx.Digest() + + return pinnedFixture{ + img: img, imgDigest: imgDigest.String(), idxDigest: idxDigest.String(), + record: resolution{ + Digest: imgDigest.String(), Config: cfg.Config, + Index: rawIndex, Manifest: rawManifest, ConfigBlob: rawConfig, + }, + } +} + +// writeRecord stores rec as the resolution for ref on the host platform. +func writeRecord(t *testing.T, ref string, rec resolution) { + t.Helper() + + path, err := resolveCacheFile(ref, hostPlatform().String()) + if err != nil { + t.Fatal(err) + } + + storeResolution(path, rec) +} + +func TestResolvePinnedRefVerifiesTheChain(t *testing.T) { + // Records live under dirs.CacheDir: redirect $HOME. t.Setenv forbids t.Parallel. + t.Setenv("HOME", t.TempDir()) + + cache, err := openCache(t.TempDir()) + if err != nil { + t.Fatal(err) + } + + defer func() { _ = cache.Close() }() + + fixture := newPinnedFixture(t) + + // Pinned to the index: verified through Index → Manifest → ConfigBlob. + viaIndex := "example.com/pinned@" + fixture.idxDigest + writeRecord(t, viaIndex, fixture.record) + + for _, pull := range []string{PullNever, PullMissing} { + got, err := Resolve(context.Background(), cache, viaIndex, "", pull) + if err != nil { + t.Fatalf("Resolve(%s, pinned to index) = %v", pull, err) + } + + if "sha256:"+got.Digest != fixture.imgDigest { + t.Fatalf("Resolve(%s) digest = %s, want %s", pull, got.Digest, fixture.imgDigest) + } + } + + // Pinned to the manifest itself: no Index in the chain. + direct := "example.com/pinned@" + fixture.imgDigest + rec := fixture.record + rec.Index = nil + writeRecord(t, direct, rec) + + if _, err := Resolve(context.Background(), cache, direct, "", PullNever); err != nil { + t.Fatalf("Resolve(pinned to manifest) = %v", err) + } +} + +func TestResolvePinnedRefRefusesAnAlteredRecord(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + cache, err := openCache(t.TempDir()) + if err != nil { + t.Fatal(err) + } + + defer func() { _ = cache.Close() }() + + fixture := newPinnedFixture(t) + other := newPinnedFixture(t) + ref := "example.com/pinned@" + fixture.idxDigest + + for name, alter := range map[string]func(*resolution){ + // The digest field points elsewhere while the bytes still hash: caught + // by Digest ≠ sha256(Manifest). + "digest swapped": func(r *resolution) { r.Digest = other.imgDigest }, + // Whole chain swapped for another image's: sha256(Index) ≠ the ref. + "index swapped": func(r *resolution) { + r.Index, r.Manifest, r.ConfigBlob, r.Digest = other.record.Index, other.record.Manifest, other.record.ConfigBlob, other.imgDigest + }, + // Manifest from another image under the right index: the index does not list it. + "manifest swapped": func(r *resolution) { r.Manifest, r.Digest = other.record.Manifest, other.imgDigest }, + // The config (entrypoint, env, user — what the container runs with) + // replaced: the manifest's config digest no longer matches. + "config swapped": func(r *resolution) { r.ConfigBlob = other.record.ConfigBlob }, + "config edited": func(r *resolution) { + r.ConfigBlob = append([]byte(`{"config":{"Entrypoint":["/evil"]},"x":`), r.ConfigBlob[1:]...) + }, + } { + rec := fixture.record + alter(&rec) + writeRecord(t, ref, rec) + + for _, pull := range []string{PullNever, PullMissing} { + if _, err := Resolve(context.Background(), cache, ref, "", pull); !errors.Is(err, ErrResolve) { + t.Errorf("%s, Resolve(%s) = %v, want ErrResolve", name, pull, err) + } + } + } +} + +func TestResolvePinnedRefWithoutChainNeedsARePull(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + cache, err := openCache(t.TempDir()) + if err != nil { + t.Fatal(err) + } + + defer func() { _ = cache.Close() }() + + fixture := newPinnedFixture(t) + + // A record from before the chain was kept: digest and config only. + legacy := resolution{Digest: fixture.imgDigest, Config: fixture.record.Config} + + // Pinned ref: never refuses and says how to fix it (missing would go to + // the registry, which a unit test cannot). + pinned := "example.com/pinned@" + fixture.idxDigest + writeRecord(t, pinned, legacy) + + _, err = Resolve(context.Background(), cache, pinned, "", PullNever) + if !errors.Is(err, ErrResolve) || !strings.Contains(err.Error(), "--pull=always") { + t.Fatalf("Resolve(never, unchained pinned record) = %v, want ErrResolve naming --pull=always", err) + } + + // A tag is not pinned to anything: the same legacy record still serves it. + tag := "example.com/pinned:dev" + writeRecord(t, tag, legacy) + + got, err := Resolve(context.Background(), cache, tag, "", PullNever) + if err != nil || "sha256:"+got.Digest != fixture.imgDigest { + t.Fatalf("Resolve(never, tag with legacy record) = (%v, %v)", got, err) + } +} + func TestResolveUnknownPullPolicyErrors(t *testing.T) { t.Parallel()