SiteForCookies uses scheme when SchemefulSameSite is enabled

SiteForCookies's function will, conditionally on the SchemefulSameSite
feature, take scheme of the URLs into account when determining
first-partyness and equivalency.

Schemefully_same_ will also be used in determining IsNull() when
SchemefulSameSite is enabled.

Bug: 1030938
Change-Id: I4599555faddf3f294144023af50c1a0ade67cca0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2144258
Commit-Queue: Steven Bingler <bingler@chromium.org>
Reviewed-by: Lily Chen <chlily@chromium.org>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758224}

net/cookies/cookie_util.cc

@@ -552,6 +552,10 @@ bool IsCookiesWithoutSameSiteMustBeSecureEnabled() {
              features::kCookiesWithoutSameSiteMustBeSecure);
 }
 
+bool IsSchemefulSameSiteEnabled() {
+  return base::FeatureList::IsEnabled(features::kSchemefulSameSite);
+}
+
 bool IsRecentHttpSameSiteAccessGrantsLegacyCookieSemanticsEnabled() {
   return IsSameSiteByDefaultCookiesEnabled() &&
          base::FeatureList::IsEnabled(

net/cookies/cookie_util.h

@@ -160,6 +160,7 @@ ComputeSameSiteContextForSubresource(const GURL& url,
 // Returns whether the respective SameSite feature is enabled.
 NET_EXPORT bool IsSameSiteByDefaultCookiesEnabled();
 NET_EXPORT bool IsCookiesWithoutSameSiteMustBeSecureEnabled();
+NET_EXPORT bool IsSchemefulSameSiteEnabled();
 bool IsRecentHttpSameSiteAccessGrantsLegacyCookieSemanticsEnabled();
 bool IsRecentCreationTimeGrantsLegacyCookieSemanticsEnabled();
 

net/cookies/site_for_cookies.cc

@@ -7,6 +7,7 @@
 #include "base/strings/strcat.h"
 #include "base/strings/string_util.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
+#include "net/cookies/cookie_util.h"
 
 namespace net {
 
@@ -75,21 +76,21 @@ std::string SiteForCookies::ToDebugString() const {
 }
 
 bool SiteForCookies::IsFirstParty(const GURL& url) const {
-  if (IsNull() || !url.is_valid())
-    return false;
-
-  std::string other_registrable_domain = RegistrableDomainOrHost(url.host());
-
-  if (registrable_domain_.empty())
-    return other_registrable_domain.empty() && (scheme_ == url.scheme());
+  if (cookie_util::IsSchemefulSameSiteEnabled())
+    return IsSchemefullyFirstParty(url);
 
-  return registrable_domain_ == other_registrable_domain;
+  return IsSchemelesslyFirstParty(url);
 }
 
 bool SiteForCookies::IsEquivalent(const SiteForCookies& other) const {
   if (IsNull())
     return other.IsNull();
 
+  if (cookie_util::IsSchemefulSameSiteEnabled() &&
+      !CompatibleScheme(other.scheme())) {
+    return false;
+  }
+
   if (registrable_domain_.empty())
     return other.registrable_domain_.empty() && (scheme_ == other.scheme_);
 
@@ -103,29 +104,15 @@ void SiteForCookies::MarkIfCrossScheme(const url::Origin& other) {
   if (IsNull() || !schemefully_same_)
     return;
 
-  // Mark and return early if |other| is opaque. Opaque origins shouldn't match.
+  // Mark 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)
+  if (CompatibleScheme(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;
 }
@@ -138,10 +125,63 @@ GURL SiteForCookies::RepresentativeUrl() const {
   return result;
 }
 
+bool SiteForCookies::IsNull() const {
+  if (cookie_util::IsSchemefulSameSiteEnabled())
+    return scheme_.empty() || !schemefully_same_;
+
+  return scheme_.empty();
+}
+
+bool SiteForCookies::IsSchemefullyFirstParty(const GURL& url) const {
+  // Can't use IsNull() as we want the same behavior regardless of
+  // SchemefulSameSite feature status.
+  if (scheme_.empty() || !schemefully_same_ || !url.is_valid())
+    return false;
+
+  return CompatibleScheme(url.scheme()) && IsSchemelesslyFirstParty(url);
+}
+
+bool SiteForCookies::IsSchemelesslyFirstParty(const GURL& url) const {
+  // Can't use IsNull() as we want the same behavior regardless of
+  // SchemefulSameSite feature status.
+  if (scheme_.empty() || !url.is_valid())
+    return false;
+
+  std::string other_registrable_domain = RegistrableDomainOrHost(url.host());
+
+  if (registrable_domain_.empty())
+    return other_registrable_domain.empty() && (scheme_ == url.scheme());
+
+  return registrable_domain_ == other_registrable_domain;
+}
+
 SiteForCookies::SiteForCookies(const std::string& scheme,
                                const std::string& host)
     : scheme_(scheme),
       registrable_domain_(RegistrableDomainOrHost(host)),
       schemefully_same_(!scheme.empty()) {}
 
+bool SiteForCookies::CompatibleScheme(const std::string& other_scheme) const {
+  DCHECK(base::IsStringASCII(other_scheme));
+  DCHECK(base::ToLowerASCII(other_scheme) == other_scheme);
+
+  // Exact match case.
+  if (scheme_ == other_scheme)
+    return true;
+
+  // ["https", "wss"] case.
+  if ((scheme_ == url::kHttpsScheme || scheme_ == url::kWssScheme) &&
+      (other_scheme == url::kHttpsScheme || other_scheme == url::kWssScheme)) {
+    return true;
+  }
+
+  // ["http", "ws"] case.
+  if ((scheme_ == url::kHttpScheme || scheme_ == url::kWsScheme) &&
+      (other_scheme == url::kHttpScheme || other_scheme == url::kWsScheme)) {
+    return true;
+  }
+
+  return false;
+}
+
 }  // namespace net

net/cookies/site_for_cookies.h

@@ -23,8 +23,13 @@ namespace net {
 // 2) They both have empty hostnames and equal schemes.
 // Invalid URLs are not first party to anything.
 //
-// TODO(crbug.com/1030938): For case 1 the schemes must be "https" & "wss",
-// "http" & "ws", or they must match exactly.
+// With the SchemefulSameSite feature enabled the policy is that two valid URLs
+// would be considered the same party if either:
+// 1) They both have compatible schemes along with non-empty and equal
+// registrable domains or hostnames/IPs. See CompatibleScheme() for more details
+// on what it means to have a compatible scheme.
+// 2) They both have empty hostnames and exactly equal schemes. Invalid URLs are
+// not first party to anything.
 class NET_EXPORT SiteForCookies {
  public:
   // Matches nothing.
@@ -58,6 +63,8 @@ class NET_EXPORT SiteForCookies {
   // Equivalent to FromOrigin(url::Origin::Create(url)).
   static SiteForCookies FromUrl(const GURL& url);
 
+  // Returns a string with the values of the member variables.
+  // |schemefully_same| being false does not change the output.
   std::string ToDebugString() const;
 
   // Returns true if |url| should be considered first-party to the context
@@ -69,10 +76,7 @@ class NET_EXPORT SiteForCookies {
   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.
+  // |this|. Schemes are considered cross-scheme if they're !CompatibleScheme().
   void MarkIfCrossScheme(const url::Origin& other);
 
   // Returns a URL that's first party to this SiteForCookies (an empty URL if
@@ -96,11 +100,36 @@ class NET_EXPORT SiteForCookies {
   bool schemefully_same() const { return schemefully_same_; }
 
   // Returns true if this SiteForCookies matches nothing.
-  bool IsNull() const { return scheme_.empty(); }
+  // If the SchemefulSameSite feature is enabled then !schemefully_same_ causes
+  // this function to return true.
+  bool IsNull() const;
+
+  // Don't use this function unless you know what you're doing, if you're unsure
+  // you probably want IsFirstParty().
+  //
+  // Returns true if |url| should be considered first-party to the context
+  // |this| represents when the compatibility of the schemes are taken into
+  // account. See CompatibleScheme().
+  bool IsSchemefullyFirstParty(const GURL& url) const;
+
+  // Don't use this function unless you know what you're doing, if you're unsure
+  // you probably want IsFirstParty().
+  //
+  // Returns true if |url| should be considered first-party to the context
+  // |this| represents when the compatibility of the scheme are not taken into
+  // account. See CompatibleScheme().
+  //
+  // Note that schemes are still compared for exact equality if neither |this|
+  // nor |url| have a registered domain.
+  bool IsSchemelesslyFirstParty(const GURL& url) const;
 
  private:
   SiteForCookies(const std::string& scheme, const std::string& host);
 
+  // Two schemes are considered compatible if they exactly match, they are both
+  // in ["https", "wss"], or they are both in ["http", "ws"].
+  bool CompatibleScheme(const std::string& other_scheme) const;
+
   // These should be canonicalized appropriately by GURL/url::Origin.
   // An empty |scheme_| means that this matches nothing.
   std::string scheme_;
@@ -114,10 +143,10 @@ class NET_EXPORT SiteForCookies {
   // 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.
+  // See CompatibleScheme() 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.
+  // treated as if it matches nothing i.e. IsNull() returns 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.
@@ -125,9 +154,6 @@ class NET_EXPORT SiteForCookies {
   // 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_;
 };
 

net/cookies/site_for_cookies_unittest.cc

@@ -7,6 +7,8 @@
 #include <vector>
 
 #include "base/strings/strcat.h"
+#include "base/test/scoped_feature_list.h"
+#include "net/base/features.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 #include "url/origin.h"
@@ -15,6 +17,16 @@
 namespace net {
 namespace {
 
+class SchemefulSiteForCookiesTest : public ::testing::Test {
+ public:
+  SchemefulSiteForCookiesTest() {
+    scope_feature_list_.InitAndEnableFeature(features::kSchemefulSameSite);
+  }
+
+ protected:
+  base::test::ScopedFeatureList scope_feature_list_;
+};
+
 // Tests that all URLs from |equivalent| produce SiteForCookies that match
 // URLs in the set and are equivalent to each other, and are distinct and
 // don't match |distinct|.
@@ -90,6 +102,38 @@ TEST(SiteForCookiesTest, Basic) {
   TestEquivalentAndDistinct(equivalent, distinct, "example.com");
 }
 
+// Similar to SiteForCookiesTest_Basic with a focus on testing secure SFCs.
+TEST_F(SchemefulSiteForCookiesTest, BasicSecure) {
+  std::vector<GURL> equivalent = {GURL("https://example.com"),
+                                  GURL("wss://example.com"),
+                                  GURL("https://sub1.example.com:42/something"),
+                                  GURL("wss://sub2.example.com/something")};
+
+  std::vector<GURL> distinct = {
+      GURL("http://example.com"),      GURL("https://example.org"),
+      GURL("ws://example.com"),        GURL("https://com/i_am_a_tld"),
+      GURL("file://example.com/helo"),
+  };
+
+  TestEquivalentAndDistinct(equivalent, distinct, "example.com");
+}
+
+// Similar to SiteForCookiesTest_Basic with a focus on testing insecure SFCs.
+TEST_F(SchemefulSiteForCookiesTest, BasicInsecure) {
+  std::vector<GURL> equivalent = {GURL("http://example.com"),
+                                  GURL("ws://example.com"),
+                                  GURL("http://sub1.example.com:42/something"),
+                                  GURL("ws://sub2.example.com/something")};
+
+  std::vector<GURL> distinct = {
+      GURL("https://example.com"),     GURL("http://example.org"),
+      GURL("wss://example.com"),       GURL("http://com/i_am_a_tld"),
+      GURL("file://example.com/helo"),
+  };
+
+  TestEquivalentAndDistinct(equivalent, distinct, "example.com");
+}
+
 TEST(SiteForCookiesTest, File) {
   std::vector<GURL> equivalent = {GURL("file:///a/b/c"),
                                   GURL("file:///etc/shaaadow")};
@@ -104,6 +148,7 @@ TEST(SiteForCookiesTest, Extension) {
   url::AddStandardScheme("chrome-extension", url::SCHEME_WITH_HOST);
   std::vector<GURL> equivalent = {GURL("chrome-extension://abc/"),
                                   GURL("chrome-extension://abc/foo.txt"),
+                                  GURL("https://abc"), GURL("http://abc"),
                                   // This one is disputable.
                                   GURL("file://abc/bar.txt")};
 
@@ -112,6 +157,23 @@ TEST(SiteForCookiesTest, Extension) {
   TestEquivalentAndDistinct(equivalent, distinct, "abc");
 }
 
+// Similar to SiteForCookiesTest_Extension with a focus on ensuring that http(s)
+// schemes are distinct.
+TEST_F(SchemefulSiteForCookiesTest, Extension) {
+  url::ScopedSchemeRegistryForTests scoped_registry;
+  url::AddStandardScheme("chrome-extension", url::SCHEME_WITH_HOST);
+  std::vector<GURL> equivalent = {
+      GURL("chrome-extension://abc/"),
+      GURL("chrome-extension://abc/foo.txt"),
+  };
+
+  std::vector<GURL> distinct = {GURL("chrome-extension://def"),
+                                GURL("https://abc"), GURL("http://abc"),
+                                GURL("file://abc/bar.txt")};
+
+  TestEquivalentAndDistinct(equivalent, distinct, "abc");
+}
+
 TEST(SiteForCookiesTest, NonStandard) {
   // If we don't register the scheme, nothing matches, even identical ones
   std::vector<GURL> equivalent;
@@ -144,6 +206,45 @@ TEST(SiteForCookiesTest, Blob) {
       SiteForCookies::FromUrl(GURL("http://www.example.org:631"))));
 }
 
+// Similar to SiteForCookiesTest_Blob with a focus on a secure blob.
+TEST_F(SchemefulSiteForCookiesTest, SecureBlob) {
+  SiteForCookies from_blob = SiteForCookies::FromUrl(
+      GURL("blob:https://example.org/9115d58c-bcda-ff47-86e5-083e9a2153041"));
+
+  EXPECT_TRUE(from_blob.IsFirstParty(GURL("https://sub.example.org/resource")));
+  EXPECT_FALSE(from_blob.IsFirstParty(GURL("http://sub.example.org/resource")));
+  EXPECT_EQ("https", from_blob.scheme());
+  EXPECT_EQ(
+      "SiteForCookies: {scheme=https; registrable_domain=example.org; "
+      "schemefully_same=true}",
+      from_blob.ToDebugString());
+  EXPECT_EQ("https://example.org/", from_blob.RepresentativeUrl().spec());
+  EXPECT_TRUE(from_blob.IsEquivalent(
+      SiteForCookies::FromUrl(GURL("https://www.example.org:631"))));
+  EXPECT_FALSE(from_blob.IsEquivalent(
+      SiteForCookies::FromUrl(GURL("http://www.example.org:631"))));
+}
+
+// Similar to SiteForCookiesTest_Blob with a focus on an insecure blob.
+TEST_F(SchemefulSiteForCookiesTest, InsecureBlob) {
+  SiteForCookies from_blob = SiteForCookies::FromUrl(
+      GURL("blob:http://example.org/9115d58c-bcda-ff47-86e5-083e9a2153041"));
+
+  EXPECT_TRUE(from_blob.IsFirstParty(GURL("http://sub.example.org/resource")));
+  EXPECT_FALSE(
+      from_blob.IsFirstParty(GURL("https://sub.example.org/resource")));
+  EXPECT_EQ("http", from_blob.scheme());
+  EXPECT_EQ(
+      "SiteForCookies: {scheme=http; registrable_domain=example.org; "
+      "schemefully_same=true}",
+      from_blob.ToDebugString());
+  EXPECT_EQ("http://example.org/", from_blob.RepresentativeUrl().spec());
+  EXPECT_TRUE(from_blob.IsEquivalent(
+      SiteForCookies::FromUrl(GURL("http://www.example.org:631"))));
+  EXPECT_FALSE(from_blob.IsEquivalent(
+      SiteForCookies::FromUrl(GURL("https://www.example.org:631"))));
+}
+
 TEST(SiteForCookiesTest, Wire) {
   SiteForCookies out;
 
@@ -187,6 +288,48 @@ TEST(SiteForCookiesTest, Wire) {
       out.ToDebugString());
 }
 
+// Similar to SiteForCookiesTest_Wire except that schemefully_same has an
+// effect.
+TEST_F(SchemefulSiteForCookiesTest, Wire) {
+  SiteForCookies out;
+
+  // Empty one.
+  EXPECT_TRUE(SiteForCookies::FromWire("", "", false, &out));
+  EXPECT_TRUE(out.IsNull());
+
+  EXPECT_TRUE(SiteForCookies::FromWire("", "", true, &out));
+  EXPECT_TRUE(out.IsNull());
+
+  // Not a valid scheme.
+  EXPECT_FALSE(SiteForCookies::FromWire("aH", "example.com", false, &out));
+  EXPECT_TRUE(out.IsNull());
+
+  // Not a eTLD + 1 (or something hosty).
+  EXPECT_FALSE(
+      SiteForCookies::FromWire("http", "sub.example.com", false, &out));
+  EXPECT_TRUE(out.IsNull());
+
+  // This is fine, though.
+  EXPECT_TRUE(SiteForCookies::FromWire("https", "127.0.0.1", true, &out));
+  EXPECT_FALSE(out.IsNull());
+  EXPECT_EQ(
+      "SiteForCookies: {scheme=https; registrable_domain=127.0.0.1; "
+      "schemefully_same=true}",
+      out.ToDebugString());
+
+  // This one's schemefully_same is false
+  EXPECT_TRUE(SiteForCookies::FromWire("https", "127.0.0.1", false, &out));
+  EXPECT_TRUE(out.IsNull());
+
+  // As is actual eTLD+1.
+  EXPECT_TRUE(SiteForCookies::FromWire("wss", "example.com", true, &out));
+  EXPECT_FALSE(out.IsNull());
+  EXPECT_EQ(
+      "SiteForCookies: {scheme=wss; registrable_domain=example.com; "
+      "schemefully_same=true}",
+      out.ToDebugString());
+}
+
 TEST(SiteForCookiesTest, SameScheme) {
   struct TestCase {
     const char* first;