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

V51: Additional OAuth/OIDC proposals #1969

Open
deleterepo opened this issue May 22, 2024 · 7 comments
Open

V51: Additional OAuth/OIDC proposals #1969

deleterepo opened this issue May 22, 2024 · 7 comments
Assignees
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet V51 Group issues related to OAuth Will be closed if no response/opposite arguments _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@deleterepo
Copy link

deleterepo commented May 22, 2024

@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 #1925.

  1. 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.
  2. 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.
  3. Verify that the authorization server compares redirect URIs using exact string matching.
  4. 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.
  5. 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 elarlang added the V51 Group issues related to OAuth label May 22, 2024
@elarlang
Copy link
Collaborator

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

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: #1967

@deleterepo
Copy link
Author

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
Copy link
Collaborator

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.

@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 May 23, 2024
@damienbod
Copy link

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
Copy link
Collaborator

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 - #1970

Please share your argument there :)

@jmanico
Copy link
Member

jmanico commented May 27, 2024

I politely disagree with the comment below. See #1970 (comment)

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
Copy link
Collaborator

For me it seems we have all risen topics under discussion in separate issues and we can close this one.

ping @deleterepo

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 V51 Group issues related to OAuth Will be closed if no response/opposite arguments _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

No branches or pull requests

6 participants