Bug 1502240 - Ensure that Content Blocking allow list is applied to all cookie policies r=baku
authorEhsan Akhgari <ehsan@mozilla.com>
Fri, 02 Nov 2018 20:14:57 +0000
changeset 444183 bec76d96700206177d42ca13e789519bc7eb313a
parent 444182 591fefb0f99c9d50325fe8e4b151aab7e1ba8691
child 444184 ede2fbe20d1453bfd37cdd887d270015f620869a
push id34985
push usershindli@mozilla.com
push dateSat, 03 Nov 2018 09:40:09 +0000
treeherdermozilla-central@6655fa7cff48 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1502240
milestone65.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 1502240 - Ensure that Content Blocking allow list is applied to all cookie policies r=baku Unfortunately we can't test BEHAVIOR_REJECT using the AntiTracking framework, because the AntiTracking callbacks are incompatible with it. (The tracking callbacks expect to be able to unblock themselves, but under BEHAVIOR_REJECT, that can't happen.) Differential Revision: https://phabricator.services.mozilla.com/D10664
netwerk/cookie/nsCookieService.cpp
toolkit/components/antitracking/AntiTrackingCommon.cpp
toolkit/components/antitracking/test/browser/head.js
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -4278,48 +4278,42 @@ nsCookieService::CheckPrefs(nsICookiePer
   // access to the first-party cookie jar.
   if (aIsForeign && aIsTrackingResource && !aFirstPartyStorageAccessGranted &&
       aCookieBehavior == nsICookieService::BEHAVIOR_REJECT_TRACKER) {
       COOKIE_LOGFAILURE(aCookieHeader ? SET_COOKIE : GET_COOKIE, aHostURI, aCookieHeader, "cookies are disabled in trackers");
       *aRejectedReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_TRACKER;
       return STATUS_REJECTED;
   }
 
-  // check default prefs
-  if (aCookieBehavior == nsICookieService::BEHAVIOR_REJECT) {
+  // check default prefs.
+  // Check aFirstPartyStorageAccessGranted when checking aCookieBehavior
+  // so that we take things such as the content blocking allow list into account.
+  if (aCookieBehavior == nsICookieService::BEHAVIOR_REJECT &&
+      !aFirstPartyStorageAccessGranted) {
     COOKIE_LOGFAILURE(aCookieHeader ? SET_COOKIE : GET_COOKIE, aHostURI, aCookieHeader, "cookies are disabled");
     *aRejectedReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_ALL;
     return STATUS_REJECTED;
   }
 
   // check if cookie is foreign
   if (aIsForeign) {
-    // Check aFirstPartyStorageAccessGranted when rejecting all third-party cookies,
-    // so that we take things such as the content blocking allow list into account.
     if (aCookieBehavior == nsICookieService::BEHAVIOR_REJECT_FOREIGN &&
         !aFirstPartyStorageAccessGranted) {
       COOKIE_LOGFAILURE(aCookieHeader ? SET_COOKIE : GET_COOKIE, aHostURI, aCookieHeader, "context is third party");
       *aRejectedReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_FOREIGN;
       return STATUS_REJECTED;
     }
 
-    if (aCookieBehavior == nsICookieService::BEHAVIOR_LIMIT_FOREIGN) {
-      if (aNumOfCookies == 0) {
-        COOKIE_LOGFAILURE(aCookieHeader ? SET_COOKIE : GET_COOKIE, aHostURI, aCookieHeader, "context is third party");
-        *aRejectedReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_FOREIGN;
-        return STATUS_REJECTED;
-      }
+    if (aCookieBehavior == nsICookieService::BEHAVIOR_LIMIT_FOREIGN &&
+        !aFirstPartyStorageAccessGranted && aNumOfCookies == 0) {
+      COOKIE_LOGFAILURE(aCookieHeader ? SET_COOKIE : GET_COOKIE, aHostURI, aCookieHeader, "context is third party");
+      *aRejectedReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_FOREIGN;
+      return STATUS_REJECTED;
     }
 
-    MOZ_ASSERT(aCookieBehavior == nsICookieService::BEHAVIOR_ACCEPT ||
-               aCookieBehavior == nsICookieService::BEHAVIOR_LIMIT_FOREIGN ||
-               // But with permission granted.
-               aCookieBehavior == nsICookieService::BEHAVIOR_REJECT_FOREIGN ||
-               aCookieBehavior == nsICookieService::BEHAVIOR_REJECT_TRACKER);
-
     if (aThirdPartySession)
       return STATUS_ACCEPT_SESSION;
 
     if (aThirdPartyNonsecureSession) {
       bool isHTTPS = false;
       aHostURI->SchemeIs("https", &isHTTPS);
       if (!isHTTPS)
         return STATUS_ACCEPT_SESSION;
--- a/toolkit/components/antitracking/AntiTrackingCommon.cpp
+++ b/toolkit/components/antitracking/AntiTrackingCommon.cpp
@@ -641,52 +641,45 @@ AntiTrackingCommon::IsFirstPartyStorageA
   }
 
   int32_t behavior = CookiesBehavior(toplevelPrincipal);
   if (behavior == nsICookieService::BEHAVIOR_ACCEPT) {
     LOG(("The cookie behavior pref mandates accepting all cookies!"));
     return true;
   }
 
+  if (CheckContentBlockingAllowList(aWindow)) {
+    return true;
+  }
+
   if (behavior == nsICookieService::BEHAVIOR_REJECT) {
     LOG(("The cookie behavior pref mandates rejecting all cookies!"));
     *aRejectedReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_ALL;
     return false;
   }
 
   // Let's check if this is a 3rd party context.
   if (!nsContentUtils::IsThirdPartyWindowOrChannel(aWindow, nullptr, aURI)) {
     LOG(("Our window isn't a third-party window"));
     return true;
   }
 
-  if (behavior == nsICookieService::BEHAVIOR_REJECT_FOREIGN) {
-    if (CheckContentBlockingAllowList(aWindow)) {
-      LOG(("Allowing access even though our behavior is reject foreign"));
-      return true;
-    }
-  }
-
   if (behavior == nsICookieService::BEHAVIOR_REJECT_FOREIGN ||
       behavior == nsICookieService::BEHAVIOR_LIMIT_FOREIGN) {
     // XXX For non-cookie forms of storage, we handle BEHAVIOR_LIMIT_FOREIGN by
     // simply rejecting the request to use the storage. In the future, if we
     // change the meaning of BEHAVIOR_LIMIT_FOREIGN to be one which makes sense
     // for non-cookie storage types, this may change.
     LOG(("Nothing more to do due to the behavior code %d", int(behavior)));
     *aRejectedReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_FOREIGN;
     return false;
   }
 
   MOZ_ASSERT(behavior == nsICookieService::BEHAVIOR_REJECT_TRACKER);
 
-  if (CheckContentBlockingAllowList(aWindow)) {
-    return true;
-  }
-
   if (!nsContentUtils::IsTrackingResourceWindow(aWindow)) {
     LOG(("Our window isn't a tracking window"));
     return true;
   }
 
   nsCOMPtr<nsIPrincipal> parentPrincipal;
   nsAutoCString trackingOrigin;
   if (!GetParentPrincipalAndTrackingOrigin(nsGlobalWindowInner::Cast(aWindow),
@@ -822,16 +815,20 @@ AntiTrackingCommon::IsFirstPartyStorageA
   }
 
   int32_t behavior = CookiesBehavior(toplevelPrincipal);
   if (behavior == nsICookieService::BEHAVIOR_ACCEPT) {
     LOG(("The cookie behavior pref mandates accepting all cookies!"));
     return true;
   }
 
+  if (CheckContentBlockingAllowList(aChannel)) {
+    return true;
+  }
+
   if (behavior == nsICookieService::BEHAVIOR_REJECT) {
     LOG(("The cookie behavior pref mandates rejecting all cookies!"));
     *aRejectedReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_ALL;
     return false;
   }
 
   nsCOMPtr<mozIThirdPartyUtil> thirdPartyUtil = services::GetThirdPartyUtil();
   if (!thirdPartyUtil) {
@@ -847,40 +844,29 @@ AntiTrackingCommon::IsFirstPartyStorageA
   // Be careful to check the return value of IsThirdPartyChannel, since
   // IsThirdPartyChannel() will fail if the channel's loading principal is the
   // system principal...
   if (NS_SUCCEEDED(rv) && !thirdParty) {
     LOG(("Our channel isn't a third-party channel"));
     return true;
   }
 
-  if (behavior == nsICookieService::BEHAVIOR_REJECT_FOREIGN) {
-    if (CheckContentBlockingAllowList(aChannel)) {
-      LOG(("Allowing access even though our behavior is reject foreign"));
-      return true;
-    }
-  }
-
   if (behavior == nsICookieService::BEHAVIOR_REJECT_FOREIGN ||
       behavior == nsICookieService::BEHAVIOR_LIMIT_FOREIGN) {
     // XXX For non-cookie forms of storage, we handle BEHAVIOR_LIMIT_FOREIGN by
     // simply rejecting the request to use the storage. In the future, if we
     // change the meaning of BEHAVIOR_LIMIT_FOREIGN to be one which makes sense
     // for non-cookie storage types, this may change.
     LOG(("Nothing more to do due to the behavior code %d", int(behavior)));
     *aRejectedReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_FOREIGN;
     return false;
   }
 
   MOZ_ASSERT(behavior == nsICookieService::BEHAVIOR_REJECT_TRACKER);
 
-  if (CheckContentBlockingAllowList(aChannel)) {
-    return true;
-  }
-
   // Not a tracker.
   if (!aChannel->GetIsTrackingResource()) {
     LOG(("Our channel isn't a tracking channel"));
     return true;
   }
 
   nsIPrincipal* parentPrincipal = loadInfo->TopLevelStorageAreaPrincipal();
   if (!parentPrincipal) {
--- a/toolkit/components/antitracking/test/browser/head.js
+++ b/toolkit/components/antitracking/test/browser/head.js
@@ -10,16 +10,17 @@ const TEST_EMBEDDER_PAGE = TEST_DOMAIN +
 const TEST_POPUP_PAGE = TEST_DOMAIN + TEST_PATH + "popup.html";
 const TEST_3RD_PARTY_PAGE = TEST_3RD_PARTY_DOMAIN + TEST_PATH + "3rdParty.html";
 const TEST_3RD_PARTY_PAGE_WO = TEST_3RD_PARTY_DOMAIN + TEST_PATH + "3rdPartyWO.html";
 const TEST_3RD_PARTY_PAGE_UI = TEST_3RD_PARTY_DOMAIN + TEST_PATH + "3rdPartyUI.html";
 const TEST_3RD_PARTY_PAGE_WITH_SVG = TEST_3RD_PARTY_DOMAIN + TEST_PATH + "3rdPartySVG.html";
 const TEST_4TH_PARTY_PAGE = TEST_4TH_PARTY_DOMAIN + TEST_PATH + "3rdParty.html";
 
 const BEHAVIOR_ACCEPT         = Ci.nsICookieService.BEHAVIOR_ACCEPT;
+const BEHAVIOR_LIMIT_FOREIGN  = Ci.nsICookieService.BEHAVIOR_LIMIT_FOREIGN;
 const BEHAVIOR_REJECT_FOREIGN = Ci.nsICookieService.BEHAVIOR_REJECT_FOREIGN;
 const BEHAVIOR_REJECT_TRACKER = Ci.nsICookieService.BEHAVIOR_REJECT_TRACKER;
 
 var gFeatures = undefined;
 
 let {UrlClassifierTestUtils} = ChromeUtils.import("resource://testing-common/UrlClassifierTestUtils.jsm", {});
 
 requestLongerTimeout(5);
@@ -139,16 +140,31 @@ this.AntiTracking = {
           iframeSandbox,
           accessRemoval: null, // only passed with non-blocking callback
           callbackAfterRemoval: null,
         });
         this._createCleanupTask(cleanupFunction);
 
         this._createTask({
           name,
+          cookieBehavior: BEHAVIOR_LIMIT_FOREIGN,
+          blockingByContentBlockingRTUI: true,
+          allowList: true,
+          callback: callbackNonTracking,
+          extraPrefs: [],
+          expectedBlockingNotifications: false,
+          runInPrivateWindow,
+          iframeSandbox,
+          accessRemoval: null, // only passed with non-blocking callback
+          callbackAfterRemoval: null,
+        });
+        this._createCleanupTask(cleanupFunction);
+
+        this._createTask({
+          name,
           cookieBehavior: BEHAVIOR_REJECT_FOREIGN,
           blockingByContentBlockingRTUI: true,
           allowList: true,
           callback: callbackNonTracking,
           extraPrefs: [],
           expectedBlockingNotifications: false,
           runInPrivateWindow,
           iframeSandbox,
@@ -250,17 +266,18 @@ this.AntiTracking = {
       await AntiTracking._setupTest(win, options.cookieBehavior,
                                     options.blockingByContentBlockingRTUI,
                                     options.extraPrefs);
 
       let cookieBlocked = 0;
       let listener = {
         onSecurityChange(webProgress, request, oldState, state,
                          contentBlockingLogJSON) {
-          if (state & Ci.nsIWebProgressListener.STATE_COOKIES_BLOCKED_TRACKER) {
+          if ((state & Ci.nsIWebProgressListener.STATE_COOKIES_BLOCKED_TRACKER) ||
+              (state & Ci.nsIWebProgressListener.STATE_COOKIES_BLOCKED_FOREIGN)) {
             ++cookieBlocked;
           }
           let contentBlockingLog = {};
           try {
             contentBlockingLog = JSON.parse(contentBlockingLogJSON);
           } catch (e) {
           }