Skip to content

Commit

Permalink
Kill renderers on bad cookie requests under --site-per-process
Browse files Browse the repository at this point in the history
Add a browser test to make sure that js cookie operations from
javascript work properly under --site-per-process. Test
that an http page can set a secure cookie that's readable by
an https page even though they're in different processes, etc.

Test that renderers are killed when they request origins other
than the one their process is locked to.

BUG=467150

Review URL: https://codereview.chromium.org/1129193008

Cr-Commit-Position: refs/heads/master@{#333398}
  • Loading branch information
nick-chromium authored and Commit bot committed Jun 9, 2015
1 parent ef9850e commit 5f3d760
Show file tree
Hide file tree
Showing 6 changed files with 193 additions and 8 deletions.
2 changes: 2 additions & 0 deletions content/browser/bad_message.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ enum BadMessageReason {
FAMF_APPEND_SHARED_MEMORY_TO_STREAM = 75,
IDBDH_CAN_READ_FILE = 76,
IDBDH_GET_OR_TERMINATE = 77,
RMF_SET_COOKIE_BAD_ORIGIN = 78,
RMF_GET_COOKIES_BAD_ORIGIN = 79,
// Please add new elements here. The naming convention is abbreviated class
// name (e.g. RenderFrameHost becomes RFH) plus a unique description of the
// reason.
Expand Down
10 changes: 8 additions & 2 deletions content/browser/renderer_host/render_message_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "base/strings/utf_string_conversions.h"
#include "base/threading/thread.h"
#include "base/threading/worker_pool.h"
#include "content/browser/bad_message.h"
#include "content/browser/browser_main_loop.h"
#include "content/browser/child_process_security_policy_impl.h"
#include "content/browser/dom_storage/dom_storage_context_wrapper.h"
Expand Down Expand Up @@ -558,8 +559,11 @@ void RenderMessageFilter::OnSetCookie(int render_frame_id,
const std::string& cookie) {
ChildProcessSecurityPolicyImpl* policy =
ChildProcessSecurityPolicyImpl::GetInstance();
if (!policy->CanAccessCookiesForOrigin(render_process_id_, url))
if (!policy->CanAccessCookiesForOrigin(render_process_id_, url)) {
bad_message::ReceivedBadMessage(this,
bad_message::RMF_SET_COOKIE_BAD_ORIGIN);
return;
}

net::CookieOptions options;
if (GetContentClient()->browser()->AllowSetCookie(
Expand All @@ -579,7 +583,9 @@ void RenderMessageFilter::OnGetCookies(int render_frame_id,
ChildProcessSecurityPolicyImpl* policy =
ChildProcessSecurityPolicyImpl::GetInstance();
if (!policy->CanAccessCookiesForOrigin(render_process_id_, url)) {
SendGetCookiesResponse(reply_msg, std::string());
bad_message::ReceivedBadMessage(this,
bad_message::RMF_GET_COOKIES_BAD_ORIGIN);
delete reply_msg;
return;
}

Expand Down
178 changes: 178 additions & 0 deletions content/browser/renderer_host/render_message_filter_browsertest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
// Copyright 2015 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 <string>

#include "base/basictypes.h"
#include "base/command_line.h"
#include "base/files/file_path.h"
#include "content/browser/frame_host/frame_tree.h"
#include "content/browser/web_contents/web_contents_impl.h"
#include "content/common/frame_messages.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/common/content_switches.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/content_browser_test.h"
#include "content/public/test/content_browser_test_utils.h"
#include "content/shell/browser/shell.h"
#include "content/test/content_browser_test_utils_internal.h"
#include "ipc/ipc_security_test_util.h"
#include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "net/test/spawned_test_server/spawned_test_server.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace content {

namespace {

std::string GetCookieFromJS(RenderFrameHost* frame) {
std::string cookie;
EXPECT_TRUE(ExecuteScriptAndExtractString(
frame, "window.domAutomationController.send(document.cookie);", &cookie));
return cookie;
}

} // namespace

using RenderMessageFilterBrowserTest = ContentBrowserTest;

// Exercises basic cookie operations via javascript, including an http page
// interacting with secure cookies.
IN_PROC_BROWSER_TEST_F(RenderMessageFilterBrowserTest, Cookies) {
host_resolver()->AddRule("*", "127.0.0.1");
ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
SetupCrossSiteRedirector(embedded_test_server());

net::SpawnedTestServer https_server(
net::SpawnedTestServer::TYPE_HTTPS, net::SpawnedTestServer::kLocalhost,
base::FilePath(FILE_PATH_LITERAL("content/test/data")));
ASSERT_TRUE(https_server.Start());

// The server sends a HttpOnly cookie. The RenderMessageFilter should never
// allow this to be sent to any renderer process.
GURL https_url = https_server.GetURL("set-cookie?notforjs=1;HttpOnly");
GURL http_url = embedded_test_server()->GetURL("/frame_with_load_event.html");

Shell* shell2 = CreateBrowser();
NavigateToURL(shell(), http_url);
NavigateToURL(shell2, https_url);

WebContentsImpl* web_contents_https =
static_cast<WebContentsImpl*>(shell2->web_contents());
WebContentsImpl* web_contents_http =
static_cast<WebContentsImpl*>(shell()->web_contents());
EXPECT_EQ("http://127.0.0.1/",
web_contents_http->GetSiteInstance()->GetSiteURL().spec());
EXPECT_EQ("https://127.0.0.1/",
web_contents_https->GetSiteInstance()->GetSiteURL().spec());

EXPECT_NE(web_contents_http->GetSiteInstance()->GetProcess(),
web_contents_https->GetSiteInstance()->GetProcess());

EXPECT_EQ("", GetCookieFromJS(web_contents_https->GetMainFrame()));
EXPECT_EQ("", GetCookieFromJS(web_contents_http->GetMainFrame()));

// Non-TLS page writes secure cookie.
EXPECT_TRUE(ExecuteScript(web_contents_http->GetMainFrame(),
"document.cookie = 'A=1; secure;';"));
EXPECT_EQ("A=1", GetCookieFromJS(web_contents_https->GetMainFrame()));
EXPECT_EQ("", GetCookieFromJS(web_contents_http->GetMainFrame()));

// TLS page writes not-secure cookie.
EXPECT_TRUE(ExecuteScript(web_contents_http->GetMainFrame(),
"document.cookie = 'B=2';"));
EXPECT_EQ("A=1; B=2", GetCookieFromJS(web_contents_https->GetMainFrame()));
EXPECT_EQ("B=2", GetCookieFromJS(web_contents_http->GetMainFrame()));

// Non-TLS page writes secure cookie.
EXPECT_TRUE(ExecuteScript(web_contents_https->GetMainFrame(),
"document.cookie = 'C=3;secure;';"));
EXPECT_EQ("A=1; B=2; C=3",
GetCookieFromJS(web_contents_https->GetMainFrame()));
EXPECT_EQ("B=2", GetCookieFromJS(web_contents_http->GetMainFrame()));

// TLS page writes not-secure cookie.
EXPECT_TRUE(ExecuteScript(web_contents_https->GetMainFrame(),
"document.cookie = 'D=4';"));
EXPECT_EQ("A=1; B=2; C=3; D=4",
GetCookieFromJS(web_contents_https->GetMainFrame()));
EXPECT_EQ("B=2; D=4", GetCookieFromJS(web_contents_http->GetMainFrame()));
}

// The RenderMessageFilter will kill processes when they access the cookies of
// sites other than the site the process is dedicated to, under site isolation.
IN_PROC_BROWSER_TEST_F(RenderMessageFilterBrowserTest,
CrossSiteCookieSecurityEnforcement) {
// The code under test is only active under site isolation.
if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kSitePerProcess)) {
return;
}

host_resolver()->AddRule("*", "127.0.0.1");
ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
SetupCrossSiteRedirector(embedded_test_server());
NavigateToURL(shell(),
embedded_test_server()->GetURL("/frame_with_load_event.html"));

WebContentsImpl* tab = static_cast<WebContentsImpl*>(shell()->web_contents());

// The iframe on the http page should get its own process.
FrameTreeVisualizer v;
EXPECT_EQ(
" Site A ------------ proxies for B\n"
" +--Site B ------- proxies for A\n"
"Where A = http://127.0.0.1/\n"
" B = http://baz.com/",
v.DepictFrameTree(tab->GetFrameTree()->root()));

RenderFrameHost* main_frame = tab->GetMainFrame();
RenderFrameHost* iframe =
tab->GetFrameTree()->root()->child_at(0)->current_frame_host();

EXPECT_NE(iframe->GetProcess(), main_frame->GetProcess());

// Try to get cross-site cookies from the subframe's process and wait for it
// to be killed.
std::string response;
FrameHostMsg_GetCookies illegal_get_cookies(
iframe->GetRoutingID(), GURL("http://127.0.0.1/"),
GURL("http://127.0.0.1/"), &response);

RenderProcessHostWatcher iframe_killed(
iframe->GetProcess(), RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);

IPC::IpcSecurityTestUtil::PwnMessageReceived(
iframe->GetProcess()->GetChannel(), illegal_get_cookies);

iframe_killed.Wait();

EXPECT_EQ(
" Site A ------------ proxies for B\n"
" +--Site B ------- proxies for A\n"
"Where A = http://127.0.0.1/\n"
" B = http://baz.com/ (no process)",
v.DepictFrameTree(tab->GetFrameTree()->root()));

// Now set a cross-site cookie from the main frame's process and wait for it
// to be killed.
RenderProcessHostWatcher main_frame_killed(
tab->GetMainFrame()->GetProcess(),
RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
FrameHostMsg_SetCookie illegal_set_cookie(tab->GetMainFrame()->GetRoutingID(),
GURL("https://baz.com/"),
GURL("https://baz.com/"), "pwn=ed");
IPC::IpcSecurityTestUtil::PwnMessageReceived(
tab->GetMainFrame()->GetProcess()->GetChannel(), illegal_set_cookie);

main_frame_killed.Wait();

EXPECT_EQ(
" Site A\n"
"Where A = http://127.0.0.1/ (no process)",
v.DepictFrameTree(tab->GetFrameTree()->root()));
}

} // namespace content
8 changes: 2 additions & 6 deletions content/browser/site_per_process_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ void PostMessageAndWaitForReply(FrameTreeNode* sender_ftn,
}
}

} // anonymous namespace

class RedirectNotificationObserver : public NotificationObserver {
public:
// Register to listen for notifications of the given type from either a
Expand Down Expand Up @@ -244,6 +242,8 @@ bool ConsoleObserverDelegate::AddMessageToConsole(
return false;
}

} // namespace

//
// SitePerProcessBrowserTest
//
Expand Down Expand Up @@ -1082,10 +1082,6 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, DISABLED_CrashSubframe) {

StartFrameAtDataURL();

// These must stay in scope with replace_host.
GURL::Replacements replace_host;
std::string foo_com("foo.com");

// Load cross-site page into iframe.
EXPECT_TRUE(NavigateIframeToURL(
shell()->web_contents(), "test",
Expand Down
1 change: 1 addition & 0 deletions content/content_tests.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@
'browser/plugin_browsertest.cc',
'browser/renderer_host/input/touch_action_browsertest.cc',
'browser/renderer_host/input/touch_input_browsertest.cc',
'browser/renderer_host/render_message_filter_browsertest.cc',
'browser/renderer_host/render_process_host_browsertest.cc',
'browser/renderer_host/render_view_host_browsertest.cc',
'browser/renderer_host/render_widget_host_view_browsertest.cc',
Expand Down
2 changes: 2 additions & 0 deletions tools/metrics/histograms/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -49509,6 +49509,8 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
<int value="75" label="FAMF_APPEND_SHARED_MEMORY_TO_STREAM"/>
<int value="76" label="IDBDH_CAN_READ_FILE"/>
<int value="77" label="IDBDH_GET_OR_TERMINATE"/>
<int value="78" label="RMF_SET_COOKIE_BAD_ORIGIN"/>
<int value="79" label="RMF_GET_COOKIES_BAD_ORIGIN"/>
</enum>

<enum name="BadMessageReasonExtensions" type="int">
Expand Down

0 comments on commit 5f3d760

Please sign in to comment.