Calculate cross-schemeness for CROSS_SITE

Expand the computation of cross schemeness of same-site contexts to
include CROSS_SITE.

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

net/cookies/canonical_cookie.cc

@@ -148,6 +148,29 @@ void ApplySameSiteCookieWarningToStatus(
   status->MaybeClearSameSiteWarning();
 }
 
+// This function is used to indicate if the same-site context of a cookie should
+// recorded for the histograms SameSiteDifferentSchemeRequest and
+// SameSiteDifferentSchemeResponse. It returns true if the context is
+// cross-scheme but not cross-site and there is an effective same-site. It
+// should be removed when the histrograms are removed.
+// TODO(https://crbug.com/1066231)
+bool ShouldLogCrossSchemeForHistograms(
+    const CookieOptions::SameSiteCookieContext& context,
+    const CookieEffectiveSameSite effective_same_site) {
+  bool correct_context =
+      context.cross_schemeness !=
+          CookieOptions::SameSiteCookieContext::CrossSchemeness::NONE &&
+      context.context !=
+          CookieOptions::SameSiteCookieContext::ContextType::CROSS_SITE;
+
+  bool correct_effective_same_site =
+      effective_same_site == CookieEffectiveSameSite::LAX_MODE ||
+      effective_same_site == CookieEffectiveSameSite::STRICT_MODE ||
+      effective_same_site == CookieEffectiveSameSite::LAX_MODE_ALLOW_UNSAFE;
+
+  return correct_context && correct_effective_same_site;
+}
+
 }  // namespace
 
 // Keep defaults here in sync with content/public/common/cookie_manager.mojom.
@@ -553,11 +576,9 @@ CanonicalCookie::CookieInclusionStatus CanonicalCookie::IncludeForRequestURL(
                               effective_same_site,
                               CookieEffectiveSameSite::COUNT);
 
-    if (options.same_site_cookie_context().IsDifferentScheme() &&
-        ((effective_same_site == CookieEffectiveSameSite::LAX_MODE) ||
-         (effective_same_site == CookieEffectiveSameSite::STRICT_MODE) ||
-         (effective_same_site ==
-          CookieEffectiveSameSite::LAX_MODE_ALLOW_UNSAFE))) {
+    if (ShouldLogCrossSchemeForHistograms(options.same_site_cookie_context(),
+                                          effective_same_site)) {
+      // TODO(https://crbug.com/1066231)
       UMA_HISTOGRAM_ENUMERATION(
           "Cookie.SameSiteDifferentSchemeRequest",
           options.same_site_cookie_context().ConvertToMetricsValue(),
@@ -653,13 +674,11 @@ void CanonicalCookie::IsSetPermittedInContext(
                               effective_same_site,
                               CookieEffectiveSameSite::COUNT);
 
-    if (options.same_site_cookie_context().IsDifferentScheme() &&
-        ((effective_same_site == CookieEffectiveSameSite::LAX_MODE) ||
-         (effective_same_site == CookieEffectiveSameSite::STRICT_MODE) ||
-         (effective_same_site ==
-          CookieEffectiveSameSite::LAX_MODE_ALLOW_UNSAFE))) {
+    if (ShouldLogCrossSchemeForHistograms(options.same_site_cookie_context(),
+                                          effective_same_site)) {
       // TODO(crbug.com/1034014): Change enum to one with less confusing
       // phrasing.
+      // TODO(https://crbug.com/1066231)
       UMA_HISTOGRAM_ENUMERATION(
           "Cookie.SameSiteDifferentSchemeResponse",
           options.same_site_cookie_context().ConvertToMetricsValue(),

net/cookies/cookie_options.h

@@ -48,10 +48,6 @@ class NET_EXPORT CookieOptions {
         CrossSchemeness cross_schemeness = CrossSchemeness::NONE)
         : context(same_site_context), cross_schemeness(cross_schemeness) {}
 
-    bool IsDifferentScheme() const {
-      return cross_schemeness != SameSiteCookieContext::CrossSchemeness::NONE;
-    }
-
     // Convenience method which returns a SameSiteCookieContext with the most
     // inclusive context. This allows access to all SameSite cookies.
     static SameSiteCookieContext MakeInclusive();

net/cookies/cookie_util.cc

@@ -75,11 +75,8 @@ bool SaturatedTimeFromUTCExploded(const base::Time::Exploded& exploded,
 }
 
 CookieOptions::SameSiteCookieContext::CrossSchemeness ComputeSchemeChange(
-    CookieOptions::SameSiteCookieContext same_site_type,
     const GURL& url,
     const SiteForCookies& site_for_cookies) {
-  DCHECK(same_site_type.context >=
-         CookieOptions::SameSiteCookieContext::ContextType::SAME_SITE_LAX);
 
   CookieOptions::SameSiteCookieContext::CrossSchemeness cross_schemeness =
       CookieOptions::SameSiteCookieContext::CrossSchemeness::NONE;
@@ -115,11 +112,8 @@ CookieOptions::SameSiteCookieContext ComputeSameSiteContext(
       same_site_type.context =
           CookieOptions::SameSiteCookieContext::ContextType::SAME_SITE_LAX;
     }
-
-    same_site_type.cross_schemeness =
-        ComputeSchemeChange(same_site_type, url, site_for_cookies);
   }
-
+  same_site_type.cross_schemeness = ComputeSchemeChange(url, site_for_cookies);
   return same_site_type;
 }
 
@@ -458,7 +452,7 @@ CookieOptions::SameSiteCookieContext ComputeSameSiteContextForRequest(
     same_site_context.context =
         CookieOptions::SameSiteCookieContext::ContextType::SAME_SITE_STRICT;
     same_site_context.cross_schemeness =
-        ComputeSchemeChange(same_site_context, url, site_for_cookies);
+        ComputeSchemeChange(url, site_for_cookies);
     return same_site_context;
   }
 
@@ -485,7 +479,7 @@ ComputeSameSiteContextForScriptGet(const GURL& url,
     CookieOptions::SameSiteCookieContext same_site_context(
         CookieOptions::SameSiteCookieContext::ContextType::SAME_SITE_STRICT);
     same_site_context.cross_schemeness =
-        ComputeSchemeChange(same_site_context, url, site_for_cookies);
+        ComputeSchemeChange(url, site_for_cookies);
     return same_site_context;
   }
   return ComputeSameSiteContext(url, site_for_cookies, initiator);
@@ -502,14 +496,13 @@ CookieOptions::SameSiteCookieContext ComputeSameSiteContextForResponse(
   if (attach_same_site_cookies || site_for_cookies.IsFirstParty(url)) {
     same_site_context.context =
         CookieOptions::SameSiteCookieContext::ContextType::SAME_SITE_LAX;
-    same_site_context.cross_schemeness =
-        ComputeSchemeChange(same_site_context, url, site_for_cookies);
-    return same_site_context;
   } else {
     same_site_context.context =
         CookieOptions::SameSiteCookieContext::ContextType::CROSS_SITE;
-    return same_site_context;
   }
+  same_site_context.cross_schemeness =
+      ComputeSchemeChange(url, site_for_cookies);
+  return same_site_context;
 }
 
 CookieOptions::SameSiteCookieContext ComputeSameSiteContextForScriptSet(
@@ -520,14 +513,13 @@ CookieOptions::SameSiteCookieContext ComputeSameSiteContextForScriptSet(
   if (attach_same_site_cookies || site_for_cookies.IsFirstParty(url)) {
     same_site_context.context =
         CookieOptions::SameSiteCookieContext::ContextType::SAME_SITE_LAX;
-    same_site_context.cross_schemeness =
-        ComputeSchemeChange(same_site_context, url, site_for_cookies);
-    return same_site_context;
   } else {
     same_site_context.context =
         CookieOptions::SameSiteCookieContext::ContextType::CROSS_SITE;
-    return same_site_context;
   }
+  same_site_context.cross_schemeness =
+      ComputeSchemeChange(url, site_for_cookies);
+  return same_site_context;
 }
 
 CookieOptions::SameSiteCookieContext ComputeSameSiteContextForSubresource(
@@ -540,14 +532,13 @@ CookieOptions::SameSiteCookieContext ComputeSameSiteContextForSubresource(
   if (attach_same_site_cookies || site_for_cookies.IsFirstParty(url)) {
     same_site_context.context =
         CookieOptions::SameSiteCookieContext::ContextType::SAME_SITE_STRICT;
-    same_site_context.cross_schemeness =
-        ComputeSchemeChange(same_site_context, url, site_for_cookies);
-    return same_site_context;
   } else {
     same_site_context.context =
         CookieOptions::SameSiteCookieContext::ContextType::CROSS_SITE;
-    return same_site_context;
   }
+  same_site_context.cross_schemeness =
+      ComputeSchemeChange(url, site_for_cookies);
+  return same_site_context;
 }
 
 bool IsSameSiteByDefaultCookiesEnabled() {

net/cookies/cookie_util_unittest.cc

@@ -263,6 +263,24 @@ TEST(CookieUtilTest, TestComputeSameSiteContextForScriptGet) {
           SiteForCookies::FromUrl(GURL("http://notexample.com")),
           base::nullopt /*initiator*/, false /* attach_same_site_cookies */));
 
+  EXPECT_EQ(
+      SameSiteCookieContext(
+          SameSiteCookieContext::ContextType::CROSS_SITE,
+          SameSiteCookieContext::CrossSchemeness::INSECURE_SECURE),
+      cookie_util::ComputeSameSiteContextForScriptGet(
+          GURL("https://example.com"),
+          SiteForCookies::FromUrl(GURL("http://notexample.com")),
+          base::nullopt /*initiator*/, false /* attach_same_site_cookies */));
+
+  EXPECT_EQ(
+      SameSiteCookieContext(
+          SameSiteCookieContext::ContextType::CROSS_SITE,
+          SameSiteCookieContext::CrossSchemeness::SECURE_INSECURE),
+      cookie_util::ComputeSameSiteContextForScriptGet(
+          GURL("http://example.com"),
+          SiteForCookies::FromUrl(GURL("https://notexample.com")),
+          base::nullopt /*initiator*/, false /* attach_same_site_cookies */));
+
   EXPECT_EQ(
       SameSiteCookieContext(SameSiteCookieContext::ContextType::CROSS_SITE),
       cookie_util::ComputeSameSiteContextForScriptGet(
@@ -271,6 +289,24 @@ TEST(CookieUtilTest, TestComputeSameSiteContextForScriptGet) {
           url::Origin::Create(GURL("http://example.com")),
           false /* attach_same_site_cookies */));
 
+  EXPECT_EQ(SameSiteCookieContext(
+                SameSiteCookieContext::ContextType::CROSS_SITE,
+                SameSiteCookieContext::CrossSchemeness::INSECURE_SECURE),
+            cookie_util::ComputeSameSiteContextForScriptGet(
+                GURL("https://example.com"),
+                SiteForCookies::FromUrl(GURL("http://notexample.com")),
+                url::Origin::Create(GURL("http://example.com")),
+                false /* attach_same_site_cookies */));
+
+  EXPECT_EQ(SameSiteCookieContext(
+                SameSiteCookieContext::ContextType::CROSS_SITE,
+                SameSiteCookieContext::CrossSchemeness::SECURE_INSECURE),
+            cookie_util::ComputeSameSiteContextForScriptGet(
+                GURL("http://example.com"),
+                SiteForCookies::FromUrl(GURL("https://notexample.com")),
+                url::Origin::Create(GURL("http://example.com")),
+                false /* attach_same_site_cookies */));
+
   EXPECT_EQ(
       SameSiteCookieContext(SameSiteCookieContext::ContextType::CROSS_SITE),
       cookie_util::ComputeSameSiteContextForScriptGet(
@@ -449,6 +485,23 @@ TEST(CookieUtilTest, ComputeSameSiteContextForRequest) {
           "GET", GURL("http://example.com"),
           SiteForCookies::FromUrl(GURL("http://notexample.com")),
           base::nullopt /*initiator*/, false /*attach_same_site_cookies*/));
+  EXPECT_EQ(
+      SameSiteCookieContext(
+          CookieOptions::SameSiteCookieContext::ContextType::CROSS_SITE,
+          SameSiteCookieContext::CrossSchemeness::INSECURE_SECURE),
+      cookie_util::ComputeSameSiteContextForRequest(
+          "GET", GURL("https://example.com"),
+          SiteForCookies::FromUrl(GURL("http://notexample.com")),
+          base::nullopt /*initiator*/, false /*attach_same_site_cookies*/));
+
+  EXPECT_EQ(
+      SameSiteCookieContext(
+          CookieOptions::SameSiteCookieContext::ContextType::CROSS_SITE,
+          SameSiteCookieContext::CrossSchemeness::SECURE_INSECURE),
+      cookie_util::ComputeSameSiteContextForRequest(
+          "GET", GURL("http://example.com"),
+          SiteForCookies::FromUrl(GURL("https://notexample.com")),
+          base::nullopt /*initiator*/, false /*attach_same_site_cookies*/));
 
   // |attach_same_site_cookies| = true bypasses all checks.
   EXPECT_EQ(
@@ -614,6 +667,22 @@ TEST(CookieUtilTest, ComputeSameSiteContextForSet) {
                 SiteForCookies::FromUrl(GURL("http://notexample.com")),
                 base::nullopt, false /* attach_same_site_cookies */));
 
+  EXPECT_EQ(SameSiteCookieContext(
+                CookieOptions::SameSiteCookieContext::ContextType::CROSS_SITE,
+                SameSiteCookieContext::CrossSchemeness::INSECURE_SECURE),
+            cookie_util::ComputeSameSiteContextForResponse(
+                GURL("https://example.com"),
+                SiteForCookies::FromUrl(GURL("http://notexample.com")),
+                base::nullopt, false /* attach_same_site_cookies */));
+
+  EXPECT_EQ(SameSiteCookieContext(
+                CookieOptions::SameSiteCookieContext::ContextType::CROSS_SITE,
+                SameSiteCookieContext::CrossSchemeness::SECURE_INSECURE),
+            cookie_util::ComputeSameSiteContextForResponse(
+                GURL("http://example.com"),
+                SiteForCookies::FromUrl(GURL("https://notexample.com")),
+                base::nullopt, false /* attach_same_site_cookies */));
+
   // Same as above except |attach_same_site_cookies| makes it return LAX.
   EXPECT_EQ(
       SameSiteCookieContext(
@@ -630,6 +699,22 @@ TEST(CookieUtilTest, ComputeSameSiteContextForSet) {
                 SiteForCookies::FromUrl(GURL("http://notexample.com")),
                 false /* attach_same_site_cookies */));
 
+  EXPECT_EQ(SameSiteCookieContext(
+                CookieOptions::SameSiteCookieContext::ContextType::CROSS_SITE,
+                SameSiteCookieContext::CrossSchemeness::INSECURE_SECURE),
+            cookie_util::ComputeSameSiteContextForScriptSet(
+                GURL("https://example.com"),
+                SiteForCookies::FromUrl(GURL("http://notexample.com")),
+                false /* attach_same_site_cookies */));
+
+  EXPECT_EQ(SameSiteCookieContext(
+                CookieOptions::SameSiteCookieContext::ContextType::CROSS_SITE,
+                SameSiteCookieContext::CrossSchemeness::SECURE_INSECURE),
+            cookie_util::ComputeSameSiteContextForScriptSet(
+                GURL("http://example.com"),
+                SiteForCookies::FromUrl(GURL("https://notexample.com")),
+                false /* attach_same_site_cookies */));
+
   // Same as above except |attach_same_site_cookies| makes it return LAX.
   EXPECT_EQ(
       SameSiteCookieContext(
@@ -715,6 +800,22 @@ TEST(CookieUtilTest, TestComputeSameSiteContextForSubresource) {
           SiteForCookies::FromUrl(GURL("http://notexample.com")),
           false /* attach_same_site_cookies */));
 
+  EXPECT_EQ(SameSiteCookieContext(
+                SameSiteCookieContext::ContextType::CROSS_SITE,
+                SameSiteCookieContext::CrossSchemeness::INSECURE_SECURE),
+            cookie_util::ComputeSameSiteContextForSubresource(
+                GURL("https://example.com"),
+                SiteForCookies::FromUrl(GURL("http://notexample.com")),
+                false /* attach_same_site_cookies */));
+
+  EXPECT_EQ(SameSiteCookieContext(
+                SameSiteCookieContext::ContextType::CROSS_SITE,
+                SameSiteCookieContext::CrossSchemeness::SECURE_INSECURE),
+            cookie_util::ComputeSameSiteContextForSubresource(
+                GURL("http://example.com"),
+                SiteForCookies::FromUrl(GURL("https://notexample.com")),
+                false /* attach_same_site_cookies */));
+
   // Same as above except |attach_same_site_cookies| makes it return STRICT.
   EXPECT_EQ(SameSiteCookieContext(
                 SameSiteCookieContext::ContextType::SAME_SITE_STRICT),