mirror of
https://github.com/oven-sh/bun
synced 2026-02-02 15:08:46 +00:00
fix(tls): check SSL_is_init_finished directly for _secureEstablished (#26086)
## 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 <claude-bot@bun.sh> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
This commit is contained in:
@@ -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 =
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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)));
|
||||
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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<uWS::HttpResponseData<true>*>(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<uWS::HttpResponseData<true>*>(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()
|
||||
|
||||
Reference in New Issue
Block a user