Do not relax CORB if an extension can never get HTTP permissions.

An extension origin (e.g. an extension background page) only needs a
special URLLoaderFactory (e.g. with
URLLoaderFactoryParams::is_corb_enabled set to false) if the extension
can actually bypass CORS.  After this CL, the special factory will not
be created for extensions that do not have any HTTP permissions (and
cannot gain such permissions at runtime - e.g. via ActiveTab
permission).

Fixed: 1016904
Change-Id: I80119117fb3b1fce544a88d6013d8f920e31fe21
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2923328
Reviewed-by: K. Moon <kmoon@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/main@{#931289}

chrome/browser/extensions/corb_and_cors_extension_browsertest.cc

@@ -17,6 +17,7 @@
 #include "base/values.h"
 #include "build/build_config.h"
 #include "chrome/browser/apps/platform_apps/app_browsertest_util.h"
+#include "chrome/browser/extensions/api/permissions/permissions_api.h"
 #include "chrome/browser/extensions/api/tabs/tabs_api.h"
 #include "chrome/browser/extensions/chrome_content_browser_client_extensions_part.h"
 #include "chrome/browser/extensions/extension_action_runner.h"
@@ -264,8 +265,7 @@ class CorbAndCorsExtensionBrowserTest : public CorbAndCorsExtensionTestBase {
                               "has been blocked by CORS policy")));
   }
 
-  void VerifyFetchFromContentScriptWasBlockedByCorb(
-      const base::HistogramTester& histograms) {
+  void VerifyFetchWasBlockedByCorb(const base::HistogramTester& histograms) {
     // Make sure that histograms logged in other processes (e.g. in
     // NetworkService process) get synced.
     metrics::SubprocessMetricsProvider::MergeHistogramDeltasForTesting();
@@ -276,9 +276,8 @@ class CorbAndCorsExtensionBrowserTest : public CorbAndCorsExtensionTestBase {
                                  CORBAction::kBlockedWithoutSniffing, 1);
   }
 
-  void VerifyFetchFromContentScriptWasAllowedByCorb(
-      const base::HistogramTester& histograms,
-      bool expecting_sniffing = false) {
+  void VerifyFetchWasAllowedByCorb(const base::HistogramTester& histograms,
+                                   bool expecting_sniffing = false) {
     // Make sure that histograms logged in other processes (e.g. in
     // NetworkService process) get synced.
     metrics::SubprocessMetricsProvider::MergeHistogramDeltasForTesting();
@@ -292,6 +291,15 @@ class CorbAndCorsExtensionBrowserTest : public CorbAndCorsExtensionTestBase {
                                  1);
   }
 
+  void VerifyCorbWasDisabled(const base::HistogramTester& histograms) {
+    // Make sure that histograms logged in other processes (e.g. in
+    // NetworkService process) get synced.
+    metrics::SubprocessMetricsProvider::MergeHistogramDeltasForTesting();
+
+    EXPECT_THAT(histograms.GetTotalCountsForPrefix("SiteIsolation.XSD.Browser"),
+                ::testing::IsEmpty());
+  }
+
   // Verifies that fetching a CORB-eligible resource from a content script will
   // be blocked by CORS.
   //
@@ -325,8 +333,7 @@ class CorbAndCorsExtensionBrowserTest : public CorbAndCorsExtensionTestBase {
       const std::string& actual_fetch_result,
       const std::string& expected_fetch_result_prefix) {
     // Verify that CORB allowed the response.
-    VerifyFetchFromContentScriptWasAllowedByCorb(
-        histograms, false /* expecting_sniffing */);
+    VerifyFetchWasAllowedByCorb(histograms, false /* expecting_sniffing */);
 
     // Verify that the response body was blocked by CORS.
     EXPECT_EQ(kCorsErrorWhenFetching, actual_fetch_result);
@@ -337,6 +344,15 @@ class CorbAndCorsExtensionBrowserTest : public CorbAndCorsExtensionTestBase {
     return browser()->tab_strip_model()->GetActiveWebContents();
   }
 
+  const Extension* InstallExtensionWithManifest(base::StringPiece manifest) {
+    dir_.WriteManifest(manifest);
+    dir_.WriteFile(FILE_PATH_LITERAL("background_script.js"), "");
+    dir_.WriteFile(FILE_PATH_LITERAL("page.html"), "");
+
+    extension_ = LoadExtension(dir_.UnpackedPath());
+    return extension_;
+  }
+
   const Extension* InstallExtensionWithPermissionToAllUrls(
       bool enable_file_access = false) {
     const char kManifestTemplate[] = R"(
@@ -721,10 +737,11 @@ IN_PROC_BROWSER_TEST_F(CorbAndCorsExtensionBrowserTest,
     content::ExecuteScriptAsync(active_web_contents(), kFetchInitiatingScript);
     std::string fetch_result = PopString(&queue);
 
+    // Verify that the fetch was blocked by CORS.  (CORB only applies to
+    // `no-cors` requests.)
     EXPECT_EQ(kCorsErrorWhenFetching, fetch_result);
-    VerifyFetchFromContentScriptWasAllowedByCorb(
-        histograms, false /* expecting_sniffing */);
     VerifyFetchWasBlockedByCors(console_observer);
+    VerifyFetchWasAllowedByCorb(histograms, false /* expecting_sniffing */);
   }
 }
 
@@ -776,11 +793,11 @@ IN_PROC_BROWSER_TEST_F(CorbAndCorsExtensionBrowserTest,
     std::string fetch_result =
         FetchViaContentScript(cross_site_resource, active_web_contents());
 
-    // Verify that the fetch was blocked by CORS.
+    // Verify that the fetch was blocked by CORS.  (CORB only applies to
+    // `no-cors` requests.)
     EXPECT_EQ(kCorsErrorWhenFetching, fetch_result);
-    VerifyFetchFromContentScriptWasAllowedByCorb(
-        histograms, false /* expecting_sniffing */);
     VerifyFetchWasBlockedByCors(console_observer);
+    VerifyFetchWasAllowedByCorb(histograms, false /* expecting_sniffing */);
   }
 }
 
@@ -834,8 +851,7 @@ IN_PROC_BROWSER_TEST_F(CorbAndCorsExtensionBrowserTest,
 
     // Verify that the fetch was blocked by CORS.
     EXPECT_EQ(kCorsErrorWhenFetching, fetch_result);
-    VerifyFetchFromContentScriptWasAllowedByCorb(
-        histograms, false /* expecting_sniffing */);
+    VerifyFetchWasAllowedByCorb(histograms, false /* expecting_sniffing */);
     VerifyFetchWasBlockedByCors(console_observer);
   }
 }
@@ -1061,7 +1077,7 @@ IN_PROC_BROWSER_TEST_F(CorbAndCorsExtensionBrowserTest,
   // Verify that the fetch succeeded (because of the server's
   // Access-Control-Allow-Origin response header).
   EXPECT_EQ("cors-ok.txt - body\n", fetch_result);
-  VerifyFetchFromContentScriptWasAllowedByCorb(histograms);
+  VerifyFetchWasAllowedByCorb(histograms);
 }
 
 // Test that verifies that CORS blocks non-CORB-eligible fetches for targets
@@ -1096,8 +1112,7 @@ IN_PROC_BROWSER_TEST_F(CorbAndCorsExtensionBrowserTest,
 
   // Verify that the fetch was allowed by CORB (because CORB doesn't apply to
   // CORS requests).
-  VerifyFetchFromContentScriptWasAllowedByCorb(histograms,
-                                               false /* expecting_sniffing */);
+  VerifyFetchWasAllowedByCorb(histograms, false /* expecting_sniffing */);
 }
 
 // Tests that same-origin fetches (same-origin relative to the webpage the
@@ -1126,8 +1141,7 @@ IN_PROC_BROWSER_TEST_F(CorbAndCorsExtensionBrowserTest,
 
   // Verify that no blocking occurred.
   EXPECT_THAT(fetch_result, ::testing::StartsWith("nosniff.xml - body"));
-  VerifyFetchFromContentScriptWasAllowedByCorb(histograms,
-                                               false /* expecting_sniffing */);
+  VerifyFetchWasAllowedByCorb(histograms, false /* expecting_sniffing */);
 }
 
 // Test that responses that would have been allowed by CORB anyway are not
@@ -1355,12 +1369,16 @@ IN_PROC_BROWSER_TEST_F(CorbAndCorsExtensionBrowserTest,
   EXPECT_FALSE(IncognitoInfo::IsSplitMode(extension()));
 
   // Performs a cross-origin fetch from the background page.
-  GURL cross_site_resource(
-      embedded_test_server()->GetURL("cross-site.com", "/nosniff.xml"));
-  std::string fetch_result = FetchViaBackgroundPage(cross_site_resource);
+  {
+    base::HistogramTester histograms;
+    GURL cross_site_resource(
+        embedded_test_server()->GetURL("cross-site.com", "/nosniff.xml"));
+    std::string fetch_result = FetchViaBackgroundPage(cross_site_resource);
 
-  // Verify that no blocking occurred.
-  EXPECT_EQ("nosniff.xml - body\n", fetch_result);
+    // Verify that no blocking occurred.
+    EXPECT_EQ("nosniff.xml - body\n", fetch_result);
+    VerifyCorbWasDisabled(histograms);
+  }
 }
 
 // Test that requests from an extension background page use relaxed CORB
@@ -1463,6 +1481,145 @@ IN_PROC_BROWSER_TEST_F(CorbAndCorsExtensionBrowserTest,
   }
 }
 
+// Test that CORB+CORS are enforced for extensions with no permissions to
+// http/https origins.
+IN_PROC_BROWSER_TEST_F(CorbAndCorsExtensionBrowserTest,
+                       ExtensionWithNoHttpPermissions) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  // "permission" entry in the manifest below mimics the PDF extension which
+  // has no permissions to http/https origins (and therefore doesn't care
+  // about relaxed CORB and/or CORS).
+  const char kManifest[] = R"(
+      {
+        "name": "CrossOriginReadBlockingTest - Extension",
+        "version": "1.0",
+        "manifest_version": 2,
+        "permissions": [ "contentSettings" ],
+        "background": {"scripts": ["background_script.js"]}
+      } )";
+  ASSERT_TRUE(InstallExtensionWithManifest(kManifest));
+
+  // Perform a cross-origin CORS fetch from the background page.
+  {
+    base::HistogramTester histograms;
+    GURL cross_site_resource1(
+        embedded_test_server()->GetURL("cross-site.com", "/nosniff.empty"));
+    std::string fetch_result = FetchViaBackgroundPage(cross_site_resource1);
+
+    // Verify that the fetch was blocked by CORS.
+    //
+    // This behavior verification is a bit important, but here it mostly tests
+    // the test setup, rather than the behavior this test was created for.
+    EXPECT_EQ(kCorsErrorWhenFetching, fetch_result);
+
+    // CORB only applies to `no-cors` requests.
+    VerifyFetchWasAllowedByCorb(histograms, false /* expecting_sniffing */);
+  }
+
+  // Perform a cross-origin `no-cors` request from the background page.
+  {
+    // Use a slightly different URL to avoid having to think what effect the
+    // network cache might have on test results.
+    GURL cross_site_resource2(
+        embedded_test_server()->GetURL("cross-site.com", "/nosniff.xml"));
+
+    base::Value request_init(base::Value::Type::DICTIONARY);
+    request_init.SetStringPath("method", "GET");
+    request_init.SetStringPath("mode", "no-cors");
+
+    content::WebContents* background_web_contents =
+        ProcessManager::Get(browser()->profile())
+            ->GetBackgroundHostForExtension(extension()->id())
+            ->host_contents();
+
+    base::HistogramTester histograms;
+    content::DOMMessageQueue queue;
+    content::ExecuteScriptAsync(
+        background_web_contents,
+        CreateFetchScript(cross_site_resource2, std::move(request_init)));
+    std::string fetch_result = PopString(&queue);
+
+    // Verify that the fetch was blocked by CORB.
+    //
+    // This is the main verification in the test.  This verifies that the
+    // extension background page uses a URLLoaderFactory created with
+    // URLLoaderFactoryParams::is_corb_enabled set to the default, secure value
+    // of `true`.
+    EXPECT_EQ("", fetch_result);  // `no-cors` = empty, opaque response.
+    VerifyFetchWasBlockedByCorb(histograms);
+  }
+}
+
+// Test that CORB+CORS are enforced for extensions with optional permissions to
+// http/https origins.
+IN_PROC_BROWSER_TEST_F(CorbAndCorsExtensionBrowserTest,
+                       ExtensionWithOptionalHttpPermissions) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  // There are only *optional* HTTP permissions declared in the manifest below -
+  // they will be granted later, at runtime (but not at the extension
+  // installation time).
+  const char kManifest[] = R"(
+      {
+        "name": "CrossOriginReadBlockingTest - Extension",
+        "version": "1.0",
+        "manifest_version": 2,
+        "optional_permissions": [ "<all_urls>", "http://example.com/" ],
+        "background": {"scripts": ["background_script.js"]}
+      } )";
+  ASSERT_TRUE(InstallExtensionWithManifest(kManifest));
+
+  // Navigate a tab to the extension origin.
+  GURL extension_resource = GetExtensionResource("page.html");
+  ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), extension_resource));
+  content::RenderFrameHost* test_frame =
+      browser()->tab_strip_model()->GetActiveWebContents()->GetMainFrame();
+  ASSERT_EQ(GetExtensionOrigin(), test_frame->GetLastCommittedOrigin());
+
+  // Perform a cross-origin fetch from an extension frame and verify that it got
+  // blocked by CORS.
+  GURL cross_site_url(
+      embedded_test_server()->GetURL("cross-site.com", "/nosniff.xml"));
+  url::Origin cross_site_origin = url::Origin::Create(cross_site_url);
+  {
+    base::HistogramTester histograms;
+    std::string fetch_result = FetchViaFrame(cross_site_url, test_frame);
+    EXPECT_EQ(kCorsErrorWhenFetching, fetch_result);
+
+    // In presence of optional HTTP permissions CORB is disabled.
+    VerifyCorbWasDisabled(histograms);
+  }
+
+  // Grant the optional permissions to the extension.
+  {
+    PermissionsRequestFunction::SetAutoConfirmForTests(true);
+    const char kPermissionGrantingScriptTemplate[] = R"(
+        chrome.permissions.request(
+            { origins: [$1] },
+            granted => { domAutomationController.send(granted); });
+    )";
+    bool has_permission_been_granted = false;
+    std::string script = content::JsReplace(kPermissionGrantingScriptTemplate,
+                                            cross_site_origin.GetURL());
+    ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
+        test_frame, script, &has_permission_been_granted));
+    ASSERT_TRUE(has_permission_been_granted);
+    PermissionsRequestFunction::SetAutoConfirmForTests(false);
+  }
+
+  // Performs a cross-origin fetch from the background page and verify that it
+  // didn't get blocked by CORS.
+  {
+    base::HistogramTester histograms;
+    std::string fetch_result = FetchViaFrame(cross_site_url, test_frame);
+    EXPECT_EQ("nosniff.xml - body\n", fetch_result);
+
+    // In presence of optional HTTP permissions CORB is disabled.
+    VerifyCorbWasDisabled(histograms);
+  }
+}
+
 // Test that requests from an extension page hosted in a foreground tab use
 // relaxed CORB processing.
 IN_PROC_BROWSER_TEST_F(CorbAndCorsExtensionBrowserTest,
@@ -1782,10 +1939,7 @@ IN_PROC_BROWSER_TEST_F(CorbAndCorsExtensionBrowserTest,
     EXPECT_EQ("nosniff.xml - body\n", fetch_result);
 
     // CORB should be disabled for extension origins.
-    metrics::SubprocessMetricsProvider::MergeHistogramDeltasForTesting();
-    EXPECT_EQ(
-        0u,
-        histograms.GetTotalCountsForPrefix("SiteIsolation.XSD.Browser").size());
+    VerifyCorbWasDisabled(histograms);
   }
 
   // Test a request to a website *not* covered by extension permissions.
@@ -1807,10 +1961,7 @@ IN_PROC_BROWSER_TEST_F(CorbAndCorsExtensionBrowserTest,
     VerifyFetchWasBlockedByCors(console_observer);
 
     // CORB should be disabled for extension origins.
-    metrics::SubprocessMetricsProvider::MergeHistogramDeltasForTesting();
-    EXPECT_EQ(
-        0u,
-        histograms.GetTotalCountsForPrefix("SiteIsolation.XSD.Browser").size());
+    VerifyCorbWasDisabled(histograms);
   }
 }
 
@@ -1960,12 +2111,12 @@ IN_PROC_BROWSER_TEST_F(CorbAndCorsAppBrowserTest, WebViewContentScript) {
       } )";
   TestExtensionDir dir;
   dir.WriteManifest(kManifest);
-  const char kBackgroungScript[] = R"(
+  const char kBackgroundScript[] = R"(
       chrome.app.runtime.onLaunched.addListener(function() {
         chrome.app.window.create('page.html', {}, function () {});
       });
   )";
-  dir.WriteFile(FILE_PATH_LITERAL("background_script.js"), kBackgroungScript);
+  dir.WriteFile(FILE_PATH_LITERAL("background_script.js"), kBackgroundScript);
   const char kPage[] = R"(
       <div id="webview-tag-container"></div>
   )";

extensions/browser/url_loader_factory_manager.cc

@@ -16,8 +16,12 @@
 #include "extensions/common/cors_util.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_set.h"
+#include "extensions/common/manifest_handlers/permissions_parser.h"
 #include "extensions/common/mojom/host_id.mojom.h"
+#include "extensions/common/permissions/permissions_data.h"
 #include "extensions/common/script_constants.h"
+#include "extensions/common/url_pattern.h"
+#include "extensions/common/url_pattern_set.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "services/network/public/mojom/network_context.mojom.h"
 #include "services/network/public/mojom/url_loader_factory.mojom.h"
