Fix TLS session data race on wss:// WebSocket connections

A wss:// WebSocket enters a single TLS session from several threads: the
read path, the application's send()/close(), and the heartbeat ping thread.
The existing write_mutex_ only serializes writers, so a reader's SSL_read and
a writer's SSL_write (plus the SSL_peek in is_peer_closed() on the write path)
run concurrently on the same session. OpenSSL and the other backends forbid
concurrent access to one session, so this corrupts the record layer: messages
are silently dropped, and under ASan it shows up as a heap-buffer-overflow.
It affects wss:// only; plain ws:// is unaffected because the kernel allows
concurrent recv()/send() on a socket.

Route wss:// through a new WebSocketSSLStream that serializes every TLS call
with one per-stream mutex. The socket is kept non-blocking for the stream's
lifetime and each read()/write() performs a single non-blocking TLS call under
the lock, then waits for readiness with select() outside the lock. The lock is
therefore held only for CPU-bound work, so a reader blocked waiting for data
never stalls a concurrent sender.

Because the socket is non-blocking, a TLS call can stop needing either
direction, so read() also waits for writability on WantWrite and write() waits
for readability on WantRead. A read that shares its session with the send path
has to flush pending output before it can decrypt more input, and Mbed TLS
surfaces this on every mbedtls_ssl_read(). The read timeouts are atomic since
WebSocket::close() shortens them from the closing thread while the receive
thread is inside wait_readable().

SSLSocketStream is left untouched, so ordinary HTTP/HTTPS keeps its exact code
path and performance. The heartbeat ping thread also stays, so timer-driven
pings keep working as before.

Add test_websocket_thread_safety.cc, which drives send/close/heartbeat against
a concurrent reader over wss://. Built with ASan in CI, a regression surfaces
as a heap-buffer-overflow.
This commit is contained in:
yhirose
2026-08-23 22:34:47 -04:00
parent 6494edd8c0
commit df1239ea3c
5 changed files with 375 additions and 1 deletions

1
.gitignore vendored
View File

@@ -54,6 +54,7 @@ test/test_split_mbedtls
test/test_split_wolfssl
test/test_split_no_tls
test/test_websocket_heartbeat
test/test_websocket_thread_safety
test/test_thread_pool
test/test_benchmark
test/test.xcodeproj/xcuser*