Add a switch to enable modern SameSite cookie behavior on WebView

This adds a command-line switch for Android Webview which toggles the
CookieManager's cookie access semantics to Non-legacy if the switch is
present. The current default is Legacy, which disables
SameSite-by-default (SSbD) and Schemeful Same-Site (SSS).

With the switch present, SSbD is enabled because it is enabled by
default in the content layer. The switch additionally enables the
base::Feature for SSS (currently disabled by default), so that it
enables both SSbD and SSS ("modern" behavior).

This also adds the switch to ProductionSupportedFlagList, so that it
can be toggled in the Flags UI.

Bug: 986319
Change-Id: I0ba85642cee03bf4284983b6f4c252a24bfd020a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2570073Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834400}

android_webview/browser/aw_browser_context.cc

@@ -22,9 +22,11 @@
 #include "android_webview/browser/safe_browsing/aw_safe_browsing_allowlist_manager.h"
 #include "android_webview/browser_jni_headers/AwBrowserContext_jni.h"
 #include "android_webview/common/aw_features.h"
+#include "android_webview/common/aw_switches.h"
 #include "android_webview/common/crash_reporter/crash_keys.h"
 #include "base/base_paths_posix.h"
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/feature_list.h"
 #include "base/files/file_util.h"
 #include "base/path_service.h"
@@ -484,9 +486,11 @@ void AwBrowserContext::ConfigureNetworkContextParams(
       network::mojom::CookieManagerParams::New();
   context_params->cookie_manager_params->allow_file_scheme_cookies =
       GetCookieManager()->GetAllowFileSchemeCookies();
-  // Set all cookies to Legacy on Android WebView, for compatibility reasons.
   context_params->cookie_manager_params->cookie_access_delegate_type =
-      network::mojom::CookieAccessDelegateType::ALWAYS_LEGACY;
+      base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kWebViewEnableModernCookieSameSite)
+          ? network::mojom::CookieAccessDelegateType::ALWAYS_NONLEGACY
+          : network::mojom::CookieAccessDelegateType::ALWAYS_LEGACY;
 
   context_params->initial_ssl_config = network::mojom::SSLConfig::New();
   // Allow SHA-1 to be used for locally-installed trust anchors, as WebView

android_webview/browser/aw_feature_list_creator.cc

@@ -15,6 +15,7 @@
 #include "android_webview/browser/aw_metrics_service_client_delegate.h"
 #include "android_webview/browser/metrics/aw_metrics_service_client.h"
 #include "android_webview/browser/variations/variations_seed_loader.h"
+#include "android_webview/common/aw_switches.h"
 #include "android_webview/proto/aw_variations_seed.pb.h"
 #include "base/base_switches.h"
 #include "base/bind.h"
@@ -42,6 +43,7 @@
 #include "components/variations/service/safe_seed_manager.h"
 #include "components/variations/service/variations_service.h"
 #include "content/public/common/content_switch_dependent_feature_overrides.h"
+#include "net/base/features.h"
 #include "net/nqe/pref_names.h"
 #include "services/preferences/tracked/segregated_pref_store.h"
 
@@ -97,6 +99,24 @@ base::FilePath GetPrefStorePath() {
   return path;
 }
 
+// Adds WebView-specific switch-dependent feature overrides on top of the ones
+// from the content layer.
+std::vector<base::FeatureList::FeatureOverrideInfo>
+GetSwitchDependentFeatureOverrides(const base::CommandLine& command_line) {
+  std::vector<base::FeatureList::FeatureOverrideInfo> feature_overrides =
+      content::GetSwitchDependentFeatureOverrides(command_line);
+
+  // TODO(chlily): This can be removed when Schemeful Same-Site is enabled by
+  // default.
+  if (command_line.HasSwitch(switches::kWebViewEnableModernCookieSameSite)) {
+    feature_overrides.push_back(
+        std::make_pair(net::features::kSchemefulSameSite,
+                       base::FeatureList::OVERRIDE_ENABLE_FEATURE));
+  }
+
+  return feature_overrides;
+}
+
 }  // namespace
 
 AwFeatureListCreator::AwFeatureListCreator()