@@ -48,17 +52,54 @@ bool DoContentScriptsDependOnRelaxedCorbOrCors(const Extension& extension) {
   return false;
 }
 
-bool DoExtensionPermissionsCoverHttpOrHttpsOrigins(const Extension& extension) {
-  // TODO(lukasza): https://crbug.com/1016904: Return false if the |extension|'s
-  // permissions do not actually cover http or https origins.  For now we
-  // conservatively return true so that *all* extensions get relaxed CORS/CORB
-  // treatment.
-  return true;
+bool DoExtensionPermissionsCoverHttpOrHttpsOrigins(
+    const PermissionSet& permissions) {
+  // Looking at explicit (rather than effective) hosts results in stricter
+  // checks that better match CORB/CORS behavior.
+  const URLPatternSet& explicit_hosts = permissions.explicit_hosts();
+  return std::any_of(explicit_hosts.begin(), explicit_hosts.end(),
+                     [](const URLPattern& permission) {
+                       return permission.MatchesScheme(url::kHttpScheme) ||
+                              permission.MatchesScheme(url::kHttpsScheme);
+                     });
 }
 
-// Returns whether the default URLLoaderFactoryParams::is_corb_enabled should be
-// overridden and changed to false.
-bool ShouldDisableCorb(const Extension& extension, FactoryUser factory_user) {
+bool DoExtensionPermissionsCoverHttpOrHttpsOrigins(const Extension& extension) {
+  // Extension with an ActiveTab permission can later gain permission to access
+  // any http origin (once the ActiveTab permission is activated).
+  const PermissionsData* permissions = extension.permissions_data();
+  if (permissions->HasAPIPermission(mojom::APIPermissionID::kActiveTab))
+    return true;
+
+  // Optional extension permissions to http origins may be granted later.
+  //
+  // TODO(lukasza): Consider only handing out CORB/CORS-disabled
+  // URLLoaderFactory after the optional permission is *actually* granted.  Care
+  // might need to be take to make sure that updating the URLLoaderFactory is
+  // robust in presence of races (the new factory should reach the all [?]
+  // extension frames/contexts *before* the ack/response about the newly granted
+  // permission).
+  if (DoExtensionPermissionsCoverHttpOrHttpsOrigins(
+          PermissionsParser::GetOptionalPermissions(&extension))) {
+    return true;
+  }
+
+  // Check required extension permissions.  Note that this is broader than
+  // `permissions->GetEffectiveHostPermissions()` to account for policy that may
+  // change at runtime.
+  if (DoExtensionPermissionsCoverHttpOrHttpsOrigins(
+          PermissionsParser::GetRequiredPermissions(&extension))) {
+    return true;
+  }
+
+  // Otherwise, report that the `extension` will never get HTTP permissions.
+  return false;
+}
+
+// Returns whether to allow bypassing CORS (by disabling CORB, and paying
+// attention to the `isolated_world_origin` from content scripts, and using
+// SecFetchSiteValue::kNoOrigin from extensions).
+bool ShouldRelaxCors(const Extension& extension, FactoryUser factory_user) {
   if (!DoExtensionPermissionsCoverHttpOrHttpsOrigins(extension))
     return false;
 
@@ -70,40 +111,31 @@ bool ShouldDisableCorb(const Extension& extension, FactoryUser factory_user) {
   }
 }
 
-// Returns whether URLLoaderFactoryParams::ignore_isolated_world_origin should
-// be overridden and changed to false.
-bool ShouldInspectIsolatedWorldOrigin(const Extension& extension,
-                                      FactoryUser factory_user) {
-  if (!DoExtensionPermissionsCoverHttpOrHttpsOrigins(extension))
-    return false;
-
-  switch (factory_user) {
-    case FactoryUser::kContentScript:
-      return DoContentScriptsDependOnRelaxedCorbOrCors(extension);
-    case FactoryUser::kExtensionProcess:
-      return false;
-  }
-}
-
 bool ShouldCreateSeparateFactoryForContentScripts(const Extension& extension) {
-  return ShouldDisableCorb(extension, FactoryUser::kContentScript) ||
-         ShouldInspectIsolatedWorldOrigin(extension,
-                                          FactoryUser::kContentScript);
+  return ShouldRelaxCors(extension, FactoryUser::kContentScript);
 }
 
 void OverrideFactoryParams(const Extension& extension,
                            FactoryUser factory_user,
                            network::mojom::URLLoaderFactoryParams* params) {
-  if (ShouldDisableCorb(extension, factory_user))
-    params->is_corb_enabled = false;
+  if (!ShouldRelaxCors(extension, factory_user))
+    return;
 
-  if (ShouldInspectIsolatedWorldOrigin(extension, factory_user))
-    params->ignore_isolated_world_origin = false;
-
-  // TODO(lukasza): Do not override |unsafe_non_webby_initiator| unless
-  // DoExtensionPermissionsCoverHttpOrHttpsOrigins(extension).
-  if (factory_user == FactoryUser::kExtensionProcess)
-    params->unsafe_non_webby_initiator = true;
+  params->is_corb_enabled = false;
+  switch (factory_user) {
+    case FactoryUser::kContentScript:
+      // Requests from content scripts set
+      // network::ResourceRequest::isolated_world_origin to the origin of the
+      // extension.  This field of ResourceRequest is normally ignored, but by
+      // setting `ignore_isolated_world_origin` to false below, we ensure that
+      // OOR-CORS will use the extension origin when checking if content script
+      // requests should bypass CORS.
+      params->ignore_isolated_world_origin = false;
+      break;
+    case FactoryUser::kExtensionProcess:
+      params->unsafe_non_webby_initiator = true;
+      break;
+  }
 }
 
 void MarkIsolatedWorldsAsRequiringSeparateURLLoaderFactory(

pdf/ppapi_migration/url_loader.cc

@@ -345,6 +345,7 @@ void PepperUrlLoader::Open(const UrlRequest& request, ResultCallback callback) {
   pp::URLRequestInfo pp_request(plugin_instance_);
   pp_request.SetURL(request.url);
   pp_request.SetMethod(request.method);
+  pp_request.SetCustomReferrerURL(request.url);
 
   if (request.ignore_redirects)
     pp_request.SetFollowRedirects(false);