hapijs / crumb

CSRF crumb generation and validation for hapi
Other
171 stars 50 forks source link

Question: Does it make sense to use Crumb with CORS? #23

Closed findesk closed 9 years ago

findesk commented 9 years ago

Hi,

I may be completely misunderstanding the code and/or XSRF protection in general but...

In 3.x.x. if CORS is enabled, Crumb never generates a crumb cookie. See:

https://github.com/hapijs/crumb/blob/b019180073349ad028a83ad20bfa85d8c3c71bae/lib/index.js#L59

All requests pass through the plugin with no XSRF protection, even though I am using the plugin.

Does this mean that Crumb should not be used in an environment supporting CORS?

(For background, previously with Crumb 2.2.0, I had CORS set up, was using Crumb restful=false, and passing XSRF tokens in the payload. Which was working great. This no longer works with Crumb 3.x.x. With my same 2.2.0 setup, Crumb doesn't do anything. I was surprised to discover this. Again, perhaps I'm misunderstand XSRF protection when using CORS.)

Thanks fd

stongo commented 9 years ago

Very good question! CSRF Tokens are not needed when using CORS, as CORS adequately handles CSRF. I'm about to add an Origin check. If the origin doesn't match the server name and CORS is enabled, crumb validation will be bypassed. This should handle instances when CORS is enabled, but same origin calls are still made to the server. The reason for not setting a crumb cookie when CORS is enabled to prevent token leakage on cross site requests. Otherwise the crumb cookie would be set on non-origin sites.

stongo commented 9 years ago

See https://github.com/hapijs/crumb/issues/24

findesk commented 9 years ago

With an app at http://api.somedomain.com, if I have cors: { origin: ['http://app.mydomain.com'] } in my server config, can you describe a scenario where token leakage can occur?

If I'd hit http://api.somedomain.com from http://attacksite.com, wouldn't cors in hapi prevent the request, and therefore prevent the CSRF tokens from leaking?

(I'm assuming you can't spoof origin: http://stackoverflow.com/questions/21058183/whats-to-stop-malicious-code-from-spoofing-the-origin-header-to-exploit-cors)

Or are you working to protect against scenarios where cors: { origin: ['*'] } (i.e. hapi default.)

Just trying to understand the circumstances where token leak is at issue.

(With cors origin white listed, I could have other other cookies too, yar session cookie for example. So I'm trying to figure out...what is the harm in exposing CSRF tokens to my white list when other cookies, potentially quite precious cookies, could be exposed.)

stongo commented 9 years ago

I definitely agree it is a wider issue, and there is even more sensitive information that can be leaked with cookies, and I believe this is already on the radar for hapi core, as well as this issue for setting cookies during proxy https://github.com/hapijs/hapi/issues/1813 Crumb went through a security audit, and the CORS token leakage was flagged as well as the proxy leakage.

findesk commented 9 years ago

Got it. Closing now.

Fyi...I had been taking guidance from:

https://www.owasp.org/index.php/HTML5_Security_Cheat_Sheet#Cross_Origin_Resource_Sharing

See bullet 4.

"Keep in mind that CORS does not prevent the requested data from going to an unauthenticated location. It's still important for the server to perform usual CSRF prevention."

stongo commented 9 years ago

In light of that owasp segment, I could leave crumb as is now, as in still performing crumb check on CORS requests. One can set a route to issue crumb tokens via a CORS route like https://github.com/hapijs/crumb/blob/master/example/restful.js#L13-L22 This would allow for CORS to still use CSRF Tokens and not leak tokens via cookies on every CORS request. Going to review with my security team before closing this.

stongo commented 9 years ago

OK the conclusion of this upon review is that token leakage when CORS origin is set to specified hosts is acceptable, and the cookie should only be unset if cors.origin is set to '*' See issue https://github.com/hapijs/crumb/issues/25

tomsteele commented 9 years ago

@findesk The scenario where you wouldn't want to leak the token is this. Say you have http://api.mydomain.com, you would like a 3rd party http://foobar.com to access some CORS handlers, but not others. You wouldn't want to leak the CSRF token, because if you did that, well they could make requests to every route.

But we still need support actually allowing POST requests. And you're right, having CSRF protection on these routes is absolutely necessary, as hapi's cor's settings do not prevent simple http requests from being processed by handlers.

There are two ways I can see of handling this. Use the suggestion @stongo pointed to above. Which I think is fine, most frameworks just leak the tokens ;)

The other option, is to set an option of allow csrf origins. This is how sails handles this. https://github.com/balderdashy/sails-docs/blob/master/reference/sails.config/sails.config.csrf.md

findesk commented 9 years ago

@tomsteele

I think 25 suits.

I can't think of a scenario where one would set CSRF origins (like sails allows) different to CORS origins. So in Hapi, CORS origins should guard token leakage to foreign domains. (I'd hope!)

(Apologies for not replying sooner.)

lock[bot] commented 4 years ago

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.