Repository navigation
auth: issuer mix-up mitigation - #859
Conversation
|
Hi @max-stytch, thanks for the PR. Is this planned to be used as a reference implementation in the official SEP process? If so, could you give me some background where in the process you are currently? More precisely: Is it coming from some WG / IG? Did you reach out for core maintainer feedback already? Do you need this PR to be merged as part of the process or it can stay open until the SEP is accepted? Thanks! |
|
Hey @maciej-kisiel -
Yes, that was my intention!
This is part of the
I do not believe
I believe the PR can stay open until the SEP is accepted based on the workflow steps. I would be grateful for a review in the meantime of course :) |
|
The PR is reasonable, I don't have any larger concerns. |
|
@max-stytch can you solve the conflict? Then we can merge this PR |
|
@guglielmo-san back to you! |
|
@max-stytch Thank you for the contribution! |
`AuthorizationCodeHandler` validates the `iss` parameter of the authorization response ([RFC 9207](https://www.rfc-editor.org/rfc/rfc9207)), the mitigation for mix-up attacks added in #859. `extauth.PerformOIDCLogin`, which obtains the ID token for Enterprise Managed Authorization, only compared the `state`: the `Iss` reported by the `AuthorizationCodeFetcher` was ignored. So a response naming another issuer, or one without `iss` from an IdP that advertises `authorization_response_iss_parameter_supported`, was accepted and its code exchanged at the IdP's token endpoint. This checks `iss` right after the `state`, against the issuer and the flag from the IdP metadata that `initiateOIDCLogin` already fetches: - If the IdP advertises support, `iss` is required. - A present `iss` must equal the metadata issuer, whether or not support is advertised (RFC 9207 §2.4 asks clients to compare it). - A missing `iss` from an IdP that does not advertise support is accepted, as before. Unlike `AuthorizationCodeHandler`, a matching but unadvertised `iss` is accepted (what `AcceptUnadvertisedIss` opts into there): `PerformOIDCLogin` never rejected it, so refusing it now would break IdPs that send `iss` without advertising it, and RFC 9207 §2.4 asks for validation rather than rejection (see #1152). Tests: - `internal/oauthtest`: the fake IdP gets an `IssParameterSupported` option that advertises support and adds `iss` to its redirects. - `TestPerformOIDCLogin_Issuer`: advertised and not advertised, each with a missing, matching and foreign `iss`. The three rejected cases fail without the change; the existing `TestPerformOIDCLogin` cases are unchanged. _AI assistance: prepared with Claude Code._ Co-authored-by: Guglielmo Colombo <guglielmoc@google.com>
This PR functions as a Reference implementation of SEP-2468 / RFC9207.
This PR hardens the MCP OAuth Client functionality against Mix-Up attacks:
This PR hardens the client by adding support for a new
issparameter in authorization responses:issparameter via theauthorization_response_iss_parameter_supportedmetadata parameterissparameter in the authorization responseissparameter in the authorization response to theIssuerparameter in the authorization metadata. The two must match exactly for the response to be processed.Fixes #941