diff --git a/doc/api/cli.md b/doc/api/cli.md index bb053193f5b785..0f2e17ffd3dd18 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -391,6 +391,13 @@ default) is not firewall-protected.** See the [debugging security implications][] section for more information. +### `--inspect-allow-host=host` + + +Have the HTTP endpoint accept HTTP GET requests addressed to this host name. + ### `--inspect-publish-uid=stderr,http` Specify ways of the inspector web socket url exposure. @@ -1084,6 +1091,7 @@ Node.js options that are allowed are: * `--icu-data-dir` * `--input-type` * `--insecure-http-parser` +* `--inspect-allow-host` * `--inspect-brk` * `--inspect-port`, `--debug-port` * `--inspect-publish-uid` diff --git a/doc/node.1 b/doc/node.1 index 9a75cd073eecdd..e423b93db24f4f 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -199,6 +199,11 @@ Set the .Ar host:port to be used when the inspector is activated. . +.It Fl -inspect-allow-host Ns = Ns Ar host +Have the HTTP endpoint accept HTTP GET requests addressed to this +.Ar host +name. +. .It Fl -inspect-publish-uid=stderr,http Specify how the inspector WebSocket URL is exposed. Valid values are diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index fe60c82cb5590d..77210493fbf3e9 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -823,10 +823,13 @@ bool Agent::StartIoThread() { CHECK_NOT_NULL(client_); - io_ = InspectorIo::Start(client_->getThreadHandle(), - path_, - host_port_, - debug_options_.inspect_publish_uid); + io_ = InspectorIo::Start( + client_->getThreadHandle(), + path_, + host_port_, + std::shared_ptr>( + new std::vector(debug_options_.inspector_allowed_hosts)), + debug_options_.inspect_publish_uid); if (io_ == nullptr) { return false; } diff --git a/src/inspector_io.cc b/src/inspector_io.cc index ed9035136c51db..426c4ad7b8b41a 100644 --- a/src/inspector_io.cc +++ b/src/inspector_io.cc @@ -240,24 +240,28 @@ std::unique_ptr InspectorIo::Start( std::shared_ptr main_thread, const std::string& path, std::shared_ptr host_port, + std::shared_ptr> allowed_http_get_hosts, const InspectPublishUid& inspect_publish_uid) { - auto io = std::unique_ptr( - new InspectorIo(main_thread, - path, - host_port, - inspect_publish_uid)); + auto io = std::unique_ptr(new InspectorIo(main_thread, + path, + host_port, + allowed_http_get_hosts, + inspect_publish_uid)); if (io->request_queue_->Expired()) { // Thread is not running return nullptr; } return io; } -InspectorIo::InspectorIo(std::shared_ptr main_thread, - const std::string& path, - std::shared_ptr host_port, - const InspectPublishUid& inspect_publish_uid) +InspectorIo::InspectorIo( + std::shared_ptr main_thread, + const std::string& path, + std::shared_ptr host_port, + std::shared_ptr> allowed_http_get_hosts, + const InspectPublishUid& inspect_publish_uid) : main_thread_(main_thread), host_port_(host_port), + allowed_http_get_hosts_(allowed_http_get_hosts), inspect_publish_uid_(inspect_publish_uid), thread_(), script_name_(path), @@ -297,6 +301,7 @@ void InspectorIo::ThreadMain() { &loop, host_port_->host(), host_port_->port(), + allowed_http_get_hosts_, inspect_publish_uid_); request_queue_ = queue->handle(); // Its lifetime is now that of the server delegate diff --git a/src/inspector_io.h b/src/inspector_io.h index e9f94056733994..3ef92b4968eb28 100644 --- a/src/inspector_io.h +++ b/src/inspector_io.h @@ -31,6 +31,7 @@ class InspectorIo { std::shared_ptr main_thread, const std::string& path, std::shared_ptr host_port, + std::shared_ptr> allowed_http_get_hosts, const InspectPublishUid& inspect_publish_uid); // Will block till the transport thread shuts down @@ -43,6 +44,7 @@ class InspectorIo { InspectorIo(std::shared_ptr handle, const std::string& path, std::shared_ptr host_port, + std::shared_ptr> allowed_http_get_hosts, const InspectPublishUid& inspect_publish_uid); // Wrapper for agent->ThreadMain() @@ -58,6 +60,7 @@ class InspectorIo { // running std::shared_ptr request_queue_; std::shared_ptr host_port_; + std::shared_ptr> allowed_http_get_hosts_; InspectPublishUid inspect_publish_uid_; // The IO thread runs its own uv_loop to implement the TCP server off diff --git a/src/inspector_socket.cc b/src/inspector_socket.cc index ec347732404189..aced6e1e63d87a 100644 --- a/src/inspector_socket.cc +++ b/src/inspector_socket.cc @@ -579,9 +579,10 @@ class HttpHandler : public ProtocolHandler { bool IsAllowedHost(const std::string& host_with_port) const { std::string host = TrimPort(host_with_port); - return host.empty() || IsIPAddress(host) - || node::StringEqualNoCase(host.data(), "localhost") - || node::StringEqualNoCase(host.data(), "localhost6"); + return host.empty() || IsIPAddress(host) || + node::StringEqualNoCase(host.data(), "localhost") || + node::StringEqualNoCase(host.data(), "localhost6") || + tcp_->delegate()->IsAllowedHttpGetHost(host); } bool parsing_value_; diff --git a/src/inspector_socket.h b/src/inspector_socket.h index a6a18923d1b5a4..b6caf252a771e5 100644 --- a/src/inspector_socket.h +++ b/src/inspector_socket.h @@ -19,6 +19,7 @@ class InspectorSocket { public: class Delegate { public: + virtual bool IsAllowedHttpGetHost(const std::string& host) = 0; virtual void OnHttpGet(const std::string& host, const std::string& path) = 0; virtual void OnSocketUpgrade(const std::string& host, diff --git a/src/inspector_socket_server.cc b/src/inspector_socket_server.cc index 9e27bd30f76871..9658b03d586837 100644 --- a/src/inspector_socket_server.cc +++ b/src/inspector_socket_server.cc @@ -182,6 +182,7 @@ class SocketSession { ~Delegate() override { server_->SessionTerminated(session_id_); } + bool IsAllowedHttpGetHost(const std::string& host) override; void OnHttpGet(const std::string& host, const std::string& path) override; void OnSocketUpgrade(const std::string& host, const std::string& path, const std::string& ws_key) override; @@ -251,13 +252,18 @@ void PrintDebuggerReadyMessage( } InspectorSocketServer::InspectorSocketServer( - std::unique_ptr delegate, uv_loop_t* loop, - const std::string& host, int port, - const InspectPublishUid& inspect_publish_uid, FILE* out) + std::unique_ptr delegate, + uv_loop_t* loop, + const std::string& host, + int port, + std::shared_ptr> allowed_http_get_hosts, + const InspectPublishUid& inspect_publish_uid, + FILE* out) : loop_(loop), delegate_(std::move(delegate)), host_(host), port_(port), + allowed_http_get_hosts_(allowed_http_get_hosts), inspect_publish_uid_(inspect_publish_uid), next_session_id_(0), out_(out) { @@ -309,6 +315,16 @@ void InspectorSocketServer::SessionTerminated(int session_id) { } } +bool InspectorSocketServer::IsAllowedHttpGetHost(const std::string& host) { + if (allowed_http_get_hosts_ == nullptr) { + return false; + } + for (auto& it : *allowed_http_get_hosts_) { + if (node::StringEqualNoCase(host.data(), it.data())) return true; + } + return false; +} + bool InspectorSocketServer::HandleGetRequest(int session_id, const std::string& host, const std::string& path) { @@ -495,6 +511,10 @@ void SocketSession::Send(const std::string& message) { ws_socket_->Write(message.data(), message.length()); } +bool SocketSession::Delegate::IsAllowedHttpGetHost(const std::string& host) { + return server_->IsAllowedHttpGetHost(host); +} + void SocketSession::Delegate::OnHttpGet(const std::string& host, const std::string& path) { if (!server_->HandleGetRequest(session_id_, host, path)) diff --git a/src/inspector_socket_server.h b/src/inspector_socket_server.h index 98d4e7d0150169..b4a6eb0f61056b 100644 --- a/src/inspector_socket_server.h +++ b/src/inspector_socket_server.h @@ -43,12 +43,14 @@ class SocketServerDelegate { class InspectorSocketServer { public: - InspectorSocketServer(std::unique_ptr delegate, - uv_loop_t* loop, - const std::string& host, - int port, - const InspectPublishUid& inspect_publish_uid, - FILE* out = stderr); + InspectorSocketServer( + std::unique_ptr delegate, + uv_loop_t* loop, + const std::string& host, + int port, + std::shared_ptr> allowed_http_get_hosts, + const InspectPublishUid& inspect_publish_uid, + FILE* out = stderr); ~InspectorSocketServer(); // Start listening on host/port @@ -65,6 +67,7 @@ class InspectorSocketServer { // Session connection lifecycle void Accept(int server_port, uv_stream_t* server_socket); + bool IsAllowedHttpGetHost(const std::string& host); bool HandleGetRequest(int session_id, const std::string& host, const std::string& path); void SessionStarted(int session_id, const std::string& target_id, @@ -93,6 +96,7 @@ class InspectorSocketServer { std::unique_ptr delegate_; const std::string host_; int port_; + std::shared_ptr> allowed_http_get_hosts_; InspectPublishUid inspect_publish_uid_; std::vector server_sockets_; std::map>> diff --git a/src/node_options.cc b/src/node_options.cc index 2f0d1a9ad7ed04..51dc00bb2bb8e0 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -309,6 +309,12 @@ DebugOptionsParser::DebugOptionsParser() { "(default: stderr,http)", &DebugOptions::inspect_publish_uid_string, kAllowedInEnvironment); + + AddOption("--inspect-allow-host", + "allow host on upgrading http requests on inspector (option can be " + "repeated)", + &DebugOptions::inspector_allowed_hosts, + kAllowedInEnvironment); } EnvironmentOptionsParser::EnvironmentOptionsParser() { diff --git a/src/node_options.h b/src/node_options.h index 788a4815e50906..814588ca16bee3 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -77,6 +77,8 @@ class DebugOptions : public Options { bool break_node_first_line = false; // --inspect-publish-uid std::string inspect_publish_uid_string = "stderr,http"; + // --inspect-allow-host + std::vector inspector_allowed_hosts; InspectPublishUid inspect_publish_uid; diff --git a/test/cctest/test_inspector_socket.cc b/test/cctest/test_inspector_socket.cc index dc8cd962141e81..2a54acaa140826 100644 --- a/test/cctest/test_inspector_socket.cc +++ b/test/cctest/test_inspector_socket.cc @@ -106,6 +106,8 @@ class TestInspectorDelegate : public InspectorSocket::Delegate { delegate = nullptr; } + bool IsAllowedHttpGetHost(const std::string& host) override { return false; } + void OnHttpGet(const std::string& host, const std::string& path) override { process(kInspectorHandshakeHttpGet, path); } diff --git a/test/cctest/test_inspector_socket_server.cc b/test/cctest/test_inspector_socket_server.cc index 4e4ebbc32854ac..36ca0387433594 100644 --- a/test/cctest/test_inspector_socket_server.cc +++ b/test/cctest/test_inspector_socket_server.cc @@ -245,8 +245,20 @@ class ServerHolder { ServerHolder(bool has_targets, uv_loop_t* loop, int port) : ServerHolder(has_targets, loop, HOST, port, nullptr) { } - ServerHolder(bool has_targets, uv_loop_t* loop, - const std::string& host, int port, FILE* out); + ServerHolder(bool has_targets, + uv_loop_t* loop, + const std::string& host, + int port, + FILE* out) + : ServerHolder(has_targets, loop, host, port, nullptr, out) { } + + ServerHolder( + bool has_targets, + uv_loop_t* loop, + const std::string& host, + int port, + const std::shared_ptr>& allowed_http_get_hosts, + FILE* out); InspectorSocketServer* operator->() { return server_.get(); @@ -352,8 +364,13 @@ class TestSocketServerDelegate : public SocketServerDelegate { int session_id_; }; -ServerHolder::ServerHolder(bool has_targets, uv_loop_t* loop, - const std::string& host, int port, FILE* out) { +ServerHolder::ServerHolder( + bool has_targets, + uv_loop_t* loop, + const std::string& host, + int port, + const std::shared_ptr>& allowed_http_get_hosts, + FILE* out) { std::vector targets; if (has_targets) targets = { MAIN_TARGET_ID }; @@ -362,8 +379,13 @@ ServerHolder::ServerHolder(bool has_targets, uv_loop_t* loop, node::InspectPublishUid inspect_publish_uid; inspect_publish_uid.console = true; inspect_publish_uid.http = true; - server_ = std::make_unique( - std::move(delegate), loop, host, port, inspect_publish_uid, out); + server_ = std::make_unique(std::move(delegate), + loop, + host, + port, + allowed_http_get_hosts, + inspect_publish_uid, + out); } static void TestHttpRequest(int port, const std::string& path, @@ -374,9 +396,14 @@ static void TestHttpRequest(int port, const std::string& path, socket.Close(); } -static const std::string WsHandshakeRequest(const std::string& target_id) { - return "GET /" + target_id + " HTTP/1.1\r\n" - "Host: localhost:9229\r\n" +static const std::string WsHandshakeRequest( + const std::string& target_id, + const std::string& hostname = "localhost:9229") { + return "GET /" + target_id + + " HTTP/1.1\r\n" + "Host: " + + hostname + + "\r\n" "Upgrade: websocket\r\n" "Connection: Upgrade\r\n" "Sec-WebSocket-Key: aaa==\r\n" @@ -459,6 +486,55 @@ TEST_F(InspectorSocketServerTest, InspectorSessions) { stays_till_termination_socket.ExpectEOF(); } +TEST_F(InspectorSocketServerTest, ServerHostAllowlist) { + std::shared_ptr> allowed_http_get_hosts( + new std::vector{"foobar.local"}); + ServerHolder server(true, &loop, HOST, 0, allowed_http_get_hosts, nullptr); + ASSERT_TRUE(server->Start()); + + SocketWrapper well_behaved_socket(&loop); + // Regular connection + well_behaved_socket.Connect(HOST, server.port()); + well_behaved_socket.Write(WsHandshakeRequest(MAIN_TARGET_ID, "foobar.local")); + well_behaved_socket.Expect(WS_HANDSHAKE_RESPONSE); + + EXPECT_EQ(1, server.connected); + + well_behaved_socket.Write("\x81\x84\x7F\xC2\x66\x31\x4E\xF0\x55\x05"); + + server.Expect("1234"); + server.Write("5678"); + + well_behaved_socket.Expect("\x81\x4" + "5678"); + well_behaved_socket.Write(CLIENT_CLOSE_FRAME); + well_behaved_socket.Expect(SERVER_CLOSE_FRAME); + + EXPECT_EQ(1, server.disconnected); + + well_behaved_socket.Close(); + + // Declined connection + SocketWrapper socket_host_with_port(&loop); + socket_host_with_port.Connect(HOST, server.port()); + socket_host_with_port.Write( + WsHandshakeRequest(MAIN_TARGET_ID, "bar.local:1234")); + socket_host_with_port.Expect("HTTP/1.0 400 Bad Request"); + socket_host_with_port.ExpectEOF(); + + SocketWrapper socket_host_without_port(&loop); + socket_host_without_port.Connect(HOST, server.port()); + socket_host_without_port.Write( + WsHandshakeRequest(MAIN_TARGET_ID, "bar.local")); + socket_host_without_port.Expect("HTTP/1.0 400 Bad Request"); + socket_host_without_port.ExpectEOF(); + + server->Stop(); + server->TerminateConnections(); + SPIN_WHILE(!server.done()); + SPIN_WHILE(uv_loop_alive(&loop)); +} + TEST_F(InspectorSocketServerTest, ServerDoesNothing) { ServerHolder server(true, &loop, 0); ASSERT_TRUE(server->Start());