Skip to content

tls: report a first handshake that the peer ends with close_notify (Duplex, named pipe, proxy tunnel) - #44516

Closed
robobun wants to merge 1 commit into
mainfrom
robobun/85a11241/tls-duplex-close-notify-handshake
Closed

robobun wants to merge 1 commit into
mainfrom
robobun/85a11241/tls-duplex-close-notify-handshake

Conversation

@robobun

@robobun robobun commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator

Draft until its self-review returns.

Problem

  • A TLS peer answers the ClientHello with a close_notify alert and keeps the stream open. tls.connect({ socket: duplex }) emits no event and stays writable. Proxied fetch and wss wait for their timers.
  • Cause: the SSL_ERROR_ZERO_RETURN branch of SSLWrapper::update_handshake_state (src/uws/lib.rs:1061) runs no callback for a first handshake.

Fix

  • The branch reports the handshake as failed with the ECONNRESET report of openssl.c, then closes.
  • Every owner already handles that pair: the fatal-error branch sends it. node:tls emits end, error ECONNRESET, close, as Node does. Proxied fetch and wss fail with EPROTO and 1015, as direct connections do.
  • Verified: 15 new cases in test/js/node/tls/node-tls-duplex-end-verify.test.ts and four other files. 14 fail on main. Node v26.3.0 passes the 9 client cases.

Background

  • Bun has two TLS engines. openssl.c drives sockets with a file descriptor. SSLWrapper drives TLS over a Duplex, a named pipe or a proxy tunnel, and reports through on_handshake and on_close.
  • close_notify is the alert that ends a peer's write side. BoringSSL returns SSL_ERROR_ZERO_RETURN for it.
  • Considered a close with no report: proxied fetch would report ConnectionClosed and a server wrap nothing.

Downsides

Notes

Repro (Node v26.3.0 prints end, error ECONNRESET, close; Bun on main prints nothing and the socket stays open)

const tls = require("tls"), { Duplex } = require("stream");
let first = true;
const transport = new Duplex({
  read() {},
  write(chunk, encoding, callback) {
    callback();
    // The peer answers the ClientHello with a close_notify alert and keeps the stream open.
    if (first) setImmediate(() => transport.push(Buffer.from([0x15, 0x03, 0x03, 0x00, 0x02, 0x01, 0x00])));
    first = false;
  },
});
const socket = tls.connect({ socket: transport, servername: "a.test" });
const events = [];
socket.on("end", () => events.push("end"));
socket.on("error", err => events.push("error " + err.code));
socket.on("close", () => console.log(events.concat("close").join(", ")));

What changes, per entry point (debug builds, main against this change, peer sends the alert and keeps the stream open)

Entry point main This change Node v26.3.0
tls.connect({ socket: duplex }), rejectUnauthorized true or false no event end, error ECONNRESET, close(true) the same
The alert inside the transport's write(), readable before tls.connect(), or in two reads no event as above the same
end() on the client, then the alert no event as above the same
The alert in place of the server's last TLS 1.2 flight no event as above the same
TLS in TLS (socket is a TLSSocket) no event as above the same
fetch through a CONNECT proxy pending until its timer rejects with EPROTO n/a
WebSocket (wss) through a CONNECT proxy pending until its timer TLS handshake failed, close 1015 n/a
new TLSSocket(duplex, { isServer: true }) no event error ECONNRESET "socket hang up" ERR_SSL_UNEXPECTED_MESSAGE before a ClientHello, end after one
tls.Server and http2 secure server, injected connection no event tlsClientError ECONNRESET "socket hang up" tlsClientError ERR_SSL_UNEXPECTED_MESSAGE

The server rows cannot equal Node. BoringSSL reads a close_notify ahead of the ClientHello as the close of the peer. OpenSSL refuses it as an unexpected message.

Measurements (release builds of base 4b02e1031d and of this change, unless a line says debug)

  • per-record fast path: 31 instructions before, 31 after, 0 differing, in each of 2 compiled copies of update_handshake_state (llvm-objdump -l on bun-profile). The HTTPClient and UpgradedDuplex instances are one body after identical code folding. traffic_pass (80 instructions) and handle_traffic (33) do not differ. The new branch ran 0 times in 1,000 echoed records and in one proxied fetch (gdb breakpoint, debug build), and 1 time in the repro.
  • release binary: +256 bytes .text, +0 bytes .rodata (llvm-size). Stripped bun: 80,848,456 bytes before and after. update_handshake_state +62 bytes and trigger_handshake_callback +38 bytes per copy. SSLWrapper Inner<T>: 224 -> 224 bytes.
  • bundled node:net: +0 bytes. http2 upgrade module: +213 bytes.
  • per peer-closed handshake: 1 on_handshake and 1 on_close at each of 5 entry points (main: 0 and 0). Scoped debug logs, 3 runs each.
  • allocations inside TLSSocket::on_handshake (gdb, debug build, libc malloc): 2 for this report, the stored code and reason. A clean handshake: 162 on main, 162 with this change.
  • node:tls client orders over a Duplex equal to Node: 12 of 17 (main: 3). Not equal: three orders with a write queued before the handshake, https.request over a supplied Duplex (it queues its headers), and an untrusted chain with rejectUnauthorized (the existing inline reject).

A write queued before the handshake

Order over a Duplex main This change Node
alert, stream kept open no event write cb ERR_SOCKET_CLOSED, error ERR_SOCKET_CLOSED, close(true) end, error ECONNRESET, write cb ECANCELED, close(true)
alert, then FIN write cb ERR_SOCKET_CLOSED, end, error ECONNRESET, close(true) write cb ERR_SOCKET_CLOSED, error ERR_SOCKET_CLOSED, close(true) as above
FIN only write cb ERR_SOCKET_CLOSED, end, error ECONNRESET, close(true) unchanged as above

The ERR_SOCKET_CLOSED form is what a TCP socket gives on main for a FIN with a queued write. It comes from the close handler in net.ts, which fails the queued write before 'end' reaches the listener that reports ECONNRESET. Issue #43381 tracks it. #43392 and #43250 change that handler, so this change leaves it alone.

Not in this change

Open pull requests that touch the same function

Test runs

  • New cases: node-tls-duplex-end-verify.test.ts (9, node:test, also run on Node), node-tls-connect.test.ts (3, server wraps), node-http2-upgrade.test.mts (1), proxy.test.ts (1), websocket-proxy.test.ts (1), node-tls-namedpipes.test.ts (2, Windows only).
  • On a debug ASAN build of this change at base faac63e6d4: the five Linux files pass in full (40, 142, 16, 98 and 44 tests), test/js/node/tls/ passes (499 tests), and 244 of 245 vendored test-tls-* and test-https-* scripts exit 0. The other one needs bun test and fails the same way on main. The rebase onto the current main changed none of the four source files.
  • bun run rust:check-all x86_64-pc-windows-msvc aarch64-apple-darwin passes.

…uplex, named pipe, proxy tunnel)

SSLWrapper::update_handshake_state took the SSL_ERROR_ZERO_RETURN branch
for a first handshake and ran no callback. The owner got no handshake
report and no close, so the connection stayed open for as long as the
peer held the stream: tls.connect({ socket: duplex }) emitted nothing,
and fetch and WebSocket through a CONNECT proxy waited for their timers.

The branch now reports the handshake as failed, with the ECONNRESET
report that openssl.c uses for a peer that leaves mid-handshake, and
then closes. node:tls over a Duplex emits 'end', 'error' ECONNRESET and
'close', as Node does. The http2 upgrade of an injected socket maps the
report to "socket hang up", like tls.Server.
@github-actions github-actions Bot added the claude label Oct 3, 2026
@robobun

robobun commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 11:59 PM PT - Oct 2nd, 2026

✅ @robobun, your commit daebdb13def226338d6881a703f91802e620eab3 passed in Build #123175! 🎉


🧪   To try this PR locally:

bunx bun-pr 44516

That installs a local version of the PR into your bun-44516 executable, so you can run:

bun-44516 --bun

@robobun

robobun commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator Author

Status: reproduced with the script in the Notes of the description. A client runs TLS over a Duplex. The peer answers the ClientHello with a close_notify alert and keeps the stream open. Main emits no event. This change emits end, error ECONNRESET, close, as Node v26.3.0 does.

@Jarred-Sumner

Copy link
Copy Markdown
Collaborator

Superseded by #44618, which consolidates the open TLS pull requests. This fix and its tests are in there, either as written, rewritten smaller, or merged with the other PRs that patched the same cause (see the "By area" list in that PR). Closing in favor of it.

Jarred-Sumner added a commit that referenced this pull request Oct 6, 2026
…#44516, #44517)

A peer that answers the ClientHello with close_notify and keeps the stream open:
- SSLWrapper (Duplex, named pipe, proxy tunnel) ran no callback at all for
  SSL_ERROR_ZERO_RETURN in a first handshake, so nothing was ever reported;
- openssl.c drives the handshake from SSL_read, which turns
  SSL_do_handshake() == 0 into SSL_R_SSL_HANDSHAKE_FAILURE, so the fd engine
  reported ERR_SSL_SSL_HANDSHAKE_FAILURE where Node reports ECONNRESET.

Both now give the report of a FIN mid-handshake, (0, ECONNRESET), then close.
The fetch tunnel maps that report like a direct connection does.
Jarred-Sumner added a commit that referenced this pull request Oct 7, 2026
…#44516, #44517)

A peer that answers the ClientHello with close_notify and keeps the stream open:
- SSLWrapper (Duplex, named pipe, proxy tunnel) ran no callback at all for
  SSL_ERROR_ZERO_RETURN in a first handshake, so nothing was ever reported;
- openssl.c drives the handshake from SSL_read, which turns
  SSL_do_handshake() == 0 into SSL_R_SSL_HANDSHAKE_FAILURE, so the fd engine
  reported ERR_SSL_SSL_HANDSHAKE_FAILURE where Node reports ECONNRESET.

Both now give the report of a FIN mid-handshake, (0, ECONNRESET), then close.
The fetch tunnel maps that report like a direct connection does.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants