You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "cpansuriya-simba (via GitHub)" <gi...@apache.org> on 2026/09/08 06:48:35 UTC
[PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
cpansuriya-simba opened a new pull request, #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591
**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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#discussion_r4186381706
##########
src/ssl/ssl_openssl_impl.cpp:
##########
@@ -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);
Review Comment:
This kinda sucks, though, 'cause even if I do this based on TLS1_3_VERSION it's still a compile-time thing. It's entirely realistic that we could compile against an older OpenSSL (i.e. pre-1.1.1) and run against 1.1.1 or later... in that case we'd really want TLS 1.3 to be used. Something like this:
```
#ifdef TLS1_3_VERSION
SSL_CTX_set_max_proto_version(ssl_ctx_, TLS1_3_VERSION);
#else
SSL_CTX_set_max_proto_version(ssl_ctx_, TLS1_2_VERSION);
#endif
```
Doesn't really allow for that.
I'm wondering if there's a _runtime_ function to detect whether TLS 1.3 is available.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#discussion_r4191184740
##########
src/ssl/ssl_openssl_impl.cpp:
##########
@@ -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;
Review Comment:
Gate added, this should be resolved now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "cpansuriya-simba (via GitHub)" <gi...@apache.org>.
cpansuriya-simba commented on code in PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#discussion_r4024813749
##########
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()) {
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#discussion_r4117235141
##########
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()) {
Review Comment:
Ah, got it, thanks for the explanation @cpansuriya-simba! That does indeed clear it up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#issuecomment-6002539585
FTR Jenkins run against the changes at this point was green so we haven't seen any regressions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce merged PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#discussion_r4186351219
##########
src/ssl/ssl_openssl_impl.cpp:
##########
@@ -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);
Review Comment:
Also a good catch! Yeah, I think I like that better... I'll make that change as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#issuecomment-5859815730
I can confirm that with the most recent change PoolUnitTest and RequestProcessorUnitTest both now pass. Kicked off a full Jenkins run on the branch with these updates to get a better sense of where we are overall.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "cpansuriya-simba (via GitHub)" <gi...@apache.org>.
cpansuriya-simba commented on PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#issuecomment-5714573614
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#discussion_r4188551373
##########
tests/src/unit/mockssandra.cpp:
##########
@@ -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*/,
+ bool enable_tls_13 /*= true*/) {
Review Comment:
Default is now to _not_ set a max value at all... we only do that in cases where a test passes in some non-zero value for the TLS constant. In all other cases OpenSSL performs negotiation as it normally should, which (I would argue) is exactly what we want.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#discussion_r4191240847
##########
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
Review Comment:
Per the [earlier conversation](https://github.com/apache/cassandra-cpp-driver/pull/591#discussion_r4181025089) this gate based on OpenSSL lib version has been removed completely. We now gate inclusion of TLS 1.3 into the supported version based on the availability of TLS 1.3 support (see below) but we shouldn't need other bounds.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#discussion_r4187928771
##########
src/ssl/ssl_openssl_impl.cpp:
##########
@@ -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;
Review Comment:
This case will also face compilation problems and should be gated. Upshot would be that depending on the OpenSSL version used for compilation a driver lib might or might not have TLS 1.3 support. Unit and integration tests _will_ require TLS 1.3 support just 'cause we need to validate behaviours against the current supported TLS version. There's an interesting conversation to have about whether we should support unit/integration tests against arbitrary TLS versions... but that's probably unrelated to this ticket.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#issuecomment-6021571325
Just leaving this as a note here: future tickets mentioned above have been filed as CASSCPP-18, CASSCPP-19 and CASSCPP-20.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#discussion_r4186187013
##########
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*/, bool enable_tls_13 /*= false*/) {
Review Comment:
Yeah, this looks like a copy/paste error. Will fix; good catch!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#discussion_r4191246105
##########
tests/src/unit/http_server.hpp:
##########
@@ -65,7 +65,7 @@ class Server {
}
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);
Review Comment:
My hope is that shifting to a parameter that explicitly specifies a max TLS version is clearer and easier to understand than the old "enable TLS 1.3" parameter.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#issuecomment-6008630630
Note to self (and perhaps others): there's a larger conversation to have here about whether we should support unit/integration test runs with a configurable TLS version (i.e. should we expose it as a gtest flag). There seem to be decent arguments both ways, but regardless that concern is out-of-band for the current scope of work for this ticket.
I'm going to file a separate ticket to consider that question more broadly, likely for a future release; I don't see any reason to hold up 2.17.2 in order to get that resolved.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#issuecomment-5860430407
With the changes above I'm still seeing the following test failures:
```
[ RUN ] HttpClientUnitTest.NoClientCertProvidedSsl
SSL error: error:0A0000C7:SSL routines::peer did not return a certificate
/home/mersault/work/git/cpp-driver/tests/src/unit/tests/test_http_client.cpp:42: Failure
Value of: client->is_ok()
Actual: true
Expected: false
/home/mersault/work/git/cpp-driver/tests/src/unit/tests/test_http_client.cpp:231: Failure
Expected: client->error_code()
Which is: 0
To be equal to: HttpClient::HTTP_CLIENT_ERROR_SOCKET
Which is: 2
[ FAILED ] HttpClientUnitTest.NoClientCertProvidedSsl (59 ms)
[ RUN ] HttpClientUnitTest.InvalidClientCertSsl
SSL error: error:0A000086:SSL routines::certificate verify failed
/home/mersault/work/git/cpp-driver/tests/src/unit/tests/test_http_client.cpp:42: Failure
Value of: client->is_ok()
Actual: true
Expected: false
/home/mersault/work/git/cpp-driver/tests/src/unit/tests/test_http_client.cpp:263: Failure
Expected: client->error_code()
Which is: 0
To be equal to: HttpClient::HTTP_CLIENT_ERROR_SOCKET
Which is: 2
[ FAILED ] HttpClientUnitTest.InvalidClientCertSsl (50 ms)
…
[----------] 12 tests from HttpClientUnitTest (611 ms total)
[----------] Global test environment tear-down
[==========] 12 tests from 1 test case ran. (611 ms total)
[ PASSED ] 10 tests.
[ FAILED ] 2 tests, listed below:
[ FAILED ] HttpClientUnitTest.NoClientCertProvidedSsl
[ FAILED ] HttpClientUnitTest.InvalidClientCertSsl
```
These are _not_ failing on trunk... but I can't see any obvious way in which the changes specified here would result in these specific errors. Still digging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#discussion_r4186197714
##########
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, bool enable_tls_13 /*= false*/) {
Review Comment:
Also copy/paste error; will fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#discussion_r4186397181
##########
src/ssl/ssl_openssl_impl.cpp:
##########
@@ -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);
Review Comment:
Wait, I'm overthinking this... what's the case for bounding the TLS version _at all_ now?!?!?! The whole reason it was done before was because TLS 1.3 explicitly broke things... but this PR explicitly changes that. We should be able to remove the SSL_CTX_set_max_proto_version() call all together now and just let OpenSSL determine it in whatever way it sees fit, right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#issuecomment-6002535329
Ready for another review @yifan-c. I think everything you raised has been addressed and I _know_ I'm much happier with where this is now than where it was when you took your first pass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#issuecomment-5693255499
Ping @yifan-c for visibility as to what's going on here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#issuecomment-5915638011
Jenkins run after test fixes above was green so I think we're good to go here. @yifan-c can you take a look at this one when you have some time?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "yifan-c (via GitHub)" <gi...@apache.org>.
yifan-c commented on code in PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#discussion_r4181025089
##########
src/ssl/ssl_openssl_impl.cpp:
##########
@@ -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);
Review Comment:
https://github.com/openssl/openssl/wiki/TLS1.3
`TLS1_3_VERSION` only exists in OpenSSL 1.1.1 and later. The condition `OPENSSL_VERSION_NUMBER >= 0x10100000L` will fail to compile with OpenSSL 1.1.0
Instead of guarding the version, maybe guarding against the availability of `TLS1_3_VERSION`.
##########
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*/, bool enable_tls_13 /*= false*/) {
Review Comment:
`enable_tls_13 = true` in the header file. The comment on the default value `false` is not true.
##########
tests/src/unit/mockssandra.cpp:
##########
@@ -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*/,
+ bool enable_tls_13 /*= true*/) {
Review Comment:
It's interesting that the default of `enable_tls_13` is false. Can we update the header?
##########
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, bool enable_tls_13 /*= false*/) {
Review Comment:
`enable_tls_13` default is `true`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#discussion_r4191207702
##########
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
Review Comment:
Another ticket to be filed, this one likely for 2.17.2.
I wound up reviewing a fair amount of the content on this page while adding this section and... well, we basically need to give the whole thing a once-over. There's _a lot_ of content here that doesn't make much sense anymore.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "yifan-c (via GitHub)" <gi...@apache.org>.
yifan-c commented on code in PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#discussion_r4191966666
##########
tests/src/unit/http_server.hpp:
##########
@@ -65,7 +65,7 @@ class Server {
}
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);
Review Comment:
Sure. So `0` means using library default.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#issuecomment-5693253224
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#discussion_r4023184229
##########
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()) {
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#discussion_r4188535659
##########
src/ssl/ssl_openssl_impl.cpp:
##########
@@ -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);
Review Comment:
In the end it just made sense to remove this constraint all together.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#issuecomment-5883670301
The changes in [pr591.diff.txt](https://github.com/user-attachments/files/32784911/pr591.diff.txt) will get the test passing again. It adds an extra arg to use_ssl() which enables the ability to enable TLS 1.3 (with a default value of true). The two HTTP tests that are failing can then use this flag to disable TLS 1.3... which causes the tests to pass (as expected).
This is a pretty contrived and brittle approach to solving the problem and isn't something I want to do long-term. But as mentioned above we need to come up with a better mechanism to specify when certain tests require certain behaviours like this. I'm comfortable going with an approach something like this one for now with the expectation that we'll clean this up in a later release. The goal at the moment is to get 2.17.2 out the door (ideally including this fix) and a semi-hack like the one described here does that job.
Outside of that I think everything else has been addressed @cpansuriya-simba so if you're good with the change proposed here (or something like it) you can apply it to the PR... or I can apply it on your behalf if you prefer; it's entirely up to you. Once that's in this guy should be ready to go.
@yifan-c I'm giving you a heads-up on this one as well so that you can do a review and give me an additional +1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#issuecomment-5877934874
With a little bit of digging and some help from Gemini I'm pretty sure I understand the HttpClientUnitTest failures. It's something similar to the changes that were put in place for socket connections; with TLS 1.3 cert validations aren't completed until the client writes their first bit of data (due to the RTT changes) which means we won't see the expected errors at connect-time anymore.
We should be able to validate this by constraining the test to TLS 1.2 and confirming that this gets everything passing. We can then potentially implement different validation procedures depending on whether we want to test TLS 1.2 (or earlier) or TLS 1.3. This ties into the point I made earlier about whether we need different testing regimes for the two different lines under test ("TLS 1.3" and "everything before TLS 1.3"). That's probably a larger conversation (to say nothing about how we want to automate that testing in a CI context) so I'm happy deferring that to a subsequent release.
For now I think we're good enough to (a) constrain the test to TLS 1.2 (assuming that does indeed fix the problem) and (b) mark the conditional test behaviour as a TODO and create a follow-up ticket to consider the prospect of whether/how to test the two different lines referenced above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "cpansuriya-simba (via GitHub)" <gi...@apache.org>.
cpansuriya-simba commented on PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#issuecomment-5909340478
@absurdfarce it would be good if you can apply suggested changes on behalf of me. Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSCPP-16 Fix TLS 1.3 handshake race with in-flight Finished write [cassandra-cpp-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #591:
URL: https://github.com/apache/cassandra-cpp-driver/pull/591#discussion_r4186172939
##########
tests/src/unit/mockssandra.cpp:
##########
@@ -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*/,
+ bool enable_tls_13 /*= true*/) {
Review Comment:
I kept the default for mockssandra as TLS 1.2 precisely because there are some tests (the HttpClientUnitTests mentioned above) which fail when run with TLS 1.3. As we talk about this I'm coming around to what I think is your larger point; we can invert that sense (the same way we did for the driver itself) and make TLS 1.3 the default for mockssandra connections, allowing tests to mark it false in cases where TLS 1.3 just doesn't make sense (like HttpClientUnitTests). I'm all right with that... I'll make that change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org