CSRF gets reset too often causing race condition in browser

[roberth]

When CSRF is enabled, servant-auth-server will set the cookie on every response. The following will happen in the browser with concurrent requests:

1. Request A is performed by the browser with token=t1
2. Request B is constructed in JavaScript with token=t1
3. Response A is received by the browser, sets the cookie to token=t2
4. Request B gets sent by the browser with t1 in the XSRF header and t2 in the Cookie header
5. Request B gets rejected even though it is legitimate

This may be more or less of a problem depending on the JavaScript technology used. The context switching by GHCJS probably makes this even more likely than otherwise, but Angular may also suffer from this race condition.

[3noch]

Exactly! I suspected this but failed to report it. Thanks for the detail. In the meantime I disable XSRF and check the Origin/Referer headers in my fork #54.

[alpmestan]

@jkarni This still appears to be a problem (mebassett_ just mentionned this on IRC), do we have any plans to address it?

[mebassett]

aside from @3noch 's PR, we had thought about addressing it by including an option for CSRF Source. Setting that source to, say, Random would leave the implementation as is. Setting it to JWTHash would calculate the XSRF cookie as a hash of the JWT cookie. see https://security.stackexchange.com/questions/115766/is-a-jwt-usable-as-a-csrf-token

any thoughts on this approach ?

[saevarb]

What is the proper solution to this? We're constantly seeing this bug in our webapp which is soon being deployed into production. It can be triggered by simply navigating too fast.

[mebassett]

@saevarb in the absence of some direction from @3noch @jkarni we will likely implement the solution I describe above in a couple weeks and create a PR for it. We would welcome other eyes looking at it. I don’t feel we should be writing our own auth code. We are also going into production 20 Apr.

There are a few other prs on this matter, do any of those help you?

[saevarb]

@mebassett Thanks for the swift response :)

I looked around at some of the PRs and mostly decided that I would rather avoid depending on a fork. Instead, I upped the versions of the packages so I could use xsrfExcludeGet on CookieSettings, which seems to have fixed the problems for the most part, since a majority of our requests are GET requests.

I'd definitely be interested in helping with/looking at solutions to this. I think your solution sounds very sensible. Is there any reason at all to want the current solution? If both solutions are equally secure, I see no reason to keep one solution around if it tends to lead to race conditions.

[roberth]

@mebassett if the JWT value is not sufficiently unique, its hash can be guessed by the attacker.
Another downside is that the token can only be reset when the JWT changes (which may never happen in some apps). The same race condition can occur when the JWT changes.

The OWASP site has good recommendations.

• What we seem to have now is Double Submit Cookie. Their reference implementation is not suited for concurrent requests either.
• The next best option seems to be Encrypted Token Pattern. This requires some effort to implement, because we will need a concept of 'user id', which is not necessarily the same as whatever is in the JWT or what is in a in Auth auths a. I'm quite confident though that there will always be a pure function from a to 'user id'. It seems that this can be implemented without race conditions, because the verification only depends on the user id, (server-side) encryption key and timestamps that are independent of concurrent requests. Omitting the timestamps opens up the possibility of replay attacks, although the adversary seems to have to acquire a token first.
• Check Origin and Referer headers

I think we need to find a suitable reference implementation of the encrypted token pattern, because we should not design our own authentication.

[3noch]

My fork has the third option implemented (check origin and referer), but I've never had time to write tests for it. If anyone wants to try that I'd be very thankful.

[3noch]

FWIW, I think the trouble with Double-Submit is that it's designed for traditional page-by-page apps (no AJAX). With AJAX-heavy apps or SPA, you need to use something else.

[roberth]

@3noch It depends on which Double-Submit.

It's probably fine to reset it only on login/logout. I've finally found a reasonably compelling source to confirm this. If you have an XSS vulnerability, a 'single-submit' token (what we have now) doesn't seem to help that much. It complicates the exploit by a few lines, but it does not seem to be a real barrier.

Also, this presentation (look for defenses) has some hints for strengthening double-submit. Some of this is easy to add and doesn't introduce a bunch of configuration hassle/brittleness.

What I think we should strive for is to have most/all the options available and let the developer pick what works well for them and have easy to use but sufficiently secure defaults.

[3noch]

It's probably fine to reset it only on login/logout.

I recall seeing this option used for SPA. This would be great to have.

[roberth]

This branch seems to do the trick. acceptLogin still sets the token. Servant-auth does not have a concept of log out, so that one is for the user to implement.