DevTools: reject debugging web socket connections with a defined Orig…

…in header

Unless the browser is started with a new flag `--remote-allow-origins=<origin>[,<origin>, ...]`. The star origin `*` allows all origins.
This CL should not affect non-browser clients such as Puppeteer and WebDriver. It affects DevTools e2e tests in the hosted mode which is fixed in [1]. It should not affect features like remote debugging that
don't use web sockets.

[1]: https://crrev.com/c/4112007

Bug: chromium:1385982
Change-Id: I721f7db3167ebab63416c8a1f48281735f063e48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4106102
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Alex Rudenko <alexrudenko@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Danil Somsikov <dsv@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1085812}

content/browser/devtools/devtools_http_handler.cc

@@ -11,6 +11,7 @@
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
+#include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/files/file_util.h"
 #include "base/guid.h"
@@ -40,6 +41,7 @@
 #include "content/public/browser/devtools_manager_delegate.h"
 #include "content/public/browser/devtools_socket_factory.h"
 #include "content/public/common/content_client.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/common/user_agent.h"
 #include "net/base/io_buffer.h"
@@ -750,6 +752,13 @@ void DevToolsHttpHandler::OnWebSocketRequest(
   if (!thread_)
     return;
 
+  if (request.headers.count("origin") &&
+      !remote_allow_origins_.count(request.headers.at("origin")) &&
+      !remote_allow_origins_.count("*")) {
+    Send403(connection_id);
+    return;
+  }
+
   if (base::StartsWith(request.path, browser_guid_,
                        base::CompareCase::SENSITIVE)) {
     scoped_refptr<DevToolsAgentHost> browser_agent =
@@ -821,6 +830,14 @@ DevToolsHttpHandler::DevToolsHttpHandler(
                        output_directory, debug_frontend_dir, browser_guid_,
                        delegate_->HasBundledFrontendResources()));
   }
+  std::string remote_allow_origins = base::ToLowerASCII(
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kRemoteAllowOrigins));
+
+  auto origins =
+      base::SplitString(remote_allow_origins, ",", base::TRIM_WHITESPACE,
+                        base::SPLIT_WANT_NONEMPTY);
+  remote_allow_origins_.insert(origins.begin(), origins.end());
 }
 
 void DevToolsHttpHandler::ServerStarted(
@@ -880,6 +897,18 @@ void DevToolsHttpHandler::Send404(int connection_id) {
                      base::Unretained(server_wrapper_.get()), connection_id));
 }
 
