diff --git a/.gitignore b/.gitignore index 5ccd2b3f..eb48d448 100644 --- a/.gitignore +++ b/.gitignore @@ -44,3 +44,6 @@ tmp/ .env .env.* + +# gstack scratch output (browse/QA session logs) +.gstack/ diff --git a/config.go b/config.go index 144ec6b3..0b095041 100644 --- a/config.go +++ b/config.go @@ -106,6 +106,22 @@ type CIMDConfig struct { // (recommended for enterprise/closed deployments). YAML only (a list), e.g.: // cimd: // allowed_domains: ["client.example.com", "apps.acme.dev"] + // + // It is not only a fetch lever. It also decides whether a CIMD client is a + // destination this AS will redirect a user agent to — both the RFC 6749 + // §4.1.2.1 error redirect and the interactive-login redirect are refused for + // a self-asserted client heading for a REMOTE https destination while this is + // empty. Loopback and private-use callbacks are exempt — they deliver to the + // requester's own device — so the ordinary desktop/CLI MCP client is + // unaffected and completes with no allowlist. Set this if you serve CIMD + // clients whose callbacks are remote https URLs; without it they can never + // sign a user in. + // + // Listing a host asserts that you vet who publishes there, and ZeroID holds + // the destination to the same bar: an https redirect_uri must be on the + // client_id's own host or on this list. Do not list a host where anyone can + // publish a path (user content, a raw-file CDN, a broadly writable bucket) — + // there, "allow-listed publisher" stops meaning "vetted party". AllowedDomains []string `koanf:"allowed_domains"` // AllowPrivateMetadataEndpoints relaxes the SSRF guard on the metadata diff --git a/docs/cimd.md b/docs/cimd.md index 52eeab8a..240f2fa5 100644 --- a/docs/cimd.md +++ b/docs/cimd.md @@ -63,20 +63,67 @@ to carry application state only. ZeroID round-trips it verbatim when present and does not require it. It is shown because most clients have somewhere to return the user to. -**The browser leg needs a GET-capable `PrincipalResolver`, which ZeroID does not -ship.** A browser cannot set a custom header on a top-level navigation, and the -resolver-facing `Form` accessor is bound to the POST body, so a resolver that -reads `req.Form(...)` sees nothing on a GET. The deployer must register one that -reads a session cookie (`req.Cookie(...)`) and own the login and consent screens -behind it. +**Serving the browser leg *directly* at `/oauth2/authorize` needs a GET-capable +`PrincipalResolver`, which ZeroID does not ship.** A browser cannot set a custom +header on a top-level navigation, and the resolver-facing `Form` accessor is +bound to the POST body, so a resolver that reads `req.Form(...)` sees nothing on +a GET. + +Direct access is not the only shape, though. There are two ways to connect the +browser leg, and the second — which needs no GET-capable resolver at all — is +usually the better one: + +1. **Register a cookie-reading resolver** (`req.Cookie(...)`) and own the login + and consent screens behind it. This makes `/oauth2/authorize` itself the + browser-facing endpoint, which brings the CSRF obligations described below — + `SameSite=Lax` still sends the cookie on a cross-site top-level navigation, + and CIMD accepts an attacker-published `client_id` with its own + `redirect_uri`. + + ZeroID will send the user to that login screen for you: return + `ErrPrincipalInteractionRequired` from the resolver when there is no session + and register the surface with `Server.SetInteractiveLoginURL`. See the + resolver bullet below for what that does and does not do — in particular + **for a CIMD client it is refused only when the `redirect_uri` is a remote + `https://` host that `cimd.allowed_domains` does not cover.** A loopback or + private-use callback — the ordinary desktop/CLI MCP client — is exempt and + completes this route with no allowlist. A hosted client with a real `https://` + callback needs you to name the hosts that may publish. +2. **Front the browser leg above ZeroID and hand off over POST.** Your own + surface owns the redirect, authenticates the human however you already do, + and then POSTs to `/oauth2/authorize` with a credential a form-based resolver + reads — an RFC 7523 assertion signed by that surface, say, verified against + its published JWKS. The browser never reaches this endpoint, so no GET-capable + resolver is needed. + + What route 2 does **not** remove is authorization-request CSRF — it moves it + to your surface. An attacker can still navigate a victim's browser to that + surface with an attacker-published `client_id`; if it authenticates from a + `SameSite=Lax` session cookie and mints the assertion without further + interaction, the same code is issued for the victim, one hop earlier. The + CSRF-protected interaction — an explicit consent gesture behind an + anti-forgery token — has to happen at the fronting surface before the + assertion is minted. What the route removes is the exposure at + `/oauth2/authorize` itself, which is no longer reachable by navigation. + +Highflame's own deployment takes route 2 — Studio authenticates the user, mints +an assertion, and POSTs; AuthN's assertion resolver verifies it and ZeroID mints +the code. (For MCP clients specifically that routing is in flight: today Studio +mints their codes locally with its own CIMD check, and highflame-studio#1392 +brings them back through this path.) Route 1 exists for deployers with no such +surface. Either way ZeroID stays the engine: it validates the CIMD document, +enforces the `redirect_uri` allow-list, and issues the code. **ZeroID cannot detect this for you.** Its AS metadata omits the `authorization_code` grant when *no* resolver is registered, but it cannot introspect what a registered resolver reads — so a deployment whose resolvers are -all form-based advertises the grant and then 401s every browser redirect. If that -is you, call `Server.SetAuthorizationCodeAvailable(func() bool { return false })` -until a GET-capable resolver exists; otherwise the metadata promises a flow the +all form-based *and* has no fronting surface advertises the grant and then 401s +every browser redirect. If that is you, call +`Server.SetAuthorizationCodeAvailable(func() bool { return false })` until one of +the two routes above exists; otherwise the metadata promises a flow the endpoint cannot finish, which is exactly the failure this is meant to prevent. +(A route-2 deployment is fine as-is: its form-based resolver *is* the browser +leg's back end, fed by the surface.) A `false` answer turns the flow **off**, not merely unadvertised: `/oauth2/authorize` answers 503 on both GET and POST. Reach for it if you run a @@ -85,7 +132,7 @@ cookie resolver is safe while POST is the only route, because `SameSite=Lax` withholds the cookie on a cross-site POST, and becomes reachable by cross-site top-level navigation once GET is mounted. -**Errors are not redirected to a CIMD client.** RFC 6749 §4.1.2.1 says report most +**Errors are not redirected to an *unvetted, remote* CIMD destination.** RFC 6749 §4.1.2.1 says report most `/oauth2/authorize` failures by redirecting to the client's registered `redirect_uri`, and ZeroID does — for clients somebody registered. A CIMD client's `redirect_uris` come from a document it published itself, so with `allowed_domains` @@ -96,11 +143,21 @@ your origin. CIMD clients therefore get the §5.2 JSON body instead, and the interactive-login redirect is refused for them too — an unvetted client does not get to borrow your login surface's credibility. -The cost is real and worth naming: a browser-driven CIMD client cannot learn its -error from the callback and has to read the JSON body. Setting -`cimd.allowed_domains` restores the redirect, because vetting which hosts may -publish restores the assumption §4.1.2.1 is built on. The gate is provenance, not -CIMD. +**A loopback or private-use `redirect_uri` is exempt, and that is most MCP +clients.** The threat above is about a *remote* destination — an unauthenticated +redirector with your origin as the first hop. A 302 to `127.0.0.1` has no remote +hop: the code lands on the machine the user is sitting at, and an attacker able +to listen there already has local code execution. RFC 8252 §7.3 accepts loopback +callbacks from clients nobody registered for exactly that reason, and CIMD does +not weaken it. Since the document shape at the top of this page — the ordinary +desktop/CLI MCP client — lists only loopback callbacks, the carve-out simply does +not apply to it, and its browser leg works with no allowlist configured. + +The cost, for the clients it does apply to: a browser-driven CIMD client with a +real `https://` callback cannot learn its error from that callback and has to +read the JSON body. Setting `cimd.allowed_domains` restores the redirect there +too, because vetting which hosts may publish restores the assumption §4.1.2.1 is +built on. The gate is provenance and reach, not CIMD. **That hatch only works on a single-tenant deployment.** `allowed_domains` is one deployment-wide set — `domainAllowed` takes no tenant — so on a multi-tenant AS it @@ -127,8 +184,10 @@ Three things a deployer must handle: Only GET is redirected: a POST caller has no user agent. With no target configured the sentinel degrades to `access_denied`, because a resolver cannot - conjure a surface the deployment does not have — and it is refused outright for a - CIMD client, per the provenance rule above. + conjure a surface the deployment does not have — and it is refused for a CIMD + client headed to an unvetted *remote* destination, per the rule above. Loopback + and private-use callbacks are exempt, and `cimd.allowed_domains` lifts the + refusal for remote hosts. Same check as the error redirect, either way. Use `Server.Use` middleware instead if you want to own the whole interaction including the 302. @@ -196,11 +255,13 @@ Implemented in [`internal/service/cimd.go`](../internal/service/cimd.go); wired Two of these carry their own weight beyond conformance. Userinfo is the phishing shape — `https://legit.example.com@evil.example/client.json` resolves to `evil.example` while *reading* as `legit.example.com` on a consent screen or in an audit log. Dot segments would give one document many spellings, splitting the resolution cache and handing one client several identities the §4 self-reference check cannot distinguish. ZeroID also rejects a **query string**, where the draft says only SHOULD NOT. That is deliberate and stricter than required: the `client_id` a client presents must stay byte-identical to the URL the document was fetched from, which is exactly what the self-reference check compares. -3. **Domain policy.** If `cimd.allowed_domains` is configured, the host must be in it (exact, case-insensitive). Empty allowlist ⇒ any public HTTPS host — which is the **default**, and ZeroID warns at startup when CIMD is enabled without one. Note what this control can and cannot do: it constrains *which hosts may publish*, at domain granularity. It does not establish that the party presenting a `client_id` controls that document — CIMD has no proof of possession, so any client may present any published URL, and for a native client whose document lists a loopback `redirect_uri` the code is delivered to the presenter's own listener. Treat the allowlist as ecosystem scoping, not client authentication. +3. **Domain policy.** If `cimd.allowed_domains` is configured, the host must be in it (exact, case-insensitive). Empty allowlist ⇒ any public HTTPS host — which is the **default**, and ZeroID warns at startup when CIMD is enabled without one. Note what this control can and cannot do: it constrains *which hosts may publish*, at domain granularity — and, since it also gates redirects (§ below), *where documents from those hosts may send a user*. **Only list hosts whose publishing you control.** On a host where anyone can serve a path — user content, a raw-file CDN, a broadly writable bucket — allow-listing hands that party a vetted-client status the redirect gate then honours. It does not establish that the party presenting a `client_id` controls that document — CIMD has no proof of possession, so any client may present any published URL, and for a native client whose document lists a loopback `redirect_uri` the code is delivered to the presenter's own listener. Treat the allowlist as ecosystem scoping, not client authentication. 4. **Fetch (SSRF-guarded, no redirects).** `GET` via the same DNS-rebinding-safe client the OIDC attestation verifier and CIBA dispatch use ([`attestation.NewSSRFGuardedHTTPClient`](../internal/attestation/oidc.go)): the host is resolved once, every answer is checked against the private/loopback/link-local/multicast/CGN/reserved blocklist, and the connection is pinned to the validated IP. TLS still verifies against the original hostname. Response is size-capped (5 KiB default) and timeout-bounded (5 s). **HTTP redirects are not followed** — the `client_id` is a canonical location; a 3xx is a resolution failure. 5. **Validate the document.** - **Self-reference** (draft §4): the document's `client_id` field MUST equal the URL it was fetched from. This is what stops a document from claiming someone else's identity. - `redirect_uris` is **required and non-empty** — CIMD's primary anti-impersonation control. Each entry must satisfy OAuth 2.1 scheme rules: `https://`, loopback `http://`, or a private-use scheme (native apps); plaintext non-loopback `http://` is rejected. The requested `redirect_uri` is matched against the list by the existing `redirectURIAllowed` logic (exact match, with RFC 8252 §7.3 port-agnostic matching for loopback callbacks). + + When `cimd.allowed_domains` is set, an `https://` entry must additionally be on the `client_id`'s own host or on that list — a **deviation**, and the reason the allowlist can be trusted as the switch that restores redirects (see below). Allow-listing the *publisher* only vets the destination if the destination is vetted too; otherwise any host where more than one party can publish a path lets an attacker name `https://evil.example/cb` and collect codes from a real sign-in. Loopback and private-use schemes are exempt: they deliver to the caller's own machine, not to a published host. In open mode this constrains nothing, because open mode refuses those redirects outright. - `token_endpoint_auth_method` must be `none` (omitted defaults to `none`). **Confidential CIMD clients (`private_key_jwt`) are not supported in v1.** - `grant_types` defaults to `["authorization_code"]`, must include `authorization_code`, and may only contain `authorization_code` / `refresh_token`. - `response_types`, if present, must include `code`. diff --git a/docs/spec/zeroid-oauth-extensions.md b/docs/spec/zeroid-oauth-extensions.md index b1a3ba9a..1ad0093c 100644 --- a/docs/spec/zeroid-oauth-extensions.md +++ b/docs/spec/zeroid-oauth-extensions.md @@ -1005,6 +1005,14 @@ overrides the document. - `grant_types` defaults to `["authorization_code"]`, **MUST** include `authorization_code`, and may contain only `authorization_code` and `refresh_token`. +- When `cimd.allowed_domains` is non-empty, every `https://` `redirect_uris` + entry **MUST** be on the `client_id`'s own host or on that allow-list. **This + is a deviation**, and it is what makes the allow-list load-bearing for Section + 12.5: allow-listing a publisher vets the destination only if the destination is + vetted too, and on a host with multiple publishers it otherwise does not. + Loopback `http://` and private-use schemes are exempt — they resolve on the + caller's own device, not at a published host. Constrains nothing in open mode, + where those redirects are refused regardless. - `response_types`, when present, **MUST** include `code`. - Outer-shape validation only, by default. Per-type schema validation is opt-in through the `RegisterAuthorizationDetailValidator`-style hook pattern. @@ -1013,24 +1021,40 @@ A CIMD `client_id` is **not** accepted as an authenticated client at the introspection or revocation endpoints; the `none` advertised in those metadata arrays (Section 11.1) applies to *registered* public clients only. -### 12.5 Error reporting is not redirected +### 12.5 Error reporting is not redirected to an unvetted remote destination RFC 6749 §4.1.2.1 requires most authorization-endpoint failures to be reported by redirecting to the client's registered `redirect_uri`. ZeroID does that for -registered clients and **deliberately does not for CIMD clients**, which answer -with an RFC 6749 §5.2 JSON body instead. The interactive-authentication redirect +registered clients and **deliberately does not for CIMD clients whose publishing +host is not on an allow-list**, which answer with an RFC 6749 §5.2 JSON body +instead. The interactive-authentication redirect (`ErrPrincipalInteractionRequired`) is likewise refused for them. The rule presumes `redirect_uri` was vetted at registration. Under CIMD it is -self-asserted, and with no `allowed_domains` allow-list (Section 12.6, the default) +self-asserted, and with no `allowed_domains` allow-list (Section 12.7, the default) any host may publish a document naming any destination — so the redirect target is attacker-chosen. Honouring §4.1.2.1 there yields an unauthenticated open redirect from the authorization server's own origin, because the failure being reported is precisely "no credential was presented". Configuring `cimd.allowed_domains` re-establishes the vetting the rule assumes, and -error redirection applies again. The discriminator is registration provenance -(`registration_source`), not the CIMD mechanism. +both redirects apply again — the error redirect and the interactive-login one, +which are the same check. The discriminator is registration provenance +(`registration_source`) qualified by that allow-list, not the CIMD mechanism. + +A loopback `http://` or private-use `redirect_uri` is **exempt from both +refusals**, whatever the allow-list says. Those destinations resolve on the +requester's own device, so the redirect has no remote hop and no third-party +recipient — the property RFC 8252 §7.3 already relies on to accept such callbacks +from unregistered native clients. The carve-out's premise is a remote +attacker-chosen destination and does not hold for them. + +This is what makes a browser-driven CIMD client — the MCP 2025-11-25 case — +completable. The ordinary desktop/CLI client publishes loopback callbacks and is +therefore unaffected: it reaches the login surface and completes with no +allow-list. A deployment serving CIMD clients whose callbacks are remote +`https://` URLs **MUST** set `cimd.allowed_domains`, or those clients can never +sign a user in. `allowed_domains` is deployment-wide, with no tenant dimension, so this re-enablement is meaningful only where the deployment serves a single tenant. A diff --git a/internal/handler/authorize.go b/internal/handler/authorize.go index 4422a5cf..7aba4478 100644 --- a/internal/handler/authorize.go +++ b/internal/handler/authorize.go @@ -475,12 +475,15 @@ func (a *API) authorizeHandler(w http.ResponseWriter, r *http.Request) { // caller is a CLI or a server posting an assertion. // - No target configured. The deployment has no login surface, so there is // nowhere to go. -// - A self-asserted (CIMD) client. Sending a user through the deployment's real -// login page on behalf of a client nobody vetted is the more damaging half of -// the same problem failAuthorize declines: the victim authenticates for real, -// and the flow resumes toward an attacker-published redirect_uri. Refusing -// here means an unvetted client cannot borrow the login surface's credibility. -// Set cimd.allowed_domains to vet the publishing hosts and this applies again. +// - A self-asserted (CIMD) client heading for an unvetted REMOTE destination. +// Sending a user through the deployment's real login page on behalf of such a +// client is the more damaging half of the same problem failAuthorize declines: +// the victim authenticates for real, and the flow resumes toward an +// attacker-published redirect_uri. A loopback or private-use redirect_uri is +// not that — the code lands on the user's own machine — so those proceed, which +// is what keeps the native/CLI/MCP browser leg working. Setting +// cimd.allowed_domains lifts the refusal for remote hosts too. See +// refusesRedirectTo. // // The return_to it appends is rebuilt from the VALIDATED protocol parameters, not // copied from the inbound URL. That is deliberate: the inbound query is @@ -499,7 +502,7 @@ func (a *API) redirectToInteractiveLogin( client *domain.OAuthClient, resolverName string, ) bool { if r.Method != http.MethodGet || a.interactiveLoginURL == nil || - client == nil || client.SelfAsserted() { + a.refusesRedirectTo(client, req.RedirectURI) { return false } @@ -582,9 +585,13 @@ func (a *API) redirectToInteractiveLogin( // target nobody vetted. Registered and dynamically-registered clients — where // somebody did — are unaffected and get the conformant redirect. // -// Deployers who want CIMD clients to receive redirects can restore them by -// setting cimd.allowed_domains, which re-establishes the vetting the rule assumes; -// see docs/cimd.md. The gate is provenance, not the CIMD feature itself. +// Two things narrow that deviation, because it is provenance-and-reach, not the +// CIMD feature itself. A loopback or private-use redirect_uri is redirected +// normally — it delivers to the caller's own device, so there is no third party +// to hand an error or a code to, which is the same reasoning RFC 8252 §7.3 uses +// to accept those callbacks from unregistered native clients. And setting +// cimd.allowed_domains restores redirects to remote hosts as well, by +// re-establishing the vetting the rule assumes. See docs/cimd.md. // // jsonCode vs redirectCode: the wire code sometimes has to differ between the // two shapes. A failed resolver is invalid_client to a programmatic POST caller @@ -601,7 +608,7 @@ func (a *API) failAuthorize( w http.ResponseWriter, r *http.Request, req *service.AuthorizeRequest, client *domain.OAuthClient, status int, jsonCode, redirectCode, description string, ) { - if r.Method != http.MethodGet || client == nil || client.SelfAsserted() { + if r.Method != http.MethodGet || a.refusesRedirectTo(client, req.RedirectURI) { writeAuthorizeError(w, status, jsonCode, description) return diff --git a/internal/handler/authorize_selfasserted_test.go b/internal/handler/authorize_selfasserted_test.go index 8bd2d99c..a1309f69 100644 --- a/internal/handler/authorize_selfasserted_test.go +++ b/internal/handler/authorize_selfasserted_test.go @@ -5,6 +5,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "strings" "testing" "github.com/highflame-ai/zeroid/domain" @@ -159,3 +160,150 @@ func TestSelfAsserted_InteractiveLoginIsRefused(t *testing.T) { } }) } + +// TestVettedPublishers_RestoresRedirects is the other side of the carve-out, and +// the reason it is a carve-out rather than a ban. +// +// The refusal above is not about CIMD the feature — it is about a destination +// nobody vetted. cimd.allowed_domains is the deployer naming which hosts may +// publish a metadata document, which restores exactly the assumption §4.1.2.1's +// redirect rule is built on. So with an allow-list in force a CIMD client is a +// redirect destination like any other, and both refusals lift together. +// +// This test exists because the behaviour was documented in three places — +// failAuthorize's godoc, redirectToInteractiveLogin's, and docs/cimd.md — before +// it was implemented, and a promise nobody executes is the kind that rots. +func TestVettedPublishers_RestoresRedirects(t *testing.T) { + get := httptest.NewRequest(http.MethodGet, "/oauth2/authorize", nil) + + vetted := func() *API { + api := &API{issuer: "https://as.example.test"} + api.SetCIMDPublishersVetted(true) + api.SetInteractiveLoginURL(func(*service.AuthorizeRequest) string { return saLoginURL }) + + return api + } + + t.Run("error redirects to the CIMD client", func(t *testing.T) { + rec := httptest.NewRecorder() + vetted().failAuthorize(rec, get, saRequest(), cimdClient(), + http.StatusUnauthorized, oautherror.InvalidClient, oautherror.AccessDenied, "no credential") + + if rec.Code != http.StatusFound { + t.Fatalf("an allow-list vets the publisher, so §4.1.2.1 applies again; got %d", rec.Code) + } + + loc, err := url.Parse(rec.Header().Get("Location")) + if err != nil { + t.Fatalf("Location did not parse: %v", err) + } + + if got := loc.Query().Get("error"); got != oautherror.AccessDenied { + t.Errorf("error = %q, want access_denied", got) + } + + if got := loc.Query().Get("state"); got != "sa-state" { + t.Errorf("state = %q, want it echoed back", got) + } + }) + + t.Run("interactive login is reachable", func(t *testing.T) { + rec := httptest.NewRecorder() + + if !vetted().redirectToInteractiveLogin(rec, get, saRequest(), cimdClient(), "session") { + t.Fatal("with vetted publishers a CIMD client must reach the login surface — " + + "this is what makes the MCP browser leg completable") + } + + if rec.Header().Get("Location") == "" { + t.Fatal("expected a Location to the login surface") + } + }) + + // The gate is the allow-list, not the client: a nil client is still refused, + // and a non-GET still gets JSON. Pinned so "vetted" never becomes a blanket + // bypass of the other two conditions. + t.Run("vetting does not bypass the other refusals", func(t *testing.T) { + rec := httptest.NewRecorder() + vetted().failAuthorize(rec, get, saRequest(), nil, + http.StatusUnauthorized, oautherror.InvalidClient, oautherror.AccessDenied, "no credential") + + if loc := rec.Header().Get("Location"); loc != "" { + t.Fatalf("a nil client must not be redirected, got %q", loc) + } + + post := httptest.NewRequest(http.MethodPost, "/oauth2/authorize", nil) + if vetted().redirectToInteractiveLogin(httptest.NewRecorder(), post, saRequest(), cimdClient(), "session") { + t.Fatal("POST has no user agent to redirect, allow-list or not") + } + }) +} + +// TestSelfAsserted_LocalDeliveryIsRedirected is the exemption that makes the +// carve-out proportionate, and it is the one that decides whether MCP works. +// +// The carve-out's stated threat is an unauthenticated redirector "with the AS's +// own origin as the first hop" toward an attacker-published destination. That is +// a claim about a REMOTE host. A 302 to 127.0.0.1 has no remote hop: the code +// lands on the machine the user is sitting at, and an attacker who can listen +// there already has local code execution. RFC 8252 §7.3 accepts loopback +// callbacks from clients nobody registered for exactly this reason. +// +// It is not a corner case. The canonical MCP document in docs/cimd.md lists +// loopback callbacks and nothing else, so refusing here would cost the whole +// browser leg for the dominant client shape while preventing nothing. +func TestSelfAsserted_LocalDeliveryIsRedirected(t *testing.T) { + // No allow-list: the exemption must stand on its own, not ride on vetting. + api := &API{issuer: "https://as.example.test"} + api.SetInteractiveLoginURL(func(*service.AuthorizeRequest) string { return saLoginURL }) + + get := httptest.NewRequest(http.MethodGet, "/oauth2/authorize", nil) + + local := func(uri string) *service.AuthorizeRequest { + r := saRequest() + r.RedirectURI = uri + + return r + } + + for _, ru := range []string{ + "http://127.0.0.1:3000/callback", + "http://localhost:3000/callback", + "myapp:/cb", + } { + t.Run("error redirects to "+ru, func(t *testing.T) { + rec := httptest.NewRecorder() + api.failAuthorize(rec, get, local(ru), cimdClient(), + http.StatusUnauthorized, oautherror.InvalidClient, oautherror.AccessDenied, "no credential") + + if rec.Code != http.StatusFound { + t.Fatalf("%s delivers to the caller's own device; refusing buys nothing, got %d", ru, rec.Code) + } + + if loc := rec.Header().Get("Location"); !strings.HasPrefix(loc, ru) { + t.Errorf("Location = %q, want it to start with %q", loc, ru) + } + }) + + t.Run("interactive login is reachable for "+ru, func(t *testing.T) { + rec := httptest.NewRecorder() + + if !api.redirectToInteractiveLogin(rec, get, local(ru), cimdClient(), "session") { + t.Fatalf("%s: a native CIMD client must be able to sign a user in", ru) + } + }) + } + + // The exemption is about reach, so a remote destination stays refused even + // though the client is otherwise identical. This is the line that keeps the + // carve-out meaningful. + t.Run("remote https is still refused", func(t *testing.T) { + rec := httptest.NewRecorder() + api.failAuthorize(rec, get, local("https://evil.example/cb"), cimdClient(), + http.StatusUnauthorized, oautherror.InvalidClient, oautherror.AccessDenied, "no credential") + + if loc := rec.Header().Get("Location"); loc != "" { + t.Fatalf("a remote attacker-chosen destination must not be redirected to, got %q", loc) + } + }) +} diff --git a/internal/handler/routes.go b/internal/handler/routes.go index 1067a53f..d64b1508 100644 --- a/internal/handler/routes.go +++ b/internal/handler/routes.go @@ -15,6 +15,7 @@ import ( gojson "github.com/goccy/go-json" "github.com/uptrace/bun" + "github.com/highflame-ai/zeroid/domain" "github.com/highflame-ai/zeroid/internal/attestation" "github.com/highflame-ai/zeroid/internal/service" "github.com/highflame-ai/zeroid/internal/signing" @@ -55,6 +56,15 @@ type API struct { // issuance (token.require_dpop). Atomic because tests toggle it while the // server is handling requests; defaults false (Bearer fallback stands). dpopRequired atomic.Bool + // cimdPublishersVetted reports whether cimd.allowed_domains names at least + // one host, i.e. whether a deployer decided which hosts may publish a + // metadata document. It is the gate that lets a CIMD client be treated as + // a redirect destination — see failAuthorize and redirectToInteractiveLogin, + // which refuse a self-asserted client only while this is false. Set via + // SetCIMDPublishersVetted from the EFFECTIVE allow-list, not the raw config + // slice: NewCIMDService lower-cases and drops blank entries, so + // allowed_domains: [""] has length 1 and vets nothing. + cimdPublishersVetted bool // authorizationCodeAvailable reports whether /oauth2/authorize can // actually serve a request — i.e. whether the deployer registered any @@ -288,6 +298,57 @@ func (a *API) SetCIMDEnabled(enabled bool) { a.cimdEnabled = enabled } +// SetCIMDPublishersVetted records whether cimd.allowed_domains is in force, +// which decides whether a CIMD client may be redirected to — its error +// redirect per RFC 6749 §4.1.2.1, and the interactive-login redirect. +// +// Both are refused for a self-asserted client by default because a CIMD +// document's redirect_uris are attacker-CHOSEN, not merely attacker-supplied: +// with no allow-list, any public HTTPS host can publish one. An allow-list is +// the deployer saying which hosts may publish, which restores the vetting +// §4.1.2.1's redirect rule assumes — so the refusal lifts with it. +// +// Called by Server.NewServer with CIMDService.AllowedDomainCount() > 0. A bool +// rather than a predicate because the allow-list comes from config and cannot +// change after construction, unlike the resolver registry. +func (a *API) SetCIMDPublishersVetted(vetted bool) { + a.cimdPublishersVetted = vetted +} + +// refusesRedirectTo reports whether redirectURI is a destination this deployment +// declines to send a user agent to on behalf of client. The single predicate +// behind both failAuthorize's §4.1.2.1 carve-out and redirectToInteractiveLogin's +// refusal, so the two cannot drift: they answer the same question about the same +// request. +// +// The question is whether the destination can reach a THIRD PARTY the client +// chose for itself. Three ways it cannot, in the order they matter: +// +// - Local delivery. A loopback or private-use redirect_uri resolves on the +// machine the user is sitting at, so there is no remote hop and nobody but +// the caller receives anything — see service.RedirectDeliversLocally. This is +// the native/CLI/MCP shape and the reason most CIMD clients are unaffected by +// the carve-out at all. +// - Not self-asserted. Somebody vetted the client at registration. +// - Vetted publisher. cimd.allowed_domains names who may publish, and +// redirectHostsAllowed holds https destinations to that same list. +// +// A nil client refuses. Both callers happen to check that themselves, but a +// security predicate that answers "go ahead" for the case where there is no +// client to reason about is the wrong default to leave lying around for the +// third caller. +func (a *API) refusesRedirectTo(client *domain.OAuthClient, redirectURI string) bool { + if client == nil { + return true + } + + if !client.SelfAsserted() || a.cimdPublishersVetted { + return false + } + + return !service.RedirectDeliversLocally(redirectURI) +} + // SetAuthorizationCodeAvailable records a predicate reporting whether // /oauth2/authorize can serve a request on this deployment. // diff --git a/internal/service/cimd.go b/internal/service/cimd.go index 17b87af8..6586f60c 100644 --- a/internal/service/cimd.go +++ b/internal/service/cimd.go @@ -380,7 +380,7 @@ func (s *CIMDService) ResolveClient(ctx context.Context, clientID string) (*doma return client, nil } - return s.resolveUncached(ctx, clientID) + return s.resolveUncached(ctx, clientID, u.Hostname()) }) if err != nil { return nil, err @@ -402,7 +402,15 @@ func (s *CIMDService) ResolveClient(ctx context.Context, clientID string) (*doma // for one client_id. Split out of ResolveClient so the singleflight callback // stays readable; it must only be called from inside a flight, because it // assumes the caller has established there is no usable cache entry. -func (s *CIMDService) resolveUncached(ctx context.Context, clientID string) (*domain.OAuthClient, error) { +// +// clientIDHost is the already-parsed host of clientID, passed in rather than +// re-derived: ResolveClient has parsed it and run it past domainAllowed before +// entering the flight, so re-parsing here would repeat that work and introduce +// an error branch that cannot be reached. It is a pure function of clientID, +// which is the singleflight key, so every caller sharing a flight agrees on it. +func (s *CIMDService) resolveUncached( + ctx context.Context, clientID, clientIDHost string, +) (*domain.OAuthClient, error) { doc, docTTL, err := s.fetch(ctx, clientID) if err != nil { // Negative-cache the failure so replaying a dead URL can't force a @@ -421,6 +429,10 @@ func (s *CIMDService) resolveUncached(ctx context.Context, clientID string) (*do s.storeResult(clientID, cimdCacheEntry{err: err}, cimdNegativeCacheTTL) return nil, err } + if err := s.redirectHostsAllowed(clientIDHost, client.RedirectURIs); err != nil { + s.storeResult(clientID, cimdCacheEntry{err: err}, cimdNegativeCacheTTL) + return nil, err + } s.storeResult(clientID, cimdCacheEntry{client: client}, docTTL) log.Info(). @@ -431,6 +443,53 @@ func (s *CIMDService) resolveUncached(ctx context.Context, clientID string) (*do return client, nil } +// redirectHostsAllowed enforces the allow-list on https redirect destinations, +// not just on the host that published the document. +// +// The allow-list is what lets a CIMD client be redirected to at all — see +// API.refusesRedirectTo. That gate reads "an allow-listed publisher is a vetted +// party", and the two only mean the same thing if the destination is vetted too. +// Without this check they come apart on any host where more than one party can +// publish a path: a user-content path, a raw-file CDN, a bucket with broad +// write, a shared internal app host. Publish a document there naming +// redirect_uri https://evil.example/cb and the deployment 302s an +// unauthenticated caller to evil.example — and worse, sends a victim through +// the real login page first, so the code arrives after a genuine sign-in. That +// is exactly what the self-asserted carve-out was added to prevent, reachable +// through the switch that is supposed to lift it safely. +// +// Same host as the client_id passes without being listed: a document may always +// name callbacks on the host that published it, which is the ordinary case and +// needs no extra configuration. +// +// Loopback http and private-use schemes are exempt. Their destination is the +// caller's own machine, not a host anybody publishes to, so a deployment-wide +// host allow-list has nothing to say about them — and they are the native and +// MCP client shape, which must keep working. +// +// In open mode (no allow-list) domainAllowed admits everything, so this is a +// no-op — correctly, since open mode refuses these redirects outright. +func (s *CIMDService) redirectHostsAllowed(clientIDHost string, redirectURIs []string) error { + for _, raw := range redirectURIs { + u, err := url.Parse(raw) + if err != nil || u.Scheme != "https" { + // Non-https was already scheme-checked by validateCIMDRedirectURI; + // a parse failure cannot survive it either. + continue + } + + host := u.Hostname() + if strings.EqualFold(host, clientIDHost) || s.domainAllowed(host) { + continue + } + + return fmt.Errorf("%w: redirect_uri host %q is neither the client_id host nor in cimd.allowed_domains", + ErrCIMDDomainNotAllowed, host) + } + + return nil +} + // domainAllowed reports whether host passes the optional allowlist. An empty // allowlist means "any host" (SSRF guard still applies at fetch time). func (s *CIMDService) domainAllowed(host string) bool { @@ -614,6 +673,52 @@ func synthesizeCIMDClient(clientID string, doc *cimdMetadataDocument, now time.T }, nil } +// RedirectDeliversLocally reports whether a redirect destination can only reach +// the caller's own device: a loopback http:// callback (RFC 8252 §7.3) or a +// private-use scheme (§7.1). Both are the native/CLI/MCP client shape. +// +// It exists so the self-asserted carve-out can ask the question it actually +// means. That carve-out refuses to redirect to a CIMD client because its +// redirect_uris are attacker-CHOSEN, which would make the authorization +// endpoint an unauthenticated redirector "with the AS's own origin as the first +// hop" — a threat about a REMOTE destination. A 302 to 127.0.0.1 has no remote +// hop and no third party: the code lands on the machine the user is sitting at, +// and an attacker who can listen there already has local code execution. That is +// the same reasoning RFC 8252 uses to accept loopback callbacks from clients +// nobody registered, and it does not weaken with CIMD. +// +// It matters because loopback is the DOMINANT MCP shape — the canonical document +// in docs/cimd.md lists exactly these — so refusing there costs the entire +// browser leg and buys nothing. Web MCP clients with real https callbacks are a +// different case and stay subject to the allow-list. +// +// Private-use schemes are included on the same footing validateCIMDRedirectURI +// already gives them. Squatting one needs an attacker app installed on the +// victim's device, which is the local-code-execution case again; RFC 8252 §8.1 +// notes the risk and still accepts them. +// +// Callers must pass a redirect_uri already matched against the client's +// registered list — this judges reachability, not membership. +func RedirectDeliversLocally(raw string) bool { + u, err := url.Parse(raw) + if err != nil { + return false + } + + switch u.Scheme { + case "https": + return false + case "http": + return isLoopbackHost(u.Hostname()) + case "": + // Not an absolute URI. validateCIMDRedirectURI rejects these, so this is + // unreachable for a CIMD client; fail closed rather than guess. + return false + default: + return true + } +} + // validateCIMDRedirectURI enforces OAuth 2.1 §2.3-style redirect URI scheme // rules on a CIMD document entry: https:// always OK; http:// only for // loopback hosts (native/CLI callbacks per RFC 8252 §7.3); any other non-empty diff --git a/internal/service/cimd_test.go b/internal/service/cimd_test.go index 11bb27df..dd215be3 100644 --- a/internal/service/cimd_test.go +++ b/internal/service/cimd_test.go @@ -295,6 +295,68 @@ func TestCIMDResolveClient_DomainAllowlist(t *testing.T) { } } +// TestCIMDResolveClient_OffListRedirectURIRefused pins the invariant the +// authorize-handler gate depends on but cannot see. +// +// Once cimd.allowed_domains names a host, API.refusesRedirectTo stops refusing +// redirects for self-asserted clients deployment-wide — it does NOT re-check the +// individual client's redirect host. That is only safe if resolution has already +// refused any document declaring an off-list https redirect_uri. Vetting the +// PUBLICATION host is not enough: on any host where more than one party can +// publish a path, an attacker publishes a document on the allow-listed host +// naming redirect_uri https://evil.example/cb and gets an unauthenticated 302. +// +// TestRedirectHostsAllowed covers the predicate in isolation. This covers the +// wiring — that ResolveClient actually calls it — which is the part that can +// silently disappear. It did: rebasing this branch onto the #312 singleflight +// refactor moved the fetch into resolveUncached and severed this call. That +// break happened to be a compile error, so it surfaced; a refactor that left a +// same-named host variable in scope would have kept compiling while vetting the +// wrong host, and no existing test would have noticed. +func TestCIMDResolveClient_OffListRedirectURIRefused(t *testing.T) { + // The document is served BY the allow-listed host (the httptest server binds + // 127.0.0.1), so publication-host vetting passes and the redirect host is the + // only thing left to catch this. + doc := func(clientID, redirectURI string) string { + return fmt.Sprintf( + `{"client_id":%q,"client_name":"Test","redirect_uris":[%q]}`, + clientID, redirectURI, + ) + } + + t.Run("off-list https redirect host is refused at resolution", func(t *testing.T) { + var docURL string + svc, base, _ := newCIMDTestServer(t, + CIMDConfig{Enabled: true, AllowedDomains: []string{"127.0.0.1"}}, + func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, doc(docURL, "https://evil.example/cb")) + }) + docURL = base + "/client.json" + + _, err := svc.ResolveClient(context.Background(), docURL) + if !errors.Is(err, ErrCIMDDomainNotAllowed) { + t.Fatalf("a document on an allow-listed host declaring an off-list https "+ + "redirect_uri must be refused: got %v", err) + } + }) + + t.Run("redirect host on the allow-list resolves", func(t *testing.T) { + // The control. Without it the test above would still pass if resolution + // rejected every https redirect_uri, which would break real clients. + var docURL string + svc, base, _ := newCIMDTestServer(t, + CIMDConfig{Enabled: true, AllowedDomains: []string{"127.0.0.1"}}, + func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, doc(docURL, "https://127.0.0.1/cb")) + }) + docURL = base + "/client.json" + + if _, err := svc.ResolveClient(context.Background(), docURL); err != nil { + t.Fatalf("a redirect host that is itself allow-listed must resolve: %v", err) + } + }) +} + // TestCIMDAllowedDomainCount_ReportsEffectivePolicy — the constructor lower-cases // and drops blank/whitespace-only entries, so the raw config slice and the policy // actually in force disagree. They disagree in the worst direction: a config of @@ -650,3 +712,135 @@ func TestCIMDResolveClient_CoalescesConcurrentFetches(t *testing.T) { "slice is shared, so the clone is shallow where it must be deep") } } + +// TestRedirectHostsAllowed is the check that makes cimd.allowed_domains mean +// what the authorize-handler gate assumes it means. +// +// That gate (API.refusesRedirectTo) reads "an allow-listed publisher is a vetted +// party, so §4.1.2.1 redirects and the interactive-login redirect apply again." +// Allow-listing the client_id host alone does not establish that: on any host +// where more than one party can publish a path — user content, a raw-file CDN, a +// bucket with broad write, a shared internal app host — an attacker publishes a +// document naming redirect_uri https://evil.example/cb and gets an +// unauthenticated 302 to evil.example, plus a victim walked through the real +// login page first. The redirect destination has to be vetted too, or the switch +// re-opens what the carve-out closed. +func TestRedirectHostsAllowed(t *testing.T) { + svc := NewCIMDService(CIMDConfig{Enabled: true, AllowedDomains: []string{"apps.acme.dev"}}) + + const publisher = "apps.acme.dev" + + t.Run("same host as client_id passes unlisted", func(t *testing.T) { + // The ordinary case: a document names callbacks on the host that + // published it. Requiring that host to also appear in the allow-list + // would be redundant — it is already there, since it resolved at all. + if err := svc.redirectHostsAllowed(publisher, []string{"https://apps.acme.dev/cb"}); err != nil { + t.Fatalf("same-host redirect must pass: %v", err) + } + }) + + t.Run("another allow-listed host passes", func(t *testing.T) { + svc2 := NewCIMDService(CIMDConfig{ + Enabled: true, + AllowedDomains: []string{"apps.acme.dev", "cb.acme.dev"}, + }) + if err := svc2.redirectHostsAllowed(publisher, []string{"https://cb.acme.dev/cb"}); err != nil { + t.Fatalf("a vetted destination host must pass: %v", err) + } + }) + + t.Run("foreign https host is refused", func(t *testing.T) { + err := svc.redirectHostsAllowed(publisher, []string{"https://evil.example/cb"}) + if !errors.Is(err, ErrCIMDDomainNotAllowed) { + t.Fatalf("want ErrCIMDDomainNotAllowed, got %v", err) + } + }) + + t.Run("one bad entry poisons the document", func(t *testing.T) { + // Not "drop the bad one and keep going": the client would then hold a + // redirect_uris list the deployer never approved, and redirectURIAllowed + // matches against whichever entry the request names. + err := svc.redirectHostsAllowed(publisher, []string{ + "https://apps.acme.dev/cb", + "https://evil.example/cb", + }) + if !errors.Is(err, ErrCIMDDomainNotAllowed) { + t.Fatalf("want ErrCIMDDomainNotAllowed, got %v", err) + } + }) + + t.Run("loopback and private-use schemes are exempt", func(t *testing.T) { + // These deliver to the caller's own machine, not to a host anybody + // publishes to, so a deployment-wide host allow-list has nothing to say + // about them — and they are the native/MCP client shape. + for _, ru := range []string{ + "http://127.0.0.1:9000/cb", + "http://localhost:9000/cb", + "myapp:/cb", + } { + if err := svc.redirectHostsAllowed(publisher, []string{ru}); err != nil { + t.Errorf("%s must stay usable: %v", ru, err) + } + } + }) + + t.Run("open mode constrains nothing", func(t *testing.T) { + // domainAllowed admits every host with no allow-list, so this is a + // no-op there — correctly: open mode refuses these redirects outright, + // so there is no vetting claim to keep honest. + open := NewCIMDService(CIMDConfig{Enabled: true}) + if err := open.redirectHostsAllowed(publisher, []string{"https://anywhere.example/cb"}); err != nil { + t.Fatalf("open mode must not reject: %v", err) + } + }) + + t.Run("host match is case-insensitive", func(t *testing.T) { + if err := svc.redirectHostsAllowed("APPS.ACME.DEV", []string{"https://apps.acme.dev/cb"}); err != nil { + t.Fatalf("host comparison must be case-insensitive: %v", err) + } + }) +} + +// TestRedirectDeliversLocally pins the reachability judgement the authorize +// carve-out delegates to. A false negative costs the native/MCP browser leg; a +// false positive hands an attacker-chosen remote host a redirect from the AS's +// own origin. The look-alike case is the one worth having a test for. +func TestRedirectDeliversLocally(t *testing.T) { + local := []string{ + "http://127.0.0.1:3000/callback", + "http://localhost/cb", + "http://[::1]:8080/cb", + "myapp:/cb", + "com.example.app:/oauth", + } + remote := []string{ + "https://app.example.com/cb", + "https://127.0.0.1/cb", // https is remote-capable regardless of host + "http://127.0.0.1.evil.com/cb", // look-alike: not loopback + "http://evil.example/cb", + "/relative/cb", // no scheme — validateCIMDRedirectURI rejects; fail closed + // Userinfo confusion: the authority is `evil.com`, and `127.0.0.1` is a + // username. Anything eyeballing the prefix — or splitting on the wrong + // delimiter — reads this as loopback and hands an authorization code to + // evil.com. url.Parse().Hostname() gets it right; this pins that we keep + // using it rather than string-matching the raw URI. + "http://127.0.0.1@evil.com/cb", + // Browsers commonly normalise 0.0.0.0 to loopback, so a client could + // plausibly listen there. We classify it remote, which fails CLOSED: the + // redirect is refused rather than wrongly trusted. Pinned so the choice + // is deliberate — widening it later is a security decision, not a typo fix. + "http://0.0.0.0/cb", + } + + for _, u := range local { + if !RedirectDeliversLocally(u) { + t.Errorf("%s must count as local delivery — refusing it breaks native clients", u) + } + } + + for _, u := range remote { + if RedirectDeliversLocally(u) { + t.Errorf("%s must NOT count as local delivery", u) + } + } +} diff --git a/server.go b/server.go index 530b534c..9475cdb0 100644 --- a/server.go +++ b/server.go @@ -385,7 +385,9 @@ func NewServer(cfg Config, opts ...ServerOption) (*Server, error) { // one for a closed deployment, and the difference is invisible unless // somebody says so at boot. if effectiveDomains == 0 { - log.Warn().Msg("CIMD: cimd.allowed_domains is empty — any public HTTPS host may publish a client_id metadata document. Set an allowlist to run CIMD as a closed ecosystem.") + log.Warn().Msg("CIMD: cimd.allowed_domains is empty — any public HTTPS host may publish a client_id metadata document. " + + "Set an allowlist to run CIMD as a closed ecosystem. Until then CIMD clients also get no error redirect and no " + + "interactive-login redirect (RFC 6749 4.1.2.1 carve-out), so a browser-driven CIMD client cannot sign a user in.") } } @@ -463,6 +465,11 @@ func NewServer(cfg Config, opts ...ServerOption) (*Server, error) { // Advertise CIMD support in the AS metadata document only when enabled. apiHandler.SetCIMDEnabled(cfg.CIMD.Enabled) apiHandler.SetDPoPRequired(cfg.Token.RequireDPoP) + // An allow-list is the deployer vetting which hosts may publish a metadata + // document, which is what lets a CIMD client be redirected to at all. The + // EFFECTIVE count, for the same reason the startup log uses it: the raw + // slice counts entries the service already dropped. + apiHandler.SetCIMDPublishersVetted(cimdSvc.AllowedDomainCount() > 0) // Shared middleware state — closures reference these holders; the actual functions // are set after NewServer returns (before Start) via setter methods. diff --git a/zeroid.yaml b/zeroid.yaml index d20da06e..2baf54fd 100644 --- a/zeroid.yaml +++ b/zeroid.yaml @@ -141,6 +141,18 @@ cimd: # Set this to the hosts you trust to run CIMD as a closed ecosystem — # recommended for enterprise/closed deployments. Leave empty only for open # agent ecosystems, and rate-limit /oauth2/authorize at the edge. YAML only. + # + # REQUIRED to serve CIMD clients whose redirect_uri is a REMOTE https URL. + # While this is empty such a client gets neither the RFC 6749 4.1.2.1 error + # redirect nor the interactive-login redirect, so a user with no session has + # nowhere to be sent and the flow cannot complete for them. Loopback and + # private-use callbacks are exempt (they deliver to the user's own device), so + # the ordinary desktop/CLI MCP client needs nothing here. + # + # Listing a host asserts you vet who publishes there — an https redirect_uri + # must then be on the client_id's own host or on this list. Do not list a host + # where anyone can publish a path (user content, a raw-file CDN, a broadly + # writable bucket). # allowed_domains: ["client.example.com", "apps.acme.dev"] # # Relaxes the SSRF guard on the metadata-document fetch. Default false rejects