diff --git a/pkg/appview/authgate/anonymous_authorizer.go b/pkg/appview/authgate/anonymous_authorizer.go new file mode 100644 index 0000000..3f5097b --- /dev/null +++ b/pkg/appview/authgate/anonymous_authorizer.go @@ -0,0 +1,179 @@ +package authgate + +import ( + "context" + "database/sql" + "errors" + "fmt" + "log/slog" + "strings" + + "atcr.io/pkg/atproto" + "atcr.io/pkg/auth" + "atcr.io/pkg/auth/token" +) + +// AnonymousAuthorizer decides which repositories of a credential-less pull +// request /auth/token will actually sign a token for. +// +// It answers the same question the registry middleware asks before serving an +// anonymous request (CheckReadAccess against the owner's hold, which for an +// identity-less reader reduces to captain.Public), just earlier and from local +// state. Without it the two layers disagree: /auth/token would sign `pull` on a +// private-hold repository and /v2/ would refuse that very token. +// +// Every lookup is local. hold_captain_records is Jetstream-fed and already read +// sub-millisecond by the push gate; the one new dependency on this path is +// handle -> DID resolution, which the identity directory caches for 24h. +// +// Every failure mode returns "allow". /v2/ remains the enforcing layer, so +// failing open costs nothing but keeps a DNS blip or a cold captain cache from +// refusing tokens for public images. +type AnonymousAuthorizer struct { + holdResolver + + // resolveOwnerDID maps a repository's identity component (a handle or a + // DID) to a DID. A field rather than a direct call so tests can drive the + // gate without a live identity directory; production wiring installs the + // cached atproto.ResolveIdentity in NewAnonymousAuthorizer. + resolveOwnerDID func(ctx context.Context, identity string) (string, error) +} + +var _ token.AnonymousAuthorizer = (*AnonymousAuthorizer)(nil) + +// NewAnonymousAuthorizer constructs the credential-less pull gate. +// defaultHoldDID is the AppView's fallback hold, used when the owner's cached +// sailor profile has not recorded one. +func NewAnonymousAuthorizer(db *sql.DB, defaultHoldDID string) *AnonymousAuthorizer { + return &AnonymousAuthorizer{ + holdResolver: holdResolver{db: db, defaultHoldDID: defaultHoldDID}, + resolveOwnerDID: func(ctx context.Context, identity string) (string, error) { + did, _, _, err := atproto.ResolveIdentity(ctx, identity) + return did, err + }, + } +} + +// AuthorizeAnonymous satisfies token.AnonymousAuthorizer: it returns the subset +// of access an anonymous reader may have. Entries are dropped, never widened +// and never edited, so what survives is exactly what was asked for. +// +// A request naming several repositories can name several owners and therefore +// several holds, so the verdict is per entry: the public ones are kept and the +// private ones dropped, rather than failing the whole request. +func (a *AnonymousAuthorizer) AuthorizeAnonymous(ctx context.Context, access []auth.AccessEntry) []auth.AccessEntry { + kept := make([]auth.AccessEntry, 0, len(access)) + for _, entry := range access { + if a.allowEntry(ctx, entry) { + kept = append(kept, entry) + } + } + return kept +} + +// allowEntry reports whether one access entry survives the gate. +func (a *AnonymousAuthorizer) allowEntry(ctx context.Context, entry auth.AccessEntry) bool { + // Grants nothing, so there is nothing to deny. This is the actionless entry + // NarrowToPullOnly keeps on purpose; dropping it would break callers that + // rely on it surviving. + if len(entry.Actions) == 0 { + return true + } + + // Only a repository scope names an owner whose hold can be checked. + // Anything else (registry:catalog:*, say) is not hold-gated, so this gate + // has no opinion and leaves it as it was. + if entry.Type != "repository" { + return true + } + + identity, _, found := strings.Cut(entry.Name, "/") + if !found || identity == "" { + // Malformed name with no owner component. /v2/ answers NAME_INVALID for + // it; that is not this gate's verdict to pre-empt. + return true + } + + // OCI reference grammar forbids colons in path components, so DIDs arrive + // hyphen-encoded — the same decode the registry middleware performs. + if decoded, ok := auth.DecodeDIDFromHyphens(identity); ok { + identity = decoded + } + + ownerDID, err := a.resolveOwnerDID(ctx, identity) + if err != nil { + slog.Warn("anonymous gate: identity resolution failed, deferring to /v2/", + "identity", identity, "repository", entry.Name, "error", err) + return true + } + + holdDID, err := a.resolveHoldDID(ctx, ownerDID) + if err != nil { + slog.Warn("anonymous gate: hold resolution failed, deferring to /v2/", + "did", ownerDID, "repository", entry.Name, "error", err) + return true + } + if holdDID == "" { + // No hold configured anywhere. /v2/ skips its own check in exactly this + // case (it requires a non-empty holdDID), so skip it here too. + return true + } + + public, err := a.holdAllowsAnonymousRead(ctx, holdDID) + if err != nil { + slog.Warn("anonymous gate: captain lookup failed, deferring to /v2/", + "hold_did", holdDID, "repository", entry.Name, "error", err) + return true + } + if !public { + slog.Debug("anonymous gate: dropping scope, hold denies anonymous reads", + "hold_did", holdDID, "repository", entry.Name) + } + return public +} + +// holdAllowsAnonymousRead reports whether holdDID admits a reader with no +// identity. For an empty user DID auth.CheckReadAccessWithCaptain reduces to +// captain.Public, so that single column is the whole decision. +// +// The successor hop matters: /v2/ resolves the hold, then applies a single-hop +// migration redirect (resolveSuccessor) before checking read access, so it +// judges the successor's captain record. Skipping that here would check a +// migrated hold under its old identity and reintroduce the disagreement this +// gate closes. Both reads hit the local Jetstream-fed table. +// +// A missing row is returned as an error, not as "private": an un-ingested hold +// is an unknown, and the contract on this path is to fail open. +func (a *AnonymousAuthorizer) holdAllowsAnonymousRead(ctx context.Context, holdDID string) (bool, error) { + public, successor, err := a.captainRow(ctx, holdDID) + if err != nil { + return false, err + } + if successor == "" { + return public, nil + } + + // Single hop only, matching resolveSuccessor: a successor's own successor + // is not followed. + successorPublic, _, err := a.captainRow(ctx, successor) + if err != nil { + return false, fmt.Errorf("successor of %s: %w", holdDID, err) + } + return successorPublic, nil +} + +// captainRow reads the two fields of the cached captain record this gate needs. +func (a *AnonymousAuthorizer) captainRow(ctx context.Context, holdDID string) (bool, string, error) { + var public bool + var successor sql.NullString + err := a.db.QueryRowContext(ctx, + "SELECT public, successor FROM hold_captain_records WHERE hold_did = ?", holdDID, + ).Scan(&public, &successor) + if errors.Is(err, sql.ErrNoRows) { + return false, "", fmt.Errorf("no cached captain record for hold %s", holdDID) + } + if err != nil { + return false, "", fmt.Errorf("look up hold captain %s: %w", holdDID, err) + } + return public, successor.String, nil +} diff --git a/pkg/appview/authgate/anonymous_authorizer_test.go b/pkg/appview/authgate/anonymous_authorizer_test.go new file mode 100644 index 0000000..0c1ddb6 --- /dev/null +++ b/pkg/appview/authgate/anonymous_authorizer_test.go @@ -0,0 +1,299 @@ +package authgate + +import ( + "context" + "database/sql" + "fmt" + "testing" + + "atcr.io/pkg/appview/db" + "atcr.io/pkg/auth" +) + +// seedCaptainRecord inserts a hold_captain_records row with full control over +// public and successor, which seedCaptain (private, no successor) does not give. +func seedCaptainRecord(t *testing.T, d *sql.DB, rec db.HoldCaptainRecord) { + t.Helper() + if err := db.BatchUpsertCaptainRecords(d, []db.HoldCaptainRecord{rec}); err != nil { + t.Fatalf("BatchUpsertCaptainRecords(%s): %v", rec.HoldDID, err) + } +} + +// newAnonGate builds an AnonymousAuthorizer whose identity resolution is a +// static map, so the tests stay hermetic (the production resolver would issue +// live DNS/HTTPS lookups). +func newAnonGate(t *testing.T, d *sql.DB, defaultHoldDID string, identities map[string]string) *AnonymousAuthorizer { + t.Helper() + a := NewAnonymousAuthorizer(d, defaultHoldDID) + a.resolveOwnerDID = func(_ context.Context, identity string) (string, error) { + if did, ok := identities[identity]; ok { + return did, nil + } + return "", fmt.Errorf("no such identity: %s", identity) + } + return a +} + +func pullEntry(name string) auth.AccessEntry { + return auth.AccessEntry{Type: "repository", Name: name, Actions: []string{"pull"}} +} + +func names(access []auth.AccessEntry) []string { + out := make([]string, 0, len(access)) + for _, e := range access { + out = append(out, e.Name) + } + return out +} + +// A public hold admits an identity-less reader, which is exactly what +// CheckReadAccessWithCaptain concludes at /v2/ for an empty user DID. +func TestAnonymousAuthorizer_PublicHoldKeepsScope(t *testing.T) { + d := newTestDB(t) + seedUser(t, d, "did:plc:alice", "alice.test", "did:web:public.hold") + seedCaptainRecord(t, d, db.HoldCaptainRecord{ + HoldDID: "did:web:public.hold", OwnerDID: "did:plc:alice", Public: true, + }) + + gate := newAnonGate(t, d, "", map[string]string{"alice.test": "did:plc:alice"}) + got := gate.AuthorizeAnonymous(context.Background(), []auth.AccessEntry{pullEntry("alice.test/app")}) + + if len(got) != 1 { + t.Fatalf("public hold must keep the scope, got %v", names(got)) + } +} + +// The defect: a private hold refuses the token at /v2/, so /auth/token must not +// sign it in the first place. +func TestAnonymousAuthorizer_PrivateHoldDropsScope(t *testing.T) { + d := newTestDB(t) + seedUser(t, d, "did:plc:alice", "alice.test", "did:web:private.hold") + seedCaptainRecord(t, d, db.HoldCaptainRecord{ + HoldDID: "did:web:private.hold", OwnerDID: "did:plc:alice", Public: false, + }) + + gate := newAnonGate(t, d, "", map[string]string{"alice.test": "did:plc:alice"}) + got := gate.AuthorizeAnonymous(context.Background(), []auth.AccessEntry{pullEntry("alice.test/app")}) + + if len(got) != 0 { + t.Fatalf("private hold must drop the scope, got %v", names(got)) + } +} + +// Different owners mean different holds, so the verdict is per entry. +func TestAnonymousAuthorizer_MixedOwnersNarrowsToThePublicOne(t *testing.T) { + d := newTestDB(t) + seedUser(t, d, "did:plc:alice", "alice.test", "did:web:public.hold") + seedUser(t, d, "did:plc:bob", "bob.test", "did:web:private.hold") + seedCaptainRecord(t, d, db.HoldCaptainRecord{ + HoldDID: "did:web:public.hold", OwnerDID: "did:plc:alice", Public: true, + }) + seedCaptainRecord(t, d, db.HoldCaptainRecord{ + HoldDID: "did:web:private.hold", OwnerDID: "did:plc:bob", Public: false, + }) + + gate := newAnonGate(t, d, "", map[string]string{ + "alice.test": "did:plc:alice", + "bob.test": "did:plc:bob", + }) + got := gate.AuthorizeAnonymous(context.Background(), []auth.AccessEntry{ + pullEntry("alice.test/app"), + pullEntry("bob.test/app"), + }) + + if len(got) != 1 || got[0].Name != "alice.test/app" { + t.Fatalf("expected only alice.test/app to survive, got %v", names(got)) + } +} + +// A DID owner arrives hyphen-encoded, because OCI reference grammar forbids +// colons in path components. The registry middleware decodes it; so must this. +func TestAnonymousAuthorizer_DecodesHyphenEncodedDIDOwner(t *testing.T) { + d := newTestDB(t) + seedUser(t, d, "did:plc:alice", "alice.test", "did:web:private.hold") + seedCaptainRecord(t, d, db.HoldCaptainRecord{ + HoldDID: "did:web:private.hold", OwnerDID: "did:plc:alice", Public: false, + }) + + gate := newAnonGate(t, d, "", map[string]string{"did:plc:alice": "did:plc:alice"}) + got := gate.AuthorizeAnonymous(context.Background(), []auth.AccessEntry{pullEntry("did-plc-alice/app")}) + + if len(got) != 0 { + t.Fatalf("hyphen-encoded DID owner must resolve to the same private hold, got %v", names(got)) + } +} + +// /v2/ applies a single-hop successor redirect before checking read access, so +// a migrated hold is judged by its successor's captain record. Checking the old +// identity here would reintroduce the disagreement this gate closes. +func TestAnonymousAuthorizer_FollowsSuccessorToPrivate(t *testing.T) { + d := newTestDB(t) + seedUser(t, d, "did:plc:alice", "alice.test", "did:web:old.hold") + seedCaptainRecord(t, d, db.HoldCaptainRecord{ + HoldDID: "did:web:old.hold", OwnerDID: "did:plc:alice", + Public: true, Successor: "did:web:new.hold", + }) + seedCaptainRecord(t, d, db.HoldCaptainRecord{ + HoldDID: "did:web:new.hold", OwnerDID: "did:plc:alice", Public: false, + }) + + gate := newAnonGate(t, d, "", map[string]string{"alice.test": "did:plc:alice"}) + got := gate.AuthorizeAnonymous(context.Background(), []auth.AccessEntry{pullEntry("alice.test/app")}) + + if len(got) != 0 { + t.Fatalf("the successor is private, so the scope must be dropped, got %v", names(got)) + } +} + +// The mirror image: the old hold is private, the successor public. Blobs go to +// the successor, so the successor's answer is the one that counts. +func TestAnonymousAuthorizer_FollowsSuccessorToPublic(t *testing.T) { + d := newTestDB(t) + seedUser(t, d, "did:plc:alice", "alice.test", "did:web:old.hold") + seedCaptainRecord(t, d, db.HoldCaptainRecord{ + HoldDID: "did:web:old.hold", OwnerDID: "did:plc:alice", + Public: false, Successor: "did:web:new.hold", + }) + seedCaptainRecord(t, d, db.HoldCaptainRecord{ + HoldDID: "did:web:new.hold", OwnerDID: "did:plc:alice", Public: true, + }) + + gate := newAnonGate(t, d, "", map[string]string{"alice.test": "did:plc:alice"}) + got := gate.AuthorizeAnonymous(context.Background(), []auth.AccessEntry{pullEntry("alice.test/app")}) + + if len(got) != 1 { + t.Fatalf("the successor is public, so the scope must survive, got %v", names(got)) + } +} + +// Single hop only, matching resolveSuccessor: the second hop is not followed, +// so the first successor's own record decides. +func TestAnonymousAuthorizer_SuccessorChainIsNotFollowed(t *testing.T) { + d := newTestDB(t) + seedUser(t, d, "did:plc:alice", "alice.test", "did:web:hop0.hold") + seedCaptainRecord(t, d, db.HoldCaptainRecord{ + HoldDID: "did:web:hop0.hold", OwnerDID: "did:plc:alice", + Public: false, Successor: "did:web:hop1.hold", + }) + seedCaptainRecord(t, d, db.HoldCaptainRecord{ + HoldDID: "did:web:hop1.hold", OwnerDID: "did:plc:alice", + Public: true, Successor: "did:web:hop2.hold", + }) + seedCaptainRecord(t, d, db.HoldCaptainRecord{ + HoldDID: "did:web:hop2.hold", OwnerDID: "did:plc:alice", Public: false, + }) + + gate := newAnonGate(t, d, "", map[string]string{"alice.test": "did:plc:alice"}) + got := gate.AuthorizeAnonymous(context.Background(), []auth.AccessEntry{pullEntry("alice.test/app")}) + + if len(got) != 1 { + t.Fatalf("only one hop is followed, so hop1's public flag decides, got %v", names(got)) + } +} + +// Fail open on an unresolvable identity: /v2/ still enforces, and a DNS blip +// must not start refusing tokens for public images. +func TestAnonymousAuthorizer_IdentityErrorFailsOpen(t *testing.T) { + d := newTestDB(t) + gate := newAnonGate(t, d, "", nil) // every lookup errors + + got := gate.AuthorizeAnonymous(context.Background(), []auth.AccessEntry{pullEntry("nobody.test/app")}) + if len(got) != 1 { + t.Fatalf("an identity resolution failure must fail open, got %v", names(got)) + } +} + +// A hold with no cached captain record is an unknown, not a private hold. +func TestAnonymousAuthorizer_MissingCaptainRecordFailsOpen(t *testing.T) { + d := newTestDB(t) + seedUser(t, d, "did:plc:alice", "alice.test", "did:web:uningested.hold") + + gate := newAnonGate(t, d, "", map[string]string{"alice.test": "did:plc:alice"}) + got := gate.AuthorizeAnonymous(context.Background(), []auth.AccessEntry{pullEntry("alice.test/app")}) + + if len(got) != 1 { + t.Fatalf("a cold captain cache must fail open, got %v", names(got)) + } +} + +// A successor whose own record has not been ingested is the same unknown. +func TestAnonymousAuthorizer_MissingSuccessorRecordFailsOpen(t *testing.T) { + d := newTestDB(t) + seedUser(t, d, "did:plc:alice", "alice.test", "did:web:old.hold") + seedCaptainRecord(t, d, db.HoldCaptainRecord{ + HoldDID: "did:web:old.hold", OwnerDID: "did:plc:alice", + Public: false, Successor: "did:web:unknown.hold", + }) + + gate := newAnonGate(t, d, "", map[string]string{"alice.test": "did:plc:alice"}) + got := gate.AuthorizeAnonymous(context.Background(), []auth.AccessEntry{pullEntry("alice.test/app")}) + + if len(got) != 1 { + t.Fatalf("an unknown successor must fail open, got %v", names(got)) + } +} + +// A DB error is a lookup failure like any other. +func TestAnonymousAuthorizer_DBErrorFailsOpen(t *testing.T) { + d := newTestDB(t) + _ = d.Close() + + gate := newAnonGate(t, d, "", map[string]string{"alice.test": "did:plc:alice"}) + got := gate.AuthorizeAnonymous(context.Background(), []auth.AccessEntry{pullEntry("alice.test/app")}) + + if len(got) != 1 { + t.Fatalf("a DB error must fail open, got %v", names(got)) + } +} + +// No hold configured anywhere: /v2/ skips its own check in exactly this case +// (it requires a non-empty hold DID), so this gate must skip it too. +func TestAnonymousAuthorizer_NoHoldConfiguredKeepsScope(t *testing.T) { + d := newTestDB(t) + seedUser(t, d, "did:plc:alice", "alice.test", "") + + gate := newAnonGate(t, d, "", map[string]string{"alice.test": "did:plc:alice"}) + got := gate.AuthorizeAnonymous(context.Background(), []auth.AccessEntry{pullEntry("alice.test/app")}) + + if len(got) != 1 { + t.Fatalf("no hold means no verdict to pre-empt, got %v", names(got)) + } +} + +// The AppView default hold covers users whose cached profile has no defaultHold. +func TestAnonymousAuthorizer_FallsBackToDefaultHold(t *testing.T) { + d := newTestDB(t) + seedUser(t, d, "did:plc:alice", "alice.test", "") + seedCaptainRecord(t, d, db.HoldCaptainRecord{ + HoldDID: "did:web:default.hold", OwnerDID: "did:plc:operator", Public: false, + }) + + gate := newAnonGate(t, d, "did:web:default.hold", map[string]string{"alice.test": "did:plc:alice"}) + got := gate.AuthorizeAnonymous(context.Background(), []auth.AccessEntry{pullEntry("alice.test/app")}) + + if len(got) != 0 { + t.Fatalf("the fallback hold's captain record must decide, got %v", names(got)) + } +} + +// Entries that grant nothing, and scopes that name no hold, are passed through +// without a lookup — they are not denials. +func TestAnonymousAuthorizer_NonRepositoryAndActionlessEntriesPassThrough(t *testing.T) { + d := newTestDB(t) + gate := newAnonGate(t, d, "", nil) + gate.resolveOwnerDID = func(_ context.Context, identity string) (string, error) { + t.Errorf("no identity resolution should happen, but %q was resolved", identity) + return "", nil + } + + in := []auth.AccessEntry{ + {Type: "repository", Name: "alice.test/app"}, // no actions + {Type: "registry", Name: "catalog", Actions: []string{"pull"}}, // not hold-gated + {Type: "repository", Name: "noslash", Actions: []string{"pull"}}, // malformed name + } + got := gate.AuthorizeAnonymous(context.Background(), in) + + if len(got) != len(in) { + t.Fatalf("expected all %d entries to pass through, got %v", len(in), names(got)) + } +} diff --git a/pkg/appview/server.go b/pkg/appview/server.go index f14a24d..69d6038 100644 --- a/pkg/appview/server.go +++ b/pkg/appview/server.go @@ -637,6 +637,11 @@ func NewAppViewServer(cfg *Config, branding *BrandingOverrides) (*AppViewServer, // re-check on every blob. tokenHandler.SetAuthorizer(authgate.New(s.Database, s.HoldAuthorizer, s.Refresher, defaultHoldDID)) + // Credential-less pull gate: drop the scopes whose hold denies + // anonymous reads, so /auth/token stops signing pull tokens that + // /v2/ then refuses. Local reads only; fails open. + tokenHandler.SetAnonymousAuthorizer(authgate.NewAnonymousAuthorizer(s.Database, defaultHoldDID)) + // Bind the registry JWT lifetime to the AppView↔hold service-auth. // Pre-minting the service-auth here lets us stamp the JWT's exp // from the cached expiry, so both expire concurrently. diff --git a/pkg/auth/token/handler.go b/pkg/auth/token/handler.go index 1d99603..130dd07 100644 --- a/pkg/auth/token/handler.go +++ b/pkg/auth/token/handler.go @@ -58,6 +58,29 @@ type Authorizer interface { Authorize(ctx context.Context, did, authMethod string, access []auth.AccessEntry) error } +// AnonymousAuthorizer gates the credential-less pull path, answering which of +// the requested repositories this AppView is actually willing to sign a pull +// token for. +// +// It exists because /auth/token and /v2/ are the authorization server and the +// resource server for the same request, and they used to disagree: the token +// endpoint minted `pull` on any repository, while the registry refused that +// same token when the owner's hold denies anonymous reads. Nothing was exposed +// (the hold gates the bytes, and the registry middleware still enforces), but a +// signed grant the issuer knows will be refused is a lie the client cannot act +// on. Deciding here makes docker see one 401 and prompt for credentials. +// +// Implementations must follow the same narrowing contract as Authorizer: return +// a subset of `access`, never a superset, and never add actions to an entry. +// Entries that grant nothing (the actionless placeholder NarrowToPullOnly +// preserves) must be passed through rather than treated as denials. +// Implementations fail open — a lookup failure returns the entry, since /v2/ +// remains the enforcing layer and a transient error must not break anonymous +// pulls of public images. +type AnonymousAuthorizer interface { + AuthorizeAnonymous(ctx context.Context, access []auth.AccessEntry) []auth.AccessEntry +} + // ServiceAuthFetcher pre-mints the AppView↔hold service-auth at /auth/token // time so the registry JWT can be bound to its lifetime. JWT and service-auth // then expire concurrently; when Docker hits 401, the next /auth/token call @@ -77,6 +100,7 @@ type Handler struct { postAuthCallback PostAuthCallback oauthSessionValidator OAuthSessionValidator authorizer Authorizer + anonymousAuthorizer AnonymousAuthorizer serviceAuthFetcher ServiceAuthFetcher // services is the set of registry domains this AppView fronts, keyed by @@ -114,6 +138,15 @@ func (h *Handler) SetAuthorizer(authorizer Authorizer) { h.authorizer = authorizer } +// SetAnonymousAuthorizer wires the credential-less pull gate. When set, an +// anonymous token request is narrowed to the repositories the gate is willing +// to grant, and a request where nothing grantable survives gets the standard +// 401 challenge instead of a token the registry would refuse. Unset leaves the +// previous behavior: mint pull for whatever was asked and let /v2/ decide. +func (h *Handler) SetAnonymousAuthorizer(authorizer AnonymousAuthorizer) { + h.anonymousAuthorizer = authorizer +} + // SetServiceAuthFetcher binds JWT issuance to the AppView↔hold service-auth. // When set, the handler pre-mints the service-auth and stamps the JWT's exp // from the cached expiry, so both tokens expire concurrently. @@ -345,13 +378,21 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // so a push or delete still requires credentials. A scope with no pull // component at all gets the standard challenge. // - // Whether an anonymous reader is actually admitted is not decided here. The - // hold owns that call via captain.Public, and the appview enforces the same - // answer from its local captain cache before serving anything. Minting a - // pull-only token is not a grant. + // The hold still owns the real decision via captain.Public, and /v2/ still + // enforces it from the local captain cache before serving anything. But the + // same answer is available here, so we ask for it: signing `pull` on a + // repository whose hold denies anonymous reads produces a token the registry + // then refuses, which is the authorization server contradicting the resource + // server. gateAnonymous drops the entries that would be refused, and a + // request with nothing grantable left falls through to the challenge below. if username == "" { if pullAccess, ok := NarrowToPullOnly(access); ok { - h.issueAnonymousToken(w, r, pullAccess, service) + if granted, allowed := h.gateAnonymous(r.Context(), pullAccess); allowed { + h.issueAnonymousToken(w, r, granted, service) + return + } + slog.Debug("Anonymous pull denied: no requested repository admits anonymous reads") + sendAuthError(w, r, "authentication required") return } slog.Debug("No Basic auth credentials provided") @@ -587,6 +628,47 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { render.JSON(w, r, resp) } +// gateAnonymous asks the anonymous authorizer which of an already-narrowed +// pull scope this AppView will sign, and reports whether a token is still worth +// issuing. Returns the surviving access on true; on false the caller must send +// the 401 challenge rather than a token with nothing in it — an empty-access +// token lets the client proceed to /v2/ and collect its 401 there, which is a +// smaller version of the disagreement this gate exists to remove, and the +// challenge is what makes docker prompt for credentials. +func (h *Handler) gateAnonymous(ctx context.Context, access []auth.AccessEntry) ([]auth.AccessEntry, bool) { + if h.anonymousAuthorizer == nil { + return access, true + } + + // Nothing in the request grants anything: either the /v2/ ping (empty + // access) or the actionless entry NarrowToPullOnly preserves on purpose. + // Neither names a hold to check and both must keep working, so they are + // returned untouched — no identity resolution, no hold lookup, no cost on + // the discovery path. + if countGranting(access) == 0 { + return access, true + } + + granted := h.anonymousAuthorizer.AuthorizeAnonymous(ctx, access) + if countGranting(granted) == 0 { + return nil, false + } + return granted, true +} + +// countGranting counts the entries that actually authorize something. After +// NarrowToPullOnly every such entry carries exactly ["pull"]; the rest are the +// deliberately preserved placeholders that grant nothing. +func countGranting(access []auth.AccessEntry) int { + n := 0 + for _, entry := range access { + if len(entry.Actions) > 0 { + n++ + } + } + return n +} + // issueAnonymousToken mints a credential-less registry JWT for a pull-only // scope. The token carries an empty Subject (no puller DID) and the anonymous // auth method. It deliberately skips the authorizer gate and service-auth diff --git a/pkg/auth/token/handler_anonymous_gate_test.go b/pkg/auth/token/handler_anonymous_gate_test.go new file mode 100644 index 0000000..0f09083 --- /dev/null +++ b/pkg/auth/token/handler_anonymous_gate_test.go @@ -0,0 +1,242 @@ +package token + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "slices" + "testing" + "time" + + "atcr.io/pkg/auth" +) + +// stubAnonymousAuthorizer keeps the entries whose repository name appears in +// public, drops the rest, and records what it was asked. Entries that grant +// nothing pass through, mirroring the contract the real gate follows. +type stubAnonymousAuthorizer struct { + public []string + calls int + saw []auth.AccessEntry +} + +func (s *stubAnonymousAuthorizer) AuthorizeAnonymous(_ context.Context, access []auth.AccessEntry) []auth.AccessEntry { + s.calls++ + s.saw = append(s.saw, access...) + + kept := make([]auth.AccessEntry, 0, len(access)) + for _, entry := range access { + if len(entry.Actions) == 0 || slices.Contains(s.public, entry.Name) { + kept = append(kept, entry) + } + } + return kept +} + +// passthroughAnonymousAuthorizer is the fail-open shape: whatever comes in +// comes back out. This is what the real gate returns when a hold lookup errors. +type passthroughAnonymousAuthorizer struct{ calls int } + +func (p *passthroughAnonymousAuthorizer) AuthorizeAnonymous(_ context.Context, access []auth.AccessEntry) []auth.AccessEntry { + p.calls++ + return access +} + +func newGateTestHandler(t *testing.T, gate AnonymousAuthorizer) *Handler { + t.Helper() + issuer, err := NewIssuer(getSharedTestKey(t), "atcr.io", "registry", 15*time.Minute) + if err != nil { + t.Fatalf("NewIssuer() error = %v", err) + } + h := NewHandler(issuer, nil) + h.SetAnonymousAuthorizer(gate) + return h +} + +// anonymousGet drives the credential-less GET form and returns the recorder. +func anonymousGet(t *testing.T, h *Handler, query string) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequest(http.MethodGet, "/auth/token?"+query, nil) + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + return w +} + +func decodeToken(t *testing.T, w *httptest.ResponseRecorder) string { + t.Helper() + var resp TokenResponse + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("decode token response: %v", err) + } + if resp.Token == "" { + t.Fatal("expected a non-empty token") + } + return resp.Token +} + +// A public hold is the case that must not regress: anonymous pull of a public +// image is the whole point of the credential-less path. +func TestHandler_AnonymousGate_PublicHoldStillGrantsPull(t *testing.T) { + gate := &stubAnonymousAuthorizer{public: []string{"alice.test/public-app"}} + h := newGateTestHandler(t, gate) + + w := anonymousGet(t, h, "service=registry&scope=repository:alice.test/public-app:pull") + if w.Code != http.StatusOK { + t.Fatalf("expected 200 for a public hold, got %d. Body: %s", w.Code, w.Body.String()) + } + + tok := decodeToken(t, w) + if am := ExtractAuthMethod(tok); am != AuthMethodAnonymous { + t.Errorf("expected auth method %q, got %q", AuthMethodAnonymous, am) + } + access := ExtractAccess(tok) + if len(access) != 1 || access[0].Name != "alice.test/public-app" { + t.Fatalf("expected pull access for alice.test/public-app, got %+v", access) + } + if !slices.Equal(access[0].Actions, []string{"pull"}) { + t.Errorf("expected actions [pull], got %v", access[0].Actions) + } + if gate.calls != 1 { + t.Errorf("expected the gate to be consulted once, got %d calls", gate.calls) + } +} + +// The defect this gate exists to fix: /auth/token used to sign a pull token for +// a repository whose hold denies anonymous reads, and /v2/ then refused that +// exact token. A 401 challenge here is what makes docker ask for credentials. +func TestHandler_AnonymousGate_PrivateHoldChallengesWithNoToken(t *testing.T) { + gate := &stubAnonymousAuthorizer{} // nothing is public + h := newGateTestHandler(t, gate) + + w := anonymousGet(t, h, "service=registry&scope=repository:alice.test/private-app:pull") + if w.Code != http.StatusUnauthorized { + t.Fatalf("expected 401 for a private hold, got %d. Body: %s", w.Code, w.Body.String()) + } + if w.Header().Get("WWW-Authenticate") == "" { + t.Error("expected a WWW-Authenticate challenge so docker prompts for credentials") + } + + // An empty-access token would be the smaller version of the same bug: the + // client proceeds to /v2/ and collects its 401 there instead. + var resp TokenResponse + if err := json.NewDecoder(w.Body).Decode(&resp); err == nil && resp.Token != "" { + t.Errorf("expected no token in the challenge body, got one: %q", resp.Token) + } +} + +// Scopes in one request can name different owners and therefore different +// holds, so the verdict is per entry: keep what is grantable, drop the rest. +func TestHandler_AnonymousGate_MixedScopeKeepsOnlyPublicEntries(t *testing.T) { + gate := &stubAnonymousAuthorizer{public: []string{"alice.test/public-app"}} + h := newGateTestHandler(t, gate) + + // Multiple repositories arrive as one space-separated scope value, which is + // the form this handler parses. + w := anonymousGet(t, h, + "service=registry&scope=repository:alice.test/public-app:pull%20repository:bob.test/private-app:pull") + if w.Code != http.StatusOK { + t.Fatalf("expected 200 when one scope survives, got %d. Body: %s", w.Code, w.Body.String()) + } + + access := ExtractAccess(decodeToken(t, w)) + if len(access) != 1 { + t.Fatalf("expected exactly the public entry to survive, got %+v", access) + } + if access[0].Name != "alice.test/public-app" { + t.Errorf("expected alice.test/public-app, got %q", access[0].Name) + } + + // Both entries must reach the gate: dropping one is its decision, not the + // handler's. + if len(gate.saw) != 2 { + t.Errorf("expected the gate to see both entries, saw %+v", gate.saw) + } +} + +// Anonymous discovery must not pay for the gate, and there is no hold to look +// up: the ping carries no scope at all. +func TestHandler_AnonymousGate_ScopelessPingSkipsTheGate(t *testing.T) { + gate := &stubAnonymousAuthorizer{} // would deny everything if consulted + h := newGateTestHandler(t, gate) + + w := anonymousGet(t, h, "service=registry") + if w.Code != http.StatusOK { + t.Fatalf("expected 200 for the /v2/ ping, got %d. Body: %s", w.Code, w.Body.String()) + } + if access := ExtractAccess(decodeToken(t, w)); len(access) != 0 { + t.Errorf("expected empty access on the ping token, got %+v", access) + } + if gate.calls != 0 { + t.Errorf("the ping must not trigger a hold lookup, gate was called %d times", gate.calls) + } +} + +// NarrowToPullOnly preserves an actionless entry deliberately ("callers rely on +// the entry surviving"). It grants nothing, so the gate must pass it through +// rather than read it as a denial, and it must not cost a hold lookup. +func TestHandler_AnonymousGate_ActionlessScopeSurvivesUnchecked(t *testing.T) { + gate := &stubAnonymousAuthorizer{public: []string{"alice.test/public-app"}} + h := newGateTestHandler(t, gate) + + w := anonymousGet(t, h, + "service=registry&scope=repository:alice.test/app:%20repository:alice.test/public-app:pull") + if w.Code != http.StatusOK { + t.Fatalf("expected 200 for an actionless scope alongside a public one, got %d. Body: %s", w.Code, w.Body.String()) + } + + access := ExtractAccess(decodeToken(t, w)) + if len(access) != 2 { + t.Fatalf("expected both entries to survive, got %+v", access) + } + var actionless *auth.AccessEntry + for i := range access { + if access[i].Name == "alice.test/app" { + actionless = &access[i] + } + } + if actionless == nil { + t.Fatalf("the actionless entry was dropped: %+v", access) + } + if len(actionless.Actions) != 0 { + t.Errorf("expected the actionless entry to survive unchanged, got actions %v", actionless.Actions) + } +} + +// The gate fails open on any lookup error, because /v2/ is still the enforcing +// layer and a DNS blip or a cold captain cache must not start refusing tokens +// for public images. The handler's job is to honour that verdict. +func TestHandler_AnonymousGate_LookupErrorFailsOpen(t *testing.T) { + gate := &passthroughAnonymousAuthorizer{} + h := newGateTestHandler(t, gate) + + w := anonymousGet(t, h, "service=registry&scope=repository:alice.test/app:pull") + if w.Code != http.StatusOK { + t.Fatalf("expected 200 when the gate fails open, got %d. Body: %s", w.Code, w.Body.String()) + } + access := ExtractAccess(decodeToken(t, w)) + if len(access) != 1 || access[0].Name != "alice.test/app" { + t.Errorf("expected the entry to survive a failed-open lookup, got %+v", access) + } + if gate.calls != 1 { + t.Errorf("expected the gate to be consulted once, got %d calls", gate.calls) + } +} + +// With no gate wired (a deployment that has not configured one), the path keeps +// its previous behavior rather than failing closed. +func TestHandler_AnonymousGate_UnsetAuthorizerGrantsAsBefore(t *testing.T) { + issuer, err := NewIssuer(getSharedTestKey(t), "atcr.io", "registry", 15*time.Minute) + if err != nil { + t.Fatalf("NewIssuer() error = %v", err) + } + h := NewHandler(issuer, nil) + + w := anonymousGet(t, h, "service=registry&scope=repository:alice.test/app:pull") + if w.Code != http.StatusOK { + t.Fatalf("expected 200 with no gate configured, got %d. Body: %s", w.Code, w.Body.String()) + } + if access := ExtractAccess(decodeToken(t, w)); len(access) != 1 { + t.Errorf("expected the requested entry to survive, got %+v", access) + } +}