From 1608dec4b25fae0d3ee57bfab004c3fe6140bd6d Mon Sep 17 00:00:00 2001 From: Andrey Kosyakov Date: Fri, 30 Nov 2018 17:39:00 +0000 Subject: [PATCH] DevTools: do not expose raw headers for cross-origin requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Matt Menke Reviewed-by: Yutaka Hirano Reviewed-by: Dmitry Gozman Reviewed-by: Łukasz Anforowicz Commit-Queue: Andrey Kosyakov Cr-Commit-Position: refs/heads/master@{#612685} --- .../render_frame_devtools_agent_host.cc | 103 ++++++------ .../render_frame_devtools_agent_host.h | 6 +- .../navigation_url_loader_impl_unittest.cc | 4 +- .../browser/websockets/websocket_manager.cc | 2 +- services/network/network_context.cc | 18 ++- services/network/network_service.cc | 23 ++- services/network/network_service.h | 11 +- services/network/network_service_unittest.cc | 71 ++++++++- .../public/mojom/network_service.mojom | 13 +- services/network/public/mojom/websocket.mojom | 2 - services/network/url_loader.cc | 13 +- services/network/url_loader.h | 6 +- services/network/url_loader_factory.cc | 13 +- services/network/url_loader_unittest.cc | 150 +++++++++--------- services/network/websocket.cc | 29 ++-- services/network/websocket.h | 2 +- services/network/websocket_factory.cc | 4 +- .../enable-features=NetworkService | 3 - 18 files changed, 291 insertions(+), 182 deletions(-) diff --git a/content/browser/devtools/render_frame_devtools_agent_host.cc b/content/browser/devtools/render_frame_devtools_agent_host.cc index 18b0397a618a..991a10dd3135 100644 --- a/content/browser/devtools/render_frame_devtools_agent_host.cc +++ b/content/browser/devtools/render_frame_devtools_agent_host.cc @@ -182,6 +182,55 @@ void RenderFrameDevToolsAgentHost::WebContentsCreated( } } +// static +void RenderFrameDevToolsAgentHost::UpdateRawHeadersAccess( + RenderFrameHostImpl* old_rfh, + RenderFrameHostImpl* new_rfh) { + DCHECK_NE(old_rfh, new_rfh); + RenderProcessHost* old_rph = old_rfh ? old_rfh->GetProcess() : nullptr; + RenderProcessHost* new_rph = new_rfh ? new_rfh->GetProcess() : nullptr; + if (old_rph == new_rph) + return; + std::set old_process_origins; + std::set new_process_origins; + for (const auto& entry : g_agent_host_instances.Get()) { + RenderFrameHostImpl* frame_host = entry.second->frame_host_; + if (!frame_host) + continue; + // Do not skip the nodes if they're about to get attached. + if (!entry.second->IsAttached() && + (!new_rfh || entry.first != new_rfh->frame_tree_node())) { + continue; + } + RenderProcessHost* process_host = frame_host->GetProcess(); + if (process_host == old_rph) + old_process_origins.insert(frame_host->GetLastCommittedOrigin()); + else if (process_host == new_rph) + new_process_origins.insert(frame_host->GetLastCommittedOrigin()); + } + if (!base::FeatureList::IsEnabled(network::features::kNetworkService)) { + if (old_rph && old_process_origins.empty()) { + ChildProcessSecurityPolicyImpl::GetInstance()->RevokeReadRawCookies( + old_rph->GetID()); + } + if (new_rph && !new_process_origins.empty()) { + ChildProcessSecurityPolicyImpl::GetInstance()->GrantReadRawCookies( + new_rph->GetID()); + } + return; + } + if (old_rph) { + GetNetworkService()->SetRawHeadersAccess( + old_rph->GetID(), std::vector(old_process_origins.begin(), + old_process_origins.end())); + } + if (new_rph) { + GetNetworkService()->SetRawHeadersAccess( + new_rph->GetID(), std::vector(new_process_origins.begin(), + new_process_origins.end())); + } +} + RenderFrameDevToolsAgentHost::RenderFrameDevToolsAgentHost( FrameTreeNode* frame_tree_node) : DevToolsAgentHostImpl(frame_tree_node->devtools_frame_token().ToString()), @@ -266,7 +315,7 @@ bool RenderFrameDevToolsAgentHost::AttachSession(DevToolsSession* session) { // DevToolsFrameTraceRecorder. Taking snapshots happens in TracingHandler. if (!use_video_capture_api) frame_trace_recorder_.reset(new DevToolsFrameTraceRecorder()); - GrantPolicy(); + UpdateRawHeadersAccess(nullptr, frame_host_); #if defined(OS_ANDROID) GetWakeLock()->RequestWakeLock(); #endif @@ -278,7 +327,7 @@ void RenderFrameDevToolsAgentHost::DetachSession(DevToolsSession* session) { // Destroying session automatically detaches in renderer. if (sessions().empty()) { frame_trace_recorder_.reset(); - RevokePolicy(); + UpdateRawHeadersAccess(frame_host_, nullptr); #if defined(OS_ANDROID) GetWakeLock()->CancelWakeLock(); #endif @@ -371,10 +420,10 @@ void RenderFrameDevToolsAgentHost::UpdateFrameHost( return; } - if (IsAttached()) - RevokePolicy(); - + RenderFrameHostImpl* old_host = frame_host_; frame_host_ = frame_host; + if (IsAttached()) + UpdateRawHeadersAccess(old_host, frame_host); std::vector restricted_sessions; for (DevToolsSession* session : sessions()) { @@ -390,46 +439,9 @@ void RenderFrameDevToolsAgentHost::UpdateFrameHost( inspector->TargetReloadedAfterCrash(); } - if (IsAttached()) - GrantPolicy(); UpdateRendererChannel(IsAttached()); } -void RenderFrameDevToolsAgentHost::GrantPolicy() { - if (!frame_host_) - return; - uint32_t process_id = frame_host_->GetProcess()->GetID(); - if (base::FeatureList::IsEnabled(network::features::kNetworkService)) - GetNetworkService()->SetRawHeadersAccess(process_id, true); - ChildProcessSecurityPolicyImpl::GetInstance()->GrantReadRawCookies( - process_id); -} - -void RenderFrameDevToolsAgentHost::RevokePolicy() { - if (!frame_host_) - return; - - bool process_has_agents = false; - RenderProcessHost* process_host = frame_host_->GetProcess(); - for (const auto& ftn_agent : g_agent_host_instances.Get()) { - RenderFrameDevToolsAgentHost* agent = ftn_agent.second; - if (!agent->IsAttached()) - continue; - if (agent->frame_host_ && agent->frame_host_ != frame_host_ && - agent->frame_host_->GetProcess() == process_host) { - process_has_agents = true; - } - } - - // We are the last to disconnect from the renderer -> revoke permissions. - if (!process_has_agents) { - if (base::FeatureList::IsEnabled(network::features::kNetworkService)) - GetNetworkService()->SetRawHeadersAccess(process_host->GetID(), false); - ChildProcessSecurityPolicyImpl::GetInstance()->RevokeReadRawCookies( - process_host->GetID()); - } -} - void RenderFrameDevToolsAgentHost::DidStartNavigation( NavigationHandle* navigation_handle) { NavigationHandleImpl* handle = @@ -472,9 +484,10 @@ void RenderFrameDevToolsAgentHost::RenderFrameDeleted(RenderFrameHost* rfh) { void RenderFrameDevToolsAgentHost::DestroyOnRenderFrameGone() { scoped_refptr protect(this); - if (IsAttached()) - RevokePolicy(); - ForceDetachAllSessions(); + if (IsAttached()) { + ForceDetachAllSessions(); + UpdateRawHeadersAccess(frame_host_, nullptr); + } frame_host_ = nullptr; UpdateRendererChannel(IsAttached()); SetFrameTreeNode(nullptr); diff --git a/content/browser/devtools/render_frame_devtools_agent_host.h b/content/browser/devtools/render_frame_devtools_agent_host.h index 40a1dcedd1be..a2281d79339d 100644 --- a/content/browser/devtools/render_frame_devtools_agent_host.h +++ b/content/browser/devtools/render_frame_devtools_agent_host.h @@ -90,6 +90,10 @@ class CONTENT_EXPORT RenderFrameDevToolsAgentHost private: friend class DevToolsAgentHost; + + static void UpdateRawHeadersAccess(RenderFrameHostImpl* old_rfh, + RenderFrameHostImpl* new_rfh); + explicit RenderFrameDevToolsAgentHost(FrameTreeNode*); ~RenderFrameDevToolsAgentHost() override; @@ -118,8 +122,6 @@ class CONTENT_EXPORT RenderFrameDevToolsAgentHost void OnSwapCompositorFrame(const IPC::Message& message); void DestroyOnRenderFrameGone(); void UpdateFrameHost(RenderFrameHostImpl* frame_host); - void GrantPolicy(); - void RevokePolicy(); void SetFrameTreeNode(FrameTreeNode* frame_tree_node); bool ShouldAllowSession(DevToolsSession* session); diff --git a/content/browser/loader/navigation_url_loader_impl_unittest.cc b/content/browser/loader/navigation_url_loader_impl_unittest.cc index a4552b6fc64d..5f254e8cd00c 100644 --- a/content/browser/loader/navigation_url_loader_impl_unittest.cc +++ b/content/browser/loader/navigation_url_loader_impl_unittest.cc @@ -91,8 +91,8 @@ class TestNavigationLoaderInterceptor : public NavigationLoaderInterceptor { base::BindOnce(&TestNavigationLoaderInterceptor::DeleteURLLoader, base::Unretained(this)), std::move(request), 0 /* options */, resource_request, - false /* report_raw_headers */, std::move(client), - TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0, /* request_id */ + std::move(client), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, + 0, /* request_id */ resource_scheduler_client_, nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); } diff --git a/content/browser/websockets/websocket_manager.cc b/content/browser/websockets/websocket_manager.cc index 3f51142864c6..a825a6d71bd2 100644 --- a/content/browser/websockets/websocket_manager.cc +++ b/content/browser/websockets/websocket_manager.cc @@ -81,7 +81,7 @@ class WebSocketManager::Delegate final : public network::WebSocket::Delegate { OnLostConnectionToClient(impl); } - bool CanReadRawCookies() override { + bool CanReadRawCookies(const GURL& url) override { return ChildProcessSecurityPolicyImpl::GetInstance()->CanReadRawCookies( manager_->process_id_); } diff --git a/services/network/network_context.cc b/services/network/network_context.cc index f9488a1daa80..00615f99adef 100644 --- a/services/network/network_context.cc +++ b/services/network/network_context.cc @@ -356,12 +356,24 @@ class NetworkContext::ContextNetworkDelegate GURL* new_url) override { if (!enable_referrers_) request->SetReferrer(std::string()); - if (network_context_->network_service()) - network_context_->network_service()->OnBeforeURLRequest(); + NetworkService* network_service = network_context_->network_service(); + if (network_service) + network_service->OnBeforeURLRequest(); auto* loader = URLLoader::ForRequest(*request); - if (loader && loader->new_redirect_url()) + if (!loader) + return; + const GURL* effective_url = nullptr; + if (loader->new_redirect_url()) { *new_url = loader->new_redirect_url().value(); + effective_url = new_url; + } else { + effective_url = &request->url(); + } + if (network_service) { + loader->SetAllowReportingRawHeaders(network_service->HasRawHeadersAccess( + loader->GetProcessId(), *effective_url)); + } } void OnBeforeRedirectInternal(net::URLRequest* request, diff --git a/services/network/network_service.cc b/services/network/network_service.cc index dd50a53680e6..57746b17a574 100644 --- a/services/network/network_service.cc +++ b/services/network/network_service.cc @@ -426,20 +426,27 @@ void NetworkService::ConfigureHttpAuthPrefs( #endif } -void NetworkService::SetRawHeadersAccess(uint32_t process_id, bool allow) { +void NetworkService::SetRawHeadersAccess( + uint32_t process_id, + const std::vector& origins) { DCHECK(process_id); - if (allow) - processes_with_raw_headers_access_.insert(process_id); - else - processes_with_raw_headers_access_.erase(process_id); + if (!origins.size()) { + raw_headers_access_origins_by_pid_.erase(process_id); + } else { + raw_headers_access_origins_by_pid_[process_id] = + base::flat_set(origins.begin(), origins.end()); + } } -bool NetworkService::HasRawHeadersAccess(uint32_t process_id) const { +bool NetworkService::HasRawHeadersAccess(uint32_t process_id, + const GURL& resource_url) const { // Allow raw headers for browser-initiated requests. if (!process_id) return true; - return processes_with_raw_headers_access_.find(process_id) != - processes_with_raw_headers_access_.end(); + auto it = raw_headers_access_origins_by_pid_.find(process_id); + if (it == raw_headers_access_origins_by_pid_.end()) + return false; + return it->second.find(url::Origin::Create(resource_url)) != it->second.end(); } net::NetLog* NetworkService::net_log() const { diff --git a/services/network/network_service.h b/services/network/network_service.h index be4f3bd6a13a..01e76ca41d84 100644 --- a/services/network/network_service.h +++ b/services/network/network_service.h @@ -10,6 +10,7 @@ #include #include "base/component_export.h" +#include "base/containers/flat_set.h" #include "base/containers/span.h" #include "base/containers/unique_ptr_adapters.h" #include "base/macros.h" @@ -158,7 +159,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService mojom::HttpAuthStaticParamsPtr http_auth_static_params) override; void ConfigureHttpAuthPrefs( mojom::HttpAuthDynamicParamsPtr http_auth_dynamic_params) override; - void SetRawHeadersAccess(uint32_t process_id, bool allow) override; + void SetRawHeadersAccess(uint32_t process_id, + const std::vector& origins) override; void GetNetworkChangeManager( mojom::NetworkChangeManagerRequest request) override; void GetNetworkQualityEstimatorManager( @@ -192,7 +194,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService void OnBeforeURLRequest(); bool quic_disabled() const { return quic_disabled_; } - bool HasRawHeadersAccess(uint32_t process_id) const; + bool HasRawHeadersAccess(uint32_t process_id, const GURL& resource_url) const; mojom::NetworkServiceClient* client() { return client_.get(); } net::NetworkQualityEstimator* network_quality_estimator() { @@ -296,7 +298,10 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService // this with |owned_network_contexts_|. std::set network_contexts_; - std::set processes_with_raw_headers_access_; + // A per-process_id map of origins that are white-listed to allow + // them to request raw headers for resources they request. + std::map> + raw_headers_access_origins_by_pid_; bool quic_disabled_ = false; diff --git a/services/network/network_service_unittest.cc b/services/network/network_service_unittest.cc index 8da2228f1e77..4ef51b53337a 100644 --- a/services/network/network_service_unittest.cc +++ b/services/network/network_service_unittest.cc @@ -36,6 +36,7 @@ #include "services/network/network_context.h" #include "services/network/network_service.h" #include "services/network/public/cpp/features.h" +#include "services/network/public/cpp/network_switches.h" #include "services/network/public/mojom/net_log.mojom.h" #include "services/network/public/mojom/network_change_manager.mojom.h" #include "services/network/public/mojom/network_service.mojom.h" @@ -817,7 +818,10 @@ TEST_F(NetworkServiceTestWithService, RawRequestAccessControl) { StartLoadingURL(request, process_id); client()->RunUntilComplete(); EXPECT_FALSE(client()->response_head().raw_request_response_info); - service()->SetRawHeadersAccess(process_id, true); + service()->SetRawHeadersAccess( + process_id, + {url::Origin::CreateFromNormalizedTuple("http", "example.com", 80), + url::Origin::Create(request.url)}); StartLoadingURL(request, process_id); client()->RunUntilComplete(); { @@ -828,10 +832,73 @@ TEST_F(NetworkServiceTestWithService, RawRequestAccessControl) { EXPECT_EQ("OK", request_response_info->http_status_text); } - service()->SetRawHeadersAccess(process_id, false); + service()->SetRawHeadersAccess(process_id, {}); StartLoadingURL(request, process_id); client()->RunUntilComplete(); EXPECT_FALSE(client()->response_head().raw_request_response_info.get()); + + service()->SetRawHeadersAccess( + process_id, + {url::Origin::CreateFromNormalizedTuple("http", "example.com", 80)}); + StartLoadingURL(request, process_id); + client()->RunUntilComplete(); + EXPECT_FALSE(client()->response_head().raw_request_response_info.get()); +} + +class NetworkServiceTestWithResolverMap : public NetworkServiceTestWithService { + void SetUp() override { + base::CommandLine::ForCurrentProcess()->AppendSwitchASCII( + network::switches::kHostResolverRules, "MAP *.test 127.0.0.1"); + NetworkServiceTestWithService::SetUp(); + } +}; + +TEST_F(NetworkServiceTestWithResolverMap, RawRequestAccessControlWithRedirect) { + CreateNetworkContext(); + + const uint32_t process_id = 42; + // initial_url in a.test redirects to b_url (in b.test) that then redirects to + // url_a in a.test. + GURL url_a = test_server()->GetURL("a.test", "/echo"); + GURL url_b = + test_server()->GetURL("b.test", "/server-redirect?" + url_a.spec()); + GURL initial_url = + test_server()->GetURL("a.test", "/server-redirect?" + url_b.spec()); + ResourceRequest request; + request.url = initial_url; + request.method = "GET"; + request.report_raw_headers = true; + request.request_initiator = url::Origin(); + + service()->SetRawHeadersAccess(process_id, {url::Origin::Create(url_a)}); + + StartLoadingURL(request, process_id); + client()->RunUntilRedirectReceived(); // from a.test to b.test + EXPECT_TRUE(client()->response_head().raw_request_response_info); + + loader()->FollowRedirect(base::nullopt, base::nullopt, base::nullopt); + client()->ClearHasReceivedRedirect(); + client()->RunUntilRedirectReceived(); // from b.test to a.test + EXPECT_FALSE(client()->response_head().raw_request_response_info); + + loader()->FollowRedirect(base::nullopt, base::nullopt, base::nullopt); + client()->RunUntilComplete(); // Done loading a.test + EXPECT_TRUE(client()->response_head().raw_request_response_info.get()); + + service()->SetRawHeadersAccess(process_id, {url::Origin::Create(url_b)}); + + StartLoadingURL(request, process_id); + client()->RunUntilRedirectReceived(); // from a.test to b.test + EXPECT_FALSE(client()->response_head().raw_request_response_info); + + loader()->FollowRedirect(base::nullopt, base::nullopt, base::nullopt); + client()->ClearHasReceivedRedirect(); + client()->RunUntilRedirectReceived(); // from b.test to a.test + EXPECT_TRUE(client()->response_head().raw_request_response_info); + + loader()->FollowRedirect(base::nullopt, base::nullopt, base::nullopt); + client()->RunUntilComplete(); // Done loading a.test + EXPECT_FALSE(client()->response_head().raw_request_response_info.get()); } TEST_F(NetworkServiceTestWithService, SetNetworkConditions) { diff --git a/services/network/public/mojom/network_service.mojom b/services/network/public/mojom/network_service.mojom index 0ac205326cc4..11c47620304d 100644 --- a/services/network/public/mojom/network_service.mojom +++ b/services/network/public/mojom/network_service.mojom @@ -291,10 +291,15 @@ interface NetworkService { ConfigureHttpAuthPrefs(HttpAuthDynamicParams http_auth_dynamic_params); // Specifies whether requests for raw headers coming through URLLoaderFactory - // associated with the specified process will be granted. Granting such a - // permission increases risks in case the child process becomes compromised, - // so this should be done only in specific cases (e.g. DevTools attached). - SetRawHeadersAccess(uint32 process_id, bool allow); + // associated with the specified process will be granted. Only raw headers + // for requests with URLs matching a listed origin will be reported (this + // permission has no effect on the network request itself). + // The list overwrites the list set for given process with a previous call + // to this method. + // Granting a permission increases risks in case the child process becomes + // compromised, so this should be done only in specific cases + // (e.g. DevTools attached). + SetRawHeadersAccess(uint32 process_id, array origins); // Gets the NetworkChangeManager. GetNetworkChangeManager( diff --git a/services/network/public/mojom/websocket.mojom b/services/network/public/mojom/websocket.mojom index b007d38a7c41..bc9bde7ad034 100644 --- a/services/network/public/mojom/websocket.mojom +++ b/services/network/public/mojom/websocket.mojom @@ -52,8 +52,6 @@ interface WebSocketClient { OnFailChannel(string reason); // Notify the renderer that the browser has started an opening handshake. - // This message is for showing the request in the inspector and - // can be omitted if the inspector is not active. OnStartOpeningHandshake(WebSocketHandshakeRequest request); // Notify the renderer that the browser has finished an opening handshake. diff --git a/services/network/url_loader.cc b/services/network/url_loader.cc index 7b2c72c6434c..3a164319007f 100644 --- a/services/network/url_loader.cc +++ b/services/network/url_loader.cc @@ -307,7 +307,6 @@ URLLoader::URLLoader( mojom::URLLoaderRequest url_loader_request, int32_t options, const ResourceRequest& request, - bool report_raw_headers, mojom::URLLoaderClientPtr url_loader_client, const net::NetworkTrafficAnnotationTag& traffic_annotation, const mojom::URLLoaderFactoryParams* factory_params, @@ -336,7 +335,8 @@ URLLoader::URLLoader( peer_closed_handle_watcher_(FROM_HERE, mojo::SimpleWatcher::ArmingPolicy::MANUAL, base::SequencedTaskRunnerHandle::Get()), - report_raw_headers_(report_raw_headers), + want_raw_headers_(request.report_raw_headers), + report_raw_headers_(false), resource_scheduler_client_(std::move(resource_scheduler_client)), keepalive_statistics_recorder_(std::move(keepalive_statistics_recorder)), network_usage_accumulator_(std::move(network_usage_accumulator)), @@ -358,13 +358,12 @@ URLLoader::URLLoader( << "disabled, as that skips security checks in ResourceDispatcherHost. " << "The only acceptable usage is the browser using SimpleURLLoader."; } - if (report_raw_headers_) { + if (want_raw_headers_) { options_ |= mojom::kURLLoadOptionSendSSLInfoWithResponse | mojom::kURLLoadOptionSendSSLInfoForCertificateError; } binding_.set_connection_error_handler( base::BindOnce(&URLLoader::OnConnectionError, base::Unretained(this))); - url_request_ = url_request_context_->CreateRequest( GURL(request.url), request.priority, this, traffic_annotation); url_request_->set_method(request.method); @@ -416,7 +415,7 @@ URLLoader::URLLoader( url_request_->set_allow_credentials(false); } - if (report_raw_headers_) { + if (want_raw_headers_) { url_request_->SetRequestHeadersCallback( base::Bind(&net::HttpRawRequestHeaders::Assign, base::Unretained(&raw_request_headers_))); @@ -1023,6 +1022,10 @@ uint32_t URLLoader::GetProcessId() const { return factory_params_->process_id; } +void URLLoader::SetAllowReportingRawHeaders(bool allow) { + report_raw_headers_ = want_raw_headers_ && allow; +} + // static URLLoader* URLLoader::ForRequest(const net::URLRequest& request) { auto* pointer = diff --git a/services/network/url_loader.h b/services/network/url_loader.h index 25635750b51a..aedaec527527 100644 --- a/services/network/url_loader.h +++ b/services/network/url_loader.h @@ -60,7 +60,6 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) URLLoader mojom::URLLoaderRequest url_loader_request, int32_t options, const ResourceRequest& request, - bool report_raw_headers, mojom::URLLoaderClientPtr url_loader_client, const net::NetworkTrafficAnnotationTag& traffic_annotation, const mojom::URLLoaderFactoryParams* factory_params, @@ -132,6 +131,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) URLLoader return new_redirect_url_; } + void SetAllowReportingRawHeaders(bool allow); + // Gets the URLLoader associated with this request. static URLLoader* ForRequest(const net::URLRequest& request); @@ -253,6 +254,9 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) URLLoader std::unique_ptr resource_scheduler_request_handle_; + // Whether client requested raw headers. + const bool want_raw_headers_; + // Whether we actually should report them. bool report_raw_headers_; net::HttpRawRequestHeaders raw_request_headers_; scoped_refptr raw_response_headers_; diff --git a/services/network/url_loader_factory.cc b/services/network/url_loader_factory.cc index c6b5ddd25d44..0b93d46285aa 100644 --- a/services/network/url_loader_factory.cc +++ b/services/network/url_loader_factory.cc @@ -83,16 +83,6 @@ void URLLoaderFactory::CreateLoaderAndStart( origin_head_same_as_request_origin); } - bool report_raw_headers = false; - if (url_request.report_raw_headers) { - const NetworkService* service = context_->network_service(); - report_raw_headers = - service && service->HasRawHeadersAccess(params_->process_id); - if (!report_raw_headers) - DLOG(ERROR) << "Denying raw headers request by process " - << params_->process_id; - } - mojom::NetworkServiceClient* network_service_client = nullptr; base::WeakPtr keepalive_statistics_recorder; base::WeakPtr network_usage_accumulator; @@ -143,8 +133,7 @@ void URLLoaderFactory::CreateLoaderAndStart( context_->url_request_context(), network_service_client, base::BindOnce(&cors::CorsURLLoaderFactory::DestroyURLLoader, base::Unretained(cors_url_loader_factory_)), - std::move(request), options, url_request, report_raw_headers, - std::move(client), + std::move(request), options, url_request, std::move(client), static_cast(traffic_annotation), params_.get(), request_id, resource_scheduler_client_, std::move(keepalive_statistics_recorder), diff --git a/services/network/url_loader_unittest.cc b/services/network/url_loader_unittest.cc index b4b78cf029cc..0112c579acdc 100644 --- a/services/network/url_loader_unittest.cc +++ b/services/network/url_loader_unittest.cc @@ -434,7 +434,7 @@ class URLLoaderTest : public testing::Test { url_loader = std::make_unique( context(), network_service_client.get(), DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), options, request, false, + mojo::MakeRequest(&loader), options, request, client_.CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); @@ -962,9 +962,9 @@ TEST_F(URLLoaderTest, DestroyOnURLLoaderPipeClosed) { url_loader = std::make_unique( context(), nullptr /* network_service_client */, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); // Run until the response body pipe arrives, to make sure that a live body @@ -1014,9 +1014,9 @@ TEST_F(URLLoaderTest, CloseResponseBodyConsumerBeforeProducer) { url_loader = std::make_unique( context(), nullptr /* network_service_client */, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); client()->RunUntilResponseBodyArrived(); @@ -1068,9 +1068,9 @@ TEST_F(URLLoaderTest, PauseReadingBodyFromNetBeforeResponseHeaders) { url_loader = std::make_unique( context(), nullptr /* network_service_client */, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); // Pausing reading response body from network stops future reads from the @@ -1144,9 +1144,9 @@ TEST_F(URLLoaderTest, PauseReadingBodyFromNetWhenReadIsPending) { url_loader = std::make_unique( context(), nullptr /* network_service_client */, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); response_controller.WaitForRequest(); @@ -1209,9 +1209,9 @@ TEST_F(URLLoaderTest, ResumeReadingBodyFromNetAfterClosingConsumer) { url_loader = std::make_unique( context(), nullptr /* network_service_client */, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); loader->PauseReadingBodyFromNet(); @@ -1269,9 +1269,9 @@ TEST_F(URLLoaderTest, MultiplePauseResumeReadingBodyFromNet) { url_loader = std::make_unique( context(), nullptr /* network_service_client */, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); // It is okay to call ResumeReadingBodyFromNet() even if there is no prior @@ -1451,9 +1451,9 @@ TEST_F(URLLoaderTest, UploadFileCanceled) { std::unique_ptr url_loader = std::make_unique( context(), network_service_client.get(), DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); mojom::NetworkServiceClient::OnFileUploadRequestedCallback callback; @@ -1635,9 +1635,9 @@ TEST_F(URLLoaderTest, UploadChunkedDataPipe) { url_loader = std::make_unique( context(), nullptr /* network_service_client */, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false /* report_raw_headers */, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, nullptr /* resource_scheduler_client */, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + nullptr /* resource_scheduler_client */, nullptr /* keepalive_statistics_reporter */, nullptr /* network_usage_accumulator */, nullptr /* header_client */); @@ -1703,7 +1703,7 @@ TEST_F(URLLoaderTest, RedirectModifiedHeaders) { url_loader = std::make_unique( context(), nullptr /* network_service_client */, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), mojom::kURLLoadOptionNone, request, false, + mojo::MakeRequest(&loader), mojom::kURLLoadOptionNone, request, client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); @@ -1748,7 +1748,7 @@ TEST_F(URLLoaderTest, RedirectRemoveHeader) { url_loader = std::make_unique( context(), nullptr /* network_service_client */, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), mojom::kURLLoadOptionNone, request, false, + mojo::MakeRequest(&loader), mojom::kURLLoadOptionNone, request, client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); @@ -1790,7 +1790,7 @@ TEST_F(URLLoaderTest, RedirectRemoveHeaderAndAddItBack) { url_loader = std::make_unique( context(), nullptr /* network_service_client */, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), mojom::kURLLoadOptionNone, request, false, + mojo::MakeRequest(&loader), mojom::kURLLoadOptionNone, request, client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); @@ -1908,7 +1908,7 @@ TEST_F(URLLoaderTest, ResourceSchedulerIntegration) { std::unique_ptr url_loader = std::make_unique( context(), nullptr /* network_service_client */, NeverInvokedDeleteLoaderCallback(), - mojo::MakeRequest(&loaderInterfacePtr), 0, request, false, + mojo::MakeRequest(&loaderInterfacePtr), 0, request, client.CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); @@ -1929,7 +1929,7 @@ TEST_F(URLLoaderTest, ResourceSchedulerIntegration) { std::unique_ptr loader = std::make_unique( context(), nullptr /* network_service_client */, NeverInvokedDeleteLoaderCallback(), - mojo::MakeRequest(&loader_interface_ptr), 0, request, false, + mojo::MakeRequest(&loader_interface_ptr), 0, request, client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); @@ -1964,7 +1964,7 @@ TEST_F(URLLoaderTest, ReadPipeClosedWhileReadTaskPosted) { url_loader = std::make_unique( context(), nullptr /* network_service_client */, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), mojom::kURLLoadOptionNone, request, false, + mojo::MakeRequest(&loader), mojom::kURLLoadOptionNone, request, client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); @@ -2018,7 +2018,7 @@ TEST_F(URLLoaderTest, EnterSuspendModeWhileNoPendingRead) { url_loader = std::make_unique( context(), nullptr /* network_service_client */, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), mojom::kURLLoadOptionNone, request, false, + mojo::MakeRequest(&loader), mojom::kURLLoadOptionNone, request, client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); @@ -2066,8 +2066,8 @@ TEST_F(URLLoaderTest, EnterSuspendModePaused) { context(), nullptr /* network_service_client */, DeleteLoaderCallback(&delete_run_loop, &url_loader), mojo::MakeRequest(&loader), mojom::kURLLoadOptionSniffMimeType, request, - false, client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, - ¶ms, 0 /* request_id */, resource_scheduler_client(), nullptr, + client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, + 0 /* request_id */, resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); url_loader->PauseReadingBodyFromNet(); @@ -2109,7 +2109,7 @@ TEST_F(URLLoaderTest, EnterSuspendDiskCacheWriteQueued) { url_loader = std::make_unique( context(), nullptr /* network_service_client */, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), mojom::kURLLoadOptionNone, request, false, + mojo::MakeRequest(&loader), mojom::kURLLoadOptionNone, request, client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); @@ -2378,9 +2378,9 @@ TEST_F(URLLoaderTest, SetAuth) { std::unique_ptr url_loader = std::make_unique( context(), &network_service_client, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); base::RunLoop().RunUntilIdle(); @@ -2419,9 +2419,9 @@ TEST_F(URLLoaderTest, CancelAuth) { std::unique_ptr url_loader = std::make_unique( context(), &network_service_client, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); base::RunLoop().RunUntilIdle(); @@ -2461,9 +2461,9 @@ TEST_F(URLLoaderTest, TwoChallenges) { std::unique_ptr url_loader = std::make_unique( context(), &network_service_client, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); base::RunLoop().RunUntilIdle(); @@ -2504,9 +2504,9 @@ TEST_F(URLLoaderTest, NoAuthRequiredForFavicon) { std::unique_ptr url_loader = std::make_unique( context(), &network_service_client, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); base::RunLoop().RunUntilIdle(); @@ -2546,9 +2546,9 @@ TEST_F(URLLoaderTest, HttpAuthResponseHeadersAvailable) { std::unique_ptr url_loader = std::make_unique( context(), &network_service_client, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); base::RunLoop().RunUntilIdle(); @@ -2586,9 +2586,9 @@ TEST_F(URLLoaderTest, CorbEffectiveWithCors) { url_loader = std::make_unique( context(), nullptr /* network_service_client */, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); client()->RunUntilResponseBodyArrived(); @@ -2624,9 +2624,9 @@ TEST_F(URLLoaderTest, CorbExcludedWithNoCors) { url_loader = std::make_unique( context(), nullptr /* network_service_client */, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); client()->RunUntilResponseBodyArrived(); @@ -2665,9 +2665,9 @@ TEST_F(URLLoaderTest, CorbEffectiveWithNoCorsWhenNoActualPlugin) { url_loader = std::make_unique( context(), nullptr /* network_service_client */, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); client()->RunUntilResponseBodyArrived(); @@ -2698,7 +2698,7 @@ TEST_F(URLLoaderTest, FollowRedirectTwice) { url_loader = std::make_unique( context(), nullptr /* network_service_client */, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), mojom::kURLLoadOptionNone, request, false, + mojo::MakeRequest(&loader), mojom::kURLLoadOptionNone, request, client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); @@ -2775,9 +2775,9 @@ TEST_F(URLLoaderTest, ClientAuthCancelConnection) { std::unique_ptr url_loader = std::make_unique( context(), &network_service_client, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); network_service_client.set_url_loader_ptr(&loader); @@ -2814,9 +2814,9 @@ TEST_F(URLLoaderTest, ClientAuthCancelCertificateSelection) { std::unique_ptr url_loader = std::make_unique( context(), &network_service_client, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); RunUntilIdle(); @@ -2862,9 +2862,9 @@ TEST_F(URLLoaderTest, ClientAuthNoCertificate) { std::unique_ptr url_loader = std::make_unique( context(), &network_service_client, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); RunUntilIdle(); @@ -2916,9 +2916,9 @@ TEST_F(URLLoaderTest, ClientAuthCertificateWithValidSignature) { std::unique_ptr url_loader = std::make_unique( context(), &network_service_client, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); RunUntilIdle(); @@ -2972,9 +2972,9 @@ TEST_F(URLLoaderTest, ClientAuthCertificateWithInvalidSignature) { std::unique_ptr url_loader = std::make_unique( context(), &network_service_client, DeleteLoaderCallback(&delete_run_loop, &url_loader), - mojo::MakeRequest(&loader), 0, request, false, - client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, - 0 /* request_id */, resource_scheduler_client(), nullptr, + mojo::MakeRequest(&loader), 0, request, client()->CreateInterfacePtr(), + TRAFFIC_ANNOTATION_FOR_TESTS, ¶ms, 0 /* request_id */, + resource_scheduler_client(), nullptr, nullptr /* network_usage_accumulator */, nullptr /* header_client */); RunUntilIdle(); diff --git a/services/network/websocket.cc b/services/network/websocket.cc index 55e853a4d27b..feda8658aae1 100644 --- a/services/network/websocket.cc +++ b/services/network/websocket.cc @@ -204,37 +204,44 @@ void WebSocket::WebSocketEventHandler::OnFailChannel( void WebSocket::WebSocketEventHandler::OnStartOpeningHandshake( std::unique_ptr request) { - bool should_send = impl_->delegate_->CanReadRawCookies(); + bool can_read_raw_cookies = impl_->delegate_->CanReadRawCookies(request->url); DVLOG(3) << "WebSocketEventHandler::OnStartOpeningHandshake @" - << reinterpret_cast(this) << " should_send=" << should_send; - - if (!should_send) - return; + << reinterpret_cast(this) + << " can_read_raw_cookies =" << can_read_raw_cookies; mojom::WebSocketHandshakeRequestPtr request_to_pass( mojom::WebSocketHandshakeRequest::New()); request_to_pass->url.Swap(&request->url); + std::string headers_text = base::StringPrintf( + "GET %s HTTP/1.1\r\n", request_to_pass->url.spec().c_str()); net::HttpRequestHeaders::Iterator it(request->headers); while (it.GetNext()) { + if (!can_read_raw_cookies && + base::EqualsCaseInsensitiveASCII(it.name(), + net::HttpRequestHeaders::kCookie)) { + continue; + } mojom::HttpHeaderPtr header(mojom::HttpHeader::New()); header->name = it.name(); header->value = it.value(); request_to_pass->headers.push_back(std::move(header)); + headers_text.append(base::StringPrintf("%s: %s\r\n", it.name().c_str(), + it.value().c_str())); } - request_to_pass->headers_text = - base::StringPrintf("GET %s HTTP/1.1\r\n", - request_to_pass->url.spec().c_str()) + - request->headers.ToString(); + headers_text.append("\r\n"); + request_to_pass->headers_text = std::move(headers_text); impl_->client_->OnStartOpeningHandshake(std::move(request_to_pass)); } void WebSocket::WebSocketEventHandler::OnFinishOpeningHandshake( std::unique_ptr response) { + bool can_read_raw_cookies = + impl_->delegate_->CanReadRawCookies(response->url); DVLOG(3) << "WebSocketEventHandler::OnFinishOpeningHandshake " << reinterpret_cast(this) - << " CanReadRawCookies=" << impl_->delegate_->CanReadRawCookies(); + << " CanReadRawCookies=" << can_read_raw_cookies; mojom::WebSocketHandshakeResponsePtr response_to_pass( mojom::WebSocketHandshakeResponse::New()); @@ -246,7 +253,7 @@ void WebSocket::WebSocketEventHandler::OnFinishOpeningHandshake( size_t iter = 0; std::string name, value; while (response->headers->EnumerateHeaderLines(&iter, &name, &value)) { - if (impl_->delegate_->CanReadRawCookies() || + if (can_read_raw_cookies || !net::HttpResponseHeaders::IsCookieResponseHeader(name)) { // We drop cookie-related headers such as "set-cookie" when the // renderer doesn't have access. diff --git a/services/network/websocket.h b/services/network/websocket.h index c54b9b2114d1..8b736249cff9 100644 --- a/services/network/websocket.h +++ b/services/network/websocket.h @@ -55,7 +55,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) WebSocket : public mojom::WebSocket { bool fatal) = 0; // This function may delete |impl|. virtual void ReportBadMessage(BadMessageReason reason, WebSocket* impl) = 0; - virtual bool CanReadRawCookies() = 0; + virtual bool CanReadRawCookies(const GURL& url) = 0; virtual void OnCreateURLRequest(int child_id, int frame_id, net::URLRequest* request) = 0; diff --git a/services/network/websocket_factory.cc b/services/network/websocket_factory.cc index 75e9516ff7a1..fd21e41daec0 100644 --- a/services/network/websocket_factory.cc +++ b/services/network/websocket_factory.cc @@ -59,9 +59,9 @@ class WebSocketFactory::Delegate final : public WebSocket::Delegate { OnLostConnectionToClient(impl); } - bool CanReadRawCookies() override { + bool CanReadRawCookies(const GURL& url) override { return factory_->context_->network_service()->HasRawHeadersAccess( - process_id_); + process_id_, url); } void OnCreateURLRequest(int child_id, diff --git a/third_party/blink/web_tests/FlagExpectations/enable-features=NetworkService b/third_party/blink/web_tests/FlagExpectations/enable-features=NetworkService index 3b51d5c09802..67be9926eef9 100644 --- a/third_party/blink/web_tests/FlagExpectations/enable-features=NetworkService +++ b/third_party/blink/web_tests/FlagExpectations/enable-features=NetworkService @@ -22,9 +22,6 @@ crbug.com/899303 http/tests/inspector-protocol/fetch/fetch-basic.js [ Pass ] # enabled this fails in both content_shell and chrome. Bug(none) http/tests/misc/redirect-to-about-blank.html [ Timeout ] -crbug.com/898306 http/tests/inspector-protocol/network/raw-headers-for-protected-document.js [ Failure ] -crbug.com/898306 http/tests/inspector-protocol/network/security-info-on-response.js [ Failure ] - # Reports aren't allowed under content_shell NS crbug.com/910212 virtual/network-error-logging/external/wpt/network-error-logging/sends-report-on-404.https.html [ Failure ] crbug.com/910212 virtual/network-error-logging/external/wpt/network-error-logging/sends-report-on-cache-validation.https.html [ Failure ]