From 7d640cccd1a84cf0e5e4823a65ee7df2406dc4dd Mon Sep 17 00:00:00 2001 From: robobun Date: Wed, 14 Jan 2026 18:48:09 -0800 Subject: [PATCH] fix(tls): check SSL_is_init_finished directly for _secureEstablished (#26086) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes flaky test `test/js/node/test/parallel/test-http-url.parse-https.request.js` where `request.socket._secureEstablished` sometimes returned `false` even when the TLS handshake had completed. ## Root Cause There's a race condition between when the TLS handshake completes and when the `on_handshake` callback fires. The HTTP request handler could start executing before the callback set `httpResponseData->isAuthorized = true`, causing `_secureEstablished` to return `false`. ## Previous Failed Approach (PR #25946) Attempted to trigger the handshake callback earlier in `ssl_on_data`, but this broke gRPC and HTTP/2 tests because the callback has side effects that disrupted the data processing. ## This Fix Instead of changing when the callback fires, directly query OpenSSL's `SSL_is_init_finished()` when checking `_secureEstablished`: 1. Added `us_socket_is_ssl_handshake_finished()` API that wraps `SSL_is_init_finished()` 2. Modified `JSNodeHTTPServerSocket::isAuthorized()` to use this function directly This approach is non-invasive - it doesn't change any TLS processing logic, just reads the correct state at the point where it's needed. ## Test plan - [x] Original flaky test passes under high parallelism (50/50 runs) - [x] gRPC tests pass (`test-channel-credentials.test.ts`) - [x] All `test-http-url.parse-*.js` tests pass - [x] HTTPS tests pass (`test-https-simple.js`, `test-https-agent.js`) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Claude Bot Co-authored-by: Claude Opus 4.5 Co-authored-by: Jarred Sumner --- packages/bun-usockets/src/crypto/openssl.c | 9 ++++++ packages/bun-usockets/src/internal/internal.h | 2 ++ packages/bun-usockets/src/libusockets.h | 6 ++++ packages/bun-usockets/src/socket.c | 20 +++++++++++++ .../bindings/node/JSNodeHTTPServerSocket.cpp | 30 +++++++++++++++---- 5 files changed, 62 insertions(+), 5 deletions(-) diff --git a/packages/bun-usockets/src/crypto/openssl.c b/packages/bun-usockets/src/crypto/openssl.c index a3f12133e7..2049da5a10 100644 --- a/packages/bun-usockets/src/crypto/openssl.c +++ b/packages/bun-usockets/src/crypto/openssl.c @@ -315,6 +315,15 @@ int us_internal_ssl_socket_is_closed(struct us_internal_ssl_socket_t *s) { return us_socket_is_closed(0, &s->s); } +int us_internal_ssl_socket_is_handshake_finished(struct us_internal_ssl_socket_t *s) { + if (!s || !s->ssl) return 0; + return SSL_is_init_finished(s->ssl); +} + +int us_internal_ssl_socket_handshake_callback_has_fired(struct us_internal_ssl_socket_t *s) { + if (!s) return 0; + return s->handshake_state == HANDSHAKE_COMPLETED; +} void us_internal_trigger_handshake_callback_econnreset(struct us_internal_ssl_socket_t *s) { struct us_internal_ssl_socket_context_t *context = diff --git a/packages/bun-usockets/src/internal/internal.h b/packages/bun-usockets/src/internal/internal.h index e3c53c02f0..daa858338f 100644 --- a/packages/bun-usockets/src/internal/internal.h +++ b/packages/bun-usockets/src/internal/internal.h @@ -439,6 +439,8 @@ void *us_internal_ssl_socket_ext(us_internal_ssl_socket_r s); void *us_internal_connecting_ssl_socket_ext(struct us_connecting_socket_t *c); int us_internal_ssl_socket_is_shut_down(us_internal_ssl_socket_r s); int us_internal_ssl_socket_is_closed(us_internal_ssl_socket_r s); +int us_internal_ssl_socket_is_handshake_finished(us_internal_ssl_socket_r s); +int us_internal_ssl_socket_handshake_callback_has_fired(us_internal_ssl_socket_r s); void us_internal_ssl_socket_shutdown(us_internal_ssl_socket_r s); struct us_internal_ssl_socket_t *us_internal_ssl_socket_context_adopt_socket( diff --git a/packages/bun-usockets/src/libusockets.h b/packages/bun-usockets/src/libusockets.h index 3d59b7502c..3a0ad20eac 100644 --- a/packages/bun-usockets/src/libusockets.h +++ b/packages/bun-usockets/src/libusockets.h @@ -457,6 +457,12 @@ int us_socket_is_shut_down(int ssl, us_socket_r s) nonnull_fn_decl; /* Returns whether this socket has been closed. Only valid if memory has not yet been released. */ int us_socket_is_closed(int ssl, us_socket_r s) nonnull_fn_decl; +/* Returns 1 if the TLS handshake has completed, 0 otherwise. For non-SSL sockets, always returns 1. */ +int us_socket_is_ssl_handshake_finished(int ssl, us_socket_r s) nonnull_fn_decl; + +/* Returns 1 if the TLS handshake callback has been invoked, 0 otherwise. For non-SSL sockets, always returns 1. */ +int us_socket_ssl_handshake_callback_has_fired(int ssl, us_socket_r s) nonnull_fn_decl; + /* Immediately closes the socket */ struct us_socket_t *us_socket_close(int ssl, us_socket_r s, int code, void *reason) __attribute__((nonnull(2))); diff --git a/packages/bun-usockets/src/socket.c b/packages/bun-usockets/src/socket.c index 872d81047d..bbe2acad06 100644 --- a/packages/bun-usockets/src/socket.c +++ b/packages/bun-usockets/src/socket.c @@ -128,6 +128,26 @@ int us_socket_is_closed(int ssl, struct us_socket_t *s) { return s->flags.is_closed; } +int us_socket_is_ssl_handshake_finished(int ssl, struct us_socket_t *s) { +#ifndef LIBUS_NO_SSL + if(ssl) { + return us_internal_ssl_socket_is_handshake_finished((struct us_internal_ssl_socket_t *) s); + } +#endif + // Non-SSL sockets are always "handshake finished" + return 1; +} + +int us_socket_ssl_handshake_callback_has_fired(int ssl, struct us_socket_t *s) { +#ifndef LIBUS_NO_SSL + if(ssl) { + return us_internal_ssl_socket_handshake_callback_has_fired((struct us_internal_ssl_socket_t *) s); + } +#endif + // Non-SSL sockets are always "callback fired" + return 1; +} + int us_connecting_socket_is_closed(int ssl, struct us_connecting_socket_t *c) { return c->closed; } diff --git a/src/bun.js/bindings/node/JSNodeHTTPServerSocket.cpp b/src/bun.js/bindings/node/JSNodeHTTPServerSocket.cpp index a310e533a3..a83540e486 100644 --- a/src/bun.js/bindings/node/JSNodeHTTPServerSocket.cpp +++ b/src/bun.js/bindings/node/JSNodeHTTPServerSocket.cpp @@ -17,6 +17,8 @@ extern "C" void us_socket_free_stream_buffer(us_socket_stream_buffer_t* streamBu extern "C" uint64_t uws_res_get_remote_address_info(void* res, const char** dest, int* port, bool* is_ipv6); extern "C" uint64_t uws_res_get_local_address_info(void* res, const char** dest, int* port, bool* is_ipv6); extern "C" EncodedJSValue us_socket_buffered_js_write(void* socket, bool is_ssl, bool ended, us_socket_stream_buffer_t* streamBuffer, JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue data, JSC::EncodedJSValue encoding); +extern "C" int us_socket_is_ssl_handshake_finished(int ssl, struct us_socket_t* s); +extern "C" int us_socket_ssl_handshake_callback_has_fired(int ssl, struct us_socket_t* s); namespace Bun { @@ -72,11 +74,29 @@ bool JSNodeHTTPServerSocket::isAuthorized() const // is secure means that tls was established successfully if (!is_ssl || !socket) return false; - // Read from per-socket HttpResponseData instead of context-level data - auto* httpResponseData = reinterpret_cast*>(us_socket_ext(is_ssl, socket)); - if (!httpResponseData) - return false; - return httpResponseData->isAuthorized; + + // Check if the handshake callback has fired. If so, use the isAuthorized flag + // which reflects the actual certificate verification result. + if (us_socket_ssl_handshake_callback_has_fired(is_ssl, socket)) { + auto* httpResponseData = reinterpret_cast*>(us_socket_ext(is_ssl, socket)); + if (!httpResponseData) + return false; + return httpResponseData->isAuthorized; + } + + // The handshake callback hasn't fired yet, but we're in an HTTP handler, + // which means we received HTTP data. Check if the TLS handshake has actually + // completed using OpenSSL's state (SSL_is_init_finished). + // + // If the handshake is complete but the callback hasn't fired, we're in a race + // condition. The callback will fire shortly and either: + // 1. Set isAuthorized = true (success) + // 2. Close the socket (if rejectUnauthorized and verification failed) + // + // Since we're in an HTTP handler and the socket isn't closed, we can safely + // assume the handshake will succeed. If it fails, the socket will be closed + // and subsequent operations will fail appropriately. + return us_socket_is_ssl_handshake_finished(is_ssl, socket); } JSNodeHTTPServerSocket::~JSNodeHTTPServerSocket()