From 647afe9d9a96b21eadc01759edc17e4e2872971b Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Fri, 30 Sep 2016 17:40:44 -0700 Subject: [PATCH] inspector: generate UUID for debug targets PR-URL: https://github.com/nodejs/node-private/pull/79 Fixes: https://github.com/nodejs/security/issues/81 Reviewed-By: Ben Noordhuis --- src/inspector_agent.cc | 129 ++++++++++++++++------------- test/inspector/inspector-helper.js | 65 ++++++++------- test/inspector/test-inspector.js | 5 +- 3 files changed, 109 insertions(+), 90 deletions(-) diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index 740565444e61cf..aec0bc06e32334 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -4,6 +4,7 @@ #include "env.h" #include "env-inl.h" #include "node.h" +#include "node_crypto.h" #include "node_mutex.h" #include "node_version.h" #include "v8-platform.h" @@ -23,14 +24,6 @@ #include #include -// We need pid to use as ID with Chrome -#if defined(_MSC_VER) -#include -#include -#define getpid GetCurrentProcessId -#else -#include // setuid, getuid -#endif namespace node { namespace inspector { @@ -39,29 +32,27 @@ namespace { const char TAG_CONNECT[] = "#connect"; const char TAG_DISCONNECT[] = "#disconnect"; -const char DEVTOOLS_PATH[] = "/node"; -const char DEVTOOLS_HASH[] = V8_INSPECTOR_REVISION; - static const uint8_t PROTOCOL_JSON[] = { #include "v8_inspector_protocol_json.h" // NOLINT(build/include_order) }; -void PrintDebuggerReadyMessage(int port) { +std::string GetWsUrl(int port, const std::string& id) { + char buf[1024]; + snprintf(buf, sizeof(buf), "localhost:%d/%s", port, id.c_str()); + return buf; +} + +void PrintDebuggerReadyMessage(int port, const std::string& id) { fprintf(stderr, "Debugger listening on port %d.\n" "Warning: This is an experimental feature and could change at any time.\n" "To start debugging, open the following URL in Chrome:\n" " chrome-devtools://devtools/remote/serve_file/" - "@%s/inspector.html?" - "experiments=true&v8only=true&ws=localhost:%d/node\n", - port, DEVTOOLS_HASH, port); + "@" V8_INSPECTOR_REVISION "/inspector.html?" + "experiments=true&v8only=true&ws=%s\n", + port, GetWsUrl(port, id).c_str()); fflush(stderr); } -bool AcceptsConnection(InspectorSocket* socket, const std::string& path) { - return StringEqualNoCaseN(path.c_str(), DEVTOOLS_PATH, - sizeof(DEVTOOLS_PATH) - 1); -} - void Escape(std::string* string) { for (char& c : *string) { c = (c == '\"' || c == '\\') ? '_' : c; @@ -126,20 +117,22 @@ std::string GetProcessTitle() { void SendTargentsListResponse(InspectorSocket* socket, const std::string& script_name_, const std::string& script_path_, - int port) { + const std::string& id, + const std::string& ws_url) { const char LIST_RESPONSE_TEMPLATE[] = "[ {" " \"description\": \"node.js instance\"," " \"devtoolsFrontendUrl\": " "\"https://chrome-devtools-frontend.appspot.com/serve_file/" - "@%s/inspector.html?experiments=true&v8only=true" - "&ws=localhost:%d%s\"," + "@" V8_INSPECTOR_REVISION + "/inspector.html?experiments=true&v8only=true" + "&ws=%s\"," " \"faviconUrl\": \"https://nodejs.org/static/favicon.ico\"," - " \"id\": \"%d\"," + " \"id\": \"%s\"," " \"title\": \"%s\"," " \"type\": \"node\"," " \"url\": \"%s\"," - " \"webSocketDebuggerUrl\": \"ws://localhost:%d%s\"" + " \"webSocketDebuggerUrl\": \"ws://%s\"" "} ]"; std::string title = script_name_.empty() ? GetProcessTitle() : script_name_; @@ -150,17 +143,13 @@ void SendTargentsListResponse(InspectorSocket* socket, Escape(&title); Escape(&url); - const int NUMERIC_FIELDS_LENGTH = 5 * 2 + 20; // 2 x port + 1 x pid (64 bit) - - int buf_len = sizeof(LIST_RESPONSE_TEMPLATE) + sizeof(DEVTOOLS_HASH) + - sizeof(DEVTOOLS_PATH) * 2 + title.length() + - url.length() + NUMERIC_FIELDS_LENGTH; + int buf_len = sizeof(LIST_RESPONSE_TEMPLATE) + ws_url.length() * 2 + + id.length() + title.length() + url.length(); std::string buffer(buf_len, '\0'); int len = snprintf(&buffer[0], buf_len, LIST_RESPONSE_TEMPLATE, - DEVTOOLS_HASH, port, DEVTOOLS_PATH, getpid(), - title.c_str(), url.c_str(), - port, DEVTOOLS_PATH); + ws_url.c_str(), id.c_str(), title.c_str(), url.c_str(), + ws_url.c_str()); buffer.resize(len); ASSERT_LT(len, buf_len); // Buffer should be big enough! SendHttpResponse(socket, buffer.data(), len); @@ -196,27 +185,24 @@ const char* match_path_segment(const char* path, const char* expected) { return nullptr; } -bool RespondToGet(InspectorSocket* socket, const std::string& script_name_, - const std::string& script_path_, const std::string& path, - int port) { - const char* command = match_path_segment(path.c_str(), "/json"); - if (command == nullptr) - return false; +// UUID RFC: https://www.ietf.org/rfc/rfc4122.txt +// Used ver 4 - with numbers +std::string GenerateID() { + uint16_t buffer[8]; + CHECK(crypto::EntropySource(reinterpret_cast(buffer), + sizeof(buffer))); - if (match_path_segment(command, "list") || command[0] == '\0') { - SendTargentsListResponse(socket, script_name_, script_path_, port); - } else if (match_path_segment(command, "protocol")) { - SendProtocolJson(socket); - } else if (match_path_segment(command, "version")) { - SendVersionResponse(socket); - } else { - const char* pid = match_path_segment(command, "activate"); - if (pid == nullptr || atoi(pid) != getpid()) - return false; - const char TARGET_ACTIVATED[] = "Target activated"; - SendHttpResponse(socket, TARGET_ACTIVATED, sizeof(TARGET_ACTIVATED) - 1); - } - return true; + char uuid[256]; + snprintf(uuid, sizeof(uuid), "%04x%04x-%04x-%04x-%04x-%04x%04x%04x", + buffer[0], // time_low + buffer[1], // time_mid + buffer[2], // time_low + (buffer[3] & 0x0fff) | 0x4000, // time_hi_and_version + (buffer[4] & 0x3fff) | 0x8000, // clk_seq_hi clk_seq_low + buffer[5], // node + buffer[6], + buffer[7]); + return uuid; } } // namespace @@ -267,6 +253,7 @@ class AgentImpl { void PostIncomingMessage(const String16& message); void WaitForFrontendMessage(); void NotifyMessageReceived(); + bool RespondToGet(InspectorSocket* socket, const std::string& path); State ToState(State state); uv_sem_t start_sem_; @@ -294,6 +281,7 @@ class AgentImpl { std::string script_name_; std::string script_path_; + const std::string id_; friend class ChannelImpl; friend class DispatchOnInspectorBackendTask; @@ -431,7 +419,8 @@ AgentImpl::AgentImpl(Environment* env) : port_(0), platform_(nullptr), dispatching_messages_(false), frontend_session_id_(0), - backend_session_id_(0) { + backend_session_id_(0), + id_(GenerateID()) { CHECK_EQ(0, uv_sem_init(&start_sem_, 0)); memset(&io_thread_req_, 0, sizeof(io_thread_req_)); CHECK_EQ(0, uv_async_init(env->event_loop(), data_written_, nullptr)); @@ -639,10 +628,10 @@ bool AgentImpl::OnInspectorHandshakeIO(InspectorSocket* socket, AgentImpl* agent = static_cast(socket->data); switch (state) { case kInspectorHandshakeHttpGet: - return RespondToGet(socket, agent->script_name_, agent->script_path_, path, - agent->port_); + return agent->RespondToGet(socket, path); case kInspectorHandshakeUpgrading: - return AcceptsConnection(socket, path); + return path.length() == agent->id_.length() + 1 && + path.find(agent->id_) == 1; case kInspectorHandshakeUpgraded: agent->OnInspectorConnectionIO(socket); return true; @@ -684,6 +673,28 @@ void AgentImpl::OnRemoteDataIO(InspectorSocket* socket, } } +bool AgentImpl::RespondToGet(InspectorSocket* socket, const std::string& path) { + const char* command = match_path_segment(path.c_str(), "/json"); + if (command == nullptr) + return false; + + if (match_path_segment(command, "list") || command[0] == '\0') { + SendTargentsListResponse(socket, script_name_, script_path_, id_, + GetWsUrl(port_, id_)); + } else if (match_path_segment(command, "protocol")) { + SendProtocolJson(socket); + } else if (match_path_segment(command, "version")) { + SendVersionResponse(socket); + } else { + const char* pid = match_path_segment(command, "activate"); + if (pid != id_) + return false; + const char TARGET_ACTIVATED[] = "Target activated"; + SendHttpResponse(socket, TARGET_ACTIVATED, sizeof(TARGET_ACTIVATED) - 1); + } + return true; +} + // static void AgentImpl::WriteCbIO(uv_async_t* async) { AgentImpl* agent = static_cast(async->data); @@ -732,7 +743,7 @@ void AgentImpl::WorkerRunIO() { uv_sem_post(&start_sem_); return; } - PrintDebuggerReadyMessage(port_); + PrintDebuggerReadyMessage(port_, id_); if (!wait_) { uv_sem_post(&start_sem_); } @@ -816,7 +827,7 @@ void AgentImpl::DispatchMessages() { if (shutting_down_) { state_ = State::kDone; } else { - PrintDebuggerReadyMessage(port_); + PrintDebuggerReadyMessage(port_, id_); state_ = State::kAccepting; } inspector_->quitMessageLoopOnPause(); diff --git a/test/inspector/inspector-helper.js b/test/inspector/inspector-helper.js index a4e1094340993e..850df8718fbe7c 100644 --- a/test/inspector/inspector-helper.js +++ b/test/inspector/inspector-helper.js @@ -5,6 +5,7 @@ const fs = require('fs'); const http = require('http'); const path = require('path'); const spawn = require('child_process').spawn; +const url = require('url'); const DEBUG = false; @@ -349,37 +350,43 @@ Harness.prototype.testHttpResponse = function(path, check) { }); }; +Harness.prototype.wsHandshake = function(devtoolsUrl, tests, readyCallback) { + http.get({ + port: this.port, + path: url.parse(devtoolsUrl).path, + headers: { + 'Connection': 'Upgrade', + 'Upgrade': 'websocket', + 'Sec-WebSocket-Version': 13, + 'Sec-WebSocket-Key': 'key==' + } + }).on('upgrade', (message, socket) => { + const session = new TestSession(socket, this); + if (!(tests instanceof Array)) + tests = [tests]; + function enqueue(tests) { + session.enqueue((sessionCb) => { + if (tests.length) { + tests[0](session); + session.enqueue((cb2) => { + enqueue(tests.slice(1)); + cb2(); + }); + } else { + readyCallback(); + } + sessionCb(); + }); + } + enqueue(tests); + }).on('response', () => common.fail('Upgrade was not received')); +}; + Harness.prototype.runFrontendSession = function(tests) { return this.enqueue_((callback) => { - http.get({ - port: this.port, - path: '/node', - headers: { - 'Connection': 'Upgrade', - 'Upgrade': 'websocket', - 'Sec-WebSocket-Version': 13, - 'Sec-WebSocket-Key': 'key==' - } - }).on('upgrade', (message, socket) => { - const session = new TestSession(socket, this); - if (!(tests instanceof Array)) - tests = [tests]; - function enqueue(tests) { - session.enqueue((sessionCb) => { - if (tests.length) { - tests[0](session); - session.enqueue((cb2) => { - enqueue(tests.slice(1)); - cb2(); - }); - } else { - callback(); - } - sessionCb(); - }); - } - enqueue(tests); - }).on('response', () => common.fail('Upgrade was not received')); + checkHttpResponse(this.port, '/json/list', (response) => { + this.wsHandshake(response[0]['webSocketDebuggerUrl'], tests, callback); + }); }); }; diff --git a/test/inspector/test-inspector.js b/test/inspector/test-inspector.js index 91d930851bd381..0d01127d6b35d8 100644 --- a/test/inspector/test-inspector.js +++ b/test/inspector/test-inspector.js @@ -8,8 +8,9 @@ let scopeId; function checkListResponse(response) { assert.strictEqual(1, response.length); assert.ok(response[0]['devtoolsFrontendUrl']); - assert.strictEqual('ws://localhost:' + this.port + '/node', - response[0]['webSocketDebuggerUrl']); + assert.ok( + response[0]['webSocketDebuggerUrl'] + .match(/ws:\/\/localhost:\d+\/[0-9A-Fa-f]{8}-/)); } function expectMainScriptSource(result) {