From 482459edd4f19e8f437b8711c4af3fcc5682a0e5 Mon Sep 17 00:00:00 2001 From: Chris Opperwall Date: Mon, 19 Oct 2020 18:24:08 -0700 Subject: [PATCH] debugger: validate sec-websocket-accept response header This addresses a TODO to validate that the sec-websocket-accept header in the WebSocket handshake response is valid. To do this we need to append the WebSocket GUID to the original key sent in sec-websocket-key, sha1 hash it, and then compare the base64 encoding with the value sent in the sec-websocket-accept response header. If they don't match, an error is thrown. PR-URL: https://github.com/nodejs/node/pull/39357 Refs: https://github.com/nodejs/node-inspect/pull/93 Reviewed-By: Colin Ihrig --- lib/internal/debugger/inspect_client.js | 26 +++++++++++++++++++++---- src/node_native_module.cc | 1 + 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/lib/internal/debugger/inspect_client.js b/lib/internal/debugger/inspect_client.js index 8fc9011ca888ed..8863f06d8f794f 100644 --- a/lib/internal/debugger/inspect_client.js +++ b/lib/internal/debugger/inspect_client.js @@ -11,6 +11,7 @@ const { } = primordials; const Buffer = require('buffer').Buffer; +const crypto = require('crypto'); const { ERR_DEBUGGER_ERROR } = require('internal/errors').codes; const { EventEmitter } = require('events'); const http = require('http'); @@ -35,6 +36,10 @@ const kTwoBytePayloadLengthField = 126; const kEightBytePayloadLengthField = 127; const kMaskingKeyWidthInBytes = 4; +// This guid is defined in the Websocket Protocol RFC +// https://tools.ietf.org/html/rfc6455#section-1.3 +const WEBSOCKET_HANDSHAKE_GUID = '258EAFA5-E914-47DA-95CA-C5AB0DC85B11'; + function unpackError({ code, message, data }) { const err = new ERR_DEBUGGER_ERROR(`${message} - ${data}`); err.code = code; @@ -42,6 +47,19 @@ function unpackError({ code, message, data }) { return err; } +function validateHandshake(requestKey, responseKey) { + const expectedResponseKeyBase = requestKey + WEBSOCKET_HANDSHAKE_GUID; + const shasum = crypto.createHash('sha1'); + shasum.update(expectedResponseKeyBase); + const shabuf = shasum.digest(); + + if (shabuf.toString('base64') !== responseKey) { + throw new ERR_DEBUGGER_ERROR( + `WebSocket secret mismatch: ${requestKey} did not match ${responseKey}` + ); + } +} + function encodeFrameHybi17(payload) { var i; @@ -292,8 +310,8 @@ class Client extends EventEmitter { _connectWebsocket(urlPath) { this.reset(); - const key1 = require('crypto').randomBytes(16).toString('base64'); - debuglog('request websocket', key1); + const requestKey = crypto.randomBytes(16).toString('base64'); + debuglog('request WebSocket', requestKey); const httpReq = this._http = http.request({ host: this._host, @@ -302,7 +320,7 @@ class Client extends EventEmitter { headers: { 'Connection': 'Upgrade', 'Upgrade': 'websocket', - 'Sec-WebSocket-Key': key1, + 'Sec-WebSocket-Key': requestKey, 'Sec-WebSocket-Version': '13', }, }); @@ -319,7 +337,7 @@ class Client extends EventEmitter { }); const handshakeListener = (res, socket) => { - // TODO: we *could* validate res.headers[sec-websocket-accept] + validateHandshake(requestKey, res.headers['sec-websocket-accept']); debuglog('websocket upgrade'); this._socket = socket; diff --git a/src/node_native_module.cc b/src/node_native_module.cc index 9776d12cc1b488..f13e53221eb018 100644 --- a/src/node_native_module.cc +++ b/src/node_native_module.cc @@ -71,6 +71,7 @@ void NativeModuleLoader::InitializeModuleCategories() { std::vector prefixes = { #if !HAVE_OPENSSL "internal/crypto/", + "internal/debugger/", #endif // !HAVE_OPENSSL "internal/bootstrap/",