Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions packages/bun-usockets/src/crypto/openssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -1903,7 +1903,43 @@ struct us_socket_t *us_internal_ssl_on_data(struct us_socket_t *s, char *data, i
}

if (err == SSL_ERROR_SSL || err == SSL_ERROR_SYSCALL) {
/* ssl_park_fatal_reason handles the handshake-pending side and always
* clears the queue, so a completed handshake must capture the reason
* on the stack first and dispatch it right here instead. */
char reason[US_SSL_FATAL_ERROR_REASON_MAX];
reason[0] = 0;
if (s->ssl_handshake_state == HANDSHAKE_COMPLETED) {
unsigned long ssl_queue_err = ERR_peek_last_error();
if (ssl_queue_err != 0) {
ERR_error_string_n(ssl_queue_err, reason, sizeof(reason));
}
}
ssl_park_fatal_reason(s);
if (reason[0]) {
/* The SSL library failed once the handshake was already done: a
* received fatal alert (TLS 1.3 delivers the server's mTLS
* rejection here, because the client finished one flight earlier)
* or a protocol violation such as a bad record MAC. Node reports
* these through TLSWrap's onerror; dropping them leaves the peer's
* "you are not authenticated" indistinguishable from a clean
* end-of-connection. */
/* Deliver what this read already decrypted first: a peer can put
* application data and the fatal record in one segment, and both
* the ZERO_RETURN sibling above and Node's ClearOut hand the
* plaintext to the consumer before reporting. */
ssl_flush_pending_session(s);
ssl_flush_pending_keylog(s);
if (ssl_gone(s)) return NULL;
if (read) {
s = us_dispatch_data(s, loop_ssl_data->ssl_read_output + LIBUS_RECV_BUFFER_PADDING, read);
if (!s || ssl_gone(s)) return NULL;
}
struct us_bun_verify_error_t verify_error = {
.error = -71, .code = "EPROTO", .reason = reason};
us_dispatch_ssl_error(s, verify_error);
/* The JS error handler may have destroyed the socket. */
if (ssl_gone(s)) return NULL;
}
}
ssl_close(s, 0, NULL);
Comment thread
claude[bot] marked this conversation as resolved.
loop_ssl_data->ssl_last_fatal_error[0] = 0;
Expand Down
4 changes: 4 additions & 0 deletions packages/bun-usockets/src/internal/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ extern struct us_socket_t *us_dispatch_end(us_socket_r s);
extern struct us_socket_t *us_dispatch_connect_error(us_socket_r s, int code);
extern struct us_connecting_socket_t *us_dispatch_connecting_error(struct us_connecting_socket_t *c, int code);
extern void us_dispatch_handshake(us_socket_r s, int success, struct us_bun_verify_error_t err);
/* A fatal SSL error surfaced after the handshake completed (a received fatal
* alert, a protocol violation). Reported to the socket's JS error handler right
* before the connection is torn down. */
extern void us_dispatch_ssl_error(us_socket_r s, struct us_bun_verify_error_t err);
extern void us_dispatch_session(us_socket_r s, const unsigned char *data, int length);
extern void us_dispatch_keylog(us_socket_r s, const unsigned char *data, int length);
extern struct us_socket_t *us_dispatch_ssl_raw_tap(us_socket_r s, char *data, int length);
Expand Down
16 changes: 16 additions & 0 deletions src/js/node/net.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,19 @@ function tlsHandshakeError(verifyError) {
return new ConnResetException("socket hang up");
}

/**
* A fatal SSL error the native layer reports after the handshake completed (a
* received fatal alert, a protocol violation) arrives as EPROTO carrying the
* OpenSSL error string. Node's TLSWrap onerror emits those on the socket rather
* than destroying it; the close that follows still delivers 'end' and 'close'.
* Returns true when the error was handled here.
*/
function emitPostHandshakeTLSError(self, error): boolean {
if (error?.code !== "EPROTO" || !self._secureEstablished) return false;
self.emit("error", tlsHandshakeError(error));
return true;
}

