On dev-platform, Henri suggested: https://groups.google.com/d/msg/mozilla.dev.platform/xaGffxAM-hs/aVgYuS3QA2MJ I think we should start by, by default, forgetting all cookies that don't have the "secure" flag set at the end of the Firefox session. Persistent cookies have two main use cases: * On login-requiring sites, not requiring the user to have to re-enter credentials in every browser session. * Behavioral profiling. The first has a clear user-facing benefit. The second is something that users typically don't want and breaking it has no obvious user-visible effect of breaking Web compat of the browser. Fortunately, the most-used login-requiring sites use https already, so forgetting insecure cookies at the end of the session would have no adverse effect on the most-user-visible use of persistent cookies. Also, if a login-requiring site is not already using https, it's pretty non-controversial that they are Doing It Wrong and should migrate to https. So I think withholding cookie persistence from insecure cookies could well be way more effective per unit of disruption of user-perceived Web compat than anything in your proposal. ... Note that I didn't propose dropping support for insecure cookies right away. I proposed forgetting (by default) insecure cookies when quitting Firefox. At least at the start, it would probably make sense not to forget cookies from sites that the users has put in the explicit "Allow" category in the cookie manager. AFAICT, this can't "Break the Web" for the usual definition of that phrase, since the forgetting behavior wouldn't be site-detectable in mid-browsing. It would affect the UX on non-https login-requiring sites (including ones whose login is https but whose session cookie is insecure to allow everything except supposedly sensitive things like sending the password or sending a credit card number happen over http due to legacy performance memes), which are, as noted, Doing It Wrong.

Richard, Martin, what kind of dependency markings should this bug have to put it on the radar for larger insecure HTTP deprecation plan? I think we could easily *start* with this bug without risking breaking the Web.

There we go. There are sure to be plenty of concrete actions like this that we can identify. Many of them will need coordination with other browser vendors. My view is that we can make this call without a great deal of coordination for the reasons Henri cited. We should look to identify other persistent features here. I'm inclined to clear all site state for http:// origins, but each will need its own analysis.

Note that Firefox session restore persists session cookies across browser restarts so "session" cookies can live a lot longer than people think. This is "eternal session" bug 530594.

I implemented this feature. Unfortunately, I don't think it's practical yet. According to our COOKIE_SCHEME_SECURITY telemetry [1], only about 6% of cookies include the Secure flag. On my machine, only 2% of 4000+ cookies are secure. Clearing the 98% non-secure cookies logs me out of most websites. Even Google, who serves HTTPS nearly everywhere, only set 24 of the 111 Google cookies on my machine as secure and those secure cookies are not enough to keep me logged into my Google accounts. Having to log back into all these websites every session would be very annoying. The COOKIE_SCHEME_SECURITY telemetry shows about 30% of non-secure cookies are set from an HTTPS origin. Perhaps we could allow those non-secure HTTPS cookies to be persisted, hoping those sites will continue to be serve them over HTTPS, even though they didn't set the Secure flag. [1] http://mzl.la/20WWhoK

(In reply to Chris Peterson [:cpeterson] from comment #0) > On dev-platform, Henri suggested: > > https://groups.google.com/d/msg/mozilla.dev.platform/xaGffxAM-hs/aVgYuS3QA2MJ > > I think we should start by, by default, forgetting all cookies that don't > have the "secure" flag set at the end of the Firefox session. Persistent > cookies have two main use cases: > > * On login-requiring sites, not requiring the user to have to re-enter > credentials in every browser session. > * Behavioral profiling. I believe there is a third use case. Not a month ago, a colleague of mine implemented with cookies client-side storage that should have been implemented localStorage (saving some favorite items on the client side). The server didn't set any cookies. Setting document.cookies does not seem to set the Secure flag by default (as one could expect?). Caught it and on its way to being fixed but I wonder how much that practice is still a thing. If it still is, dropping the cookies would result in a poorer user experience. > The COOKIE_SCHEME_SECURITY telemetry shows about 30% of non-secure cookies are set from an HTTPS origin. Perhaps we could allow those non-secure HTTPS cookies to be persisted, hoping those sites will continue to be serve them over HTTPS, even though they didn't set the Secure flag. Does this option fix the Google problem? Given the amount of nonsecure/http, it looks like only dropping these would already be a decent step forward (assuming web compat).

(In reply to David Bruant from comment #5) > > The COOKIE_SCHEME_SECURITY telemetry shows about 30% of non-secure cookies are set from an HTTPS origin. Perhaps we could allow those non-secure HTTPS cookies to be persisted, hoping those sites will continue to be serve them over HTTPS, even though they didn't set the Secure flag. > > Does this option fix the Google problem? Yes. This fixes the logout problem for the sites I tested: Google, Twitter, Wikipedia, Hacker News. > Given the amount of nonsecure/http, it looks like only dropping these would > already be a decent step forward (assuming web compat). As an incremental improvement, we could tag cookies set over HTTPS without an explicit Secure flag as "Secure so far". If we ever send a "Secure so far" cookie with a non-HTTPS request, then we downgrade it to a non-secure session cookie. This suggestion might require changing our cookie database schema so Secure is a tri-state value: No, Yes, and So Far.

Chris, why try so hard? You could just make two cookies, one that is upgraded and persistent and another that is ephemeral. Or mark the persistent cookies as "save this as secure".

My new proposal doesn't even need to check the Secure cookie flag: 1. If a new cookie is set by a non-HTTPS response, then make it a session cookie. 2. If a currently-persisted cookie is submitted with a non-HTTPS request, then downgrade it to a session cookie. So any cookie set or sent over HTTP will be downgraded to a session cookie. We don't need to change the cookie database schema. We don't need to treat legacy persisted cookies any differently than HTTPS cookies persisted from a previous session. Any cookie on disk will continue to be persisted, and assumed HTTPS-only, until it is sent with a non-HTTPS request, at which point it will be downgraded to a session cookie.

I have a WIP patch for this feature.

Part 1: Treat cookies set over non-secure HTTP as session cookies. Henri suggested (comment 1) treating all cookies without the `Secure` flag as session cookies. However, my testing shows (comment 4) that far too few cookies set the `Secure` flag for this to be practical. Instead, this patch implements part of what I described in comment 8: treat cookies set over HTTP, instead of HTTPS, as session cookies. I have not yet implemented downgrading of non-session cookies to session cookies if they are ever sent over HTTP. Updating the cookie stores in memory makes that trickier. I want to see if this first approach is practical before we start "sessionizing" even more cookies. Here is a comparison of the number of normal (non-session) and session cookies set when I open the Alexa Top 25 News sites. Firefox's Session Restore only seems to restore session cookies for sites that still have open tabs. So if I close all 25 tabs and restart Firefox, all of the session cookies are deleted, even when Session Restore is enabled. Thus "sessionizing" non-HTTPS cookies will cause Firefox to forget (after browser restart) more cookies than blocking all third-party cookies or enabling Basic Tracking Protection, but without causing the login problems associated with those features. Test Normal Session Total ==== ====== ======= ===== Default Firefox settings 1100 1801 1281 No third-party cookies 332 100 432 Sessionize non-HTTPS cookies 189 1117 1306 <-- This patch Sessionize non-`Secure` cookies 7 1153 1160 Tracking Protection (Basic) 246 70 316 Tracking Protection (Strict) 166 55 221 uBlock Origin 128 69 197

Comment on attachment 8737685 [details] [diff] [review] patch-1-sessionize-HTTP-cookies.patch Review of attachment 8737685 [details] [diff] [review]: ----------------------------------------------------------------- ::: modules/libpref/init/all.js @@ +1890,5 @@ > pref("network.cookie.thirdparty.sessionOnly", false); > pref("network.cookie.lifetimePolicy", 0); // 0-accept, 1-dontUse 2-acceptForSession, 3-acceptForNDays > pref("network.cookie.prefsMigrated", false); > pref("network.cookie.lifetime.days", 90); // Ignored unless network.cookie.lifetimePolicy is 3. > +pref("network.cookie.lifetime.httpSessionOnly", true); btw, I don't really like this pref name, so suggestions are welcome. It's pretty awkward, but it mirrors the existing "network.cookie.lifetime.days" and "network.cookie.thirdparty.sessionOnly" prefs. ::: netwerk/cookie/nsCookieService.cpp @@ +703,5 @@ > > nsCookieService::nsCookieService() > : mDBState(nullptr) > , mCookieBehavior(nsICookieService::BEHAVIOR_ACCEPT) > + , mHTTPSessionOnly(true) I don't really like the `mHTTPSessionOnly` variable name. It's a little vague, but it mirrors the `mThirdPartySession` variable name (below) and adds `Only` from the pref name.

Part 2: Clean up some cruft in nsCookie.h from PRBool/bool conversions. Also make nsCookie's member variables private.

Part 3: Change some cookie tests to use HTTPS URLs so their cookies won't be sessionized.

Part 4: Add tests for sessionization of cookies set over non-secure HTTP. Also note that some test cookies' expiration dates will be ignored.

Part 5: Clean up some confusing browser_storage_listings.js checks. Also fix a reference error about undeclared variable i. I fixed these test nits when updating the cookies tests to work with HTTP session cookies, but these changes are not really related to or necessary for HTTP session cookies.

> Persistent cookies have two main use cases: > * On login-requiring sites, not requiring the user to have to re-enter credentials in every browser session. > * Behavioral profiling. > The first has a clear user-facing benefit. The second ... no There are many other legitimate use cases, for example Preferences: * Changing search engine settings like "Literal search only" or similar. * Regional redirection: I go to google.com, it redirects me to google.de. I click on the "I really want it" link "Go to google.com". Google stores this as preference and no longer redirects me to google.de. The same can be true inverse. * I intentionally do not log into Google, because I don't want them to connect my searches to an account. I clear cookies sometimes, to break the profiling. Cookies are a way to store user data without a login. That's a great thing from a privacy perspective.

Comment on attachment 8737688 [details] [diff] [review] patch-4-add-cookie-sessionization-tests.patch Review of attachment 8737688 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/server/tests/browser/storage-listings.html @@ +14,5 @@ > "use strict"; > const partialHostname = location.hostname.match(/^[^.]+(\..*)$/)[1]; > + > +// These cookie expiration times will be ignored because these cookies will be > +// set on nonsecure HTTP connection. I'd be tempted to disable the preference for this test or else serve it over HTTPS, just so we don't lose test coverage of non-session cookie expiration times. ::: extensions/cookie/test/unit/test_cookies_http_session.js @@ +2,5 @@ > + http://creativecommons.org/publicdomain/zero/1.0/ */ > + > +// test third party persistence across sessions, for the cases: > +// 1) network.cookie.thirdparty.sessionOnly = false > +// 2) network.cookie.thirdparty.sessionOnly = true This comment should be updated.

I really appreciate how the patches were split up for review!

We should probably have an intent to ship message for dev.platform about this change, too.

Thanks, Josh. I'll follow up on dev.platform.

Comment on attachment 8737685 [details] [diff] [review] patch-1-sessionize-HTTP-cookies.patch Review of attachment 8737685 [details] [diff] [review]: ----------------------------------------------------------------- I really like this patch, so it's weird to write a f- comment for it. But I think its going to break some auto-login use cases people are dependent on. The behavior is going to feel inconsistent to users and firefox will become more annoying. Ultimately potentially costing us users. At the recent IETF Mike West expressed interest in exploring this strategy for Chrome (because we all like this strategy!) - deploying it together would be a good mitigation to my concern. But I would be very wary of leading the way here. I have no objections to landing it pref'd off to prepare for potential coordination.

We don't need to set "affected" branch status flags for enhancements unless we've decided to start tracking fixing the bug in a particular release (and tracking for prior releases would be unusual for an enhancement). This isn't a "regression" we need to monitor. I have to +1 Patrick's f-: we don't have the marketshare to waste pissing off users who have to log in all the time on our browser and not others, and not the marketshare to convince the affected sites to switch to https because of just this issue if they weren't already planning to do it. There are several other cookie improvements I'd offer first before going this route (some on the near-term SecEng roadmap), and save this particular change for later.

(In reply to Patrick McManus [:mcmanus] from comment #21) > But I think its going to break some auto-login use cases people are > dependent on. The behavior is going to feel inconsistent to users and > firefox will become more annoying. Ultimately potentially costing us users. I don't disagree; there are people who do that, and they'd be confused. That said, do we have any metrics on how common staying logged into a site forever is? Hard data would be nice, even though it's unlikely to change this specific decision by itself.

Thanks for the feedback. I anticipated this proposal might not be practical with real world sites, so it's better to halt it here. :) I would still like to land two of these patches that have some minor code cleanups. I will ask for f+ on those patches.

Comment on attachment 8737686 [details] [diff] [review] patch-2-clean-up-nsCookie.patch Review of attachment 8737686 [details] [diff] [review]: ----------------------------------------------------------------- Patrick, I'd like to land this code cleanup patch, even if we're not taking my session cookie patch. This patch cleans up some cruft in nsCookie.h from PRBool/bool conversions and makes nsCookie's member variables private.

Comment on attachment 8737690 [details] [diff] [review] patch-5-clean-up-cookie-tests.patch Review of attachment 8737690 [details] [diff] [review]: ----------------------------------------------------------------- Patrick, I'd like to land this test cleanup patch, even if we're not taking my session cookie patch. This patch cleans up some confusing browser_storage_listings.js checks and fixes a JavaScript reference error about undeclared variable i.

Landing the whole feature, but prefed off as suggested above sounds best to me. It's not a complex feature and it's something we will eventually want; just no idea when that "eventually" will be.

I see what you mean, but I'm reluctant to land a feature that has no clear path to shipping. We would pay a maintenance code for having the code in mozilla-central and if we ever tried to ship it, the actual shape of the feature would probably need to change. Existing users would be annoyed.

(In reply to Chris Peterson [:cpeterson] from comment #28) > I see what you mean, but I'm reluctant to land a feature that has no clear > path to shipping. We do have a path to shipping: work with another browser vendor to restrict HTTP cookies in that way. The Security Engineering team has talked about various things we'd like to see to tighten up cookie handling. Your patch (disabled by default) is a good fit for that effort IMHO.

Comment on attachment 8737686 [details] [diff] [review] patch-2-clean-up-nsCookie.patch Review of attachment 8737686 [details] [diff] [review]: ----------------------------------------------------------------- this would be good to have

https://hg.mozilla.org/integration/mozilla-inbound/rev/3558dbf6f998 https://hg.mozilla.org/integration/mozilla-inbound/rev/3135bc1a480a

I landed the test cleanup patches, but I don't plan to land the feature patches as disabled. I will look into implementing this feature for non-secure third-party cookies. If it seems practical, I'll file a new bug because I have a slightly different approach in mind. In the meantime, I'm going to resolve this bug as incomplete.

https://hg.mozilla.org/mozilla-central/rev/3558dbf6f998 https://hg.mozilla.org/mozilla-central/rev/3135bc1a480a

I am still interested in experimenting with this insecure session cookie feature behind a pref. This could encourage ad networks to move to HTTPS so they don't lose their user cookies. Insecure third-party ads have been a blocker for some websites moving their own first-party content to HTTPS.

This patch adds a flag to treat third-party cookies set over nonsecure HTTP as session cookies. "Nonsecure HTTP" here just means regular, not-HTTPS HTTP. It doesn't mean HTTPS without the `Secure` cookie flag. This patch honors the expiration time of third-party cookies set over HTTPS, whether or not they have the `Secure` cookie flag. If a third-party cookie is set over HTTPS and then later sent in nonsecure HTTP request (which is allowed for cookies without the `Secure` cookie flag), the cookie won't be turned into a session cookie unless the nonsecure HTTP response sets a new cookie value. In my testing of many websites, 72% (891 of 1232) first-party Set-Cookie calls were from HTTPS, but only 44% (9701 of 22,2284) third-party Set-Cookie calls were from HTTPS. Only about 10% of Set-Cookie calls include the `Secure` cookie flag.

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Josh, now that Nightly 57 has wrapped up, do you have time for this review?

Comment on attachment 8892290 [details] Bug 1160368 - Part 2: Collect telemetry on how often first- and third-party cookies are set from HTTPS origins. https://reviewboard.mozilla.org/r/163230/#review194496 This will need a data review as well.

Comment on attachment 8892289 [details] Bug 1160368 - Part 1: Add flag to treat third-party cookies set over nonsecure HTTP as session cookies. https://reviewboard.mozilla.org/r/163228/#review194514

Comment on attachment 8892290 [details] Bug 1160368 - Part 2: Collect telemetry on how often first- and third-party cookies are set from HTTPS origins. https://reviewboard.mozilla.org/r/163230/#review194542 datareview+ ::: toolkit/components/telemetry/Histograms.json:10318 (Diff revision 1) > "releaseChannelCollection": "opt-out", > "description": "Strict Secure Cookies: 0=blocked secure cookie from http; 1=blocked shadowing secure cookie from http; 2=shadowing secure cookie from https; 3=evicted newer insecure cookie; 4=evicted oldest insecure cookie; 5=evicted secure cookie; 6=evicting secure cookie blocked; 7=blocked downgrading secure cookie from http; 8=downgraded secure cookie from https" > }, > + "COOKIE_SCHEME_HTTPS": { > + "record_in_processes": ["main", "content"], > + "alert_emails": ["seceng-telemetry@mozilla.com"], Having a team email in there is fine, but we now ask for the email of the person responsible for the probe to be present as well.

(In reply to François Marier [:francois] from comment #43) > Having a team email in there is fine, but we now ask for the email of the > person responsible for the probe to be present as well. Thanks! I will fix the alert email addresses before landing.

(Since the feature is disabled by default for now, it won't break anything in the wild.)

Pushed by cpeterson@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/2fc7552a16ec Part 1: Add flag to treat third-party cookies set over nonsecure HTTP as session cookies. r=jdm https://hg.mozilla.org/integration/mozilla-inbound/rev/dabc52f35ddb Part 2: Collect telemetry on how often first- and third-party cookies are set from HTTPS origins. r=jdm data-review=francois

https://hg.mozilla.org/mozilla-central/rev/2fc7552a16ec https://hg.mozilla.org/mozilla-central/rev/dabc52f35ddb

btw, here is a snapshot of the COOKIE_HTTPS_SECURE telemetry from Firefox Beta 58 users: First-party HTTP = 2.88B ( 2.37%) First-party HTTP + Secure?! = 7.25M ( 0.01%) Firefox ignores these bogus cookies. First-party HTTPS = 12.55B (10.30%) First-party HTTPS + Secure = 4.28B ( 3.51%) Third-party HTTP = 28.26B (23.18%) Third-party HTTP + Secure?! = 85.98M ( 0.07%) Firefox ignores these bogus cookies. Third-party HTTPS = 61.33B (50.30%) Third-party HTTPS + Secure = 12.51B (10.26%) Thus: * 16.18% of all cookies are first-party and 85% of them use HTTPS. * 83.74% of all cookies are third-party and 72% of them use HTTPS. This bug added functionality to treat non-HTTPS third-party cookies as session cookies. It would only "sessionize" 28% of third-party cookies. https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2017-12-04&keys=__none__!__none__!__none__&max_channel_version=beta%252F58&measure=COOKIE_SCHEME_HTTPS&min_channel_version=null&processType=*&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2017-11-15&table=0&trim=1&use_submission_date=0

Chris, there is a pref 'network.cookie.thirdparty.sessionOnly' which when true, sets all third party cookies as session only (IDK about sessionized ones, so ignore that part). How does this pref 'network.cookie.thirdparty.nonsecureSessionOnly' work with that one. Which one overrides if they are different? If .sessionOnly=true but .nonsecureSessionOnly= false, then the HTTP cookie is... [enter answer here] If .sessionOnly=false but .nonsecureSessionOnly= true, then the HTTP cookie is... [enter answer here] please advise, thanks

(In reply to Simon Mainey from comment #49) > Chris, there is a pref 'network.cookie.thirdparty.sessionOnly' which when > true, sets all third party cookies as session only (IDK about sessionized > ones, so ignore that part). How does this pref > 'network.cookie.thirdparty.nonsecureSessionOnly' work with that one. Which > one overrides if they are different? sessionOnly is a superset of nonsecureSessionOnly and overrides it, so both the HTTP cookies in both examples would be session-only. > If .sessionOnly=true but .nonsecureSessionOnly= false, then the HTTP cookie > is... session-only. > If .sessionOnly=false but .nonsecureSessionOnly= true, then the HTTP cookie > is... session-only.

(In reply to Chris Peterson [:cpeterson] from comment #50) > sessionOnly is a superset of nonsecureSessionOnly and overrides it, so both > the HTTP cookies in both examples would be session-only. > > > If .sessionOnly=false but .nonsecureSessionOnly= true, then the HTTP cookie > > is... > > session-only. If .sessionOnly (false) always overrides .nonsecureSessionOnly (true), as per the statement above, then there would be no need for the new pref (and the above answer would be not forced to session-only)? I'll just take it as being "most-restrictive"? 1. SO= true + nsSO= true --> all cookies = SO 2. SO= true + nsSO=false --> all cookies = SO 3. SO=false + nsSO=false --> all cookies = nonSO 4. SO=false + nsSO= true --> HTTP cookie = SO (allows users to session HTTP cookies and non-session HTTPS cookies) - which is what this pref was all about Right?

(In reply to Simon Mainey from comment #51) > If .sessionOnly (false) always overrides .nonsecureSessionOnly (true), as > per the statement above, then there would be no need for the new pref (and > the above answer would be not forced to session-only)? > > I'll just take it as being "most-restrictive"? Yes. That's a better way of saying it. > 1. SO= true + nsSO= true --> all cookies = SO > 2. SO= true + nsSO=false --> all cookies = SO > 3. SO=false + nsSO=false --> all cookies = nonSO > 4. SO=false + nsSO= true --> HTTP cookie = SO (allows users to session HTTP > cookies and non-session HTTPS cookies) - which is what this pref was all > about Yes. That is correct.

^^ Thanks Chris :)

cmills, this feature is pref'd off by default. It might be used for some user experiments, but I don't know when or if it might ever be enabled. Do you still think it warrants dev-doc-needed?

(In reply to Chris Peterson [:cpeterson] from comment #54) > cmills, this feature is pref'd off by default. It might be used for some > user experiments, but I don't know when or if it might ever be enabled. Do > you still think it warrants dev-doc-needed? Probably not ... removing ;-)