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 - OAuth - sender-contrained refresh tokens #2110

Closed
elarlang opened this issue Sep 23, 2024 · 28 comments
Closed

V51 - OAuth - sender-contrained refresh tokens #2110

elarlang opened this issue Sep 23, 2024 · 28 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

@elarlang
Copy link
Collaborator

elarlang commented Sep 23, 2024

Spin-off from #2040 (comment)

Verify that refresh tokens for public clients are either sender-constrained, using DPoP or Certificate-Bound Access Tokens (mTLS), or use refresh token rotation to mitigate refresh token replay attacks. (L1, L2, L3)

The proposal updates the requirement we already have as 51.2.4

Verify that refresh tokens are sender-constrained or use refresh token rotation to prevent token replay attacks. Refresh token rotation prevents usage in the event of a compromised refresh token. Sender-constrained refresh tokens cryptographically binds the refresh token to a particular Client.


Verify that refresh tokens for public clients ... (L1, L2, L3)

For how many levels we allow "public clients"?

And we have a separate discussion/requirement for confidential clients in #2109

Verify that confidential clients must use client authentication in refresh token requests. (L1, L2, L3)

@elarlang elarlang added the V51 Group issues related to OAuth label Sep 23, 2024
@randomstuff
Copy link

Verify that refresh tokens for public clients are either sender-constrained, using DPoP or Certificate-Bound Access Tokens (mTLS), or use refresh token rotation to mitigate refresh token replay attacks. (L1, L2, L3)

I'm wondering whether it would make sense to require the usage of sender-constrained refresh tokens (as opposed to refresh token rotation) in L3.

For how many levels we allow "public clients"?

