-
-
Notifications
You must be signed in to change notification settings - Fork 668
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 nonce claim requirement #2002
Comments
We also have requirement
I prefer the requirement to say clearly, what security problem it solves. E g for be sure that the ID-token is sent to the same client that started the auth* process, check the nonce parameter before accepting the token. |
Well, some input for feedback and wordsmithing. Terminology and grammar needs validation as well:
|
Suggestion:
Verify that the OAuth client mitigates replay attacks by ensuring that the `nonce` claim in the ID token matches the `nonce` value sent in the authentication request to the authorization server.
|
So the proposal is delete current 51.2.2 and add new requirement to OIDC Client section (not made yet):
Should be in the requirement also OIDC client? |
A thing to note here is that when implementing the code flow according to FAPI2 there seems to be no need to verify the ID token in the same way as when using other OIDC flows, see A suggestion to align with that is to have
But from reading OIDC specs, at this point, I´m not 100% sure that all 3 conditions are a requirement for skipping the nonce and signature check as FAPI2 states? So perhaps just have (as suggested in previous comments)
|
@TobiasAhnoff, when receiving the ID token directly from the backend (authorization code flow), OpenID Connect allows you to skip the signature verification. This is explained in 3.1.3.7 (6) :
As for skipping the nonce signature, this is because using PKCE is considered to already protect against replay attacks and ensure freshness. If this is good enough for FAPI 2.0, I believe this might be good enough for ASVS (right?). (However, as the |
If we are not sure what is the latest-latest-latest technical solution or we are not sure how mature is the latest spec and will it be used in practice, then we should not take the risk and write it to the requirement as "hard-coded one-way to do things". We can go with requirements like: Verify that OAuth client mitigates ID-token replay attacks. For example, by ensuring that the nonce claim in the ID token matches the nonce value sent in the authentication request to the authorization server OR /using something else/. From the defense point of view, we should only care, that it is not possible to do the attack. How it is achieved, is not that important. |
@randomstuff thank you, there are a lot of specs to read! Perhaps this is enough for an ASVS verification?
|
This simplicity works for me, everything that must be said by ASVS is said, and the rest is the responsibility for projects like testing guides or cheat sheet series. |
What should be the chapter name for this one? |
I would suggest "OAuth Replay Attack Prevention" |
Since ID Tokens are OIDC (not OAuth) a suggestion is to use terminology from the OIDC spec (see https://openid.net/specs/openid-connect-core-1_0.html#CodeIDToken), perhaps:
And the chapter name "OIDC ID Token Replay Attack Prevention" Or should we keep using some OAuth terminology e g "authorization request" and "authorization server"? |
Then, it should be "Verify that the Relying Party mitigates ID token replay attacks. […]” |
Yes, but maybe we should also clarify that for ID Tokens the Relying Party is the client, not the OAuth Resource Server (which sometimes also is referred as a relying party, and the OIDC spec use the term Client
@randomstuff Maybe this better captures the OIDC terminology?
|
Let's go with that, propose section name for that as well. |
@TobiasAhnoff, yes you are right OpenID Connect uses the term "client" throughout so maybe there was no need to ass the "relying party" wording in there after all. |
Asking feedback for the idea (the last sentence in brackets)
Chapter "OIDC Client"? |
I think the chapter/section heading should be "OIDC Client", but I´m not sure on the "bracket sentence".... One argument is that is should not be needed, the reader should understand how OIDC and OAuth terminology overlaps. I´m not sure if this is better but an alternative is to have:
Perhaps this will be easier to decide when we see the full OAuth/OIDC chapter? |
We have:
The current FAPI 2.0 draft have:
Should we align with this and require the usage of PKCE at least for L3 (even if |
Just in case saying, that the referenced requirement was proposed to be deleted and now is deleted. |
Yes, I think so, maybe 51.51. should be a mix with the deleted 51.3.2?
|
PKCE is becoming fairly standard. I suggest level 2.
|
@jmanico, Actually usage of PKCE is mostly required by the OAuth 2.1 draft so I think it would make sense to require it in L1:
|
I support that as well. |
The scope for this issue is 51.5.1. The goal of the requirement is to mitigate ID token replay attacks. With PKCE you take down the vector, that the client can not ask for tokens from AS, with ID token nonce you take down the vector, that the client does not accept ID token, if it is not from the user's own authorization flow/transaction. From a testing/verifying point of view - I would say, that the nonce solution, that is made for exactly that, is the first preferred option. If in practice I can not do the replay attack, then whatever defense works, including PKCE. Where I'm wrong? There is some discussion about levels. ID token replay, requirement 51.5.1, and PKCE usage, requirement 51.2.3 - both are level 1 at the moment. |
PKCE was originally designed to prevent authorization code injection bit it does have an indirect benefit of stopping some replay attacks. PKCE adds valuable security, but it shouldn’t be seen as the main line of defense for ID token replay in the context of 51.5.1. From a testing perspective, the nonce solution seems preferred - since it directly stops ID token replay. But as you said, if a defense (like PKCE) blocks the replay attack in practice, it’s still valid, even if it's not targeted a lot. Both requirements (51.5.1 for ID token replay and 51.2.3 for PKCE) are level 1 because they address serious risks but in slightly different contexts. I'd say this makes sense. I am just trying to see if I can reading your comments right, are we in sync? |
Your translation from my comments is ok. But I can not understand the problem to solve here. This is the requirement as it stands:
From the comments above, I'm not convinced we need to change the requirement to have PKCE focus as the 1st option. PKCE is required for code flow anyway from level 1 by 51.2.3. |
I would consider adding this to the end of 51.5.1 Additionally, while PKCE primarily protects against authorization code injection, it can also provide an extra layer of defense by securing the authorization code exchange, indirectly helping to mitigate some replay attacks. |
It is already long and it's going to cheat sheet side already. The only required part of the requirement is:
If the requirement is not wrong or we are not against any best practices or recommendation, I prefer to stay with that requirement as it is. |
I think that's fair Elar. I do not think it really needs to be added. I was just throwing that in there in case you or others want it. |
51.5.1 may not need to change, but I still feel it is unclear that when using a backend client with code+PKCE there is no need to validate ID Token nonce, a suggestion is to have
|
I think my comment (#2002 (comment)) is still valid. If I lack some important nuances here, please explain this during the meeting. |
As per the discussion in #1969, 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
We can have a requirement such as this:
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.
@elarlang
The text was updated successfully, but these errors were encountered: