Skip to content
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

Proposal/discussion: OIDC requirement to ensure issuer URL == issuer claim #2003

Closed
deleterepo opened this issue Jul 26, 2024 · 13 comments · Fixed by #2159
Closed

Proposal/discussion: OIDC requirement to ensure issuer URL == issuer claim #2003

deleterepo opened this issue Jul 26, 2024 · 13 comments · Fixed by #2159
Assignees
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet 6) PR awaiting review V51 Group issues related to OAuth _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@deleterepo
Copy link

As per the discussion in #1969. From https://openid.net/specs/openid-connect-discovery-1_0.html#Security:

An attacker may attempt to impersonate an OpenID Provider by publishing a Discovery document that contains an issuer Claim using the Issuer URL of the OP being impersonated, but with its own endpoints and signing keys. This would enable it to issue ID Tokens as that OP, if accepted by the RP

We can have a requirement such as this:

Verify that relying parties ensure that the issuer URL they are using for the configuration request exactly matches the value of the issuer claim in the OpenID provider metadata document received by the relying party, and that this also exactly matches the iss claim value in ID tokens that are supposed to be from that issuer.

@elarlang

@elarlang elarlang added the V51 Group issues related to OAuth label Jul 26, 2024
@randomstuff
Copy link

Verify that relying parties ensure that the issuer URL they are using for the configuration request exactly matches the value of the issuer claim in the OpenID provider metadata document received by the relying party, and that this also exactly matches the iss claim value in ID tokens that are supposed to be from that issuer.

These look like two requirements:

  1. Verify that relying parties ensure that the issuer URL they are using for the configuration request exactly matches the value of the issuer claim in the OpenID provider metadata document received by the relying party
  2. Verify that this also exactly matches the iss claim value in ID tokens that are supposed to be from that issuer.

Could this be reformulated something like:

  1. Verify that the relying parties configured using OpenID Provider Configuration Information ensure that the issuer URL advertised in the OpenID Provider Configuration Information exactly matches the issuer URL of this OpenID provider.
  2. Verify that the issuer (iss) claim in the ID tokens received from an OpenID Provider exactly matches the issuer URL of this OpenID Provider.

All this could apply to plain OAuth as well.

@tghosth tghosth added 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet _5.0 - prep This needs to be addressed to prepare 5.0 labels Jul 29, 2024
@elarlang
Copy link
Collaborator

Do you think it is OAuth and OIDC specific requirement, or is it in general token validation requirement?

We have requirement:

# Description L1 L2 L3 CWE NIST §
3.5.6 [ADDED] Verify that other, security-sensitive attributes of a stateless token are being verified. For example, in a JWT this may include issuer, subject, and audience. 287

The changes for mentioned requirement are discussed in #1967

@elarlang
Copy link
Collaborator

ping @deleterepo

Do you think it is OAuth and OIDC specific requirement, or is it in general token validation requirement?

I personally would like to address with general requirement for tokens, with OIDC as an example if needed. See #1967 (comment)

@elarlang elarlang added the 2) Awaiting response Awaiting a response from the original poster label Sep 15, 2024
@elarlang
Copy link
Collaborator

Seems duplicate of #1826 or at least related.

@randomstuff
Copy link

Related : #2095 and Authorization Server Issuer Identification in general.

@elarlang
Copy link
Collaborator

elarlang commented Oct 5, 2024

Do we need a separate requirement for the OIDC Client section?

@elarlang
Copy link
Collaborator

ping @randomstuff

@elarlang elarlang removed the 2) Awaiting response Awaiting a response from the original poster label Oct 14, 2024
@randomstuff
Copy link

randomstuff commented Oct 14, 2024

I'm not sure about this one.

On the one hand, you want the issuer URL to be consistent with the URL of the metadata document (if any). On the other hand, in some cases, you want to override the URL of the metadata document because the metadata document is not exactly in the expected location.

And:

  1. sometime you might configure the issuer URL in the client and the metadata is automatically found;
  2. and in other cases, you might configure the metadata URL directly and the issuer will be automatically derived.

(But (2) is dangerous right? You probably should never let the issuer URL be derived from the metadata document because might, in some software, give the AS the ability to spoof another AS.)

Which means I'm not certain what wording would be OK in this context. (@deleterepo?)

@elarlang
Copy link
Collaborator

Is there a clear attack vector to address that? Maybe we can build a requirement based on the attack vector and just give a principle, what must be done or avoided?

In general - we should not write any requirement till we are not sure ourselves.

@randomstuff
Copy link

randomstuff commented Oct 15, 2024

Yes, the attack is described in the cited referenced by @deleterepo:

An attacker may attempt to impersonate an OpenID Provider by publishing a Discovery document that contains an issuer Claim using the Issuer URL of the OP being impersonated, but with its own endpoints and signing keys. This would enable it to issue ID Tokens as that OP, if accepted by the RP

I believe that the requirement proposed by @deleterepo can be reformulated to explicitly reference the type of attack we intend to mitigate.

Maybe something like (borrowing wording from the OIDC spec): "Verify that the client rejects attempts by a malicious authorization server to impersonate another authorization server through authorization server metadata. The client must reject authorization server metadata if the issuer URL in the authorization server metadata does not exactly match with a issuer URL statically configured in the client."

Among other things, I am not so happy with "statically configured" which should probably be replaced with something clearer.

@elarlang elarlang added the 4) proposal for review Issue contains clear proposal for add/change something label Oct 16, 2024
@TobiasAhnoff
Copy link

I´m not sure if this is better, but perhaps

"Verify that the client rejects attempts by a malicious authorization server to impersonate another authorization server through authorization server metadata. The client must reject authorization server metadata if the issuer URL in the authorization server metadata does not exactly match the pre-configured issuer URL expected by client."

@elarlang elarlang added the 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR label Oct 17, 2024
@elarlang
Copy link
Collaborator

elarlang commented Oct 17, 2024

Goes in via #2159

# Description L1 L2 L3
51.5.3 [ADDED] Verify that the client rejects attempts by a malicious authorization server to impersonate another authorization server through authorization server metadata. The client must reject authorization server metadata if the issuer URL in the authorization server metadata does not exactly match the pre-configured issuer URL expected by client.

elarlang pushed a commit to elarlang/ASVS that referenced this issue Oct 17, 2024
@elarlang elarlang added 6) PR awaiting review and removed 4) proposal for review Issue contains clear proposal for add/change something 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR labels Oct 17, 2024
@elarlang elarlang linked a pull request Oct 17, 2024 that will close this issue
elarlang pushed a commit that referenced this issue Oct 17, 2024
@randomstuff
Copy link

randomstuff commented Oct 17, 2024

For reference here is the wording from FAPI 2.0 draft:

Clients [...] shall ensure that the issuer URL used as the basis for retrieving the authorization server metadata is obtained from an authoritative source and using a secure channel, such that it cannot be modified by an attacker; shall ensure that this issuer URL and the issuer value in the obtained metadata match;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet 6) PR awaiting review V51 Group issues related to OAuth _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants