Bug 1160368 - Part 1: Add flag to treat third-party cookies set over nonsecure HTTP as session cookies. r?jdm draft
authorChris Peterson <cpeterson@mozilla.com>
Thu, 16 Feb 2017 19:27:49 -0800
changeset 618799 ef440380ea69770a6632a0f6fe9ab46a140df372
parent 618757 44121dbcac6a9d3ff18ed087a09b3205e5a04db1
child 618800 fd4b243ee0fac0d45210c339f4185366c6e0a81f
push id71464
push usercpeterson@mozilla.com
push dateTue, 01 Aug 2017 04:52:02 +0000
reviewersjdm
bugs1160368
milestone56.0a1
Bug 1160368 - Part 1: Add flag to treat third-party cookies set over nonsecure HTTP as session cookies. r?jdm "Nonsecure HTTP" here just means regular, not-HTTPS HTTP. It doesn't mean HTTPS without the `Secure` cookie flag. Honor 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 news websites, 72% (891 of 1232) first-party cookies were set of HTTPS, but only 44% (9701 of 22,2284) third-party cookies were set of HTTPS. This feature is controlled by the pref "network.cookie.thirdparty.nonsecureSessionOnly". MozReview-Commit-ID: HlCg21JyvNC
extensions/cookie/test/unit/test_cookies_thirdparty_nonsecure_session.js
extensions/cookie/test/unit/xpcshell.ini
modules/libpref/init/all.js
netwerk/cookie/nsCookieService.cpp
netwerk/cookie/nsCookieService.h
copy from extensions/cookie/test/unit/test_cookies_thirdparty_session.js
copy to extensions/cookie/test/unit/test_cookies_thirdparty_nonsecure_session.js
--- a/extensions/cookie/test/unit/test_cookies_thirdparty_session.js
+++ b/extensions/cookie/test/unit/test_cookies_thirdparty_nonsecure_session.js
@@ -1,14 +1,16 @@
 /* Any copyright is dedicated to the Public Domain.
    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
+// test nonsecure third party persistence across sessions, for the cases:
+// 1) network.cookie.thirdparty.nonsecureSessionOnly = false
+// 2) network.cookie.thirdparty.nonsecureSessionOnly = true
+
+"use strict";
 
 var test_generator = do_run_test();
 
 function run_test() {
   do_test_pending();
   test_generator.next();
 }
 
@@ -21,49 +23,49 @@ function finish_test() {
 
 function* do_run_test() {
   // Set up a profile.
   let profile = do_get_profile();
 
   // Create URIs and channels pointing to foo.com and bar.com.
   // We will use these to put foo.com into first and third party contexts.
   var spec1 = "http://foo.com/foo.html";
-  var spec2 = "http://bar.com/bar.html";
+  var spec2 = "https://bar.com/bar.html";
   var uri1 = NetUtil.newURI(spec1);
   var uri2 = NetUtil.newURI(spec2);
   var channel1 = NetUtil.newChannel({uri: uri1, loadUsingSystemPrincipal: true});
   var channel2 = NetUtil.newChannel({uri: uri2, loadUsingSystemPrincipal: true});
 
   // Force the channel URI to be used when determining the originating URI of
   // the channel.
   var httpchannel1 = channel1.QueryInterface(Ci.nsIHttpChannelInternal);
   var httpchannel2 = channel2.QueryInterface(Ci.nsIHttpChannelInternal);
   httpchannel1.forceAllowThirdPartyCookie = true;
   httpchannel2.forceAllowThirdPartyCookie = true;
 
-  // test with cookies enabled, and third party cookies persistent.
+  // test with cookies enabled and nonsecure third party cookies persistent.
   Services.prefs.setIntPref("network.cookie.cookieBehavior", 0);
-  Services.prefs.setBoolPref("network.cookie.thirdparty.sessionOnly", false);
-  do_set_cookies(uri1, channel2, false, [1, 2, 3, 4]);
-  do_set_cookies(uri2, channel1, true, [1, 2, 3, 4]);
+  Services.prefs.setBoolPref("network.cookie.thirdparty.nonsecureSessionOnly", false);
+  do_set_cookies(uri1, channel2, false, [1, 2, 3, 4]); // third-party HTTP
+  do_set_cookies(uri2, channel1, false, [1, 2, 3, 4]); // third-party HTTPS
 
   // fake a profile change
   do_close_profile(test_generator);
   yield;
   do_load_profile();
-  do_check_eq(Services.cookies.countCookiesFromHost(uri1.host), 4);
-  do_check_eq(Services.cookies.countCookiesFromHost(uri2.host), 0);
+  do_check_eq(Services.cookies.countCookiesFromHost(uri1.host), 4); // HTTP cookies OK
+  do_check_eq(Services.cookies.countCookiesFromHost(uri2.host), 4); // HTTPS cookies OK
 
-  // test with third party cookies for session only.
-  Services.prefs.setBoolPref("network.cookie.thirdparty.sessionOnly", true);
+  // test with nonsecure third party cookies for session only.
+  Services.prefs.setBoolPref("network.cookie.thirdparty.nonsecureSessionOnly", true);
   Services.cookies.removeAll();
-  do_set_cookies(uri1, channel2, false, [1, 2, 3, 4]);
-  do_set_cookies(uri2, channel1, true, [1, 2, 3, 4]);
+  do_set_cookies(uri1, channel2, false, [1, 2, 3, 4]); // third-party HTTP
+  do_set_cookies(uri2, channel1, false, [1, 2, 3, 4]); // third-party HTTPS
 
   // fake a profile change
   do_close_profile(test_generator);
   yield;
   do_load_profile();
-  do_check_eq(Services.cookies.countCookiesFromHost(uri1.host), 0);
-  do_check_eq(Services.cookies.countCookiesFromHost(uri2.host), 0);
+  do_check_eq(Services.cookies.countCookiesFromHost(uri1.host), 0); // no HTTP cookies!
+  do_check_eq(Services.cookies.countCookiesFromHost(uri2.host), 4); // HTTPS cookies OK
 
   finish_test();
 }
--- a/extensions/cookie/test/unit/xpcshell.ini
+++ b/extensions/cookie/test/unit/xpcshell.ini
@@ -8,16 +8,17 @@ skip-if = toolkit == 'android'
 [test_cookies_async_failure.js]
 [test_cookies_persistence.js]
 skip-if = true # Bug 863738
 [test_cookies_privatebrowsing.js]
 [test_cookies_profile_close.js]
 [test_cookies_read.js]
 [test_cookies_sync_failure.js]
 [test_cookies_thirdparty.js]
+[test_cookies_thirdparty_nonsecure_session.js]
 [test_cookies_thirdparty_session.js]
 [test_domain_eviction.js]
 [test_eviction.js]
 [test_permmanager_defaults.js]
 [test_permmanager_expiration.js]
 [test_permmanager_getAllForURI.js]
 [test_permmanager_getPermissionObject.js]
 [test_permmanager_notifications.js]
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -2205,16 +2205,17 @@ pref("network.proxy.proxy_over_tls",    
 pref("network.proxy.no_proxies_on",         "localhost, 127.0.0.1");
 pref("network.proxy.failover_timeout",      1800); // 30 minutes
 pref("network.online",                      true); //online/offline
 pref("network.cookie.cookieBehavior",       0); // 0-Accept, 1-dontAcceptForeign, 2-dontAcceptAny, 3-limitForeign
 #ifdef ANDROID
 pref("network.cookie.cookieBehavior",       0); // Keep the old default of accepting all cookies
 #endif
 pref("network.cookie.thirdparty.sessionOnly", false);
+pref("network.cookie.thirdparty.nonsecureSessionOnly", false);
 pref("network.cookie.leave-secure-alone",   true);
 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.
 
 // The PAC file to load.  Ignored unless network.proxy.type is 2.
 pref("network.proxy.autoconfig_url", "");
 // Strip off paths when sending URLs to PAC scripts
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -114,16 +114,17 @@ static const uint32_t kMaxBytesPerCookie
 static const uint32_t kMaxBytesPerPath    = 1024;
 
 // pref string constants
 static const char kPrefCookieBehavior[]       = "network.cookie.cookieBehavior";
 static const char kPrefMaxNumberOfCookies[]   = "network.cookie.maxNumber";
 static const char kPrefMaxCookiesPerHost[]    = "network.cookie.maxPerHost";
 static const char kPrefCookiePurgeAge[]       = "network.cookie.purgeAge";
 static const char kPrefThirdPartySession[]    = "network.cookie.thirdparty.sessionOnly";
+static const char kPrefThirdPartyNonsecureSession[] = "network.cookie.thirdparty.nonsecureSessionOnly";
 static const char kCookieLeaveSecurityAlone[] = "network.cookie.leave-secure-alone";
 
 // For telemetry COOKIE_LEAVE_SECURE_ALONE
 #define BLOCKED_SECURE_SET_FROM_HTTP          0
 #define BLOCKED_DOWNGRADE_SECURE_INEXACT      1
 #define DOWNGRADE_SECURE_FROM_SECURE_INEXACT  2
 #define EVICTED_NEWER_INSECURE                3
 #define EVICTED_OLDEST_COOKIE                 4
@@ -714,16 +715,17 @@ NS_IMPL_ISUPPORTS(nsCookieService,
                   nsIObserver,
                   nsISupportsWeakReference,
                   nsIMemoryReporter)
 
 nsCookieService::nsCookieService()
  : mDBState(nullptr)
  , mCookieBehavior(nsICookieService::BEHAVIOR_ACCEPT)
  , mThirdPartySession(false)
+ , mThirdPartyNonsecureSession(false)
  , mLeaveSecureAlone(true)
  , mMaxNumberOfCookies(kMaxNumberOfCookies)
  , mMaxCookiesPerHost(kMaxCookiesPerHost)
  , mCookiePurgeAge(kCookiePurgeAge)
 {
 }
 
 nsresult
@@ -742,16 +744,17 @@ nsCookieService::Init()
   // init our pref and observer
   nsCOMPtr<nsIPrefBranch> prefBranch = do_GetService(NS_PREFSERVICE_CONTRACTID);
   if (prefBranch) {
     prefBranch->AddObserver(kPrefCookieBehavior,        this, true);
     prefBranch->AddObserver(kPrefMaxNumberOfCookies,    this, true);
     prefBranch->AddObserver(kPrefMaxCookiesPerHost,     this, true);
     prefBranch->AddObserver(kPrefCookiePurgeAge,        this, true);
     prefBranch->AddObserver(kPrefThirdPartySession,     this, true);
+    prefBranch->AddObserver(kPrefThirdPartyNonsecureSession, this, true);
     prefBranch->AddObserver(kCookieLeaveSecurityAlone,  this, true);
     PrefChanged(prefBranch);
   }
 
   mStorageService = do_GetService("@mozilla.org/storage/service;1", &rv);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Init our default, and possibly private DBStates.
@@ -2328,16 +2331,19 @@ nsCookieService::PrefChanged(nsIPrefBran
     mCookiePurgeAge =
       int64_t(LIMIT(val, 0, INT32_MAX, INT32_MAX)) * PR_USEC_PER_SEC;
   }
 
   bool boolval;
   if (NS_SUCCEEDED(aPrefBranch->GetBoolPref(kPrefThirdPartySession, &boolval)))
     mThirdPartySession = boolval;
 
+  if (NS_SUCCEEDED(aPrefBranch->GetBoolPref(kPrefThirdPartyNonsecureSession, &boolval)))
+    mThirdPartyNonsecureSession = boolval;
+
   if (NS_SUCCEEDED(aPrefBranch->GetBoolPref(kCookieLeaveSecurityAlone, &boolval)))
     mLeaveSecureAlone = boolval;
 }
 
 /******************************************************************************
  * nsICookieManager impl:
  * nsICookieManager
  ******************************************************************************/
@@ -4200,34 +4206,42 @@ nsCookieService::CheckPrefs(nsIURI      
   // check default prefs
   if (mCookieBehavior == nsICookieService::BEHAVIOR_REJECT) {
     COOKIE_LOGFAILURE(aCookieHeader ? SET_COOKIE : GET_COOKIE, aHostURI, aCookieHeader, "cookies are disabled");
     return STATUS_REJECTED;
   }
 
   // check if cookie is foreign
   if (aIsForeign) {
-    if (mCookieBehavior == nsICookieService::BEHAVIOR_ACCEPT && mThirdPartySession)
-      return STATUS_ACCEPT_SESSION;
-
     if (mCookieBehavior == nsICookieService::BEHAVIOR_REJECT_FOREIGN) {
       COOKIE_LOGFAILURE(aCookieHeader ? SET_COOKIE : GET_COOKIE, aHostURI, aCookieHeader, "context is third party");
       return STATUS_REJECTED;
     }
 
     if (mCookieBehavior == nsICookieService::BEHAVIOR_LIMIT_FOREIGN) {
       uint32_t priorCookieCount = 0;
       nsAutoCString hostFromURI;
       aHostURI->GetHost(hostFromURI);
       CountCookiesFromHost(hostFromURI, &priorCookieCount);
       if (priorCookieCount == 0) {
         COOKIE_LOGFAILURE(aCookieHeader ? SET_COOKIE : GET_COOKIE, aHostURI, aCookieHeader, "context is third party");
         return STATUS_REJECTED;
       }
-      if (mThirdPartySession)
+    }
+
+    MOZ_ASSERT(mCookieBehavior == nsICookieService::BEHAVIOR_ACCEPT ||
+               mCookieBehavior == nsICookieService::BEHAVIOR_LIMIT_FOREIGN);
+
+    if (mThirdPartySession)
+      return STATUS_ACCEPT_SESSION;
+
+    if (mThirdPartyNonsecureSession) {
+      bool isHTTPS = false;
+      aHostURI->SchemeIs("https", &isHTTPS);
+      if (!isHTTPS)
         return STATUS_ACCEPT_SESSION;
     }
   }
 
   // if nothing has complained, accept cookie
   return STATUS_ACCEPTED;
 }
 
--- a/netwerk/cookie/nsCookieService.h
+++ b/netwerk/cookie/nsCookieService.h
@@ -350,16 +350,17 @@ class nsCookieService final : public nsI
     // want to be dealing with the on-disk DB when in private browsing.
     DBState                      *mDBState;
     RefPtr<DBState>             mDefaultDBState;
     RefPtr<DBState>             mPrivateDBState;
 
     // cached prefs
     uint8_t                       mCookieBehavior; // BEHAVIOR_{ACCEPT, REJECTFOREIGN, REJECT, LIMITFOREIGN}
     bool                          mThirdPartySession;
+    bool                          mThirdPartyNonsecureSession;
     bool                          mLeaveSecureAlone;
     uint16_t                      mMaxNumberOfCookies;
     uint16_t                      mMaxCookiesPerHost;
     int64_t                       mCookiePurgeAge;
 
     // friends!
     friend class DBListenerErrorHandler;
     friend class ReadCookieDBListener;