OWASP / ASVS

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

cleanup V3.5 Token-based Session Management #1917

Open elarlang opened 5 months ago

elarlang commented 5 months ago

Started to re-investigate issue #1790, then checked the section "V3.5 Token-based Session Management" and reached to the conclusion, that the entire section should be just cleaned up.

At the moment collecting first impressions as feedback. If needed, I split each topic into separate issues.

TLDR

3.5.1

# 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

Is it really a session management issue? Seems more like a business logical permission management - if a user has given permission for the application to access some 3rd party application on his/her behalf, then:

CWE is not helpful here.

Also, if it is OAuth specific, it should be moved there.

In case it is not OAuth specific, it should be considered as access control requirement and move together with 3.5.7.

3.5.3, 3.5.5

# Description L1 L2 L3 CWE NIST §
3.5.3 [MODIFIED, LEVEL L2 > L1] Verify that stateless session tokens make use of a digital signature to protect against tampering and this is checked before processing it further. 345
3.5.5 [ADDED] Verify that only allow-listed signing algorithms are allowed for a stateless token. 757

3.5.3 should not be limited with "stateless session tokens", just "stateless tokens".

Not a session specific requirements.

3.5.4

# Description L1 L2 L3 CWE NIST §
3.5.4 [ADDED] Verify that stateless tokens are checked for expiration before processing them further. 613

So it should cover "time window" when the token is valid.

To check the expiration, we need to require exp to be present and set.

Not sure what security problem it takes down, but technically it should also cover to check nbf (Not Before).

Related RFC https://datatracker.ietf.org/doc/html/rfc7519

Not a session specific requirement.

3.5.6

# Description L1 L2 L3 CWE NIST §
3.5.6 [ADDED] Verify that other, security-sensitive attributes of a stateless token are being verified. For example, in a JWT this may include issuer, subject, and audience. 287

Not a session specific requirement.

3.5.7

# Description L1 L2 L3 CWE NIST §
3.5.7 [ADDED] Verify that all active stateless tokens, which are being relied upon for access control decisions, are revoked when admins change the entitlements or roles of the user. 613

The context here - first-party party tokens. It should be clarified in the requirement if we'll keep it.

Another problem - to have the requirement in the session category, it assumes, that stateless tokens are used as a replacement for stateful session.

As I wrote here: https://github.com/OWASP/ASVS/issues/1790#issuecomment-1823972356

From NIST SP 800-63B version 4 (not released) 7.1.2 Access Tokens:

An access token — such as found in OAuth — is used to allow an application to access a set of services on a subscriber’s behalf following an authentication event. The presence of an OAuth access token SHALL NOT be interpreted by the RP as presence of the subscriber, in the absence of other signals. The OAuth access token, and any associated refresh tokens, MAY be valid long after the authentication session has ended and the subscriber has left the application.

If we remove the session context from the requirement, it is still a valid requirement - but should be moved to the "V4 Access Control" category.

tghosth commented 5 months ago

3.5.1 - Agree it should be part of the OAuth Chapter 3.5.7 - Agree it can be moved to access control.

The rest, I would support some rewording of the requirements and the section name but I am not convinced about moving them to an entirely different chapter as people will mostly (admittedly not always) come across these tokens in a session management context.

elarlang commented 5 months ago

Let's try to get the first 2 sorted...

3.5.1

Thinking more about 3.5.1 - I think it is not the best fit for OAuth, although suits there better than in the session category, but maybe we should look at it as general access management logic, not as an OAuth protocol-specific requirement.

3.5.7

We don't have a matching requirement for the same situation in the "classical session management" - if the permissions for the user are changed, it must have immediate effect. Such as in case those are buffered to session data, those also must be "revoked".

So maybe we need to make this requirement to cover both situations and to be more abstract. Or do we need to have another requirement for "classical session management".

both

Till we don't have anything better, I propose moving them to the current V4.3 Other Access Control Considerations, or, if anyone has a good proposal, to a new section with the meaning "Access Control Management". Need to re-check when we going to re-work V4.

ping @tghosth

tghosth commented 5 months ago

Thinking more about 3.5.1 - I think it is not the best fit for OAuth, although suits there better than in the session category, but maybe we should look at it as general access management logic, not as an OAuth protocol-specific requirement.

I agree with the concept where access tokens have been issued and I agree with moving it to access control but you would need to reword it to make it a little clearer as to why.

So maybe we need to make this requirement to cover both situations and to be more abstract. Or do we need to have another requirement for "classical session management".

So the reason this exists is that in a stateless session, the session token might be relied upon for the permissions the user has whereas in stateful it will always be checked against the database. I agree that it could be combined into one requirement and made more abstract

randomstuff commented 3 months ago

Verify that stateless session tokens make use of a digital signature to protect against tampering and this is checked before processing it further.

