Remove no-Domain requirement from Partitioned cookies

This change will be released in 104 for the last milestone of the CHIPS
origin trial. Whether this change is permanent remains TBD.

Bug: 1333622
Change-Id: Ib9a46e1a193124459f3e9201ba4ee6ddefcd52a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3686339
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Commit-Queue: Dylan Cutler <dylancutler@google.com>
Cr-Commit-Position: refs/heads/main@{#1012493}

net/cookies/canonical_cookie.cc

@@ -348,8 +348,7 @@ int ValidateAndAdjustSourcePort(int port) {
 }
 
 // Tests that a cookie has the attributes for a valid __Host- prefix without
-// testing that the prefix is in the cookie name. This is used to verify the
-// Partitioned attribute.
+// testing that the prefix is in the cookie name.
 bool HasValidHostPrefixAttributes(const GURL& url,
                                   bool secure,
                                   const std::string& domain,
@@ -359,6 +358,17 @@ bool HasValidHostPrefixAttributes(const GURL& url,
   return domain.empty() || (url.HostIsIPAddress() && url.host() == domain);
 }
 
+// Test that a cookie has the attributes for a valid Parititioned attribute.
+// For M104, we do not require that Partitioned cookies do not have the Domain
+// attribute.
+// TODO(crbug.com/1296161): Determine if we need to delete this function.
+bool HasValidAttributesForPartitioned(const GURL& url,
+                                      bool secure,
+                                      const std::string& path,
+                                      bool is_same_party) {
+  return url.SchemeIsCryptographic() && secure && path == "/" && !is_same_party;
+}
+
 }  // namespace
 
 CookieAccessParams::CookieAccessParams(CookieAccessSemantics access_semantics,
@@ -803,7 +813,7 @@ std::unique_ptr<CanonicalCookie> CanonicalCookie::CreateSanitizedCookie(
     status->AddExclusionReason(
         net::CookieInclusionStatus::EXCLUDE_INVALID_SAMEPARTY);
   }
-  if (!IsCookiePartitionedValid(url, secure, domain_attribute, cookie_path,
+  if (!IsCookiePartitionedValid(url, secure, cookie_path,
                                 /*is_partitioned=*/partition_key.has_value(),
                                 /*is_same_party=*/same_party,
                                 /*partition_has_nonce=*/
@@ -1504,23 +1514,28 @@ bool CanonicalCookie::IsCanonicalForFromStorage() const {
     return false;
 
   CookiePrefix prefix = GetCookiePrefix(name_);
-  bool partition_key_has_nonce = CookiePartitionKey::HasNonce(partition_key_);
-  if (prefix == COOKIE_PREFIX_HOST ||
-      (IsPartitioned() && !partition_key_has_nonce)) {
-    if (!secure_ || path_ != "/" || domain_.empty() || domain_[0] == '.')
-      return false;
-  } else if (prefix == COOKIE_PREFIX_SECURE && !secure_) {
-    return false;
+  switch (prefix) {
+    case COOKIE_PREFIX_HOST:
+      if (!secure_ || path_ != "/" || domain_.empty() || domain_[0] == '.')
+        return false;
+      break;
+    case COOKIE_PREFIX_SECURE:
+      if (!secure_)
+        return false;
+      break;
+    default:
+      break;
   }
 
   if (!IsCookieSamePartyValid(same_party_, secure_, same_site_))
     return false;
 
   if (IsPartitioned()) {
-    if (partition_key_has_nonce)
+    if (CookiePartitionKey::HasNonce(partition_key_))
       return true;
-    if (same_party_)
+    if (!secure_ || path_ != "/" || same_party_) {
       return false;
+    }
   }
 
   return true;
@@ -1663,7 +1678,6 @@ bool CanonicalCookie::IsCookiePartitionedValid(
     bool partition_has_nonce) {
   return IsCookiePartitionedValid(
       url, /*secure=*/parsed_cookie.IsSecure(),
-      parsed_cookie.HasDomain() ? parsed_cookie.Domain() : "",
       parsed_cookie.HasPath() ? parsed_cookie.Path() : "",
       /*is_partitioned=*/parsed_cookie.IsPartitioned(),
       /*is_same_party=*/parsed_cookie.IsSameParty(), partition_has_nonce);
@@ -1672,7 +1686,6 @@ bool CanonicalCookie::IsCookiePartitionedValid(
 // static
 bool CanonicalCookie::IsCookiePartitionedValid(const GURL& url,
                                                bool secure,
-                                               const std::string& domain,
                                                const std::string& path,
                                                bool is_partitioned,
                                                bool is_same_party,
@@ -1682,9 +1695,9 @@ bool CanonicalCookie::IsCookiePartitionedValid(const GURL& url,
   if (partition_has_nonce)
     return true;
   bool result =
-      HasValidHostPrefixAttributes(url, secure, domain, path) && !is_same_party;
+      HasValidAttributesForPartitioned(url, secure, path, is_same_party);
   DLOG_IF(WARNING, !result)
-      << "CanonicalCookie has invalid Partitioned attribute";
+      << "CanonicalCookie has invalid Partitioned attribute ";
   return result;
 }
 

net/cookies/canonical_cookie.h

@@ -514,7 +514,6 @@ class NET_EXPORT CanonicalCookie {
                                        bool partition_has_nonce);
   static bool IsCookiePartitionedValid(const GURL& url,
                                        bool secure,
-                                       const std::string& domain,
                                        const std::string& path,
                                        bool is_partitioned,
                                        bool is_same_party,

net/cookies/canonical_cookie_unittest.cc

@@ -663,14 +663,16 @@ TEST(CanonicalCookieTest, CreateWithPartitioned) {
   EXPECT_TRUE(status.HasExactlyExclusionReasonsForTesting(
       {CookieInclusionStatus::EXCLUDE_INVALID_PARTITIONED}));
 
-  // Invalid Partitioned attribute: Domain cookie.
+  // Partitioned attribute: Domain cookie.
   status = CookieInclusionStatus();
   cookie = CanonicalCookie::Create(
-      url, "A=2; Partitioned; Path=/; Domain=example.com", creation_time,
-      server_time, partition_key, &status);
-  EXPECT_FALSE(cookie.get());
-  EXPECT_TRUE(status.HasExactlyExclusionReasonsForTesting(
-      {CookieInclusionStatus::EXCLUDE_INVALID_PARTITIONED}));
+      url, "A=2; Partitioned; Path=/; Secure; Domain=example.com",
+      creation_time, server_time, partition_key, &status);
+  EXPECT_TRUE(cookie.get());
+  LOG(ERROR) << status;
+  EXPECT_TRUE(status.IsInclude());
+  EXPECT_TRUE(cookie->IsPartitioned());
+  EXPECT_EQ(partition_key, cookie->PartitionKey());
 
   // Invalid Partitioned attribute: SameParty cookie.
   status = CookieInclusionStatus();
@@ -3005,16 +3007,16 @@ TEST(CanonicalCookieTest, IsCanonical) {
                        GURL("https://toplevelsite.com")))
                    ->IsCanonical());
 
-  // Partitioned attribute invalid, Domain attribute also included.
-  EXPECT_FALSE(CanonicalCookie::CreateUnsafeCookieForTesting(
-                   "A", "B", ".x.y", "/", base::Time(), base::Time(),
-                   base::Time(), base::Time(), /*secure=*/true,
-                   /*httponly=*/false, CookieSameSite::UNSPECIFIED,
-                   COOKIE_PRIORITY_LOW,
-                   /*same_party=*/false,
-                   CookiePartitionKey::FromURLForTesting(
-                       GURL("https://toplevelsite.com")))
-                   ->IsCanonical());
+  // Partitioned attribute is valid when Domain attribute also included.
+  EXPECT_TRUE(CanonicalCookie::CreateUnsafeCookieForTesting(
+                  "A", "B", ".x.y", "/", base::Time(), base::Time(),
+                  base::Time(), base::Time(), /*secure=*/true,
+                  /*httponly=*/false, CookieSameSite::UNSPECIFIED,
+                  COOKIE_PRIORITY_LOW,
+                  /*same_party=*/false,
+                  CookiePartitionKey::FromURLForTesting(
+                      GURL("https://toplevelsite.com")))
+                  ->IsCanonical());
 
   // Partitioned attribute invalid, SameParty attribute also included.
   EXPECT_FALSE(CanonicalCookie::CreateUnsafeCookieForTesting(
@@ -3755,9 +3757,9 @@ TEST(CanonicalCookieTest, CreateSanitizedCookie_Logic) {
       &status));
   EXPECT_TRUE(status.HasExactlyExclusionReasonsForTesting(
       {CookieInclusionStatus::EXCLUDE_INVALID_PARTITIONED}));
-  // Invalid: Domain attribute present.
+  // Domain attribute present is still valid.
   status = CookieInclusionStatus();
-  EXPECT_FALSE(CanonicalCookie::CreateSanitizedCookie(
+  EXPECT_TRUE(CanonicalCookie::CreateSanitizedCookie(
       GURL("https://www.foo.com"), "A", "B", ".foo.com", "/", two_hours_ago,
       one_hour_from_now, one_hour_ago, /*secure=*/true, /*http_only=*/false,
       CookieSameSite::NO_RESTRICTION, CookiePriority::COOKIE_PRIORITY_DEFAULT,
@@ -3765,8 +3767,7 @@ TEST(CanonicalCookieTest, CreateSanitizedCookie_Logic) {
       absl::optional<CookiePartitionKey>(CookiePartitionKey::FromURLForTesting(
           GURL("https://toplevelsite.com"))),
       &status));
-  EXPECT_TRUE(status.HasExactlyExclusionReasonsForTesting(
-      {CookieInclusionStatus::EXCLUDE_INVALID_PARTITIONED}));
+  EXPECT_TRUE(status.IsInclude());
   // Invalid: SameParty attribute present.
   status = CookieInclusionStatus();
   EXPECT_FALSE(CanonicalCookie::CreateSanitizedCookie(

net/cookies/cookie_monster.cc

@@ -2405,9 +2405,18 @@ void CookieMonster::OnConvertPartitionedCookiesToUnpartitioned(
   // We only want cookies whose domain is a match for the entire URL host.
   // This should exclude cookies set on subdomains.
   std::vector<CanonicalCookie*> cookie_ptrs;
+  CookieOptions options = CookieOptions::MakeAllInclusive();
+  bool delegate_treats_url_as_trustworthy =
+      cookie_access_delegate() &&
+      cookie_access_delegate()->ShouldTreatUrlAsTrustworthy(url);
+  CookieAccessParams accesss_params{
+      net::CookieAccessSemantics::UNKNOWN, delegate_treats_url_as_trustworthy,
+      CookieSamePartyStatus::kNoSamePartyEnforcement};
   for (auto* cookie : cookie_ptrs_for_site) {
-    if (cookie->Domain() == url.host())
+    if (cookie->IncludeForRequestURL(url, options, accesss_params)
+            .status.IsInclude()) {
       cookie_ptrs.push_back(cookie);
+    }
   }
 
   std::map<std::tuple<std::string, std::string, std::string>,
@@ -2483,13 +2492,14 @@ void CookieMonster::OnConvertPartitionedCookiesToUnpartitioned(
       CanonicalCookie* cc = cur_cookie_it->second.get();
       ++cookie_it;
 
-      if (cc->Domain() != url.host())
+      if (!cc->IncludeForRequestURL(url, options, accesss_params)
+               .status.IsInclude() ||
+          cc->PartitionKey()->nonce()) {
         continue;
-
-      if (!cc->PartitionKey()->nonce()) {
-        InternalDeletePartitionedCookie(cur_partition_it, cur_cookie_it, true,
-                                        DELETE_COOKIE_EXPLICIT);
       }
+
+      InternalDeletePartitionedCookie(cur_partition_it, cur_cookie_it, true,
+                                      DELETE_COOKIE_EXPLICIT);
     }
   }
 }

net/cookies/cookie_monster_unittest.cc

@@ -5543,7 +5543,33 @@ TEST_F(CookieMonsterTest, ConvertPartitionedCookiesToUnpartitioned) {
   EXPECT_EQ(cookies.size(), 6u);
   EXPECT_EQ(cookies[5].Name(), "__Host-G");
   EXPECT_EQ(cookies[5].Value(), "0");
-  EXPECT_FALSE(cookies[4].IsPartitioned());
+  EXPECT_FALSE(cookies[5].IsPartitioned());
+
+  // Test that a Domain cookie is converted if one of its subdomains converts
+  // their cookies.
+  EXPECT_TRUE(CreateAndSetCookie(
+      cm.get(), https_www_foo_.url(),
+      "H=0; Secure; Path=/; SameSite=None; Partitioned; Domain=foo.com",
+      options, absl::nullopt, absl::nullopt,
+      CookiePartitionKey::FromURLForTesting(GURL("https://example.com"))));
+  // Include a hostname bound cookie as well to test interaction.
+  EXPECT_TRUE(CreateAndSetCookie(
+      cm.get(), https_www_foo_.url(),
+      "H=1; Secure; Path=/; SameSite=None; Partitioned", options, absl::nullopt,
+      absl::nullopt,
+      CookiePartitionKey::FromURLForTesting(GURL("https://example.com"))));
+
+  cm->ConvertPartitionedCookiesToUnpartitioned(https_www_foo_.url());
+
+  cookies = GetAllCookiesForURL(cm.get(), https_www_foo_.url(),
+                                CookiePartitionKeyCollection::ContainsAll());
+  EXPECT_EQ(cookies.size(), 8u);
+  EXPECT_EQ(cookies[6].Name(), "H");
+  EXPECT_EQ(cookies[6].Value(), "0");
+  EXPECT_FALSE(cookies[6].IsPartitioned());
+  EXPECT_EQ(cookies[7].Name(), "H");
+  EXPECT_EQ(cookies[7].Value(), "1");
+  EXPECT_FALSE(cookies[7].IsPartitioned());
 }
 
 // Tests which use this class verify the expiry date clamping behavior when

third_party/blink/web_tests/http/tests/cookies/partitioned-cookies/partitioned-attribute.https.html

@@ -26,13 +26,21 @@ test(() => {
   deleteExistingCookie(prefixedCookieLine)
 
   // Partitioned cookie with no __Host- prefix are valid as long as they have
-  // the Secure, Path=/, and no Domain attributes.
+  // the Secure and Path=/.
   const noPrefixCookie = "foo=bar";
   const noPrefixCookieLine = noPrefixCookie + validCookieAttributes;
   document.cookie = noPrefixCookieLine;
   assert_equals(document.cookie, "foo=bar");
   deleteExistingCookie(noPrefixCookieLine);
 
+  // Partitioned cookie with Domain is still valid.
+  const domainCookie = "foo=baz";
+  const domainCookieLine = domainCookie + validCookieAttributes + `; Domain=${
+      window.location.hostname}`;
+  document.cookie = domainCookieLine;
+  assert_equals(document.cookie, "foo=baz");
+  deleteExistingCookie(domainCookieLine);
+
   // Invalid Partitioned cookie: SameParty attribute.
   assertInvalidCookie(prefixedCookieLine + "; SameParty");