Merge pull request ClickHouse#69582 from ClickHouse/keeper-better-ssl…

…-support

Support more advanced SSL options for Keeper internal communication

base/poco/Crypto/include/Poco/Crypto/EVPPKey.h

@@ -188,8 +188,9 @@ namespace Crypto
                     pFile = fopen(keyFile.c_str(), "r");
                     if (pFile)
                     {
-                        pem_password_cb * pCB = pass.empty() ? (pem_password_cb *)0 : &passCB;
-                        void * pPassword = pass.empty() ? (void *)0 : (void *)pass.c_str();
+                        pem_password_cb * pCB = &passCB;
+                        static constexpr char * no_password = "";
+                        void * pPassword = pass.empty() ? (void *)no_password : (void *)pass.c_str();
                         if (readFunc(pFile, &pKey, pCB, pPassword))
                         {
                             fclose(pFile);
@@ -225,6 +226,13 @@ namespace Crypto
         error:
             if (pFile)
                 fclose(pFile);
+            if (*ppKey)
+            {
+                if constexpr (std::is_same_v<K, EVP_PKEY>)
+                    EVP_PKEY_free(*ppKey);
+                else
+                    EC_KEY_free(*ppKey);
+            }
             throw OpenSSLException("EVPKey::loadKey(string)");
         }
 
@@ -286,6 +294,13 @@ namespace Crypto
         error:
             if (pBIO)
                 BIO_free(pBIO);
+            if (*ppKey)
+            {
+                if constexpr (std::is_same_v<K, EVP_PKEY>)
+                    EVP_PKEY_free(*ppKey);
+                else
+                    EC_KEY_free(*ppKey);
+            }
             throw OpenSSLException("EVPKey::loadKey(stream)");
         }
 

base/poco/NetSSL_OpenSSL/include/Poco/Net/Context.h

@@ -248,6 +248,9 @@ namespace Net
         SSL_CTX * sslContext() const;
         /// Returns the underlying OpenSSL SSL Context object.
 
+        SSL_CTX * takeSslContext();
+        /// Takes ownership of the underlying OpenSSL SSL Context object.
+
         Usage usage() const;
         /// Returns whether the context is for use by a client or by a server
         /// and whether TLSv1 is required.
@@ -401,6 +404,13 @@ namespace Net
         return _pSSLContext;
     }
 
