Change sec-fetch-site to none for extension background requests

Bug: 995475
Change-Id: I2eba159d9a462bf29e3f3c944a57b70d19914040
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1903362
Commit-Queue: Yifan Luo <lyf@google.com>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719183}

chrome/browser/extensions/background_header_browsertest.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/json/json_reader.h"
+#include "chrome/browser/extensions/extension_browsertest.h"
+#include "chrome/browser/net/profile_network_context_service.h"
+#include "chrome/browser/net/profile_network_context_service_factory.h"
+#include "components/network_session_configurator/common/network_switches.h"
+#include "extensions/test/extension_test_message_listener.h"
+#include "extensions/test/test_extension_dir.h"
+#include "net/dns/mock_host_resolver.h"
+#include "net/ssl/client_cert_store.h"
+#include "services/network/public/cpp/features.h"
+#include "url/gurl.h"
+
+namespace extensions {
+
+namespace {
+
+std::unique_ptr<net::ClientCertStore> CreateNullCertStore() {
+  return nullptr;
+}
+
+}  // namespace
+
+class BackgroundHeaderTest : public ExtensionBrowserTest {
+ public:
+  BackgroundHeaderTest()
+      : https_test_server_(net::EmbeddedTestServer::TYPE_HTTPS) {
+    feature_list_.InitWithFeatures(
+        {network::features::kFetchMetadata,
+         network::features::kFetchMetadataDestination},
+        {});
+  }
+
+  BackgroundHeaderTest(const BackgroundHeaderTest& other) = delete;
+  BackgroundHeaderTest& operator=(const BackgroundHeaderTest& other) = delete;
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    ExtensionBrowserTest::SetUpCommandLine(command_line);
+    command_line->AppendSwitch(switches::kIgnoreCertificateErrors);
+  }
+
+  GURL GetSecFetchUrl(const std::string& hostname) {
+    if (hostname.empty())
+      return https_test_server_.GetURL("/echoheader?sec-fetch-site");
+    return https_test_server_.GetURL(hostname, "/echoheader?sec-fetch-site");
+  }
+
+  const base::FilePath GetTestDataFilePath() {
+    return base::FilePath(FILE_PATH_LITERAL("chrome/test/data"));
+  }
+
+  void SetUpOnMainThread() override {
+    ExtensionBrowserTest::SetUpOnMainThread();
+
+    host_resolver()->AddRule("*", "127.0.0.1");
+    https_test_server_.SetSSLConfig(net::EmbeddedTestServer::CERT_OK);
+    https_test_server_.AddDefaultHandlers(GetTestDataFilePath());
+    ASSERT_TRUE(https_test_server_.Start());
+
+    ProfileNetworkContextServiceFactory::GetForContext(browser()->profile())
+        ->set_client_cert_store_factory_for_testing(
+            base::BindRepeating(&CreateNullCertStore));
+  }
+
+  std::string ExecuteFetch(const Extension* extension, const GURL& url) {
+    content::DOMMessageQueue message_queue;
+    browsertest_util::ExecuteScriptInBackgroundPageNoWait(
+        profile(), extension->id(),
+        content::JsReplace("executeFetch($1);", url));
+    std::string json;
+    EXPECT_TRUE(message_queue.WaitForMessage(&json));
+    base::JSONReader reader(base::JSON_ALLOW_TRAILING_COMMAS);
+    base::Optional<base::Value> value = reader.ReadToValue(json);
+    std::string result;
+    if (!value) {
+      ADD_FAILURE() << "Received invalid response: " << json;
+      return std::string();
+    }
+    EXPECT_TRUE(value->GetAsString(&result));
+    std::string trimmed_result;
+    base::TrimWhitespaceASCII(result, base::TRIM_ALL, &trimmed_result);
+    return trimmed_result;
+  }
+
+  const Extension* LoadFetchExtension(const std::string& host) {
+    ExtensionTestMessageListener listener("ready", false);
+    TestExtensionDir test_dir;
+    constexpr char kManifestTemplate[] = R"(
+    {
+      "name": "XHR Test",
+      "manifest_version": 2,
+      "version": "0.1",
+      "background": {"scripts": ["background.js"]},
+      "permissions": ["%s"]
+    })";
+    test_dir.WriteManifest(base::StringPrintf(kManifestTemplate, host.c_str()));
+    constexpr char kBackgroundScriptFile[] = R"(
+    function executeFetch(url) {
+      console.warn('Fetching: ' + url);
+      fetch(url)
+          .then(response => response.text())
+          .then(text => domAutomationController.send(text))
+          .catch(err => domAutomationController.send('ERROR: ' + err));
+    }
+    chrome.test.sendMessage('ready');)";
+
+    test_dir.WriteFile(FILE_PATH_LITERAL("background.js"),
+                       kBackgroundScriptFile);
+    const Extension* extension = LoadExtension(test_dir.UnpackedPath());
+    EXPECT_TRUE(listener.WaitUntilSatisfied());
+    return extension;
+  }
+
+ private:
+  net::EmbeddedTestServer https_test_server_;
+  base::test::ScopedFeatureList feature_list_;
+};
+
+// Test the response headers of fetch a HTTPS request in extension background
+// page.
+IN_PROC_BROWSER_TEST_F(BackgroundHeaderTest, SecFetchSite) {
+  const Extension* extension = LoadFetchExtension("<all_urls>");
+  ASSERT_TRUE(extension);
+
+  EXPECT_EQ("none", ExecuteFetch(extension, GetSecFetchUrl("example.com")));
+}
+
+// Test the response headers of fetch a HTTPS request with non-privileged host
+// in extension background page.
+IN_PROC_BROWSER_TEST_F(BackgroundHeaderTest,
+                       SecFetchSiteFromPermissionBlockedHost) {
+  const Extension* extension = LoadFetchExtension("*://example.com:*/*");
+  ASSERT_TRUE(extension);
+
+  EXPECT_EQ("cross-site",
+            ExecuteFetch(extension, GetSecFetchUrl("example2.com")));
+}
+
+}  // namespace extensions

chrome/test/BUILD.gn

@@ -1704,6 +1704,7 @@ if (!is_android) {
         "../browser/extensions/app_process_apitest.cc",
         "../browser/extensions/app_window_overrides_browsertest.cc",
         "../browser/extensions/autoplay_browsertest.cc",
+        "../browser/extensions/background_header_browsertest.cc",
         "../browser/extensions/background_page_apitest.cc",
         "../browser/extensions/background_scripts_apitest.cc",
         "../browser/extensions/background_xhr_browsertest.cc",

extensions/browser/url_loader_factory_manager.cc

@@ -309,6 +309,7 @@ bool IsSpecialURLLoaderFactoryRequired(const Extension& extension,
 }
 
 void OverrideFactoryParams(const Extension& extension,
+                           FactoryUser factory_user,
                            network::mojom::URLLoaderFactoryParams* params) {
   // Setup factory bound allow list that overwrites per-profile common list
   // to allow tab specific permissions only for this newly created factory.
@@ -324,6 +325,9 @@ void OverrideFactoryParams(const Extension& extension,
   // TODO(lukasza): https://crbug.com/1016904: Use more granular CORB
   // enforcement based on the specific |extension|'s permissions.
   params->is_corb_enabled = false;
+
+  if (factory_user == FactoryUser::kExtensionProcess)
+    params->unsafe_non_webby_initiator = true;
 }
 
 void MarkIsolatedWorldsAsRequiringSeparateURLLoaderFactory(
@@ -541,7 +545,7 @@ void URLLoaderFactoryManager::OverrideURLLoaderFactoryParams(
   if (!IsSpecialURLLoaderFactoryRequired(*extension, factory_user))
     return;
 
-  OverrideFactoryParams(*extension, factory_params);
+  OverrideFactoryParams(*extension, factory_user, factory_params);
 }
 
 // static

net/test/embedded_test_server/default_handlers.cc

@@ -102,6 +102,7 @@ std::unique_ptr<HttpResponse> HandleEchoHeader(const std::string& url,
   http_response->AddCustomHeader("Vary", vary);
   http_response->set_content(content);
   http_response->set_content_type("text/plain");
+  http_response->AddCustomHeader("Access-Control-Allow-Origin", "*");
   http_response->AddCustomHeader("Cache-Control", cache_control);
   return http_response;
 }

services/network/public/mojom/network_context.mojom

@@ -536,8 +536,15 @@ struct URLLoaderFactoryParams {
   // Cross-origin read blocking (CORB) configuration.
   bool is_corb_enabled = true;
 
-  // True if web related security (e.g., CORS) should be disabled. This is
-  // mainly used by people testing their sites, via a command line switch.
+  // Indicate whether a request is not from web page ( probably from chrome
+  // extension background page ).
+  bool unsafe_non_webby_initiator = false;
+
+  // If `true`, this factory is being created for a context that isn't
+  // considered to be of the web. "Site"-based features, like Fetch
+  // Metadata's `Sec-Fetch-Site` header will treat all requests from
+  // this factory as initiated by the user agent itself, not from any
+  // particular page.
   bool disable_web_security = false;
 
   // https://mikewest.github.io/corpp/#integration-html

services/network/sec_header_helpers.cc

@@ -12,6 +12,7 @@
 #include "net/http/http_request_headers.h"
 #include "net/url_request/url_request.h"
 #include "services/network/initiator_lock_compatibility.h"
+#include "services/network/public/cpp/cors/origin_access_list.h"
 #include "services/network/public/cpp/features.h"
 #include "services/network/public/cpp/is_potentially_trustworthy.h"
 #include "services/network/public/cpp/request_mode.h"
@@ -73,17 +74,32 @@ void SetSecFetchSiteHeader(
     const GURL* pending_redirect_url,
     const mojom::URLLoaderFactoryParams& factory_params) {
   SecFetchSiteValue header_value;
-
-  // Browser-initiated requests with no initiator origin will send
-  // `Sec-Fetch-Site: None`. Other requests start with `kSameOrigin`, and walk
-  // the request's URL chain to calculate the right value.
-  if (factory_params.process_id == mojom::kBrowserProcessId &&
-      !request->initiator().has_value()) {
+  url::Origin initiator = GetTrustworthyInitiator(
+      factory_params.request_initiator_site_lock, request->initiator());
+
+  // Browser-initiated requests with no initiator origin, and
+  // privileged requests initiated from a "non-webby" context will send
+  // `Sec-Fetch-Site: None` while unprivileged ones will send
+  // `Sec-Fetch-Site: cross-site`. Other requests default to `kSameOrigin`,
+  // and walk through the request's URL chain to calculate the
+  // correct value.
+  if (factory_params.unsafe_non_webby_initiator) {
+    cors::OriginAccessList origin_access_list;
+    origin_access_list.SetAllowListForOrigin(
+        factory_params.factory_bound_access_patterns->source_origin,
+        factory_params.factory_bound_access_patterns->allow_patterns);
+    if (origin_access_list.CheckAccessState(
+            factory_params.factory_bound_access_patterns->source_origin,
+            request->url()) == cors::OriginAccessList::AccessState::kAllowed) {
+      header_value = SecFetchSiteValue::kNoOrigin;
+    } else {
+      header_value = SecFetchSiteValue::kCrossSite;
+    }
+  } else if (factory_params.process_id == mojom::kBrowserProcessId &&
+             !request->initiator().has_value()) {
     header_value = SecFetchSiteValue::kNoOrigin;
   } else {
     header_value = SecFetchSiteValue::kSameOrigin;
-    url::Origin initiator = GetTrustworthyInitiator(
-        factory_params.request_initiator_site_lock, request->initiator());
     for (const GURL& target_url : request->url_chain()) {
       header_value = std::max(header_value,
                               SecFetchSiteHeaderValue(target_url, initiator));

services/network/sec_header_helpers_unittest.cc

@@ -9,6 +9,9 @@
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_test_util.h"
+#include "services/network/public/mojom/cors_origin_pattern.mojom.h"
+#include "services/network/public/mojom/fetch_api.mojom.h"
+#include "services/network/public/mojom/network_context.mojom.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 #include "url/gurl.h"
@@ -19,7 +22,8 @@ constexpr char kSecureSite[] = "https://site.tld";
 constexpr char kInsecureSite[] = "http://othersite.tld";
 
 constexpr char kKnownSecChHeader[] = "Sec-CH-UA";
-constexpr char kKnownSecFetchHeader[] = "Sec-Fetch-Site";
+constexpr char kKnownSecFetchSiteHeader[] = "Sec-Fetch-Site";
+constexpr char kKnownSecFetchModeHeader[] = "Sec-Fetch-Mode";
 constexpr char kOtherSecHeader[] = "sec-other-info-header";
 constexpr char kOtherHeader[] = "Other-Header";
 
@@ -56,7 +60,7 @@ TEST_F(SecHeaderHelpersTest, SecHeadersRemovedOnDowngrade) {
   current_url_request->SetExtraRequestHeaderByName(kKnownSecChHeader,
                                                    kHeaderValue,
                                                    /*overwrite=*/false);
-  current_url_request->SetExtraRequestHeaderByName(kKnownSecFetchHeader,
+  current_url_request->SetExtraRequestHeaderByName(kKnownSecFetchSiteHeader,
                                                    kHeaderValue,
                                                    /*overwrite=*/false);
   current_url_request->SetExtraRequestHeaderByName(kOtherSecHeader,
@@ -77,7 +81,7 @@ TEST_F(SecHeaderHelpersTest, SecHeadersRemovedOnDowngrade) {
   ASSERT_FALSE(current_url_request->extra_request_headers().GetHeader(
       kKnownSecChHeader, &header_value));
   ASSERT_FALSE(current_url_request->extra_request_headers().GetHeader(
-      kKnownSecFetchHeader, &header_value));
+      kKnownSecFetchSiteHeader, &header_value));
   ASSERT_TRUE(current_url_request->extra_request_headers().GetHeader(
       kOtherSecHeader, &header_value));
   ASSERT_TRUE(current_url_request->extra_request_headers().GetHeader(
@@ -92,7 +96,7 @@ TEST_F(SecHeaderHelpersTest, SecHeadersRemainOnSecureRedirect) {
   current_url_request->SetExtraRequestHeaderByName(kKnownSecChHeader,
                                                    kHeaderValue,
                                                    /*overwrite=*/false);
-  current_url_request->SetExtraRequestHeaderByName(kKnownSecFetchHeader,
+  current_url_request->SetExtraRequestHeaderByName(kKnownSecFetchSiteHeader,
                                                    kHeaderValue,
                                                    /*overwrite=*/false);
   current_url_request->SetExtraRequestHeaderByName(kOtherSecHeader,
@@ -104,7 +108,6 @@ TEST_F(SecHeaderHelpersTest, SecHeadersRemainOnSecureRedirect) {
                                     .GetHeaderVector()
                                     .size()));
 
-  MaybeRemoveSecHeaders(current_url_request, GURL(kSecureSite));
   ASSERT_EQ(4, static_cast<int>(current_url_request->extra_request_headers()
                                     .GetHeaderVector()
                                     .size()));
@@ -113,7 +116,7 @@ TEST_F(SecHeaderHelpersTest, SecHeadersRemainOnSecureRedirect) {
   ASSERT_TRUE(current_url_request->extra_request_headers().GetHeader(
       kKnownSecChHeader, &header_value));
   ASSERT_TRUE(current_url_request->extra_request_headers().GetHeader(
-      kKnownSecFetchHeader, &header_value));
+      kKnownSecFetchSiteHeader, &header_value));
   ASSERT_TRUE(current_url_request->extra_request_headers().GetHeader(
       kOtherSecHeader, &header_value));
   ASSERT_TRUE(current_url_request->extra_request_headers().GetHeader(
@@ -125,7 +128,7 @@ TEST_F(SecHeaderHelpersTest, SecHeadersRemainOnSecureRedirect) {
 TEST_F(SecHeaderHelpersTest, SecHeadersRemoveFirstLast) {
   net::URLRequest* current_url_request = url_request();
 
-  current_url_request->SetExtraRequestHeaderByName(kKnownSecFetchHeader,
+  current_url_request->SetExtraRequestHeaderByName(kKnownSecFetchSiteHeader,
                                                    kHeaderValue,
                                                    /*overwrite=*/false);
   current_url_request->SetExtraRequestHeaderByName(kOtherHeader, kHeaderValue,
@@ -144,11 +147,75 @@ TEST_F(SecHeaderHelpersTest, SecHeadersRemoveFirstLast) {
 
   std::string header_value;
   ASSERT_FALSE(current_url_request->extra_request_headers().GetHeader(
-      kKnownSecFetchHeader, &header_value));
+      kKnownSecFetchSiteHeader, &header_value));
   ASSERT_TRUE(current_url_request->extra_request_headers().GetHeader(
       kOtherHeader, &header_value));
   ASSERT_FALSE(current_url_request->extra_request_headers().GetHeader(
       kKnownSecChHeader, &header_value));
 }
 
+// Validate Sec-Fetch-Site and Sec-Fetch-Mode are set correctly with
+// unprivileged requests from chrome extension background page.
+TEST_F(SecHeaderHelpersTest, UnprivilegedRequestOnExtension) {
+  net::URLRequest* current_url_request = url_request();
+  GURL url = GURL(kSecureSite);
+
+  network::mojom::URLLoaderFactoryParams params;
+  params.unsafe_non_webby_initiator = true;
+  params.factory_bound_access_patterns =
+      network::mojom::CorsOriginAccessPatterns::New();
+  params.factory_bound_access_patterns->source_origin =
+      url::Origin::Create(url);
+
+  SetFetchMetadataHeaders(current_url_request,
+                          network::mojom::RequestMode::kCors, &url, params);
+  ASSERT_EQ(2, static_cast<int>(current_url_request->extra_request_headers()
+                                    .GetHeaderVector()
+                                    .size()));
+
+  std::string header_value;
+  ASSERT_TRUE(current_url_request->extra_request_headers().GetHeader(
+      kKnownSecFetchSiteHeader, &header_value));
+  ASSERT_EQ(header_value, "cross-site");
+
+  ASSERT_TRUE(current_url_request->extra_request_headers().GetHeader(
+      kKnownSecFetchModeHeader, &header_value));
+  ASSERT_EQ(header_value, "cors");
+}
+
+// Validate Sec-Fetch-Site and Sec-Fetch-Mode are set correctly with privileged
+// requests from chrome extension background page.
+TEST_F(SecHeaderHelpersTest, PrivilegedRequestOnExtension) {
+  net::URLRequest* current_url_request = url_request();
+  GURL url = GURL(kSecureSite);
+
+  network::mojom::URLLoaderFactoryParams params;
+  params.unsafe_non_webby_initiator = true;
+  params.factory_bound_access_patterns =
+      network::mojom::CorsOriginAccessPatterns::New();
+  params.factory_bound_access_patterns->source_origin =
+      url::Origin::Create(url);
+  params.factory_bound_access_patterns->allow_patterns.push_back(
+      mojom::CorsOriginPattern::New(
+          url.scheme(), url.host(), 0,
+          mojom::CorsDomainMatchMode::kDisallowSubdomains,
+          mojom::CorsPortMatchMode::kAllowAnyPort,
+          mojom::CorsOriginAccessMatchPriority::kDefaultPriority));
+
+  SetFetchMetadataHeaders(current_url_request,
+                          network::mojom::RequestMode::kCors, &url, params);
+  ASSERT_EQ(2, static_cast<int>(current_url_request->extra_request_headers()
+                                    .GetHeaderVector()
+                                    .size()));
+
+  std::string header_value;
+  ASSERT_TRUE(current_url_request->extra_request_headers().GetHeader(
+      kKnownSecFetchSiteHeader, &header_value));
+  ASSERT_EQ(header_value, "none");
+
+  ASSERT_TRUE(current_url_request->extra_request_headers().GetHeader(
+      kKnownSecFetchModeHeader, &header_value));
+  ASSERT_EQ(header_value, "cors");
+}
+
 }  // namespace network