-
-
Notifications
You must be signed in to change notification settings - Fork 671
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
review V51.4.2 - OAuth and OIDC audience restrictions for tokens #2182
Comments
We have also requirement 3.5.6:
Maybe we should cover it with spliting 3.5.6 to more precise requirements, as recommended in #1967 (comment). At the same time I don't want the message to be hidden, that |
FWIW, this condition is less important (sometimes) when using sender-constrained tokens. |
For direction:
Need to cover:
|
Updated:
ping @randomstuff |
Proposition of minor rewording:
|
Or:
|
I vote for the more positive wording
|
OK for the positive one. Is it clear enough? |
"to be usable" > "to be used"?
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."? |
I don't know :) it's probably OK like that. |
Nitpicky changes:
(added ", it") Moreover I would not say "JWT from token introspection" because this is not stricly speaking a JWT:
|
Note that the context for this requirement is OAuth resource server Proposed update for V51.4.2:
|
Both 51.4.2 (option 2) and 51.5.4 are good, perhaps add introspection to 51.4.2 to make it more clear
But this (except the note on OAuth introspection) is also covered by #2363 (comment), maybe suggested 51.4.2 can be "merged" to that? |
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. |
I would like to suggest the following tweaked wording for 3.5.8 and 51.4.2:
|
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.
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. |
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:
Many of the requirements about tokens (in Oauth or otherwise) are applicable to them to some extent. |
I added a comment here for 3.5.8: #2363 (comment) My proposal for 51.4.2:
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. |
For a starter, some questions to @randomstuff
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? |
Answering myself:
The usage of "cryptographically secured" is valid only
.. 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:
Changes:
|
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:
|
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
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?
|
Ok so I am ok with this then:
|
@randomstuff - the requirement got updated and merged as:
I'll wait for your feedback or confirmation, does it need further improvements or can we close this? |
OK for me. |
From the initial OAuth paragraph draft we have requirements:
Additionally to some formating improvements, we need to (re)validate the content, the need, the problem to solve and sections.
The text was updated successfully, but these errors were encountered: