Fix TLS session data race on wss:// WebSocket connections (#2551)

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 authored and GitHub committed 2026-08-24 07:04:43 -04:00
1 parent 6494edd8c0
commit 228af9033b
5 files changed
+375 -1

No files matched your search

+186 -1
View File
@@ -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<time_t> read_timeout_sec_;
std::atomic<time_t> read_timeout_usec_;
time_t write_timeout_sec_;
time_t write_timeout_usec_;
const std::chrono::time_point<std::chrono::steady_clock> start_time_;
};
#ifdef CPPHTTPLIB_OPENSSL_SUPPORT
inline std::string message_digest(const std::string &s, const EVP_MD *algo) {
auto context = std::unique_ptr<EVP_MD_CTX, decltype(&EVP_MD_CTX_free)>(
@@ -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<SSL *>(session_), SSL_MODE_AUTO_RETRY);
#endif
}
inline WebSocketSSLStream::~WebSocketSSLStream() = default;
inline bool WebSocketSSLStream::is_readable() const {
std::lock_guard<std::mutex> 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<std::mutex> 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_t>(size, (std::numeric_limits<int>::max)());
tls::TlsError err;
auto n = 1000;
while (--n >= 0) {
{
std::lock_guard<std::mutex> 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::milliseconds>(
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<Stream>(new detail::WebSocketSSLStream(
strm.socket(), const_cast<tls::session_t>(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<Stream> &strm,
return false;
}
strm = std::unique_ptr<Stream>(new detail::SSLSocketStream(
strm = std::unique_ptr<Stream>(new detail::WebSocketSSLStream(
sock_, tls_session_, read_timeout_sec_, read_timeout_usec_,
write_timeout_sec_, write_timeout_usec_));
return true;