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 <info@bnoordhuis.nl>

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 <utility>
 #include <vector>
 
-// We need pid to use as ID with Chrome
-#if defined(_MSC_VER)
-#include <direct.h>
-#include <io.h>
-#define getpid GetCurrentProcessId
-#else
-#include <unistd.h>  // 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?"
+    "@" V8_INSPECTOR_REVISION "/inspector.html?"
-    "experiments=true&v8only=true&ws=localhost:%d/node\n",
+    "experiments=true&v8only=true&ws=%s\n",
-      port, DEVTOOLS_HASH, port);
+      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"
+            "@" V8_INSPECTOR_REVISION
-            "&ws=localhost:%d%s\","
+            "/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) + ws_url.length() * 2 +
-
+                id.length() + title.length() + url.length();
-  int buf_len = sizeof(LIST_RESPONSE_TEMPLATE) + sizeof(DEVTOOLS_HASH) +
-                sizeof(DEVTOOLS_PATH) * 2 + title.length() +
-                url.length() + NUMERIC_FIELDS_LENGTH;
   std::string buffer(buf_len, '\0');
 
   int len = snprintf(&buffer[0], buf_len, LIST_RESPONSE_TEMPLATE,
-                     DEVTOOLS_HASH, port, DEVTOOLS_PATH, getpid(),
+                     ws_url.c_str(), id.c_str(), title.c_str(), url.c_str(),
-                     title.c_str(), url.c_str(),
+                     ws_url.c_str());
-                     port, DEVTOOLS_PATH);
   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_,
+// UUID RFC: https://www.ietf.org/rfc/rfc4122.txt
-                  const std::string& script_path_, const std::string& path,
+// Used ver 4 - with numbers
-                  int port) {
+std::string GenerateID() {
-  const char* command = match_path_segment(path.c_str(), "/json");
+  uint16_t buffer[8];
-  if (command == nullptr)
+  CHECK(crypto::EntropySource(reinterpret_cast<unsigned char*>(buffer),
-    return false;
+                              sizeof(buffer)));
 
-  if (match_path_segment(command, "list") || command[0] == '\0') {
+  char uuid[256];
-    SendTargentsListResponse(socket, script_name_, script_path_, port);
+  snprintf(uuid, sizeof(uuid), "%04x%04x-%04x-%04x-%04x-%04x%04x%04x",
-  } else if (match_path_segment(command, "protocol")) {
+           buffer[0],  // time_low
-    SendProtocolJson(socket);
+           buffer[1],  // time_mid
-  } else if (match_path_segment(command, "version")) {
+           buffer[2],  // time_low
-    SendVersionResponse(socket);
+           (buffer[3] & 0x0fff) | 0x4000,  // time_hi_and_version
-  } else {
+           (buffer[4] & 0x3fff) | 0x8000,  // clk_seq_hi clk_seq_low
-    const char* pid = match_path_segment(command, "activate");
+           buffer[5],  // node
-    if (pid == nullptr || atoi(pid) != getpid())
+           buffer[6],
-      return false;
+           buffer[7]);
-    const char TARGET_ACTIVATED[] = "Target activated";
+  return uuid;
-    SendHttpResponse(socket, TARGET_ACTIVATED, sizeof(TARGET_ACTIVATED) - 1);
-  }
-  return true;
 }
 
 }  // 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<AgentImpl*>(socket->data);
   switch (state) {
   case kInspectorHandshakeHttpGet:
-    return RespondToGet(socket, agent->script_name_, agent->script_path_, path,
+    return agent->RespondToGet(socket, path);
-                        agent->port_);
   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<AgentImpl*>(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();

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({
+    checkHttpResponse(this.port, '/json/list', (response) => {
-      port: this.port,
+      this.wsHandshake(response[0]['webSocketDebuggerUrl'], tests, callback);
-      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'));
   });
 };
 

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',
+  assert.ok(
-                     response[0]['webSocketDebuggerUrl']);
+    response[0]['webSocketDebuggerUrl']
+      .match(/ws:\/\/localhost:\d+\/[0-9A-Fa-f]{8}-/));
 }
 
 function expectMainScriptSource(result) {