const SocketHandlers: SocketHandler = {
close(socket, err) {
const self = socket.data;
Expand Down Expand Up @@ -329,6 +342,7 @@ const SocketHandlers: SocketHandler = {
callback(error);
}

if (emitPostHandshakeTLSError(self, error)) return;
self.emit("error", error);
},
open(socket) {
Expand Down Expand Up @@ -829,6 +843,7 @@ const ServerHandlers: SocketHandler<NetSocket> = {

if (data._hadError) return;
data._hadError = true;
if (emitPostHandshakeTLSError(data, error)) return;
const bunTLS = this[bunTlsSymbol];

if (typeof bunTLS === "function") {
Expand Down Expand Up @@ -1198,6 +1213,7 @@ const SocketHandlers2: SocketHandler<NonNullable<import("node:net").Socket["_han
callback(error);
}

if (emitPostHandshakeTLSError(self, error)) return;
if (!self.destroyed) process.nextTick(destroyNT, self, error);
},
timeout(socket) {
Expand Down
39 changes: 39 additions & 0 deletions src/runtime/socket/socket_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1730,6 +1730,45 @@ impl<const SSL: bool> NewSocket<SSL> {
Ok(())
}

/// A fatal SSL error surfaced after the handshake completed (a received
/// fatal alert, a protocol violation). Reports it through the `error`
/// handler the way Node's `TLSWrap::ClearOut` reports it through `onerror`,
/// before the close that follows. The handler may destroy the socket, so
/// `openssl.c` re-checks liveness after this returns.
///
/// Takes `ThisPtr<Self>` for the same re-entrancy reason as `on_writable`.
pub fn on_ssl_error(
this: bun_ptr::ThisPtr<Self>,
ssl_error: uws::us_bun_verify_error_t,
) -> JsResult<()> {
jsc::mark_binding!();
if this.socket.get().is_detached() {
return Ok(());
}
// Same late-event guard as the other dispatch entry points: the
// socket may already have released its Handlers.
if !this.has_handlers() {
return Ok(());
}
let handlers = this.get_handlers();
if handlers.vm.is_shutting_down() || this.flags.get().contains(Flags::FINALIZING) {
return Ok(());
}
let scope = handlers.enter();
let global = handlers.global_object;
let this_value = this.get_this_value(&global);
let err_value = match super::uws_jsc::verify_error_to_js(&ssl_error, &global) {
Ok(v) => v,
Err(e) => {
this.exit_scope(scope);
return Err(e);
}
};
let _ = handlers.call_error_handler(this_value, &[this_value, err_value]);
this.exit_scope(scope);
Ok(())
}

/// A new resumable TLS session arrived (the peer's NewSessionTicket was
/// processed during an earlier `SSL_read`). Hands the serialized session
/// to the JS `session` handler, mirroring Node's `onnewsession` callback.
Expand Down
24 changes: 24 additions & 0 deletions src/runtime/socket/uws_dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,30 @@ pub(crate) unsafe extern "C" fn us_dispatch_ssl_raw_tap(
s
}

/// A fatal SSL error surfaced after the handshake already completed: a received
/// fatal alert (under TLS 1.3 a server's mTLS rejection lands here, because the
/// client finishes its handshake one flight earlier) or a protocol violation.
/// Mirrors Node's `TLSWrap::ClearOut` → `onerror`, which reports these on the
/// socket instead of letting them pass as a clean end-of-connection. Only
/// `bun_socket_tls` sockets reach this; every other kind already tears the
/// connection down through its own error path.
///
/// # Safety
/// `openssl.c` must pass a live, non-null `s` whose ext slot holds a valid
/// `*mut TLSSocket`.
#[unsafe(no_mangle)]
pub unsafe extern "C" fn us_dispatch_ssl_error(s: *mut us_socket_t, err: us_bun_verify_error_t) {
let s_ref = us_socket_t::opaque_mut(s);
if s_ref.kind() != SocketKind::BunSocketTls {
return;
}
Comment thread
claude[bot] marked this conversation as resolved.
type TLSSocket = super::NewSocket<true>;
let Some(tls) = *s_ref.ext::<Option<bun_ptr::ThisPtr<TLSSocket>>>() else {
return;
};
let _ = TLSSocket::on_ssl_error(tls, err);
}

/// A new (resumable) TLS session is ready. BoringSSL's new-session callback
/// parks the serialized session while `SSL_read`/`SSL_do_handshake` runs;
/// `ssl_flush_pending_session()` dispatches it here once that stack has
Expand Down
55 changes: 55 additions & 0 deletions test/js/node/tls/node-tls-connect.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, expect, it } from "bun:test";
import { once } from "events";
import { readFileSync } from "fs";
import { bunEnv, bunExe, tls as COMMON_CERT_, isASAN } from "harness";
import https from "https";
import net from "net";
Expand Down Expand Up @@ -747,3 +748,57 @@ it("https.request reports an impossible version window as a TLS error, not a cer
await once(response, "end");
expect(body).toBe("ok");
});

it("reports the server's fatal alert rejecting a missing client certificate", async () => {
// TLS1.3 finishes the client's handshake one flight before the server has
// validated the client certificate, so the server's fatal
// certificate_required alert arrives after 'secureConnect'. Node reports it
// through TLSWrap's onerror; swallowing it leaves an mTLS rejection
// indistinguishable from a clean end-of-connection.
const server = tls.createServer({
key: readFileSync(join(import.meta.dir, "fixtures", "agent1-key.pem")),
cert: readFileSync(join(import.meta.dir, "fixtures", "agent1-cert.pem")),
ca: readFileSync(join(import.meta.dir, "fixtures", "ca1-cert.pem")),
requestCert: true,
rejectUnauthorized: true,
minVersion: "TLSv1.3",
});
server.on("tlsClientError", () => {});
server.listen(0, "127.0.0.1");
await once(server, "listening");
const port = (server.address() as AddressInfo).port;

// The client holds no certificate to offer.
const client = tlsConnect({ port, host: "127.0.0.1", rejectUnauthorized: false });
const events: string[] = [];
let error: any;
// `once(client, "close")` would reject on the 'error' this test is about.
const closed = Promise.withResolvers<boolean>();
client.on("secureConnect", () => events.push("secureConnect"));
client.on("error", e => {
events.push("error");
error = e;
});
client.on("end", () => events.push("end"));
client.on("close", hadError => closed.resolve(hadError));
const hadError = await closed.promise;

// BoringSSL names alert 116 TLSV1_ALERT_CERTIFICATE_REQUIRED; OpenSSL builds
// of Node spell the same alert TLSV13_ALERT_CERTIFICATE_REQUIRED.
expect({
events,
hadError,
code: error?.code,
library: error?.library,
reason: error?.reason,
}).toEqual({
events: ["secureConnect", "error", "end"],
hadError: false,
code: "ERR_SSL_TLSV1_ALERT_CERTIFICATE_REQUIRED",
library: "SSL routines",
reason: "TLSV1_ALERT_CERTIFICATE_REQUIRED",
});

server.close();
await once(server, "close");
});
115 changes: 111 additions & 4 deletions test/js/node/tls/node-tls-server.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import crypto from "crypto";
import { readFileSync, realpathSync } from "fs";
import { tls as cert1, isDebug } from "harness";
import { AddressInfo } from "net";
import { AddressInfo, connect as netConnect, createServer as netCreateServer } from "net";
import { createTest } from "node-harness";
import { once } from "node:events";
import { tmpdir } from "os";
Expand Down Expand Up @@ -1114,9 +1114,11 @@ it("SNICallback runs even when the requested servername matches the bind hostnam
});
server.listen(0, "localhost");
await once(server, "listening");
const port = (server.address() as AddressInfo).port;
// host: "localhost" defaults servername to "localhost" - the bind hostname.
const client = connect({ port, host: "localhost", rejectUnauthorized: false });
// Dial the address the listener actually bound to: on a dual-stack host
// `localhost` can resolve to a different family for connect() than it did for
// listen(). `servername` still carries the bind hostname, which is the point.
const { address, port } = server.address() as AddressInfo;
const client = connect({ port, host: address, servername: "localhost", rejectUnauthorized: false });
await once(client, "secureConnect");
expect(sniCalls).toBe(1);
// The peer certificate must be the SNICallback's RSA cert, not COMMON_CERT.
Expand Down Expand Up @@ -1318,3 +1320,108 @@ it("tls.connect honors secureOptions when negotiating the protocol version", asy
}
await once(server, "close");
});

