dtls: large update to the dtls implementation - #65511
Open
jasnell wants to merge 81 commits into
Open
Conversation
jasnell
added
net
Collaborator
|
Review requested:
|
nodejs-github-bot
added
lib / src
Member
Author
Review guideMost of the following was AI agent generated, verified by me. 80 commits is a lot to read end to end, so here is a route through them. The commits are ordered by dependency, not by theme, so the groups below jump around the history; each commit appears in exactly one group. Every commit builds and passes the suite on its own, so anything here can be checked out and run in isolation. Numbers are positions in the branch, oldest first. Datagram framing and the BIO layerOpenSSL's DTLS record layer assumes one BIO read yields exactly one datagram. The module used byte-stream BIOs, so that assumption held only by accident. Start here: several later commits depend on both BIOs being datagram BIOs.
Denial of service and resource boundsWork an unauthenticated peer could make the server do, and limits on what an authenticated one can hold or retain.
Peer address identityThe session table is keyed on the peer address, so what counts as the same peer matters. Note that 910a8cf changes shared code and 6f27438 reverts that part -- read them together; the net effect on node_sockaddr is additive only.
Certificate verification and peer identityThere was no way to see why a handshake was rejected, and two paths where verification silently did not happen.
ALPNProtocol list encoding and what happens when nothing is shared.
New features: secure contexts, SNI, PSK, resumptionThe largest group and the bulk of the new API surface. Read in order -- the later commits fix interactions the earlier ones created.
Exception safety and OpenSSL error reportingCallbacks that run inside SSL_do_handshake() cannot report anything to JavaScript from where they stand, and OpenSSL's error queue is shared process-wide.
Session and endpoint lifecyclePromises that never settled, and ordering between a session reaching JavaScript and its handshake running.
Public surface and argument validationOptions that reached a CHECK in the binding (a caller typo aborting the process), and internals that were reachable as public API.
Sockets and addressingWhich local socket an endpoint binds, and the UDP options it exposes.
Allocation gatingTwo paths that built V8 values whether or not anything was listening.
DocumentationCorrections and additions. 7988ed8 is structural (heading levels only, anchors preserved); the rest are content.
HousekeepingTest fixes and mechanical cleanups.
Worth a closer lookBehaviour changes that could affect an existing user of the experimental module:
Security-relevant:
Notes for the reviewer
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #65511 +/- ##
==========================================
+ Coverage 90.05% 90.10% +0.04%
==========================================
Files 751 751
Lines 254420 255257 +837
Branches 47975 47982 +7
==========================================
+ Hits 229121 229989 +868
+ Misses 16483 16445 -38
- Partials 8816 8823 +7
🚀 New features to boost your workflow:
|
aduh95
reviewed
Aug 24, 2026
jasnell
force-pushed
the
jasnell/dtls-improvements
branch
from
August 25, 2026 02:09
3da1292 to
272930e
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Member
Author
|
linux ci with Smoke test is good. Only failures are known flakeys. |
This comment was marked as resolved.
This comment was marked as resolved.
The test connects to the IP literal 127.0.0.1 with rejectUnauthorized defaulting to true and no servername, so the peer identity is verified against that IP. agent1-cert.pem is CN = agent1 with no subjectAltName, so verification fails with X509_V_ERR_IP_ADDRESS_MISMATCH before the default CA set is exercised at all. Pass servername so the identity is matched against the certificate CN, keeping verification enabled while testing what the file is named for. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
The error queue is per-thread and shared with every other OpenSSL consumer in the process. DTLS spends most of its time handling unauthenticated input, so failures are routine: rejected handshakes, and DTLSv1_listen() choking on garbage datagrams. None of those entries were discarded. ERR_get_error() in Cycle() and ClearOut() popped only the first entry, and nothing cleared the queue after a failed DTLSv1_listen(), SSL_write() or SSL_shutdown(). The residue was picked up by whatever crypto operation ran next and reported as its error: after 32 junk datagrams, crypto.createPrivateKey() on malformed PEM reported "record too small" with the real DECODER error demoted into opensslErrorStack. Add MarkPopErrorOnReturn to the entry points that drive OpenSSL, so each discards whatever it queued on the way out. Route error rendering through a helper that falls back to a description of the SSL error code when the queue is empty, instead of "error:00000000:lib(0)::reason(0)". Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
OpenSSL emits one BIO_write per DTLS record, each fragmented to fit SSL_set_mtu(). enc_out_ was a byte-stream BIO, so those boundaries were lost and EncOut() drained an entire handshake flight into one datagram, defeating the MTU setting. With an agent1 chain and mtu 512, the server flight went out as 60, 2490, 266 bytes -- the 2490 being five correctly sized records concatenated into one datagram that requires IP fragmentation, which NATs and middleboxes routinely drop. SSL_OP_NO_QUERY_MTU also disables OpenSSL's black-hole recovery, so such a handshake retransmits at the same broken size until it gives up. Use BIO_s_dgram_mem() for enc_out_, which returns exactly one datagram per BIO_read. It reports "empty" as a retry and grows on write, so it needs no BIO_set_mem_eof_return(). EncOut() now sends one record per iteration instead of one flight. Loopback has a 64 KiB MTU so no existing test could see this; the new one measures datagram sizes through a relay. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
A zero length datagram is legal UDP, costs the sender nothing and can never carry a DTLS record, but OnRecv() forwarded it to ProcessDatagram() like any other. With no matching session it reached AcceptConnection(), which spent an SSL_new(), two BIO_new()s, a DTLSv1_listen() and an SSL_free() establishing there was nothing there -- before any address validation, so the source is spoofable. It also blocks moving enc_in_ to a datagram BIO: a zero length BIO_write enqueues an empty datagram, and the subsequent BIO_read returns 0, which the record layer reads as EOF rather than "try again". Reject len == 0 in ProcessDatagram(), covering both the session and accept paths. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
OpenSSL's DTLS record layer assumes a BIO read returns exactly one datagram, and clamps a read to the bytes remaining in one. enc_in_ was a byte-stream BIO, where that count means "bytes remaining in the queue", so a record header declaring a length longer than its own datagram could consume bytes belonging to the next. Not reachable today: Receive() runs Cycle() after every BIO_write, and Cycle() drains, so enc_in_ never holds more than one datagram and the clamp lands on the boundary by coincidence. The invariant is an emergent property of when Cycle() runs rather than a property of the BIO, so anything that lets two datagrams queue turns it into a silent framing desync. Use BIO_s_dgram_mem(), matching enc_out_. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
SSL_get_verify_result() was never called or exposed, so there was no way to inspect the verification result or apply an authorization policy: an application could only get an opaque "certificate verify failed". Add session.authorized and session.authorizationError, the latter carrying the short X509 code such as 'CERT_HAS_EXPIRED'. Route the lookup through ncrypto's verifyPeerCertificate() rather than SSL_get_verify_result() directly, because the latter reports X509_V_OK when the peer sent no certificate at all. ncrypto reports that as absent, while still allowing for PSK and resumption, which is mapped to UNABLE_TO_GET_ISSUER_CERT to match node:tls. These are meaningful when rejectUnauthorized is false: OpenSSL verifies the chain under SSL_VERIFY_NONE and simply does not abort. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
createContext() tested rejectUnauthorized first and requestCert only as
an else-if, so { requestCert: true, rejectUnauthorized: false } set
SSL_VERIFY_NONE. No CertificateRequest was sent and the server saw no
peer certificate even when the client offered a valid trusted one. That
combination is the node:tls idiom for "ask for a certificate and let the
application decide", so code ported from node:tls lost client
authentication silently. rejectUnauthorized also wrongly implied
requestCert.
Follow node:tls and drive the server off requestCert first:
requestCert: false -> SSL_VERIFY_NONE
requestCert, rejectUnauthorized -> PEER | FAIL_IF_NO_PEER_CERT
requestCert, !rejectUnauthorized -> PEER
and the client off rejectUnauthorized alone.
The permissive verify callback is installed in exactly one case, the
server that asked for a certificate but disabled rejection, because it is
the only combination where OpenSSL would otherwise abort a handshake the
application wants to judge.
Also validate requestCert, and CHECK the arguments to setVerifyMode
instead of Int32Value(...).FromJust() on an unchecked value.
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
SSL_CTX_set_keylog_callback() was called unconditionally, so every handshake's CLIENT_RANDOM and master secret were formatted and copied into V8 strings whether or not the application had set onkeylog -- the JS side only gated delivery. Once a secret is a JS string it is reachable from heap snapshots, core dumps and the inspector for as long as the string lives. node:tls installs its keylog callback only when a listener is attached. Match that: add a has_keylog_listener flag to the shared session state, set it from the onkeylog setter, and return from SSLKeylogCallback before touching V8 when it is clear. Registration also moves to DTLSContext, since keylog is a per-SSL_CTX setting that was being rewritten once per session. While adding a state field, pin the session state offsets with static_asserts the way the endpoint state already does. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
Every datagram arriving at a listening endpoint that did not match an existing session went straight to AcceptConnection(), which spent an SSL_new(), two BIO_new()s, a DTLSv1_listen() and an SSL_free() before concluding it was not a ClientHello. None of that is gated on anything the sender had to prove, so a spoofed-source flood bought that work at the cost of a UDP send. Screen the datagram first: handshake content type, DTLS version major, a record length that fits the datagram, and a client_hello handshake type. Deliberately structural -- parsing the ClientHello is OpenSSL's job, and getting it wrong would turn away real clients. Under a 30000 datagram flood the server absorbed all of them at 2.8us each, against roughly half of them at 6.1us each before. Add endpointStats.serverRejectedCount so this traffic is visible. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
session_count was written in five places and read in none: there was no limit on how many sessions a listening endpoint would hold. Each owns an SSL, two BIOs and a retransmit timer, so a peer willing to complete cookie exchanges could grow the table until the process ran out of memory. Cookie exchange proves a peer can receive at its claimed address, so this is not spoofable, but it does not bound what that peer may do. Add maxSessions (default 10000) and maxSessionsPerHost (default 1000), checked in AcceptConnection before anything is allocated. The per-host cap is the one that matters: without it a single peer can take the entire table. It is keyed on IP only, so a peer cannot evade it by varying source port, and erases entries at zero so it tracks live peers. A refused peer gets silence rather than an alert: it has not completed cookie exchange, so replying would make this an amplification vector. A real client retransmits and is admitted once there is room. Refusals are counted by endpointStats.serverRefusedCount. Either cap can be set to 0 to disable it. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
EncOut() and the HelloVerifyRequest path each declared a 64 KiB stack buffer to receive a datagram that is normally around 1200 bytes. EncOut() is reached from Cycle(), which can re-enter, so those frames can nest. Both were large enough to force a page-probing prologue. Now that both BIOs are datagram BIOs, BIO_pending() reports the size of the next datagram exactly, so the read can be sized to it. Use MaybeStackBuffer, which keeps the common case on the stack. Sizing from BIO_pending() also removes the possibility of a short read truncating a record, which is what a datagram BIO does when the buffer is too small. EncOut()'s frame drops from over 4 KiB with probing to 1560 bytes without. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
Neither layer checked it. The JS wrapper passed the argument straight through, and the binding did Int32Value(...).FromJust() and handed the result to std::vector<uint8_t>(length), where a negative value became a huge size_t. Three ordinary-looking arguments terminated the process: session.exportKeyingMaterial(-1, label) -> core dump session.exportKeyingMaterial(4294967295, label) -> core dump session.exportKeyingMaterial(1e12, label) -> core dump Validate in JS the way node:tls does, and CHECK in the binding rather than coercing, since by then a bad value is our bug and not the caller's. Also bound the length at 65536. RFC 5705 sets no limit and node:tls does not impose one, but node:tls allocates through a BackingStore, which fails gracefully, whereas std::vector aborts. 65536 is three orders of magnitude above the largest defined exporter, DTLS-SRTP's 60 bytes. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
SocketAddress::Hash covers family, port and address. SocketAddress::Map paired it with operator==, which memcmps the whole sockaddr and so also compares sin_zero, sin6_flowinfo and sin6_scope_id. Keys that hash the same could compare unequal, putting one peer in two entries of a single bucket. The DTLS session table is the only user of Hash, and it is keyed on the peer address, so a peer whose padding differed between two datagrams would get a second session rather than matching its existing one. The kernel zeroes sin_zero on receive, so this is latent today; it stops being latent as soon as addresses reach the table from anywhere other than a recvmsg. Add SocketAddress::Equal alongside the existing IpHash/IpEqual pair and use it in the Map alias. Equal covers scope_id and Hash now folds it in: two link-local peers reachable as the same address on different interfaces are genuinely different peers. flowinfo is a QoS label and stays out of both. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
ComputeCookie() HMACed the raw sockaddr bytes. For IPv4 that spans
sin_zero, and for IPv6 sin6_flowinfo: padding the kernel is not obliged
to zero, and a QoS label that can legitimately differ between two
datagrams from one host. Either changes the cookie for an unchanged peer,
which fails the handshake, since the peer echoes the cookie it was given
and the server recomputes a different one.
Serialise {family, port, address, scope id} instead. scope id stays in
because it identifies a link-local peer. The cookie format is
process-local and lives for one time window, so changing it costs
nothing.
Also value-initialise current_cookie_peer_, so an unset value reports an
unknown family and ComputeCookie() fails closed rather than deriving a
cookie from stale bytes.
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
The server cache mode was SSL_SESS_CACHE_SERVER | NO_AUTO_CLEAR. That pairing is only coherent alongside NO_INTERNAL, the way node:tls uses it, where there is no internal cache for the auto-clear to walk. With the internal cache enabled it meant nothing ever evicted anything: every accepted session stayed, with its master secret, for the 7200 second default timeout and beyond. Over 700 sequential handshakes from a non-ticket client, all 700 were retained. Only reachable for peers that do not offer session tickets, which excludes node's own client but not much of the CoAP/IoT population. Dropping NO_AUTO_CLEAR alone does nothing: the periodic flush only removes expired entries and only on a 255-session boundary. Use NO_INTERNAL, matching node:tls and the client branch below it. This gives up server-side session-id resumption for non-ticket clients, which nothing exercised and no API could drive; ticket resumption is stateless and unaffected. Also set a session id context, defaulting the way node:tls does from a hash of process.argv, and expose it as the sessionIdContext option. OpenSSL will not resume a session whose id context differs from the accepting SSL's, which keeps a session issued under one configuration from being resumed under another. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
Pairing SocketAddress::Hash with a matching Equal was done by changing SocketAddress::Map, which is shared. SocketAddressLRU is built on Map, and QUIC's address validation is built on SocketAddressLRU, so that changed which peers QUIC treats as the same peer: two addresses differing only in sin6_flowinfo or sin_zero padding stopped being distinct entries. There was no defect there to justify it. operator== is stricter than Hash, not looser: equal keys still hash equal, so unordered_map's invariant held. The mismatch only matters for a table where two datagrams from one peer must find one entry, which is a session table, which is DTLS. Map goes back to what it was, byte for byte. Equal stays, and a PeerMap alias pairs the two for callers that want peer identity rather than sockaddr identity. DTLS's session table is the only one. The cctest keeps both behaviours pinned. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
DTLS_CB_SESSION_TICKET was in the callback enum and its name was in the list SetCallbacks requires, so every embedder had to supply an onSessionTicket function or SetCallbacks would throw. Nothing emitted it, and the one implementation was an empty body. Resumption landed without needing it: OpenSSL issues and verifies tickets with its own keys, so there is no point at which JavaScript has to be asked anything. sni_contexts_size() goes too. It has no callers. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
The constructor tested endpoint for null before reading the handshake timeout from it, then read the MTU from it unconditionally eight lines later. Only one of those can be right. It is the second: Create() and CreateFromSSL() are the only callers and both pass the endpoint that owns the session. Checked once at the top, so the invariant is stated where it holds rather than implied in one place and contradicted in another. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
EncOut() called SendTo() and discarded what it returned, so a datagram
the kernel refused was not sent and nothing said so.
Retransmission does not help: EMSGSIZE means the MTU is wrong for the
path and ENETUNREACH means there is no path, so every retry fails the
same way and the handshake goes quiet until the deadline. The report was
then "DTLS handshake timeout", which is both late and the wrong cause:
connect('255.255.255.255', 4433, { handshakeTimeout: 1500 })
before: 1500ms DTLS handshake timeout
after: 999ms permission denied
Kept rather than emitted from the send loop, which runs with the SSL
mid-flight, and reported from the same point at the end of Cycle() as an
exception from a callback. The first is kept and the loop stops: the rest
of the flight is going the same way.
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
FormatSSLError took the bottom of the OpenSSL error queue with ERR_get_error(). Cycle() holds a MarkPopErrorOnReturn, but that records a position to unwind to on the way out; it does not empty the queue on the way in. Anything already queued sits below the mark and comes out first, so the message could describe an error from somewhere else entirely. Peek from the top instead. An SSL_ERROR_SSL means this operation queued at least one entry, and the newest is certainly one of ours. Peeking also leaves the queue for the mark to unwind, rather than removing one entry from underneath it. This changes no message any test produces, since every failure they cover queues exactly one error. It is a fix to the contract: the previous comment claimed the oldest entry is the most specific, which is true within one operation and is precisely what ERR_get_error() could not guarantee it was reading. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
session.ownsEndpoint was a public getter and setter over an internal lifecycle flag. It decides whether closing a session should close its endpoint too, which is true only for the endpoint connect() builds to carry one session. Set on a server session, it makes that session's close tear down the listener: serverSession.ownsEndpoint = true; await serverSession.close(); server.address // undefined -- the endpoint is gone connect(HOST, port, ...) // times out; every other session went too Symbol-keyed, like the other internal plumbing on these classes. It was undocumented, so nothing describes it as available. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
Every connect() example passed a host name. Addresses are not resolved, so all of them throw ERR_INVALID_ARG_VALUE, "Invalid remote address". They pass a literal now. Where the name was the point of the example -- one context verified against several identities, and a resumed session -- it moves to servername, which is what selects the name a certificate is checked against. The mtu and handshakeTimeout entries under listen() were spliced together: mtu had no description, its text had been absorbed into the middle of handshakeTimeout's, and "**Default:** `1200`" appeared twice, once under the wrong option. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
Both were compared rather than coerced -- `=== true` and `!== false` --
so a value that was not a boolean took the branch it did not look like:
createSecureContext({ isServer: 'yes' }) // a client context
connect(..., { rejectUnauthorized: 0 }) // verification stays on
Neither failed open: a client context is refused by listen(), and 0
meaning "verify" is the safe reading. But both decide something
security-relevant from a value the caller plainly meant the other way,
and said nothing.
Checked with validateBoolean where they are read, as requestCert already
was. Comparing rather than coercing stays.
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
isConnected is documented as false once a stats object is no longer tracking anything. Nothing ever set it. kFinishClose was defined on both stats classes and imported by the module, and no caller invoked it, so the flag was true for the lifetime of the object. Reading them after a close was safe -- the AliasedStruct's backing store is a shared_ptr the ArrayBuffer keeps alive -- so there was no dangling pointer, only numbers that had stopped moving with nothing saying so. Called now on every path a session or endpoint ends by: the peer closing, close(), destroy(), and the endpoint's close callback. That snapshots the values, so the last state stays readable, and flips isConnected. node:quic, which these stats were modelled on, calls it from its close paths. The symbol and both implementations came across; the calls did not. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
A libuv failure was rethrown as ERR_INVALID_STATE carrying only uv_strerror()'s text, which dropped both the errno and the syscall: code=ERR_INVALID_STATE errno=undefined syscall=undefined code=EADDRINUSE errno=-98 syscall=bind The second is what net and dgram give for the same condition, and err.code === 'EADDRINUSE' is how this is normally handled. Against DTLS that could never pass, and ERR_INVALID_STATE is also what the module throws for a closed session, so the two were indistinguishable. Thrown with ThrowUVException instead. Rebinding a bound endpoint now reports EALREADY. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
…class rejectUnauthorized: false was documented as not verifying the certificate. It verifies it and continues, reporting authorized false with an authorizationError, which is what makes those two properties worth reading and what the prose further down the page already said. session.closed was "Resolves when the session is fully closed". It rejects when the session was destroyed with an error, or when its endpoint was. session.authorizationError was written as an escaped literal, which renders the brackets instead of linking and silences the missing- reference warning rather than answering it. The reference is defined now. Callback properties and session[Symbol.asyncDispose]() were under "Class: DTLSSession.Stats", which documents the stats object. They are members of DTLSSession and are now inside it. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
session.servername, session.endpoint, session.destroyed and endpoint.destroyed are on the prototypes and none appeared in the documentation. session.endpoint is worth stating plainly: on a server session it is the listening endpoint itself, shared with every other session on it, so a session handler holds the whole listener. connect() accepts handshakeTimeout and only listen() listed it, so the option looked server-only. servername is undefined when the client sends no name, which the entry now says rather than leaving "the SNI name" to imply otherwise. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
Four calls whose failure was ignored. SSL_CTX_set_min/max_proto_version pin the context to DTLS 1.2. Refusing DTLS 1.0 is the point of setting them -- RFC 8996 deprecates it and it has no AEAD suites -- so an OpenSSL that rejected the call left a context whose floor was the version being excluded. Checked together, since either failing has that effect. BIO_write and BIO_ADDR_new in the cookie-exchange path are allocation failures. An unwritten BIO would have put DTLSv1_listen() to work on an empty buffer, and a null BIO_ADDR is not something it accepts. The datagram is dropped and the peer retransmits. uv_udp_recv_start on the connect path was ignored where Listen() checks it and unwinds. A failure there left a session in the table that no datagram could reach, reported only by its handshake timing out a minute later. It now unwinds the same way and throws the libuv error. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
Returning 0 from the PSK callback tells OpenSSL there is no PSK, and it was what every failure took. A callback returning the wrong shape, a non-string identity, a key that was not a view, or a value too long for the buffer all reached the caller identically: error:0A0000DF:SSL routines::psk identity not found which names nothing the caller did and is also what a genuinely absent PSK produces. Each now reports what was wrong with what it gave back, through the same pending-error path an exception from the callback already used. An empty identity or key still returns 0 silently: that one really is "no PSK". Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
send() took a Buffer or a string and refused a Uint8Array, which is the obvious thing to send, while exportKeyingMaterial() on the same object accepted one. Bare ArrayBuffers stay refused, as they are there too. The gate was Buffer.isBuffer() in JavaScript. The binding's check was Buffer::HasInstance(), which is defined as IsArrayBufferView() and so had been accepting every view all along. It is spelled IsArrayBufferView() now, and reads the bytes through ArrayBufferViewContents, so what it takes is stated rather than inherited from what a Buffer happens to be. A view sends the bytes it covers and not the buffer behind it: a subarray, a DataView at an offset, and an Int16Array all arrive as the bytes they span. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
Five options only a server can act on were handled four different ways when a client named one: sni threw, sessionIdContext was ignored, ticketKeys was applied to a client that has no tickets to issue, and requestCert was validated and then ignored. All refused now, by one rule checked before any of them is read. A client naming one has misunderstood the option, and the difference between "ignored" and "applied" was not something a caller could see. sni's own check goes away in favour of the shared one. pskIdentityHint names which key a client should pick. Given without psk there was no key to name, so it was dropped and the handshake failed for want of a PSK without mentioning the option that had been set. Each option is still accepted by a server context, so the rule is about which side may use it. ticketKeys and sni keep their own validation. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
unwrapSession folded the Buffer check in with the prefix and length checks, so all four failures reported ERR_INVALID_ARG_VALUE. Passing a string got the code that means the type was right and the contents were wrong. Split out. A Buffer that is not one of ours still reports ERR_INVALID_ARG_VALUE, which is what it is: the right type, contents that cannot be resumed. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
The binding says "Session is closed" where JavaScript says "Session is destroyed" for what looks like the same situation. The first is unreachable: JavaScript drops the handle on close and on destroy, and send() refuses a null handle before the binding is reached. That holds for a peer-initiated close too, where the close callback clears the handle before control returns to user code. The guard stays, because being unreachable today is not a reason to write into a closed SSL if that changes. The comment records why its wording is not being brought into line with a message it will never appear beside. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
Bind() set UV_UDP_IPV6ONLY for every IPv6 address, unconditionally. An
endpoint on :: therefore served IPv6 only and an IPv4 peer could not
reach it, with nothing to say so and no way to ask for anything else:
listen(..., { host: '::' })
connect('127.0.0.1', port) // handshake timeout
node:dgram and node:quic both bind dual stack by default. DTLS does now
too, and ipv6Only: true selects the old behaviour.
A dual-stack socket reports IPv4 peers with mapped addresses,
::ffff:127.0.0.1 rather than 127.0.0.1, so maxSessionsPerHost and
anything else keyed on the peer address sees them in that form.
The plumbing is a setSocketOptions() binding method read by Bind().
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
An endpoint took whatever socket the system gave it. There was no way to spread a server over several processes, and no way to give it room for bursts the default buffers drop. reusePort sets SO_REUSEPORT, where the kernel spreads datagrams between everyone bound to the port. Not SO_REUSEADDR, which libuv also offers and node:dgram exposes: on Linux that lets the last binder take the port from a running server. Without reusePort the port stays exclusive. udpReceiveBufferSize, udpSendBufferSize and udpTTL are applied once the bind succeeds, since there is no socket to set them on before that. Not naming one leaves the system default rather than substituting a number of ours. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
Both blocks enumerate the options they take and neither mentioned the five added for the UDP socket. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
Mentioning C++ in the dtls.md doc exposes implementation detail Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
bind() moved to a symbol key so an endpoint cannot be rebound from outside. test-permission-net-dtls.mjs still called endpoint.bind() and had been failing with: TypeError: endpoint.bind is not a function which assert.throws() reported as the wrong error rather than as a missing method, so it read like a permission-check failure. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
The entry read "live and updated data flows through the endpoint". The session equivalent reads "updated as data flows". Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode
Signed-off-by: James M Snell <jasnell@gmail.com>
Signed-off-by: James M Snell <jasnell@gmail.com>
ReportPSKError took a const char* and passed it to ToV8Value(), which already has a std::string_view overload. Every call site hands it a literal, so the length is known rather than recovered with strlen(). Signed-off-by: James M Snell <jasnell@gmail.com>
Use timers/promise setTimeout and fix a hang in a test Signed-off-by: James M Snell <jasnell@gmail.com>
jasnell
force-pushed
the
jasnell/dtls-improvements
branch
from
August 26, 2026 03:56
272930e to
34f252d
Compare
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
node:dtlslanded with the transport working but with many gaps. This addresses those, and fills in the API surface.This is a large PR but the commits are structured logically and sequentially. I chose to keep multiple PRs rather than squashing due to the size. Each has it's own description. I recommend stepping through and reviewing commit-by-commit.
A separate review guide comment will be included.