From 923ec0fd64be569bada0ef5c04db197b7edc8796 Mon Sep 17 00:00:00 2001 From: FalacerSelene Date: Thu, 24 Mar 2022 14:46:28 +0000 Subject: [PATCH 1/4] Allow users to request TLS client-side enforcement --- include/cassandra.h | 20 ++++++++++++++++ src/ssl.cpp | 4 ++++ src/ssl.hpp | 1 + src/ssl/ssl_no_impl.cpp | 4 ++++ src/ssl/ssl_no_impl.hpp | 1 + src/ssl/ssl_openssl_impl.cpp | 44 ++++++++++++++++++++++++++++++++++++ src/ssl/ssl_openssl_impl.hpp | 1 + 7 files changed, 75 insertions(+) diff --git a/include/cassandra.h b/include/cassandra.h index 5fa1cd015..f653e155b 100644 --- a/include/cassandra.h +++ b/include/cassandra.h @@ -629,6 +629,12 @@ typedef enum CassSslVerifyFlags_ { CASS_SSL_VERIFY_PEER_IDENTITY_DNS = 0x04 } CassSslVerifyFlags; +typedef enum CassSslTlsVersion_ { + CASS_SSL_VERSION_TLS1 = 0x00, + CASS_SSL_VERSION_TLS1_1 = 0x01, + CASS_SSL_VERSION_TLS1_2 = 0x02, +} CassSslTlsVersion; + typedef enum CassProtocolVersion_ { CASS_PROTOCOL_VERSION_V1 = 0x01, /**< Deprecated */ CASS_PROTOCOL_VERSION_V2 = 0x02, /**< Deprecated */ @@ -4687,6 +4693,20 @@ cass_ssl_set_private_key_n(CassSsl* ssl, const char* password, size_t password_length); +/** + * Set minimum supported client-side protocol version. This will prevent the + * connection using protocol versions earlier than the specified one. Useful + * for preventing TLS downgrade attacks. + * + * @public @memberof CassSsl + * + * @param[in] ssl + * @param[in] min_version + * @return CASS_OK if successful, otherwise an error occurred. + */ +CASS_EXPORT CassError +cass_ssl_set_min_protocol_version(CassSsl* ssl, CassSslTlsVersion min_version); + /*********************************************************************************** * * Authenticator diff --git a/src/ssl.cpp b/src/ssl.cpp index 679a73429..fbe1e0f5a 100644 --- a/src/ssl.cpp +++ b/src/ssl.cpp @@ -65,6 +65,10 @@ CassError cass_ssl_set_private_key_n(CassSsl* ssl, const char* key, size_t key_l return ssl->set_private_key(key, key_length, password, password_length); } +CassError cass_ssl_set_min_protocol_version(CassSsl* ssl, CassSslTlsVersion min_version) { + return ssl->set_min_protocol_version(min_version); +} + } // extern "C" template diff --git a/src/ssl.hpp b/src/ssl.hpp index 495aa7357..16ef85ea2 100644 --- a/src/ssl.hpp +++ b/src/ssl.hpp @@ -87,6 +87,7 @@ class SslContext : public RefCounted { virtual CassError set_cert(const char* cert, size_t cert_length) = 0; virtual CassError set_private_key(const char* key, size_t key_length, const char* password, size_t password_length) = 0; + virtual CassError set_min_protocol_version(CassSslTlsVersion min_version) = 0; protected: int verify_flags_; diff --git a/src/ssl/ssl_no_impl.cpp b/src/ssl/ssl_no_impl.cpp index ff539211d..947b3c4a0 100644 --- a/src/ssl/ssl_no_impl.cpp +++ b/src/ssl/ssl_no_impl.cpp @@ -44,4 +44,8 @@ CassError NoSslContext::set_private_key(const char* key, size_t key_length, cons return CASS_ERROR_LIB_NOT_IMPLEMENTED; } +CassError NoSslContext::set_min_protocol_version(CassSslTlsVersion min_version) { + return CASS_ERROR_LIB_NOT_IMPLEMENTED; +} + SslContext::Ptr NoSslContextFactory::create() { return SslContext::Ptr(new NoSslContext()); } diff --git a/src/ssl/ssl_no_impl.hpp b/src/ssl/ssl_no_impl.hpp index fabbb5634..cef98b905 100644 --- a/src/ssl/ssl_no_impl.hpp +++ b/src/ssl/ssl_no_impl.hpp @@ -40,6 +40,7 @@ class NoSslContext : public SslContext { virtual CassError set_cert(const char* cert, size_t cert_length); virtual CassError set_private_key(const char* key, size_t key_length, const char* password, size_t password_length); + virtual CassError set_min_protocol_version(CassSslTlsVersion min_version); }; class NoSslContextFactory : public SslContextFactoryBase { diff --git a/src/ssl/ssl_openssl_impl.cpp b/src/ssl/ssl_openssl_impl.cpp index 80e2ab6a4..524d494de 100644 --- a/src/ssl/ssl_openssl_impl.cpp +++ b/src/ssl/ssl_openssl_impl.cpp @@ -41,12 +41,14 @@ !defined(LIBRESSL_VERSION_NUMBER) // Required as OPENSSL_VERSION_NUMBER for LibreSSL is defined // as 2.0.0 #if (OPENSSL_VERSION_NUMBER >= 0x10100000L) +#define SSL_CAN_SET_MIN_VERSION #define SSL_CLIENT_METHOD TLS_client_method #else #define SSL_CLIENT_METHOD SSLv23_client_method #endif #else #if (LIBRESSL_VERSION_NUMBER >= 0x20302000L) +#define SSL_CAN_SET_MIN_VERSION #define SSL_CLIENT_METHOD TLS_client_method #else #define SSL_CLIENT_METHOD SSLv23_client_method @@ -611,6 +613,48 @@ CassError OpenSslContext::set_private_key(const char* key, size_t key_length, co return CASS_OK; } +CassError OpenSslContext::set_min_protocol_version(CassSslTlsVersion min_version) { +#ifdef SSL_CAN_SET_MIN_VERSION + int method; + switch (min_version) { + case CassSslTlsVersion::CASS_SSL_VERSION_TLS1: + method = TLS1_VERSION; + break; + case CassSslTlsVersion::CASS_SSL_VERSION_TLS1_1: + method = TLS1_1_VERSION; + break; + case CassSslTlsVersion::CASS_SSL_VERSION_TLS1_2: + method = TLS1_2_VERSION; + break; + default: + // unsupported version + return CASS_ERROR_LIB_BAD_PARAMS; + } + SSL_CTX_set_min_proto_version(ssl_ctx_, method); + return CASS_OK; +#else + // If we don't have the `set_min_proto_version` function then we do this via + // the (deprecated in later versions) options function. + int options = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3; + switch (min_version) { + case CassSslTlsVersion::CASS_SSL_VERSION_TLS1: + break; + case CassSslTlsVersion::CASS_SSL_VERSION_TLS1_1: + options |= SSL_OP_NO_TLSv1; + break; + case CassSslTlsVersion::CASS_SSL_VERSION_TLS1_2: + options |= SSL_OP_NO_TLSv1; + options |= SSL_OP_NO_TLSv1_1; + break; + default: + // unsupported version + return CASS_ERROR_LIB_BAD_PARAMS; + } + SSL_CTX_set_options(ssl_ctx_, options); + return CASS_OK; +#endif +} + SslContext::Ptr OpenSslContextFactory::create() { return SslContext::Ptr(new OpenSslContext()); } namespace openssl { diff --git a/src/ssl/ssl_openssl_impl.hpp b/src/ssl/ssl_openssl_impl.hpp index f347caaa8..6f3880e91 100644 --- a/src/ssl/ssl_openssl_impl.hpp +++ b/src/ssl/ssl_openssl_impl.hpp @@ -61,6 +61,7 @@ class OpenSslContext : public SslContext { virtual CassError set_cert(const char* cert, size_t cert_length); virtual CassError set_private_key(const char* key, size_t key_length, const char* password, size_t password_length); + virtual CassError set_min_protocol_version(CassSslTlsVersion min_version); private: SSL_CTX* ssl_ctx_; From 500f9c74c0a12ab460ab41320f708bdd4de45199 Mon Sep 17 00:00:00 2001 From: FalacerSelene Date: Fri, 25 Mar 2022 10:17:40 +0000 Subject: [PATCH 2/4] Add unit testing for min_tls_version Test covers trying to connect to a server that only supports TLS 1.0-, while trying to enforce TLS 1.2+ on client-side. As expected, the connection does not succeed. --- tests/src/unit/mockssandra.cpp | 17 +++++++++++++++++ tests/src/unit/mockssandra.hpp | 13 ++++++++++++- tests/src/unit/tests/test_socket.cpp | 21 +++++++++++++++++++++ 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/tests/src/unit/mockssandra.cpp b/tests/src/unit/mockssandra.cpp index 94b1cc8f6..71cddbd5e 100644 --- a/tests/src/unit/mockssandra.cpp +++ b/tests/src/unit/mockssandra.cpp @@ -52,12 +52,14 @@ using datastax::internal::core::UuidGen; !defined(LIBRESSL_VERSION_NUMBER) // Required as OPENSSL_VERSION_NUMBER for LibreSSL is defined // as 2.0.0 #if (OPENSSL_VERSION_NUMBER >= 0x10100000L) +#define SSL_CAN_SET_MAX_VERSION #define SSL_SERVER_METHOD TLS_server_method #else #define SSL_SERVER_METHOD SSLv23_server_method #endif #else #if (LIBRESSL_VERSION_NUMBER >= 0x20302000L) +#define SSL_CAN_SET_MAX_VERSION #define SSL_SERVER_METHOD TLS_server_method #else #define SSL_SERVER_METHOD SSLv23_server_method @@ -555,6 +557,21 @@ bool ServerConnection::use_ssl(const String& key, const String& cert, return true; } +// Weaken the SSL connection, enforcing that it can only use TLS1.0 at max. +// This is used for testing client-side enforcement of more secure TLS +// protocols. +void ServerConnection::weaken_ssl() { + if (!ssl_context_) { + return; + } + +#ifdef SSL_CAN_SET_MAX_VERSION + SSL_CTX_set_max_proto_version(ssl_context_, TLS1_VERSION); +#else + SSL_CTX_set_options(ssl_context_, SSL_OP_NO_TLSv1_1 | SSL_OP_NO_TLSv1_2); +#endif +} + using datastax::internal::core::Task; class RunListen : public Task { diff --git a/tests/src/unit/mockssandra.hpp b/tests/src/unit/mockssandra.hpp index 1f57cab42..ecd3d45f7 100644 --- a/tests/src/unit/mockssandra.hpp +++ b/tests/src/unit/mockssandra.hpp @@ -175,6 +175,7 @@ class ServerConnection : public RefCounted { bool use_ssl(const String& key, const String& cert, const String& ca_cert = "", bool require_client_cert = false); + void weaken_ssl(); void listen(EventLoopGroup* event_loop_group); int wait_listen(); @@ -1161,6 +1162,7 @@ class Cluster { ~Cluster(); String use_ssl(const String& cn = ""); + void weaken_ssl(); int start_all(EventLoopGroup* event_loop_group); void start_all_async(EventLoopGroup* event_loop_group); @@ -1264,7 +1266,8 @@ class SimpleEchoServer { public: SimpleEchoServer() : factory_(new EchoClientConnectionFactory()) - , event_loop_group_(1) {} + , event_loop_group_(1) + , ssl_weaken_(false) {} ~SimpleEchoServer() { close(); } @@ -1281,6 +1284,8 @@ class SimpleEchoServer { return ssl_cert_; } + void weaken_ssl() { ssl_weaken_ = true; } + void use_connection_factory(internal::ClientConnectionFactory* factory) { factory_.reset(factory); } @@ -1290,6 +1295,11 @@ class SimpleEchoServer { if (!ssl_key_.empty() && !ssl_cert_.empty() && !server_->use_ssl(ssl_key_, ssl_cert_)) { return -1; } + + if (ssl_weaken_) { + server_->weaken_ssl(); + } + server_->listen(&event_loop_group_); return server_->wait_listen(); } @@ -1316,6 +1326,7 @@ class SimpleEchoServer { internal::ServerConnection::Ptr server_; String ssl_key_; String ssl_cert_; + bool ssl_weaken_; }; } // namespace mockssandra diff --git a/tests/src/unit/tests/test_socket.cpp b/tests/src/unit/tests/test_socket.cpp index 37bf730be..9f7123c2c 100644 --- a/tests/src/unit/tests/test_socket.cpp +++ b/tests/src/unit/tests/test_socket.cpp @@ -135,6 +135,8 @@ class SocketUnitTest : public LoopTest { return settings; } + void weaken_ssl() { server_.weaken_ssl(); } + void listen(const Address& address = Address("127.0.0.1", 8888)) { ASSERT_EQ(server_.listen(address), 0); } @@ -409,3 +411,22 @@ TEST_F(SocketUnitTest, SslVerifyIdentityDns) { EXPECT_EQ(result, "The socket is successfully connected and wrote data - Closed"); } + +TEST_F(SocketUnitTest, SslEnforceTlsVersion) { + SocketSettings settings(use_ssl("127.0.0.1")); + weaken_ssl(); + + listen(); + + settings.ssl_context->set_min_protocol_version(CASS_SSL_VERSION_TLS1_2); + + bool is_closed; + SocketConnector::Ptr connector( + new SocketConnector(Address("127.0.0.1", 8888), bind_callback(on_socket_closed, &is_closed))); + + connector->with_settings(settings)->connect(loop()); + + uv_run(loop(), UV_RUN_DEFAULT); + + EXPECT_TRUE(is_closed); +} From e279ae368e452517221835ce57027bdffc57a5c3 Mon Sep 17 00:00:00 2001 From: FalacerSelene Date: Mon, 28 Mar 2022 10:19:58 +0000 Subject: [PATCH 3/4] Auto-formatter compliance --- include/cassandra.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/cassandra.h b/include/cassandra.h index f653e155b..c1e5fdc71 100644 --- a/include/cassandra.h +++ b/include/cassandra.h @@ -632,7 +632,7 @@ 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 } CassSslTlsVersion; typedef enum CassProtocolVersion_ { From ffc0605e3a93541e11b20053302157607f29d64f Mon Sep 17 00:00:00 2001 From: FalacerSelene Date: Tue, 19 Apr 2022 14:47:42 +0000 Subject: [PATCH 4/4] Support socket handshake failures in tests --- tests/src/unit/tests/test_socket.cpp | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/src/unit/tests/test_socket.cpp b/tests/src/unit/tests/test_socket.cpp index 9f7123c2c..a44c53782 100644 --- a/tests/src/unit/tests/test_socket.cpp +++ b/tests/src/unit/tests/test_socket.cpp @@ -187,6 +187,17 @@ class SocketUnitTest : public LoopTest { } } + /* SSL handshake failures have different error codes on different versions of + * OpenSSL - this accounts for both of them + */ + static void on_socket_ssl_error(SocketConnector* connector, bool* is_error) { + SocketConnector::SocketError err = connector->error_code(); + if ((err == SocketConnector::SOCKET_ERROR_CLOSE) || + (err == SocketConnector::SOCKET_ERROR_SSL_HANDSHAKE)) { + *is_error = true; + } + } + static void on_socket_canceled(SocketConnector* connector, bool* is_canceled) { if (connector->is_canceled()) { *is_canceled = true; @@ -420,13 +431,13 @@ TEST_F(SocketUnitTest, SslEnforceTlsVersion) { settings.ssl_context->set_min_protocol_version(CASS_SSL_VERSION_TLS1_2); - bool is_closed; - SocketConnector::Ptr connector( - new SocketConnector(Address("127.0.0.1", 8888), bind_callback(on_socket_closed, &is_closed))); + bool is_error; + SocketConnector::Ptr connector(new SocketConnector( + Address("127.0.0.1", 8888), bind_callback(on_socket_ssl_error, &is_error))); connector->with_settings(settings)->connect(loop()); uv_run(loop(), UV_RUN_DEFAULT); - EXPECT_TRUE(is_closed); + EXPECT_TRUE(is_error); }