From 3bea74288a693a46abccba5b5186f334f1e6afe6 Mon Sep 17 00:00:00 2001 From: m8mble Date: Thu, 26 Mar 2020 19:34:41 +0100 Subject: [PATCH 1/9] Add cass_ssl_set_default_verify_paths API Forwards SSL-configuration to use system default directories for finding certificate authorities. --- include/cassandra.h | 12 ++++++++++++ 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 | 10 ++++++++++ src/ssl/ssl_openssl_impl.hpp | 1 + topics/security/ssl/README.md | 13 +++++++++++++ 8 files changed, 46 insertions(+) diff --git a/include/cassandra.h b/include/cassandra.h index fed4f48ee..808150e8d 100644 --- a/include/cassandra.h +++ b/include/cassandra.h @@ -4687,6 +4687,18 @@ cass_ssl_set_private_key_n(CassSsl* ssl, const char* password, size_t password_length); +/** + * Configures the context to use the default directories + * for finding certification authority certificates. + * + * @public @memberof CassSsl + * + * @param[in] ssl + * @return CASS_OK if successful, otherwise an error occurred + */ +CASS_EXPORT CassError +cass_ssl_set_default_verify_paths(CassSsl* ssl); + /*********************************************************************************** * * Authenticator diff --git a/src/ssl.cpp b/src/ssl.cpp index 679a73429..4e84d0f42 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_default_verify_paths(CassSsl* ssl) { + return ssl->set_default_verify_paths(); +} + } // extern "C" template diff --git a/src/ssl.hpp b/src/ssl.hpp index 495aa7357..75476a6e1 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_default_verify_paths() = 0; protected: int verify_flags_; diff --git a/src/ssl/ssl_no_impl.cpp b/src/ssl/ssl_no_impl.cpp index ff539211d..2348eeaf6 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_default_verify_paths() { + 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..7f8919e99 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_default_verify_paths(); }; class NoSslContextFactory : public SslContextFactoryBase { diff --git a/src/ssl/ssl_openssl_impl.cpp b/src/ssl/ssl_openssl_impl.cpp index 3b1124378..1785e9ef8 100644 --- a/src/ssl/ssl_openssl_impl.cpp +++ b/src/ssl/ssl_openssl_impl.cpp @@ -599,6 +599,16 @@ CassError OpenSslContext::set_private_key(const char* key, size_t key_length, co return CASS_OK; } +CassError OpenSslContext::set_default_verify_paths() +{ + int rc = SSL_CTX_set_default_verify_paths(ssl_ctx_); + if (!rc) { + ssl_log_errors("Unable to load default verification paths"); + return CASS_ERROR_SSL_INVALID_CERT; + } + return CASS_OK; +} + 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..73e4ce48f 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_default_verify_paths(); private: SSL_CTX* ssl_ctx_; diff --git a/topics/security/ssl/README.md b/topics/security/ssl/README.md index 764042a30..16631aaa6 100644 --- a/topics/security/ssl/README.md +++ b/topics/security/ssl/README.md @@ -165,6 +165,19 @@ cass_ssl_set_verify_flags(ssl, CASS_SSL_VERIFY_NONE); cass_ssl_free(ssl); ``` +System wide certificate authorities can be enabled as well: + +```c +CassSsl* ssl = cass_ssl_new(); + +// Use system default directories for finding certificate authorities. +cass_ssl_set_default_verify_paths(ssl); + +/* ... */ + +cass_ssl_free(ssl); +``` + #### Enabling Cassandra identity verification If a unique certificate has been generated for each Cassandra node with the IP address or domain name in the CN or SAN fields, you also need to enable identity verification. From b5ce7e3347dda9b7a9ce46c07d5b3547c616730c Mon Sep 17 00:00:00 2001 From: m8mble Date: Fri, 10 Apr 2020 16:21:08 +0200 Subject: [PATCH 2/9] Add unit test for cass_ssl_set_default_verify_paths Ensure certificate validation fails prior to calling said function, and succeeds afterwards. The used certificate is specified to openssl via environment variables. --- tests/src/unit/tests/test_connection.cpp | 45 ++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/src/unit/tests/test_connection.cpp b/tests/src/unit/tests/test_connection.cpp index 1f392faa1..f78bf2475 100644 --- a/tests/src/unit/tests/test_connection.cpp +++ b/tests/src/unit/tests/test_connection.cpp @@ -23,6 +23,9 @@ #include "request_callback.hpp" #include "ssl.hpp" +#include +#include + #ifdef WIN32 #undef STATUS_TIMEOUT #endif @@ -190,6 +193,48 @@ TEST_F(ConnectionUnitTest, Ssl) { EXPECT_EQ(state.status, STATUS_SUCCESS); } +TEST_F(ConnectionUnitTest, SslDefaultVerifyPaths) { + const String host = "127.0.0.1"; + const int verification_flags = CASS_SSL_VERIFY_PEER_CERT | CASS_SSL_VERIFY_PEER_IDENTITY; + const char* cert_path = "cassandra-unit-test.cert"; + + mockssandra::SimpleCluster cluster(simple()); + const String cert = cluster.use_ssl(host); + EXPECT_FALSE(cert.empty()) << "Unable to enable SSL"; + ConnectionSettings settings; + settings.socket_settings.ssl_context = SslContextFactory::create(); + settings.socket_settings.ssl_context->set_verify_flags(verification_flags); + ASSERT_EQ(cluster.start_all(), 0); + + // Test that cert verification fails prior to calling set_default_verify_paths + Connector::ConnectionError connect_rc = Connector::CONNECTION_OK; + Connector::Ptr connector0(new Connector(Host::Ptr(new Host(Address(host, PORT))), + PROTOCOL_VERSION, + bind_callback(on_connection_error_code, &connect_rc))); + connector0->with_settings(settings)->connect(loop()); + uv_run(loop(), UV_RUN_DEFAULT); + EXPECT_EQ(connect_rc, Connector::CONNECTION_ERROR_SSL_VERIFY) + << "Verification succeeded without certificate."; + + // Generate certificate as file (which is used by our mock cluster) and import it + std::ofstream cert_buffer(cert_path); + cert_buffer << cert; + cert_buffer.close(); + ASSERT_EQ(uv_os_setenv("SSL_CERT_FILE", cert_path), 0) << "Failed to prepare openssl environment"; + ASSERT_EQ(settings.socket_settings.ssl_context->set_default_verify_paths(), CASS_OK) + << "Failed to import default / system SSL certificates."; + ASSERT_EQ(std::remove(cert_path), 0) << "Failed to cleanup temporary certificate file."; + + // Ensure verification succeeds with this certificate. + State state; + Connector::Ptr connector1(new Connector(Host::Ptr(new Host(Address(host, PORT))), + PROTOCOL_VERSION, + bind_callback(on_connection_connected, &state))); + connector1->with_settings(settings)->connect(loop()); + uv_run(loop(), UV_RUN_DEFAULT); + EXPECT_EQ(state.status, STATUS_SUCCESS); +} + TEST_F(ConnectionUnitTest, Refused) { // Don't start cluster From c133d3d0b4640e3a28b5567088cde9ddcacfd24d Mon Sep 17 00:00:00 2001 From: m8mble Date: Sun, 12 Apr 2020 19:18:58 +0200 Subject: [PATCH 3/9] Use absolute path for SSL_CERT_FILE --- tests/src/unit/tests/test_connection.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/src/unit/tests/test_connection.cpp b/tests/src/unit/tests/test_connection.cpp index f78bf2475..f00b34a94 100644 --- a/tests/src/unit/tests/test_connection.cpp +++ b/tests/src/unit/tests/test_connection.cpp @@ -196,7 +196,7 @@ TEST_F(ConnectionUnitTest, Ssl) { TEST_F(ConnectionUnitTest, SslDefaultVerifyPaths) { const String host = "127.0.0.1"; const int verification_flags = CASS_SSL_VERIFY_PEER_CERT | CASS_SSL_VERIFY_PEER_IDENTITY; - const char* cert_path = "cassandra-unit-test.cert"; + const String cert_path = std::tmpnam(nullptr); mockssandra::SimpleCluster cluster(simple()); const String cert = cluster.use_ssl(host); @@ -217,13 +217,14 @@ TEST_F(ConnectionUnitTest, SslDefaultVerifyPaths) { << "Verification succeeded without certificate."; // Generate certificate as file (which is used by our mock cluster) and import it - std::ofstream cert_buffer(cert_path); + std::ofstream cert_buffer(cert_path.c_str()); cert_buffer << cert; cert_buffer.close(); - ASSERT_EQ(uv_os_setenv("SSL_CERT_FILE", cert_path), 0) << "Failed to prepare openssl environment"; + ASSERT_EQ(uv_os_setenv("SSL_CERT_FILE", cert_path.c_str()), 0) + << "Failed to prepare openssl environment"; ASSERT_EQ(settings.socket_settings.ssl_context->set_default_verify_paths(), CASS_OK) << "Failed to import default / system SSL certificates."; - ASSERT_EQ(std::remove(cert_path), 0) << "Failed to cleanup temporary certificate file."; + ASSERT_EQ(std::remove(cert_path.c_str()), 0) << "Failed to cleanup temporary certificate file."; // Ensure verification succeeds with this certificate. State state; From 685cb7f93b5b68ba7f030f969b5a75149c14ce74 Mon Sep 17 00:00:00 2001 From: m8mble Date: Tue, 14 Apr 2020 13:41:13 +0200 Subject: [PATCH 4/9] Add environment debut output --- tests/src/unit/tests/test_connection.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/src/unit/tests/test_connection.cpp b/tests/src/unit/tests/test_connection.cpp index f00b34a94..0f59ebc1f 100644 --- a/tests/src/unit/tests/test_connection.cpp +++ b/tests/src/unit/tests/test_connection.cpp @@ -222,6 +222,15 @@ TEST_F(ConnectionUnitTest, SslDefaultVerifyPaths) { cert_buffer.close(); ASSERT_EQ(uv_os_setenv("SSL_CERT_FILE", cert_path.c_str()), 0) << "Failed to prepare openssl environment"; + std::cout << "Debug SslDefaultVerifyPaths: SSL_CERT_FILE " << cert_path << " " << cert << std::endl; + for (const auto var: {"SSL_CERT_FILE", "SSL_CERT_DIR"}) { + const char* value = std::getenv(var); + if (value == nullptr) { + std::cout << "Debug SslDefaultVerifyPaths: Env " << var << " is not set!" << std::endl; + continue; + } + std::cout << "Debug SslDefaultVerifyPaths: Env " << var << " " << value << std::endl; + } ASSERT_EQ(settings.socket_settings.ssl_context->set_default_verify_paths(), CASS_OK) << "Failed to import default / system SSL certificates."; ASSERT_EQ(std::remove(cert_path.c_str()), 0) << "Failed to cleanup temporary certificate file."; From 7172797088937d27854df8e54ed72c6e0564838c Mon Sep 17 00:00:00 2001 From: m8mble Date: Tue, 14 Apr 2020 14:31:06 +0200 Subject: [PATCH 5/9] Try setenv instead of us_os_setenv --- tests/src/unit/tests/test_connection.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/unit/tests/test_connection.cpp b/tests/src/unit/tests/test_connection.cpp index 0f59ebc1f..14d539bf7 100644 --- a/tests/src/unit/tests/test_connection.cpp +++ b/tests/src/unit/tests/test_connection.cpp @@ -220,7 +220,7 @@ TEST_F(ConnectionUnitTest, SslDefaultVerifyPaths) { std::ofstream cert_buffer(cert_path.c_str()); cert_buffer << cert; cert_buffer.close(); - ASSERT_EQ(uv_os_setenv("SSL_CERT_FILE", cert_path.c_str()), 0) + ASSERT_EQ(setenv("SSL_CERT_FILE", cert_path.c_str(), 1), 0) << "Failed to prepare openssl environment"; std::cout << "Debug SslDefaultVerifyPaths: SSL_CERT_FILE " << cert_path << " " << cert << std::endl; for (const auto var: {"SSL_CERT_FILE", "SSL_CERT_DIR"}) { From 53776dc63b1d56715448cadcab445fcc44c4556d Mon Sep 17 00:00:00 2001 From: m8mble Date: Tue, 14 Apr 2020 15:10:39 +0200 Subject: [PATCH 6/9] ... with double colon --- tests/src/unit/tests/test_connection.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/unit/tests/test_connection.cpp b/tests/src/unit/tests/test_connection.cpp index 14d539bf7..8de4990e8 100644 --- a/tests/src/unit/tests/test_connection.cpp +++ b/tests/src/unit/tests/test_connection.cpp @@ -220,7 +220,7 @@ TEST_F(ConnectionUnitTest, SslDefaultVerifyPaths) { std::ofstream cert_buffer(cert_path.c_str()); cert_buffer << cert; cert_buffer.close(); - ASSERT_EQ(setenv("SSL_CERT_FILE", cert_path.c_str(), 1), 0) + ASSERT_EQ(::setenv("SSL_CERT_FILE", cert_path.c_str(), 1), 0) << "Failed to prepare openssl environment"; std::cout << "Debug SslDefaultVerifyPaths: SSL_CERT_FILE " << cert_path << " " << cert << std::endl; for (const auto var: {"SSL_CERT_FILE", "SSL_CERT_DIR"}) { From 50c96ee6ce426684bcd8a22bdd9f9314b0daf13b Mon Sep 17 00:00:00 2001 From: m8mble Date: Tue, 14 Apr 2020 15:53:14 +0200 Subject: [PATCH 7/9] ... use putenv on windows --- tests/src/unit/tests/test_connection.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tests/src/unit/tests/test_connection.cpp b/tests/src/unit/tests/test_connection.cpp index 8de4990e8..6b53c149a 100644 --- a/tests/src/unit/tests/test_connection.cpp +++ b/tests/src/unit/tests/test_connection.cpp @@ -33,6 +33,18 @@ using namespace datastax::internal; using namespace datastax::internal::core; +namespace { + +static void setenv(const std::string& name, const std::string& value) { +#ifdef _WIN32 + putenv(const_cast(std::string(name + "=" + value).c_str())); +#else + ::setenv(name.c_str(), value.c_str(), 1); +#endif +} + +} + class ConnectionUnitTest : public LoopTest { public: enum Status { @@ -220,8 +232,7 @@ TEST_F(ConnectionUnitTest, SslDefaultVerifyPaths) { std::ofstream cert_buffer(cert_path.c_str()); cert_buffer << cert; cert_buffer.close(); - ASSERT_EQ(::setenv("SSL_CERT_FILE", cert_path.c_str(), 1), 0) - << "Failed to prepare openssl environment"; + setenv("SSL_CERT_FILE", cert_path.c_str()); std::cout << "Debug SslDefaultVerifyPaths: SSL_CERT_FILE " << cert_path << " " << cert << std::endl; for (const auto var: {"SSL_CERT_FILE", "SSL_CERT_DIR"}) { const char* value = std::getenv(var); From 0e9b95a7aff094e2d2a2d3c1a84bb4192362836f Mon Sep 17 00:00:00 2001 From: m8mble Date: Tue, 14 Apr 2020 17:25:30 +0200 Subject: [PATCH 8/9] ... use _putenv / delay cert removal --- tests/src/unit/tests/test_connection.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/src/unit/tests/test_connection.cpp b/tests/src/unit/tests/test_connection.cpp index 6b53c149a..93b58220b 100644 --- a/tests/src/unit/tests/test_connection.cpp +++ b/tests/src/unit/tests/test_connection.cpp @@ -37,7 +37,7 @@ namespace { static void setenv(const std::string& name, const std::string& value) { #ifdef _WIN32 - putenv(const_cast(std::string(name + "=" + value).c_str())); + _putenv(const_cast(std::string(name + "=" + value).c_str())); #else ::setenv(name.c_str(), value.c_str(), 1); #endif @@ -244,7 +244,6 @@ TEST_F(ConnectionUnitTest, SslDefaultVerifyPaths) { } ASSERT_EQ(settings.socket_settings.ssl_context->set_default_verify_paths(), CASS_OK) << "Failed to import default / system SSL certificates."; - ASSERT_EQ(std::remove(cert_path.c_str()), 0) << "Failed to cleanup temporary certificate file."; // Ensure verification succeeds with this certificate. State state; @@ -254,6 +253,7 @@ TEST_F(ConnectionUnitTest, SslDefaultVerifyPaths) { connector1->with_settings(settings)->connect(loop()); uv_run(loop(), UV_RUN_DEFAULT); EXPECT_EQ(state.status, STATUS_SUCCESS); + ASSERT_EQ(std::remove(cert_path.c_str()), 0) << "Failed to cleanup temporary certificate file."; } TEST_F(ConnectionUnitTest, Refused) { From b6c59d7d1009c9a95dbd5655e0dff07fbfac5d88 Mon Sep 17 00:00:00 2001 From: m8mble Date: Wed, 15 Apr 2020 09:52:17 +0200 Subject: [PATCH 9/9] ... with local cert and SSL_CERT_DIR --- tests/src/unit/tests/test_connection.cpp | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/tests/src/unit/tests/test_connection.cpp b/tests/src/unit/tests/test_connection.cpp index 93b58220b..1fc273488 100644 --- a/tests/src/unit/tests/test_connection.cpp +++ b/tests/src/unit/tests/test_connection.cpp @@ -35,7 +35,7 @@ using namespace datastax::internal::core; namespace { -static void setenv(const std::string& name, const std::string& value) { +void setenv(const std::string& name, const std::string& value) { #ifdef _WIN32 _putenv(const_cast(std::string(name + "=" + value).c_str())); #else @@ -43,6 +43,17 @@ static void setenv(const std::string& name, const std::string& value) { #endif } +String current_dir() +{ + char buffer[256]; +#ifdef _WIN32 + _getcwd(buffer, 256); +#else + getcwd(buffer, 256); +#endif + return buffer; +} + } class ConnectionUnitTest : public LoopTest { @@ -208,7 +219,7 @@ TEST_F(ConnectionUnitTest, Ssl) { TEST_F(ConnectionUnitTest, SslDefaultVerifyPaths) { const String host = "127.0.0.1"; const int verification_flags = CASS_SSL_VERIFY_PEER_CERT | CASS_SSL_VERIFY_PEER_IDENTITY; - const String cert_path = std::tmpnam(nullptr); + const String cert_path = "tmp.cassandra.unit-test.cert"; mockssandra::SimpleCluster cluster(simple()); const String cert = cluster.use_ssl(host); @@ -228,11 +239,14 @@ TEST_F(ConnectionUnitTest, SslDefaultVerifyPaths) { EXPECT_EQ(connect_rc, Connector::CONNECTION_ERROR_SSL_VERIFY) << "Verification succeeded without certificate."; + const String cwd = current_dir(); + // Generate certificate as file (which is used by our mock cluster) and import it std::ofstream cert_buffer(cert_path.c_str()); cert_buffer << cert; cert_buffer.close(); setenv("SSL_CERT_FILE", cert_path.c_str()); + setenv("SSL_CERT_DIR", cwd.c_str()); std::cout << "Debug SslDefaultVerifyPaths: SSL_CERT_FILE " << cert_path << " " << cert << std::endl; for (const auto var: {"SSL_CERT_FILE", "SSL_CERT_DIR"}) { const char* value = std::getenv(var);