-
Notifications
You must be signed in to change notification settings - Fork 1.2k
SEP-991 specification changes; OAuth client id metadata document #1296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| This attack is concerning because the server sees the correct metadata | ||
| document and the user sees the correct client name, making detection | ||
| difficult. While platform-specific attestations (iOS DeviceCheck, Android | ||
| Play Integrity) could address this, they're not universally available. An |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's expand on this and describe how an app developer can run a backend service that validates the DeviceCheck/Play Integrity signatures and returns a JWT the app can use with the private_key_jwt client authentication method. This would actually be a pretty reasonable level of assurance of the app's identity. This doesn't need to be specified normatively in this proposal, since the actual underlying OAuth mechanism used is only the private_key_jwt auth method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps the normative text can be left in the IETF draft, and we can just normatively reference that instead and outline the concept of using "private_key_jwt" here. Alternatively we can add a section to talk about client authentication as a core part of the spec, and not just as a mitigation in this section.
Proposing some expanded text below.
Co-authored-by: Aaron Parecki <[email protected]>
Co-authored-by: PieterKas <[email protected]> Co-authored-by: Aaron Parecki <[email protected]>
0112e87 to
dd96c23
Compare
tnorimat
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about referring the version of IETF Web Authorization Protocol
https://datatracker.ietf.org/doc/html/draft-ietf-oauth-client-id-metadata-document-00
Instead of
https://www.ietf.org/archive/id/draft-parecki-oauth-client-id-metadata-document-03.txt
?
Co-authored-by: Aaron Parecki <[email protected]>
Co-authored-by: Aaron Parecki <[email protected]>
Co-authored-by: Aaron Parecki <[email protected]>
Added explicit authentication prompt and user credential steps to the sequence diagram for clarity. Improved wording regarding OAuth 2.0 Dynamic Client Registration Protocol support.
Co-authored-by: Aaron Parecki <[email protected]>
|
|
||
| ```json | ||
| { | ||
| "client_id": "https://app.example.com/oauth/client-metadata.json", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there some kind of validation we could use? could jus t people impersonate any client there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
someone running software bound on localhost can impersonate another client running on localhost. Solving that requires more work, but is not different from DCR today. there's a longer write up in the blog about this topic: https://blog.modelcontextprotocol.io/posts/client_registration/#challenge-2-client-identity-and-impersonation
|
|
||
| MCP supports three client registration mechanisms. Choose based on your scenario: | ||
|
|
||
| - **Client ID Metadata Documents**: When client and server have no prior relationship (most common) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how are clients choosing the approach to use? is there any concept of priority?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally the way I see clients doing this is:
- If they have a pre-registered credential available, use that
- If not, use CIMD if supported
- If not, use DCR if supported
- If not, prompt user that they can't connect / they need to go pre-register a client
This would probably be good to explicitly spell out
| [Server-Side Request Forgery (SSRF)](https://developer.mozilla.org/docs/Web/Security/Attacks/SSRF) attacks, | ||
| as well as against being used as Denial of Service (DoS) amplifiers: | ||
|
|
||
| - Validate URLs and resolved IP addresses before fetching |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what kind of validation are we talking about? block some ip ranges?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea good question, I wonder if we want to just link to the CIMD RFC rather than spelling this all out here as well.
Authorization servers fetching the client metadata document and resolving URLs located in the metadata document should be aware of possible SSRF attacks. Authorization servers SHOULD avoid fetching any URLs using private or loopback addresses and consider network policies or other measures to prevent making requests to these addresses. Authorization servers SHOULD also be aware of the possibility that URLs might be non-http-based URI schemes which can lead to other possible SSRF attack vectors.
|
|
||
| #### Example Metadata Document | ||
|
|
||
| ```json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about private_key_jwt? can you show an example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather leave that to the CIMD RFC
see #991