Commit 5d11c9e1 authored by Karandeep Bhatia's avatar Karandeep Bhatia Committed by Commit Bot

Extensions: Hide OneGoogleBar requests from extensions.

LocalNtpSource serves resources for the local New Tab page. It makes browser
requests to fetch one google bar data over the network. This is then served to
the local NTP renderer.

Currently, extensions can intercept these requests using the Web Request API.
However, this is not desired as the NTP is a semi-privileged process. Hide these
requests from extensions.

BUG=797461

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ic8bb3268ba37ec859237f6805bba9a96cae6d464
Reviewed-on: https://chromium-review.googlesource.com/1026996
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554631}
parent 61fa9d45
......@@ -35,6 +35,7 @@
#include "chrome/browser/search/instant_io_context.h"
#include "chrome/browser/ui/pdf/chrome_pdf_web_contents_helper_client.h"
#include "chrome/browser/ui/webui/devtools_ui.h"
#include "chrome/common/webui_url_constants.h"
#include "components/pdf/browser/pdf_web_contents_helper.h"
#include "components/signin/core/browser/signin_header_helper.h"
#include "content/public/browser/browser_context.h"
......@@ -110,16 +111,25 @@ bool ChromeExtensionsAPIClient::ShouldHideBrowserNetworkRequest(
const WebRequestInfo& request) const {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
bool is_protected_devtools_request =
request.render_process_id == -1 &&
request.type != content::RESOURCE_TYPE_MAIN_FRAME &&
DevToolsUI::IsFrontendResourceURL(request.url);
// Exclude main frame navigation requests.
bool is_browser_request = request.render_process_id == -1 &&
request.type != content::RESOURCE_TYPE_MAIN_FRAME;
// Hide requests made by Devtools frontend and the NTP Instant renderer from
// extensions.
return is_protected_devtools_request ||
InstantIOContext::IsInstantProcess(request.resource_context,
request.render_process_id);
// Hide requests made by the Devtools frontend.
bool is_sensitive_request =
is_browser_request && DevToolsUI::IsFrontendResourceURL(request.url);
// Hide requests made by the browser on behalf of the NTP.
is_sensitive_request |=
(is_browser_request &&
request.initiator ==
url::Origin::Create(GURL(chrome::kChromeUINewTabURL)));
// Hide requests made by the NTP Instant renderer.
is_sensitive_request |= InstantIOContext::IsInstantProcess(
request.resource_context, request.render_process_id);
return is_sensitive_request;
}
AppViewGuestDelegate* ChromeExtensionsAPIClient::CreateAppViewGuestDelegate()
......
......@@ -4,13 +4,26 @@
#include "chrome/browser/extensions/api/chrome_extensions_api_client.h"
#include "base/macros.h"
#include "chrome/common/webui_url_constants.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "extensions/browser/api/web_request/web_request_info.h"
#include "google_apis/gaia/gaia_urls.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
namespace extensions {
TEST(TestChromeExtensionsAPIClient, ShouldHideResponseHeader) {
class ChromeExtensionsAPIClientTest : public testing::Test {
public:
ChromeExtensionsAPIClientTest() = default;
private:
content::TestBrowserThreadBundle thread_bundle_;
DISALLOW_COPY_AND_ASSIGN(ChromeExtensionsAPIClientTest);
};
TEST_F(ChromeExtensionsAPIClientTest, ShouldHideResponseHeader) {
ChromeExtensionsAPIClient client;
EXPECT_TRUE(client.ShouldHideResponseHeader(
GaiaUrls::GetInstance()->gaia_url(), "X-Chrome-ID-Consistency-Response"));
......@@ -22,4 +35,26 @@ TEST(TestChromeExtensionsAPIClient, ShouldHideResponseHeader) {
GaiaUrls::GetInstance()->gaia_url(), "Google-Accounts-SignOut"));
}
TEST_F(ChromeExtensionsAPIClientTest, ShouldHideBrowserNetworkRequest) {
ChromeExtensionsAPIClient client;
// Requests made by the browser with chrome://newtab as its initiator should
// not be visible to extensions.
WebRequestInfo request;
request.url = GURL("https://example.com/script.js");
request.initiator = url::Origin::Create(GURL(chrome::kChromeUINewTabURL));
request.render_process_id = -1;
request.type = content::ResourceType::RESOURCE_TYPE_SCRIPT;
EXPECT_TRUE(client.ShouldHideBrowserNetworkRequest(request));
// Main frame requests should always be visible to extensions.
request.type = content::ResourceType::RESOURCE_TYPE_MAIN_FRAME;
EXPECT_FALSE(client.ShouldHideBrowserNetworkRequest(request));
// Similar requests made by the renderer should be visible to extensions.
request.type = content::ResourceType::RESOURCE_TYPE_SCRIPT;
request.render_process_id = 2;
EXPECT_FALSE(client.ShouldHideBrowserNetworkRequest(request));
}
} // namespace extensions
......@@ -3,13 +3,16 @@
// found in the LICENSE file.
#include <memory>
#include <utility>
#include "base/command_line.h"
#include "base/feature_list.h"
#include "base/macros.h"
#include "base/memory/ptr_util.h"
#include "base/optional.h"
#include "base/run_loop.h"
#include "base/strings/stringprintf.h"
#include "base/synchronization/lock.h"
#include "base/test/scoped_feature_list.h"
#include "base/values.h"
#include "build/build_config.h"
......@@ -25,6 +28,9 @@
#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"
#include "chrome/browser/search/one_google_bar/one_google_bar_fetcher.h"
#include "chrome/browser/search/one_google_bar/one_google_bar_service.h"
#include "chrome/browser/search/one_google_bar/one_google_bar_service_factory.h"
#include "chrome/browser/search/search.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "chrome/browser/ui/browser.h"
......@@ -32,11 +38,13 @@
#include "chrome/browser/ui/login/login_handler.h"
#include "chrome/browser/ui/search/local_ntp_test_utils.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/url_constants.h"
#include "chrome/test/base/search_test_utils.h"
#include "chrome/test/base/ui_test_utils.h"
#include "chromeos/login/scoped_test_public_session_login_state.h"
#include "components/google/core/browser/google_switches.h"
#include "components/prefs/pref_service.h"
#include "components/proxy_config/proxy_config_dictionary.h"
#include "components/proxy_config/proxy_config_pref_names.h"
......@@ -1357,12 +1365,15 @@ IN_PROC_BROWSER_TEST_F(NTPInterceptionWebRequestAPITest,
NTPRendererRequestsHidden) {
// Loads an extension which tries to intercept requests to
// "fake_ntp_script.js", which will be loaded as part of the NTP renderer.
ExtensionTestMessageListener listener("ready", false /*will_reply*/);
ExtensionTestMessageListener listener("ready", true /*will_reply*/);
const Extension* extension =
LoadExtension(test_data_dir_.AppendASCII("extension"));
ASSERT_TRUE(extension);
EXPECT_TRUE(listener.WaitUntilSatisfied());
// Have the extension listen for requests to |fake_ntp_script.js|.
listener.Reply(https_test_server()->GetURL("/fake_ntp_script.js").spec());
// Returns true if the given extension was able to intercept the request to
// "fake_ntp_script.js".
auto was_script_request_intercepted =
......@@ -1404,6 +1415,139 @@ IN_PROC_BROWSER_TEST_F(NTPInterceptionWebRequestAPITest,
EXPECT_TRUE(was_script_request_intercepted(extension->id()));
}
// Test fixture testing that requests made for the OneGoogleBar on behalf of
// the local NTP can't be intercepted by extensions.
class LocalNTPInterceptionWebRequestAPITest
: public ExtensionApiTest,
public OneGoogleBarServiceObserver {
public:
LocalNTPInterceptionWebRequestAPITest()
: https_test_server_(net::EmbeddedTestServer::TYPE_HTTPS) {}
// ExtensionApiTest override:
void SetUp() override {
https_test_server_.RegisterRequestMonitor(base::BindRepeating(
&LocalNTPInterceptionWebRequestAPITest::MonitorRequest,
base::Unretained(this)));
ASSERT_TRUE(https_test_server_.InitializeAndListen());
ExtensionApiTest::SetUp();
feature_list_.InitWithFeatures(
{::features::kUseGoogleLocalNtp, ::features::kOneGoogleBarOnLocalNtp},
{});
}
void SetUpCommandLine(base::CommandLine* command_line) override {
ExtensionApiTest::SetUpCommandLine(command_line);
command_line->AppendSwitchASCII(switches::kGoogleBaseURL,
https_test_server_.base_url().spec());
}
void SetUpOnMainThread() override {
ExtensionApiTest::SetUpOnMainThread();
https_test_server_.StartAcceptingConnections();
one_google_bar_url_ = one_google_bar_service()
->fetcher_for_testing()
->GetFetchURLForTesting();
// Can't declare |runloop_| as a data member on the stack since it needs to
// be be constructed from a single-threaded context.
runloop_ = std::make_unique<base::RunLoop>();
one_google_bar_service()->AddObserver(this);
}
// OneGoogleBarServiceObserver overrides:
void OnOneGoogleBarDataUpdated() override { runloop_->Quit(); }
void OnOneGoogleBarServiceShuttingDown() override {
one_google_bar_service()->RemoveObserver(this);
}
GURL one_google_bar_url() const { return one_google_bar_url_; }
// Waits for OneGoogleBar data to be updated. Should only be used once.
void WaitForOneGoogleBarDataUpdate() { runloop_->Run(); }
bool GetAndResetOneGoogleBarRequestSeen() {
base::AutoLock lock(lock_);
bool result = one_google_bar_request_seen_;
one_google_bar_request_seen_ = false;
return result;
}
private:
OneGoogleBarService* one_google_bar_service() {
return OneGoogleBarServiceFactory::GetForProfile(profile());
}
void MonitorRequest(const net::test_server::HttpRequest& request) {
if (request.GetURL() == one_google_bar_url_) {
base::AutoLock lock(lock_);
one_google_bar_request_seen_ = true;
}
}
net::EmbeddedTestServer https_test_server_;
base::test::ScopedFeatureList feature_list_;
std::unique_ptr<base::RunLoop> runloop_;
// Initialized on the UI thread in SetUpOnMainThread. Read on UI and Embedded
// Test Server IO thread thereafter.
GURL one_google_bar_url_;
// Accessed on multiple threads- UI and Embedded Test Server IO thread. Access
// requires acquiring |lock_|.
bool one_google_bar_request_seen_ = false;
base::Lock lock_;
DISALLOW_COPY_AND_ASSIGN(LocalNTPInterceptionWebRequestAPITest);
};
IN_PROC_BROWSER_TEST_F(LocalNTPInterceptionWebRequestAPITest,
OneGoogleBarRequestsHidden) {
// Loads an extension which tries to intercept requests to the OneGoogleBar.
ExtensionTestMessageListener listener("ready", true /*will_reply*/);
const Extension* extension =
LoadExtension(test_data_dir_.AppendASCII("webrequest")
.AppendASCII("ntp_request_interception")
.AppendASCII("extension"));
ASSERT_TRUE(extension);
EXPECT_TRUE(listener.WaitUntilSatisfied());
// Have the extension listen for requests to |one_google_bar_url()|.
listener.Reply(one_google_bar_url().spec());
// Returns true if the given extension was able to intercept the request to
// |one_google_bar_url()|.
auto was_script_request_intercepted =
[this](const std::string& extension_id) {
const std::string result = ExecuteScriptInBackgroundPage(
extension_id, "getAndResetRequestIntercepted();");
EXPECT_TRUE(result == "true" || result == "false")
<< "Unexpected result " << result;
return result == "true";
};
WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
ASSERT_FALSE(GetAndResetOneGoogleBarRequestSeen());
ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUINewTabURL));
ASSERT_TRUE(search::IsInstantNTP(web_contents));
ASSERT_EQ(GURL(chrome::kChromeSearchLocalNtpUrl),
web_contents->GetController().GetVisibleEntry()->GetURL());
WaitForOneGoogleBarDataUpdate();
ASSERT_TRUE(GetAndResetOneGoogleBarRequestSeen());
// Ensure that the extension wasn't able to intercept the request.
EXPECT_FALSE(was_script_request_intercepted(extension->id()));
// A normal request to |one_google_bar_url()| (i.e. not made by
// OneGoogleBarFetcher) should be intercepted by extensions.
ui_test_utils::NavigateToURL(browser(), one_google_bar_url());
EXPECT_TRUE(was_script_request_intercepted(extension->id()));
ASSERT_TRUE(GetAndResetOneGoogleBarRequestSeen());
}
// Ensure that devtools frontend requests are hidden from the webRequest API.
IN_PROC_BROWSER_TEST_F(DevToolsFrontendInWebRequestApiTest, HiddenRequests) {
// Test expectations differ with the Network Service because of the way
......
......@@ -8,6 +8,7 @@
#include "base/callback_forward.h"
#include "base/optional.h"
class GURL;
struct OneGoogleBarData;
// Interface for fetching OneGoogleBarData over the network.
......@@ -32,6 +33,9 @@ class OneGoogleBarFetcher {
// Initiates a fetch from the network. On completion (successful or not), the
// callback will be called with the result, which will be nullopt on failure.
virtual void Fetch(OneGoogleCallback callback) = 0;
// Retrieves the URL from which OneGoogleBarData will be fetched.
virtual GURL GetFetchURLForTesting() const = 0;
};
#endif // CHROME_BROWSER_SEARCH_ONE_GOOGLE_BAR_ONE_GOOGLE_BAR_FETCHER_H_
......@@ -13,6 +13,7 @@
#include "base/values.h"
#include "chrome/browser/search/one_google_bar/one_google_bar_data.h"
#include "chrome/common/chrome_content_client.h"
#include "chrome/common/webui_url_constants.h"
#include "components/google/core/browser/google_url_tracker.h"
#include "components/google/core/browser/google_util.h"
#include "components/signin/core/browser/chrome_connected_header_helper.h"
......@@ -133,9 +134,7 @@ class OneGoogleBarFetcherImpl::AuthenticatedURLFetcher
using FetchDoneCallback = base::OnceCallback<void(const net::URLFetcher*)>;
AuthenticatedURLFetcher(net::URLRequestContextGetter* request_context,
const GURL& google_base_url,
const std::string& application_locale,
const base::Optional<std::string>& api_url_override,
GURL api_url,
bool account_consistency_mirror_required,
FetchDoneCallback callback);
~AuthenticatedURLFetcher() override = default;
......@@ -143,16 +142,13 @@ class OneGoogleBarFetcherImpl::AuthenticatedURLFetcher
void Start();
private:
GURL GetApiUrl() const;
std::string GetExtraRequestHeaders(const GURL& url) const;
std::string GetExtraRequestHeaders() const;
// URLFetcherDelegate implementation.
void OnURLFetchComplete(const net::URLFetcher* source) override;
net::URLRequestContextGetter* const request_context_;
const GURL google_base_url_;
const std::string application_locale_;
const base::Optional<std::string> api_url_override_;
const GURL api_url_;
#if defined(OS_CHROMEOS)
const bool account_consistency_mirror_required_;
#endif
......@@ -165,43 +161,23 @@ class OneGoogleBarFetcherImpl::AuthenticatedURLFetcher
OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::AuthenticatedURLFetcher(
net::URLRequestContextGetter* request_context,
const GURL& google_base_url,
const std::string& application_locale,
const base::Optional<std::string>& api_url_override,
GURL api_url,
bool account_consistency_mirror_required,
FetchDoneCallback callback)
: request_context_(request_context),
google_base_url_(google_base_url),
application_locale_(application_locale),
api_url_override_(api_url_override),
api_url_(std::move(api_url)),
#if defined(OS_CHROMEOS)
account_consistency_mirror_required_(account_consistency_mirror_required),
#endif
callback_(std::move(callback)) {}
GURL OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::GetApiUrl() const {
GURL api_url =
google_base_url_.Resolve(api_url_override_.value_or(kNewTabOgbApiPath));
// Add the "hl=" parameter.
api_url = net::AppendQueryParameter(api_url, "hl", application_locale_);
// Add the "async=" parameter. We can't use net::AppendQueryParameter for
// this because we need the ":" to remain unescaped.
GURL::Replacements replacements;
std::string query = api_url.query();
query += "&async=fixed:0";
replacements.SetQueryStr(query);
return api_url.ReplaceComponents(replacements);
}
std::string
OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::GetExtraRequestHeaders(
const GURL& url) const {
OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::GetExtraRequestHeaders()
const {
net::HttpRequestHeaders headers;
// Note: It's OK to pass SignedIn::kNo if it's unknown, as it does not affect
// transmission of experiments coming from the variations server.
variations::AppendVariationHeaders(url, variations::InIncognito::kNo,
variations::AppendVariationHeaders(api_url_, variations::InIncognito::kNo,
variations::SignedIn::kNo, &headers);
#if defined(OS_CHROMEOS)
signin::ChromeConnectedHeaderHelper chrome_connected_header_helper(
......@@ -218,7 +194,7 @@ OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::GetExtraRequestHeaders(
}
std::string chrome_connected_header_value =
chrome_connected_header_helper.BuildRequestHeader(
/*is_header_request=*/true, url,
/*is_header_request=*/true, api_url_,
// Account ID is only needed for (drive|docs).google.com.
/*account_id=*/std::string(), profile_mode);
if (!chrome_connected_header_value.empty()) {
......@@ -230,7 +206,6 @@ OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::GetExtraRequestHeaders(
}
void OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::Start() {
GURL url = GetApiUrl();
net::NetworkTrafficAnnotationTag traffic_annotation =
net::DefineNetworkTrafficAnnotation("one_google_bar_service", R"(
semantics {
......@@ -255,12 +230,15 @@ void OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::Start() {
}
}
})");
url_fetcher_ = net::URLFetcher::Create(0, url, net::URLFetcher::GET, this,
traffic_annotation);
url_fetcher_ = net::URLFetcher::Create(0, api_url_, net::URLFetcher::GET,
this, traffic_annotation);
url_fetcher_->SetRequestContext(request_context_);
url_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SEND_AUTH_DATA);
url_fetcher_->SetExtraRequestHeaders(GetExtraRequestHeaders(url));
url_fetcher_->SetExtraRequestHeaders(GetExtraRequestHeaders());
url_fetcher_->SetInitiator(
url::Origin::Create(GURL(chrome::kChromeUINewTabURL)));
url_fetcher_->Start();
}
......@@ -288,22 +266,41 @@ OneGoogleBarFetcherImpl::~OneGoogleBarFetcherImpl() = default;
void OneGoogleBarFetcherImpl::Fetch(OneGoogleCallback callback) {
callbacks_.push_back(std::move(callback));
GURL google_base_url = google_util::CommandLineGoogleBaseURL();
if (!google_base_url.is_valid()) {
google_base_url = google_url_tracker_->google_url();
}
// Note: If there is an ongoing request, abandon it. It's possible that
// something has changed in the meantime (e.g. signin state) that would make
// the result obsolete.
pending_request_ = std::make_unique<AuthenticatedURLFetcher>(
request_context_, google_base_url, application_locale_, api_url_override_,
account_consistency_mirror_required_,
request_context_, GetApiUrl(), account_consistency_mirror_required_,
base::BindOnce(&OneGoogleBarFetcherImpl::FetchDone,
base::Unretained(this)));
pending_request_->Start();
}
GURL OneGoogleBarFetcherImpl::GetFetchURLForTesting() const {
return GetApiUrl();
}
GURL OneGoogleBarFetcherImpl::GetApiUrl() const {
GURL google_base_url = google_util::CommandLineGoogleBaseURL();
if (!google_base_url.is_valid()) {
google_base_url = google_url_tracker_->google_url();
}
GURL api_url =
google_base_url.Resolve(api_url_override_.value_or(kNewTabOgbApiPath));
// Add the "hl=" parameter.
api_url = net::AppendQueryParameter(api_url, "hl", application_locale_);
// Add the "async=" parameter. We can't use net::AppendQueryParameter for
// this because we need the ":" to remain unescaped.
GURL::Replacements replacements;
std::string query = api_url.query();
query += "&async=fixed:0";
replacements.SetQueryStr(query);
return api_url.ReplaceComponents(replacements);
}
void OneGoogleBarFetcherImpl::FetchDone(const net::URLFetcher* source) {
// The fetcher will be deleted when the request is handled.
std::unique_ptr<AuthenticatedURLFetcher> deleter(std::move(pending_request_));
......
......@@ -46,6 +46,9 @@ class OneGoogleBarFetcherImpl : public OneGoogleBarFetcher {
private:
class AuthenticatedURLFetcher;
GURL GetFetchURLForTesting() const override;
GURL GetApiUrl() const;
void FetchDone(const net::URLFetcher* source);
void JsonParsed(std::unique_ptr<base::Value> value);
......
......@@ -33,6 +33,8 @@ class FakeOneGoogleBarFetcher : public OneGoogleBarFetcher {
callbacks_.push_back(std::move(callback));
}
GURL GetFetchURLForTesting() const override { return GURL(); }
size_t GetCallbackCount() const { return callbacks_.size(); }
void RespondToAllCallbacks(Status status,
......
......@@ -36,6 +36,8 @@ class FakeOneGoogleBarFetcher : public OneGoogleBarFetcher {
std::move(callback).Run(Status::OK, one_google_bar_data_);
}
GURL GetFetchURLForTesting() const override { return GURL(); }
void set_one_google_bar_data(
const base::Optional<OneGoogleBarData>& one_google_bar_data) {
one_google_bar_data_ = one_google_bar_data;
......
......@@ -2,10 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
var urlToIntercept;
var interceptedRequest = false;
chrome.webRequest.onBeforeRequest.addListener(function(details) {
if (details.url.endsWith('fake_ntp_script.js'))
if (urlToIntercept && details.url === urlToIntercept)
interceptedRequest = true;
}, {
urls: ['<all_urls>'],
......@@ -16,4 +17,6 @@ function getAndResetRequestIntercepted() {
interceptedRequest = false;
}
chrome.test.sendMessage('ready');
chrome.test.sendMessage('ready', function(url) {
urlToIntercept = url;
});
......@@ -154,6 +154,7 @@
-ExtensionWebRequestApiTest.WebRequestDeclarative2
-ExtensionWebRequestApiTest.WebRequestDiceHeaderProtection
-ExtensionWebRequestApiTest.WebRequestTypes
-LocalNTPInterceptionWebRequestAPITest.OneGoogleBarRequestsHidden
# https://crbug.com/721400
# WebSocket with the network service
......
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