Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rethink cookie upgrades #73

Open
aidantwoods opened this issue Apr 23, 2018 · 2 comments
Open

Rethink cookie upgrades #73

aidantwoods opened this issue Apr 23, 2018 · 2 comments

Comments

@aidantwoods
Copy link
Owner

aidantwoods commented Apr 23, 2018

In future versions we should rework how cookie upgrades are handled.

Sites which care about security related headers being reliably delivered should already be using HTTPS on all their pages – which means that HTTP cookies shouldn't be a thing. Adding Secure to all cookies seems like a pretty damn easy win. (this would replace our current approach which is only slightly better than a whitelist). For cookies that must be sent over HTTP, we can permit cookies to be manually deselected from this upgrade.

I think this approach may be valuable for all settings though: it adopts a "permissions" view of the world where everything is disallowed by default, and the developer must actively opt-in to less protection. i.e. instead of having to remember to think about "we must disallow JavaScript from accessing a cookie", developers should instead think "we should grant JavaScript access to this cookie".

My current thinking is the following:

  • All cookies receive the Secure flag by default.

  • All cookies receive the httpOnly flag by default.

  • All cookies receive the SameSite=Lax flag by default.

  • Allow manual exceptions to be written for all of the above based on cookie name, i.e.

    • Explicit opt-in for cookies to be allowed to be sent over HTTP

    • Explicit opt-in for JavaScript to be allowed access to a cookie

    • Explicit opt-in for cookies to be allowed to be sent when not using top-level cross-origin navigations

  • Allow specific cookies to be marked as "write cookies", which will upgrade the SameSite setting to SameSite=Strict. These are cookies which (if your site can be built to allow for it) should be reserved for granting write permissions to a particular user. SameSite=Lax cookies are perfectly okay for where the user need only read information.

    There is a section in the SameSite cookie spec detailing a scenario where lax would be used for read permissions, and strict would be used for write permissions. Scott Helme has also done a nice explainer on using different cookies for read/write permissions in CSRF is dead.

    If you're developing a new app – this is something you should definitely look into in addition to current CRSF mechanisms (when all browsers have added support for SameSite it might be possible to ditch CSRF form tokens, but that is way in the future!).

@franzliedke
Copy link
Contributor

This sounds very sensible. 👍

How would you plan the upgrade path? IMO, there should be another release in version 2 that allows an opt-in to the new default behavior.

@aidantwoods
Copy link
Owner Author

Thanks :)

How would you plan the upgrade path? IMO, there should be another release in version 2 that allows an opt-in to the new default behavior.

I've marked this under the Version 3 milestone since that'll be the first place we can do this by default, but we can certainly add this as opt-in behaviour in a subsequent version 2 release too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants