Bug 1623313 - disable cookie sameSite=laxByDefault for a list of hosts by pref, r=mayhemer
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 24 Mar 2020 16:08:36 +0000
changeset 520230 82d5ba25c1a8311f4252b1df52e216152e2f3158
parent 520229 dbabf2e388fa750b4901df8dfab60d8a17a0c5ff
child 520231 17611bdb12f105a1fa843f5165a32c41426c9888
push id37246
push useropoprus@mozilla.com
push dateWed, 25 Mar 2020 03:40:33 +0000
treeherdermozilla-central@14b59d4adc95 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmayhemer
bugs1623313
milestone76.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1623313 - disable cookie sameSite=laxByDefault for a list of hosts by pref, r=mayhemer Differential Revision: https://phabricator.services.mozilla.com/D67305
modules/libpref/init/all.js
netwerk/cookie/nsCookieService.cpp
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -1993,16 +1993,23 @@ pref("network.proxy.no_proxies_on",     
 pref("network.proxy.allow_hijacking_localhost", false);
 pref("network.proxy.failover_timeout",      1800); // 30 minutes
 pref("network.online",                      true); //online/offline
 
 // The interval in seconds to move the cookies in the child process.
 // Set to 0 to disable moving the cookies.
 pref("network.cookie.move.interval_sec",    10);
 
+// This pref contains the list of hostnames (such as
+// "mozilla.org,example.net"). For these hosts, firefox will treat
+// sameSite=none if nothing else is specified, even if
+// network.cookie.sameSite.laxByDefault if set to true.
+// To know the correct syntax, see nsContentUtils::IsURIInList()
+pref("network.cookie.sameSite.laxByDefault.disabledHosts", "");
+
 pref("network.cookie.maxNumber", 3000);
 pref("network.cookie.maxPerHost", 180);
 // Cookies quota for each host. If cookies exceed the limit maxPerHost,
 // (maxPerHost - quotaPerHost) cookies will be evicted.
 pref("network.cookie.quotaPerHost", 150);
 
 // The PAC file to load.  Ignored unless network.proxy.type is 2.
 pref("network.proxy.autoconfig_url", "");
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -492,33 +492,33 @@ class CompareCookiesByIndex {
 
     return a.index < b.index;
   }
 };
 
 // Return false if the cookie should be ignored for the current channel.
 bool ProcessSameSiteCookieForForeignRequest(nsIChannel* aChannel,
                                             nsCookie* aCookie,
-                                            bool aIsSafeTopLevelNav) {
+                                            bool aIsSafeTopLevelNav,
+                                            bool aLaxByDefault) {
   int32_t sameSiteAttr = 0;
   aCookie->GetSameSite(&sameSiteAttr);
 
   // it if's a cross origin request and the cookie is same site only (strict)
   // don't send it
   if (sameSiteAttr == nsICookie::SAMESITE_STRICT) {
     return false;
   }
 
   int64_t currentTimeInUsec = PR_Now();
 
   // 2 minutes of tolerance for 'sameSite=lax by default' for cookies set
   // without a sameSite value when used for unsafe http methods.
   if (StaticPrefs::network_cookie_sameSite_laxPlusPOST_timeout() > 0 &&
-      StaticPrefs::network_cookie_sameSite_laxByDefault() &&
-      sameSiteAttr == nsICookie::SAMESITE_LAX &&
+      aLaxByDefault && sameSiteAttr == nsICookie::SAMESITE_LAX &&
       aCookie->RawSameSite() == nsICookie::SAMESITE_NONE &&
       currentTimeInUsec - aCookie->CreationTime() <=
           (StaticPrefs::network_cookie_sameSite_laxPlusPOST_timeout() *
            PR_USEC_PER_SEC) &&
       !NS_IsSafeMethodNav(aChannel)) {
     return true;
   }
 
@@ -3071,29 +3071,35 @@ void nsCookieService::GetCookiesForURI(
   bool stale = false;
 
   nsCookieKey key(baseDomain, aOriginAttrs);
 
   // perform the hash lookup
   nsCookieEntry* entry = mDBState->hostTable.GetEntry(key);
   if (!entry) return;
 
+  bool laxByDefault =
+      StaticPrefs::network_cookie_sameSite_laxByDefault() &&
+      !nsContentUtils::IsURIInPrefList(
+          aHostURI, "network.cookie.sameSite.laxByDefault.disabledHosts");
+
   // iterate the cookies!
   const nsCookieEntry::ArrayType& cookies = entry->GetCookies();
   for (nsCookieEntry::IndexType i = 0; i < cookies.Length(); ++i) {
     cookie = cookies[i];
 
     // check the host, since the base domain lookup is conservative.
     if (!DomainMatches(cookie, hostFromURI)) continue;
 
     // if the cookie is secure and the host scheme isn't, we can't send it
     if (cookie->IsSecure() && !potentiallyTurstworthy) continue;
 
-    if (aIsSameSiteForeign && !ProcessSameSiteCookieForForeignRequest(
-                                  aChannel, cookie, aIsSafeTopLevelNav)) {
+    if (aIsSameSiteForeign &&
+        !ProcessSameSiteCookieForForeignRequest(
+            aChannel, cookie, aIsSafeTopLevelNav, laxByDefault)) {
       continue;
     }
 
     // if the cookie is httpOnly and it's not going directly to the HTTP
     // connection, don't send it
     if (cookie->IsHttpOnly() && !aHttpBound) continue;
 
     // if the nsIURI path doesn't match the cookie path, don't send it back
@@ -3769,17 +3775,22 @@ bool nsCookieService::ParseAttributes(ns
   aCookieHeader.BeginReading(cookieStart);
   aCookieHeader.EndReading(cookieEnd);
 
   aCookieData.isSecure() = false;
   aCookieData.isHttpOnly() = false;
   aCookieData.sameSite() = nsICookie::SAMESITE_NONE;
   aCookieData.rawSameSite() = nsICookie::SAMESITE_NONE;
 
-  if (StaticPrefs::network_cookie_sameSite_laxByDefault()) {
+  bool laxByDefault =
+      StaticPrefs::network_cookie_sameSite_laxByDefault() &&
+      !nsContentUtils::IsURIInPrefList(
+          aHostURI, "network.cookie.sameSite.laxByDefault.disabledHosts");
+
+  if (laxByDefault) {
     aCookieData.sameSite() = nsICookie::SAMESITE_LAX;
   }
 
   nsDependentCSubstring tokenString(cookieStart, cookieStart);
   nsDependentCSubstring tokenValue(cookieStart, cookieStart);
   bool newCookie, equalsFound;
 
   // extract cookie <NAME> & <VALUE> (first attribute), and copy the strings.
@@ -3857,17 +3868,17 @@ bool nsCookieService::ParseAttributes(ns
 
   // re-assign aCookieHeader, in case we need to process another cookie
   aCookieHeader.Assign(Substring(cookieStart, cookieEnd));
 
   // If same-site is set to 'none' but this is not a secure context, let's abort
   // the parsing.
   if (!aCookieData.isSecure() &&
       aCookieData.sameSite() == nsICookie::SAMESITE_NONE) {
-    if (StaticPrefs::network_cookie_sameSite_laxByDefault() &&
+    if (laxByDefault &&
         StaticPrefs::network_cookie_sameSite_noneRequiresSecure()) {
       LogMessageToConsole(aChannel, aHostURI, nsIScriptError::infoFlag,
                           CONSOLE_SAMESITE_CATEGORY,
                           NS_LITERAL_CSTRING("CookieRejectedNonRequiresSecure"),
                           aCookieData.name());
       return newCookie;
     }
 
@@ -3876,17 +3887,17 @@ bool nsCookieService::ParseAttributes(ns
         aChannel, aHostURI, nsIScriptError::warningFlag,
         CONSOLE_SAMESITE_CATEGORY,
         NS_LITERAL_CSTRING("CookieRejectedNonRequiresSecureForBeta"),
         aCookieData.name(), SAMESITE_MDN_URL);
   }
 
   if (aCookieData.rawSameSite() == nsICookie::SAMESITE_NONE &&
       aCookieData.sameSite() == nsICookie::SAMESITE_LAX) {
-    if (StaticPrefs::network_cookie_sameSite_laxByDefault()) {
+    if (laxByDefault) {
       LogMessageToConsole(aChannel, aHostURI, nsIScriptError::infoFlag,
                           CONSOLE_SAMESITE_CATEGORY,
                           NS_LITERAL_CSTRING("CookieLaxForced"),
                           aCookieData.name());
     } else {
       LogMessageToConsole(aChannel, aHostURI, nsIScriptError::warningFlag,
                           CONSOLE_SAMESITE_CATEGORY,
                           NS_LITERAL_CSTRING("CookieLaxForcedForBeta"),