Maybe, this does not really matter in this verification but would be the object of another verification (?). Even if public clients are disallowed in L3, you would still want to have (because some L3 applications won't be 100% conformant from day one):

Verify that refresh tokens for public clients are either sender-constrained, using DPoP or Certificate-Bound Access Tokens (mTLS), or use refresh token rotation to mitigate refresh token replay attacks. (L1, L2, L3)

@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 Sep 23, 2024
@elarlang
Copy link
Collaborator Author

We must be aligned with proposal from #2038

Verify that all clients are confidential and configured to use strong client authentication methods, i. e. 'mTLS' or 'private-key-jwt'.

@jmanico
Copy link
Member

jmanico commented Sep 23, 2024 via email

@TobiasAhnoff
Copy link

I'm wondering whether it would make sense to require the usage of sender-constrained refresh tokens (as opposed to refresh token rotation) in L3.

From OAuth 2.1 and FAPI 2, as far as I understand, refresh-token misuse is primary mitigated by client authentication for confidential clients. So, L3 is covered by requiring confidential clients (with strong authentication methods) and client authentication for all token requests (#2009)

For public clients (which I think needs to be allowed for L1 and L2) OAuth 2.1 states: Either DPoP/mTLS or Refresh-token rotation. Which was the intent to capture with

Verify that refresh tokens for public clients are either sender-constrained, using DPoP or Certificate-Bound Access Tokens (mTLS), or use refresh token rotation to mitigate refresh token replay attacks. (L1, L2, L3)

But e g FAPI 2 notes that refresh-token rotation is a weaker mitigation (and sometimes problematic), so, as I think @jmanico suggests, ASVS should reflect this and more clearly recommend DPoP or mTLS over just rotation, correct? Perhaps like this

Verify that refresh tokens for public clients are sender-constrained using DPoP or Certificate-Bound Access Tokens (mTLS) and, if appropriate, also use refresh token rotation to mitigate refresh token replay attacks. (L1, L2, L3)

@elarlang
Copy link
Collaborator Author

Verify that refresh tokens for public clients are sender-constrained using Demonstrating Proof of Possession (DPoP) or Certificate-Bound Access Tokens (mTLS) and, if appropriate, also use refresh token rotation to mitigate refresh token replay attacks.

@randomstuff - any further feedback or need for changes?

@randomstuff
Copy link

I think that requiring sender-constrained tokens even for L1 is a little drastic. Using sender-constrained access tokens all the time is a good idea but I'm not sure this matches much with the current state of software/frameworks, etc. This might therefore be a little too much for L1?

@elarlang
Copy link
Collaborator Author

I agree with that. To this direction?

Verify that the authorization server mitigates refresh token replay attacks for public clients by using (L1, L2) refresh token rotation or (L3) sender-constrained refresh tokens using Demonstrating Proof of Possession (DPoP) or Certificate-Bound Access Tokens (mTLS).

@randomstuff
Copy link

Maybe:

Verify that the authorization server mitigates refresh token replay attacks for public clients by using (L1, L2) refresh token rotation or (L1, L2, L3) sender-constrained refresh tokens using Demonstrating Proof of Possession (DPoP) or Certificate-Bound Access Tokens (mTLS).

@randomstuff
Copy link

randomstuff commented Sep 30, 2024

Verify that the authorization server mitigates refresh token replay attacks for public clients by using (L1, L2) refresh
token rotation or (L1, L2, L3) sender-constrained refresh tokens using Demonstrating Proof of Possession (DPoP) or
Certificate-Bound Access Tokens (mTLS).

Actually, I noticed that this conflicts with DPoP:

Refresh tokens issued to confidential clients (those having established authentication credentials with the authorization server) are not bound to the DPoP proof public key because they are already sender-constrained with a different existing mechanism.

The existing mechanism being client authentication.

@randomstuff
Copy link

Actually, I noticed that this conflicts with DPoP

Well actually no, it does not because of "public clients" 🤦

@elarlang
Copy link
Collaborator Author

So, all good or need for changes?

@elarlang
Copy link
Collaborator Author

elarlang commented Oct 5, 2024

Current 51.2.4:

# Description L1 L2 L3
51.2.4 [ADDED] Verify that the authorization server mitigates refresh token replay attacks for public clients by using (L1, L2) refresh token rotation or (L1, L2, L3) sender-constrained refresh tokens using Demonstrating Proof of Possession (DPoP) or Certificate-Bound Access Tokens (mTLS).

For refresh token rotation - we should require revoking previous refresh token, as decribed in:
https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-11#section-4.3.1-10.2.1

@randomstuff
Copy link

For refresh token rotation - we should require revoking previous refresh token, as decribed in:
https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-11#section-4.3.1-10.2.1

I was going to say that this is implied in "refresh token rotation" but actually that's not true. So you are right! 👍

@elarlang
Copy link
Collaborator Author

elarlang commented Oct 9, 2024

Proposal for add-on to the current requirement 51.2.4:

If the refresh token rotation is used, verify that the authorization server invalidates the refresh token after usage and revokes all refresh tokens for that authorization if an already used and invalidated refresh token is provided.

"for that authorization" should be thought carefully - is it correct and is it understandable?

@randomstuff
Copy link

for that authorization" should be thought carefully - is it correct and is it understandable?

I think the word we are looking for is the same we are discussing in #1968.

@randomstuff
Copy link

randomstuff commented Oct 9, 2024

@elarlang, Yes, I think "authorization" if probably the better word.

See these snippets from the Oauth 2.1 draft:

The access token represents the authorization granted to the client.

[...]

An authorization grant represents the resource owner's authorization (to access its protected resources) used by the client to obtain an access token.

[...]

Access tokens are credentials used to access protected resources. An access token is a string representing an authorization issued to the client.

Ad discussed in #1968, grant could be used to have a similar meaning of "granted authorization" but in OAuth it is always used to mean "authorization grant" which is some concrete representation of a granted authorization (such as a refresh token or an authorization code).

@randomstuff
Copy link

Wording nitpick: " If the refresh token rotation is used" → " If refresh token rotation is used"?

Otherwise LGTM.

@elarlang elarlang added the 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR label Oct 10, 2024
elarlang pushed a commit to elarlang/ASVS that referenced this issue Oct 12, 2024
@elarlang elarlang added 6) PR awaiting review and removed 4) proposal for review Issue contains clear proposal for add/change something 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR labels Oct 12, 2024
tghosth pushed a commit that referenced this issue Oct 13, 2024
@TobiasAhnoff
Copy link

