From 2fe598633b4948b35a48442240f20c1835493b0a Mon Sep 17 00:00:00 2001 From: sec-check Date: Tue, 8 Sep 2026 02:39:14 -0400 Subject: [PATCH] [architect] refactor: delete dead mint HTTP server + caller-auth stack (pkg/mint/server.go, caller.go, tokenreview.go) The /mint HTTP transport built in #4436 (Server, handleMint, handleJWKS, SharedSecretAuthenticator, Entitlements, TokenReviewAuthenticator, MultiAuthenticator) was never wired to a listener: cmd/hive/main.go only constructs the in-process AgentMinter via mint.NewMinter/NewAgentMinter, and MintConfig has no listen-address, secret, or entitlement fields, so the stack is unreachable from any binary. golang.org/x/tools/cmd/deadcode confirms every symbol in these files is dead. Delete the three files, their four test files, and the package README that documented only the deleted front door; update src/docs/token-mint.md to point at git history instead. The live minter (mint.go, agent.go) is untouched; go build ./... and go test ./pkg/mint ./cmd/hive pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: sec-check --- src/docs/token-mint.md | 19 +- src/pkg/mint/README.md | 168 --------- src/pkg/mint/caller.go | 216 ----------- src/pkg/mint/caller_test.go | 276 -------------- src/pkg/mint/server.go | 234 ------------ src/pkg/mint/server_test.go | 271 ------------- src/pkg/mint/tokenreview.go | 430 --------------------- src/pkg/mint/tokenreview_loader_test.go | 107 ------ src/pkg/mint/tokenreview_test.go | 481 ------------------------ 9 files changed, 7 insertions(+), 2195 deletions(-) delete mode 100644 src/pkg/mint/README.md delete mode 100644 src/pkg/mint/caller.go delete mode 100644 src/pkg/mint/caller_test.go delete mode 100644 src/pkg/mint/server.go delete mode 100644 src/pkg/mint/server_test.go delete mode 100644 src/pkg/mint/tokenreview.go delete mode 100644 src/pkg/mint/tokenreview_loader_test.go delete mode 100644 src/pkg/mint/tokenreview_test.go diff --git a/src/docs/token-mint.md b/src/docs/token-mint.md index 7b213c6be..34edbab70 100644 --- a/src/docs/token-mint.md +++ b/src/docs/token-mint.md @@ -113,13 +113,10 @@ too — treat it as effectively immutable once anything trusts it. ADR-0007 and the mint package describe a `.well-known/jwks.json` endpoint that downstream WIF providers fetch to verify tokens (`Minter.JWKS`, `mint.go:237-254`). **As of this branch, nothing in the hive process serves -that endpoint.** `pkg/mint/README.md` states this explicitly: - -> `Server.Handler()` returns a handler; it does not listen. Nothing in this -> repo currently serves it — the mint is used in-process through -> `AgentMinter`. Whoever wires a listener owns the short-term hardening the -> finding asks for: bind it to localhost or a pod-internal interface, and do -> not expose `/mint` beyond the pod network. +that endpoint.** An HTTP transport for `/mint` + JWKS (with a caller-identity +seam, per-caller entitlements, and a Kubernetes TokenReview backend) was built +in #4436 but never wired to a listener, and has since been removed as dead +code — recover it from git history if an HTTP mint front door is ever wired. Confirmed by search: no route registers `jwks` or `well-known` anywhere under `pkg/dashboard/` or `cmd/hive/`. Today, `mint.enabled: true` only @@ -140,11 +137,9 @@ turning `mint.enabled: true` on: per agent on the same refresh cadence as its GitHub App token (`main.go:1747`, `agent.go:80-97`). There is no exposed HTTP endpoint for an external caller to request a mint token from this hive (see JWKS - section above) — the "front door" described in `pkg/mint/README.md` - (`SharedSecretAuthenticator` / `TokenReviewAuthenticator` / - `MultiAuthenticator`) is real code but is not wired into `cmd/hive/main.go` - in this branch. Do not assume caller-identity checking is active just - because it exists in the package. + section above) — the HTTP "front door" (`SharedSecretAuthenticator` / + `TokenReviewAuthenticator` / `MultiAuthenticator`) built in #4436 was + never wired into `cmd/hive/main.go` and has been deleted as dead code. 2. **What a token grants**: exactly the scopes for the subject's tier — never more (fail-closed on unknown tiers, `agent.go:53-58`) — for the fixed `hive-agent` audience, for at most `min(max_ttl_seconds, diff --git a/src/pkg/mint/README.md b/src/pkg/mint/README.md deleted file mode 100644 index beb874f61..000000000 --- a/src/pkg/mint/README.md +++ /dev/null @@ -1,168 +0,0 @@ -# `pkg/mint` — caller authentication on `POST /mint` - -Who is allowed to ask this mint for a token, and what may they ask for. - -For what the mint *issues* (claims, TTL clamping, JWKS, WIF exchange) see the -package doc comments in `mint.go` and `agent.go`. This file is only about the -front door. - -## The finding, and where it stands - -[#3915](https://github.com/hivecommons/hive/issues/3915): `/mint` was gated by a -shared bearer secret alone. A shared secret proves **trusted network position**, -not **who is calling** — and nothing bounded what a holder could ask for, so any -process that obtained it could mint a token for any subject, any audience and -any scope, limited only by the TTL ceiling. Mints were logged by subject and -audience, with no record of who requested them. - -| Gap | Status | -|---|---| -| No caller identity | `CallerAuthenticator` seam (`caller.go`) + `TokenReviewAuthenticator` (`tokenreview.go`) | -| Any holder may mint anything | `Entitlements`, deny-by-default per verified identity | -| No per-caller audit | the verified identity is logged on every mint and every refusal | -| mTLS backend for non-Kubernetes deployments | not implemented — same interface when it is | - -## Authenticating a caller - -Three implementations of one interface. `Server` does not know which is in use. - -```go -Authenticate(r *http.Request) (Identity, error) -``` - -### `SharedSecretAuthenticator` — the original gate - -Constant-time comparison against a bearer secret in `Authorization`. Still the -default, so behaviour is unchanged for anything already deployed. Every holder -gets the same identity, `shared-secret:any-holder`, which is deliberate: a -shared secret **cannot** distinguish its holders, and inventing a per-request -name would make the audit log claim an identity the mechanism never established. - -### `TokenReviewAuthenticator` — a real Kubernetes identity - -The caller presents its own projected ServiceAccount token; the mint asks the -API server to vouch for it and takes the answer, -`system:serviceaccount::`, as the identity. - -```go -auth, err := mint.NewInClusterTokenReviewAuthenticator("hive-mint") -srv, err := mint.NewServer(minter, secret, logger, - mint.WithAuthenticator(auth), - mint.WithEntitlements(entitlements), -) -``` - -Four properties are load-bearing. Removing any of them leaves the mint no better -off than the shared secret, while looking like it is: - -- **Audience scoping is checked on the response, not the request.** The review - asks for the mint's audience, but an API server whose authenticators do not - implement audience validation answers `authenticated: true` with the - `audiences` field *absent*. A backend that read only `authenticated` would - accept the API-server token every pod already has mounted at a well-known - path. The mint's audience must appear in the returned list; empty or absent is - a refusal. -- **A dedicated header**, `X-Hive-Mint-SA-Token`, never `Authorization`. In a - dual-accept deployment `Authorization` carries the shared secret, and - reviewing it would POST the mint's own secret to the API server as "a token to - please review" — writing it into someone else's audit log. -- **Real TLS** against the in-cluster CA bundle. There is no - insecure-skip-verify option, not even a documented one. -- **Fail closed.** A timeout, a response-size cap, and a refusal on every - infrastructure failure — unreachable API server, missing RBAC, unparseable - response. A TokenReview that failed open would be *worse* than the shared - secret, because operators would believe identity was being checked. - -Only `system:serviceaccount:` usernames are accepted. An entitlement map keyed -on ServiceAccount names must not be satisfiable by a human with a kubeconfig. - -### `MultiAuthenticator` — migrating without a flag day - -```go -auth, _ := mint.NewMultiAuthenticator(tokenReview, sharedSecret) -``` - -Tries backends in order and takes the first identity established. Put -TokenReview **first**: it reads its own header, so a caller presenting both is -recorded under its real identity rather than as `any-holder` — the difference -between an audit log that shows the migration finishing and one that shows -nothing changing. - -Rollout: enable both → watch the audit log until no `shared-secret:any-holder` -lines remain → drop the secret from the list. - -## Bounding what an identity may mint - -`Entitlements` maps a verified `Identity.Name` to what it may request. - -```go -mint.Entitlements{ - "system:serviceaccount:hive:hive-spoke": { - Subjects: []string{"spoke-alpha"}, - Audiences: []string{"//iam.googleapis.com/projects/123/locations/global/wif/providers/hive"}, - Scopes: []string{"registry:pull"}, - }, -} -``` - -- **An empty dimension allows nothing**, not everything. An entitlement that - grants an audience but forgets the scopes grants a token with no scopes. -- **`"*"` is the explicit wildcard.** A caller that genuinely mints arbitrary - subjects has to have that written down, not acquire it by omission. -- **An identity absent from a non-empty map may mint nothing.** -- **An empty map means entitlements are not configured** and the mint keeps its - historical unbounded behaviour. `NewServer` logs a warning saying so, because - an unbounded mint should be a visible choice rather than something discovered - from a token that should never have been issued. - -A caller that authenticates but is not entitled gets **403**, not 401 — it is -known, just not permitted. The reason is logged server-side and never returned. - -## Deploying the TokenReview backend - -**The mint needs permission to review tokens.** Without it every review returns -403 and every caller is refused (fail closed, but a confusing outage): - -```yaml -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: hive-mint-tokenreview -rules: - - apiGroups: ["authentication.k8s.io"] - resources: ["tokenreviews"] - verbs: ["create"] -``` - -…bound to the ServiceAccount the mint pod runs as. - -**Each caller needs a token projected for the mint's audience.** Not the default -mounted token — that one is for the API server, and the audience check above -will (correctly) refuse it: - -```yaml -volumes: - - name: mint-token - projected: - sources: - - serviceAccountToken: - path: mint-token - audience: hive-mint # must match the mint's configured audience - expirationSeconds: 3600 -``` - -Read `/var/run/secrets/mint/mint-token` and send it as `X-Hive-Mint-SA-Token`. -Re-read it per request rather than caching: projected tokens are rotated on -disk, which is also why the mint re-reads its own reviewer token each time -instead of loading it once at startup. - -## Network exposure - -`Server.Handler()` returns a handler; it does not listen. Nothing in this repo -currently serves it — the mint is used in-process through `AgentMinter`. Whoever -wires a listener owns the short-term hardening the finding asks for: bind it to -localhost or a pod-internal interface, and do not expose `/mint` beyond the pod -network. Caller authentication is a second line, not a substitute for it. - -`/.well-known/jwks.json` is public by design — it serves public keys, and WIF -providers must be able to reach it. diff --git a/src/pkg/mint/caller.go b/src/pkg/mint/caller.go deleted file mode 100644 index b98dece3a..000000000 --- a/src/pkg/mint/caller.go +++ /dev/null @@ -1,216 +0,0 @@ -package mint - -import ( - "crypto/subtle" - "fmt" - "net/http" - "sort" - "strings" -) - -// Caller authentication and entitlement for POST /mint (#3915). -// -// THE FINDING. /mint was gated by a shared bearer secret alone. A shared secret -// proves TRUSTED NETWORK POSITION, not WHO IS CALLING, and the mint placed no -// bound on what a holder could ask for: Minter.Mint validates only that subject -// and audience are non-empty and clamps the TTL, so any holder could mint a -// token for ANY subject, ANY audience and ANY scope — including a privileged -// hub subject — limited only by the TTL ceiling. Mints were logged by subject -// and audience with no record of who requested them. -// -// WHAT THIS FILE CHANGES, AND WHAT IT DELIBERATELY DOES NOT. -// -// Two of the three gaps are closed here without any new dependency: -// -// identity CallerAuthenticator is the seam the TODO(caller-auth) asked -// for. The shared secret becomes one implementation of it rather -// than the only mechanism, so a Kubernetes TokenReview or mTLS -// backend drops in behind the same interface without touching the -// handler. -// blast radius Entitlements bound what a VERIFIED identity may mint. Where an -// entitlement set is configured the mint is deny-by-default: an -// identity may only mint the subjects, audiences and scopes it is -// granted, so possession of a credential stops being permission to -// mint anything. -// audit the authenticated identity is logged on every mint and every -// refusal. -// -// The third gap — verifying a Kubernetes ServiceAccount token via TokenReview — -// is in tokenreview.go, added behind this interface with no change to the -// handler and no new module dependency. It was deferred here on the assumption -// that it needed k8s.io/client-go; it does not. TokenReview is one POST of a -// small, stable JSON object to authentication.k8s.io/v1, so the standard -// library covers it, and the maintainer decision about client-go is no longer -// in the way of closing the finding. -// -// A shared-secret caller therefore still exists, but it is now a MIGRATION -// STATE rather than the only option: run MultiAuthenticator with both, watch -// the audit log until every caller arrives as a ServiceAccount, then drop the -// secret from the list. - -// Identity is a verified caller of /mint. -// -// Kind names the mechanism that vouched for the caller, so an audit line says -// how the caller was established and not merely that it was. Name is the -// identity within that mechanism and is what Entitlements are keyed on. -type Identity struct { - // Kind is the authentication mechanism, e.g. "shared-secret" or - // "serviceaccount". - Kind string - // Name is the caller within that mechanism. For a shared secret every - // holder is indistinguishable, which is the point of the finding, so the - // name is the constant SharedSecretIdentityName. - Name string -} - -// String renders an identity for logs: "kind:name". -func (i Identity) String() string { - if i.Kind == "" && i.Name == "" { - return "unknown" - } - return i.Kind + ":" + i.Name -} - -// SharedSecretIdentityName is the Name every shared-secret caller gets. -// -// It is deliberately not a per-caller value: a shared secret CANNOT distinguish -// its holders, and inventing a name per request would make the audit log claim -// an identity the mechanism never established. An operator reading -// "shared-secret:any-holder" is being told exactly what was proven — that the -// caller held the secret — and nothing more. -const SharedSecretIdentityName = "any-holder" - -// KindSharedSecret is the Identity.Kind for shared-secret callers. -const KindSharedSecret = "shared-secret" - -// CallerAuthenticator establishes who is calling /mint. -// -// Implementations must be safe for concurrent use. An error means the caller is -// not authenticated; the handler answers 401 without distinguishing which of -// missing, malformed or wrong credential it was. -type CallerAuthenticator interface { - // Authenticate returns the verified identity of the request's caller. - Authenticate(r *http.Request) (Identity, error) - // Name identifies the mechanism for logs and errors. - Name() string -} - -// ErrUnauthenticated is returned by a CallerAuthenticator when the caller -// cannot be established. Callers of Authenticate must not surface the specific -// reason to the client. -var ErrUnauthenticated = fmt.Errorf("mint: caller not authenticated") - -// SharedSecretAuthenticator is the original gate, now behind the interface: a -// constant-time comparison against a shared bearer secret. -// -// It authenticates POSSESSION OF A SECRET, not a caller. Every holder gets the -// same identity. It remains the default so existing behaviour is byte-identical -// until an operator configures something stronger. -type SharedSecretAuthenticator struct { - secret string -} - -// NewSharedSecretAuthenticator builds the shared-secret gate. An empty secret -// is rejected — an empty secret would let anyone mint (fail closed). -func NewSharedSecretAuthenticator(secret string) (*SharedSecretAuthenticator, error) { - if secret == "" { - return nil, fmt.Errorf("mint: shared secret is required (fail closed)") - } - return &SharedSecretAuthenticator{secret: secret}, nil -} - -// Name implements CallerAuthenticator. -func (a *SharedSecretAuthenticator) Name() string { return KindSharedSecret } - -// Authenticate implements CallerAuthenticator. -func (a *SharedSecretAuthenticator) Authenticate(r *http.Request) (Identity, error) { - h := r.Header.Get("Authorization") - if !strings.HasPrefix(h, authScheme) { - return Identity{}, ErrUnauthenticated - } - presented := strings.TrimPrefix(h, authScheme) - // Constant-time compare; ConstantTimeCompare returns 0 on length mismatch - // without leaking which byte differed. - if subtle.ConstantTimeCompare([]byte(presented), []byte(a.secret)) != 1 { - return Identity{}, ErrUnauthenticated - } - return Identity{Kind: KindSharedSecret, Name: SharedSecretIdentityName}, nil -} - -// Entitlement is what one verified identity may mint. -// -// Every field is an allow-list and an EMPTY LIST MEANS NOTHING IS ALLOWED for -// that dimension, not everything. That is the deny-by-default the finding asks -// for: an entitlement that grants an audience but forgets the scopes grants a -// token with no scopes, never a token with every scope. -// -// A "*" entry is an explicit, greppable wildcard for that dimension — for a -// caller that genuinely mints arbitrary subjects, an operator has to write it -// down rather than get it by omission. -type Entitlement struct { - // Subjects the identity may mint tokens FOR. - Subjects []string - // Audiences the identity may mint tokens for. - Audiences []string - // Scopes the identity may request. A request for any scope outside this - // set is refused rather than silently reduced — a caller that believes it - // holds a scope it does not is a bug worth surfacing. - Scopes []string -} - -// Wildcard is the explicit "any value" entry inside an Entitlement dimension. -const Wildcard = "*" - -// Entitlements maps a verified Identity.Name to what it may mint. -// -// A nil or empty map means entitlements are NOT configured, and the mint keeps -// its historical behaviour of allowing any subject/audience/scope. That is the -// only backwards-compatible default, and NewServer logs a warning when it is -// the case so the posture is visible rather than assumed. Once the map is -// non-empty it is authoritative: an identity absent from it may mint nothing. -type Entitlements map[string]Entitlement - -// permits reports whether the identity may mint this request, and if not, why. -// The reason is for the server's own log; it is never returned to the client. -func (e Entitlements) permits(id Identity, subject, audience string, scopes []string) (bool, string) { - if len(e) == 0 { - return true, "" - } - ent, ok := e[id.Name] - if !ok { - return false, "identity has no entitlement entry" - } - if !allowed(ent.Subjects, subject) { - return false, "subject not entitled" - } - if !allowed(ent.Audiences, audience) { - return false, "audience not entitled" - } - for _, s := range scopes { - if !allowed(ent.Scopes, s) { - return false, "scope not entitled: " + s - } - } - return true, "" -} - -// allowed reports whether value is in list, treating Wildcard as any value. An -// empty list allows nothing. -func allowed(list []string, value string) bool { - for _, v := range list { - if v == Wildcard || v == value { - return true - } - } - return false -} - -// identityNames returns the configured identity names, sorted, for logging. -func (e Entitlements) identityNames() []string { - names := make([]string, 0, len(e)) - for k := range e { - names = append(names, k) - } - sort.Strings(names) - return names -} diff --git a/src/pkg/mint/caller_test.go b/src/pkg/mint/caller_test.go deleted file mode 100644 index 82aa830b8..000000000 --- a/src/pkg/mint/caller_test.go +++ /dev/null @@ -1,276 +0,0 @@ -package mint - -import ( - "bytes" - "encoding/json" - "net/http" - "net/http/httptest" - "testing" -) - -// Tests for caller identity and entitlement on /mint (#3915). -// -// The finding had three parts and each gets a test: the shared secret proves -// possession and not identity, any holder could mint ANYTHING, and nothing -// recorded who asked. The third is covered by asserting the identity reaches -// the handler; the log line itself is built from it. - -// mintOnce posts a mint request through the handler and returns the status. -func mintOnce(t *testing.T, srv *Server, authHeader string, req MintRequest) (int, map[string]any) { - t.Helper() - body, _ := json.Marshal(req) - r := httptest.NewRequest(http.MethodPost, MintPath, bytes.NewReader(body)) - if authHeader != "" { - r.Header.Set("Authorization", authHeader) - } - w := httptest.NewRecorder() - srv.Handler().ServeHTTP(w, r) - var out map[string]any - _ = json.Unmarshal(w.Body.Bytes(), &out) - return w.Code, out -} - -func fullRequest() MintRequest { - return MintRequest{Subject: testSub, Audience: testAud, Scopes: []string{"registry:pull"}} -} - -// --- the seam ----------------------------------------------------------------- - -func TestSharedSecretAuthenticatorIsTheDefault(t *testing.T) { - srv, _ := newTestServer(t) - if srv.auth == nil { - t.Fatal("server has no authenticator") - } - if got := srv.auth.Name(); got != KindSharedSecret { - t.Errorf("default authenticator = %q, want %q", got, KindSharedSecret) - } -} - -func TestSharedSecretAuthenticatorRejectsEmptySecret(t *testing.T) { - if _, err := NewSharedSecretAuthenticator(""); err == nil { - t.Error("empty secret must be rejected (fail closed)") - } -} - -func TestSharedSecretIdentityIsNotPerCaller(t *testing.T) { - // A shared secret cannot tell its holders apart. The identity must say so - // rather than invent a name the mechanism never established. - a, err := NewSharedSecretAuthenticator(testSecret) - if err != nil { - t.Fatalf("NewSharedSecretAuthenticator: %v", err) - } - r := httptest.NewRequest(http.MethodPost, MintPath, nil) - r.Header.Set("Authorization", "Bearer "+testSecret) - id, err := a.Authenticate(r) - if err != nil { - t.Fatalf("Authenticate: %v", err) - } - if id.Kind != KindSharedSecret || id.Name != SharedSecretIdentityName { - t.Errorf("identity = %+v, want kind=%s name=%s", id, KindSharedSecret, SharedSecretIdentityName) - } - if id.String() != KindSharedSecret+":"+SharedSecretIdentityName { - t.Errorf("String() = %q", id.String()) - } -} - -func TestSharedSecretAuthenticatorRejectsBadCredentials(t *testing.T) { - a, _ := NewSharedSecretAuthenticator(testSecret) - for _, tc := range []struct{ name, header string }{ - {"missing", ""}, - {"wrong scheme", "Token " + testSecret}, - {"wrong secret", "Bearer nope"}, - {"empty bearer", "Bearer "}, - {"prefix of secret", "Bearer " + testSecret[:5]}, - } { - t.Run(tc.name, func(t *testing.T) { - r := httptest.NewRequest(http.MethodPost, MintPath, nil) - if tc.header != "" { - r.Header.Set("Authorization", tc.header) - } - if _, err := a.Authenticate(r); err == nil { - t.Error("expected rejection") - } - }) - } -} - -// stubAuthenticator stands in for a TokenReview or mTLS backend: it proves the -// seam works without pulling in a Kubernetes client. -type stubAuthenticator struct { - id Identity - ok bool -} - -func (s stubAuthenticator) Name() string { return "stub" } -func (s stubAuthenticator) Authenticate(*http.Request) (Identity, error) { - if !s.ok { - return Identity{}, ErrUnauthenticated - } - return s.id, nil -} - -func TestWithAuthenticatorReplacesTheSharedSecret(t *testing.T) { - m, _ := newTestMinter(t) - spoke := Identity{Kind: "serviceaccount", Name: "system:serviceaccount:hive:spoke"} - srv, err := NewServer(m, testSecret, nil, WithAuthenticator(stubAuthenticator{id: spoke, ok: true})) - if err != nil { - t.Fatalf("NewServer: %v", err) - } - // No Authorization header at all: the substituted backend vouches for the caller. - code, _ := mintOnce(t, srv, "", fullRequest()) - if code != http.StatusOK { - t.Fatalf("status = %d, want 200 — the substituted authenticator should have admitted the caller", code) - } -} - -func TestWithAuthenticatorRefusalIs401(t *testing.T) { - m, _ := newTestMinter(t) - srv, _ := NewServer(m, testSecret, nil, WithAuthenticator(stubAuthenticator{ok: false})) - // Even presenting the (still-configured) shared secret must not help once a - // different authenticator is in force. - code, _ := mintOnce(t, srv, "Bearer "+testSecret, fullRequest()) - if code != http.StatusUnauthorized { - t.Errorf("status = %d, want 401", code) - } -} - -func TestWithAuthenticatorIgnoresNil(t *testing.T) { - // A nil option must never leave the mint unauthenticated. - m, _ := newTestMinter(t) - srv, err := NewServer(m, testSecret, nil, WithAuthenticator(nil)) - if err != nil { - t.Fatalf("NewServer: %v", err) - } - if srv.auth == nil || srv.auth.Name() != KindSharedSecret { - t.Fatal("nil authenticator must be ignored, leaving the shared-secret default") - } - if code, _ := mintOnce(t, srv, "", fullRequest()); code != http.StatusUnauthorized { - t.Errorf("status = %d, want 401", code) - } -} - -// --- blast radius -------------------------------------------------------------- - -func TestWithoutEntitlementsAnyHolderMintsAnything(t *testing.T) { - // The behaviour the finding describes, pinned so the default stays a - // deliberate, documented choice rather than an accident. - srv, _ := newTestServer(t) - code, _ := mintOnce(t, srv, "Bearer "+testSecret, MintRequest{ - Subject: "system:serviceaccount:hive:hub-privileged", - Audience: "any-audience-at-all", - Scopes: []string{"admin:everything"}, - }) - if code != http.StatusOK { - t.Fatalf("status = %d, want 200 — unconfigured entitlements must preserve historical behaviour", code) - } -} - -func entitledServer(t *testing.T) *Server { - t.Helper() - m, _ := newTestMinter(t) - srv, err := NewServer(m, testSecret, nil, WithEntitlements(Entitlements{ - SharedSecretIdentityName: { - Subjects: []string{testSub}, - Audiences: []string{testAud}, - Scopes: []string{"registry:pull"}, - }, - })) - if err != nil { - t.Fatalf("NewServer: %v", err) - } - return srv -} - -func TestEntitlementAllowsWhatIsGranted(t *testing.T) { - if code, _ := mintOnce(t, entitledServer(t), "Bearer "+testSecret, fullRequest()); code != http.StatusOK { - t.Errorf("status = %d, want 200", code) - } -} - -func TestEntitlementRefusesBeyondTheGrant(t *testing.T) { - // Each dimension of the finding — "any subject, audience, and scope". - for _, tc := range []struct { - name string - req MintRequest - }{ - {"subject", MintRequest{Subject: "system:serviceaccount:hive:hub", Audience: testAud, Scopes: []string{"registry:pull"}}}, - {"audience", MintRequest{Subject: testSub, Audience: "somewhere-else", Scopes: []string{"registry:pull"}}}, - {"scope", MintRequest{Subject: testSub, Audience: testAud, Scopes: []string{"registry:push"}}}, - {"one bad scope among good", MintRequest{Subject: testSub, Audience: testAud, Scopes: []string{"registry:pull", "admin:all"}}}, - } { - t.Run(tc.name, func(t *testing.T) { - code, body := mintOnce(t, entitledServer(t), "Bearer "+testSecret, tc.req) - // 403, not 401: the caller is authenticated but not entitled. - if code != http.StatusForbidden { - t.Fatalf("status = %d, want 403", code) - } - if _, hasToken := body["token"]; hasToken { - t.Error("a refused mint must not return a token") - } - }) - } -} - -func TestEntitlementDeniesUnknownIdentity(t *testing.T) { - // Deny-by-default: an identity with no entry mints nothing, even though it - // authenticated successfully. - m, _ := newTestMinter(t) - srv, _ := NewServer(m, testSecret, nil, - WithAuthenticator(stubAuthenticator{id: Identity{Kind: "serviceaccount", Name: "stranger"}, ok: true}), - WithEntitlements(Entitlements{"known": {Subjects: []string{Wildcard}, Audiences: []string{Wildcard}, Scopes: []string{Wildcard}}}), - ) - if code, _ := mintOnce(t, srv, "", fullRequest()); code != http.StatusForbidden { - t.Errorf("status = %d, want 403 for an identity with no entitlement entry", code) - } -} - -func TestEmptyDimensionAllowsNothing(t *testing.T) { - // The trap this design avoids: an entitlement that grants an audience but - // forgets the scopes must grant NO scope, never every scope. - m, _ := newTestMinter(t) - srv, _ := NewServer(m, testSecret, nil, WithEntitlements(Entitlements{ - SharedSecretIdentityName: {Subjects: []string{testSub}, Audiences: []string{testAud}}, - })) - if code, _ := mintOnce(t, srv, "Bearer "+testSecret, fullRequest()); code != http.StatusForbidden { - t.Errorf("status = %d, want 403 — an empty scope list must allow nothing", code) - } - // ...and a request for no scopes at all is still fine. - if code, _ := mintOnce(t, srv, "Bearer "+testSecret, - MintRequest{Subject: testSub, Audience: testAud}); code != http.StatusOK { - t.Errorf("status = %d, want 200 for a scopeless request within the grant", code) - } -} - -func TestWildcardIsExplicit(t *testing.T) { - // A caller that genuinely mints arbitrary subjects has to say so. - m, _ := newTestMinter(t) - srv, _ := NewServer(m, testSecret, nil, WithEntitlements(Entitlements{ - SharedSecretIdentityName: {Subjects: []string{Wildcard}, Audiences: []string{testAud}, Scopes: []string{Wildcard}}, - })) - if code, _ := mintOnce(t, srv, "Bearer "+testSecret, - MintRequest{Subject: "anything", Audience: testAud, Scopes: []string{"any:scope"}}); code != http.StatusOK { - t.Errorf("status = %d, want 200 under an explicit wildcard", code) - } - // The wildcard is per-dimension: audience is still bounded. - if code, _ := mintOnce(t, srv, "Bearer "+testSecret, - MintRequest{Subject: "anything", Audience: "elsewhere", Scopes: nil}); code != http.StatusForbidden { - t.Errorf("status = %d, want 403 — a wildcard on one dimension must not widen another", code) - } -} - -func TestEntitlementsPermitsIsPureAndOrdered(t *testing.T) { - e := Entitlements{"a": {Subjects: []string{"s"}, Audiences: []string{"aud"}, Scopes: []string{"x"}}} - id := Identity{Kind: "k", Name: "a"} - if ok, why := e.permits(id, "s", "aud", []string{"x"}); !ok { - t.Errorf("expected permit, got refusal: %s", why) - } - if ok, why := e.permits(id, "other", "aud", nil); ok || why != "subject not entitled" { - t.Errorf("subject refusal = (%v, %q)", ok, why) - } - if ok, why := e.permits(Identity{Name: "zzz"}, "s", "aud", nil); ok || why != "identity has no entitlement entry" { - t.Errorf("unknown-identity refusal = (%v, %q)", ok, why) - } - if names := e.identityNames(); len(names) != 1 || names[0] != "a" { - t.Errorf("identityNames = %v", names) - } -} diff --git a/src/pkg/mint/server.go b/src/pkg/mint/server.go deleted file mode 100644 index c881049a3..000000000 --- a/src/pkg/mint/server.go +++ /dev/null @@ -1,234 +0,0 @@ -package mint - -import ( - "encoding/json" - "fmt" - "log/slog" - "net/http" - "time" -) - -const ( - // MintPath is the endpoint that issues scoped tokens. - MintPath = "/mint" - // JWKSPath is the standard public-key discovery endpoint. - JWKSPath = "/.well-known/jwks.json" - - // jwksCacheMaxAge advertises how long a verifier may cache the JWKS. Keys - // are long-lived so a generous cache is fine. - jwksCacheMaxAge = 5 * time.Minute - - // maxMintBodyBytes bounds the request body to avoid unbounded reads. - maxMintBodyBytes = 16 << 10 // 16 KiB - - // authScheme is the Authorization header scheme for the shared-secret gate. - authScheme = "Bearer " -) - -// Server exposes the mint over HTTP. -// -// Caller authentication on /mint goes through a CallerAuthenticator (caller.go). -// The default is the original shared-secret bearer token, constant-time -// compared, so behaviour is unchanged unless an operator configures otherwise. -// -// What a shared secret proves is "trusted network position", not "who" — see -// caller.go for the finding (#3915) and for what changed. In short: the mint no -// longer treats possession of a credential as permission to mint ANYTHING. -// Where Entitlements are configured the mint is deny-by-default per identity, -// and the verified identity is recorded on every mint and every refusal. -// -// The Kubernetes TokenReview backend now exists behind that seam and needed no -// change here, which was the point: see tokenreview.go for -// TokenReviewAuthenticator (audience-scoped, so a token minted for the API -// server cannot be replayed at the mint and vice versa) and -// MultiAuthenticator, the dual-accept step for migrating off the shared secret -// without a flag day. It needs no new module dependency. -// -// TODO(caller-auth): an mTLS client-certificate backend, for deployments with -// no Kubernetes API server to ask. Same interface, same absence of changes -// here. -// -// TODO(cloud-wif): the returned token is designed to be exchanged at a cloud -// WIF provider (GCP STS / AWS AssumeRoleWithWebIdentity / Azure federated -// credentials / registry token endpoint) configured to trust this issuer + -// JWKS. That exchange is provider-side and out of scope here. -type Server struct { - minter *Minter - auth CallerAuthenticator - ents Entitlements - logger *slog.Logger -} - -// ServerOption configures a Server. Options are additive: a Server built -// without any behaves exactly as it did before #3915. -type ServerOption func(*Server) - -// WithAuthenticator replaces the caller-authentication mechanism. This is the -// seam for a TokenReview or mTLS backend; passing nil is ignored so a caller -// cannot accidentally disable authentication. -func WithAuthenticator(a CallerAuthenticator) ServerOption { - return func(s *Server) { - if a != nil { - s.auth = a - } - } -} - -// WithEntitlements bounds what each verified identity may mint. Once a non-empty -// set is supplied the mint is deny-by-default: an identity with no entry may -// mint nothing, and an entitlement's empty dimension allows nothing rather than -// everything. See Entitlement. -func WithEntitlements(e Entitlements) ServerOption { - return func(s *Server) { s.ents = e } -} - -// NewServer builds a mint HTTP server. secret is the shared bearer secret that -// gates /mint; it must be non-empty (fail closed — an empty secret would allow -// anyone to mint). The secret is supplied by config/env, never hardcoded. -// -// The signature is unchanged from before #3915 and the default posture is -// identical: a shared-secret gate with no entitlement bound. Use -// WithAuthenticator and WithEntitlements to tighten it. -func NewServer(minter *Minter, secret string, logger *slog.Logger, opts ...ServerOption) (*Server, error) { - if minter == nil { - return nil, fmt.Errorf("mint: nil minter") - } - auth, err := NewSharedSecretAuthenticator(secret) - if err != nil { - return nil, err - } - if logger == nil { - logger = slog.Default() - } - s := &Server{minter: minter, auth: auth, logger: logger} - for _, opt := range opts { - opt(s) - } - - // Say the posture out loud at construction. An unbounded mint is a - // deliberate configuration, not an accident to discover from a token that - // should never have been issued. - if len(s.ents) == 0 { - s.logger.Warn("mint: no caller entitlements configured — any authenticated caller may mint any subject, audience and scope (bounded only by the TTL cap)", - "authenticator", s.auth.Name()) - } else { - s.logger.Info("mint: caller entitlements active (deny-by-default)", - "authenticator", s.auth.Name(), "identities", s.ents.identityNames()) - } - return s, nil -} - -// Handler returns an http.Handler serving MintPath and JWKSPath. -func (s *Server) Handler() http.Handler { - mux := http.NewServeMux() - mux.HandleFunc(MintPath, s.handleMint) - mux.HandleFunc(JWKSPath, s.handleJWKS) - return mux -} - -// MintRequest is the JSON body of a POST /mint call. -type MintRequest struct { - Subject string `json:"subject"` - Audience string `json:"audience"` - Scopes []string `json:"scopes,omitempty"` - // TTLSeconds is the requested lifetime; 0 uses the configured max. Values - // above the cap are clamped down, never honored above the ceiling. - TTLSeconds int `json:"ttl_seconds,omitempty"` -} - -// MintResponse is the JSON body returned on success. -type MintResponse struct { - Token string `json:"token"` - TokenType string `json:"token_type"` - ExpiresInSecs int `json:"expires_in"` - Issuer string `json:"issuer"` -} - -func (s *Server) handleMint(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodPost { - writeErr(w, http.StatusMethodNotAllowed, "method not allowed") - return - } - identity, err := s.auth.Authenticate(r) - if err != nil { - // Fail closed. Do not distinguish missing vs malformed vs wrong - // credential — the client learns only that it is not authorized. - s.logger.Warn("mint: unauthenticated /mint request refused", - "authenticator", s.auth.Name(), "remote_addr", r.RemoteAddr) - writeErr(w, http.StatusUnauthorized, "unauthorized") - return - } - - r.Body = http.MaxBytesReader(w, r.Body, maxMintBodyBytes) - var req MintRequest - dec := json.NewDecoder(r.Body) - dec.DisallowUnknownFields() - if err := dec.Decode(&req); err != nil { - writeErr(w, http.StatusBadRequest, "invalid request body") - return - } - - // Deny-by-default once entitlements are configured. This is what stops a - // credential holder minting for a subject it has no business asserting — - // the "any subject, any audience, any scope" half of #3915. Refused with - // 403, not 401: the caller IS authenticated, it is simply not entitled. - if ok, why := s.ents.permits(identity, req.Subject, req.Audience, req.Scopes); !ok { - s.logger.Warn("mint: refused, caller not entitled", - "caller", identity.String(), "caller_kind", identity.Kind, - "subject", req.Subject, "audience", req.Audience, - "scopes", req.Scopes, "reason", why) - writeErr(w, http.StatusForbidden, "not entitled") - return - } - - ttl := time.Duration(req.TTLSeconds) * time.Second - token, err := s.minter.Mint(req.Subject, req.Audience, req.Scopes, ttl) - if err != nil { - // Mint only errors on caller-supplied invalid input (missing - // subject/audience) — treat as a 400, never leak internals. - writeErr(w, http.StatusBadRequest, "cannot mint token") - return - } - - // Re-derive the honored TTL for the response (Mint clamps internally). - honored := s.minter.clampTTL(ttl) - resp := MintResponse{ - Token: token, - TokenType: "Bearer", - ExpiresInSecs: int(honored.Seconds()), - Issuer: s.minter.issuer, - } - writeJSON(w, http.StatusOK, resp) - // The caller is part of the audit record (#3915): a mint line that names - // only the subject cannot answer "who asked for this token". - s.logger.Info("token minted", - "caller", identity.String(), "caller_kind", identity.Kind, - "subject", req.Subject, "audience", req.Audience, - "scopes", req.Scopes, "ttl_seconds", int(honored.Seconds())) -} - -func (s *Server) handleJWKS(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodGet { - writeErr(w, http.StatusMethodNotAllowed, "method not allowed") - return - } - body, err := s.minter.JWKS() - if err != nil { - writeErr(w, http.StatusInternalServerError, "jwks unavailable") - return - } - w.Header().Set("Content-Type", "application/json") - w.Header().Set("Cache-Control", fmt.Sprintf("public, max-age=%d", int(jwksCacheMaxAge.Seconds()))) - w.WriteHeader(http.StatusOK) - _, _ = w.Write(body) -} - -func writeJSON(w http.ResponseWriter, status int, v interface{}) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - _ = json.NewEncoder(w).Encode(v) -} - -func writeErr(w http.ResponseWriter, status int, msg string) { - writeJSON(w, status, map[string]string{"error": msg}) -} diff --git a/src/pkg/mint/server_test.go b/src/pkg/mint/server_test.go deleted file mode 100644 index dec7a7df1..000000000 --- a/src/pkg/mint/server_test.go +++ /dev/null @@ -1,271 +0,0 @@ -package mint - -import ( - "bytes" - "encoding/json" - "net/http" - "net/http/httptest" - "strings" - "testing" -) - -const testSecret = "s3cret-shared-token" - -func newTestServer(t *testing.T) (*Server, *Minter) { - t.Helper() - m, _ := newTestMinter(t) - srv, err := NewServer(m, testSecret, nil) - if err != nil { - t.Fatalf("NewServer: %v", err) - } - return srv, m -} - -func TestNewServerRejectsEmptySecret(t *testing.T) { - m, _ := newTestMinter(t) - if _, err := NewServer(m, "", nil); err == nil { - t.Error("expected error for empty secret (fail closed)") - } - if _, err := NewServer(nil, testSecret, nil); err == nil { - t.Error("expected error for nil minter") - } -} - -func TestMintHandlerReturnsToken(t *testing.T) { - srv, m := newTestServer(t) - h := srv.Handler() - - body, _ := json.Marshal(MintRequest{ - Subject: testSub, - Audience: testAud, - Scopes: []string{"registry:pull"}, - TTLSeconds: 300, - }) - req := httptest.NewRequest(http.MethodPost, MintPath, bytes.NewReader(body)) - req.Header.Set("Authorization", "Bearer "+testSecret) - rr := httptest.NewRecorder() - h.ServeHTTP(rr, req) - - if rr.Code != http.StatusOK { - t.Fatalf("status = %d, want 200; body=%s", rr.Code, rr.Body.String()) - } - var resp MintResponse - if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil { - t.Fatalf("decode response: %v", err) - } - if resp.Token == "" { - t.Fatal("empty token") - } - if resp.TokenType != "Bearer" { - t.Errorf("token_type = %q, want Bearer", resp.TokenType) - } - if resp.ExpiresInSecs != 300 { - t.Errorf("expires_in = %d, want 300", resp.ExpiresInSecs) - } - // Token must verify against the minter. - claims, err := m.Verify(resp.Token) - if err != nil { - t.Fatalf("Verify minted token: %v", err) - } - if len(claims.Scopes) != 1 || claims.Scopes[0] != "registry:pull" { - t.Errorf("scopes = %v", claims.Scopes) - } -} - -func TestMintHandlerRejectsUnauthorized(t *testing.T) { - srv, _ := newTestServer(t) - h := srv.Handler() - - cases := map[string]string{ - "no header": "", - "wrong secret": "Bearer wrong", - "wrong scheme": "Basic " + testSecret, - } - for name, auth := range cases { - t.Run(name, func(t *testing.T) { - body, _ := json.Marshal(MintRequest{Subject: testSub, Audience: testAud}) - req := httptest.NewRequest(http.MethodPost, MintPath, bytes.NewReader(body)) - if auth != "" { - req.Header.Set("Authorization", auth) - } - rr := httptest.NewRecorder() - h.ServeHTTP(rr, req) - if rr.Code != http.StatusUnauthorized { - t.Errorf("status = %d, want 401", rr.Code) - } - }) - } -} - -func TestMintHandlerBadRequest(t *testing.T) { - srv, _ := newTestServer(t) - h := srv.Handler() - - // Missing subject/audience -> Mint errors -> 400. - body, _ := json.Marshal(MintRequest{Scopes: []string{"x"}}) - req := httptest.NewRequest(http.MethodPost, MintPath, bytes.NewReader(body)) - req.Header.Set("Authorization", "Bearer "+testSecret) - rr := httptest.NewRecorder() - h.ServeHTTP(rr, req) - if rr.Code != http.StatusBadRequest { - t.Errorf("status = %d, want 400", rr.Code) - } - - // Malformed JSON -> 400. - req2 := httptest.NewRequest(http.MethodPost, MintPath, strings.NewReader("{not json")) - req2.Header.Set("Authorization", "Bearer "+testSecret) - rr2 := httptest.NewRecorder() - h.ServeHTTP(rr2, req2) - if rr2.Code != http.StatusBadRequest { - t.Errorf("malformed status = %d, want 400", rr2.Code) - } -} - -func TestMintHandlerMethodNotAllowed(t *testing.T) { - srv, _ := newTestServer(t) - h := srv.Handler() - req := httptest.NewRequest(http.MethodGet, MintPath, nil) - req.Header.Set("Authorization", "Bearer "+testSecret) - rr := httptest.NewRecorder() - h.ServeHTTP(rr, req) - if rr.Code != http.StatusMethodNotAllowed { - t.Errorf("status = %d, want 405", rr.Code) - } -} - -func TestJWKSHandlerServesKeys(t *testing.T) { - srv, m := newTestServer(t) - h := srv.Handler() - - req := httptest.NewRequest(http.MethodGet, JWKSPath, nil) - rr := httptest.NewRecorder() - h.ServeHTTP(rr, req) - - if rr.Code != http.StatusOK { - t.Fatalf("status = %d, want 200", rr.Code) - } - if ct := rr.Header().Get("Content-Type"); ct != "application/json" { - t.Errorf("content-type = %q", ct) - } - - var set struct { - Keys []map[string]string `json:"keys"` - } - if err := json.Unmarshal(rr.Body.Bytes(), &set); err != nil { - t.Fatalf("decode JWKS: %v", err) - } - if len(set.Keys) != 1 { - t.Fatalf("keys = %d, want 1", len(set.Keys)) - } - - // JWKS is unauthenticated (public discovery) and validates a real token. - tok, err := m.Mint(testSub, testAud, nil, 60_000_000_000) // 60s - if err != nil { - t.Fatalf("Mint: %v", err) - } - pub := rebuildPublicKey(t, set.Keys[0]["n"], set.Keys[0]["e"]) - if pub == nil { - t.Fatal("nil reconstructed key") - } - // Sanity: verify through the minter (JWKS-independence covered in mint_test). - if _, err := m.Verify(tok); err != nil { - t.Fatalf("Verify: %v", err) - } -} - -func TestJWKSHandlerSetsCacheControl(t *testing.T) { - srv, _ := newTestServer(t) - h := srv.Handler() - req := httptest.NewRequest(http.MethodGet, JWKSPath, nil) - rr := httptest.NewRecorder() - h.ServeHTTP(rr, req) - - if rr.Code != http.StatusOK { - t.Fatalf("status = %d, want 200", rr.Code) - } - cc := rr.Header().Get("Cache-Control") - if cc == "" { - t.Fatal("missing Cache-Control header") - } - // jwksCacheMaxAge is 5m -> 300s. - if want := "public, max-age=300"; cc != want { - t.Errorf("Cache-Control = %q, want %q", cc, want) - } -} - -func TestJWKSHandlerMethodNotAllowed(t *testing.T) { - srv, _ := newTestServer(t) - h := srv.Handler() - req := httptest.NewRequest(http.MethodPost, JWKSPath, strings.NewReader("{}")) - rr := httptest.NewRecorder() - h.ServeHTTP(rr, req) - if rr.Code != http.StatusMethodNotAllowed { - t.Errorf("status = %d, want 405", rr.Code) - } -} - -func TestMintHandlerRejectsUnknownFields(t *testing.T) { - srv, _ := newTestServer(t) - h := srv.Handler() - // DisallowUnknownFields -> a stray field yields a decode error -> 400. - raw := `{"subject":"s","audience":"a","surprise":"nope"}` - req := httptest.NewRequest(http.MethodPost, MintPath, strings.NewReader(raw)) - req.Header.Set("Authorization", "Bearer "+testSecret) - rr := httptest.NewRecorder() - h.ServeHTTP(rr, req) - if rr.Code != http.StatusBadRequest { - t.Errorf("status = %d, want 400 for unknown field", rr.Code) - } -} - -func TestMintHandlerRejectsOversizedBody(t *testing.T) { - srv, _ := newTestServer(t) - h := srv.Handler() - // Build a syntactically valid JSON object larger than maxMintBodyBytes so - // MaxBytesReader trips during decode -> 400. - huge := strings.Repeat("x", (16<<10)+1024) - raw := `{"subject":"s","audience":"a","scopes":["` + huge + `"]}` - req := httptest.NewRequest(http.MethodPost, MintPath, strings.NewReader(raw)) - req.Header.Set("Authorization", "Bearer "+testSecret) - rr := httptest.NewRecorder() - h.ServeHTTP(rr, req) - if rr.Code != http.StatusBadRequest { - t.Errorf("status = %d, want 400 for oversized body", rr.Code) - } -} - -func TestMintHandlerEmptyBearer(t *testing.T) { - srv, _ := newTestServer(t) - h := srv.Handler() - // "Bearer " with an empty secret must not match the non-empty configured - // secret (constant-time compare returns 0 on length mismatch). - body, _ := json.Marshal(MintRequest{Subject: testSub, Audience: testAud}) - req := httptest.NewRequest(http.MethodPost, MintPath, bytes.NewReader(body)) - req.Header.Set("Authorization", "Bearer ") - rr := httptest.NewRecorder() - h.ServeHTTP(rr, req) - if rr.Code != http.StatusUnauthorized { - t.Errorf("status = %d, want 401 for empty bearer", rr.Code) - } -} - -func TestMintHandlerZeroTTLUsesMax(t *testing.T) { - srv, m := newTestServer(t) - h := srv.Handler() - // TTLSeconds omitted (0) -> honored TTL is the configured max. - body, _ := json.Marshal(MintRequest{Subject: testSub, Audience: testAud}) - req := httptest.NewRequest(http.MethodPost, MintPath, bytes.NewReader(body)) - req.Header.Set("Authorization", "Bearer "+testSecret) - rr := httptest.NewRecorder() - h.ServeHTTP(rr, req) - if rr.Code != http.StatusOK { - t.Fatalf("status = %d, want 200; body=%s", rr.Code, rr.Body.String()) - } - var resp MintResponse - if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil { - t.Fatalf("decode: %v", err) - } - if want := int(m.MaxTTL().Seconds()); resp.ExpiresInSecs != want { - t.Errorf("expires_in = %d, want %d (max ttl)", resp.ExpiresInSecs, want) - } -} diff --git a/src/pkg/mint/tokenreview.go b/src/pkg/mint/tokenreview.go deleted file mode 100644 index d8922a90f..000000000 --- a/src/pkg/mint/tokenreview.go +++ /dev/null @@ -1,430 +0,0 @@ -package mint - -import ( - "bytes" - "context" - "crypto/tls" - "crypto/x509" - "encoding/json" - "fmt" - "io" - "net" - "net/http" - "os" - "strings" - "time" -) - -// Kubernetes ServiceAccount caller authentication for /mint (#3915). -// -// This is the backend the CallerAuthenticator seam in caller.go was added for, -// and the last of the finding's three gaps: the shared secret proves TRUSTED -// NETWORK POSITION, not WHO IS CALLING. A caller here presents its own -// projected ServiceAccount token, the mint asks the Kubernetes API server to -// vouch for it, and the answer — `system:serviceaccount::` — is a -// real identity that Entitlements can be keyed on. -// -// NO NEW DEPENDENCY. The seam landed without this backend because TokenReview -// was assumed to need k8s.io/client-go, which this module does not depend on, -// and pulling in a dependency of that weight is a maintainer call. It turns out -// not to be needed: TokenReview is one POST of a small, stable JSON object to -// authentication.k8s.io/v1 (GA since Kubernetes 1.6), so net/http and -// encoding/json cover it. If client-go is ever added for other reasons this -// file can be swapped for it behind the same interface, with no change to -// Server. That is the whole point of the seam. -// -// WHAT MAKES THIS SAFE, AND WHAT WOULD MAKE IT NOT: -// -// audience-scoped The caller's token must be projected with the mint's own -// audience, and the review REQUESTS that audience. A token -// minted for the API server cannot be replayed here, and a -// token projected for the mint cannot be replayed at the API -// server. The check that makes this real is on the RESPONSE, -// not the request — see verifyAudience. -// dedicated header The reviewed token is read from its own header, never from -// Authorization. In dual-accept deployments Authorization -// still carries the SHARED SECRET, and sending that to the -// API server as "a token to please review" would write the -// mint's secret into someone else's audit log. -// real TLS The API server is verified against the in-cluster CA -// bundle. There is no insecure-skip-verify option, not even -// a documented one. -// bounded A timeout and a response-size cap, so an API server that -// hangs or floods cannot wedge or exhaust the mint. The -// failure is a refusal — TokenReview failing open would be -// strictly worse than the shared secret it replaces. - -const ( - // ServiceAccountTokenHeader carries the caller's projected ServiceAccount - // token. Deliberately NOT Authorization: that header holds the shared - // secret in a dual-accept deployment, and this value is forwarded to the - // Kubernetes API server. - ServiceAccountTokenHeader = "X-Hive-Mint-SA-Token" - - // KindServiceAccount is the Identity.Kind for TokenReview-verified callers. - KindServiceAccount = "serviceaccount" - - // serviceAccountUsernamePrefix is what the API server reports for a - // ServiceAccount: system:serviceaccount::. Any other - // username shape is a human or node identity and is refused — an - // entitlement map keyed on ServiceAccount names must not be satisfiable by - // a kubeconfig user who happens to have a token. - serviceAccountUsernamePrefix = "system:serviceaccount:" - - // tokenReviewPath is the API server endpoint. authentication.k8s.io/v1 has - // been GA since Kubernetes 1.6. - tokenReviewPath = "/apis/authentication.k8s.io/v1/tokenreviews" - - // inClusterTokenPath / inClusterCAPath are the projected paths every pod - // with a mounted ServiceAccount gets. - inClusterTokenPath = "/var/run/secrets/kubernetes.io/serviceaccount/token" // #nosec G101 -- path, not a credential - inClusterCAPath = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" - - // defaultReviewTimeout bounds one TokenReview round trip. - defaultReviewTimeout = 5 * time.Second - - // maxReviewResponseBytes bounds the API server's response. - maxReviewResponseBytes = 1 << 20 // 1 MiB -) - -// TokenReviewAuthenticator verifies a caller's Kubernetes ServiceAccount token -// by asking the API server, and reports the ServiceAccount as the identity. -// -// Safe for concurrent use. -type TokenReviewAuthenticator struct { - // apiURL is the API server base, e.g. https://10.0.0.1:443. - apiURL string - // audience is the audience the caller's token must be projected for, and - // the one the review requests. - audience string - // client performs the review call, TLS-verified against the cluster CA. - client *http.Client - // reviewerToken authenticates the MINT to the API server (not the caller). - // It is re-read on each use because projected tokens are rotated on disk; - // caching it forever would start failing an hour into the pod's life. - reviewerToken func() (string, error) -} - -// TokenReviewOption configures a TokenReviewAuthenticator. -type TokenReviewOption func(*TokenReviewAuthenticator) - -// WithReviewHTTPClient overrides the HTTP client. Intended for tests, which -// point it at an httptest server; production uses the in-cluster CA bundle. -// A nil client is ignored so the verified default cannot be cleared. -func WithReviewHTTPClient(c *http.Client) TokenReviewOption { - return func(a *TokenReviewAuthenticator) { - if c != nil { - a.client = c - } - } -} - -// WithReviewerToken overrides how the mint authenticates ITSELF to the API -// server. Default: re-read the pod's projected token from disk on each review. -func WithReviewerToken(f func() (string, error)) TokenReviewOption { - return func(a *TokenReviewAuthenticator) { - if f != nil { - a.reviewerToken = f - } - } -} - -// NewTokenReviewAuthenticator builds the backend from explicit parameters. -// -// audience MUST be non-empty. An empty audience would send a review with no -// audience constraint, and the response check below would have nothing to -// verify against — which is exactly the replay hole this backend exists to -// close, so it is a construction error rather than a silent downgrade. -func NewTokenReviewAuthenticator(apiURL, audience string, caPEM []byte, opts ...TokenReviewOption) (*TokenReviewAuthenticator, error) { - if apiURL == "" { - return nil, fmt.Errorf("mint: TokenReview requires an API server URL") - } - if audience == "" { - return nil, fmt.Errorf("mint: TokenReview requires an audience (an unscoped review would accept a token minted for the API server)") - } - - a := &TokenReviewAuthenticator{ - apiURL: strings.TrimRight(apiURL, "/"), - audience: audience, - reviewerToken: readFileTrimmed(inClusterTokenPath), - } - - if len(caPEM) > 0 { - pool := x509.NewCertPool() - if !pool.AppendCertsFromPEM(caPEM) { - return nil, fmt.Errorf("mint: TokenReview CA bundle contains no usable certificate") - } - a.client = &http.Client{ - Timeout: defaultReviewTimeout, - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{RootCAs: pool, MinVersion: tls.VersionTLS12}, - }, - } - } else { - // No explicit bundle: the host trust store. Callers in-cluster should - // use NewInClusterTokenReviewAuthenticator, which always supplies one. - a.client = &http.Client{ - Timeout: defaultReviewTimeout, - Transport: &http.Transport{TLSClientConfig: &tls.Config{MinVersion: tls.VersionTLS12}}, - } - } - - for _, opt := range opts { - opt(a) - } - return a, nil -} - -// NewInClusterTokenReviewAuthenticator builds the backend from the pod's own -// ServiceAccount mount and the KUBERNETES_SERVICE_* environment, the same -// inputs client-go's rest.InClusterConfig uses. -// -// It fails rather than degrading: a missing CA bundle or an absent -// KUBERNETES_SERVICE_HOST means this is not running in a cluster, and a mint -// that quietly fell back to a weaker gate on that basis would be the finding -// again in a new costume. -func NewInClusterTokenReviewAuthenticator(audience string, opts ...TokenReviewOption) (*TokenReviewAuthenticator, error) { - host := os.Getenv("KUBERNETES_SERVICE_HOST") - port := os.Getenv("KUBERNETES_SERVICE_PORT") - if host == "" || port == "" { - return nil, fmt.Errorf("mint: not running in a Kubernetes cluster (KUBERNETES_SERVICE_HOST/PORT unset)") - } - caPEM, err := os.ReadFile(inClusterCAPath) - if err != nil { - return nil, fmt.Errorf("mint: reading in-cluster CA bundle: %w", err) - } - return NewTokenReviewAuthenticator("https://"+net.JoinHostPort(host, port), audience, caPEM, opts...) -} - -// Name implements CallerAuthenticator. -func (a *TokenReviewAuthenticator) Name() string { return KindServiceAccount } - -// Authenticate implements CallerAuthenticator. -// -// Every failure returns ErrUnauthenticated with no detail: the handler answers -// 401 without telling a prober whether the token was missing, expired, for the -// wrong audience, or belonged to a user rather than a ServiceAccount. -func (a *TokenReviewAuthenticator) Authenticate(r *http.Request) (Identity, error) { - presented := strings.TrimSpace(r.Header.Get(ServiceAccountTokenHeader)) - // Tolerate a "Bearer " prefix: callers reuse HTTP client middleware that - // adds one, and refusing it produces a 401 with no way to tell why. - presented = strings.TrimPrefix(presented, authScheme) - if presented == "" { - return Identity{}, ErrUnauthenticated - } - - ctx := r.Context() - if ctx == nil { - ctx = context.Background() - } - status, err := a.review(ctx, presented) - if err != nil { - return Identity{}, ErrUnauthenticated - } - if !status.Authenticated || status.Error != "" { - return Identity{}, ErrUnauthenticated - } - if !a.verifyAudience(status.Audiences) { - return Identity{}, ErrUnauthenticated - } - name := status.User.Username - if !strings.HasPrefix(name, serviceAccountUsernamePrefix) { - return Identity{}, ErrUnauthenticated - } - // system:serviceaccount:: — both parts must be present, or the - // identity is not the shape Entitlements are keyed on. - rest := strings.TrimPrefix(name, serviceAccountUsernamePrefix) - ns, sa, ok := strings.Cut(rest, ":") - if !ok || ns == "" || sa == "" { - return Identity{}, ErrUnauthenticated - } - return Identity{Kind: KindServiceAccount, Name: name}, nil -} - -// verifyAudience is the check that makes audience scoping real. -// -// The REQUEST asking for an audience proves nothing on its own: an API server -// whose authenticators do not implement audience validation answers -// `authenticated: true` with the audiences field ABSENT, and a caller that -// looked only at `authenticated` would accept a token minted for the API server -// itself — precisely the replay this backend claims to prevent. The Kubernetes -// API contract puts the obligation on the client: the returned audiences are -// the intersection of what was asked for and what the token carries, and an -// empty intersection means "not validated for your audience". -// -// So: the mint's audience must appear in the RESPONSE, and an empty or absent -// list is a refusal, never a pass. -func (a *TokenReviewAuthenticator) verifyAudience(returned []string) bool { - for _, aud := range returned { - if aud == a.audience { - return true - } - } - return false -} - -// tokenReviewRequest / tokenReviewResponse are the minimal shapes of -// authentication.k8s.io/v1 TokenReview. Only the fields this backend acts on -// are modelled; the API server ignores what it does not need and we ignore what -// we do not read. -type tokenReviewRequest struct { - APIVersion string `json:"apiVersion"` - Kind string `json:"kind"` - Spec tokenReviewRequestSpec `json:"spec"` -} - -type tokenReviewRequestSpec struct { - Token string `json:"token"` - Audiences []string `json:"audiences,omitempty"` -} - -type tokenReviewResponse struct { - Status tokenReviewStatus `json:"status"` -} - -type tokenReviewStatus struct { - Authenticated bool `json:"authenticated"` - User tokenReviewUser `json:"user"` - Audiences []string `json:"audiences"` - Error string `json:"error"` -} - -type tokenReviewUser struct { - Username string `json:"username"` - UID string `json:"uid"` -} - -// review performs one TokenReview round trip. -// -// The presented token is placed in the request BODY and never in a header, a -// log line, or an error. The only Authorization on this call is the mint's own -// reviewer token. -func (a *TokenReviewAuthenticator) review(ctx context.Context, token string) (tokenReviewStatus, error) { - var zero tokenReviewStatus - - body, err := json.Marshal(tokenReviewRequest{ - APIVersion: "authentication.k8s.io/v1", - Kind: "TokenReview", - Spec: tokenReviewRequestSpec{ - Token: token, - Audiences: []string{a.audience}, - }, - }) - if err != nil { - return zero, err - } - - ctx, cancel := context.WithTimeout(ctx, defaultReviewTimeout) - defer cancel() - - req, err := http.NewRequestWithContext(ctx, http.MethodPost, a.apiURL+tokenReviewPath, bytes.NewReader(body)) - if err != nil { - return zero, err - } - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Accept", "application/json") - - reviewer, err := a.reviewerToken() - if err != nil { - return zero, fmt.Errorf("mint: reading reviewer token: %w", err) - } - if reviewer == "" { - return zero, fmt.Errorf("mint: empty reviewer token") - } - req.Header.Set("Authorization", authScheme+reviewer) - - resp, err := a.client.Do(req) - if err != nil { - return zero, err - } - defer func() { - // Drain (bounded) so the connection can be reused, then close. - _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, maxReviewResponseBytes)) - _ = resp.Body.Close() - }() - - if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { - // A 401/403 here means the MINT cannot call TokenReview (its RBAC is - // missing), not that the caller is bad. Both refuse the caller — fail - // closed — but the distinction matters when reading logs, so it is in - // the error the server logs rather than swallowed. - return zero, fmt.Errorf("mint: TokenReview returned HTTP %d (the mint's own ServiceAccount may lack create on tokenreviews)", resp.StatusCode) - } - - var out tokenReviewResponse - if err := json.NewDecoder(io.LimitReader(resp.Body, maxReviewResponseBytes)).Decode(&out); err != nil { - return zero, err - } - return out.Status, nil -} - -// readFileTrimmed returns a loader that reads path fresh on each call. Fresh, -// not cached: projected ServiceAccount tokens are rotated on disk (hourly by -// default), so a token read once at construction stops working while the -// process is still healthy. -func readFileTrimmed(path string) func() (string, error) { - return func() (string, error) { - b, err := os.ReadFile(path) - if err != nil { - return "", err - } - return strings.TrimSpace(string(b)), nil - } -} - -// MultiAuthenticator tries several backends in order and returns the first -// identity established. -// -// This is the migration path the finding's rollout needs: run TokenReview -// alongside the shared secret, watch the audit log until every caller is -// arriving as a ServiceAccount, then drop the secret from the list. Without it -// the cutover is a flag day. -// -// Order matters, and TokenReview should come FIRST. It reads its own header, so -// putting it ahead of the shared secret means a caller that presents both is -// recorded under its real identity rather than as "any-holder" — which is the -// difference between an audit log that shows the migration finishing and one -// that shows nothing changing. -type MultiAuthenticator struct { - backends []CallerAuthenticator -} - -// NewMultiAuthenticator builds a dual-accept authenticator. Nil backends are -// dropped; an empty list is an error, because an authenticator that -// authenticates nothing would answer 401 to everything and look like an outage -// rather than a misconfiguration. -func NewMultiAuthenticator(backends ...CallerAuthenticator) (*MultiAuthenticator, error) { - var kept []CallerAuthenticator - for _, b := range backends { - if b != nil { - kept = append(kept, b) - } - } - if len(kept) == 0 { - return nil, fmt.Errorf("mint: MultiAuthenticator needs at least one backend") - } - return &MultiAuthenticator{backends: kept}, nil -} - -// Name implements CallerAuthenticator, listing the backends in the order tried -// so a startup log says which mechanisms are live. -func (m *MultiAuthenticator) Name() string { - names := make([]string, 0, len(m.backends)) - for _, b := range m.backends { - names = append(names, b.Name()) - } - return strings.Join(names, "+") -} - -// Authenticate implements CallerAuthenticator, returning the first identity any -// backend establishes. It does not report WHICH backend failed for a refused -// caller — the handler must not turn a 401 into an oracle for which mechanisms -// are configured. -func (m *MultiAuthenticator) Authenticate(r *http.Request) (Identity, error) { - for _, b := range m.backends { - if id, err := b.Authenticate(r); err == nil { - return id, nil - } - } - return Identity{}, ErrUnauthenticated -} diff --git a/src/pkg/mint/tokenreview_loader_test.go b/src/pkg/mint/tokenreview_loader_test.go deleted file mode 100644 index d9718c14e..000000000 --- a/src/pkg/mint/tokenreview_loader_test.go +++ /dev/null @@ -1,107 +0,0 @@ -package mint - -import ( - "os" - "path/filepath" - "strings" - "testing" -) - -// Tests for the projected-token loader and the in-cluster constructor's CA -// branch — the pieces TestNewInClusterTokenReviewAuthenticatorRefusesOutsideACluster -// does not reach. -// -// readFileTrimmed is what lets the mint keep authenticating ITSELF after the -// kubelet rotates the projected ServiceAccount token on disk: it must re-read -// on every call and strip the trailing newline the projection writes. A loader -// that cached, or that passed the newline through, would start failing reviews -// an hour after boot while looking healthy. - -func TestReadFileTrimmedRereadsRotatedToken(t *testing.T) { - path := filepath.Join(t.TempDir(), "token") - if err := os.WriteFile(path, []byte(" first-token\n"), 0o600); err != nil { - t.Fatalf("writing token: %v", err) - } - - load := readFileTrimmed(path) - - got, err := load() - if err != nil { - t.Fatalf("first load: %v", err) - } - if got != "first-token" { - t.Errorf("first load = %q, want %q (whitespace must be trimmed)", got, "first-token") - } - - // Rotate the token on disk. The loader must observe the new value: a - // cached read here is the bug the doc comment on readFileTrimmed warns - // about — the process stays healthy while every review starts failing. - if err := os.WriteFile(path, []byte("second-token\n"), 0o600); err != nil { - t.Fatalf("rotating token: %v", err) - } - got, err = load() - if err != nil { - t.Fatalf("load after rotation: %v", err) - } - if got != "second-token" { - t.Errorf("load after rotation = %q, want %q (loader must re-read, not cache)", got, "second-token") - } -} - -func TestReadFileTrimmedPropagatesMissingFile(t *testing.T) { - load := readFileTrimmed(filepath.Join(t.TempDir(), "does-not-exist")) - if _, err := load(); err == nil { - t.Error("loading a missing token file succeeded — the error must propagate so review failures are attributable") - } -} - -func TestReadFileTrimmedEmptyFileYieldsEmptyToken(t *testing.T) { - path := filepath.Join(t.TempDir(), "token") - if err := os.WriteFile(path, []byte("\n\t \n"), 0o600); err != nil { - t.Fatalf("writing token: %v", err) - } - got, err := readFileTrimmed(path)() - if err != nil { - t.Fatalf("load: %v", err) - } - if got != "" { - t.Errorf("whitespace-only file loaded as %q, want empty string", got) - } -} - -// The in-cluster constructor must fail rather than degrade when the -// KUBERNETES_SERVICE_* environment is present but the mounted CA bundle is -// not readable — a half-present cluster environment is a misconfiguration, -// not a cue to fall back to weaker trust. -// -// The assertion is conditional on whether the projected CA path exists so the -// test stays hermetic both on plain hosts (the common case: the path is -// absent, construction must fail naming the CA) and inside a real pod (the -// path exists, construction must succeed against the fake host/port because -// no network I/O happens at construction time). -func TestNewInClusterTokenReviewAuthenticatorCABranch(t *testing.T) { - t.Setenv("KUBERNETES_SERVICE_HOST", "kubernetes.default.svc.hive.invalid") - t.Setenv("KUBERNETES_SERVICE_PORT", "443") - - a, err := NewInClusterTokenReviewAuthenticator(testAudience) - - if _, statErr := os.Stat(inClusterCAPath); statErr != nil { - // Plain host: the CA bundle is absent, so construction must refuse. - if err == nil { - t.Fatal("built an in-cluster authenticator with no readable CA bundle — it must fail rather than degrade to host trust") - } - if !strings.Contains(err.Error(), "CA bundle") { - t.Errorf("error = %q, want it to name the CA bundle so the operator knows what is missing", err) - } - return - } - - // Real pod: the projected CA exists, so construction succeeds without - // touching the network. - if err != nil { - t.Fatalf("NewInClusterTokenReviewAuthenticator with a present CA bundle: %v", err) - } - if a == nil { - t.Fatal("nil authenticator with nil error") - } -} diff --git a/src/pkg/mint/tokenreview_test.go b/src/pkg/mint/tokenreview_test.go deleted file mode 100644 index cd41882b7..000000000 --- a/src/pkg/mint/tokenreview_test.go +++ /dev/null @@ -1,481 +0,0 @@ -package mint - -import ( - "encoding/json" - "io" - "net/http" - "net/http/httptest" - "strings" - "testing" -) - -// Tests for the Kubernetes ServiceAccount caller backend (#3915). -// -// The finding's remaining gap was that /mint could only prove "the caller holds -// a secret". These tests are about what the mint now proves instead, and — more -// importantly — about the ways this backend could LOOK like it proves it while -// not doing so. Each of the following would leave the mint no better off than -// the shared secret, and each has a test: -// -// - accepting a token the API server authenticated but did NOT validate for -// the mint's audience (the API-server-token replay), -// - accepting a username that is not a ServiceAccount, -// - forwarding the caller's Authorization header (the shared secret) to the -// API server as the token to review, -// - failing OPEN when the API server is unreachable or refuses the mint's own -// TokenReview call. - -const ( - testAudience = "hive-mint" - testSAName = "system:serviceaccount:hive:hive-spoke" -) - -// fakeAPIServer stands in for the Kubernetes API server. It records what the -// mint sent so the tests can assert on the request, not only the outcome. -type fakeAPIServer struct { - srv *httptest.Server - - // captured request - gotPath string - gotAuthorization string - gotSpec tokenReviewRequestSpec - - // canned response - status tokenReviewStatus - httpStatus int -} - -func newFakeAPIServer(t *testing.T, status tokenReviewStatus) *fakeAPIServer { - t.Helper() - f := &fakeAPIServer{status: status, httpStatus: http.StatusCreated} - f.srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - f.gotPath = r.URL.Path - f.gotAuthorization = r.Header.Get("Authorization") - body, _ := io.ReadAll(r.Body) - var req tokenReviewRequest - _ = json.Unmarshal(body, &req) - f.gotSpec = req.Spec - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(f.httpStatus) - _ = json.NewEncoder(w).Encode(tokenReviewResponse{Status: f.status}) - })) - t.Cleanup(f.srv.Close) - return f -} - -// authenticatorFor wires a TokenReviewAuthenticator at the fake API server. -func authenticatorFor(t *testing.T, f *fakeAPIServer) *TokenReviewAuthenticator { - t.Helper() - a, err := NewTokenReviewAuthenticator(f.srv.URL, testAudience, nil, - WithReviewHTTPClient(f.srv.Client()), - WithReviewerToken(func() (string, error) { return "mint-own-token", nil }), - ) - if err != nil { - t.Fatalf("NewTokenReviewAuthenticator: %v", err) - } - return a -} - -func requestWithSAToken(token string) *http.Request { - r := httptest.NewRequest(http.MethodPost, MintPath, strings.NewReader("{}")) - if token != "" { - r.Header.Set(ServiceAccountTokenHeader, token) - } - return r -} - -func authenticatedStatus() tokenReviewStatus { - return tokenReviewStatus{ - Authenticated: true, - User: tokenReviewUser{Username: testSAName, UID: "uid-1"}, - Audiences: []string{testAudience}, - } -} - -// --- the happy path, and what it establishes --------------------------------- - -func TestTokenReviewYieldsTheServiceAccountIdentity(t *testing.T) { - f := newFakeAPIServer(t, authenticatedStatus()) - a := authenticatorFor(t, f) - - id, err := a.Authenticate(requestWithSAToken("caller-projected-token")) - if err != nil { - t.Fatalf("Authenticate: %v", err) - } - if id.Kind != KindServiceAccount { - t.Errorf("Kind = %q, want %q", id.Kind, KindServiceAccount) - } - if id.Name != testSAName { - t.Errorf("Name = %q, want %q", id.Name, testSAName) - } - // The whole point of the finding: this is a name, not "someone held a - // secret". It must be distinguishable from the shared-secret identity. - if id.Name == SharedSecretIdentityName { - t.Error("TokenReview produced the indistinguishable shared-secret identity") - } - if id.String() != KindServiceAccount+":"+testSAName { - t.Errorf("audit rendering = %q", id.String()) - } -} - -func TestTokenReviewSendsAnAudienceScopedReview(t *testing.T) { - f := newFakeAPIServer(t, authenticatedStatus()) - a := authenticatorFor(t, f) - - if _, err := a.Authenticate(requestWithSAToken("caller-projected-token")); err != nil { - t.Fatalf("Authenticate: %v", err) - } - if f.gotPath != tokenReviewPath { - t.Errorf("posted to %q, want %q", f.gotPath, tokenReviewPath) - } - if f.gotSpec.Token != "caller-projected-token" { - t.Errorf("reviewed token = %q, want the caller's token", f.gotSpec.Token) - } - if len(f.gotSpec.Audiences) != 1 || f.gotSpec.Audiences[0] != testAudience { - t.Errorf("review audiences = %v, want [%s] — an unscoped review would accept an API-server token", f.gotSpec.Audiences, testAudience) - } - // The mint authenticates ITSELF with its own token, never the caller's. - if f.gotAuthorization != authScheme+"mint-own-token" { - t.Errorf("reviewer Authorization = %q, want the mint's own token", f.gotAuthorization) - } -} - -func TestTokenReviewAcceptsABearerPrefixedHeader(t *testing.T) { - f := newFakeAPIServer(t, authenticatedStatus()) - a := authenticatorFor(t, f) - - if _, err := a.Authenticate(requestWithSAToken("Bearer caller-projected-token")); err != nil { - t.Fatalf("Authenticate with Bearer prefix: %v", err) - } - if f.gotSpec.Token != "caller-projected-token" { - t.Errorf("reviewed token = %q — the Bearer prefix should be stripped, not reviewed", f.gotSpec.Token) - } -} - -// --- the ways this could be wrong -------------------------------------------- - -// TestTokenReviewRefusesUnvalidatedAudience is the most important test here. -// -// An API server whose authenticators do not implement audience validation -// answers `authenticated: true` with the audiences field absent. A backend that -// checked only `authenticated` would accept a token minted for the API SERVER — -// which every pod already has at a well-known path — and audience scoping would -// be decorative. -func TestTokenReviewRefusesUnvalidatedAudience(t *testing.T) { - cases := []struct { - name string - audiences []string - }{ - {"absent audiences (API server did not validate)", nil}, - {"empty audiences (no intersection)", []string{}}, - {"a different audience", []string{"https://kubernetes.default.svc"}}, - {"our audience only as a prefix", []string{testAudience + "-other"}}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - st := authenticatedStatus() - st.Audiences = tc.audiences - f := newFakeAPIServer(t, st) - a := authenticatorFor(t, f) - - if _, err := a.Authenticate(requestWithSAToken("some-token")); err == nil { - t.Fatal("accepted a token the API server did not validate for the mint's audience") - } - }) - } - - // And the case that must still pass: our audience among several. - st := authenticatedStatus() - st.Audiences = []string{"other", testAudience} - f := newFakeAPIServer(t, st) - if _, err := authenticatorFor(t, f).Authenticate(requestWithSAToken("some-token")); err != nil { - t.Fatalf("refused a token whose returned audiences include ours: %v", err) - } -} - -func TestTokenReviewRefusesNonServiceAccountUsernames(t *testing.T) { - cases := []string{ - "alice", // a human with a kubeconfig - "system:node:worker-1", // a kubelet - "system:anonymous", // - "system:serviceaccount:", // malformed: no namespace or name - "system:serviceaccount:hive", // malformed: no name - "system:serviceaccount::name", // malformed: empty namespace - "system:serviceaccount:hive:", // malformed: empty name - "", // nothing at all - } - for _, username := range cases { - t.Run(username, func(t *testing.T) { - st := authenticatedStatus() - st.User.Username = username - f := newFakeAPIServer(t, st) - if _, err := authenticatorFor(t, f).Authenticate(requestWithSAToken("some-token")); err == nil { - t.Fatalf("accepted username %q as a ServiceAccount identity", username) - } - }) - } -} - -func TestTokenReviewRefusesUnauthenticatedAndErrored(t *testing.T) { - st := authenticatedStatus() - st.Authenticated = false - f := newFakeAPIServer(t, st) - if _, err := authenticatorFor(t, f).Authenticate(requestWithSAToken("bad")); err == nil { - t.Error("accepted a token the API server said was not authenticated") - } - - // authenticated:true WITH an error string set is contradictory; refuse. - st = authenticatedStatus() - st.Error = "token expired" - f = newFakeAPIServer(t, st) - if _, err := authenticatorFor(t, f).Authenticate(requestWithSAToken("expired")); err == nil { - t.Error("accepted a review that carried an error string") - } -} - -func TestTokenReviewRefusesAMissingHeader(t *testing.T) { - f := newFakeAPIServer(t, authenticatedStatus()) - a := authenticatorFor(t, f) - - if _, err := a.Authenticate(requestWithSAToken("")); err == nil { - t.Error("accepted a request with no ServiceAccount token") - } - if f.gotPath != "" { - t.Error("called the API server for a request that carried no token at all") - } -} - -// TestTokenReviewNeverReviewsTheAuthorizationHeader guards a leak that would be -// invisible in behaviour: in a dual-accept deployment Authorization carries the -// SHARED SECRET, and reviewing it would write the mint's secret into the -// Kubernetes audit log. -func TestTokenReviewNeverReviewsTheAuthorizationHeader(t *testing.T) { - f := newFakeAPIServer(t, authenticatedStatus()) - a := authenticatorFor(t, f) - - r := requestWithSAToken("caller-projected-token") - r.Header.Set("Authorization", authScheme+"the-shared-secret") - if _, err := a.Authenticate(r); err != nil { - t.Fatalf("Authenticate: %v", err) - } - if strings.Contains(f.gotSpec.Token, "the-shared-secret") { - t.Fatal("the caller's Authorization header was forwarded to the API server as a token to review") - } - if f.gotAuthorization == authScheme+"the-shared-secret" { - t.Fatal("the caller's Authorization header was replayed as the mint's own reviewer credential") - } -} - -// TestTokenReviewFailsClosed: every infrastructure failure refuses the caller. -// A TokenReview that failed open would be strictly worse than the shared secret -// it replaces, because operators would believe identity was being checked. -func TestTokenReviewFailsClosed(t *testing.T) { - t.Run("API server refuses the mint's own review call", func(t *testing.T) { - f := newFakeAPIServer(t, authenticatedStatus()) - f.httpStatus = http.StatusForbidden // mint's SA lacks create on tokenreviews - if _, err := authenticatorFor(t, f).Authenticate(requestWithSAToken("good-token")); err == nil { - t.Error("authenticated a caller although the review call itself was refused") - } - }) - - t.Run("API server unreachable", func(t *testing.T) { - f := newFakeAPIServer(t, authenticatedStatus()) - a := authenticatorFor(t, f) - f.srv.Close() // dial will fail - if _, err := a.Authenticate(requestWithSAToken("good-token")); err == nil { - t.Error("authenticated a caller with the API server down") - } - }) - - t.Run("reviewer token unreadable", func(t *testing.T) { - f := newFakeAPIServer(t, authenticatedStatus()) - a, err := NewTokenReviewAuthenticator(f.srv.URL, testAudience, nil, - WithReviewHTTPClient(f.srv.Client()), - WithReviewerToken(func() (string, error) { return "", nil }), - ) - if err != nil { - t.Fatalf("NewTokenReviewAuthenticator: %v", err) - } - if _, err := a.Authenticate(requestWithSAToken("good-token")); err == nil { - t.Error("authenticated a caller although the mint has no credential to review with") - } - }) - - t.Run("garbage response body", func(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - _, _ = w.Write([]byte("not json")) - })) - defer srv.Close() - a, err := NewTokenReviewAuthenticator(srv.URL, testAudience, nil, - WithReviewHTTPClient(srv.Client()), - WithReviewerToken(func() (string, error) { return "t", nil }), - ) - if err != nil { - t.Fatalf("NewTokenReviewAuthenticator: %v", err) - } - if _, err := a.Authenticate(requestWithSAToken("good-token")); err == nil { - t.Error("authenticated a caller from an unparseable review response") - } - }) -} - -// --- construction ------------------------------------------------------------ - -func TestNewTokenReviewAuthenticatorValidatesItsInputs(t *testing.T) { - if _, err := NewTokenReviewAuthenticator("", testAudience, nil); err == nil { - t.Error("accepted an empty API server URL") - } - // An empty audience is the silent downgrade this backend exists to prevent: - // the review would carry no audience and verifyAudience nothing to check. - if _, err := NewTokenReviewAuthenticator("https://api", "", nil); err == nil { - t.Error("accepted an empty audience — an unscoped review accepts API-server tokens") - } - if _, err := NewTokenReviewAuthenticator("https://api", testAudience, []byte("not a certificate")); err == nil { - t.Error("accepted a CA bundle with no usable certificate") - } - a, err := NewTokenReviewAuthenticator("https://api/", testAudience, nil) - if err != nil { - t.Fatalf("NewTokenReviewAuthenticator: %v", err) - } - if a.apiURL != "https://api" { - t.Errorf("trailing slash not trimmed: %q", a.apiURL) - } - if a.Name() != KindServiceAccount { - t.Errorf("Name() = %q", a.Name()) - } - // Nil options must not be able to clear the verified defaults. - before := a.client - WithReviewHTTPClient(nil)(a) - WithReviewerToken(nil)(a) - if a.client != before || a.reviewerToken == nil { - t.Error("a nil option cleared a default it should have left alone") - } -} - -func TestNewInClusterTokenReviewAuthenticatorRefusesOutsideACluster(t *testing.T) { - t.Setenv("KUBERNETES_SERVICE_HOST", "") - t.Setenv("KUBERNETES_SERVICE_PORT", "") - if _, err := NewInClusterTokenReviewAuthenticator(testAudience); err == nil { - t.Error("built an in-cluster authenticator outside a cluster — it must fail rather than degrade") - } -} - -// --- dual-accept -------------------------------------------------------------- - -func TestMultiAuthenticatorPrefersTheRealIdentity(t *testing.T) { - f := newFakeAPIServer(t, authenticatedStatus()) - tr := authenticatorFor(t, f) - secret, err := NewSharedSecretAuthenticator(testSecret) - if err != nil { - t.Fatalf("NewSharedSecretAuthenticator: %v", err) - } - multi, err := NewMultiAuthenticator(tr, secret) - if err != nil { - t.Fatalf("NewMultiAuthenticator: %v", err) - } - - if got, want := multi.Name(), KindServiceAccount+"+"+KindSharedSecret; got != want { - t.Errorf("Name() = %q, want %q", got, want) - } - - // A caller presenting BOTH must be recorded under its real identity — an - // audit log that still says "any-holder" cannot show a migration finishing. - r := requestWithSAToken("caller-projected-token") - r.Header.Set("Authorization", authScheme+testSecret) - id, err := multi.Authenticate(r) - if err != nil { - t.Fatalf("Authenticate: %v", err) - } - if id.Kind != KindServiceAccount { - t.Errorf("a caller with both credentials was recorded as %q, want the ServiceAccount", id.Kind) - } - - // A legacy caller with only the secret still works — that is the whole - // point of dual-accept. - legacy := httptest.NewRequest(http.MethodPost, MintPath, strings.NewReader("{}")) - legacy.Header.Set("Authorization", authScheme+testSecret) - id, err = multi.Authenticate(legacy) - if err != nil { - t.Fatalf("legacy caller refused: %v", err) - } - if id.Kind != KindSharedSecret { - t.Errorf("legacy caller recorded as %q", id.Kind) - } - - // A caller with neither is refused. - if _, err := multi.Authenticate(httptest.NewRequest(http.MethodPost, MintPath, strings.NewReader("{}"))); err == nil { - t.Error("authenticated a caller presenting no credential at all") - } -} - -func TestNewMultiAuthenticatorNeedsABackend(t *testing.T) { - if _, err := NewMultiAuthenticator(); err == nil { - t.Error("built an authenticator with no backends — it would 401 everything and read as an outage") - } - if _, err := NewMultiAuthenticator(nil, nil); err == nil { - t.Error("built an authenticator from nothing but nils") - } - secret, _ := NewSharedSecretAuthenticator(testSecret) - m, err := NewMultiAuthenticator(nil, secret, nil) - if err != nil { - t.Fatalf("nil backends should be dropped, not fatal: %v", err) - } - if m.Name() != KindSharedSecret { - t.Errorf("Name() = %q, want the surviving backend", m.Name()) - } -} - -// --- end to end through the server ------------------------------------------- - -// TestServerMintsUnderServiceAccountEntitlement is the payoff: an entitlement -// map keyed on a real ServiceAccount name, enforced deny-by-default. Under the -// shared secret this could not be written down at all, because there was only -// ever one identity to key on. -func TestServerMintsUnderServiceAccountEntitlement(t *testing.T) { - f := newFakeAPIServer(t, authenticatedStatus()) - m, _ := newTestMinter(t) - srv, err := NewServer(m, testSecret, nil, - WithAuthenticator(authenticatorFor(t, f)), - WithEntitlements(Entitlements{ - testSAName: { - Subjects: []string{testSub}, - Audiences: []string{testAud}, - Scopes: []string{"registry:pull"}, - }, - }), - ) - if err != nil { - t.Fatalf("NewServer: %v", err) - } - - post := func(req MintRequest) int { - body, _ := json.Marshal(req) - r := httptest.NewRequest(http.MethodPost, MintPath, strings.NewReader(string(body))) - r.Header.Set(ServiceAccountTokenHeader, "caller-projected-token") - w := httptest.NewRecorder() - srv.Handler().ServeHTTP(w, r) - return w.Code - } - - if got := post(MintRequest{Subject: testSub, Audience: testAud, Scopes: []string{"registry:pull"}}); got != http.StatusOK { - t.Errorf("entitled mint returned %d, want 200", got) - } - // Outside the entitlement: authenticated, but not permitted. 403, not 401. - if got := post(MintRequest{Subject: "hub-admin", Audience: testAud}); got != http.StatusForbidden { - t.Errorf("unentitled subject returned %d, want 403", got) - } - if got := post(MintRequest{Subject: testSub, Audience: testAud, Scopes: []string{"registry:push"}}); got != http.StatusForbidden { - t.Errorf("unentitled scope returned %d, want 403", got) - } - - // A caller with the shared secret but no SA token is now refused outright: - // the server was built with the TokenReview authenticator alone. - body, _ := json.Marshal(MintRequest{Subject: testSub, Audience: testAud}) - r := httptest.NewRequest(http.MethodPost, MintPath, strings.NewReader(string(body))) - r.Header.Set("Authorization", authScheme+testSecret) - w := httptest.NewRecorder() - srv.Handler().ServeHTTP(w, r) - if w.Code != http.StatusUnauthorized { - t.Errorf("shared-secret caller returned %d against a TokenReview-only server, want 401", w.Code) - } -}