Add schemeful same-site to SameSite enterprise policy

Determine which cookie context, schemeless vs schemeful, to use based
on the LegacySameSiteCookieBehaviorEnabled and
LegacySameSiteCookieBehaviorEnabledForDomainList enterprise policies.

This functionality may also be triggered if SameSiteByDefaultCookies
is disabled. This seems acceptable because it's odd to decrease
SameSite protections by disabling SameSiteByDefaultCookies but also
want to increase those protections by having Schemeful Same-Site.

Also update the documentation for the policies.

Bug: 1101037
Change-Id: I6f74f6448147b451bbf8ae5aa0157277dd00639c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2283505Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Lily Chen <chlily@chromium.org>
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Commit-Queue: Steven Bingler <bingler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#789467}

chrome/browser/net/samesite_cookies_policy_browsertest.cc

@@ -26,15 +26,17 @@ const char kURL[] = "http://example.com";
 }  // namespace
 
 // Test fixture that enables (if param is true) and disables (if param is false)
-// SameSite-by-default and Cookies-without-SameSite-must-be-Secure, to test the
-// policies that override those features, under both conditions.
+// SameSite-by-default, Cookies-without-SameSite-must-be-Secure, and Schemeful
+// Same-Site to test the policies that override those features, under both
+// conditions.
 class SameSiteCookiesPolicyTest : public PolicyTest,
                                   public ::testing::WithParamInterface<bool> {
  public:
   SameSiteCookiesPolicyTest() {
     std::vector<base::Feature> samesite_features = {
         net::features::kSameSiteByDefaultCookies,
-        net::features::kCookiesWithoutSameSiteMustBeSecure};
+        net::features::kCookiesWithoutSameSiteMustBeSecure,
+        net::features::kSchemefulSameSite};
     if (AreSameSiteFeaturesEnabled()) {
       feature_list_.InitWithFeatures(samesite_features /* enabled */, {});
     } else {
@@ -90,6 +92,25 @@ IN_PROC_BROWSER_TEST_P(SameSiteCookiesPolicyTest,
                                 net::CookieOptions::SameSiteCookieContext(
                                     net::CookieOptions::SameSiteCookieContext::
                                         ContextType::CROSS_SITE)));
+
+  // When Schemeful Same-Site is enabled a context downgrade to an insufficient
+  // context should still be allowed with legacy access. This'll always work if
+  // Schemeful Same-Site is disabled because the schemeless context is Lax
+  // which is sufficient.
+  EXPECT_TRUE(content::SetCookie(
+      profile, url, "samesite-lax=1; SameSite=Lax",
+      net::CookieOptions::SameSiteCookieContext(
+          net::CookieOptions::SameSiteCookieContext::ContextType::SAME_SITE_LAX,
+          net::CookieOptions::SameSiteCookieContext::ContextType::CROSS_SITE)));
+  // Similarly when we try to get the cookie.
+  EXPECT_THAT(
+      content::GetCookies(profile, url,
+                          net::CookieOptions::SameSiteCookieContext(
+                              net::CookieOptions::SameSiteCookieContext::
+                                  ContextType::SAME_SITE_LAX,
+                              net::CookieOptions::SameSiteCookieContext::
+                                  ContextType::CROSS_SITE)),
+      testing::HasSubstr("samesite-lax=1"));
 }
 
 IN_PROC_BROWSER_TEST_P(SameSiteCookiesPolicyTest,
@@ -132,6 +153,48 @@ IN_PROC_BROWSER_TEST_P(SameSiteCookiesPolicyTest,
                                 net::CookieOptions::SameSiteCookieContext(
                                     net::CookieOptions::SameSiteCookieContext::
                                         ContextType::CROSS_SITE)));