To be strict this should be "digital signature or MAC": "Verify that stateless session tokens are protected against tampering (eg. using a digital signature or a MAC) and this protection is checked before processing it further."

randomstuff commented 3 months ago

Verify that all active stateless tokens, which are being relied upon for access control decisions, are revoked when admins change the entitlements or roles of the user.

We can't revoke the token if it is really stateless, though.

jmanico commented 3 months ago

This is not true. There are several revocation strategies first discussed here. https://datatracker.ietf.org/doc/html/rfc7009 And certain tokens like refresh token require revocation after users cancel that relationship and more.

A common strategy is to maintain a unique token ID or use redis/memcache to maintain temporary revoke lists.

And although this may mean by a strict definition the architecture is not completely stateless, these revocation strategies are still mostly stateless and not not require a full-on server-side sessions.

We can't revoke the token if it is really stateless, though.

randomstuff commented 3 months ago

A common strategy is to maintain a unique token ID or use redis/memcache to maintain temporary revoke lists.

Yes, my point was that it is not really stateless anymore. I believe some people want really-stateless stateless tokens and use short-lived access-tokens and that this should not be forbidden (?).

What about something such as: « Verify that changes in the entitlements or roles of the user are reflected in all active stateless tokens after at most {DURATION}. ».

If you have a short-lived stateless token which a duration of 1 minutes, it might be OK if role/entitlement change is not reflected on the RP before the next token refresh. Anyway, in many cases, you will have some delay because of caching and such. For example, if you use Status List.

jmanico commented 3 months ago

What is the use case for very short-lived tokens that last just a few minutes? I'm sincerely asking because I have not encountered that before.

randomstuff commented 3 months ago

I would say the use case for access tokens which last a few minutes is to have really stateless access tokens while providing short grant revocation propagation.

RFC6819 says:

tokens may expire after a few minutes (e.g., for payment transactions) or stay valid for hours (e.g., read access to contacts).

Some examples in the wild:

tghosth commented 3 months ago

I think this issue is waiting for the V3 rework which I don't think has been assigned yet but the points will certainly be considered when it happens

jmanico commented 3 months ago

Before we move 3.5.7 I want to make a comment.

3.5.7 [ADDED] Verify that all active stateless tokens, which are being relied upon for access control decisions, are revoked when admins change the entitlements or roles of the user. 613

Revoking the token when the admin entitlement is not the best strategy. Just like using REDIS for revocation, I can set a REDIS rule to over-ride the tokens access control with the new entitlement and not force a user to log in again, if this is used for session management. Token revocation here is a fairly weak and user-unfriendly method. So if we keep 3.5.7 I'd suggest:

3.5.7 [ADDED] Verify that all active stateless tokens, which are being relied upon for access control decisions, are revoked or overridden with the new access control entitlement when admins change the entitlements or roles of the user. 613
randomstuff commented 3 months ago

And although this may mean by a strict definition the architecture is not completely stateless, these revocation strategies are still mostly stateless and not not require a full-on server-side sessions.

I think that "stateless" token should be stateless by definition :wink:. If you don't intend to mean that the stateless token is stateless the name should be changed for something more precise.

jmanico commented 3 months ago

In real-world scenarios, systems frequently mix stateless and stateful design elements to find a good balance between security, performance, and scalability.

One of the common strategies is cache-based revocation so the performance impact is less:

  1. Better Security: Revocation helps reduce the risk of compromised tokens being used until they expire.
  2. Trade-offs: This approach adds some state management, which can impact scalability and performance, especially in systems that are spread out over many servers.
  3. Implementation: Managing the cache efficiently, such as using distributed caches like Redis or Memcach (very common revocation strategies), can help reduce some of the performance issues.
jmanico commented 3 months ago

I suggest a change to 3.5.3 from:

3.5.3 [MODIFIED, LEVEL L2 > L1] Verify that stateless session tokens make use of a digital signature to protect against tampering and this is checked before processing it further. 345

to

3.5.3 [MODIFIED, LEVEL L2 > L1] Verify that stateless tokens use a digital signature or HMAC to protect against tampering, ensuring it is checked before processing further.. 345

This accounts for the comments above as well.

randomstuff commented 3 months ago

"HMAC" should be replaced by "MAC": any other (non-broken) MAC could be used.

(Would it be useful, to mention authenticated encryption algorithms here?)

randomstuff commented 3 months ago

"before processing further" is somewhat vague/ambiguous.

jmanico commented 3 months ago

So how about: How about this?

3.5.3 [MODIFIED, LEVEL L2 > L1] Verify that stateless tokens use a digital signature or MAC to protect against tampering, ensuring it is checked before performing any other actions based on the token's contents. 345

It's a LOT more specific, but too much so?