ASVS icon indicating copy to clipboard operation
ASVS copied to clipboard

review V51.4.2

Open elarlang opened this issue 1 year ago • 3 comments

From the initial OAuth paragraph draft we have requirements:

# Description L1 L2 L3
51.4.2 [ADDED] Verify that access tokens are restricted to certain resource servers (audience restriction), preferably to a single resource server. Every resource server is obliged to verify, for every request, whether the access token sent with that request was meant to be used for that particular resource server. If not, the resource server must refuse to serve the respective request.

Additionally to some formating improvements, we need to (re)validate the content, the need, the problem to solve and sections.

elarlang avatar Oct 22 '24 15:10 elarlang

We have also requirement 3.5.6:

# Description L1 L2 L3 CWE
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

Maybe we should cover it with spliting 3.5.6 to more precise requirements, as recommended in https://github.com/OWASP/ASVS/issues/1967#issuecomment-2351456688.

At the same time I don't want the message to be hidden, that aud in an access token points to resource server and aud in ID token points to client_id.

elarlang avatar Oct 22 '24 15:10 elarlang

preferably to a single resource server.

FWIW, this condition is less important (sometimes) when using sender-constrained tokens.

randomstuff avatar Oct 23 '24 17:10 randomstuff

For direction:

Verify that the resource server validates the access token to be made for that resource server (audience) by checking the 'aud' claim from the access token to be an expected value.

Need to cover:

  • reference token and introspection
  • fix the last part - the aud need to contain expected value referencing to resource server

elarlang avatar Oct 23 '24 18:10 elarlang

Updated:

Verify that the resource server validates the access token to be made for that resource server (audience), for example by checking the 'aud' claim from the access token to be an expected value.

ping @randomstuff

elarlang avatar Nov 06 '24 17:11 elarlang

Proposition of minor rewording:

Verify that the resource server validates that the access token is intended to be usable on that resource server (audience), for example by checking the 'aud' claim from the access token to be an expected value.

randomstuff avatar Nov 06 '24 21:11 randomstuff

Or:

Verify that the resource server rejects the request if the access token was not intended to be usable on that resource server (audience), for example by checking the 'aud' claim from the access token to be an expected value.

randomstuff avatar Nov 06 '24 21:11 randomstuff

I vote for the more positive wording

Verify that the resource server validates that the access token is intended to be usable on that resource server (audience), for example by checking the 'aud' claim from the access token to be an expected value.

TobiasAhnoff avatar Nov 06 '24 22:11 TobiasAhnoff

OK for the positive one. Is it clear enough?

randomstuff avatar Nov 07 '24 10:11 randomstuff

"to be usable" > "to be used"?

OK for the positive one. Is it clear enough?

Do you want to highlight the problem to solve"? Like "To be sure that access tokens from the same issuer for different resources (audience) can not be cross-used."?

elarlang avatar Nov 08 '24 09:11 elarlang

I don't know :) it's probably OK like that.

randomstuff avatar Nov 09 '24 21:11 randomstuff

Re-open, via #2363 we will have general requirement for audience.

I would like to keep OAuth specific requirement, as this is a quite widespread mistake. Do not have a clear duplicate, I think we need to improve the OAuth version - at the moment it does not cover the introspection scenario.

elarlang avatar Nov 16 '24 08:11 elarlang

Current requirement:

# Description L1 L2 L3
51.4.2 [ADDED] Verify that the resource server validates that the access token is intended to be used on that resource server (audience), for example by checking the 'aud' claim from the access token to be an expected value.

Does anyone want to wordsmith the introspection and reference token into it?

elarlang avatar Nov 16 '24 18:11 elarlang

Proposition including token introspection:

Verify that the resource server validates that the access token is intended to be used on that resource server (audience). When the access token is a JWT, this should be can done by checking the 'aud' claim from the access token to be an expected value. When using token introspection, the 'aud' property should be used the same way.

randomstuff avatar Nov 16 '24 19:11 randomstuff

Section "V51.4 OAuth Resource Server", update requirement V51.4.2, for resource server and access token, attempt to shorten it a bit:

Verify that the resource server validates that the access token is intended to be used on that resource server (audience). For access token in JWT format or using JWT from token introspection can be done by checking the 'aud' claim.

Section "V51.5 OIDC Client" - new requirement proposal for ID token and OIDC client (requires wordsmithing):

Verify that the client validates that the ID token is intended to be used for that client (audience) by checking that the 'aud' claim from the token is equal to the 'client_id' value for the client.

elarlang avatar Nov 17 '24 07:11 elarlang

Goes in via #2383:

