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

proposal/discussion: OAuth: requirement for refresh_token lifetime #1968

Closed
elarlang opened this issue May 21, 2024 · 22 comments
Closed

proposal/discussion: OAuth: requirement for refresh_token lifetime #1968

elarlang opened this issue May 21, 2024 · 22 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

spin-off from #1925 "proposal 7", from @TobiasAhnoff

7 Verify that refresh-tokens expires according to threat model and business requirements

Proppoal from Elar:

The refresh_token topic requires more attention. But there is more than one point of view.

It is different attitude, is the OAuth used as first-party or 3rd party solution. For the first-party and "session management replacement" (which should be disallowed or not recommended) I would say we can apply V3.3 Session Timeout and V3.8 Session Termination requirements.

# Description L1 L2 L3 CWE NIST §
3.3.2 [MODIFIED, SPLIT TO 3.3.5] Verify that there is an absolute maximum session lifetime such that re-authentication is required at least every 30 days for L1 applications or every 12 hours for L2 and L3 applications. 613 7.2

For 3rd party solution, we have a requirement currently located in 3.5.1 (it is subject to change via #1917 (comment))

# Description L1 L2 L3 CWE NIST §
3.5.1 [GRAMMAR] Verify that the application allows users to revoke OAuth tokens that form trust relationships with linked applications. 290 7.1.2

I think we need to be really precise, about what architecture and solution we address with the requirement.

One extra issue to cover with the refresh_token topic expiration is that with new refresh_token the AS must keep the exp value like it was before (and not extend it).

@elarlang elarlang added the V51 Group issues related to OAuth label May 21, 2024
@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 May 23, 2024
@elarlang
Copy link
Collaborator Author

I move discussion from #2040 (comment) to this issue. The latest proposal:

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)

@elarlang
Copy link
Collaborator Author

Proposed update:

Verify that refresh tokens have an absolute expiration. Even if sliding refresh token expiration is applied, user re-authentication must be forced after a defined absolute lifetime.

@randomstuff
Copy link

We are in OAuth, so we are talking about authorization but …

Even if sliding refresh token expiration is applied, user re-authentication should be forced at some point in time.

Do we need to talk about re-authentication here or about re-authorization? Would it be considered bad practice to re-authorize without going through all the OAuth flow dance?

Would it be OK if the authorization server was sending you a notification saying "authorization XXX which was granted for application YYYY is about to expire" and then you could expand its lifetime from the authorization server instead? One downside of this approach is that there might be a higher chance that a rogue application/instance would impersonate a legitimate one.

@TobiasAhnoff
Copy link

TobiasAhnoff commented Sep 25, 2024

Verify that refresh tokens have an absolute expiration. Even if sliding refresh token expiration is applied, user re-authentication must be forced after a defined absolute lifetime.

I believe this is correct, at some point the user should re-authenticate doing the full "OAuth dance" to prove that the user is in control of the users credentials (in example MFA), not allowing "forever sliding" refresh tokens (or sessions).

But as @randomstuff noted, the way this was written it is unclear (or if it also forbids) the use of re-authorization, which was not intended...perhaps this will make it clearer?

Verify that refresh tokens have an absolute expiration. Even if sliding refresh token expiration is applied, user re-authentication must be forced after a defined absolute lifetime. Note that before the absolute expiration point in time, sliding expiration might be applied allowing the user to re-authorize presenting a valid refresh token. (L1,L2,L3)

@elarlang
Copy link
Collaborator Author

Note that before the absolute expiration point in time, sliding expiration might be applied allowing ...

Even in this case sliding can not go over refresh token expiration time.

Verify that refresh tokens have an absolute expiration. Even if sliding refresh token expiration is applied, user re-authentication must be forced after a defined absolute lifetime. Before the absolute expiration point, the application should allow the user to re-authorize by presenting a valid refresh token.

At the same time I'm not convinced we should cover to extension in the same requirement, Maybe the focus should be only on the expiration time.

@jmanico
Copy link
Member

jmanico commented Sep 25, 2024 via email

@randomstuff
Copy link

But as @randomstuff noted, the way this was written it is unclear (or if it also forbids) the use of re-authorization, which was not intended...perhaps this will make it clearer?

@TobiasAhnoff, This is actually not what I meant, sorry :)

@randomstuff
Copy link

I do not think refresh-refresh or expiration is enough of a control for refresh tokens in L 2/3 apps.

FAPI mandates sender-constrained tokens using Mutual TLS (MTLS), Proof of Possession (PoP) or similar

@jmanico, I don't think anyone was saying that refresh token expiration was in any way an alternative to sender-constrained tokens. The idea here is that is that all grants should eventually expire after a given duration even if it tokens are being refreshed periodically.

The proposal from #2040 (comment) was “user re-authentication should be forced at some point in time» i.e. OAuth 2.0 code flow should be done again after this duration.

My argument was that the authorization server might accept other methods for extending the duration of the grant through user interaction involving verification of the user consent.

@TobiasAhnoff
Copy link

This is actually not what I meant, sorry :)

my apologies :)

My argument was that the authorization server might accept other methods for extending the duration of the grant through user interaction involving verification of the user consent.

@randomstuff maybe a dumb question, but could you give an example on verification methods (if not using the same method as the initial user authentication)?

