[spec] Auth: Confused deputy #407

pcarleton · 2025-04-24T11:57:15Z

Motivation and Context

Breaking out of #387 to avoid merge conflicts. This adds a security consideration about the Confused Deputy problem mentioned in #265

pcarleton · 2025-04-24T12:03:15Z

Previous discussion:
#387 (comment)

@aaronpk: This section needs some cleanup on the terminology used. There's a few different terms being used to refer to the same things or aren't previously defined:

OAuth proxy

"...fronts an authorization server"

"backing authorization server"

"upstream API"

We need to be much clearer about which roles we're talking about here. I would recommend starting with an abbreviated sequence diagram to show the relationships between the different parties, as well as a definition of each term, prior to discussing the attack.

docs/specification/draft/basic/authorization.mdx

Co-authored-by: Den Delimarsky 🌺 <[email protected]>

D-McAdams · 2025-04-28T23:07:53Z

docs/specification/draft/basic/authorization.mdx

+An attacker can exploit configurations where an MCP server acts as an intermediary between MCP clients and a protected third-party API, leading to the confused deputy problem. The attacker can acquire and exchange a stolen authorization code for access tokens for the MCP server without the user's consent. Consult [Security Best Practices section 2.1](/specification/draft/basic/security_best_practices) for a more detailed description.
+
+MCP proxy servers that use a static client ID for third-party services MUST require explicit 
+approval for each dynamically registered client before forwarding requests to the 


The wording "explicit approval" is ambiguous about what constitutes approval. Suggested rephrasing: MCP proxy servers that use a static client ID for third-party services MUST prompt the end user for consent for each dynamically registered client before forwarding the user to the third-party authorization server (where the third-party may also prompt for its own user consent).

Would trim this down a bit more:

MCP proxy servers using static client IDs **MUST** obtain user consent for each dynamically registered client before forwarding to third-party authorization servers (which may require additional consent).

D-McAdams · 2025-04-28T23:41:18Z

docs/specification/draft/basic/security_best_practices.mdx

+#### 2.1.4 Mitigation
+
+MCP proxy servers that use a static client ID for third-party services MUST require explicit 
+approval for each dynamically registered client before forwarding requests to the 


Similar comment as above about ambiguity of "explicit approval"

geelen · 2025-04-29T01:35:48Z

This is great. One thing that took me a bit to get my head around when this was first raised was that it's the "cookie present, consent skipped" that makes the attack different from the normal third-party proxy OAuth flow.

Not sure if it's possible to highlight this more explicitly for the user? Potentially breaking the diagram into two makes it easier to understand?

Details

Normal OAuth proxy usage (preserves user consent)

sequenceDiagram
    participant UA as User-Agent (Browser)  
    participant MC as MCP Client
    participant M as MCP Proxy Server
    participant TAS as Third-Party Authorization Server
    Note over UA,M: Initial Auth flow completed
    Note over UA,TAS: Step 1: Legitimate user consent for Third Party Server
    M->>UA: Redirect to third party authorization server
    UA->>TAS: Authorization request (client_id: mcp-proxy)
    TAS->>UA: Authorization consent screen
    Note over UA: Review consent screen
    UA->>TAS: Approve
    TAS->>UA: Set consent cookie for client ID: mcp-proxy
    TAS->>UA: 3P Authorization code + redirect to mcp-proxy-server.com
    UA->>M: 3P Authorization code
    Note over M,TAS: Exchange 3P code for 3P token
    Note over M: Generate MCP authorization code
    M->>UA: Redirect to MCP Client with MCP authorization code
    Note over M,UA: Exchange code for token, etc.

Malicious OAuth proxy usage (skips user consent)

sequenceDiagram
    participant UA as User-Agent (Browser)  
    participant A as Attacker
    participant M as MCP Proxy Server
    participant TAS as Third-Party Authorization Server
    Note over UA,M: Step 2: Attack (leveraging existing cookie)
    A->>M: Dynamically register malicious client, redirect_uri: attacker.com
    A->>UA: Sends malicious link
    UA->>TAS: Authorization request (client_id: mcp-proxy) + consent cookie
    TAS->>TAS: Cookie present, consent skipped
   TAS->>UA: 3P Authorization code + redirect to mcp-proxy-server.com
   UA->>M: 3P Authorization code
   Note over M,TAS: Exchange 3P code for 3P token
   Note over M: Generate MCP authorization code
   M->>UA: Redirect to attacker.com with MCP Authorization code
   UA->>A: MCP Authorization code delivered to attacker.com
   Note over M,A: Attacker exchanges MCP code for MCP token
   A->>M: Attacker impersonates user to MCP server

I'm sure someone who is better at mermaid diagrams could do a decent job of that. But otherwise, this is great.

FWIW, I sketched out what an example consent dialog needs to look like: cloudflare/ai#99. Not sure if there's a better place to leave examples like that, or whether it's something that should be added to the SDKs?

localden · 2025-04-29T04:52:49Z

@geelen we're working on bootstrapping a separate section for security-related documentation that should better accomodate examples such as the dialog you're referring to. FWIW, I took inspiration from that and am building something similar for our (Microsoft) samples as well since the problem is the same. This almost makes me think that we need a more generic solution here (outside teh scope of the spec).

docs/specification/draft/basic/authorization.mdx

localden · 2025-04-29T04:56:15Z

docs/specification/draft/basic/authorization.mdx

+An attacker can exploit configurations where an MCP server acts as an intermediary between MCP clients and a protected third-party API, leading to the confused deputy problem. The attacker can acquire and exchange a stolen authorization code for access tokens for the MCP server without the user's consent. Consult [Security Best Practices section 2.1](/specification/draft/basic/security_best_practices) for a more detailed description.
+
+MCP proxy servers that use a static client ID for third-party services MUST require explicit 
+approval for each dynamically registered client before forwarding requests to the 


Would trim this down a bit more:

MCP proxy servers using static client IDs **MUST** obtain user consent for each dynamically registered client before forwarding to third-party authorization servers (which may require additional consent).

docs/specification/draft/basic/security_best_practices.mdx

localden · 2025-04-29T05:05:12Z

docs/specification/draft/basic/security_best_practices.mdx

+**MCP Proxy Server**
+: An MCP server that acts as an intermediary between MCP clients and a protected 
+  third-party API. The MCP proxy server provides MCP functionality while delegating
+API operations to a third-party API server.  The MCP proxy server acts as a single OAuth client to the third-party API server.


Can be trimmed down:

MCP proxy server connects MCP clients to third-party APIs, offering MCP features while delegating operations and acting as a single OAuth client to the third-party API server.

localden · 2025-04-29T05:07:45Z

docs/specification/draft/basic/security_best_practices.mdx

+API operations to a third-party API server.  The MCP proxy server acts as a single OAuth client to the third-party API server.
+
+**Third-Party Authorization Server**
+: The authorization server that protects access to the third-party API. This server may not


Also can be trimmed down a bit:

Authorization server that protects the third-party API. It may lack dynamic client registration support, requiring MCP proxy to use a static client ID for all requests.

localden · 2025-04-29T05:08:52Z

docs/specification/draft/basic/security_best_practices.mdx

+
+**Static Client ID**
+: A fixed OAuth 2.0 client identifier used by the MCP proxy server when communicating with
+  the third-party authorization server, shared across all MCP clients.


I am not sure "shared across all MCP clients" is the right framing here. It's used by the server to request access tokens for the server (I don't think we're talking about them being passed upstream to the client)?

ah I see, yea I meant "shared" as in "used for all of them", not necessarily exposed to them, will edit

docs/specification/draft/basic/security_best_practices.mdx

localden · 2025-04-29T05:11:03Z

docs/specification/draft/basic/security_best_practices.mdx

+
+1. A user authenticates normally through the MCP proxy server to access the third-party API
+2. During this legitimate flow, the third-party authorization server sets a cookie on the user agent
+   indicating consent for the static client ID


Is it really going to be the same client ID re-used across all clients? We're not requiring the MCP server to do an in-server registration of clients?

this is the MCP server's client ID that it gets from e.g. the Github API (which doesn't support DCR). So it will be the MCP server is acting as a client to the Github API with a static client ID, meanwhile MCP clients have different client ID's per client.

docs/specification/draft/basic/security_best_practices.mdx

Co-authored-by: Den Delimarsky 🌺 <[email protected]>

pcarleton · 2025-04-29T15:13:10Z

@geelen good call! I split it up and put a red rect over the cookie consent skipping to make it more obvious where the bad thing happens.

ty @dend & @D-McAdams , ptal

…sed-deputy

add back confused deputy

fdf53d5

pcarleton mentioned this pull request Apr 24, 2025

[spec] Auth: consolidate security considerations into new section #387

Merged

pcarleton changed the title ~~[spec] Authz security considerations: Confused deputy~~ [spec] Auth: Confused deputy Apr 24, 2025

pcarleton added 2 commits April 24, 2025 14:09

add details

9992fe6

some clean up

30796d0

pcarleton marked this pull request as ready for review April 24, 2025 13:14

pcarleton requested a review from aaronpk April 24, 2025 13:15

localden reviewed Apr 26, 2025

View reviewed changes

pcarleton and others added 2 commits April 28, 2025 14:21

Apply suggestions from code review, ty Den

7769a6d

Co-authored-by: Den Delimarsky 🌺 <[email protected]>

move confused deputy details into best practices

7f98a4e

D-McAdams reviewed Apr 28, 2025

View reviewed changes

localden reviewed Apr 29, 2025

View reviewed changes

pcarleton and others added 3 commits April 29, 2025 15:57

Apply suggestions from code review

7ac293c

Co-authored-by: Den Delimarsky 🌺 <[email protected]>

trim down

68ec276

split to 2 sequence diagrams

2eada8a

Merge branch 'pcarleton-security-considerations' into pcarleton/confu…

3f709e2

…sed-deputy

D-McAdams approved these changes Apr 29, 2025

View reviewed changes

localden approved these changes Apr 30, 2025

View reviewed changes

Merge branch 'pcarleton-security-considerations' into pcarleton/confu…

e7f2c58

…sed-deputy

localden merged commit b7909f9 into pcarleton-security-considerations Apr 30, 2025
2 checks passed

localden deleted the pcarleton/confused-deputy branch April 30, 2025 06:06

[spec] Auth: Confused deputy #407

[spec] Auth: Confused deputy #407

Uh oh!

Conversation

pcarleton commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation and Context

Uh oh!

pcarleton commented Apr 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

D-McAdams Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geelen commented Apr 29, 2025

Normal OAuth proxy usage (preserves user consent)

Malicious OAuth proxy usage (skips user consent)

Uh oh!

localden commented Apr 29, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pcarleton commented Apr 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pcarleton commented Apr 24, 2025 •

edited

Loading

D-McAdams Apr 28, 2025 •

edited

Loading