# Description L1 L2 L3
51.4.2 [ADDED] Verify that the resource server validates that the access token is intended to be used on that resource server (audience). For access token in JWT format or using JWT from token introspection can be done by checking the 'aud' claim.
51.5.4 [ADDED] Verify that the client validates that the ID token is intended to be used for that client (audience) by checking that the 'aud' claim from the token is equal to the 'client_id' value for the client.

elarlang avatar Nov 18 '24 12:11 elarlang

Nitpicky changes:

# Description L1 L2 L3
51.4.2 [ADDED] Verify that the resource server validates that the access token is intended to be used on that resource server (audience). For access token in JWT format or using JWT from token introspection, it can be done by checking the 'aud' claim.

(added ", it")

Moreover I would not say "JWT from token introspection" because this is not stricly speaking a JWT:

# Description L1 L2 L3
51.4.2 [ADDED] Verify that the resource server validates that the access token is intended to be used on that resource server (audience). For access token in JWT format or using OAuth, it can be done by checking the 'aud' claim.

randomstuff avatar Nov 18 '24 20:11 randomstuff

Note that the context for this requirement is OAuth resource server

Proposed update for V51.4.2:

Verify that the resource server ensures that the access token is intended to be used with that server (audience). Validation can be done by checking the 'aud' claim from the access token or from the token introspection response.

elarlang avatar Nov 19 '24 05:11 elarlang

Both 51.4.2 (option 2) and 51.5.4 are good, perhaps add introspection to 51.4.2 to make it more clear

Verify that the resource server validates that the access token is intended to be used on that resource server (audience). For access token in JWT format or using OAuth introspection, it can be done by checking the 'aud' claim.

But this (except the note on OAuth introspection) is also covered by https://github.com/OWASP/ASVS/issues/2363#issuecomment-2482783246, maybe suggested 51.4.2 can be "merged" to that?

TobiasAhnoff avatar Nov 19 '24 06:11 TobiasAhnoff

except the note on OAuth introspection

This is the main reason why we have a separate requirement for OAuth, as it is a special case. The same applies for 'aud' validation from ID token.

elarlang avatar Nov 19 '24 07:11 elarlang

I would like to suggest the following tweaked wording for 3.5.8 and 51.4.2:

# Description L1 L2 L3 CWE
3.5.8 [ADDED] Verify that the service only accepts tokens which are intended for use with that service. For JWTs, this can be achieved by validating the 'aud' claim against an allowlist defined in the service.
51.4.2 [ADDED] Verify that the resource server only accepts access tokens which are intended for use with that server (audience). The audience may be included in a cryptographically secured token or it can be checked using the token introspection endpoint.

tghosth avatar Nov 19 '24 16:11 tghosth

We have a separate issue for general requirement (proposed 3.5.8): #2363. To keep everything followable, I recommend to add this proposal there taking into account abstract feedback from here.

With this post it would be nice to have some diff of changes and reasons behind that.

The audience may be included in a cryptographically secured token or it can be checked using the token introspection endpoint.

Technically correct is: the audience IS included in access token JWT and MAY be included in introspection response.

"Validation can be done" is not good, but it carries the meaning: "most likely you can use the 'aud' claim, but if something else is used to achieve that, it is also ok".

I don't want to mess with "cryptographically secured token" terminology here - all OAuth / OIDC specs are talking about JWT and "cryptographically secured token" is just a alien here. There is no abstraction needed in that field as it is fixed to use JWT. It is more precise and correct.

In general: both requirements have gone through long discussions to have every word in place. If something different is proposed, I expect it take into account everything that is said before starting the entire discussion from the start.

elarlang avatar Nov 19 '24 16:11 elarlang

I don't want to mess with "cryptographically secured token" terminology here - all OAuth / OIDC specs are talking about JWT and "cryptographically secured token" is just a alien here. There is no abstraction needed in that field as it is fixed to use JWT. It is more precise and correct.

OIDC is fixed to using JWT for ID token and related tokens. For OAuth, JWT is required in many places (DPoP, etc.). For other cases (OAuth access tokens), some people could find it interesting (or have other reasons) to use other token formats such as:

  • SAML ← yes this is still used
  • PASETO ← self-proclaimed better-than-JWT
  • CWT ← often used if the token is transferred as QR code
  • itsdangerous
  • Biscuit ← have funny properties

Many of the requirements about tokens (in Oauth or otherwise) are applicable to them to some extent.

randomstuff avatar Nov 19 '24 21:11 randomstuff

I added a comment here for 3.5.8: https://github.com/OWASP/ASVS/issues/2363#issuecomment-2487625616

My proposal for 51.4.2:

# Description L1 L2 L3 CWE
51.4.2 [ADDED] Verify that the resource server only accepts access tokens which are intended for use with that server (audience). The audience may be included in a cryptographically secured token or it can be checked using the token introspection endpoint.

As @randomstuff says, I don't think OAuth access tokens have to be JWTs which is why I used the "cryptographically secured token" terminology. Similarly, the "aud" claim is not limited to OAuth and might appear differently in different tokens which is why I don't think it should be specifically mentioned here. As further proof, the OAuth token introspection endpoint exists specifically for a situation when the access token is not a cryptographically secured token or a JWT.

tghosth avatar Nov 20 '24 06:11 tghosth

For a starter, some questions to @randomstuff

some people could find it interesting (or have other reasons) to use other token formats such as:

Is it supported by spec or is it something that "I can do it"?

Is is widespread enough to rewrite entire chapter to be abstract and with a danger to not be that clear and understandable?

Is it something "you can technically do if you want" or something that "I want to rewrite OAuth and OIDC chapter"?

If it is supported (not limited) by the OAuth spec, is it enough to just mention it with one line in chapter text?

If someone is using SAML, ist it called OAuth or is it called SAML that by theory requires it's own specific requirements?

elarlang avatar Nov 20 '24 06:11 elarlang

Answering myself:

  • OAuth does not dictate the format for the access token
  • The choice is between "opaque token format" (requires introspection endpoint call) and "structured token format" (can read information directly from the token)
    • https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-12#section-1.4

The usage of "cryptographically secured" is valid only

  • for the context to provide defense against tampering
  • for requirements dealing with that

.. and it should not carried anywhere else, where it is out of context, potentially incorrect, and potentially limiting the requirement. For example here, if the token is "cryptographically secured" does not mean it has structured information (to provide the aud information).

Proposal update:

Verify that the resource server only accepts access tokens that are intended for use with that service (audience). The audience may be included in a structured access token (such as the 'aud' claim in JWT) or it can be checked using the token introspection endpoint.

Changes:

  • "which" > "that" - grammarly
  • "server (audience)" > "service (audience)" - align with #2363
  • "The audience may be included in a cryptographically secured token" > "The audience may be included in a structured access token (such as the 'aud' claim in JWT)"

elarlang avatar Nov 20 '24 10:11 elarlang

So it bothers me to allow relying on something in the token without being specific that it would need to be signed in order to rely upon it. Whilst I agree that "if the token is "cryptographically secured" does not mean it has structured information", the opposite cannot be the case as we cannot rely on a structured access token which is not cryptographically secured.

So maybe:

Verify that the resource server only accepts access tokens that are intended for use with that service (audience). The audience may be included in a cryptographically secured, structured access token (such as the 'aud' claim in JWT) or it can be checked using the token introspection endpoint.

tghosth avatar Nov 20 '24 11:11 tghosth

Do you want to duplicate the crypto part to every requirement that has an integrity check as a pre-condition or requirements in current V3.5 related to crypto and is integrity check enough for that? (The question is rhetorical)

More from https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-12#section-1.4

The authorization server MUST ensure that access tokens cannot be generated, modified, or guessed to produce valid access tokens by unauthorized parties.

Crypto is one solution for achieving that. It is covered somewhere else in the ASVS. Additionally, OAuth itself does not say how it must be achieved. My point with this (and all the other requirements in OAuth and OIDC chapter) is - in case "cryptographically secured token" is used, it is validated and the integrity check for the token is done before reaching to other requirements. We don't need to duplicate it here.

To just explain the idea of duplication, why to limit with "cryptographically secured" tokens?

Verify that the resource server only accepts access tokens that are intended for use with that service (audience). The audience may be included in a cryptographically secured (3.5.3, 3.5.5, 3.5.7), in valid duration (3.5.4), validated type (3.5.9) structured access token (such as the 'aud' claim in JWT) or it can be checked using the token introspection endpoint.

elarlang avatar Nov 20 '24 11:11 elarlang

Ok so I am ok with this then:

Verify that the resource server only accepts access tokens that are intended for use with that service (audience). The audience may be included in a structured access token (such as the 'aud' claim in JWT) or it can be checked using the token introspection endpoint.

tghosth avatar Nov 20 '24 12:11 tghosth

@randomstuff - the requirement got updated and merged as:

# Description L1 L2 L3
51.4.2 [ADDED] Verify that the resource server only accepts access tokens that are intended for use with that service (audience). The audience may be included in a structured access token (such as the 'aud' claim in JWT) or it can be checked using the token introspection endpoint.

I'll wait for your feedback or confirmation, does it need further improvements or can we close this?

elarlang avatar Nov 20 '24 12:11 elarlang

OK for me.

randomstuff avatar Nov 20 '24 18:11 randomstuff