Closed jmanico closed 2 months ago
8.2.2 Verify that data stored in browser storage (such as localStorage, sessionStorage, IndexedDB, or cookies) does not contain sensitive data, with the exception of session tokens which should be stored in either cookies or sessionStorage.
Related:
In one application I use, the session token is indeed not shared across tabs. When you open a new tab with the application, you are redirected to the SSO provider and automatically logged in into the application. So while the application's token is stored in sessionStorage, it still works with multi-tab browsing because of the interaction with the SSO provider.
I suggest we allow localStorage for sessions, but only if the token has both idle and absolute timeout so long terms token storage is less risky.
So does this restriction automatically rule out storing a JWT in local storage? @jmanico
Not at all. JWT have expiration times. If it’s low it’s effectively both idle and absolute timeout.
In one application I use, the session token is indeed not shared across tabs. When you open a new tab with the application, you are redirected to the SSO provider and automatically logged in into the application. So while the application's token is stored in sessionStorage, it still works with multi-tab browsing because of the interaction with the SSO provider.
This is very compelling info, thank you for this.
Hi @Tib3rius, I believe you originated this request. As @Sjord pointed out above, there has already been a lot of discussion on this. Nevertheless, I take your point about practicality.
If we talk about using "Local Storage but with tokens that have short expiration times".
1) What do you consider to be a short expiration time 2) Do you have a suggestion on how do we handle the refresh token pattern (i.e. where would a refresh token be stored) or is the expectation that users will reauthenticate after the short expiration time?
In one application I use, the session token is indeed not shared across tabs. When you open a new tab with the application, you are redirected to the SSO provider and automatically logged in into the application. So while the application's token is stored in sessionStorage, it still works with multi-tab browsing because of the interaction with the SSO provider.
This is vulnerability itself and I addressed it via https://github.com/OWASP/ASVS/issues/1815
# | Description | L1 | L2 | L3 | CWE | NIST § |
---|---|---|---|---|---|---|
3.2.5 | [ADDED] Verify that creating a session for the application requires the user's consent and that the application is protected against a CSRF-style attack where a new application session for the user is created via SSO without user interaction. | ✓ | ✓ |
The advice to use sessionStorage is really non-standard. It breaks multi-tab browser use.
Why we should mandate multi-tab browser use? sessionStorage is made for a reason - to be more secure than localStorage. For example, can you use reflected XSS or clickjacking against authenticated users or not - do you need to access the correct tab or it work in any opened tab?
I really don't like the idea of saying that they are equal or it does not matter which one of them you use.
I just wanted to comment here to let you know I've been at DEF CON + some vacation time after but I'll be back tomorrow so will get involved in the discussion then. I think there are some good points that have been brought up already. I'll share my full thoughts tomorrow.
OK only just getting around to responding here, I mostly blame covid, but apologies.
@tghosth
If we talk about using https://github.com/OWASP/CheatSheetSeries/issues/1458#issue-2436606952. What do you consider to be a short expiration time
I would say it depends on the nature of the app, but an hour would be OK for most. Banking apps etc. could be 15-20 minutes.
Do you have a suggestion on how do we handle the refresh token pattern (i.e. where would a refresh token be stored) or is the expectation that users will reauthenticate after the short expiration time?
I don't really have a suggestion for this tbh. I assume right now the recommendation is to store it in session storage? If that is the case, I guess technically we could still keep it there, the idea being that even though the refresh token is in session storage, if a user does open the app in a new tab, they should be authenticated via the local storage token, which could then be used to fetch a refresh token? That may cause other issues however, especially if we only expect a refresh token to be generated at login, etc.
@elarlang
Why we should mandate multi-tab browser use?
This is less about "mandating" multi-tab browser use, and more about providing realistic security recommendations that apps can use. That is to say, I have no issue with keeping the current recommendations providing we make it clear that they only work for single-tab apps. The larger point is that we should also provide security recommendations for apps which people expect to work in multiple tabs. It is fine, IMO, to state categorically that those security recommendations are less secure / less ideal, but I imagine opening links in new tabs is a feature expected by quite a lot of users, and may also be part of a developer's design for the app.
I really don't like the idea of saying that they are equal or it does not matter which one of them you use.
I agree, and I don't think I was trying to imply that. Clearly session storage is more secure because it's only accessible by a single tab, rather than all tabs with the same origin. However, that increase in security comes with a usability tradeoff that affects session management and makes the current recommendations practically impossible to implement in most apps due to user / dev expectations.
The comments from @Tib3rius are astute and well thought-out and makes me re-think our recommendation to use sessionStorage.
Our options here are to say:
1) It is ok to store session tokens in localStorage - to support multi-tab functionality and to lose security mechanisms provided by sessionStorage
OR
2) Use sessionStorage over localStorage - you don't have multi-tab support but you have an extra protection (tab scope + tab lifetime) provided by sessionStorage
I think this is not a general decision we can make for ASVS to make everyone happy and this is something to be decided by the application owner based on threat-modeling or risk-analysis (vs functionality expectations).
this is something to be decided by the application owner
I think the ASVS should not make a recommendation, but just state what it allows. Currently it says:
with the exception of session tokens which should be stored in either cookies or sessionStorage.
But we could make that less of a recommendation with:
with the exception of session tokens which may be stored in either cookies or sessionStorage.
OK, I spent ages going back through old discussion and guidance on this.
In summary, I think we need to stop being prescriptive about this particular point because there is no one true answer and any secure system requires a combination of controls.
Instead, we should aim for a requirement which details the goals that need to be achieved rather than implementation detail.
I would therefore suggesting bringing back the original requirement 3.2.3 in a less prescriptive way. We would also need to modify 8.2.2 to avoid any contradictions.
My DRAFT suggestion would be:
# | Description | L1 | L2 | L3 | CWE | NIST § |
---|---|---|---|---|---|---|
3.2.3 | [MODIFIED] Verify that the application's session management model uses a combination of controls around secure storage, identifier expiration and identifier rotation to avoid session identifiers being stolen through attacks such as cross-site scripting or direct access to the browser. | ✓ | ✓ | ✓ | 539 | 7.1 |
# | Description | L1 | L2 | L3 | CWE |
---|---|---|---|---|---|
8.2.2 | [MODIFIED] Verify that data stored in browser storage (such as localStorage, sessionStorage, IndexedDB, or cookies) does not contain sensitive data, with the exception of session identifiers which should be stored in accordance with the application's session management model. | ✓ | ✓ | ✓ | 922 |
I would also suggest providing the following supplemental background guidance which talks about different storage mechanisms:
Seeing how complex this issue is, I think this is a wise suggestion, Josh.
I'm ok to make 8.2.2 more abstract.
I can not see the point or the need for proposed 3.2.3.
I think that is very reasonable suggestion from @elarlang - I like the "new" abstract 8.2.2 as well.
@elarlang @jmanico I just feel like we should say something about session identifier storage, no?
@elarlang
I can not see the point or the need for proposed 3.2.3.
I think currently it says session tokens should only be stored in secured cookies or session storage. Whereas the proposed 8.2.2 implies that session tokens can be stored in local storage provided the session management model allows for it. That would seem like the contradiction @tghosth hinted at.
I agree with Elar and Tiberius since there are so many considerations and ways to do session storage that we can’t easily be prescriptive without a serious run of 5-6 requirements for different situations.
I would therefore suggesting bringing back the original requirement 3.2.3 in a less prescriptive way
The original 3.2.3:
Verify the application only stores session tokens in the browser using secure methods such as appropriately secured cookies (see section 3.4) or HTML 5 session storage.
Proposed 3.2.3 - it is quite far away from the original one and tries to "fix the world" without actually fixing anything:
Verify that the application's session management model uses a combination of controls around secure storage, identifier expiration and identifier rotation to avoid session identifiers being stolen through attacks such as cross-site scripting or direct access to the browser.
@elarlang @jmanico I just feel like we should say something about session identifier storage, no?
I'm not against the idea to bring back 3.2.3 or to address the problem. I'm against proposed new 3.2.3. Maybe let's start with the question: what problem it should solve and then we can find a suitable requirement text.
Hey Tiberius, would you be willing to work on https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html
It’s missing a discussion on sessionstorage vs localstorage, as well as multi-tab and multi-device issues as well at token rotation.
You have a lot of expertise here Tiberius. Would you care to take this on (just updates to the cheat sheet) so we can reference it from ASVS?
Hey Tiberius, would you be willing to work on https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html
This issue is spin-off from https://github.com/OWASP/CheatSheetSeries/issues/1458#issue-2436606952 (linked by yourself) so you can carry another project-specific questions there.
Regarding 3.2.3, based on related discussions, it seems recommending a Session Storage approach to mitigate XSS has been abandoned due to the fact that XSS is essentially "Game over" as I believe it was said. Therefore, the remaining concern appears to be browser persistence of session identifiers as per @elarlang's comment here:
Difference comes, when users closes a browser without properly log out (it may also happen with hard reset like computer crash, losing electricity etc). If you then reopen the same workstation - question is, should valid token be available in that situation or not (we don't take in account browser own recovery mechanisms here).
Session expiration/timeout does mitigate the risk (as captured by other requirements), but perhaps there is a case for some (L3?) apps to be more strict in client-side storage of sessions?
it seems recommending a Session Storage approach to mitigate XSS has been abandoned due to the fact that XSS is essentially "Game over" as I believe it was said
@ryarmst So I think that it is less a case of XSS being game over and more the fact that from a functionality perspective it is problematic to store the identifier in session storage because it doesn't persist even between different browser tabs.
Maybe let's start with the question: what problem it should solve and then we can find a suitable requirement text.
@elarlang so I think we started off by solving the problem of 'how do you "securely" store session tokens' but I guess the real problem to solve is 'how do you prevent session tokens being stolen and misused' which makes me think that short lived tokens is a better option although that then leads to questions around the refresh token pattern...
perhaps there is a case for some (L3?) apps to be more strict in client-side storage of sessions?
@ryarmst I agree conceptually but I am not what that stricter guidance would look like. It also occurs to me that the main problem with local storage is that it is long lived which is in some ways irrelevant because the identifier should be expired at the server side anyway.
@tghosth To clarify, my original point was just confirming that the problem to address is not XSS. As far as multi-tab support, I mentioned here that there are apps already that accept this limitation (admittedly, these are annoying to use) BUT there may also be patterns to implement multi-tab support with Session Storage (disclaimer: I have not thoroughly investigated these and have never encountered them in actual apps).
Regarding your point about server side expiration, you could similarly say that Content-Security-Policy is irrelevant because XSS issues should be addressed where the injection vulnerability occurs. I think strictly controlled client storage of session identifiers is similarly a defense in depth approach (for the most part). That said, CSP by comparison is certainly a more effective control practically and implementing proper session expiration should be much easier than mitigating all XSS vectors.
@ryarmst thanks, that is useful to have a couple of examples for that.
So based on that, we could bring back 3.2.3 but make it L3 only. What do others thing @jmanico @elarlang @Tib3rius.
The proposal could be as follows:
# | Description | L1 | L2 | L3 | CWE | NIST § |
---|---|---|---|---|---|---|
3.2.3 | [MODIFIED] Verify that the application only stores session identifiers in the browser using secure methods such as session storage, web workers, JavaScript closures or appropriately secured cookies. | ✓ | 539 | 7.1 |
# | Description | L1 | L2 | L3 | CWE |
---|---|---|---|---|---|
8.2.2 | [MODIFIED] Verify that data stored in browser storage (such as localStorage, sessionStorage, IndexedDB, or cookies) does not contain sensitive data, with the exception of session identifiers for which guidance is provided in the session management chapter. | ✓ | ✓ | ✓ | 922 |
I did some additional thinking here and have a lot of suggestions. I'll keep it brief but please consider. If this too far off-track please disregard. I think this is really hard to get right.
3.2.3: Verify that the application only stores session identifiers in the browser using secure methods such as session storage, secured cookies (with Secure, HttpOnly, and SameSite attributes), and ensure that any JavaScript handling session data is protected against cross-site scripting (XSS) attacks through proper output encoding and Content Security Policies (CSP). Web workers and JavaScript closures should be used to isolate sensitive computations, but not for long-term session identifier storage.
8.2.2: Verify that data stored in browser storage (such as localStorage, sessionStorage, IndexedDB, or cookies) does not contain sensitive data. If sensitive data must be stored, ensure it is encrypted and integrity-protected, and that it is only stored temporarily with appropriate expiration mechanisms. Exceptions are made for session identifiers, provided their handling follows other guidance in the session management section, including the use of secure cookies and robust protection against cross-site scripting (XSS) attacks.
I think for 3.2.3 I prefer the shorter text as other requirements are covered elsewhere.
For 8.2.2 I think the idea of allowing encrypted data is interesting. It leads to a key management problem but may still be worthwhile a a cut out, I think I would carve this out as a separate question, opened #2029.
What about abstracting it to the level of the problem (just focusing on locally persisted identifiers here):
3.2.3: Verify that active session identifiers are not persisted when the associated browsing contexts are closed, such as when a user closes their browser or restarts their system.
This does not impose a mechanism and the active qualifier provides some additional flexibility.
So that is a possibility although that doesn't necessarily highlight the specific problem of session theft.
Also, would this still be L3 requirement only @ryarmst?
Good point, I'll suggest the following then, which I think makes the purpose more clear:
3.2.3: Verify that active session identifiers are not persisted when the associated browsing contexts are closed, such as when a user closes their browser or restarts their system, preventing client-side session theft following context termination.
In terms of setting the level (or adding this requirement at all), I would like to hear more feedback from others. This particular issue has an extensive discussion history (mostly relating to Session Storage). The requirement above avoids mandating an implementation, but may be impractical in some scenarios.
What about remember me type functionality? Even several high risk sites (like Google) keeps the session persisted almost permanently and then requires re-auth for admin level functions. I don't think closing the browsing context should always kill a session, especially when re-auth is in play properly.
Another example is a popular payment app. I can stay logged in for months and I just need to re-auth before issuing a payment.
And I don't mean to keep throttling these conversations. I just find that almost any advice on session management is hard.
@jmanico does it make sense to add an exception for when sessions must be persisted in this way, or would this make the requirement pointless? Should user informed consent for Remember Me be a factor? I didn't see a reference to Remember Me functionality in the ASVS, but this is an item we have reported for critical/sensitive applications:
Warning that use of the function is less secure and should only be used on a private system
The scenarios you mention mitigate the risk of extended/persisted sessions with re-auth/step up and I have also seen a number of apps use additional detection heuristics to identify session theft. Would it make sense to split requirements based on the intended lifetime of sessions?
3.2.3: Verify that active session identifiers are not persisted when the associated browsing contexts are closed, such as when a user closes their browser or restarts their system, preventing client-side session theft following context termination.
I agree with that context and I have pointed out this myself, but I'm not sure how realistic it is nowadays, as browsers support "restore previous session" or "restore previous tab" functionality, that kind of ignores the point for "session cookies" or even sessionStorage mechanisms.
As we have separate sections for cookies and tokens, we can also consider specific requirements based on technology used.
My take on this is that session expiration and re-authentication are the fundamental session controls these days and that depending on the removal of browser based tokens has a very low security benefit.
And I’m just thinking through this as the comments come up. Previous comments from me may have had a different perspective.
I agree with both of your points: this is a low value requirement and is challenging to ensure/implement in practice. @elarlang I will explore technology-specific requirements if any make sense.
To keep things moving, I will suggest we simply leave 3.2.3 as it was (deleted) for now.
Ok, I think being non-prescriptive is the only thing that makes sense at this point and hopefully the other session management requirements will cover us.
As such, I think the final proposal is that 3.2.3 stays deleted and we modify 8.2.2 as follows:
# | Description | L1 | L2 | L3 | CWE |
---|---|---|---|---|---|
8.2.2 | [MODIFIED, MERGED FROM 3.2.3] Verify that data stored in browser storage (such as localStorage, sessionStorage, IndexedDB, or cookies) does not contain sensitive data, with the exception of session identifiers. | ✓ | ✓ | ✓ | 922 |
Thanks all for a thoughtful discussion :)
Any further comments @elarlang @jmanico @ryarmst ?
After this long and helpful conversation, I think your conclusion is spot on, @tghosth :)
The advice to use sessionStorage is really non-standard. It breaks multi-tab browser use.
You can see us talking about this in the cheatsheet series here https://github.com/OWASP/CheatSheetSeries/issues/1458
I suggest we allow localStorage for sessions, but only if the token has both idle and absolute timeout so long terms token storage is less risky.