[rSAFor] Require CORS for TopLevelStorageAccess grants eligibility

TopLevelStorageAccess grants should only be considered when the request mode is 'cors'. To perform this check, add enum CookieSettingOverride::kTopLevelStorageAccessGrantEligible to url_request.cookie_setting_overrides_ when the request mode is cors, and only allow cross-site cookie granted by TopLevelStorageAccess if this require enum exists. Bug: 1401091 Change-Id: I41a74f530882e38c8051a53013e8a3bf149f1b02 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4144296 Reviewed-by: Maks Orlovich <morlovich@chromium.org> Reviewed-by: Chris Fredrickson <cfredric@chromium.org> Commit-Queue: Shuran Huang <shuuran@chromium.org> Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org> Reviewed-by: Balazs Engedy <engedy@chromium.org> Cr-Commit-Position: refs/heads/main@{#1096976}
chromium · Jan 25, 2023 · ae8c328 · ae8c328
1 parent 1290607
commit ae8c328
Show file tree

Hide file tree

Showing 12 changed files with 435 additions and 67 deletions.
diff --git a/chrome/browser/net/storage_test_utils.cc b/chrome/browser/net/storage_test_utils.cc
@@ -170,4 +170,16 @@ bool HasStorageAccessForFrame(content::RenderFrameHost* frame) {
       .ExtractBool();
 }
 
+std::string FetchWithCredentials(content::RenderFrameHost* frame,
+                                 const GURL& url,
+                                 const bool cors_enabled) {
+  constexpr char script[] = R"(
+      fetch($1, {mode: $2, credentials: 'include'})
+      .then((result) => result.text());
+    )";
+  const std::string mode = cors_enabled ? "cors" : "no-cors";
+  return content::EvalJs(frame, content::JsReplace(script, url, mode))
+      .ExtractString();
+}
+
 }  // namespace storage::test
diff --git a/chrome/browser/net/storage_test_utils.h b/chrome/browser/net/storage_test_utils.h
@@ -7,6 +7,8 @@
 
 #include <string>
 
+class GURL;
+
 namespace content {
 class RenderFrameHost;
 }  // namespace content
@@ -45,6 +47,13 @@ bool RequestStorageAccessForOrigin(content::RenderFrameHost* frame,
 // value of true; false otherwise.
 bool HasStorageAccessForFrame(content::RenderFrameHost* frame);
 
+// Helper to see if a credentialed fetch has cookies access via top-level
+// storage access grants. Returns the content of the response if the promise
+// resolves. `cors_enabled` sets fetch RequestMode to be "cors" or "no-cors".
+std::string FetchWithCredentials(content::RenderFrameHost* frame,
+                                 const GURL& url,
+                                 const bool cors_enabled);
+
 }  // namespace test
 }  // namespace storage
 #endif  // CHROME_BROWSER_NET_STORAGE_TEST_UTILS_H_
diff --git a/chrome/browser/top_level_storage_access_api/request_storage_access_for_origin_browsertest.cc b/chrome/browser/top_level_storage_access_api/request_storage_access_for_origin_browsertest.cc
@@ -188,6 +188,13 @@ class RequestStorageAccessForOriginBaseBrowserTest
     return ChildFrameAt(GetFrame(), 0);
   }
 
+  std::string CookiesFromFetchWithCredentials(content::RenderFrameHost* frame,
+                                              const std::string& host,
+                                              const bool cors_enabled) {
+    return storage::test::FetchWithCredentials(
+        frame, https_server_.GetURL(host, "/echoheader?cookie"), cors_enabled);
+  }
+
   net::test_server::EmbeddedTestServer& https_server() { return https_server_; }
 
  private:
