Commit 97748430 authored by Devlin Cronin's avatar Devlin Cronin Committed by Commit Bot

[Extensions Click-To-Script] Allow webRequest access based on initiator

With runtime host permissions, the webRequest API only delivers events
for the requests that an extension has access to. As the user grants
the extension access (either through the activeTab-like mechanism or
through allowing the extension to always run a site), the extension
should be able to run on that page.

However, the webRequest API traditionally requires access to the URL
that is being requested. This means that if the user allows the
extension to always run on example.com, but example.com includes a
script from chromium.org, that script request will not be visible to the
extension. This is a pretty significant breakage for any extension using
webRequest with activeTab or runtime host permissions.

Instead, relax the restriction so that the extension has access to the
request if it a) would normally have access, but it was withheld (via
runtime host permissions) and b) it has access to the initiator of the
request. This will allow the extension to intercept a request made by
example.com to chromium.org. Note that this won't apply to requests made
by cross-origin subframes, since the initiator won't be the same as the
top-level frame's origin.

Bug: 851722

Change-Id: Ibd4dab7e69672782cae2c63f5d4f7d156cd05e0b
Reviewed-on: https://chromium-review.googlesource.com/1103003
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarKaran Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567889}
parent e41a59d1
......@@ -184,6 +184,29 @@ int GetWebRequestCountFromBackgroundPage(const Extension* extension,
"window.webRequestCount");
}
// Returns true if the |extension|'s background page saw an event for a request
// with the given |hostname| (|hostname| should exclude port).
bool HasSeenWebRequestInBackgroundPage(const Extension* extension,
content::BrowserContext* context,
const std::string& hostname) {
// TODO(devlin): Here and in Get*CountFromBackgroundPage(), we should leverage
// ExecuteScriptInBackgroundPage().
ExtensionHost* host =
ProcessManager::Get(context)->GetBackgroundHostForExtension(
extension->id());
if (!host || !host->host_contents())
return false;
bool seen = false;
std::string script = base::StringPrintf(
R"(domAutomationController.send(
window.requestedHostnames.includes('%s'));)",
hostname.c_str());
EXPECT_TRUE(
ExecuteScriptAndExtractBool(host->host_contents(), script, &seen));
return seen;
}
// The DevTool's remote front-end is hardcoded to a URL with a fixed port.
// Redirect all responses to a URL with port.
class DevToolsFrontendInterceptor : public net::URLRequestInterceptor {
......@@ -805,6 +828,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
// The extension shouldn't have currently received any webRequest events,
// since it doesn't have permission (and shouldn't receive any from an XHR).
EXPECT_EQ(0, GetWebRequestCountFromBackgroundPage(extension, profile()));
EXPECT_FALSE(
HasSeenWebRequestInBackgroundPage(extension, profile(), "b.com"));
content::RenderFrameHost* main_frame = nullptr;
content::RenderFrameHost* child_frame = nullptr;
......@@ -842,8 +867,10 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
EXPECT_EQ(BLOCKED_ACTION_NONE, runner->GetBlockedActions(extension));
int xhr_count = GetWebRequestCountFromBackgroundPage(extension, profile());
// ... which means that we should have a non-zero xhr count...
// ... which means that we should have a non-zero xhr count, and the extension
// should see the request for the cross-site subframe...
EXPECT_GT(xhr_count, 0);
EXPECT_TRUE(HasSeenWebRequestInBackgroundPage(extension, profile(), "b.com"));
// ... and the extension should receive future events.
PerformXhrInFrame(main_frame, kHost, port, kXhrPath);
++xhr_count;
......@@ -872,6 +899,59 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
EXPECT_EQ(BLOCKED_ACTION_WEB_REQUEST, runner->GetBlockedActions(extension));
}
// Test that extensions with granted runtime host permissions to a tab can
// intercept cross-origin requests from that tab.
IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
WebRequestWithheldPermissionsCrossOriginRequests) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(features::kRuntimeHostPermissions);
content::SetupCrossSiteRedirector(embedded_test_server());
ASSERT_TRUE(embedded_test_server()->Start());
// Load an extension that registers a listener for webRequest events, and
// wait until it's initialized.
ExtensionTestMessageListener listener("ready", false);
const Extension* extension =
LoadExtension(test_data_dir_.AppendASCII("webrequest_activetab"));
ASSERT_TRUE(extension) << message_;
ScriptingPermissionsModifier(profile(), base::WrapRefCounted(extension))
.SetWithholdAllUrls(true);
EXPECT_TRUE(listener.WaitUntilSatisfied());
ui_test_utils::NavigateToURL(
browser(), embedded_test_server()->GetURL(
"a.com", "/extensions/cross_site_script.html"));
const std::string kCrossSiteHost("b.com");
EXPECT_FALSE(
HasSeenWebRequestInBackgroundPage(extension, profile(), kCrossSiteHost));
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
ExtensionActionRunner* runner =
ExtensionActionRunner::GetForWebContents(web_contents);
ASSERT_TRUE(runner);
EXPECT_EQ(BLOCKED_ACTION_WEB_REQUEST, runner->GetBlockedActions(extension));
// Grant runtime host permission to the page. The page should refresh. Even
// though the request is for b.com (and the extension only has access to
// a.com), it should still see the request. This is necessary for extensions
// with webRequest to work with runtime host permissions.
// https://crbug.com/851722.
runner->set_default_bubble_close_action_for_testing(
base::WrapUnique(new ToolbarActionsBarBubbleDelegate::CloseAction(
ToolbarActionsBarBubbleDelegate::CLOSE_EXECUTE)));
runner->RunAction(extension, true /* grant tab permissions */);
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(content::WaitForLoadStop(web_contents));
EXPECT_EQ(BLOCKED_ACTION_NONE, runner->GetBlockedActions(extension));
EXPECT_TRUE(
HasSeenWebRequestInBackgroundPage(extension, profile(), kCrossSiteHost));
}
// Verify that requests to clientsX.google.com are protected properly.
// First test requests from a standard renderer and then a request from the
// browser process.
......
......@@ -3,9 +3,11 @@
// found in the LICENSE file.
window.webRequestCount = 0;
window.requestedHostnames = [];
chrome.webRequest.onBeforeRequest.addListener(function(details) {
++window.webRequestCount;
window.requestedHostnames.push((new URL(details.url)).hostname);
}, {urls:['<all_urls>']});
chrome.test.sendMessage('ready');
<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/extensions/cross_site_script.js"></script>
</html>
// Copyright 2018 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.
// This space intentionally left blank.
......@@ -153,6 +153,18 @@ PermissionsData::PageAccess CanExtensionAccessURLInternal(
break;
case WebRequestPermissions::REQUIRE_HOST_PERMISSION_FOR_URL:
access = GetHostAccessForURL(*extension, url, tab_id);
// If access to the host was withheld, check if the extension has access
// to the initiator. If it does, allow the extension to see the request.
// This is important for extensions with webRequest to work well with
// runtime host permissions.
if (access == PermissionsData::PageAccess::kWithheld) {
PermissionsData::PageAccess initiator_access =
initiator
? GetHostAccessForURL(*extension, initiator->GetURL(), tab_id)
: PermissionsData::PageAccess::kDenied;
if (initiator_access == PermissionsData::PageAccess::kAllowed)
access = PermissionsData::PageAccess::kAllowed;
}
break;
case WebRequestPermissions::REQUIRE_HOST_PERMISSION_FOR_URL_AND_INITIATOR: {
PermissionsData::PageAccess request_access =
......
......@@ -4,10 +4,19 @@
#include "extensions/browser/api/web_request/web_request_permissions.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"
#include "extensions/browser/info_map.h"
#include "extensions/common/extension.h"
#include "extensions/common/extension_builder.h"
#include "extensions/common/permissions/permission_set.h"
#include "extensions/common/permissions/permissions_data.h"
#include "extensions/common/url_pattern.h"
#include "extensions/common/url_pattern_set.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
#include "url/origin.h"
namespace extensions {
......@@ -76,4 +85,91 @@ TEST(ExtensionWebRequestPermissions, IsSensitiveRequest) {
}
}
TEST(ExtensionWebRequestPermissions,
CanExtensionAccessURLWithWithheldPermissions) {
// The InfoMap requires methods to be called on the IO thread. Fake it.
content::TestBrowserThreadBundle thread_bundle(
content::TestBrowserThreadBundle::IO_MAINLOOP);
scoped_refptr<const Extension> extension =
ExtensionBuilder("ext").AddPermission("<all_urls>").Build();
URLPatternSet all_urls(
{URLPattern(Extension::kValidHostPermissionSchemes, "<all_urls>")});
// Simulate withholding the <all_urls> permission.
extension->permissions_data()->SetPermissions(
std::make_unique<PermissionSet>(), // active permissions.
std::make_unique<PermissionSet>(
APIPermissionSet(), ManifestPermissionSet(), all_urls,
URLPatternSet()) /* 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;
return WebRequestPermissions::CanExtensionAccessURL(
info_map.get(), extension->id(), url, kTabId,
false /* crosses incognito */, kPermissionsCheck, initiator);
};
const GURL example_com("https://example.com");
const GURL chromium_org("https://chromium.org");
const url::Origin example_com_origin(url::Origin::Create(example_com));
const url::Origin chromium_org_origin(url::Origin::Create(chromium_org));
// With all permissions withheld, the result of any request should be
// kWithheld.
EXPECT_EQ(PermissionsData::PageAccess::kWithheld,
get_access(example_com, base::nullopt));
EXPECT_EQ(PermissionsData::PageAccess::kWithheld,
get_access(example_com, example_com_origin));
EXPECT_EQ(PermissionsData::PageAccess::kWithheld,
get_access(example_com, chromium_org_origin));
EXPECT_EQ(PermissionsData::PageAccess::kWithheld,
get_access(chromium_org, base::nullopt));
EXPECT_EQ(PermissionsData::PageAccess::kWithheld,
get_access(chromium_org, chromium_org_origin));
EXPECT_EQ(PermissionsData::PageAccess::kWithheld,
get_access(chromium_org, example_com_origin));
// Grant access to chromium.org.
URLPatternSet chromium_org_patterns({URLPattern(
Extension::kValidHostPermissionSchemes, "https://chromium.org/*")});
extension->permissions_data()->SetPermissions(
std::make_unique<PermissionSet>(APIPermissionSet(),
ManifestPermissionSet(),
chromium_org_patterns, URLPatternSet()),
std::make_unique<PermissionSet>(APIPermissionSet(),
ManifestPermissionSet(), all_urls,
URLPatternSet()));
// example.com isn't granted, so without an initiator or with an initiator
// that the extension doesn't have access to, access is withheld.
EXPECT_EQ(PermissionsData::PageAccess::kWithheld,
get_access(example_com, base::nullopt));
EXPECT_EQ(PermissionsData::PageAccess::kWithheld,
get_access(example_com, example_com_origin));
// However, if a request is made to example.com from an initiator that the
// extension has access to, access is allowed. This is functionally necessary
// for any extension with webRequest to work with the runtime host permissions
// feature. See https://crbug.com/851722.
EXPECT_EQ(PermissionsData::PageAccess::kAllowed,
get_access(example_com, chromium_org_origin));
// With access to the requested origin, access is always allowed, independent
// of initiator.
EXPECT_EQ(PermissionsData::PageAccess::kAllowed,
get_access(chromium_org, base::nullopt));
EXPECT_EQ(PermissionsData::PageAccess::kAllowed,
get_access(chromium_org, chromium_org_origin));
EXPECT_EQ(PermissionsData::PageAccess::kAllowed,
get_access(chromium_org, example_com_origin));
}
} // namespace extensions
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment