-
-
Notifications
You must be signed in to change notification settings - Fork 670
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
3.5.3 update (stateless token signature or mac) #2184
Comments
This looks good. Verify that stateless tokens use a digital signature or MAC to protect against tampering, ensuring it is checked before accepting the token's contents. I personally would like to see 3.5.5 as separate, since it's addressing a different issue that signature verification. It's addressing signature type abuse. However, most of those problems (like changing the signature to NONE to bypass verification) have been fixed in verification libraries. I can see 3.5.5. getting merged if you really want to. |
Should we verify as well that in a given context either MAC or public-key signature are used but not both interchangeably (notably as a way to prevent JWT key confusion type attacks)? I think this is what @jmanico is referring to when talking about "signature type abuse" but if that's the case, the requirement should be more explicit about this (i.e. don't mix MAC and signatures). |
requirement 3.5.3 got updated and merged. Is there need to update 3.5.5 as well? Or we should discuss it in a separate issue? |
Another type of historical signature type abuse is when you forcibly set the JWT signature type to "none" and bypass signature validation. This is less of an issue when you set up an allow list of legal signatures and use modern JWT libraries. |
ping @randomstuff |
Yes, it would probably be good to open an issue about 3.5.3 specifically. |
assuming it was about 3.5.5, opened #2204 |
Reopening to suggest an update (following discussion):
I would also like to propose moving to V3.1 due to divergence from surrounding requirements. |
A suggestion is to update 3.5.3 and move it to a new "Access token" section (where examples are given for JWTs).
|
Current requirement:
Proposed requirement #2184 (comment):
Why do we need to make this requirement specific for user-sessions (if cryptographically secured tokens are used to represent user sessions)? I think it is a general requirement. Proposed requirement #2184 (comment):
I'm also not on the board with the access topic direction and I think this requirement is not limited to the access tokens. Let's fix the requirement text if it requires update as general stateless token requirement. Where it will be located and grouped, we can decide if we have other related requirements in place. At the moment I would say I like the current requirement as it is. |
The requirement can be generalized beyond sessions. Consider 3.1.3:
As long as the terminology is consistent, the requirements for cryptographically secured tokens can be independent. Consider:
|
Thoughts on my current proposal restated below? @TobiasAhnoff @randomstuff
It can be generalized to sessions, access tokens, or anything else. |
@ryarmst, this looks good but the first half is somewhat tautological,
where,
|
Maybe a general glossary thingy - did not find immediately, why do we use the term "Cryptographically Secured Token"? My concern is, is it intuitive and understandable for wide-audience and community from the requirement text that it applies to stateless tokens and JWTs? |
@randomstuff Good point. The wording could be changed to focus not on use but to ensure validation occurs, which may also fit better with V5 (a discussed possible location for token validation type requirements). How about:
@elarlang I understand your point, but issues exist with other candidate terms that have been used:
The glossary does mention JWTs and could include "stateless" as well. I have not seen a better alternative yet and I think it is a good opportunity for the ASVS to set precedent if terminology is lacking. EDIT: this is the term in the Glossary:
|
Discussion over the term "Cryptographically Secured Token" is offtopic from this issue and I'll open a separate issue if there is need for that. My feeling is, that this term is not clear and intuitive and makes requirements only using this term not understandable. Linking the glossary here just proves it in a way. I think there is agreement on the latest proposed requirement text and this can be updated as it is now:
|
Spin-off from #1917
Discussion between @jmanico and @randomstuff started here #1917 (comment)
and ended with proposal:
I think that last sentence should be updated:
Option to discuss - should requirement 3.5.5 be merged into this one:
The text was updated successfully, but these errors were encountered: