[spec] Auth: consolidate security considerations into new section #387
Conversation
dsp-ant
force-pushed
the
davidsp/auth-rs-as-prm
branch
from
47aab66 to
ef76ec2
Compare
Co-authored-by: Den Delimarsky 🌺 <[email protected]>
Co-authored-by: Den Delimarsky 🌺 <[email protected]>
Co-authored-by: Den Delimarsky 🌺 <[email protected]>
| 1. MCP clients **MUST** implement PKCE according to [OAuth 2.1 section 7.5.2](https://www.ietf.org/archive/id/draft-ietf-oauth-v2-1-12.html#name-countermeasures). PKCE helps prevent authorization code interception attacks by requiring clients to create a secret verifier-challenge pair, ensuring that only the original requestor can exchange an authorization code for tokens. | ||
| 1. All redirect URIs **MUST** be either `localhost` or use HTTPS to prevent token and code interception. | ||
|
|
||
| ### 3.3 Open Redirection |
There was a problem hiding this comment.
There is a scenario in Section 4.11.2 of RFC9700 that is directly related to the MCP usage of Dynamic Client Registration.
The attacker could, for example, register a client via dynamic client registration [RFC7591] and execute one of the following attacks:
Intentionally send an erroneous authorization request, e.g., by using an invalid scope value, thus instructing the authorization server to redirect the user agent to its phishing site.
...
There are some recommendations for countermeasures in that section.
There was a problem hiding this comment.
I added a reference to this section in the paragraph below and copied over the "only automatically redirect..." bit (maybe should quote it? lmkwyt)
Co-authored-by: Den Delimarsky 🌺 <[email protected]>
pcarleton
changed the title
Co-authored-by: Den Delimarsky 🌺 <[email protected]>
| MCP authorization servers SHOULD enforce token expiration and rotation to limit the window of exploitation. | ||
|
|
||
| ### 3.2 Communication Security | ||
| An attacker positioned between MCP clients and MCP servers can intercept tokens via [Man-in-the-Middle (MITM)](https://en.wikipedia.org/wiki/Man-in-the-middle_attack) attacks. |
There was a problem hiding this comment.
This is not a very compelling sentence, since it doesn't describe the full extent of the risk. I would recommend either removing it entirely so that the text below just points people to OAuth 2.1, or copy the initial paragraph from Section 1.5 inline here.
D-McAdams
left a comment
D-McAdams
left a comment
There was a problem hiding this comment.
Aaron just added a few good wordsmithing suggestions. When those are incorporated, it looks good to go.
Co-authored-by: Aaron Parecki <[email protected]>
Co-authored-by: Aaron Parecki <[email protected]>
Co-authored-by: Aaron Parecki <[email protected]>
Co-authored-by: Aaron Parecki <[email protected]>
…deputy [spec] Auth: Confused deputy
Motivation and Context
Security requirements / considerations were a few different places, and best-practices was mostly redundant with other spec language.
This promotes Security Requirements to it's own section, and expands a bit more on the motivation behind the mitigations by describing the attack path.
These should mostly be 1:1 to the existing requirements, just with more explanation.
The exception is the confused deputy addition which is to address this issue:#265
I've split 2 other considerations into separate PR's here: