Defense-In-Depth Post-Login CSRF Support

[madaster97]

Describe the problem you'd like to have solved

This SDK has one post-login CSRF protection method (SameSite=Lax cookies discussed here), but would benefit from additional methods.

I've been using this SDK for prototyping, and only recently realized I had a post-login CSRF attack. For reasons most servers I work with need Same=None; Secure in their cookies, and so lose the default CSRF protection from this library.

Describe the ideal solution

At the very least, I'd like to add a warning when someone sets the appSession cookie to SameSite=None, reminding them to also add CSRF protection.

Taking that a step further, I'd like this library to offer additional CSRF protections, and potentially enforce them (with an override) when users set SameSite=None.

CSRF Token in AppSession

I've been pouring over the OWASP CSRF Cheat Sheet, and I think a reasonable approach that would be easy for this library would be the Synchronizer Token Pattern, specifically per-session (instead of per-request) tokens.

In per-session token implementations after the initial generation of a token, the value is stored in the session and is used for each subsequent request until the session expires.

Implementing this could look like the following:

• A system for creating + storing per-session CSRF tokens in the appSession cookie. We'd rotate this token every time a new session ID is set
• A new request or response method for retrieving the CSRF token from the session. You'd store this in your rendered HTML forms (as a hidden form field) or in the DOM for use in your AJAX requests.
• A new config option called csrfCheck, which would be a function expected to return the CSRF token supplied in the request. The function would have the request headers and body (assumed x-www-form-urlencoded) from the incoming request as inputs, since limiting to these mitigates the risk of insecure requests that can leak the token (via query params).

The csrfCheck could be used in a few different ways, such as:

1. Providing a new middleware called requiresCsrfToken (similar to requiresAuth) that will check requests for the token, and make sure it matches what's in the session
2. Having a sensible default, where this SDK automatically checks all insecure requests (POST, PUT, PATCH, and DELETE)

Alternatives and current work-arounds

As a workaround, you can use another CSRF library from npm. If you're using SameSite=None, you NEED to remember this!

Additional information, if any

The cheat sheet mentions that custom HTTP request headers are preferred, but notably that only works for AJAX requests:

Inserting the CSRF token in the custom HTTP request header via JavaScript is considered more secure than adding the token in the hidden field form parameter because it uses custom request headers.

Side-note, since the csurf library was deprecated last month, I feel like many people are being hung out to dry with regards to CSRF.

Minimal Example

TODO: I have a really complex example here, but need to trim it down

[adamjmcgrath]

Hi @madaster97 - thanks for your recommendations

At the very least, I'd like to add a warning when someone sets the appSession cookie to SameSite=None, reminding them to also add CSRF protection.

We're not keen on adding a runtime warning mainly just because we don't want to add a warning for users that don't need to add CSRF protection (because they've added it already for example). Happy to add more docs if you want to suggest a PR.

Taking that a step further, I'd like this library to offer additional CSRF protections, and potentially enforce them (with an override) when users set SameSite=None.

CSRF protection is a separate concern to login and we encourage you to use one of the available express middleware libraries if you want to implement it

[madaster97]

Hey @adamjmcgrath , I'd like to talk about this more:

CSRF protection is a separate concern to login and we encourage you to use one of the available express middleware libraries if you want to implement it

For a server that is using this SDK as their entire session management system, do you think it makes sense to include a CSRF token in their appSession? I wrote up an example here and documentation here.

[adamjmcgrath]

Hey @madaster97 - sure, and thanks for sharing your code and docs example.

We've considered bundling CSRF middleware along with our session middleware, but decided against it - generally the express solution is to use different middleware for specific jobs, cors, sec headers, csrf, serssion etc, for example the main session library (express-session) wouldn't bundle in csrf

Also, if you look at express's solution to csrf https://github.com/expressjs/csurf - you'll see this is a hard thing to guarantee, the "npm module is currently deprecated due to the large influx of security vulunerability reports received, most of which are simply exploiting the underlying limitations of CSRF itself." - informing users they can add state changes to their GETs because we provide CSRF protection would be problematic. We think it's better to set some sensible/secure defaults and leave adding extra security measures to the application

[madaster97]

Yeah that makes sense. Thanks for talking it over