From 15231ac7c949c3c45feaf5a6a8efc8077f602b10 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 20 Jul 2026 17:48:00 +0000 Subject: [PATCH 1/5] Close auth-code phishing via a hosted-redirect allow-list Open dynamic client registration let anyone register a client with a hosted redirect_uri they control, so the same-browser phishing variant that Consent-Bound Completion does not close could deliver a victim's authorization code to an attacker-registered hosted destination (CWE-601). Constrain hosted redirects to an allow-list of registrable domains; loopback stays exempt (a loopback code resolves on the consenter's own machine, so native/CLI clients are unaffected). - redirect_uri_permitted(): loopback always allowed; a hosted redirect must be https and its host must equal, or be a subdomain of, an allow-listed registrable domain. The host is read from the parsed URL, so authority tricks (https://claude.ai@evil.com, claude.ai.evil.com) resolve to their real host and are refused. - Seeded (DEFAULT_ALLOWED_REDIRECT_DOMAINS) with the MCP connector vendors seen in practice: antigravity.google, chatgpt.com, claude.ai, grok.com, perplexity.ai, perplexity.com. Widen per deployment with OAUTH_ALLOWED_REDIRECT_DOMAINS (additive), no rebuild. - Enforced at /oauth/register (reject before storing) and re-checked in redirect_allowed, so a pre-existing disallowed registration cannot be used at /oauth/authorize either. - Module docs + README updated (the same-browser variant is now closed). A test covers vendors/subdomains, loopback, the finding's payloads, and look-alike / authority-trick rejection. cargo test: 125 passing; no new clippy warnings. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_014T9N8USDfNK5yzznPGg7Ym --- README.md | 24 ++++++--- src/auth.rs | 151 +++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 153 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index ea7e326..b9ebac6 100644 --- a/README.md +++ b/README.md @@ -355,7 +355,10 @@ Endpoints: - `GET /.well-known/oauth-protected-resource` — points clients at the AS - `POST /oauth/register` — dynamic client registration (RFC 7591); `redirect_uris` are stored and persisted to `OAUTH_CLIENTS_FILE`; requested `grant_types` are - honoured (intersected with `authorization_code`) + honoured (intersected with `authorization_code`). A **hosted** `redirect_uri` is + rejected unless its host is on the allow-list (see the Companion-control note + below); loopback redirects are always accepted + - `GET /oauth/authorize` — validates the client + redirect, requires PKCE, sets the binding cookie, then redirects to II's handshake (with `registration_key`) - `GET /oauth/connect/callback` — the **pinned callback page**: II navigates here @@ -408,12 +411,17 @@ incl. loopback** (a loopback redirect resolves on the consenter's own machine). `SameSite=Lax` still rides the top-level cross-site GET II uses to navigate back to the callback page. -> **Companion control (not in this change).** The *same-browser* variant — a victim -> socially engineered into running the whole flow toward an attacker-registered -> **hosted** `redirect_uri` — is not closed by this (the victim's browser -> legitimately holds both proofs). It needs **hosted-redirect allow-listing**; -> loopback/native clients are safe either way (the code resolves on the consenter's -> own machine). +> **Companion control: the hosted-redirect allow-list.** The *same-browser* variant +> (a victim socially engineered into running the whole flow toward an +> attacker-registered **hosted** `redirect_uri`) is not closed by Consent-Bound +> Completion alone, since the victim's browser legitimately holds both proofs. It +> **is** closed by a **hosted-redirect allow-list**: dynamic client registration +> accepts only a loopback redirect, or a hosted redirect whose host is (a subdomain +> of) an allow-listed registrable domain, so an attacker cannot register a hosted +> destination it controls. The list is seeded with the known MCP connector vendors +> and widened per deployment with `OAUTH_ALLOWED_REDIRECT_DOMAINS` (additive); +> loopback/native clients are exempt (the code resolves on the consenter's own +> machine). Enforced at both `/oauth/register` and `/oauth/authorize`. ### The registration-delegation connect handshake @@ -701,7 +709,7 @@ delegation. Omitting `account` uses the default account. delegation redeemed via `mcp_register_v2`), with **Consent-Bound Completion** binding `/oauth/connect/redeem` to both the initiator (`sid` cookie) and the consenter (the fragment delegation); expiring tokens. (The RFC 8628 device grant was dropped. - Same-browser-variant closure needs hosted-redirect allow-listing — see Auth.) + The same-browser variant is closed by the hosted-redirect allow-list, see Auth.) - [x] On-demand **domain identities**: the registered session key mints per-app account delegations directly via II canister methods (`call_canister`/`get_app_principal` `derivation_origin`); no per-app browser flow. diff --git a/src/auth.rs b/src/auth.rs index f68f743..1175070 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -46,11 +46,12 @@ //! In the confused-deputy path the delegation lands in the honest page in the //! VICTIM's browser, whose cookie does not match the one `X` was bound to, so the //! redeem aborts. This closes the split-browser injection for all transports incl. -//! loopback (a loopback redirect resolves on the consenter's own machine). Not -//! closed here (companion control): the *same-browser* variant where a victim is -//! socially engineered into running the whole flow toward an attacker-registered -//! **hosted** `redirect_uri`; that needs hosted-redirect allow-listing (loopback is -//! safe either way). +//! loopback (a loopback redirect resolves on the consenter's own machine). The +//! *same-browser* variant (a victim socially engineered into running the whole +//! flow toward an attacker-registered **hosted** `redirect_uri`) is closed by a +//! **hosted-redirect allow-list** ([`redirect_uri_permitted`]): open DCR may +//! register only loopback or allow-listed-domain hosted redirects, so an attacker +//! cannot register a hosted destination it controls (loopback is safe either way). //! //! The wire shapes match the merged II contract (verified against the beta II //! canister's live `.did`, `fgte5-ciaaa-aaaad-aaatq-cai`): the connect link @@ -157,15 +158,89 @@ fn persist_clients(clients: &HashMap) { } } +/// Registrable domains whose hosts (and subdomains) may register a **hosted** +/// (non-loopback) OAuth `redirect_uri`. Dynamic client registration is open, so +/// without this an attacker could register a client with a hosted redirect it +/// controls and phish an authorization code to it (the same-browser variant that +/// Consent-Bound Completion does not close). Loopback redirects are exempt (a +/// loopback code resolves on the consenter's own machine), so native/CLI clients +/// need no entry. Seeded with the MCP connector vendors seen in practice; widen +/// at deploy time with `OAUTH_ALLOWED_REDIRECT_DOMAINS` (comma/space-separated, +/// additive) without a rebuild. +const DEFAULT_ALLOWED_REDIRECT_DOMAINS: &[&str] = &[ + "antigravity.google", // Google Antigravity + "chatgpt.com", // OpenAI ChatGPT connectors + "claude.ai", // Anthropic Claude + "grok.com", // xAI Grok + "perplexity.ai", // Perplexity (any subdomain) + "perplexity.com", // Perplexity (any subdomain) +]; + +/// The effective hosted-redirect allow-list: the compiled-in defaults plus any +/// entries in `OAUTH_ALLOWED_REDIRECT_DOMAINS`. Each env entry may be a bare +/// domain or an `https://…` origin (scheme/port/path are stripped to the host). +/// Additive: the shipped binary is safe by default and ops can only widen the +/// set, never accidentally drop a known vendor. +fn allowed_redirect_domains() -> Vec { + let mut domains: Vec = + DEFAULT_ALLOWED_REDIRECT_DOMAINS.iter().map(|d| d.to_string()).collect(); + if let Ok(extra) = std::env::var("OAUTH_ALLOWED_REDIRECT_DOMAINS") { + for raw in extra.split([',', ' ', '\t', '\n']).map(str::trim).filter(|s| !s.is_empty()) { + let host = raw + .trim_start_matches("https://") + .trim_start_matches("http://") + .split('/') + .next() + .unwrap_or(raw) + .trim_end_matches('.') + .to_ascii_lowercase(); + if !host.is_empty() { + domains.push(host); + } + } + } + domains +} + +/// Whether a `redirect_uri` may be registered or receive an authorization code. +/// Loopback (`http://localhost` / `127.0.0.1` / `[::1]`, any port) is always +/// allowed. A hosted redirect must be `https` and its host must equal, or be a +/// subdomain of, an allow-listed registrable domain. The host is read from the +/// PARSED URL, not the raw string, so authority tricks such as +/// `https://claude.ai@evil.com` or `https://claude.ai.evil.com` resolve to their +/// real host and are refused. +fn redirect_uri_permitted(redirect_uri: &str) -> bool { + if is_loopback_redirect(redirect_uri) { + return true; + } + let Ok(url) = url::Url::parse(redirect_uri) else { + return false; + }; + if url.scheme() != "https" { + return false; + } + let Some(host) = url.host_str() else { + return false; + }; + let host = host.trim_end_matches('.').to_ascii_lowercase(); + allowed_redirect_domains() + .iter() + .any(|d| host == *d || host.ends_with(&format!(".{d}"))) +} + /// Acceptance rule for a redirect (OAuth 2.1): the client must be REGISTERED, -/// and the requested redirect must either exactly match a registered URI, or be -/// a loopback URI matching a registered loopback URI on everything but the -/// port. RFC 8252 §7.3 requires the any-port latitude — native clients bind an -/// ephemeral loopback port at runtime, so the exact port can't be registered — -/// but registration itself is still required, so every client that can receive -/// a code is on record (DCR is open, so this is an audit trail, not vetting). +/// the redirect must pass the hosted-redirect allow-list +/// ([`redirect_uri_permitted`]), and it must either exactly match a registered +/// URI or be a loopback URI matching a registered loopback URI on everything but +/// the port. RFC 8252 §7.3 requires the any-port latitude, since native clients +/// bind an ephemeral loopback port at runtime, so the exact port can't be +/// registered. The allow-list is re-checked here (not only at registration) so a +/// pre-existing registration whose domain is no longer allowed can't be used. fn redirect_allowed(reg: Option<&ClientReg>, redirect_uri: &str) -> bool { let Some(reg) = reg else { return false }; + if !redirect_uri_permitted(redirect_uri) { + return false; + } reg.redirect_uris .iter() .any(|u| u == redirect_uri || loopback_match(u, redirect_uri)) @@ -1127,6 +1202,20 @@ pub async fn register(State(store): State, Json(req): Json evil.com + // A hosted redirect must be https even to an allowed domain. + assert!(!redirect_uri_permitted("http://claude.ai/cb")); + // Defense in depth: a client registered before the allow-list (or via a + // now-removed domain) still can't receive a code at /oauth/authorize. + let junk = ClientReg { redirect_uris: vec!["https://example.com/cb".to_string()] }; + assert!(!redirect_allowed(Some(&junk), "https://example.com/cb")); + } + /// A registered loopback redirect matches at ANY port (RFC 8252 §7.3 — the /// client binds an ephemeral port each run), but host and path must match, /// and a registered hosted URI grants no loopback latitude. @@ -1568,14 +1691,14 @@ mod tests { // Register a client so `validate_client` passes and we reach the redirect. store.clients.write().await.insert( "client-x".into(), - super::ClientReg { redirect_uris: vec!["https://app.example/cb".into()] }, + super::ClientReg { redirect_uris: vec!["https://claude.ai/api/mcp/auth_callback".into()] }, ); let resp = super::authorize( State(store.clone()), Query(super::AuthorizeQuery { response_type: Some("code".into()), client_id: "client-x".into(), - redirect_uri: "https://app.example/cb".into(), + redirect_uri: "https://claude.ai/api/mcp/auth_callback".into(), state: Some("xyz".into()), code_challenge: Some("E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM".into()), code_challenge_method: Some("S256".into()), From 1fb01cc493f919c124d04eb429ff01b5a25ce71d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 20 Jul 2026 19:20:06 +0000 Subject: [PATCH 2/5] Address review: strip port from env allow-list entries; README punctuation - allowed_redirect_domains(): parse each OAUTH_ALLOWED_REDIRECT_DOMAINS entry as a URL so scheme, userinfo, port, and path all reduce to the bare host, matching Url::host_str() at match time. Previously a port (e.g. vendor.example:8443) survived and could never match, silently rejecting a legitimate redirect. Docstring now holds. - README: terminate the /oauth/register bullet; split the dangling Roadmap parenthetical so the allow-list fact and the dropped device grant are separate sentences. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_014T9N8USDfNK5yzznPGg7Ym --- README.md | 6 +++--- src/auth.rs | 20 +++++++++++--------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index b9ebac6..87bd3bf 100644 --- a/README.md +++ b/README.md @@ -357,7 +357,7 @@ Endpoints: are stored and persisted to `OAUTH_CLIENTS_FILE`; requested `grant_types` are honoured (intersected with `authorization_code`). A **hosted** `redirect_uri` is rejected unless its host is on the allow-list (see the Companion-control note - below); loopback redirects are always accepted + below); loopback redirects are always accepted. - `GET /oauth/authorize` — validates the client + redirect, requires PKCE, sets the binding cookie, then redirects to II's handshake (with `registration_key`) @@ -708,8 +708,8 @@ delegation. Omitting `account` uses the default account. handshake binds the connection's session key (a fragment-delivered, canister-signed delegation redeemed via `mcp_register_v2`), with **Consent-Bound Completion** binding `/oauth/connect/redeem` to both the initiator (`sid` cookie) and the consenter (the - fragment delegation); expiring tokens. (The RFC 8628 device grant was dropped. - The same-browser variant is closed by the hosted-redirect allow-list, see Auth.) + fragment delegation); expiring tokens. The same-browser phishing variant is closed + by a hosted-redirect allow-list (see Auth). (The RFC 8628 device grant was dropped.) - [x] On-demand **domain identities**: the registered session key mints per-app account delegations directly via II canister methods (`call_canister`/`get_app_principal` `derivation_origin`); no per-app browser flow. diff --git a/src/auth.rs b/src/auth.rs index 1175070..dce2bae 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -186,15 +186,17 @@ fn allowed_redirect_domains() -> Vec { DEFAULT_ALLOWED_REDIRECT_DOMAINS.iter().map(|d| d.to_string()).collect(); if let Ok(extra) = std::env::var("OAUTH_ALLOWED_REDIRECT_DOMAINS") { for raw in extra.split([',', ' ', '\t', '\n']).map(str::trim).filter(|s| !s.is_empty()) { - let host = raw - .trim_start_matches("https://") - .trim_start_matches("http://") - .split('/') - .next() - .unwrap_or(raw) - .trim_end_matches('.') - .to_ascii_lowercase(); - if !host.is_empty() { + // Parse as a URL to robustly reduce an entry to its bare host, stripping + // scheme, userinfo, port, and path alike. Bare domains have no scheme, so + // give them one first (`vendor.example` / `vendor.example:8443` -> host + // `vendor.example`), matching what `Url::host_str()` yields at match time. + let candidate = + if raw.contains("://") { raw.to_string() } else { format!("https://{raw}") }; + let host = url::Url::parse(&candidate) + .ok() + .and_then(|u| u.host_str().map(|h| h.trim_end_matches('.').to_ascii_lowercase())) + .filter(|h| !h.is_empty()); + if let Some(host) = host { domains.push(host); } } From 968a0187e64a94117e088486b735fc0ac6ecd5a4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Jul 2026 09:53:28 +0000 Subject: [PATCH 3/5] Show a friendly "not allowed" page for allow-list rejections A client turned away by the hosted-redirect allow-list now gets an actionable next step instead of an opaque failure: - /oauth/authorize: a well-formed hosted redirect_uri that simply isn't allow-listed now renders an on-brand HTML page (not_allowlisted_page) naming the cause and a contact address, rather than a raw JSON invalid_client blob. This path is reached by clients that skip DCR or use a stored-but-disallowed registration; the standard flow is blocked earlier at /oauth/register. Other client-validation failures (unknown client_id, mismatched redirect) still return the terse JSON error. - /oauth/register: the JSON error_description now points the caller at the same contact so the client's setup UI can surface it. The page reuses the pinned callback page's self-contained shell (connect.css + inlined logo), carries no script, ships a strict nonce'd CSP (default-src 'none', no script-src), reflects no request input, and returns 403. Both themes verified via headless render. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_014T9N8USDfNK5yzznPGg7Ym --- README.md | 6 +- src/assets/connect-error.html | 13 +++ src/assets/connect.css | 5 +- src/auth.rs | 150 +++++++++++++++++++++++++++++++++- 4 files changed, 171 insertions(+), 3 deletions(-) create mode 100644 src/assets/connect-error.html diff --git a/README.md b/README.md index 87bd3bf..381c3f5 100644 --- a/README.md +++ b/README.md @@ -421,7 +421,11 @@ the callback page. > destination it controls. The list is seeded with the known MCP connector vendors > and widened per deployment with `OAUTH_ALLOWED_REDIRECT_DOMAINS` (additive); > loopback/native clients are exempt (the code resolves on the consenter's own -> machine). Enforced at both `/oauth/register` and `/oauth/authorize`. +> machine). Enforced at both `/oauth/register` and `/oauth/authorize`. A client +> turned away is pointed at a contact address to request approval: +> `/oauth/register` says so in its JSON `error_description`, and a browser that +> reaches `/oauth/authorize` gets an on-brand "not allowed" page (not a raw error) +> naming the same contact. ### The registration-delegation connect handshake diff --git a/src/assets/connect-error.html b/src/assets/connect-error.html new file mode 100644 index 0000000..3b1dd78 --- /dev/null +++ b/src/assets/connect-error.html @@ -0,0 +1,13 @@ + + +MCP client not allowed + +
+
+

This MCP client isn't allowed

+

This server only accepts connections from approved MCP clients, and the one you're connecting from is not on the allow-list.

+

To request access, email __CONTACT__ and include your MCP client's redirect URI.

+
+
Hosted by +
+ diff --git a/src/assets/connect.css b/src/assets/connect.css index ac158d5..3b31319 100644 --- a/src/assets/connect.css +++ b/src/assets/connect.css @@ -82,7 +82,10 @@ body { color: var(--ink); } .detail { margin: 0; max-width: 32ch; font-size: 1rem; color: var(--secondary); } -.hint { margin: 0; font-size: 0.9rem; color: var(--muted); } +.hint { margin: 0; max-width: 34ch; font-size: 0.9rem; color: var(--muted); } +/* Links appear only on the error page (the callback page has none): keep them on + the Ember accent so they read as links in both themes without a UA blue. */ +.detail a, .hint a { color: var(--accent); } /* Foot-of-page attribution: an eyebrow label + the DFINITY mark in a soft pill. */ .hosted { display: flex; diff --git a/src/auth.rs b/src/auth.rs index dce2bae..db84bc0 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -52,6 +52,10 @@ //! **hosted-redirect allow-list** ([`redirect_uri_permitted`]): open DCR may //! register only loopback or allow-listed-domain hosted redirects, so an attacker //! cannot register a hosted destination it controls (loopback is safe either way). +//! A client turned away by the allow-list is pointed at [`ALLOWLIST_CONTACT`] to +//! request approval: `/oauth/register` says so in its JSON `error_description`, +//! and a browser that reaches `/oauth/authorize` gets the on-brand +//! [`not_allowlisted_page`] instead of a raw error. //! //! The wire shapes match the merged II contract (verified against the beta II //! canister's live `.did`, `fgte5-ciaaa-aaaad-aaatq-cai`): the connect link @@ -412,6 +416,15 @@ pub async fn authorize(State(store): State, Query(q): Query return oauth_err(StatusCode::BAD_REQUEST, "invalid_request", "response_type=code required"), } if !store.validate_client(&q.client_id, &q.redirect_uri).await { + // A well-formed hosted `redirect_uri` that simply isn't on the allow-list + // is an approval gap, not a malformed request: show the browser a friendly + // page naming the cause and the contact, rather than a raw JSON blob. This + // covers clients that skip DCR or use a stored-but-disallowed registration + // (the standard flow is blocked earlier, at `/oauth/register`). The page is + // static and reflects nothing, so naming the cause leaks nothing. + if !redirect_uri_permitted(&q.redirect_uri) { + return not_allowlisted_page(); + } // `invalid_client` (not `invalid_request`): the request is well-formed, // it's the CLIENT identification that failed — the AS error code the // MCP server guide (and RFC 6749's taxonomy) expects here. No redirect: @@ -752,6 +765,60 @@ fn pinned_callback_page(prefix: &str) -> Response { resp } +/// Where a rejected MCP client is told to request allow-listing. Shown both in +/// the browser-facing `/oauth/authorize` error page ([`not_allowlisted_page`]) +/// and in the `/oauth/register` JSON `error_description`, so a vendor blocked by +/// the hosted-redirect allow-list ([`redirect_uri_permitted`]) has a clear next +/// step rather than an opaque failure. +const ALLOWLIST_CONTACT: &str = "mcp@dfinity.org"; + +/// HTML for the "MCP client not allowed" page, a real `.html` asset (compiled in +/// via `include_str!`, no runtime file I/O). It reuses the pinned callback page's +/// self-contained shell (`assets/connect.css`, the inlined logo) but carries no +/// script. `__NONCE__`/`__CSS__`/`__LOGO__`/`__CONTACT__` are spliced in at render +/// time; none is user-influenced (the contact is a compiled-in constant), so no +/// request value is ever reflected into the markup. +const CONNECT_ERROR_HTML: &str = include_str!("assets/connect-error.html"); + +/// The friendly, browser-facing rejection for a client whose `redirect_uri` is +/// not on the hosted-redirect allow-list ([`redirect_uri_permitted`]). A real +/// browser reaches `/oauth/authorize` by top-level navigation, so a terse JSON +/// `invalid_client` renders as a raw error blob; this serves an on-brand page +/// that names the cause and points the vendor at [`ALLOWLIST_CONTACT`]. Static +/// and non-reflecting: no query or redirect value is interpolated, so it can't be +/// turned into a content-injection or open-redirect surface. `403 Forbidden`: the +/// request is understood but refused by policy. +fn not_allowlisted_page() -> Response { + let nonce = csp_nonce(); + let html = CONNECT_ERROR_HTML + .replace("__NONCE__", &nonce) + .replace("__CSS__", CONNECT_PAGE_CSS) + .replace("__LOGO__", CONNECT_LOGO_SVG) + .replace("__CONTACT__", ALLOWLIST_CONTACT); + // No script on this page, so no `script-src`; the only inline is the nonce'd + // `