A little late...but the way I read 51.2.4 now, it is hard to understand that sender-constrained should be used (if possible) since that is a stronger mitigation than rotation. For L3 that is somewhat clear since rotation as mitigation is only for L1 and L2, but for L1 and L2 we seem to suggest that they are equally strong mitigations, that it doesn´t matter which one you choose.

Perhaps this is more clear, but a bit long?

Verify that the authorization server mitigates refresh token replay attacks for public clients by using refresh token rotation or sender-constrained refresh tokens using Demonstrating Proof of Possession (DPoP) or Certificate-Bound Access Tokens (mTLS). Note that for L3 sender-constrained refresh tokens are required, while preferred for L1 and L2, since it is a stronger form of mitigation than rotation. If refresh token rotation is used, verify that the authorization server invalidates the refresh token after usage and revokes all refresh tokens for that authorization if an already used and invalidated refresh token is provided.

@randomstuff
Copy link

What about something such as:

[ADDED] Verify that the authorization server mitigates refresh token replay attacks for public clients by using either sender-constrained refresh tokens (i.e. DPoP or Certificate-Bound Access Tokens) if possible or, if not possible and for L1 only, refresh token rotation. If refresh token rotation is used, verify that the authorization server invalidates the refresh token after usage and revokes all refresh tokens for that authorization if an already used and invalidated refresh token is provided.

@elarlang
Copy link
Collaborator Author

One more version:

Verify that the authorization server mitigates refresh token replay attacks for public clients. For L1, L2, and L3 applications sender-constrained refresh tokens (i.e. DPoP or Certificate-Bound Access Tokens) can be used. For L1 applications it is an option to use refresh token rotation, and if used, verify that the authorization server invalidates the refresh token after usage and revokes all refresh tokens for that authorization if an already used and invalidated refresh token is provided.

@randomstuff
Copy link

randomstuff commented Oct 17, 2024

Verify that the authorization server mitigates refresh token replay attacks for public clients, preferably using sender-constrained refresh tokens (i.e. DPoP or Certificate-Bound Access Tokens). For L1 applications only, refresh token rotation may be used instead. If refresh token rotation is used, verify that the authorization server invalidates the refresh token after usage and revokes all refresh tokens for that authorization if an already used and invalidated refresh token is provided.

@elarlang elarlang added the 4) proposal for review Issue contains clear proposal for add/change something label Oct 17, 2024
@TobiasAhnoff
Copy link

TobiasAhnoff commented Oct 18, 2024

Yet another version (mix of the two)

Verify that the authorization server mitigates refresh token replay attacks for public clients. For L1, L2, and L3 applications sender-constrained refresh tokens (i.e. DPoP or Certificate-Bound Access Tokens) should be used. For L1 applications it is an option to use refresh token rotation, and if used, verify that the authorization server invalidates the refresh token after usage and revokes all refresh tokens for that authorization if an already used and invalidated refresh token is provided.

But I think I like the one from @randomstuff better

elarlang pushed a commit to elarlang/ASVS that referenced this issue Oct 20, 2024
@elarlang elarlang added 6) PR awaiting review and removed 4) proposal for review Issue contains clear proposal for add/change something labels Oct 20, 2024
@randomstuff
Copy link

Is there any need for a requirement about DPoP proof replay attacks?

@elarlang
Copy link
Collaborator Author

Updated via #2170

# Description L1 L2 L3
51.2.4 [ADDED] Verify that the authorization server mitigates refresh token replay attacks for public clients, preferably using sender-constrained refresh tokens (i.e. Demonstrating Proof of Possession (DPoP) or Certificate-Bound Access Tokens (mTLS)). For L1 applications only, refresh token rotation may be used instead. If refresh token rotation is used, verify that the authorization server invalidates the refresh token after usage and revokes all refresh tokens for that authorization if an already used and invalidated refresh token is provided.

@elarlang
Copy link
Collaborator Author

Is there any need for a requirement about DPoP proof replay attacks?

seems like a separate requirement and worth a separate issue?

@elarlang
Copy link
Collaborator Author

ping @randomstuff

@randomstuff
Copy link

I opened this issue about DPoP proof replay attack #2188

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

5 participants