@@ -204,14 +211,12 @@ IN_PROC_BROWSER_TEST_F(RequestStorageAccessForOriginBrowserTest,
   SetBlockThirdPartyCookies(true);
 
   // Set a cookie on `kHostB` and `kHostC`.
-  content::SetCookie(browser()->profile(), GetURL(kHostB),
-                     "thirdparty=b;SameSite=None;Secure");
+  SetCrossSiteCookieOnHost(kHostB);
   ASSERT_EQ(content::GetCookies(browser()->profile(), GetURL(kHostB)),
-            "thirdparty=b");
-  content::SetCookie(browser()->profile(), GetURL(kHostC),
-                     "thirdparty=c;SameSite=None;Secure");
+            "cross-site=b.test");
+  SetCrossSiteCookieOnHost(kHostC);
   ASSERT_EQ(content::GetCookies(browser()->profile(), GetURL(kHostC)),
-            "thirdparty=c");
+            "cross-site=c.test");
 
   NavigateToPageWithFrame(kHostA);
   NavigateFrameTo(kHostB, "/iframe.html");
@@ -253,15 +258,23 @@ IN_PROC_BROWSER_TEST_F(RequestStorageAccessForOriginBrowserTest,
       ->GetCookieManagerForBrowserProcess()
       ->SetTopLevelStorageAccessSettings(settings, base::DoNothing());
 
+  // document.hasStorageAccess() does not have cookie access with top-level
+  // storage access grant.
   EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
-  EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetNestedFrame()));
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetNestedFrame()));
 
   NavigateFrameTo(kHostB, "/iframe.html");
   NavigateNestedFrameTo(kHostC, "/echoheader?cookie");
   EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
-  EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetNestedFrame()));
-  EXPECT_EQ(GetNestedFrameContent(), "thirdparty=c");
-  EXPECT_EQ(ReadCookiesViaJS(GetNestedFrame()), "thirdparty=c");
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetNestedFrame()));
+  EXPECT_EQ(GetNestedFrameContent(), "None");
+  EXPECT_EQ(ReadCookiesViaJS(GetNestedFrame()), "");
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+                                            /*cors_enabled=*/true),
+            "None");
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetNestedFrame(), kHostC,
+                                            /*cors_enabled=*/true),
+            "cross-site=c.test");
 }
 
 IN_PROC_BROWSER_TEST_F(RequestStorageAccessForOriginBrowserTest,
@@ -336,6 +349,9 @@ IN_PROC_BROWSER_TEST_F(RequestStorageAccessForOriginEnabledBrowserTest,
   EXPECT_EQ(GetFrameContent(), "None");
   EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
   EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostASubdomain,
+                                            /*cors_enabled=*/true),
+            "None");
 }
 
 // Tests to validate First-Party Set use with `requestStorageAccessForOrigin`.
@@ -384,23 +400,29 @@ IN_PROC_BROWSER_TEST_F(
   // the requestor, because it is a service domain.
   EXPECT_TRUE(storage::test::RequestStorageAccessForOrigin(
       GetPrimaryMainFrame(), GetURL(kHostB).spec()));
-  EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+                                            /*cors_enabled=*/true),
+            "cross-site=b.test");
   // Repeated calls should also return true.
   EXPECT_TRUE(storage::test::RequestStorageAccessForOrigin(
       GetPrimaryMainFrame(), GetURL(kHostB).spec()));
 
   // Navigate iframe to a cross-site, cookie-reading endpoint, and verify that
-  // the cookie is sent.
+  // the cookie is sent for the cors-enabled subresource request.
   NavigateFrameTo(kHostB, "/echoheader?cookie");
-  EXPECT_EQ(GetFrameContent(), "cross-site=b.test");
-  EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "cross-site=b.test");
-  EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
-
-  // Also validate that an additional site C was not granted access.
-  NavigateFrameTo(kHostC, "/echoheader?cookie");
   EXPECT_EQ(GetFrameContent(), "None");
   EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
   EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+                                            /*cors_enabled=*/true),
+            "cross-site=b.test");
+
+  // Also validate that an additional site C was not granted access.
+  NavigateFrameTo(kHostC, "/echoheader?cookie");
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostC,
+                                            /*cors_enabled=*/true),
+            "None");
 
   content::FetchHistogramsFromChildProcesses();
 
@@ -426,6 +448,9 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_EQ(GetFrameContent(), "None");
   EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
   EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostA,
+                                            /*cors_enabled=*/true),
+            "None");
   // The promise should be rejected; `khostB` is a service domain.
   EXPECT_FALSE(storage::test::RequestStorageAccessForOrigin(
       GetPrimaryMainFrame(), GetURL(kHostA).spec()));
@@ -437,6 +462,9 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_EQ(GetFrameContent(), "None");
   EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
   EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostA,
+                                            /*cors_enabled=*/true),
+            "None");
 
   content::FetchHistogramsFromChildProcesses();
   EXPECT_THAT(histogram_tester.GetBucketCount(
@@ -461,19 +489,28 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_EQ(GetFrameContent(), "None");
   EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
   EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+                                            /*cors_enabled=*/true),
+            "None");
   // `kHostB` cannot be granted access via `RequestStorageAccessForOrigin`,
   // because the call is not from the top-level page and because `kHostB` is a
   // service domain.
   EXPECT_FALSE(storage::test::RequestStorageAccessForOrigin(
       GetFrame(), GetURL(kHostA).spec()));
   EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+                                            /*cors_enabled=*/true),
+            "None");
 
   // Navigate iframe to a cross-site, cookie-reading endpoint, and verify that
   // the cookie is not sent.
   NavigateFrameTo(kHostB, "/echoheader?cookie");
   EXPECT_EQ(GetFrameContent(), "None");
   EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
   EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+                                            /*cors_enabled=*/true),
+            "None");
 
   // However, a regular `requestStorageAccess` call should be granted;
   // requesting on behalf of another domain is what is not acceptable.
@@ -511,13 +548,19 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_FALSE(storage::test::RequestStorageAccessForOrigin(
       GetPrimaryMainFrame(), GetURL(kHostD).spec()));
   EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostD,
+                                            /*cors_enabled=*/true),
+            "None");
 
   // Navigate iframe to a cross-site, cookie-reading endpoint, and verify that
   // the cookie is not sent.
   NavigateFrameTo(kHostD, "/echoheader?cookie");
   EXPECT_EQ(GetFrameContent(), "None");
   EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
   EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostD,
+                                            /*cors_enabled=*/true),
+            "None");
 
   content::FetchHistogramsFromChildProcesses();
   EXPECT_THAT(histogram_tester.GetBucketCount(
@@ -542,24 +585,37 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_EQ(GetFrameContent(), "None");
   EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
   EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+                                            /*cors_enabled=*/true),
+            "None");
   EXPECT_TRUE(storage::test::RequestStorageAccessForOrigin(
       GetPrimaryMainFrame(), GetURL(kHostB).spec()));
-  EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
 
   // Navigate iframe to a cross-site, cookie-reading endpoint, and verify that
-  // the cookie is sent:
+  // the cookie is sent for the cors-enabled subresource request.
   NavigateFrameTo(kHostB, "/echoheader?cookie");
-  EXPECT_EQ(GetFrameContent(), "cross-site=b.test");
-  EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "cross-site=b.test");
-  EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(GetFrameContent(), "None");
+  EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+                                            /*cors_enabled=*/true),
+            "cross-site=b.test");
+  // Subresource request with cors disabled does not have cookie access.
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+                                            /*cors_enabled=*/false),
+            "None");
 
   NavigateToPageWithFrame(kHostASubdomain);
   NavigateFrameTo(kHostB, "/echoheader?cookie");
   // Storage access grants are scoped to the embedded origin on the top-level
-  // site. Accordingly, the access should be granted.
-  EXPECT_EQ(GetFrameContent(), "cross-site=b.test");
-  EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "cross-site=b.test");
-  EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  // site. Accordingly, the access is be granted for subresource request.
+  EXPECT_EQ(GetFrameContent(), "None");
+  EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+                                            /*cors_enabled=*/true),
+            "cross-site=b.test");
 }
 
 IN_PROC_BROWSER_TEST_F(
@@ -578,24 +634,37 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_EQ(GetFrameContent(), "None");
   EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
   EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+                                            /*cors_enabled=*/true),
+            "None");
   EXPECT_TRUE(storage::test::RequestStorageAccessForOrigin(
       GetPrimaryMainFrame(), GetURL(kHostB).spec()));
-  EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
 
   // Navigate iframe to a cross-site, cookie-reading endpoint, and verify that
-  // the cookie is sent:
+  // the cookie is sent for the cors-enabled subresource request.
   NavigateFrameTo(kHostB, "/echoheader?cookie");
-  EXPECT_EQ(GetFrameContent(), "cross-site=b.test");
-  EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "cross-site=b.test");
-  EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(GetFrameContent(), "None");
+  EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+                                            /*cors_enabled=*/true),
+            "cross-site=b.test");
+  // Subresource request with cors disabled does not have cookie access.
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+                                            /*cors_enabled=*/false),
+            "None");
 
   NavigateToPageWithFrame(kHostA);
   NavigateFrameTo(kHostB, "/echoheader?cookie");
   // When top-level site scoping is enabled, the subdomain's grant counts for
   // the less-specific domain; otherwise, it does not.
-  EXPECT_EQ(GetFrameContent(), "cross-site=b.test");
-  EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "cross-site=b.test");
-  EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(GetFrameContent(), "None");
+  EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+                                            /*cors_enabled=*/true),
+            "cross-site=b.test");
 }
 
 // Tests to validate `requestStorageAccessForOrigin` behavior with FPS disabled.
@@ -737,22 +806,22 @@ IN_PROC_BROWSER_TEST_F(RequestStorageAccessForOriginWithCHIPSBrowserTest,
   EXPECT_EQ(GetFrameContent(), "cross-site=b.test(partitioned)");
   EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "cross-site=b.test(partitioned)");
   EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+                                            /*cors_enabled=*/true),
+            "cross-site=b.test(partitioned)");
 
   // kHostA can request storage access on behalf of kHostB, and it is granted
   // (by an implicit grant):
   EXPECT_TRUE(storage::test::RequestStorageAccessForOrigin(
       GetPrimaryMainFrame(), GetURL(kHostB).spec()));
-  EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
-
-  // When the frame subsequently navigates to an endpoint on kHostB, kHostB's
-  // unpartitioned and partitioned cookies are sent, and the iframe retains
-  // storage access.
+  // When the frame makes a subresource request to an endpoint on kHostB,
+  // kHostB's unpartitioned and partitioned cookies are sent, and the iframe
+  // retains storage access.
   NavigateFrameTo(kHostB, "/echoheader?cookie");
-  EXPECT_EQ(GetFrameContent(),
-            "cross-site=b.test; cross-site=b.test(partitioned)");
-  EXPECT_EQ(ReadCookiesViaJS(GetFrame()),
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+                                            /*cors_enabled=*/true),
             "cross-site=b.test; cross-site=b.test(partitioned)");
-  EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
 }
 
 }  // namespace
diff --git a/components/content_settings/core/browser/cookie_settings.cc b/components/content_settings/core/browser/cookie_settings.cc
@@ -222,7 +222,10 @@ ContentSetting CookieSettings::GetCookieSettingInternal(
     }
   }
 
-  if (block && ShouldConsiderTopLevelStorageAccessGrants(query_reason)) {
+  if (block &&
+      overrides.Has(
+          net::CookieSettingOverride::kTopLevelStorageAccessGrantEligible) &&
+      ShouldConsiderTopLevelStorageAccessGrants(query_reason)) {
     ContentSetting host_setting = host_content_settings_map_->GetContentSetting(
         url, first_party_url, ContentSettingsType::TOP_LEVEL_STORAGE_ACCESS);