This is an automated email from the ASF dual-hosted git repository.

absurdfarce pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra-cpp-driver.git


The following commit(s) were added to refs/heads/trunk by this push:
     new badde68f CASSCPP-16 Revisit support for TLS 1.3
badde68f is described below

commit badde68f8f3f395846df3b491f97a609d674d227
Author: INSIGHTSOFTWARE\chintan.pansuriya 
<[email protected]>
AuthorDate: Mon Sep 7 14:54:03 2026 +0530

    CASSCPP-16 Revisit support for TLS 1.3
    
    patch by Chintan Pansuriya; reviewed by Bret McGuire and Yifan Cai
    reference: https://github.com/apache/cassandra-cpp-driver/pull/591
---
 include/cassandra.h                       |  3 ++-
 src/socket.cpp                            | 16 +++++++++++--
 src/socket.hpp                            | 15 +++++++-----
 src/socket_connector.cpp                  | 38 ++++++++++++++++++++++---------
 src/socket_connector.hpp                  |  7 ++++++
 src/ssl/ssl_openssl_impl.cpp              | 13 +++++++----
 tests/src/unit/http_server.cpp            |  4 ++--
 tests/src/unit/http_server.hpp            |  2 +-
 tests/src/unit/http_test.cpp              |  4 ++--
 tests/src/unit/http_test.hpp              |  2 +-
 tests/src/unit/mockssandra.cpp            | 35 +++++++++++++++++++---------
 tests/src/unit/mockssandra.hpp            |  2 +-
 tests/src/unit/tests/test_http_client.cpp | 16 +++++++++++--
 topics/building/README.md                 |  7 ++++++
 14 files changed, 120 insertions(+), 44 deletions(-)

diff --git a/include/cassandra.h b/include/cassandra.h
index c51bfa28..dc169a10 100644
--- a/include/cassandra.h
+++ b/include/cassandra.h
@@ -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_ {
diff --git a/src/socket.cpp b/src/socket.cpp
index 76b17651..ca1a4aeb 100644
--- a/src/socket.cpp
+++ b/src/socket.cpp
@@ -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()) {
+  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();
@@ -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 {
@@ -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;
@@ -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_) {
diff --git a/src/socket.hpp b/src/socket.hpp
index b54e56c2..c3f70cb0 100644
--- a/src/socket.hpp
+++ b/src/socket.hpp
@@ -206,12 +206,7 @@ public:
    *
    * @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() {}
 
@@ -260,6 +255,8 @@ protected:
   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_;
 };
@@ -298,6 +295,11 @@ public:
    */
   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.
@@ -367,6 +369,7 @@ private:
 
   uv_tcp_t tcp_;
   ScopedPtr<SocketHandlerBase> handler_;
+  size_t handler_generation_;
 
   SocketWriteBase::List pending_writes_;
   SocketWriteVec free_writes_;
diff --git a/src/socket_connector.cpp b/src/socket_connector.cpp
index 3d396a4d..d5fce7b7 100644
--- a/src/socket_connector.cpp
+++ b/src/socket_connector.cpp
@@ -65,6 +65,11 @@ public:
     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();
     }
   }
 
@@ -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;
@@ -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() {
diff --git a/src/socket_connector.hpp b/src/socket_connector.hpp
index c7a1d33e..da48007a 100644
--- a/src/socket_connector.hpp
+++ b/src/socket_connector.hpp
@@ -138,6 +138,7 @@ public:
 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);
@@ -167,6 +168,12 @@ private:
 
   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_;
 };
 
diff --git a/src/ssl/ssl_openssl_impl.cpp b/src/ssl/ssl_openssl_impl.cpp
index 596fe904..badf3c8a 100644
--- a/src/ssl/ssl_openssl_impl.cpp
+++ b/src/ssl/ssl_openssl_impl.cpp
@@ -540,10 +540,6 @@ OpenSslContext::OpenSslContext()
     , trusted_store_(X509_STORE_new()) {
   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);
-#endif
 #if DEBUG_SSL
   SSL_CTX_set_info_callback(ssl_ctx_, ssl_info_callback);
 #endif
@@ -632,6 +628,15 @@ CassError 
OpenSslContext::set_min_protocol_version(CassSslTlsVersion min_version
     case CassSslTlsVersion::CASS_SSL_VERSION_TLS1_2:
       method = TLS1_2_VERSION;
       break;
+/* TLS 1.3 support came along with OpenSSL 1.1.1 so only add it if it's 
actually present.
+
+   Note that this means the library can be compiled _without_ TLS 1.3 support 
if an older
+   OpenSSL lib is used. */
+#ifdef TLS1_3_VERSION
+    case CassSslTlsVersion::CASS_SSL_VERSION_TLS1_3:
+      method = TLS1_3_VERSION;
+      break;
+#endif
     default:
       // unsupported version
       return CASS_ERROR_LIB_BAD_PARAMS;
diff --git a/tests/src/unit/http_server.cpp b/tests/src/unit/http_server.cpp
index dcd6d608..4fa152b7 100644
--- a/tests/src/unit/http_server.cpp
+++ b/tests/src/unit/http_server.cpp
@@ -60,8 +60,8 @@ void Server::close() {
 }
 
 bool Server::use_ssl(const String& key, const String& cert, const String& 
ca_cert /*= ""*/,
-                     bool require_client_cert /*= false*/) {
-  return server_connection_->use_ssl(key, cert, ca_cert, require_client_cert);
+                     bool require_client_cert /*= false*/, int max_tls_version 
/* = 0 */) {
+  return server_connection_->use_ssl(key, cert, ca_cert, require_client_cert, 
max_tls_version);
 }
 
 Server::ClientConnection::ClientConnection(internal::ServerConnection* 
server_connection,
diff --git a/tests/src/unit/http_server.hpp b/tests/src/unit/http_server.hpp
index 48b7d6c3..3e5f2522 100644
--- a/tests/src/unit/http_server.hpp
+++ b/tests/src/unit/http_server.hpp
@@ -65,7 +65,7 @@ public:
   }
 
   bool use_ssl(const String& key, const String& cert, const String& ca_cert = 
"",
-               bool require_client_cert = false);
+               bool require_client_cert = false, int max_tls_version = 0);
 
   void listen();
   void close();
diff --git a/tests/src/unit/http_test.cpp b/tests/src/unit/http_test.cpp
index f66db06f..82d48b74 100644
--- a/tests/src/unit/http_test.cpp
+++ b/tests/src/unit/http_test.cpp
@@ -52,12 +52,12 @@ SocketSettings HttpTest::use_ssl(const String& cn, bool 
is_server_using_ssl /*=
   return settings;
 }
 
-void HttpTest::use_ssl(const String& ca_key, const String& ca_cert, const 
String& cn) {
+void HttpTest::use_ssl(const String& ca_key, const String& ca_cert, const 
String& cn, int max_tls_version /* = 0 */) {
 #ifdef HAVE_OPENSSL
   key_ = mockssandra::Ssl::generate_key();
   cert_ = mockssandra::Ssl::generate_cert(key_, cn, ca_cert, ca_key);
   ca_cert_ = ca_cert;
 
-  server_.use_ssl(key_, cert_, ca_cert_, true);
+  server_.use_ssl(key_, cert_, ca_cert_, true, max_tls_version);
 #endif
 }
diff --git a/tests/src/unit/http_test.hpp b/tests/src/unit/http_test.hpp
index deee07ac..f1cd7090 100644
--- a/tests/src/unit/http_test.hpp
+++ b/tests/src/unit/http_test.hpp
@@ -55,7 +55,7 @@ public:
   datastax::internal::core::SocketSettings use_ssl(const String& cn = 
HTTP_MOCK_HOSTNAME,
                                                    bool is_server_using_ssl = 
true);
 
-  void use_ssl(const String& ca_cert, const String& ca_key, const String& cn);
+  void use_ssl(const String& ca_cert, const String& ca_key, const String& cn, 
int max_tls_version = 0);
 
 private:
   datastax::String ca_cert_;
diff --git a/tests/src/unit/mockssandra.cpp b/tests/src/unit/mockssandra.cpp
index f30b22eb..c99cf648 100644
--- a/tests/src/unit/mockssandra.cpp
+++ b/tests/src/unit/mockssandra.cpp
@@ -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)
@@ -491,7 +493,8 @@ uv_loop_t* ServerConnection::loop() {
 
 bool ServerConnection::use_ssl(const String& key, const String& cert,
                                const String& ca_cert /*= ""*/,
-                               bool require_client_cert /*= false*/) {
+                               bool require_client_cert /*= false*/,
+                               int max_tls_version /* = 0 */) {
   if (ssl_context_) {
     SSL_CTX_free(ssl_context_);
   }
@@ -505,6 +508,16 @@ bool ServerConnection::use_ssl(const String& key, const 
String& cert,
   SSL_CTX_set_default_passwd_cb(ssl_context_, on_password);
   SSL_CTX_set_verify(ssl_context_, SSL_VERIFY_NONE, NULL);
 
+  /* CASSCPP-16
+
+    Default max for TLS negotiation with clients will be whatever's supported 
by
+    the underlying OpenSSL impl (in order to maximally test the newest version)
+    but some tests may include behaviours that rely on earlier TLS versions.
+  */
+  if (max_tls_version) {
+    SSL_CTX_set_max_proto_version(ssl_context_, max_tls_version);
+  }
+
   { // Load server certificate
     Scoped<X509> x509(load_cert(cert));
     if (!x509) return false;
diff --git a/tests/src/unit/mockssandra.hpp b/tests/src/unit/mockssandra.hpp
index 26050fd9..e5be182f 100644
--- a/tests/src/unit/mockssandra.hpp
+++ b/tests/src/unit/mockssandra.hpp
@@ -176,7 +176,7 @@ public:
   const ClientConnections& clients() const { return clients_; }
 
   bool use_ssl(const String& key, const String& cert, const String& ca_cert = 
"",
-               bool require_client_cert = false);
+               bool require_client_cert = false, int max_tls_version = 0);
   void weaken_ssl();
 
   void listen(EventLoopGroup* event_loop_group);
diff --git a/tests/src/unit/tests/test_http_client.cpp 
b/tests/src/unit/tests/test_http_client.cpp
index bba6ddbc..350f0c6e 100644
--- a/tests/src/unit/tests/test_http_client.cpp
+++ b/tests/src/unit/tests/test_http_client.cpp
@@ -210,7 +210,13 @@ TEST_F(HttpClientUnitTest, NoClientCertProvidedSsl) {
   String ca_key = mockssandra::Ssl::generate_key();
   String ca_cert = mockssandra::Ssl::generate_cert(ca_key, "CA");
 
-  use_ssl(ca_key, ca_cert, HTTP_MOCK_HOSTNAME);
+  /* CASSCPP-16
+
+     Disable TLS 1.3 here so that we're only testing against TLS 1.2.  Since 
client cert compares
+     don't happen until the first bit of client data is read this test would 
need to be restructured
+     to work with TLS 1.3.  We can pursue this in later work but for now we 
just constrain this
+     test at the earlier TLS version. */
+  use_ssl(ca_key, ca_cert, HTTP_MOCK_HOSTNAME, TLS1_2_VERSION);
   start_http_server();
 
   bool is_failed = false;
@@ -240,7 +246,13 @@ TEST_F(HttpClientUnitTest, InvalidClientCertSsl) {
   String client_key = mockssandra::Ssl::generate_key();
   String client_cert = mockssandra::Ssl::generate_cert(client_key, ""); // 
Self-signed
 
-  use_ssl(ca_key, ca_cert, HTTP_MOCK_HOSTNAME);
+  /* CASSCPP-16
+
+     Disable TLS 1.3 here so that we're only testing against TLS 1.2.  Since 
client cert compares
+     don't happen until the first bit of client data is read this test would 
need to be restructured
+     to work with TLS 1.3.  We can pursue this in later work but for now we 
just constrain this
+     test at the earlier TLS version. */
+  use_ssl(ca_key, ca_cert, HTTP_MOCK_HOSTNAME, TLS1_2_VERSION);
   start_http_server();
 
   bool is_failed = false;
diff --git a/topics/building/README.md b/topics/building/README.md
index 17a99989..5e01c036 100644
--- a/topics/building/README.md
+++ b/topics/building/README.md
@@ -48,6 +48,11 @@ the [migration guide] for details).  The driver does not use 
any of these functi
 3.x to be relatively painless.  Note that two officially supported platforms 
(Ubuntu 22.04 and Rocky Linux 9.2) come with
 OpenSSL 3.x by default and the unit and integration tests all pass on these 
platforms.
 
+### A Brief Note on TLS 1.3
+TLS 1.3 support was introduced to OpenSSL with the [1.1.1 release].  As of 
version 2.17.2 the driver includes support for
+at least basic connectivity with TLS 1.3.  Note that TLS 1.3 support will only 
be available if a version of OpenSSL >= 1.1.1
+is utilized at build-time.  See [PR 591] for more on this point.
+
 ## Linux/Mac OS
 
 The driver is known to build on Rocky Linux 8/9, Mac OS X 10.10/10.11 (Yosemite
@@ -342,3 +347,5 @@ cmake -G "Visual Studio 16 2019" -A x64 
-DCASS_BUILD_UNIT_TESTS=On ..
 [OpenSSL]: https://www.openssl.org
 [zlib]: https://www.zlib.net
 [migration guide]: 
https://www.openssl.org/docs/man3.0/man7/migration_guide.html
+[1.1.1 release]: https://github.com/openssl/openssl/wiki/TLS1.3
+[PR 591]: https://github.com/apache/cassandra-cpp-driver/pull/591
\ No newline at end of file


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to