Skip to content

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

Open
cpansuriya-simba wants to merge 3 commits into
apache:trunkfrom
cpansuriya-simba:tls-13-support
Open

cpansuriya-simba wants to merge 3 commits into
apache:trunkfrom
cpansuriya-simba:tls-13-support

Conversation

@cpansuriya-simba

Copy link
Copy Markdown

Problem
The driver pinned OpenSSL to TLS 1.2 because the handshake state machine assumed a 2-RTT flow where writes and "handshake done" are always sequential. TLS 1.3's 1-RTT handshake breaks that assumption: SSL_connect() can report completion in the same call that produces the outgoing Finished message, so "done" and "still have data to write" happen simultaneously rather than one after another.

Root cause
The old code only checked is_handshake_done() when there was nothing left to write (size == 0). Under TLS 1.3, that check could pass while the Finished message write was still in flight — causing the driver to verify the peer cert and hand the socket off to the post-handshake handler/write-recycling pool before the write actually completed, racing with (and risking corruption of) that write.

Fix

  1. ssl_openssl_impl.cpp — Raise max negotiated protocol to TLS 1.3; log negotiated protocol/cipher on successful handshake.
  2. socket_connector.hpp/.cpp — Extract cert verification + handshake completion into ssl_handshake_finish(). If the handshake completes while the Finished write is still queued, completion is now deferred to on_write (i.e., after the write actually lands) instead of firing immediately.
  3. socket.hpp/.cpp — Add a handler_generation_ counter to Socket/SocketWriteBase so a plain write object allocated during the handshake can't be recycled into a later request after the handler has since switched to SSL — closing a use-after-handoff/leak path into the encrypted session.

Net effect
TLS 1.3 connections now complete the handshake safely, with no race on the final handshake write and no stale pre-handoff write object leaking into the encrypted session.

…assandra.h).

Handled the new value in OpenSslContext::set_min_protocol_version() (ssl_openssl_impl.cpp), mapping it to TLS1_3_VERSION so cass_ssl_set_min_protocol_version() can enforce a TLS 1.3 minimum.

@absurdfarce absurdfarce left a comment

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.

Heart of the change (the handling of the 1 round-trip handshake in TLS 1.3) makes good sense to me. I've also confirmed that this change allows a test client to communicate with a Apache Cassandra 5.0.4 server configured to use TLS 1.3 exclusively. To validate backwards compat this change allows the same configuration to function with TLS 1.2 as well... so we didn't break anything there.

I do have a few questions/observations that we should at least give some time to:

  • Some of the unit tests are currently failing with this change. Noted on the latest Jenkins run and it looks to be fairly consistent across all four test platforms. I'll have some details on these failures momentarily.
  • Do we need explicit unit/integration tests for the TLS 1.2 vs. TLS 1.3 case? Answering this probably involves a deeper dive into the current test infrastructure than I've undertaken so far.
  • I think we should be careful about saying this release support TLS 1.3 in it's entirety. There's a lot of things that come along with TLS 1.3 and I'm not sure we cover everything in the changes in this PR. I'm willing to listen if somebody thinks that's wrong... that's just the way it looked to me on first blush. To be clear: even saying we support the streamlined handshake introduced in TLS 1.3 is (a) technically correct while also being (b) a significant achievement.

Comment thread src/socket.cpp
SocketWriteBase::SocketWriteBase(Socket* socket)
: socket_(socket)
, is_flushed_(false)
, handler_generation_(socket->handler_generation()) {

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.

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.

@absurdfarce

Copy link
Copy Markdown
Contributor

An example of some of the test failures I've seen (pulled from the Jenkins runs):

[2026-09-15T20:32:20.548Z] [ RUN      ] PoolUnitTest.Ssl
[2026-09-15T20:32:25.426Z] /home/jenkins/workspace/drivers_cpp_oss_PR-591/tests/src/unit/tests/test_pool.cpp:512: Failure
[2026-09-15T20:32:25.426Z]       Expected: status.count(RequestStatus::SUCCESS)
[2026-09-15T20:32:25.426Z]       Which is: 1
[2026-09-15T20:32:25.426Z] To be equal to: 3u
[2026-09-15T20:32:25.426Z]       Which is: 3
[2026-09-15T20:32:25.426Z] [ERROR_NO_CONNECTION, ERROR_NO_CONNECTION, SUCCESS]
[2026-09-15T20:32:25.426Z] [  FAILED  ] PoolUnitTest.Ssl (5006 ms)
[2026-09-15T20:32:25.960Z] [ RUN      ] RequestProcessorUnitTest.Ssl
[2026-09-15T20:32:30.893Z] /home/jenkins/workspace/drivers_cpp_oss_PR-591/tests/src/unit/tests/test_request_processor.cpp:369: Failure
[2026-09-15T20:32:30.893Z] Value of: connect_future->wait_for(WAIT_FOR_TIME)
[2026-09-15T20:32:30.893Z]   Actual: false
[2026-09-15T20:32:30.893Z] Expected: true
[2026-09-15T20:32:31.451Z] /home/jenkins/workspace/drivers_cpp_oss_PR-591@tmp/durable-5fd157ad/script.sh.copy: line 4: 13535 Aborted                 (core dumped) build/cassandra-unit-tests --gtest_output=xml:cassandra-unit-tests-${OS_DISTRO}-${OS_DISTRO_RELEASE}-results.xml

There are a few others but these two stood out right away. These tests aren't failing on a base run of current trunk.

I don't know that we necessarily have to fix every test that breaks here but I'd like to at least understand what is breaking and what (if anything) we need to change to make Jenkins happy.

@absurdfarce

Copy link
Copy Markdown
Contributor

Ping @yifan-c for visibility as to what's going on here

@cpansuriya-simba

Copy link
Copy Markdown
Author

Because of below reason, we show 2 test failures
Bug — mockssandra.cpp:436: the mock test server's ClientConnection::on_ssl_read() silently dropped any application data (e.g. the client's OPTIONS request) that arrived in the same TCP read as the final TLS handshake record. With TLS 1.2's extra round trips this rarely happened, but TLS 1.3's 1‑RTT handshake means the client's Finished message and its first request are very likely to land together — so the mock server would swallow the request and the connection would hang until the driver's connect timeout (~5s), producing ERROR_NO_CONNECTION / test timeouts, and (combined with test-harness fragility around late callbacks) a UAF-triggered abort in RequestProcessorUnitTest.Ssl.
Defensive fix — socket_connector.cpp:227/.hpp: SocketConnector::ssl_handshake_finish() could theoretically run twice under TLS 1.3 (once from the deferred on_write(), once from a subsequent on_read()). Added an is_handshake_finished_ guard so it only executes once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants