openidconnect-rs
openidconnect-rs copied to clipboard
Note about the security of `Nonce::new_random`
Hello and thank you for this great library !
I am implementing a web server and was wondering whether Nonce::new_random was not misleading, especially since it is used without warning in the docs' examples.
The oidc spec advises:
15.5.2. Nonce Implementation Notes
The nonce parameter value needs to include per-session state and be unguessable to attackers. One method to achieve this for Web Server Clients is to store a cryptographically random value as an HttpOnly session cookie and use a cryptographic hash of the value as the nonce parameter. In that case, the nonce in the returned ID Token is compared to the hash of the session cookie to detect ID Token replay by third parties. A related method applicable to JavaScript Clients and other Browser-Based Clients is to store the cryptographically random value in HTML5 local storage and use a cryptographic hash of this value.
https://openid.net/specs/openid-connect-core-1_0.html#NonceNotes
But if we follow, the example code from the documentation:
client.authorize_url(
CoreAuthenticationFlow::AuthorizationCode,
CsrfToken::new_random,
Nonce::new_random,
)
then the nonce would be generated and immediately included in the url, instead of being generated, then stored, then hashed, then being included in the url as a hash.
Maybe the docs should warn about the example being inappropriate in the common case where the nonce has to be stored separately in a cookie or a local storage ?
Ideally, maybe the library itself could handle this case, and the url should include a HashedNonce instead of a Nonce, and the verification function should take a Nonce and a HashedNonce instead of two nonces ?