Skip to content

Commit

Permalink
Prevent extensions from accessing the Dice HTTP response header
Browse files Browse the repository at this point in the history
Gaia can send a Oauth2 authorization code in the Dice response header.
This is very sensitive information, and may allow an extension to
generate a refresh token for the user account.
For this reason, we choose to hide the Dice response headers to extensions.

This header should be only hidden when sent from a Gaia origin, otherwise
this could allow a website to hide information from extensions.

This CL adds support for hiding response headers to extensions, and
affects the web_request and declarative_web_request APIs.

[email protected]

(cherry picked from commit 1f0a8bf)

Bug: 757478
Change-Id: I79adc8ae7bfad828647f1a8bd792a2976a69e280
Reviewed-on: https://chromium-review.googlesource.com/629081
Reviewed-by: Devlin <[email protected]>
Reviewed-by: Mihai Sardarescu <[email protected]>
Commit-Queue: David Roger <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#499173}
Reviewed-on: https://chromium-review.googlesource.com/652449
Reviewed-by: David Roger <[email protected]>
Cr-Commit-Position: refs/branch-heads/3202@{crosswalk-project#42}
Cr-Branched-From: fa6a5d8-refs/heads/master@{#499098}
  • Loading branch information
David Roger committed Sep 6, 2017
1 parent 4fee1fa commit 9c00d71
Show file tree
Hide file tree
Showing 23 changed files with 430 additions and 16 deletions.
1 change: 1 addition & 0 deletions chrome/browser/extensions/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -872,6 +872,7 @@ static_library("extensions") {
"//extensions/common/api",
"//extensions/features",
"//extensions/strings",
"//google_apis",
"//media:media_features",
"//net",
"//ppapi/features",
Expand Down
17 changes: 17 additions & 0 deletions chrome/browser/extensions/api/chrome_extensions_api_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@

#include "chrome/browser/extensions/api/chrome_extensions_api_client.h"

#include <utility>

#include "base/bind.h"
#include "base/files/file_path.h"
#include "base/memory/ptr_util.h"
#include "base/strings/string_util.h"
#include "build/build_config.h"
#include "chrome/browser/data_use_measurement/data_use_web_contents_observer.h"
#include "chrome/browser/extensions/api/chrome_device_permissions_prompt.h"
Expand All @@ -31,12 +34,15 @@
#include "chrome/browser/guest_view/web_view/chrome_web_view_permission_helper_delegate.h"
#include "chrome/browser/ui/pdf/chrome_pdf_web_contents_helper_client.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"
#include "content/public/browser/browser_thread.h"
#include "extensions/browser/api/virtual_keyboard_private/virtual_keyboard_delegate.h"
#include "extensions/browser/guest_view/web_view/web_view_guest.h"
#include "extensions/browser/guest_view/web_view/web_view_permission_helper.h"
#include "google_apis/gaia/gaia_urls.h"
#include "printing/features/features.h"
#include "url/gurl.h"

#if defined(OS_CHROMEOS)
#include "chrome/browser/extensions/api/file_handlers/non_native_file_system_delegate_chromeos.h"
Expand Down Expand Up @@ -85,6 +91,17 @@ void ChromeExtensionsAPIClient::AttachWebContentsHelpers(
web_contents);
}

bool ChromeExtensionsAPIClient::ShouldHideResponseHeader(
const GURL& url,
const std::string& header_name) const {
// Gaia may send a OAUth2 authorization code in the Dice response header,
// which could allow an extension to generate a refresh token for the account.
return (
(url.host_piece() == GaiaUrls::GetInstance()->gaia_url().host_piece()) &&
(base::CompareCaseInsensitiveASCII(header_name,
signin::kDiceResponseHeader) == 0));
}

AppViewGuestDelegate* ChromeExtensionsAPIClient::CreateAppViewGuestDelegate()
const {
return new ChromeAppViewGuestDelegate();
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/extensions/api/chrome_extensions_api_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ class ChromeExtensionsAPIClient : public ExtensionsAPIClient {
override;
void AttachWebContentsHelpers(content::WebContents* web_contents) const
override;
bool ShouldHideResponseHeader(const GURL& url,
const std::string& header_name) const override;
AppViewGuestDelegate* CreateAppViewGuestDelegate() const override;
ExtensionOptionsGuestDelegate* CreateExtensionOptionsGuestDelegate(
ExtensionOptionsGuest* guest) const override;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2017 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 "chrome/browser/extensions/api/chrome_extensions_api_client.h"

#include "google_apis/gaia/gaia_urls.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"

namespace extensions {

TEST(TestChromeExtensionsAPIClient, ShouldHideResponseHeader) {
ChromeExtensionsAPIClient client;
EXPECT_TRUE(client.ShouldHideResponseHeader(
GaiaUrls::GetInstance()->gaia_url(), "X-Chrome-ID-Consistency-Response"));
EXPECT_TRUE(client.ShouldHideResponseHeader(
GaiaUrls::GetInstance()->gaia_url(), "x-cHroMe-iD-CoNsiStenCY-RESPoNSE"));
EXPECT_FALSE(client.ShouldHideResponseHeader(
GURL("http://www.example.com"), "X-Chrome-ID-Consistency-Response"));
EXPECT_FALSE(client.ShouldHideResponseHeader(
GaiaUrls::GetInstance()->gaia_url(), "Google-Accounts-SignOut"));
}

} // namespace extensions
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,15 @@
#include "extensions/common/api/web_request.h"
#include "extensions/common/extension_messages.h"
#include "extensions/common/features/feature.h"
#include "google_apis/gaia/gaia_urls.h"
#include "net/base/auth.h"
#include "net/base/elements_upload_data_stream.h"
#include "net/base/request_priority.h"
#include "net/base/upload_bytes_element_reader.h"
#include "net/base/upload_file_element_reader.h"
#include "net/dns/mock_host_resolver.h"
#include "net/http/http_response_headers.h"
#include "net/http/http_util.h"
#include "net/log/net_log_with_source.h"
#include "net/log/test_net_log.h"
#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
Expand Down Expand Up @@ -1485,6 +1488,7 @@ TEST(ExtensionWebRequestHelpersTest, TestCalculateOnHeadersReceivedDelta) {
"Key2: Value2, Bar\r\n"
"Key3: Value3\r\n"
"Key5: Value5, end5\r\n"
"X-Chrome-ID-Consistency-Response: Value6\r\n"
"\r\n";
scoped_refptr<net::HttpResponseHeaders> base_headers(
new net::HttpResponseHeaders(
Expand All @@ -1497,11 +1501,39 @@ TEST(ExtensionWebRequestHelpersTest, TestCalculateOnHeadersReceivedDelta) {
// Key3 is deleted
new_headers.push_back(ResponseHeader("Key4", "Value4")); // Added
new_headers.push_back(ResponseHeader("Key5", "Value5, end5")); // Unchanged
GURL effective_new_url;

std::unique_ptr<EventResponseDelta> delta(CalculateOnHeadersReceivedDelta(
"extid", base::Time::Now(), cancel, effective_new_url, base_headers.get(),
&new_headers));
new_headers.push_back(ResponseHeader("X-Chrome-ID-Consistency-Response",
"Value1")); // Modified
GURL url;

// The X-Chrome-ID-Consistency-Response is a protected header, but only for
// Gaia URLs. It should be modifiable when sent from anywhere else.
// Non-Gaia URL:
std::unique_ptr<EventResponseDelta> delta(
CalculateOnHeadersReceivedDelta("extid", base::Time::Now(), cancel, url,
url, base_headers.get(), &new_headers));
ASSERT_TRUE(delta.get());
EXPECT_TRUE(delta->cancel);
EXPECT_EQ(3u, delta->added_response_headers.size());
EXPECT_TRUE(base::ContainsValue(delta->added_response_headers,
ResponseHeader("Key2", "Value1")));
EXPECT_TRUE(base::ContainsValue(delta->added_response_headers,
ResponseHeader("Key4", "Value4")));
EXPECT_TRUE(base::ContainsValue(
delta->added_response_headers,
ResponseHeader("X-Chrome-ID-Consistency-Response", "Value1")));
EXPECT_EQ(3u, delta->deleted_response_headers.size());
EXPECT_TRUE(base::ContainsValue(delta->deleted_response_headers,
ResponseHeader("Key2", "Value2, Bar")));
EXPECT_TRUE(base::ContainsValue(delta->deleted_response_headers,
ResponseHeader("Key3", "Value3")));
EXPECT_TRUE(base::ContainsValue(
delta->deleted_response_headers,
ResponseHeader("X-Chrome-ID-Consistency-Response", "Value6")));

// Gaia URL:
delta.reset(CalculateOnHeadersReceivedDelta(
"extid", base::Time::Now(), cancel, GaiaUrls::GetInstance()->gaia_url(),
url, base_headers.get(), &new_headers));
ASSERT_TRUE(delta.get());
EXPECT_TRUE(delta->cancel);
EXPECT_EQ(2u, delta->added_response_headers.size());
Expand Down
118 changes: 118 additions & 0 deletions chrome/browser/extensions/api/web_request/web_request_apitest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "chromeos/login/scoped_test_public_session_login_state.h"
#include "content/public/browser/navigation_controller.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/notification_registrar.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/render_frame_host.h"
Expand All @@ -45,7 +46,9 @@
#include "extensions/common/features/feature.h"
#include "extensions/test/extension_test_message_listener.h"
#include "extensions/test/result_catcher.h"
#include "google_apis/gaia/gaia_switches.h"
#include "net/dns/mock_host_resolver.h"
#include "net/http/http_util.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "net/test/embedded_test_server/http_request.h"
#include "net/test/test_data_directory.h"
Expand Down Expand Up @@ -104,6 +107,10 @@ const char kPerformXhrJs[] =
"};\n"
"xhr.send();\n";

// Header values set by the server and by the extension.
const char kHeaderValueFromExtension[] = "ValueFromExtension";
const char kHeaderValueFromServer[] = "ValueFromServer";

// Performs an XHR in the given |frame|, replying when complete.
void PerformXhrInFrame(content::RenderFrameHost* frame,
const std::string& host,
Expand Down Expand Up @@ -204,6 +211,11 @@ class ExtensionWebRequestApiTest : public ExtensionApiTest {
host_resolver()->AddRule("*", "127.0.0.1");
}

void SetUpCommandLine(base::CommandLine* command_line) override {
ExtensionApiTest::SetUpCommandLine(command_line);
command_line->AppendSwitchASCII(switches::kGaiaUrl, "http://gaia.com");
}

void RunPermissionTest(
const char* extension_directory,
bool load_extension_with_incognito_permission,
Expand Down Expand Up @@ -846,6 +858,112 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
GetWebRequestCountFromBackgroundPage(extension, profile()));
}

// Checks that the Dice response header is protected for Gaia URLs, but not
// other URLs.
IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
WebRequestDiceHeaderProtection) {
// Load an extension that registers a listener for webRequest events, and
// wait until it is initialized.
ExtensionTestMessageListener listener("ready", false);
const Extension* extension =
LoadExtension(test_data_dir_.AppendASCII("webrequest_dice_header"));
ASSERT_TRUE(extension) << message_;
EXPECT_TRUE(listener.WaitUntilSatisfied());

ASSERT_TRUE(embedded_test_server()->Start());

// Setup a web contents observer to inspect the response headers after the
// extension was run.
class TestWebContentsObserver : public content::WebContentsObserver {
public:
explicit TestWebContentsObserver(content::WebContents* contents)
: WebContentsObserver(contents) {}

void DidFinishNavigation(
content::NavigationHandle* navigation_handle) override {
// Check that the extension cannot add a Dice header.
const net::HttpResponseHeaders* headers =
navigation_handle->GetResponseHeaders();
EXPECT_TRUE(headers->GetNormalizedHeader(
"X-Chrome-ID-Consistency-Response", &dice_header_value_));
EXPECT_TRUE(
headers->GetNormalizedHeader("X-New-Header", &new_header_value_));
EXPECT_TRUE(
headers->GetNormalizedHeader("X-Control", &control_header_value_));
did_finish_navigation_called_ = true;
}

bool did_finish_navigation_called() const {
return did_finish_navigation_called_;
}

const std::string& dice_header_value() const { return dice_header_value_; }

const std::string& new_header_value() const { return new_header_value_; }

const std::string& control_header_value() const {
return control_header_value_;
}

void Clear() {
did_finish_navigation_called_ = false;
dice_header_value_.clear();
new_header_value_.clear();
control_header_value_.clear();
}

private:
bool did_finish_navigation_called_ = false;
std::string dice_header_value_;
std::string new_header_value_;
std::string control_header_value_;
};

content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
TestWebContentsObserver test_webcontents_observer(web_contents);

// Navigate to the Gaia URL intercepted by the extension.
GURL url =
embedded_test_server()->GetURL("gaia.com", "/extensions/dice.html");
ui_test_utils::NavigateToURL(browser(), url);

// Check that the Dice header was not changed by the extension.
EXPECT_TRUE(test_webcontents_observer.did_finish_navigation_called());
EXPECT_EQ(kHeaderValueFromServer,
test_webcontents_observer.dice_header_value());
EXPECT_EQ(kHeaderValueFromExtension,
test_webcontents_observer.new_header_value());
EXPECT_EQ(kHeaderValueFromExtension,
test_webcontents_observer.control_header_value());

// Check that the Dice header cannot be read by the extension.
EXPECT_EQ(0, GetCountFromBackgroundPage(extension, profile(),
"window.diceResponseHeaderCount"));
EXPECT_EQ(1, GetCountFromBackgroundPage(extension, profile(),
"window.controlResponseHeaderCount"));

// Navigate to a non-Gaia URL intercepted by the extension.
test_webcontents_observer.Clear();
url = embedded_test_server()->GetURL("example.com", "/extensions/dice.html");
ui_test_utils::NavigateToURL(browser(), url);

// Check that the Dice header was changed by the extension.
EXPECT_TRUE(test_webcontents_observer.did_finish_navigation_called());
EXPECT_EQ(kHeaderValueFromExtension,
test_webcontents_observer.dice_header_value());
EXPECT_EQ(kHeaderValueFromExtension,
test_webcontents_observer.new_header_value());
EXPECT_EQ(kHeaderValueFromExtension,
test_webcontents_observer.control_header_value());

// Check that the Dice header can be read by the extension.
EXPECT_EQ(1, GetCountFromBackgroundPage(extension, profile(),
"window.diceResponseHeaderCount"));
EXPECT_EQ(2, GetCountFromBackgroundPage(extension, profile(),
"window.controlResponseHeaderCount"));
}

// Test that the webRequest events are dispatched for the WebSocket handshake
// requests.
IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, WebSocketRequest) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,17 @@

#include "extensions/browser/api/web_request/web_request_event_details.h"

#include "base/message_loop/message_loop.h"
#include "base/values.h"
#include "extensions/browser/api/web_request/web_request_api_constants.h"
#include "extensions/browser/api/web_request/web_request_api_helpers.h"
#include "google_apis/gaia/gaia_urls.h"
#include "net/http/http_response_headers.h"
#include "net/http/http_util.h"
#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
#include "net/url_request/url_request_test_util.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"

namespace extensions {

Expand Down Expand Up @@ -64,4 +74,60 @@ TEST(WebRequestEventDetailsTest, WhitelistedCopyForPublicSession) {
EXPECT_EQ(arraysize(safe_attributes) + 1, copy->dict_.size());
}

TEST(WebRequestEventDetailsTest, SetResponseHeaders) {
const int kFilter =
extension_web_request_api_helpers::ExtraInfoSpec::RESPONSE_HEADERS;
base::MessageLoop message_loop;
net::TestURLRequestContext context;

char headers_string[] =
"HTTP/1.0 200 OK\r\n"
"Key1: Value1\r\n"
"X-Chrome-ID-Consistency-Response: Value2\r\n"
"\r\n";
scoped_refptr<net::HttpResponseHeaders> headers(
new net::HttpResponseHeaders(net::HttpUtil::AssembleRawHeaders(
headers_string, sizeof(headers_string))));

{
// Non-Gaia URL.
std::unique_ptr<net::URLRequest> request = context.CreateRequest(
GURL("http://www.example.com"), net::DEFAULT_PRIORITY, nullptr,
TRAFFIC_ANNOTATION_FOR_TESTS);
WebRequestEventDetails details(request.get(), kFilter);
details.SetResponseHeaders(request.get(), headers.get());
std::unique_ptr<base::DictionaryValue> dict =
details.GetFilteredDict(kFilter);
base::Value* filtered_headers = dict->FindPath({"responseHeaders"});
ASSERT_TRUE(filtered_headers);
EXPECT_EQ(2u, filtered_headers->GetList().size());
EXPECT_EQ("Key1",
filtered_headers->GetList()[0].FindPath({"name"})->GetString());
EXPECT_EQ("Value1",
filtered_headers->GetList()[0].FindPath({"value"})->GetString());
EXPECT_EQ("X-Chrome-ID-Consistency-Response",
filtered_headers->GetList()[1].FindPath({"name"})->GetString());
EXPECT_EQ("Value2",
filtered_headers->GetList()[1].FindPath({"value"})->GetString());
}

{
// Gaia URL.
std::unique_ptr<net::URLRequest> gaia_request = context.CreateRequest(
GaiaUrls::GetInstance()->gaia_url(), net::DEFAULT_PRIORITY, nullptr,
TRAFFIC_ANNOTATION_FOR_TESTS);
WebRequestEventDetails gaia_details(gaia_request.get(), kFilter);
gaia_details.SetResponseHeaders(gaia_request.get(), headers.get());
std::unique_ptr<base::DictionaryValue> dict =
gaia_details.GetFilteredDict(kFilter);
base::Value* filtered_headers = dict->FindPath({"responseHeaders"});
ASSERT_TRUE(filtered_headers);
EXPECT_EQ(1u, filtered_headers->GetList().size());
EXPECT_EQ("Key1",
filtered_headers->GetList()[0].FindPath({"name"})->GetString());
EXPECT_EQ("Value1",
filtered_headers->GetList()[0].FindPath({"value"})->GetString());
}
}

} // namespace extensions
1 change: 0 additions & 1 deletion chrome/browser/signin/chrome_signin_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ namespace {
const char kChromeManageAccountsHeader[] = "X-Chrome-Manage-Accounts";

#if BUILDFLAG(ENABLE_DICE_SUPPORT)
const char kDiceResponseHeader[] = "X-Chrome-ID-Consistency-Response";
const char kGoogleSignoutResponseHeader[] = "Google-Accounts-SignOut";
#endif

Expand Down
Loading

0 comments on commit 9c00d71

Please sign in to comment.