Bug 1345474 - Check incognito access for cookies api, r=rpl
authorShane Caraveo <scaraveo@mozilla.com>
Wed, 12 Dec 2018 14:35:17 +0000
changeset 450325 4d887e5467d0ca1d282d348c92820fe943449802
parent 450324 43b66faad595b12f39eaf14e1e3f2624adc795f3
child 450326 acbcbc1e09adb29fc3eb763f5e330997e82b833a
push id110479
push usercsabou@mozilla.com
push dateThu, 13 Dec 2018 04:02:11 +0000
treeherdermozilla-inbound@3ecc407c0cc8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrpl
bugs1345474
milestone66.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 1345474 - Check incognito access for cookies api, r=rpl Differential Revision: https://phabricator.services.mozilla.com/D11699
toolkit/components/extensions/parent/ext-cookies.js
toolkit/components/extensions/test/mochitest/mochitest-common.ini
toolkit/components/extensions/test/mochitest/test_ext_cookies_incognito.html
--- a/toolkit/components/extensions/parent/ext-cookies.js
+++ b/toolkit/components/extensions/parent/ext-cookies.js
@@ -133,19 +133,23 @@ const checkSetCookiePermissions = (exten
 
   // We don't do any significant checking of path permissions. RFC2109
   // suggests we only allow sites to add cookies for sub-paths, similar to
   // same origin policy enforcement, but no-one implements this.
 
   return true;
 };
 
+/**
+ * Query the cookie store for matching cookies.
+ * @param {Object} detailsIn
+ * @param {Array} props          Properties the extension is interested in matching against.
+ * @param {BaseContext} context  The context making the query.
+ */
 const query = function* (detailsIn, props, context) {
-  // Different callers want to filter on different properties. |props|
-  // tells us which ones they're interested in.
   let details = {};
   props.forEach(property => {
     if (detailsIn[property] !== null) {
       details[property] = detailsIn[property];
     }
   });
 
   if ("domain" in details) {
@@ -173,16 +177,19 @@ const query = function* (detailsIn, prop
   }
 
   let storeId = DEFAULT_STORE;
   if (isPrivate) {
     storeId = PRIVATE_STORE;
   } else if ("storeId" in details) {
     storeId = details.storeId;
   }
+  if (storeId == PRIVATE_STORE && !context.privateBrowsingAllowed) {
+    throw new ExtensionError("Extension disallowed access to the private cookies storeId.");
+  }
 
   // We can use getCookiesFromHost for faster searching.
   let enumerator;
   let host;
   let url;
   let originAttributes = {
     userContextId,
     privateBrowsingId: isPrivate ? 1 : 0,
@@ -355,16 +362,19 @@ this.cookies = class extends ExtensionAP
           let httpOnly = details.httpOnly !== null ? details.httpOnly : false;
           let isSession = details.expirationDate === null;
           let expiry = isSession ? Number.MAX_SAFE_INTEGER : details.expirationDate;
           let isPrivate = context.incognito;
           let userContextId = 0;
           if (isDefaultCookieStoreId(details.storeId)) {
             isPrivate = false;
           } else if (isPrivateCookieStoreId(details.storeId)) {
+            if (!context.privateBrowsingAllowed) {
+              return Promise.reject({message: "Extension disallowed access to the private cookies storeId."});
+            }
             isPrivate = true;
           } else if (isContainerCookieStoreId(details.storeId)) {
             let containerId = getContainerForCookieStoreId(details.storeId);
             if (containerId === null) {
               return Promise.reject({message: `Illegal storeId: ${details.storeId}`});
             }
             isPrivate = false;
             userContextId = containerId;
@@ -394,16 +404,20 @@ this.cookies = class extends ExtensionAP
           return self.cookies.get(details);
         },
 
         remove: function(details) {
           normalizeFirstPartyDomain(details);
 
           let allowed = ["url", "name", "storeId", "firstPartyDomain"];
           for (let {cookie, storeId} of query(details, allowed, context)) {
+            if (isPrivateCookieStoreId(details.storeId) &&
+                !context.privateBrowsingAllowed) {
+              return Promise.reject({message: "Unknown storeId"});
+            }
             Services.cookies.remove(cookie.host, cookie.name, cookie.path, false, cookie.originAttributes);
 
             // TODO Bug 1387957: could there be multiple per subdomain?
             return Promise.resolve({
               url: details.url,
               name: details.name,
               storeId,
               firstPartyDomain: details.firstPartyDomain,
@@ -464,20 +478,24 @@ this.cookies = class extends ExtensionAP
                       notify(true, cookie, "evicted");
                     }
                   }
                   break;
               }
             };
 
             Services.obs.addObserver(observer, "cookie-changed");
-            Services.obs.addObserver(observer, "private-cookie-changed");
+            if (context.privateBrowsingAllowed) {
+              Services.obs.addObserver(observer, "private-cookie-changed");
+            }
             return () => {
               Services.obs.removeObserver(observer, "cookie-changed");
-              Services.obs.removeObserver(observer, "private-cookie-changed");
+              if (context.privateBrowsingAllowed) {
+                Services.obs.removeObserver(observer, "private-cookie-changed");
+              }
             };
           },
         }).api(),
       },
     };
 
     return self;
   }
--- a/toolkit/components/extensions/test/mochitest/mochitest-common.ini
+++ b/toolkit/components/extensions/test/mochitest/mochitest-common.ini
@@ -72,16 +72,18 @@ skip-if = (verify && debug && (os == 'li
 [test_ext_contentscript_devtools_metadata.html]
 [test_ext_contentscript_incognito.html]
 skip-if = os == 'android' # Android does not support multiple windows.
 [test_ext_contentscript_permission.html]
 [test_ext_cookies.html]
 [test_ext_cookies_containers.html]
 [test_ext_cookies_expiry.html]
 [test_ext_cookies_first_party.html]
+[test_ext_cookies_incognito.html]
+skip-if = os == 'android' # Bug 1513544 Android does not support multiple windows.
 [test_ext_cookies_permissions_bad.html]
 [test_ext_cookies_permissions_good.html]
 [test_ext_exclude_include_globs.html]
 [test_ext_external_messaging.html]
 [test_ext_generate.html]
 [test_ext_geolocation.html]
 skip-if = os == 'android' # Android support Bug 1336194
 [test_ext_identity.html]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/test_ext_cookies_incognito.html
@@ -0,0 +1,109 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>WebExtension test</title>
+  <meta charset="utf-8">
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/AddTask.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/ExtensionTestUtils.js"></script>
+  <script type="text/javascript" src="head.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+
+<script type="text/javascript">
+"use strict";
+
+add_task(async function test_cookies_incognito_not_allowed() {
+  let privateExtension = ExtensionTestUtils.loadExtension({
+    async background() {
+      let window = await browser.windows.create({incognito: true});
+      browser.test.onMessage.addListener(async () => {
+        await browser.windows.remove(window.id);
+        browser.test.sendMessage("done");
+      });
+      browser.test.sendMessage("ready");
+    },
+    manifest: {
+      permissions: ["cookies", "*://example.org/"],
+    },
+  });
+  await privateExtension.startup();
+  await privateExtension.awaitMessage("ready");
+
+  async function background() {
+    const storeId = "firefox-private";
+    const url = "http://example.org/";
+
+    // Getting the wrong storeId will fail, otherwise we should finish the test fine.
+    browser.cookies.onChanged.addListener(changeInfo => {
+      let {cookie} = changeInfo;
+      browser.test.assertTrue(cookie.storeId != storeId, "cookie store is correct");
+    });
+
+    browser.test.onMessage.addListener(async () => {
+      let stores = await browser.cookies.getAllCookieStores();
+      let store = stores.find(s => s.incognito);
+      browser.test.assertTrue(!store, "incognito cookie store should not be available");
+      browser.test.notifyPass("cookies");
+    });
+
+    await browser.test.assertRejects(
+      browser.cookies.set({url, name: "test", storeId}),
+      /Extension disallowed access/,
+      "API should reject setting cookie");
+    await browser.test.assertRejects(
+      browser.cookies.get({url, name: "test", storeId}),
+      /Extension disallowed access/,
+      "API should reject getting cookie");
+    await browser.test.assertRejects(
+      browser.cookies.getAll({url, storeId}),
+      /Extension disallowed access/,
+      "API should reject getting cookie");
+    await browser.test.assertRejects(
+      browser.cookies.remove({url, name: "test", storeId}),
+      /Extension disallowed access/,
+      "API should reject getting cookie");
+    await browser.test.assertRejects(
+      browser.cookies.getAll({url, storeId}),
+      /Extension disallowed access/,
+      "API should reject getting cookie");
+
+    browser.test.sendMessage("set-cookies");
+  }
+
+  let extension = ExtensionTestUtils.loadExtension({
+    background,
+    manifest: {
+      permissions: ["cookies", "*://example.org/"],
+    },
+    incognitoOverride: "not_allowed",
+  });
+
+  await extension.startup();
+  await extension.awaitMessage("set-cookies");
+
+  let chromeScript = SpecialPowers.loadChromeScript(() => {
+    ChromeUtils.import("resource://gre/modules/Services.jsm");
+
+    Services.cookies.add("example.org", "/", "public", `foo${Math.random()}`,
+                         false, false, false, Number.MAX_SAFE_INTEGER, {},
+                         Ci.nsICookie2.SAMESITE_UNSET);
+    Services.cookies.add("example.org", "/", "private", `foo${Math.random()}`,
+                         false, false, false, Number.MAX_SAFE_INTEGER, {privateBrowsingId: 1},
+                         Ci.nsICookie2.SAMESITE_UNSET);
+  });
+  extension.sendMessage("test-cookie-store");
+  await extension.awaitFinish("cookies");
+
+  await extension.unload();
+  privateExtension.sendMessage("close");
+  await privateExtension.awaitMessage("done");
+  await privateExtension.unload();
+  chromeScript.destroy();
+});
+
+</script>
+
+</body>
+</html>