-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
oauth2 filter: nonce implementation refinements #36276
Comments
@denniskniep Thanks for opening this discussion. I appreciate the thorough review of the nonce implementation in the OAuth2 filter and agree that there are several improvements we can consider. Below are my thoughts: 1. Nonce Generation 2. One-time Use of Nonce
The 10-minute validity period was chosen as a reasonable window, as users typically complete the authentication process within that time frame. However, I agree that this doesn’t strictly enforce single-use. This could be improved by deleting the nonce cookie after the first use. 3. Utilization of OIDC Nonce Parameter 4. Including Nonce in the HMAC Cookie Thanks again for bringing these points to. Looking forward to your thoughts! |
Happy to hear your positive feedback. 2. One-time Use of NonceYou are right, we need to solve the problem, that multiple authentication requests could be sent, which results in multiple callbacks What about generating a unique nonce cookie linked to the authentication request? The cookie containing this nonce would have a unique identifier related to that specific request. This way, multiple requests won't cause issues because each callback will only delete the cookie related to its own request. To prevent the browser from being filled with unused authentication request nonce cookies (for example, if the callback never happens), we can restrict the cookie's validity period to 10 minutes. Example:
3. Utilization of OIDC Nonce ParameterI fully agree to create new issue for topic 3 and to model it as an opt-in feature (also to not break existing adoptions) 4. Including Nonce in the HMAC Cookie
That's actually true. And even more difficult if we might have multiple nonce cookies. Therefore I propose to implement this mechanism independent of the nonce. We could simply generate a secure, httpOnly, unique and non-guessable session cookie. That is always generated, when the Maybe we should also create this as a separate ticket, if we are not using the nonce value for this. |
This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions. |
not stale |
I just want to open a discussion about the newly introduced
nonce
cookie in the oauth2 filter.There are several aspects I want to highlight.
1. Generation
The current implementation sets a timestamp as nonce and encodes it into the state param of the oidc request.
envoy/source/extensions/filters/http/oauth2/filter.cc
Lines 177 to 180 in 4d99ab1
As far as I understood, the current implementation wants to mitigate CSRF. Which is considered a best practice (see here: OAuth 2.0 Threat Model and Security Considerations)
From my point of view setting the
nonce
to the current timestamp creates an opportunity for attackers to predict or guess thenonce
value. It would be better to use a randomly generated value combined with the current timestamp. The randomly generated value makes it non-guessable while the timestamp ensures uniqueness. If we apply a hash on the combined values then we get an opaque value.See the following as references:
https://datatracker.ietf.org/doc/html/rfc6749#section-10.12
https://datatracker.ietf.org/doc/html/rfc6819#section-5.3.5
https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
https://auth0.com/docs/secure/attack-protection/state-parameters#csrf-attacks
https://auth0.com/docs/glossary?term=nonce
2. One-time use
Furthermore the nonce cookie expires after 10 minutes.
envoy/source/extensions/filters/http/oauth2/filter.cc
Line 504 in 4d99ab1
Meaning there is a chance that the nonce is used for multiple authentication requests, which is not recommended.
See the following as references:
3. Use also oidc's nonce parameter
The current implementation uses the RECOMMENDED
state
parameter to mitigate CSRF, but there is also an OPTIONALnonce
parameter defined in the spec which mitigates replay attacks.Because we are using the nonce cookie already for the state parameter, we could easily use it for the nonce parameter. We must add another validation when we receive the ID Token. Validate that the ID Token contains a nonce claim, which must match the value of the nonce cookie. This would mitigate replay attacks.
see also:
https://openid.net/specs/openid-connect-core-1_0.html#NonceNotes
https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics#section-4.5.3.2
4. Include Nonce into the HMAC cookie
The entropy of the HMAC-Cookie could be enhanced by adding the nonce value. It would improve security especially in scenarios where the tokens are not stored into cookies. And therefore also not part of the HMAC-Cookie. Adding the Nonce value would ensure that the HMAC-Cookie incorporates always and in every scenario a unique and non-guessable value, which enhances security significantly.
Summary
In summary from my point of view we should do the following:
hash(randomValue + timestamp)
which would fulfill the requirements: unique, non-guessable & opaqueWould be happy to discuss this and I'm looking forward to your feedback!
cc: @zhaohuabing @mattklein123 @derekargueta
The text was updated successfully, but these errors were encountered: