support samesite xsrf cookie

[minrk]

A somewhat more recent approach to CSRF is setting samesite=strict on a cookie. SameSite is handy because it allows the browser to enforce the same-site-ness of a request, without needing application code to send the token separately. SameSite can be used almost everywhere, but not quite. It would be handy to support this strategy of csrf checking (cookie is present and samesite=strict) as equivalent to the current check of token delivery by both cookie and argument.

If I wanted to adopt this strategy now, do you suppose it would be preferable to:

1. create a new samesite cookie for this purpose and implement my own handling, or
2. modify tornado's xsrf handling to attempt to apply samesite restrictions on the cookie

?

Unfortunately, only Python 3.8 has support for setting samesite cookies in the stdlib. This is easily patched (Morsel._reserved['samesite'] = 'SameSite'), but needing a patch is never great.

[bdarnell]

Samesite cookies are new to me, but here are my initial thoughts:

If you send a cookie with samesite=strict to a browser that doesn't support it, the attribute gets silently ignored, right? And there's no way to tell on the server side whether the samesite flag is supported/in use (except for user-agent sniffing). So there's not really a good way to use samesite where available and fall back to the old way.

If you can eliminate the legacy fallback, things get a lot simpler: you can avoid all the xsrf_form_html stuff. In fact, I don't know if you need any explicit XSRF protection at all: just mark your login cookie as samesite, and then you're done. So I think if you're willing to rely on samesite being available, you can just disable tornado's xsrf protection and use samesite instead.

If you're not willing to give up on users of older browsers that don't support samesite, is it worthwhile to support two modes so you can use samesite where available? I'm not sure that it is - if you're not willing to rely solely on samesite I'd probably just stick with old-style XSRF protection. (You can set samesite in xsrf_cookie_kwargs for a little defense-in-depth, but you still need the template html and other parts of application support).

[tugceakin]

This will cause issues after Chrome 80 upgrade in cases where tornado server is running inside a cross origin iframe.

A cookie associated with a cross-site resource at was set without the SameSite attribute. It has been blocked, as Chrome now only delivers cookies with cross-site requests if they are set with SameSite=None and Secure. You can review cookies in developer tools under Application>Storage>Cookies and see more details at https://www.chromestatus.com/feature/5088147346030592 and https://www.chromestatus.com/feature/5633521622188032.

[bdarnell]

If your application requires the use of the SameSite attribute, I think you'll need to either upgrade to python 3.8 or apply the Morsel monkey-patch yourself. I don't think it would be appropriate for Tornado to do this kind of monkey-patch.

If SameSite cookies are becoming more important, consider lobbying python-dev to backport this change to older versions of python, or see if someone will create and maintain a copy of http.cookies as a standalone package (similar to what was once done with unittest2 and backports_abc).

It's also possible for tornado to move away from http.cookies entirely if there is an alternative cookie package available (or as a last resort reimplement the relevant RFCs), But that's a much larger change and wouldn't happen quickly.

[tugceakin]

Thanks Ben. We ended up monkey-patching our student virtual environments at Udacity. Upgrading to 3.8 was not an immediate option, because some of our courses are taught in Python 3.6 which requires a larger scale of content updates.

[jbvsmo]

Or maybe Tornado should just implement its own Morsel class since this is simple and defined on a RFC. This class could also be extensible to support future names instead of how Python handles it. There is nothing special, to be honest, in this class that would require it to enforce a small collection of names

[bdarnell]

There is nothing special, to be honest, in this class that would require it to enforce a small collection of names

I think there are good reasons to enforce that names are on a small list - it helps catch mistakes earlier, and it can handle special formatting requirements (notice that there is special per-field handling of integer arguments to the expires and max-age fields, and the secure and httponly settings are special because they're not encoded as a key-value pair.

The problem is that the web platform evolves at its own pace and it doesn't make sense for these limits to be baked into slow-moving language releases. There must either be a way to say "trust me, i want to use this flag that's not on the list" or the list must be updated more frequently even if this looks like "adding features to old releases".

Moving away from the standard library's cookie module makes sense, but somebody has to do the work to make that happen. And given the non-trivial security risks of anything touching cookie parsing, I wouldn't count on that being ready for production usage by the time the chrome policy change goes into effect, so monkey-patching Morsel is likely to be the most expedient approach.

[jbvsmo]

Thanks @bdarnell, this is actually a good argument. Well, I had monkey patched the std lib anyway but would be good if this can go through some roadmap to be part of tornado at some point.

[stalkerg]

@bdarnell I suppose we can do same as Bottle guys did
bottlepy/bottle#983
or at least we should notify about it in the documentation.

[enolfc]

#2911 is related to this. While one can set the SameSite when using py3.8, the cookies are not cleared unless this is also specified.

[bdarnell]

Since this issue was opened, Chrome and Edge have made SameSite=lax the default. (Firefox has planned a similar change although I don't know when it might happen)

I've also talked to a couple of security researchers about SameSite and modern XSRF protection, and come to the following conclusions:

• Tornado's xsrf_token is an implementation of the double-submit pattern which offers equivalent protection to that of SameSite cookies (in either lax or strict mode)
• The important thing is to apply SameSite to the cookies used for authentication. Using SameSite on the _xsrf cookie itself is nice but it doesn't really improve security.
• Both SameSite and the double-submit pattern are vulnerable to "same-site request forgeries" (or related-domain attacks).
• Best practice for defense against same-site attacks involves the use of stateful synchronizer tokens, which require more invasive changes to the application; we cannot implement this with an interface like xsrf_token.
• The double-submit pattern combined with the _Host- cookie prefix may provide protection against same-site attacks, and this could be a simple change to the current xsrf_token feature. However, there are apparently some browser bugs in the implementation of cookie prefixes that prevent this from being bulletproof protection today.
• Even without _Host- prefixes, the current implementation of xsrf_token may be useful in situations where the primary authentication mechanism is not a cookie (e.g. if it's based on a network address or a TLS certificate).

So now we can either deprecate xsrf_token in favor of just using SameSite on all auth cookies (which simplifies things while preserving the same security properties, or we can start using _Host- cookie prefixes and offer multiple levels of protection (samesite for "basic" protection, xsrf_token for a higher level of security, or a hypothetical synchronizer token and associated application changes for the highest level). Given the awkwardness of using xsrf_token in a modern JS-heavy application and doubts about the strength of _Host- prefixes, does it make sense to invest in this option or is it better to just focus on samesite for basic protection and synchronizer tokens for more advanced usage?

[bdarnell]

I've created PR #3226 to deprecate xsrf_cookies. I've added the ability to rename the xsrf cookie with the __Host- prefix as an experiment. If it turns out to be helpful for security without hampering usability, I may revisit the deprecation and bring it back, but as things stand I do not expect to fix the open issues around this feature (relating to expiration and resetting the cookie).