@@ -212,7 +232,7 @@ void AwFeatureListCreator::SetUpFieldTrials() {
   variations_field_trial_creator_->SetupFieldTrials(
       cc::switches::kEnableGpuBenchmarking, switches::kEnableFeatures,
       switches::kDisableFeatures, std::vector<std::string>(),
-      content::GetSwitchDependentFeatureOverrides(
+      GetSwitchDependentFeatureOverrides(
           *base::CommandLine::ForCurrentProcess()),
       /*low_entropy_provider=*/nullptr, std::make_unique<base::FeatureList>(),
       aw_field_trials_.get(), &ignored_safe_seed_manager,

android_webview/browser/cookie_manager.cc

@@ -13,12 +13,14 @@
 #include "android_webview/browser/aw_browser_context.h"
 #include "android_webview/browser/aw_cookie_access_policy.h"
 #include "android_webview/browser_jni_headers/AwCookieManager_jni.h"
+#include "android_webview/common/aw_switches.h"
 #include "base/android/build_info.h"
 #include "base/android/callback_android.h"
 #include "base/android/jni_string.h"
 #include "base/android/path_utils.h"
 #include "base/bind.h"
 #include "base/callback_helpers.h"
+#include "base/command_line.h"
 #include "base/containers/circular_deque.h"
 #include "base/files/file_util.h"
 #include "base/location.h"
@@ -348,11 +350,14 @@ net::CookieStore* CookieManager::GetCookieStore() {
     }
 
     cookie_store_ = content::CreateCookieStore(cookie_config, nullptr);
-    // Use a CookieAccessDelegate that always returns Legacy mode, for
-    // compatibility reasons.
+    auto cookie_access_delegate_type =
+        base::CommandLine::ForCurrentProcess()->HasSwitch(
+            switches::kWebViewEnableModernCookieSameSite)
+            ? network::mojom::CookieAccessDelegateType::ALWAYS_NONLEGACY
+            : network::mojom::CookieAccessDelegateType::ALWAYS_LEGACY;
     cookie_store_->SetCookieAccessDelegate(
         std::make_unique<network::CookieAccessDelegateImpl>(
-            network::mojom::CookieAccessDelegateType::ALWAYS_LEGACY,
+            cookie_access_delegate_type,
             nullptr /* preloaded_first_party_sets */));
   }
 

android_webview/common/aw_switches.cc

@@ -46,4 +46,11 @@ const char kFinchSeedMinDownloadPeriod[] = "finch-seed-min-download-period";
 // variations seed.
 const char kFinchSeedMinUpdatePeriod[] = "finch-seed-min-update-period";
 
+// Enables modern SameSite cookie behavior (as opposed to legacy behavior). This
+// is used for WebView versions prior to when the modern behavior will be
+// enabled by default. This enables the same-site-by-default-cookies,
+// cookies-without-SameSite-must-be-secure, and schemeful-same-site features.
+const char kWebViewEnableModernCookieSameSite[] =
+    "webview-enable-modern-cookie-same-site";
+
 }  // namespace switches

android_webview/common/aw_switches.h

@@ -17,6 +17,7 @@ extern const char kFinchSeedExpirationAge[];
 extern const char kFinchSeedIgnorePendingDownload[];
 extern const char kFinchSeedMinDownloadPeriod[];
 extern const char kFinchSeedMinUpdatePeriod[];
+extern const char kWebViewEnableModernCookieSameSite[];
 
 }  // namespace switches
 

android_webview/java/src/org/chromium/android_webview/common/ProductionSupportedFlagList.java

@@ -68,6 +68,10 @@ public final class ProductionSupportedFlagList {
             Flag.commandLine(GpuSwitches.IGNORE_GPU_BLOCKLIST,
                     "Overrides the built-in software rendering list and enables "
                             + "GPU acceleration on unsupported device configurations."),
+            Flag.commandLine(AwSwitches.WEBVIEW_ENABLE_MODERN_COOKIE_SAME_SITE,
+                    "Enables modern SameSite cookie behavior: 1) SameSite=Lax by default "
+                            + "(cookies without a SameSite attribute are treated as SameSite=Lax); "
+                            + "2) Schemeful Same-Site (site boundaries include the URL scheme)."),
             Flag.baseFeature(GpuFeatures.ENABLE_SHARED_IMAGE_FOR_WEBVIEW,
                     "Enables shared images for WebView."),
             Flag.baseFeature(GpuFeatures.WEBVIEW_VULKAN,

android_webview/javatests/src/org/chromium/android_webview/test/CookieManagerTest.java

@@ -23,6 +23,7 @@ import org.junit.runner.RunWith;
 import org.chromium.android_webview.AwContents;
 import org.chromium.android_webview.AwCookieManager;
 import org.chromium.android_webview.AwSettings;
+import org.chromium.android_webview.common.AwSwitches;
 import org.chromium.android_webview.test.util.CookieUtils;
 import org.chromium.android_webview.test.util.CookieUtils.TestCallback;
 import org.chromium.android_webview.test.util.JSUtils;
@@ -1072,6 +1073,49 @@ public class CookieManagerTest {
         }
     }
 
+    @Test
+    @MediumTest
+    @Feature({"AndroidWebView", "Privacy"})
+    public void testModernCookieSameSite_Disabled() throws Throwable {
+        // Tests that the legacy behavior is active when "modern" SameSite behavior is not specified
+        // via command-line flag.
+        TestWebServer httpWebServer = TestWebServer.start();
+        TestWebServer httpsWebServer = TestWebServer.startSsl();
+        try {
+            ModernCookieSameSiteTestHelper httpHelper =
+                    new ModernCookieSameSiteTestHelper(httpWebServer, httpsWebServer);
+            ModernCookieSameSiteTestHelper httpsHelper =
+                    new ModernCookieSameSiteTestHelper(httpsWebServer, httpWebServer);
+
+            httpHelper.assertModernCookieSameSiteResult("-disabled-http", false);
+            httpsHelper.assertModernCookieSameSiteResult("-disabled-https", false);
+        } finally {
+            httpWebServer.shutdown();
+            httpsWebServer.shutdown();
+        }
+    }
+
+    @Test
+    @MediumTest
+    @Feature({"AndroidWebView", "Privacy"})
+    @CommandLineFlags.Add(AwSwitches.WEBVIEW_ENABLE_MODERN_COOKIE_SAME_SITE)
+    public void testModernCookieSameSite_Enabled() throws Throwable {
+        TestWebServer httpWebServer = TestWebServer.start();
+        TestWebServer httpsWebServer = TestWebServer.startSsl();
+        try {
+            ModernCookieSameSiteTestHelper httpHelper =
+                    new ModernCookieSameSiteTestHelper(httpWebServer, httpsWebServer);
+            ModernCookieSameSiteTestHelper httpsHelper =
+                    new ModernCookieSameSiteTestHelper(httpsWebServer, httpWebServer);
+
+            httpHelper.assertModernCookieSameSiteResult("-enabled-http", true);
+            httpsHelper.assertModernCookieSameSiteResult("-enabled-https", true);
+        } finally {
+            httpWebServer.shutdown();
+            httpsWebServer.shutdown();
+        }
+    }
+
     @Test
     @MediumTest
     @Feature({"AndroidWebView", "Privacy"})
@@ -1216,6 +1260,122 @@ public class CookieManagerTest {
         }
     }
 
+    class ModernCookieSameSiteTestHelper {
+        protected final AwContents mAwContents;
+        protected final TestWebServer mWebServer;
+        protected final TestWebServer mCrossSchemeWebServer;
+
+        ModernCookieSameSiteTestHelper(
+                TestWebServer webServer, TestWebServer crossSchemeWebServer) {
+            mWebServer = webServer;
+            mCrossSchemeWebServer = crossSchemeWebServer;
+            final AwTestContainerView containerView =
+                    mActivityTestRule.createAwTestContainerViewOnMainSync(mContentsClient);
+            mAwContents = containerView.getAwContents();
+            mAwContents.getSettings().setJavaScriptEnabled(true);
+
+            allowFirstPartyCookies();
+            allowThirdPartyCookies(mAwContents);
+        }
+
+        void assertModernCookieSameSiteResult(
+                String suffix, boolean expectedIsSameSiteBehaviorModern) throws Throwable {
+            assertSameSiteLaxByDefaultResult(suffix, expectedIsSameSiteBehaviorModern);
+            assertSameSiteNoneRequiresSecureResult(suffix, expectedIsSameSiteBehaviorModern);
+            assertSchemefulSameSiteResult(suffix, expectedIsSameSiteBehaviorModern);
+        }
+
+        private void assertSameSiteLaxByDefaultResult(String suffix, boolean expectedIsLaxByDefault)
+                throws Throwable {
+            final String key = "test-lax-by-default" + suffix;
+            final String value = "value" + suffix;
+            final String iframePath = "/iframe_" + suffix + ".html";
+            final String pagePath = "/content_" + suffix + ".html";
+
+            // We create a script which tries to set a cookie on a cross-site URL. The cookie does
+            // not specify a SameSite attribute, so its SameSite mode is whatever the default is.
+            final String cookieUrl =
+                    toThirdPartyUrl(makeCookieScriptUrl(mWebServer, iframePath, key, value));
+
+            // Then we load it as an iframe, to attempt to set a default cookie in a cross-site
+            // context. It should be rejected if SameSite=Lax by Default is active.
+            final String url = makeIframeUrl(mWebServer, pagePath, cookieUrl);
+            mActivityTestRule.loadUrlSync(
+                    mAwContents, mContentsClient.getOnPageFinishedHelper(), url);
+
+            if (expectedIsLaxByDefault) {
+                assertNoCookies(cookieUrl);
+            } else {
+                assertHasCookies(cookieUrl);
+                validateCookies(cookieUrl, key);
+            }
+
+            // Clear the cookies.
+            clearCookies();
+            Assert.assertFalse(mCookieManager.hasCookies());
+        }
+
+        private void assertSameSiteNoneRequiresSecureResult(
+                String suffix, boolean expectedDoesNoneRequireSecure) throws Throwable {
+            final String path = "/cookie_test.html";
+            final String responseStr =
+                    "<html><head><title>TEST!</title></head><body>HELLO!</body></html>";
+            final List<Pair<String, String>> responseHeaders =
+                    new ArrayList<Pair<String, String>>();
+            final String headerCookieName = "test-none-requires-secure" + suffix;
+            final String headerCookieValue = "value" + suffix;
+
+            // Attempt to set a SameSite=None cookie without Secure. It should be rejected if
+            // SameSite=None Requires Secure is active.
+            responseHeaders.add(Pair.create(
+                    "Set-Cookie", headerCookieName + "=" + headerCookieValue + "; SameSite=None"));
+            String url = mWebServer.setResponse(path, responseStr, responseHeaders);
+            mActivityTestRule.loadUrlSync(
+                    mAwContents, mContentsClient.getOnPageFinishedHelper(), url);
+
+            if (expectedDoesNoneRequireSecure) {
+                assertNoCookies(url);
+            } else {
+                waitForCookie(url);
+                assertHasCookies(url);
+                validateCookies(url, headerCookieName);
+            }
+
+            // Clear the cookies.
+            clearCookies();
+            Assert.assertFalse(mCookieManager.hasCookies());
+        }
+
+        private void assertSchemefulSameSiteResult(
+                String suffix, boolean expectedIsSameSiteSchemeful) throws Throwable {
+            final String key = "test-schemeful-same-site" + suffix;
+            final String value = "value" + suffix;
+            final String iframePath = "/iframe_" + suffix + ".html";
+            final String pagePath = "/content_" + suffix + ".html";
+
+            // We create a script which tries to set a Lax cookie on a cross-scheme URL.
+            final String cookieUrl =
+                    makeSameSiteLaxCookieScriptUrl(mCrossSchemeWebServer, iframePath, key, value);
+
+            // Then we load it as an iframe, to attempt to set a Lax cookie in a cross-scheme
+            // context. It should be rejected if same-site includes scheme.
+            final String url = makeIframeUrl(mWebServer, pagePath, cookieUrl);
+            mActivityTestRule.loadUrlSync(
+                    mAwContents, mContentsClient.getOnPageFinishedHelper(), url);
+
+            if (expectedIsSameSiteSchemeful) {
+                assertNoCookies(cookieUrl);
+            } else {
+                assertHasCookies(cookieUrl);
+                validateCookies(cookieUrl, key);
+            }
+
+            // Clear the cookies.
+            clearCookies();
+            Assert.assertFalse(mCookieManager.hasCookies());
+        }
+    }
+
     /**
      * Creates a response on the TestWebServer which load a given URL in an iframe,
      * and provides helpers for forwarding JavaScript calls to that iframe via postMessage.
@@ -1259,6 +1419,23 @@ public class CookieManagerTest {
         return webServer.setResponse(path, response, null);
     }
 
+    /**
+     * Creates a response on the TestWebServer with a script that attempts to set a SameSite=Lax
+     * cookie.
+     * @param  webServer  the webServer on which to create the response
+     * @param  path the path component of the url (e.g "/cookie_test.html")
+     * @param  key the key of the cookie
+     * @param  value the value of the cookie
+     * @return  the url which gets the response
+     */
+    private String makeSameSiteLaxCookieScriptUrl(
+            TestWebServer webServer, String path, String key, String value) {
+        String response = "<html><head></head><body>"
+                + "<script>document.cookie = \"" + key + "=" + value + "; SameSite=Lax\";"
+                + "</script></body></html>";
+        return webServer.setResponse(path, response, null);
+    }
+
     /**
      * Returns code fragment to be embedded into an async function to set a cookie with CookieStore
      * API

services/network/cookie_access_delegate_impl.cc

@@ -30,9 +30,15 @@ bool CookieAccessDelegateImpl::ShouldTreatUrlAsTrustworthy(
 
 net::CookieAccessSemantics CookieAccessDelegateImpl::GetAccessSemantics(
     const net::CanonicalCookie& cookie) const {
-  if (type_ == mojom::CookieAccessDelegateType::ALWAYS_LEGACY)
-    return net::CookieAccessSemantics::LEGACY;
-  return cookie_settings_->GetCookieAccessSemanticsForDomain(cookie.Domain());
+  switch (type_) {
+    case mojom::CookieAccessDelegateType::ALWAYS_LEGACY:
+      return net::CookieAccessSemantics::LEGACY;
+    case mojom::CookieAccessDelegateType::ALWAYS_NONLEGACY:
+      return net::CookieAccessSemantics::NONLEGACY;
+    case mojom::CookieAccessDelegateType::USE_CONTENT_SETTINGS:
+      return cookie_settings_->GetCookieAccessSemanticsForDomain(
+          cookie.Domain());
+  }
 }
 
 bool CookieAccessDelegateImpl::ShouldIgnoreSameSiteRestrictions(

services/network/public/mojom/cookie_manager.mojom

@@ -52,6 +52,8 @@ enum CookieAccessDelegateType {
   USE_CONTENT_SETTINGS,
   // Always returns Legacy access semantics.
   ALWAYS_LEGACY,
+  // Always returns Non-Legacy access semantics.
+  ALWAYS_NONLEGACY,
 };
 
 enum CookiePriority {

tools/metrics/histograms/enums.xml

@@ -44038,6 +44038,7 @@ from previous Chrome versions.
   <int value="-32385086" label="NtpRecipeTasksModule:disabled"/>
   <int value="-31444029" label="MediaInspectorLogging:disabled"/>
   <int value="-30966385" label="enable-hardware-overlays"/>
+  <int value="-30208692" label="webview-enable-modern-cookie-same-site"/>
   <int value="-29877377" label="TabHoverCardImages:disabled"/>
   <int value="-29847483" label="MemoryAblation:enabled"/>
   <int value="-29507521" label="ContextualNudges:disabled"/>