Skip to content

Commit 3d6da28

Browse files
caseqcaseq
committed
DevTools: do not expose raw headers for cross-origin requests
Same as https://chromium-review.googlesource.com/c/chromium/src/+/821410/, but now for the network service. Bug: 898306, 793692, 721408 Change-Id: I96a2a25e66f4ff528d84baf03d600e4f1c89dd30 Reviewed-on: https://chromium-review.googlesource.com/c/1313739 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Matt Menke <mmenke@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Dmitry Gozman <dgozman@chromium.org> Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org> Commit-Queue: Andrey Kosyakov <caseq@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#612685}(cherry picked from commit 1608dec) Reviewed-on: https://chromium-review.googlesource.com/c/1361790 Reviewed-by: Andrey Kosyakov <caseq@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#52} Cr-Branched-From: d897fb1-refs/heads/master@{#612437}
1 parent cdd5484 commit 3d6da28

18 files changed

+291
-182
lines changed

‎content/browser/devtools/render_frame_devtools_agent_host.cc‎

Lines changed: 58 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,55 @@ void RenderFrameDevToolsAgentHost::WebContentsCreated(
182182
}
183183
}
184184

185+
// static
186+
void RenderFrameDevToolsAgentHost::UpdateRawHeadersAccess(
187+
RenderFrameHostImpl* old_rfh,
188+
RenderFrameHostImpl* new_rfh) {
189+
DCHECK_NE(old_rfh, new_rfh);
190+
RenderProcessHost* old_rph = old_rfh ? old_rfh->GetProcess() : nullptr;
191+
RenderProcessHost* new_rph = new_rfh ? new_rfh->GetProcess() : nullptr;
192+
if (old_rph == new_rph)
193+
return;
194+
std::set<url::Origin> old_process_origins;
195+
std::set<url::Origin> new_process_origins;
196+
for (const auto& entry : g_agent_host_instances.Get()) {
197+
RenderFrameHostImpl* frame_host = entry.second->frame_host_;
198+
if (!frame_host)
199+
continue;
200+
// Do not skip the nodes if they're about to get attached.
201+
if (!entry.second->IsAttached() &&
202+
(!new_rfh || entry.first != new_rfh->frame_tree_node())) {
203+
continue;
204+
}
205+
RenderProcessHost* process_host = frame_host->GetProcess();
206+
if (process_host == old_rph)
207+
old_process_origins.insert(frame_host->GetLastCommittedOrigin());
208+
else if (process_host == new_rph)
209+
new_process_origins.insert(frame_host->GetLastCommittedOrigin());
210+
}
211+
if (!base::FeatureList::IsEnabled(network::features::kNetworkService)) {
212+
if (old_rph && old_process_origins.empty()) {
213+
ChildProcessSecurityPolicyImpl::GetInstance()->RevokeReadRawCookies(
214+
old_rph->GetID());
215+
}
216+
if (new_rph && !new_process_origins.empty()) {
217+
ChildProcessSecurityPolicyImpl::GetInstance()->GrantReadRawCookies(
218+
new_rph->GetID());
219+
}
220+
return;
221+
}
222+
if (old_rph) {
223+
GetNetworkService()->SetRawHeadersAccess(
224+
old_rph->GetID(), std::vector<url::Origin>(old_process_origins.begin(),
225+
old_process_origins.end()));
226+
}
227+
if (new_rph) {
228+
GetNetworkService()->SetRawHeadersAccess(
229+
new_rph->GetID(), std::vector<url::Origin>(new_process_origins.begin(),
230+
new_process_origins.end()));
231+
}
232+
}
233+
185234
RenderFrameDevToolsAgentHost::RenderFrameDevToolsAgentHost(
186235
FrameTreeNode* frame_tree_node)
187236
: DevToolsAgentHostImpl(frame_tree_node->devtools_frame_token().ToString()),
@@ -266,7 +315,7 @@ bool RenderFrameDevToolsAgentHost::AttachSession(DevToolsSession* session) {
266315
// DevToolsFrameTraceRecorder. Taking snapshots happens in TracingHandler.
267316
if (!use_video_capture_api)
268317
frame_trace_recorder_.reset(new DevToolsFrameTraceRecorder());
269-
GrantPolicy();
318+
UpdateRawHeadersAccess(nullptr, frame_host_);
270319
#if defined(OS_ANDROID)
271320
GetWakeLock()->RequestWakeLock();
272321
#endif
@@ -278,7 +327,7 @@ void RenderFrameDevToolsAgentHost::DetachSession(DevToolsSession* session) {
278327
// Destroying session automatically detaches in renderer.
279328
if (sessions().empty()) {
280329
frame_trace_recorder_.reset();
281-
RevokePolicy();
330+
UpdateRawHeadersAccess(frame_host_, nullptr);
282331
#if defined(OS_ANDROID)
283332
GetWakeLock()->CancelWakeLock();
284333
#endif
@@ -371,10 +420,10 @@ void RenderFrameDevToolsAgentHost::UpdateFrameHost(
371420
return;
372421
}
373422

374-
if (IsAttached())
375-
RevokePolicy();
376-
423+
RenderFrameHostImpl* old_host = frame_host_;
377424
frame_host_ = frame_host;
425+
if (IsAttached())
426+
UpdateRawHeadersAccess(old_host, frame_host);
378427

379428
std::vector<DevToolsSession*> restricted_sessions;
380429
for (DevToolsSession* session : sessions()) {
@@ -390,46 +439,9 @@ void RenderFrameDevToolsAgentHost::UpdateFrameHost(
390439
inspector->TargetReloadedAfterCrash();
391440
}
392441

393-
if (IsAttached())
394-
GrantPolicy();
395442
UpdateRendererChannel(IsAttached());
396443
}
397444

398-
void RenderFrameDevToolsAgentHost::GrantPolicy() {
399-
if (!frame_host_)
400-
return;
401-
uint32_t process_id = frame_host_->GetProcess()->GetID();
402-
if (base::FeatureList::IsEnabled(network::features::kNetworkService))
403-
GetNetworkService()->SetRawHeadersAccess(process_id, true);
404-
ChildProcessSecurityPolicyImpl::GetInstance()->GrantReadRawCookies(
405-
process_id);
406-
}
407-
408-
void RenderFrameDevToolsAgentHost::RevokePolicy() {
409-
if (!frame_host_)
410-
return;
411-
412-
bool process_has_agents = false;
413-
RenderProcessHost* process_host = frame_host_->GetProcess();
414-
for (const auto& ftn_agent : g_agent_host_instances.Get()) {
415-
RenderFrameDevToolsAgentHost* agent = ftn_agent.second;
416-
if (!agent->IsAttached())
417-
continue;
418-
if (agent->frame_host_ && agent->frame_host_ != frame_host_ &&
419-
agent->frame_host_->GetProcess() == process_host) {
420-
process_has_agents = true;
421-
}
422-
}
423-
424-
// We are the last to disconnect from the renderer -> revoke permissions.
425-
if (!process_has_agents) {
426-
if (base::FeatureList::IsEnabled(network::features::kNetworkService))
427-
GetNetworkService()->SetRawHeadersAccess(process_host->GetID(), false);
428-
ChildProcessSecurityPolicyImpl::GetInstance()->RevokeReadRawCookies(
429-
process_host->GetID());
430-
}
431-
}
432-
433445
void RenderFrameDevToolsAgentHost::DidStartNavigation(
434446
NavigationHandle* navigation_handle) {
435447
NavigationHandleImpl* handle =
@@ -472,9 +484,10 @@ void RenderFrameDevToolsAgentHost::RenderFrameDeleted(RenderFrameHost* rfh) {
472484

473485
void RenderFrameDevToolsAgentHost::DestroyOnRenderFrameGone() {
474486
scoped_refptr<RenderFrameDevToolsAgentHost> protect(this);
475-
if (IsAttached())
476-
RevokePolicy();
477-
ForceDetachAllSessions();
487+
if (IsAttached()) {
488+
ForceDetachAllSessions();
489+
UpdateRawHeadersAccess(frame_host_, nullptr);
490+
}
478491
frame_host_ = nullptr;
479492
UpdateRendererChannel(IsAttached());
480493
SetFrameTreeNode(nullptr);

‎content/browser/devtools/render_frame_devtools_agent_host.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,10 @@ class CONTENT_EXPORT RenderFrameDevToolsAgentHost
9090

9191
private:
9292
friend class DevToolsAgentHost;
93+
94+
static void UpdateRawHeadersAccess(RenderFrameHostImpl* old_rfh,
95+
RenderFrameHostImpl* new_rfh);
96+
9397
explicit RenderFrameDevToolsAgentHost(FrameTreeNode*);
9498
~RenderFrameDevToolsAgentHost() override;
9599

@@ -118,8 +122,6 @@ class CONTENT_EXPORT RenderFrameDevToolsAgentHost
118122
void OnSwapCompositorFrame(const IPC::Message& message);
119123
void DestroyOnRenderFrameGone();
120124
void UpdateFrameHost(RenderFrameHostImpl* frame_host);
121-
void GrantPolicy();
122-
void RevokePolicy();
123125
void SetFrameTreeNode(FrameTreeNode* frame_tree_node);
124126

125127
bool ShouldAllowSession(DevToolsSession* session);

‎content/browser/loader/navigation_url_loader_impl_unittest.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ class TestNavigationLoaderInterceptor : public NavigationLoaderInterceptor {
9191
base::BindOnce(&TestNavigationLoaderInterceptor::DeleteURLLoader,
9292
base::Unretained(this)),
9393
std::move(request), 0 /* options */, resource_request,
94-
false /* report_raw_headers */, std::move(client),
95-
TRAFFIC_ANNOTATION_FOR_TESTS, &params, 0, /* request_id */
94+
std::move(client), TRAFFIC_ANNOTATION_FOR_TESTS, &params,
95+
0, /* request_id */
9696
resource_scheduler_client_, nullptr,
9797
nullptr /* network_usage_accumulator */, nullptr /* header_client */);
9898
}

‎content/browser/websockets/websocket_manager.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ class WebSocketManager::Delegate final : public network::WebSocket::Delegate {
8181
OnLostConnectionToClient(impl);
8282
}
8383

84-
bool CanReadRawCookies() override {
84+
bool CanReadRawCookies(const GURL& url) override {
8585
return ChildProcessSecurityPolicyImpl::GetInstance()->CanReadRawCookies(
8686
manager_->process_id_);
8787
}

‎services/network/network_context.cc‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -356,12 +356,24 @@ class NetworkContext::ContextNetworkDelegate
356356
GURL* new_url) override {
357357
if (!enable_referrers_)
358358
request->SetReferrer(std::string());
359-
if (network_context_->network_service())
360-
network_context_->network_service()->OnBeforeURLRequest();
359+
NetworkService* network_service = network_context_->network_service();
360+
if (network_service)
361+
network_service->OnBeforeURLRequest();
361362

362363
auto* loader = URLLoader::ForRequest(*request);
363-
if (loader && loader->new_redirect_url())
364+
if (!loader)
365+
return;
366+
const GURL* effective_url = nullptr;
367+
if (loader->new_redirect_url()) {
364368
*new_url = loader->new_redirect_url().value();
369+
effective_url = new_url;
370+
} else {
371+
effective_url = &request->url();
372+
}
373+
if (network_service) {
374+
loader->SetAllowReportingRawHeaders(network_service->HasRawHeadersAccess(
375+
loader->GetProcessId(), *effective_url));
376+
}
365377
}
366378

367379
void OnBeforeRedirectInternal(net::URLRequest* request,

‎services/network/network_service.cc‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -426,20 +426,27 @@ void NetworkService::ConfigureHttpAuthPrefs(
426426
#endif
427427
}
428428

429-
void NetworkService::SetRawHeadersAccess(uint32_t process_id, bool allow) {
429+
void NetworkService::SetRawHeadersAccess(
430+
uint32_t process_id,
431+
const std::vector<url::Origin>& origins) {
430432
DCHECK(process_id);
431-
if (allow)
432-
processes_with_raw_headers_access_.insert(process_id);
433-
else
434-
processes_with_raw_headers_access_.erase(process_id);
433+
if (!origins.size()) {
434+
raw_headers_access_origins_by_pid_.erase(process_id);
435+
} else {
436+
raw_headers_access_origins_by_pid_[process_id] =
437+
base::flat_set<url::Origin>(origins.begin(), origins.end());
438+
}
435439
}
436440

437-
bool NetworkService::HasRawHeadersAccess(uint32_t process_id) const {
441+
bool NetworkService::HasRawHeadersAccess(uint32_t process_id,
442+
const GURL& resource_url) const {
438443
// Allow raw headers for browser-initiated requests.
439444
if (!process_id)
440445
return true;
441-
return processes_with_raw_headers_access_.find(process_id) !=
442-
processes_with_raw_headers_access_.end();
446+
auto it = raw_headers_access_origins_by_pid_.find(process_id);
447+
if (it == raw_headers_access_origins_by_pid_.end())
448+
return false;
449+
return it->second.find(url::Origin::Create(resource_url)) != it->second.end();
443450
}
444451

445452
net::NetLog* NetworkService::net_log() const {

‎services/network/network_service.h‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include <string>
1111

1212
#include "base/component_export.h"
13+
#include "base/containers/flat_set.h"
1314
#include "base/containers/span.h"
1415
#include "base/containers/unique_ptr_adapters.h"
1516
#include "base/macros.h"
@@ -158,7 +159,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
158159
mojom::HttpAuthStaticParamsPtr http_auth_static_params) override;
159160
void ConfigureHttpAuthPrefs(
160161
mojom::HttpAuthDynamicParamsPtr http_auth_dynamic_params) override;
161-
void SetRawHeadersAccess(uint32_t process_id, bool allow) override;
162+
void SetRawHeadersAccess(uint32_t process_id,
163+
const std::vector<url::Origin>& origins) override;
162164
void GetNetworkChangeManager(
163165
mojom::NetworkChangeManagerRequest request) override;
164166
void GetNetworkQualityEstimatorManager(
@@ -192,7 +194,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
192194
void OnBeforeURLRequest();
193195

194196
bool quic_disabled() const { return quic_disabled_; }
195-
bool HasRawHeadersAccess(uint32_t process_id) const;
197+
bool HasRawHeadersAccess(uint32_t process_id, const GURL& resource_url) const;
196198

197199
mojom::NetworkServiceClient* client() { return client_.get(); }
198200
net::NetworkQualityEstimator* network_quality_estimator() {
@@ -296,7 +298,10 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
296298
// this with |owned_network_contexts_|.
297299
std::set<NetworkContext*> network_contexts_;
298300

299-
std::set<uint32_t> processes_with_raw_headers_access_;
301+
// A per-process_id map of origins that are white-listed to allow
302+
// them to request raw headers for resources they request.
303+
std::map<uint32_t, base::flat_set<url::Origin>>
304+
raw_headers_access_origins_by_pid_;
300305

301306
bool quic_disabled_ = false;
302307

‎services/network/network_service_unittest.cc‎

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include "services/network/network_context.h"
3737
#include "services/network/network_service.h"
3838
#include "services/network/public/cpp/features.h"
39+
#include "services/network/public/cpp/network_switches.h"
3940
#include "services/network/public/mojom/net_log.mojom.h"
4041
#include "services/network/public/mojom/network_change_manager.mojom.h"
4142
#include "services/network/public/mojom/network_service.mojom.h"
@@ -817,7 +818,10 @@ TEST_F(NetworkServiceTestWithService, RawRequestAccessControl) {
817818
StartLoadingURL(request, process_id);
818819
client()->RunUntilComplete();
819820
EXPECT_FALSE(client()->response_head().raw_request_response_info);
820-
service()->SetRawHeadersAccess(process_id, true);
821+
service()->SetRawHeadersAccess(
822+
process_id,
823+
{url::Origin::CreateFromNormalizedTuple("http", "example.com", 80),
824+
url::Origin::Create(request.url)});
821825
StartLoadingURL(request, process_id);
822826
client()->RunUntilComplete();
823827
{
@@ -828,10 +832,73 @@ TEST_F(NetworkServiceTestWithService, RawRequestAccessControl) {
828832
EXPECT_EQ("OK", request_response_info->http_status_text);
829833
}
830834

831-
service()->SetRawHeadersAccess(process_id, false);
835+
service()->SetRawHeadersAccess(process_id, {});
832836
StartLoadingURL(request, process_id);
833837
client()->RunUntilComplete();
834838
EXPECT_FALSE(client()->response_head().raw_request_response_info.get());
839+
840+
service()->SetRawHeadersAccess(
841+
process_id,
842+
{url::Origin::CreateFromNormalizedTuple("http", "example.com", 80)});
843+
StartLoadingURL(request, process_id);
844+
client()->RunUntilComplete();
845+
EXPECT_FALSE(client()->response_head().raw_request_response_info.get());
846+
}
847+
848+
class NetworkServiceTestWithResolverMap : public NetworkServiceTestWithService {
849+
void SetUp() override {
850+
base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
851+
network::switches::kHostResolverRules, "MAP *.test 127.0.0.1");
852+
NetworkServiceTestWithService::SetUp();
853+
}
854+
};
855+
856+
TEST_F(NetworkServiceTestWithResolverMap, RawRequestAccessControlWithRedirect) {
857+
CreateNetworkContext();
858+
859+
const uint32_t process_id = 42;
860+
// initial_url in a.test redirects to b_url (in b.test) that then redirects to
861+
// url_a in a.test.
862+
GURL url_a = test_server()->GetURL("a.test", "/echo");
863+
GURL url_b =
864+
test_server()->GetURL("b.test", "/server-redirect?" + url_a.spec());
865+
GURL initial_url =
866+
test_server()->GetURL("a.test", "/server-redirect?" + url_b.spec());
867+
ResourceRequest request;
868+
request.url = initial_url;
869+
request.method = "GET";
870+
request.report_raw_headers = true;
871+
request.request_initiator = url::Origin();
872+
873+
service()->SetRawHeadersAccess(process_id, {url::Origin::Create(url_a)});
874+
875+
StartLoadingURL(request, process_id);
876+
client()->RunUntilRedirectReceived(); // from a.test to b.test
877+
EXPECT_TRUE(client()->response_head().raw_request_response_info);
878+
879+
loader()->FollowRedirect(base::nullopt, base::nullopt, base::nullopt);
880+
client()->ClearHasReceivedRedirect();
881+
client()->RunUntilRedirectReceived(); // from b.test to a.test
882+
EXPECT_FALSE(client()->response_head().raw_request_response_info);
883+
884+
loader()->FollowRedirect(base::nullopt, base::nullopt, base::nullopt);
885+
client()->RunUntilComplete(); // Done loading a.test
886+
EXPECT_TRUE(client()->response_head().raw_request_response_info.get());
887+
888+
service()->SetRawHeadersAccess(process_id, {url::Origin::Create(url_b)});
889+
890+
StartLoadingURL(request, process_id);
891+
client()->RunUntilRedirectReceived(); // from a.test to b.test
892+
EXPECT_FALSE(client()->response_head().raw_request_response_info);
893+
894+
loader()->FollowRedirect(base::nullopt, base::nullopt, base::nullopt);
895+
client()->ClearHasReceivedRedirect();
896+
client()->RunUntilRedirectReceived(); // from b.test to a.test
897+
EXPECT_TRUE(client()->response_head().raw_request_response_info);
898+
899+
loader()->FollowRedirect(base::nullopt, base::nullopt, base::nullopt);
900+
client()->RunUntilComplete(); // Done loading a.test
901+
EXPECT_FALSE(client()->response_head().raw_request_response_info.get());
835902
}
836903

837904
TEST_F(NetworkServiceTestWithService, SetNetworkConditions) {

0 commit comments

Comments
 (0)