+void DevToolsHttpHandler::Send403(int connection_id) {
+  if (!thread_) {
+    return;
+  }
+  net::HttpServerResponseInfo response(net::HTTP_FORBIDDEN);
+  response.SetBody(std::string(), "text/html");
+  thread_->task_runner()->PostTask(
+      FROM_HERE, base::BindOnce(&ServerWrapper::SendResponse,
+                                base::Unretained(server_wrapper_.get()),
+                                connection_id, response));
+}
+
 void DevToolsHttpHandler::Send500(int connection_id,
                                   const std::string& message) {
   if (!thread_)

content/browser/devtools/devtools_http_handler.h

@@ -7,6 +7,7 @@
 
 #include <map>
 #include <memory>
+#include <set>
 #include <string>
 
 #include "base/files/file_path.h"
@@ -91,6 +92,7 @@ class DevToolsHttpHandler {
                const std::string& data,
                const std::string& mime_type);
   void Send404(int connection_id);
+  void Send403(int connection_id);
   void Send500(int connection_id,
                const std::string& message);
   void AcceptWebSocket(int connection_id,
@@ -108,6 +110,7 @@ class DevToolsHttpHandler {
       scoped_refptr<DevToolsAgentHost> agent_host,
       const std::string& host);
 
+  std::set<std::string> remote_allow_origins_;
   // The thread used by the devtools handler to run server socket.
   std::unique_ptr<base::Thread> thread_;
   std::string browser_guid_;

content/browser/devtools/devtools_http_handler_unittest.cc

@@ -10,8 +10,10 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
+#include "base/json/json_reader.h"
 #include "base/location.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
@@ -30,6 +32,7 @@
 #include "content/public/browser/devtools_manager_delegate.h"
 #include "content/public/browser/devtools_socket_factory.h"
 #include "content/public/common/content_client.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/test/browser_task_environment.h"
 #include "content/public/test/mock_devtools_agent_host.h"
 #include "content/public/test/test_utils.h"
@@ -346,4 +349,143 @@ TEST_F(DevToolsHttpHandlerTest, TestJsonNew) {
   run_loop_2.Run();
 }
 
+class DevToolsWebSocketHandlerTest : public DevToolsHttpHandlerTest {
+ public:
+  DevToolsWebSocketHandlerTest() = default;
+
+  int StartServer() {
+    std::unique_ptr<TCPServerSocketFactory> factory =
+        std::make_unique<TCPServerSocketFactory>(run_loop_.QuitClosure(),
+                                                 run_loop_2_.QuitClosure());
+    TCPServerSocketFactory* factory_raw = factory.get();
+
+    DevToolsAgentHost::StartRemoteDebuggingServer(
+        std::move(factory), base::FilePath(), base::FilePath());
+    // Our dummy socket factory will post a quit message once the server will
+    // become ready.
+    run_loop_.Run();
+    return factory_raw->port();
+  }
+
+  void StopServer() {
+    DevToolsAgentHost::StopRemoteDebuggingServer();
+    // Make sure the handler actually stops.
+    run_loop_2_.Run();
+  }
+
+  std::string GetWebSocketDebuggingURL(int port) {
+    GURL url(base::StringPrintf("http://127.0.0.1:%d/json/version", port));
+    net::TestDelegate delegate;
+    auto request = request_context_->CreateRequest(
+        url, net::DEFAULT_PRIORITY, &delegate, TRAFFIC_ANNOTATION_FOR_TESTS);
+    request->Start();
+    delegate.RunUntilComplete();
+    EXPECT_GE(delegate.request_status(), 0);
+    absl::optional<base::Value> response =
+        base::JSONReader::Read(delegate.data_received());
+    base::Value::Dict* dict = response->GetIfDict();
+    // Compute HTTP upgrade request URL.
+    std::string debugging_url = *dict->FindString("webSocketDebuggerUrl");
+    std::string prefix = "ws://";
+    return "http://" + debugging_url.substr(prefix.length());
+  }
+
+  std::unique_ptr<net::URLRequest> RunRequestUntilCompletion(
+      std::string url,
+      std::map<std::string, std::string> headers) {
+    net::TestDelegate delegate;
+    auto request = request_context_->CreateRequest(
+        GURL(url), net::DEFAULT_PRIORITY, &delegate,
+        TRAFFIC_ANNOTATION_FOR_TESTS);
+    for (auto const& [key, value] : headers) {
+      request->SetExtraRequestHeaderByName(key, value, true);
+    }
+
+    request->Start();
+    delegate.RunUntilComplete();
+    EXPECT_GE(delegate.request_status(), 0);
+    return request;
+  }
+
+ private:
+  std::unique_ptr<net::URLRequestContext> request_context_ =
+      net::CreateTestURLRequestContextBuilder()->Build();
+  base::RunLoop run_loop_;
+  base::RunLoop run_loop_2_;
+};
+
+TEST_F(DevToolsWebSocketHandlerTest,
+       TestRejectsWebSocketConnectionsWithOrigin) {
+  int port = StartServer();
+
+  std::string debugging_url = GetWebSocketDebuggingURL(port);
+
+  // Accepts an upgrade request without an Origin header.
+  auto request =
+      RunRequestUntilCompletion(debugging_url, {
+                                                   {"connection", "upgrade"},
+                                                   {"upgrade", "websocket"},
+                                               });
+  // This error is expected because it's not a well-formed WS request.
+  // It means that the request is accepted by the server though.
+  EXPECT_EQ(request->GetResponseCode(), 500);
+
+  // Denies an upgrade request with an Origin header.
+  request = RunRequestUntilCompletion(debugging_url,
+                                      {{"connection", "upgrade"},
+                                       {"upgrade", "websocket"},
+                                       {"origin", "http://localhost"}});
+  EXPECT_EQ(request->GetResponseCode(), 403);
+
+  StopServer();
+}
+
+TEST_F(DevToolsWebSocketHandlerTest, TestAllowsCLIOverrideAllowsOriginsStar) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+      switches::kRemoteAllowOrigins, "*");
+  int port = StartServer();
+
+  std::string debugging_url = GetWebSocketDebuggingURL(port);
+
+  auto request = RunRequestUntilCompletion(debugging_url,
+                                           {
+                                               {"connection", "upgrade"},
+                                               {"upgrade", "websocket"},
+                                               {"origin", "http://localhost"},
+                                           });
+  // This error is expected because it's not a well-formed WS request.
+  // It means that the request is accepted by the server though.
+  EXPECT_EQ(request->GetResponseCode(), 500);
+
+  StopServer();
+}
+
+TEST_F(DevToolsWebSocketHandlerTest, TestAllowsCLIOverrideAllowsOrigins) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+      switches::kRemoteAllowOrigins, "http://localhost");
+  int port = StartServer();
+
+  std::string debugging_url = GetWebSocketDebuggingURL(port);
+
+  auto request = RunRequestUntilCompletion(debugging_url,
+                                           {
+                                               {"connection", "upgrade"},
+                                               {"upgrade", "websocket"},
+                                               {"origin", "http://localhost"},
+                                           });
+  // This error is expected because it's not a well-formed WS request.
+  // It means that the request is accepted by the server though.
+  EXPECT_EQ(request->GetResponseCode(), 500);
+
+  request = RunRequestUntilCompletion(debugging_url,
+                                      {
+                                          {"connection", "upgrade"},
+                                          {"upgrade", "websocket"},
+                                          {"origin", "http://127.0.0.1"},
+                                      });
+  EXPECT_EQ(request->GetResponseCode(), 403);
+
+  StopServer();
+}
+
 }  // namespace content

content/public/common/content_switches.cc

@@ -673,6 +673,10 @@ const char kRemoteDebuggingPipe[] = "remote-debugging-pipe";
 // Enables remote debug over HTTP on the specified port.
 const char kRemoteDebuggingPort[]           = "remote-debugging-port";
 
+// Enables web socket connections from the specified origins only. '*' allows
+// any origin.
+const char kRemoteAllowOrigins[] = "remote-allow-origins";
+
 const char kRendererClientId[] = "renderer-client-id";
 
 // The contents of this flag are prepended to the renderer command line.

content/public/common/content_switches.h

@@ -195,6 +195,7 @@ CONTENT_EXPORT extern const char kReduceUserAgentPlatformOsCpu[];
 CONTENT_EXPORT extern const char kRegisterPepperPlugins[];
 CONTENT_EXPORT extern const char kRemoteDebuggingPipe[];
 CONTENT_EXPORT extern const char kRemoteDebuggingPort[];
+CONTENT_EXPORT extern const char kRemoteAllowOrigins[];
 CONTENT_EXPORT extern const char kRendererClientId[];
 extern const char kRendererCmdPrefix[];
 CONTENT_EXPORT extern const char kRendererProcess[];