Discussion: securecookie tags

[jeremyn]

Type: other

I want to revisit how we use securecookie tags. I generally dislike these tags. They are hard to reason about, and the most common use with a general .+/.+ can break website functionality. See my comments at #7527 (comment) and #11475 (comment).

I think securecookies should rarely be used. From my comment at #2907 (comment):

1. securecookie tags should only be used in targeted situations where we are trying to fix a identified vulnerability, instead of used as a default catch-all, but
2. we don't need to make special efforts to remove existing securecookies, and
3. casual contributors should not be encouraged to create them.

The underlying theme here is that I prefer to be cautious in rulesets, because it is generally difficult for a regular user to effectively report problems and get them fixed.

However, I've noticed an increase in the number of PRs that include securecookies. I sometimes avoid these PRs because I don't want to have the securecookie argument again. However if the community generally feels that we should be more free with securecookies then I will stop worrying about it as much.

Ping @brainwane @gloomy-ghost @Hainish @J0WI @wonderchook .

[bardiharborow]

I agree with your interpretation in #7527 (comment), but per #13177 (comment) I'm still very unclear how ^\w rules are treated. If you are able to clarify this, it would help inform my ruleset work. I generally only create securecookie rules if all the subdomains seem to support HTTPS but maybe I need to be even more cautious.

[Bisaloo]

However, I've noticed an increase in the number of PRs that include securecookies.

My guess is that this is due to the arrival of new contributors. When I first started out, I read that most cookies were missing the secure flag and that we should add it whenever possible.

I don't recall any warning about the issues associated with it. It's only later, when I saw bug reports about it, that I understood it was a dangerous tool.

My take on this is that we should add a warning in CONTRIBUTING.md using your comments.

One question: With HSTS preloaded domains, do cookies get only accepted from secure domains? If that's the case then emulating their behavior shouldn't be problematic it appears.

[bardiharborow]

With HSTS preloaded domains, do cookies get only accepted from secure domains?

@ihave100openPRs A HSTS domain with the includeSubDomains flag will never be accessed over an insecure connection under any circumstances, so any cookies set for that domain will always be secure. Non-includeSubDomains domains may leak cookies to their unsecured subdomains unless those cookies are marked as Secure.

emulating their behavior shouldn't be problematic it appears

HTTP-Everywhere rulesets with specified subdomains (as opposed to wildcard entries) operate like non-includeSubDomains domains by default, leaking cookies to their unsecured subdomains. securecookie rules plug this hole. The issue discussed by this ticket, is that in many cases this leakage is actually required for the website to function correctly, and in many cases we are too haphazard in applying securecookie rules such that we break websites.

[jeremyn]

@bardiharborow I don't know what's going on in the code, but my instinct for ^\w would be to replace it with .+ as a matter of style. If I saw it in a new ruleset, I would probably ask the contributor to replace it with .+ without thinking about it too much. If there is some significant behavior difference between ^\w and .+, that should be clarified, and we shouldn't be relying on it.

Also, even if all subdomains for some domain currently support HTTPS, new ones can be added later that don't. There are also sometimes many domains that aren't publicly visible at for example Google, so unless you are exhaustively searching with something like Sublist3r, you might be missing some. (I've started to prefer Google to Sublist3r myself, since I find Sublist3r often presents too many domains that are not worth covering.)

@ihave100openPRs A problem with relying on HSTS preloading and things like that is that it varies by domain and requires careful thought each time. Look at the discussion that @J0WI and I have in PR #11475. I don't want to have that sort of discussion for every PR that has a general securecookie and some domains that don't support HTTPS.

---

By the way, just to argue against my own position, in practice it is very rare to see a problem caused by a securecookie. It may be that the increased security of blanket securecookies outweighs these rare usability problems. In that case though, maybe we should make the code "autoinclude" a .+/.+ securecookie, and instead rely on contributors to exclude or override this if there is a problem. Or, we could autoinclude a securecookie that is strictly limited to the given targets.

[Bisaloo]

By the way, just to argue against my own position, in practice it is very rare to see a problem caused by a securecookie.

The flickr neverending login issue was enough for me: #239, #728, #1592

I think the correct procedure would be to require the PR submitter to manually test whether the site's functionality isn't affected due to problems with cookies, and to remove that tag if he can't/doesn't want to test it (of course this shouldn't be required for static sites or ones where problems with securecookie won't arise), and not that securecookie itself should be discouraged as much as possible (and as noted instances of breakage due to securecookie are very rare).

[Bisaloo]

@ihave100openPRs, I agree with this. This is also the rule I follow in my PR.

I still think this should be documented in CONTRIBUTING.md.

[jeremyn]

It is almost impossible to test cookies thoroughly. Suppose you see a cookie set by a.example.com with name lasksjddflkasda and value 2093080948320483208403. What is it supposed to do on b.example.com? You probably don't know. As a maintainer who is supposed to at least theoretically be able to double-check you, I definitely don't know.

(Also, contributors should be manually testing their rulesets before submitting them anyway.)

[Hainish]

This problem is probably exacerbated by our example here:

https-everywhere/CONTRIBUTING.md

Lines 266 to 272 in c7da7b7

	For `securecookie` tags, if you know that all cookies on the included targets can be secured (which in particular means that the cookies are not used by any of its non-securable subdomains), use the trivial
	```xml
	<securecookie host=".+" name=".+" />
	```
	where we prefer `.+` over `.*` and `.`. They are functionally equivalent, but it's nice to be consistent.

This is a clarification on regex styling, but we perhaps don't draw out some of the difficulties with ensuring that a cookie won't be used by an insecure subdomain.

I've looked to our git history in order to determine the original inclusion the securecookie tag, since looking for where it first solved a real problem should illuminate its ideal use case. It was introduced in 43b1536, where we add it to the Facebook.xml file:

diff --git a/src/chrome/content/rules/Facebook.xml b/src/chrome/content/rules/Facebook.xml
index eee23f1..3a9f053 100644
--- a/src/chrome/content/rules/Facebook.xml
+++ b/src/chrome/content/rules/Facebook.xml
@@ -1,10 +1,16 @@
 <ruleset name="Facebook">
-  <target host="www.facebook.com" />
   <target host="facebook.com" />
-  <target host="m.facebook.com" />
+  <target host="*.facebook.com" />
   <target host="www.v6.facebook.com" />
 
+  <securecookie host=".*\.facebook.com" name="c_user" />
+  <securecookie host=".*\.facebook.com" name="lu" />
+  <securecookie host=".*\.facebook.com" name="sct" />
+  <securecookie host=".*\.facebook.com" name="xs" />
+
   <rule from="^http://(www\.)?facebook\.com/" to="https://www.facebook.com/"/>
   <rule from="^http://m\.facebook\.com/" to="https://m.facebook.com/"/>
+  <rule from="^http://ssl\.facebook\.com/" to="https://ssl.facebook.com/"/>
+  <rule from="^http://login\.facebook\.com/" to="https://login.facebook.com/"/>
   <rule from="^http://www\.v6\.facebook\.com/" to="https://www.v6.facebook.com/"/>
 </ruleset>

In this context, it makes sense. We were preventing Firesheep-style attacks from leaking login sessions. See how we're using it here - we have specific name attributes enumerated, so I presume we were fairly certain how these cookies were being used.

Since then, we've liberalized how we use this tag. Effectively, using it with a wildcard (either ^\w or .+, effectively the same in this context) causes cookies set on the eTLD+1 to be siloed by protocol. At worst, this can cause problems where insecure cookies are presumed to be set in a secure environment, then used in other contexts, as @jeremyn has described previously in this thread.

---

Creating a hard-and-fast rule for the securecookies flag seems to draw a relatively arbitrary line in the sand. If the above Facebook.xml rule, instead of enumerating all cookies, simply set all cookies to be secure, would that pass for an acceptable rule? It would protect users, but may impede usability. This tradeoff between usability and security would be particular to each site. In an attack scenario, where Facebook login sessions were being stolen in real time, that tradeoff would, I argue, be worth it. But how are we to know if it's worth it for any given site?

In the creation of rulesets, in general, we (perhaps ironically) air on the side of usability rather than security. See our discussion about when to disable rulesets. Any new ruleset is checked for broken functionality before it is merged. This can be a pretty tricky process, but that's another issue. It seems to me that usability should be the guide here, too. We don't want HTTPS Everywhere to impede functionality of a site.

I think if a wildcard securecookie tag is included, we could require the submitter to justify why. Anyone submitting a rule should be able to justify why all cookies should be secure for this entire eTLD+1, such as asserting that there is sensitive data behind an HTTPS-only logged-in portal. This justification doesn't need to pass some high bar, but just having it referenceable (perhaps requiring a comment above the securecookie tag line) may be enough. Down the line, if the securecookie does start causing problems, at least then we can have a reasonable reference point to weigh whether it's worth removal.

[jeremyn]

@Hainish The Facebook example is consistent with my item (1) in the first comment: "securecookie tags should only be used in targeted situations where we are trying to fix a identified vulnerability".

Facebook is not a typical example because the site is super high visibility, and you can probably count on at least some of the maintainers having Facebook accounts. A more typical example is a top-100k site in a language I don't know, with some invalid domains listed, maybe with a forum or a shop or something else that you can expect is especially sensitive to cookies but that can't be completely tested without making an account or buying something.

An advantage of a hard-and-fast rule is that we don't have to argue about it after we make the rule. Requiring contributors to justify the tag each time is not going to work. People will say too much or not enough. It will be an argument. They will need repeated reminding. For security reasons we shouldn't trust things we can't test anyways.

Regarding usability vs. security, in a discussion last year about mixed content blocking (MCB) (issue #6629), we collectively agreed to be permissive with MCB, which supports security over usability. So I don't think we favor one or the other in all cases.

[Hainish]

@jeremyn

Regarding usability vs. security, in a discussion last year about mixed content blocking (MCB) (issue #6629), we collectively agreed to be permissive with MCB, which supports security over usability. So I don't think we favor one or the other in all cases.

We agreed to allow forcing users into mixed content blocking as long as it doesn't impede usability, as defined specifically under the phrase "feature breakage": #6629 (comment). I do view this as favoring usability over security. Ultimately, if a site is very secure but breaks features of the website, usability is the deciding factor.

---

An advantage of a hard-and-fast rule is that we don't have to argue about it after we make the rule. Requiring contributors to justify the tag each time is not going to work. People will say too much or not enough. It will be an argument. They will need repeated reminding. For security reasons we shouldn't trust things we can't test anyways.

Requiring a justification is a hard-and-fast rule, in my opinion. Within that context, we can instruct contributors that we favor excluding the securecookie tag. Unless a user feels particularly compelled to include it, they'll probably be willing to work with you and simply remove it. But if inclusion of a wildcard securecookie is important, we should give them a mechanism to include it.

[Hainish]

On my point:

Requiring a justification is a hard-and-fast rule, in my opinion. Within that context, we can instruct contributors that we favor excluding the securecookie tag. Unless a user feels particularly compelled to include it, they'll probably be willing to work with you and simply remove it. But if inclusion of a wildcard securecookie is important, we should give them a mechanism to include it.

If this is a policy we decide on, we should definitely include it in CONTRIBUTING.md.

[jeremyn]

Requiring a justification may be a hard-and-fast rule to you. As a maintainer it means hundreds of individual conversations about why

	<!-- insecure cookies present -->
	<securecookie host=".+" name=".+" />

on the one hand or

	<!-- [insert several paragraphs here] -->
	<securecookie host=".+" name=".+" />

on the other is or is not appropriate. A hard-and-fast rule to me means a red X or a green check in Travis.

[J0WI]

I feel like we already discussed this in #2907. Maybe GitHub issues are the wrong place for such decisions, because it's quite hard to find them once they have been closed/resolved.
#1584 and #6964 may also be worth to look at.