Add field to SiteForCookies to indicate schemeful same-site of ancestors

Add a new field to SiteforCookies which is used to indicate if |this|
would be the same when considered with schemeful same-site.

The field is updated in the browser and renderer when th computation for
a sub-frame's SiteForCookies is done.

This CL is groundwork for implementing schemeful same-site and thus
nothing is done with this new field yet.

Bug: 1030938
Change-Id: I3fe59fd58604b370e71f6b1d8edcd29d0f3d47a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2133237
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Lily Chen <chlily@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Steven Bingler <bingler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757419}

content/browser/frame_host/render_frame_host_impl.cc

@@ -2303,6 +2303,7 @@ net::IsolationInfo RenderFrameHostImpl::ComputeIsolationInfoInternal(
       return net::IsolationInfo::Create(redirect_mode, top_frame_origin,
                                         frame_origin, net::SiteForCookies());
     }
+    candidate.MarkIfCrossScheme(rfh->last_committed_origin_);
   }
 
   // If |redirect_mode| is kUpdateNothing, then IsolationInfo is being computed

content/browser/frame_host/render_frame_host_impl_browsertest.cc

@@ -3315,6 +3315,84 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
   SetBrowserClientForTesting(old_client);
 }
 
+// Test that when ancestor iframes differ in scheme that the SiteForCookies
+// state is updated accordingly.
+IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
+                       ComputeSiteForCookiesSchemefulIsSameForAncestorFrames) {
+  https_server()->ServeFilesFromSourceDirectory(GetTestDataFilePath());
+  https_server()->SetSSLConfig(net::EmbeddedTestServer::CERT_TEST_NAMES);
+  ASSERT_TRUE(https_server()->Start());
+
+  GURL url = https_server()->GetURL(
+      "a.test", "/cross_site_iframe_factory.html?a.test(a.test)");
+  GURL insecure_url = embedded_test_server()->GetURL(
+      "a.test", "/cross_site_iframe_factory.html?a.test(a.test(a.test))");
+  GURL other_url = https_server()->GetURL("c.test", "/");
+  EXPECT_TRUE(NavigateToURL(shell(), insecure_url));
+  {
+    WebContentsImpl* wc =
+        static_cast<WebContentsImpl*>(shell()->web_contents());
+    RenderFrameHostImpl* main_frame = wc->GetMainFrame();
+
+    EXPECT_EQ("a.test", main_frame->GetLastCommittedURL().host());
+    EXPECT_EQ("http", main_frame->frame_tree_node()->current_origin().scheme());
+    ASSERT_EQ(1u, main_frame->child_count());
+    FrameTreeNode* child = main_frame->child_at(0);
+    EXPECT_EQ("a.test", child->current_url().host());
+    EXPECT_EQ("http", child->current_origin().scheme());
+    ASSERT_EQ(1u, child->child_count());
+    FrameTreeNode* grandchild = child->child_at(0);
+    EXPECT_EQ("a.test", grandchild->current_url().host());
+
+    // Both the frames above grandchild are the same scheme, so
+    // SiteForCookies::schemefully_same() should indicate that.
+    EXPECT_TRUE(child->current_frame_host()
+                    ->ComputeSiteForCookiesForNavigation(other_url)
+                    .schemefully_same());
+    EXPECT_EQ("a.test", child->current_frame_host()
+                            ->ComputeSiteForCookiesForNavigation(other_url)
+                            .registrable_domain());
+
+    net::SiteForCookies grandchild_same_scheme =
+        grandchild->current_frame_host()->ComputeSiteForCookies();
+    EXPECT_TRUE(grandchild_same_scheme.schemefully_same());
+    EXPECT_EQ("a.test", grandchild_same_scheme.registrable_domain());
+
+    net::SiteForCookies grandchild_same_scheme_navigation =
+        grandchild->current_frame_host()->ComputeSiteForCookiesForNavigation(
+            other_url);
+    EXPECT_TRUE(grandchild_same_scheme_navigation.schemefully_same());
+    EXPECT_EQ("a.test", grandchild_same_scheme_navigation.registrable_domain());
+
+    // Navigate the middle child frame to https.
+    NavigateFrameToURL(child, url);
+    EXPECT_EQ("a.test", child->current_url().host());
+    EXPECT_EQ("https", child->current_origin().scheme());
+    EXPECT_EQ(1u, child->child_count());
+
+    grandchild = child->child_at(0);
+
+    // Now the frames above grandchild differ only in scheme. SiteForCookies
+    // should be the same except that schemefully_same() should be false.
+    net::SiteForCookies grandchild_cross_scheme =
+        grandchild->current_frame_host()->ComputeSiteForCookies();
+    EXPECT_FALSE(grandchild_cross_scheme.schemefully_same());
+    EXPECT_EQ("a.test", grandchild_cross_scheme.registrable_domain());
+
+    net::SiteForCookies grandchild_cross_scheme_navigation =
+        grandchild->current_frame_host()->ComputeSiteForCookiesForNavigation(
+            other_url);
+    EXPECT_FALSE(grandchild_cross_scheme_navigation.schemefully_same());
+    EXPECT_EQ("a.test",
+              grandchild_cross_scheme_navigation.registrable_domain());
+
+    // IsEquivalent() doesn't check schemefully_same.
+    EXPECT_TRUE(grandchild_cross_scheme.IsEquivalent(grandchild_same_scheme));
+    EXPECT_TRUE(grandchild_cross_scheme_navigation.IsEquivalent(
+        grandchild_same_scheme_navigation));
+  }
+}
+
 IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
                        ComputeSiteForCookiesForNavigationSandbox) {
   // Test sandboxed subframe.

net/cookies/site_for_cookies.cc

@@ -20,7 +20,7 @@ std::string RegistrableDomainOrHost(const std::string& host) {
 
 }  // namespace
 
-SiteForCookies::SiteForCookies() = default;
+SiteForCookies::SiteForCookies() : schemefully_same_(false) {}
 
 SiteForCookies::SiteForCookies(const SiteForCookies& other) = default;
 SiteForCookies::SiteForCookies(SiteForCookies&& other) = default;
@@ -35,6 +35,7 @@ SiteForCookies& SiteForCookies::operator=(SiteForCookies&& site_for_cookies) =
 // static
 bool SiteForCookies::FromWire(const std::string& scheme,
                               const std::string& registrable_domain,
+                              bool schemefully_same,
                               SiteForCookies* out) {
   // Make sure scheme meets precondition of methods like
   // GURL::SchemeIsCryptographic.
@@ -46,6 +47,8 @@ bool SiteForCookies::FromWire(const std::string& scheme,
   if (registrable_domain != candidate.registrable_domain_)
     return false;
 
+  candidate.schemefully_same_ = schemefully_same;
+
   *out = std::move(candidate);
   return true;
 }
@@ -65,8 +68,10 @@ SiteForCookies SiteForCookies::FromUrl(const GURL& url) {
 }
 
 std::string SiteForCookies::ToDebugString() const {
+  std::string same_scheme_string = schemefully_same_ ? "true" : "false";
   return base::StrCat({"SiteForCookies: {scheme=", scheme_,
-                       "; registrable_domain=", registrable_domain_, "}"});
+                       "; registrable_domain=", registrable_domain_,
+                       "; schemefully_same=", same_scheme_string, "}"});
 }
 
 bool SiteForCookies::IsFirstParty(const GURL& url) const {
@@ -91,6 +96,40 @@ bool SiteForCookies::IsEquivalent(const SiteForCookies& other) const {
   return registrable_domain_ == other.registrable_domain_;
 }
 
+void SiteForCookies::MarkIfCrossScheme(const url::Origin& other) {
+  // If |this| is IsNull() then |this| doesn't match anything which means that
+  // the scheme check is pointless. Also exit early if schemefully_same_ is
+  // already false.
+  if (IsNull() || !schemefully_same_)
+    return;
+
+  // Mark and return early if |other| is opaque. Opaque origins shouldn't match.
+  if (other.opaque()) {
+    schemefully_same_ = false;
+    return;
+  }
+  const std::string& other_scheme = other.scheme();
+
+  // Exact match case.
+  if (scheme_ == other_scheme)
+    return;
+
+  // ["https", "wss"] case.
+  if ((scheme_ == url::kHttpsScheme || scheme_ == url::kWssScheme) &&
+      (other_scheme == url::kHttpsScheme || other_scheme == url::kWssScheme)) {
+    return;
+  }
+
+  // ["http", "ws"] case.
+  if ((scheme_ == url::kHttpScheme || scheme_ == url::kWsScheme) &&
+      (other_scheme == url::kHttpScheme || other_scheme == url::kWsScheme)) {
+    return;
+  }
+
+  // The two are cross-scheme to each other.
+  schemefully_same_ = false;
+}
+
 GURL SiteForCookies::RepresentativeUrl() const {
   if (IsNull())
     return GURL();
@@ -101,6 +140,8 @@ GURL SiteForCookies::RepresentativeUrl() const {
 
 SiteForCookies::SiteForCookies(const std::string& scheme,
                                const std::string& host)
-    : scheme_(scheme), registrable_domain_(RegistrableDomainOrHost(host)) {}
+    : scheme_(scheme),
+      registrable_domain_(RegistrableDomainOrHost(host)),
+      schemefully_same_(!scheme.empty()) {}
 
 }  // namespace net

net/cookies/site_for_cookies.h

@@ -23,12 +23,8 @@ namespace net {
 // 2) They both have empty hostnames and equal schemes.
 // Invalid URLs are not first party to anything.
 //
-// TODO(morlovich): It may make sense to require scheme to match in case (1)
-// too, where the notion of matching makes http/https/ws/wss equivalent, but
-// all other schemes are distinct.
-//
-// This should wait until SiteForCookies type is used everywhere relevant, so
-// any changes are consistent.
+// TODO(crbug.com/1030938): For case 1 the schemes must be "https" & "wss",
+// "http" & "ws", or they must match exactly.
 class NET_EXPORT SiteForCookies {
  public:
   // Matches nothing.
@@ -50,6 +46,7 @@ class NET_EXPORT SiteForCookies {
   // did not lie, merely that they are well-formed.
   static bool FromWire(const std::string& scheme,
                        const std::string& registrable_domain,
+                       bool schemefully_same,
                        SiteForCookies* out);
 
   // If the origin is opaque, returns SiteForCookies that matches nothing.
@@ -71,6 +68,13 @@ class NET_EXPORT SiteForCookies {
   // as |this->IsFirstParty| (potentially none).
   bool IsEquivalent(const SiteForCookies& other) const;
 
+  // Clears the schemefully_same_ flag if |other|'s scheme is cross-scheme to
+  // |this|.
+  // Two schemes are considered the same (not cross-scheme) if they exactly
+  // match, they are both in ["https", "wss"], or they are both in ["http",
+  // "ws"]. All other cases are cross-scheme.
+  void MarkIfCrossScheme(const url::Origin& other);
+
   // Returns a URL that's first party to this SiteForCookies (an empty URL if
   // none) --- that is, it has the property that
   // site_for_cookies.IsEquivalent(
@@ -87,6 +91,10 @@ class NET_EXPORT SiteForCookies {
 
   const std::string& registrable_domain() const { return registrable_domain_; }
 
+  // Used for serialization/deserialization. This value is irrelevant if
+  // IsNull() is true.
+  bool schemefully_same() const { return schemefully_same_; }
+
   // Returns true if this SiteForCookies matches nothing.
   bool IsNull() const { return scheme_.empty(); }
 
@@ -102,6 +110,25 @@ class NET_EXPORT SiteForCookies {
   // just the bare hostname or IP, or an empty string if this represents
   // something like file:///
   std::string registrable_domain_;
+
+  // Used to indicate if the SiteForCookies would be the same if computed
+  // schemefully. A schemeful computation means to take the |scheme_| as well as
+  // the |registrable_domain_| into account when determining first-partyness.
+  // See MarkIfCrossScheme() for more information on scheme comparison.
+  //
+  // True means to treat |this| as-is while false means that |this| should be
+  // treated as if it matches nothing i.e. as if IsNull() returned true.
+  //
+  // This value is important in the case where the SiteForCookies is being used
+  // to assess the first-partyness of a sub-frame in a document.
+  //
+  // For a SiteForCookies with !scheme_.empty() this value starts as true and
+  // will only go false via MarkIfCrossScheme(), otherwise this value is
+  // irrelevant.
+  //
+  // TODO(https://crbug.com/1030938): Actually use this for decisions in other
+  // functions.
+  bool schemefully_same_;
 };
 
 }  // namespace net