search

EFForg / https-everywhere

A browser extension that encrypts your communications with many websites that offer HTTPS but still allow unencrypted connections.
https://eff.org/https-everywhere
Other
3.37k stars 1.09k forks source link

cookie changed to "secure" even if connection was NOT https #1584

Closed dan42 closed 5 years ago

dan42 commented 9 years ago

According to https://www.eff.org/https-everywhere/rulesets#secure-cookies the <securecookie> tag should only set the secure flag on a cookie if it was sent over HTTPS. However if you go to http://fast.animenewsnetwork.com/ the session cookie's secure flag is set. This is in direct contradiction of the docs.

ghost commented 9 years ago

My bad. Apparently the subdomain sets a session cookie originating from ".animenewsnetwork.com", which is secured in the ruleset I've pushed.

Nevertheless, the subdomain you mentioned and all others which I can find seem securable too. It should be for the better to redirect those to HTTPS instead of commenting out the cookie ruleset, presuming no functionality is lost of course.

dan42 commented 9 years ago

Thanks for improving the ruleset... but it's not what my bug report was about. Honestly I think the original ruleset didn't really have a problem; the problem is with the fact that https-everywhere changes the cookie to "secure" even if it was not sent over a secure connection. Not only is that in contradiction of the docs, it also makes no sense; a cookie set by a http://whatever.com will never be sent back on future visits to http://whatever.com if it's marked secure; it's completely useless.

This is a bug with https-everywhere, not with this specific rule.

More specifically, a few of our subdomains do not support HTTPS. For example http://www.d.animenewsnetwork.com/ is meant to bypass Cloudflare (because sometimes it's wonky) and therefore doesn't have HTTPS. But the session cookie is still set with domain=.animenewsnetwork.com and therefore is converted to "secure". Which makes it impossible to be logged in at that url.

I very much appreciate the fact that Anime News Network has been added to https-everywhere. An easy fix would be to remove the <securecookie> tag in this ruleset, but the real fix is to set the secure flag only for cookies sent over a HTTPS connection.

ghost commented 9 years ago

Ah, I've misunderstood you a bit first because of your previous title. Well, some additional coverage isn't bad neither, right? :)

You've got a point though, I can confirm this strange behaviour with FireFox. Clearly the fast subdomain shouldn't be triggered with the current ruleset. You can definitely notice the ruleset isn't listed on the menu when visiting the subdomain, as it should be. But appearently it still triggers the securecookie rule for animenewsnetwork.com, overriding the listed target hosts rules. The ruleset shouldn't have been loaded at all.

Hopefully someone of the development team can give some more clarification...

dan42 commented 9 years ago

It's worth noting this is a problem with both the Firefox ands Chrome plugins.

jsha commented 9 years ago

@dan42: I wasn't able to reproduce on Firefox. Can you provide detailed repro instructions?

dan42 commented 9 years ago
  1. go to http://www.d.animenewsnetwork.com/
  2. try to login

Even though the login "works" (no error message), you will not be logged in because the session cookie is flagged as secure even though the url is not HTTPS.

jsha commented 9 years ago

Which cookie is the one you find is marked secure?

dan42 commented 9 years ago

ann5_session_id

jsha commented 9 years ago

Also, I'm having trouble signing up for an account (email hasn't arrived yet). If you can send me a test login (jsha@eff.org) that would be very helpful.

dan42 commented 9 years ago

Ok I sent you a test account

jsha commented 9 years ago

Great, thanks! I can reproduce now. I'll look into it.

ghost commented 9 years ago

I can definitely reproduce it too. When I go to http://www.d.animenewsnetwork.com/, which shouldn't be included in the ruleset, it still secures the session cookie. Somehow it still makes use of the ruleset...

It's like the sercure cookie rules are treated seperately, rather than first taking into account if the site itself was triggered by a target rule.

dan42 commented 9 years ago

If this bug cannot be resolved in time for the next release, please remove the <securecookie> tag in this ruleset. Thank you.

ghost commented 9 years ago

All right, I've disabled the securecookie rule for (.)animenewsnetwork.com in my pull request. It still secures the other cookies though, but that shouldn't give any problems.

jsha commented 9 years ago

Sounds good. I still want to try and figure out why HTTPS Everywhere is securing a cookie sent over HTTP, but I've merged @DeathlyX's change.

gloomy-ghost commented 7 years ago

Does https://github.com/EFForg/https-everywhere/commit/0fcd111fff64e5dd25ed30d6fab4c7606f89297d resolve the issue?