V51: Additional OAuth/OIDC proposals

deleterepo commented 1 month ago

@elarlang and team I'll begin with a few proposals of my own here to start the discussion about them (more to come). I'll try not to duplicate the ones already mentioned here https://github.com/OWASP/ASVS/issues/1925.

Verify that the Implicit Flow grant is not used or configured by the Authorization Server, as it is vulnerable to access token leakage and access token replay attacks.
- This applies to both OAuth and OIDC. As per https://oauth.net/2/grant-types and https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics#name-implicit-grant. If preferred we can consolidate this with 51.1.6 about Resource Owner Password Credentials Grant, but it might make sense to make it a separate requirement to keep it granular, because these grants are vulnerable to different attacks that we are highlighting.
Verify that the unique identifier in the ID token for an end-user is a combination of the sub claim and the iss claim. Avoid usage of other claims such as email, phone_number, preferred_username, or name, as they may be mutable and can be potentially falsified.
- This is specific to OIDC and goes back to issues such as the vulnerability reported from Microsoft that I mentioned in my other issue here: https://github.com/OWASP/ASVS/issues/1826
- @jmanico also touches on this in https://github.com/OWASP/ASVS/issues/996#issuecomment-2003085972 for the * Prevent Authorization Server Mix-Up Attacks: comment
Verify that the authorization server compares redirect URIs using exact string matching.
- This applies to both OAuth and OIDC. Using anything other than exact string matching, such as substrings, patterns etc opens up the door to open redirects. See here: https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics#name-insufficient-redirect-uri-v.
- I see this was mentioned here: https://github.com/OWASP/ASVS/issues/1965, and perhaps this can be the verification requirement for it
Verify that the nonce parameter sent in the authorization request is included in the nonce claim of the issued ID token, and that the client ensures that the nonce claim value is equal to the original value sent in the authorization request.
- This applies to OIDC. The nonce claim is a specific OIDC claim to bind an ID token to a client, to mitigate replay attacks. See here: https://openid.net/specs/openid-connect-core-1_0.html#IDToken
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.
- This applies to OIDC. 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

Please let me know what you think and happy to discuss these further. More to come!

elarlang commented 1 month ago

Thank you for your input, I do first quick feedback round:

proposal 1

I opened a separate issue, with a point of view that we just need to accept PKCE: https://github.com/OWASP/ASVS/issues/1970

proposal 2

Is it already covered by the issue #1826, and if not, what is the difference?

proposal 3

We have #1965 for this opened now.

proposal 4

I think we have requirement 51.2.2 to cover it.

If you have a proposal, how to improve it, please open a separate issue for that.

If you would like to touch the topic from a different angle (as having a new separate requirement), then what could it be?

proposal 5

Is it OIDC specific or is it general JWT validation (https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.1)?

V3.5.6 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.

For mentioned requirement, we also have a opened issue: https://github.com/OWASP/ASVS/issues/1967

deleterepo commented 1 month ago

Thank you for your input, I do first quick feedback round:

proposal 1

I opened a separate issue, with a point of view that we just need to accept PKCE: #1970

Sounds good let's continue the discussion in #1970. Please see my latest comments.

proposal 2

Is it already covered by the issue #1826, and if not, what is the difference?

It's covered. Let's keep the discussion going in #1826. Part of these proposals is pointing out what is OIDC-specific, in case we want to have a separate section for that in V51. I also wanted to make sure this issue is still considered as it seems to be an important requirement.

proposal 3

We have #1965 for this opened now.

👍

proposal 4

I think we have requirement 51.2.2 to cover it.

If you have a proposal, how to improve it, please open a separate issue for that.

If you would like to touch the topic from a different angle (as having a new separate requirement), then what could it be?

For 51.2.2 it might be best if I open a separate issue. I think this requirement has too much going on, as it states that we need to only use PKCE (see comments in #1970), and it also talks about using nonce when not using PKCE. PKCE or more specifically Authorization Code flow with PKCE can be used for both OAuth and OIDC, whereas the nonce is specific to OIDC. So the more I think about it the more it makes sense to have a separate section for OIDC, to avoid confusion. One way to start with that is to move or break out any requirements that involve the ID token. What do you think about that?

proposal 5

Is it OIDC specific or is it general JWT validation (https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.1)?

V3.5.6 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.

For mentioned requirement, we also have a opened issue: #1967

This requirement I mention is specific to OpenID Provider Issuer Discovery. See here: https://openid.net/specs/openid-connect-discovery-1_0.html#IssuerDiscovery. This requirement ensures that the issuer URL used to perform the OpenID Provider configuration request for the configuration document at /.well-known/openid-configuration, is the same as the issuer URL (claim) in the document itself, and that the same issuer URL is validated against the one in the ID token.

For example, here is one of Google's: https://accounts.google.com/.well-known/openid-configuration?callback=angular.callbacks._0. To perform issuer Discovery, you would make a GET request to retrieve this document, ensuring that the issuer claim matches https://accounts.google.com. Subsequent ID tokens issued from the token endpoint should have iss match https://accounts.google.com as well.

elarlang commented 1 month ago

Agree with

to open separate issue for proposal 4
to make separate section for OIDC (it was basically agreed long time ago somewhere else), just let's get first requirement(s) in
ack, it makes sense to open separate issue for proposal 5 as well

If separate issues are opened, we can close this issue.

For requirements - first let's have proposal in the issue, and if it finds agreement, then it's time for PR.

damienbod commented 1 month ago

A confidential Implicit Flow grant is fine to use if only the id_token is returned and no access tokens are used/allowed in this flow.

This is sometimes required.

I suggest maybe changing the text in proposal 1 a bit?

elarlang commented 1 month ago

A confidential Implicit Flow grant is fine to use if only the id_token is returned and no access tokens are used/allowed in this flow.

This is sometimes required.

I suggest maybe changing the text in proposal 1 a bit?

Hi @damienbod , I understand the comment is on "proposal 1". We have a separate issue for this opened now - https://github.com/OWASP/ASVS/issues/1970

Please share your argument there :)

jmanico commented 1 month ago

I politely disagree with the comment below. See https://github.com/OWASP/ASVS/issues/1970#issuecomment-2132789742

A confidential Implicit Flow grant is fine to use if only the id_token is returned and no access tokens are used/allowed in this flow.

This is sometimes required.

I suggest maybe changing the text in proposal 1 a bit?

OWASP / ASVS