+
+  // When Schemeful Same-Site is enabled a context downgrade to an insufficient
+  // context should always be blocked. If Schemeful Same-Site is disabled then
+  // this shouldn't be blocked.
+  // Similarly when we try to get the cookie.
+  if (AreSameSiteFeaturesEnabled()) {
+    EXPECT_FALSE(
+        content::SetCookie(profile, url, "samesite-lax=1; SameSite=Lax",
+                           net::CookieOptions::SameSiteCookieContext(
+                               net::CookieOptions::SameSiteCookieContext::
+                                   ContextType::SAME_SITE_LAX,
+                               net::CookieOptions::SameSiteCookieContext::
+                                   ContextType::CROSS_SITE)));
+    // We should be able to get the cookie which was previously added.
+    EXPECT_EQ("samesite-unspecified=1", content::GetCookies(profile, url));
+    // But no cookies should be returned for a downgrade to an insufficient
+    // context, since SameSite-by-default is active which requires a minimum of
+    // a Lax context.
+    EXPECT_EQ(
+        "", content::GetCookies(profile, url,
+                                net::CookieOptions::SameSiteCookieContext(
+                                    net::CookieOptions::SameSiteCookieContext::
+                                        ContextType::SAME_SITE_LAX,
+                                    net::CookieOptions::SameSiteCookieContext::
+                                        ContextType::CROSS_SITE)));
+  } else {
+    EXPECT_TRUE(
+        content::SetCookie(profile, url, "samesite-lax=1; SameSite=Lax",
+                           net::CookieOptions::SameSiteCookieContext(
+                               net::CookieOptions::SameSiteCookieContext::
+                                   ContextType::SAME_SITE_LAX,
+                               net::CookieOptions::SameSiteCookieContext::
+                                   ContextType::CROSS_SITE)));
+    EXPECT_THAT(
+        content::GetCookies(profile, url,
+                            net::CookieOptions::SameSiteCookieContext(
+                                net::CookieOptions::SameSiteCookieContext::
+                                    ContextType::SAME_SITE_LAX,
+                                net::CookieOptions::SameSiteCookieContext::
+                                    ContextType::CROSS_SITE)),
+        testing::HasSubstr("samesite-lax=1"));
+  }
 }
 
 IN_PROC_BROWSER_TEST_P(SameSiteCookiesPolicyTest,
@@ -190,6 +253,25 @@ IN_PROC_BROWSER_TEST_P(SameSiteCookiesPolicyTest,
                                 net::CookieOptions::SameSiteCookieContext(
                                     net::CookieOptions::SameSiteCookieContext::
                                         ContextType::CROSS_SITE)));
+  // When Schemeful Same-Site is enabled a context downgrade to an insufficient
+  // context should still be allowed with legacy access. This'll always work if
+  // Schemeful Same-Site is disabled because the schemeless context is Lax
+  // which is sufficient.
+  EXPECT_TRUE(content::SetCookie(
+      profile, legacy_allowed_domain_url, "samesite-lax=1; SameSite=Lax",
+      net::CookieOptions::SameSiteCookieContext(
+          net::CookieOptions::SameSiteCookieContext::ContextType::SAME_SITE_LAX,
+          net::CookieOptions::SameSiteCookieContext::ContextType::CROSS_SITE)));
+  // Similarly when we try to get the cookie.
+  EXPECT_THAT(
+      content::GetCookies(profile, legacy_allowed_domain_url,
+                          net::CookieOptions::SameSiteCookieContext(
+                              net::CookieOptions::SameSiteCookieContext::
+                                  ContextType::SAME_SITE_LAX,
+                              net::CookieOptions::SameSiteCookieContext::
+                                  ContextType::CROSS_SITE)),
+      testing::HasSubstr("samesite-lax=1"));
+
   // For the domain that is not Legacy by policy, we expect it to work only if
   // the SameSite features are disabled.
   if (AreSameSiteFeaturesEnabled()) {
@@ -205,6 +287,26 @@ IN_PROC_BROWSER_TEST_P(SameSiteCookiesPolicyTest,
                                 net::CookieOptions::SameSiteCookieContext(
                                     net::CookieOptions::SameSiteCookieContext::
                                         ContextType::CROSS_SITE)));
+    EXPECT_FALSE(content::SetCookie(
+        profile, other_domain_url, "samesite-lax=1; SameSite=Lax",
+        net::CookieOptions::SameSiteCookieContext(
+            net::CookieOptions::SameSiteCookieContext::ContextType::
+                SAME_SITE_LAX,
+            net::CookieOptions::SameSiteCookieContext::ContextType::
+                CROSS_SITE)));
+    // We should be able to get the cookie which was previously added.
+    EXPECT_EQ("samesite-unspecified=1",
+              content::GetCookies(profile, other_domain_url));
+    // But no cookies should be returned for a downgrade to an insufficient
+    // context, since SameSite-by-default is active which requires a minimum of
+    // a Lax context.
+    EXPECT_EQ(
+        "", content::GetCookies(profile, other_domain_url,
+                                net::CookieOptions::SameSiteCookieContext(
+                                    net::CookieOptions::SameSiteCookieContext::
+                                        ContextType::SAME_SITE_LAX,
+                                    net::CookieOptions::SameSiteCookieContext::
+                                        ContextType::CROSS_SITE)));
   } else {
     EXPECT_TRUE(
         content::SetCookie(profile, other_domain_url, "samesite-unspecified=2",
@@ -219,6 +321,22 @@ IN_PROC_BROWSER_TEST_P(SameSiteCookiesPolicyTest,
                             net::CookieOptions::SameSiteCookieContext(
                                 net::CookieOptions::SameSiteCookieContext::
                                     ContextType::CROSS_SITE)));
+
+    EXPECT_TRUE(content::SetCookie(
+        profile, other_domain_url, "samesite-lax=1; SameSite=Lax",
+        net::CookieOptions::SameSiteCookieContext(
+            net::CookieOptions::SameSiteCookieContext::ContextType::
+                SAME_SITE_LAX,
+            net::CookieOptions::SameSiteCookieContext::ContextType::
+                CROSS_SITE)));
+    EXPECT_THAT(
+        content::GetCookies(profile, other_domain_url,
+                            net::CookieOptions::SameSiteCookieContext(
+                                net::CookieOptions::SameSiteCookieContext::
+                                    ContextType::SAME_SITE_LAX,
+                                net::CookieOptions::SameSiteCookieContext::
+                                    ContextType::CROSS_SITE)),
+        testing::HasSubstr("samesite-lax=1"));
   }
 }
 

components/content_settings/core/common/content_settings_types.h

