diff --git a/test/integration/clients.go b/test/integration/clients.go index 1249a3c..65f7b43 100644 --- a/test/integration/clients.go +++ b/test/integration/clients.go @@ -4,6 +4,7 @@ package integration import ( "context" + "encoding/json" "errors" "fmt" "io" @@ -22,6 +23,7 @@ import ( ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/regclient/regclient" "github.com/regclient/regclient/config" + "github.com/regclient/regclient/types/manifest" "github.com/regclient/regclient/types/ref" "oras.land/oras-go/v2" orasoci "oras.land/oras-go/v2/content/oci" @@ -195,12 +197,19 @@ func (orasClient) Pull(ctx context.Context, ref string, a testharness.Auth) (v1. } // Resolve performs HEAD /v2//manifests/ and returns the // manifest descriptor — its Digest matches ggcr's img.Digest() for the - // same content. We don't need to fetch blobs; that mirrors crane.Pull - // followed by .Digest(), which is also manifest-only. + // same content. desc, err := repo.Resolve(ctx, tag.TagStr()) if err != nil { return v1.Hash{}, normalizeErr(err) } + // Then read the layer bytes, for the same reason craneClient.Pull does: + // resolving the manifest alone never asks the registry for a blob, and + // ATCR serves manifests from the user's PDS where they are world-readable. + // A manifest-only "pull" cannot fail an authorization check on blobs, so + // every pull row in the matrix would pass without testing what it claims. + if err := orasReadLayers(ctx, repo, desc); err != nil { + return v1.Hash{}, normalizeErr(err) + } return v1.Hash{Algorithm: desc.Digest.Algorithm().String(), Hex: desc.Digest.Encoded()}, nil } @@ -240,6 +249,44 @@ func (orasClient) PushIndex(ctx context.Context, t *testing.T, refStr string, id return nil } +// orasReadLayers fetches every layer blob named by a manifest and discards the +// bytes. Pulling is the only matrix operation that can exercise blob +// authorization, and it only does so if something actually reads a blob. +func orasReadLayers(ctx context.Context, repo *orasremote.Repository, desc ocispec.Descriptor) error { + rc, err := repo.Manifests().Fetch(ctx, desc) + if err != nil { + return err + } + body, err := io.ReadAll(rc) + if closeErr := rc.Close(); err == nil { + err = closeErr + } + if err != nil { + return err + } + + var m ocispec.Manifest + if err := json.Unmarshal(body, &m); err != nil { + // An index rather than an image manifest: PullIndex covers that shape. + return nil //nolint:nilerr + } + for _, layer := range m.Layers { + lr, err := repo.Blobs().Fetch(ctx, layer) + if err != nil { + return err + } + _, copyErr := io.Copy(io.Discard, lr) + closeErr := lr.Close() + if copyErr != nil { + return copyErr + } + if closeErr != nil { + return closeErr + } + } + return nil +} + func (orasClient) PullIndex(ctx context.Context, refStr string, a testharness.Auth) (v1.Hash, error) { // Same head-only resolve as Pull — manifest digest is the same value for // images and indexes (it's just the manifest body's sha256). @@ -337,13 +384,34 @@ func (regclientClient) Pull(ctx context.Context, refStr string, a testharness.Au rc := newRegclient(parsed.Context().RegistryStr(), a) defer rc.Close(ctx, r) - // ManifestHead matches crane's lazy-pull semantics: HEAD /manifests/ - // returns the descriptor with the registry-computed digest. - mh, err := rc.ManifestHead(ctx, r) + // ManifestGet rather than ManifestHead: the head gives the digest, but the + // body is what names the layers, and the layers are the only part of a + // pull that exercises blob authorization. See craneClient.Pull. + m, err := rc.ManifestGet(ctx, r) if err != nil { return v1.Hash{}, normalizeErr(err) } - desc := mh.GetDescriptor() + desc := m.GetDescriptor() + if imager, ok := m.(manifest.Imager); ok { + layers, err := imager.GetLayers() + if err != nil { + return v1.Hash{}, normalizeErr(err) + } + for _, l := range layers { + rdr, err := rc.BlobGet(ctx, r, l) + if err != nil { + return v1.Hash{}, normalizeErr(err) + } + _, copyErr := io.Copy(io.Discard, rdr) + closeErr := rdr.Close() + if copyErr != nil { + return v1.Hash{}, normalizeErr(copyErr) + } + if closeErr != nil { + return v1.Hash{}, normalizeErr(closeErr) + } + } + } return v1.Hash{Algorithm: desc.Digest.Algorithm().String(), Hex: desc.Digest.Encoded()}, nil } diff --git a/test/integration/clients_guard_test.go b/test/integration/clients_guard_test.go new file mode 100644 index 0000000..eaa169c --- /dev/null +++ b/test/integration/clients_guard_test.go @@ -0,0 +1,121 @@ +//go:build integration + +package integration + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + + "atcr.io/internal/testharness" +) + +// TestPullClientsReadBlobs guards the guard. +// +// craneClient.Pull deliberately materializes the layer bytes after crane.Pull, +// because crane is lazy: crane.Pull plus img.Digest() fetches only the +// manifest, and ATCR serves manifests from the user's PDS where they are +// world-readable by design. A "pull" that stops at the manifest never asks the +// hold for a blob, so it never exercises blob authorization at all. +// +// That loop is the only reason the pull rows of the auth matrix test anything +// about who may read blobs — and nothing was protecting it. Deleting it leaves +// the entire integration suite green, including TestAuthMatrixPrivateHold, +// which still passes because the manifest is refused too. The blob half simply +// stops being covered, silently. +// +// This asserts the property directly, against a registry that serves the +// manifest happily and refuses the blob. A client that never reads blobs +// returns success here and fails this test. +func TestPullClientsReadBlobs(t *testing.T) { + for _, c := range Clients { + t.Run(c.Name(), func(t *testing.T) { + var blobRequested atomic.Bool + + // Minimal registry: /v2/ ping, a real manifest, and a blob endpoint + // that always refuses. The config blob is served so nothing fails + // for an unrelated reason — only the layer is forbidden. + layerBody := []byte("layer-bytes-that-must-be-fetched") + layerDigest := digestOf(layerBody) + configBody := []byte(`{"architecture":"amd64","os":"linux","rootfs":{"type":"layers","diff_ids":[]},"config":{}}`) + configDigest := digestOf(configBody) + + manifest := map[string]any{ + "schemaVersion": 2, + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "config": map[string]any{ + "mediaType": "application/vnd.oci.image.config.v1+json", + "digest": configDigest, + "size": len(configBody), + }, + "layers": []map[string]any{{ + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "digest": layerDigest, + "size": len(layerBody), + }}, + } + manifestJSON, err := json.Marshal(manifest) + if err != nil { + t.Fatalf("marshal manifest: %v", err) + } + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/v2/": + w.WriteHeader(http.StatusOK) + + case strings.Contains(r.URL.Path, "/manifests/"): + w.Header().Set("Content-Type", "application/vnd.oci.image.manifest.v1+json") + w.Header().Set("Docker-Content-Digest", digestOf(manifestJSON)) + if r.Method == http.MethodHead { + w.Header().Set("Content-Length", fmt.Sprint(len(manifestJSON))) + w.WriteHeader(http.StatusOK) + return + } + _, _ = w.Write(manifestJSON) + + case strings.HasSuffix(r.URL.Path, "/blobs/"+configDigest): + _, _ = w.Write(configBody) + + case strings.Contains(r.URL.Path, "/blobs/"): + // The layer, and only the layer, is refused. This is the + // hold saying no to an unauthorized reader. + blobRequested.Store(true) + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"errors":[{"code":"DENIED","message":"blob read denied"}]}`)) + + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer srv.Close() + + ref := strings.TrimPrefix(srv.URL, "http://") + "/alice.test/guard:v1" + _, err = c.Pull(context.Background(), ref, testharness.Auth{}) + + t.Logf("%s.Pull returned err=%v (blobRequested=%v)", c.Name(), err, blobRequested.Load()) + + if !blobRequested.Load() { + t.Fatalf("%s.Pull never requested a layer blob — it stops at the manifest, "+ + "so every pull row in the auth matrix is passing without exercising blob "+ + "authorization. Restore the layer materialization in clients.go.", c.Name()) + } + if err == nil { + t.Errorf("%s.Pull succeeded against a registry that refuses the layer blob; "+ + "the refusal must surface as an error", c.Name()) + } + }) + } +} + +func digestOf(b []byte) string { + sum := sha256.Sum256(b) + return "sha256:" + hex.EncodeToString(sum[:]) +}