Skip to content
Closed
70 changes: 64 additions & 6 deletions packages/bun-usockets/src/crypto/openssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -2396,8 +2396,26 @@ struct us_socket_t *us_internal_ssl_on_end(struct us_socket_t *s) {
return s;
}

/* A socket marked ssl_fatal_error can neither send (us_internal_ssl_write
* refuses it) nor receive (us_internal_ssl_on_data closes it on the next
* bytes); the only thing left to do with it is the close the data path would
* perform, which also dispatches a reason parked before the handshake was
* reported. Used by us_internal_ssl_on_writable, both for the writable event
* us_internal_ssl_write arms when SSL_write fails and for a consumer whose
* writable handler hit that failure (the loop drops writable interest from a
* shut-down socket right after the dispatch, so no further event would come). */
static struct us_socket_t *ssl_close_if_fatal(struct us_socket_t *s) {
if (!s || ssl_gone(s) || !s->ssl_fatal_error) return s;
return ssl_close(s, 0, NULL);
}

struct us_socket_t *us_internal_ssl_on_writable(struct us_socket_t *s) {
ssl_set_loop_data(s);
/* Before the spill drain: nothing may wait behind a dead connection (the
* loop stops polling a shut-down socket for writable, so a spill could not
* finish draining anyway); ssl_close makes one last attempt to send it. */
s = ssl_close_if_fatal(s);
if (!s || ssl_gone(s)) return s;
{
struct loop_ssl_data *loop_ssl_data = (struct loop_ssl_data *)s->group->loop->data.ssl_data;
/* Ciphertext from a partial batch flush goes out before anything else;
Expand All @@ -2421,7 +2439,7 @@ struct us_socket_t *us_internal_ssl_on_writable(struct us_socket_t *s) {
us_socket_stalled_write_means_peer_gone(s)) {
ssl_release_spill(s->group->loop, s);
s->ssl_fatal_error = 1;
return us_dispatch_writable(s);
return ssl_close_if_fatal(us_dispatch_writable(s));
}
return s;
}
Expand All @@ -2446,15 +2464,19 @@ struct us_socket_t *us_internal_ssl_on_writable(struct us_socket_t *s) {
s = us_internal_ssl_on_data(s, "", 0);
if (!s || ssl_gone(s)) return s;
}
if (ssl_gone(s) || s->ssl_fatal_error) return s;
/* Fatal here means the handshake step above failed and was just dispatched:
* close like the data-path driver does instead of leaving the dead socket
* open. */
s = ssl_close_if_fatal(s);
if (!s || ssl_gone(s)) return s;
/* uWS HTTP sockets keep the pre-existing SENT_SHUTDOWN suppression: their
* onWritable clears the teardown timeout armed at shutdown. node sockets
* still get write-completion dispatch after a half-close in either
* direction. */
if (ssl_is_uws_http_tls(s) && us_internal_ssl_is_shut_down(s)) return s;

if (s->ssl_handshake_state == HANDSHAKE_COMPLETED) {
s = us_dispatch_writable(s);
s = ssl_close_if_fatal(us_dispatch_writable(s));
}
return s;
}
Expand Down Expand Up @@ -2819,11 +2841,32 @@ int us_internal_ssl_write(struct us_socket_t *s, const char *data, int length) {
int batching = (loop_ssl_data->ssl_spill_owner == NULL);
loop_ssl_data->ssl_write_batching = batching;

/* Same clean-queue rule as the SSL_read and SSL_do_handshake drivers: the
* classification at the end peeks the per-thread queue, and an entry left
* there by another socket would turn this socket's WANT_READ into a fatal
* error. */
ERR_clear_error();

int total = 0;
int last_ssl_written = 1;
while (total < length) {
int chunk = length - total;
if (chunk > 16384) chunk = 16384;
#if defined(LIBUS_SOCKET_FAULT_INJECTION) && LIBUS_SOCKET_FAULT_INJECTION
{
/* One check per SSL_write call, so a rule's after_n_calls can target a
* later chunk of a multi-chunk write. */
ssize_t injected = 0;
int unused = 0;
if (US_FAULT_CHECK(US_FAULT_SSL_WRITE, us_poll_fd(&s->p), injected, unused)) {
/* What a record-layer failure inside SSL_write looks like from here:
* -1 with the reason on the error queue (SSL_get_error reads it). */
ERR_put_error(ERR_LIB_SSL, 0, ERR_R_INTERNAL_ERROR, __FILE__, __LINE__);
last_ssl_written = -1;
break;
}
}
#endif
/* Same deferred-close protocol as the SSL_do_handshake/SSL_read drivers. */
s->ssl_in_use = 1;
last_ssl_written = SSL_write(s_ssl(s), data + total, chunk);
Expand Down Expand Up @@ -2851,9 +2894,12 @@ int us_internal_ssl_write(struct us_socket_t *s, const char *data, int length) {
if (batching) {
ssl_flush_write_batch(loop_ssl_data, s);
}
if (s->ssl_fatal_error) return 0;
if (total > 0) return total;
if (last_ssl_written <= 0) {
if (!s->ssl_fatal_error && last_ssl_written <= 0) {
/* Classify the failing chunk whether or not earlier chunks went through.
* Only the bytes before it are reported as consumed, and the caller
* buffers the rest; a WANT_READ or fatal error on a later chunk has to
* be recorded exactly like one on the first, or that remainder waits for
* an event nothing would produce. */
int err = SSL_get_error(s_ssl(s), last_ssl_written);
if (err == SSL_ERROR_WANT_READ) {
s->ssl_write_wants_read = 1;
Expand All @@ -2866,6 +2912,18 @@ int us_internal_ssl_write(struct us_socket_t *s, const char *data, int length) {
ssl_park_fatal_reason(s);
}
}
if (!s->ssl_fatal_error) return total;
/* The connection is dead (BoringSSL latched the error, or sealed records
* were lost to an allocation failure), but every caller reads the 0 below as
* backpressure and waits for a writable event that nothing would otherwise
* produce: with the handshake done there is no handshake dispatch to report
* through, and the read side only notices if the peer happens to send more.
* Closing here would run on_close underneath the caller's write (uWS tears
* its per-socket data down in on_close), so arm writable instead;
* us_internal_ssl_on_writable closes fatal sockets. Before the handshake is
* reported, that close also dispatches the reason parked above. */
s->flags.last_write_failed = 1;
us_internal_rearm_writable(s);
return 0;
}

Expand Down
8 changes: 8 additions & 0 deletions packages/bun-usockets/src/internal/fault_inject.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ enum us_fault_syscall {
* US_FAULT_ERRNO applies, and the errno value is ignored — the simulated
* failure is a thrown JS out-of-memory error, not an errno. */
US_FAULT_SESSION_BUFFER,
/* Not a syscall: SSL_write itself failing with a fatal SSL error in
* us_internal_ssl_write (a record-layer failure, as opposed to the
* transport errors US_FAULT_SEND covers). Outside of a library bug or
* allocation failure BoringSSL never fails an established connection's
* write, so the path is unreachable without injection. Only
* US_FAULT_ERRNO applies, and the errno value is ignored: the failure is
* an SSL error, not an errno. */
US_FAULT_SSL_WRITE,
US_FAULT_COUNT
};

Expand Down
3 changes: 3 additions & 0 deletions packages/bun-usockets/src/internal/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,9 @@ void us_internal_ssl_ctx_up_ref(struct ssl_ctx_st *ssl_ctx);
void us_internal_ssl_ctx_unref(struct ssl_ctx_st *ssl_ctx);
/* TCP-level FIN, bypassing the SSL layer (used by ssl_on_end). */
void us_internal_socket_raw_shutdown(us_socket_r s);
/* Poll for writable (keeping readable unless paused / past the peer's FIN), so
* the next writable dispatch reaches the socket. */
void us_internal_rearm_writable(us_socket_r s);

int us_internal_handle_dns_results(us_loop_r loop);

Expand Down
23 changes: 20 additions & 3 deletions packages/bun-usockets/src/socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ struct us_socket_t *us_socket_pair(struct us_socket_group_t *group, unsigned cha
* a paused socket: us_poll_change sets absolute flags, so including READABLE
* unconditionally would silently undo us_socket_pause mid-backpressure and
* deliver data the caller asked to defer. */
static void us_internal_rearm_writable(struct us_socket_t *s) {
void us_internal_rearm_writable(struct us_socket_t *s) {
us_poll_change(&s->p, s->group->loop,
LIBUS_SOCKET_WRITABLE | ((s->flags.is_paused || s->read_eof) ? 0 : LIBUS_SOCKET_READABLE));
}
Expand Down Expand Up @@ -551,8 +551,25 @@ int us_socket_write_check_error(struct us_socket_t *s, const char *data, int len
return 0;
}
if (s->ssl) {
/* TLS writes have their own error propagation; keep the existing path. */
return us_socket_write(s, data, length);
int written = us_socket_write(s, data, length);
/* ssl_fatal_error was clear on entry (us_socket_is_shut_down above),
* so it was this write that failed inside the TLS layer; the SSL layer
* is closing the connection (see us_internal_ssl_write). Report it the
* way a peer-gone send is reported, so the caller fails the write
* instead of buffering it as backpressure that never drains. Failures
* before the handshake has been reported stay with the handshake
* dispatch, which carries the real reason. */
Comment thread
robobun marked this conversation as resolved.
Resolved
if (fatal_write_error && s->ssl_fatal_error && us_internal_ssl_handshake_callback_has_fired(s)) {
#ifdef _WIN32
/* The Windows errno table only names libuv-synthetic codes;
* UCRT's EPROTO is unnameable there and failWrite would reshape
* it as ECONNRESET. Node reports UV_EPROTO on every platform. */
*fatal_write_error = -UV_EPROTO;
#else
*fatal_write_error = EPROTO;
#endif
}
Comment thread
robobun marked this conversation as resolved.
Resolved
return written;
}

int written = bsd_send(us_poll_fd(&s->p), data, length);
Expand Down
7 changes: 4 additions & 3 deletions src/js/internal-for-testing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -580,8 +580,8 @@ export const setSocketOptions: setSocketOptionsFn = $newRustFunction(
/**
* The syscalls instrumented in bsd.c, plus non-syscall hooks whose failure
* paths are otherwise unreachable without injection ("ssl_loop_buffer",
* "poll_start", "session_buffer"; see fault_inject.h for the per-hook
* description). Arming anything else is rejected.
* "poll_start", "session_buffer", "ssl_write"; see fault_inject.h for the
* per-hook description). Arming anything else is rejected.
Comment thread
robobun marked this conversation as resolved.
Resolved
*/
export type SocketFaultSyscall =
| "recv"
Expand All @@ -593,7 +593,8 @@ export type SocketFaultSyscall =
| "accept"
| "ssl_loop_buffer"
| "poll_start"
| "session_buffer";
| "session_buffer"
| "ssl_write";

export type SocketFaultRule = {
syscall: SocketFaultSyscall;
Expand Down
7 changes: 4 additions & 3 deletions src/runtime/socket/socket_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2547,8 +2547,7 @@ impl<const SSL: bool> NewSocket<SSL> {

// The raw [raw, tls] upgrade twin shares the TLS half's us_socket_t
// (`s->ssl` is set) but must write raw bytes: write_check_error would
// route it through the SSL-encrypting us_socket_write, and its fatal
// signal is never set for TLS sockets anyway.
// route it through the SSL-encrypting us_socket_write.
if flags.contains(Flags::BYPASS_TLS) {
let res = self.do_socket_write(buffer);
let uwrote: usize = usize::try_from(res.max(0)).expect("int cast");
Expand Down Expand Up @@ -5062,11 +5061,13 @@ pub mod testing_apis {
fi::POLL_START
} else if syscall_str.eq_ascii(b"session_buffer") {
fi::SESSION_BUFFER
} else if syscall_str.eq_ascii(b"ssl_write") {
fi::SSL_WRITE
} else {
// socket/close/shutdown have enum slots but no bsd.c hooks;
// accepting them would arm rules that can never fire.
return Err(global.throw(format_args!(
"rule.syscall must be one of: recv, send, writev, sendmsg, recvmsg, connect, accept, ssl_loop_buffer, poll_start, session_buffer"
"rule.syscall must be one of: recv, send, writev, sendmsg, recvmsg, connect, accept, ssl_loop_buffer, poll_start, session_buffer, ssl_write"
)));
};

Expand Down
3 changes: 3 additions & 0 deletions src/uws_sys/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,9 @@ pub mod fault_inject {
/// Not a syscall: the JS `Buffer` allocated for a TLS session/keylog
/// payload in the `on_session`/`on_keylog` dispatch.
pub const SESSION_BUFFER: c_int = 12;
/// Not a syscall: `SSL_write` failing with a fatal SSL error inside
/// `us_internal_ssl_write`.
Comment thread
robobun marked this conversation as resolved.
Resolved
pub const SSL_WRITE: c_int = 13;

pub const ACTION_NONE: c_int = 0;
pub const ACTION_ERRNO: c_int = 1;
Expand Down
5 changes: 3 additions & 2 deletions src/uws_sys/socket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,9 @@ impl<const IS_SSL: bool> NewSocketHandler<IS_SSL> {
// ── state queries ───────────────────────────────────────────────────────

/// Raw-TCP write that also reports a fatal send error as the positive
/// errno of the failed `send()` (0 = none); non-Connected and TLS-wrapped
/// sockets fall back to the plain write (no fatal signal).
/// errno of the failed `send()` (0 = none); non-Connected sockets fall
/// back to the plain write (no fatal signal), and a TLS-wrapped socket
/// reports `EPROTO` when a post-handshake `SSL_write` failed fatally.
Comment thread
robobun marked this conversation as resolved.
Resolved
pub fn write_check_error(&self, data: &[u8]) -> (i32, i32) {
on_socket!(self.socket;
connected s => s.write_check_error(data),
Expand Down
19 changes: 17 additions & 2 deletions test/js/bun/util/socket-fault-injection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,15 @@ describe.skipIf(skip)("socketFaultInjection control surface", () => {
// syscall used to succeed silently and never fire. ssl_loop_buffer is an
// allocation, so it has no byte count either.
test("set() rejects 'short' for syscalls that cannot clamp a byte count", () => {
for (const syscall of ["writev", "sendmsg", "recvmsg", "connect", "accept", "ssl_loop_buffer"] as const) {
for (const syscall of [
"writev",
"sendmsg",
"recvmsg",
"connect",
"accept",
"ssl_loop_buffer",
"ssl_write",
] as const) {
expect(() => fault.set({ syscall, action: "short", bytes: 1 })).toThrow(/only supported for syscall/);
}
expect(fault.set({ syscall: "recv", action: "short", bytes: 1 })).toBe(true);
Expand All @@ -34,7 +42,7 @@ describe.skipIf(skip)("socketFaultInjection control surface", () => {
// A zero return only means something for the data syscalls (EOF on the read
// side, backpressure on the write side); connect's wrapper returns errno.
test("set() rejects 'zero' for syscalls with no zero-return semantics", () => {
for (const syscall of ["connect", "accept", "ssl_loop_buffer"] as const) {
for (const syscall of ["connect", "accept", "ssl_loop_buffer", "ssl_write"] as const) {
expect(() => fault.set({ syscall, action: "zero" })).toThrow(/only supported for syscall/);
}
for (const syscall of ["recv", "send", "writev", "sendmsg", "recvmsg"] as const) {
Expand All @@ -46,6 +54,13 @@ describe.skipIf(skip)("socketFaultInjection control surface", () => {
expect(fault.set({ syscall: "ssl_loop_buffer", action: "errno", errno: "ENOMEM" })).toBe(true);
});

// ssl_write's hook runs on a socket, so unlike the allocation hooks it can be
// pinned to one descriptor.
test("set() accepts ssl_write with action 'errno', with or without an fd", () => {
expect(fault.set({ syscall: "ssl_write", action: "errno", errno: "EINVAL" })).toBe(true);
expect(fault.set({ syscall: "ssl_write", action: "errno", errno: "EINVAL", fd: 3 })).toBe(true);
});

// ssl_loop_buffer's hook is an allocation, so it checks with fd = -1; a rule
// pinned to a descriptor would arm and then silently never fire.
test("set() rejects 'fd' for ssl_loop_buffer, which has no descriptor", () => {
Expand Down
61 changes: 61 additions & 0 deletions test/js/node/tls/node-tls-server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,67 @@ it("keeps socket.authorized false when a client without a certificate resumes a
]);
});

it("completes a write that fails inside the TLS layer after the handshake instead of leaving the socket open", async () => {
// After a TLSv1.3 requestCert handshake the server's first write also
// carries its NewSessionTickets, each embedding the client's whole chain.
// With a ~48 KiB chain that flight overflows the record layer's write
// buffer and SSL_write fails with an internal error (a separate record-layer
// fix makes it go through). Either way the socket layer's contract is the
// same: the write completes, with an error that destroys the socket or with
// the reply reaching the client. It used to do neither: write() returned
// true and the connection stayed open and silent.
const fixtures = join(import.meta.dir, "fixtures");
const agent1Key = readFileSync(join(fixtures, "agent1-key.pem"), "utf8");
const agent1Cert = readFileSync(join(fixtures, "agent1-cert.pem"), "utf8");
const ca1 = readFileSync(join(fixtures, "ca1-cert.pem"), "utf8");
// agent1 still verifies against ca1; the 52 extra copies of ca1 only pad
// the chain the client sends.
const paddedChain = agent1Cert + Buffer.alloc(ca1.length * 52, ca1).toString();

const serverEvents: string[] = [];
const serverSocketDone = Promise.withResolvers<{ writeError: NodeJS.ErrnoException | null | undefined }>();
await using server = createServer(
{ key: agent1Key, cert: agent1Cert, ca: [ca1], requestCert: true, minVersion: "TLSv1.3", maxVersion: "TLSv1.3" },
socket => {
socket.on("error", err => serverEvents.push(`error:${(err as NodeJS.ErrnoException).code}`));
socket.on("close", hadError => serverEvents.push(`close:${hadError}`));
socket.on("data", () => {
socket.write("pong", writeError => {
if (writeError) {
socket.once("close", () => serverSocketDone.resolve({ writeError }));
} else {
serverSocketDone.resolve({ writeError });
}
});
});
},
);
server.on("tlsClientError", serverSocketDone.reject);
await once(server.listen(0, "127.0.0.1"), "listening");
const { port } = server.address() as AddressInfo;

const client = connect({ port, host: "127.0.0.1", key: agent1Key, cert: paddedChain, rejectUnauthorized: false });
client.on("error", () => {});
const clientClosed = new Promise<void>(resolve => client.on("close", () => resolve()));
const clientReply = new Promise<string>(resolve => client.once("data", chunk => resolve(String(chunk))));
await once(client, "secureConnect");
client.write("ping");

const { writeError } = await serverSocketDone.promise;
if (writeError) {
expect({ code: writeError.code, syscall: writeError.syscall, serverEvents }).toEqual({
code: "EPROTO",
syscall: "write",
serverEvents: ["error:EPROTO", "close:true"],
});
} else {
expect(await clientReply).toBe("pong");
client.end();
}
// In both outcomes the connection itself is gone afterwards.
await clientClosed;
});

it("keeps req.socket.authorized false for an unverified client after the server socket shuts down", async () => {
type Verdict = [boolean | undefined, string | null | undefined];
const { promise, resolve, reject } = Promise.withResolvers<{ before: Verdict; after: Verdict }>();
Expand Down
Loading
Loading