+    inline SSL_CTX * Context::takeSslContext()
+    {
+        auto * result = _pSSLContext;
+        _pSSLContext = nullptr;
+        return result;
+    }
+
 
     inline bool Context::extendedCertificateVerificationEnabled() const
     {

base/poco/NetSSL_OpenSSL/src/Context.cpp

@@ -106,6 +106,11 @@ Context::Context(
 
 Context::~Context()
 {
+    if (_pSSLContext == nullptr)
+    {
+        return;
+    }
+
 	try
 	{
 		SSL_CTX_free(_pSSLContext);

src/Coordination/KeeperServer.cpp

@@ -30,6 +30,20 @@
 #include <Common/getMultipleKeysFromConfig.h>
 #include <Common/getNumberOfPhysicalCPUCores.h>
 
+#if USE_SSL
+#    include <Server/CertificateReloader.h>
+#    include <openssl/ssl.h>
+#    include <Poco/Crypto/EVPPKey.h>
+#    include <Poco/Net/Context.h>
+#    include <Poco/Net/SSLManager.h>
+#    include <Poco/Net/Utility.h>
+#    include <Poco/StringTokenizer.h>
+#endif
+
+#include <chrono>
+#include <mutex>
+#include <string>
+
 #pragma clang diagnostic ignored "-Wdeprecated-declarations"
 #include <fmt/chrono.h>
 
@@ -46,6 +60,7 @@ namespace ErrorCodes
     extern const int SUPPORT_IS_DISABLED;
     extern const int LOGICAL_ERROR;
     extern const int INVALID_CONFIG_PARAMETER;
+    extern const int BAD_ARGUMENTS;
 }
 
 using namespace std::chrono_literals;
@@ -54,6 +69,16 @@ namespace
 {
 
 #if USE_SSL
+
+int callSetCertificate(SSL * ssl, void * arg)
+{
+    if (!arg)
+        return -1;
+
+    const CertificateReloader::Data * data = reinterpret_cast<CertificateReloader::Data *>(arg);
+    return setCertificateCallback(ssl, data, getLogger("SSLContext"));
+}
+
 void setSSLParams(nuraft::asio_service::options & asio_opts)
 {
     const Poco::Util::LayeredConfiguration & config = Poco::Util::Application::instance().config();
@@ -67,18 +92,55 @@ void setSSLParams(nuraft::asio_service::options & asio_opts)
     if (!config.has(private_key_file_property))
         throw Exception(ErrorCodes::NO_ELEMENTS_IN_CONFIG, "Server private key file is not set.");
 
-    asio_opts.enable_ssl_ = true;
-    asio_opts.server_cert_file_ = config.getString(certificate_file_property);
-    asio_opts.server_key_file_ = config.getString(private_key_file_property);
+    Poco::Net::Context::Params params;
+    params.certificateFile = config.getString(certificate_file_property);
+    if (params.certificateFile.empty())
+        throw Exception(ErrorCodes::BAD_ARGUMENTS, "Server certificate file in config '{}' is empty", certificate_file_property);
+
+    params.privateKeyFile = config.getString(private_key_file_property);
+    if (params.privateKeyFile.empty())
+        throw Exception(ErrorCodes::BAD_ARGUMENTS, "Server key file in config '{}' is empty", private_key_file_property);
+
+    auto pass_phrase = config.getString("openSSL.server.privateKeyPassphraseHandler.options.password", "");
+    auto certificate_data = std::make_shared<CertificateReloader::Data>(params.certificateFile, params.privateKeyFile, pass_phrase);
 
     if (config.has(root_ca_file_property))
-        asio_opts.root_cert_file_ = config.getString(root_ca_file_property);
+        params.caLocation = config.getString(root_ca_file_property);
 
-    if (config.getBool("openSSL.server.loadDefaultCAFile", false))
-        asio_opts.load_default_ca_file_ = true;
+    params.loadDefaultCAs = config.getBool("openSSL.server.loadDefaultCAFile", false);
+    params.verificationMode = Poco::Net::Utility::convertVerificationMode(config.getString("openSSL.server.verificationMode", "none"));
+
+    std::string disabled_protocols_list = config.getString("openSSL.server.disableProtocols", "");
+    Poco::StringTokenizer dp_tok(disabled_protocols_list, ";,", Poco::StringTokenizer::TOK_TRIM | Poco::StringTokenizer::TOK_IGNORE_EMPTY);
+    int disabled_protocols = 0;
+    for (const auto & token : dp_tok)
+    {
+        if (token == "sslv2")
+            disabled_protocols |= Poco::Net::Context::PROTO_SSLV2;
+        else if (token == "sslv3")
+            disabled_protocols |= Poco::Net::Context::PROTO_SSLV3;
+        else if (token == "tlsv1")
+            disabled_protocols |= Poco::Net::Context::PROTO_TLSV1;
+        else if (token == "tlsv1_1")
+            disabled_protocols |= Poco::Net::Context::PROTO_TLSV1_1;
+        else if (token == "tlsv1_2")
+            disabled_protocols |= Poco::Net::Context::PROTO_TLSV1_2;
+    }
 
-    if (config.getString("openSSL.server.verificationMode", "none") == "none")
-        asio_opts.skip_verification_ = true;
+    asio_opts.ssl_context_provider_server_ = [params, certificate_data, disabled_protocols]
+    {
+        Poco::Net::Context context(Poco::Net::Context::Usage::TLSV1_2_SERVER_USE, params);
+        context.disableProtocols(disabled_protocols);
+        SSL_CTX * ssl_ctx = context.takeSslContext();
+        SSL_CTX_set_cert_cb(ssl_ctx, callSetCertificate, reinterpret_cast<void *>(certificate_data.get()));
+        return ssl_ctx;
+    };
+
+    asio_opts.ssl_context_provider_client_ = [ctx_params = std::move(params)]
+    {
+        Poco::Net::Context context(Poco::Net::Context::Usage::TLSV1_2_CLIENT_USE, ctx_params);
+        return context.takeSslContext();
+    };
 }
 #endif
 

src/Server/CertificateReloader.cpp

@@ -28,29 +28,33 @@ int CertificateReloader::setCertificate(SSL * ssl)
     auto current = data.get();
     if (!current)
         return -1;
+    return setCertificateCallback(ssl, current.get(), log);
+}
 
-    if (current->certs_chain.empty())
+int setCertificateCallback(SSL * ssl, const CertificateReloader::Data * current_data, LoggerPtr log)
+{
+    if (current_data->certs_chain.empty())
         return -1;
 
     if (auto err = SSL_clear_chain_certs(ssl); err != 1)
     {
         LOG_ERROR(log, "Clear certificates {}", Poco::Net::Utility::getLastError());
         return -1;
     }
-    if (auto err = SSL_use_certificate(ssl, const_cast<X509 *>(current->certs_chain[0].certificate())); err != 1)
+    if (auto err = SSL_use_certificate(ssl, const_cast<X509 *>(current_data->certs_chain[0].certificate())); err != 1)
     {
         LOG_ERROR(log, "Use certificate {}", Poco::Net::Utility::getLastError());
         return -1;
     }
-    for (auto cert = current->certs_chain.begin() + 1; cert != current->certs_chain.end(); cert++)
+    for (auto cert = current_data->certs_chain.begin() + 1; cert != current_data->certs_chain.end(); cert++)
     {
         if (auto err = SSL_add1_chain_cert(ssl, const_cast<X509 *>(cert->certificate())); err != 1)
         {
             LOG_ERROR(log, "Add certificate to chain {}", Poco::Net::Utility::getLastError());
             return -1;
         }
     }
-    if (auto err = SSL_use_PrivateKey(ssl, const_cast<EVP_PKEY *>(static_cast<const EVP_PKEY *>(current->key))); err != 1)
+    if (auto err = SSL_use_PrivateKey(ssl, const_cast<EVP_PKEY *>(static_cast<const EVP_PKEY *>(current_data->key))); err != 1)
     {
         LOG_ERROR(log, "Use private key {}", Poco::Net::Utility::getLastError());
         return -1;

src/Server/CertificateReloader.h

@@ -31,6 +31,14 @@ class CertificateReloader
 public:
     using stat_t = struct stat;
 
+    struct Data
+    {
+        Poco::Crypto::X509Certificate::List certs_chain;
+        Poco::Crypto::EVPPKey key;
+
+        Data(std::string cert_path, std::string key_path, std::string pass_phrase);
+    };
+
     /// Singleton
     CertificateReloader(CertificateReloader const &) = delete;
     void operator=(CertificateReloader const &) = delete;
@@ -68,18 +76,13 @@ class CertificateReloader
     File cert_file{"certificate"};
     File key_file{"key"};
 
-    struct Data
-    {
-        Poco::Crypto::X509Certificate::List certs_chain;
-        Poco::Crypto::EVPPKey key;
-
-        Data(std::string cert_path, std::string key_path, std::string pass_phrase);
-    };
-
     MultiVersion<Data> data;
     bool init_was_not_made = true;
 };
 
+/// A callback for OpenSSL
+int setCertificateCallback(SSL * ssl, const CertificateReloader::Data * current_data, LoggerPtr log);
+
 }
 
 #endif

tests/integration/helpers/cluster.py

@@ -3917,7 +3917,7 @@ def contains_in_log(
         self, substring, from_host=False, filename="clickhouse-server.log"
     ):
         if from_host:
-            # We check fist file exists but want to look for all rotated logs as well
+            # We check first file exists but want to look for all rotated logs as well
             result = subprocess_check_call(
                 [
                     "bash",

tests/integration/test_keeper_internal_secure/configs/WithPassPhrase.crt

@@ -0,0 +1,20 @@
+-----BEGIN CERTIFICATE-----
+MIIDPDCCAiQCFBXNOvsLA+dqmX/TkYG9JXdD5m72MA0GCSqGSIb3DQEBCwUAMFox
+CzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRl
+cm5ldCBXaWRnaXRzIFB0eSBMdGQxEzARBgNVBAMMCmNsaWNraG91c2UwIBcNMjIw
+NDIxMTAzNDU1WhgPMjEyMjAzMjgxMDM0NTVaMFkxCzAJBgNVBAYTAkFVMRMwEQYD
+VQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBM
+dGQxEjAQBgNVBAMMCWxvY2FsaG9zdDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCC
+AQoCggEBAKaXz596N4NC2zZdIqdwZbSYAtNdBCsBVPt5YT9F640aF5zOogPZyxGP
+ENyOZwABi/7HhwFbH657xyRvi8lTau8dZL+0tbakyoIn1Tw6j+/3GXTjLduJSy6C
+mOf4OzsrFC8mYgU+7p5ijvWVlO9h5NMbLdAPSIB5WSHhmSORH5LgjoK6oMOYdRod
+GmfHqSbwPVwy3Li5SXlniCQmJsM0zl64LFbJ/NU+13qETmhBiDgmh0Svi+wzSzqZ
+q1PIX92T3k44IXNZbvF7lKbUOS9Xb3BoxA4cDqRcTx4x73xRDwodSmqiuQOC99HI
+A0C/tZJ25VNAGjLKukPSHqYscq2PAsUCAwEAATANBgkqhkiG9w0BAQsFAAOCAQEA
+IDQwjf/ja3TfOXrz+Gn1eErSKnWS3asjRT9rYWQsy3tzVUkMIcszrG+FqTR16g5H
+ZWyuEOi6KIRmda3SYKdLKmtQLrgx6/d/jvH5TQ0LTFZrp6vh0lo3pV+L6fLo1ZRD
+V1i8jW/7HHNyqJamUXOjwA0DpPOMkdtwuyV+rJ+2bTG1ZSK33O4Ae2CY5+dad6zy
+YI6b1c9flWfDznuNEMH7jDDjKgXwjZGeU53FiuuhHiNyRchsr/B9eIBsom8oykiD
+kch4cnAxx2E+m3hLYzupkXHOVQ5CNpVk8PGUCIGcyqDxPt+fOj1WbDQ9laEcfhmV
+kR+vHmzOqWZnHU4QeMqDig==
+-----END CERTIFICATE-----

tests/integration/test_keeper_internal_secure/configs/WithPassPhrase.key

@@ -0,0 +1,30 @@
+-----BEGIN RSA PRIVATE KEY-----
+Proc-Type: 4,ENCRYPTED
+DEK-Info: AES-256-CBC,4E14FF586022476CD22AAFB662BB0E40
+
+dpJZKq5k+fMuC7XECfTSRjPeOEl9wNuVtZkcjEWaHN8ky4umND7ARyRyuU1Nk7cy
+fpCFlFKOqDfCkT5zVK/fB6pF32wqAI7sqeSuYPfQY0+L77yRdwM6L46WslzVKZYE
+lXD1AmqKT/LgF3+eBY5slkAAJo10zYDgKEwnoQVBp31YW2/+6oAGaY/O6x3p7aTG
+dw9CP+SFc0o8lPl1lsSovdNXDUiVCftvClog7hwyDv8AhHyGgynw3UJXX8UlyWu+
+Zz5zpgrvB2gvDLeoZZ6qjMGvtpEwlYBh4de9ZOsvQUpXEEfkQFtJV0j613OCQune
+LoTxKpYV1V/mZX4HPaJ1oC0OJ7XcliAOSS9K49YobTuz7Kg5Wg3bVGo9xRfYDjch
+rVeMP4u5RXSGuHL23FPMfK6EqcldrFyRwLaY/IV1Yl6UNUMKAphn/WMtWVuT3TiT
+QMCI2VRt7ItwZwwFn5RgyDweWdFf5v3AmN/lOhATDBqosahkPxDZ1VZ6OBPoJLPM
+UrDWH/lqrByeEjtAOwr5UsWKwLuJ8qUxQ4TchHwFKOwy6VsrRwMQ3ZWi2govPF9I
+W0sfLj5Ulfjx6zHdqnF48a1Elit4JH6inBEWFuj7bmlOotq+PHoeT61zAwW+gnrG
+3JTo3XnaE2WwRDpqvKYHWLv/J218rq8PtIaq9gjr55odPfIt8lkJ1XzF4WQ21rIJ
+GNWZ3xz4fxpvrKnQyAKGu0ZcdjA1nqs16oiVr+UnJoXmkM5yBCic4fZYwPTSQHYS
+ZxwaTzEjfeGxrSeLrN9CgoweucvogOvUjJOBcW/py80du8vWz0YyzMhg3o0YeGME
+C+Kts/YWxmyfw4DaWt8RtWCKl85hEmz8RODvkMLGtLzvVoSyLQWqp1NhGIlFtzXs
+7sPLulUeyD2avTC/RB/Pu9Nk80c0368BxCoeYbiFWZpaN70SJmCUE5H59J2d0olw
+5v2RVjLBi8wqnzoa0+2L8wnG7IQGadS97dj0eBR+JXNtoJhVrurS80RJ6B0bNxdu
+gX8otfnJYsZyK5hbEhcQqLdnyGhDEE8YHe7Hv9stWwLAFOfOMzyzC06lFS1eNiw4
+FJyXJUhDieb8EqetouAC8dNVXz4Q1zOTlGuAbGoKm5v0U5IhCQap9GUSW5QiUgOQ
+AEMs9aGfd91R+IcDf19mZptsQLYA6MGBN6fm+3O2iZImKIbF+ZZo0S6liFFmn6lm
+M+diTzaoiqgEkiXOuRhdQUMaiGV8BMZxv8qUH6/vyC3gSueoTio0f9PfASDYfvXD
+A3GuI87P6LF1it2UlN6ssFoXTZdfQQZwRmNuqOqw+BJOJHrR6trcXOCZOQ77Qnvd
+M5a348gIzluVUkExAPGCsySQWMx4Of5NBF28jEC3+TAwkRqBV2ZHmfGLWnvwaB+A
+YUeKtpWblfG1lsrDAdwL2dilU95oby+35sExX7M2dCrL9Y2P5oTCW3u12//ZSLeL
+Yhi1Rzol6LAuesZCVF0Zv/YYDhzAckJfT/qXK5B5pz9saswxCUBEpiKlLpVsjOFJ
+2bHm8NgOMD5b3cdh1kvts4wZe+giry7LHsn46f+9VqN+gA6XxeVsPyb4uO1KW3SN
+-----END RSA PRIVATE KEY-----

tests/integration/test_keeper_internal_secure/configs/enable_secure_keeper1.xml

@@ -1,5 +1,6 @@
 <clickhouse>
     <keeper_server>
+        <use_cluster>0</use_cluster>
         <tcp_port>9181</tcp_port>
         <server_id>1</server_id>
         <log_storage_path>/var/lib/clickhouse/coordination/log</log_storage_path>

tests/integration/test_keeper_internal_secure/configs/enable_secure_keeper2.xml

@@ -1,5 +1,6 @@
 <clickhouse>
     <keeper_server>
+        <use_cluster>0</use_cluster>
         <tcp_port>9181</tcp_port>
         <server_id>2</server_id>
         <log_storage_path>/var/lib/clickhouse/coordination/log</log_storage_path>

tests/integration/test_keeper_internal_secure/configs/enable_secure_keeper3.xml

@@ -1,5 +1,6 @@
 <clickhouse>
     <keeper_server>
+        <use_cluster>0</use_cluster>
         <tcp_port>9181</tcp_port>
         <server_id>3</server_id>
         <log_storage_path>/var/lib/clickhouse/coordination/log</log_storage_path>

tests/integration/test_keeper_internal_secure/configs/ssl_conf.yml

@@ -0,0 +1,10 @@
+openSSL:
+  server:
+    certificateFile: '/etc/clickhouse-server/config.d/WithoutPassPhrase.crt'
+    privateKeyFile: '/etc/clickhouse-server/config.d/WithoutPassPhrase.key'
+    caConfig: '/etc/clickhouse-server/config.d/rootCA.pem'
+    loadDefaultCAFile: true
+    verificationMode: 'none'
+    cacheSessions: true
+    disableProtocols: 'sslv2,sslv3'
+    preferServerCiphers: true

tests/integration/test_keeper_internal_secure/configs/ssl_conf_password.yml

@@ -0,0 +1,14 @@
+openSSL:
+  server:
+    certificateFile: '/etc/clickhouse-server/config.d/WithoutPassPhrase.crt'
+    privateKeyFile: '/etc/clickhouse-server/config.d/WithoutPassPhrase.key'
+    privateKeyPassphraseHandler:
+        name: KeyFileHandler
+        options:
+            password: 'PASSWORD'
+    caConfig: '/etc/clickhouse-server/config.d/rootCA.pem'
+    loadDefaultCAFile: true
+    verificationMode: 'none'
+    cacheSessions: true
+    disableProtocols: 'sslv2,sslv3'
+    preferServerCiphers: true