@@ -149,10 +149,12 @@ enum class ContentSettingsType : int32_t {
   WAKE_LOCK_SCREEN,
   WAKE_LOCK_SYSTEM,
 
-  // Legacy SameSite cookie behavior. This disables SameSiteByDefaultCookies
-  // and CookiesWithoutSameSiteMustBeSecure, and forces the legacy behavior
-  // where cookies that don't specify SameSite are treated as SameSite=None and
-  // SameSite=None cookies are not required to be Secure.
+  // Legacy SameSite cookie behavior. This disables SameSiteByDefaultCookies,
+  // CookiesWithoutSameSiteMustBeSecure, and SchemefulSameSite, forcing the
+  // legacy behavior wherein cookies that don't specify SameSite are treated as
+  // SameSite=None, SameSite=None cookies are not required to be Secure, and
+  // schemeful same-site is not active.
+  //
   // This will also be used to revert to legacy behavior when future changes
   // in cookie handling are introduced.
   LEGACY_COOKIE_ACCESS,

components/policy/resources/policy_templates.json

@@ -6172,9 +6172,9 @@
       'id': 623,
       'caption': '''Default legacy <ph name="ATTRIBUTE_SAMESITE_NAME">SameSite</ph> cookie behavior setting''',
       'tags': [],
-      'desc': '''Allows you to revert all cookies to legacy <ph name="ATTRIBUTE_SAMESITE_NAME">SameSite</ph> behavior. Reverting to legacy behavior causes cookies that don't specify a <ph name="ATTRIBUTE_SAMESITE_NAME">SameSite</ph> attribute to be treated as if they were "<ph name="ATTRIBUTE_VALUE_SAMESITE_NONE">SameSite=None</ph>", and removes the requirement for "<ph name="ATTRIBUTE_VALUE_SAMESITE_NONE">SameSite=None</ph>" cookies to carry the "<ph name="ATTRIBUTE_SECURE_NAME">Secure</ph>" attribute. See https://www.chromium.org/administrators/policy-list-3/cookie-legacy-samesite-policies for full description.
+      'desc': '''Allows you to revert all cookies to legacy <ph name="ATTRIBUTE_SAMESITE_NAME">SameSite</ph> behavior. Reverting to legacy behavior causes cookies that don't specify a <ph name="ATTRIBUTE_SAMESITE_NAME">SameSite</ph> attribute to be treated as if they were "<ph name="ATTRIBUTE_VALUE_SAMESITE_NONE">SameSite=None</ph>", removes the requirement for "<ph name="ATTRIBUTE_VALUE_SAMESITE_NONE">SameSite=None</ph>" cookies to carry the "<ph name="ATTRIBUTE_SECURE_NAME">Secure</ph>" attribute, and skips the scheme comparison when evaluating if two sites are same-site. See https://www.chromium.org/administrators/policy-list-3/cookie-legacy-samesite-policies for full description.
 
-          When this policy is not set, the default <ph name="ATTRIBUTE_SAMESITE_NAME">SameSite</ph> behavior for cookies that don't specify a <ph name="ATTRIBUTE_SAMESITE_NAME">SameSite</ph> attribute will depend on the user's personal configuration for the <ph name="FEATURE_NAME_SAMESITE_BY_DEFAULT_COOKIES">SameSite-by-default</ph> feature, which may be set by a field trial or by enabling or disabling the flag <ph name="FLAG_NAME_SAMESITE_BY_DEFAULT_COOKIES">same-site-by-default-cookies</ph> flag.''',
+          When this policy is not set, the default <ph name="ATTRIBUTE_SAMESITE_NAME">SameSite</ph> behavior for cookies will depend on the user's personal configuration for the <ph name="FEATURE_NAME_SAMESITE_BY_DEFAULT_COOKIES">SameSite-by-default</ph> feature, the <ph name="FEATURE_NAME_SAMESITE_NONE_MUST_BE_SECURE">Cookies-without-SameSite-must-be-secure</ph> feature, and the <ph name="FEATURE_NAME_SCHEMEFUL_SAME_SITE">Schemeful Same-Site</ph> feature which may be set by a field trial or by enabling or disabling the <ph name="FLAG_NAME_SAMESITE_BY_DEFAULT_COOKIES">same-site-by-default-cookies</ph> flag, the <ph name="FLAG_NAME_SAMESITE_NONE_MUST_BE_SECURE">cookies-without-same-site-must-be-secure</ph> flag, or the <ph name="FLAG_NAME_SCHEMEFUL_SAME_SITE">schemeful-same-site</ph> flag, respectively.''',
     },
     {
       'name': 'LegacySameSiteCookieBehaviorEnabledForDomainList',
@@ -6197,7 +6197,7 @@
       'id': 624,
       'caption': '''Revert to legacy <ph name="ATTRIBUTE_SAMESITE_NAME">SameSite</ph> behavior for cookies on these sites''',
       'tags': [],
-      'desc': '''Cookies set for domains matching these patterns will revert to legacy <ph name="ATTRIBUTE_SAMESITE_NAME">SameSite</ph> behavior. Reverting to legacy behavior causes cookies that don't specify a <ph name="ATTRIBUTE_SAMESITE_NAME">SameSite</ph> attribute to be treated as if they were "<ph name="ATTRIBUTE_VALUE_SAMESITE_NONE">SameSite=None</ph>", and removes the requirement for "<ph name="ATTRIBUTE_VALUE_SAMESITE_NONE">SameSite=None</ph>" cookies to carry the "<ph name="ATTRIBUTE_SECURE_NAME">Secure</ph>" attribute. See https://www.chromium.org/administrators/policy-list-3/cookie-legacy-samesite-policies for full description.
+      'desc': '''Cookies set for domains matching these patterns will revert to legacy <ph name="ATTRIBUTE_SAMESITE_NAME">SameSite</ph> behavior. Reverting to legacy behavior causes cookies that don't specify a <ph name="ATTRIBUTE_SAMESITE_NAME">SameSite</ph> attribute to be treated as if they were "<ph name="ATTRIBUTE_VALUE_SAMESITE_NONE">SameSite=None</ph>", removes the requirement for "<ph name="ATTRIBUTE_VALUE_SAMESITE_NONE">SameSite=None</ph>" cookies to carry the "<ph name="ATTRIBUTE_SECURE_NAME">Secure</ph>" attribute, and skips the scheme comparison when evaluating if two sites are same-site. See https://www.chromium.org/administrators/policy-list-3/cookie-legacy-samesite-policies for full description.
 
           For cookies on domains not covered by the patterns specified here, or for all cookies if this policy is not set, the global default value will be used either from the <ph name="LEGACY_SAMESITE_COOKIE_BEHAVIOR_ENABLED_POLICY_NAME">LegacySameSiteCookieBehaviorEnabled</ph> policy, if it is set, or the user's personal configuration otherwise.
 

net/cookies/canonical_cookie.cc

@@ -589,6 +589,14 @@ CookieAccessResult CanonicalCookie::IncludeForRequestURL(
   // match the cookie-path.
   if (!IsOnPath(url.path()))
     status.AddExclusionReason(CookieInclusionStatus::EXCLUDE_NOT_ON_PATH);
+
+  // For LEGACY cookies we should always return the schemeless context,
+  // otherwise let GetContextForCookieInclusion() decide.
+  CookieOptions::SameSiteCookieContext::ContextType cookie_inclusion_context =
+      access_semantics == CookieAccessSemantics::LEGACY
+          ? options.same_site_cookie_context().context()
+          : options.same_site_cookie_context().GetContextForCookieInclusion();
+
   // Don't include same-site cookies for cross-site requests.
   CookieEffectiveSameSite effective_same_site =
       GetEffectiveSameSite(access_semantics);
@@ -601,20 +609,19 @@ CookieAccessResult CanonicalCookie::IncludeForRequestURL(
                               CookieEffectiveSameSite::COUNT);
   }
   UMA_HISTOGRAM_ENUMERATION(
-      "Cookie.RequestSameSiteContext",
-      options.same_site_cookie_context().GetContextForCookieInclusion(),
+      "Cookie.RequestSameSiteContext", cookie_inclusion_context,
       CookieOptions::SameSiteCookieContext::ContextType::COUNT);
 
   switch (effective_same_site) {
     case CookieEffectiveSameSite::STRICT_MODE:
-      if (options.same_site_cookie_context().GetContextForCookieInclusion() <
+      if (cookie_inclusion_context <
           CookieOptions::SameSiteCookieContext::ContextType::SAME_SITE_STRICT) {
         status.AddExclusionReason(
             CookieInclusionStatus::EXCLUDE_SAMESITE_STRICT);
       }
       break;
     case CookieEffectiveSameSite::LAX_MODE:
-      if (options.same_site_cookie_context().GetContextForCookieInclusion() <
+      if (cookie_inclusion_context <
           CookieOptions::SameSiteCookieContext::ContextType::SAME_SITE_LAX) {
         status.AddExclusionReason(
             (SameSite() == CookieSameSite::UNSPECIFIED)
@@ -626,7 +633,7 @@ CookieAccessResult CanonicalCookie::IncludeForRequestURL(
     // TODO(crbug.com/990439): Add a browsertest for this behavior.
     case CookieEffectiveSameSite::LAX_MODE_ALLOW_UNSAFE:
       DCHECK(SameSite() == CookieSameSite::UNSPECIFIED);
-      if (options.same_site_cookie_context().GetContextForCookieInclusion() <
+      if (cookie_inclusion_context <
           CookieOptions::SameSiteCookieContext::ContextType::
               SAME_SITE_LAX_METHOD_UNSAFE) {
         // TODO(chlily): Do we need a separate CookieInclusionStatus for this?
@@ -707,16 +714,22 @@ void CanonicalCookie::IsSetPermittedInContext(
     UMA_HISTOGRAM_BOOLEAN("Cookie.SameSiteNoneIsSecure", IsSecure());
   }
 
+  // For LEGACY cookies we should always return the schemeless context,
+  // otherwise let GetContextForCookieInclusion() decide.
+  CookieOptions::SameSiteCookieContext::ContextType cookie_inclusion_context =
+      access_semantics == CookieAccessSemantics::LEGACY
+          ? options.same_site_cookie_context().context()
+          : options.same_site_cookie_context().GetContextForCookieInclusion();
+
   access_result->effective_same_site = GetEffectiveSameSite(access_semantics);
   DCHECK(access_result->effective_same_site !=
          CookieEffectiveSameSite::UNDEFINED);
-
   switch (access_result->effective_same_site) {
     case CookieEffectiveSameSite::STRICT_MODE:
       // This intentionally checks for `< SAME_SITE_LAX`, as we allow
       // `SameSite=Strict` cookies to be set for top-level navigations that
       // qualify for receipt of `SameSite=Lax` cookies.
-      if (options.same_site_cookie_context().GetContextForCookieInclusion() <
+      if (cookie_inclusion_context <
           CookieOptions::SameSiteCookieContext::ContextType::SAME_SITE_LAX) {
         DVLOG(net::cookie_util::kVlogSetCookies)
             << "Trying to set a `SameSite=Strict` cookie from a "
@@ -727,7 +740,7 @@ void CanonicalCookie::IsSetPermittedInContext(
       break;
     case CookieEffectiveSameSite::LAX_MODE:
     case CookieEffectiveSameSite::LAX_MODE_ALLOW_UNSAFE:
-      if (options.same_site_cookie_context().GetContextForCookieInclusion() <
+      if (cookie_inclusion_context <
           CookieOptions::SameSiteCookieContext::ContextType::SAME_SITE_LAX) {
         if (SameSite() == CookieSameSite::UNSPECIFIED) {
           DVLOG(net::cookie_util::kVlogSetCookies)