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-Commit-Position: refs/heads/master@{#612685}
nwjs · Nov 30, 2018 · 1608dec · 1608dec
1 parent 9448fce
commit 1608dec
Show file tree

Hide file tree

Showing 18 changed files with 291 additions and 182 deletions.
diff --git 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<url::Origin> old_process_origins;
+  std::set<url::Origin> 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<url::Origin>(old_process_origins.begin(),
+                                                   old_process_origins.end()));
+  }
+  if (new_rph) {
+    GetNetworkService()->SetRawHeadersAccess(
+        new_rph->GetID(), std::vector<url::Origin>(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<DevToolsSession*> 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<RenderFrameDevToolsAgentHost> 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
@@ -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
@@ -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, &params, 0, /* request_id */
+        std::move(client), TRAFFIC_ANNOTATION_FOR_TESTS, &params,
+        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
@@ -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
@@ -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
@@ -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<url::Origin>& 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<url::Origin>(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
@@ -10,6 +10,7 @@
 #include <string>
 
 #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<url::Origin>& 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<NetworkContext*> network_contexts_;
 
-  std::set<uint32_t> 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<uint32_t, base::flat_set<url::Origin>>
+      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
@@ -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) {