Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inspector: add --inspect-allow-host option #31071

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,13 @@ default) is not firewall-protected.**

See the [debugging security implications][] section for more information.

### `--inspect-allow-host=host`
<!-- YAML
added: REPLACEME
-->

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.
Expand Down Expand Up @@ -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`
Expand Down
5 changes: 5 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 7 additions & 4 deletions src/inspector_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::vector<std::string>>(
new std::vector<std::string>(debug_options_.inspector_allowed_hosts)),
debug_options_.inspect_publish_uid);
if (io_ == nullptr) {
return false;
}
Expand Down
23 changes: 14 additions & 9 deletions src/inspector_io.cc
Original file line number Diff line number Diff line change
Expand Up @@ -240,24 +240,28 @@ std::unique_ptr<InspectorIo> InspectorIo::Start(
std::shared_ptr<MainThreadHandle> main_thread,
const std::string& path,
std::shared_ptr<HostPort> host_port,
std::shared_ptr<std::vector<std::string>> allowed_http_get_hosts,
const InspectPublishUid& inspect_publish_uid) {
auto io = std::unique_ptr<InspectorIo>(
new InspectorIo(main_thread,
path,
host_port,
inspect_publish_uid));
auto io = std::unique_ptr<InspectorIo>(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<MainThreadHandle> main_thread,
const std::string& path,
std::shared_ptr<HostPort> host_port,
const InspectPublishUid& inspect_publish_uid)
InspectorIo::InspectorIo(
std::shared_ptr<MainThreadHandle> main_thread,
const std::string& path,
std::shared_ptr<HostPort> host_port,
std::shared_ptr<std::vector<std::string>> 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),
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions src/inspector_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class InspectorIo {
std::shared_ptr<MainThreadHandle> main_thread,
const std::string& path,
std::shared_ptr<HostPort> host_port,
std::shared_ptr<std::vector<std::string>> allowed_http_get_hosts,
const InspectPublishUid& inspect_publish_uid);

// Will block till the transport thread shuts down
Expand All @@ -43,6 +44,7 @@ class InspectorIo {
InspectorIo(std::shared_ptr<MainThreadHandle> handle,
const std::string& path,
std::shared_ptr<HostPort> host_port,
std::shared_ptr<std::vector<std::string>> allowed_http_get_hosts,
const InspectPublishUid& inspect_publish_uid);

// Wrapper for agent->ThreadMain()
Expand All @@ -58,6 +60,7 @@ class InspectorIo {
// running
std::shared_ptr<RequestQueue> request_queue_;
std::shared_ptr<HostPort> host_port_;
std::shared_ptr<std::vector<std::string>> allowed_http_get_hosts_;
InspectPublishUid inspect_publish_uid_;

// The IO thread runs its own uv_loop to implement the TCP server off
Expand Down
7 changes: 4 additions & 3 deletions src/inspector_socket.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down
1 change: 1 addition & 0 deletions src/inspector_socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
26 changes: 23 additions & 3 deletions src/inspector_socket_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -251,13 +252,18 @@ void PrintDebuggerReadyMessage(
}

InspectorSocketServer::InspectorSocketServer(
std::unique_ptr<SocketServerDelegate> delegate, uv_loop_t* loop,
const std::string& host, int port,
const InspectPublishUid& inspect_publish_uid, FILE* out)
std::unique_ptr<SocketServerDelegate> delegate,
uv_loop_t* loop,
const std::string& host,
int port,
std::shared_ptr<std::vector<std::string>> 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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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))
Expand Down
16 changes: 10 additions & 6 deletions src/inspector_socket_server.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,14 @@ class SocketServerDelegate {

class InspectorSocketServer {
public:
InspectorSocketServer(std::unique_ptr<SocketServerDelegate> delegate,
uv_loop_t* loop,
const std::string& host,
int port,
const InspectPublishUid& inspect_publish_uid,
FILE* out = stderr);
InspectorSocketServer(
std::unique_ptr<SocketServerDelegate> delegate,
uv_loop_t* loop,
const std::string& host,
int port,
std::shared_ptr<std::vector<std::string>> allowed_http_get_hosts,
const InspectPublishUid& inspect_publish_uid,
FILE* out = stderr);
~InspectorSocketServer();

// Start listening on host/port
Expand All @@ -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,
Expand Down Expand Up @@ -93,6 +96,7 @@ class InspectorSocketServer {
std::unique_ptr<SocketServerDelegate> delegate_;
const std::string host_;
int port_;
std::shared_ptr<std::vector<std::string>> allowed_http_get_hosts_;
InspectPublishUid inspect_publish_uid_;
std::vector<ServerSocketPtr> server_sockets_;
std::map<int, std::pair<std::string, std::unique_ptr<SocketSession>>>
Expand Down
6 changes: 6 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 2 additions & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string> inspector_allowed_hosts;

InspectPublishUid inspect_publish_uid;

Expand Down
2 changes: 2 additions & 0 deletions test/cctest/test_inspector_socket.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
94 changes: 85 additions & 9 deletions test/cctest/test_inspector_socket_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::vector<std::string>>& allowed_http_get_hosts,
FILE* out);

InspectorSocketServer* operator->() {
return server_.get();
Expand Down Expand Up @@ -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<std::vector<std::string>>& allowed_http_get_hosts,
FILE* out) {
std::vector<std::string> targets;
if (has_targets)
targets = { MAIN_TARGET_ID };
Expand All @@ -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<InspectorSocketServer>(
std::move(delegate), loop, host, port, inspect_publish_uid, out);
server_ = std::make_unique<InspectorSocketServer>(std::move(delegate),
loop,
host,
port,
allowed_http_get_hosts,
inspect_publish_uid,
out);
}

static void TestHttpRequest(int port, const std::string& path,
Expand All @@ -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"
Expand Down Expand Up @@ -459,6 +486,55 @@ TEST_F(InspectorSocketServerTest, InspectorSessions) {
stays_till_termination_socket.ExpectEOF();
}

TEST_F(InspectorSocketServerTest, ServerHostAllowlist) {
std::shared_ptr<std::vector<std::string>> allowed_http_get_hosts(
new std::vector<std::string>{"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());
Expand Down