From d793a4b16ccfedb2c68128da0a2bfff30d515408 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 5 Jul 2026 18:30:17 -0700 Subject: [PATCH 01/12] net: improve dtls cert verification Signed-off-by: James M Snell Assisted-by: Claude/Opus --- doc/api/dtls.md | 11 ++- lib/internal/dtls/dtls.js | 35 +++++++-- src/dtls/dtls_endpoint.cc | 27 ++++++- src/dtls/dtls_endpoint.h | 8 +- src/dtls/dtls_session.cc | 40 +++++++++- src/dtls/dtls_session.h | 10 ++- test/parallel/test-dtls-verify-identity.mjs | 86 +++++++++++++++++++++ 7 files changed, 201 insertions(+), 16 deletions(-) create mode 100644 test/parallel/test-dtls-verify-identity.mjs diff --git a/doc/api/dtls.md b/doc/api/dtls.md index 444e5cbf0d7bb1..df0e5b50a42840 100644 --- a/doc/api/dtls.md +++ b/doc/api/dtls.md @@ -123,8 +123,15 @@ added: REPLACEME * `ca` {string|Buffer|string\[]|Buffer\[]} CA certificates in PEM format. * `cert` {string|Buffer} Client certificate in PEM format. * `key` {string|Buffer} Client private key in PEM format. - * `rejectUnauthorized` {boolean} Reject connections with unverifiable - certificates. **Default:** `true`. + * `rejectUnauthorized` {boolean} When `true`, the server's certificate must + both chain to a trusted CA and match the expected identity (`servername`, + or `host` when `servername` is not set); otherwise the handshake is + aborted and `session.opened` rejects. When `false`, the certificate is not + verified. **Default:** `true`. + * `servername` {string} Server name used for the SNI (Server Name + Indication) extension and as the identity checked during certificate + verification. **Default:** the `host` argument. Set to `''` to disable SNI. + SNI is never sent for IP address literals. * `bindHost` {string} Local bind address. **Default:** `'0.0.0.0'`. * `bindPort` {number} Local bind port. **Default:** `0` (ephemeral). * `alpn` {string\[]|Buffer} ALPN protocol names. diff --git a/lib/internal/dtls/dtls.js b/lib/internal/dtls/dtls.js index c4aab52e6e76f3..5913b00dc70280 100644 --- a/lib/internal/dtls/dtls.js +++ b/lib/internal/dtls/dtls.js @@ -41,6 +41,10 @@ const { Buffer, } = require('buffer'); +const { + isIP, +} = require('internal/net'); + const { DTLSEndpointState, DTLSSessionState, @@ -358,11 +362,12 @@ class DTLSEndpoint { // --- Client mode --- - connect(context, host, port, servername) { - const sessionHandle = this.#handle.connect(context, host, port); - if (servername) { - sessionHandle.setServername(servername); - } + connect(context, host, port, servername, verifyHost, verifyIsIp) { + // servername (SNI) and verifyHost (expected peer identity) are applied to + // the client SSL inside the binding, before the handshake's ClientHello is + // emitted. They cannot be set afterwards from JS. + const sessionHandle = this.#handle.connect( + context, host, port, servername, verifyHost, verifyIsIp); const session = new DTLSSession( kPrivateConstructor, sessionHandle, this); this.#sessions.add(session); @@ -634,11 +639,27 @@ function connect(host, port, options = kEmptyObject) { // Default SNI servername to the host argument (matching Node.js TLS). // Can be overridden with options.servername, or disabled with '' or false. - const servername = options.servername !== undefined ? + // SNI is never sent for IP literals (also matching Node.js TLS). + let servername = options.servername !== undefined ? (options.servername || undefined) : host; + if (servername !== undefined && isIP(servername) !== 0) { + servername = undefined; + } + + // Peer identity to verify. Unless rejectUnauthorized is explicitly false, + // the server certificate must match this name (or IP address) in addition + // to chaining to a trusted CA. Defaults to the requested servername, + // falling back to the connect host. + let verifyHost; + let verifyIsIp = false; + if (options.rejectUnauthorized !== false) { + verifyHost = options.servername || host; + verifyIsIp = isIP(verifyHost) !== 0; + } - const session = endpoint.connect(context, host, port, servername); + const session = endpoint.connect( + context, host, port, servername, verifyHost, verifyIsIp); // Mark that this session owns the endpoint so it gets closed // automatically when the session closes, allowing process exit. session.ownsEndpoint = true; diff --git a/src/dtls/dtls_endpoint.cc b/src/dtls/dtls_endpoint.cc index 9433241a2d1433..018a098ca13277 100644 --- a/src/dtls/dtls_endpoint.cc +++ b/src/dtls/dtls_endpoint.cc @@ -155,7 +155,10 @@ int DTLSEndpoint::Listen(DTLSContext* context) { } BaseObjectPtr DTLSEndpoint::Connect(DTLSContext* context, - const SocketAddress& remote) { + const SocketAddress& remote, + const char* servername, + const char* verify_host, + bool verify_is_ip) { if (IsHandleClosing()) { THROW_ERR_INVALID_STATE(env(), "Endpoint is closing"); return {}; @@ -168,8 +171,14 @@ BaseObjectPtr DTLSEndpoint::Connect(DTLSContext* context, return {}; } - auto session = DTLSSession::Create( - env(), this, context->ssl_ctx(), remote, false /* is_server */); + auto session = DTLSSession::Create(env(), + this, + context->ssl_ctx(), + remote, + false /* is_server */, + servername, + verify_host, + verify_is_ip); if (!session) return {}; @@ -576,7 +585,17 @@ void DTLSEndpoint::DoConnect(const FunctionCallbackInfo& args) { THROW_IF_INSUFFICIENT_PERMISSIONS( env, permission::PermissionScope::kNet, remote.ToString()); - auto session = endpoint->Connect(context, remote); + // Optional: servername (SNI), verifyHost (expected peer identity), and + // whether verifyHost is an IP literal. These are resolved in JS and applied + // to the client SSL before the handshake starts. + Utf8Value servername(env->isolate(), args[3]); + Utf8Value verify_host(env->isolate(), args[4]); + const char* servername_ptr = args[3]->IsString() ? *servername : nullptr; + const char* verify_host_ptr = args[4]->IsString() ? *verify_host : nullptr; + bool verify_is_ip = args[5]->IsTrue(); + + auto session = endpoint->Connect( + context, remote, servername_ptr, verify_host_ptr, verify_is_ip); if (session) { args.GetReturnValue().Set(session->object()); } diff --git a/src/dtls/dtls_endpoint.h b/src/dtls/dtls_endpoint.h index a6fe94fff5b8cc..f2a72f08413b91 100644 --- a/src/dtls/dtls_endpoint.h +++ b/src/dtls/dtls_endpoint.h @@ -59,9 +59,15 @@ class DTLSEndpoint final : public HandleWrap { int Listen(DTLSContext* context); // Initiate a client connection to the given address. + // |servername|/|verify_host|/|verify_is_ip| configure SNI and peer identity + // verification on the client SSL before the handshake begins; see + // DTLSSession::Create. // Returns the created DTLSSession. BaseObjectPtr Connect(DTLSContext* context, - const SocketAddress& remote); + const SocketAddress& remote, + const char* servername = nullptr, + const char* verify_host = nullptr, + bool verify_is_ip = false); // Send a raw UDP datagram to the given address. // Called by DTLSSession to send encrypted packets. diff --git a/src/dtls/dtls_session.cc b/src/dtls/dtls_session.cc index 8bcd06a4ae71d9..9047a84ae77b8f 100644 --- a/src/dtls/dtls_session.cc +++ b/src/dtls/dtls_session.cc @@ -18,6 +18,8 @@ #include #include #include +#include +#include #include @@ -153,7 +155,10 @@ BaseObjectPtr DTLSSession::Create(Environment* env, DTLSEndpoint* endpoint, SSL_CTX* ssl_ctx, const SocketAddress& remote, - bool is_server) { + bool is_server, + const char* servername, + const char* verify_host, + bool verify_is_ip) { // Create the SSL object. SSL* ssl_raw = SSL_new(ssl_ctx); if (ssl_raw == nullptr) { @@ -188,6 +193,39 @@ BaseObjectPtr DTLSSession::Create(Environment* env, SSL_set_accept_state(ssl.get()); } else { SSL_set_connect_state(ssl.get()); + + // Configure SNI and peer identity verification BEFORE the handshake + // starts. The caller (DTLSEndpoint::Connect) runs Cycle() immediately + // after Create() returns, which emits the ClientHello, so anything that + // must appear in that flight (SNI) has to be set here rather than via a + // post-construction setter. + if (servername != nullptr && servername[0] != '\0') { + SSL_set_tlsext_host_name(ssl.get(), servername); + } + + // When identity verification is requested, bind the expected peer name + // (or IP) into the verification parameters. Combined with the context's + // SSL_VERIFY_PEER mode this makes a name mismatch fail the handshake, + // rather than accepting any certificate that merely chains to a trusted + // CA. A failure to configure it is fatal: proceeding would silently skip + // the identity check. + if (verify_host != nullptr && verify_host[0] != '\0') { + if (verify_is_ip) { + if (!X509_VERIFY_PARAM_set1_ip_asc(SSL_get0_param(ssl.get()), + verify_host)) { + THROW_ERR_CRYPTO_OPERATION_FAILED( + env, "Failed to set peer IP address for verification"); + return {}; + } + } else { + SSL_set_hostflags(ssl.get(), X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS); + if (!SSL_set1_host(ssl.get(), verify_host)) { + THROW_ERR_CRYPTO_OPERATION_FAILED( + env, "Failed to set peer hostname for verification"); + return {}; + } + } + } } // Create the JS wrapper object. diff --git a/src/dtls/dtls_session.h b/src/dtls/dtls_session.h index d64d0e4d48736a..922085447aaede 100644 --- a/src/dtls/dtls_session.h +++ b/src/dtls/dtls_session.h @@ -54,11 +54,19 @@ class DTLSSession final : public AsyncWrap { // |ssl_ctx| - the SSL_CTX to create the SSL* from // |remote| - the peer address // |is_server| - true if this is a server-side session + // |servername| - SNI to advertise (client only); nullptr to omit. + // |verify_host| - expected peer identity to verify (client only); + // nullptr disables identity checking. + // |verify_is_ip| - true if |verify_host| is an IP literal (verified + // against iPAddress SANs) rather than a DNS name. static BaseObjectPtr Create(Environment* env, DTLSEndpoint* endpoint, SSL_CTX* ssl_ctx, const SocketAddress& remote, - bool is_server); + bool is_server, + const char* servername = nullptr, + const char* verify_host = nullptr, + bool verify_is_ip = false); // Create a session from an already-initialized SSL object. // Used by the server after DTLSv1_listen() returns 1 — the SSL diff --git a/test/parallel/test-dtls-verify-identity.mjs b/test/parallel/test-dtls-verify-identity.mjs new file mode 100644 index 00000000000000..7e6a263f95cef5 --- /dev/null +++ b/test/parallel/test-dtls-verify-identity.mjs @@ -0,0 +1,86 @@ +// Flags: --experimental-dtls --no-warnings + +// Test: the DTLS client verifies the server certificate's identity (hostname +// or IP address) against the expected name, not merely that the certificate +// chains to a trusted CA. Also exercises that SNI is actually carried in the +// ClientHello (it is applied before the handshake begins). + +import { hasCrypto, skip, mustCall, mustNotCall } from '../common/index.mjs'; +import assert from 'node:assert'; +import * as fixtures from '../common/fixtures.mjs'; + +const { match, rejects, strictEqual } = assert; +const { readKey } = fixtures; + +if (!hasCrypto) { + skip('missing crypto'); +} + +if (!process.features.dtls) { + skip('DTLS is not enabled'); +} + +const { listen, connect } = await import('node:dtls'); + +// agent1's certificate has CN=agent1 and no subjectAltName, so OpenSSL matches +// the identity "agent1" (via the subject CN) and rejects any other name. +const cert = readKey('agent1-cert.pem').toString(); +const key = readKey('agent1-key.pem').toString(); +const ca = readKey('ca1-cert.pem').toString(); + +// --------------------------------------------------------------------------- +// Case 1: a matching servername with rejectUnauthorized succeeds, and the +// server observes the SNI the client sent (proving it is in the ClientHello). +{ + const sawServername = Promise.withResolvers(); + + const server = listen(mustCall((session) => { + session.onhandshake = mustCall(() => { + sawServername.resolve(session.servername); + }); + }), { cert, key, port: 0, host: '127.0.0.1' }); + + const { port } = server.address; + + const session = connect('127.0.0.1', port, { + ca: [ca], + rejectUnauthorized: true, + servername: 'agent1', + }); + + const { protocol } = await session.opened; + match(protocol, /DTLS/i); + + strictEqual(await sawServername.promise, 'agent1'); + + await session.close(); + await server.close(); +} + +// --------------------------------------------------------------------------- +// Case 2: a servername that does not match the certificate is rejected during +// the handshake, even though the certificate chain is otherwise valid. +{ + const server = listen(mustCall((session) => { + // The client aborts after verifying the certificate, so the server-side + // handshake never completes. + session.onhandshake = mustNotCall(); + // The server session's `opened` is never awaited here; ensure it can't + // surface as an unhandled rejection if the stalled handshake later errors. + session.opened.catch(() => {}); + }), { cert, key, port: 0, host: '127.0.0.1' }); + + const { port } = server.address; + + const session = connect('127.0.0.1', port, { + ca: [ca], + rejectUnauthorized: true, + servername: 'wrong.example.com', + }); + + await rejects(session.opened, /certificate verify failed/i); + + // Closing the session tears down the internally-owned client endpoint. + await session.close(); + await server.close(); +} From 1f1b7bd2c31cf5497036be5e096aec2672bc46fa Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 5 Jul 2026 18:44:05 -0700 Subject: [PATCH 02/12] net: fixing up dtls details Signed-off-by: James M Snell Assisted-by: Claude/Opus --- lib/internal/dtls/dtls.js | 58 +++++++++---------- src/dtls/dtls_session.cc | 11 ---- src/dtls/dtls_session.h | 1 - .../test-dtls-interop-openssl-client.mjs | 6 +- 4 files changed, 32 insertions(+), 44 deletions(-) diff --git a/lib/internal/dtls/dtls.js b/lib/internal/dtls/dtls.js index 5913b00dc70280..6b5072a190a5de 100644 --- a/lib/internal/dtls/dtls.js +++ b/lib/internal/dtls/dtls.js @@ -362,12 +362,25 @@ class DTLSEndpoint { // --- Client mode --- - connect(context, host, port, servername, verifyHost, verifyIsIp) { - // servername (SNI) and verifyHost (expected peer identity) are applied to - // the client SSL inside the binding, before the handshake's ClientHello is - // emitted. They cannot be set afterwards from JS. + connect(context, host, port, servername) { + // Resolve SNI and the expected peer identity here so that every caller of + // the endpoint API -- not only the top-level dtls.connect() -- gets safe + // defaults. The identity is always bound to the requested servername (or, + // failing that, the host). OpenSSL only *enforces* it when the context is + // in a verifying mode, so binding it is a no-op for non-verifying + // (rejectUnauthorized: false) contexts. + // + // These are applied to the client SSL inside the binding, before the + // handshake's ClientHello is emitted; they cannot be set afterwards. + let sni = servername !== undefined ? (servername || undefined) : host; + if (sni !== undefined && isIP(sni) !== 0) { + sni = undefined; // SNI is never sent for IP literals (matching TLS). + } + const verifyHost = servername || host; + const verifyIsIp = isIP(verifyHost) !== 0; + const sessionHandle = this.#handle.connect( - context, host, port, servername, verifyHost, verifyIsIp); + context, host, port, sni, verifyHost, verifyIsIp); const session = new DTLSSession( kPrivateConstructor, sessionHandle, this); this.#sessions.add(session); @@ -609,7 +622,12 @@ function listen(onsession, options = kEmptyObject) { * @param {string|Buffer|Array} [options.ca] CA certificates (PEM). * @param {string|Buffer} [options.cert] Client certificate (PEM). * @param {string|Buffer} [options.key] Client private key (PEM). - * @param {boolean} [options.rejectUnauthorized] Reject unauthorized. + * @param {boolean} [options.rejectUnauthorized] When true (default), verify + * the server certificate against the trusted CAs and check its identity + * against servername (or host); aborts the handshake on failure. + * @param {string} [options.servername] Server name for the SNI extension and + * the identity checked during certificate verification. Defaults to host; + * set to '' to disable SNI. Never sent for IP address literals. * @param {string} [options.bindHost] Local bind address. * @param {number} [options.bindPort] Local bind port (0 = ephemeral). * @param {number} [options.mtu] MTU for DTLS records. @@ -637,29 +655,11 @@ function connect(host, port, options = kEmptyObject) { endpoint.bind(bindHost, bindPort); - // Default SNI servername to the host argument (matching Node.js TLS). - // Can be overridden with options.servername, or disabled with '' or false. - // SNI is never sent for IP literals (also matching Node.js TLS). - let servername = options.servername !== undefined ? - (options.servername || undefined) : - host; - if (servername !== undefined && isIP(servername) !== 0) { - servername = undefined; - } - - // Peer identity to verify. Unless rejectUnauthorized is explicitly false, - // the server certificate must match this name (or IP address) in addition - // to chaining to a trusted CA. Defaults to the requested servername, - // falling back to the connect host. - let verifyHost; - let verifyIsIp = false; - if (options.rejectUnauthorized !== false) { - verifyHost = options.servername || host; - verifyIsIp = isIP(verifyHost) !== 0; - } - - const session = endpoint.connect( - context, host, port, servername, verifyHost, verifyIsIp); + // SNI and peer-identity verification are resolved inside + // DTLSEndpoint.connect(), which defaults both to the host argument (matching + // Node.js TLS). The identity is enforced whenever the context verifies, i.e. + // unless rejectUnauthorized is false. + const session = endpoint.connect(context, host, port, options.servername); // Mark that this session owns the endpoint so it gets closed // automatically when the session closes, allowing process exit. session.ownsEndpoint = true; diff --git a/src/dtls/dtls_session.cc b/src/dtls/dtls_session.cc index 9047a84ae77b8f..1cbc903ec4d198 100644 --- a/src/dtls/dtls_session.cc +++ b/src/dtls/dtls_session.cc @@ -117,7 +117,6 @@ Local DTLSSession::GetConstructorTemplate(Environment* env) { SetProtoMethod(isolate, tmpl, "getALPNProtocol", GetALPNProtocol); SetProtoMethod(isolate, tmpl, "exportKeyingMaterial", ExportKeyingMaterial); SetProtoMethod(isolate, tmpl, "getSRTPProfile", GetSRTPProfile); - SetProtoMethod(isolate, tmpl, "setServername", SetServername); SetProtoMethod(isolate, tmpl, "getServername", GetServername); env->set_dtls_session_constructor_template(tmpl); @@ -147,7 +146,6 @@ void DTLSSession::RegisterExternalReferences( registry->Register(GetALPNProtocol); registry->Register(ExportKeyingMaterial); registry->Register(GetSRTPProfile); - registry->Register(SetServername); registry->Register(GetServername); } @@ -697,15 +695,6 @@ void DTLSSession::GetSRTPProfile(const FunctionCallbackInfo& args) { } } -void DTLSSession::SetServername(const FunctionCallbackInfo& args) { - DTLSSession* session; - ASSIGN_OR_RETURN_UNWRAP(&session, args.This()); - - CHECK(args[0]->IsString()); - Utf8Value servername(session->env()->isolate(), args[0]); - SSL_set_tlsext_host_name(session->ssl_.get(), *servername); -} - void DTLSSession::GetServername(const FunctionCallbackInfo& args) { DTLSSession* session; ASSIGN_OR_RETURN_UNWRAP(&session, args.This()); diff --git a/src/dtls/dtls_session.h b/src/dtls/dtls_session.h index 922085447aaede..162752b6eb4c8c 100644 --- a/src/dtls/dtls_session.h +++ b/src/dtls/dtls_session.h @@ -127,7 +127,6 @@ class DTLSSession final : public AsyncWrap { static void ExportKeyingMaterial( const v8::FunctionCallbackInfo& args); static void GetSRTPProfile(const v8::FunctionCallbackInfo& args); - static void SetServername(const v8::FunctionCallbackInfo& args); static void GetServername(const v8::FunctionCallbackInfo& args); public: diff --git a/test/sequential/test-dtls-interop-openssl-client.mjs b/test/sequential/test-dtls-interop-openssl-client.mjs index 683d725e76698c..580cc0401a18e1 100644 --- a/test/sequential/test-dtls-interop-openssl-client.mjs +++ b/test/sequential/test-dtls-interop-openssl-client.mjs @@ -8,7 +8,7 @@ import { hasCrypto, skip, mustCall } from '../common/index.mjs'; import { createRequire } from 'module'; import assert from 'node:assert'; import { spawn } from 'node:child_process'; -import { setTimeout } from 'node:timers/promises'; +import { setTimeout as sleep } from 'node:timers/promises'; import * as fixtures from '../common/fixtures.mjs'; if (!hasCrypto) { @@ -73,13 +73,13 @@ const timeout = setTimeout(() => { // Wait for the handshake to start (s_client writes TLS info to stdout), // then send data. await new Promise((resolve) => client.stdout.once('data', resolve)); -await setTimeout(500); +await sleep(500); client.stdin.write('hello from openssl\n'); // Wait for the server to receive and reply. await serverReceivedData.promise; -await setTimeout(500); +await sleep(500); // Close stdin so s_client exits. client.stdin.end(); From 5fb654d22f70446a176c3aa770f7199fe797533e Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 5 Jul 2026 18:59:05 -0700 Subject: [PATCH 03/12] net: fixup UAF in dtls Signed-off-by: James M Snell Assisted-by: Claude/Opus --- src/dtls/dtls_session.cc | 13 +++ .../test-dtls-destroy-in-callback.mjs | 84 +++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 test/parallel/test-dtls-destroy-in-callback.mjs diff --git a/src/dtls/dtls_session.cc b/src/dtls/dtls_session.cc index 1cbc903ec4d198..45704af69348df 100644 --- a/src/dtls/dtls_session.cc +++ b/src/dtls/dtls_session.cc @@ -55,6 +55,10 @@ DTLSSession::DTLSSession(Environment* env, retransmit_timer_(env, [this] { if (destroyed_) return; + // Keep ourselves alive across the callback: emitting + // an error or running Cycle() below can synchronously + // destroy this session, and this timer lives on it. + BaseObjectPtr strong_ref{this}; DTLS_STAT_INCREMENT(DTLSSessionStats, retransmit_count); int ret = DTLSv1_handle_timeout(ssl_.get()); @@ -282,6 +286,12 @@ void DTLSSession::Receive(const uint8_t* data, size_t len) { void DTLSSession::Cycle() { if (destroyed_) return; + // Pin a strong reference to ourselves for the duration of the pump. A JS + // callback dispatched below (message/handshake/error) can synchronously + // destroy this session, which removes the endpoint's only strong reference + // and would otherwise free `this` while we are still using ssl_/state_. + BaseObjectPtr strong_ref{this}; + // Prevent infinite recursion. if (++cycle_depth_ > 1) { cycle_depth_--; @@ -352,6 +362,9 @@ void DTLSSession::ClearOut() { .ToLocalChecked(), }; EmitCallback(DTLS_CB_SESSION_MESSAGE, 1, argv); + // The message handler may have destroyed the session synchronously; stop + // reading if so (Cycle()'s strong reference keeps `this` itself alive). + if (destroyed_) return; } int err = SSL_get_error(ssl_.get(), read); diff --git a/test/parallel/test-dtls-destroy-in-callback.mjs b/test/parallel/test-dtls-destroy-in-callback.mjs new file mode 100644 index 00000000000000..e8fa0f5b3eed34 --- /dev/null +++ b/test/parallel/test-dtls-destroy-in-callback.mjs @@ -0,0 +1,84 @@ +// Flags: --experimental-dtls --no-warnings + +// Test: destroying a DTLS session synchronously from within a callback that is +// dispatched from the session's own I/O pump must not crash. The endpoint's +// session table holds the only strong reference to the session, so a reentrant +// destroy() removes it mid-pump; the implementation must keep the object alive +// until the pump unwinds (regression test for a use-after-free). + +import { hasCrypto, skip, mustCall } from '../common/index.mjs'; +import * as fixtures from '../common/fixtures.mjs'; + +const { readKey } = fixtures; + +if (!hasCrypto) { + skip('missing crypto'); +} + +if (!process.features.dtls) { + skip('DTLS is not enabled'); +} + +const { listen, connect } = await import('node:dtls'); + +const cert = readKey('agent1-cert.pem').toString(); +const key = readKey('agent1-key.pem').toString(); +const ca = readKey('ca1-cert.pem').toString(); + +// --------------------------------------------------------------------------- +// Case 1: destroy the (server) session from inside onmessage. The datagram +// carrying the message drives receive -> pump -> onmessage -> destroy(), which +// frees the session's map entry while ClearOut() is still looping over ssl_. +{ + const destroyed = Promise.withResolvers(); + + const server = listen(mustCall((session) => { + session.onmessage = mustCall(() => { + session.destroy(); + destroyed.resolve(); + }); + }), { cert, key, port: 0, host: '127.0.0.1' }); + + const client = connect('127.0.0.1', server.address.port, { + ca: [ca], + rejectUnauthorized: false, + }); + + await client.opened; + client.send('destroy me from onmessage'); + + await destroyed.promise; + + await client.close(); + await server.close(); +} + +// --------------------------------------------------------------------------- +// Case 2: destroy the (server) session from inside onhandshake. Handshake +// completion is emitted from the middle of the pump (Cycle), so destroying +// there must not free the session before the pump finishes unwinding. +{ + const destroyed = Promise.withResolvers(); + + const server = listen(mustCall((session) => { + session.onhandshake = mustCall(() => { + session.destroy(); + destroyed.resolve(); + }); + }), { cert, key, port: 0, host: '127.0.0.1' }); + + const client = connect('127.0.0.1', server.address.port, { + ca: [ca], + rejectUnauthorized: false, + }); + + // The server tears its session down the moment its handshake completes. The + // client's own handshake normally resolves, but guard against it rejecting + // (racing the teardown) so it can't surface as an unhandled rejection. + client.opened.catch(() => {}); + + await destroyed.promise; + + await client.close(); + await server.close(); +} From 315bba9a99b93f882778e61cba7742c32e4b24d6 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 5 Jul 2026 19:33:57 -0700 Subject: [PATCH 04/12] net: fix session leak in dtls Signed-off-by: James M Snell Assisted-by: Claude/Opus --- src/dtls/dtls_session.cc | 16 ++++ .../test-dtls-session-table-cleanup.mjs | 81 +++++++++++++++++++ test/parallel/test-dtls-verify-identity.mjs | 4 +- 3 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-dtls-session-table-cleanup.mjs diff --git a/src/dtls/dtls_session.cc b/src/dtls/dtls_session.cc index 45704af69348df..972f3d70d2b445 100644 --- a/src/dtls/dtls_session.cc +++ b/src/dtls/dtls_session.cc @@ -385,6 +385,10 @@ void DTLSSession::ClearOut() { EncOut(); Local argv[] = {}; EmitCallback(DTLS_CB_SESSION_CLOSE, 0, argv); + // Drop ourselves from the endpoint's session table now that the peer + // has closed. Safe here: Cycle() holds a strong reference for the + // duration of the pump. + Destroy(); } break; @@ -453,6 +457,12 @@ int DTLSSession::Send(const uint8_t* data, size_t len) { void DTLSSession::Close() { if (destroyed_ || closed_) return; + // Emitting the close below can synchronously free this session (a client + // session that owns its endpoint tears the endpoint -- and thus itself -- + // down from the close callback), and we call Destroy() afterwards. Pin a + // strong reference so `this` survives until we return. + BaseObjectPtr strong_ref{this}; + closed_ = true; state_->closing = 1; DTLS_STAT_RECORD_TIMESTAMP(DTLSSessionStats, closing_at); @@ -474,6 +484,12 @@ void DTLSSession::Close() { Context::Scope context_scope(env()->context()); Local argv[] = {}; EmitCallback(DTLS_CB_SESSION_CLOSE, 0, argv); + + // Remove ourselves from the endpoint's session table and release resources. + // Without this, a gracefully-closed session would linger in the table for + // the life of the endpoint, leaking memory and blocking reuse of its + // address. + Destroy(); } void DTLSSession::Destroy() { diff --git a/test/parallel/test-dtls-session-table-cleanup.mjs b/test/parallel/test-dtls-session-table-cleanup.mjs new file mode 100644 index 00000000000000..5e8ff89f5d6dde --- /dev/null +++ b/test/parallel/test-dtls-session-table-cleanup.mjs @@ -0,0 +1,81 @@ +// Flags: --experimental-dtls --no-warnings + +// Test: a session is removed from its endpoint's session table when it closes, +// whether closed locally or by the peer. Regression test for closed sessions +// leaking in the table (which also blocks reuse of the peer's address). + +import { hasCrypto, skip, mustCall } from '../common/index.mjs'; +import assert from 'node:assert'; +import * as fixtures from '../common/fixtures.mjs'; + +const { strictEqual } = assert; +const { readKey } = fixtures; + +if (!hasCrypto) { + skip('missing crypto'); +} + +if (!process.features.dtls) { + skip('DTLS is not enabled'); +} + +const { listen, connect } = await import('node:dtls'); + +const cert = readKey('agent1-cert.pem').toString(); +const key = readKey('agent1-key.pem').toString(); +const ca = readKey('ca1-cert.pem').toString(); + +// --------------------------------------------------------------------------- +// Case 1: the server closes its own session -> removed from the table. +{ + const gotSession = Promise.withResolvers(); + + const server = listen(mustCall((session) => { + gotSession.resolve(session); + }), { cert, key, port: 0, host: '127.0.0.1' }); + + const client = connect('127.0.0.1', server.address.port, { + ca: [ca], + rejectUnauthorized: false, + }); + + await client.opened; + const session = await gotSession.promise; + + strictEqual(server.state.sessionCount, 1); + await session.close(); + strictEqual(server.state.sessionCount, 0); + + await client.close(); + await server.close(); +} + +// --------------------------------------------------------------------------- +// Case 2: the client closes -> the server sees the peer close_notify and +// removes the session from the table. +{ + const gotSession = Promise.withResolvers(); + + const server = listen(mustCall((session) => { + gotSession.resolve(session); + }), { cert, key, port: 0, host: '127.0.0.1' }); + + const client = connect('127.0.0.1', server.address.port, { + ca: [ca], + rejectUnauthorized: false, + }); + + await client.opened; + const session = await gotSession.promise; + + strictEqual(server.state.sessionCount, 1); + await client.close(); // Sends close_notify to the server + await session.closed; // Server processes it and emits its close + // The session is dropped from the table immediately after its close callback + // runs (which can execute synchronously during the close emit's microtask + // drain); yield to the event loop so teardown settles before reading count. + await new Promise((resolve) => setImmediate(resolve)); + strictEqual(server.state.sessionCount, 0); + + await server.close(); +} diff --git a/test/parallel/test-dtls-verify-identity.mjs b/test/parallel/test-dtls-verify-identity.mjs index 7e6a263f95cef5..43d8c73a25678e 100644 --- a/test/parallel/test-dtls-verify-identity.mjs +++ b/test/parallel/test-dtls-verify-identity.mjs @@ -22,8 +22,8 @@ if (!process.features.dtls) { const { listen, connect } = await import('node:dtls'); -// agent1's certificate has CN=agent1 and no subjectAltName, so OpenSSL matches -// the identity "agent1" (via the subject CN) and rejects any other name. +// The agent1 certificate has CN=agent1 and no subjectAltName, so OpenSSL +// matches the identity "agent1" (via the subject CN) and rejects other names. const cert = readKey('agent1-cert.pem').toString(); const key = readKey('agent1-key.pem').toString(); const ca = readKey('ca1-cert.pem').toString(); From cc991f35e636f01e7c7502c743a5b6de1802db41 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 5 Jul 2026 19:48:12 -0700 Subject: [PATCH 05/12] net: fixup failed-connect hang in dtls Signed-off-by: James M Snell Assisted-by: Claude/Opus --- lib/internal/dtls/dtls.js | 16 +++++++ .../test-dtls-connect-error-cleanup.mjs | 48 +++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 test/parallel/test-dtls-connect-error-cleanup.mjs diff --git a/lib/internal/dtls/dtls.js b/lib/internal/dtls/dtls.js index 6b5072a190a5de..503831e4446f6e 100644 --- a/lib/internal/dtls/dtls.js +++ b/lib/internal/dtls/dtls.js @@ -265,6 +265,22 @@ class DTLSSession { this.#onerror(error); } this.#pendingOpen.reject(error); + + // The session has failed and cannot continue. Tear it down so it does not + // linger in the endpoint's table, and -- for a client session that owns + // its internal endpoint -- close the endpoint too so the event loop can + // drain. destroy() removes the session from the C++ table first, so the + // endpoint.close() below won't try to re-close it. Reentrant destroy from + // within the error emit is safe: Cycle()/the timer hold a strong ref. + const endpoint = this.#endpoint; + const ownsEndpoint = this.#ownsEndpoint; + this.destroy(); + if (endpoint) { + endpoint.sessions.delete(this); + if (ownsEndpoint) { + endpoint.close(); + } + } } [kSessionClose]() { diff --git a/test/parallel/test-dtls-connect-error-cleanup.mjs b/test/parallel/test-dtls-connect-error-cleanup.mjs new file mode 100644 index 00000000000000..63297f499b942d --- /dev/null +++ b/test/parallel/test-dtls-connect-error-cleanup.mjs @@ -0,0 +1,48 @@ +// Flags: --experimental-dtls --no-warnings + +// Test: a client connect() whose handshake fails must tear down its internally +// owned endpoint, so the event loop can drain. Regression test for a failed +// connect leaking the endpoint (and hanging the process). + +import { hasCrypto, skip, mustCall } from '../common/index.mjs'; +import assert from 'node:assert'; +import * as fixtures from '../common/fixtures.mjs'; + +const { rejects } = assert; +const { readKey } = fixtures; + +if (!hasCrypto) { + skip('missing crypto'); +} + +if (!process.features.dtls) { + skip('DTLS is not enabled'); +} + +const { listen, connect } = await import('node:dtls'); + +const cert = readKey('agent1-cert.pem').toString(); +const key = readKey('agent1-key.pem').toString(); +const ca = readKey('ca1-cert.pem').toString(); + +const server = listen(mustCall((session) => { + // The client rejects the certificate mid-handshake, so this server session + // never opens; ignore its unsettled promise. + session.opened.catch(() => {}); +}), { cert, key, port: 0, host: '127.0.0.1' }); + +// A servername that does not match the certificate, under rejectUnauthorized, +// makes the client's handshake fail during verification. +const session = connect('127.0.0.1', server.address.port, { + ca: [ca], + rejectUnauthorized: true, + servername: 'wrong.example.com', +}); + +await rejects(session.opened, /certificate verify failed/i); + +// The failed connect must have closed its internally-owned endpoint. Without +// that, this await never settles and the test times out. +await session.endpoint.closed; + +await server.close(); From e9a512e670ce03a5930dff8d081828d4ae884693 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 5 Jul 2026 20:00:23 -0700 Subject: [PATCH 06/12] net: fix dtls unhandled rejections Signed-off-by: James M Snell Assisted-by: Claude/Opus --- lib/internal/dtls/dtls.js | 10 ++++ .../test-dtls-unhandled-rejection.mjs | 49 +++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 test/parallel/test-dtls-unhandled-rejection.mjs diff --git a/lib/internal/dtls/dtls.js b/lib/internal/dtls/dtls.js index 503831e4446f6e..e6017e3e876194 100644 --- a/lib/internal/dtls/dtls.js +++ b/lib/internal/dtls/dtls.js @@ -7,6 +7,7 @@ const { ArrayIsArray, FunctionPrototypeBind, + PromisePrototypeThen, PromiseWithResolvers, SafeSet, SymbolAsyncDispose, @@ -106,6 +107,12 @@ class DTLSSession { kPrivateConstructor, handle.getStats()); this.#pendingOpen = PromiseWithResolvers(); this.#pendingClose = PromiseWithResolvers(); + // opened/closed may reject (handshake error, destroy(error)). Attach a + // no-op rejection handler so a caller that uses the callback API and never + // awaits them does not trigger an unhandled rejection; an explicit + // await/then/catch on opened/closed still observes the rejection. + PromisePrototypeThen(this.#pendingOpen.promise, undefined, () => {}); + PromisePrototypeThen(this.#pendingClose.promise, undefined, () => {}); } // --- Callback setters --- @@ -334,6 +341,9 @@ class DTLSEndpoint { this.#stats = new DTLSEndpointStats( kPrivateConstructor, this.#handle.getStats()); this.#pendingClose = PromiseWithResolvers(); + // See DTLSSession: keep an unobserved closed rejection from surfacing as an + // unhandled rejection. + PromisePrototypeThen(this.#pendingClose.promise, undefined, () => {}); if (options.mtu !== undefined) { validateInteger(options.mtu, 'options.mtu', 256, 65535); diff --git a/test/parallel/test-dtls-unhandled-rejection.mjs b/test/parallel/test-dtls-unhandled-rejection.mjs new file mode 100644 index 00000000000000..721afee7d5982d --- /dev/null +++ b/test/parallel/test-dtls-unhandled-rejection.mjs @@ -0,0 +1,49 @@ +// Flags: --experimental-dtls --no-warnings + +// Test: using the callback style (onerror) without awaiting opened/closed must +// not produce an unhandled promise rejection when the session errors. + +import { hasCrypto, skip, mustCall, mustNotCall } from '../common/index.mjs'; +import * as fixtures from '../common/fixtures.mjs'; + +const { readKey } = fixtures; + +if (!hasCrypto) { + skip('missing crypto'); +} + +if (!process.features.dtls) { + skip('DTLS is not enabled'); +} + +// Any unhandled rejection fails the test. +process.on('unhandledRejection', mustNotCall()); + +const { listen, connect } = await import('node:dtls'); + +const cert = readKey('agent1-cert.pem').toString(); +const key = readKey('agent1-key.pem').toString(); +const ca = readKey('ca1-cert.pem').toString(); + +const server = listen(mustCall(), { + cert, key, port: 0, host: '127.0.0.1', +}); + +const errored = Promise.withResolvers(); + +// A verification failure makes connect() reject its opened promise. Handle the +// error only through the callback API and never touch opened/closed. +const session = connect('127.0.0.1', server.address.port, { + ca: [ca], + rejectUnauthorized: true, + servername: 'wrong.example.com', +}); +session.onerror = mustCall(() => errored.resolve()); + +await errored.promise; + +// Give the rejected (but internally handled) opened promise a chance to be +// reported as unhandled, if it were, before the process exits. +await new Promise((resolve) => setImmediate(resolve)); + +await server.close(); From 2c39e2aec103a4ea4a78903ca5d1f9c24264591b Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 5 Jul 2026 20:06:59 -0700 Subject: [PATCH 07/12] net: add missing dtls permission check Signed-off-by: James M Snell Assisted-by: Claude/Opus --- src/dtls/dtls_endpoint.cc | 3 +++ test/parallel/test-permission-net-dtls.mjs | 11 +++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/dtls/dtls_endpoint.cc b/src/dtls/dtls_endpoint.cc index 018a098ca13277..ae19d464afed71 100644 --- a/src/dtls/dtls_endpoint.cc +++ b/src/dtls/dtls_endpoint.cc @@ -541,6 +541,9 @@ void DTLSEndpoint::DoBind(const FunctionCallbackInfo& args) { return THROW_ERR_INVALID_ARG_VALUE(env, "Invalid address"); } + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, permission::PermissionScope::kNet, addr.ToString()); + int err = endpoint->Bind(addr); if (err != 0) { return THROW_ERR_INVALID_STATE(env, uv_strerror(err)); diff --git a/test/parallel/test-permission-net-dtls.mjs b/test/parallel/test-permission-net-dtls.mjs index f2f2b7af2d2c3b..54c6235b3df1c4 100644 --- a/test/parallel/test-permission-net-dtls.mjs +++ b/test/parallel/test-permission-net-dtls.mjs @@ -51,9 +51,16 @@ const ca = readFileSync(join(fixturesDir, 'ca1-cert.pem')).toString(); ); } -// Test: Creating a DTLSEndpoint without connect/listen is allowed -// since no network I/O occurs at construction time. +// Test: Creating a DTLSEndpoint is allowed (no network I/O at construction +// time), but bind() requires net permission. { const endpoint = new DTLSEndpoint(); assert.ok(endpoint); + assert.throws( + () => endpoint.bind('127.0.0.1', 0), + { + code: 'ERR_ACCESS_DENIED', + permission: 'Net', + }, + ); } From ec94e962a83064c50e73fb9e2d06cbed55a3c0e9 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 5 Jul 2026 20:21:29 -0700 Subject: [PATCH 08/12] net: fix multiple minor dtls issues Signed-off-by: James M Snell Assisted-by: Claude/Opus --- src/dtls/dtls_context.cc | 86 ++++++++++++------- src/dtls/dtls_context.h | 8 ++ src/dtls/dtls_session.cc | 12 ++- .../parallel/test-dtls-servername-invalid.mjs | 27 ++++++ 4 files changed, 100 insertions(+), 33 deletions(-) create mode 100644 test/parallel/test-dtls-servername-invalid.mjs diff --git a/src/dtls/dtls_context.cc b/src/dtls/dtls_context.cc index ca5003df46c295..d59ee74feeb9e0 100644 --- a/src/dtls/dtls_context.cc +++ b/src/dtls/dtls_context.cc @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -35,6 +36,11 @@ namespace dtls { namespace { // The cookie secret is 32 bytes (256 bits). constexpr size_t kCookieSecretLen = 32; +// Cookies are bound to a coarse time window so they expire. A cookie is +// accepted for the window it was minted in and the immediately preceding one, +// giving ~30-60s of validity -- ample for the cookie exchange while bounding +// how long a captured cookie can be replayed. +constexpr uint64_t kCookieWindowNs = 30ull * 1000 * 1000 * 1000; } // namespace DTLSContext::DTLSContext(Environment* env, @@ -317,8 +323,11 @@ void DTLSContext::SetALPN(const FunctionCallbackInfo& args) { ctx->alpn_protos_.assign(data, data + len); SSL_CTX_set_alpn_select_cb(ctx->ctx_.get(), ALPNSelectCallback, ctx); } else { - // Client: advertise protocols to the server. - SSL_CTX_set_alpn_protos(ctx->ctx_.get(), data, len); + // Client: advertise protocols to the server. Returns 0 on success. + if (SSL_CTX_set_alpn_protos(ctx->ctx_.get(), data, len) != 0) { + return THROW_ERR_CRYPTO_OPERATION_FAILED( + env, "SSL_CTX_set_alpn_protos failed"); + } } } @@ -368,64 +377,77 @@ void DTLSContext::SetECDHCurve(const FunctionCallbackInfo& args) { } } -// HMAC-SHA256 based cookie generation using the peer's address. -// During DTLSv1_listen(), the peer address is taken from -// DTLSContext::current_cookie_peer_ (set synchronously before the call). -// During session handshake, the peer address is taken from the +// HMAC-SHA256 cookie derived from the peer's address and a coarse time window +// so cookies expire (see kCookieWindowNs). During DTLSv1_listen() the peer +// address comes from DTLSContext::current_cookie_peer_ (set synchronously +// before the call); during the session handshake it comes from the // DTLSSession stored in SSL app_data. -int DTLSContext::CookieGenerateCallback(SSL* ssl, - unsigned char* cookie, - unsigned int* cookie_len) { +bool DTLSContext::ComputeCookie(SSL* ssl, + uint64_t window, + unsigned char* out, + unsigned int* out_len) { SSL_CTX* ctx = SSL_get_SSL_CTX(ssl); DTLSContext* dtls_ctx = static_cast(SSL_CTX_get_app_data(ctx)); CHECK_NOT_NULL(dtls_ctx); - unsigned char addr_buf[sizeof(struct sockaddr_storage)]; + // Message = peer address bytes followed by the 8-byte window counter. + unsigned char msg[sizeof(struct sockaddr_storage) + sizeof(uint64_t)]; size_t addr_len = 0; void* app_data = SSL_get_app_data(ssl); if (app_data != nullptr) { // Session handshake path. - auto* session = static_cast(app_data); - const sockaddr* sa = session->remote_address().data(); + const sockaddr* sa = + static_cast(app_data)->remote_address().data(); addr_len = SocketAddress::GetLength(sa); - memcpy(addr_buf, sa, addr_len); + memcpy(msg, sa, addr_len); } else { - // DTLSv1_listen path — use the peer address stored on the context. + // DTLSv1_listen path -- use the peer address stored on the context. const sockaddr* sa = dtls_ctx->current_cookie_peer_.data(); addr_len = SocketAddress::GetLength(sa); - memcpy(addr_buf, sa, addr_len); + memcpy(msg, sa, addr_len); + } + + // Append the window counter in a fixed byte order. + for (size_t i = 0; i < sizeof(uint64_t); i++) { + msg[addr_len + i] = static_cast((window >> (8 * i)) & 0xff); } - unsigned int hmac_len = 0; unsigned char* result = HMAC(EVP_sha256(), dtls_ctx->cookie_secret_.data(), dtls_ctx->cookie_secret_.size(), - addr_buf, - addr_len, - cookie, - &hmac_len); - - if (result == nullptr) return 0; + msg, + addr_len + sizeof(uint64_t), + out, + out_len); + return result != nullptr; +} - *cookie_len = hmac_len; - return 1; +int DTLSContext::CookieGenerateCallback(SSL* ssl, + unsigned char* cookie, + unsigned int* cookie_len) { + const uint64_t window = uv_hrtime() / kCookieWindowNs; + return ComputeCookie(ssl, window, cookie, cookie_len) ? 1 : 0; } int DTLSContext::CookieVerifyCallback(SSL* ssl, const unsigned char* cookie, unsigned int cookie_len) { - // Generate the expected cookie and compare. + const uint64_t window = uv_hrtime() / kCookieWindowNs; + + // Accept a cookie minted in the current window or the immediately preceding + // one, so a handshake that straddles a window boundary still succeeds. unsigned char expected[EVP_MAX_MD_SIZE]; unsigned int expected_len = 0; - - if (CookieGenerateCallback(ssl, expected, &expected_len) != 1) { - return 0; + for (int i = 0; i < 2; i++) { + if (i == 1 && window == 0) break; + if (ComputeCookie(ssl, window - i, expected, &expected_len) && + cookie_len == expected_len && + CRYPTO_memcmp(cookie, expected, expected_len) == 0) { + return 1; + } } - - if (cookie_len != expected_len) return 0; - - return CRYPTO_memcmp(cookie, expected, expected_len) == 0 ? 1 : 0; + return 0; } int DTLSContext::ALPNSelectCallback(SSL* ssl, diff --git a/src/dtls/dtls_context.h b/src/dtls/dtls_context.h index 11d8113d308125..5c994ea769de21 100644 --- a/src/dtls/dtls_context.h +++ b/src/dtls/dtls_context.h @@ -57,6 +57,14 @@ class DTLSContext final : public BaseObject { static void LoadDefaultCAs(const v8::FunctionCallbackInfo& args); static void SetECDHCurve(const v8::FunctionCallbackInfo& args); + // Compute the address-and-time-window-bound cookie for |window| into |out| + // (which must have room for EVP_MAX_MD_SIZE bytes). Shared by the cookie + // generate/verify callbacks. + static bool ComputeCookie(SSL* ssl, + uint64_t window, + unsigned char* out, + unsigned int* out_len); + // Automatic DTLS cookie callbacks static int CookieGenerateCallback(SSL* ssl, unsigned char* cookie, diff --git a/src/dtls/dtls_session.cc b/src/dtls/dtls_session.cc index 972f3d70d2b445..f822888667db27 100644 --- a/src/dtls/dtls_session.cc +++ b/src/dtls/dtls_session.cc @@ -202,7 +202,11 @@ BaseObjectPtr DTLSSession::Create(Environment* env, // must appear in that flight (SNI) has to be set here rather than via a // post-construction setter. if (servername != nullptr && servername[0] != '\0') { - SSL_set_tlsext_host_name(ssl.get(), servername); + if (!SSL_set_tlsext_host_name(ssl.get(), servername)) { + THROW_ERR_CRYPTO_OPERATION_FAILED(env, + "Failed to set servername (SNI)"); + return {}; + } } // When identity verification is requested, bind the expected peer name @@ -310,6 +314,9 @@ void DTLSSession::Cycle() { unsigned long ossl_err = ERR_get_error(); // NOLINT(runtime/int) char err_buf[256]; ERR_error_string_n(ossl_err, err_buf, sizeof(err_buf)); + // Flush any fatal alert OpenSSL queued for the peer before emitting the + // error, which tears the session down and detaches the endpoint. + EncOut(); Local argv[] = { String::NewFromUtf8(env()->isolate(), err_buf).ToLocalChecked(), }; @@ -397,6 +404,9 @@ void DTLSSession::ClearOut() { unsigned long ossl_err = ERR_get_error(); // NOLINT(runtime/int) char err_buf[256]; ERR_error_string_n(ossl_err, err_buf, sizeof(err_buf)); + // Flush any fatal alert OpenSSL queued for the peer before emitting the + // error, which tears the session down and detaches the endpoint. + EncOut(); Local argv[] = { String::NewFromUtf8(env()->isolate(), err_buf).ToLocalChecked(), }; diff --git a/test/parallel/test-dtls-servername-invalid.mjs b/test/parallel/test-dtls-servername-invalid.mjs new file mode 100644 index 00000000000000..a15752345cc761 --- /dev/null +++ b/test/parallel/test-dtls-servername-invalid.mjs @@ -0,0 +1,27 @@ +// Flags: --experimental-dtls --no-warnings + +// Test: connect() surfaces a failure to set the SNI servername (for example a +// name longer than the TLS maximum of 255 bytes) as an error, rather than +// silently ignoring it. + +import { hasCrypto, skip } from '../common/index.mjs'; +import assert from 'node:assert'; + +if (!hasCrypto) { + skip('missing crypto'); +} + +if (!process.features.dtls) { + skip('DTLS is not enabled'); +} + +const { connect } = await import('node:dtls'); + +// SNI host names are limited to 255 bytes; a longer one must throw. +assert.throws( + () => connect('127.0.0.1', 12345, { + servername: 'a'.repeat(256), + rejectUnauthorized: false, + }), + { code: 'ERR_CRYPTO_OPERATION_FAILED' }, +); From 0c6d64890059571bfc65f2c91d806ff890b82013 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 5 Jul 2026 20:31:03 -0700 Subject: [PATCH 09/12] net: improve dtls buffer handling Signed-off-by: James M Snell Assisted-by: Claude/Opus --- src/dtls/dtls_endpoint.cc | 19 ++++++++++++------- src/dtls/dtls_endpoint.h | 6 ++++++ src/dtls/dtls_session.cc | 5 +++-- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/dtls/dtls_endpoint.cc b/src/dtls/dtls_endpoint.cc index ae19d464afed71..366c779ed1374e 100644 --- a/src/dtls/dtls_endpoint.cc +++ b/src/dtls/dtls_endpoint.cc @@ -340,8 +340,16 @@ void DTLSEndpoint::SetCallbacks(Local callbacks) { void DTLSEndpoint::OnAlloc(uv_handle_t* handle, size_t suggested_size, uv_buf_t* buf) { - buf->base = new char[65536]; - buf->len = 65536; + DTLSEndpoint* endpoint = static_cast(handle->data); + // Reuse a single receive buffer. libuv delivers datagrams one at a time on + // this thread, and OnRecv fully consumes each datagram (copying it into the + // session's BIO) before the next OnAlloc, so a per-endpoint buffer suffices + // and avoids a heap allocation on every packet. + if (endpoint->recv_buf_.empty()) { + endpoint->recv_buf_.resize(65536); + } + buf->base = endpoint->recv_buf_.data(); + buf->len = endpoint->recv_buf_.size(); } void DTLSEndpoint::OnRecv(uv_udp_t* handle, @@ -351,13 +359,12 @@ void DTLSEndpoint::OnRecv(uv_udp_t* handle, unsigned int flags) { DTLSEndpoint* endpoint = static_cast(handle->data); + // buf->base is the endpoint's reusable recv_buf_; it is not freed here. if (nread == 0 && addr == nullptr) { - delete[] buf->base; return; } if (nread < 0) { - delete[] buf->base; HandleScope handle_scope(endpoint->env()->isolate()); Context::Scope context_scope(endpoint->env()->context()); Local argv[] = { @@ -372,7 +379,6 @@ void DTLSEndpoint::OnRecv(uv_udp_t* handle, } if (addr == nullptr) { - delete[] buf->base; return; } @@ -384,8 +390,6 @@ void DTLSEndpoint::OnRecv(uv_udp_t* handle, SocketAddress remote(addr); endpoint->ProcessDatagram( reinterpret_cast(buf->base), nread, remote); - - delete[] buf->base; } void DTLSEndpoint::OnSend(uv_udp_send_t* req, int status) { @@ -663,6 +667,7 @@ void DTLSEndpoint::DoSetCallbacks(const FunctionCallbackInfo& args) { void DTLSEndpoint::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("sessions", sessions_.size()); + tracker->TrackFieldWithSize("recv_buf", recv_buf_.size()); } } // namespace dtls diff --git a/src/dtls/dtls_endpoint.h b/src/dtls/dtls_endpoint.h index f2a72f08413b91..d0a1e5652dcb1d 100644 --- a/src/dtls/dtls_endpoint.h +++ b/src/dtls/dtls_endpoint.h @@ -14,6 +14,7 @@ #include #include +#include #include "dtls.h" #include "dtls_context.h" @@ -135,6 +136,11 @@ class DTLSEndpoint final : public HandleWrap { uv_udp_t handle_; + // Reusable receive buffer for uv_udp_recv (see OnAlloc). libuv delivers one + // datagram at a time and OnRecv consumes each before the next OnAlloc, so a + // single buffer per endpoint avoids a heap allocation on every packet. + std::vector recv_buf_; + // Session table: maps remote address -> session. std::unordered_map, diff --git a/src/dtls/dtls_session.cc b/src/dtls/dtls_session.cc index f822888667db27..6f510a94efc232 100644 --- a/src/dtls/dtls_session.cc +++ b/src/dtls/dtls_session.cc @@ -356,8 +356,9 @@ void DTLSSession::Cycle() { void DTLSSession::ClearOut() { if (destroyed_) return; - // Try to read decrypted application data from OpenSSL. - uint8_t buf[65536]; + // Try to read decrypted application data from OpenSSL. A DTLS record's + // plaintext is at most 2^14 bytes, so one SSL_read yields at most that much. + uint8_t buf[16384]; int read; while ((read = SSL_read(ssl_.get(), buf, sizeof(buf))) > 0) { From 344f9f23b845591edb1ed7422e790b1b86762d44 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 5 Jul 2026 21:13:30 -0700 Subject: [PATCH 10/12] net: additional dtls fixups and tests Signed-off-by: James M Snell Assisted-by: Claude/Opus --- src/dtls/dtls_session.cc | 19 +++-- test/parallel/test-dtls-alpn.mjs | 1 - .../test-dtls-connect-error-cleanup.mjs | 8 +- .../test-dtls-destroy-in-callback.mjs | 5 -- .../test-dtls-session-table-cleanup.mjs | 6 +- test/parallel/test-dtls-srtp.mjs | 78 +++++++++++++++++++ .../test-dtls-unhandled-rejection.mjs | 7 +- test/parallel/test-dtls-verify-identity.mjs | 3 - 8 files changed, 98 insertions(+), 29 deletions(-) create mode 100644 test/parallel/test-dtls-srtp.mjs diff --git a/src/dtls/dtls_session.cc b/src/dtls/dtls_session.cc index 6f510a94efc232..02f1563abac7c8 100644 --- a/src/dtls/dtls_session.cc +++ b/src/dtls/dtls_session.cc @@ -391,11 +391,12 @@ void DTLSSession::ClearOut() { // Send our close_notify back. SSL_shutdown(ssl_.get()); EncOut(); + // Detach from the endpoint's session table before notifying JS so an + // observer of the close sees a consistent session count. Cycle() holds + // a strong reference for the duration of the pump. + if (auto ep = endpoint_.get()) ep->RemoveSession(remote_address_); Local argv[] = {}; EmitCallback(DTLS_CB_SESSION_CLOSE, 0, argv); - // Drop ourselves from the endpoint's session table now that the peer - // has closed. Safe here: Cycle() holds a strong reference for the - // duration of the pump. Destroy(); } break; @@ -490,16 +491,20 @@ void DTLSSession::Close() { state_->open = 0; + // Detach from the endpoint's session table before notifying JS, so an + // observer of the close (e.g. one awaiting `closed`) sees a consistent + // session count. We stay alive via strong_ref, and endpoint_ remains valid + // for the callback below; the Destroy() that follows clears it. + if (auto ep = endpoint_.get()) ep->RemoveSession(remote_address_); + // Notify JS. HandleScope handle_scope(env()->isolate()); Context::Scope context_scope(env()->context()); Local argv[] = {}; EmitCallback(DTLS_CB_SESSION_CLOSE, 0, argv); - // Remove ourselves from the endpoint's session table and release resources. - // Without this, a gracefully-closed session would linger in the table for - // the life of the endpoint, leaking memory and blocking reuse of its - // address. + // Release the remaining resources. RemoveSession above already detached us, + // so the one inside Destroy() is a no-op. Destroy(); } diff --git a/test/parallel/test-dtls-alpn.mjs b/test/parallel/test-dtls-alpn.mjs index b51721760fdfad..da9a6cbb56b136 100644 --- a/test/parallel/test-dtls-alpn.mjs +++ b/test/parallel/test-dtls-alpn.mjs @@ -26,7 +26,6 @@ const ca = readKey('ca1-cert.pem'); const serverAlpnChecked = Promise.withResolvers(); const endpoint = listen(mustCall(async (session) => { - session.onmessage = () => {}; await session.opened; // Server should see the negotiated ALPN protocol. strictEqual(session.alpnProtocol, 'coap'); diff --git a/test/parallel/test-dtls-connect-error-cleanup.mjs b/test/parallel/test-dtls-connect-error-cleanup.mjs index 63297f499b942d..049c248790072f 100644 --- a/test/parallel/test-dtls-connect-error-cleanup.mjs +++ b/test/parallel/test-dtls-connect-error-cleanup.mjs @@ -25,11 +25,9 @@ const cert = readKey('agent1-cert.pem').toString(); const key = readKey('agent1-key.pem').toString(); const ca = readKey('ca1-cert.pem').toString(); -const server = listen(mustCall((session) => { - // The client rejects the certificate mid-handshake, so this server session - // never opens; ignore its unsettled promise. - session.opened.catch(() => {}); -}), { cert, key, port: 0, host: '127.0.0.1' }); +// The client rejects the certificate mid-handshake, so this server session +// never opens; its opened rejection is handled internally by the library. +const server = listen(mustCall(), { cert, key, port: 0, host: '127.0.0.1' }); // A servername that does not match the certificate, under rejectUnauthorized, // makes the client's handshake fail during verification. diff --git a/test/parallel/test-dtls-destroy-in-callback.mjs b/test/parallel/test-dtls-destroy-in-callback.mjs index e8fa0f5b3eed34..97344bb7071921 100644 --- a/test/parallel/test-dtls-destroy-in-callback.mjs +++ b/test/parallel/test-dtls-destroy-in-callback.mjs @@ -72,11 +72,6 @@ const ca = readKey('ca1-cert.pem').toString(); rejectUnauthorized: false, }); - // The server tears its session down the moment its handshake completes. The - // client's own handshake normally resolves, but guard against it rejecting - // (racing the teardown) so it can't surface as an unhandled rejection. - client.opened.catch(() => {}); - await destroyed.promise; await client.close(); diff --git a/test/parallel/test-dtls-session-table-cleanup.mjs b/test/parallel/test-dtls-session-table-cleanup.mjs index 5e8ff89f5d6dde..386e6d427fc1c8 100644 --- a/test/parallel/test-dtls-session-table-cleanup.mjs +++ b/test/parallel/test-dtls-session-table-cleanup.mjs @@ -70,11 +70,7 @@ const ca = readKey('ca1-cert.pem').toString(); strictEqual(server.state.sessionCount, 1); await client.close(); // Sends close_notify to the server - await session.closed; // Server processes it and emits its close - // The session is dropped from the table immediately after its close callback - // runs (which can execute synchronously during the close emit's microtask - // drain); yield to the event loop so teardown settles before reading count. - await new Promise((resolve) => setImmediate(resolve)); + await session.closed; // Server processes it, detaches, and emits its close strictEqual(server.state.sessionCount, 0); await server.close(); diff --git a/test/parallel/test-dtls-srtp.mjs b/test/parallel/test-dtls-srtp.mjs new file mode 100644 index 00000000000000..f7c2c17e27d290 --- /dev/null +++ b/test/parallel/test-dtls-srtp.mjs @@ -0,0 +1,78 @@ +// Flags: --experimental-dtls --no-warnings + +// Test: SRTP profile negotiation and keying-material export (DTLS-SRTP). Both +// peers must negotiate the same SRTP profile and derive identical keying +// material from exportKeyingMaterial. + +import { hasCrypto, skip, mustCall } from '../common/index.mjs'; +import assert from 'node:assert'; +import * as fixtures from '../common/fixtures.mjs'; + +const { strictEqual, deepStrictEqual, notDeepStrictEqual } = assert; +const { readKey } = fixtures; + +if (!hasCrypto) { + skip('missing crypto'); +} + +if (!process.features.dtls) { + skip('DTLS is not enabled'); +} + +const { listen, connect } = await import('node:dtls'); + +const cert = readKey('agent1-cert.pem').toString(); +const key = readKey('agent1-key.pem').toString(); +const ca = readKey('ca1-cert.pem').toString(); + +const SRTP_PROFILE = 'SRTP_AEAD_AES_128_GCM'; + +const gotServerSession = Promise.withResolvers(); + +const server = listen(mustCall((session) => { + gotServerSession.resolve(session); +}), { + cert, + key, + port: 0, + host: '127.0.0.1', + srtp: SRTP_PROFILE, +}); + +const client = connect('127.0.0.1', server.address.port, { + ca: [ca], + rejectUnauthorized: false, + srtp: SRTP_PROFILE, +}); + +await client.opened; +const serverSession = await gotServerSession.promise; +await serverSession.opened; + +// Both peers negotiate the same SRTP profile. +strictEqual(client.srtpProfile, SRTP_PROFILE); +strictEqual(serverSession.srtpProfile, SRTP_PROFILE); + +// Both peers derive identical keying material for the same label and length. +const label = 'EXTRACTOR-dtls_srtp'; +const clientKM = client.exportKeyingMaterial(60, label); +const serverKM = serverSession.exportKeyingMaterial(60, label); +strictEqual(clientKM.length, 60); +deepStrictEqual(clientKM, serverKM); + +// The optional context is bound into the derivation: the same context yields +// the same material on both peers, and a different context yields different +// material. +const contextA = Buffer.from('context-a'); +const contextB = Buffer.from('context-b'); +deepStrictEqual( + client.exportKeyingMaterial(32, label, contextA), + serverSession.exportKeyingMaterial(32, label, contextA), +); +notDeepStrictEqual( + client.exportKeyingMaterial(32, label, contextA), + client.exportKeyingMaterial(32, label, contextB), +); + +await client.close(); +await server.close(); diff --git a/test/parallel/test-dtls-unhandled-rejection.mjs b/test/parallel/test-dtls-unhandled-rejection.mjs index 721afee7d5982d..31578997938ae1 100644 --- a/test/parallel/test-dtls-unhandled-rejection.mjs +++ b/test/parallel/test-dtls-unhandled-rejection.mjs @@ -42,8 +42,9 @@ session.onerror = mustCall(() => errored.resolve()); await errored.promise; -// Give the rejected (but internally handled) opened promise a chance to be -// reported as unhandled, if it were, before the process exits. -await new Promise((resolve) => setImmediate(resolve)); +// The failed session tears itself down; awaiting its closed promise is a real +// boundary that also gives any (incorrectly) unhandled opened rejection a turn +// to surface before the process exits. +await session.closed; await server.close(); diff --git a/test/parallel/test-dtls-verify-identity.mjs b/test/parallel/test-dtls-verify-identity.mjs index 43d8c73a25678e..363a380f60d9e2 100644 --- a/test/parallel/test-dtls-verify-identity.mjs +++ b/test/parallel/test-dtls-verify-identity.mjs @@ -65,9 +65,6 @@ const ca = readKey('ca1-cert.pem').toString(); // The client aborts after verifying the certificate, so the server-side // handshake never completes. session.onhandshake = mustNotCall(); - // The server session's `opened` is never awaited here; ensure it can't - // surface as an unhandled rejection if the stalled handshake later errors. - session.opened.catch(() => {}); }), { cert, key, port: 0, host: '127.0.0.1' }); const { port } = server.address; From 9953529abc78fbf4304b47daeee2ed79b13ff977 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 6 Jul 2026 07:26:27 -0700 Subject: [PATCH 11/12] test: dtls test improvements Signed-off-by: James M Snell Assisted-by: Claude/Opus --- src/dtls/dtls.h | 16 +-- src/dtls/dtls_endpoint.cc | 36 ++++++ src/dtls/dtls_endpoint.h | 5 + test/parallel/test-dtls-accessors.mjs | 104 ++++++++++++++++++ test/parallel/test-dtls-alpn.mjs | 30 +++++ test/parallel/test-dtls-ciphers.mjs | 79 +++++++++++++ test/parallel/test-dtls-client-cert.mjs | 73 ++++++++++++ test/parallel/test-dtls-errors.mjs | 48 ++++++++ test/parallel/test-dtls-keylog.mjs | 49 +++++++++ test/parallel/test-dtls-mtu.mjs | 68 ++++++++++++ test/parallel/test-dtls-options.mjs | 14 +++ .../test-dtls-reject-unauthorized.mjs | 63 +++++++++++ test/parallel/test-dtls-robustness.mjs | 49 +++++++++ test/parallel/test-dtls-send.mjs | 70 ++++++++++++ 14 files changed, 697 insertions(+), 7 deletions(-) create mode 100644 test/parallel/test-dtls-accessors.mjs create mode 100644 test/parallel/test-dtls-ciphers.mjs create mode 100644 test/parallel/test-dtls-client-cert.mjs create mode 100644 test/parallel/test-dtls-errors.mjs create mode 100644 test/parallel/test-dtls-keylog.mjs create mode 100644 test/parallel/test-dtls-mtu.mjs create mode 100644 test/parallel/test-dtls-reject-unauthorized.mjs create mode 100644 test/parallel/test-dtls-robustness.mjs create mode 100644 test/parallel/test-dtls-send.mjs diff --git a/src/dtls/dtls.h b/src/dtls/dtls.h index 6f1737347433f2..1faed3910e21ad 100644 --- a/src/dtls/dtls.h +++ b/src/dtls/dtls.h @@ -58,16 +58,18 @@ void RecordTimestampStat(Stats* stats) { V(MESSAGES_SENT, messages_sent) \ V(RETRANSMIT_COUNT, retransmit_count) -// State indices shared between C++ and JS via AliasedStruct/DataView. +// State "indices" shared between C++ and JS via AliasedStruct/DataView. These +// are BYTE OFFSETS into the state struct, not sequential indices: session_count +// is a uint32, so `busy`, which follows it, sits at byte offset 8. The +// static_asserts in dtls_endpoint.cc pin these to the actual struct layout. // Keep in sync with lib/internal/dtls/state.js. enum DTLSEndpointStateIndex { IDX_ENDPOINT_STATE_BOUND = 0, - IDX_ENDPOINT_STATE_LISTENING, - IDX_ENDPOINT_STATE_CLOSING, - IDX_ENDPOINT_STATE_DESTROYED, - IDX_ENDPOINT_STATE_SESSION_COUNT, - IDX_ENDPOINT_STATE_BUSY, - IDX_ENDPOINT_STATE_COUNT + IDX_ENDPOINT_STATE_LISTENING = 1, + IDX_ENDPOINT_STATE_CLOSING = 2, + IDX_ENDPOINT_STATE_DESTROYED = 3, + IDX_ENDPOINT_STATE_SESSION_COUNT = 4, + IDX_ENDPOINT_STATE_BUSY = 8, }; enum DTLSSessionStateIndex { diff --git a/src/dtls/dtls_endpoint.cc b/src/dtls/dtls_endpoint.cc index 366c779ed1374e..b33c6b1f03cacc 100644 --- a/src/dtls/dtls_endpoint.cc +++ b/src/dtls/dtls_endpoint.cc @@ -19,6 +19,7 @@ #include #include +#include #include namespace node { @@ -45,6 +46,22 @@ struct SendReq { }; } // namespace +// The endpoint state "indices" are byte offsets into DTLSEndpointStateData, +// accessed from JS via a DataView. Pin them to the actual struct layout so a +// mismatch (as once existed for `busy`, which follows a uint32) can't recur. +static_assert(IDX_ENDPOINT_STATE_BOUND == + offsetof(DTLSEndpointStateData, bound)); +static_assert(IDX_ENDPOINT_STATE_LISTENING == + offsetof(DTLSEndpointStateData, listening)); +static_assert(IDX_ENDPOINT_STATE_CLOSING == + offsetof(DTLSEndpointStateData, closing)); +static_assert(IDX_ENDPOINT_STATE_DESTROYED == + offsetof(DTLSEndpointStateData, destroyed)); +static_assert(IDX_ENDPOINT_STATE_SESSION_COUNT == + offsetof(DTLSEndpointStateData, session_count)); +static_assert(IDX_ENDPOINT_STATE_BUSY == + offsetof(DTLSEndpointStateData, busy)); + DTLSEndpoint::DTLSEndpoint(Environment* env, Local wrap) : HandleWrap(env, wrap, @@ -268,6 +285,11 @@ void DTLSEndpoint::CloseGracefully() { server_context_.reset(); + // Keep ourselves alive until OnClose() runs, so a garbage collection while + // uv_close() is in flight cannot collect the wrapper before the close is + // reported. Released in OnClose(). + self_ref_ = BaseObjectPtr(this); + // HandleWrap::Close() calls uv_close and manages the lifecycle. HandleWrap::Close(); } @@ -293,6 +315,9 @@ void DTLSEndpoint::Destroy() { state_->listening = 0; } + // Keep ourselves alive until OnClose() (see CloseGracefully()). + self_ref_ = BaseObjectPtr(this); + HandleWrap::Close(); } @@ -402,6 +427,17 @@ void DTLSEndpoint::OnClose() { state_->destroyed = 1; DTLS_STAT_RECORD_TIMESTAMP(DTLSEndpointStats, destroyed_at); + // Release the strong self-reference taken when the close was initiated. + // HandleWrap::OnClose still holds its own reference for the duration of this + // call, so this does not free us here. + self_ref_.reset(); + + // A close initiated outside CloseGracefully()/Destroy() (e.g. an endpoint + // abandoned mid-construction and closed at environment teardown) takes no + // self-reference, so its wrapper may already be collected. There is no JS + // side to notify in that case; skip it rather than touch a freed wrapper. + if (persistent().IsEmpty()) return; + Local cb = GetCallback(DTLS_CB_ENDPOINT_CLOSE); if (!cb.IsEmpty()) { Local argv[] = {}; diff --git a/src/dtls/dtls_endpoint.h b/src/dtls/dtls_endpoint.h index d0a1e5652dcb1d..bed49a7db0e0ee 100644 --- a/src/dtls/dtls_endpoint.h +++ b/src/dtls/dtls_endpoint.h @@ -156,6 +156,11 @@ class DTLSEndpoint final : public HandleWrap { AliasedStruct state_; AliasedStruct stats_; + // Strong self-reference held while a graceful close/destroy is in flight, so + // the wrapper is not garbage-collected before OnClose() runs and reports the + // close. Cleared in OnClose(). + BaseObjectPtr self_ref_; + bool listening_ = false; uint32_t mtu_ = 1200; // Conservative default MTU for data payload }; diff --git a/test/parallel/test-dtls-accessors.mjs b/test/parallel/test-dtls-accessors.mjs new file mode 100644 index 00000000000000..41c7ca7b2382a1 --- /dev/null +++ b/test/parallel/test-dtls-accessors.mjs @@ -0,0 +1,104 @@ +// Flags: --experimental-dtls --no-warnings + +// Test: DTLSEndpoint/DTLSSession state fields and callback accessors reflect +// what is set and the connection lifecycle. + +import { + hasCrypto, skip, mustCall, mustNotCall, mustCallAtLeast, +} from '../common/index.mjs'; +import assert from 'node:assert'; +import * as fixtures from '../common/fixtures.mjs'; + +const { strictEqual } = assert; +const { readKey } = fixtures; + +if (!hasCrypto) { + skip('missing crypto'); +} + +if (!process.features.dtls) { + skip('DTLS is not enabled'); +} + +const { listen, connect } = await import('node:dtls'); + +const cert = readKey('agent1-cert.pem').toString(); +const key = readKey('agent1-key.pem').toString(); +const ca = readKey('ca1-cert.pem').toString(); + +const gotServerSession = Promise.withResolvers(); + +const server = listen(mustCall((session) => { + gotServerSession.resolve(session); +}), { cert, key, port: 0, host: '127.0.0.1' }); + +// --- Endpoint state after listen(): bound and listening. --- +const es = server.state; +strictEqual(es.bound, true); +strictEqual(es.listening, true); +strictEqual(es.closing, false); +strictEqual(es.destroyed, false); +strictEqual(es.sessionCount, 0); + +// The busy property is settable via the endpoint and reflected in the state view. +strictEqual(server.busy, false); +strictEqual(es.busy, false); +server.busy = true; +strictEqual(server.busy, true); +strictEqual(es.busy, true); +server.busy = false; +strictEqual(es.busy, false); + +// --- Endpoint onerror accessor. --- +strictEqual(server.onerror, undefined); +const onEndpointError = mustNotCall(); +server.onerror = onEndpointError; +strictEqual(server.onerror, onEndpointError); + +const client = connect('127.0.0.1', server.address.port, { + ca: [ca], + rejectUnauthorized: false, +}); + +// --- Session state during the handshake. --- +const cs = client.state; +strictEqual(cs.handshaking, true); +strictEqual(cs.open, false); +strictEqual(cs.closing, false); +strictEqual(cs.destroyed, false); +strictEqual(cs.hasMessageListener, false); + +// --- Session callback accessors: unset, then set. --- +strictEqual(client.onmessage, undefined); +strictEqual(client.onerror, undefined); +strictEqual(client.onhandshake, undefined); +strictEqual(client.onkeylog, undefined); +// A connect() session owns its internal endpoint. +strictEqual(client.ownsEndpoint, true); + +client.onmessage = mustNotCall(); +strictEqual(typeof client.onmessage, 'function'); +// Attaching a message listener flips the shared flag. +strictEqual(cs.hasMessageListener, true); + +client.onerror = mustNotCall(); +strictEqual(typeof client.onerror, 'function'); + +client.onhandshake = mustCall(); +strictEqual(typeof client.onhandshake, 'function'); + +client.onkeylog = mustCallAtLeast(); +strictEqual(typeof client.onkeylog, 'function'); + +await client.opened; + +// --- Session state after the handshake completes. --- +strictEqual(cs.handshaking, false); +strictEqual(cs.open, true); + +const serverSession = await gotServerSession.promise; +await serverSession.opened; +strictEqual(es.sessionCount, 1); + +await client.close(); +await server.close(); diff --git a/test/parallel/test-dtls-alpn.mjs b/test/parallel/test-dtls-alpn.mjs index da9a6cbb56b136..dd454fdf8be50a 100644 --- a/test/parallel/test-dtls-alpn.mjs +++ b/test/parallel/test-dtls-alpn.mjs @@ -53,3 +53,33 @@ await serverAlpnChecked.promise; await session.close(); await endpoint.close(); + +// ALPN with no protocol in common: the handshake still completes and neither +// peer reports a negotiated protocol. +{ + const gotServerSession = Promise.withResolvers(); + + const server = listen(mustCall((s) => gotServerSession.resolve(s)), { + cert: serverCert.toString(), + key: serverKey.toString(), + port: 0, + host: '127.0.0.1', + alpn: ['bar'], + }); + + const client = connect('127.0.0.1', server.address.port, { + ca: [ca.toString()], + rejectUnauthorized: false, + alpn: ['foo'], + }); + + await client.opened; + const serverSession = await gotServerSession.promise; + await serverSession.opened; + + strictEqual(client.alpnProtocol, undefined); + strictEqual(serverSession.alpnProtocol, undefined); + + await client.close(); + await server.close(); +} diff --git a/test/parallel/test-dtls-ciphers.mjs b/test/parallel/test-dtls-ciphers.mjs new file mode 100644 index 00000000000000..e58a1443c9c72d --- /dev/null +++ b/test/parallel/test-dtls-ciphers.mjs @@ -0,0 +1,79 @@ +// Flags: --experimental-dtls --no-warnings + +// Test: cipher and ECDH-curve selection and validation. + +import { hasCrypto, skip, mustCall, mustNotCall } from '../common/index.mjs'; +import assert from 'node:assert'; +import * as fixtures from '../common/fixtures.mjs'; + +const { strictEqual, throws } = assert; +const { readKey } = fixtures; + +if (!hasCrypto) { + skip('missing crypto'); +} + +if (!process.features.dtls) { + skip('DTLS is not enabled'); +} + +const { listen, connect } = await import('node:dtls'); + +const cert = readKey('agent1-cert.pem').toString(); +const key = readKey('agent1-key.pem').toString(); +const ca = readKey('ca1-cert.pem').toString(); + +const CIPHER = 'ECDHE-RSA-AES128-GCM-SHA256'; + +// Case 1: a specific cipher is negotiated and reported on both peers. +{ + const gotServerSession = Promise.withResolvers(); + + const server = listen(mustCall((session) => { + gotServerSession.resolve(session); + }), { cert, key, port: 0, host: '127.0.0.1', ciphers: CIPHER }); + + const client = connect('127.0.0.1', server.address.port, { + ca: [ca], + rejectUnauthorized: false, + ciphers: CIPHER, + }); + + await client.opened; + const serverSession = await gotServerSession.promise; + await serverSession.opened; + + strictEqual(client.cipher.name, CIPHER); + strictEqual(serverSession.cipher.name, CIPHER); + + await client.close(); + await server.close(); +} + +// Case 2: an invalid cipher list is rejected. +throws(() => listen(mustNotCall(), { + cert, key, port: 0, host: '127.0.0.1', ciphers: 'THIS-IS-NOT-A-CIPHER', +}), { code: 'ERR_CRYPTO_OPERATION_FAILED' }); + +// Case 3: a valid ECDH curve completes a handshake. +{ + const server = listen(mustCall(), { + cert, key, port: 0, host: '127.0.0.1', ecdhCurve: 'P-256', + }); + + const client = connect('127.0.0.1', server.address.port, { + ca: [ca], + rejectUnauthorized: false, + ecdhCurve: 'P-256', + }); + + await client.opened; + + await client.close(); + await server.close(); +} + +// Case 4: an invalid ECDH curve is rejected. +throws(() => listen(mustNotCall(), { + cert, key, port: 0, host: '127.0.0.1', ecdhCurve: 'not-a-curve', +}), { code: 'ERR_CRYPTO_OPERATION_FAILED' }); diff --git a/test/parallel/test-dtls-client-cert.mjs b/test/parallel/test-dtls-client-cert.mjs new file mode 100644 index 00000000000000..2505eb6ff7c2f2 --- /dev/null +++ b/test/parallel/test-dtls-client-cert.mjs @@ -0,0 +1,73 @@ +// Flags: --experimental-dtls --no-warnings + +// Test: DTLS mutual authentication. A server with requestCert verifies the +// client's certificate; a client that presents no certificate is rejected. + +import { hasCrypto, skip, mustCall } from '../common/index.mjs'; +import assert from 'node:assert'; +import * as fixtures from '../common/fixtures.mjs'; + +const { ok, rejects } = assert; +const { readKey } = fixtures; + +if (!hasCrypto) { + skip('missing crypto'); +} + +if (!process.features.dtls) { + skip('DTLS is not enabled'); +} + +const { listen, connect } = await import('node:dtls'); + +const cert = readKey('agent1-cert.pem').toString(); +const key = readKey('agent1-key.pem').toString(); +const ca = readKey('ca1-cert.pem').toString(); + +// Case 1: the client presents a certificate the server can verify. +{ + const gotServerSession = Promise.withResolvers(); + + const server = listen(mustCall((session) => { + gotServerSession.resolve(session); + }), { + cert, key, ca: [ca], requestCert: true, port: 0, host: '127.0.0.1', + }); + + const client = connect('127.0.0.1', server.address.port, { + cert, key, ca: [ca], rejectUnauthorized: false, + }); + + await client.opened; + const serverSession = await gotServerSession.promise; + await serverSession.opened; + + // The server received and verified the client's certificate. + const clientCert = serverSession.peerCertificate; + ok(clientCert); + ok(clientCert.includes('BEGIN CERTIFICATE')); + + await client.close(); + await server.close(); +} + +// Case 2: the client presents no certificate; the server requires one and +// rejects the handshake. +{ + const server = listen(mustCall(), { + cert, key, ca: [ca], requestCert: true, port: 0, host: '127.0.0.1', + }); + + const client = connect('127.0.0.1', server.address.port, { + ca: [ca], rejectUnauthorized: false, + }); + + // The exact alert text varies, so assert only that the handshake is rejected. + await rejects(client.opened, { + message: /handshake failure/ + }); + + // The failed client tears down its internally-owned endpoint. + await client.endpoint.closed; + await server.close(); +} diff --git a/test/parallel/test-dtls-errors.mjs b/test/parallel/test-dtls-errors.mjs new file mode 100644 index 00000000000000..e17497fcac6cb0 --- /dev/null +++ b/test/parallel/test-dtls-errors.mjs @@ -0,0 +1,48 @@ +// Flags: --experimental-dtls --no-warnings + +// Test: DTLS error handling for invalid certificate/key material and endpoint +// state. + +import { hasCrypto, skip, mustNotCall } from '../common/index.mjs'; +import assert from 'node:assert'; +import * as fixtures from '../common/fixtures.mjs'; + +const { throws } = assert; +const { readKey } = fixtures; + +if (!hasCrypto) { + skip('missing crypto'); +} + +if (!process.features.dtls) { + skip('DTLS is not enabled'); +} + +const { listen, DTLSEndpoint } = await import('node:dtls'); + +const cert = readKey('agent1-cert.pem').toString(); +const key = readKey('agent1-key.pem').toString(); +const mismatchedKey = readKey('agent2-key.pem').toString(); + +// A malformed certificate PEM is rejected. +throws(() => listen(mustNotCall(), { + cert: 'not a certificate', key, port: 0, +}), { code: 'ERR_CRYPTO_OPERATION_FAILED' }); + +// A malformed private key PEM is rejected. +throws(() => listen(mustNotCall(), { + cert, key: 'not a key', port: 0, +}), { code: 'ERR_CRYPTO_OPERATION_FAILED' }); + +// A private key that does not match the certificate is rejected. +throws(() => listen(mustNotCall(), { + cert, key: mismatchedKey, port: 0, +}), { code: 'ERR_CRYPTO_OPERATION_FAILED' }); + +// Binding the same endpoint twice fails. +{ + const endpoint = new DTLSEndpoint(); + endpoint.bind('127.0.0.1', 0); + throws(() => endpoint.bind('127.0.0.1', 0), { code: 'ERR_INVALID_STATE' }); + await endpoint.close(); +} diff --git a/test/parallel/test-dtls-keylog.mjs b/test/parallel/test-dtls-keylog.mjs new file mode 100644 index 00000000000000..49d70a4566f5ad --- /dev/null +++ b/test/parallel/test-dtls-keylog.mjs @@ -0,0 +1,49 @@ +// Flags: --experimental-dtls --no-warnings + +// Test: the onkeylog callback delivers NSS-format key material during the +// handshake (useful for decrypting captures in Wireshark). + +import { hasCrypto, skip, mustCall, mustCallAtLeast } from '../common/index.mjs'; +import assert from 'node:assert'; +import * as fixtures from '../common/fixtures.mjs'; + +const { strictEqual, match } = assert; +const { readKey } = fixtures; + +if (!hasCrypto) { + skip('missing crypto'); +} + +if (!process.features.dtls) { + skip('DTLS is not enabled'); +} + +const { listen, connect } = await import('node:dtls'); + +const cert = readKey('agent1-cert.pem').toString(); +const key = readKey('agent1-key.pem').toString(); +const ca = readKey('ca1-cert.pem').toString(); + +const gotKeylog = Promise.withResolvers(); + +const server = listen(mustCall(), { + cert, key, port: 0, host: '127.0.0.1', +}); + +const client = connect('127.0.0.1', server.address.port, { + ca: [ca], + rejectUnauthorized: false, +}); + +// A keylog line is "