Skip to content

CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write #591

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cpansuriya-simba wants to merge 3 commits into
base: trunk
Choose a base branch
from
+76 −32
Open

CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write #591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
daa6693
tls 1.3 support added
cpansuriya-simba Sep 7, 2026
fb99828
Added CASS_SSL_VERSION_TLS1_3 to the public CassSslTlsVersion enum (c…
cpansuriya-simba Sep 9, 2026
931f54f
Fixed regression test failures
cpansuriya-simba Sep 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion include/cassandra.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,8 @@ typedef enum CassSslVerifyFlags_ {
typedef enum CassSslTlsVersion_ {
CASS_SSL_VERSION_TLS1 = 0x00,
CASS_SSL_VERSION_TLS1_1 = 0x01,
CASS_SSL_VERSION_TLS1_2 = 0x02
CASS_SSL_VERSION_TLS1_2 = 0x02,
CASS_SSL_VERSION_TLS1_3 = 0x03
} CassSslTlsVersion;

typedef enum CassProtocolVersion_ {
Expand Down
16 changes: 14 additions & 2 deletions src/socket.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,14 @@ void SslSocketHandler::on_read(Socket* socket, ssize_t nread, const uv_buf_t* bu

uv_tcp_t* SocketWriteBase::tcp() { return &socket_->tcp_; }

SocketWriteBase::SocketWriteBase(Socket* socket)
: socket_(socket)
, is_flushed_(false)
, handler_generation_(socket->handler_generation()) {

@absurdfarce absurdfarce Sep 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the only part I felt like I didn't quite understand. Can you speak more to what the goal of this notion of "handler generations" is @cpansuriya-simba? Maybe I'm missing something obvious (in fact I probably am) but I wasn't immediately clear on why you'd need a mechanism like this.

@cpansuriya-simba cpansuriya-simba Sep 16, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given full explanation below. Want to suppress it but full explanation is batter so I keep it as it is.

handler_generation_() captures a "snapshot" of which handler was active on the Socket at the moment this write object was created, so it can be compared later before recycling it. Here's why it became necessary for TLS 1.3:

The reuse mechanism that existed before: Socket keeps a pool of completed SocketWriteBase objects (free_writes_) to avoid re-allocating on every write. When a write finishes (handle_write()), if under the pool limit, the object is cleared and pushed back into free_writes_ for the next write request — regardless of what that next request is for.

Why that's dangerous now: During the SSL handshake, Socket's handler is a plain SocketHandler, so handshake bytes are written using a plain SocketWrite object. Once the handshake finishes, set_handler() swaps the socket over to the real SslSocketHandler for encrypted application traffic.

With TLS 1.2's handshake, "handshake done" and "the last handshake write completing" were separated enough in timing that this ordering issue didn't surface. With TLS 1.3's 1-RTT handshake, is_handshake_done() can become true in the very same step that produces the client's Finished message — so we now deliberately defer calling finish() (which swaps the handler) until after that final handshake write's on_write callback actually fires (see the earlier ssl_handshake_finish() change). That means the handler swap and the completion of the handshake's own write object happen close together, right around when that write object would normally be recycled into free_writes_.

The bug this prevents: Without tracking which handler-generation a write object belongs to, that now-completed plain handshake write object could get recycled into free_writes_ right as (or after) the handler switches to SslSocketHandler. The next application write would then pop that stale plain SocketWrite object from the pool instead of creating a proper encrypted SslSocketWrite, causing application data to be written unencrypted, in plaintext straight over the socket.

What the line actually does: handler_generation_(socket->handler_generation()) records the handler's generation counter (bumped once per set_handler() call) at construction time. Later, in handle_write(), the object is only put back in the free pool if handler_generation_ == socket->handler_generation_ — i.e., the handler hasn't changed since this write object was created. If it has changed, the object is simply deleted instead of reused, forcing a fresh, correctly-typed write object (SslSocketWrite) to be created for the next request.

req_.data = this;
buffers_.reserve(MIN_BUFFERS_SIZE);
}

void SocketWriteBase::on_close() {
for (RequestVec::iterator i = requests_.begin(), end = requests_.end(); i != end; ++i) {
(*i)->on_close();
Expand Down Expand Up @@ -267,7 +275,9 @@ void SocketWriteBase::handle_write(uv_write_t* req, int status) {

socket->pending_writes_.remove(this);

if (socket->free_writes_.size() < socket->max_reusable_write_objects_) {
// Don't recycle a write created under a handler that's since been replaced.
if (handler_generation_ == socket->handler_generation_ &&
socket->free_writes_.size() < socket->max_reusable_write_objects_) {
clear();
socket->free_writes_.push_back(this);
} else {
Expand All @@ -278,7 +288,8 @@ void SocketWriteBase::handle_write(uv_write_t* req, int status) {
}

Socket::Socket(const Address& address, size_t max_reusable_write_objects)
: is_defunct_(false)
: handler_generation_(0)
, is_defunct_(false)
, max_reusable_write_objects_(max_reusable_write_objects)
, address_(address) {
tcp_.data = this;
Expand All @@ -288,6 +299,7 @@ Socket::~Socket() { cleanup_free_writes(); }

void Socket::set_handler(SocketHandlerBase* handler) {
handler_.reset(handler);
++handler_generation_;
cleanup_free_writes();
free_writes_.clear();
if (handler_) {
Expand Down
15 changes: 9 additions & 6 deletions src/socket.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,7 @@ class SocketWriteBase
*
* @param The socket handling the write.
*/
SocketWriteBase(Socket* socket)
: socket_(socket)
, is_flushed_(false) {
req_.data = this;
buffers_.reserve(MIN_BUFFERS_SIZE);
}
SocketWriteBase(Socket* socket);

virtual ~SocketWriteBase() {}

Expand Down Expand Up @@ -260,6 +255,8 @@ class SocketWriteBase
Socket* socket_;
uv_write_t req_;
bool is_flushed_;
// Socket's handler generation when this write was created, to avoid reuse under a new handler.
size_t handler_generation_;
BufferVec buffers_;
RequestVec requests_;
};
Expand Down Expand Up @@ -298,6 +295,11 @@ class Socket : public RefCounted<Socket> {
*/
void set_handler(SocketHandlerBase* handler);

/**
* The number of times the socket's handler has been set.
*/
size_t handler_generation() const { return handler_generation_; }

/**
* Write a request to the socket and coalesce with outstanding requests. This
* method doesn't flush.
Expand Down Expand Up @@ -367,6 +369,7 @@ class Socket : public RefCounted<Socket> {

uv_tcp_t tcp_;
ScopedPtr<SocketHandlerBase> handler_;
size_t handler_generation_;

SocketWriteBase::List pending_writes_;
SocketWriteVec free_writes_;
Expand Down
38 changes: 27 additions & 11 deletions src/socket_connector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ class SslHandshakeHandler : public SocketHandler {
delete request;
if (status != 0) {
connector_->on_error(SocketConnector::SOCKET_ERROR_WRITE, "Write error");
return;
}
// TLS 1.3 can finish the handshake while this write was still queued.
if (connector_->ssl_session_->is_handshake_done()) {
connector_->ssl_handshake_finish();
}
}

Expand Down Expand Up @@ -106,7 +111,8 @@ SocketConnector::SocketConnector(const Address& address, const Callback& callbac
: address_(address)
, callback_(callback)
, error_code_(SOCKET_OK)
, ssl_error_code_(CASS_OK) {}
, ssl_error_code_(CASS_OK)
, is_handshake_finished_(false) {}

SocketConnector* SocketConnector::with_settings(const SocketSettings& settings) {
settings_ = settings;
Expand Down Expand Up @@ -207,21 +213,31 @@ void SocketConnector::ssl_handshake() {
}
}

// Write any outgoing data created by the handshake process.
// Write any outgoing data created by the handshake process. Finishing is
// deferred to on_write() if the handshake is already done (e.g. TLS 1.3).
char buf[SSL_HANDSHAKE_MAX_BUFFER_SIZE];
size_t size = ssl_session_->outgoing().read(buf, SSL_HANDSHAKE_MAX_BUFFER_SIZE);
if (size > 0) {
socket_->write_and_flush(new BufferSocketRequest(Buffer(buf, size)));
} else if (ssl_session_->is_handshake_done()) { // If the handshake process is done then verify
// the certificate and finish.
ssl_session_->verify();
if (ssl_session_->has_error()) {
on_error(SOCKET_ERROR_SSL_VERIFY,
"Error verifying peer certificate: " + ssl_session_->error_message());
return;
}
finish();
} else if (ssl_session_->is_handshake_done()) {
ssl_handshake_finish();
}
}

void SocketConnector::ssl_handshake_finish() {
// Handshake completion can be observed from both the deferred on_write()
// and a subsequent on_read() with TLS 1.3; only run this once.
if (is_handshake_finished_) return;
is_handshake_finished_ = true;

// If the handshake process is done then verify the certificate and finish.
ssl_session_->verify();
if (ssl_session_->has_error()) {
on_error(SOCKET_ERROR_SSL_VERIFY,
"Error verifying peer certificate: " + ssl_session_->error_message());
return;
}
finish();
}

void SocketConnector::finish() {
Expand Down
7 changes: 7 additions & 0 deletions src/socket_connector.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ class SocketConnector : public RefCounted<SocketConnector> {
private:
void internal_connect(uv_loop_t* loop);
void ssl_handshake();
void ssl_handshake_finish();
void finish();

void on_error(SocketError code, const String& message);
Expand Down Expand Up @@ -167,6 +168,12 @@ class SocketConnector : public RefCounted<SocketConnector> {

ScopedPtr<SslSession> ssl_session_;

// Guards against ssl_handshake_finish() running more than once. With TLS 1.3
// the handshake can be reported done both from a deferred on_write() (after
// the final client flight is flushed) and from a subsequent on_read() (e.g.
// post-handshake data arriving before that write completes).
bool is_handshake_finished_;

SocketSettings settings_;
};

Expand Down
7 changes: 5 additions & 2 deletions src/ssl/ssl_openssl_impl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,8 +541,8 @@ OpenSslContext::OpenSslContext()
SSL_CTX_set_cert_store(ssl_ctx_, trusted_store_);
SSL_CTX_set_verify(ssl_ctx_, SSL_VERIFY_NONE, ssl_no_verify_callback);
#if (OPENSSL_VERSION_NUMBER >= 0x10100000L)
// Limit to TLS 1.2 for now. TLS 1.3 has broken the handshake code.
SSL_CTX_set_max_proto_version(ssl_ctx_, TLS1_2_VERSION);
// Allow up to TLS 1.3.
SSL_CTX_set_max_proto_version(ssl_ctx_, TLS1_3_VERSION);
#endif
#if DEBUG_SSL
SSL_CTX_set_info_callback(ssl_ctx_, ssl_info_callback);
Expand Down Expand Up @@ -632,6 +632,9 @@ CassError OpenSslContext::set_min_protocol_version(CassSslTlsVersion min_version
case CassSslTlsVersion::CASS_SSL_VERSION_TLS1_2:
method = TLS1_2_VERSION;
break;
case CassSslTlsVersion::CASS_SSL_VERSION_TLS1_3:
method = TLS1_3_VERSION;
break;
default:
// unsupported version
return CASS_ERROR_LIB_BAD_PARAMS;
Expand Down
22 changes: 12 additions & 10 deletions tests/src/unit/mockssandra.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,23 +444,25 @@ void ClientConnection::on_ssl_read(const char* data, size_t len) {
}

char buf[SSL_BUF_SIZE];
bool data_written = false;
int num_bytes;
while ((num_bytes = BIO_read(outgoing_bio_, buf, sizeof(buf))) > 0) {
data_written = true;
internal_write(buf, num_bytes);
}

if (is_handshake_done() && data_written) {
return; // Handshake is not completed; ingore remaining data
if (!is_handshake_done()) {
return; // Handshake still isn't complete; wait for more data.
}
} else {
char buf[SSL_BUF_SIZE];
while ((rc = SSL_read(ssl_, buf, sizeof(buf))) > 0) {
on_read(buf, rc);
}
has_ssl_error(rc);
// Handshake just completed. With TLS 1.3 the client's first application
// data (e.g. OPTIONS) can arrive in the same read as the final handshake
// record, so fall through and drain any decrypted data below instead of
// dropping it.
}

char buf[SSL_BUF_SIZE];
while ((rc = SSL_read(ssl_, buf, sizeof(buf))) > 0) {
on_read(buf, rc);
}
has_ssl_error(rc);
}

ServerConnection::ServerConnection(const Address& address, const ClientConnectionFactory& factory)
Expand Down