diff --git a/packages/bun-usockets/src/crypto/openssl.c b/packages/bun-usockets/src/crypto/openssl.c index 260ebfd190ca..b80a4fc22f86 100644 --- a/packages/bun-usockets/src/crypto/openssl.c +++ b/packages/bun-usockets/src/crypto/openssl.c @@ -1962,6 +1962,8 @@ static int ssl_renegotiate(struct us_socket_t *s) { us_reneg_policy(s_ssl(s), &limit, &window); struct us_ssl_rare_t *st = us_ssl_rare_ensure(s_ssl(s)); s->ssl_handshake_state = HANDSHAKE_RENEGOTIATION_PENDING; + /* The check of the handshake before is not the check of this one. */ + s->ssl_peer_chain_checked = 0; /* Wall-clock time can step backwards (NTP, manual adjustment); the * unsigned subtraction below would underflow and reset the window every * time. Only treat the window as elapsed when time has moved forward. */ diff --git a/src/js/node/net.ts b/src/js/node/net.ts index d04376457b4f..e5ce6b19cb34 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -1324,6 +1324,15 @@ function onSocketTLSError(err) { } } +// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L1246-L1257 +function onSocketTLSClose(hadError) { + if (hadError) return; + if (!this._controlReleased && !this[kerrorEmitted]) { + this[kerrorEmitted] = true; + this.server?.emit("tlsClientError", new ConnResetException("socket hang up"), this); + } +} + function onHandshakeTimeout() { this._emitTLSError($ERR_TLS_HANDSHAKE_TIMEOUT()); } @@ -1346,6 +1355,7 @@ function initAcceptedTLSSocket(server, socket) { socket[kerrorEmitted] = false; socket.on("_tlsError", onSocketTLSError); socket.on("error", onSocketTLSError); + socket.on("close", onSocketTLSClose); const handshakeTimeout = server._handshakeTimeout; if (!(handshakeTimeout > 0)) return; // The deadline surfaces as the socket's own 'timeout' event with diff --git a/test/js/bun/net/socket.test.ts b/test/js/bun/net/socket.test.ts index 44a5482f23a6..2e733a744a9a 100644 --- a/test/js/bun/net/socket.test.ts +++ b/test/js/bun/net/socket.test.ts @@ -19,7 +19,7 @@ import { once } from "node:events"; import net from "node:net"; import { join } from "node:path"; import { Duplex } from "node:stream"; -import { createSecureContext, connect as tlsConnect } from "node:tls"; +import { createSecureContext, connect as tlsConnect, createServer as tlsCreateServer } from "node:tls"; describe.concurrent("socket", () => { it("should throw when a socket from a file descriptor has a bad file descriptor", async () => { const open = jest.fn(); @@ -3950,7 +3950,7 @@ Reo= it("a rejecting client closes when its handshake fails with no error", async () => { const events: string[] = []; - const closed = Promise.withResolvers(); + const reported = Promise.withResolvers(); // Never answers, and keeps the connection open after the client's FIN. const peer = net.createServer({ allowHalfOpen: true }, socket => socket.on("error", () => {})); await once(peer.listen(0, "127.0.0.1"), "listening"); @@ -3966,19 +3966,21 @@ Reo= }, handshake(_socket, success, authorizationError) { events.push(`handshake success=${success} error=${authorizationError?.message ?? null}`); + reported.resolve(); }, data() {}, close() { events.push("close"); - closed.resolve(); }, error() {}, connectError(_socket, err) { - closed.reject(err); + reported.reject(err); }, }, }); - await closed.promise; + await reported.promise; + // The close comes with the report, not when the peer closes. + await pendingReadsDone(); expect(events).toEqual(["handshake success=false error=null", "close"]); } finally { peer.close(); @@ -4033,6 +4035,8 @@ Reo= callbackCode: string | null; getterCode: string | null; }>(); + // Before the handshake reports, the SSL answers: it has no certificate to show. + const beforeTheReport: (string | null)[] = []; // Never answers, and keeps the connection open after the client's FIN. const peer = net.createServer({ allowHalfOpen: true }, socket => socket.on("error", () => {})); await once(peer.listen(0, "127.0.0.1"), "listening"); @@ -4044,7 +4048,10 @@ Reo= socket: { // Before the ClientHello, so the handshake never starts. open(socket) { + const code = () => (socket.getAuthorizationError() as NodeJS.ErrnoException | null)?.code ?? null; + beforeTheReport.push(code()); socket.shutdown(); + beforeTheReport.push(code()); }, handshake(socket, success, authorizationError) { handshake.resolve({ @@ -4062,12 +4069,85 @@ Reo= }, }); expect(await handshake.promise).toEqual({ successArg: false, callbackCode: null, getterCode: null }); + expect(beforeTheReport).toEqual(["UNABLE_TO_GET_ISSUER_CERT", "UNABLE_TO_GET_ISSUER_CERT"]); // This client does not reject, and its peer did not close: the socket is still open. expect(client.getAuthorizationError()).toBeNull(); } finally { peer.close(); } }); + + // A TLS 1.2 client checks the chain of the server when it reads the + // server's first flight. Its handshake is not complete then. + it("a handshake that fails after shutdown() reports the check of the chain that it read", async () => { + const server = tlsCreateServer({ key: ROGUE_KEY, cert: ROGUE_CRT, maxVersion: "TLSv1.2" }, socket => + socket.on("error", () => {}), + ); + server.on("tlsClientError", () => {}); + await once(server.listen(0, "127.0.0.1"), "listening"); + // Holds the server's flight until the client's FIN arrived, and keeps the connection open. + const flightForwarded = Promise.withResolvers(); + const relayed: net.Socket[] = []; + const relay = net.createServer({ allowHalfOpen: true }, downstream => { + const upstream = net.connect({ + port: (server.address() as net.AddressInfo).port, + host: "127.0.0.1", + allowHalfOpen: true, + }); + relayed.push(downstream, upstream); + const held: Buffer[] = []; + let sawClientFin = false; + const forward = (data: Buffer) => downstream.write(data, () => flightForwarded.resolve()); + downstream.on("data", data => upstream.write(data)); + downstream.on("end", () => { + sawClientFin = true; + for (const data of held.splice(0)) forward(data); + }); + upstream.on("data", data => (sawClientFin ? forward(data) : held.push(data))); + downstream.on("error", () => {}); + upstream.on("error", () => {}); + }); + await once(relay.listen(0, "127.0.0.1"), "listening"); + const events: string[] = []; + const closed = Promise.withResolvers(); + try { + const client = await Bun.connect({ + hostname: "127.0.0.1", + port: (relay.address() as net.AddressInfo).port, + tls: { ca: CA_CRT, rejectUnauthorized: false }, + socket: { + // The ClientHello leaves when `open` returns. + open(socket) { + setImmediate(() => socket.shutdown()); + }, + handshake(_socket, success, authorizationError) { + const code = (authorizationError as NodeJS.ErrnoException | null)?.code ?? null; + events.push(`handshake success=${success} code=${code}`); + }, + data() {}, + close() { + events.push("close"); + closed.resolve(); + }, + error() {}, + connectError(_socket, err) { + closed.reject(err); + }, + }, + }); + await flightForwarded.promise; + await pendingReadsDone(); + // The server waits for the client's second flight, which cannot leave. + expect(events).toEqual([]); + client.terminate(); + await closed.promise; + expect(events).toEqual(["handshake success=false code=UNABLE_TO_VERIFY_LEAF_SIGNATURE", "close"]); + } finally { + for (const socket of relayed) socket.destroy(); + relay.close(); + server.close(); + } + }); }); }); diff --git a/test/js/node/tls/node-tls-duplex-end-verify.test.ts b/test/js/node/tls/node-tls-duplex-end-verify.test.ts index 35671bf4d96c..08cd0056f98a 100644 --- a/test/js/node/tls/node-tls-duplex-end-verify.test.ts +++ b/test/js/node/tls/node-tls-duplex-end-verify.test.ts @@ -183,10 +183,10 @@ async function behindProxy(server, wire, proxyOptions = {}, overDuplex = false) }; } -// The same shape on a plain TCP socket. The client calls end() when its last handshake flight left. The proxy holds -// the server's final flight until the client's FIN arrived, so the handshake completes on a socket that is already -// shut down. The proxy never forwards the FIN, so the server keeps writing. The server's certificate is not trusted, -// unless `trusted`. Returns the ordered events of the client. +// The same shape on a plain TCP socket. The client calls end() when its last handshake flight left. The proxy +// delivers what the server sends from then on after the client's FIN arrived, in `pieces` parts, so the handshake +// completes on a socket that is already shut down. The proxy never forwards the FIN, so the server keeps writing. +// The server's certificate is not trusted, unless `trusted`. Returns the ordered events of the client. async function endMidHandshakeOverTcp(maxVersion, rejectUnauthorized, { pieces = 1, trusted = false } = {}) { const events = []; const { promise, resolve } = Promise.withResolvers(); @@ -202,8 +202,8 @@ async function endMidHandshakeOverTcp(maxVersion, rejectUnauthorized, { pieces = (downstream, upstream) => { let sawChangeCipherSpec = false; let ending = false; - let clientEnded = false; - const held = []; + const clientEnded = Promise.withResolvers(); + let delivered = clientEnded.promise; eachRecord(downstream, record => { upstream.write(record); if (ending) return; @@ -215,14 +215,11 @@ async function endMidHandshakeOverTcp(maxVersion, rejectUnauthorized, { pieces = } sawChangeCipherSpec ||= record[0] === CHANGE_CIPHER_SPEC; }); - downstream.on("end", async () => { - await inPieces(part => downstream.write(part), Buffer.concat(held.splice(0)), pieces); - clientEnded = true; - for (const chunk of held.splice(0)) downstream.write(chunk); - }); + downstream.on("end", clientEnded.resolve); upstream.on("data", chunk => { - if (ending && !clientEnded) held.push(chunk); - else downstream.write(chunk); + if (!ending) return void downstream.write(chunk); + // The FIN can arrive before the server's answer does: each chunk waits for it, and for the chunk before. + delivered = delivered.then(() => inPieces(part => downstream.write(part), chunk, pieces)); }); }, { allowHalfOpen: true }, @@ -582,3 +579,65 @@ test("a bad record behind the client's Finished does not make a server accept an // Node reports the bad record. Its code depends on the cipher, so only the class of the error is fixed. assert.match(events[0], isBun ? /^tlsClientError DEPTH_ZERO_SELF_SIGNED_CERT$/ : /^tlsClientError ERR_SSL_/); }); + +// The peer can go away while the handshake of a socket that called end() still runs. The socket closes with no error, +// and the server reports the handshake that never finished. `half`: the peer sends the first half of its last flight +// before its FIN. +for (const maxVersion of ["TLSv1.2", "TLSv1.3"]) { + for (const half of [false, true]) { + test(`${maxVersion}: a server that end()s reports the handshake that its peer left ${half ? "in the middle of a flight" : "before its last flight"}`, async () => { + const events = []; + const refused = Promise.withResolvers(); + const serverClosed = Promise.withResolvers(); + let serverSocket; + const server = tls.createServer({ key, cert, maxVersion }, () => events.push("secureConnection")); + server.on("tlsClientError", err => { + events.push(`tlsClientError ${err.code}`); + refused.resolve(); + }); + server.on("connection", socket => { + serverSocket = socket; + socket.on("error", () => {}); + socket.on("close", serverClosed.resolve); + }); + const { port, close } = await behindProxy( + server, + (downstream, upstream) => { + let chunks = 0; + const held = []; + downstream.on("data", chunk => { + // The ClientHello goes through. The proxy holds the client's last flight, and the server calls end(). + if (++chunks === 1) return void upstream.write(chunk); + held.push(chunk); + if (held.length === 1) serverSocket.end(); + }); + upstream.on("data", chunk => downstream.write(chunk)); + upstream.on("end", async () => { + const flight = Buffer.concat(held.splice(0)); + if (half) { + upstream.write(flight.subarray(0, Math.ceil(flight.length / 2))); + await pendingReadsDone(); + } + upstream.end(); + }); + }, + { allowHalfOpen: true }, + ); + const client = tls.connect({ + port, + host: "127.0.0.1", + servername: "agent1", + rejectUnauthorized: false, + maxVersion, + }); + client.on("error", () => {}); + try { + await Promise.all([refused.promise, serverClosed.promise]); + assert.deepStrictEqual(events, ["tlsClientError ECONNRESET"]); + } finally { + client.destroy(); + close(); + } + }); + } +} diff --git a/test/js/node/tls/node-tls-server.test.ts b/test/js/node/tls/node-tls-server.test.ts index e9e6c3c22d9c..416e9f4c831f 100644 --- a/test/js/node/tls/node-tls-server.test.ts +++ b/test/js/node/tls/node-tls-server.test.ts @@ -1254,7 +1254,7 @@ it("a socket that end()s while an asynchronous SNICallback is pending reports tl const events: string[] = []; const sniCalled = Promise.withResolvers(); const sawServerFin = Promise.withResolvers(); - const closed = Promise.withResolvers(); + const refused = Promise.withResolvers(); let answerSni: (() => void) | undefined; let serverSocket: net.Socket | undefined; const server: Server = createServer({ @@ -1267,14 +1267,14 @@ it("a socket that end()s while an asynchronous SNICallback is pending reports tl }, }); server.on("secureConnection", () => events.push("secureConnection")); - server.on("tlsClientError", err => events.push(`tlsClientError ${(err as NodeJS.ErrnoException).code}`)); + server.on("tlsClientError", err => { + events.push(`tlsClientError ${(err as NodeJS.ErrnoException).code}`); + refused.resolve(); + }); server.on("connection", socket => { serverSocket = socket; socket.on("error", () => {}); - socket.on("close", hadError => { - events.push(`close hadError=${hadError}`); - closed.resolve(); - }); + socket.on("close", hadError => events.push(`close hadError=${hadError}`)); }); server.listen(0); await once(server, "listening"); @@ -1308,7 +1308,9 @@ it("a socket that end()s while an asynchronous SNICallback is pending reports tl await sawServerFin.promise; // The handshake fails inside this call: the socket is shut down. answerSni!(); - await closed.promise; + await refused.promise; + // 'close' follows in the check phase of the loop. + for (let turn = 0; turn < 10 && events.length < 2; turn++) await new Promise(resolve => setImmediate(resolve)); expect(events).toEqual(["tlsClientError ECONNRESET", "close hadError=true"]); } finally { client.destroy(); diff --git a/test/js/node/tls/renegotiation.test.ts b/test/js/node/tls/renegotiation.test.ts index 5ed458b2b553..48f62c879efa 100644 --- a/test/js/node/tls/renegotiation.test.ts +++ b/test/js/node/tls/renegotiation.test.ts @@ -3,7 +3,8 @@ import { afterAll, beforeAll, expect, it } from "bun:test"; import { readFileSync } from "fs"; import { bunEnv, bunExe, isIPv6, tls } from "harness"; import type { IncomingMessage } from "http"; -import { connect as netConnect } from "net"; +import { createServer as createNetServer, connect as netConnect, type AddressInfo, type Socket } from "net"; +import { once } from "node:events"; import { join } from "path"; import { Duplex } from "stream"; import { connect as tlsConnect } from "tls"; @@ -441,6 +442,100 @@ it.concurrent.each([false, true])( }, ); +// The request for a renegotiation can arrive after the client called end(). The client cannot answer it, and the +// certificate check of its first handshake is not the report of a second one. +it("a client that called end() emits 'secureConnect' once when the server then asks for a renegotiation", async () => { + await using server = Bun.spawn({ + cmd: [ + "node", + "-e", + ` + const tls = require("tls"); + let accepted; + const server = tls.createServer( + { + cert: process.env.SERVER_CERT, + key: process.env.SERVER_KEY, + minVersion: "TLSv1.2", + maxVersion: "TLSv1.2", + allowHalfOpen: true, + }, + socket => { + accepted = socket; + socket.on("error", () => {}); + socket.resume(); + }, + ); + // A line on stdin asks for the renegotiation. + process.stdin.on("data", () => accepted.renegotiate({ rejectUnauthorized: false }, () => {})); + server.listen(0, "127.0.0.1", () => console.log(server.address().port)); + `, + ], + stdout: "pipe", + stderr: "inherit", + stdin: "pipe", + env: { ...bunEnv, SERVER_CERT: tls.cert, SERVER_KEY: tls.key }, + }); + const reader = server.stdout.getReader(); + const decoder = new TextDecoder(); + let output = ""; + while (!output.includes("\n")) { + const { value, done } = await reader.read(); + if (done) throw new Error("the server exited before it printed its port"); + output += decoder.decode(value, { stream: true }); + } + reader.releaseLock(); + const port = Number(output.trim()); + + // Holds what the client sends after its handshake, so the server does not see the client's close_notify. + let holding = false; + const sawClientFin = Promise.withResolvers(); + const proxied: Socket[] = []; + const proxy = createNetServer({ allowHalfOpen: true }, downstream => { + const upstream = netConnect({ port, host: "127.0.0.1", allowHalfOpen: true }); + proxied.push(downstream, upstream); + downstream.on("data", chunk => { + if (!holding) upstream.write(chunk); + }); + downstream.on("end", () => sawClientFin.resolve()); + upstream.on("data", chunk => downstream.write(chunk)); + upstream.on("end", () => downstream.end()); + downstream.on("error", () => {}); + upstream.on("error", () => {}); + }); + await once(proxy.listen(0, "127.0.0.1"), "listening"); + + const events: string[] = []; + const closed = Promise.withResolvers(); + const client = tlsConnect({ + port: (proxy.address() as AddressInfo).port, + host: "127.0.0.1", + servername: "localhost", + rejectUnauthorized: false, + }); + client.on("secureConnect", () => { + events.push(`secureConnect authorized=${client.authorized} authError=${client.authorizationError}`); + if (holding) return; + holding = true; + client.end(); + }); + client.on("finish", () => events.push("finish")); + client.on("error", (err: NodeJS.ErrnoException) => events.push(`error ${err.code}`)); + client.on("close", () => closed.resolve()); + client.resume(); + try { + await sawClientFin.promise; + server.stdin.write("renegotiate\n"); + await server.stdin.flush(); + await closed.promise; + expect(events).toEqual(["secureConnect authorized=false authError=DEPTH_ZERO_SELF_SIGNED_CERT", "finish"]); + } finally { + client.destroy(); + for (const socket of proxied) socket.destroy(); + proxy.close(); + } +}); + it("should fail if renegotiation fails using tls module", async () => { const { promise, resolve, reject } = Promise.withResolvers();