Compute SameParty cookie context in RestrictedCookieManager.

Use the passed-in isolation_info to calculate the SameParty cookie
context of CookieOptions.

Bug: 1136102
Change-Id: I31d95d5f93c66622fbc516b1e956d0cc571ee2a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2615998
Commit-Queue: Shuran Huang <shuuran@chromium.org>
Reviewed-by: Lily Chen <chlily@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843728}

content/browser/renderer_host/cookie_browsertest.cc

@@ -443,4 +443,110 @@ IN_PROC_BROWSER_TEST_F(CookieBrowserTest, CrossSiteCookieSecurityEnforcement) {
       v.DepictFrameTree(tab->GetFrameTree()->root()));
 }
 
+class SamePartyEnabledCookieBrowserTest : public CookieBrowserTest {
+ public:
+  ~SamePartyEnabledCookieBrowserTest() override = default;
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    CookieBrowserTest::SetUpCommandLine(command_line);
+    // Set up First-Party Sets and also enables net::Feature::kFirstPartySets.
+    // a.test, b.test and c.test are in the same FPS.
+    command_line->AppendSwitchASCII(
+        "use-first-party-set", "https://a.test,https://b.test,https://c.test");
+  }
+};
+
+// SameParty cookies (that aren't marked as http-only) should be available to
+// JavaScript.
+IN_PROC_BROWSER_TEST_F(SamePartyEnabledCookieBrowserTest, SamePartyCookies) {
+  // Must use HTTPS because SameParty cookies must be Secure.
+  net::EmbeddedTestServer server(net::EmbeddedTestServer::TYPE_HTTPS);
+  server.SetSSLConfig(net::EmbeddedTestServer::CERT_TEST_NAMES);
+  server.AddDefaultHandlers(GetTestDataFilePath());
+  SetupCrossSiteRedirector(&server);
+  ASSERT_TRUE(server.Start());
+
+  // The server sets five cookies on a.test/b.test/c.test/d.test.
+  // d.test is not a member site from the same First-Party Sets with others.
+  std::string cookies_to_set =
+      "/set-cookie?unspecified=1;Secure"
+      "&samesite-none-sameparty=1;SameSite=None;SameParty;Secure"
+      "&samesite-lax-sameparty=1;SameSite=Lax;SameParty;Secure"
+      "&sameparty=1;SameParty;Secure"
+      "&sameparty-http=1;SameParty;Secure;httponly";
+
+  GURL url = server.GetURL("a.test", cookies_to_set);
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+  url = server.GetURL("b.test", cookies_to_set);
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+  url = server.GetURL("c.test", cookies_to_set);
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+  // d.test is not in the FPS.
+  url = server.GetURL("d.test", cookies_to_set);
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+
+  url = server.GetURL("a.test",
+                      "/cross_site_iframe_factory.html?a.test(a.test(),b.test("
+                      "c.test),d.test(b.test(c.test)))");
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(shell()->web_contents());
+  RenderFrameHostImpl* main_frame = web_contents->GetMainFrame();
+  ASSERT_EQ(3u, main_frame->child_count());
+
+  RenderFrameHostImpl* a_iframe = main_frame->child_at(0)->current_frame_host();
+  RenderFrameHostImpl* b_iframe = main_frame->child_at(1)->current_frame_host();
+  RenderFrameHostImpl* d_iframe = main_frame->child_at(2)->current_frame_host();
+
+  EXPECT_EQ("a.test", a_iframe->GetLastCommittedURL().host());
+  EXPECT_EQ("b.test", b_iframe->GetLastCommittedURL().host());
+  EXPECT_EQ("d.test", d_iframe->GetLastCommittedURL().host());
+
+  // The top-level frame should get all cookies.
+  EXPECT_EQ(
+      "unspecified=1; samesite-none-sameparty=1; samesite-lax-sameparty=1; "
+      "sameparty=1",
+      GetCookieFromJS(main_frame));
+
+  // All same-site/SameParty cookies will be delievered to the a.test -> a.test
+  // frame, as it is same-site with its ancestors.
+  EXPECT_EQ(
+      "unspecified=1; samesite-none-sameparty=1; samesite-lax-sameparty=1; "
+      "sameparty=1",
+      GetCookieFromJS(a_iframe));
+
+  // SameParty cookies will be delievered to the a.test -> b.test frame, as it
+  // is in the same First-Party Sets with its ancestors.
+  EXPECT_EQ("samesite-none-sameparty=1; samesite-lax-sameparty=1; sameparty=1",
+            GetCookieFromJS(b_iframe));
+
+  // samesite-none-sameparty cookie will be delivered to the a.test -> d.test
+  // frame, as d.test is not in any FPS.
+  EXPECT_EQ("samesite-none-sameparty=1", GetCookieFromJS(d_iframe));
+
+  ASSERT_EQ(1u, b_iframe->child_count());
+  RenderFrameHostImpl* bc_iframe = b_iframe->child_at(0)->current_frame_host();
+  EXPECT_EQ("c.test", bc_iframe->GetLastCommittedURL().host());
+  // SameParty cookies will be delievered to the a.test -> b.test -> c.test
+  // frame, as it is in the same First-Party Sets with its ancestors.
+  EXPECT_EQ("samesite-none-sameparty=1; samesite-lax-sameparty=1; sameparty=1",
+            GetCookieFromJS(bc_iframe));
+
+  ASSERT_EQ(1u, d_iframe->child_count());
+  RenderFrameHostImpl* db_iframe = d_iframe->child_at(0)->current_frame_host();
+  EXPECT_EQ("b.test", db_iframe->GetLastCommittedURL().host());
+  // No cookies will be delievered to the a.test -> d.test -> b.test frame, as
+  // it is embedded in a non-member site.
+  EXPECT_EQ("", GetCookieFromJS(db_iframe));
+
+  ASSERT_EQ(1u, db_iframe->child_count());
+  RenderFrameHostImpl* dbc_iframe =
+      db_iframe->child_at(0)->current_frame_host();
+  EXPECT_EQ("c.test", dbc_iframe->GetLastCommittedURL().host());
+  // No cookies will be delievered to the a.test -> d.test -> b.test -> c.test
+  // frame, as it's ancestors has non-member site.
+  EXPECT_EQ("", GetCookieFromJS(dbc_iframe));
+}
+
 }  // namespace content

net/cookies/cookie_util.cc

@@ -21,8 +21,10 @@
 #include "base/strings/string_util.h"
 #include "build/build_config.h"
 #include "net/base/features.h"
+#include "net/base/isolation_info.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "net/base/url_util.h"
+#include "net/cookies/cookie_access_delegate.h"
 #include "net/http/http_util.h"
 #include "url/gurl.h"
 #include "url/url_constants.h"
@@ -610,6 +612,25 @@ bool IsFirstPartySetsEnabled() {
   return base::FeatureList::IsEnabled(features::kFirstPartySets);
 }
 
+// Return SamePartyCookieContextType::kCrossParty when:
+// 1) `isolation_info` is not fully populated.
+// 2) `isolation_info.party_context` is null.
+// 3) `cookie_access_delegate.IsContextSamePartyWithSite` returns false.
+CookieOptions::SamePartyCookieContextType ComputeSamePartyContext(
+    const net::SchemefulSite& request_site,
+    const IsolationInfo& isolation_info,
+    const CookieAccessDelegate* cookie_access_delegate) {
+  if (!isolation_info.IsEmpty() && isolation_info.party_context().has_value() &&
+      cookie_access_delegate &&
+      cookie_access_delegate->IsContextSamePartyWithSite(
+          request_site,
+          isolation_info.network_isolation_key().GetTopFrameSite().value(),
+          isolation_info.party_context().value())) {
+    return CookieOptions::SamePartyCookieContextType::kSameParty;
+  }
+  return CookieOptions::SamePartyCookieContextType::kCrossParty;
+}
+
 CookieSamePartyStatus GetSamePartyStatus(const CanonicalCookie& cookie,
                                          const CookieOptions& options) {
   if (!IsFirstPartySetsEnabled() || !cookie.IsSameParty() ||

net/cookies/cookie_util.h

@@ -21,6 +21,11 @@
 class GURL;
 
 namespace net {
+
+class IsolationInfo;
+class SchemefulSite;
+class CookieAccessDelegate;
+
 namespace cookie_util {
 
 // Constants for use in VLOG
@@ -217,6 +222,16 @@ NET_EXPORT bool IsSchemefulSameSiteEnabled();
 
 NET_EXPORT bool IsFirstPartySetsEnabled();
 
+// Compute SameParty context, determines which of the cookies for `request_url`
+// can be accessed. Returns either kCrossParty or kSameParty. `isolation_info`
+// must be fully populated. In Chrome, all requests with credentials enabled
+// have a fully populated IsolationInfo.  But that might not be true for other
+// embedders yet (including cast, WebView, etc).  Also not sure about iOS.
+NET_EXPORT CookieOptions::SamePartyCookieContextType ComputeSamePartyContext(
+    const net::SchemefulSite& request_url,
+    const IsolationInfo& isolation_info,
+    const CookieAccessDelegate* cookie_access_delegate);
+
 // Get the SameParty inclusion status. If the cookie is not SameParty, returns
 // kNoSamePartyEnforcement; if the cookie is SameParty but does not have a
 // valid context, returns kEnforceSamePartyExclude.
@@ -241,6 +256,7 @@ StripAccessResults(const CookieAccessResultList& cookie_access_result_list);
 NET_EXPORT void RecordCookiePortOmniboxHistograms(const GURL& url);
 
 }  // namespace cookie_util
+
 }  // namespace net
 
 #endif  // NET_COOKIES_COOKIE_UTIL_H_

net/url_request/url_request_http_job.cc

@@ -173,29 +173,6 @@ net::CookieOptions CreateCookieOptions(
   return options;
 }
 
-// Return SamePartyCookieContextType::kCrossParty when:
-// 1) `isolation_info` is not fully populated.
-// 2) `isolation_info.party_context` is null.
-// 3) `cookie_access_delegate.IsContextSamePartyWithSite` returns false.
-//
-// In Chrome, all requests with credentials enabled have a fully populated
-// IsolationInfo.  But that might not be true for other embedders yet
-// (including cast, WebView, etc).  Also not sure about iOS.
-net::CookieOptions::SamePartyCookieContextType ComputeSamePartyContext(
-    const net::SchemefulSite& request_site,
-    const net::IsolationInfo& isolation_info,
-    const net::CookieAccessDelegate* cookie_access_delegate) {
-  if (!isolation_info.IsEmpty() && isolation_info.party_context().has_value() &&
-      cookie_access_delegate &&
-      cookie_access_delegate->IsContextSamePartyWithSite(
-          request_site,
-          isolation_info.network_isolation_key().GetTopFrameSite().value(),
-          isolation_info.party_context().value())) {
-    return net::CookieOptions::SamePartyCookieContextType::kSameParty;
-  }
-  return net::CookieOptions::SamePartyCookieContextType::kCrossParty;
-}
-
 }  // namespace
 
 namespace net {
@@ -582,8 +559,8 @@ void URLRequestHttpJob::AddCookieHeaderAndStart() {
         cookie_store->cookie_access_delegate();
 
     CookieOptions::SamePartyCookieContextType same_party_context =
-        ComputeSamePartyContext(request_site, request_->isolation_info(),
-                                delegate);
+        net::cookie_util::ComputeSamePartyContext(
+            request_site, request_->isolation_info(), delegate);
     bool is_in_nontrivial_first_party_set =
         delegate && delegate->IsInNontrivialFirstPartySet(request_site);
     CookieOptions options = CreateCookieOptions(
@@ -729,12 +706,12 @@ void URLRequestHttpJob::SaveCookiesAndNotifyHeadersComplete(int result) {
 
   const CookieAccessDelegate* delegate = cookie_store->cookie_access_delegate();
   net::SchemefulSite request_site(request_->url());
+
   CookieOptions::SamePartyCookieContextType same_party_context =
-      ComputeSamePartyContext(request_site, request_->isolation_info(),
-                              delegate);
+      net::cookie_util::ComputeSamePartyContext(
+          request_site, request_->isolation_info(), delegate);
   bool is_in_nontrivial_first_party_set =
       delegate && delegate->IsInNontrivialFirstPartySet(request_site);
-
   CookieOptions options = CreateCookieOptions(
       same_site_context, same_party_context, request_->isolation_info(),
       is_in_nontrivial_first_party_set);

services/network/restricted_cookie_manager.cc

@@ -40,7 +40,9 @@ net::CookieOptions MakeOptionsForSet(
     mojom::RestrictedCookieManagerRole role,
     const GURL& url,
     const net::SiteForCookies& site_for_cookies,
-    const CookieSettings* cookie_settings) {
+    const net::IsolationInfo& isolation_info,
+    const CookieSettings* cookie_settings,
+    const net::CookieAccessDelegate* cookie_access_delegate) {
   net::CookieOptions options;
   bool force_ignore_site_for_cookies =
       cookie_settings->ShouldIgnoreSameSiteRestrictions(
@@ -57,6 +59,16 @@ net::CookieOptions MakeOptionsForSet(
         net::cookie_util::ComputeSameSiteContextForSubresource(
             url, site_for_cookies, force_ignore_site_for_cookies));
   }
+  net::SchemefulSite request_site(url);
+  options.set_same_party_cookie_context_type(
+      net::cookie_util::ComputeSamePartyContext(request_site, isolation_info,
+                                                cookie_access_delegate));
+  bool is_in_nontrivial_first_party_set =
+      cookie_access_delegate &&
+      cookie_access_delegate->IsInNontrivialFirstPartySet(request_site);
+  options.set_is_in_nontrivial_first_party_set(
+      is_in_nontrivial_first_party_set);
+
   return options;
 }
 
@@ -64,7 +76,9 @@ net::CookieOptions MakeOptionsForGet(
     mojom::RestrictedCookieManagerRole role,
     const GURL& url,
     const net::SiteForCookies& site_for_cookies,
-    const CookieSettings* cookie_settings) {
+    const net::IsolationInfo& isolation_info,
+    const CookieSettings* cookie_settings,
+    const net::CookieAccessDelegate* cookie_access_delegate) {
   // TODO(https://crbug.com/925311): Wire initiator here.
   net::CookieOptions options;
   bool force_ignore_site_for_cookies =
@@ -83,6 +97,16 @@ net::CookieOptions MakeOptionsForGet(
         net::cookie_util::ComputeSameSiteContextForSubresource(
             url, site_for_cookies, force_ignore_site_for_cookies));
   }
+  net::SchemefulSite request_site(url);
+  options.set_same_party_cookie_context_type(
+      net::cookie_util::ComputeSamePartyContext(request_site, isolation_info,
+                                                cookie_access_delegate));
+  bool is_in_nontrivial_first_party_set =
+      cookie_access_delegate &&
+      cookie_access_delegate->IsInNontrivialFirstPartySet(request_site);
+  options.set_is_in_nontrivial_first_party_set(
+      is_in_nontrivial_first_party_set);
+
   return options;
 }
 
@@ -202,6 +226,7 @@ RestrictedCookieManager::RestrictedCookieManager(
       origin_(isolation_info.frame_origin().value()),
       site_for_cookies_(isolation_info.site_for_cookies()),
       top_frame_origin_(isolation_info.top_frame_origin().value()),
+      isolation_info_(isolation_info),
       cookie_observer_(std::move(cookie_observer)) {
   DCHECK(cookie_store);
 }
@@ -233,8 +258,9 @@ void RestrictedCookieManager::GetAllForUrl(
 
   // TODO(morlovich): Try to validate site_for_cookies as well.
 
-  net::CookieOptions net_options =
-      MakeOptionsForGet(role_, url, site_for_cookies, cookie_settings());
+  net::CookieOptions net_options = MakeOptionsForGet(
+      role_, url, site_for_cookies, isolation_info_, cookie_settings(),
+      cookie_store_->cookie_access_delegate());
   // TODO(https://crbug.com/977040): remove set_return_excluded_cookies() once
   //                                 removing deprecation warnings.
   net_options.set_return_excluded_cookies();
@@ -376,8 +402,9 @@ void RestrictedCookieManager::SetCanonicalCookie(
   DCHECK(sanitized_cookie);
   net::CanonicalCookie cookie_copy = *sanitized_cookie;
 
-  net::CookieOptions options =
-      MakeOptionsForSet(role_, url, site_for_cookies, cookie_settings());
+  net::CookieOptions options = MakeOptionsForSet(
+      role_, url, site_for_cookies, isolation_info_, cookie_settings(),
+      cookie_store_->cookie_access_delegate());
   // TODO(chlily): |url| is validated to be the same origin as |origin_|, but
   // the path is not checked. If we ever decide to enforce the path constraint
   // for setting a cookie, we would need to validate the path of |url| somehow
@@ -425,8 +452,9 @@ void RestrictedCookieManager::AddChangeListener(
     return;
   }
 
-  net::CookieOptions net_options =
-      MakeOptionsForGet(role_, url, site_for_cookies, cookie_settings());
+  net::CookieOptions net_options = MakeOptionsForGet(
+      role_, url, site_for_cookies, isolation_info_, cookie_settings(),
+      cookie_store_->cookie_access_delegate());
   auto listener = std::make_unique<Listener>(
       cookie_store_, this, url, site_for_cookies, top_frame_origin, net_options,
       std::move(mojo_listener));

services/network/restricted_cookie_manager.h

@@ -61,6 +61,13 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) RestrictedCookieManager
       const url::Origin& new_top_frame_origin) {
     top_frame_origin_ = new_top_frame_origin;
   }
+  void OverrideIsolationInfoForTesting(
+      const net::IsolationInfo& new_isolation_info) {
+    site_for_cookies_ = new_isolation_info.site_for_cookies();
+    origin_ = new_isolation_info.frame_origin().value();
+    top_frame_origin_ = new_isolation_info.top_frame_origin().value();
+    isolation_info_ = new_isolation_info;
+  }
 
   const CookieSettings* cookie_settings() const { return cookie_settings_; }
 
@@ -144,9 +151,14 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) RestrictedCookieManager
   const mojom::RestrictedCookieManagerRole role_;
   net::CookieStore* const cookie_store_;
   const CookieSettings* const cookie_settings_;
+
+  // TODO(https://crbug/1166215): Consolidate these three fields since
+  // `isolation_info_` holds copy of those values.
   url::Origin origin_;
   net::SiteForCookies site_for_cookies_;
   url::Origin top_frame_origin_;
+
+  net::IsolationInfo isolation_info_;
   mojo::Remote<mojom::CookieAccessObserver> cookie_observer_;
 
   base::LinkedList<Listener> listeners_;