search

OWASP / ASVS

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

V3.2.3 Session storage #843

Closed bretik closed 2 years ago

bretik commented 3 years ago

3.2.3 states "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."

I see this was changed in #553 - I would like to reopen the discussion about session storage being a secure location for storing session identifiers.

1) NIST 800-63B 7.1: Secrets used for session binding SHOULD NOT be placed in insecure locations such as HTML5 Local Storage due to the potential exposure of local storage to cross-site scripting (XSS) attacks.

I believe HTML5 Local Storage here refers to both window.localStorage and window.sessionStorage from the "Web Storage API", since both are prone to XSS

2) HTML5 Security Cheat Sheet Also known as Offline Storage, Web Storage.... A single Cross Site Scripting can be used to steal all the data in these objects, so again it's recommended not to store sensitive information in local storage.

The HTML5 Security Cheat Sheet refers to sessionStorage only in one place and states, that it is better to use sessionStorage instead of localStorage in case where you do not need persistence.

3) 8.2.2 Verify that data stored in browser storage (such as HTML5 local storage, session storage, IndexedDB, or cookies) does not contain sensitive data or PII.

jmanico commented 3 years ago

HTML5 local and session storage are both vulnerable to XSS were an HTTPonly cookie is not.

I agree to drop off the HTML5 session storage from this requirement completely.

-- Jim Manico @Manicode

On Sep 23, 2020, at 2:31 AM, bretik notifications@github.com wrote:

 3.2.3 states "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."

I see this was chaned in #553 - I would like to reopen the discussion about session storage being a secure location for storing session identifiers.

NIST 800-63B 7.1: Secrets used for session binding SHOULD NOT be placed in insecure locations such as HTML5 Local Storage due to the potential exposure of local storage to cross-site scripting (XSS) attacks. I believe HTML5 Local Storage here refers to both window.localStorage and window.sessionStorage from the "Web Storage API", since both are prone to XSS

HTML5 Security Cheat Sheet Also known as Offline Storage, Web Storage.... A single Cross Site Scripting can be used to steal all the data in these objects, so again it's recommended not to store sensitive information in local storage. The HTML5 Security Cheat Sheet refers to sessionStorage only in one place and states, that it is better to use sessionStorage instead of localStorage in case where you do not need persistence.

8.2.2 Verify that data stored in browser storage (such as HTML5 local storage, session storage, IndexedDB, or cookies) does not contain sensitive data or PII. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

jmanico commented 3 years ago

Hook us up with a PR on this and I’ll approve it super fast.

-- Jim Manico @Manicode

On Sep 23, 2020, at 2:31 AM, bretik notifications@github.com wrote:

 3.2.3 states "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."

I see this was chaned in #553 - I would like to reopen the discussion about session storage being a secure location for storing session identifiers.

NIST 800-63B 7.1: Secrets used for session binding SHOULD NOT be placed in insecure locations such as HTML5 Local Storage due to the potential exposure of local storage to cross-site scripting (XSS) attacks. I believe HTML5 Local Storage here refers to both window.localStorage and window.sessionStorage from the "Web Storage API", since both are prone to XSS

HTML5 Security Cheat Sheet Also known as Offline Storage, Web Storage.... A single Cross Site Scripting can be used to steal all the data in these objects, so again it's recommended not to store sensitive information in local storage. The HTML5 Security Cheat Sheet refers to sessionStorage only in one place and states, that it is better to use sessionStorage instead of localStorage in case where you do not need persistence.

8.2.2 Verify that data stored in browser storage (such as HTML5 local storage, session storage, IndexedDB, or cookies) does not contain sensitive data or PII. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

elarlang commented 3 years ago

And where you going to hold your JWT tokens? :)

My argumentation on that topic: https://github.com/OWASP/ASVS/issues/696#issuecomment-626231907 and it's waiting for ASVS v4.1

jmanico commented 3 years ago

for mobile: iOS keychain and similar websites: HTTPOnly cookies (cannot be stolen via XSS) for servers: key/secret vaults

🤙🏼

-- Jim Manico @Manicode

On Sep 23, 2020, at 8:24 AM, Elar Lang notifications@github.com wrote:

 And where you going to hold your JWT tokens? :)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

elarlang commented 3 years ago

This change is no-no-no.

