From 550505c7d85924a7c4a9c19f6f21756b5a40459d Mon Sep 17 00:00:00 2001
From: Tim Huang <tihuang@mozilla.com>
Date: Tue, 13 Feb 2024 10:07:37 +0000
Subject: [PATCH] Bug 1873626 - Restrict cookies with partitioned attribution
 to be used only in secure contexts. r=bvandersloot,cookie-reviewers,edgul

Differential Revision: https://phabricator.services.mozilla.com/D199231
---
 netwerk/cookie/CookieService.cpp              | 13 ++++
 .../browser/browser_partitionedConsole.js     | 68 +++++++++++++++++++
 netwerk/cookie/test/browser/partitioned.sjs   | 22 ++++--
 netwerk/locales/en-US/necko.properties        |  2 +
 4 files changed, 98 insertions(+), 7 deletions(-)

diff --git a/netwerk/cookie/CookieService.cpp b/netwerk/cookie/CookieService.cpp
index d8cc9b1ef88f..d306203c2fd9 100644
--- a/netwerk/cookie/CookieService.cpp
+++ b/netwerk/cookie/CookieService.cpp
@@ -1663,6 +1663,19 @@ bool CookieService::ParseAttributes(nsIConsoleReportCollector* aCRC,
                                 SAMESITE_MDN_URL});
   }
 
+  // Ensure the partitioned cookie is set with the secure attribute.
+  if (aCookieData.isPartitioned() && !aCookieData.isSecure()) {
+    CookieLogging::LogMessageToConsole(
+        aCRC, aHostURI, nsIScriptError::errorFlag, CONSOLE_REJECTION_CATEGORY,
+        "CookieRejectedPartitionedRequiresSecure"_ns,
+        AutoTArray<nsString, 1>{NS_ConvertUTF8toUTF16(aCookieData.name())});
+
+    // We only drop the cookie if CHIPS is enabled.
+    if (StaticPrefs::network_cookie_cookieBehavior_optInPartitioning()) {
+      return newCookie;
+    }
+  }
+
   if (aCookieData.rawSameSite() == nsICookie::SAMESITE_NONE &&
       aCookieData.sameSite() == nsICookie::SAMESITE_LAX) {
     bool laxByDefault =
diff --git a/netwerk/cookie/test/browser/browser_partitionedConsole.js b/netwerk/cookie/test/browser/browser_partitionedConsole.js
index 1e200eecd311..ec834bfbcf26 100644
--- a/netwerk/cookie/test/browser/browser_partitionedConsole.js
+++ b/netwerk/cookie/test/browser/browser_partitionedConsole.js
@@ -131,3 +131,71 @@ add_task(async _ => {
   // Let's close the tab.
   BrowserTestUtils.removeTab(tab);
 });
+
+// Run the test with CHIPS enabled, ensuring the partitioned cookies require
+// secure context.
+add_task(async function partitionedAttrRequiresSecure() {
+  await SpecialPowers.pushPrefEnv({
+    set: [["network.cookie.cookieBehavior.optInPartitioning", true]],
+  });
+
+  // Clear all cookies before testing.
+  Services.cookies.removeAll();
+
+  const expected = [];
+
+  const consoleListener = {
+    observe(what) {
+      if (!(what instanceof Ci.nsIConsoleMessage)) {
+        return;
+      }
+
+      info("Console Listener: " + what);
+      for (let i = expected.length - 1; i >= 0; --i) {
+        const e = expected[i];
+
+        if (what.message.includes(e.match)) {
+          ok(true, "Message received: " + e.match);
+          expected.splice(i, 1);
+          e.resolve();
+        }
+      }
+    },
+  };
+
+  Services.console.registerListener(consoleListener);
+
+  registerCleanupFunction(() =>
+    Services.console.unregisterListener(consoleListener)
+  );
+
+  const netPromises = [
+    new Promise(resolve => {
+      expected.push({
+        resolve,
+        match:
+          "Cookie “c” has been rejected because it has the “Partitioned” attribute but is missing the “secure” attribute.",
+      });
+    }),
+  ];
+
+  // Let's open our tab.
+  const tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TOP_PAGE);
+
+  // Set cookies with cross-site HTTP
+  await SpecialPowers.spawn(tab.linkedBrowser, [], async function () {
+    await content.fetch(
+      "https://example.org/browser/netwerk/cookie/test/browser/partitioned.sjs?nosecure",
+      { credentials: "include" }
+    );
+  });
+
+  // Let's wait for the first set of console events.
+  await Promise.all(netPromises);
+
+  // Ensure no cookie is set.
+  is(Services.cookies.cookies.length, 0, "No cookie is set.");
+
+  // Let's close the tab.
+  BrowserTestUtils.removeTab(tab);
+});
diff --git a/netwerk/cookie/test/browser/partitioned.sjs b/netwerk/cookie/test/browser/partitioned.sjs
index 8074f57f5375..5649b88f2f01 100644
--- a/netwerk/cookie/test/browser/partitioned.sjs
+++ b/netwerk/cookie/test/browser/partitioned.sjs
@@ -13,12 +13,20 @@ function handleRequest(aRequest, aResponse) {
     aResponse.setStatusLine(aRequest.httpVersion, 200);
   }
 
-  if (!params.has("nocookie")) {
-    aResponse.setHeader("Set-Cookie", "a=1; SameSite=None; Secure", true);
-    aResponse.setHeader(
-      "Set-Cookie",
-      "b=2; Partitioned; SameSite=None; Secure",
-      true
-    );
+  if (params.has("nocookie")) {
+    return;
   }
+
+  if (params.has("nosecure")) {
+    aResponse.setHeader("Set-Cookie", "c=3; Partitioned;", true);
+
+    return;
+  }
+
+  aResponse.setHeader("Set-Cookie", "a=1; SameSite=None; Secure", true);
+  aResponse.setHeader(
+    "Set-Cookie",
+    "b=2; Partitioned; SameSite=None; Secure",
+    true
+  );
 }
diff --git a/netwerk/locales/en-US/necko.properties b/netwerk/locales/en-US/necko.properties
index 250ebdc23887..7498cca20b9d 100644
--- a/netwerk/locales/en-US/necko.properties
+++ b/netwerk/locales/en-US/necko.properties
@@ -86,6 +86,8 @@ CookieRejectedThirdParty=Cookie “%1$S” has been rejected as third-party.
 CookieRejectedNonsecureOverSecure=Cookie “%1$S” has been rejected because there is an existing “secure” cookie.
 # LOCALIZATION NOTE (CookieRejectedForNonSameSiteness): %1$S is the cookie name.
 CookieRejectedForNonSameSiteness=Cookie “%1$S” has been rejected because it is in a cross-site context and its “SameSite” is “Lax” or “Strict”.
+# LOCALIZATION NOTE (CookieRejectedPartitionedRequiresSecure): %1$S is the cookie name.
+CookieRejectedPartitionedRequiresSecure=Cookie “%1$S” has been rejected because it has the “Partitioned” attribute but is missing the “secure” attribute.
 
 # LOCALIZATION NOTE (CookieForeignNoPartitionedWarning): %1$S is the cookie name. Do not translate "Partitioned"
 CookieForeignNoPartitionedWarning=Cookie “%1$S” will soon be rejected because it is foreign and does not have the “Partitioned“ attribute.