-
Notifications
You must be signed in to change notification settings - Fork 19
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
Additional options for wallet-provided nonces for PoP #357
Comments
The From a security and design perspective, when the RS supports the issuance of multiple credentials using the same Access Token (AT), it is practical to issue a c_nonce in the RS's response; in this way the RS hints the client about its support of multiple issuance with the same AT. This c_nonce should then be expected in the next request to the RS, utilizing the nonce parameter. Given these considerations, I support option 2, using the jti (JWT ID). This choice avoids conflicts with the optional nature of the nonce during the initial request and aligns with the requirement for a nonce in subsequent requests if it is provided in the RS's response. |
@peppelinux Can you please clarify - to understand your comment - whether the RS term corresponds to the Credential Issuer (protected endpoints)? |
@babisRoutis yes, the credential endpoint is an RS in the OAuth 2.0 paradigm |
I think the Would this be a purely Wallet side mechanism, no Issuer involvement needed? |
@nemqe yes jti is self-issued by the issuer, it's a pure identifier that uniquely identifies a JWT within its issuer a non-exaustive comparison between nonce and jti was outlined in the following I-D: https://peppelinux.github.io/draft-demarco-oauth-nonce-endpoint/draft-demarco-oauth-nonce-endpoint.html#name-considerations-about-nonce- |
@peppelinux I was thinking more about who is protecting who in this case. Wasn't sure where the responsibility for safety of this interaction lies, and if the Issuer will need to keep track of |
jti unlike nonce is not absolute but relative to its issuer nonce is absolute because who evaluates its uniqueness is at the same time its issuer jti is "unique together" with its issuer and who evaluates its uniqueness must keep track of it until the jwt in which it is contained is active and not expired (or revoked, see introspection endpoint) for this reason nonce is cheaper, because we "suppose" that a jwt may have a longer lifetime of a nonce that is it issued just in time for a specific transaction, where this transaction assumes a short duration. I support jti to uniquely identify a jwt, together with its issuer, like a best practice (you see in all the italian specs its presence). |
the RS is the party that must protect itself from replay attacks, therefore it should keep track of jwts against any replay of these, until the jwts expires. |
It is also possible to include both, |
These options remind me DPoP DPoP JWT contains Anyway, with regards to the issue I strongly believe that a single mandatory solution would be better than have multiple alternative (yet optional) choices. Should
if |
Small brain dump, I apologize in advance for possibly saying things people know, have discussed, or just plain incorrect things The way I see it:
Being a defensive Wallet I might not trust the Issuer and his way of generating tokens, and for this reason I would want to introduce a Now the problem with a Meaning:
|
@nemqe I believe I agree with this. @babisRoutis on removing |
On the DCP WG call on June 27th, we concluded that
c_nonce
is in fact optional. To mitigate the attack discussed here #19 without ac_nonce
, several options were brought up and we agreed to create a dedicated issue for this.Some options are:
ath
(access token hash)jti
(chosen by the wallet with enough entropy)nonce
(keep nonce in PoP but allow wallet-generated nonces similar tojti
; probably not a good idea because in some situations/policies a server might fail distinguishing betweenc_nonce
values and wallet-generated nonces in thenonce
claim)(cc @David-Chadwick @Sakurann @jogu @bc-pi @babisRoutis)
The text was updated successfully, but these errors were encountered: