From df1239ea3c7efefd8b7bf7a9300e8b781f50cfbb Mon Sep 17 00:00:00 2001 From: yhirose Date: Sun, 23 Aug 2026 22:34:47 -0400 Subject: [PATCH] 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. --- .github/workflows/test.yaml | 6 + .gitignore | 1 + httplib.h | 187 ++++++++++++++++++++++++++- test/Makefile | 4 + test/test_websocket_thread_safety.cc | 178 +++++++++++++++++++++++++ 5 files changed, 375 insertions(+), 1 deletion(-) create mode 100644 test/test_websocket_thread_safety.cc diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 2b7f3bb..8f7b54d 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -114,6 +114,9 @@ jobs: - name: build and run WebSocket heartbeat test if: matrix.tls_backend == 'openssl' run: cd test && make test_websocket_heartbeat && ./test_websocket_heartbeat + - name: build and run WebSocket TLS thread safety test + if: matrix.tls_backend == 'openssl' + run: cd test && make test_websocket_thread_safety && ./test_websocket_thread_safety - name: build and run ThreadPool test run: cd test && make test_thread_pool && ./test_thread_pool @@ -414,6 +417,9 @@ jobs: - name: build and run WebSocket heartbeat test if: matrix.tls_backend == 'openssl' run: cd test && make test_websocket_heartbeat && ./test_websocket_heartbeat + - name: build and run WebSocket TLS thread safety test + if: matrix.tls_backend == 'openssl' + run: cd test && make test_websocket_thread_safety && ./test_websocket_thread_safety - name: build and run ThreadPool test run: cd test && make test_thread_pool && ./test_thread_pool diff --git a/.gitignore b/.gitignore index 0e0be8b..2d4b6c7 100644 --- a/.gitignore +++ b/.gitignore @@ -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* diff --git a/httplib.h b/httplib.h index 294bae1..4daa656 100644 --- a/httplib.h +++ b/httplib.h @@ -9882,6 +9882,52 @@ private: bool readable_hint_ = false; }; +// A TLS stream for WebSocket connections, where the receive path and the +// send path (application send() plus the heartbeat ping thread) run on +// different threads. A single TLS session must never be entered +// concurrently, so every call into the session is serialized by one mutex. +// +// Unlike SSLSocketStream, the socket is kept non-blocking for the stream's +// whole 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. +// +// This stream is used only for wss:// connections. Plain ws:// and ordinary +// HTTP/HTTPS keep using SocketStream/SSLSocketStream unchanged. +class WebSocketSSLStream final : public Stream { +public: + WebSocketSSLStream(socket_t sock, tls::session_t session, + time_t read_timeout_sec, time_t read_timeout_usec, + time_t write_timeout_sec, time_t write_timeout_usec); + ~WebSocketSSLStream() override; + + bool is_readable() const override; + bool wait_readable() const override; + bool wait_writable() const override; + ssize_t read(char *ptr, size_t size) override; + ssize_t write(const char *ptr, size_t size) override; + void get_remote_ip_and_port(std::string &ip, int &port) const override; + void get_local_ip_and_port(std::string &ip, int &port) const override; + socket_t socket() const override; + time_t duration() const override; + void set_read_timeout(time_t sec, time_t usec = 0) override; + +private: + mutable std::mutex session_mutex_; + + socket_t sock_; + tls::session_t session_; + // WebSocket::close() shortens the read timeout from the closing thread + // while the receive thread is inside wait_readable(), so these two are read + // and written concurrently. The write timeouts are never mutated. + std::atomic read_timeout_sec_; + std::atomic read_timeout_usec_; + time_t write_timeout_sec_; + time_t write_timeout_usec_; + const std::chrono::time_point start_time_; +}; + #ifdef CPPHTTPLIB_OPENSSL_SUPPORT inline std::string message_digest(const std::string &s, const EVP_MD *algo) { auto context = std::unique_ptr( @@ -12182,6 +12228,127 @@ inline void SSLSocketStream::set_read_timeout(time_t sec, time_t usec) { read_timeout_usec_ = usec; } +inline WebSocketSSLStream::WebSocketSSLStream(socket_t sock, + tls::session_t session, + time_t read_timeout_sec, + time_t read_timeout_usec, + time_t write_timeout_sec, + time_t write_timeout_usec) + : sock_(sock), session_(session), read_timeout_sec_(read_timeout_sec), + read_timeout_usec_(read_timeout_usec), + write_timeout_sec_(write_timeout_sec), + write_timeout_usec_(write_timeout_usec), + start_time_(std::chrono::steady_clock::now()) { + // The receive and send paths run on different threads, so each TLS call is + // driven in non-blocking mode and readiness is awaited with select() + // outside the session lock. Set the socket non-blocking once here; it is + // never flipped back, so no thread races on the flag. + detail::set_nonblocking(sock_, true); +#ifdef CPPHTTPLIB_OPENSSL_SUPPORT + SSL_clear_mode(static_cast(session_), SSL_MODE_AUTO_RETRY); +#endif +} + +inline WebSocketSSLStream::~WebSocketSSLStream() = default; + +inline bool WebSocketSSLStream::is_readable() const { + std::lock_guard guard(session_mutex_); + return tls::pending(session_) > 0; +} + +inline bool WebSocketSSLStream::wait_readable() const { + return select_read(sock_, read_timeout_sec_, read_timeout_usec_) > 0; +} + +inline bool WebSocketSSLStream::wait_writable() const { + // Unlike SSLSocketStream, this deliberately does not call is_peer_closed(): + // that probe toggles the socket's blocking flag, which would race with the + // concurrent reader on a permanently non-blocking socket. + return select_write(sock_, write_timeout_sec_, write_timeout_usec_) > 0; +} + +inline ssize_t WebSocketSSLStream::read(char *ptr, size_t size) { + tls::TlsError err; + auto n = 1000; + while (--n >= 0) { + { + std::lock_guard guard(session_mutex_); + auto ret = tls::read(session_, ptr, size, err); + if (ret > 0) { return ret; } + if (ret == 0 || err.code == tls::ErrorCode::PeerClosed) { + error_ = Error::ConnectionClosed; + return ret; + } + } + // ret < 0. On a non-blocking socket a TLS read can stop needing either + // direction: the send path shares this session, so output it left pending + // has to be flushed before more input can be decrypted. Anything else is + // a hard error. + auto needs_readable = err.code == tls::ErrorCode::WantRead; +#ifdef _WIN32 + // On Windows a socket timeout surfaces as a syscall error, not WantRead. + needs_readable = + needs_readable || (err.code == tls::ErrorCode::SyscallError && + WSAGetLastError() == WSAETIMEDOUT); +#endif + if (!needs_readable && err.code != tls::ErrorCode::WantWrite) { return -1; } + if (!(needs_readable ? wait_readable() : wait_writable())) { + error_ = Error::Timeout; + return -1; + } + } + return -1; +} + +inline ssize_t WebSocketSSLStream::write(const char *ptr, size_t size) { + auto handle_size = std::min(size, (std::numeric_limits::max)()); + tls::TlsError err; + auto n = 1000; + while (--n >= 0) { + { + std::lock_guard guard(session_mutex_); + auto ret = tls::write(session_, ptr, handle_size, err); + if (ret >= 0) { return ret; } + } + // ret < 0. As in read(), either direction can be needed: a renegotiation + // or a post-handshake message must be consumed before the record goes + // out. Anything else is a hard error. + auto needs_writable = err.code == tls::ErrorCode::WantWrite; +#ifdef _WIN32 + // On Windows a socket timeout surfaces as a syscall error, not WantWrite. + needs_writable = + needs_writable || (err.code == tls::ErrorCode::SyscallError && + WSAGetLastError() == WSAETIMEDOUT); +#endif + if (!needs_writable && err.code != tls::ErrorCode::WantRead) { return -1; } + if (!(needs_writable ? wait_writable() : wait_readable())) { return -1; } + } + return -1; +} + +inline void WebSocketSSLStream::get_remote_ip_and_port(std::string &ip, + int &port) const { + detail::get_remote_ip_and_port(sock_, ip, port); +} + +inline void WebSocketSSLStream::get_local_ip_and_port(std::string &ip, + int &port) const { + detail::get_local_ip_and_port(sock_, ip, port); +} + +inline socket_t WebSocketSSLStream::socket() const { return sock_; } + +inline time_t WebSocketSSLStream::duration() const { + return std::chrono::duration_cast( + std::chrono::steady_clock::now() - start_time_) + .count(); +} + +inline void WebSocketSSLStream::set_read_timeout(time_t sec, time_t usec) { + read_timeout_sec_ = sec; + read_timeout_usec_ = usec; +} + } // namespace detail #endif // CPPHTTPLIB_SSL_ENABLED @@ -13673,6 +13840,24 @@ Server::process_request(Stream &strm, const std::string &remote_addr, if (websocket_upgraded) { *websocket_upgraded = true; } { +#ifdef CPPHTTPLIB_SSL_ENABLED + if (req.ssl) { + // wss: the heartbeat ping thread and the read path enter the same + // TLS session from different threads. Hand the WebSocket a stream + // that serializes every TLS call, so the shared SSLSocketStream on + // the plain HTTP/HTTPS paths stays untouched. + auto ws_strm = + std::unique_ptr(new detail::WebSocketSSLStream( + strm.socket(), const_cast(req.ssl), + CPPHTTPLIB_WEBSOCKET_READ_TIMEOUT_SECOND, 0, + write_timeout_sec_, write_timeout_usec_)); + ws::WebSocket ws(std::move(ws_strm), req, true, + websocket_ping_interval_sec_, + websocket_max_missed_pongs_); + entry.handler(req, ws); + return true; + } +#endif // Use WebSocket-specific read timeout instead of HTTP timeout strm.set_read_timeout(CPPHTTPLIB_WEBSOCKET_READ_TIMEOUT_SECOND, 0); ws::WebSocket ws(strm, req, true, websocket_ping_interval_sec_, @@ -21765,7 +21950,7 @@ inline bool WebSocketClient::create_stream(std::unique_ptr &strm, return false; } - strm = std::unique_ptr(new detail::SSLSocketStream( + strm = std::unique_ptr(new detail::WebSocketSSLStream( sock_, tls_session_, read_timeout_sec_, read_timeout_usec_, write_timeout_sec_, write_timeout_usec_)); return true; diff --git a/test/Makefile b/test/Makefile index 84a7f13..01e8621 100644 --- a/test/Makefile +++ b/test/Makefile @@ -264,6 +264,10 @@ test_websocket_heartbeat : test_websocket_heartbeat.cc ../httplib.h Makefile $(CXX) -o $@ -I.. $(CXXFLAGS) test_websocket_heartbeat.cc $(TEST_ARGS) @file $@ +test_websocket_thread_safety : test_websocket_thread_safety.cc ../httplib.h Makefile cert.pem + $(CXX) -o $@ -I.. $(CXXFLAGS) test_websocket_thread_safety.cc $(TEST_ARGS) + @file $@ + test_proxy : test_proxy.cc ../httplib.h Makefile cert.pem $(CXX) -o $@ -I.. $(CXXFLAGS) test_proxy.cc $(TEST_ARGS) diff --git a/test/test_websocket_thread_safety.cc b/test/test_websocket_thread_safety.cc new file mode 100644 index 0000000..7478cf7 --- /dev/null +++ b/test/test_websocket_thread_safety.cc @@ -0,0 +1,178 @@ +// Standalone test for TLS-session thread safety on wss:// connections. +// +// A wss:// WebSocket enters one TLS session from multiple threads: the read +// path, the application's send()/close(), and the heartbeat ping thread. A +// TLS session must never be entered concurrently, so httplib routes wss:// +// through WebSocketSSLStream, which serializes every TLS call. These tests +// drive that concurrency directly. Built with ASan in CI, so a regression +// surfaces as a heap-buffer-overflow, not just a flaky assertion. + +// Fire the heartbeat every second so the ping-vs-read case actually crosses a +// ping while the reader is idle. +#define CPPHTTPLIB_WEBSOCKET_PING_INTERVAL_SECOND 1 +#include + +#include "gtest/gtest.h" + +#include +#include +#include +#include + +#ifdef CPPHTTPLIB_SSL_ENABLED + +using namespace httplib; + +namespace { + +const size_t kPayloadBytes = 2048; +const size_t kSendCount = 2000; +const size_t kBurstPerFrame = 100; +const int kCloseCycles = 20; + +} // namespace + +class WebSocketTlsThreadSafetyTest : public ::testing::Test { +protected: + WebSocketTlsThreadSafetyTest() : svr_("cert.pem", "key.pem") {} + + void TearDown() override { + if (thread_.joinable()) { + svr_.stop(); + thread_.join(); + } + } + + // Registers the handler and starts the TLS server. Called by each test + // after its own server configuration, since the heartbeat test needs the + // pings the others switch off. + bool start(Server::WebSocketHandler handler) { + if (!svr_.is_valid()) { return false; } + svr_.WebSocket("/ws", std::move(handler)); + port_ = svr_.bind_to_any_port("localhost"); + if (port_ <= 0) { return false; } + thread_ = std::thread([this]() { svr_.listen_after_bind(); }); + svr_.wait_until_ready(); + return true; + } + + std::string url() const { + return "wss://localhost:" + std::to_string(port_) + "/ws"; + } + + SSLServer svr_; + int port_ = 0; + std::thread thread_; +}; + +// A sender thread hammers send() while another thread loops read(). Both +// enter the same TLS session, and every echoed frame must arrive intact. +TEST_F(WebSocketTlsThreadSafetyTest, SendWhileAnotherThreadReads) { + svr_.set_websocket_ping_interval(0); + ASSERT_TRUE(start([](const Request &, ws::WebSocket &sock) { + std::string msg; + while (sock.read(msg) != ws::ReadResult::Fail) { + if (!sock.send(msg.data(), msg.size())) { break; } + } + })); + + ws::WebSocketClient cli(url()); + cli.enable_server_certificate_verification(false); + ASSERT_TRUE(cli.connect()); + + const std::string payload(kPayloadBytes, 'x'); + + // close() drains the peer's Close reply with its own frame reader, so once + // it starts, two threads parse frames from one stream and can split a + // payload between them. That is frame-level, not TLS-level, and happens on + // ws:// too, so only frames completed before close() are checked here. + std::atomic closing(false); + std::atomic frames_read(0); + std::atomic corrupt_frames(0); + std::thread reader([&]() { + std::string msg; + while (cli.read(msg) != ws::ReadResult::Fail) { + if (msg != payload && !closing.load()) { corrupt_frames++; } + frames_read++; + } + }); + + size_t sent = 0; + for (size_t i = 0; i < kSendCount; i++) { + if (!cli.send(payload.data(), payload.size())) { break; } + sent++; + } + + closing.store(true); + cli.close(); + reader.join(); + + EXPECT_EQ(kSendCount, sent); + EXPECT_EQ(static_cast(0), corrupt_frames.load()); + EXPECT_GT(frames_read.load(), static_cast(0)); +} + +// close() sends a Close frame and drains the peer's reply while a second +// thread is inside read(). Repeated to shake out the race. +TEST_F(WebSocketTlsThreadSafetyTest, CloseWhileAnotherThreadReads) { + svr_.set_websocket_ping_interval(0); + ASSERT_TRUE(start([](const Request &, ws::WebSocket &sock) { + const std::string burst(64, 'p'); + std::string msg; + while (sock.read(msg) != ws::ReadResult::Fail) { + for (size_t i = 0; i < kBurstPerFrame; i++) { + if (!sock.send(burst.data(), burst.size())) { return; } + } + } + })); + + for (int cycle = 0; cycle < kCloseCycles; cycle++) { + ws::WebSocketClient cli(url()); + cli.enable_server_certificate_verification(false); + ASSERT_TRUE(cli.connect()) << "cycle " << cycle; + + std::thread reader([&]() { + std::string msg; + while (cli.read(msg) != ws::ReadResult::Fail) {} + }); + + const std::string trigger(64, 't'); + ASSERT_TRUE(cli.send(trigger.data(), trigger.size())); + + cli.close(); + reader.join(); + } +} + +// The heartbeat ping thread writes to the TLS session on its own timer while +// the application blocks in read() with no traffic. The ping's write must not +// collide with the reader. The 1-second interval above means several pings +// fire on both sides during this idle window. +TEST_F(WebSocketTlsThreadSafetyTest, HeartbeatPingWhileReaderIsIdle) { + ASSERT_TRUE(start([](const Request &, ws::WebSocket &sock) { + std::string msg; + while (sock.read(msg) != ws::ReadResult::Fail) {} + })); + + ws::WebSocketClient cli(url()); + cli.enable_server_certificate_verification(false); + ASSERT_TRUE(cli.connect()); + + // No data frames are sent, so the reader stays parked inside read() while + // both sides exchange pings and pongs on the heartbeat timer. read() only + // returns once close() below tears the connection down. + std::thread reader([&]() { + std::string msg; + while (cli.read(msg) != ws::ReadResult::Fail) {} + }); + + std::this_thread::sleep_for(std::chrono::seconds(4)); + + // The connection survived the heartbeat exchange without a TLS-session race. + EXPECT_TRUE(cli.is_open()); + + cli.close(); + reader.join(); +} + +#endif // CPPHTTPLIB_SSL_ENABLED