From 3b8e28392ed1d71af439aaf5b3ac245e1768fc13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Mon, 20 May 2019 13:16:29 +0300 Subject: [PATCH] MXS-2483: Make server SSL private The old server_ssl member is now renamed and private. The ssl_context and set_ssl_context methods provide access to it. --- include/maxscale/server.hh | 22 ++++++++++++++----- server/core/config_runtime.cc | 8 ++----- server/core/dcb.cc | 6 ++--- server/core/internal/server.hh | 7 ++++-- server/core/mysql_utils.cc | 2 +- server/core/server.cc | 7 +++--- .../GSSAPIBackendAuth/gssapi_backend_auth.cc | 2 +- .../MariaDBBackendAuth/mysql_backend_auth.cc | 2 +- .../PAM/PAMBackendAuth/pam_backend_auth.cc | 2 +- server/modules/protocol/MySQL/mysql_common.cc | 4 ++-- server/modules/routing/binlogrouter/blr.cc | 8 +++---- .../modules/routing/binlogrouter/blr_slave.cc | 6 ++--- 12 files changed, 42 insertions(+), 34 deletions(-) diff --git a/include/maxscale/server.hh b/include/maxscale/server.hh index 434fe57fb..f2bc2480a 100644 --- a/include/maxscale/server.hh +++ b/include/maxscale/server.hh @@ -181,9 +181,8 @@ public: * routing sessions. */ // Base variables - bool is_active = false; /**< Server is active and has not been "destroyed" */ - mxs::SSLContext* server_ssl = nullptr; /**< SSL data */ - uint8_t charset = DEFAULT_CHARSET; /**< Character set. Read from backend and sent to client. */ + bool is_active = false; /**< Server is active and has not been "destroyed" */ + uint8_t charset = DEFAULT_CHARSET; /**< Character set. Read from backend and sent to client. */ // Statistics and events ConnStats stats; /**< The server statistics, e.g. number of connections */ @@ -518,13 +517,26 @@ public: */ void response_time_add(double ave, int num_samples); + mxs::SSLContext* ssl_context() const + { + return m_ssl_context; + } + + void set_ssl_context(mxs::SSLContext* ssl) + { + m_ssl_context = ssl; + } + protected: - SERVER() - : m_response_time(maxbase::EMAverage {0.04, 0.35, 500}) + SERVER(mxs::SSLContext* ssl_context = nullptr) + : m_response_time{0.04, 0.35, 500} + , m_ssl_context{ssl_context} { } + private: static const int DEFAULT_CHARSET = 0x08; /**< The latin1 charset */ maxbase::EMAverage m_response_time; /**< Response time calculations for this server */ std::mutex m_average_write_mutex; /**< Protects response time from concurrent writing */ + mxs::SSLContext* m_ssl_context; /**< SSL context */ }; diff --git a/server/core/config_runtime.cc b/server/core/config_runtime.cc index 3158f6df2..32a654178 100644 --- a/server/core/config_runtime.cc +++ b/server/core/config_runtime.cc @@ -441,11 +441,7 @@ bool runtime_enable_server_ssl(Server* server, if (ssl) { - // TODO: Properly discard old SSL configurations - - /** Sync to prevent reads on partially initialized server_ssl */ - atomic_synchronize(); - server->server_ssl = ssl; + server->set_ssl_context(ssl); if (server->serialize()) { @@ -1902,7 +1898,7 @@ static bool validate_ssl_json(json_t* params, object_type type) static bool process_ssl_parameters(Server* server, json_t* params) { - mxb_assert(server->server_ssl == NULL); + mxb_assert(server->ssl_context() == NULL); bool rval = true; if (have_ssl_json(params)) diff --git a/server/core/dcb.cc b/server/core/dcb.cc index 88f14bdab..a2fd89f5b 100644 --- a/server/core/dcb.cc +++ b/server/core/dcb.cc @@ -2227,10 +2227,10 @@ int dcb_connect_SSL(DCB* dcb) int ssl_rval; int return_code; - if ((NULL == dcb->server || NULL == dcb->server->server_ssl) - || (NULL == dcb->ssl && dcb_create_SSL(dcb, dcb->server->server_ssl) != 0)) + if ((NULL == dcb->server || NULL == dcb->server->ssl_context()) + || (NULL == dcb->ssl && dcb_create_SSL(dcb, dcb->server->ssl_context()) != 0)) { - mxb_assert((NULL != dcb->server) && (NULL != dcb->server->server_ssl)); + mxb_assert((NULL != dcb->server) && (NULL != dcb->server->ssl_context())); return -1; } dcb->ssl_state = SSL_HANDSHAKE_REQUIRED; diff --git a/server/core/internal/server.hh b/server/core/internal/server.hh index 1bb2990bd..684772dd2 100644 --- a/server/core/internal/server.hh +++ b/server/core/internal/server.hh @@ -28,8 +28,11 @@ class Server : public SERVER { public: - Server(const std::string& name, const std::string& protocol = "", const std::string& authenticator = "") - : SERVER() + Server(const std::string& name, + const std::string& protocol = "", + const std::string& authenticator = "", + mxs::SSLContext* ssl = nullptr) + : SERVER(ssl) , m_name(name) { m_settings.protocol = protocol; diff --git a/server/core/mysql_utils.cc b/server/core/mysql_utils.cc index ba4f9f50d..f2bef85b9 100644 --- a/server/core/mysql_utils.cc +++ b/server/core/mysql_utils.cc @@ -155,7 +155,7 @@ char* mxs_lestr_consume(uint8_t** c, size_t* size) MYSQL* mxs_mysql_real_connect(MYSQL* con, SERVER* server, const char* user, const char* passwd) { - mxs::SSLContext* ssl = server->server_ssl; + mxs::SSLContext* ssl = server->ssl_context(); if (ssl) { diff --git a/server/core/server.cc b/server/core/server.cc index 22b934937..0ab4a4ac7 100644 --- a/server/core/server.cc +++ b/server/core/server.cc @@ -203,7 +203,7 @@ Server* Server::server_alloc(const char* name, const MXS_CONFIG_PARAMETER& param return NULL; } - Server* server = new(std::nothrow) Server(name, protocol, authenticator); + Server* server = new(std::nothrow) Server(name, protocol, authenticator, ssl); DCB** persistent = (DCB**)MXS_CALLOC(config_threadcount(), sizeof(*persistent)); if (!server || !persistent) @@ -231,7 +231,6 @@ Server* Server::server_alloc(const char* name, const MXS_CONFIG_PARAMETER& param server->proxy_protocol = params.get_bool(CN_PROXY_PROTOCOL); server->is_active = true; server->m_auth_instance = auth_instance; - server->server_ssl = ssl; server->persistent = persistent; server->last_event = SERVER_UP_EVENT; server->status = SERVER_RUNNING; @@ -526,9 +525,9 @@ void Server::print_to_dcb(DCB* dcb) const + server->stats.n_from_pool + 1); dcb_printf(dcb, "\tPool availability: %0.2lf%%\n", d * 100.0); } - if (server->server_ssl) + if (server->ssl_context()) { - dcb_printf(dcb, "%s", server->server_ssl->to_string().c_str()); + dcb_printf(dcb, "%s", server->ssl_context()->to_string().c_str()); } if (server->proxy_protocol) { diff --git a/server/modules/authenticator/GSSAPI/GSSAPIBackendAuth/gssapi_backend_auth.cc b/server/modules/authenticator/GSSAPI/GSSAPIBackendAuth/gssapi_backend_auth.cc index 99af55368..0026ae58e 100644 --- a/server/modules/authenticator/GSSAPI/GSSAPIBackendAuth/gssapi_backend_auth.cc +++ b/server/modules/authenticator/GSSAPI/GSSAPIBackendAuth/gssapi_backend_auth.cc @@ -191,7 +191,7 @@ static bool gssapi_backend_auth_extract(DCB* dcb, GWBUF* buffer) */ static bool gssapi_backend_auth_connectssl(DCB* dcb) { - return dcb->server->server_ssl != NULL; + return dcb->server->ssl_context() != NULL; } /** diff --git a/server/modules/authenticator/MariaDBBackendAuth/mysql_backend_auth.cc b/server/modules/authenticator/MariaDBBackendAuth/mysql_backend_auth.cc index c136e8180..8fa6ea897 100644 --- a/server/modules/authenticator/MariaDBBackendAuth/mysql_backend_auth.cc +++ b/server/modules/authenticator/MariaDBBackendAuth/mysql_backend_auth.cc @@ -140,7 +140,7 @@ static int auth_backend_authenticate(DCB* dcb) */ static bool auth_backend_ssl(DCB* dcb) { - return dcb->server->server_ssl != NULL; + return dcb->server->ssl_context() != NULL; } extern "C" diff --git a/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_auth.cc b/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_auth.cc index 92c45a260..1220e647b 100644 --- a/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_auth.cc +++ b/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_auth.cc @@ -53,7 +53,7 @@ static bool pam_backend_auth_extract(DCB* dcb, GWBUF* buffer) */ static bool pam_backend_auth_connectssl(DCB* dcb) { - return dcb->server->server_ssl != NULL; + return dcb->server->ssl_context() != NULL; } /** diff --git a/server/modules/protocol/MySQL/mysql_common.cc b/server/modules/protocol/MySQL/mysql_common.cc index 1df80559e..a405933a8 100644 --- a/server/modules/protocol/MySQL/mysql_common.cc +++ b/server/modules/protocol/MySQL/mysql_common.cc @@ -954,13 +954,13 @@ mxs_auth_state_t gw_send_backend_auth(DCB* dcb) if (dcb->session == NULL || (dcb->session->state != SESSION_STATE_CREATED && dcb->session->state != SESSION_STATE_STARTED) - || (dcb->server->server_ssl + || (dcb->server->ssl_context() && dcb->ssl_state == SSL_HANDSHAKE_FAILED)) { return rval; } - bool with_ssl = dcb->server->server_ssl; + bool with_ssl = dcb->server->ssl_context(); bool ssl_established = dcb->ssl_state == SSL_ESTABLISHED; MYSQL_session client; diff --git a/server/modules/routing/binlogrouter/blr.cc b/server/modules/routing/binlogrouter/blr.cc index 59fe35b96..21e7b4a29 100644 --- a/server/modules/routing/binlogrouter/blr.cc +++ b/server/modules/routing/binlogrouter/blr.cc @@ -1477,9 +1477,9 @@ static void diagnostics(MXS_ROUTER* router, DCB* dcb) } /* SSL options */ - if (router_inst->ssl_enabled) + if (auto ssl = router_inst->service->dbref->server->ssl_context()) { - dcb_printf(dcb, "%s", router_inst->service->dbref->server->server_ssl->to_string().c_str()); + dcb_printf(dcb, "%s", ssl->to_string().c_str()); } /* Binlog Encryption options */ @@ -1954,9 +1954,9 @@ static json_t* diagnostics_json(const MXS_ROUTER* router) min5 /= 5.0; /* SSL options */ - if (router_inst->ssl_enabled) + if (auto ssl = router_inst->service->dbref->server->ssl_context()) { - json_object_set_new(rval, "master_ssl", router_inst->service->dbref->server->server_ssl->to_json()); + json_object_set_new(rval, "master_ssl", ssl->to_json()); } /* Binlog Encryption options */ diff --git a/server/modules/routing/binlogrouter/blr_slave.cc b/server/modules/routing/binlogrouter/blr_slave.cc index df27c3fd2..5142c4737 100644 --- a/server/modules/routing/binlogrouter/blr_slave.cc +++ b/server/modules/routing/binlogrouter/blr_slave.cc @@ -4850,9 +4850,8 @@ static void blr_master_get_config(ROUTER_INSTANCE* router, MasterServerConfig* c curr_master->password = router->password; curr_master->filestem = router->fileroot; /* SSL options */ - if (router->service->dbref->server->server_ssl) + if (auto server_ssl = router->service->dbref->server->ssl_context()) { - auto server_ssl = router->service->dbref->server->server_ssl; curr_master->ssl_enabled = router->ssl_enabled; if (router->ssl_version) { @@ -6353,8 +6352,7 @@ static int blr_set_master_ssl(ROUTER_INSTANCE* router, if (ssl) { updated = 1; - delete router->service->dbref->server->server_ssl; - router->service->dbref->server->server_ssl = ssl; + router->service->dbref->server->set_ssl_context(ssl); /* Update options in router fields */ if (!config.ssl_key.empty())