If you make this change, it's clear duplicate for section 3.4 or it has basically meaning: "check section 3.4". There is no need for this kind of requirement.

Seems, that you have not taken your time to read my comment and discussion in other issue on that topic, so I copy-paste it here, from #696:

In practice, maybe we need to reorganise the idea for this requirement with V3.2.3.

V3.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.

It has the same problem - use correct settings for cookie or use HTML5 session storage. And we reach to one setting - is HttpOnly flag for cookie important or not. In other words - should session token be readable for JavaScript or not. Should we have this extra mitigation for "XSS situations" or not. If you have XSS on site... your active session is usable for attacker via your browser anyway.

I think, we need to check separately "Cookie-based" and "Token-based" session management for those requirements.

If we have "Cookie-based Session management"

If we have "Token-based Session management"

V8.2.2 Verify that data stored in client side storage (such as HTML5 local storage, session storage, IndexedDB, regular cookies or Flash cookies) does not contain sensitive data or PII.

And then there are solutions, where Token is used in cookie... where we should have question, do you use token-based session management correctly.

My current conclusions/ideas:

bretik commented 3 years ago

Sorry, I should have reacted to your comment before creating the PR. This change is only quick fix to remove the unsecure recommendation. #696 will not be resolved by this change.

If token-based session should not use cookie, use only "session storage part" from V3.2.3

If we take NISTs term "Secrets used for session binding" as both cookie-based and token-based, we cannot recommend using session storage, it would be in direct conflict with that recommendation.

Add new requirement to Cookie-based Session Management with meaning "session token is allowed to send only with httponly cookies" (but isn't it too strict?)

Requirements in V3.4 do not give me options how to store the session cookie other than a cookie with HttpOnly, Secure and SameSite attributes, with __Host prefix and precise path

If token-based session should be able to use cookies, move current V3.2.3 to Token-based requirement

I agree with the statement, that requirement V3.2.3 will be redundant for V3.4 after the change (but not in conflict with it) and could be moved to V3.5.

jmanico commented 3 years ago

Elar,

I read your comment in detail. I politely do not agrre with your logic around session storage. It’s a dangerous suggestion when faced with XSS and several standards also reflect this.

I’ll give this PR more time to make sure we also fix the secondary issue that is being impacted. I’m in a rush now but will look at this carefully over the weekend when less so.

I’m really excited to see this PR because I know the session storage recomendation was against several other solid standards.

Regards,

Jim Manico @Manicode

On Sep 23, 2020, at 8:53 PM, bretik notifications@github.com wrote:

 Sorry, I should have reacted to your comment before creating the PR. This change is only quick fix to remove the unsecure recommendation. #696 will not be resolved by this change.

If token-based session should not use cookie, use only "session storage part" from V3.2.3

If we take NISTs term "Secrets used for session binding" as both cookie-based and token-based, we cannot recommend using session storage, it would be in direct conflict with that recommendation.

Add new requirement to Cookie-based Session Management with meaning "session token is allowed to send only with httponly cookies" (but isn't it too strict?)

Requirements in V3.4 do not give me options how to store the session cookie other than a cookie with HttpOnly, Secure and SameSite attributes, with __Host prefix and precise path

If token-based session should be able to use cookies, move current V3.2.3 to Token-based requirement

I agree with the statement, that requirement V3.2.3 will be redundant for V3.4 after the change (but not in conflict with it) and could be moved to V3.5.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

elarlang commented 3 years ago

If session token handling is done with cookies, then it's no-brainer to not talk about keeping this value in sessionStorage or localStorage.

Now, if session token is sent to API with bearer header (for example), then where you going to keep session token value (or JWT) on the client-side? With this change you need to solve that situation also.

From attacking perspective - IMO stealing session token and then using it yourself is a bit yesterday. I could automate victim browser with some nice XSS "main in the browser" payload and make victim browser just a gateway. Immediate and effective. You can force victim browser to do everything without making requests to some application from your own IP. I recommend to use HttpOnly for session cookies, but on the other hand - you just need to be sure that you don't have an XSS, otherwise you are <beep> anyway.

ropnop commented 3 years ago

Came here from Twitter, but wanted to chime in in a more organized fashion:

I agree the wording on 3.2.3 may be problematic, since it seems to imply that HTML5 Session Storage is secure, or as secure, as Cookies. But I'm worried that by only recommending Cookies to store session information in the browser, it dictates requirements that will need to go in the 1.3 Session Management Architectural Requirements, and then also heavily affects the implementation of 3.5 Token-based Session Management. If the only recommended way of storing tokens is via a Cookie, then the only recommended way of accepting a token has to be via a Cookie.

The problems with a Cookie-only approach are:

Personally I think that Cookies introduce too much security debt we've been trying to clean up for years. They essentially bypass the SOP and are over-scoped by default (entire domain, not origin). Even with SameSite there are "gotchas" you have to be careful of, which then lead to the Public Suffixes list (e.g. github.io, amazonaws.com, etc). As a backend architect, I'd rather be explicit in allowing authentication material from a whitelisted set of origins and headers via CORS for my services than have to read values from cookies.

I wish there was a better way to keep a secret in a browser, and I do agree that from a purely client side perspective HttpOnly cookies offer more protection than HTML5 storage - but architecturally I don't agree that moving to Cookie-only session management is a security best practice ASVS should be recommending. Just my $0.02 👍

jmanico commented 3 years ago

This debate point is, since I can abuse a cookie with XSS there is no point in protecting it. It’s a fair argument, but I politely disagree with it. Persistant cookie theft is much higher severity that man in the browser XSS, especially whe JWT’s are in play.

I’d like ASVS to make strong, opiniated suggestions and putting highly sensitive data in an unprotected area of the browser is bad practice and modern standards are catching up to this.

-- Jim Manico @Manicode Secure Coding Education +1 (808) 652-3805

On Sep 24, 2020, at 2:28 AM, Elar Lang notifications@github.com wrote:

 If session token handling is done with cookies, then it's no-brainer to not talk about keeping this value in sessionStorage or localStorage.

Now, if session token is sent to API with bearer header (for example), then where you going to keep session token value (or JWT) on the client-side? With this change you need to solve that situation also.

From attacking perspective - IMO stealing session token and then using it yourself is a bit yesterday. I could automate victim browser with some nice XSS "main in the browser" payload and make victim browser just a gateway. Immediate and effective. You can force victim browser to do everything without making requests to some application from your own IP. I recommend to use HttpOnly for session cookies, but on the other hand - you just need to be sure that you don't have an XSS, otherwise you are anyway.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

ropnop commented 3 years ago

Well said - I do think that is a point we politely disagree on. I don't believe there is no point in protecting it - only that protecting data post compromise (i.e. post XSS) is a defense in depth measure and not a first layer security control. I do believe XSS and CSRF are "first order" vulnerabilities that need to be dealt with first and foremost. Using cookies adds a second layer of defense to XSS, but at the cost of opening the door to CSRF. I guess my argument is XSS is always going to be a threat no matter what; if we can eliminate one class of vulnerability entirely (CSRF) we can spend more effort mitigating the other.

jmanico commented 3 years ago

My comments:

1) CSRF defense is many factors easier to implement that CSRF defense

2) Cross domain authentication (Federation) should be implemented with OIDC or a similar tech, and practices are well documented in that standard. “by hand” federation is insecure at best.

-- Jim Manico @Manicode Secure Coding Education +1 (808) 652-3805

On Sep 24, 2020, at 6:21 AM, Ronnie Flathers notifications@github.com wrote:

 Well said - I do think that is a point we politely disagree on. I don't believe there is no point in protecting it - only that protecting data post compromise (i.e. post XSS) is a defense in depth measure and not a first layer security control. I do believe XSS and CSRF are "first order" vulnerabilities that need to be dealt with first and foremost. Using cookies adds a second layer of defense to XSS, but at the cost of opening the door to CSRF. I guess my argument is XSS is always going to be a threat no matter what; if we can eliminate one class of vulnerability entirely (CSRF) we can spend more effort mitigating the other.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

elarlang commented 3 years ago

This debate point is, since I can abuse a cookie with XSS there is no point in protecting it.

No. The point is - if you going to remove this sessionStorage options for session tokens (which completely make sense and it's covered also in my proposals), you need to keep in mind that it's not the only solution. We need to provide solution for cross-origin API also.

jmanico commented 3 years ago

Cross orgin authentication (federation) should be done using a Federation standard like SAML or a OIDC which have well-established conventions for all client types.

-- Jim Manico @Manicode

On Sep 24, 2020, at 6:34 AM, Elar Lang notifications@github.com wrote:

 This debate point is, since I can abuse a cookie with XSS there is no point in protecting it.

No. The point is - if you going to remove this sessionStorage for cookies (which completely make sense and it's covered also in my proposals), you need to keep in mind that it's not the only solution. We need to provide solution for cross-origin API also.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

jmanico commented 3 years ago

Elar,

Seeing how debatable this is - both the discussion on Twitter and here - I’m going to delay approving it so we can finish the debate.

I’m not gonna repeat the same thing I did to Robyn at the CS series and just forcibly checked this. I will wait until everyone has had a chance to discuss this completely so there is no rush, let’s keep talking. I’m on the fence myself I know this is a very controversial move.

❤️

-- Jim Manico @Manicode

On Sep 24, 2020, at 6:34 AM, Elar Lang notifications@github.com wrote:

 This debate point is, since I can abuse a cookie with XSS there is no point in protecting it.

No. The point is - if you going to remove this sessionStorage for cookies (which completely make sense and it's covered also in my proposals), you need to keep in mind that it's not the only solution. We need to provide solution for cross-origin API also.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

tghosth commented 3 years ago

Hi, when it comes to session storage and cookies, I really really don't think we should be mandating one and banning the other. They both have valid use cases.

jmanico commented 3 years ago

Also, citing the “cross domain limitations of cookies” is not a technically accurate argument. CORS withCredentials will let you send and set cookies cross domain https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/withCredentials

-- Jim Manico @Manicode

On Sep 24, 2020, at 6:34 AM, Elar Lang notifications@github.com wrote:

 This debate point is, since I can abuse a cookie with XSS there is no point in protecting it.

No. The point is - if you going to remove this sessionStorage for cookies (which completely make sense and it's covered also in my proposals), you need to keep in mind that it's not the only solution. We need to provide solution for cross-origin API also.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

jmanico commented 3 years ago

Elar,

Forgive the repeat comment, I just wanted to reply in context that citing the “cross domain limitation of cookies” is not a technically accurate argument. CORS withCredentials will let you send and set cookies cross domain: https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/withCredentials

-- Jim Manico @Manicode

On Sep 24, 2020, at 6:34 AM, Elar Lang notifications@github.com wrote:

 This debate point is, since I can abuse a cookie with XSS there is no point in protecting it.

No. The point is - if you going to remove this sessionStorage for cookies (which completely make sense and it's covered also in my proposals), you need to keep in mind that it's not the only solution. We need to provide solution for cross-origin API also.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

mackowski commented 3 years ago

What about leaving HTML 5 session storage in the description but mark it as a valid option for L1 and L2 apps but mandate cookies for L3 as it adds additional layer of the security. It will make clear that in some cases session storage is a valid option but if you want the highest level of security use browser cookies. I see both arguments and I just try to find compromise that will not harm security. If ASVS does not mandate for example "3.2.4 | Verify that session token are generated using approved cryptographic algorithms" for L1 apps than storing session id in session storage also seems to me reasonable.

elarlang commented 3 years ago

I conclude my comments and context here:

I think that's it from me, otherwise it's going to loop :)

jmanico commented 3 years ago

... I am ok with this if we also replace session storage with web workers and JS closures which are resistant to XSS token theft. =)

On 9/24/20 11:40 PM, mackowski wrote:

What about leaving HTML 5 session storage in the description but mark it as a valid option for L1 and L2 apps but mandate cookies for L3 as it adds additional layer of the security. It will make clear that in some cases session storage is a valid option but if you want the highest level of security use browser cookies. I see both arguments and I just try to find compromise that will not harm security. If ASVS does not mandate for example "3.2.4 | Verify that session token are generated using approved cryptographic algorithms" for L1 apps than storing session id in session storage also seems to me reasonable.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/OWASP/ASVS/issues/843#issuecomment-698832409, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBYCMLIBUCFRZCLZ2NWHTSHRQSJANCNFSM4RW7FPPQ.

-- Jim Manico Manicode Security https://www.manicode.com

mackowski commented 3 years ago

Yes, web workers and JS closures makes a lot of sense in that scenario.

jmanico commented 3 years ago

So if we redo the requirement and drop the language for session storage and replace it with web workers and closures I would hope we’re all closer to being in agreement?

OP, care to edit this PR with that in mind?

-- Jim Manico @Manicode

On Sep 29, 2020, at 3:04 AM, mackowski notifications@github.com wrote:

 Yes, web workers and JS closures makes a lot of sense in that scenario.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

bretik commented 3 years ago

I already did update the PR, see https://github.com/OWASP/ASVS/pull/844#issuecomment-700423796

tghosth commented 3 years ago

Sorry but regardless of my feelings on this change, it breaks the rules for 4.0.2 because it changes what is acceptable under requirement 3.2.3. An app that previously passed this requirement because it used session storage would now fail this requirement.

Andrew's original rules for 4.0.2:

bug fixes only. Will require some work to back port the fixes from master. The only thing I would countenance here is duplicate issue removal, spelling fixes, and clarifications.

I have opened #846 instead but I want to take the time to read through this thread before I merge it.

tghosth commented 3 years ago

I dug into this a bit more.

This blog post from @philippederyck would seem to indicate that web workers are also vulnerable to XSS. (hi @philippederyck I would be keen to hear your thoughts on this thread.)

This just leaves us with JavaScript closures.

Overall, I am concerned that we are mandating a very complex mechanism for a L1 control. I would be more comfortable allowing session storage for L1 but not for L2+.

jmanico commented 3 years ago

Good call. I agree this is a L2 control. And makes since to delay until after 4.02.

So long as session/local storage of powerful tokens goes away we are good.

And closures are very straight forward and old school JavaScript at this point. There is nothing fancy or complex about closures for data protection.

I dug into this a bit more.

This blog post https://pragmaticwebsecurity.com/articles/oauthoidc/localstorage-xss.html from @philippederyck https://github.com/philippederyck would seem to indicate that web workers are also vulnerable to XSS. (hi @philippederyck https://github.com/philippederyck I would be keen to hear your thoughts on this thread.)

This just leaves us with JavaScript closures.

Overall, I am concerned that we are mandating a very complex mechanism for a L1 control. I would be more comfortable allowing session storage for L1 but not for L2+.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/OWASP/ASVS/issues/843#issuecomment-701644868, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBYCMQCYVEJWIZQ7ND3FDSIOMODANCNFSM4RW7FPPQ.

-- Jim Manico Manicode Security https://www.manicode.com

philippederyck commented 3 years ago

I dug into this a bit more.

This blog post from @philippederyck would seem to indicate that web workers are also vulnerable to XSS. (hi @philippederyck I would be keen to hear your thoughts on this thread.)

This just leaves us with JavaScript closures.

Overall, I am concerned that we are mandating a very complex mechanism for a L1 control. I would be more comfortable allowing session storage for L1 but not for L2+.

Web workers themselves are typically not vulnerable to XSS. That post is to illustrate that hiding tokens in a worker is fine, but if the app needs to use them, there is a way to extract them anyway. The same holds for closures in combination with prototype poisoning. Cookies are better in some cases, but don't work well across domains. That's why you see Backend-For-Frontend gaining traction for security-sensitive applications ...

While I agree that not putting data in localStorage may make an attack more difficult, it will never "solve" the problem. Only preventing malicious JS code does.

deveras commented 3 years ago

Effective and efficient usage of Content Security Policy together with interceptors at the application layer, does it not allow secure usage of local storage? If the user browser doesn't have the required features for it to work is it not up to the developer to create a fallback implementation to secure the application? Being technology restrictive is bad! Awareness seams to better than denying what can be securely achieved. Automating application security checks and this feature in particular, would raise false positives if the above is implemented, how to go around it?

jmanico commented 3 years ago

So do you endorse keeping the requirement as sessionstorage, Philippe?

On 9/30/20 9:31 PM, Philippe De Ryck wrote:

I dug into this a bit more.

This blog post
<https://pragmaticwebsecurity.com/articles/oauthoidc/localstorage-xss.html>
from @philippederyck <https://github.com/philippederyck> would
seem to indicate that web workers are also vulnerable to XSS. (hi
@philippederyck <https://github.com/philippederyck> I would be
keen to hear your thoughts on this thread.)

This just leaves us with JavaScript closures.

Overall, I am concerned that we are mandating a very complex
mechanism for a L1 control. I would be more comfortable allowing
session storage for L1 but not for L2+.

Web workers themselves are typically not vulnerable to XSS. That post is to illustrate that hiding tokens in a worker is fine, but if the app needs to use them, there is a way to extract them anyway. The same holds for closures in combination with prototype poisoning. Cookies are better in some cases, but don't work well across domains. That's why you see Backend-For-Frontend gaining traction for security-sensitive applications ...

While I agree that not putting data in localStorage may make an attack more difficult, it will never "solve" the problem. Only preventing malicious JS code does.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/OWASP/ASVS/issues/843#issuecomment-701947356, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBYCOUISTBUKYCMTNDL5LSIQV4ZANCNFSM4RW7FPPQ.

tghosth commented 3 years ago

Hoping this session from @esarafianou at AppSec Israel 2020 is going to also provide some useful information on this :) https://appsecil2020.sched.com/event/fF7l/

jmanico commented 3 years ago

These are the same folks from Auth0 who suggested (in their formal documents) to avoid local/sessionStorage and to use closures, web workers, and HTTPOnly cookies.... I'm looking forward to watching it. I hope she can shed more light on the situation.

On 10/14/20 1:27 AM, Josh Grossman wrote:

Hoping this session from @esarafianou https://github.com/esarafianou at AppSec Israel 2020 is going to also provide some useful information on this :) https://appsecil2020.sched.com/event/fF7l/ https://appsecil2020.sched.com/event/fF7l/

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/OWASP/ASVS/issues/843#issuecomment-708340496, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBYCLCYDHIJXKKRUR3F3TSKWDKJANCNFSM4RW7FPPQ.

-- Jim Manico Manicode Security https://www.manicode.com

esarafianou commented 3 years ago

@tghosth, @jmanico I'll indeed include in-memory within Web Workers, JS closures with their shortcomings and cookies in the talk!

jmanico commented 3 years ago

Eva,

As we debate this requirement can you kindly give us your token storage recommendations and conclusions before your talk? We will not tell anyone! ;)

Aloha.

On 10/18/20 6:31 AM, Eva Sarafianou wrote:

@tghosth https://github.com/tghosth, @jmanico https://github.com/jmanico I'll indeed include in-memory within Web Workers, JS closures with their shortcomings and cookies in the talk!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/ASVS/issues/843#issuecomment-711277064, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBYCPQMEOKZUN7S4RZHQDSLMJ5BANCNFSM4RW7FPPQ.

-- Jim Manico Manicode Security https://www.manicode.com

esarafianou commented 3 years ago

Let me write my conclusions as clear as possible:

  1. If you can avoid browser storage and use BFF, this is the best option
  2. If the secret is needed for a specific functionality that can be isolated, store the token in memory within a Web Worker a. Requirement: The token should never leave the Web Worker environment b. Shortcoming: CSRF using the token is still possible by asking the Web Worker to make a request and send back to the main process the data needed from the response. We protect the token's confidentiality for offline usage.
  3. JS closures are another form of isolation but they have shortcomings: Any function defined outside of the closure but used inside the closure that makes use of the token can be overridden to reveal the token to the attacker. A solution would be to keep a local copy of that function within the closure but things get cumbersome and definitely a mess with 3rd party dependencies using other dependencies.
  4. Based on 2 and 3, Web Workers win but come with complexity for the developer.
  5. If the requirement that the token is kept in isolation cannot be met, any other browser storage is susceptible to XSS.

For recommendations, I think we should move away of generic statements such as "Use Web Workers' or "Use JS Closures" as they can as well be vulnerable if the main requirement is not met. We need to arm the developers with the knowledge of the requirements and the shortcomings of each option along with the security guarantees they have. Unfortunately secure browser storage is so complex that I have a hard time finding a way to recommend something in a single line.

philippederyck commented 3 years ago

+1 for this advice

— Pragmatic Web Security Security for developers https://pragmaticwebsecurity.com/

On 21 Oct 2020, at 18:50, Eva Sarafianou notifications@github.com wrote:

Let me write my conclusions as clear as possible:

If you can avoid browser storage and use BFF, this is the best option If the secret is needed for a specific functionality that can be isolated, store the token in memory within a Web Worker a. Requirement: The token should never leave the Web Worker environment b. Shortcoming: CSRF using the token is still possible by asking the Web Worker to make a request and send back to the main process the data needed from the response. We protect the token's confidentiality for offline usage. JS closures are another form of isolation but they have shortcomings: Any function defined outside of the closure but used inside the closure that makes use of the token can be overridden to reveal the token to the attacker. A solution would be to keep a local copy of that function within the closure but things get cumbersome and definitely a mess with 3rd party dependencies using other dependencies. Based on 2 and 3, Web Workers win but come with complexity for the developer. If the requirement that the token is kept in isolation cannot be met, any other browser storage is susceptible to XSS. For recommendations, I think we should move away of generic statements such as "Use Web Workers' or "Use JS Closures" as they can as well be vulnerable if the main requirement is not met. We need to arm the developers with the knowledge of the requirements and the shortcomings of each option along with the security guarantees they have. Unfortunately secure browser storage is so complex that I have a hard time finding a way to recommend something in a single line.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/ASVS/issues/843#issuecomment-713712375, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACJAZDWASFLVFN23WMVF663SL4GNLANCNFSM4RW7FPPQ.

esarafianou commented 3 years ago

I realized I forgot to comment on HTTPOnly cookies: That's a great option but it has the requirement that the secret is needed only in the backend and it has the shortcoming of CSRF (similar to Web Workers).

tghosth commented 3 years ago

@jmanico Sounds like we need @esarafianou to expand the relevant OWASP cheatsheet section :) https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html#html5-web-storage-api

Sjord commented 3 years ago

If you can avoid browser storage and use BFF, this is the best option

Does BFF here mean Backends For Frontends?

esarafianou commented 3 years ago

@Sjord yes that's it

@tghosth @jmanico I'd be happy to contribute to the cheatsheet.

jmanico commented 3 years ago

Thank you for helping clear cough, cough things up for us.

My suggestion is that we simply drop requirement 3.2.2. It is irresponsible to directly suggest storing a token in localstorage as a security requirement. We need to rethink this from scratch.

On 10/21/20 6:50 AM, Eva Sarafianou wrote:

Let me write my conclusions as clear as possible:

  1. If you can avoid browser storage and use BFF, this is the best option
  2. If the secret is needed for a specific functionality that can be isolated, store the token in memory within a Web Worker a. Requirement: The token should never leave the Web Worker environment b. Shortcoming: CSRF using the token is still possible by asking the Web Worker to make a request and send back to the main process the data needed from the response. We protect the token's confidentiality for offline usage.
  3. JS closures are another form of isolation but they have shortcomings: Any function defined outside of the closure but used inside the closure that makes use of the token can be overridden to reveal the token to the attacker. A solution would be to keep a local copy of that function within the closure but things get cumbersome and definitely a mess with 3rd party dependencies using other dependencies.
  4. Based on 2 and 3, Web Workers win but come with complexity for the developer.
  5. If the requirement that the token is kept in isolation cannot be met, any other browser storage is susceptible to XSS.

For recommendations, I think we should move away of generic statements such as "Use Web Workers' or "Use JS Closures" as they can as well be vulnerable if the main requirement is not met. We need to arm the developers with the knowledge of the requirements and the shortcomings of each option along with the security guarantees they have. Unfortunately secure browser storage is so complex that I have a hard time finding a way to recommend something in a single line.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/ASVS/issues/843#issuecomment-713712375, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBYCMZ2VHXF7Z4GPDDEWTSL4GNJANCNFSM4RW7FPPQ.

-- Jim Manico Manicode Security https://www.manicode.com

jmanico commented 3 years ago

And Eva, most importantly, this is outstanding research. I appreciate you clarifying just how complex and nuanced this issue is. This was extremely helpful.

Aloha,

-- Jim Manico Manicode Security https://www.manicode.com

On 10/21/20 6:50 AM, Eva Sarafianou wrote:

Let me write my conclusions as clear as possible:

  1. If you can avoid browser storage and use BFF, this is the best option
  2. If the secret is needed for a specific functionality that can be isolated, store the token in memory within a Web Worker a. Requirement: The token should never leave the Web Worker environment b. Shortcoming: CSRF using the token is still possible by asking the Web Worker to make a request and send back to the main process the data needed from the response. We protect the token's confidentiality for offline usage.
  3. JS closures are another form of isolation but they have shortcomings: Any function defined outside of the closure but used inside the closure that makes use of the token can be overridden to reveal the token to the attacker. A solution would be to keep a local copy of that function within the closure but things get cumbersome and definitely a mess with 3rd party dependencies using other dependencies.
  4. Based on 2 and 3, Web Workers win but come with complexity for the developer.
  5. If the requirement that the token is kept in isolation cannot be met, any other browser storage is susceptible to XSS.

For recommendations, I think we should move away of generic statements such as "Use Web Workers' or "Use JS Closures" as they can as well be vulnerable if the main requirement is not met. We need to arm the developers with the knowledge of the requirements and the shortcomings of each option along with the security guarantees they have. Unfortunately secure browser storage is so complex that I have a hard time finding a way to recommend something in a single line.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/ASVS/issues/843#issuecomment-713712375, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBYCMZ2VHXF7Z4GPDDEWTSL4GNJANCNFSM4RW7FPPQ.

jmanico commented 3 years ago

Hey @esarafianou https://github.com/esarafianou are you into it? We would love your help!

Aloha, Jim

PS: hit me up at Jim@manicode.com if you have any questions...

On 10/21/20 10:46 AM, Josh Grossman wrote:

@jmanico https://github.com/jmanico Sounds like we need @esarafianou https://github.com/esarafianou to expand the relevant OWASP cheatsheet section :) https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html#html5-web-storage-api https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html#html5-web-storage-api

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/ASVS/issues/843#issuecomment-713867597, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBYCJJMJQWYM623E5EYTDSL5CCFANCNFSM4RW7FPPQ.

esarafianou commented 3 years ago

Hi @jmanico,

Apologies for the late response. Yes I'd happily contribute to the cheatsheet. I'll start working on a PR and I'll definitely email you with any questions. Thanks!

esarafianou commented 3 years ago

@jmanico, @tghosth A bit late but I just pushed a PR in CheatSheetSeries for suggested changes in the Session Management CheatSheet.

mackowski commented 3 years ago

Thanks @esarafianou I reviewed and merged your PR on Cheat Sheets side.

Sjord commented 3 years ago

Link: Extend Web Storage #524

jmanico commented 3 years ago

I've been slow these days but you are in my inbox. Give me a few more days and I'll review this carefully.

Forgive the slow response! I've been busy doomscrolling. I need more productive activity anyhow.

Aloha,

On 1/1/21 8:53 AM, Eva Sarafianou wrote:

@jmanico https://github.com/jmanico, @tghosth https://github.com/tghosth A bit late but I just pushed a PR in CheatSheetSeries for suggested changes in the Session Management CheatSheet.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/ASVS/issues/843#issuecomment-753367592, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBYCM5SBWKAOWMJ3JBBMLSXYK3XANCNFSM4RW7FPPQ.

-- Jim Manico Manicode Security https://www.manicode.com

tghosth commented 3 years ago

Hi @bretik @jmanico
Just revisiting this threat in the context of #846. Having read through the thread again and looked at the PR. I am still not comfortable with mandating Web Workers or JavaScript closures for L1 or even L2 applications. It seems very complicated and I have never seen any app using these up until now. Is there any way we can make this more realistic/accessible or is it a question of an L1 requirement that allows cookies or session storage and an L3 requirement which mandates cookies or Web Workers or JavaScript closures?

bretik commented 3 years ago

HI, I agree that using closures or web workers does not have to be "absolute must" requirement. I still don't think we should recommend using session storage (which was the main point of this issue) few lines after:

these requirements have been adapted to be a compliant subset of selected NIST 800-63b controls

when NIST 800-63b 7.1 states:

Secrets used for session binding: SHOULD NOT be placed in insecure locations such as HTML5 Local Storage due to the potential exposure of local storage to cross-site scripting (XSS) attacks.

So it would be probably best to drop this requirement from lower level(s) unless someone can come with a clear requirement other than "Verify the application only stores session tokens in the browser using secure methods", since we cannot say only cookies are the only correct option.