From 2011126f0e248f5a99a7caa237ab1ee476bbf22d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 9 Jul 2026 16:36:21 +0000 Subject: [PATCH 1/2] tls: implement OCSP stapling (OCSPRequest / OCSPResponse) Rebased onto fc865b398e across #31859's Handlers rework: the OCSP callbacks are now two more fields on JSSocketHandlers (JSInternalFieldObjectImpl<16>), and the socket dispatches use the ThisPtr + Rc shape the sibling dispatches moved to. Server side: BoringSSL's cert_cb (the one server hook that runs after the ClientHello extensions are parsed, after select_certificate_cb's SNI context swap, and can pause) dispatches 'OCSPRequest' with the connection's certificate and issuer as DER; an asynchronous listener suspends the handshake (SSL_ERROR_WANT_X509_LOOKUP, parked like the existing async-SNICallback suspension) until handle.resumeOCSP() re-drives it. callback(err) surfaces as 'tlsClientError' and drops the connection. Client side: requestOCSP enables stapling before the ClientHello goes out, and BoringSSL's legacy OCSP callback delivers the response from inside the handshake via us_dispatch_ocsp_response, so destroying the socket in the listener still aborts the session. Destroying a client socket from inside a handshake callback no longer emits 'secureConnect'; us_internal_ssl_close settles the handshake on its way out. TLS over a generic Duplex / a Windows named pipe (the SSLWrapper engine) warns once instead of silently accepting requestOCSP. scripts/build/codegen.ts declares bindgenv2's generated headers as ninja outputs, so adding a field to a .bindv2.ts dictionary no longer leaves an object file compiled against the old struct layout. --- packages/bun-usockets/src/context.c | 1 + packages/bun-usockets/src/crypto/openssl.c | 207 ++++++++++++++++- packages/bun-usockets/src/internal/internal.h | 6 + packages/bun-usockets/src/libusockets.h | 20 ++ scripts/build/codegen.ts | 30 ++- src/codegen/bindgenv2/script.ts | 8 + src/js/node/net.ts | 151 +++++++++++- src/js/node/tls.ts | 7 + src/jsc/bindings/JSSocketHandlers.h | 6 +- src/jsc/generated.rs | 6 + src/runtime/socket/Handlers.rs | 10 + src/runtime/socket/JSSocketHandlers.rs | 8 +- src/runtime/socket/Listener.rs | 124 ++++++++++ src/runtime/socket/SocketConfig.bindv2.ts | 2 + src/runtime/socket/socket_body.rs | 77 ++++++ src/runtime/socket/sockets.classes.ts | 8 + src/runtime/socket/tls_socket_functions.rs | 151 ++++++++++++ src/runtime/socket/uws_dispatch.rs | 34 +++ src/uws_sys/ListenSocket.rs | 15 ++ src/uws_sys/socket.rs | 32 +++ src/uws_sys/us_socket_t.rs | 27 +++ .../node/tls/node-tls-ocsp-duplex.fixture.ts | 57 +++++ test/js/node/tls/node-tls-ocsp.test.ts | 219 ++++++++++++++++++ 23 files changed, 1177 insertions(+), 29 deletions(-) create mode 100644 test/js/node/tls/node-tls-ocsp-duplex.fixture.ts create mode 100644 test/js/node/tls/node-tls-ocsp.test.ts diff --git a/packages/bun-usockets/src/context.c b/packages/bun-usockets/src/context.c index 6bd57a6887cb..f3b1bde9e8c6 100644 --- a/packages/bun-usockets/src/context.c +++ b/packages/bun-usockets/src/context.c @@ -357,6 +357,7 @@ static void us_internal_init_listen_socket(struct us_listen_socket_t *ls, if (ssl_ctx) us_internal_ssl_ctx_up_ref(ssl_ctx); ls->sni = NULL; ls->on_server_name = NULL; + ls->on_ocsp_request = NULL; ls->socket_ext_size = socket_ext_size; ls->deferred_accept = 0; diff --git a/packages/bun-usockets/src/crypto/openssl.c b/packages/bun-usockets/src/crypto/openssl.c index c8528b0016e2..a1537602d67f 100644 --- a/packages/bun-usockets/src/crypto/openssl.c +++ b/packages/bun-usockets/src/crypto/openssl.c @@ -148,6 +148,10 @@ long us_ssl_ctx_live_count(void) { * SSL_CTX is shared and can outlive any one listener, so storing ls as the * CTX-level servername_arg is a UAF after listener close (and overwritten * on multi-listen). + * - us_ssl_ocsp_pending_idx (SSL): per-connection async-'OCSPRequest' + * suspension state, malloc'd only when a handler answers asynchronously. + * - us_ssl_loop_bio_idx (SSL): marks the SSLs whose BIO pair is the loop's + * shared one, i.e. the ones a callback may recover `loop_ssl_data` from. * * SSL_CTX creation runs from both the JS thread (SecureContext, Bun.connect/ * listen) and the HTTP-client thread (HTTPContext.initWithOpts). A racy `<0` @@ -165,6 +169,14 @@ static int us_ctx_user_ca_ex_idx = -1; static int us_ssl_reneg_state_idx = -1; /* Per-connection async-SNI suspension state (select_certificate_cb retry). */ static int us_ssl_sni_pending_idx = -1; +/* Per-connection async-OCSP suspension state (cert_cb pause). */ +static int us_ssl_ocsp_pending_idx = -1; +/* Marks an SSL whose BIO pair is the loop's shared one (set in + * us_internal_ssl_attach, the only place that installs it). A callback may only + * recover `loop_ssl_data` from `BIO_get_data(SSL_get_wbio(ssl))` when it is set: + * SSLs driven by the Rust SSLWrapper (TLS-over-duplex, HTTP proxy tunnels) own a + * private memory BIO whose data is something else entirely. */ +static int us_ssl_loop_bio_idx = -1; static int us_ssl_listener_ex_idx = -1; /* Set (to a non-NULL marker) only on SSLs attached to a real us_socket_t via * us_internal_ssl_attach. The new-session callback uses it to ignore SSLs @@ -207,6 +219,22 @@ static void us_ssl_sni_pending_free(void *parent, void *ptr, CRYPTO_EX_DATA *ad, us_free(st); } +/* Async 'OCSPRequest' suspension state, hung off the SSL via ex_data. + * Allocated the first time the JS handler answers "pending"; freed with the + * SSL. The response bytes themselves never live here - the resolution calls + * SSL_set_ocsp_response() on the in-flight SSL (which copies) before + * us_socket_ocsp_resolve() re-drives the handshake. */ +struct us_ssl_ocsp_pending_t { + /* 0 = none, 1 = waiting for the JS resolution, 2 = resolved */ + int state; +}; + +static void us_ssl_ocsp_pending_free(void *parent, void *ptr, CRYPTO_EX_DATA *ad, + int index, long argl, void *argp) { + (void)parent; (void)ad; (void)index; (void)argl; (void)argp; + if (ptr) us_free(ptr); +} + struct us_ssl_reneg_state_t { uint64_t window_start_ms; uint32_t count; @@ -372,6 +400,8 @@ static void us_ex_idx_init(void) { us_ctx_user_ca_ex_idx = SSL_CTX_get_ex_new_index(0, NULL, NULL, NULL, NULL); us_ssl_reneg_state_idx = SSL_get_ex_new_index(0, NULL, NULL, NULL, us_ssl_reneg_state_free); us_ssl_sni_pending_idx = SSL_get_ex_new_index(0, NULL, NULL, NULL, us_ssl_sni_pending_free); + us_ssl_ocsp_pending_idx = SSL_get_ex_new_index(0, NULL, NULL, NULL, us_ssl_ocsp_pending_free); + us_ssl_loop_bio_idx = SSL_get_ex_new_index(0, NULL, NULL, NULL, NULL); us_ssl_listener_ex_idx = SSL_get_ex_new_index(0, NULL, NULL, NULL, NULL); us_ssl_is_socket_ex_idx = SSL_get_ex_new_index(0, NULL, NULL, NULL, NULL); us_ssl_pending_session_idx = SSL_get_ex_new_index(0, NULL, NULL, NULL, us_ssl_pending_session_free); @@ -464,6 +494,8 @@ static inline struct us_ssl_reneg_state_t *us_reneg_state(SSL *ssl) { extern void us_internal_socket_raw_shutdown(struct us_socket_t *s); static void ssl_update_handshake(struct us_socket_t *s); +static int us_cert_cb(SSL *ssl, void *arg); +static int us_ocsp_status_cb(SSL *ssl, void *arg); /* ── BIO plumbing ───────────────────────────────────────────────────────── * The same shared mem-BIO pair is reused for every SSL* on a loop. The write @@ -903,6 +935,16 @@ SSL_CTX *us_ssl_ctx_build_raw(struct us_bun_socket_context_options_t options, /* Default options we rely on — changing these breaks the BIO logic. */ SSL_CTX_set_read_ahead(ssl_context, 1); SSL_CTX_set_mode(ssl_context, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); + /* Server-side 'OCSPRequest' dispatch. Registered on every context (not only + * the ones a listener with a handler ends up using) because SSL_set_SSL_CTX + * re-derives the per-connection CERT from the SNI-selected context, and + * cert_cb travels with the CERT. us_cert_cb short-circuits unless the + * connection is a server handshake whose listener registered a handler and + * whose ClientHello asked for stapling. */ + SSL_CTX_set_cert_cb(ssl_context, us_cert_cb, NULL); + /* Client-side 'OCSPResponse' dispatch; only fires for a connection that + * enabled stapling (us_socket_request_ocsp_stapling). */ + SSL_CTX_set_tlsext_status_cb(ssl_context, us_ocsp_status_cb); /* Honor explicit minVersion/maxVersion (Node's secureProtocol/min/maxVersion); * default to a TLS1.2 floor when no minimum is requested. */ SSL_CTX_set_min_proto_version(ssl_context, options.ssl_min_version ? options.ssl_min_version : TLS1_2_VERSION); @@ -1294,6 +1336,10 @@ void us_internal_ssl_attach(struct us_socket_t *s, SSL_CTX *ctx, SSL_set_bio(ssl, loop_ssl_data->shared_rbio, loop_ssl_data->shared_wbio); BIO_up_ref(loop_ssl_data->shared_rbio); BIO_up_ref(loop_ssl_data->shared_wbio); + /* This SSL's write BIO now carries `loop_ssl_data`; callbacks that reach for + * it through BIO_get_data gate on this marker. */ + us_ex_idx_ensure(); + SSL_set_ex_data(ssl, us_ssl_loop_bio_idx, (void *)1); /* renegotiation: ssl_renegotiate_explicit lets us bound it on the client * (issues #6197/#5363); never on the server (DoS vector). */ @@ -1712,10 +1758,11 @@ static void ssl_update_handshake(struct us_socket_t *s) { if (result <= 0) { int err = SSL_get_error(s_ssl(s), result); - if (err == SSL_ERROR_PENDING_CERTIFICATE) { - /* Suspended by an async SNICallback: stay in HANDSHAKE_PENDING with no - * poll re-arm; us_socket_sni_resolve() re-drives the handshake when the - * JS resolution arrives. */ + if (err == SSL_ERROR_PENDING_CERTIFICATE || err == SSL_ERROR_WANT_X509_LOOKUP) { + /* Suspended by an async SNICallback (select_certificate_cb retry) or an + * async 'OCSPRequest' handler (cert_cb pause): stay in HANDSHAKE_PENDING + * with no poll re-arm; us_socket_sni_resolve() / us_socket_ocsp_resolve() + * re-drive the handshake when the JS resolution arrives. */ s->ssl_handshake_state = HANDSHAKE_PENDING; return; } @@ -1874,13 +1921,15 @@ struct us_socket_t *us_internal_ssl_on_data(struct us_socket_t *s, char *data, i if (just_read <= 0) { int err = SSL_get_error(s_ssl(s), just_read); - /* SSL_ERROR_PENDING_CERTIFICATE: the handshake is suspended waiting for - * an async SNICallback (us_select_cert_cb returned retry). Treat it - * like WANT_READ - stop the read loop, deliver whatever was decrypted, - * and park the socket; us_socket_sni_resolve() re-drives the handshake - * when the JS resolution arrives. */ + /* SSL_ERROR_PENDING_CERTIFICATE / SSL_ERROR_WANT_X509_LOOKUP: the + * handshake is suspended waiting for an async SNICallback + * (us_select_cert_cb returned retry) or an async 'OCSPRequest' handler + * (us_cert_cb paused). Treat both like WANT_READ - stop the read loop, + * deliver whatever was decrypted, and park the socket; + * us_socket_sni_resolve() / us_socket_ocsp_resolve() re-drive the + * handshake when the JS resolution arrives. */ if (err != SSL_ERROR_WANT_READ && err != SSL_ERROR_WANT_WRITE && - err != SSL_ERROR_PENDING_CERTIFICATE) { + err != SSL_ERROR_PENDING_CERTIFICATE && err != SSL_ERROR_WANT_X509_LOOKUP) { if (err == SSL_ERROR_WANT_RENEGOTIATE) { if (ssl_renegotiate(s)) continue; if (ssl_gone(s)) return NULL; @@ -2218,6 +2267,144 @@ void us_socket_sni_resolve(struct us_socket_t *s, struct ssl_ctx_st *ctx, int er ssl_update_handshake(s); } +/* ── OCSP stapling ───────────────────────────────────────────────────────── */ + +/* Server-side: stage the DER OCSP response this connection staples. Must run + * before the Certificate message is built; BoringSSL copies the bytes. */ +int us_socket_set_ocsp_response(struct us_socket_t *s, const unsigned char *response, + size_t length) { + if (!s || !s->ssl || !s_ssl(s) || !response || !length) return 0; + return SSL_set_ocsp_response(s_ssl(s), response, length) == 1; +} + +/* Client-side: ask the server for a stapled OCSP response. Must run before the + * ClientHello goes out (the JS `open` dispatch, which precedes the first + * ssl_update_handshake). */ +void us_socket_request_ocsp_stapling(struct us_socket_t *s) { + if (!s || !s->ssl || !s_ssl(s)) return; + SSL_enable_ocsp_stapling(s_ssl(s)); +} + +/* BoringSSL's legacy OpenSSL OCSP callback. Its contract depends on the role: + * + * On a client it stands in for OpenSSL's status callback and fires right after + * the server's certificate is verified, before the client sends its own flight + * - the point Node emits 'OCSPResponse' from, so a listener that rejects the + * staple can still abort the handshake by destroying the socket. Returning one + * accepts the response (Node always does; the listener has no say in the return + * value). + * + * On a server it fires after cert_cb, where us_cert_cb already staged any + * response; SSL_TLSEXT_ERR_OK just lets the staple through. */ +static int us_ocsp_status_cb(SSL *ssl, void *arg) { + (void)arg; + if (!ssl) return SSL_TLSEXT_ERR_OK; + if (SSL_is_server(ssl)) return SSL_TLSEXT_ERR_OK; + + /* Bun's HTTP client enables stapling on every connection it opens (it mimics + * a browser's ClientHello), including proxy tunnels whose SSL is driven by the + * Rust SSLWrapper over a private memory BIO. Reading `loop_ssl_data` out of + * that BIO would be a wild pointer, so only SSLs that us_internal_ssl_attach + * gave the loop's shared pair may do it. */ + if (us_ssl_loop_bio_idx < 0 || !SSL_get_ex_data(ssl, us_ssl_loop_bio_idx)) return 1; + struct loop_ssl_data *lsd = (struct loop_ssl_data *)BIO_get_data(SSL_get_wbio(ssl)); + struct us_socket_t *s = lsd ? lsd->ssl_socket : NULL; + /* Only Bun.connect / node:tls sockets surface an 'OCSPResponse' event; the + * HTTP client's own sockets have nowhere to deliver one. */ + if (!s || us_socket_kind(s) != BUN_SOCKET_KIND_BUN_SOCKET_TLS) return 1; + + const unsigned char *response = NULL; + size_t length = 0; + SSL_get0_ocsp_response(ssl, &response, &length); + if (length > INT_MAX) return 1; + + void *saved_loop_state[5]; + us_internal_ssl_loop_state_save(ssl, saved_loop_state); + us_dispatch_ocsp_response(s, response, response ? (int)length : -1); + us_internal_ssl_loop_state_restore(saved_loop_state); + return 1; +} + +/* Resume a handshake suspended by an asynchronous 'OCSPRequest' handler. The + * response (if any) was already staged with us_socket_set_ocsp_response; a + * handler that reported an error destroys the socket from JS instead of + * resuming. No-op when the socket already closed or was never suspended. */ +void us_socket_ocsp_resolve(struct us_socket_t *s) { + if (!s || us_socket_is_closed(s) || !s->ssl || !s_ssl(s)) return; + if (us_ssl_ocsp_pending_idx < 0) return; + struct us_ssl_ocsp_pending_t *pending = SSL_get_ex_data(s_ssl(s), us_ssl_ocsp_pending_idx); + if (!pending || pending->state != 1) return; /* late/duplicate resolution */ + pending->state = 2; + /* Re-drive the handshake; cert_cb re-fires and consumes the state. */ + ssl_set_loop_data(s); + ssl_update_handshake(s); +} + +/* BoringSSL's cert_cb: the last hook that runs after the ClientHello + * extensions are parsed (so SSL_get_tlsext_status_type is meaningful) and + * after select_certificate_cb's SSL_set_SSL_CTX (which would otherwise discard + * a response staged against the old CERT), and the only server-side hook that + * can pause the handshake (rv < 0 -> SSL_ERROR_WANT_X509_LOOKUP). */ +static int us_cert_cb(SSL *ssl, void *arg) { + (void)arg; + if (!ssl || !SSL_is_server(ssl)) return 1; + if (us_ssl_listener_ex_idx < 0 || us_ssl_ocsp_pending_idx < 0) return 1; + + struct us_ssl_ocsp_pending_t *pending = SSL_get_ex_data(ssl, us_ssl_ocsp_pending_idx); + if (pending) { + /* Re-entry after a suspension: 1 = still waiting, 2 = the resolution + * already staged its response (or chose not to staple). */ + if (pending->state == 1) return -1; + if (pending->state == 2) { + pending->state = 0; + return 1; + } + } + + /* Nothing to do unless this ClientHello carried status_request. */ + if (SSL_get_tlsext_status_type(ssl) != TLSEXT_STATUSTYPE_ocsp) return 1; + + struct us_listen_socket_t *ls = + (struct us_listen_socket_t *)SSL_get_ex_data(ssl, us_ssl_listener_ex_idx); + if (!ls || !ls->on_ocsp_request) return 1; + + struct loop_ssl_data *cb_lsd = (struct loop_ssl_data *)BIO_get_data(SSL_get_wbio(ssl)); + struct us_socket_t *cb_socket = cb_lsd ? cb_lsd->ssl_socket : NULL; + if (!cb_socket) return 1; + + void *saved_loop_state[5]; + us_internal_ssl_loop_state_save(ssl, saved_loop_state); + int abort_handshake = 0; + ls->on_ocsp_request(ls, cb_socket, &abort_handshake); + us_internal_ssl_loop_state_restore(saved_loop_state); + + if (abort_handshake == 1) { + /* The handler reported an error: it destroyed the socket from JS, so the + * handshake has nothing left to drive. */ + return 0; + } + if (abort_handshake == 2) { + /* Asynchronous handler: suspend until us_socket_ocsp_resolve(). With no + * state to resume through there is nothing to suspend on, so proceed + * without a staple rather than park the handshake forever. */ + if (!pending) { + pending = us_calloc(1, sizeof(*pending)); + if (!pending) return 1; + SSL_set_ex_data(ssl, us_ssl_ocsp_pending_idx, pending); + } + pending->state = 1; + return -1; + } + /* Synchronous handler: the response (if any) is already staged. */ + return 1; +} + +void us_listen_socket_on_ocsp_request(struct us_listen_socket_t *ls, + void (*cb)(struct us_listen_socket_t *, + struct us_socket_t *, int *)) { + ls->on_ocsp_request = cb; +} + void us_internal_ssl_handshake_abort(struct us_socket_t *s) { s->ssl_fatal_error = 1; ssl_close(s, 0, NULL); diff --git a/packages/bun-usockets/src/internal/internal.h b/packages/bun-usockets/src/internal/internal.h index a6bace29201c..24bbb7496179 100644 --- a/packages/bun-usockets/src/internal/internal.h +++ b/packages/bun-usockets/src/internal/internal.h @@ -135,6 +135,9 @@ extern struct us_connecting_socket_t *us_dispatch_connecting_error(struct us_con extern void us_dispatch_handshake(us_socket_r s, int success, 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); +/* `length < 0` means the server stapled nothing (Node emits 'OCSPResponse' + * with null in that case). Runs from inside the handshake. */ +extern void us_dispatch_ocsp_response(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); extern int Bun__addrinfo_get(struct us_loop_t* loop, const char* host, uint16_t port, struct addrinfo_request** ptr); @@ -410,6 +413,9 @@ struct us_listen_socket_t { * in-flight handshake only (the caller does not cache it), or NULL to fall * through to the default context. */ struct ssl_ctx_st *(*on_server_name)(struct us_listen_socket_t *, const char *hostname, int *abort_handshake, struct us_socket_t *socket); + /* Server-side 'OCSPRequest' dispatch; NULL when the owning server has no + * handler. Runs from cert_cb, once per handshake that asked for stapling. */ + void (*on_ocsp_request)(struct us_listen_socket_t *, struct us_socket_t *socket, int *abort_handshake); unsigned int socket_ext_size; /* kind to stamp on accepted sockets. */ unsigned char accept_kind; diff --git a/packages/bun-usockets/src/libusockets.h b/packages/bun-usockets/src/libusockets.h index c1ccae19acfe..3622baec84da 100644 --- a/packages/bun-usockets/src/libusockets.h +++ b/packages/bun-usockets/src/libusockets.h @@ -371,6 +371,26 @@ void us_listen_socket_on_server_name(struct us_listen_socket_t *ls, void us_socket_sni_resolve(us_socket_r s, struct ssl_ctx_st *ctx, int error); void *us_socket_server_name_userdata(us_socket_r s); +/* ── OCSP stapling ───────────────────────────────────────────────────────── */ +/* Registers the server-side 'OCSPRequest' dispatch. It runs once per handshake + * whose ClientHello carried status_request, and reports back through + * `abort_handshake`: 0 = done (any response was staged with + * us_socket_set_ocsp_response), 1 = the handler errored and destroyed the + * socket, 2 = the handler is asynchronous, suspend until + * us_socket_ocsp_resolve(). */ +void us_listen_socket_on_ocsp_request(struct us_listen_socket_t *ls, + void (*cb)(struct us_listen_socket_t *, struct us_socket_t *socket, int *abort_handshake)) + nonnull_fn_decl; +/* Server-side: stage the DER response this connection staples. Returns 0 when + * the bytes were rejected. */ +int us_socket_set_ocsp_response(us_socket_r s, const unsigned char *response, size_t length); +/* Resume a handshake suspended by an async 'OCSPRequest' handler. Safe to call + * after the socket closed (no-op). */ +void us_socket_ocsp_resolve(us_socket_r s); +/* Client-side: request a stapled response, which arrives through the + * `us_dispatch_ocsp_response` hook. Must run before the ClientHello. */ +void us_socket_request_ocsp_stapling(us_socket_r s); + /* ── Connect ────────────────────────────────────────────────────────────── * Returns either us_socket_t* (fast path, *is_connecting=1) or * us_connecting_socket_t* (DNS / happy-eyeballs in flight, *is_connecting=0). diff --git a/scripts/build/codegen.ts b/scripts/build/codegen.ts index 746fdb22463d..5368c2e8508a 100644 --- a/scripts/build/codegen.ts +++ b/scripts/build/codegen.ts @@ -18,19 +18,22 @@ * * Several scripts emit MORE files than they report: * - bindgen.ts emits Generated.h per namespace (only .cpp declared) - * - bindgenv2 emits Generated.h per type (list-outputs skips .h) * - generate-node-errors.ts emits ErrorCode.d.ts (not declared) * - bundle-modules.ts emits eval/ subdir, BunBuiltinNames+extras.h, etc. * - * It WORKS because: + * It mostly WORKS because: * 1. The declared .cpp outputs guarantee the step runs before compile * 2. Compilation emits .d depfiles that track the .h files for NEXT build * 3. PCH order-depends on ALL codegen outputs; every cxx() waits on PCH * → all codegen completes before any compile, undeclared .h exist * - * Fixing properly (declaring all outputs) would require patching the - * src/codegen/ scripts to report everything — changing contract with - * existing tooling. + * "for NEXT build" is the hole, and it bites whenever a header changes while + * the .cpp that includes it does not: ninja settled the build plan before the + * header was rewritten, so the object keeps the layout it was last compiled + * against. For a struct shared with Rust that links a binary which memcpys past + * the end of the other side's stack slot. bindgenv2 therefore declares its + * headers (list-outputs reports them); the rest remain undeclared, and should + * follow when someone touches them. */ import { spawnSync } from "node:child_process"; @@ -838,11 +841,22 @@ function emitBindgenV2({ n, cfg, sources, o, dirStamp }: Ctx): void { assert(allOutputs.length > 0, "bindgenv2 list-outputs returned no files"); const cppOutputs = allOutputs.filter(p => p.endsWith(".cpp")); - const other = allOutputs.filter(p => !p.endsWith(".cpp")); + const other = allOutputs.filter(p => !p.endsWith(".cpp") && !p.endsWith(".h")); assert(other.length === 0, `bindgenv2 emitted unexpected output type: ${other.join(", ")}`); + // Declare the headers, not just the sources. A dictionary gaining a field + // grows `Generated.h` while leaving the `Generated.cpp` that + // embeds it textually identical — ninja settles the build plan before the + // codegen edge rewrites the header, so the object keeps the layout it was last + // compiled against and the depfile only catches up on the NEXT build. For a + // struct shared with Rust that links a binary whose C++ side memcpys past the + // end of the Rust side's stack slot. Derived from the sources rather than + // taken on faith from list-outputs: every bindgenv2 type that emits a .cpp + // emits the matching .h, and the declaration is what keeps the build sound. + const declaredOutputs = [...new Set([...allOutputs, ...cppOutputs.map(p => p.replace(/\.cpp$/, ".h"))])]; + n.build({ - outputs: allOutputs, + outputs: declaredOutputs, rule: "codegen", inputs: [script, ...sources.bindgenV2, ...sources.bindgenV2Internal], orderOnlyInputs: [dirStamp], @@ -859,7 +873,7 @@ function emitBindgenV2({ n, cfg, sources, o, dirStamp }: Ctx): void { }, }); - o.all.push(...allOutputs); + o.all.push(...declaredOutputs); o.bindgenV2Cpp.push(...cppOutputs); } diff --git a/src/codegen/bindgenv2/script.ts b/src/codegen/bindgenv2/script.ts index a057d1a0725c..f28e7fb09648 100755 --- a/src/codegen/bindgenv2/script.ts +++ b/src/codegen/bindgenv2/script.ts @@ -53,9 +53,17 @@ function toZigNamespace(name: string): string { return result; } +// Headers are declared alongside the sources even though nothing `#include`s +// them from ninja's point of view. A header whose dependent .cpp is textually +// unchanged (a dictionary gaining a field grows `Generated.h` but not the +// `Generated.cpp` that embeds it) would otherwise only be noticed on the +// NEXT build, linking an object compiled against the old struct layout. For a +// struct that crosses the C++/Rust FFI boundary that is a memory-corrupting +// build, not a stale one. function listOutputs(): void { const outputs: string[] = []; for (const type of getNamedExports()) { + if (type.hasCppHeader) outputs.push(cppHeaderPath(type)); if (type.hasCppSource) outputs.push(cppSourcePath(type)); } process.stdout.write(outputs.join(";")); diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 8d2305b5ef50..5b59a1412d73 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -254,6 +254,23 @@ function tlsHandshakeError(verifyError) { return new ConnResetException("socket hang up"); } +/** + * `requestOCSP` is honored on every transport backed by a real socket, but TLS + * over a generic Duplex stream (and Windows named pipes) runs on the SSLWrapper + * engine, whose private memory BIO leaves the native status callback with no + * socket to dispatch 'OCSPResponse' on. Asking for a staple we could never hand + * back would be the silent no-op this option already suffered from, so say so. + */ +let warnedOCSPUnsupported = false; +function applyRequestOCSP(socket) { + if (socket.requestOCSP?.() !== false || warnedOCSPUnsupported) return; + warnedOCSPUnsupported = true; + process.emitWarning( + "requestOCSP is ignored for TLS over a Duplex stream or a named pipe: no OCSP response will be requested or delivered", + "BunTLSWarning", + ); +} + const SocketHandlers: SocketHandler = { close(socket, err) { const self = socket.data; @@ -595,6 +612,61 @@ function onSNIResolution(state, err, context) { } } +// --- 'OCSPRequest' dispatch helpers (hoisted: no per-handshake closures) --- + +let isArrayBufferView; + +// Normalizes a non-Error rejection (cb(true), cb("reason"), throw true): the +// native dispatch recognizes Error returns as the abort signal, and a literal +// `true` would collide with the handshake-suspension sentinel. +function toOCSPError(err) { + return err instanceof Error ? err : Object.assign(new Error("OCSPRequest error"), { reason: err }); +} + +// Node's `callback(err)` destroys the connection, which on a server socket that +// never reached 'secureConnection' surfaces as 'tlsClientError'. Going through +// destroy(err) here instead would emit 'error' on a socket that has no listener +// yet, i.e. an uncaught exception. +function failOCSPConnection(socketHandle, err) { + const self = socketHandle?.data; + if (!self || self._hadError) return; + self._hadError = true; + self.emit("_tlsError", err); + self.server?.emit("tlsClientError", err, self); + self.destroy(); +} + +// The completion callback an 'OCSPRequest' listener is handed. Synchronous +// resolutions are carried by ocspRequest's return value; asynchronous ones +// complete the parked handshake via resumeOCSP. +// https://nodejs.org/api/tls.html#event-ocsprequest +function onOCSPResolution(state, err, response) { + if (state.settled) return; // the callback must resolve exactly once + state.settled = true; + const { socketHandle } = state; + if (!err && response != null) { + // Not `$isTypedArrayView`: it excludes DataView, which the native side + // accepts and Node's validateBuffer (where this error comes from) allows. + isArrayBufferView ??= require("node:util/types").isArrayBufferView; + if (!isArrayBufferView(response)) { + err = $ERR_INVALID_ARG_TYPE("response", ["Buffer", "TypedArray", "DataView"], response); + } + } + if (err) { + state.failed = toOCSPError(err); + failOCSPConnection(socketHandle, state.failed); + return; + } + state.response = response == null ? undefined : response; + if (!state.suspended) return; // synchronous resolution - ocspRequest's return carries it + // A response the TLS layer refuses to stage leaves the handshake parked, so + // drop the connection rather than complete it without the staple. + if (socketHandle?.resumeOCSP(state.response) === false) { + state.failed = new Error("Failed to attach the OCSP response to the TLS connection"); + failOCSPConnection(socketHandle, state.failed); + } +} + const ServerHandlers: SocketHandler = { data(socket, buffer) { const { data: self } = socket; @@ -697,6 +769,39 @@ const ServerHandlers: SocketHandler = { } return state.selected; }, + ocspRequest(server, socketHandle, certificate, issuer) { + // The client asked for a stapled OCSP response. Returns what this + // handshake should staple: + // - a Buffer (the listener answered synchronously) + // - undefined to staple nothing (no listener, or callback(null, null)) + // - `true` to SUSPEND the handshake until `socketHandle.resumeOCSP(...)` + // - an Error: the listener reported one and the connection was destroyed + if (!(server?.listenerCount?.("OCSPRequest") > 0)) return undefined; + const state = { + socketHandle, + response: undefined, + failed: undefined, + settled: false, + suspended: false, + }; + try { + server.emit("OCSPRequest", certificate, issuer, onOCSPResolution.bind(null, state)); + } catch (err) { + state.settled = true; + state.failed = toOCSPError(err); + failOCSPConnection(socketHandle, state.failed); + } + const { failed } = state; + if (failed !== undefined) return failed; + if (!state.settled) { + // Asynchronous listener. Without a connection handle the suspension + // could never be resumed - staple nothing in that (unexpected) case. + if (!socketHandle) return undefined; + state.suspended = true; + return true; + } + return state.response; + }, close(socket, err) { $debug("Bun.Server close"); const data = this.data; @@ -1004,6 +1109,12 @@ const SocketHandlers2: SocketHandler { +class JSSocketHandlers final : public JSC::JSInternalFieldObjectImpl<16> { public: - using Base = JSC::JSInternalFieldObjectImpl<14>; + using Base = JSC::JSInternalFieldObjectImpl<16>; // Field order is ABI shared with src/runtime/socket/Handlers.rs. enum class Field : uint32_t { @@ -35,6 +35,8 @@ class JSSocketHandlers final : public JSC::JSInternalFieldObjectImpl<14> { Keylog, ServerName, ALPNCallback, + OCSPRequest, + OCSPResponse, // Not a callback: the `Bun.connect` promise, cleared once settled. Promise, }; diff --git a/src/jsc/generated.rs b/src/jsc/generated.rs index f23a92f17812..f697394dc7fd 100644 --- a/src/jsc/generated.rs +++ b/src/jsc/generated.rs @@ -201,6 +201,8 @@ pub struct SocketConfigHandlers { pub on_keylog: JSValue, pub on_server_name: JSValue, pub on_alpn_callback: JSValue, + pub on_ocsp_request: JSValue, + pub on_ocsp_response: JSValue, pub binary_type: SocketConfigHandlersBinaryType, } @@ -220,6 +222,8 @@ struct ExternSocketConfigHandlers { onKeylog: JSValue, onServerName: JSValue, onALPNCallback: JSValue, + onOCSPRequest: JSValue, + onOCSPResponse: JSValue, binary_type: SocketConfigHandlersBinaryType, } @@ -251,6 +255,8 @@ impl SocketConfigHandlers { on_keylog: ext.onKeylog, on_server_name: ext.onServerName, on_alpn_callback: ext.onALPNCallback, + on_ocsp_request: ext.onOCSPRequest, + on_ocsp_response: ext.onOCSPResponse, binary_type: ext.binary_type, } } diff --git a/src/runtime/socket/Handlers.rs b/src/runtime/socket/Handlers.rs index fd255093921c..6ce7afa3c92d 100644 --- a/src/runtime/socket/Handlers.rs +++ b/src/runtime/socket/Handlers.rs @@ -157,6 +157,12 @@ impl Handlers { pub fn on_alpn_callback(&self) -> JSValue { self.cell.on_alpn_callback() } + pub fn on_ocsp_request(&self) -> JSValue { + self.cell.on_ocsp_request() + } + pub fn on_ocsp_response(&self) -> JSValue { + self.cell.on_ocsp_response() + } /// Drops the `open` callback for every holder of this `Handlers` — a client /// socket does this after its first TLS handshake so renegotiations do not @@ -368,6 +374,8 @@ impl Handlers { let on_keylog = validated_callback!(on_keylog, "onKeylog"); let on_server_name = validated_callback!(on_server_name, "onServerName"); let on_alpn_callback = validated_callback!(on_alpn_callback, "onALPNCallback"); + let on_ocsp_request = validated_callback!(on_ocsp_request, "onOCSPRequest"); + let on_ocsp_response = validated_callback!(on_ocsp_response, "onOCSPResponse"); if on_data.is_empty() && on_writable.is_empty() { return Err(global_object.throw_invalid_arguments(format_args!( @@ -389,6 +397,8 @@ impl Handlers { on_keylog, on_server_name, on_alpn_callback, + on_ocsp_request, + on_ocsp_response, ]) } diff --git a/src/runtime/socket/JSSocketHandlers.rs b/src/runtime/socket/JSSocketHandlers.rs index 6b928a56867f..fce1d8b52f94 100644 --- a/src/runtime/socket/JSSocketHandlers.rs +++ b/src/runtime/socket/JSSocketHandlers.rs @@ -10,7 +10,7 @@ use bun_jsc::{JSGlobalObject, JSValue, Strong}; unsafe extern "C" { - /// Allocates the cell with the 13 callback fields populated barrier-free + /// Allocates the cell with the callback fields populated barrier-free /// (the cell is not yet GC-visible); the promise field starts `undefined`. safe fn Bun__SocketHandlers__create( global: &JSGlobalObject, @@ -25,7 +25,7 @@ unsafe extern "C" { index: u32, value: JSValue, ); - /// Overwrites all 13 callback fields on a live cell with one trailing + /// Overwrites all callback fields on a live cell with one trailing /// write barrier. safe fn Bun__SocketHandlers__setCallbacks( global: &JSGlobalObject, @@ -54,6 +54,8 @@ enum Field { Keylog, ServerName, AlpnCallback, + OcspRequest, + OcspResponse, /// Not a callback: the pending `Bun.connect` promise, cleared once settled. Promise, } @@ -123,6 +125,8 @@ impl JSSocketHandlers { on_keylog => Keylog, on_server_name => ServerName, on_alpn_callback => AlpnCallback, + on_ocsp_request => OcspRequest, + on_ocsp_response => OcspResponse, } /// Replaces every callback on a live cell. `JSValue::ZERO` entries clear diff --git a/src/runtime/socket/Listener.rs b/src/runtime/socket/Listener.rs index 7f19e2ccdf49..409edc7a4e82 100644 --- a/src/runtime/socket/Listener.rs +++ b/src/runtime/socket/Listener.rs @@ -509,6 +509,15 @@ impl Listener { // S008: `ListenSocket` is an `opaque_ffi!` ZST - safe deref. bun_opaque::opaque_deref_mut(listen_socket).on_server_name(us_dispatch_server_name); } + // Register the 'OCSPRequest' dispatch when the JS config provided an + // `ocspRequest` handler. `us_cert_cb` only reaches it for handshakes + // whose ClientHello asked for stapling, so a server that never + // staples pays nothing. + if !this_ref.handlers.on_ocsp_request().is_empty() { + // S008: `ListenSocket` is an `opaque_ffi!` ZST - safe deref. + bun_opaque::opaque_deref_mut(listen_socket) + .on_ocsp_request(us_dispatch_ocsp_request); + } } let this = scopeguard::ScopeGuard::into_inner(cleanup); // ownership transfers to JS wrapper @@ -1869,3 +1878,118 @@ pub(crate) extern "C" fn us_dispatch_server_name( } core::ptr::null_mut() } + +/// `openssl.c`'s `us_cert_cb` calls this once per server handshake whose +/// ClientHello asked for OCSP stapling, on listeners that registered an +/// `ocspRequest` handler. The JS handler emits `'OCSPRequest'` with the +/// connection's certificate and issuer (DER) and reports back: +/// - a `Buffer`/TypedArray -> staple it on this handshake +/// - `undefined`/`null`/`false` -> staple nothing (no listeners, or the +/// handler answered `callback(null, null)`) +/// - `true` -> the handler is asynchronous; suspend the handshake until +/// `handle.resumeOCSP(...)` (`*abort_handshake = 2`) +/// - anything else (an Error) -> the handler reported one and destroyed the +/// socket; abort (`*abort_handshake = 1`) +/// +/// # Safety +/// `ls` is a live listen socket whose accept-group ext holds a `*mut Listener`, +/// and `socket` is the live accepted socket processing this ClientHello. +/// JS-thread only. +pub(crate) extern "C" fn us_dispatch_ocsp_request( + ls: *mut uws_sys::ListenSocket, + socket: *mut uws_sys::us_socket_t, + abort_handshake: *mut core::ffi::c_int, +) { + jsc::mark_binding!(); + if ls.is_null() || socket.is_null() { + return; + } + // The accept group's ext holds the owning `*mut Listener` for the lifetime + // of the listen socket. S008: `ListenSocket` is an `opaque_ffi!` ZST. + let listener_ptr: *mut Listener = bun_opaque::opaque_deref_mut(ls).group().owner::(); + if listener_ptr.is_null() { + return; + } + // SAFETY: see above - the listen socket keeps the `Listener` alive for the + // duration of this synchronous handshake dispatch. + let listener = unsafe { bun_ptr::ThisPtr::new(listener_ptr) }; + let handlers = &listener.handlers; + if handlers.vm.is_shutting_down() { + return; + } + let callback = handlers.on_ocsp_request(); + if callback.is_empty() { + return; + } + let global = handlers.global_object; + + // Reports the outcome to `us_cert_cb` through its out-parameter: 1 = abort, + // 2 = suspend. 0 (the initial value) is "proceed". + let report = |outcome: core::ffi::c_int| { + if !abort_handshake.is_null() { + // SAFETY: the C caller passes a live out-parameter for the duration + // of this synchronous dispatch. + unsafe { *abort_handshake = outcome }; + } + }; + + // The accepted socket processing this ClientHello: its JS wrapper carries + // the owning node:tls socket and is the resume handle an asynchronous + // handler uses (`handle.resumeOCSP(...)`). + let s_ref = uws_sys::us_socket_t::opaque_mut(socket); + if s_ref.kind() != uws_sys::SocketKind::BunSocketTls { + return; + } + let Some(tls) = *s_ref.ext::>>() else { + return; + }; + let socket_handle = tls.get_this_value(&global); + + let (certificate, issuer) = match tls.ocsp_certificates(&global) { + Ok(pair) => pair, + Err(err) => { + // Clear the exception before unwinding into BoringSSL, and drop the + // connection rather than silently serving it without the staple the + // client asked for. + global.take_exception(err); + report(1); + return; + } + }; + + let this_value = listener + .strong_data + .get() + .get() + .unwrap_or(JSValue::UNDEFINED); + let result = match callback.call( + &global, + this_value, + &[this_value, socket_handle, certificate, issuer], + ) { + Ok(v) => v, + Err(err) => global.take_exception(err), + }; + + if result.is_boolean() { + if result.to_boolean() { + report(2); + } + return; + } + if result.is_undefined_or_null() { + return; + } + if let Some(buffer) = result.as_array_buffer(&global) { + let bytes = buffer.byte_slice(); + // `set_ocsp_response` fails on a socket the handler destroyed while it + // ran, and on bytes BoringSSL rejects; either way there is nothing left + // to serve, so abort instead of finishing the handshake unstapled. + if !bytes.is_empty() && !tls.socket.get().set_ocsp_response(bytes) { + report(1); + } + return; + } + // An Error (or any other shape): the JS side already destroyed the socket. + report(1); +} diff --git a/src/runtime/socket/SocketConfig.bindv2.ts b/src/runtime/socket/SocketConfig.bindv2.ts index bb320349ee5e..123f58ca00c0 100644 --- a/src/runtime/socket/SocketConfig.bindv2.ts +++ b/src/runtime/socket/SocketConfig.bindv2.ts @@ -27,6 +27,8 @@ export const Handlers = b.dictionary( keylog: { type: b.RawAny, internalName: "onKeylog" }, serverName: { type: b.RawAny, internalName: "onServerName" }, alpnCallback: { type: b.RawAny, internalName: "onALPNCallback" }, + ocspRequest: { type: b.RawAny, internalName: "onOCSPRequest" }, + ocspResponse: { type: b.RawAny, internalName: "onOCSPResponse" }, binaryType: { type: BinaryType, default: "buffer", diff --git a/src/runtime/socket/socket_body.rs b/src/runtime/socket/socket_body.rs index 87aeb928f003..28e413f04b2f 100644 --- a/src/runtime/socket/socket_body.rs +++ b/src/runtime/socket/socket_body.rs @@ -1830,6 +1830,55 @@ impl NewSocket { Ok(()) } + /// The server's stapled OCSP response (`None` when it stapled nothing), + /// delivered from inside the client handshake. + /// + /// Takes `ThisPtr` for the same re-entrancy reason as `on_session`: + /// the JS listener may destroy this socket, which is the documented way to + /// reject a staple. Returns nothing: the caller unwinds straight into + /// BoringSSL's handshake, so every exception is consumed here rather than + /// left pending. + pub fn on_ocsp_response(this: bun_ptr::ThisPtr, response: Option<&[u8]>) { + jsc::mark_binding!(); + if this.socket.get().is_detached() { + return; + } + if !this.has_handlers() { + return; + } + let handlers = this.get_handlers(); + if handlers.vm.is_shutting_down() { + return; + } + let callback = handlers.on_ocsp_response(); + if callback.is_empty() { + return; + } + let scope = handlers.enter(); + let global = handlers.global_object; + let this_value = this.get_this_value(&global); + let response_value = match response { + Some(bytes) => match jsc::ArrayBuffer::create_buffer(&global, bytes) { + Ok(buffer) => buffer, + Err(err) => { + let err_value = global.take_exception(err); + let _ = handlers.call_error_handler(this_value, &[this_value, err_value]); + this.exit_scope(scope); + return; + } + }, + None => JSValue::NULL, + }; + let result = match callback.call(&global, this_value, &[this_value, response_value]) { + Ok(v) => v, + Err(err) => global.take_exception(err), + }; + if let Some(err_value) = result.to_error() { + let _ = handlers.call_error_handler(this_value, &[this_value, err_value]); + } + this.exit_scope(scope); + } + /// Takes `ThisPtr` for the same re-entrancy reason as `on_writable`. pub fn on_close( this: bun_ptr::ThisPtr, @@ -3553,6 +3602,34 @@ impl NewSocket { Ok(JSValue::UNDEFINED) } } + /// `(certificateDER, issuerDER)` for the certificate this server connection + /// is about to send - the arguments Node's `'OCSPRequest'` listener takes. + /// Either slot is `undefined` when it cannot be resolved. + pub(crate) fn ocsp_certificates(&self, g: &JSGlobalObject) -> JsResult<(JSValue, JSValue)> { + if !SSL { + return Ok((JSValue::UNDEFINED, JSValue::UNDEFINED)); + } + let Some(ssl_ptr) = self.socket.get().ssl() else { + return Ok((JSValue::UNDEFINED, JSValue::UNDEFINED)); + }; + tls_socket_functions::ocsp_certificates(ssl_ptr, g) + } + #[bun_jsc::host_fn(method)] + pub fn request_ocsp(this: &Self, g: &JSGlobalObject, f: &CallFrame) -> JsResult { + if SSL { + tls_socket_functions::request_ocsp(Self::as_tls(this), g, f) + } else { + Ok(JSValue::FALSE) + } + } + #[bun_jsc::host_fn(method)] + pub fn resume_ocsp(this: &Self, g: &JSGlobalObject, f: &CallFrame) -> JsResult { + if SSL { + tls_socket_functions::resume_ocsp(Self::as_tls(this), g, f) + } else { + Ok(JSValue::TRUE) + } + } #[bun_jsc::host_fn(method)] pub fn set_session(this: &Self, g: &JSGlobalObject, f: &CallFrame) -> JsResult { if SSL { diff --git a/src/runtime/socket/sockets.classes.ts b/src/runtime/socket/sockets.classes.ts index 834dd71caa67..198f16e6e000 100644 --- a/src/runtime/socket/sockets.classes.ts +++ b/src/runtime/socket/sockets.classes.ts @@ -121,6 +121,14 @@ function generate(ssl) { fn: "resumeSNI", length: 2, }, + requestOCSP: { + fn: "requestOCSP", + length: 0, + }, + resumeOCSP: { + fn: "resumeOCSP", + length: 1, + }, setKeepAlive: { fn: "setKeepAlive", length: 2, diff --git a/src/runtime/socket/tls_socket_functions.rs b/src/runtime/socket/tls_socket_functions.rs index bad552304938..f8ffbe529b1a 100644 --- a/src/runtime/socket/tls_socket_functions.rs +++ b/src/runtime/socket/tls_socket_functions.rs @@ -266,6 +266,9 @@ pub(super) mod ffi { ) -> c_int; // Returns X509_V_OK (0) when `issuer` could have issued `subject`. pub(crate) fn X509_check_issued(issuer: *mut X509, subject: *mut X509) -> c_int; + // DER-encodes `x`. A null `outp` only measures; otherwise it must point + // at a writable buffer of that size and is advanced past the output. + pub(crate) fn i2d_X509(x: *mut X509, outp: *mut *mut u8) -> c_int; } } use crate::node::StringOrBuffer; @@ -1266,6 +1269,154 @@ pub(super) fn is_session_reused( )) } +// ── OCSP stapling ────────────────────────────────────────────────────────── + +/// DER-encodes `cert` (a live X509) into a JS `Buffer`, or `undefined` when it +/// cannot be encoded. +fn x509_to_der_buffer(cert: *mut boringssl::X509, global: &JSGlobalObject) -> JsResult { + // SAFETY: `cert` is a live X509; a null out-ptr requests the size only. + let size = unsafe { ffi::i2d_X509(cert, core::ptr::null_mut()) }; + let Ok(size) = usize::try_from(size) else { + return Ok(JSValue::UNDEFINED); + }; + if size == 0 { + return Ok(JSValue::UNDEFINED); + } + let buffer = JSValue::create_buffer_from_length(global, size)?; + let Some(array_buffer) = buffer.as_array_buffer(global) else { + return Ok(JSValue::UNDEFINED); + }; + let mut ptr = array_buffer.ptr; + // SAFETY: `ptr` points at `size` writable bytes owned by the JS buffer. + let written = unsafe { ffi::i2d_X509(cert, &raw mut ptr) }; + // A second encode that disagrees with the sizing pass would leave the tail + // of the buffer as the zeroes `create_buffer_from_length` left there. + if written != size as c_int { + return Ok(JSValue::UNDEFINED); + } + Ok(buffer) +} + +/// The certificate that issued `leaf`, as Node's `SecureContext.getIssuer()` +/// resolves it: first the extra chain certificates configured on the context, +/// then its trust store. Returns a +1 reference the caller must free. +fn resolve_issuer( + ssl_ptr: *mut boringssl::SSL, + leaf: *mut boringssl::X509, +) -> *mut boringssl::X509 { + let ctx = ffi::SSL_get_SSL_CTX(boringssl::SSL::opaque_ref(ssl_ptr)); + if ctx.is_null() { + return core::ptr::null_mut(); + } + + // SAFETY: `ctx` is the live parent CTX of a live SSL. `chain` is a valid + // out-param; the stack it receives (and every X509 in it) is borrowed from + // the CTX, which outlives this synchronous walk. `X509_check_issued` + // borrows both certificates. The store_ctx created below is freed on every + // exit, and `get1_issuer` writes a +1 reference we hand to the caller. + unsafe { + let mut chain: *mut c_void = core::ptr::null_mut(); + if ffi::SSL_CTX_get0_chain_certs(ctx, &raw mut chain) == 1 && !chain.is_null() { + let count = ffi::OPENSSL_sk_num(chain); + let stack = boringssl::struct_stack_st_X509::opaque_ref(chain.cast()); + for i in 0..count { + let candidate = ffi::sk_X509_value(stack, i); + if !candidate.is_null() && ffi::X509_check_issued(candidate, leaf) == 0 { + ffi::X509_up_ref(boringssl::X509::opaque_ref(candidate)); + return candidate; + } + } + } + + let store = ffi::SSL_CTX_get_cert_store(boringssl::SSL_CTX::opaque_ref(ctx)); + if store.is_null() { + return core::ptr::null_mut(); + } + let store_ctx = ffi::X509_STORE_CTX_new(); + if store_ctx.is_null() { + return core::ptr::null_mut(); + } + let mut issuer: *mut boringssl::X509 = core::ptr::null_mut(); + if ffi::X509_STORE_CTX_init( + store_ctx, + store, + core::ptr::null_mut(), + core::ptr::null_mut(), + ) != 1 + || ffi::X509_STORE_CTX_get1_issuer(&raw mut issuer, store_ctx, leaf) <= 0 + { + issuer = core::ptr::null_mut(); + } + ffi::X509_STORE_CTX_free(store_ctx); + issuer + } +} + +/// `(certificateDER, issuerDER)` for the certificate this server connection is +/// about to send - the arguments Node's `'OCSPRequest'` listener receives. +/// Either slot is `undefined` when it cannot be resolved. +pub(super) fn ocsp_certificates( + ssl_ptr: *mut boringssl::SSL, + global: &JSGlobalObject, +) -> JsResult<(JSValue, JSValue)> { + let cert = ffi::SSL_get_certificate(boringssl::SSL::opaque_ref(ssl_ptr)); + if cert.is_null() { + return Ok((JSValue::UNDEFINED, JSValue::UNDEFINED)); + } + let cert_der = x509_to_der_buffer(cert, global)?; + + let issuer = resolve_issuer(ssl_ptr, cert); + if issuer.is_null() { + return Ok((cert_der, JSValue::UNDEFINED)); + } + // SAFETY: `issuer` is the +1 reference `resolve_issuer` handed us. + let _guard = scopeguard::guard(issuer, |c| unsafe { boringssl::X509_free(c) }); + Ok((cert_der, x509_to_der_buffer(issuer, global)?)) +} + +/// `handle.requestOCSP()` - client-side: ask the server to staple an OCSP +/// response. Must run before the ClientHello goes out. Returns false when this +/// transport cannot deliver one (see `NewSocketHandler::request_ocsp_stapling`). +pub(super) fn request_ocsp( + this: &This, + _global: &JSGlobalObject, + _frame: &CallFrame, +) -> JsResult { + Ok(JSValue::from(this.socket.get().request_ocsp_stapling())) +} + +/// `handle.resumeOCSP(responseOrNull)` - resumes a server handshake suspended +/// by an asynchronous `'OCSPRequest'` handler, stapling `response` when the +/// handler produced one. Returns false when the response was rejected: the +/// handshake stays parked and the caller tears the connection down instead of +/// completing it without the staple the client asked for. +pub(super) fn resume_ocsp( + this: &This, + global: &JSGlobalObject, + frame: &CallFrame, +) -> JsResult { + let socket = this.socket.get(); + if socket.is_detached() { + return Ok(JSValue::TRUE); + } + let args = frame.arguments_old::<1>(); + if args.len >= 1 && !args.ptr[0].is_undefined_or_null() { + let Some(buffer) = args.ptr[0].as_array_buffer(global) else { + return Err(global.throw_invalid_arguments(format_args!( + "Expected the OCSP response to be a Buffer or TypedArray" + ))); + }; + let bytes = buffer.byte_slice(); + // An empty response is Node's "nothing to staple": its setOCSPResponse + // accepts one and nothing ever reaches the wire. Not a failure. + if !bytes.is_empty() && !socket.set_ocsp_response(bytes) { + return Ok(JSValue::FALSE); + } + } + socket.ocsp_resolve(); + Ok(JSValue::TRUE) +} + pub(super) fn set_verify_mode( this: &This, global: &JSGlobalObject, diff --git a/src/runtime/socket/uws_dispatch.rs b/src/runtime/socket/uws_dispatch.rs index 508b43ab029c..8e3a8afba999 100644 --- a/src/runtime/socket/uws_dispatch.rs +++ b/src/runtime/socket/uws_dispatch.rs @@ -277,3 +277,37 @@ pub unsafe extern "C" fn us_dispatch_keylog(s: *mut us_socket_t, data: *const u8 let slice = unsafe { core::slice::from_raw_parts(data, len) }; let _ = TLSSocket::on_keylog(tls, slice); } + +/// Hands the server's stapled OCSP response to the JS `ocspResponse` handler. +/// `len < 0` means nothing was stapled (Node emits `'OCSPResponse'` with null). +/// Unlike session/keylog this runs from *inside* the handshake, so a listener +/// that rejects the staple can still abort the connection by destroying it. +/// +/// # Safety +/// `openssl.c` must pass a live, non-null `s` whose ext slot holds a valid +/// `*mut TLSSocket`, and `data` must point to `len` readable bytes when +/// `len >= 0`. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn us_dispatch_ocsp_response( + s: *mut us_socket_t, + data: *const u8, + len: c_int, +) { + let s_ref = us_socket_t::opaque_mut(s); + if s_ref.kind() != SocketKind::BunSocketTls { + return; + } + type TLSSocket = super::NewSocket; + let Some(tls) = *s_ref.ext::>>() else { + return; + }; + let response = match usize::try_from(len) { + // SAFETY: `data` points to `len` readable bytes owned by the TLS session + // for the duration of this call. + Ok(len) if !data.is_null() => Some(unsafe { core::slice::from_raw_parts(data, len) }), + _ => None, + }; + // `on_ocsp_response` consumes any exception itself - this returns straight + // into BoringSSL. + TLSSocket::on_ocsp_response(tls, response); +} diff --git a/src/uws_sys/ListenSocket.rs b/src/uws_sys/ListenSocket.rs index db68a98429ae..efacd25d3de5 100644 --- a/src/uws_sys/ListenSocket.rs +++ b/src/uws_sys/ListenSocket.rs @@ -116,6 +116,17 @@ impl ListenSocket { ) { us_listen_socket_on_server_name(self, cb) } + + /// Registers the server-side `'OCSPRequest'` dispatch. The callback reports + /// back through its `*mut c_int` out-param: 0 = done, 1 = errored (the + /// handler destroyed the socket), 2 = asynchronous (suspend the handshake + /// until `us_socket_t::ocsp_resolve`). + pub fn on_ocsp_request( + &mut self, + cb: extern "C" fn(*mut ListenSocket, *mut us_socket_t, *mut c_int), + ) { + us_listen_socket_on_ocsp_request(self, cb) + } } // This file IS the *_sys crate, so externs live here. @@ -142,4 +153,8 @@ unsafe extern "C" { ls: &mut ListenSocket, cb: extern "C" fn(*mut ListenSocket, *const c_char, *mut c_int, *mut c_void) -> *mut c_void, ); + safe fn us_listen_socket_on_ocsp_request( + ls: &mut ListenSocket, + cb: extern "C" fn(*mut ListenSocket, *mut us_socket_t, *mut c_int), + ); } diff --git a/src/uws_sys/socket.rs b/src/uws_sys/socket.rs index 6f377c8b0228..eb76244e00ca 100644 --- a/src/uws_sys/socket.rs +++ b/src/uws_sys/socket.rs @@ -565,6 +565,38 @@ impl NewSocketHandler { } } + /// Server-side: stage the DER OCSP response this connection staples. + pub fn set_ocsp_response(&self, response: &[u8]) -> bool { + match self.socket { + InternalSocket::Connected(s) => sock(s).set_ocsp_response(response), + _ => false, + } + } + + /// Resume a handshake suspended by an asynchronous `'OCSPRequest'` handler. + pub fn ocsp_resolve(&self) { + if let InternalSocket::Connected(s) = self.socket { + sock(s).ocsp_resolve(); + } + } + + /// Client-side: request a stapled OCSP response from the server. Returns + /// false when the transport cannot carry one: TLS over a generic JS `Duplex` + /// and Windows named pipes run on the SSLWrapper engine, whose private memory + /// BIO gives `us_ocsp_status_cb` no socket to dispatch `'OCSPResponse'` on + /// (hence its `us_ssl_loop_bio_idx` gate). Putting `status_request` on the + /// wire and then dropping the answer would be worse than not asking, so the + /// caller reports the limitation instead. + pub fn request_ocsp_stapling(&self) -> bool { + match self.socket { + InternalSocket::Connected(s) => { + sock(s).request_ocsp_stapling(); + true + } + _ => false, + } + } + // ── TLS ───────────────────────────────────────────────────────────────── /// Kick TLS open (ClientHello / accept) on an already-connected socket. diff --git a/src/uws_sys/us_socket_t.rs b/src/uws_sys/us_socket_t.rs index e4e148e4e6e9..26393993caa9 100644 --- a/src/uws_sys/us_socket_t.rs +++ b/src/uws_sys/us_socket_t.rs @@ -197,6 +197,25 @@ impl us_socket_t { c::us_socket_sni_resolve(self, ctx, error as c_int); } + /// Server-side: stage the DER OCSP response this connection staples. + /// BoringSSL copies the bytes. Returns false when they were rejected. + pub fn set_ocsp_response(&mut self, response: &[u8]) -> bool { + // SAFETY: `response` is valid for `response.len()` bytes for the call; + // BoringSSL copies it. + unsafe { c::us_socket_set_ocsp_response(self, response.as_ptr(), response.len()) != 0 } + } + + /// Resume a handshake suspended by an asynchronous `'OCSPRequest'` handler. + pub fn ocsp_resolve(&mut self) { + c::us_socket_ocsp_resolve(self); + } + + /// Client-side: request a stapled OCSP response, which arrives through the + /// `us_dispatch_ocsp_response` hook. Must run before the ClientHello. + pub fn request_ocsp_stapling(&mut self) { + c::us_socket_request_ocsp_stapling(self); + } + /// `SSL*` if TLS, else null. Use `get_fd()` for the descriptor. pub fn ssl(&mut self) -> Option<&mut bun_boringssl_sys::SSL> { if !self.is_tls() { @@ -504,6 +523,14 @@ mod c { ctx: *mut SslCtx, error: c_int, ); + // SAFETY (unsafe fn): `response` must be readable for `length` bytes. + pub(super) fn us_socket_set_ocsp_response( + s: &mut us_socket_t, + response: *const u8, + length: usize, + ) -> c_int; + pub(super) safe fn us_socket_ocsp_resolve(s: &mut us_socket_t); + pub(super) safe fn us_socket_request_ocsp_stapling(s: &mut us_socket_t); pub(super) safe fn us_socket_keepalive( s: &mut us_socket_t, enable: c_int, diff --git a/test/js/node/tls/node-tls-ocsp-duplex.fixture.ts b/test/js/node/tls/node-tls-ocsp-duplex.fixture.ts new file mode 100644 index 000000000000..0770ddfb7437 --- /dev/null +++ b/test/js/node/tls/node-tls-ocsp-duplex.fixture.ts @@ -0,0 +1,57 @@ +// `tls.connect({ socket: , requestOCSP: true })`. Runs in its own +// process because the "requestOCSP is ignored" warning fires once per process. +// Prints the two endpoints' event logs as JSON on stdout; the warning itself goes +// to stderr, which is what the caller asserts on. +import net from "node:net"; +import { Duplex } from "node:stream"; +import tls from "node:tls"; + +const cert = process.env.OCSP_CERT!; +const key = process.env.OCSP_KEY!; + +const serverLog: string[] = []; +const clientLog: string[] = []; + +const server = tls.createServer({ cert, key }, socket => { + serverLog.push("secureConnection"); + socket.end(); +}); +server.on("OCSPRequest", (_certificate, _issuer, callback) => { + serverLog.push("OCSPRequest"); + callback(null, Buffer.from("never requested")); +}); + +server.listen(0, () => { + const { port } = server.address() as net.AddressInfo; + const raw = net.connect(port, "127.0.0.1", () => { + // A plain net.Socket is adopted into a native TLS socket; wrapping it in a + // bare Duplex forces the SSLWrapper path instead. + const duplex = new Duplex({ + read() {}, + write(chunk, _encoding, callback) { + raw.write(chunk, callback); + }, + final(callback) { + raw.end(); + callback(); + }, + }); + raw.on("data", chunk => duplex.push(chunk)); + raw.on("end", () => duplex.push(null)); + + const client = tls.connect({ socket: duplex, requestOCSP: true, rejectUnauthorized: false }); + client.on("OCSPResponse", () => clientLog.push("OCSPResponse")); + client.on("secureConnect", () => { + clientLog.push("secureConnect"); + client.end(); + }); + client.on("error", () => {}); + client.on("close", () => { + raw.destroy(); + server.close(() => { + process.stdout.write(JSON.stringify({ serverLog, clientLog })); + process.exit(0); + }); + }); + }); +}); diff --git a/test/js/node/tls/node-tls-ocsp.test.ts b/test/js/node/tls/node-tls-ocsp.test.ts new file mode 100644 index 000000000000..49ada2948cfc --- /dev/null +++ b/test/js/node/tls/node-tls-ocsp.test.ts @@ -0,0 +1,219 @@ +// OCSP stapling: the server-side 'OCSPRequest' event and the client-side +// 'OCSPResponse' event, the two halves of `tls.connect({ requestOCSP: true })`. +// https://nodejs.org/api/tls.html#event-ocsprequest +import { describe, expect, it } from "bun:test"; +import { bunEnv, bunExe, tls as cert1 } from "harness"; +import { AddressInfo } from "net"; +import { once } from "node:events"; +import { join } from "node:path"; +import { connect, createServer, TLSSocket } from "tls"; + +const COMMON_CERT = { ...cert1 }; + +describe("OCSP stapling", () => { + // One full connect/close cycle, reported as the two endpoints' ordered event + // logs. Events are asserted per endpoint because the interleaving between + // them is not deterministic. + async function exchange({ + serverOptions = {}, + clientOptions = {}, + onOCSPRequest, + onOCSPResponse, + awaitServer = true, + }: { + serverOptions?: object; + clientOptions?: object; + onOCSPRequest?: (callback: (err: Error | null, response?: unknown) => void) => void; + onOCSPResponse?: (client: TLSSocket, response: Buffer | null) => void; + awaitServer?: boolean; + }) { + const serverLog: string[] = []; + const clientLog: string[] = []; + const serverSettled = Promise.withResolvers(); + // Not `once(client, "close")`: that rejects as soon as the socket emits + // 'error', and the error paths below are exactly what some cases assert. + const clientClosed = Promise.withResolvers(); + + const server = createServer({ ...COMMON_CERT, ...serverOptions }, socket => { + serverLog.push("secureConnection"); + serverSettled.resolve(); + socket.end(); + }); + if (onOCSPRequest) { + server.on("OCSPRequest", (certificate, issuer, callback) => { + serverLog.push( + `OCSPRequest certificate=${certificate?.constructor?.name} issuer=${issuer?.constructor?.name ?? "undefined"}`, + ); + onOCSPRequest(callback); + }); + } + server.on("tlsClientError", err => { + serverLog.push(`tlsClientError ${err.message}`); + serverSettled.resolve(); + }); + + try { + server.listen(0); + await once(server, "listening"); + const client = connect({ + port: (server.address() as AddressInfo).port, + rejectUnauthorized: false, + ...clientOptions, + }); + client.on("OCSPResponse", response => { + clientLog.push(`OCSPResponse ${response === null ? "null" : response.toString()}`); + onOCSPResponse?.(client, response); + }); + client.on("secureConnect", () => { + clientLog.push("secureConnect"); + client.end(); + }); + client.on("error", err => clientLog.push(`clientError ${(err as any).code ?? err.message}`)); + client.on("close", () => clientClosed.resolve()); + await Promise.all([clientClosed.promise, awaitServer ? serverSettled.promise : Promise.resolve()]); + } finally { + server.close(); + } + return { serverLog, clientLog }; + } + + it("staples a response the server produces asynchronously", async () => { + const { serverLog, clientLog } = await exchange({ + clientOptions: { requestOCSP: true }, + onOCSPRequest: callback => setImmediate(callback, null, Buffer.from("hello ocsp")), + }); + expect(serverLog).toEqual(["OCSPRequest certificate=Buffer issuer=undefined", "secureConnection"]); + expect(clientLog).toEqual(["OCSPResponse hello ocsp", "secureConnect"]); + }); + + it("staples a response the server produces synchronously", async () => { + const { serverLog, clientLog } = await exchange({ + clientOptions: { requestOCSP: true }, + onOCSPRequest: callback => callback(null, Buffer.from("sync ocsp")), + }); + expect(serverLog).toEqual(["OCSPRequest certificate=Buffer issuer=undefined", "secureConnection"]); + expect(clientLog).toEqual(["OCSPResponse sync ocsp", "secureConnect"]); + }); + + // Node's validateBuffer accepts any ArrayBufferView, DataView included, and so + // does the native side; only the JS guard could have rejected it. + it("staples a response handed back as a DataView", async () => { + const { serverLog, clientLog } = await exchange({ + clientOptions: { requestOCSP: true }, + onOCSPRequest: callback => { + const bytes = Buffer.from("dataview ocsp"); + callback(null, new DataView(bytes.buffer, bytes.byteOffset, bytes.byteLength)); + }, + }); + expect(serverLog).toEqual(["OCSPRequest certificate=Buffer issuer=undefined", "secureConnection"]); + expect(clientLog).toEqual(["OCSPResponse dataview ocsp", "secureConnect"]); + }); + + it("emits OCSPResponse with null when the server declines to staple", async () => { + const { serverLog, clientLog } = await exchange({ + clientOptions: { requestOCSP: true }, + onOCSPRequest: callback => setImmediate(callback, null, null), + }); + expect(serverLog).toEqual(["OCSPRequest certificate=Buffer issuer=undefined", "secureConnection"]); + expect(clientLog).toEqual(["OCSPResponse null", "secureConnect"]); + }); + + it("emits OCSPResponse with null when the server has no OCSPRequest listener", async () => { + const { serverLog, clientLog } = await exchange({ clientOptions: { requestOCSP: true } }); + expect(serverLog).toEqual(["secureConnection"]); + expect(clientLog).toEqual(["OCSPResponse null", "secureConnect"]); + }); + + // Node stages an empty response and nothing ever reaches the wire, so the + // client sees no staple and the handshake completes. + it("treats an empty response as no staple", async () => { + const { serverLog, clientLog } = await exchange({ + clientOptions: { requestOCSP: true }, + onOCSPRequest: callback => setImmediate(callback, null, Buffer.alloc(0)), + }); + expect(serverLog).toEqual(["OCSPRequest certificate=Buffer issuer=undefined", "secureConnection"]); + expect(clientLog).toEqual(["OCSPResponse null", "secureConnect"]); + }); + + it("never emits OCSPRequest when the client did not ask for stapling", async () => { + const { serverLog, clientLog } = await exchange({ + onOCSPRequest: callback => callback(null, Buffer.from("never sent")), + }); + expect(serverLog).toEqual(["secureConnection"]); + expect(clientLog).toEqual(["secureConnect"]); + }); + + it("staples over TLSv1.2", async () => { + const { serverLog, clientLog } = await exchange({ + clientOptions: { requestOCSP: true, maxVersion: "TLSv1.2" }, + onOCSPRequest: callback => setImmediate(callback, null, Buffer.from("tls 1.2 ocsp")), + }); + expect(serverLog).toEqual(["OCSPRequest certificate=Buffer issuer=undefined", "secureConnection"]); + expect(clientLog).toEqual(["OCSPResponse tls 1.2 ocsp", "secureConnect"]); + }); + + it("passes the issuer when the server context can resolve one", async () => { + const { serverLog, clientLog } = await exchange({ + serverOptions: { ca: [COMMON_CERT.cert] }, + clientOptions: { requestOCSP: true }, + onOCSPRequest: callback => callback(null, Buffer.from("with issuer")), + }); + expect(serverLog).toEqual(["OCSPRequest certificate=Buffer issuer=Buffer", "secureConnection"]); + expect(clientLog).toEqual(["OCSPResponse with issuer", "secureConnect"]); + }); + + it("reports callback(err) through tlsClientError and drops the connection", async () => { + const { serverLog, clientLog } = await exchange({ + clientOptions: { requestOCSP: true }, + onOCSPRequest: callback => setImmediate(callback, new Error("ocsp lookup failed")), + }); + expect(serverLog).toEqual(["OCSPRequest certificate=Buffer issuer=undefined", "tlsClientError ocsp lookup failed"]); + expect(clientLog).toEqual(["clientError ECONNRESET"]); + }); + + it("rejects a response that is not a Buffer", async () => { + const { serverLog, clientLog } = await exchange({ + clientOptions: { requestOCSP: true }, + // @ts-expect-error the listener is allowed to misbehave + onOCSPRequest: callback => setImmediate(callback, null, "not a buffer"), + }); + expect(serverLog).toEqual([ + "OCSPRequest certificate=Buffer issuer=undefined", + "tlsClientError The \"response\" argument must be an instance of Buffer, TypedArray, or DataView. Received type string ('not a buffer')", + ]); + expect(clientLog).toEqual(["clientError ECONNRESET"]); + }); + + it("destroying the client from the OCSPResponse listener stops the connection", async () => { + const { clientLog } = await exchange({ + clientOptions: { requestOCSP: true }, + onOCSPRequest: callback => callback(null, Buffer.from("rejected by client")), + onOCSPResponse: client => client.destroy(), + awaitServer: false, + }); + expect(clientLog).toEqual(["OCSPResponse rejected by client"]); + }); +}); + +// TLS over a generic Duplex stream runs on the SSLWrapper engine, whose private +// memory BIO leaves the native status callback with no socket to dispatch +// 'OCSPResponse' on. Node honors `requestOCSP` there; Bun cannot yet, so it says +// so rather than putting `status_request` on the wire and dropping the answer. +describe("OCSP stapling over a Duplex stream", () => { + it("warns that requestOCSP is ignored instead of silently dropping it", async () => { + // A fresh process: the warning is emitted once per process, so an earlier + // test tripping it would leave nothing here to observe. + await using proc = Bun.spawn({ + cmd: [bunExe(), join(import.meta.dir, "node-tls-ocsp-duplex.fixture.ts")], + env: { ...bunEnv, OCSP_CERT: COMMON_CERT.cert, OCSP_KEY: COMMON_CERT.key }, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + const warning = + "requestOCSP is ignored for TLS over a Duplex stream or a named pipe: no OCSP response will be requested or delivered"; + expect(stderr.split(warning).length - 1).toBe(1); + expect(JSON.parse(stdout)).toEqual({ serverLog: ["secureConnection"], clientLog: ["secureConnect"] }); + expect(exitCode).toBe(0); + }); +}); From 3159ecf69eabb32ca3506413f1902e21a1c787e2 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 9 Jul 2026 17:07:07 +0000 Subject: [PATCH 2/2] tls: treat a falsy OCSPRequest callback response as no staple Node's onOCSP (internal/tls/wrap.js) gates on `if (response)`, not `response != null`: callback(null, 0) / callback(null, false) / callback(null, "") complete the handshake unstapled. A truthy non-Buffer is still rejected the way Node's native setOCSPResponse would (THROW_AND_RETURN_IF_NOT_BUFFER), only routed through 'tlsClientError' instead of landing as an uncaught exception in the listener's call stack. --- src/js/node/net.ts | 10 ++++++---- test/js/node/tls/node-tls-ocsp.test.ts | 13 +++++++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 5b59a1412d73..fe85b2d90877 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -644,9 +644,11 @@ function onOCSPResolution(state, err, response) { if (state.settled) return; // the callback must resolve exactly once state.settled = true; const { socketHandle } = state; - if (!err && response != null) { - // Not `$isTypedArrayView`: it excludes DataView, which the native side - // accepts and Node's validateBuffer (where this error comes from) allows. + // Node's `if (response)` gate: any falsy response means "nothing to staple". + // A truthy non-Buffer is rejected the way Node's native setOCSPResponse would + // (THROW_AND_RETURN_IF_NOT_BUFFER), only routed through 'tlsClientError' + // instead of landing as an uncaught exception in the listener's call stack. + if (!err && response) { isArrayBufferView ??= require("node:util/types").isArrayBufferView; if (!isArrayBufferView(response)) { err = $ERR_INVALID_ARG_TYPE("response", ["Buffer", "TypedArray", "DataView"], response); @@ -657,7 +659,7 @@ function onOCSPResolution(state, err, response) { failOCSPConnection(socketHandle, state.failed); return; } - state.response = response == null ? undefined : response; + state.response = response || undefined; if (!state.suspended) return; // synchronous resolution - ocspRequest's return carries it // A response the TLS layer refuses to stage leaves the handshake parked, so // drop the connection rather than complete it without the staple. diff --git a/test/js/node/tls/node-tls-ocsp.test.ts b/test/js/node/tls/node-tls-ocsp.test.ts index 49ada2948cfc..77212e33e3c0 100644 --- a/test/js/node/tls/node-tls-ocsp.test.ts +++ b/test/js/node/tls/node-tls-ocsp.test.ts @@ -135,6 +135,19 @@ describe("OCSP stapling", () => { expect(clientLog).toEqual(["OCSPResponse null", "secureConnect"]); }); + // Node's onOCSP gates on `if (response)` (internal/tls/wrap.js), not + // `response != null`: a falsy primitive is "nothing to staple", not a + // validation error. + it.each([0, false, ""])("treats callback(null, %p) as no staple", async falsy => { + const { serverLog, clientLog } = await exchange({ + clientOptions: { requestOCSP: true }, + // @ts-expect-error the listener is allowed to hand back nonsense + onOCSPRequest: callback => setImmediate(callback, null, falsy), + }); + expect(serverLog).toEqual(["OCSPRequest certificate=Buffer issuer=undefined", "secureConnection"]); + expect(clientLog).toEqual(["OCSPResponse null", "secureConnect"]); + }); + it("never emits OCSPRequest when the client did not ask for stapling", async () => { const { serverLog, clientLog } = await exchange({ onOCSPRequest: callback => callback(null, Buffer.from("never sent")),