From 68f3b235e1b949dc4ed27bd3e3b0d11385209c6b Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Wed, 28 Aug 2019 15:37:23 +0300 Subject: [PATCH] MXS-2650 Fix SSL-use with Connector-C Authenticators and monitors now use SSL when configured. The fix has two parts: 1) Removed the extra SSLConfig inside SSLProvider, as SSLContext already contains the config. 2) When inputting parameter values to mysql_ssl_set(), empty strings are converted to NULL-pointers as the function expects those for unused values. --- include/maxscale/ssl.hh | 11 +++++------ server/core/mysql_utils.cc | 12 ++++++++---- server/core/server.cc | 2 +- server/core/ssl.cc | 19 +++++++++---------- server/modules/routing/binlogrouter/blr.cc | 2 +- .../modules/routing/binlogrouter/blr_slave.cc | 14 +++++++------- 6 files changed, 31 insertions(+), 29 deletions(-) diff --git a/include/maxscale/ssl.hh b/include/maxscale/ssl.hh index ce9ea4dbd..095916294 100644 --- a/include/maxscale/ssl.hh +++ b/include/maxscale/ssl.hh @@ -77,6 +77,9 @@ struct SSLConfig return ca.empty(); } + // Convert to human readable string representation + std::string to_string() const; + std::string key; /**< SSL private key */ std::string cert; /**< SSL certificate */ std::string ca; /**< SSL CA certificate */ @@ -144,20 +147,16 @@ public: return m_context.get(); } - // Current configuration - const mxs::SSLConfig& config() const; + // Current configuration, or null if none is set. + const mxs::SSLConfig* config() const; // The context or nullptr if no context is set mxs::SSLContext* context() const; - // Convert to human readable string representation - std::string to_string() const; - // NOTE: Do not use this, required by binlogrouter void set_context(std::unique_ptr ssl); private: std::unique_ptr m_context; /**< SSL context */ - mxs::SSLConfig m_config; /**< SSL configuration */ }; } diff --git a/server/core/mysql_utils.cc b/server/core/mysql_utils.cc index beb88bcc2..98ff5aa6c 100644 --- a/server/core/mysql_utils.cc +++ b/server/core/mysql_utils.cc @@ -36,10 +36,14 @@ MYSQL* mxs_mysql_real_connect(MYSQL* con, SERVER* server, const char* user, const char* passwd) { auto ssl = server->ssl().config(); - - if (!ssl.empty()) + bool have_ssl = ssl && !ssl->empty(); + if (have_ssl) { - mysql_ssl_set(con, ssl.key.c_str(), ssl.cert.c_str(), ssl.ca.c_str(), NULL, NULL); + // If an option is empty, a null-pointer should be given to mysql_ssl_set. + const char* ssl_key = ssl->key.empty() ? nullptr : ssl->key.c_str(); + const char* ssl_cert = ssl->cert.empty() ? nullptr : ssl->cert.c_str(); + const char* ssl_ca = ssl->ca.empty() ? nullptr : ssl->ca.c_str(); + mysql_ssl_set(con, ssl_key, ssl_cert, ssl_ca, NULL, NULL); } char yes = 1; @@ -84,7 +88,7 @@ MYSQL* mxs_mysql_real_connect(MYSQL* con, SERVER* server, const char* user, cons mysql_get_character_set_info(mysql, &cs_info); server->charset = cs_info.number; - if (!ssl.empty() && mysql_get_ssl_cipher(con) == NULL) + if (have_ssl && mysql_get_ssl_cipher(con) == NULL) { if (server->warn_ssl_not_enabled) { diff --git a/server/core/server.cc b/server/core/server.cc index a3276b4f6..c14c0490d 100644 --- a/server/core/server.cc +++ b/server/core/server.cc @@ -527,7 +527,7 @@ void Server::print_to_dcb(DCB* dcb) const } if (server->ssl().enabled()) { - dcb_printf(dcb, "%s", server->ssl().to_string().c_str()); + dcb_printf(dcb, "%s", server->ssl().config()->to_string().c_str()); } if (server->proxy_protocol) { diff --git a/server/core/ssl.cc b/server/core/ssl.cc index ac72351da..3ada44133 100644 --- a/server/core/ssl.cc +++ b/server/core/ssl.cc @@ -374,29 +374,28 @@ mxs::SSLContext* SSLProvider::context() const return m_context.get(); } -const mxs::SSLConfig& SSLProvider::config() const +const mxs::SSLConfig* SSLProvider::config() const { - return m_config; + return m_context ? &(m_context->config()) : nullptr; } void SSLProvider::set_context(std::unique_ptr ssl) { mxb_assert(ssl); m_context = std::move(ssl); - m_config = m_context->config(); } -std::string SSLProvider::to_string() const +std::string SSLConfig::to_string() const { std::ostringstream ss; ss << "\tSSL initialized: yes\n" - << "\tSSL method type: " << ssl_method_type_to_string(m_config.version) << "\n" - << "\tSSL certificate verification depth: " << m_config.verify_depth << "\n" - << "\tSSL peer verification : " << (m_config.verify_peer ? "true" : "false") << "\n" - << "\tSSL certificate: " << m_config.cert << "\n" - << "\tSSL key: " << m_config.key << "\n" - << "\tSSL CA certificate: " << m_config.ca << "\n"; + << "\tSSL method type: " << ssl_method_type_to_string(version) << "\n" + << "\tSSL certificate verification depth: " << verify_depth << "\n" + << "\tSSL peer verification : " << (verify_peer ? "true" : "false") << "\n" + << "\tSSL certificate: " << cert << "\n" + << "\tSSL key: " << key << "\n" + << "\tSSL CA certificate: " << ca << "\n"; return ss.str(); } diff --git a/server/modules/routing/binlogrouter/blr.cc b/server/modules/routing/binlogrouter/blr.cc index 79148c7e4..ed8edae32 100644 --- a/server/modules/routing/binlogrouter/blr.cc +++ b/server/modules/routing/binlogrouter/blr.cc @@ -1481,7 +1481,7 @@ static void diagnostics(MXS_ROUTER* router, DCB* dcb) if (ssl.enabled()) { - dcb_printf(dcb, "%s", ssl.to_string().c_str()); + dcb_printf(dcb, "%s", ssl.config()->to_string().c_str()); } /* Binlog Encryption options */ diff --git a/server/modules/routing/binlogrouter/blr_slave.cc b/server/modules/routing/binlogrouter/blr_slave.cc index 128cce272..4af8c6698 100644 --- a/server/modules/routing/binlogrouter/blr_slave.cc +++ b/server/modules/routing/binlogrouter/blr_slave.cc @@ -4852,24 +4852,24 @@ static void blr_master_get_config(ROUTER_INSTANCE* router, MasterServerConfig* c /* SSL options */ auto server_ssl = router->service->dbref->server->ssl().config(); - if (!server_ssl.empty()) + if (server_ssl && !server_ssl->empty()) { curr_master->ssl_enabled = router->ssl_enabled; if (router->ssl_version) { curr_master->ssl_version = router->ssl_version; } - if (!server_ssl.key.empty()) + if (!server_ssl->key.empty()) { - curr_master->ssl_key = server_ssl.key; + curr_master->ssl_key = server_ssl->key; } - if (!server_ssl.cert.empty()) + if (!server_ssl->cert.empty()) { - curr_master->ssl_cert = server_ssl.cert; + curr_master->ssl_cert = server_ssl->cert; } - if (!server_ssl.ca.empty()) + if (!server_ssl->ca.empty()) { - curr_master->ssl_ca = server_ssl.ca; + curr_master->ssl_ca = server_ssl->ca; } } /* Connect options */