Declarative Net Request: Support runtime host permissions.

This CL adds support for runtime host permissions to the Declarative Net Request
API. Changes:

- The behavior of REQUIRE_HOST_PERMISSION_FOR_URL_AND_INITIATOR is modified for
  the case when an extension has access to the initiator but the access to the
  request url is withheld. In this case, the extension is granted access to the
  request. This is similar to the current behavior of
  REQUIRE_HOST_PERMISSION_FOR_URL (used by web request API) and necessary for
  runtime host permissions to work. This allows extensions to intercept withheld
  cross-origin requests from a frame to which they have access.
- RulesetManager is modified to notify the chrome layer that access to a request
  was withheld. This is necessary for us to track an extension's
  blocked/withheld actions on a tab.

This CL also paves the way to transition the web request API to require host
permissions to the initiator i.e. REQUIRE_HOST_PERMISSION_FOR_URL_AND_INITIATOR.

BUG=157736, 809680

Change-Id: Ic4737a55a3ad6f88625149bcb39eefeb84df7d91
Reviewed-on: https://chromium-review.googlesource.com/c/1256219
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596450}

chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc

@@ -23,12 +23,15 @@
 #include "base/synchronization/lock.h"
 #include "base/task/post_task.h"
 #include "base/test/metrics/histogram_tester.h"
+#include "base/test/scoped_feature_list.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/values.h"
+#include "chrome/browser/extensions/extension_action_runner.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/load_error_reporter.h"
+#include "chrome/browser/extensions/scripting_permissions_modifier.h"
 #include "chrome/browser/net/profile_network_context_service.h"
 #include "chrome/browser/net/profile_network_context_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
@@ -57,6 +60,7 @@
 #include "extensions/browser/api/declarative_net_request/test_utils.h"
 #include "extensions/browser/api/declarative_net_request/utils.h"
 #include "extensions/browser/api/web_request/web_request_info.h"
+#include "extensions/browser/blocked_action_type.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
@@ -67,6 +71,7 @@
 #include "extensions/common/api/declarative_net_request/constants.h"
 #include "extensions/common/api/declarative_net_request/test_utils.h"
 #include "extensions/common/constants.h"
+#include "extensions/common/extension_features.h"
 #include "extensions/common/extension_id.h"
 #include "extensions/common/file_util.h"
 #include "extensions/common/url_pattern.h"
@@ -257,6 +262,8 @@ class DeclarativeNetRequestBrowserTest
         base::BindRepeating(&DeclarativeNetRequestBrowserTest::MonitorRequest,
                             base::Unretained(this)));
 
+    content::SetupCrossSiteRedirector(embedded_test_server());
+
     ASSERT_TRUE(embedded_test_server()->Start());
 
     // Map all hosts to localhost.
@@ -1916,6 +1923,121 @@ IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest_Packed,
       true /*sample*/, 1 /*count*/);
 }
 
+// Tests that declarativeNetRequest API works with
+// extensions_features::kRuntimeHostPermissions.
+IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest, WithheldPermissions) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      extensions_features::kRuntimeHostPermissions);
+
+  // Load an extension which blocks all script requests to "b.com".
+  TestRule rule = CreateGenericRule();
+  rule.condition->url_filter = std::string("b.com");
+  rule.condition->resource_types = std::vector<std::string>({"script"});
+  std::vector<std::string> host_permissions = {"*://a.com/", "*://b.com/*"};
+  ASSERT_NO_FATAL_FAILURE(LoadExtensionWithRules(
+      {rule}, "extension" /* directory */, host_permissions));
+
+  const Extension* extension = extension_service()->GetExtensionById(
+      last_loaded_extension_id(), false /*include_disabled*/);
+  ASSERT_TRUE(extension);
+
+  auto verify_script_load = [this, extension](const GURL& page_url,
+                                              bool expect_script_load,
+                                              int expected_blocked_actions) {
+    ui_test_utils::NavigateToURL(browser(), page_url);
+
+    // The page should have loaded correctly.
+    EXPECT_EQ(content::PAGE_TYPE_NORMAL, GetPageType());
+    EXPECT_EQ(expect_script_load, WasFrameWithScriptLoaded(GetMainFrame()));
+
+    // The EmbeddedTestServer sees requests after the hostname has been
+    // resolved.
+    const GURL script_url =
+        embedded_test_server()->GetURL("/subresources/script.js");
+    bool did_see_script_request =
+        base::ContainsKey(GetAndResetRequestsToServer(), script_url);
+    EXPECT_EQ(expect_script_load, did_see_script_request);
+
+    ExtensionActionRunner* runner =
+        ExtensionActionRunner::GetForWebContents(web_contents());
+    ASSERT_TRUE(runner);
+    EXPECT_EQ(expected_blocked_actions, runner->GetBlockedActions(extension));
+  };
+
+  {
+    const GURL page_url = embedded_test_server()->GetURL(
+        "example.com", "/cross_site_script.html");
+    SCOPED_TRACE(
+        base::StringPrintf("Navigating to %s", page_url.spec().c_str()));
+
+    // The extension should not block the request to b.com. It has access to the
+    // |script_url| but not its initiator |page_url|.
+    bool expect_script_load = true;
+    verify_script_load(page_url, expect_script_load, BLOCKED_ACTION_NONE);
+  }
+
+  {
+    const GURL page_url =
+        embedded_test_server()->GetURL("a.com", "/cross_site_script.html");
+    SCOPED_TRACE(
+        base::StringPrintf("Navigating to %s", page_url.spec().c_str()));
+
+    // The extension should block the request to b.com. It has access to both
+    // the |script_url| and its initiator |page_url|.
+    bool expect_script_load = false;
+    verify_script_load(page_url, expect_script_load, BLOCKED_ACTION_NONE);
+  }
+
+  // Withhold access to all hosts.
+  ScriptingPermissionsModifier scripting_modifier(
+      profile(), base::WrapRefCounted(extension));
+  scripting_modifier.SetWithholdHostPermissions(true);
+
+  {
+    const GURL page_url =
+        embedded_test_server()->GetURL("a.com", "/cross_site_script.html");
+    SCOPED_TRACE(base::StringPrintf("Navigating to %s with all hosts withheld",
+                                    page_url.spec().c_str()));
+
+    // The extension should not block the request to b.com. It's access to both
+    // the |script_url| and its initiator |page_url| is withheld.
+    bool expect_script_load = true;
+    verify_script_load(page_url, expect_script_load,
+                       BLOCKED_ACTION_WEB_REQUEST);
+  }
+
+  // Grant access to only "b.com".
+  scripting_modifier.GrantHostPermission(GURL("http://b.com"));
+  {
+    const GURL page_url =
+        embedded_test_server()->GetURL("a.com", "/cross_site_script.html");
+    SCOPED_TRACE(base::StringPrintf("Navigating to %s with a.com withheld",
+                                    page_url.spec().c_str()));
+
+    // The extension should not block the request to b.com. It has access to the
+    // |script_url|, baseut access to its initiator |page_url| is withheld.
+    bool expect_script_load = true;
+    verify_script_load(page_url, expect_script_load,
+                       BLOCKED_ACTION_WEB_REQUEST);
+  }
+
+  // Grant access to only "a.com".
+  scripting_modifier.RemoveAllGrantedHostPermissions();
+  scripting_modifier.GrantHostPermission(GURL("http://a.com"));
+  {
+    const GURL page_url =
+        embedded_test_server()->GetURL("a.com", "/cross_site_script.html");
+    SCOPED_TRACE(base::StringPrintf("Navigating to %s with b.com withheld",
+                                    page_url.spec().c_str()));
+
+    // The extension should block the request to b.com. It's access to the
+    // |script_url| is withheld, but it has access to its initiator |page_url|.
+    bool expect_script_load = false;
+    verify_script_load(page_url, expect_script_load, BLOCKED_ACTION_NONE);
+  }
+}
+
 // Test fixture to verify that host permissions for the request url and the
 // request initiator are properly checked. Loads an example.com url with four
 // sub-frames named frame_[1..4] from hosts frame_[1..4].com. The initiator for

chrome/test/data/extensions/declarative_net_request/cross_site_script.html

+<html>
+  <!-- This only works if the CrossSiteRedirector is running on the embedded
+       test server, and the host_resolver is set up to handle b.com. -->
+  <script src="/cross-site/b.com/subresources/script.js"></script>
+</html>

extensions/browser/api/declarative_net_request/ruleset_manager.cc

@@ -18,6 +18,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/resource_request_info.h"
 #include "extensions/browser/api/declarative_net_request/ruleset_matcher.h"
+#include "extensions/browser/api/extensions_api_client.h"
 #include "extensions/browser/api/web_request/web_request_info.h"
 #include "extensions/browser/api/web_request/web_request_permissions.h"
 #include "extensions/browser/info_map.h"
@@ -349,13 +350,23 @@ RulesetManager::Action RulesetManager::EvaluateRequest(
     size_t i = 0;
     auto ruleset_data = rulesets_.begin();
     for (; ruleset_data != rulesets_.end(); ++ruleset_data, ++i) {
-      // As a minor optimization, cache the value of
-      // |ShouldEvaluateRulesetForRequest|.
-      should_evaluate_rulesets_for_request[i] = ShouldEvaluateRulesetForRequest(
+      PermissionsData::PageAccess page_access = ShouldEvaluateRulesetForRequest(
           *ruleset_data, request, is_incognito_context);
 
-      if (!should_evaluate_rulesets_for_request[i])
+      // As a minor optimization, cache the value of
+      // |ShouldEvaluateRulesetForRequest|.
+      should_evaluate_rulesets_for_request[i] =
+          page_access == PermissionsData::PageAccess::kAllowed;
+
+      if (!should_evaluate_rulesets_for_request[i]) {
+        if (page_access == PermissionsData::PageAccess::kWithheld) {
+          DCHECK(ExtensionsAPIClient::Get());
+          ExtensionsAPIClient::Get()->NotifyWebRequestWithheld(
+              request.render_process_id, request.frame_id,
+              ruleset_data->extension_id);
+        }
         continue;
+      }
 
       if (ruleset_data->matcher->ShouldBlockRequest(
               url, first_party_origin, element_type, is_third_party)) {
@@ -444,7 +455,7 @@ bool RulesetManager::ShouldEvaluateRequest(
   return true;
 }
 
-bool RulesetManager::ShouldEvaluateRulesetForRequest(
+PermissionsData::PageAccess RulesetManager::ShouldEvaluateRulesetForRequest(
     const ExtensionRulesetData& ruleset,
     const WebRequestInfo& request,
     bool is_incognito_context) const {
@@ -452,11 +463,11 @@ bool RulesetManager::ShouldEvaluateRulesetForRequest(
   // incognito context.
   if (is_incognito_context &&
       !info_map_->IsIncognitoEnabled(ruleset.extension_id)) {
-    return false;
+    return PermissionsData::PageAccess::kDenied;
   }
 
   if (IsRequestPageAllowed(request, ruleset.allowed_pages))
-    return false;
+    return PermissionsData::PageAccess::kDenied;
 
   const int tab_id = request.frame_data ? request.frame_data->tab_id
                                         : extension_misc::kUnknownTabId;
@@ -466,15 +477,10 @@ bool RulesetManager::ShouldEvaluateRulesetForRequest(
   // have to do for split mode incognito extensions, pass false for
   // |crosses_incognito|.
   const bool crosses_incognito = false;
-  PermissionsData::PageAccess result =
-      WebRequestPermissions::CanExtensionAccessURL(
-          info_map_, ruleset.extension_id, request.url, tab_id,
-          crosses_incognito,
-          WebRequestPermissions::REQUIRE_HOST_PERMISSION_FOR_URL_AND_INITIATOR,
-          request.initiator);
-
-  // TODO(crbug.com/809680): Handle ACCESS_WITHHELD.
-  return result == PermissionsData::PageAccess::kAllowed;
+  return WebRequestPermissions::CanExtensionAccessURL(
+      info_map_, ruleset.extension_id, request.url, tab_id, crosses_incognito,
+      WebRequestPermissions::REQUIRE_HOST_PERMISSION_FOR_URL_AND_INITIATOR,
+      request.initiator);
 }
 
 }  // namespace declarative_net_request

extensions/browser/api/declarative_net_request/ruleset_manager.h

@@ -13,6 +13,7 @@
 #include "base/sequence_checker.h"
 #include "base/time/time.h"
 #include "extensions/common/extension_id.h"
+#include "extensions/common/permissions/permissions_data.h"
 #include "extensions/common/url_pattern_set.h"
 
 class GURL;
@@ -108,9 +109,10 @@ class RulesetManager {
   bool ShouldEvaluateRequest(const WebRequestInfo& request) const;
 
   // Returns whether |ruleset| should be evaluated for the given |request|.
-  bool ShouldEvaluateRulesetForRequest(const ExtensionRulesetData& ruleset,
-                                       const WebRequestInfo& request,
-                                       bool is_incognito_context) const;
+  PermissionsData::PageAccess ShouldEvaluateRulesetForRequest(
+      const ExtensionRulesetData& ruleset,
+      const WebRequestInfo& request,
+      bool is_incognito_context) const;
 
   // Sorted in decreasing order of |extension_install_time|.
   // Use a flat_set instead of std::set/map. This makes [Add/Remove]Ruleset

extensions/browser/api/web_request/web_request_permissions.cc

@@ -74,27 +74,6 @@ PermissionsData::PageAccess GetHostAccessForURL(
                                                      nullptr /*error*/);
 }
 
-// Returns the most restricted access type out of |access1| and |access2|.
-PermissionsData::PageAccess GetMinimumAccessType(
-    PermissionsData::PageAccess access1,
-    PermissionsData::PageAccess access2) {
-  PermissionsData::PageAccess access = PermissionsData::PageAccess::kDenied;
-  switch (access1) {
-    case PermissionsData::PageAccess::kDenied:
-      access = PermissionsData::PageAccess::kDenied;
-      break;
-    case PermissionsData::PageAccess::kWithheld:
-      access = (access2 == PermissionsData::PageAccess::kDenied
-                    ? PermissionsData::PageAccess::kDenied
-                    : PermissionsData::PageAccess::kWithheld);
-      break;
-    case PermissionsData::PageAccess::kAllowed:
-      access = access2;
-      break;
-  }
-  return access;
-}
-
 PermissionsData::PageAccess CanExtensionAccessURLInternal(
     const extensions::InfoMap* extension_info_map,
     const std::string& extension_id,
@@ -160,11 +139,31 @@ PermissionsData::PageAccess CanExtensionAccessURLInternal(
     case WebRequestPermissions::REQUIRE_HOST_PERMISSION_FOR_URL_AND_INITIATOR: {
       PermissionsData::PageAccess request_access =
           GetHostAccessForURL(*extension, url, tab_id);
-      PermissionsData::PageAccess initiator_access =
-          initiator && !initiator->opaque()
-              ? GetHostAccessForURL(*extension, initiator->GetURL(), tab_id)
-              : PermissionsData::PageAccess::kAllowed;
-      access = GetMinimumAccessType(request_access, initiator_access);
+      if (!initiator || initiator->opaque() ||
+          request_access == PermissionsData::PageAccess::kDenied) {
+        return request_access;
+      }
+
+      DCHECK(request_access == PermissionsData::PageAccess::kWithheld ||
+             request_access == PermissionsData::PageAccess::kAllowed);
+
+      // Possible remaining states:
+      // ----------------------------------------------------
+      // | Initiator access| Request access| Expected access|
+      // ----------------------------------------------------
+      // | Withheld        | Withheld      | Withheld       |
+      // | Withheld        | Allowed       | Withheld       |
+      // | Allowed         | Withheld      | Allowed        |
+      // | Allowed         | Allowed       | Allowed        |
+      // | Denied          | *             | Denied         |
+      // ----------------------------------------------------
+
+      // Note: The only interesting case is when the access to request is
+      // withheld but the access to initiator is allowed. In this case, we allow
+      // access to the request. This is important for extensions with webRequest
+      // to work well with runtime host permissions. See crbug.com/851722.
+
+      return GetHostAccessForURL(*extension, initiator->GetURL(), tab_id);
       break;
     }
     case WebRequestPermissions::REQUIRE_ALL_URLS:

extensions/browser/api/web_request/web_request_permissions_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "extensions/browser/api/web_request/web_request_permissions.h"
 
+#include "base/strings/stringprintf.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "extensions/browser/api/extensions_api_client.h"
 #include "extensions/browser/api/web_request/web_request_info.h"
@@ -235,4 +236,88 @@ TEST(ExtensionWebRequestPermissions,
             get_access(chromium_org, example_com_origin));
 }
 
+TEST(ExtensionWebRequestPermissions,
+     RequireAccessToURLAndInitiatorWithWithheldPermissions) {
+  // The InfoMap requires methods to be called on the IO thread. Fake it.
+  content::TestBrowserThreadBundle thread_bundle(
+      content::TestBrowserThreadBundle::IO_MAINLOOP);
+  const char* kGoogleCom = "https://google.com/";
+  const char* kExampleCom = "https://example.com/";
+  const char* kYahooCom = "https://yahoo.com";
+
+  // Set up the extension to have access to kGoogleCom and withheld access to
+  // kExampleCom.
+  scoped_refptr<const Extension> extension =
+      ExtensionBuilder("ext").AddPermissions({kGoogleCom, kExampleCom}).Build();
+
+  URLPatternSet kActivePatternSet(
+      {URLPattern(Extension::kValidHostPermissionSchemes, kGoogleCom)});
+  URLPatternSet kWithheldPatternSet(
+      {URLPattern(Extension::kValidHostPermissionSchemes, kExampleCom)});
+
+  extension->permissions_data()->SetPermissions(
+      std::make_unique<PermissionSet>(
+          APIPermissionSet(), ManifestPermissionSet(), kActivePatternSet,
+          kActivePatternSet),  // active permissions.
+      std::make_unique<PermissionSet>(
+          APIPermissionSet(), ManifestPermissionSet(), kWithheldPatternSet,
+          kWithheldPatternSet) /* withheld permissions */);
+
+  scoped_refptr<InfoMap> info_map = base::MakeRefCounted<InfoMap>();
+  info_map->AddExtension(extension.get(), base::Time(), false, false);
+
+  auto get_access = [extension, info_map](
+                        const GURL& url,
+                        base::Optional<url::Origin> initiator) {
+    constexpr int kTabId = 42;
+    constexpr WebRequestPermissions::HostPermissionsCheck kPermissionsCheck =
+        WebRequestPermissions::REQUIRE_HOST_PERMISSION_FOR_URL_AND_INITIATOR;
+    return WebRequestPermissions::CanExtensionAccessURL(
+        info_map.get(), extension->id(), url, kTabId,
+        false /* crosses incognito */, kPermissionsCheck, initiator);
+  };
+
+  using PageAccess = PermissionsData::PageAccess;
+  const GURL kAllowedUrl(kGoogleCom);
+  const GURL kWithheldUrl(kExampleCom);
+  const GURL kDeniedUrl(kYahooCom);
+  const url::Origin kAllowedOrigin(url::Origin::Create(kAllowedUrl));
+  const url::Origin kWithheldOrigin(url::Origin::Create(kWithheldUrl));
+  const url::Origin kDeniedOrigin(url::Origin::Create(kDeniedUrl));
+  const url::Origin kOpaqueOrigin;
+  struct {
+    base::Optional<url::Origin> initiator;
+    GURL url;
+    PermissionsData::PageAccess expected_access;
+  } cases[] = {
+      {base::nullopt, kAllowedUrl, PageAccess::kAllowed},
+      {base::nullopt, kWithheldUrl, PageAccess::kWithheld},
+      {base::nullopt, kDeniedUrl, PageAccess::kDenied},
+
+      {kOpaqueOrigin, kAllowedUrl, PageAccess::kAllowed},
+      {kOpaqueOrigin, kWithheldUrl, PageAccess::kWithheld},
+      {kOpaqueOrigin, kDeniedUrl, PageAccess::kDenied},
+
+      {kDeniedOrigin, kAllowedUrl, PageAccess::kDenied},
+      {kDeniedOrigin, kWithheldUrl, PageAccess::kDenied},
+      {kDeniedOrigin, kDeniedUrl, PageAccess::kDenied},
+      {kAllowedOrigin, kDeniedUrl, PageAccess::kDenied},
+      {kWithheldOrigin, kDeniedUrl, PageAccess::kDenied},
+
+      {kWithheldOrigin, kWithheldUrl, PageAccess::kWithheld},
+      {kWithheldOrigin, kAllowedUrl, PageAccess::kWithheld},
+      {kAllowedOrigin, kWithheldUrl, PageAccess::kAllowed},
+      {kAllowedOrigin, kAllowedUrl, PageAccess::kAllowed},
+  };
+
+  for (const auto& test_case : cases) {
+    SCOPED_TRACE(base::StringPrintf(
+        "url-%s initiator-%s", test_case.url.spec().c_str(),
+        test_case.initiator ? test_case.initiator->Serialize().c_str()
+                            : "empty"));
+    EXPECT_EQ(get_access(test_case.url, test_case.initiator),
+              test_case.expected_access);
+  }
+}
+
 }  // namespace extensions