OWASP / ASVS

Application Security Verification Standard
Creative Commons Attribution Share Alike 4.0 International
2.69k stars 653 forks source link

V51 OAuth: Add refresh token verfications #2040

Open TobiasAhnoff opened 2 weeks ago

TobiasAhnoff commented 2 weeks ago

A suggestion is to add the following verifications to address refresh tokens:

V51.2 Authorization Server

Verify that refresh tokens for public clients are sender-constrained (confidential clients must use client authentication in refresh token requests). Note that refresh token rotation to prevent e g token replay attacks is no longer recommended, but allowed. (L1, L2, L3)

Verify that refresh and reference tokens can be revoked. (L1, L2, L3)

Verify that refresh tokens have an absolute expiration. Even if sliding refresh token expiration is applied, re-authentication should be forced at some point in time. Note that how often depends on the application in scope, from hours to months. (L1, L2, L3)

elarlang commented 2 weeks ago

Verify that refresh tokens for public clients are sender-constrained (confidential clients must use client authentication in refresh token requests). Note that refresh token rotation to prevent e g token replay attacks is no longer recommended, but allowed. (L1, L2, L3)

It is / replaces current 51.1.4

It is not clear now, are we saying, that refresh token rotation is not ok or it is ok, but is is recommended to use sender-contstrained tokens?

Verify that refresh and reference tokens can be revoked. (L1, L2, L3)

I'm not sure we need this as separate requirement, we should instead cover it in general token handling section. Till it is not clearly covered elsewhere, we can keep this requirement in.

Verify that refresh tokens have an absolute expiration. Even if sliding refresh token expiration is applied, re-authentication should be forced at some point in time. Note that how often depends on the application in scope, from hours to months. (L1, L2, L3)

Depends on the refresh token rotation requirement - if we kind of support refresh token rotation, it should be clearly said, that when rotating the token, the lifetime for the token must stay the same (and not be extended)

related or partly duplicate to https://github.com/OWASP/ASVS/issues/1968

TobiasAhnoff commented 2 weeks ago

Yes, the intention is to modify/replace 51.1.4 to align with (as far as I now) the latest best current practices, see FAPI at https://openid.bitbucket.io/fapi/fapi-2_0-security-profile.html#section-5.3.2.1 which in Note 2 states:

The use of refresh token rotation does not provide security benefits when used with confidential clients and sender-constrained access tokens. This specification prohibits the use of refresh token rotation for security reasons as it causes user experience degradation and operational issues whenever the client fails to store or receive the new refresh token and has no option to retry.

However, as refresh token rotation may be required from time to time for infrastructure migration or similar extraordinary circumstances, this specification allows it, provided that authorization servers offer clients the time-limited option to retry with the old refresh token in case of failure. Implementers need to consider a secure mechanism for clients to recover from a loss of a new refresh token on issue. The details of this mechanism are outside the scope of this specification.

Compared the old from 2022-2023 :), where refresh-token rotation was defined as an alternative to sender-contrained tokens, see https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-10#section-4.3

The authorization server MUST verify the binding between the refresh token and client identity whenever the client identity can be authenticated. When client authentication is not possible, the authorization server SHOULD issue sender-constrained refresh tokens or use refresh token rotation as described in Section 4.3.1.

Maybe the note-part should be removed and just have:

Verify that refresh tokens for public clients are sender-constrained (confidential clients must use client authentication in refresh token requests). (L1, L2, L3)

or, since other suggested verifications in #2038 cover client authentication and sender-constrained token requirements, simply have:

Verify that refresh tokens for public clients are sender-constrained. (L1, L2, L3)

This is (in my opinion) a good example of how hard it is to keep ASVS updated with OAuth/OIDC BCPs and security profiles, and the importance a well-defined scope etc (#2036)

elarlang commented 1 week ago

Verify that refresh and reference tokens can be revoked. (L1, L2, L3)

It should be handled together and to solve also 3.5.1 from https://github.com/OWASP/ASVS/issues/1917

csfreak92 commented 1 week ago

The use of refresh token rotation does not provide security benefits when used with confidential clients and sender-constrained access tokens. This specification prohibits the use of refresh token rotation for security reasons as it causes user experience degradation and operational issues whenever the client fails to store or receive the new refresh token and has no option to retry. However, as refresh token rotation may be required from time to time for infrastructure migration or similar extraordinary circumstances, this specification allows it, provided that authorization servers offer clients the time-limited option to retry with the old refresh token in case of failure. Implementers need to consider a secure mechanism for clients to recover from a loss of a new refresh token on issue. The details of this mechanism are outside the scope of this specification.

@TobiasAhnoff, it seems to be a fair point, but since the details of that mechanism (Implementers need to consider a secure mechanism for clients to recover from a loss of a new refresh token on issue) is out of scope of the specification, it will read and sound like we are not sure what to recommend either. I guess this is where I see a problem with it. I would be more cautious with it as it sounds like there is no proposed solution either and it is up to the implementer to figure it out which might lead to more design/implementation flaws.

Just my opinion on this, if we cannot have a bit of a solid solution for any user of ASVS that would be harder to justify using it. I am inclined to keep the current 51.1.4 unless there's a clearer path what to do with this dilemma. What do you guys think @elarlang, @TobiasAhnoff, @deleterepo?

randomstuff commented 1 week ago

Note that refresh token rotation to prevent e g token replay attacks is no longer recommended, but allowed.

When I read this, the first thing I did was check what the OAuth 2.1 draft had to say about that (they mention the option of using refresh token rotation would discouraging its use). If this is to be kept, maybe something like:

Note that refresh token rotation to prevent e g token replay attacks is prohibited by FAPI 2.0.

randomstuff commented 1 week ago

Verify that refresh and reference tokens can be revoked.

Note: I did not know what a reference token was :) but it is mentioned in the current OAuth 2.1 draft:

Access tokens generally fall into two categories: reference tokens or self-encoded tokens. Reference tokens can be validated by querying the authorization server or looking up the token in a token database, whereas self-encoded tokens contain the authorization information in an encrypted and/or signed string which can be extracted by the resource server.

However, it is not mentioned in current OAuth specifications AFAIU so maybe it might be worthwhile to add a short note explaining the term.

Also we are talking about reference tokens which are access tokens, right? Or does this apply to other kind of tokens as well? Can we say "reference access token" (or would it make it more difficult to find what we are talking about?). Maybe "access tokens which are reference tokens"?

randomstuff commented 1 week ago

Verify that refresh and reference tokens can be revoked.

Do we need to clarify what kind of revocation we are talking about? Revocation by the user using the AS user interface ? Revocation by the client using the token revocation API? Both?

randomstuff commented 1 week ago

Verify that refresh tokens for public clients are sender-constrained (confidential clients must use client authentication in refresh token requests).

Maybe add something like "This can be achieved using DPoP or Certificate-Bound Access Tokens." in order to make it clearer for the layman what we are talking about?

TobiasAhnoff commented 1 week ago

Do we need to clarify what kind of revocation we are talking about? Revocation by the user using the AS user interface ? Revocation by the client using the token revocation API? Both?

A clarification including both cases will be good to add.

TobiasAhnoff commented 1 week ago

Also we are talking about reference tokens which are access tokens, right? Or does this apply to other kind of tokens as well? Can we say "reference access token" (or would it make it more difficult to find what we are talking about?). Maybe "access tokens which are reference tokens"?

Yes we are talking about access tokens, good to make that clearer with in example "reference access tokens"

TobiasAhnoff commented 1 week ago

Note that refresh token rotation to prevent e g token replay attacks is prohibited by FAPI 2.0.

Also good to make this clearer, I will modify suggested verifications to try and make things clearer, probably later today....

TobiasAhnoff commented 6 days ago

Refresh tokens is a challenging topic! I will try to address comments above from @randomstuff and @csfreak92 by suggesting 4 verifications (where the second is a modification of 51.1.4):

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

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)

Verify that refresh tokens have an absolute expiration. Even if sliding refresh token expiration is applied, user re-authentication should be forced at some point in time. Note that how often depends on the application in scope, from e g hours to months. (L1, L2, L3)

Verify that refresh tokens and reference access tokens can be revoked by an authorized user, using the AS user interface, or by a client using AS APIs for revocation. (L1, L2, L3)

@elarlang this might be "related or partly duplicate" to #1968 and #1917 but perhaps it is easiest to first finish refresh token verifications for the OAuth/OIDC chapter here and then address if it is covered by #1968 and #1917 ?

randomstuff commented 6 days ago

Verify that refresh tokens and reference access tokens can be revoked by an authorized user, using the AS user interface, or by a client using AS APIs for revocation. (L1, L2, L3)

I am wondering whether this should be a "and" instead of a "or"?

So I would suggest replacing the "or" by a "and" or (better?) split it into:

Questions: Is this science-fiction? Are major authorization servers providing these features? Are there cases where there is no AS user interface?

I checked how Keycloak works:

elarlang commented 5 days ago

Not an easy task...

But let's stop finetuning the technical testing guide or cheat sheet content here and state - what problem do we address with those requirements? What risk it mitigates? Maybe we can reach to not that technically detailed abstraction.

Also, I'm afraid, that all the refresh_token topic things are changing so fast, that we are outdated already next day after we release it.

csfreak92 commented 3 days ago

But let's stop finetuning the technical testing guide or cheat sheet content here and state - what problem do we address with those requirements? What risk it mitigates? Maybe we can reach to not that technically detailed abstraction.

@elarlang, @randomstuff, I would say @TobiasAhnoff's modifications below here are quite good and addresses the risks that we are trying to mitigate. Don't you think?

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

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)

Verify that refresh tokens have an absolute expiration. Even if sliding refresh token expiration is applied, user re-authentication should be forced at some point in time. Note that how often depends on the application in scope, from e g hours to months. (L1, L2, L3)

Verify that refresh tokens and reference access tokens can be revoked by an authorized user, using the AS user interface, or by a client using AS APIs for revocation. (L1, L2, L3)