@randomstuff
Copy link

randomstuff commented Sep 27, 2024

Even if sliding refresh token expiration is applied, user re-authentication should be forced at some point in time.

@TobiasAhnoff, actually I understood “user re-authentication” as “the user must use the client application to restart the OAuth flow in order to obtain new refresh token” which is actually not exactly what is being said in the text. I retract the comments on this topic which were useless :)

(My point was, it might (maybe) be OK for the user to extend the lifetime of the existing grant by going directly from the authorization server without going through the client application.)

@TobiasAhnoff
Copy link

@randomstuff not useless at all, now I understand! And yes, I was actually thinking of "using the same client", but as you noted the way it is written it allows for other scenarios as well (like "going directly from the authorization server").

@elarlang
Copy link
Collaborator Author

I'm a bit lost here in comments, what is the latest proposal to discuss?

@TobiasAhnoff
Copy link

I think the last comments are on

Verify that refresh tokens have an absolute expiration. Even if sliding refresh token expiration is applied, user re-authentication must be forced after a defined absolute lifetime. Before the absolute expiration point, the application should allow the user to re-authorize by presenting a valid refresh token. (L1,L2,L3)

But, as you noted, maybe it is good to just focus on expiration time? Then it could be like this

Verify that refresh tokens have an absolute expiration. Even if sliding refresh token expiration is applied, where the user to re-authorize by presenting a valid refresh token, user re-authentication must be forced after a defined absolute lifetime. (L1,L2,L3)

or

Verify that refresh tokens have an absolute expiration, where user re-authentication must be forced after a defined absolute lifetime. (L1,L2,L3)

@elarlang
Copy link
Collaborator Author

elarlang commented Oct 3, 2024

I think it makes sense just to deliver the point of absolute expiration.

ping @randomstuff - any comments?

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

Last version from @TobiasAhnoff looks good to me. I guess it should be enough (?).

Could this be formulated as?

Verify that refresh tokens have an maximum absolute expiration, after which user re-authentication must be forced. (L1,L2,L3).

What worries me somewhat with this formulation is that it's not the refresh token which has a maximum expiration time but the grant. When you do the token refresh token request you (may) obtain a new refresh token but the expiration delay is not restarted i.e. the expiration deadline is shared for all associated refresh tokens.

Would something like this be OK?

Verify that each grant has an maximum absolute expiration, after which all associated tokens must be invalid and user re-authentication must be forced. Token refresh requests must not expand the duration of the expiration deadline (L1,L2,L3).

@elarlang
Copy link
Collaborator Author

elarlang commented Oct 4, 2024

@randomstuff - for example Keycloak extending (sliding?) the refresh_token exp but still has maximum lifespan set. I would like to have wording to cover / not restricting that.

I agree that terminology "refresh token" vs " grant" requires some work.

@TobiasAhnoff
Copy link

I think the suggested requirement from @randomstuff is good and address that sliding must not pass the expiration deadline, or is something missing @elarlang ?

Using the right terminology is a bit challenging...this is the best guidance I know of https://aaronparecki.com/2024/03/29/3/oauth-terminology. Aaron notes that

"Grant" also refers to the abstract concept of the user having granted authorization, which is expressed as the authorization code, or implicitly with the client credentials grant. This is a bit of an academic definition of the term, and is used much less frequently in normal conversation around OAuth.

Perhaps add "authorized" to make it a little more clear to ASVS readers what "grant" means?

Verify that each authorized grant has an maximum absolute expiration, after which all associated tokens must be invalid and user re-authentication must be forced. Token refresh requests must not expand the duration of the expiration deadline (L1,L2,L3).

One more note, since "grant" is more general than "refresh-token lifetime", and applies to both code and client-credentials, maybe remove "user" or add "and client"?

@randomstuff
Copy link

randomstuff commented Oct 7, 2024

@TobiasAhnoff, "authorized grant" is probably more explicit although "authorized grant" seems somewhat tautological to me 😆

@elarlang
Copy link
Collaborator Author

elarlang commented Oct 8, 2024

I try to keep it as simple as possible with only one clear focus - if a new refresh token is provided, it does not extend the max lifetime, including when sliding exp is used. Whatever business logical decision needs to be made before the expiration happens, is not in the scope of this requirement.

Verify that refresh tokens have an absolute expiration, including if sliding refresh token expiration is applied.

The more interesting part is, should we cover somehow the 'offline_access' in this requirement?

@randomstuff
Copy link

The more interesting part is, should we cover somehow the 'offline_access' in this requirement?

I don't think this is really related, is it? This should probably go into the OpenID Connect chapter, instead.

@elarlang
Copy link
Collaborator Author

I don't think this is really related, is it? This should probably go into the OpenID Connect chapter, instead.

Yes, but this is an option to remove expires time for refresh token JWT.

elarlang pushed a commit to elarlang/ASVS that referenced this issue Oct 12, 2024
@elarlang
Copy link
Collaborator Author

Goes in via #2140

# Description L1 L2 L3
51.2.13 [ADDED] Verify that refresh tokens have an absolute expiration, including if sliding refresh token expiration is applied.

If it requires update, we can do that. It's more to get the requirement in and having a number for it.

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

6 participants