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

Secure Flag cannot be set for unproxied localhost #837

Open
shartte opened this issue Jul 30, 2021 · 8 comments
Open

Secure Flag cannot be set for unproxied localhost #837

shartte opened this issue Jul 30, 2021 · 8 comments

Comments

@shartte
Copy link

shartte commented Jul 30, 2021

Browsers consider localhost to be a secure origin (i.e. see https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#restrict_access_to_cookies).

express-session does not. This means for my local configuration, I need to disable the Secure flag, which has other repercussions:

  • I cannot set the Cookie Name to __Host-sid and need to make a special-case development exception
  • I cannot use CORS Requests to localhost with Cookies, since non-secure Cookies aren't sent, even with SameSite=none

I understand that this is the result of express-session trying to be smart and trying to prevent sending cookies over insecure connections.

The code causing this is in index.js:

if (req.session.cookie.secure && !issecure(req, trustProxy)) {

I don't think that issecure can reliably detect if the connection attempt is secure or not for the localhost case.

I see two possible solutions:

  • Allow disabling the issecure check and unconditionally set the cookie (this is what I actually want, because otherwise it just silently fails)
  • Potentially inspect the origin-header for localhost as a heuristic, this (should?) remain unchanged through proxies too
@dougwilson
Copy link
Contributor

I have never heard of being able to set Secure attribute and it work if your site is http://localhost (no https). Is that Firefox-specific (those are Firefox docs) or does that work in other web browsers? Is it part of the HTTP session spec somewhere?

@dougwilson
Copy link
Contributor

Ok, I did a bit of digging and looks like this is a relatively recent change in the web browsers. Firefox implemented this first, and then Chrome I think added it earlier this year. They state that the cookie spec leaves the definition for a secure context up to the user agent (them), not specifically the scheme.

Originally this bit if code was added in order to adhere to the cookie spec security guide for the server to not send secure cookies over a non secure context when possible. But with the recent change in interpretation of the spec by the web browsers, it is not realistic to understand what they consider secure contexts. I will see what the http specs are saying these days regarding servers sending secure cookies over non secure contexts and if that is a worry or not still.

@shartte
Copy link
Author

shartte commented Jul 30, 2021

I think what browser generally recognized is that TLS on localhost doesn't really add any security (since no public CA will ever issue a cert for localhost anyhow).

It'd be fine to disable the check using a flag and keep it enabled by default if you still have concerns about this. I think accessing apps via localhost is usually a development-only activity, but it'd allow me to test '__Host-' header/CORS locally.

@dougwilson
Copy link
Contributor

I mean, if you just wanted a flag, there already is one: this modulr simply honors the req.secure flag on the request. Just defining it as true will have the cookie sent. Adding a flag in the constructor here only helps this one module; you may just end up in the same situation with other cookie setting in the app.

I would say if all you want is to add a flag, that shouldn't be necessary; just have req.secure return true for those (or all) requests and you're good (it's already the flag).

@shartte
Copy link
Author

shartte commented Jul 30, 2021

I tried doing that, actually. It failed because secure is read-only (TypeError: Cannot set property secure of #<IncomingMessage> which has only a getter). I know I might have worked around that by trying to hack together a proxy object if that's possible, but honestly was too lazy.

I am also somewhat hesitant because some other middleware might try to use secure to decide whether a redirect-url should be https or not, and https://localhost isn't gonna work so great :-|

@dougwilson
Copy link
Contributor

Yes, you have to use Object.defineProperty as it is a getter (just how Javascript works for getters). I can understand that. So it sounds like then just wait for me to research the current state of the specs to see what the need is for server side checking. I don't want to quickly add a switch when it turns out checking is not needed, then need to go though depreciation cycles to relove it, etc. Just a bit of checking up front will keep this module's API options frlm growing to an even bigger spaghetti than it already is 😂

@Axedyson

This comment was marked as spam.

@timbotnik
Copy link

timbotnik commented Nov 12, 2021

Looks like Chromium and Firefox both allow usage of secure cookies on localhost:

While Safari still does not and looks to be considered a bug:

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

No branches or pull requests

4 participants