describe("tls.Server post-handshake TLS errors", () => {
it("reports a corrupted record on the accepted socket instead of closing cleanly", async () => {
// A record that cannot be authenticated fails SSL_read after the handshake
// completed. Node reports those through TLSWrap's onerror; swallowing one
// hides a protocol failure behind an ordinary end-of-connection.
const { promise, resolve, reject } = Promise.withResolvers<any>();
const server = createServer(COMMON_CERT, socket => {
socket.on("error", resolve);
socket.on("close", () => reject(new Error("accepted socket closed without an 'error'")));
// The client waits for this before corrupting the stream, so the server's
// handshake has provably completed by then.
socket.write("hello");
});
server.on("error", reject);
server.listen(0, "127.0.0.1");
await once(server, "listening");

const raw = netConnect((server.address() as AddressInfo).port, "127.0.0.1");
const client = connect({ socket: raw, rejectUnauthorized: false });
raw.on("error", () => {});
client.on("error", () => {});
client.once("data", () => {
// A TLS1.3 application_data record whose payload cannot authenticate.
const payload = Buffer.alloc(32, 0xab);
raw.write(Buffer.concat([Buffer.from([0x17, 0x03, 0x03, 0x00, payload.length]), payload]));
});

try {
const error = await promise;
expect({ code: error?.code, library: error?.library, reason: error?.reason }).toEqual({
code: "ERR_SSL_DECRYPTION_FAILED_OR_BAD_RECORD_MAC",
library: "SSL routines",
reason: "DECRYPTION_FAILED_OR_BAD_RECORD_MAC",
});
} finally {
client.destroy();
raw.destroy();
server.close();
}
await once(server, "close");
});

it("delivers data decrypted alongside a fatal record before reporting the error", async () => {
// A peer can put application data and the record that fails in one segment.
// Node's ClearOut pushes each decrypted chunk to the consumer before it
// reports, so the plaintext must not be dropped on the way to the error.
const events: string[] = [];
const { promise, resolve, reject } = Promise.withResolvers<string[]>();
const server = createServer(COMMON_CERT, socket => {
socket.on("data", chunk => events.push(`data:${chunk}`));
socket.on("error", err => {
events.push(`error:${err.code}`);
resolve(events);
});
socket.on("close", () => reject(new Error(`closed without an 'error': ${events}`)));
// The client waits for this, so the server's handshake has provably
// completed and its flight is flushed before the relay splices anything.
socket.write("hello");
});
server.on("error", reject);
server.listen(0, "127.0.0.1");
await once(server, "listening");
const serverPort = (server.address() as AddressInfo).port;

// A plain TCP relay, so one client record and one corrupt record reach the
// server in a single recv() and the server's SSL_read loop decrypts the
// first before the second fails.
let spliceCorruptRecord = false;
const relay = netCreateServer(downstream => {
const upstream = netConnect(serverPort, "127.0.0.1");
upstream.pipe(downstream);
downstream.on("data", chunk => {
if (!spliceCorruptRecord) return void upstream.write(chunk);
spliceCorruptRecord = false;
const payload = Buffer.alloc(32, 0xab);
const corrupt = Buffer.concat([Buffer.from([0x17, 0x03, 0x03, 0x00, payload.length]), payload]);
upstream.write(Buffer.concat([chunk, corrupt]));
});
downstream.on("error", () => {});
upstream.on("error", () => {});
});
relay.listen(0, "127.0.0.1");
await once(relay, "listening");

const client = connect({
port: (relay.address() as AddressInfo).port,
host: "127.0.0.1",
rejectUnauthorized: false,
});
client.on("error", () => {});
await once(client, "data");
spliceCorruptRecord = true;
client.write("ping");

try {
expect(await promise).toEqual(["data:ping", "error:ERR_SSL_DECRYPTION_FAILED_OR_BAD_RECORD_MAC"]);
} finally {
client.destroy();
relay.close();
server.close();
}
await once(server, "close");
});
});
Loading