diff --git a/packages/bun-usockets/src/crypto/openssl.c b/packages/bun-usockets/src/crypto/openssl.c index 529d81169e37..a25c59ad68d6 100644 --- a/packages/bun-usockets/src/crypto/openssl.c +++ b/packages/bun-usockets/src/crypto/openssl.c @@ -1796,6 +1796,20 @@ static long us_internal_verify_peer_certificate(const SSL *ssl, long def) { return err; } +/* The packed error that names a fatal SSL_read failure the way node reports + * it: the first SSL-library entry on the thread's queue (for a bad record + * BoringSSL queues the cipher's BAD_DECRYPT ahead of the TLS reason), else the + * oldest entry, else 0. Drains the queue up to the entry it returns. */ +uint32_t us_ssl_take_fatal_error(void) { + uint32_t oldest = ERR_peek_error(); + for (uint32_t queued; (queued = ERR_get_error()) != 0;) { + if (ERR_GET_LIB(queued) == ERR_LIB_SSL) { + return queued; + } + } + return oldest; +} + struct us_bun_verify_error_t us_ssl_socket_verify_error_from_ssl(SSL *ssl) { long x509_verify_error = us_internal_verify_peer_certificate(ssl, X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT); diff --git a/src/http/ProxyTunnel.rs b/src/http/ProxyTunnel.rs index abdb981d4541..75d2d04c75ad 100644 --- a/src/http/ProxyTunnel.rs +++ b/src/http/ProxyTunnel.rs @@ -610,6 +610,7 @@ impl ProxyTunnel { // opting out keeps its SSL off the parked queues entirely. on_session: None, on_keylog: None, + on_ssl_error: None, server_identity: Some(server_identity), ctx: this.as_erased_ptr().as_ptr(), }, diff --git a/src/http_jsc/websocket_client/WebSocketProxyTunnel.rs b/src/http_jsc/websocket_client/WebSocketProxyTunnel.rs index 38f1e22846a2..ae6866be52f2 100644 --- a/src/http_jsc/websocket_client/WebSocketProxyTunnel.rs +++ b/src/http_jsc/websocket_client/WebSocketProxyTunnel.rs @@ -188,6 +188,7 @@ impl WebSocketProxyTunnel { // SSL off the parked session/keylog queues entirely. on_session: None, on_keylog: None, + on_ssl_error: None, server_identity: Some(Self::server_identity), }, ) diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 4900f52f83e0..8acc965ebbf5 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -1240,7 +1240,8 @@ const ServerHandlers = { reportError(err); } }, - error(socket, error) { + // See SocketHandlers2.error for `tlsFatal`. + error(socket, error, tlsFatal?: boolean) { const data = this.data; if (!data) return; @@ -1269,6 +1270,9 @@ const ServerHandlers = { ) { // Ignore server's authorization errors data.destroy(); + } else if (tlsFatal && !callback) { + // The native close that follows ends the socket, so emit it the way Node does. + data._emitTLSError(error); } else { // Node emits through _emitTLSError and leaves the socket alive. Bun // still destroys here: its tls.Server completes the handshake for a @@ -1596,7 +1600,8 @@ const SocketHandlers2 = { const { self } = socket.data; onClientHandshake(self, socket, success, verifyError); }, - error(socket, error) { + // `tlsFatal`: a fatal TLS error on the established session. The native close follows it. + error(socket, error, tlsFatal?: boolean) { $debug("Bun.Socket error"); if (socket.data === undefined) return; const { self } = socket.data; @@ -1607,6 +1612,10 @@ const SocketHandlers2 = { if (callback) { self[kwriteCallback] = null; callback(error); + } else if (tlsFatal && self._secureEstablished) { + // No destroy, like https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L467-L498 + self._emitTLSError(error); + return; } if (!self.destroyed) process.nextTick(destroyNT, self, error); diff --git a/src/runtime/socket/UpgradedDuplex.rs b/src/runtime/socket/UpgradedDuplex.rs index a25d5b648497..6f4517bf0729 100644 --- a/src/runtime/socket/UpgradedDuplex.rs +++ b/src/runtime/socket/UpgradedDuplex.rs @@ -103,6 +103,8 @@ pub(crate) struct Handlers { pub(crate) on_handshake: fn(*mut (), bool, us_bun_verify_error_t), pub(crate) on_data: fn(*mut (), &[u8]), pub on_close: fn(*mut ()), + /// See `ssl_wrapper::Handlers::on_ssl_error`. + pub(crate) on_ssl_error: fn(*mut (), u32), pub(crate) on_end: fn(*mut ()), pub(crate) on_writable: fn(*mut ()), pub(crate) on_error: fn(*mut (), JSValue), @@ -211,6 +213,13 @@ impl UpgradedDuplex { } } + fn on_ssl_error(this: *mut Self, err: u32) { + bun_output::scoped_log!(UpgradedDuplex, "onSslError"); + // SAFETY: see handler note above. + let this = unsafe { &*this }; + (this.handlers.on_ssl_error)(this.handlers.ctx, err); + } + fn on_close(this: *mut Self) { bun_output::scoped_log!(UpgradedDuplex, "onClose"); // SAFETY: see handler note above. @@ -492,6 +501,7 @@ impl UpgradedDuplex { on_handshake: Self::on_handshake, on_data: Self::on_data, on_close: Self::on_close, + on_ssl_error: Some(Self::on_ssl_error), write: Self::internal_write, on_session: Some(Self::on_session), on_keylog: Some(Self::on_keylog), diff --git a/src/runtime/socket/WindowsNamedPipe.rs b/src/runtime/socket/WindowsNamedPipe.rs index 1e7239863e4a..851093295597 100644 --- a/src/runtime/socket/WindowsNamedPipe.rs +++ b/src/runtime/socket/WindowsNamedPipe.rs @@ -136,6 +136,8 @@ pub(crate) struct Handlers { pub(crate) on_handshake: fn(*mut c_void, bool, us_bun_verify_error_t), pub(crate) on_data: fn(*mut c_void, &[u8]), pub on_close: fn(*mut c_void), + /// See `ssl_wrapper::Handlers::on_ssl_error`. + pub(crate) on_ssl_error: fn(*mut c_void, u32), pub(crate) on_end: fn(*mut c_void), pub(crate) on_writable: fn(*mut c_void), pub(crate) on_error: fn(*mut c_void, bun_sys::Error), @@ -362,6 +364,11 @@ impl WindowsNamedPipe { (self.handlers.on_keylog)(self.handlers.ctx, line); } + fn on_ssl_error(&self, err: u32) { + bun_output::scoped_log!(WindowsNamedPipe, "onSslError"); + (self.handlers.on_ssl_error)(self.handlers.ctx, err); + } + // ── SSLWrapper trampolines ─────────────────────────────────────────────── // `ssl_wrapper::Handlers<*mut Self>` carries `fn(*mut Self, ..)` slots. // SAFETY (all): `this` is the `ctx` set in `wrapper_handlers`; the engine @@ -394,6 +401,10 @@ impl WindowsNamedPipe { // SAFETY: see block note above. unsafe { &*this }.on_keylog(d) } + fn ssl_on_ssl_error(this: *mut Self, err: u32) { + // SAFETY: see block note above. + unsafe { &*this }.on_ssl_error(err) + } fn ssl_on_close(this: *mut Self) { // SAFETY: see block note above. unsafe { &*this }.on_close() @@ -411,6 +422,7 @@ impl WindowsNamedPipe { on_handshake: Self::ssl_on_handshake, on_data: Self::ssl_on_data, on_close: Self::ssl_on_close, + on_ssl_error: Some(Self::ssl_on_ssl_error), write: Self::ssl_write, on_session: Some(Self::ssl_on_session), on_keylog: Some(Self::ssl_on_keylog), diff --git a/src/runtime/socket/WindowsNamedPipeContext.rs b/src/runtime/socket/WindowsNamedPipeContext.rs index a26276be6aa3..cc33f18dd9ad 100644 --- a/src/runtime/socket/WindowsNamedPipeContext.rs +++ b/src/runtime/socket/WindowsNamedPipeContext.rs @@ -232,6 +232,13 @@ impl WindowsNamedPipeContext { } } + fn on_ssl_error(this: *mut Self, err: u32) { + // SAFETY: see `on_open`. + if let SocketType::Tls(s) = unsafe { (*this).socket } { + crate::dispatch::fold(TLSSocket::on_ssl_error(s, err)); + } + } + fn on_handshake(this: *mut Self, success: bool, ssl_error: us_bun_verify_error_t) { // SAFETY: see `on_open`. let (socket, pipe) = unsafe { ((*this).socket, ptr::addr_of_mut!((*this).named_pipe)) }; @@ -393,6 +400,7 @@ impl WindowsNamedPipeContext { on_error: |p, e| Self::on_error(p.cast::(), &e), on_timeout: |p| Self::on_timeout(p.cast::()), on_close: |p| Self::on_close(p.cast::()), + on_ssl_error: |p, err| Self::on_ssl_error(p.cast::(), err), on_session: |p, d| Self::on_session(p.cast::(), d), on_keylog: |p, d| Self::on_keylog(p.cast::(), d), server_identity: |p, ssl| Self::server_identity(p.cast::(), ssl), diff --git a/src/runtime/socket/socket_body.rs b/src/runtime/socket/socket_body.rs index d0de384d037d..54f510b89ca1 100644 --- a/src/runtime/socket/socket_body.rs +++ b/src/runtime/socket/socket_body.rs @@ -2142,6 +2142,43 @@ impl NewSocket { Ok(()) } + /// A fatal TLS error before the close. The third argument tells node:net that no handler threw it. + pub(crate) fn on_ssl_error(this: bun_ptr::ThisPtr, err_code: u32) -> JsResult<()> { + jsc::mark_binding!(); + if !this.has_handlers() || this.flags.get().contains(Flags::FINALIZING) { + return Ok(()); + } + if this.socket.get().is_detached() { + return Ok(()); + } + let handlers = this.get_handlers(); + if handlers.vm.script_execution_status() != jsc::ScriptExecutionStatus::Running { + return Ok(()); + } + let callback = handlers.on_error(); + if callback.is_empty() { + return Ok(()); + } + let global = handlers.global_object; + if global.has_exception() { + return Err(jsc::JsError::Thrown); + } + let _scope = ScopeExit { + socket: this, + scope: Some(handlers.enter()), + }; + let this_value = this.get_this_value(&global); + let err_value = boringssl_err_to_js(&global, err_code); + global.bun_vm().event_loop_mut().run_callback( + bun_event_loop::ContextId::NONE, + callback, + &global, + this_value, + &[this_value, err_value, JSValue::TRUE], + ); + Ok(()) + } + /// Takes `ThisPtr` for the same re-entrancy reason as `on_writable`. pub(crate) fn on_close( this: bun_ptr::ThisPtr, @@ -4484,6 +4521,12 @@ impl DuplexUpgradeContext { } } + fn on_ssl_error(this: bun_ptr::ThisPtr, err: u32) { + if let Some(tls) = this.tls_this_ptr() { + crate::dispatch::fold(TLSSocket::on_ssl_error(tls, err)); + } + } + fn on_close(this: bun_ptr::ThisPtr) { let socket = this.duplex_socket(); if let Some(tls) = this.tls.replace(None) { @@ -4880,6 +4923,10 @@ pub(crate) fn js_upgrade_duplex_to_tls( DuplexUpgradeContext::on_close(bun_ptr::ThisPtr::new(c.cast())) }, // SAFETY: `c` is `ctx` below — the live `DuplexUpgradeContext` heap allocation. + on_ssl_error: |c: *mut (), err| { + DuplexUpgradeContext::on_ssl_error(bun_ptr::ThisPtr::new(c.cast()), err) + }, + // SAFETY: `c` is `ctx` below — the live `DuplexUpgradeContext` heap allocation. on_end: |c: *mut ()| DuplexUpgradeContext::on_end(bun_ptr::ThisPtr::new(c.cast())), // SAFETY: `c` is `ctx` below — the live `DuplexUpgradeContext` heap allocation. on_writable: |c: *mut ()| { diff --git a/src/uws/lib.rs b/src/uws/lib.rs index 31ee23f5b94f..d4631876dfac 100644 --- a/src/uws/lib.rs +++ b/src/uws/lib.rs @@ -406,6 +406,9 @@ pub mod ssl_wrapper { pub write: fn(T, &[u8]), pub on_data: fn(T, &[u8]), pub on_close: fn(T), + /// A fatal `SSL_read` failure (a packed BoringSSL error), reported just + /// before `on_close`. `None`: the owner only needs the close. + pub on_ssl_error: Option, /// A new resumable TLS session arrived (serialized SSL_SESSION bytes) /// - node's `'session'` event. `None` opts the SSL out of session /// parking entirely (fetch / WebSocket tunnels have no consumer). @@ -979,6 +982,16 @@ pub mod ssl_wrapper { (handlers.on_close)(handlers.ctx); } + fn trigger_ssl_error_callback(&self, err: u32) { + if self.flags.closed_notified() { + return; + } + let handlers = self.handlers.get(); + if let Some(on_ssl_error) = handlers.on_ssl_error { + on_ssl_error(handlers.ctx, err); + } + } + /// The SSL's X509 verdict. Shutdown state does not change it. fn verify_error(&self) -> us_bun_verify_error_t { let Some(ssl) = self.ssl.get() else { @@ -1128,6 +1141,8 @@ pub mod ssl_wrapper { // SAFETY: write-only view of the unfilled tail; SSL_read only stores into it. let available = unsafe { &mut buffer.as_bytes_mut()[read..] }; + // An entry another operation on this thread left must not be taken for this read's. + boring_sys::ERR_clear_error(); // SAFETY: ssl is a live SSL*; available is a valid mutable slice. let just_read = unsafe { boring_sys::SSL_read( @@ -1140,6 +1155,14 @@ pub mod ssl_wrapper { if just_read <= 0 { // SAFETY: ssl is still valid. let err = unsafe { boring_sys::SSL_get_error(ssl.as_ptr(), just_read) }; + let is_fatal = + err == boring_sys::SSL_ERROR_SSL || err == boring_sys::SSL_ERROR_SYSCALL; + // Take the error before the queue is cleared. + let fatal_error = if is_fatal { + us_ssl_take_fatal_error() + } else { + 0 + }; boring_sys::ERR_clear_error(); if err != boring_sys::SSL_ERROR_WANT_READ @@ -1186,8 +1209,7 @@ pub mod ssl_wrapper { self.flags.set_received_ssl_shutdown(true); self.handle_end_of_renegotiation(); } - if err == boring_sys::SSL_ERROR_SSL || err == boring_sys::SSL_ERROR_SYSCALL - { + if is_fatal { self.flags.set_fatal_error(true); } @@ -1201,6 +1223,14 @@ pub mod ssl_wrapper { return false; } } + if is_fatal { + // Send the alert BoringSSL sealed into the write BIO + // before the close frees it, so the peer gets its error. + self.handle_writing(buffer); + if self.ssl.get().is_none() || self.flags.closed_notified() { + return false; + } + } // A NewSessionTicket/keylog line that rode in ahead of the // peer's close_notify is still parked; deliver it before the // close tears the wrapper down (mirrors the C ZERO_RETURN path). @@ -1208,6 +1238,13 @@ pub mod ssl_wrapper { if self.ssl.get().is_none() || self.flags.closed_notified() { return false; } + if fatal_error != 0 { + // Like node's ClearOut: the error goes to the owner, then the close. + self.trigger_ssl_error_callback(fatal_error); + if self.ssl.get().is_none() || self.flags.closed_notified() { + return false; + } + } if err == boring_sys::SSL_ERROR_ZERO_RETURN { // 2-step shutdown, last: write_data fails once our close_notify is out. let _ = self.shutdown(false); @@ -1500,6 +1537,9 @@ pub mod ssl_wrapper { /// Implemented in uSockets C; reads /// `SSL_get_verify_result` and maps it onto the C `us_bun_verify_error_t`. fn us_ssl_socket_verify_error_from_ssl(ssl: *mut boring_sys::SSL) -> us_bun_verify_error_t; + /// openssl.c: the packed error that names a fatal `SSL_read` failure, taken off the thread's error queue (0 if none). + // safe: no args; reads the calling thread's own queue. + safe fn us_ssl_take_fatal_error() -> u32; fn SSL_SESSION_up_ref(session: *mut boring_sys::SSL_SESSION) -> c_int; /// openssl.c: 1 when the verify step of this handshake asked the owner for the server's name. fn us_ssl_identity_checked(ssl: *mut boring_sys::SSL) -> c_int; diff --git a/test/js/node/http2/node-http2-upgrade.test.mts b/test/js/node/http2/node-http2-upgrade.test.mts index 57a8b73d93bc..4c9bc767a58e 100644 --- a/test/js/node/http2/node-http2-upgrade.test.mts +++ b/test/js/node/http2/node-http2-upgrade.test.mts @@ -461,6 +461,55 @@ describe("HTTP/2 upgrade — server TLS options", () => { }); }); +describe("HTTP/2 upgrade — fatal TLS error after the handshake", () => { + test("a record that fails to decrypt reaches the server as sessionError", async () => { + const h2Server = http2.createSecureServer(TLS, (_req, res) => { + res.writeHead(200); + res.end("ok"); + }); + h2Server.on("error", () => {}); + // A clean session 'close' with no error before it is the bug. + const outcome = new Promise<{ event: string; code?: string }>(resolve => { + h2Server.on("sessionError", (err: NodeJS.ErrnoException) => resolve({ event: "sessionError", code: err.code })); + h2Server.on("session", session => session.on("close", () => resolve({ event: "close" }))); + }); + const netServer = net.createServer(socket => { + socket.on("error", () => {}); + h2Server.emit("connection", socket); + }); + // A plain proxy in front of the net.Server, to inject bytes toward it. + let toServer: net.Socket | undefined; + const proxy = net.createServer(c => { + toServer = net.connect((netServer.address() as net.AddressInfo).port, "127.0.0.1"); + c.pipe(toServer); + toServer.pipe(c); + c.on("error", () => {}); + toServer.on("error", () => {}); + }); + let client: http2.ClientHttp2Session | undefined; + try { + await once(netServer.listen(0, "127.0.0.1"), "listening"); + await once(proxy.listen(0, "127.0.0.1"), "listening"); + client = connectClient((proxy.address() as net.AddressInfo).port); + const first = await request(client, "GET", "/"); + assert.strictEqual(first.status, 200); + + // application_data, 32 bytes of ciphertext that cannot authenticate. + toServer!.write(Buffer.concat([Buffer.from([0x17, 0x03, 0x03, 0x00, 0x20]), Buffer.alloc(32, 0x42)])); + + assert.deepStrictEqual(await outcome, { + event: "sessionError", + code: "ERR_SSL_DECRYPTION_FAILED_OR_BAD_RECORD_MAC", + }); + } finally { + client?.destroy(); + toServer?.destroy(); + proxy.close(); + netServer.close(); + } + }); +}); + if (typeof Bun !== "undefined") { describe("Node.js compatibility", () => { test("tests should run on node.js", async () => { diff --git a/test/js/node/tls/node-tls-connect.test.ts b/test/js/node/tls/node-tls-connect.test.ts index 532fc746cbae..0877d2eca947 100644 --- a/test/js/node/tls/node-tls-connect.test.ts +++ b/test/js/node/tls/node-tls-connect.test.ts @@ -211,8 +211,156 @@ it("should be able to grab the JSStreamSocket constructor", () => { //@ts-ignore expect(socket._handle._parentWrap.constructor).toBeFunction(); }); +// A TLS record that fails after the handshake is a fatal protocol error. Node +// reports it as the socket's ERR_SSL_ 'error' and keeps the socket, so +// a clean 'end' follows when the connection goes away. The cases that use +// these helpers put a TCP proxy between the two TLS peers and inject a record +// that cannot authenticate once both handshakes are done. Every assertion also +// holds on Node.js; only the alert's code name depends on the SSL library. + +// application_data, legacy version TLS 1.2, 32 bytes of ciphertext that cannot +// authenticate: the receiver fails the AEAD open and alerts bad_record_mac. +const BAD_RECORD = Buffer.concat([Buffer.from([0x17, 0x03, 0x03, 0x00, 0x20]), Buffer.alloc(32, 0x42)]); +// BoringSSL and OpenSSL 3 name the bad_record_mac alert differently. +const ALERT_BAD_RECORD_MAC = (process.features as { openssl_is_boringssl?: boolean }).openssl_is_boringssl + ? "ERR_SSL_SSLV3_ALERT_BAD_RECORD_MAC" + : "ERR_SSL_SSL/TLS_ALERT_BAD_RECORD_MAC"; + +type FaultedPeers = { + client: TLSSocket; + server: TLSSocket; + toClient: net.Socket; + toServer: net.Socket; + // Append `bytes` to the next chunk the proxy forwards from the server. + appendToNextServerChunk(bytes: Buffer): void; +}; + +async function withFaultProxy( + engines: { connect: typeof tlsConnect; serverOverDuplex?: boolean }, + body: (peers: FaultedPeers) => Promise, +): Promise { + const sockets: (net.Socket | undefined)[] = []; + const servers: net.Server[] = []; + try { + const server = tls.createServer(COMMON_CERT_); + servers.push(server); + const accepted = once(server, "secureConnection") as Promise<[TLSSocket]>; + let upstream: net.Server = server; + if (engines.serverOverDuplex) { + // This server never listens. A TCP server hands it each connection as a generic Duplex. + upstream = net.createServer(raw => { + sockets.push(raw); + server.emit("connection", new SocketProxy(raw)); + }); + servers.push(upstream); + } + await once(upstream.listen(0, "127.0.0.1"), "listening"); + + let toClient: net.Socket | undefined; + let toServer: net.Socket | undefined; + let appendOnce: Buffer | undefined; + const proxy = net.createServer(c => { + toClient = c; + toServer = net.connect((upstream.address() as AddressInfo).port, "127.0.0.1"); + sockets.push(c, toServer); + c.on("data", chunk => toServer!.write(chunk)); + toServer.on("data", chunk => { + if (appendOnce) { + chunk = Buffer.concat([chunk, appendOnce]); + appendOnce = undefined; + } + c.write(chunk); + }); + // However one side goes away, the other side gets a clean FIN. + c.on("close", () => toServer!.end()); + toServer.on("close", () => c.end()); + c.on("error", () => {}); + toServer.on("error", () => {}); + }); + servers.push(proxy); + await once(proxy.listen(0, "127.0.0.1"), "listening"); + + const client = engines.connect({ + host: "127.0.0.1", + port: (proxy.address() as AddressInfo).port, + servername: "localhost", + rejectUnauthorized: false, + }); + sockets.push(client); + await once(client, "secureConnect"); + const [accepted_] = await accepted; + sockets.push(accepted_); + return await body({ + client, + server: accepted_, + toClient: toClient!, + toServer: toServer!, + appendToNextServerChunk: bytes => (appendOnce = bytes), + }); + } finally { + for (const socket of sockets) socket?.destroy(); + for (const server of servers) server.close(); + } +} + +// The 'error', 'end' and 'close' events of `socket`, in order, up to the first +// 'end' or 'close'. Node keeps `socket` open after the error, so `peer` ends +// then: its FIN is what ends `socket` there. +function faultEvents(socket: TLSSocket, peer: TLSSocket) { + const events: string[] = []; + let library: string | undefined; + const { promise: settled, resolve } = Promise.withResolvers(); + socket.on("error", (err: NodeJS.ErrnoException & { library?: string }) => { + events.push(`error ${err.code}`); + library = err.library; + peer.end(); + }); + socket.on("end", () => { + events.push("end"); + resolve(); + }); + socket.on("close", hadError => { + events.push(`close ${hadError}`); + resolve(); + }); + socket.resume(); + peer.on("error", () => {}); + peer.resume(); + return settled.then(() => ({ events: [...events], library })); +} + for (const { name, connect } of tests) { describe(name, () => { + // The uSockets engine (plain tls.connect) does not report these yet. Its rows + // are marked failing until it does. + const itReported = connect === tlsConnect ? it.failing : it; + + itReported("a record that fails to decrypt after the handshake is the socket's ERR_SSL_* error", async () => { + const result = await withFaultProxy({ connect }, ({ client, server, toClient }) => { + const events = faultEvents(client, server); + toClient.write(BAD_RECORD); + return events; + }); + expect(result).toEqual({ + events: ["error ERR_SSL_DECRYPTION_FAILED_OR_BAD_RECORD_MAC", "end"], + library: "SSL routines", + }); + }); + + itReported("the peer's fatal alert after the handshake is the socket's ERR_SSL_* error", async () => { + // The bad record goes to the server, whose SSL_read fails and sends a + // bad_record_mac alert back. The client's SSL_read fails on that alert. + const result = await withFaultProxy({ connect }, ({ client, server, toServer }) => { + const events = faultEvents(client, server); + toServer.write(BAD_RECORD); + return events; + }); + expect(result).toEqual({ + events: [`error ${ALERT_BAD_RECORD_MAC}`, "end"], + library: "SSL routines", + }); + }); + itNetwork("should work with alpnProtocols", done => { try { let socket: TLSSocket | null = connect({ @@ -586,6 +734,42 @@ for (const { name, connect } of tests) { }); } +describe("a fatal post-handshake SSL error over a Duplex", () => { + it("is still reported when a 'data' listener writes back", async () => { + // Good data and the bad record arrive in one chunk. The engine delivers + // the data first, the listener's write hits the now-fatal SSL, and the + // read's error must still be the one that surfaces. + const result = await withFaultProxy({ connect: duplexProxy }, async peers => { + const { client, server, appendToNextServerChunk } = peers; + const events = faultEvents(client, server); + client.on("data", () => client.write("back")); + server.write("first"); + await once(client, "data"); + appendToNextServerChunk(BAD_RECORD); + server.write("second"); + return events; + }); + // What follows the error depends on what the runtime does with the write. + expect({ first: result.events[0], library: result.library }).toEqual({ + first: "error ERR_SSL_DECRYPTION_FAILED_OR_BAD_RECORD_MAC", + library: "SSL routines", + }); + }); + + it("is reported on a server socket", async () => { + const result = await withFaultProxy({ connect: tlsConnect, serverOverDuplex: true }, peers => { + const { client, server, toServer } = peers; + const events = faultEvents(server, client); + toServer.write(BAD_RECORD); + return events; + }); + expect(result).toEqual({ + events: ["error ERR_SSL_DECRYPTION_FAILED_OR_BAD_RECORD_MAC", "end"], + library: "SSL routines", + }); + }); +}); + it("setSession() should not leak the SSL_SESSION returned by d2i_SSL_SESSION", async () => { // d2i_SSL_SESSION returns an owned SSL_SESSION; SSL_set_session takes its own // reference ("the caller retains ownership"), so the caller's reference must diff --git a/test/js/node/tls/node-tls-namedpipes.test.ts b/test/js/node/tls/node-tls-namedpipes.test.ts index 674c39ddac01..95dafaa831de 100644 --- a/test/js/node/tls/node-tls-namedpipes.test.ts +++ b/test/js/node/tls/node-tls-namedpipes.test.ts @@ -206,3 +206,78 @@ it.if(isWindows)("should be able to upgrade a named pipe connection to TLS", asy await test(`\\\\.\\pipe\\test\\${randomUUID()}`); await expectMaxObjectTypeCount(expect, "TLSSocket", 3); }); + +// Same contract as "tls.connect over a Duplex reports a fatal post-handshake +// SSL error" in node-tls-connect.test.ts, with both TLS peers on a named pipe. +// A plain pipe proxy between them injects a record that cannot authenticate +// once the handshake has completed on both sides. +describe("a fatal post-handshake SSL error over a named pipe", () => { + const BAD_RECORD = Buffer.concat([Buffer.from([0x17, 0x03, 0x03, 0x00, 0x20]), Buffer.alloc(32, 0x42)]); + // BoringSSL and OpenSSL 3 name the bad_record_mac alert differently. + const ALERT_BAD_RECORD_MAC = (process.features as { openssl_is_boringssl?: boolean }).openssl_is_boringssl + ? "ERR_SSL_SSLV3_ALERT_BAD_RECORD_MAC" + : "ERR_SSL_SSL/TLS_ALERT_BAD_RECORD_MAC"; + + type Outcome = { event: string; code?: string; library?: string }; + // Settles on the 'error' (expected), or on a 'close' with no 'error' before it (the bug). + function firstErrorOrClose(socket: net.Socket): Promise { + return new Promise(resolve => { + socket.once("error", (err: NodeJS.ErrnoException & { library?: string }) => + resolve({ event: "error", code: err.code, library: err.library }), + ); + socket.once("close", () => resolve({ event: "close" })); + }); + } + + async function run(inject: (toClient: net.Socket, toServer: net.Socket) => void) { + let toClient: net.Socket | undefined; + let toServer: net.Socket | undefined; + let client: ReturnType | undefined; + let serverSocket: ReturnType | undefined; + const serverPipe = `\\\\.\\pipe\\test\\${randomUUID()}`; + const proxyPipe = `\\\\.\\pipe\\test\\${randomUUID()}`; + const server = createServer(tls); + const serverOutcome = Promise.withResolvers(); + server.on("secureConnection", s => firstErrorOrClose(s).then(serverOutcome.resolve)); + const proxy = net.createServer(c => { + toClient = c; + toServer = net.connect(serverPipe); + c.pipe(toServer); + toServer.pipe(c); + c.on("error", () => {}); + toServer.on("error", () => {}); + }); + try { + await once(server.listen(serverPipe), "listening"); + const serverSecure = once(server, "secureConnection"); + await once(proxy.listen(proxyPipe), "listening"); + + client = connect({ path: proxyPipe, rejectUnauthorized: false }); + const clientOutcome = firstErrorOrClose(client); + await once(client, "secureConnect"); + [serverSocket] = await serverSecure; + + inject(toClient!, toServer!); + return { client: await clientOutcome, server: await serverOutcome.promise }; + } finally { + for (const s of [client, serverSocket, toClient, toServer]) s?.destroy(); + proxy.close(); + server.close(); + } + } + + it.if(isWindows)("a record that fails to decrypt on the client", async () => { + expect(await run(toClient => void toClient.write(BAD_RECORD))).toEqual({ + client: { event: "error", code: "ERR_SSL_DECRYPTION_FAILED_OR_BAD_RECORD_MAC", library: "SSL routines" }, + // The client's bad_record_mac alert reaches the server as its own error. + server: { event: "error", code: ALERT_BAD_RECORD_MAC, library: "SSL routines" }, + }); + }); + + it.if(isWindows)("a record that fails to decrypt on the server", async () => { + expect(await run((_toClient, toServer) => void toServer.write(BAD_RECORD))).toEqual({ + client: { event: "error", code: ALERT_BAD_RECORD_MAC, library: "SSL routines" }, + server: { event: "error", code: "ERR_SSL_DECRYPTION_FAILED_OR_BAD_RECORD_MAC", library: "SSL routines" }, + }); + }); +});