From 1197bd40db1b6c89a1e481d69ec3232547395125 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 17 May 2019 16:31:51 +0300 Subject: [PATCH] MXS-2483: Move unwanted SSL code to mysql_client.cc The code was only used by mysql_client.cc and should therefore be located in it. --- include/maxscale/ssl.hh | 17 -- server/core/ssl.cc | 188 ------------------ .../MySQL/mariadbclient/mysql_client.cc | 188 ++++++++++++++++++ 3 files changed, 188 insertions(+), 205 deletions(-) diff --git a/include/maxscale/ssl.hh b/include/maxscale/ssl.hh index de5d19e69..93132209c 100644 --- a/include/maxscale/ssl.hh +++ b/include/maxscale/ssl.hh @@ -73,26 +73,9 @@ struct SSLContext }; } -int ssl_authenticate_client(DCB* dcb, bool is_capable); -bool ssl_is_connection_healthy(DCB* dcb); -bool ssl_check_data_to_process(DCB* dcb); -bool ssl_required_by_dcb(DCB* dcb); -bool ssl_required_but_not_negotiated(DCB* dcb); const char* ssl_method_type_to_string(ssl_method_type_t method_type); ssl_method_type_t string_to_ssl_method_type(const char* str); -/** - * Helper function for client ssl authentication. Authenticates and checks changes - * in ssl status. - * - * @param dcb Client dcb - * - * @return MXS_AUTH_FAILED_SSL or MXS_AUTH_FAILED on error. MXS_AUTH_SSL_INCOMPLETE - * if ssl authentication is in progress and should be retried, MXS_AUTH_SSL_COMPLETE - * if ssl authentication is complete or not required. - */ -int ssl_authenticate_check_status(DCB* dcb); - // TODO: Move this to an internal ssl.h header void write_ssl_config(int fd, mxs::SSLContext* ssl); diff --git a/server/core/ssl.cc b/server/core/ssl.cc index 3b2dd4e7c..840859472 100644 --- a/server/core/ssl.cc +++ b/server/core/ssl.cc @@ -38,162 +38,6 @@ static RSA* rsa_512 = NULL; static RSA* rsa_1024 = NULL; static RSA* tmp_rsa_callback(SSL* s, int is_export, int keylength); -/** - * @brief Check client's SSL capability and start SSL if appropriate. - * - * The protocol should determine whether the client is SSL capable and pass - * the result as the second parameter. If the listener requires SSL but the - * client is not SSL capable, an error message is recorded and failure return - * given. If both sides want SSL, and SSL is not already established, the - * process is triggered by calling dcb_accept_SSL. - * - * @param dcb Request handler DCB connected to the client - * @param is_capable Indicates if the client can handle SSL - * @return 0 if ok, >0 if a problem - see return codes defined in ssl.h - */ -int ssl_authenticate_client(DCB* dcb, bool is_capable) -{ - const char* user = dcb->user ? dcb->user : ""; - const char* remote = dcb->remote ? dcb->remote : ""; - const char* service = (dcb->service && dcb->service->name()) ? dcb->service->name() : ""; - - if (NULL == dcb->session->listener || NULL == dcb->session->listener->ssl()) - { - /* Not an SSL connection on account of listener configuration */ - return SSL_AUTH_CHECKS_OK; - } - /* Now we require an SSL connection */ - if (!is_capable) - { - /* Should be SSL, but client is not SSL capable */ - MXS_INFO("User %s@%s connected to service '%s' without SSL when SSL was required.", - user, - remote, - service); - return SSL_ERROR_CLIENT_NOT_SSL; - } - /* Now we know SSL is required and client is capable */ - if (dcb->ssl_state != SSL_HANDSHAKE_DONE && dcb->ssl_state != SSL_ESTABLISHED) - { - int return_code; - /** Do the SSL Handshake */ - if (SSL_HANDSHAKE_UNKNOWN == dcb->ssl_state) - { - dcb->ssl_state = SSL_HANDSHAKE_REQUIRED; - } - /** - * Note that this will often fail to achieve its result, because further - * reading (or possibly writing) of SSL related information is needed. - * When that happens, there is a call in poll.c so that an EPOLLIN - * event that arrives while the SSL state is SSL_HANDSHAKE_REQUIRED - * will trigger dcb_accept_SSL. This situation does not result in a - * negative return code - that indicates a real failure. - */ - return_code = dcb_accept_SSL(dcb); - if (return_code < 0) - { - MXS_INFO("User %s@%s failed to connect to service '%s' with SSL.", - user, - remote, - service); - return SSL_ERROR_ACCEPT_FAILED; - } - else if (mxs_log_is_priority_enabled(LOG_INFO)) - { - if (1 == return_code) - { - MXS_INFO("User %s@%s connected to service '%s' with SSL.", - user, - remote, - service); - } - else - { - MXS_INFO("User %s@%s connect to service '%s' with SSL in progress.", - user, - remote, - service); - } - } - } - return SSL_AUTH_CHECKS_OK; -} - -/** - * @brief If an SSL connection is required, check that it has been established. - * - * This is called at the end of the authentication of a new connection. - * If the result is not true, the data packet is abandoned with further - * data expected from the client. - * - * @param dcb Request handler DCB connected to the client - * @return Boolean to indicate whether connection is healthy - */ -bool ssl_is_connection_healthy(DCB* dcb) -{ - /** - * If SSL was never expected, or if the connection has state SSL_ESTABLISHED - * then everything is as we wish. Otherwise, either there is a problem or - * more to be done. - */ - return NULL == dcb->session->listener - || NULL == dcb->session->listener->ssl() - || dcb->ssl_state == SSL_ESTABLISHED; -} - -/* Looks to be redundant - can remove include for ioctl too */ -bool ssl_check_data_to_process(DCB* dcb) -{ - /** SSL authentication is still going on, we need to call dcb_accept_SSL - * until it return 1 for success or -1 for error */ - if (dcb->ssl_state == SSL_HANDSHAKE_REQUIRED && 1 == dcb_accept_SSL(dcb)) - { - int b = 0; - ioctl(dcb->fd, FIONREAD, &b); - if (b != 0) - { - return true; - } - else - { - MXS_DEBUG("[gw_read_client_event] No data in socket after SSL auth"); - } - } - return false; -} - -/** - * @brief Check whether a DCB requires SSL. - * - * This is a very simple test, but is placed in an SSL function so that - * the knowledge of the SSL process is removed from the more general - * handling of a connection in the protocols. - * - * @param dcb Request handler DCB connected to the client - * @return Boolean indicating whether SSL is required. - */ -bool ssl_required_by_dcb(DCB* dcb) -{ - return NULL != dcb->session->listener && NULL != dcb->session->listener->ssl(); -} - -/** - * @brief Check whether a DCB requires SSL, but SSL is not yet negotiated. - * - * This is a very simple test, but is placed in an SSL function so that - * the knowledge of the SSL process is removed from the more general - * handling of a connection in the protocols. - * - * @param dcb Request handler DCB connected to the client - * @return Boolean indicating whether SSL is required and not negotiated. - */ -bool ssl_required_but_not_negotiated(DCB* dcb) -{ - return NULL != dcb->session->listener - && NULL != dcb->session->listener->ssl() - && SSL_HANDSHAKE_UNKNOWN == dcb->ssl_state; -} - /** * Returns an enum ssl_method_type value as string. * @@ -288,38 +132,6 @@ void write_ssl_config(int fd, mxs::SSLContext* ssl) } } -int ssl_authenticate_check_status(DCB* dcb) -{ - int rval = MXS_AUTH_FAILED; - /** - * We record the SSL status before and after ssl authentication. This allows - * us to detect if the SSL handshake is immediately completed, which means more - * data needs to be read from the socket. - */ - bool health_before = ssl_is_connection_healthy(dcb); - int ssl_ret = ssl_authenticate_client(dcb, dcb->authfunc.connectssl(dcb)); - bool health_after = ssl_is_connection_healthy(dcb); - - if (ssl_ret != 0) - { - rval = (ssl_ret == SSL_ERROR_CLIENT_NOT_SSL) ? MXS_AUTH_FAILED_SSL : MXS_AUTH_FAILED; - } - else if (!health_after) - { - rval = MXS_AUTH_SSL_INCOMPLETE; - } - else if (!health_before && health_after) - { - rval = MXS_AUTH_SSL_INCOMPLETE; - poll_add_epollin_event_to_dcb(dcb, NULL); - } - else if (health_before && health_after) - { - rval = MXS_AUTH_SSL_COMPLETE; - } - return rval; -} - int listener_set_ssl_version(mxs::SSLContext* ssl_listener, const char* version) { if (strcasecmp(version, "MAX") == 0) diff --git a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc index 887601580..d8b007e90 100644 --- a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc +++ b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc @@ -229,6 +229,38 @@ std::string get_version_string(SERVICE* service) return rval; } +/** + * @brief Check whether a DCB requires SSL. + * + * This is a very simple test, but is placed in an SSL function so that + * the knowledge of the SSL process is removed from the more general + * handling of a connection in the protocols. + * + * @param dcb Request handler DCB connected to the client + * @return Boolean indicating whether SSL is required. + */ +bool ssl_required_by_dcb(DCB* dcb) +{ + return NULL != dcb->session->listener && NULL != dcb->session->listener->ssl(); +} + +/** + * @brief Check whether a DCB requires SSL, but SSL is not yet negotiated. + * + * This is a very simple test, but is placed in an SSL function so that + * the knowledge of the SSL process is removed from the more general + * handling of a connection in the protocols. + * + * @param dcb Request handler DCB connected to the client + * @return Boolean indicating whether SSL is required and not negotiated. + */ +bool ssl_required_but_not_negotiated(DCB* dcb) +{ + return NULL != dcb->session->listener + && NULL != dcb->session->listener->ssl() + && SSL_HANDSHAKE_UNKNOWN == dcb->ssl_state; +} + /** * MySQLSendHandshake * @@ -689,6 +721,162 @@ static void check_packet(DCB* dcb, GWBUF* buf, int bytes) } } +/** + * @brief If an SSL connection is required, check that it has been established. + * + * This is called at the end of the authentication of a new connection. + * If the result is not true, the data packet is abandoned with further + * data expected from the client. + * + * @param dcb Request handler DCB connected to the client + * @return Boolean to indicate whether connection is healthy + */ +bool ssl_is_connection_healthy(DCB* dcb) +{ + /** + * If SSL was never expected, or if the connection has state SSL_ESTABLISHED + * then everything is as we wish. Otherwise, either there is a problem or + * more to be done. + */ + return NULL == dcb->session->listener + || NULL == dcb->session->listener->ssl() + || dcb->ssl_state == SSL_ESTABLISHED; +} + +/* Looks to be redundant - can remove include for ioctl too */ +bool ssl_check_data_to_process(DCB* dcb) +{ + /** SSL authentication is still going on, we need to call dcb_accept_SSL + * until it return 1 for success or -1 for error */ + if (dcb->ssl_state == SSL_HANDSHAKE_REQUIRED && 1 == dcb_accept_SSL(dcb)) + { + int b = 0; + ioctl(dcb->fd, FIONREAD, &b); + if (b != 0) + { + return true; + } + else + { + MXS_DEBUG("[gw_read_client_event] No data in socket after SSL auth"); + } + } + return false; +} + +/** + * @brief Check client's SSL capability and start SSL if appropriate. + * + * The protocol should determine whether the client is SSL capable and pass + * the result as the second parameter. If the listener requires SSL but the + * client is not SSL capable, an error message is recorded and failure return + * given. If both sides want SSL, and SSL is not already established, the + * process is triggered by calling dcb_accept_SSL. + * + * @param dcb Request handler DCB connected to the client + * @param is_capable Indicates if the client can handle SSL + * @return 0 if ok, >0 if a problem - see return codes defined in ssl.h + */ +int ssl_authenticate_client(DCB* dcb, bool is_capable) +{ + const char* user = dcb->user ? dcb->user : ""; + const char* remote = dcb->remote ? dcb->remote : ""; + const char* service = (dcb->service && dcb->service->name()) ? dcb->service->name() : ""; + + if (NULL == dcb->session->listener || NULL == dcb->session->listener->ssl()) + { + /* Not an SSL connection on account of listener configuration */ + return SSL_AUTH_CHECKS_OK; + } + /* Now we require an SSL connection */ + if (!is_capable) + { + /* Should be SSL, but client is not SSL capable */ + MXS_INFO("User %s@%s connected to service '%s' without SSL when SSL was required.", + user, + remote, + service); + return SSL_ERROR_CLIENT_NOT_SSL; + } + /* Now we know SSL is required and client is capable */ + if (dcb->ssl_state != SSL_HANDSHAKE_DONE && dcb->ssl_state != SSL_ESTABLISHED) + { + int return_code; + /** Do the SSL Handshake */ + if (SSL_HANDSHAKE_UNKNOWN == dcb->ssl_state) + { + dcb->ssl_state = SSL_HANDSHAKE_REQUIRED; + } + /** + * Note that this will often fail to achieve its result, because further + * reading (or possibly writing) of SSL related information is needed. + * When that happens, there is a call in poll.c so that an EPOLLIN + * event that arrives while the SSL state is SSL_HANDSHAKE_REQUIRED + * will trigger dcb_accept_SSL. This situation does not result in a + * negative return code - that indicates a real failure. + */ + return_code = dcb_accept_SSL(dcb); + if (return_code < 0) + { + MXS_INFO("User %s@%s failed to connect to service '%s' with SSL.", + user, + remote, + service); + return SSL_ERROR_ACCEPT_FAILED; + } + else if (mxs_log_is_priority_enabled(LOG_INFO)) + { + if (1 == return_code) + { + MXS_INFO("User %s@%s connected to service '%s' with SSL.", + user, + remote, + service); + } + else + { + MXS_INFO("User %s@%s connect to service '%s' with SSL in progress.", + user, + remote, + service); + } + } + } + return SSL_AUTH_CHECKS_OK; +} + +int ssl_authenticate_check_status(DCB* dcb) +{ + int rval = MXS_AUTH_FAILED; + /** + * We record the SSL status before and after ssl authentication. This allows + * us to detect if the SSL handshake is immediately completed, which means more + * data needs to be read from the socket. + */ + bool health_before = ssl_is_connection_healthy(dcb); + int ssl_ret = ssl_authenticate_client(dcb, dcb->authfunc.connectssl(dcb)); + bool health_after = ssl_is_connection_healthy(dcb); + + if (ssl_ret != 0) + { + rval = (ssl_ret == SSL_ERROR_CLIENT_NOT_SSL) ? MXS_AUTH_FAILED_SSL : MXS_AUTH_FAILED; + } + else if (!health_after) + { + rval = MXS_AUTH_SSL_INCOMPLETE; + } + else if (!health_before && health_after) + { + rval = MXS_AUTH_SSL_INCOMPLETE; + poll_add_epollin_event_to_dcb(dcb, NULL); + } + else if (health_before && health_after) + { + rval = MXS_AUTH_SSL_COMPLETE; + } + return rval; +} + /** * @brief Client read event, process when client not yet authenticated *