From 4eddec7989d4254cd20b696e92e5d2671068c90b Mon Sep 17 00:00:00 2001 From: Sriram Patil Date: Wed, 20 May 2015 17:47:58 +0530 Subject: [PATCH 01/54] Fixed MXS - 165: Concurrency issue while incrementing sessions in qlafilter --- server/modules/filter/qlafilter.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/modules/filter/qlafilter.c b/server/modules/filter/qlafilter.c index 5362cf61e..3c358cda1 100644 --- a/server/modules/filter/qlafilter.c +++ b/server/modules/filter/qlafilter.c @@ -50,6 +50,7 @@ #include #include #include +#include /** Defined in log_manager.cc */ extern int lm_enabled_logfiles_bitmask; @@ -304,7 +305,9 @@ char *remote, *userName; sprintf(my_session->filename, "%s.%d", my_instance->filebase, my_instance->sessions); - my_instance->sessions++; + + // Multiple sessions can try to update my_instance->sessions simultaneously + atomic_add(&(my_instance->sessions), 1); if (my_session->active) { From 16d6bd6d2c7f4d0b40eee154d6656902a238187d Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 28 May 2015 11:56:14 +0300 Subject: [PATCH 02/54] Added service SSL mode variables. --- server/core/config.c | 2 +- server/core/service.c | 13 +++++++++- server/include/service.h | 7 ++++++ .../include/mysql_client_server_protocol.h | 4 ++++ server/modules/protocol/mysql_client.c | 24 ++++++++++++++++--- 5 files changed, 45 insertions(+), 5 deletions(-) diff --git a/server/core/config.c b/server/core/config.c index 274522468..ccbeee0e0 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -420,7 +420,7 @@ hashtable_memory_fns(monitorhash,strdup,NULL,free,NULL); /** Add the 5.5.5- string to the start of the version string if * the version string starts with "10.". - * This mimics MariaDB 10.0 replication which adds 5.5.5- for backwards compatibility. */ + * This mimics MariaDB 10.0 behavior which adds 5.5.5- for backwards compatibility. */ if(strncmp(version_string,"10.",3) == 0) { ((SERVICE *)(obj->element))->version_string = malloc((strlen(version_string) + diff --git a/server/core/service.c b/server/core/service.c index 5ba5d539d..8297ea6fd 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -136,7 +136,8 @@ SERVICE *service; service->routerModule = strdup(router); service->users_from_all = false; service->resources = NULL; - + service->ssl_mode = SSL_REQUIRED; + if (service->name == NULL || service->routerModule == NULL) { if (service->name) @@ -855,6 +856,16 @@ serviceOptimizeWildcard(SERVICE *service, int action) return 1; } +/** Enable or disable the service SSL capability*/ +int +serviceSetSSL(SERVICE *service, int action) +{ + if(action) + service->ssl_mode = SSL_REQUIRED; + else + service->ssl_mode = SSL_DISABLED; +} + /** * Whether to strip escape characters from the name of the database the client * is connecting to. diff --git a/server/include/service.h b/server/include/service.h index f26c99806..a6fea6d56 100644 --- a/server/include/service.h +++ b/server/include/service.h @@ -105,6 +105,12 @@ typedef struct server_ref_t{ SERVER* server; }SERVER_REF; +typedef enum { + SSL_DISABLED, + SSL_ENABLED, + SSL_REQUIRED +} ssl_mode_t; + /** * Defines a service within the gateway. * @@ -149,6 +155,7 @@ typedef struct service { FILTER_DEF **filters; /**< Ordered list of filters */ int n_filters; /**< Number of filters */ int conn_timeout; /*< Session timeout in seconds */ + ssl_mode_t ssl_mode; /*< one of DISABLED, ENABLED or REQUIRED */ char *weightby; struct service *next; /**< The next service in the linked list */ } SERVICE; diff --git a/server/modules/include/mysql_client_server_protocol.h b/server/modules/include/mysql_client_server_protocol.h index 46bbe296c..de8118659 100644 --- a/server/modules/include/mysql_client_server_protocol.h +++ b/server/modules/include/mysql_client_server_protocol.h @@ -97,6 +97,10 @@ typedef enum { MYSQL_AUTH_RECV, MYSQL_AUTH_FAILED, MYSQL_HANDSHAKE_FAILED, + MYSQL_AUTH_SSL_REQ, /*< client requested SSL */ + MYSQL_AUTH_SSL_EXCHANGE_DONE, /*< SSL handshake done */ + MYSQL_AUTH_SSL_EXCHANGE_ERR, /*< SSL handshake failure */ + MYSQL_AUTH_SSL_RECV, /*< */ MYSQL_IDLE } mysql_auth_state_t; diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index abdb4422c..d1e188281 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -242,7 +242,7 @@ MySQLSendHandshake(DCB* dcb) char server_scramble[GW_MYSQL_SCRAMBLE_SIZE + 1]=""; char *version_string; int len_version_string=0; - + MySQLProtocol *protocol = DCB_PROTOCOL(dcb, MySQLProtocol); GWBUF *buf; @@ -319,7 +319,16 @@ MySQLSendHandshake(DCB* dcb) mysql_server_capabilities_one[0] &= ~GW_MYSQL_CAPABILITIES_COMPRESS; - mysql_server_capabilities_one[0] &= ~GW_MYSQL_CAPABILITIES_SSL; + + if(dcb->service->ssl_mode != SSL_DISABLED) + { + mysql_server_capabilities_one[1] |= GW_MYSQL_CAPABILITIES_SSL >> 8; + } + else + { + mysql_server_capabilities_one[0] &= ~GW_MYSQL_CAPABILITIES_SSL; + } + memcpy(mysql_handshake_payload, mysql_server_capabilities_one, sizeof(mysql_server_capabilities_one)); mysql_handshake_payload = mysql_handshake_payload + sizeof(mysql_server_capabilities_one); @@ -402,7 +411,7 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { uint8_t *stage1_hash = NULL; int auth_ret = -1; MYSQL_session *client_data = NULL; - + int ssl = 0; CHK_DCB(dcb); protocol = DCB_PROTOCOL(dcb, MySQLProtocol); @@ -451,6 +460,15 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { &protocol->client_capabilities); */ + + ssl = protocol->client_capabilities & GW_MYSQL_CAPABILITIES_SSL; + + /** Client didn't requested SSL when SSL mode was required*/ + if(!ssl && protocol->owner_dcb->service->ssl_mode == SSL_REQUIRED) + { + return 1; + } + username = get_username_from_auth(username, client_auth_packet); if (username == NULL) From 3d6259cb00d32818b9bc98f154a772543e524509 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 28 May 2015 16:33:51 +0300 Subject: [PATCH 03/54] Added configuration options for different SSL modes. --- server/core/config.c | 9 ++++++++- server/core/service.c | 20 ++++++++++++++++---- server/include/service.h | 1 + server/modules/protocol/mysql_client.c | 12 ++++++++++++ 4 files changed, 37 insertions(+), 5 deletions(-) diff --git a/server/core/config.c b/server/core/config.c index ccbeee0e0..f6721a28e 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -345,6 +345,7 @@ hashtable_memory_fns(monitorhash,strdup,NULL,free,NULL); char *weightby; char *version_string; char *subservices; + char* ssl; bool is_rwsplit = false; bool is_schemarouter = false; char *allow_localhost_match_wildcard_host; @@ -353,6 +354,8 @@ hashtable_memory_fns(monitorhash,strdup,NULL,free,NULL); user = config_get_value(obj->parameters, "user"); auth = config_get_value(obj->parameters, "passwd"); subservices = config_get_value(obj->parameters, "subservices"); + ssl = config_get_value(obj->parameters, "ssl"); + enable_root_user = config_get_value( obj->parameters, "enable_root_user"); @@ -443,7 +446,11 @@ hashtable_memory_fns(monitorhash,strdup,NULL,free,NULL); max_slave_rlag_str = config_get_value(obj->parameters, "max_slave_replication_lag"); - + + if(ssl) + if(serviceSetSSL(obj->element,ssl) != 0) + skygw_log_write(LE,"Error: Unknown parameter for service '%s': %s",obj->object,ssl); + if (enable_root_user) serviceEnableRootUser( obj->element, diff --git a/server/core/service.c b/server/core/service.c index 8297ea6fd..4ef9b3515 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -136,7 +136,7 @@ SERVICE *service; service->routerModule = strdup(router); service->users_from_all = false; service->resources = NULL; - service->ssl_mode = SSL_REQUIRED; + service->ssl_mode = SSL_DISABLED; if (service->name == NULL || service->routerModule == NULL) { @@ -858,12 +858,20 @@ serviceOptimizeWildcard(SERVICE *service, int action) /** Enable or disable the service SSL capability*/ int -serviceSetSSL(SERVICE *service, int action) +serviceSetSSL(SERVICE *service, char* action) { - if(action) + int rval = 0; + + if(strcasecmp(action,"required") == 0) service->ssl_mode = SSL_REQUIRED; - else + else if(strcasecmp(action,"enabled") == 0) + service->ssl_mode = SSL_ENABLED; + else if(strcasecmp(action,"disabled") == 0) service->ssl_mode = SSL_DISABLED; + else + rval = -1; + + return rval; } /** @@ -1029,6 +1037,8 @@ int i; printf("\tUsers data: %p\n", (void *)service->users); printf("\tTotal connections: %d\n", service->stats.n_sessions); printf("\tCurrently connected: %d\n", service->stats.n_current); + printf("\tSSL: %s\n", service->ssl_mode == SSL_DISABLED ? "Disabled": + (service->ssl_mode == SSL_ENABLED ? "Enabled":"Required")); } /** @@ -1138,6 +1148,8 @@ int i; service->stats.n_sessions); dcb_printf(dcb, "\tCurrently connected: %d\n", service->stats.n_current); + dcb_printf(dcb,"\tSSL: %s\n", service->ssl_mode == SSL_DISABLED ? "Disabled": + (service->ssl_mode == SSL_ENABLED ? "Enabled":"Required")); } /** diff --git a/server/include/service.h b/server/include/service.h index a6fea6d56..e0ae151cf 100644 --- a/server/include/service.h +++ b/server/include/service.h @@ -185,6 +185,7 @@ extern int serviceRestart(SERVICE *); extern int serviceSetUser(SERVICE *, char *, char *); extern int serviceGetUser(SERVICE *, char **, char **); extern void serviceSetFilters(SERVICE *, char *); +extern int serviceSetSSL(SERVICE *service, char* action); extern int serviceEnableRootUser(SERVICE *, int ); extern int serviceSetTimeout(SERVICE *, int ); extern void serviceWeightBy(SERVICE *, char *); diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index d1e188281..eaf061334 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -466,9 +466,21 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { /** Client didn't requested SSL when SSL mode was required*/ if(!ssl && protocol->owner_dcb->service->ssl_mode == SSL_REQUIRED) { + LOGIF(LT,(skygw_log_write(LT,"User %s@%s connected to service '%s' without SSL when SSL was required.", + protocol->owner_dcb->user, + protocol->owner_dcb->remote, + protocol->owner_dcb->service->name))); return 1; } + if(LOG_IS_ENABLED(LT)) + { + skygw_log_write(LT,"User %s@%s connected to service '%s' with SSL.", + protocol->owner_dcb->user, + protocol->owner_dcb->remote, + protocol->owner_dcb->service->name); + } + username = get_username_from_auth(username, client_auth_packet); if (username == NULL) From 449c186a668a6d48f9de484ada049af703fe43b0 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 28 May 2015 18:13:45 +0300 Subject: [PATCH 04/54] Added OpenSSL init function call. --- server/modules/protocol/mysql_client.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index eaf061334..0d5c47b48 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -46,7 +46,9 @@ #include #include #include - +#include +#include +#include MODULE_INFO info = { MODULE_API_PROTOCOL, MODULE_GA, @@ -113,6 +115,7 @@ version() void ModuleInit() { + SSL_library_init(); } /** @@ -473,7 +476,7 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { return 1; } - if(LOG_IS_ENABLED(LT)) + if(LOG_IS_ENABLED(LT) && ssl) { skygw_log_write(LT,"User %s@%s connected to service '%s' with SSL.", protocol->owner_dcb->user, @@ -481,6 +484,11 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { protocol->owner_dcb->service->name); } + if(ssl && protocol->owner_dcb->service->ssl_mode != SSL_DISABLED) + { + + } + username = get_username_from_auth(username, client_auth_packet); if (username == NULL) From a572166ffd0a3152fdf7229e17fcf0242a633587 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 28 May 2015 22:19:50 +0300 Subject: [PATCH 05/54] Added ssl handshake to mysql_client --- server/core/config.c | 30 +++++++++-- server/core/service.c | 50 +++++++++++++++++++ server/include/service.h | 15 +++++- .../include/mysql_client_server_protocol.h | 5 +- server/modules/protocol/mysql_client.c | 44 ++++++++++++++-- 5 files changed, 134 insertions(+), 10 deletions(-) diff --git a/server/core/config.c b/server/core/config.c index f6721a28e..12d4c099e 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -345,7 +345,7 @@ hashtable_memory_fns(monitorhash,strdup,NULL,free,NULL); char *weightby; char *version_string; char *subservices; - char* ssl; + char *ssl,*ssl_cert,*ssl_key,*ssl_ca_cert; bool is_rwsplit = false; bool is_schemarouter = false; char *allow_localhost_match_wildcard_host; @@ -355,7 +355,9 @@ hashtable_memory_fns(monitorhash,strdup,NULL,free,NULL); auth = config_get_value(obj->parameters, "passwd"); subservices = config_get_value(obj->parameters, "subservices"); ssl = config_get_value(obj->parameters, "ssl"); - + ssl_cert = config_get_value(obj->parameters, "ssl_cert"); + ssl_key = config_get_value(obj->parameters, "ssl_key"); + ssl_ca_cert = config_get_value(obj->parameters, "ssl_ca_cert"); enable_root_user = config_get_value( obj->parameters, "enable_root_user"); @@ -448,8 +450,28 @@ hashtable_memory_fns(monitorhash,strdup,NULL,free,NULL); "max_slave_replication_lag"); if(ssl) - if(serviceSetSSL(obj->element,ssl) != 0) - skygw_log_write(LE,"Error: Unknown parameter for service '%s': %s",obj->object,ssl); + { + if(ssl_cert == NULL) + skygw_log_write(LE,"Error: Server certificate missing for service '%s'.",obj->object); + if(ssl_ca_cert == NULL) + skygw_log_write(LE,"Error: CA Certificate missing for service '%s'.",obj->object); + if(ssl_key == NULL) + skygw_log_write(LE,"Error: Server private key missing for service '%s'.",obj->object); + + if(ssl_ca_cert != NULL && ssl_cert != NULL && ssl_key != NULL) + { + + if(serviceSetSSL(obj->element,ssl) != 0) + { + skygw_log_write(LE,"Error: Unknown parameter for service '%s': %s",obj->object,ssl); + } + else + { + serviceSetCertificates(obj->element,ssl_cert,ssl_key,ssl_ca_cert); + } + } + + } if (enable_root_user) serviceEnableRootUser( diff --git a/server/core/service.c b/server/core/service.c index 4ef9b3515..ee6d1cf26 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -137,6 +137,10 @@ SERVICE *service; service->users_from_all = false; service->resources = NULL; service->ssl_mode = SSL_DISABLED; + service->ssl_init_done = false; + service->ssl_ca_cert = NULL; + service->ssl_cert = NULL; + service->ssl_key = NULL; if (service->name == NULL || service->routerModule == NULL) { @@ -856,6 +860,14 @@ serviceOptimizeWildcard(SERVICE *service, int action) return 1; } +void +serviceSetCertificates(SERVICE *service, char* cert,char* key, char* ca_cert) +{ + service->ssl_cert = strdup(cert); + service->ssl_key = strdup(key); + service->ssl_ca_cert = strdup(ca_cert); +} + /** Enable or disable the service SSL capability*/ int serviceSetSSL(SERVICE *service, char* action) @@ -1798,3 +1810,41 @@ int *data; return set; } + + +int serviceInitSSL(SERVICE* service) +{ + if(!service->ssl_init_done) + { + service->method = (SSL_METHOD*)SSLv23_server_method(); + service->ctx = SSL_CTX_new(service->method); + + if (SSL_CTX_use_certificate_file(service->ctx, service->ssl_cert, SSL_FILETYPE_PEM) <= 0) { + return -1; + } + + /* Load the private-key corresponding to the server certificate */ + if (SSL_CTX_use_PrivateKey_file(service->ctx, service->ssl_key, SSL_FILETYPE_PEM) <= 0) { + return -1; + } + + /* Check if the server certificate and private-key matches */ + if (!SSL_CTX_check_private_key(service->ctx)) { + return -1; + } + + + /* Load the RSA CA certificate into the SSL_CTX structure */ + if (!SSL_CTX_load_verify_locations(service->ctx, service->ssl_ca_cert, NULL)) { + return -1; + } + + /* Set to require peer (client) certificate verification */ + SSL_CTX_set_verify(service->ctx,SSL_VERIFY_PEER,NULL); + + /* Set the verification depth to 1 */ + SSL_CTX_set_verify_depth(service->ctx,10); + service->ssl_init_done = true; + } + return 0; +} diff --git a/server/include/service.h b/server/include/service.h index e0ae151cf..1c6614656 100644 --- a/server/include/service.h +++ b/server/include/service.h @@ -26,7 +26,9 @@ #include #include #include - +#include +#include +#include /** * @file service.h * @@ -158,6 +160,15 @@ typedef struct service { ssl_mode_t ssl_mode; /*< one of DISABLED, ENABLED or REQUIRED */ char *weightby; struct service *next; /**< The next service in the linked list */ + SSL_CTX *ctx; + SSL *ssl; + SSL_METHOD *method; /*< SSLv2/3 or TLSv1/2 methods + * see: https://www.openssl.org/docs/ssl/SSL_CTX_new.html */ + char* ssl_cert; + char* ssl_key; + char* ssl_ca_cert; + bool ssl_init_done; + } SERVICE; typedef enum count_spec_t {COUNT_NONE=0, COUNT_ATLEAST, COUNT_EXACT, COUNT_ATMOST} count_spec_t; @@ -186,6 +197,8 @@ extern int serviceSetUser(SERVICE *, char *, char *); extern int serviceGetUser(SERVICE *, char **, char **); extern void serviceSetFilters(SERVICE *, char *); extern int serviceSetSSL(SERVICE *service, char* action); +extern int serviceInitSSL(SERVICE* service); +extern void serviceSetCertificates(SERVICE *service, char* cert,char* key, char* ca_cert); extern int serviceEnableRootUser(SERVICE *, int ); extern int serviceSetTimeout(SERVICE *, int ); extern void serviceWeightBy(SERVICE *, char *); diff --git a/server/modules/include/mysql_client_server_protocol.h b/server/modules/include/mysql_client_server_protocol.h index de8118659..87dbc50ee 100644 --- a/server/modules/include/mysql_client_server_protocol.h +++ b/server/modules/include/mysql_client_server_protocol.h @@ -54,7 +54,9 @@ #include #include #include - +#include +#include +#include #include #include #include @@ -294,6 +296,7 @@ typedef struct { unsigned long tid; /*< MySQL Thread ID, in * handshake */ unsigned int charset; /*< MySQL character set at connect time */ + SSL* ssl; /*< SSL struct for client connection */ #if defined(SS_DEBUG) skygw_chk_t protocol_chk_tail; #endif diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index 0d5c47b48..5adb8bcd0 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -46,9 +46,7 @@ #include #include #include -#include -#include -#include + MODULE_INFO info = { MODULE_API_PROTOCOL, MODULE_GA, @@ -116,6 +114,7 @@ void ModuleInit() { SSL_library_init(); + SSL_load_error_strings(); } /** @@ -484,9 +483,47 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { protocol->owner_dcb->service->name); } + /** Do the SSL Handshake */ if(ssl && protocol->owner_dcb->service->ssl_mode != SSL_DISABLED) { + if(serviceInitSSL(protocol->owner_dcb->service) != 0) + { + skygw_log_write(LOGFILE_ERROR,"Error: SSL initialization for service '%s' failed.", + protocol->owner_dcb->service->name); + return 1; + } + protocol->ssl = SSL_new(protocol->owner_dcb->service->ctx); + SSL_set_fd(protocol->ssl,dcb->fd); + protocol->protocol_auth_state = MYSQL_AUTH_SSL_REQ; + printf("%s\n",SSL_get_version(protocol->ssl)); + int errnum,rval; + char errbuf[1024]; + switch((rval = SSL_accept(protocol->ssl))) + { + case 0: + errnum = SSL_get_error(protocol->ssl,rval); + ERR_error_string(errnum,errbuf); + skygw_log_write_flush(LOGFILE_ERROR,"SSL_accept: %s",errbuf); + ERR_print_errors_fp(stdout); + ERR_error_string(errnum,errbuf); + printf("%s\n",errbuf); + fflush(stdout); + break; + case 1: + protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_DONE; + break; + default: + errnum = SSL_get_error(protocol->ssl,rval); + ERR_print_errors_fp(stdout); + ERR_error_string(errnum,errbuf); + printf("%s\n",errbuf); + fflush(stdout); + skygw_log_write_flush(LOGFILE_ERROR,"Error: Fatal error in SSL_accept: %s",errbuf); + protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_ERR; + break; + + } } username = get_username_from_auth(username, client_auth_packet); @@ -1700,4 +1737,3 @@ return_str: return str; } #endif - From f946a44620b9f709d01dbcdb11c0769f1dab733d Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 28 May 2015 23:11:32 +0300 Subject: [PATCH 06/54] Added handling of partial SSL handshakes. --- server/modules/protocol/mysql_client.c | 91 +++++++++++++++++++++++--- 1 file changed, 83 insertions(+), 8 deletions(-) diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index 5adb8bcd0..387b59aa0 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -78,6 +78,8 @@ extern char* get_username_from_auth(char* ptr, uint8_t* data); extern int check_db_name_after_auth(DCB *, char *, int); extern char* create_auth_fail_str(char *username, char *hostaddr, char *sha1, char *db); +int do_ssl_accept(MySQLProtocol* protocol); + /* * The "module object" for the mysqld client protocol module. */ @@ -498,6 +500,7 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { printf("%s\n",SSL_get_version(protocol->ssl)); int errnum,rval; char errbuf[1024]; + switch((rval = SSL_accept(protocol->ssl))) { case 0: @@ -515,12 +518,22 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { default: errnum = SSL_get_error(protocol->ssl,rval); - ERR_print_errors_fp(stdout); - ERR_error_string(errnum,errbuf); - printf("%s\n",errbuf); - fflush(stdout); - skygw_log_write_flush(LOGFILE_ERROR,"Error: Fatal error in SSL_accept: %s",errbuf); - protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_ERR; + if(errnum == SSL_ERROR_WANT_READ) + { + /** Not all of the data has been read. Go back to the poll + queue and wait for more.*/ + protocol->protocol_auth_state = MYSQL_AUTH_SSL_RECV; + return 0; + } + else + { + ERR_print_errors_fp(stdout); + ERR_error_string(errnum,errbuf); + printf("%s\n",errbuf); + fflush(stdout); + skygw_log_write_flush(LOGFILE_ERROR,"Error: Fatal error in SSL_accept: %s",errbuf); + protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_ERR; + } break; } @@ -655,6 +668,10 @@ int gw_read_client_event( CHK_DCB(dcb); protocol = DCB_PROTOCOL(dcb, MySQLProtocol); CHK_PROTOCOL(protocol); + if(protocol->protocol_auth_state == MYSQL_AUTH_SSL_RECV) + { + goto do_auth; + } rc = dcb_read(dcb, &read_buffer); @@ -794,7 +811,7 @@ int gw_read_client_event( } } - + do_auth: /** * Now there should be at least one complete mysql packet in read_buffer. */ @@ -805,7 +822,10 @@ int gw_read_client_event( int auth_val; auth_val = gw_mysql_do_authentication(dcb, read_buffer); - + + if(protocol->protocol_auth_state == MYSQL_AUTH_SSL_RECV) + break; + if (auth_val == 0) { SESSION *session; @@ -899,6 +919,15 @@ int gw_read_client_event( } break; + case MYSQL_AUTH_SSL_RECV: + { + if(do_ssl_accept(protocol) == 1) + { + protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_DONE; + } + } + break; + case MYSQL_IDLE: { uint8_t* payload = NULL; @@ -1737,3 +1766,49 @@ return_str: return str; } #endif + +int do_ssl_accept(MySQLProtocol* protocol) +{ + int rval,errnum; + char errbuf[2014]; + + switch((rval = SSL_accept(protocol->ssl))) + { + case 0: + errnum = SSL_get_error(protocol->ssl,rval); + ERR_error_string(errnum,errbuf); + skygw_log_write_flush(LOGFILE_ERROR,"SSL_accept: %s",errbuf); + ERR_print_errors_fp(stdout); + ERR_error_string(errnum,errbuf); + printf("%s\n",errbuf); + fflush(stdout); + break; + case 1: + protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_DONE; + rval = 1; + break; + + default: + errnum = SSL_get_error(protocol->ssl,rval); + if(errnum == SSL_ERROR_WANT_READ) + { + /** Not all of the data has been read. Go back to the poll + queue and wait for more.*/ + rval = 0; + } + else + { + ERR_print_errors_fp(stdout); + ERR_error_string(errnum,errbuf); + printf("%s\n",errbuf); + fflush(stdout); + skygw_log_write_flush(LOGFILE_ERROR,"Error: Fatal error in SSL_accept: %s",errbuf); + protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_ERR; + rval = -1; + } + break; + + } + + return rval; +} \ No newline at end of file From 0f814d3e73c8e57f1f78ef91a8999bf5672f099a Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 29 May 2015 13:00:37 +0300 Subject: [PATCH 07/54] Added SSL write and read functions. --- server/core/config.c | 19 +- server/core/dcb.c | 529 +++++++++++++++++- server/include/dcb.h | 8 +- .../include/mysql_client_server_protocol.h | 1 + server/modules/protocol/mysql_client.c | 245 +++++--- server/modules/protocol/mysql_common.c | 5 +- 6 files changed, 726 insertions(+), 81 deletions(-) diff --git a/server/core/config.c b/server/core/config.c index 12d4c099e..640437270 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -452,11 +452,17 @@ hashtable_memory_fns(monitorhash,strdup,NULL,free,NULL); if(ssl) { if(ssl_cert == NULL) - skygw_log_write(LE,"Error: Server certificate missing for service '%s'.",obj->object); + skygw_log_write(LE,"Error: Server certificate missing for service '%s'." + "Please provide the path to the server certificate by adding the ssl_cert= parameter", + obj->object); if(ssl_ca_cert == NULL) - skygw_log_write(LE,"Error: CA Certificate missing for service '%s'.",obj->object); + skygw_log_write(LE,"Error: CA Certificate missing for service '%s'." + "Please provide the path to the certificate authority certificate by adding the ssl_ca_cert= parameter", + obj->object); if(ssl_key == NULL) - skygw_log_write(LE,"Error: Server private key missing for service '%s'.",obj->object); + skygw_log_write(LE,"Error: Server private key missing for service '%s'. " + "Please provide the path to the server certificate key by adding the ssl_key= parameter" + ,obj->object); if(ssl_ca_cert != NULL && ssl_cert != NULL && ssl_key != NULL) { @@ -470,6 +476,13 @@ hashtable_memory_fns(monitorhash,strdup,NULL,free,NULL); serviceSetCertificates(obj->element,ssl_cert,ssl_key,ssl_ca_cert); } } + else + { + /** If SSL was configured wrong, the + * service needs to fail.*/ + skygw_log_write_flush(LE,"Error: Missing SSL certificate paths found in the configuration. " + "This service will not use SSL."); + } } diff --git a/server/core/dcb.c b/server/core/dcb.c index 6717aea41..ba2028de2 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -49,6 +49,7 @@ * backend * 07/05/2014 Mark Riddoch Addition of callback mechanism * 20/06/2014 Mark Riddoch Addition of dcb_clone + * 29/05/2015 Markus Makela Addition of dcb_write_SSL * * @endverbatim */ @@ -880,6 +881,152 @@ return_n: return n; } + +/** + * General purpose read routine to read data from a socket in the + * Descriptor Control Block and append it to a linked list of buffers. + * The list may be empty, in which case *head == NULL + * + * @param dcb The DCB to read from + * @param head Pointer to linked list to append data to + * @return -1 on error, otherwise the number of read bytes on the last + * iteration of while loop. 0 is returned if no data available. + */ +int dcb_read_SSL( + DCB *dcb, + SSL* ssl, + GWBUF **head) +{ + GWBUF *buffer = NULL; + int b; + int rc; + int n; + int nread = 0; + + CHK_DCB(dcb); + + if (dcb->fd <= 0) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Read failed, dcb is %s.", + dcb->fd == DCBFD_CLOSED ? "closed" : "cloned, not readable"))); + n = 0; + goto return_n; + } + + while (true) + { + int bufsize; + + rc = ioctl(dcb->fd, FIONREAD, &b); + + if (rc == -1) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : ioctl FIONREAD for dcb %p in " + "state %s fd %d failed due error %d, %s.", + dcb, + STRDCBSTATE(dcb->state), + dcb->fd, + errno, + strerror(errno)))); + n = -1; + goto return_n; + } + + if (b == 0 && nread == 0) + { + /** Handle closed client socket */ + if (dcb_isclient(dcb)) + { + char c; + int l_errno = 0; + int r = -1; + + /* try to read 1 byte, without consuming the socket buffer */ + r = recv(dcb->fd, &c, sizeof(char), MSG_PEEK); + l_errno = errno; + + if (r <= 0 && + l_errno != EAGAIN && + l_errno != EWOULDBLOCK && + l_errno != 0) + { + n = -1; + goto return_n; + } + } + n = 0; + goto return_n; + } + else if (b == 0) + { + n = 0; + goto return_n; + } + + dcb->last_read = hkheartbeat; + + bufsize = MIN(b, MAX_BUFFER_SIZE); + + if ((buffer = gwbuf_alloc(bufsize)) == NULL) + { + /*< + * This is a fatal error which should cause shutdown. + * Todo shutdown if memory allocation fails. + */ + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Failed to allocate read buffer " + "for dcb %p fd %d, due %d, %s.", + dcb, + dcb->fd, + errno, + strerror(errno)))); + + n = -1; + goto return_n; + } + GW_NOINTR_CALL(n = SSL_read(ssl, GWBUF_DATA(buffer), bufsize); + dcb->stats.n_reads++); + + if (n <= 0) + { + int ssl_errno = ERR_get_error(); + if(ssl_errno != SSL_ERROR_WANT_READ) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Read failed, dcb %p in state " + "%s fd %d: %s.", + dcb, + STRDCBSTATE(dcb->state), + dcb->fd, + ERR_error_string(ssl_errno,NULL)))); + + gwbuf_free(buffer); + goto return_n; + } + } + nread += n; + + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [dcb_read] Read %d bytes from dcb %p in state %s " + "fd %d.", + pthread_self(), + n, + dcb, + STRDCBSTATE(dcb->state), + dcb->fd))); + /*< Append read data to the gwbuf */ + *head = gwbuf_append(*head, buffer); + } /*< while (true) */ +return_n: + return n; +} /** * General purpose routine to write to a DCB * @@ -905,11 +1052,11 @@ int below_water; return 0; } /** - * SESSION_STATE_STOPPING means that one of the backends is closing - * the router session. Some backends may have not completed + * SESSION_STATE_STOPPING means that one of the backends is closing + * the router session. Some backends may have not completed * authentication yet and thus they have no information about router * being closed. Session state is changed to SESSION_STATE_STOPPING - * before router's closeSession is called and that tells that DCB may + * before router's closeSession is called and that tells that DCB may * still be writable. */ if (queue == NULL || @@ -932,9 +1079,9 @@ int below_water; //ss_dassert(false); return 0; } - + spinlock_acquire(&dcb->writeqlock); - + if (dcb->writeq != NULL) { /* @@ -949,7 +1096,7 @@ int below_water; if (queue) { int qlen; - + qlen = gwbuf_length(queue); atomic_add(&dcb->writeqlen, qlen); dcb->writeq = gwbuf_append(dcb->writeq, queue); @@ -998,7 +1145,7 @@ int below_water; w = gw_write(dcb, GWBUF_DATA(queue), qlen); dcb->stats.n_writes++; ); - + if (w < 0) { saved_errno = errno; @@ -1006,7 +1153,7 @@ int below_water; if (LOG_IS_ENABLED(LOGFILE_DEBUG)) { - if (saved_errno == EPIPE) + if (saved_errno == EPIPE) { LOGIF(LD, (skygw_log_write( LOGFILE_DEBUG, @@ -1019,9 +1166,9 @@ int below_water; dcb->fd, saved_errno, strerror(saved_errno)))); - } + } } - + if (LOG_IS_ENABLED(LOGFILE_ERROR)) { if (saved_errno != EPIPE && @@ -1066,7 +1213,7 @@ int below_water; if (queue) { int qlen; - + qlen = gwbuf_length(queue); atomic_add(&dcb->writeqlen, qlen); dcb->stats.n_buffered++; @@ -1086,7 +1233,7 @@ int below_water; if (GWBUF_IS_TYPE_MYSQL(queue)) { uint8_t* data = GWBUF_DATA(queue); - + if (data[4] == 0x01) { dolog = false; @@ -1116,6 +1263,262 @@ int below_water; return 1; } +/** + * General purpose routine to write to an SSL enabled DCB + * + * @param dcb The DCB of the client + * @param ssl The SSL structure for this DCB + * @param queue Queue of buffers to write + * @return 0 on failure, 1 on success + */ +int +dcb_write_SSL(DCB *dcb, SSL* ssl, GWBUF *queue) +{ + int w; + int saved_errno = 0; + int below_water; + + below_water = (dcb->high_water && dcb->writeqlen < dcb->high_water) ? 1 : 0; + ss_dassert(queue != NULL); + + if (dcb->fd <= 0) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Write failed, dcb is %s.", + dcb->fd == DCBFD_CLOSED ? "closed" : "cloned, not writable"))); + return 0; + } + + /** + * SESSION_STATE_STOPPING means that one of the backends is closing + * the router session. Some backends may have not completed + * authentication yet and thus they have no information about router + * being closed. Session state is changed to SESSION_STATE_STOPPING + * before router's closeSession is called and that tells that DCB may + * still be writable. + */ + if (queue == NULL || + (dcb->state != DCB_STATE_ALLOC && + dcb->state != DCB_STATE_POLLING && + dcb->state != DCB_STATE_LISTENING && + dcb->state != DCB_STATE_NOPOLLING && + (dcb->session == NULL || + dcb->session->state != SESSION_STATE_STOPPING))) + { + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [dcb_write] Write aborted to dcb %p because " + "it is in state %s", + pthread_self(), + dcb->stats.n_buffered, + dcb, + STRDCBSTATE(dcb->state), + dcb->fd))); + //ss_dassert(false); + return 0; + } + + spinlock_acquire(&dcb->writeqlock); + + if (dcb->writeq != NULL) + { + /* + * We have some queued data, so add our data to + * the write queue and return. + * The assumption is that there will be an EPOLLOUT + * event to drain what is already queued. We are protected + * by the spinlock, which will also be acquired by the + * the routine that drains the queue data, so we should + * not have a race condition on the event. + */ + if (queue) + { + int qlen; + + qlen = gwbuf_length(queue); + atomic_add(&dcb->writeqlen, qlen); + dcb->writeq = gwbuf_append(dcb->writeq, queue); + dcb->stats.n_buffered++; + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [dcb_write] Append to writequeue. %d writes " + "buffered for dcb %p in state %s fd %d", + pthread_self(), + dcb->stats.n_buffered, + dcb, + STRDCBSTATE(dcb->state), + dcb->fd))); + } + } + else + { + /* + * Loop over the buffer chain that has been passed to us + * from the reading side. + * Send as much of the data in that chain as possible and + * add any balance to the write queue. + */ + while (queue != NULL) + { + int qlen; +#if defined(FAKE_CODE) + if (dcb->dcb_role == DCB_ROLE_REQUEST_HANDLER && + dcb->session != NULL) + { + if (dcb_isclient(dcb) && fail_next_client_fd) { + dcb_fake_write_errno[dcb->fd] = 32; + dcb_fake_write_ev[dcb->fd] = 29; + fail_next_client_fd = false; + } else if (!dcb_isclient(dcb) && + fail_next_backend_fd) + { + dcb_fake_write_errno[dcb->fd] = 32; + dcb_fake_write_ev[dcb->fd] = 29; + fail_next_backend_fd = false; + } + } +#endif /* FAKE_CODE */ + qlen = GWBUF_LENGTH(queue); + GW_NOINTR_CALL( + w = gw_write_SSL(ssl, GWBUF_DATA(queue), qlen); + dcb->stats.n_writes++; + ); + + if (w < 0) + { + int ssl_errno = ERR_get_error(); + + if (LOG_IS_ENABLED(LOGFILE_DEBUG)) + { + switch(ssl_errno) + { + case SSL_ERROR_WANT_READ: + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [dcb_write] Write to dcb " + "%p in state %s fd %d failed " + "due error SSL_ERROR_WANT_READ", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state), + dcb->fd))); + break; + case SSL_ERROR_WANT_WRITE: + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [dcb_write] Write to dcb " + "%p in state %s fd %d failed " + "due error SSL_ERROR_WANT_WRITE", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state), + dcb->fd))); + break; + default: + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [dcb_write] Write to dcb " + "%p in state %s fd %d failed " + "due error %d", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state), + dcb->fd,ssl_errno))); + } + } + + if (LOG_IS_ENABLED(LOGFILE_ERROR)) + { + if (ssl_errno != 0) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Write to dcb %p in " + "state %s fd %d failed due " + "SSL error %d", + dcb, + STRDCBSTATE(dcb->state), + dcb->fd, + ssl_errno))); + } + } + break; + } + /* + * Pull the number of bytes we have written from + * queue with have. + */ + queue = gwbuf_consume(queue, w); + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [dcb_write] Wrote %d Bytes to dcb %p in " + "state %s fd %d", + pthread_self(), + w, + dcb, + STRDCBSTATE(dcb->state), + dcb->fd))); + } /*< while (queue != NULL) */ + /*< + * What wasn't successfully written is stored to write queue + * for suspended write. + */ + dcb->writeq = queue; + + if (queue) + { + int qlen; + + qlen = gwbuf_length(queue); + atomic_add(&dcb->writeqlen, qlen); + dcb->stats.n_buffered++; + } + } /* if (dcb->writeq) */ + + if (saved_errno != 0 && + queue != NULL && + saved_errno != EAGAIN && + saved_errno != EWOULDBLOCK) + { + bool dolog = true; + + /** + * Do not log if writing COM_QUIT to backend failed. + */ + if (GWBUF_IS_TYPE_MYSQL(queue)) + { + uint8_t* data = GWBUF_DATA(queue); + + if (data[4] == 0x01) + { + dolog = false; + } + } + if (dolog) + { + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [dcb_write] Writing to %s socket failed due %d, %s.", + pthread_self(), + dcb_isclient(dcb) ? "client" : "backend server", + saved_errno, + strerror(saved_errno)))); + } + spinlock_release(&dcb->writeqlock); + return 0; + } + spinlock_release(&dcb->writeqlock); + + if (dcb->high_water && dcb->writeqlen > dcb->high_water && below_water) + { + atomic_add(&dcb->stats.n_high_water, 1); + dcb_call_callback(dcb, DCB_REASON_HIGH_WATER); + } + + return 1; +} + /** * Drain the write queue of a DCB. This is called as part of the EPOLLOUT handling * of a socket and will try to send any buffered data from the write queue @@ -1208,6 +1611,85 @@ int above_water; return n; } + +/** + * Drain the write queue of a DCB. This is called as part of the EPOLLOUT handling + * of a socket and will try to send any buffered data from the write queue + * up until the point the write would block. + * + * @param dcb DCB to drain the write queue of + * @return The number of bytes written + */ +int +dcb_drain_writeq_SSL(DCB *dcb, SSL* ssl) +{ + int n = 0; + int w; + int saved_errno = 0; + int above_water; + + above_water = (dcb->low_water && dcb->writeqlen > dcb->low_water) ? 1 : 0; + + spinlock_acquire(&dcb->writeqlock); + + if (dcb->writeq) + { + int len; + /* + * Loop over the buffer chain in the pending writeq + * Send as much of the data in that chain as possible and + * leave any balance on the write queue. + */ + while (dcb->writeq != NULL) + { + len = GWBUF_LENGTH(dcb->writeq); + GW_NOINTR_CALL(w = gw_write_SSL(ssl, GWBUF_DATA(dcb->writeq), len);); + + if (w < 0) + { + int ssl_errno = ERR_get_error(); + + if(ssl_errno == SSL_ERROR_WANT_WRITE || + ssl_errno == SSL_ERROR_WANT_ACCEPT || + ssl_errno == SSL_ERROR_WANT_READ) + { + break; + } + + skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Write to dcb %p " + "in state %s fd %d failed: %s", + dcb, + STRDCBSTATE(dcb->state), + dcb->fd, + ERR_error_string(ssl_errno,NULL)); + break; + } + /* + * Pull the number of bytes we have written from + * queue with have. + */ + dcb->writeq = gwbuf_consume(dcb->writeq, w); + n += w; + } + } + spinlock_release(&dcb->writeqlock); + atomic_add(&dcb->writeqlen, -n); + + /* The write queue has drained, potentially need to call a callback function */ + if (dcb->writeq == NULL) + dcb_call_callback(dcb, DCB_REASON_DRAINED); + + if (above_water && dcb->writeqlen < dcb->low_water) + { + atomic_add(&dcb->stats.n_low_water, 1); + dcb_call_callback(dcb, DCB_REASON_LOW_WATER); + } + + return n; +} + /** * Removes dcb from poll set, and adds it to zombies list. As a consequense, * dcb first moves to DCB_STATE_NOPOLLING, and then to DCB_STATE_ZOMBIE state. @@ -1792,6 +2274,29 @@ static bool dcb_set_state_nomutex( return succp; } +/** + * Write data to a DCB + * + * @param ssl The SSL to write the buffer to + * @param buf Buffer to write + * @param nbytes Number of bytes to write + * @return Number of written bytes + */ +int +gw_write_SSL(SSL* ssl, const void *buf, size_t nbytes) +{ + int w = 0; + int fd = SSL_get_fd(ssl); + + if (fd > 0) + { + w = SSL_write(ssl, buf, nbytes); + } + return w; +} + + + /** * Write data to a DCB * diff --git a/server/include/dcb.h b/server/include/dcb.h index 7eedfbec3..38ebdc299 100644 --- a/server/include/dcb.h +++ b/server/include/dcb.h @@ -23,6 +23,9 @@ #include #include #include +#include +#include +#include #define ERRHANDLE @@ -337,7 +340,10 @@ bool dcb_set_state(DCB* dcb, dcb_state_t new_state, dcb_state_t* old_state); void dcb_call_foreach (struct server* server, DCB_REASON reason); size_t dcb_get_session_id(DCB* dcb); bool dcb_get_ses_log_info(DCB* dcb, size_t* sesid, int* enabled_logs); - +int gw_write_SSL(SSL* ssl, const void *buf, size_t nbytes); +int dcb_write_SSL(DCB *dcb, SSL* ssl, GWBUF *queue); +int dcb_read_SSL(DCB *dcb,SSL* ssl,GWBUF **head); +int dcb_drain_writeq_SSL(DCB *dcb, SSL* ssl); /** diff --git a/server/modules/include/mysql_client_server_protocol.h b/server/modules/include/mysql_client_server_protocol.h index 87dbc50ee..6c841d9c9 100644 --- a/server/modules/include/mysql_client_server_protocol.h +++ b/server/modules/include/mysql_client_server_protocol.h @@ -297,6 +297,7 @@ typedef struct { * handshake */ unsigned int charset; /*< MySQL character set at connect time */ SSL* ssl; /*< SSL struct for client connection */ + bool use_ssl; #if defined(SS_DEBUG) skygw_chk_t protocol_chk_tail; #endif diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index 387b59aa0..f1aa0aa94 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -37,7 +37,7 @@ * 09/09/2014 Massimiliano Pinto Added: 777 permission for socket path * 13/10/2014 Massimiliano Pinto Added: dbname authentication check * 10/11/2014 Massimiliano Pinto Added: client charset added to protocol struct - * + * 29/05/2015 Markus Makela Added SSL support */ #include #include @@ -69,7 +69,9 @@ static int gw_MySQLWrite_client(DCB *dcb, GWBUF *queue); static int gw_error_client_event(DCB *dcb); static int gw_client_close(DCB *dcb); static int gw_client_hangup_event(DCB *dcb); - +int gw_read_client_event_SSL(DCB* dcb); +int gw_MySQLWrite_client_SSL(DCB *dcb, GWBUF *queue); +int gw_write_client_event_SSL(DCB *dcb); int mysql_send_ok(DCB *dcb, int packet_number, int in_affected_rows, const char* mysql_message); int MySQLSendHandshake(DCB* dcb); static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue); @@ -464,6 +466,8 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { &protocol->client_capabilities); */ + if(protocol->protocol_auth_state == MYSQL_AUTH_SSL_EXCHANGE_DONE) + goto ssl_hs_done; ssl = protocol->client_capabilities & GW_MYSQL_CAPABILITIES_SSL; @@ -497,48 +501,19 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { protocol->ssl = SSL_new(protocol->owner_dcb->service->ctx); SSL_set_fd(protocol->ssl,dcb->fd); protocol->protocol_auth_state = MYSQL_AUTH_SSL_REQ; - printf("%s\n",SSL_get_version(protocol->ssl)); - int errnum,rval; - char errbuf[1024]; - switch((rval = SSL_accept(protocol->ssl))) + if(do_ssl_accept(protocol) < 0) { - case 0: - errnum = SSL_get_error(protocol->ssl,rval); - ERR_error_string(errnum,errbuf); - skygw_log_write_flush(LOGFILE_ERROR,"SSL_accept: %s",errbuf); - ERR_print_errors_fp(stdout); - ERR_error_string(errnum,errbuf); - printf("%s\n",errbuf); - fflush(stdout); - break; - case 1: - protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_DONE; - break; - - default: - errnum = SSL_get_error(protocol->ssl,rval); - if(errnum == SSL_ERROR_WANT_READ) - { - /** Not all of the data has been read. Go back to the poll - queue and wait for more.*/ - protocol->protocol_auth_state = MYSQL_AUTH_SSL_RECV; - return 0; - } - else - { - ERR_print_errors_fp(stdout); - ERR_error_string(errnum,errbuf); - printf("%s\n",errbuf); - fflush(stdout); - skygw_log_write_flush(LOGFILE_ERROR,"Error: Fatal error in SSL_accept: %s",errbuf); - protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_ERR; - } - break; - + return 1; + } + else + { + return 0; } } + ssl_hs_done: + username = get_username_from_auth(username, client_auth_packet); if (username == NULL) @@ -645,6 +620,65 @@ gw_MySQLWrite_client(DCB *dcb, GWBUF *queue) return dcb_write(dcb, queue); } + +/** + * Write function for client DCB: writes data from MaxScale to Client + * + * @param dcb The DCB of the client + * @param queue Queue of buffers to write + */ +int +gw_MySQLWrite_client_SSL(DCB *dcb, GWBUF *queue) +{ + MySQLProtocol *protocol = NULL; + CHK_DCB(dcb); + protocol = DCB_PROTOCOL(dcb, MySQLProtocol); + CHK_PROTOCOL(protocol); + return dcb_write_SSL(dcb, protocol->ssl, queue); +} + + +int gw_read_client_event_SSL( +DCB* dcb) +{ + SESSION *session = NULL; + ROUTER_OBJECT *router = NULL; + ROUTER *router_instance = NULL; + void *rsession = NULL; + MySQLProtocol *protocol = NULL; + GWBUF *read_buffer = NULL; + int rc = 0; + int nbytes_read = 0; + uint8_t cap = 0; + bool stmt_input = false; /*< router input type */ + + CHK_DCB(dcb); + protocol = DCB_PROTOCOL(dcb, MySQLProtocol); + CHK_PROTOCOL(protocol); + + + if(protocol->protocol_auth_state == MYSQL_AUTH_SSL_REQ) + { + if(do_ssl_accept(protocol) == 1) + { + spinlock_acquire(&protocol->protocol_lock); + protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_DONE; + spinlock_release(&protocol->protocol_lock); + + spinlock_acquire(&dcb->authlock); + dcb->func.read = gw_read_client_event_SSL; + dcb->func.write = gw_MySQLWrite_client_SSL; + dcb->func.write_ready = gw_write_client_event; + spinlock_release(&dcb->authlock); + } + goto return_rc; + } + + return_rc: + + return rc; +} + /** * Client read event triggered by EPOLLIN * @@ -668,13 +702,36 @@ int gw_read_client_event( CHK_DCB(dcb); protocol = DCB_PROTOCOL(dcb, MySQLProtocol); CHK_PROTOCOL(protocol); - if(protocol->protocol_auth_state == MYSQL_AUTH_SSL_RECV) + + /** Let the OpenSSL API do the reading from the socket */ + if(protocol->protocol_auth_state == MYSQL_AUTH_SSL_REQ) { - goto do_auth; + if(do_ssl_accept(protocol) == 1) + { + spinlock_acquire(&protocol->protocol_lock); + protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_DONE; + protocol->use_ssl = true; + spinlock_release(&protocol->protocol_lock); + + spinlock_acquire(&dcb->authlock); + //dcb->func.read = gw_read_client_event_SSL; + //dcb->func.write = gw_MySQLWrite_client_SSL; + //dcb->func.write_ready = gw_write_client_event_SSL; + spinlock_release(&dcb->authlock); + } + goto return_rc; } - rc = dcb_read(dcb, &read_buffer); - - + + + if(protocol->use_ssl) + { + rc = dcb_read_SSL(dcb,protocol->ssl, &read_buffer); + } + else + { + rc = dcb_read(dcb, &read_buffer); + } + if (rc < 0) { dcb_close(dcb); @@ -811,7 +868,7 @@ int gw_read_client_event( } } - do_auth: + /** * Now there should be at least one complete mysql packet in read_buffer. */ @@ -823,7 +880,7 @@ int gw_read_client_event( auth_val = gw_mysql_do_authentication(dcb, read_buffer); - if(protocol->protocol_auth_state == MYSQL_AUTH_SSL_RECV) + if(protocol->protocol_auth_state == MYSQL_AUTH_SSL_REQ) break; if (auth_val == 0) @@ -919,12 +976,9 @@ int gw_read_client_event( } break; - case MYSQL_AUTH_SSL_RECV: + case MYSQL_AUTH_SSL_EXCHANGE_DONE: { - if(do_ssl_accept(protocol) == 1) - { - protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_DONE; - } + protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_DONE; } break; @@ -1047,6 +1101,64 @@ return_rc: return rc; } + +/////////////////////////////////////////////// +// client write event to Client triggered by EPOLLOUT +////////////////////////////////////////////// +/** + * @node Client's fd became writable, and EPOLLOUT event + * arrived. As a consequence, client input buffer (writeq) is flushed. + * + * Parameters: + * @param dcb - in, use + * client dcb + * + * @return constantly 1 + * + * + * @details (write detailed description here) + * + */ +int gw_write_client_event(DCB *dcb) +{ + MySQLProtocol *protocol = NULL; + + CHK_DCB(dcb); + + ss_dassert(dcb->state != DCB_STATE_DISCONNECTED); + + if (dcb == NULL) { + goto return_1; + } + + if (dcb->state == DCB_STATE_DISCONNECTED) { + goto return_1; + } + + if (dcb->protocol == NULL) { + goto return_1; + } + protocol = (MySQLProtocol *)dcb->protocol; + CHK_PROTOCOL(protocol); + + if (protocol->protocol_auth_state == MYSQL_IDLE) + { + dcb_drain_writeq(dcb); + goto return_1; + } + +return_1: +#if defined(SS_DEBUG) + if (dcb->state == DCB_STATE_POLLING || + dcb->state == DCB_STATE_NOPOLLING || + dcb->state == DCB_STATE_ZOMBIE) + { + CHK_PROTOCOL(protocol); + } +#endif + return 1; +} + /////////////////////////////////////////////// // client write event to Client triggered by EPOLLOUT ////////////////////////////////////////////// @@ -1064,7 +1176,7 @@ return_rc: * @details (write detailed description here) * */ -int gw_write_client_event(DCB *dcb) +int gw_write_client_event_SSL(DCB *dcb) { MySQLProtocol *protocol = NULL; @@ -1088,7 +1200,7 @@ int gw_write_client_event(DCB *dcb) if (protocol->protocol_auth_state == MYSQL_IDLE) { - dcb_drain_writeq(dcb); + dcb_drain_writeq_SSL(dcb,protocol->ssl); goto return_1; } @@ -1776,16 +1888,17 @@ int do_ssl_accept(MySQLProtocol* protocol) { case 0: errnum = SSL_get_error(protocol->ssl,rval); - ERR_error_string(errnum,errbuf); - skygw_log_write_flush(LOGFILE_ERROR,"SSL_accept: %s",errbuf); - ERR_print_errors_fp(stdout); - ERR_error_string(errnum,errbuf); - printf("%s\n",errbuf); - fflush(stdout); + skygw_log_write_flush(LT,"SSL_accept ongoing for %s@%s", + protocol->owner_dcb->user, + protocol->owner_dcb->remote); break; case 1: protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_DONE; rval = 1; + protocol->use_ssl = true; + skygw_log_write_flush(LT,"SSL_accept done for %s@%s", + protocol->owner_dcb->user, + protocol->owner_dcb->remote); break; default: @@ -1795,14 +1908,18 @@ int do_ssl_accept(MySQLProtocol* protocol) /** Not all of the data has been read. Go back to the poll queue and wait for more.*/ rval = 0; + protocol->protocol_auth_state = MYSQL_AUTH_SSL_REQ; + skygw_log_write_flush(LT,"SSL_accept partially done for %s@%s", + protocol->owner_dcb->user, + protocol->owner_dcb->remote); } else { - ERR_print_errors_fp(stdout); - ERR_error_string(errnum,errbuf); - printf("%s\n",errbuf); - fflush(stdout); - skygw_log_write_flush(LOGFILE_ERROR,"Error: Fatal error in SSL_accept: %s",errbuf); + skygw_log_write_flush(LE, + "Error: Fatal error in SSL_accept for %s@%s: %s", + protocol->owner_dcb->user, + protocol->owner_dcb->remote, + ERR_error_string(errnum,NULL)); protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_ERR; rval = -1; } diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index ffd72c034..654d24692 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -137,7 +137,10 @@ void mysql_protocol_done ( goto retblock; } scmd = p->protocol_cmd_history; - + if(p->ssl) + { + SSL_free(p->ssl); + } while (scmd != NULL) { scmd2 = scmd->scom_next; From a2768955e75134914198bf956dc1a44cefd03b75 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 1 Jun 2015 13:50:22 +0300 Subject: [PATCH 08/54] SSL handshake now successfully completes when a client connects with SSL enabled. --- Documentation/Reference/MaxScale-and-SSL.md | 13 + server/core/dcb.c | 24 +- server/core/gateway.c | 37 +- server/core/service.c | 2 +- .../include/mysql_client_server_protocol.h | 9 +- server/modules/protocol/mysql_client.c | 347 ++++++++++-------- 6 files changed, 253 insertions(+), 179 deletions(-) create mode 100644 Documentation/Reference/MaxScale-and-SSL.md diff --git a/Documentation/Reference/MaxScale-and-SSL.md b/Documentation/Reference/MaxScale-and-SSL.md new file mode 100644 index 000000000..4c583793b --- /dev/null +++ b/Documentation/Reference/MaxScale-and-SSL.md @@ -0,0 +1,13 @@ +# MaxScale and SSL + +MaxScale supports client side SSL connections. Enabling is done on a per service basis and each service has its own set of certificates. + +## SSL Options + +Here are the options which relate to SSL and certificates. +Parameter|Values|Description +---------------------------- +ssl | disabled, enabled, required |`disable` disables SSL, `enabled` enables SSL for client connections but still allows non-SSL connections and `required` requires SSL from all client connections. With the `required` option, client connections that do not use SSL will be rejected. +ssl_cert | |Path to server certificate +ssl_key | |Path to server private key +ssl_ca_cert | |Path to Certificate Authority file diff --git a/server/core/dcb.c b/server/core/dcb.c index ba2028de2..562929774 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -941,18 +941,12 @@ int dcb_read_SSL( /** Handle closed client socket */ if (dcb_isclient(dcb)) { - char c; - int l_errno = 0; + char c = 0; int r = -1; /* try to read 1 byte, without consuming the socket buffer */ - r = recv(dcb->fd, &c, sizeof(char), MSG_PEEK); - l_errno = errno; - - if (r <= 0 && - l_errno != EAGAIN && - l_errno != EWOULDBLOCK && - l_errno != 0) + r = SSL_peek(ssl, &c, sizeof(char)); + if (r <= 0) { n = -1; goto return_n; @@ -989,13 +983,15 @@ int dcb_read_SSL( n = -1; goto return_n; } - GW_NOINTR_CALL(n = SSL_read(ssl, GWBUF_DATA(buffer), bufsize); - dcb->stats.n_reads++); + n = SSL_read(ssl, GWBUF_DATA(buffer), bufsize); + dcb->stats.n_reads++; + int ssl_errno = 0; if (n <= 0) { - int ssl_errno = ERR_get_error(); - if(ssl_errno != SSL_ERROR_WANT_READ) + ssl_errno = ERR_get_error(); + + if(ssl_errno != SSL_ERROR_WANT_READ && ssl_errno != SSL_ERROR_NONE) { LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, @@ -1023,6 +1019,8 @@ int dcb_read_SSL( dcb->fd))); /*< Append read data to the gwbuf */ *head = gwbuf_append(*head, buffer); + if(ssl_errno == SSL_ERROR_WANT_READ || ssl_errno == SSL_ERROR_NONE) + break; } /*< while (true) */ return_n: return n; diff --git a/server/core/gateway.c b/server/core/gateway.c index 9845713e1..0b07f507b 100644 --- a/server/core/gateway.c +++ b/server/core/gateway.c @@ -196,7 +196,9 @@ static bool resolve_maxscale_conf_fname( static char* check_dir_access(char* dirname,bool,bool); static int set_user(); - +static void maxscale_ssl_lock(int mode,int n,const char* file, int line); +static unsigned long maxscale_ssl_id(); +static SPINLOCK* ssl_locks; /** * Handler for SIGHUP signal. Reload the configuration for the * gateway. @@ -1370,7 +1372,23 @@ int main(int argc, char **argv) rc = MAXSCALE_INTERNALERROR; goto return_main; } + + /** OpenSSL initialization */ + SSL_library_init(); + SSL_load_error_strings(); + int n_locks = CRYPTO_num_locks(); + if((ssl_locks = malloc(n_locks*sizeof(SPINLOCK))) == NULL) + { + rc = MAXSCALE_INTERNALERROR; + goto return_main; + } + + for(i = 0;ictx,SSL_VERIFY_PEER,NULL); /* Set the verification depth to 1 */ - SSL_CTX_set_verify_depth(service->ctx,10); + SSL_CTX_set_verify_depth(service->ctx,1); service->ssl_init_done = true; } return 0; diff --git a/server/modules/include/mysql_client_server_protocol.h b/server/modules/include/mysql_client_server_protocol.h index 6c841d9c9..94db6e664 100644 --- a/server/modules/include/mysql_client_server_protocol.h +++ b/server/modules/include/mysql_client_server_protocol.h @@ -99,10 +99,11 @@ typedef enum { MYSQL_AUTH_RECV, MYSQL_AUTH_FAILED, MYSQL_HANDSHAKE_FAILED, - MYSQL_AUTH_SSL_REQ, /*< client requested SSL */ - MYSQL_AUTH_SSL_EXCHANGE_DONE, /*< SSL handshake done */ - MYSQL_AUTH_SSL_EXCHANGE_ERR, /*< SSL handshake failure */ - MYSQL_AUTH_SSL_RECV, /*< */ + MYSQL_AUTH_SSL_REQ, /*< client requested SSL but SSL_accept hasn't beed called */ + MYSQL_AUTH_SSL_HANDSHAKE_DONE, /*< SSL handshake has been fully completed */ + MYSQL_AUTH_SSL_HANDSHAKE_FAILED, /*< SSL handshake failed for any reason */ + MYSQL_AUTH_SSL_HANDSHAKE_ONGOING, /*< SSL_accept has been called but the + * SSL handshake hasn't been completed */ MYSQL_IDLE } mysql_auth_state_t; diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index f1aa0aa94..8ee4b5687 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -117,8 +117,6 @@ version() void ModuleInit() { - SSL_library_init(); - SSL_load_error_strings(); } /** @@ -466,7 +464,7 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { &protocol->client_capabilities); */ - if(protocol->protocol_auth_state == MYSQL_AUTH_SSL_EXCHANGE_DONE) + if(protocol->protocol_auth_state == MYSQL_AUTH_SSL_HANDSHAKE_DONE) goto ssl_hs_done; ssl = protocol->client_capabilities & GW_MYSQL_CAPABILITIES_SSL; @@ -637,48 +635,6 @@ gw_MySQLWrite_client_SSL(DCB *dcb, GWBUF *queue) return dcb_write_SSL(dcb, protocol->ssl, queue); } - -int gw_read_client_event_SSL( -DCB* dcb) -{ - SESSION *session = NULL; - ROUTER_OBJECT *router = NULL; - ROUTER *router_instance = NULL; - void *rsession = NULL; - MySQLProtocol *protocol = NULL; - GWBUF *read_buffer = NULL; - int rc = 0; - int nbytes_read = 0; - uint8_t cap = 0; - bool stmt_input = false; /*< router input type */ - - CHK_DCB(dcb); - protocol = DCB_PROTOCOL(dcb, MySQLProtocol); - CHK_PROTOCOL(protocol); - - - if(protocol->protocol_auth_state == MYSQL_AUTH_SSL_REQ) - { - if(do_ssl_accept(protocol) == 1) - { - spinlock_acquire(&protocol->protocol_lock); - protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_DONE; - spinlock_release(&protocol->protocol_lock); - - spinlock_acquire(&dcb->authlock); - dcb->func.read = gw_read_client_event_SSL; - dcb->func.write = gw_MySQLWrite_client_SSL; - dcb->func.write_ready = gw_write_client_event; - spinlock_release(&dcb->authlock); - } - goto return_rc; - } - - return_rc: - - return rc; -} - /** * Client read event triggered by EPOLLIN * @@ -703,26 +659,26 @@ int gw_read_client_event( protocol = DCB_PROTOCOL(dcb, MySQLProtocol); CHK_PROTOCOL(protocol); - /** Let the OpenSSL API do the reading from the socket */ - if(protocol->protocol_auth_state == MYSQL_AUTH_SSL_REQ) + if(protocol->protocol_auth_state == MYSQL_AUTH_SSL_HANDSHAKE_ONGOING || + protocol->protocol_auth_state == MYSQL_AUTH_SSL_REQ) { - if(do_ssl_accept(protocol) == 1) + switch(do_ssl_accept(protocol)) { - spinlock_acquire(&protocol->protocol_lock); - protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_DONE; - protocol->use_ssl = true; - spinlock_release(&protocol->protocol_lock); - - spinlock_acquire(&dcb->authlock); - //dcb->func.read = gw_read_client_event_SSL; - //dcb->func.write = gw_MySQLWrite_client_SSL; - //dcb->func.write_ready = gw_write_client_event_SSL; - spinlock_release(&dcb->authlock); + case 0: + return 0; + break; + case 1: + return 0; + break; + case -1: + return 1; + break; + default: + return 1; + break; } - goto return_rc; } - if(protocol->use_ssl) { rc = dcb_read_SSL(dcb,protocol->ssl, &read_buffer); @@ -880,8 +836,13 @@ int gw_read_client_event( auth_val = gw_mysql_do_authentication(dcb, read_buffer); - if(protocol->protocol_auth_state == MYSQL_AUTH_SSL_REQ) + if(protocol->protocol_auth_state == MYSQL_AUTH_SSL_REQ || + protocol->protocol_auth_state == MYSQL_AUTH_SSL_HANDSHAKE_ONGOING || + protocol->protocol_auth_state == MYSQL_AUTH_SSL_HANDSHAKE_DONE || + protocol->protocol_auth_state == MYSQL_AUTH_SSL_HANDSHAKE_FAILED) + { break; + } if (auth_val == 0) { @@ -976,9 +937,103 @@ int gw_read_client_event( } break; - case MYSQL_AUTH_SSL_EXCHANGE_DONE: + case MYSQL_AUTH_SSL_HANDSHAKE_DONE: { - protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_DONE; + int auth_val; + + auth_val = gw_mysql_do_authentication(dcb, read_buffer); + + + if (auth_val == 0) + { + SESSION *session; + + protocol->protocol_auth_state = MYSQL_AUTH_RECV; + /** + * Create session, and a router session for it. + * If successful, there will be backend connection(s) + * after this point. + */ + session = session_alloc(dcb->service, dcb); + + if (session != NULL) + { + CHK_SESSION(session); + ss_dassert(session->state != SESSION_STATE_ALLOC); + + protocol->protocol_auth_state = MYSQL_IDLE; + /** + * Send an AUTH_OK packet to the client, + * packet sequence is # 2 + */ + mysql_send_ok(dcb, 2, 0, NULL); + } + else + { + protocol->protocol_auth_state = MYSQL_AUTH_FAILED; + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [gw_read_client_event] session " + "creation failed. fd %d, " + "state = MYSQL_AUTH_FAILED.", + pthread_self(), + protocol->owner_dcb->fd))); + + /** Send ERR 1045 to client */ + mysql_send_auth_error( + dcb, + 2, + 0, + "failed to create new session"); + + dcb_close(dcb); + } + } + else + { + char* fail_str = NULL; + + protocol->protocol_auth_state = MYSQL_AUTH_FAILED; + + if (auth_val == 2) { + /** Send error 1049 to client */ + int message_len = 25 + MYSQL_DATABASE_MAXLEN; + + fail_str = calloc(1, message_len+1); + snprintf(fail_str, message_len, "Unknown database '%s'", + (char*)((MYSQL_session *)dcb->data)->db); + + modutil_send_mysql_err_packet(dcb, 2, 0, 1049, "42000", fail_str); + } else { + /** Send error 1045 to client */ + fail_str = create_auth_fail_str((char *)((MYSQL_session *)dcb->data)->user, + dcb->remote, + (char*)((MYSQL_session *)dcb->data)->client_sha1, + (char*)((MYSQL_session *)dcb->data)->db); + modutil_send_mysql_err_packet(dcb, 2, 0, 1045, "28000", fail_str); + } + if (fail_str) + free(fail_str); + + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [gw_read_client_event] after " + "gw_mysql_do_authentication, fd %d, " + "state = MYSQL_AUTH_FAILED.", + protocol->owner_dcb->fd, + pthread_self()))); + /** + * Release MYSQL_session since it is not used anymore. + */ + if (!DCB_IS_CLONE(dcb)) + { + free(dcb->data); + } + dcb->data = NULL; + + dcb_close(dcb); + } + read_buffer = gwbuf_consume(read_buffer, nbytes_read); } break; @@ -1821,111 +1876,83 @@ return_rc: return rc; } - /** - * Create a character array including the query string. - * GWBUF given as input includes either one complete or partial query. - * Length of buffer is at most the query length+4 (length of packet header). + * Do the SSL authentication handshake. + * This functions + * @param protocol + * @return */ -#if defined(NOT_USED) -static char* gw_get_or_create_querystr ( - void* data, - bool* new_allocation) -{ - GWBUF* buf = (GWBUF *)data; - size_t buflen; /*< first gw buffer data length */ - size_t packetlen; /*< length of mysql packet */ - size_t querylen; /*< total buffer length- */ - size_t nbytes_copied; - char* startpos; /*< first byte of query in gw buffer */ - char* str; /*< resulting query string */ - - CHK_GWBUF(buf); - packetlen = MYSQL_GET_PACKET_LEN((uint8_t *)GWBUF_DATA(buf)); - str = (char *)malloc(packetlen); /*< leave space for terminating null */ - - if (str == NULL) - { - goto return_str; - } - *new_allocation = true; - /** - * First buffer includes 4 bytes header and a type indicator byte. - */ - buflen = GWBUF_LENGTH(buf); - querylen = packetlen-1; - ss_dassert(buflen<=querylen+5); /*< 5 == header+type indicator */ - startpos = (char *)GWBUF_DATA(buf)+5; - nbytes_copied = MIN(querylen, buflen-5); - memcpy(str, startpos, nbytes_copied); - memset(&str[querylen-1], 0, 1); - buf = gwbuf_consume(buf, querylen-1); - - /** - * In case of multi-packet statement whole buffer consists of query - * string. - */ - while (buf != NULL) - { - buflen = GWBUF_LENGTH(buf); - memcpy(str+nbytes_copied, GWBUF_DATA(buf), buflen); - nbytes_copied += buflen; - buf = gwbuf_consume(buf, buflen); - } - ss_dassert(str[querylen-1] == 0); - -return_str: - return str; -} -#endif - int do_ssl_accept(MySQLProtocol* protocol) { int rval,errnum; char errbuf[2014]; - - switch((rval = SSL_accept(protocol->ssl))) - { - case 0: - errnum = SSL_get_error(protocol->ssl,rval); - skygw_log_write_flush(LT,"SSL_accept ongoing for %s@%s", - protocol->owner_dcb->user, - protocol->owner_dcb->remote); - break; - case 1: - protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_DONE; - rval = 1; - protocol->use_ssl = true; - skygw_log_write_flush(LT,"SSL_accept done for %s@%s", - protocol->owner_dcb->user, - protocol->owner_dcb->remote); - break; + DCB* dcb; - default: - errnum = SSL_get_error(protocol->ssl,rval); - if(errnum == SSL_ERROR_WANT_READ) - { - /** Not all of the data has been read. Go back to the poll - queue and wait for more.*/ - rval = 0; - protocol->protocol_auth_state = MYSQL_AUTH_SSL_REQ; - skygw_log_write_flush(LT,"SSL_accept partially done for %s@%s", - protocol->owner_dcb->user, - protocol->owner_dcb->remote); - } - else - { - skygw_log_write_flush(LE, - "Error: Fatal error in SSL_accept for %s@%s: %s", - protocol->owner_dcb->user, - protocol->owner_dcb->remote, - ERR_error_string(errnum,NULL)); - protocol->protocol_auth_state = MYSQL_AUTH_SSL_EXCHANGE_ERR; - rval = -1; - } - break; + rval = SSL_accept(protocol->ssl); - } + switch(rval) + { + case 0: + errnum = SSL_get_error(protocol->ssl,rval); + skygw_log_write_flush(LT,"SSL_accept shutdown for %s@%s", + protocol->owner_dcb->user, + protocol->owner_dcb->remote); + return -1; + break; + case 1: + spinlock_acquire(&protocol->protocol_lock); + dcb = protocol->owner_dcb; + protocol->protocol_auth_state = MYSQL_AUTH_SSL_HANDSHAKE_DONE; + protocol->use_ssl = true; + spinlock_release(&protocol->protocol_lock); + + spinlock_acquire(&dcb->authlock); + dcb->func.write = gw_MySQLWrite_client_SSL; + dcb->func.write_ready = gw_write_client_event_SSL; + spinlock_release(&dcb->authlock); + + rval = 1; + + skygw_log_write_flush(LT,"SSL_accept done for %s@%s", + protocol->owner_dcb->user, + protocol->owner_dcb->remote); + break; + + case -1: + errnum = SSL_get_error(protocol->ssl,rval); + + if(errnum == SSL_ERROR_WANT_READ || errnum == SSL_ERROR_WANT_WRITE || + errnum == SSL_ERROR_WANT_X509_LOOKUP) + { + /** Not all of the data has been read. Go back to the poll + queue and wait for more.*/ + + rval = 0; + skygw_log_write_flush(LT,"SSL_accept ongoing for %s@%s", + protocol->owner_dcb->user, + protocol->owner_dcb->remote); + } + else + { + spinlock_acquire(&protocol->protocol_lock); + protocol->protocol_auth_state = MYSQL_AUTH_SSL_HANDSHAKE_FAILED; + spinlock_release(&protocol->protocol_lock); + rval = -1; + + skygw_log_write_flush(LE, + "Error: Fatal error in SSL_accept for %s@%s: %s", + protocol->owner_dcb->user, + protocol->owner_dcb->remote, + ERR_error_string(errnum,NULL)); + } + break; + + default: + skygw_log_write_flush(LE, + "Error: Fatal error in SSL_accept, returned value was %d.", + rval); + break; + } return rval; } \ No newline at end of file From 4d5291c26329ebee8dd04a51bf0468a5f162727a Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 1 Jun 2015 13:58:26 +0300 Subject: [PATCH 09/54] Fixed wrong packet sequence number causing a disconnect from the client. --- server/modules/protocol/mysql_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index 8ee4b5687..2d2633b90 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -966,7 +966,7 @@ int gw_read_client_event( * Send an AUTH_OK packet to the client, * packet sequence is # 2 */ - mysql_send_ok(dcb, 2, 0, NULL); + mysql_send_ok(dcb, 3, 0, NULL); } else { From d7232d8b6ecc2fab121b1326a7f489b84bb0722d Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 1 Jun 2015 20:51:26 +0300 Subject: [PATCH 10/54] Moved SSL structure to the DCB instead of the MySQL protocol. This allows for non-MySQL SSL connections. --- server/core/dcb.c | 139 ++++++++++++++++-- server/core/service.c | 6 +- server/include/dcb.h | 9 +- .../include/mysql_client_server_protocol.h | 1 - server/modules/protocol/mysql_client.c | 64 +++----- server/modules/protocol/mysql_common.c | 5 +- 6 files changed, 164 insertions(+), 60 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 562929774..2948dae2e 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -73,6 +73,8 @@ #include #include +#include "mysql_client_server_protocol.h" + /** Defined in log_manager.cc */ extern int lm_enabled_logfiles_bitmask; extern size_t log_ses_count[]; @@ -433,7 +435,8 @@ DCB_CALLBACK *cb; free(cb); } spinlock_release(&dcb->cb_lock); - + if(dcb->ssl) + SSL_free(dcb->ssl); bitmask_free(&dcb->memdata.bitmask); free(dcb); } @@ -894,7 +897,6 @@ return_n: */ int dcb_read_SSL( DCB *dcb, - SSL* ssl, GWBUF **head) { GWBUF *buffer = NULL; @@ -945,7 +947,7 @@ int dcb_read_SSL( int r = -1; /* try to read 1 byte, without consuming the socket buffer */ - r = SSL_peek(ssl, &c, sizeof(char)); + r = SSL_peek(dcb->ssl, &c, sizeof(char)); if (r <= 0) { n = -1; @@ -983,11 +985,18 @@ int dcb_read_SSL( n = -1; goto return_n; } - n = SSL_read(ssl, GWBUF_DATA(buffer), bufsize); - dcb->stats.n_reads++; + + int npending; + n = 0; + do + { + n += SSL_read(dcb->ssl, GWBUF_DATA(buffer), bufsize); + dcb->stats.n_reads++; + }while((npending = SSL_pending(dcb->ssl)) > 0); int ssl_errno = 0; - if (n <= 0) + + if (n <= 0) { ssl_errno = ERR_get_error(); @@ -1006,6 +1015,15 @@ int dcb_read_SSL( goto return_n; } } + + if(n < b) + { + gwbuf_rtrim(buffer,b - n); + ss_dassert(gwbuf_length(buffer) == n); + LOGIF(LD,(skygw_log_write(LD,"[%lu] SSL: Truncated buffer to correct size from %d to %d bytes.\n", + b,gwbuf_length(buffer)))); + } + nread += n; LOGIF(LD, (skygw_log_write( @@ -1019,7 +1037,8 @@ int dcb_read_SSL( dcb->fd))); /*< Append read data to the gwbuf */ *head = gwbuf_append(*head, buffer); - if(ssl_errno == SSL_ERROR_WANT_READ || ssl_errno == SSL_ERROR_NONE) + if(ssl_errno == SSL_ERROR_WANT_READ || ssl_errno == SSL_ERROR_NONE || + ssl_errno == SSL_ERROR_WANT_X509_LOOKUP || SSL_ERROR_WANT_WRITE) break; } /*< while (true) */ return_n: @@ -1270,7 +1289,7 @@ int below_water; * @return 0 on failure, 1 on success */ int -dcb_write_SSL(DCB *dcb, SSL* ssl, GWBUF *queue) +dcb_write_SSL(DCB *dcb, GWBUF *queue) { int w; int saved_errno = 0; @@ -1379,7 +1398,7 @@ dcb_write_SSL(DCB *dcb, SSL* ssl, GWBUF *queue) #endif /* FAKE_CODE */ qlen = GWBUF_LENGTH(queue); GW_NOINTR_CALL( - w = gw_write_SSL(ssl, GWBUF_DATA(queue), qlen); + w = gw_write_SSL(dcb->ssl, GWBUF_DATA(queue), qlen); dcb->stats.n_writes++; ); @@ -1619,7 +1638,7 @@ int above_water; * @return The number of bytes written */ int -dcb_drain_writeq_SSL(DCB *dcb, SSL* ssl) +dcb_drain_writeq_SSL(DCB *dcb) { int n = 0; int w; @@ -1641,7 +1660,7 @@ dcb_drain_writeq_SSL(DCB *dcb, SSL* ssl) while (dcb->writeq != NULL) { len = GWBUF_LENGTH(dcb->writeq); - GW_NOINTR_CALL(w = gw_write_SSL(ssl, GWBUF_DATA(dcb->writeq), len);); + GW_NOINTR_CALL(w = gw_write_SSL(dcb->ssl, GWBUF_DATA(dcb->writeq), len);); if (w < 0) { @@ -2728,3 +2747,101 @@ DCB *ptr; spinlock_release(&dcbspin); return rval; } + +/** + * Create the SSL structure for this DCB. + * This function creates the SSL structure for the given SSL context. This context + * should be the service's context + * @param dcb + * @param context + * @return + */ +int dcb_create_SSL(DCB* dcb) +{ + + if(serviceInitSSL(dcb->service) != 0) + { + return -1; + } + + if((dcb->ssl = SSL_new(dcb->service->ctx)) == NULL) + { + skygw_log_write(LE,"Error: Failed to initialize SSL connection."); + return -1; + } + + if(SSL_set_fd(dcb->ssl,dcb->fd) == 0) + { + skygw_log_write(LE,"Error: Failed to set file descriptor for SSL connection."); + return -1; + } + + return 0; +} +/** + * Accept a SSL connection and do the SSL authentication handshake. + * This function accepts a client connection to a DCB. It assumes that the SSL + * structure has the underlying method of communication set and this method is ready + * for usage. It then proceeds with the SSL handshake and stops only if an error + * occurs or the client has not yet written enough data to complete the handshake. + * @param dcb DCB which should accept the SSL connection + * @return 1 if the handshake was successfully completed, 0 if the handshake is + * still ongoing and another call to dcb_SSL_accept should be made or -1 if an + * error occurred during the handshake and the connection should be terminated. + */ +int dcb_accept_SSL(DCB* dcb) +{ + int rval,errnum; + + rval = SSL_accept(dcb->ssl); + + switch(rval) + { + case 0: + errnum = SSL_get_error(dcb->ssl,rval); + LOGIF(LD,(skygw_log_write_flush(LD,"SSL_accept shutdown for %s@%s", + dcb->user, + dcb->remote))); + return -1; + break; + case 1: + rval = 1; + LOGIF(LD,(skygw_log_write_flush(LD,"SSL_accept done for %s@%s", + dcb->user, + dcb->remote))); + break; + + case -1: + errnum = SSL_get_error(dcb->ssl,rval); + + if(errnum == SSL_ERROR_WANT_READ || errnum == SSL_ERROR_WANT_WRITE || + errnum == SSL_ERROR_WANT_X509_LOOKUP) + { + /** Not all of the data has been read. Go back to the poll + queue and wait for more.*/ + + rval = 0; + LOGIF(LD,(skygw_log_write_flush(LD,"SSL_accept ongoing for %s@%s", + dcb->user, + dcb->remote))); + } + else + { + rval = -1; + skygw_log_write_flush(LE, + "Error: Fatal error in SSL_accept for %s@%s: %s", + dcb->user, + dcb->remote, + ERR_error_string(errnum,NULL)); + } + break; + + default: + skygw_log_write_flush(LE, + "Error: Fatal error in SSL_accept, returned value was %d.", + rval); + break; + } + + return rval; +} \ No newline at end of file diff --git a/server/core/service.c b/server/core/service.c index 77b581e81..f8b9d0e99 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -1818,24 +1818,28 @@ int serviceInitSSL(SERVICE* service) { service->method = (SSL_METHOD*)SSLv23_server_method(); service->ctx = SSL_CTX_new(service->method); - + SSL_CTX_set_read_ahead(service->ctx,1); if (SSL_CTX_use_certificate_file(service->ctx, service->ssl_cert, SSL_FILETYPE_PEM) <= 0) { + skygw_log_write(LE,"Error: Failed to set server SSL certificate."); return -1; } /* Load the private-key corresponding to the server certificate */ if (SSL_CTX_use_PrivateKey_file(service->ctx, service->ssl_key, SSL_FILETYPE_PEM) <= 0) { + skygw_log_write(LE,"Error: Failed to set server SSL key."); return -1; } /* Check if the server certificate and private-key matches */ if (!SSL_CTX_check_private_key(service->ctx)) { + skygw_log_write(LE,"Error: Server SSL certificate and key do not match."); return -1; } /* Load the RSA CA certificate into the SSL_CTX structure */ if (!SSL_CTX_load_verify_locations(service->ctx, service->ssl_ca_cert, NULL)) { + skygw_log_write(LE,"Error: Failed to set Certificate Authority file."); return -1; } diff --git a/server/include/dcb.h b/server/include/dcb.h index 38ebdc299..58a4eb532 100644 --- a/server/include/dcb.h +++ b/server/include/dcb.h @@ -268,6 +268,7 @@ typedef struct dcb { unsigned int high_water; /**< High water mark */ unsigned int low_water; /**< Low water mark */ struct server *server; /**< The associated backend server */ + SSL* ssl; /*< SSL struct for connection */ #if defined(SS_DEBUG) int dcb_port; /**< port of target server */ skygw_chk_t dcb_chk_tail; @@ -340,10 +341,12 @@ bool dcb_set_state(DCB* dcb, dcb_state_t new_state, dcb_state_t* old_state); void dcb_call_foreach (struct server* server, DCB_REASON reason); size_t dcb_get_session_id(DCB* dcb); bool dcb_get_ses_log_info(DCB* dcb, size_t* sesid, int* enabled_logs); +int dcb_create_SSL(DCB* dcb); +int dcb_accept_SSL(DCB* dcb); int gw_write_SSL(SSL* ssl, const void *buf, size_t nbytes); -int dcb_write_SSL(DCB *dcb, SSL* ssl, GWBUF *queue); -int dcb_read_SSL(DCB *dcb,SSL* ssl,GWBUF **head); -int dcb_drain_writeq_SSL(DCB *dcb, SSL* ssl); +int dcb_write_SSL(DCB *dcb,GWBUF *queue); +int dcb_read_SSL(DCB *dcb,GWBUF **head); +int dcb_drain_writeq_SSL(DCB *dcb); /** diff --git a/server/modules/include/mysql_client_server_protocol.h b/server/modules/include/mysql_client_server_protocol.h index 94db6e664..e5fd954a4 100644 --- a/server/modules/include/mysql_client_server_protocol.h +++ b/server/modules/include/mysql_client_server_protocol.h @@ -297,7 +297,6 @@ typedef struct { unsigned long tid; /*< MySQL Thread ID, in * handshake */ unsigned int charset; /*< MySQL character set at connect time */ - SSL* ssl; /*< SSL struct for client connection */ bool use_ssl; #if defined(SS_DEBUG) skygw_chk_t protocol_chk_tail; diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index 2d2633b90..b930d8c77 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -490,14 +490,7 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { /** Do the SSL Handshake */ if(ssl && protocol->owner_dcb->service->ssl_mode != SSL_DISABLED) { - if(serviceInitSSL(protocol->owner_dcb->service) != 0) - { - skygw_log_write(LOGFILE_ERROR,"Error: SSL initialization for service '%s' failed.", - protocol->owner_dcb->service->name); - return 1; - } - protocol->ssl = SSL_new(protocol->owner_dcb->service->ctx); - SSL_set_fd(protocol->ssl,dcb->fd); + protocol->protocol_auth_state = MYSQL_AUTH_SSL_REQ; if(do_ssl_accept(protocol) < 0) @@ -632,7 +625,7 @@ gw_MySQLWrite_client_SSL(DCB *dcb, GWBUF *queue) CHK_DCB(dcb); protocol = DCB_PROTOCOL(dcb, MySQLProtocol); CHK_PROTOCOL(protocol); - return dcb_write_SSL(dcb, protocol->ssl, queue); + return dcb_write_SSL(dcb, queue); } /** @@ -681,7 +674,7 @@ int gw_read_client_event( if(protocol->use_ssl) { - rc = dcb_read_SSL(dcb,protocol->ssl, &read_buffer); + rc = dcb_read_SSL(dcb, &read_buffer); } else { @@ -795,7 +788,7 @@ int gw_read_client_event( dcb->dcb_readqueue = gwbuf_append(dcb->dcb_readqueue, read_buffer); nbytes_read = gwbuf_length(dcb->dcb_readqueue); data = (uint8_t *)GWBUF_DATA(dcb->dcb_readqueue); - + int plen = MYSQL_GET_PACKET_LEN(data); if (nbytes_read < 3 || nbytes_read < MYSQL_GET_PACKET_LEN(data)) { rc = 0; @@ -1255,7 +1248,7 @@ int gw_write_client_event_SSL(DCB *dcb) if (protocol->protocol_auth_state == MYSQL_IDLE) { - dcb_drain_writeq_SSL(dcb,protocol->ssl); + dcb_drain_writeq_SSL(dcb); goto return_1; } @@ -1878,30 +1871,38 @@ return_rc: /** * Do the SSL authentication handshake. - * This functions - * @param protocol - * @return + * This creates the DCB SSL structure if one has not been created and starts the + * SSL handshake handling. + * @param protocol Protocol to connect with SSL + * @return 1 on success, 0 when the handshake is ongoing or -1 on error */ int do_ssl_accept(MySQLProtocol* protocol) { int rval,errnum; char errbuf[2014]; - DCB* dcb; - - rval = SSL_accept(protocol->ssl); + DCB* dcb = protocol->owner_dcb; + if(dcb->ssl == NULL) + { + if(dcb_create_SSL(dcb) != 0) + return -1; + } + rval = dcb_accept_SSL(dcb); + switch(rval) { case 0: - errnum = SSL_get_error(protocol->ssl,rval); - skygw_log_write_flush(LT,"SSL_accept shutdown for %s@%s", + /** Not all of the data has been read. Go back to the poll + queue and wait for more.*/ + + rval = 0; + skygw_log_write_flush(LT,"SSL_accept ongoing for %s@%s", protocol->owner_dcb->user, protocol->owner_dcb->remote); - return -1; + return 0; break; case 1: spinlock_acquire(&protocol->protocol_lock); - dcb = protocol->owner_dcb; protocol->protocol_auth_state = MYSQL_AUTH_SSL_HANDSHAKE_DONE; protocol->use_ssl = true; spinlock_release(&protocol->protocol_lock); @@ -1919,32 +1920,15 @@ int do_ssl_accept(MySQLProtocol* protocol) break; case -1: - errnum = SSL_get_error(protocol->ssl,rval); - if(errnum == SSL_ERROR_WANT_READ || errnum == SSL_ERROR_WANT_WRITE || - errnum == SSL_ERROR_WANT_X509_LOOKUP) - { - /** Not all of the data has been read. Go back to the poll - queue and wait for more.*/ - - rval = 0; - skygw_log_write_flush(LT,"SSL_accept ongoing for %s@%s", - protocol->owner_dcb->user, - protocol->owner_dcb->remote); - } - else - { spinlock_acquire(&protocol->protocol_lock); protocol->protocol_auth_state = MYSQL_AUTH_SSL_HANDSHAKE_FAILED; spinlock_release(&protocol->protocol_lock); rval = -1; - skygw_log_write_flush(LE, "Error: Fatal error in SSL_accept for %s@%s: %s", protocol->owner_dcb->user, - protocol->owner_dcb->remote, - ERR_error_string(errnum,NULL)); - } + protocol->owner_dcb->remote); break; default: diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index 654d24692..56988dc9a 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -137,10 +137,7 @@ void mysql_protocol_done ( goto retblock; } scmd = p->protocol_cmd_history; - if(p->ssl) - { - SSL_free(p->ssl); - } + while (scmd != NULL) { scmd2 = scmd->scom_next; From 76655e7136790bc189e678584681143108ada50b Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 2 Jun 2015 06:04:06 +0300 Subject: [PATCH 11/54] Added a call to a library function which adds all algorithms to OpenSSL to the SSL initialization code. --- server/core/gateway.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/core/gateway.c b/server/core/gateway.c index 0b07f507b..67d5ecf62 100644 --- a/server/core/gateway.c +++ b/server/core/gateway.c @@ -1377,6 +1377,8 @@ int main(int argc, char **argv) SSL_library_init(); SSL_load_error_strings(); + OPENSSL_add_all_algorithms_noconf(); + int n_locks = CRYPTO_num_locks(); if((ssl_locks = malloc(n_locks*sizeof(SPINLOCK))) == NULL) { From 6e01757455dd4ec94eeca482e85c616bf1572918 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 2 Jun 2015 06:39:51 +0300 Subject: [PATCH 12/54] Added error message to users when trying to connect to a MaxScale service that only allows SSL connections. --- .../include/mysql_client_server_protocol.h | 4 +++ server/modules/protocol/mysql_backend.c | 4 +-- server/modules/protocol/mysql_client.c | 27 ++++++++++++------- server/modules/protocol/mysql_common.c | 11 +++++++- 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/server/modules/include/mysql_client_server_protocol.h b/server/modules/include/mysql_client_server_protocol.h index e5fd954a4..f72416491 100644 --- a/server/modules/include/mysql_client_server_protocol.h +++ b/server/modules/include/mysql_client_server_protocol.h @@ -91,6 +91,10 @@ #define COM_QUIT_PACKET_SIZE (4+1) struct dcb; +#define MYSQL_FAILED_AUTH 1 +#define MYSQL_FAILED_AUTH_DB 2 +#define MYSQL_FAILED_AUTH_SSL 3 + typedef enum { MYSQL_ALLOC, MYSQL_PENDING_CONNECT, diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index ba5786851..d000474ab 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -72,7 +72,7 @@ static void backend_set_delayqueue(DCB *dcb, GWBUF *queue); static int gw_change_user(DCB *backend_dcb, SERVER *server, SESSION *in_session, GWBUF *queue); static GWBUF* process_response_data (DCB* dcb, GWBUF* readbuf, int nbytes_to_process); extern char* create_auth_failed_msg( GWBUF* readbuf, char* hostaddr, uint8_t* sha1); -extern char* create_auth_fail_str(char *username, char *hostaddr, char *sha1, char *db); +extern char* create_auth_fail_str(char *username, char *hostaddr, char *sha1, char *db,int); static bool sescmd_response_complete(DCB* dcb); @@ -1433,7 +1433,7 @@ static int gw_change_user( message = create_auth_fail_str(username, backend->session->client->remote, password_set, - ""); + "",auth_ret); if (message == NULL) { LOGIF(LE, (skygw_log_write_flush( diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index b930d8c77..6aeadd180 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -78,7 +78,7 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue); static int route_by_statement(SESSION *, GWBUF **); extern char* get_username_from_auth(char* ptr, uint8_t* data); extern int check_db_name_after_auth(DCB *, char *, int); -extern char* create_auth_fail_str(char *username, char *hostaddr, char *sha1, char *db); +extern char* create_auth_fail_str(char *username, char *hostaddr, char *sha1, char *db,int); int do_ssl_accept(MySQLProtocol* protocol); @@ -450,7 +450,7 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { /* Detect now if there are enough bytes to continue */ if (client_auth_packet_size < (4 + 4 + 4 + 1 + 23)) { - return 1; + return MYSQL_FAILED_AUTH; } memcpy(&protocol->client_capabilities, client_auth_packet + 4, 4); @@ -476,7 +476,7 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { protocol->owner_dcb->user, protocol->owner_dcb->remote, protocol->owner_dcb->service->name))); - return 1; + return MYSQL_FAILED_AUTH_SSL; } if(LOG_IS_ENABLED(LT) && ssl) @@ -495,7 +495,7 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { if(do_ssl_accept(protocol) < 0) { - return 1; + return MYSQL_FAILED_AUTH; } else { @@ -509,7 +509,7 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { if (username == NULL) { - return 1; + return MYSQL_FAILED_AUTH; } /* get charset */ @@ -902,7 +902,7 @@ int gw_read_client_event( fail_str = create_auth_fail_str((char *)((MYSQL_session *)dcb->data)->user, dcb->remote, (char*)((MYSQL_session *)dcb->data)->client_sha1, - (char*)((MYSQL_session *)dcb->data)->db); + (char*)((MYSQL_session *)dcb->data)->db,auth_val); modutil_send_mysql_err_packet(dcb, 2, 0, 1045, "28000", fail_str); } if (fail_str) @@ -996,14 +996,21 @@ int gw_read_client_event( snprintf(fail_str, message_len, "Unknown database '%s'", (char*)((MYSQL_session *)dcb->data)->db); - modutil_send_mysql_err_packet(dcb, 2, 0, 1049, "42000", fail_str); - } else { + modutil_send_mysql_err_packet(dcb, 3, 0, 1049, "42000", fail_str); + }else if(auth_val == 3){ /** Send error 1045 to client */ fail_str = create_auth_fail_str((char *)((MYSQL_session *)dcb->data)->user, dcb->remote, (char*)((MYSQL_session *)dcb->data)->client_sha1, - (char*)((MYSQL_session *)dcb->data)->db); - modutil_send_mysql_err_packet(dcb, 2, 0, 1045, "28000", fail_str); + (char*)((MYSQL_session *)dcb->data)->db,auth_val); + modutil_send_mysql_err_packet(dcb, 3, 0, 1045, "28000", fail_str); + }else { + /** Send error 1045 to client */ + fail_str = create_auth_fail_str((char *)((MYSQL_session *)dcb->data)->user, + dcb->remote, + (char*)((MYSQL_session *)dcb->data)->client_sha1, + (char*)((MYSQL_session *)dcb->data)->db,auth_val); + modutil_send_mysql_err_packet(dcb, 3, 0, 1045, "28000", fail_str); } if (fail_str) free(fail_str); diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index 56988dc9a..0a1d2195b 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -2199,7 +2199,8 @@ char *create_auth_fail_str( char *username, char *hostaddr, char *sha1, - char *db) + char *db, + int errcode) { char* errstr; const char* ferrstr; @@ -2214,6 +2215,10 @@ char *create_auth_fail_str( { ferrstr = "Access denied for user '%s'@'%s' (using password: %s) to database '%s'"; } + else if(errcode == MYSQL_FAILED_AUTH_SSL) + { + ferrstr = "Access without SSL denied"; + } else { ferrstr = "Access denied for user '%s'@'%s' (using password: %s)"; @@ -2233,6 +2238,10 @@ char *create_auth_fail_str( { sprintf(errstr, ferrstr, username, hostaddr, (*sha1 == '\0' ? "NO" : "YES"), db); } + else if(errcode == MYSQL_FAILED_AUTH_SSL) + { + sprintf(errstr, ferrstr); + } else { sprintf(errstr, ferrstr, username, hostaddr, (*sha1 == '\0' ? "NO" : "YES")); From 08e0a318268cd42fa0f70442c5642e9da682f1f2 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 2 Jun 2015 06:42:41 +0300 Subject: [PATCH 13/54] Fixed session creation failure messages using the wrong packet number when an SSL connection was made. --- server/modules/protocol/mysql_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index 6aeadd180..45fc65ee0 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -975,7 +975,7 @@ int gw_read_client_event( /** Send ERR 1045 to client */ mysql_send_auth_error( dcb, - 2, + 3, 0, "failed to create new session"); From fc8918b1f2d77d32880ca0297dcb069fccc1e0e1 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 2 Jun 2015 09:15:08 +0300 Subject: [PATCH 14/54] Added a dcb_connect_SSL function which starts a client SSL connection. --- server/core/dcb.c | 66 ++++++++++++++++++++++++++++++++++++++++++++ server/include/dcb.h | 1 + 2 files changed, 67 insertions(+) diff --git a/server/core/dcb.c b/server/core/dcb.c index 2948dae2e..2dcbd9b4c 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -2843,5 +2843,71 @@ int dcb_accept_SSL(DCB* dcb) break; } + return rval; +} + +/** + * Initiate an SSL client connection to a server + * + * This functions starts an SSL client connection to a server which is expecting + * an SSL handshake. The DCB should already have a TCP connection to the server and + * this connection should be in a state that expects an SSL handshake. + * @param dcb DCB to connect + * @return 1 on success, -1 on error and 0 if the SSL handshake is still ongoing + */ +int dcb_connect_SSL(DCB* dcb) +{ + int rval,errnum; + + rval = SSL_connect(dcb->ssl); + + switch(rval) + { + case 0: + errnum = SSL_get_error(dcb->ssl,rval); + LOGIF(LD,(skygw_log_write_flush(LD,"SSL_connect shutdown for %s@%s", + dcb->user, + dcb->remote))); + return -1; + break; + case 1: + rval = 1; + LOGIF(LD,(skygw_log_write_flush(LD,"SSL_connect done for %s@%s", + dcb->user, + dcb->remote))); + break; + + case -1: + errnum = SSL_get_error(dcb->ssl,rval); + + if(errnum == SSL_ERROR_WANT_READ || errnum == SSL_ERROR_WANT_WRITE || + errnum == SSL_ERROR_WANT_X509_LOOKUP) + { + /** Not all of the data has been read. Go back to the poll + queue and wait for more.*/ + + rval = 0; + LOGIF(LD,(skygw_log_write_flush(LD,"SSL_connect ongoing for %s@%s", + dcb->user, + dcb->remote))); + } + else + { + rval = -1; + skygw_log_write_flush(LE, + "Error: Fatal error in SSL_connect for %s@%s: %s", + dcb->user, + dcb->remote, + ERR_error_string(errnum,NULL)); + } + break; + + default: + skygw_log_write_flush(LE, + "Error: Fatal error in SSL_connect, returned value was %d.", + rval); + break; + } + return rval; } \ No newline at end of file diff --git a/server/include/dcb.h b/server/include/dcb.h index 58a4eb532..cc96a2c0e 100644 --- a/server/include/dcb.h +++ b/server/include/dcb.h @@ -343,6 +343,7 @@ size_t dcb_get_session_id(DCB* dcb); bool dcb_get_ses_log_info(DCB* dcb, size_t* sesid, int* enabled_logs); int dcb_create_SSL(DCB* dcb); int dcb_accept_SSL(DCB* dcb); +int dcb_connect_SSL(DCB* dcb); int gw_write_SSL(SSL* ssl, const void *buf, size_t nbytes); int dcb_write_SSL(DCB *dcb,GWBUF *queue); int dcb_read_SSL(DCB *dcb,GWBUF **head); From 57060cafecff9141e1883879c11cfe46d0b120d3 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 2 Jun 2015 17:00:39 +0300 Subject: [PATCH 15/54] Added SSL level configuration to services. --- .../Getting-Started/Configuration-Guide.md | 48 +++++++++++++++++ Documentation/Reference/MaxScale-and-SSL.md | 3 +- server/core/config.c | 16 ++++-- server/core/service.c | 51 ++++++++++++++++++- server/include/service.h | 12 +++++ 5 files changed, 124 insertions(+), 6 deletions(-) diff --git a/Documentation/Getting-Started/Configuration-Guide.md b/Documentation/Getting-Started/Configuration-Guide.md index 535eca2cc..e3cc4871b 100644 --- a/Documentation/Getting-Started/Configuration-Guide.md +++ b/Documentation/Getting-Started/Configuration-Guide.md @@ -326,6 +326,54 @@ Example: connection_timeout=300 ``` +### Service and SSL + +This section describes configuration parameters for services that control the SSL/TLS encrption method and the various certificate files involved in it. To enable SSL, you must configure the `ssl` parameter with either `enabled` or `required` and provide the three files for `ssl_cert`, `ssl_key` and `ssl_ca_cert`. After this, MySQL connections to this service can be encrypted with SSL. + +#### `ssl` + +This enables SSL connections to the service. If this parameter is set to either `required` or `enabled` and the three certificate files can be found (these are explained afterwards), then client connections will be encrypted with SSL. If the parameter is `enabled` then both SSL and non-SSL connections can connect to this service. If the parameter is set to `required` then only SSL connections can be used for this service and non-SSL connections will get an error when they try to connect to the service. + +#### `ssl_key` + +The SSL private key the service should use. This will be the private key that is used as the server side private key during a client-server SSL handshake. This is a required parameter for SSL enabled services. + +#### `ssl_cert` + +The SSL certificate the service should use. This will be the public certificate that is used as the server side certificate during a client-server SSL handshake. This is a required parameter for SSL enabled services. + +#### `ssl_ca_cert` + +This is the Certificate Authority file. It will be used to verify that both the client and the server certificates are valid. This is a required parameter for SSL enabled services. + +### `ssl_version` + +This parameter controls the level of encryption used. Accepted values are: + * SSLv2 + * SSLv3 + * TLSv10 + * TLSv11 + * TLSv12 + * MAX + +Example SSL enabled service configuration: + +``` +[ReadWriteSplitService] +type=service +router=readwritesplit +servers=server1,server2,server3 +user=myuser +passwd=mypasswd +ssl=required +ssl_cert=/home/markus/certs/server-cert.pem +ssl_key=/home/markus/certs/server-key.pem +ssl_ca_cert=/home/markus/certs/ca.pem +ssl_version=TLSv12 +``` + +This configuration requires all connections to be encryped with SSL. It also specifies that TLSv1.2 should be used as the encryption method. The paths to the server certificate files and the Certificate Authority file are also provided. + ### Server Server sections are used to define the backend database servers that can be formed into a service. A server may be a member of one or more services within MaxScale. Servers are identified by a server name which is the section name in the configuration file. Servers have a type parameter of server, plus address port and protocol parameters. diff --git a/Documentation/Reference/MaxScale-and-SSL.md b/Documentation/Reference/MaxScale-and-SSL.md index a4210b0de..ca61d52e2 100644 --- a/Documentation/Reference/MaxScale-and-SSL.md +++ b/Documentation/Reference/MaxScale-and-SSL.md @@ -5,9 +5,10 @@ MaxScale supports client side SSL connections. Enabling is done on a per service ## SSL Options Here are the options which relate to SSL and certificates. -Parameter|Values|Description +Parameter|Values |Description ---------|-----------|-------- ssl | disabled, enabled, required |`disable` disables SSL, `enabled` enables SSL for client connections but still allows non-SSL connections and `required` requires SSL from all client connections. With the `required` option, client connections that do not use SSL will be rejected. ssl_cert | |Path to server certificate ssl_key | |Path to server private key ssl_ca_cert | |Path to Certificate Authority file +ssl_version|SSLV2,SSLV3,TLSV10,TLSV11,TLSV12,MAX| The SSL method level, defaults to highest available encryption level which is TLSv1.2 diff --git a/server/core/config.c b/server/core/config.c index 640437270..5e03a3b19 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -345,7 +345,7 @@ hashtable_memory_fns(monitorhash,strdup,NULL,free,NULL); char *weightby; char *version_string; char *subservices; - char *ssl,*ssl_cert,*ssl_key,*ssl_ca_cert; + char *ssl,*ssl_cert,*ssl_key,*ssl_ca_cert,*ssl_version; bool is_rwsplit = false; bool is_schemarouter = false; char *allow_localhost_match_wildcard_host; @@ -358,6 +358,7 @@ hashtable_memory_fns(monitorhash,strdup,NULL,free,NULL); ssl_cert = config_get_value(obj->parameters, "ssl_cert"); ssl_key = config_get_value(obj->parameters, "ssl_key"); ssl_ca_cert = config_get_value(obj->parameters, "ssl_ca_cert"); + ssl_version = config_get_value(obj->parameters, "ssl_version"); enable_root_user = config_get_value( obj->parameters, "enable_root_user"); @@ -474,6 +475,10 @@ hashtable_memory_fns(monitorhash,strdup,NULL,free,NULL); else { serviceSetCertificates(obj->element,ssl_cert,ssl_key,ssl_ca_cert); + if(ssl_version) + { + serviceSetSSLVersion(obj->element,ssl_version); + } } } else @@ -1381,7 +1386,7 @@ int i; } else if (strcmp(name, "ms_timestamp") == 0) { - skygw_set_highp(config_truth_value(value)); + skygw_set_highp(config_truth_value((char*)value)); } else { @@ -1389,7 +1394,7 @@ int i; { if (strcasecmp(name, lognames[i].logname) == 0) { - if (config_truth_value(value)) + if (config_truth_value((char*)value)) skygw_log_enable(lognames[i].logfile); else skygw_log_disable(lognames[i].logfile); @@ -1967,6 +1972,11 @@ static char *service_params[] = "version_string", "filters", "weightby", + "ssl_cert", + "ssl_ca_cert", + "ssl", + "ssl_key", + "ssl_version", NULL }; diff --git a/server/core/service.c b/server/core/service.c index f8b9d0e99..a8df37f5b 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -141,7 +141,8 @@ SERVICE *service; service->ssl_ca_cert = NULL; service->ssl_cert = NULL; service->ssl_key = NULL; - + /** Use the highest possible SSL/TLS methods available */ + service->ssl_method_type = SERVICE_SSL_TLS_MAX; if (service->name == NULL || service->routerModule == NULL) { if (service->name) @@ -868,6 +869,22 @@ serviceSetCertificates(SERVICE *service, char* cert,char* key, char* ca_cert) service->ssl_ca_cert = strdup(ca_cert); } +void +serviceSetSSLVersion(SERVICE *service, char* version) +{ + if(strcasecmp(version,"SSLV2") == 0) + service->ssl_method_type = SERVICE_SSLV2; + else if(strcasecmp(version,"SSLV3") == 0) + service->ssl_method_type = SERVICE_SSLV3; + else if(strcasecmp(version,"TLSV10") == 0) + service->ssl_method_type = SERVICE_TLS10; + else if(strcasecmp(version,"TLSV11") == 0) + service->ssl_method_type = SERVICE_TLS11; + else if(strcasecmp(version,"TLSV12") == 0) + service->ssl_method_type = SERVICE_TLS12; + else if(strcasecmp(version,"MAX") == 0) + service->ssl_method_type = SERVICE_SSL_TLS_MAX; +} /** Enable or disable the service SSL capability*/ int serviceSetSSL(SERVICE *service, char* action) @@ -1816,7 +1833,37 @@ int serviceInitSSL(SERVICE* service) { if(!service->ssl_init_done) { - service->method = (SSL_METHOD*)SSLv23_server_method(); + switch(service->ssl_method_type) + { + case SERVICE_SSLV2: + service->method = (SSL_METHOD*)SSLv2_server_method(); + break; + case SERVICE_SSLV3: + service->method = (SSL_METHOD*)SSLv3_server_method(); + break; + case SERVICE_TLS10: + service->method = (SSL_METHOD*)TLSv1_server_method(); + break; + case SERVICE_TLS11: + service->method = (SSL_METHOD*)TLSv1_1_server_method(); + break; + case SERVICE_TLS12: + service->method = (SSL_METHOD*)TLSv1_2_server_method(); + break; + case SERVICE_SSL_MAX: + service->method = (SSL_METHOD*)SSLv23_server_method(); + break; + case SERVICE_TLS_MAX: + service->method = (SSL_METHOD*)SSLv23_server_method(); + break; + case SERVICE_SSL_TLS_MAX: + service->method = (SSL_METHOD*)SSLv23_server_method(); + break; + default: + service->method = (SSL_METHOD*)SSLv23_server_method(); + break; + } + service->ctx = SSL_CTX_new(service->method); SSL_CTX_set_read_ahead(service->ctx,1); if (SSL_CTX_use_certificate_file(service->ctx, service->ssl_cert, SSL_FILETYPE_PEM) <= 0) { diff --git a/server/include/service.h b/server/include/service.h index 1c6614656..7975518e7 100644 --- a/server/include/service.h +++ b/server/include/service.h @@ -113,6 +113,17 @@ typedef enum { SSL_REQUIRED } ssl_mode_t; +enum{ + SERVICE_SSLV2, + SERVICE_SSLV3, + SERVICE_TLS10, + SERVICE_TLS11, + SERVICE_TLS12, + SERVICE_SSL_MAX, + SERVICE_TLS_MAX, + SERVICE_SSL_TLS_MAX +}; + /** * Defines a service within the gateway. * @@ -164,6 +175,7 @@ typedef struct service { SSL *ssl; SSL_METHOD *method; /*< SSLv2/3 or TLSv1/2 methods * see: https://www.openssl.org/docs/ssl/SSL_CTX_new.html */ + int ssl_method_type; /*< Which of the SSLv2/3 or TLS1.0/1.1/1.2 methods to use */ char* ssl_cert; char* ssl_key; char* ssl_ca_cert; From 19ac70fc2fdb5f4276cd0ea4e62f85534c21b7e9 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 3 Jun 2015 13:15:45 +0300 Subject: [PATCH 16/54] Added unit tests for SSL. --- server/core/config.c | 46 ++++++++--- server/core/service.c | 12 ++- server/include/service.h | 1 + server/modules/protocol/CMakeLists.txt | 1 + server/modules/protocol/test/CMakeLists.txt | 10 +++ server/modules/protocol/test/bad_ca.cnf | 28 +++++++ server/modules/protocol/test/bad_cert.cnf | 28 +++++++ server/modules/protocol/test/bad_key.cnf | 28 +++++++ server/modules/protocol/test/bad_ssl.cnf | 28 +++++++ server/modules/protocol/test/no_ca.cnf | 28 +++++++ .../modules/protocol/test/no_server_cert.cnf | 28 +++++++ .../modules/protocol/test/no_server_key.cnf | 28 +++++++ server/modules/protocol/test/ok.cnf | 28 +++++++ server/modules/protocol/test/test_ssl.sh | 76 +++++++++++++++++++ 14 files changed, 360 insertions(+), 10 deletions(-) create mode 100644 server/modules/protocol/test/CMakeLists.txt create mode 100644 server/modules/protocol/test/bad_ca.cnf create mode 100644 server/modules/protocol/test/bad_cert.cnf create mode 100644 server/modules/protocol/test/bad_key.cnf create mode 100644 server/modules/protocol/test/bad_ssl.cnf create mode 100644 server/modules/protocol/test/no_ca.cnf create mode 100644 server/modules/protocol/test/no_server_cert.cnf create mode 100644 server/modules/protocol/test/no_server_key.cnf create mode 100644 server/modules/protocol/test/ok.cnf create mode 100755 server/modules/protocol/test/test_ssl.sh diff --git a/server/core/config.c b/server/core/config.c index 5e03a3b19..40be2c704 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -453,41 +453,69 @@ hashtable_memory_fns(monitorhash,strdup,NULL,free,NULL); if(ssl) { if(ssl_cert == NULL) + { + error_count++; skygw_log_write(LE,"Error: Server certificate missing for service '%s'." "Please provide the path to the server certificate by adding the ssl_cert= parameter", obj->object); + } if(ssl_ca_cert == NULL) + { + error_count++; skygw_log_write(LE,"Error: CA Certificate missing for service '%s'." "Please provide the path to the certificate authority certificate by adding the ssl_ca_cert= parameter", obj->object); + } if(ssl_key == NULL) + { + error_count++; skygw_log_write(LE,"Error: Server private key missing for service '%s'. " "Please provide the path to the server certificate key by adding the ssl_key= parameter" ,obj->object); + } - if(ssl_ca_cert != NULL && ssl_cert != NULL && ssl_key != NULL) + if(access(ssl_ca_cert,F_OK) != 0) { + skygw_log_write(LE,"Error: Certificate authority file for service '%s' not found: %s", + obj->object, + ssl_ca_cert); + error_count++; + } + if(access(ssl_cert,F_OK) != 0) + { + skygw_log_write(LE,"Error: Server certificate file for service '%s' not found: %s", + obj->object, + ssl_cert); + error_count++; + } + if(access(ssl_key,F_OK) != 0) + { + skygw_log_write(LE,"Error: Server private key file for service '%s' not found: %s", + obj->object, + ssl_key); + error_count++; + } + if(error_count == 0) + { if(serviceSetSSL(obj->element,ssl) != 0) { skygw_log_write(LE,"Error: Unknown parameter for service '%s': %s",obj->object,ssl); + error_count++; } else { serviceSetCertificates(obj->element,ssl_cert,ssl_key,ssl_ca_cert); if(ssl_version) { - serviceSetSSLVersion(obj->element,ssl_version); + if(serviceSetSSLVersion(obj->element,ssl_version) != 0) + { + skygw_log_write(LE,"Error: Unknown parameter value for 'ssl_version' for service '%s': %s",obj->object,ssl_version); + error_count++; + } } } } - else - { - /** If SSL was configured wrong, the - * service needs to fail.*/ - skygw_log_write_flush(LE,"Error: Missing SSL certificate paths found in the configuration. " - "This service will not use SSL."); - } } diff --git a/server/core/service.c b/server/core/service.c index a8df37f5b..7fc931297 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -864,12 +864,20 @@ serviceOptimizeWildcard(SERVICE *service, int action) void serviceSetCertificates(SERVICE *service, char* cert,char* key, char* ca_cert) { + if(service->ssl_cert) + free(service->ssl_cert); service->ssl_cert = strdup(cert); + + if(service->ssl_key) + free(service->ssl_key); service->ssl_key = strdup(key); + + if(service->ssl_ca_cert) + free(service->ssl_ca_cert); service->ssl_ca_cert = strdup(ca_cert); } -void +int serviceSetSSLVersion(SERVICE *service, char* version) { if(strcasecmp(version,"SSLV2") == 0) @@ -884,6 +892,8 @@ serviceSetSSLVersion(SERVICE *service, char* version) service->ssl_method_type = SERVICE_TLS12; else if(strcasecmp(version,"MAX") == 0) service->ssl_method_type = SERVICE_SSL_TLS_MAX; + else return -1; + return 0; } /** Enable or disable the service SSL capability*/ int diff --git a/server/include/service.h b/server/include/service.h index 7975518e7..af71bfe7d 100644 --- a/server/include/service.h +++ b/server/include/service.h @@ -210,6 +210,7 @@ extern int serviceGetUser(SERVICE *, char **, char **); extern void serviceSetFilters(SERVICE *, char *); extern int serviceSetSSL(SERVICE *service, char* action); extern int serviceInitSSL(SERVICE* service); +extern int serviceSetSSLVersion(SERVICE *service, char* version); extern void serviceSetCertificates(SERVICE *service, char* cert,char* key, char* ca_cert); extern int serviceEnableRootUser(SERVICE *, int ); extern int serviceSetTimeout(SERVICE *, int ); diff --git a/server/modules/protocol/CMakeLists.txt b/server/modules/protocol/CMakeLists.txt index 4ae3b8f2c..124071c44 100644 --- a/server/modules/protocol/CMakeLists.txt +++ b/server/modules/protocol/CMakeLists.txt @@ -17,6 +17,7 @@ install(TARGETS HTTPD DESTINATION ${MAXSCALE_LIBDIR}) if(BUILD_TESTS) add_library(testprotocol SHARED testprotocol.c) install(TARGETS testprotocol DESTINATION ${MAXSCALE_LIBDIR}) + add_subdirectory(test) endif() add_library(maxscaled SHARED maxscaled.c) diff --git a/server/modules/protocol/test/CMakeLists.txt b/server/modules/protocol/test/CMakeLists.txt new file mode 100644 index 000000000..f3eb3aa2b --- /dev/null +++ b/server/modules/protocol/test/CMakeLists.txt @@ -0,0 +1,10 @@ +configure_file(test_ssl.sh ${CMAKE_CURRENT_BINARY_DIR}/test_ssl.sh @ONLY) +configure_file(no_ca.cnf ${CMAKE_CURRENT_BINARY_DIR}/no_ca.cnf @ONLY) +configure_file(no_server_cert.cnf ${CMAKE_CURRENT_BINARY_DIR}/no_server_cert.cnf @ONLY) +configure_file(no_server_key.cnf ${CMAKE_CURRENT_BINARY_DIR}/no_server_key.cnf @ONLY) +configure_file(bad_ca.cnf ${CMAKE_CURRENT_BINARY_DIR}/bad_ca.cnf @ONLY) +configure_file(bad_cert.cnf ${CMAKE_CURRENT_BINARY_DIR}/bad_cert.cnf @ONLY) +configure_file(bad_key.cnf ${CMAKE_CURRENT_BINARY_DIR}/bad_key.cnf @ONLY) +configure_file(bad_ssl.cnf ${CMAKE_CURRENT_BINARY_DIR}/bad_ssl.cnf @ONLY) +configure_file(ok.cnf ${CMAKE_CURRENT_BINARY_DIR}/ok.cnf @ONLY) +add_test(NAME SSLTest COMMAND ${CMAKE_CURRENT_BINARY_DIR}/test_ssl.sh) diff --git a/server/modules/protocol/test/bad_ca.cnf b/server/modules/protocol/test/bad_ca.cnf new file mode 100644 index 000000000..9206679a9 --- /dev/null +++ b/server/modules/protocol/test/bad_ca.cnf @@ -0,0 +1,28 @@ +[maxscale] +threads=1 +logdir=@CMAKE_CURRENT_BINARY_DIR@ +datadir=@CMAKE_CURRENT_BINARY_DIR@ +piddir=@CMAKE_CURRENT_BINARY_DIR@ +cachedir=@CMAKE_CURRENT_BINARY_DIR@ + +[Testservice] +type=service +router=readconnroute +servers=server1 +user=user +passwd=pwd +ssl=enabled +ssl_ca_cert=This is not a value +ssl_cert=@CMAKE_CURRENT_BINARY_DIR@/server-cert +ssl_key=@CMAKE_CURRENT_BINARY_DIR@/server-key + +[Testlistener] +type=listener +service=Testservice +protocol=MySQLBackend +port=12345 + +[server1] +type=server +address=127.0.0.1 +port=4321 diff --git a/server/modules/protocol/test/bad_cert.cnf b/server/modules/protocol/test/bad_cert.cnf new file mode 100644 index 000000000..1b4c776cc --- /dev/null +++ b/server/modules/protocol/test/bad_cert.cnf @@ -0,0 +1,28 @@ +[maxscale] +threads=1 +logdir=@CMAKE_CURRENT_BINARY_DIR@ +datadir=@CMAKE_CURRENT_BINARY_DIR@ +piddir=@CMAKE_CURRENT_BINARY_DIR@ +cachedir=@CMAKE_CURRENT_BINARY_DIR@ + +[Testservice] +type=service +router=readconnroute +servers=server1 +user=user +passwd=pwd +ssl=enabled +ssl_ca_cert=@CMAKE_CURRENT_BINARY_DIR@/ca +ssl_cert=This is not a value +ssl_key=@CMAKE_CURRENT_BINARY_DIR@/server-key + +[Testlistener] +type=listener +service=Testservice +protocol=MySQLBackend +port=12345 + +[server1] +type=server +address=127.0.0.1 +port=4321 diff --git a/server/modules/protocol/test/bad_key.cnf b/server/modules/protocol/test/bad_key.cnf new file mode 100644 index 000000000..4e0be5f05 --- /dev/null +++ b/server/modules/protocol/test/bad_key.cnf @@ -0,0 +1,28 @@ +[maxscale] +threads=1 +logdir=@CMAKE_CURRENT_BINARY_DIR@ +datadir=@CMAKE_CURRENT_BINARY_DIR@ +piddir=@CMAKE_CURRENT_BINARY_DIR@ +cachedir=@CMAKE_CURRENT_BINARY_DIR@ + +[Testservice] +type=service +router=readconnroute +servers=server1 +user=user +passwd=pwd +ssl=enabled +ssl_ca_cert=@CMAKE_CURRENT_BINARY_DIR@/ca +ssl_cert=@CMAKE_CURRENT_BINARY_DIR@/server-cert +ssl_key=This is not a value + +[Testlistener] +type=listener +service=Testservice +protocol=MySQLBackend +port=12345 + +[server1] +type=server +address=127.0.0.1 +port=4321 diff --git a/server/modules/protocol/test/bad_ssl.cnf b/server/modules/protocol/test/bad_ssl.cnf new file mode 100644 index 000000000..f6dcff1a1 --- /dev/null +++ b/server/modules/protocol/test/bad_ssl.cnf @@ -0,0 +1,28 @@ +[maxscale] +threads=1 +logdir=@CMAKE_CURRENT_BINARY_DIR@ +datadir=@CMAKE_CURRENT_BINARY_DIR@ +piddir=@CMAKE_CURRENT_BINARY_DIR@ +cachedir=@CMAKE_CURRENT_BINARY_DIR@ + +[Testservice] +type=service +router=readconnroute +servers=server1 +user=user +passwd=pwd +ssl=testing +ssl_ca_cert=@CMAKE_CURRENT_BINARY_DIR@/ca +ssl_cert=@CMAKE_CURRENT_BINARY_DIR@/server-cert +ssl_key=@CMAKE_CURRENT_BINARY_DIR@/server-key + +[Testlistener] +type=listener +service=Testservice +protocol=MySQLBackend +port=12345 + +[server1] +type=server +address=127.0.0.1 +port=4321 diff --git a/server/modules/protocol/test/no_ca.cnf b/server/modules/protocol/test/no_ca.cnf new file mode 100644 index 000000000..56f603f6a --- /dev/null +++ b/server/modules/protocol/test/no_ca.cnf @@ -0,0 +1,28 @@ +[maxscale] +threads=1 +logdir=@CMAKE_CURRENT_BINARY_DIR@ +datadir=@CMAKE_CURRENT_BINARY_DIR@ +piddir=@CMAKE_CURRENT_BINARY_DIR@ +cachedir=@CMAKE_CURRENT_BINARY_DIR@ + +[Testservice] +type=service +router=readconnroute +servers=server1 +user=user +passwd=pwd +ssl=enabled +#ssl_ca_cert=@CMAKE_CURRENT_BINARY_DIR@/ca +ssl_cert=@CMAKE_CURRENT_BINARY_DIR@/server-cert +ssl_key=@CMAKE_CURRENT_BINARY_DIR@/server-key + +[Testlistener] +type=listener +service=Testservice +protocol=MySQLBackend +port=12345 + +[server1] +type=server +address=127.0.0.1 +port=4321 diff --git a/server/modules/protocol/test/no_server_cert.cnf b/server/modules/protocol/test/no_server_cert.cnf new file mode 100644 index 000000000..f714a0b3f --- /dev/null +++ b/server/modules/protocol/test/no_server_cert.cnf @@ -0,0 +1,28 @@ +[maxscale] +threads=1 +logdir=@CMAKE_CURRENT_BINARY_DIR@ +datadir=@CMAKE_CURRENT_BINARY_DIR@ +piddir=@CMAKE_CURRENT_BINARY_DIR@ +cachedir=@CMAKE_CURRENT_BINARY_DIR@ + +[Testservice] +type=service +router=readconnroute +servers=server1 +user=user +passwd=pwd +ssl=enabled +ssl_ca_cert=@CMAKE_CURRENT_BINARY_DIR@/ca +#ssl_cert=@CMAKE_CURRENT_BINARY_DIR@/server-cert +ssl_key=@CMAKE_CURRENT_BINARY_DIR@/server-key + +[Testlistener] +type=listener +service=Testservice +protocol=MySQLBackend +port=12345 + +[server1] +type=server +address=127.0.0.1 +port=4321 diff --git a/server/modules/protocol/test/no_server_key.cnf b/server/modules/protocol/test/no_server_key.cnf new file mode 100644 index 000000000..a820ee414 --- /dev/null +++ b/server/modules/protocol/test/no_server_key.cnf @@ -0,0 +1,28 @@ +[maxscale] +threads=1 +logdir=@CMAKE_CURRENT_BINARY_DIR@ +datadir=@CMAKE_CURRENT_BINARY_DIR@ +piddir=@CMAKE_CURRENT_BINARY_DIR@ +cachedir=@CMAKE_CURRENT_BINARY_DIR@ + +[Testservice] +type=service +router=readconnroute +servers=server1 +user=user +passwd=pwd +ssl=enabled +ssl_ca_cert=@CMAKE_CURRENT_BINARY_DIR@/ca +ssl_cert=@CMAKE_CURRENT_BINARY_DIR@/server-cert +#ssl_key=@CMAKE_CURRENT_BINARY_DIR@/server-key + +[Testlistener] +type=listener +service=Testservice +protocol=MySQLBackend +port=12345 + +[server1] +type=server +address=127.0.0.1 +port=4321 diff --git a/server/modules/protocol/test/ok.cnf b/server/modules/protocol/test/ok.cnf new file mode 100644 index 000000000..089025c0d --- /dev/null +++ b/server/modules/protocol/test/ok.cnf @@ -0,0 +1,28 @@ +[maxscale] +threads=1 +logdir=@CMAKE_CURRENT_BINARY_DIR@ +datadir=@CMAKE_CURRENT_BINARY_DIR@ +piddir=@CMAKE_CURRENT_BINARY_DIR@ +cachedir=@CMAKE_CURRENT_BINARY_DIR@ + +[Testservice] +type=service +router=readconnroute +servers=server1 +user=user +passwd=pwd +ssl=enabled +ssl_ca_cert=@CMAKE_CURRENT_BINARY_DIR@/ca +ssl_cert=@CMAKE_CURRENT_BINARY_DIR@/server-cert +ssl_key=@CMAKE_CURRENT_BINARY_DIR@/server-key + +[Testlistener] +type=listener +service=Testservice +protocol=MySQLBackend +port=12345 + +[server1] +type=server +address=127.0.0.1 +port=4321 diff --git a/server/modules/protocol/test/test_ssl.sh b/server/modules/protocol/test/test_ssl.sh new file mode 100755 index 000000000..632da0f3a --- /dev/null +++ b/server/modules/protocol/test/test_ssl.sh @@ -0,0 +1,76 @@ +#!/usr/bin/env bash + +function create_certs() +{ + echo "CA cert" > @CMAKE_CURRENT_BINARY_DIR@/ca.pem + echo "Server Certificate" > @CMAKE_CURRENT_BINARY_DIR@/server-cert.pem + echo "Server Key" > @CMAKE_CURRENT_BINARY_DIR@/server-key.pem +} + +function start_maxscale () +{ + local result=$(@CMAKE_INSTALL_PREFIX@/@MAXSCALE_BINDIR@/maxscale -d -f $1 &> $1.log;echo $?) + if [[ $result == "0" ]] + then + echo "Error: $1 exited with status $result!" + exit 1 + fi +} + +# No CA defined +printf "Testing No CA defined" +start_maxscale @CMAKE_CURRENT_BINARY_DIR@/no_ca.cnf +echo " OK" + +# No cert defined +printf "Testing No cert defined" +start_maxscale @CMAKE_CURRENT_BINARY_DIR@/no_cert.cnf +echo " OK" + +# No key defined +printf "Testing No key defined" +start_maxscale @CMAKE_CURRENT_BINARY_DIR@/no_key.cnf +echo " OK" + +# Bad SSL value +printf "Testing Bad SSL defined" +start_maxscale @CMAKE_CURRENT_BINARY_DIR@/bad_ssl.cnf +echo " OK" + +# Bad CA defined +printf "Testing Bad CA defined" +start_maxscale @CMAKE_CURRENT_BINARY_DIR@/bad_ca.cnf +echo " OK" + +# Bad cert defined +printf "Testing Bad cert defined" +start_maxscale @CMAKE_CURRENT_BINARY_DIR@/bad_cert.cnf +echo " OK" + +# Bad key defined +printf "Testing Bad key defined" +start_maxscale @CMAKE_CURRENT_BINARY_DIR@/bad_key.cnf +echo " OK" + +# No CA file +printf "Testing No CA file" +create_certs +rm @CMAKE_CURRENT_BINARY_DIR@/ca.pem +start_maxscale @CMAKE_CURRENT_BINARY_DIR@/ok.cnf +echo " OK" + +# No cert file +printf "Testing No cert file" +create_certs +rm @CMAKE_CURRENT_BINARY_DIR@/server-cert.pem +start_maxscale @CMAKE_CURRENT_BINARY_DIR@/ok.cnf +echo " OK" + +# No key file +printf "Testing No key file" +create_certs +rm @CMAKE_CURRENT_BINARY_DIR@/server-key.pem +start_maxscale @CMAKE_CURRENT_BINARY_DIR@/ok.cnf +echo " OK" + +exit 0 From 4d30cd5fd3b9bdb9a8bb03ec742b158dd558ae94 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 3 Jun 2015 13:28:35 +0300 Subject: [PATCH 17/54] Added unit test for SSL version. --- server/modules/protocol/test/CMakeLists.txt | 1 + .../modules/protocol/test/bad_ssl_version.cnf | 29 +++++++++++++++++++ server/modules/protocol/test/test_ssl.sh | 17 +++++++---- 3 files changed, 41 insertions(+), 6 deletions(-) create mode 100644 server/modules/protocol/test/bad_ssl_version.cnf diff --git a/server/modules/protocol/test/CMakeLists.txt b/server/modules/protocol/test/CMakeLists.txt index f3eb3aa2b..26653901c 100644 --- a/server/modules/protocol/test/CMakeLists.txt +++ b/server/modules/protocol/test/CMakeLists.txt @@ -6,5 +6,6 @@ configure_file(bad_ca.cnf ${CMAKE_CURRENT_BINARY_DIR}/bad_ca.cnf @ONLY) configure_file(bad_cert.cnf ${CMAKE_CURRENT_BINARY_DIR}/bad_cert.cnf @ONLY) configure_file(bad_key.cnf ${CMAKE_CURRENT_BINARY_DIR}/bad_key.cnf @ONLY) configure_file(bad_ssl.cnf ${CMAKE_CURRENT_BINARY_DIR}/bad_ssl.cnf @ONLY) +configure_file(bad_ssl_version.cnf ${CMAKE_CURRENT_BINARY_DIR}/bad_ssl_version.cnf @ONLY) configure_file(ok.cnf ${CMAKE_CURRENT_BINARY_DIR}/ok.cnf @ONLY) add_test(NAME SSLTest COMMAND ${CMAKE_CURRENT_BINARY_DIR}/test_ssl.sh) diff --git a/server/modules/protocol/test/bad_ssl_version.cnf b/server/modules/protocol/test/bad_ssl_version.cnf new file mode 100644 index 000000000..6849b1904 --- /dev/null +++ b/server/modules/protocol/test/bad_ssl_version.cnf @@ -0,0 +1,29 @@ +[maxscale] +threads=1 +logdir=@CMAKE_CURRENT_BINARY_DIR@ +datadir=@CMAKE_CURRENT_BINARY_DIR@ +piddir=@CMAKE_CURRENT_BINARY_DIR@ +cachedir=@CMAKE_CURRENT_BINARY_DIR@ + +[Testservice] +type=service +router=readconnroute +servers=server1 +user=user +passwd=pwd +ssl=enabled +ssl_ca_cert=@CMAKE_CURRENT_BINARY_DIR@/ca +ssl_cert=@CMAKE_CURRENT_BINARY_DIR@/server-cert +ssl_key=@CMAKE_CURRENT_BINARY_DIR@/server-key +ssl_version=Don't use SSL, it's not needed! + +[Testlistener] +type=listener +service=Testservice +protocol=MySQLBackend +port=12345 + +[server1] +type=server +address=127.0.0.1 +port=4321 diff --git a/server/modules/protocol/test/test_ssl.sh b/server/modules/protocol/test/test_ssl.sh index 632da0f3a..b4b4c4d46 100755 --- a/server/modules/protocol/test/test_ssl.sh +++ b/server/modules/protocol/test/test_ssl.sh @@ -32,22 +32,27 @@ printf "Testing No key defined" start_maxscale @CMAKE_CURRENT_BINARY_DIR@/no_key.cnf echo " OK" -# Bad SSL value +# Bad SSL value defined printf "Testing Bad SSL defined" start_maxscale @CMAKE_CURRENT_BINARY_DIR@/bad_ssl.cnf echo " OK" -# Bad CA defined +# Bad SSL version defined +printf "Testing Bad SSL version defined" +start_maxscale @CMAKE_CURRENT_BINARY_DIR@/bad_ssl_version.cnf +echo " OK" + +# Bad CA value defined printf "Testing Bad CA defined" start_maxscale @CMAKE_CURRENT_BINARY_DIR@/bad_ca.cnf echo " OK" -# Bad cert defined +# Bad server certificate defined printf "Testing Bad cert defined" start_maxscale @CMAKE_CURRENT_BINARY_DIR@/bad_cert.cnf echo " OK" -# Bad key defined +# Bad server key defined printf "Testing Bad key defined" start_maxscale @CMAKE_CURRENT_BINARY_DIR@/bad_key.cnf echo " OK" @@ -59,14 +64,14 @@ rm @CMAKE_CURRENT_BINARY_DIR@/ca.pem start_maxscale @CMAKE_CURRENT_BINARY_DIR@/ok.cnf echo " OK" -# No cert file +# No server certificate file printf "Testing No cert file" create_certs rm @CMAKE_CURRENT_BINARY_DIR@/server-cert.pem start_maxscale @CMAKE_CURRENT_BINARY_DIR@/ok.cnf echo " OK" -# No key file +# No server key file printf "Testing No key file" create_certs rm @CMAKE_CURRENT_BINARY_DIR@/server-key.pem From a033cbf200ae059374f78fd09deef645306f4a86 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 3 Jun 2015 14:14:05 +0300 Subject: [PATCH 18/54] Added more informative error messages when SSL handshake fails. --- server/core/dcb.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index ade2affed..95130299f 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -2789,7 +2789,7 @@ int dcb_create_SSL(DCB* dcb) int dcb_accept_SSL(DCB* dcb) { int rval,errnum; - + char errbuf[140]; rval = SSL_accept(dcb->ssl); switch(rval) @@ -2819,23 +2819,25 @@ int dcb_accept_SSL(DCB* dcb) rval = 0; LOGIF(LD,(skygw_log_write_flush(LD,"SSL_accept ongoing for %s@%s", - dcb->user, + dcb->user?dcb->user:"a connection from ", dcb->remote))); } else { rval = -1; + ERR_error_string(errnum,errbuf); skygw_log_write_flush(LE, - "Error: Fatal error in SSL_accept for %s@%s: %s", + "Error: Fatal error in SSL_accept for %s@%s: (SSL error code: %d) %s", dcb->user, dcb->remote, - ERR_error_string(errnum,NULL)); + errnum, + errbuf); } break; default: skygw_log_write_flush(LE, - "Error: Fatal error in SSL_accept, returned value was %d.", + "Error: Fatal library error in SSL_accept, returned value was %d.", rval); break; } @@ -2855,7 +2857,7 @@ int dcb_accept_SSL(DCB* dcb) int dcb_connect_SSL(DCB* dcb) { int rval,errnum; - + char errbuf[140]; rval = SSL_connect(dcb->ssl); switch(rval) @@ -2891,11 +2893,13 @@ int dcb_connect_SSL(DCB* dcb) else { rval = -1; + ERR_error_string(errnum,errbuf); skygw_log_write_flush(LE, - "Error: Fatal error in SSL_connect for %s@%s: %s", + "Error: Fatal error in SSL_accept for %s@%s: (SSL error code: %d) %s", dcb->user, dcb->remote, - ERR_error_string(errnum,NULL)); + errnum, + errbuf); } break; From a032c94d25c6557cfce3bd835d8c89d10bc22f26 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 4 Jun 2015 16:49:39 +0300 Subject: [PATCH 19/54] Added comments to SSL tests. --- server/modules/protocol/test/test_ssl.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/modules/protocol/test/test_ssl.sh b/server/modules/protocol/test/test_ssl.sh index b4b4c4d46..f14670689 100755 --- a/server/modules/protocol/test/test_ssl.sh +++ b/server/modules/protocol/test/test_ssl.sh @@ -17,6 +17,8 @@ function start_maxscale () fi } +# All test cases expect that MaxScale will not start with a bad configuration or missing certificates + # No CA defined printf "Testing No CA defined" start_maxscale @CMAKE_CURRENT_BINARY_DIR@/no_ca.cnf From dceccce2ef919d69b952fca810af621e6e68ebec Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 4 Jun 2015 16:52:43 +0300 Subject: [PATCH 20/54] Changed gwbuf_length function to GWBUF_LENGTH macro in dcb_read_SSL. --- server/core/dcb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 95130299f..ae77707b9 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1017,7 +1017,7 @@ int dcb_read_SSL( if(n < b) { gwbuf_rtrim(buffer,b - n); - ss_dassert(gwbuf_length(buffer) == n); + ss_dassert(GWBUF_LENGTH(buffer) == n); LOGIF(LD,(skygw_log_write(LD,"[%lu] SSL: Truncated buffer to correct size from %d to %d bytes.\n", b,gwbuf_length(buffer)))); } From ce570685cda436d6a9b48e7ca39adde7c5b162c6 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 4 Jun 2015 19:31:58 +0300 Subject: [PATCH 21/54] Moved assertions around. --- server/core/dcb.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index ae77707b9..653799fa6 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1014,12 +1014,14 @@ int dcb_read_SSL( } } - if(n < b) + if(n > 0 && b > 0 && n < b) { gwbuf_rtrim(buffer,b - n); - ss_dassert(GWBUF_LENGTH(buffer) == n); LOGIF(LD,(skygw_log_write(LD,"[%lu] SSL: Truncated buffer to correct size from %d to %d bytes.\n", b,gwbuf_length(buffer)))); + LOGIF(LD,(skygw_log_sync_all())); + ss_dassert(GWBUF_LENGTH(buffer) == n); + } nread += n; From cba37d2ac398ef31bf916ceb611b1ca45f922eb3 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 4 Jun 2015 19:38:33 +0300 Subject: [PATCH 22/54] Generated packages now have debug symbols. --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 03cb541b6..4f35031d8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -205,6 +205,7 @@ if(PACKAGE) else() # Generic CPack configuration variables + set(CPACK_STRIP_FILES FALSE) set(CPACK_PACKAGE_DESCRIPTION_SUMMARY "MaxScale") set(CPACK_PACKAGE_VERSION_MAJOR "${MAXSCALE_VERSION_MAJOR}") set(CPACK_PACKAGE_VERSION_MINOR "${MAXSCALE_VERSION_MINOR}") From 1f45eff135c4e657e6b0513790d25b3ff784561e Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 4 Jun 2015 19:38:33 +0300 Subject: [PATCH 23/54] Generated packages now have debug symbols. --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 03cb541b6..4f35031d8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -205,6 +205,7 @@ if(PACKAGE) else() # Generic CPack configuration variables + set(CPACK_STRIP_FILES FALSE) set(CPACK_PACKAGE_DESCRIPTION_SUMMARY "MaxScale") set(CPACK_PACKAGE_VERSION_MAJOR "${MAXSCALE_VERSION_MAJOR}") set(CPACK_PACKAGE_VERSION_MINOR "${MAXSCALE_VERSION_MINOR}") From cc1f720ea3e3fc6b992c75af28f200d35f2770de Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 4 Jun 2015 21:12:16 +0300 Subject: [PATCH 24/54] Removed log flushing on every dcb_read_SSL if debug log is enabled. --- server/core/dcb.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 653799fa6..1a7727ff8 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1019,7 +1019,11 @@ int dcb_read_SSL( gwbuf_rtrim(buffer,b - n); LOGIF(LD,(skygw_log_write(LD,"[%lu] SSL: Truncated buffer to correct size from %d to %d bytes.\n", b,gwbuf_length(buffer)))); - LOGIF(LD,(skygw_log_sync_all())); + LOGIF(LD, + if(GWBUF_LENGTH(buffer) != n){ + skygw_log_sync_all(); + } + ); ss_dassert(GWBUF_LENGTH(buffer) == n); } From e83799648a757d31c4b641365516de34b71ae321 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 5 Jun 2015 11:00:51 +0300 Subject: [PATCH 25/54] Fixed queries getting stuck when the SSL records were of the maximum allowed size. --- server/core/dcb.c | 85 ++++++++++++++------------ server/core/service.c | 1 - server/modules/protocol/mysql_client.c | 2 +- 3 files changed, 48 insertions(+), 40 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 1a7727ff8..c1e77b58d 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -898,7 +898,7 @@ int dcb_read_SSL( GWBUF **head) { GWBUF *buffer = NULL; - int b; + int b,pending; int rc; int n; int nread = 0; @@ -918,9 +918,9 @@ int dcb_read_SSL( while (true) { int bufsize; - + int ssl_errno = 0; rc = ioctl(dcb->fd, FIONREAD, &b); - + pending = SSL_pending(dcb->ssl); if (rc == -1) { LOGIF(LE, (skygw_log_write_flush( @@ -936,7 +936,7 @@ int dcb_read_SSL( goto return_n; } - if (b == 0 && nread == 0) + if (b == 0 && pending == 0 && nread == 0) { /** Handle closed client socket */ if (dcb_isclient(dcb)) @@ -948,14 +948,20 @@ int dcb_read_SSL( r = SSL_peek(dcb->ssl, &c, sizeof(char)); if (r <= 0) { + ssl_errno = SSL_get_error(dcb->ssl,r); + if(ssl_errno != SSL_ERROR_WANT_READ && + ssl_errno != SSL_ERROR_WANT_WRITE && + ssl_errno != SSL_ERROR_NONE) + { n = -1; - goto return_n; + } + goto return_n; } } n = 0; goto return_n; } - else if (b == 0) + else if (b == 0 && pending == 0) { n = 0; goto return_n; @@ -984,39 +990,36 @@ int dcb_read_SSL( goto return_n; } - int npending; - n = 0; - do - { - n += SSL_read(dcb->ssl, GWBUF_DATA(buffer), bufsize); + n = SSL_read(dcb->ssl, GWBUF_DATA(buffer), bufsize); dcb->stats.n_reads++; - }while((npending = SSL_pending(dcb->ssl)) > 0); - int ssl_errno = 0; - - if (n <= 0) - { - ssl_errno = ERR_get_error(); - - if(ssl_errno != SSL_ERROR_WANT_READ && ssl_errno != SSL_ERROR_NONE) + if (n <= 0) { - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Read failed, dcb %p in state " - "%s fd %d: %s.", - dcb, - STRDCBSTATE(dcb->state), - dcb->fd, - ERR_error_string(ssl_errno,NULL)))); + ssl_errno = SSL_get_error(dcb->ssl,n); - gwbuf_free(buffer); - goto return_n; - } + if(ssl_errno != SSL_ERROR_WANT_READ && + ssl_errno != SSL_ERROR_WANT_WRITE && + ssl_errno != SSL_ERROR_NONE) + { + char errbuf[200]; + ERR_error_string(ssl_errno,errbuf); + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Read failed, dcb %p in state " + "%s fd %d, SSL error %d: %s.", + dcb, + STRDCBSTATE(dcb->state), + dcb->fd, + ssl_errno, + errbuf))); + + gwbuf_free(buffer); + goto return_n; + } } - if(n > 0 && b > 0 && n < b) - { - gwbuf_rtrim(buffer,b - n); + buffer->end = buffer->start + n; +#ifdef SS_DEBUG LOGIF(LD,(skygw_log_write(LD,"[%lu] SSL: Truncated buffer to correct size from %d to %d bytes.\n", b,gwbuf_length(buffer)))); LOGIF(LD, @@ -1025,9 +1028,7 @@ int dcb_read_SSL( } ); ss_dassert(GWBUF_LENGTH(buffer) == n); - - } - +#endif nread += n; LOGIF(LD, (skygw_log_write( @@ -1039,11 +1040,19 @@ int dcb_read_SSL( dcb, STRDCBSTATE(dcb->state), dcb->fd))); + /*< Append read data to the gwbuf */ *head = gwbuf_append(*head, buffer); - if(ssl_errno == SSL_ERROR_WANT_READ || ssl_errno == SSL_ERROR_NONE || - ssl_errno == SSL_ERROR_WANT_X509_LOOKUP || SSL_ERROR_WANT_WRITE) + rc = ioctl(dcb->fd, FIONREAD, &b); + pending = SSL_pending(dcb->ssl); + + if(ssl_errno == SSL_ERROR_WANT_READ || + ssl_errno == SSL_ERROR_WANT_WRITE || + (b == 0 && pending == 0)) + { break; + } + } /*< while (true) */ return_n: return n; diff --git a/server/core/service.c b/server/core/service.c index 7fc931297..f4880107d 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -1875,7 +1875,6 @@ int serviceInitSSL(SERVICE* service) } service->ctx = SSL_CTX_new(service->method); - SSL_CTX_set_read_ahead(service->ctx,1); if (SSL_CTX_use_certificate_file(service->ctx, service->ssl_cert, SSL_FILETYPE_PEM) <= 0) { skygw_log_write(LE,"Error: Failed to set server SSL certificate."); return -1; diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index 45fc65ee0..5a54ca4b3 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -789,7 +789,7 @@ int gw_read_client_event( nbytes_read = gwbuf_length(dcb->dcb_readqueue); data = (uint8_t *)GWBUF_DATA(dcb->dcb_readqueue); int plen = MYSQL_GET_PACKET_LEN(data); - if (nbytes_read < 3 || nbytes_read < MYSQL_GET_PACKET_LEN(data)) + if (nbytes_read < 3 || nbytes_read < MYSQL_GET_PACKET_LEN(data) + 4) { rc = 0; goto return_rc; From 518ef5050e5ef687c05eb83aa5cb068d78a7f31a Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 5 Jun 2015 12:15:19 +0300 Subject: [PATCH 26/54] Fixed debug asserts. --- server/core/dcb.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index c1e77b58d..c2f48b39d 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1018,15 +1018,16 @@ int dcb_read_SSL( } } - buffer->end = buffer->start + n; + gwbuf_rtrim(buffer,bufsize - n); #ifdef SS_DEBUG LOGIF(LD,(skygw_log_write(LD,"[%lu] SSL: Truncated buffer to correct size from %d to %d bytes.\n", - b,gwbuf_length(buffer)))); + b,GWBUF_LENGTH(buffer)))); LOGIF(LD, if(GWBUF_LENGTH(buffer) != n){ skygw_log_sync_all(); } ); + ss_info_dassert((buffer->start <= buffer->end),"Buffer start has passed end."); ss_dassert(GWBUF_LENGTH(buffer) == n); #endif nread += n; From 61b1f3467160df1cb1f6e364ac4b860eaf81d0c0 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 5 Jun 2015 18:52:44 +0300 Subject: [PATCH 27/54] Added more descriptive debug output. --- server/core/dcb.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index c2f48b39d..728211abe 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1020,13 +1020,14 @@ int dcb_read_SSL( gwbuf_rtrim(buffer,bufsize - n); #ifdef SS_DEBUG - LOGIF(LD,(skygw_log_write(LD,"[%lu] SSL: Truncated buffer to correct size from %d to %d bytes.\n", - b,GWBUF_LENGTH(buffer)))); - LOGIF(LD, - if(GWBUF_LENGTH(buffer) != n){ - skygw_log_sync_all(); - } - ); + skygw_log_write(LD,"[%lu] SSL: Truncated buffer from %d to %d bytes. " + "Read %d bytes, %d bytes waiting.\n", + bufsize,GWBUF_LENGTH(buffer),n,b); + + if(GWBUF_LENGTH(buffer) != n){ + skygw_log_sync_all(); + } + ss_info_dassert((buffer->start <= buffer->end),"Buffer start has passed end."); ss_dassert(GWBUF_LENGTH(buffer) == n); #endif From 1989a1482c3516cea06162b1aec640beadb6f1f3 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 5 Jun 2015 19:37:33 +0300 Subject: [PATCH 28/54] Fixed empty reads causing a debug assert with large packets. --- server/core/dcb.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 728211abe..6beaec55d 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -966,6 +966,13 @@ int dcb_read_SSL( n = 0; goto return_n; } +#ifdef SS_DEBUG + else + { + skygw_log_write_flush(LD,"Total: %d Socket: %d Pending: %d", + nread,b,pending); + } +#endif dcb->last_read = hkheartbeat; @@ -993,7 +1000,7 @@ int dcb_read_SSL( n = SSL_read(dcb->ssl, GWBUF_DATA(buffer), bufsize); dcb->stats.n_reads++; - if (n <= 0) + if (n < 0) { ssl_errno = SSL_get_error(dcb->ssl,n); @@ -1016,12 +1023,17 @@ int dcb_read_SSL( gwbuf_free(buffer); goto return_n; } - } + } + else if(n == 0) + { + gwbuf_free(buffer); + goto return_n; + } gwbuf_rtrim(buffer,bufsize - n); #ifdef SS_DEBUG skygw_log_write(LD,"[%lu] SSL: Truncated buffer from %d to %d bytes. " - "Read %d bytes, %d bytes waiting.\n", + "Read %d bytes, %d bytes waiting.\n",pthread_self(), bufsize,GWBUF_LENGTH(buffer),n,b); if(GWBUF_LENGTH(buffer) != n){ @@ -1029,7 +1041,7 @@ int dcb_read_SSL( } ss_info_dassert((buffer->start <= buffer->end),"Buffer start has passed end."); - ss_dassert(GWBUF_LENGTH(buffer) == n); + ss_info_dassert(GWBUF_LENGTH(buffer) == n,"Buffer size not equal to read bytes."); #endif nread += n; From d19ccc6f846ec6d54f6ad2171acdee08a48aef63 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 5 Jun 2015 20:36:04 +0300 Subject: [PATCH 29/54] Fixed SSL_ERROR_WANT_READ, SSL_ERROR_WANT_WRITE and SSL_ERROR_NONE causing a debug assert. --- server/core/dcb.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 6beaec55d..538b0531a 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1002,13 +1002,21 @@ int dcb_read_SSL( if (n < 0) { + char errbuf[200]; ssl_errno = SSL_get_error(dcb->ssl,n); - - if(ssl_errno != SSL_ERROR_WANT_READ && - ssl_errno != SSL_ERROR_WANT_WRITE && - ssl_errno != SSL_ERROR_NONE) +#ifdef SS_DEBUG + ERR_error_string(ssl_errno,errbuf); + skygw_log_write_flush(LD,"[%lu]SSL error %d: %s", + pthread_self(),ssl_errno,errbuf); +#endif + if(ssl_errno == SSL_ERROR_WANT_READ || + ssl_errno == SSL_ERROR_WANT_WRITE || + ssl_errno == SSL_ERROR_NONE) + { + n = 0; + } + else { - char errbuf[200]; ERR_error_string(ssl_errno,errbuf); LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, @@ -1019,10 +1027,10 @@ int dcb_read_SSL( dcb->fd, ssl_errno, errbuf))); - - gwbuf_free(buffer); - goto return_n; } + + gwbuf_free(buffer); + goto return_n; } else if(n == 0) { From b8e55fe28d24327fd1b7d1641cdd2c736e8f26ad Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sun, 7 Jun 2015 12:37:45 +0300 Subject: [PATCH 30/54] Fixed SSL_accept failing when more data was in the socket buffer than was used. --- server/core/dcb.c | 98 +++++++++++++++++++++++++---------------------- 1 file changed, 52 insertions(+), 46 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 538b0531a..e7862dbd4 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -2825,60 +2825,66 @@ int dcb_create_SSL(DCB* dcb) */ int dcb_accept_SSL(DCB* dcb) { - int rval,errnum; + int rval = 0,ssl_rval,errnum,fd,b = 0; char errbuf[140]; - rval = SSL_accept(dcb->ssl); - - switch(rval) + fd = dcb->fd; + ioctl(fd,FIONREAD,&b); + while(b > 0 && rval != -1) { - case 0: - errnum = SSL_get_error(dcb->ssl,rval); - LOGIF(LD,(skygw_log_write_flush(LD,"SSL_accept shutdown for %s@%s", - dcb->user, - dcb->remote))); - return -1; - break; - case 1: - rval = 1; - LOGIF(LD,(skygw_log_write_flush(LD,"SSL_accept done for %s@%s", - dcb->user, - dcb->remote))); - break; + ssl_rval = SSL_accept(dcb->ssl); - case -1: - errnum = SSL_get_error(dcb->ssl,rval); - - if(errnum == SSL_ERROR_WANT_READ || errnum == SSL_ERROR_WANT_WRITE || - errnum == SSL_ERROR_WANT_X509_LOOKUP) + switch(ssl_rval) { - /** Not all of the data has been read. Go back to the poll - queue and wait for more.*/ - - rval = 0; - LOGIF(LD,(skygw_log_write_flush(LD,"SSL_accept ongoing for %s@%s", - dcb->user?dcb->user:"a connection from ", - dcb->remote))); - } - else - { - rval = -1; + case 0: + errnum = SSL_get_error(dcb->ssl,ssl_rval); ERR_error_string(errnum,errbuf); + LOGIF(LD,(skygw_log_write_flush(LD,"[%p] SSL_accept shutdown for %s:%s", + dcb, + dcb->remote, + errbuf))); + rval = -1; + break; + case 1: + rval = 1; + LOGIF(LD,(skygw_log_write_flush(LD,"[dcb_accept_SSL] SSL_accept done for %s", + dcb->remote))); + break; + + case -1: + errnum = SSL_get_error(dcb->ssl,ssl_rval); + + if(errnum == SSL_ERROR_WANT_READ || errnum == SSL_ERROR_WANT_WRITE) + { + /** Not all of the data has been read. Go back to the poll + queue and wait for more.*/ + rval = 0; + LOGIF(LD,(skygw_log_write_flush(LD,"[dcb_accept_SSL] SSL_accept ongoing for %s", + dcb->remote))); + return rval; + } + else + { + rval = -1; + ERR_error_string(errnum,errbuf); + skygw_log_write_flush(LE, + "Error: Fatal error in SSL_accept for %s: (SSL error code: %d) %s", + dcb->remote, + errnum, + errbuf); + } + break; + + default: skygw_log_write_flush(LE, - "Error: Fatal error in SSL_accept for %s@%s: (SSL error code: %d) %s", - dcb->user, - dcb->remote, - errnum, - errbuf); + "Error: Fatal library error in SSL_accept, returned value was %d.", + ssl_rval); + rval = -1; + break; } - break; - - default: - skygw_log_write_flush(LE, - "Error: Fatal library error in SSL_accept, returned value was %d.", - rval); - break; + ioctl(fd,FIONREAD,&b); + if(LOG_IS_ENABLED(LD) && b > 0) + skygw_log_write_flush(LD,"[dcb_accept_SSL] FD %d has %d bytes ",fd,b); } - return rval; } From 61ea0861ff8f403f14915218ceee3226e5691005 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 8 Jun 2015 14:35:31 +0300 Subject: [PATCH 31/54] Fixed some connections hanging with SSL. --- server/modules/protocol/mysql_client.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index 5a54ca4b3..dc5c08d89 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -661,7 +661,6 @@ int gw_read_client_event( return 0; break; case 1: - return 0; break; case -1: return 1; @@ -1946,4 +1945,4 @@ int do_ssl_accept(MySQLProtocol* protocol) } return rval; -} \ No newline at end of file +} From 1c36cfb28592a549d526a97ea47a6064afba278b Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 8 Jun 2015 18:04:43 +0300 Subject: [PATCH 32/54] Added more debug output. --- server/core/dcb.c | 15 +++++++++++---- .../include/mysql_client_server_protocol.h | 4 ++-- server/modules/protocol/mysql_client.c | 11 +++++++++++ server/modules/protocol/mysql_common.c | 6 +++++- 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index e7862dbd4..cc8d77259 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -2825,14 +2825,19 @@ int dcb_create_SSL(DCB* dcb) */ int dcb_accept_SSL(DCB* dcb) { - int rval = 0,ssl_rval,errnum,fd,b = 0; + int rval = 0,ssl_rval,errnum = 0,fd,b = 0; char errbuf[140]; fd = dcb->fd; ioctl(fd,FIONREAD,&b); +#ifdef SS_DEBUG + skygw_log_write(LD,"[dcb_accept_SSL] fd %d bytes: %d",fd,b); +#endif while(b > 0 && rval != -1) { ssl_rval = SSL_accept(dcb->ssl); - +#ifdef SS_DEBUG + skygw_log_write(LD,"[dcb_accept_SSL] SSL_accept returned %d.",ssl_rval); +#endif switch(ssl_rval) { case 0: @@ -2882,8 +2887,10 @@ int dcb_accept_SSL(DCB* dcb) break; } ioctl(fd,FIONREAD,&b); - if(LOG_IS_ENABLED(LD) && b > 0) - skygw_log_write_flush(LD,"[dcb_accept_SSL] FD %d has %d bytes ",fd,b); +#ifdef SS_DEBUG + skygw_log_write_flush(LD,"[dcb_accept_SSL] fd %d: %d bytes",fd,b); + skygw_log_write_flush(LD,"[dcb_accept_SSL] SSL error: %d",errnum); +#endif } return rval; } diff --git a/server/modules/include/mysql_client_server_protocol.h b/server/modules/include/mysql_client_server_protocol.h index f72416491..905f57dd3 100644 --- a/server/modules/include/mysql_client_server_protocol.h +++ b/server/modules/include/mysql_client_server_protocol.h @@ -321,7 +321,7 @@ typedef struct { #define MYSQL_IS_CHANGE_USER(payload) (MYSQL_GET_COMMAND(payload)==0x11) #define MYSQL_GET_NATTR(payload) ((int)payload[4]) -#endif /** _MYSQL_PROTOCOL_H */ + MySQLProtocol* mysql_protocol_init(DCB* dcb, int fd); void mysql_protocol_done (DCB* dcb); @@ -417,4 +417,4 @@ void init_response_status ( int* npackets, ssize_t* nbytes); - +#endif /** _MYSQL_PROTOCOL_H */ \ No newline at end of file diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index dc5c08d89..fcbb1958e 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -652,9 +652,16 @@ int gw_read_client_event( protocol = DCB_PROTOCOL(dcb, MySQLProtocol); CHK_PROTOCOL(protocol); +#ifdef SS_DEBUG + skygw_log_write(LD,"[gw_read_client_event] Protocol state: %s", + gw_mysql_protocol_state2string(protocol->protocol_auth_state)); + +#endif + if(protocol->protocol_auth_state == MYSQL_AUTH_SSL_HANDSHAKE_ONGOING || protocol->protocol_auth_state == MYSQL_AUTH_SSL_REQ) { + switch(do_ssl_accept(protocol)) { case 0: @@ -1943,6 +1950,10 @@ int do_ssl_accept(MySQLProtocol* protocol) rval); break; } +#ifdef SS_DEBUG + skygw_log_write(LD,"[do_ssl_accept] Protocol state: %s", + gw_mysql_protocol_state2string(protocol->protocol_auth_state)); +#endif return rval; } diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index 0a1d2195b..d8f9a96ba 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -890,7 +890,11 @@ gw_mysql_protocol_state2string (int state) { case MYSQL_AUTH_FAILED: return "MySQL Authentication failed"; case MYSQL_IDLE: - return "MySQL authentication is succesfully done."; + return "MySQL authentication is succesfully done."; + case MYSQL_AUTH_SSL_REQ: return "MYSQL_AUTH_SSL_REQ"; + case MYSQL_AUTH_SSL_HANDSHAKE_DONE: return "MYSQL_AUTH_SSL_HANDSHAKE_DONE"; + case MYSQL_AUTH_SSL_HANDSHAKE_FAILED: return "MYSQL_AUTH_SSL_HANDSHAKE_FAILED"; + case MYSQL_AUTH_SSL_HANDSHAKE_ONGOING: return "MYSQL_AUTH_SSL_HANDSHAKE_ONGOING"; default: return "MySQL (unknown protocol state)"; } From 06c5da7b1728609f3d05610cfaf09a74c2dea4bc Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 9 Jun 2015 02:56:55 +0300 Subject: [PATCH 33/54] Minor fix to SSL authentication. --- server/core/dcb.c | 19 +++++++------------ server/modules/protocol/mysql_client.c | 13 +++++++++++++ 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index cc8d77259..cfee65371 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1055,7 +1055,7 @@ int dcb_read_SSL( LOGIF(LD, (skygw_log_write( LOGFILE_DEBUG, - "%lu [dcb_read] Read %d bytes from dcb %p in state %s " + "%lu [dcb_read_SSL] Read %d bytes from dcb %p in state %s " "fd %d.", pthread_self(), n, @@ -2800,7 +2800,7 @@ int dcb_create_SSL(DCB* dcb) if((dcb->ssl = SSL_new(dcb->service->ctx)) == NULL) { - skygw_log_write(LE,"Error: Failed to initialize SSL connection."); + skygw_log_write(LE,"Error: Failed to initialize SSL for connection."); return -1; } @@ -2828,16 +2828,10 @@ int dcb_accept_SSL(DCB* dcb) int rval = 0,ssl_rval,errnum = 0,fd,b = 0; char errbuf[140]; fd = dcb->fd; - ioctl(fd,FIONREAD,&b); -#ifdef SS_DEBUG - skygw_log_write(LD,"[dcb_accept_SSL] fd %d bytes: %d",fd,b); -#endif - while(b > 0 && rval != -1) + + do { ssl_rval = SSL_accept(dcb->ssl); -#ifdef SS_DEBUG - skygw_log_write(LD,"[dcb_accept_SSL] SSL_accept returned %d.",ssl_rval); -#endif switch(ssl_rval) { case 0: @@ -2889,9 +2883,10 @@ int dcb_accept_SSL(DCB* dcb) ioctl(fd,FIONREAD,&b); #ifdef SS_DEBUG skygw_log_write_flush(LD,"[dcb_accept_SSL] fd %d: %d bytes",fd,b); - skygw_log_write_flush(LD,"[dcb_accept_SSL] SSL error: %d",errnum); + skygw_log_write(LD,"[dcb_accept_SSL] SSL_accept returned %d, SSL error: %d",ssl_rval,errnum); #endif - } + }while(b > 0 && rval != -1); + return rval; } diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index fcbb1958e..da5f04732 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -668,7 +668,18 @@ int gw_read_client_event( return 0; break; case 1: + { + int b = 0; + ioctl(dcb->fd,FIONREAD,&b); + if(b == 0) + { + skygw_log_write(LD, + "[gw_read_client_event] No data in socket after SSL auth"); + return 0; + } break; + } + case -1: return 1; break; @@ -1897,7 +1908,9 @@ int do_ssl_accept(MySQLProtocol* protocol) if(dcb->ssl == NULL) { if(dcb_create_SSL(dcb) != 0) + { return -1; + } } rval = dcb_accept_SSL(dcb); From 528e69b726229f9489f4f7edf81eb17bd73820d2 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 9 Jun 2015 12:51:43 +0300 Subject: [PATCH 34/54] Added a warning about 1.2 changes to the postinstall script --- etc/postinst.in | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/etc/postinst.in b/etc/postinst.in index abf2db1ef..82886990f 100755 --- a/etc/postinst.in +++ b/etc/postinst.in @@ -37,3 +37,17 @@ then cp @CMAKE_INSTALL_PREFIX@/@MAXSCALE_SHAREDIR@/maxscale.service /usr/lib/systemd/system fi /sbin/ldconfig + +cat <& 2 +********** Notice: MaxScale 1.2 Changes ************** + +MaxScale 1.2 has changed the default installation locations +and various files have changed locations. The configuration +file is now read from /etc/maxscale.cnf (note the lower case name) +and MaxScale data is in /var/lib/maxscale/. + +The default location of binary log files and the authentication cache changed from +/usr/local/mariadb-maxscale/ to /var/lib/maxscale/. + +****************************************************** +EOF From ab120cb1de746b321f5ac662daf39aafddf40465 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 9 Jun 2015 17:04:51 +0300 Subject: [PATCH 35/54] Added Diffie-Hellman key exchange for MaxScale. --- server/core/service.c | 53 +++++++++++++++++++++++++++++++++++++++- server/include/service.h | 1 + 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/server/core/service.c b/server/core/service.c index f4880107d..826c79e66 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -91,7 +91,7 @@ static SPINLOCK service_spin = SPINLOCK_INIT; static SERVICE *allServices = NULL; static int find_type(typelib_t* tl, const char* needle, int maxlen); - +DH *ssl_get_dh2236(); static void service_add_qualified_param( SERVICE* svc, CONFIG_PARAMETER* param); @@ -1841,6 +1841,8 @@ int *data; int serviceInitSSL(SERVICE* service) { + DH* dh; + if(!service->ssl_init_done) { switch(service->ssl_method_type) @@ -1875,6 +1877,14 @@ int serviceInitSSL(SERVICE* service) } service->ctx = SSL_CTX_new(service->method); + + /** Enable the Diffie-Hellman algorithms */ + if((dh = ssl_get_dh2236()) != NULL) + { + SSL_CTX_set_tmp_dh(service->ctx,dh); + DH_free(dh); + } + if (SSL_CTX_use_certificate_file(service->ctx, service->ssl_cert, SSL_FILETYPE_PEM) <= 0) { skygw_log_write(LE,"Error: Failed to set server SSL certificate."); return -1; @@ -1908,3 +1918,44 @@ int serviceInitSSL(SERVICE* service) } return 0; } + +DH *ssl_get_dh2236() +{ + static unsigned char dh2236_p[]={ + 0x0B,0xC3,0xEC,0x3F,0xCB,0xD0,0x2E,0x43,0x7B,0x13,0xF9,0x0C, + 0x4D,0xE5,0xA3,0xA4,0xDB,0x68,0x13,0xBD,0xFC,0xD2,0x35,0x05, + 0xCB,0x62,0xA1,0x85,0x33,0x20,0xC4,0x88,0x3B,0x2B,0xD5,0x76, + 0x94,0xCD,0xEB,0x9C,0x5A,0xD1,0x16,0xDB,0x51,0x82,0x7A,0x1E, + 0xC6,0xC3,0xD9,0x52,0x8F,0x54,0x33,0xF4,0x50,0x96,0x01,0xF4, + 0x71,0xA1,0x8B,0x9B,0x43,0x85,0x9C,0x95,0xFF,0x53,0x1D,0x8D, + 0xDF,0xBC,0x60,0xEB,0x4D,0x96,0xD1,0x05,0x98,0x4A,0xEB,0xC9, + 0x33,0xF6,0xE9,0x74,0x73,0x29,0x27,0xCA,0x0D,0x6D,0xEA,0x36, + 0xB9,0x3B,0x54,0xF6,0x34,0x68,0x13,0xFA,0xAC,0x3B,0x57,0x55, + 0x76,0x41,0x67,0x48,0xEF,0x3C,0xE1,0xE1,0xAF,0x3C,0x68,0x05, + 0x9C,0x32,0xD9,0x14,0x8F,0xB2,0xEE,0xEE,0xBA,0x9F,0x0D,0x75, + 0xA7,0x33,0x1F,0x3A,0x0E,0xD1,0xA6,0x5A,0x29,0xC7,0x9B,0x5E, + 0x46,0xB1,0xA6,0xA5,0x1E,0x32,0xDB,0xAF,0x23,0x83,0x94,0x12, + 0x4F,0xE4,0xC2,0x8B,0x1B,0x2C,0x01,0x79,0x92,0x21,0xFF,0x01, + 0xED,0x46,0x27,0xF0,0x70,0x2A,0xA1,0xFD,0x5C,0x8F,0x8B,0x0C, + 0xC6,0x8F,0xFF,0x4C,0x99,0xAE,0x19,0xDB,0x58,0x4C,0xC0,0xE8, + 0x70,0xCC,0x7C,0x17,0xE8,0xBD,0x6B,0x19,0x93,0xB9,0x66,0xA9, + 0xD0,0x05,0x21,0x04,0x4C,0x7E,0x87,0x9F,0xF4,0xE9,0x23,0xE1, + 0x29,0x37,0xC5,0xE2,0x0A,0xC5,0xC1,0x92,0xC7,0x69,0xB4,0xFB, + 0x84,0x06,0xCE,0x0E,0xFC,0x65,0x70,0x2F,0xF6,0xB8,0x11,0xF9, + 0x0F,0x60,0x10,0xCA,0x94,0x29,0x44,0x5E,0x4A,0x05,0x46,0xE5, + 0xE6,0xA0,0xBD,0x14,0x45,0xA6,0xA7,0xCA,0x63,0x57,0xC6,0xB0, + 0x47,0xF9,0x71,0x24,0x19,0x75,0xD2,0x64,0x16,0xB1,0xBA,0x08, + 0xE9,0xE9,0xFB,0xF3, + }; + static unsigned char dh2236_g[]={ + 0x02, + }; + DH *dh; + + if ((dh=DH_new()) == NULL) return(NULL); + dh->p=BN_bin2bn(dh2236_p,sizeof(dh2236_p),NULL); + dh->g=BN_bin2bn(dh2236_g,sizeof(dh2236_g),NULL); + if ((dh->p == NULL) || (dh->g == NULL)) + { DH_free(dh); return(NULL); } + return(dh); +} \ No newline at end of file diff --git a/server/include/service.h b/server/include/service.h index af71bfe7d..936a73058 100644 --- a/server/include/service.h +++ b/server/include/service.h @@ -29,6 +29,7 @@ #include #include #include +#include /** * @file service.h * From 1ad1a31ed7a76ffe0345f7394202639df8094c1e Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 9 Jun 2015 17:18:25 +0300 Subject: [PATCH 36/54] Fixed the OpenSSL error stack being printed wrong. --- server/core/dcb.c | 35 +++++++++++++++++++++++++++++------ server/core/gateway.c | 2 +- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index cfee65371..a91645417 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1026,7 +1026,19 @@ int dcb_read_SSL( STRDCBSTATE(dcb->state), dcb->fd, ssl_errno, - errbuf))); + strerror(errno)))); + + if(ssl_errno == SSL_ERROR_SSL || + ssl_errno == SSL_ERROR_SYSCALL) + { + while((ssl_errno = ERR_get_error()) != 0) + { + ERR_error_string(ssl_errno,errbuf); + skygw_log_write(LE, + "%s", + errbuf); + } + } } gwbuf_free(buffer); @@ -2850,6 +2862,7 @@ int dcb_accept_SSL(DCB* dcb) break; case -1: + errnum = SSL_get_error(dcb->ssl,ssl_rval); if(errnum == SSL_ERROR_WANT_READ || errnum == SSL_ERROR_WANT_WRITE) @@ -2864,12 +2877,22 @@ int dcb_accept_SSL(DCB* dcb) else { rval = -1; - ERR_error_string(errnum,errbuf); - skygw_log_write_flush(LE, - "Error: Fatal error in SSL_accept for %s: (SSL error code: %d) %s", - dcb->remote, - errnum, + skygw_log_write(LE, + "Error: Fatal error in SSL_accept for %s: (SSL error code: %d):%s", + dcb->remote, + errnum, + strerror(errno)); + if(errnum == SSL_ERROR_SSL || + errnum == SSL_ERROR_SYSCALL) + { + while((errnum = ERR_get_error()) != 0) + { + ERR_error_string(errnum,errbuf); + skygw_log_write(LE, + "%s", errbuf); + } + } } break; diff --git a/server/core/gateway.c b/server/core/gateway.c index 67d5ecf62..794ed5304 100644 --- a/server/core/gateway.c +++ b/server/core/gateway.c @@ -2038,4 +2038,4 @@ static void maxscale_ssl_lock(int mode,int n,const char* file, int line) static unsigned long maxscale_ssl_id() { return (unsigned long)pthread_self(); -} \ No newline at end of file +} From 196d41cb88c7858a31f83a530a8a4712447a340a Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 9 Jun 2015 20:02:45 +0300 Subject: [PATCH 37/54] More debug output. --- server/core/dcb.c | 53 ++++++++++++++++---------- server/modules/protocol/mysql_client.c | 3 +- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index a91645417..0ce9ec053 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1005,9 +1005,18 @@ int dcb_read_SSL( char errbuf[200]; ssl_errno = SSL_get_error(dcb->ssl,n); #ifdef SS_DEBUG - ERR_error_string(ssl_errno,errbuf); - skygw_log_write_flush(LD,"[%lu]SSL error %d: %s", - pthread_self(),ssl_errno,errbuf); + if(ssl_errno == SSL_ERROR_SSL || + ssl_errno == SSL_ERROR_SYSCALL) + { + int eno; + while((eno = ERR_get_error()) != 0) + { + ERR_error_string(eno,errbuf); + skygw_log_write(LE, + "%s", + errbuf); + } + } #endif if(ssl_errno == SSL_ERROR_WANT_READ || ssl_errno == SSL_ERROR_WANT_WRITE || @@ -1017,7 +1026,6 @@ int dcb_read_SSL( } else { - ERR_error_string(ssl_errno,errbuf); LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, "Error : Read failed, dcb %p in state " @@ -1052,7 +1060,7 @@ int dcb_read_SSL( gwbuf_rtrim(buffer,bufsize - n); #ifdef SS_DEBUG - skygw_log_write(LD,"[%lu] SSL: Truncated buffer from %d to %d bytes. " + skygw_log_write(LD,"%lu SSL: Truncated buffer from %d to %d bytes. " "Read %d bytes, %d bytes waiting.\n",pthread_self(), bufsize,GWBUF_LENGTH(buffer),n,b); @@ -1080,13 +1088,6 @@ int dcb_read_SSL( rc = ioctl(dcb->fd, FIONREAD, &b); pending = SSL_pending(dcb->ssl); - if(ssl_errno == SSL_ERROR_WANT_READ || - ssl_errno == SSL_ERROR_WANT_WRITE || - (b == 0 && pending == 0)) - { - break; - } - } /*< while (true) */ return_n: return n; @@ -2837,22 +2838,34 @@ int dcb_create_SSL(DCB* dcb) */ int dcb_accept_SSL(DCB* dcb) { - int rval = 0,ssl_rval,errnum = 0,fd,b = 0; + int rval = 0,ssl_rval,errnum = 0,fd,b = 0,pending; char errbuf[140]; fd = dcb->fd; do { ssl_rval = SSL_accept(dcb->ssl); + errnum = SSL_get_error(dcb->ssl,ssl_rval); + LOGIF(LD,(skygw_log_write_flush(LD,"[dcb_accept_SSL] SSL_accept %d, error %d", + ssl_rval,errnum))); switch(ssl_rval) { case 0: errnum = SSL_get_error(dcb->ssl,ssl_rval); - ERR_error_string(errnum,errbuf); - LOGIF(LD,(skygw_log_write_flush(LD,"[%p] SSL_accept shutdown for %s:%s", + skygw_log_write(LE,"Error: SSL authentication failed (SSL error %d):", dcb, dcb->remote, - errbuf))); + errnum); + + if(errnum == SSL_ERROR_SSL || + errnum == SSL_ERROR_SYSCALL) + { + while((errnum = ERR_get_error()) != 0) + { + ERR_error_string(errnum,errbuf); + skygw_log_write(LE,"%s",errbuf); + } + } rval = -1; break; case 1: @@ -2871,7 +2884,7 @@ int dcb_accept_SSL(DCB* dcb) queue and wait for more.*/ rval = 0; LOGIF(LD,(skygw_log_write_flush(LD,"[dcb_accept_SSL] SSL_accept ongoing for %s", - dcb->remote))); + dcb->remote))); return rval; } else @@ -2904,11 +2917,11 @@ int dcb_accept_SSL(DCB* dcb) break; } ioctl(fd,FIONREAD,&b); + pending = SSL_pending(dcb->ssl); #ifdef SS_DEBUG - skygw_log_write_flush(LD,"[dcb_accept_SSL] fd %d: %d bytes",fd,b); - skygw_log_write(LD,"[dcb_accept_SSL] SSL_accept returned %d, SSL error: %d",ssl_rval,errnum); + skygw_log_write_flush(LD,"[dcb_accept_SSL] fd %d: %d bytes, %d pending",fd,b,pending); #endif - }while(b > 0 && rval != -1); + }while((b > 0 || pending > 0) && rval != -1); return rval; } diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index da5f04732..187c33910 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -1952,8 +1952,7 @@ int do_ssl_accept(MySQLProtocol* protocol) spinlock_release(&protocol->protocol_lock); rval = -1; skygw_log_write_flush(LE, - "Error: Fatal error in SSL_accept for %s@%s: %s", - protocol->owner_dcb->user, + "Error: Fatal error in SSL_accept for %s", protocol->owner_dcb->remote); break; From de2910f75b2164ba6c5b606b036f5c7e46fd8a90 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 9 Jun 2015 22:27:15 +0300 Subject: [PATCH 38/54] Fixed SSL_accept failing if the GWBUF with the initial MySQL auth packet contains some of the SSL authentication data. --- server/core/dcb.c | 144 +++++++++++++++++++++++++ server/include/dcb.h | 1 + server/modules/protocol/mysql_client.c | 6 +- 3 files changed, 150 insertions(+), 1 deletion(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 0ce9ec053..4c9dd0324 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -883,6 +883,150 @@ return_n: } +/** + * General purpose read routine to read data from a socket in the + * Descriptor Control Block and append it to a linked list of buffers. + * The list may be empty, in which case *head == NULL + * + * @param dcb The DCB to read from + * @param head Pointer to linked list to append data to + * @return -1 on error, otherwise the number of read bytes on the last + * iteration of while loop. 0 is returned if no data available. + */ +int dcb_read_n( + DCB *dcb, + GWBUF **head, + int nbytes) +{ + GWBUF *buffer = NULL; + int b; + int rc; + int n; + int nread = 0; + + CHK_DCB(dcb); + + if (dcb->fd <= 0) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Read failed, dcb is %s.", + dcb->fd == DCBFD_CLOSED ? "closed" : "cloned, not readable"))); + n = 0; + goto return_n; + } + + int bufsize; + + rc = ioctl(dcb->fd, FIONREAD, &b); + + if (rc == -1) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : ioctl FIONREAD for dcb %p in " + "state %s fd %d failed due error %d, %s.", + dcb, + STRDCBSTATE(dcb->state), + dcb->fd, + errno, + strerror(errno)))); + n = -1; + goto return_n; + } + + if (b == 0 && nread == 0) + { + /** Handle closed client socket */ + if (dcb_isclient(dcb)) + { + char c; + int l_errno = 0; + int r = -1; + + /* try to read 1 byte, without consuming the socket buffer */ + r = recv(dcb->fd, &c, sizeof(char), MSG_PEEK); + l_errno = errno; + + if (r <= 0 && + l_errno != EAGAIN && + l_errno != EWOULDBLOCK && + l_errno != 0) + { + n = -1; + goto return_n; + } + } + n = 0; + goto return_n; + } + else if (b == 0) + { + n = 0; + goto return_n; + } + + dcb->last_read = hkheartbeat; + + bufsize = MIN(b, nbytes); + + if ((buffer = gwbuf_alloc(bufsize)) == NULL) + { + /*< + * This is a fatal error which should cause shutdown. + * Todo shutdown if memory allocation fails. + */ + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Failed to allocate read buffer " + "for dcb %p fd %d, due %d, %s.", + dcb, + dcb->fd, + errno, + strerror(errno)))); + + n = -1; + goto return_n; + } + GW_NOINTR_CALL(n = read(dcb->fd, GWBUF_DATA(buffer), bufsize); + dcb->stats.n_reads++); + + if (n <= 0) + { + if (errno != 0 && errno != EAGAIN && errno != EWOULDBLOCK) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Read failed, dcb %p in state " + "%s fd %d, due %d, %s.", + dcb, + STRDCBSTATE(dcb->state), + dcb->fd, + errno, + strerror(errno)))); + } + gwbuf_free(buffer); + goto return_n; + } + nread += n; + + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [dcb_read] Read %d bytes from dcb %p in state %s " + "fd %d.", + pthread_self(), + n, + dcb, + STRDCBSTATE(dcb->state), + dcb->fd))); + /*< Append read data to the gwbuf */ + *head = gwbuf_append(*head, buffer); + +return_n: + return n; +} + + /** * General purpose read routine to read data from a socket in the * Descriptor Control Block and append it to a linked list of buffers. diff --git a/server/include/dcb.h b/server/include/dcb.h index cc96a2c0e..19f1e72ea 100644 --- a/server/include/dcb.h +++ b/server/include/dcb.h @@ -316,6 +316,7 @@ void dcb_free(DCB *); DCB *dcb_connect(struct server *, struct session *, const char *); DCB *dcb_clone(DCB *); int dcb_read(DCB *, GWBUF **); +int dcb_read_n(DCB*,GWBUF **,int); int dcb_drain_writeq(DCB *); void dcb_close(DCB *); DCB *dcb_process_zombies(int); /* Process Zombies except the one behind the pointer */ diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index 187c33910..15c572a0f 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -490,7 +490,6 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { /** Do the SSL Handshake */ if(ssl && protocol->owner_dcb->service->ssl_mode != SSL_DISABLED) { - protocol->protocol_auth_state = MYSQL_AUTH_SSL_REQ; if(do_ssl_accept(protocol) < 0) @@ -693,6 +692,11 @@ int gw_read_client_event( { rc = dcb_read_SSL(dcb, &read_buffer); } + else if(dcb->service->ssl_mode != SSL_DISABLED && + protocol->protocol_auth_state == MYSQL_AUTH_SENT) + { + rc = dcb_read_n(dcb, &read_buffer,(4 + 4 + 4 + 1 + 23)); + } else { rc = dcb_read(dcb, &read_buffer); From 3fb1213dee9ffc9759ecc3430109ea76323eb931 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 10 Jun 2015 06:09:42 +0300 Subject: [PATCH 39/54] Added more error logging when monitor scripts fail. --- server/modules/monitor/galeramon.c | 18 +++++++++++++----- server/modules/monitor/mmmon.c | 16 +++++++++++++--- server/modules/monitor/monitor_common.c | 3 +++ server/modules/monitor/mysql_mon.c | 16 +++++++++++++--- server/modules/monitor/ndbclustermon.c | 16 +++++++++++++--- 5 files changed, 55 insertions(+), 14 deletions(-) diff --git a/server/modules/monitor/galeramon.c b/server/modules/monitor/galeramon.c index 6a842b8c1..07d9fd848 100644 --- a/server/modules/monitor/galeramon.c +++ b/server/modules/monitor/galeramon.c @@ -123,7 +123,7 @@ startMonitor(void *arg,void* opt) MONITOR* mon = arg; GALERA_MONITOR *handle = mon->handle; CONFIG_PARAMETER* params = (CONFIG_PARAMETER*)opt; - bool have_events = false; + bool have_events = false,script_error = false; if (handle != NULL) { handle->shutdown = 0; @@ -163,6 +163,7 @@ startMonitor(void *arg,void* opt) } else { + script_error = true; if(access(params->value,F_OK) == 0) { skygw_log_write(LE, @@ -175,17 +176,24 @@ startMonitor(void *arg,void* opt) "Error: The file cannot be found: %s", params->value); } - handle->script = NULL; } } else if(!strcmp(params->name,"events")) { - mon_parse_event_string((bool*)&handle->events,sizeof(handle->events),params->value); - have_events = true; + if(mon_parse_event_string((bool*)&handle->events,sizeof(handle->events),params->value) != 0) + script_error = true; + else + have_events = true; } params = params->next; } - + if(script_error) + { + skygw_log_write(LE,"Error: Errors were found in the script configuration parameters " + "for the monitor '%s'. The script will not be used.",mon->name); + free(handle->script); + handle->script = NULL; + } /** If no specific events are given, enable them all */ if(!have_events) { diff --git a/server/modules/monitor/mmmon.c b/server/modules/monitor/mmmon.c index 130cdb279..c0d54595d 100644 --- a/server/modules/monitor/mmmon.c +++ b/server/modules/monitor/mmmon.c @@ -113,7 +113,7 @@ startMonitor(void *arg,void* opt) MONITOR* mon = (MONITOR*)arg; MM_MONITOR *handle = mon->handle; CONFIG_PARAMETER* params = (CONFIG_PARAMETER*)opt; - bool have_events = false; + bool have_events = false,script_error = false; if (handle) { @@ -148,6 +148,7 @@ startMonitor(void *arg,void* opt) } else { + script_error = true; if(access(params->value,F_OK) == 0) { skygw_log_write(LE, @@ -165,11 +166,20 @@ startMonitor(void *arg,void* opt) } else if(!strcmp(params->name,"events")) { - mon_parse_event_string((bool*)&handle->events,sizeof(handle->events),params->value); - have_events = true; + if(mon_parse_event_string((bool*)&handle->events,sizeof(handle->events),params->value) != 0) + script_error = true; + else + have_events = true; } params = params->next; } + if(script_error) + { + skygw_log_write(LE,"Error: Errors were found in the script configuration parameters " + "for the monitor '%s'. The script will not be used.",mon->name); + free(handle->script); + handle->script = NULL; + } /** If no specific events are given, enable them all */ if(!have_events) { diff --git a/server/modules/monitor/monitor_common.c b/server/modules/monitor/monitor_common.c index e4b8e7dac..f4566c35a 100644 --- a/server/modules/monitor/monitor_common.c +++ b/server/modules/monitor/monitor_common.c @@ -343,7 +343,10 @@ int mon_parse_event_string(bool* events, size_t count,char* string) { event = mon_name_to_event(tok); if(event == UNDEFINED_MONITOR_EVENT) + { + skygw_log_write(LE,"Error: Invalid event name %s",tok); return -1; + } events[event] = true; tok = strtok_r(NULL,",| ",&saved); } diff --git a/server/modules/monitor/mysql_mon.c b/server/modules/monitor/mysql_mon.c index 6f10b0ac8..2e16afc7b 100644 --- a/server/modules/monitor/mysql_mon.c +++ b/server/modules/monitor/mysql_mon.c @@ -140,7 +140,7 @@ startMonitor(void *arg, void* opt) MONITOR* monitor = (MONITOR*)arg; MYSQL_MONITOR *handle = (MYSQL_MONITOR*)monitor->handle; CONFIG_PARAMETER* params = (CONFIG_PARAMETER*)opt; - bool have_events = false; + bool have_events = false,script_error = false; if (handle) { @@ -176,6 +176,7 @@ startMonitor(void *arg, void* opt) } else { + script_error = true; if(access(params->value,F_OK) == 0) { skygw_log_write(LE, @@ -193,11 +194,20 @@ startMonitor(void *arg, void* opt) } else if(!strcmp(params->name,"events")) { - mon_parse_event_string(handle->events,sizeof(handle->events),params->value); - have_events = true; + if(mon_parse_event_string((bool*)&handle->events,sizeof(handle->events),params->value) != 0) + script_error = true; + else + have_events = true; } params = params->next; } + if(script_error) + { + skygw_log_write(LE,"Error: Errors were found in the script configuration parameters " + "for the monitor '%s'. The script will not be used.",monitor->name); + free(handle->script); + handle->script = NULL; + } /** If no specific events are given, enable them all */ if(!have_events) { diff --git a/server/modules/monitor/ndbclustermon.c b/server/modules/monitor/ndbclustermon.c index c8790be59..9561e275e 100644 --- a/server/modules/monitor/ndbclustermon.c +++ b/server/modules/monitor/ndbclustermon.c @@ -111,7 +111,7 @@ startMonitor(void *arg,void* opt) MONITOR* mon = (MONITOR*)arg; MYSQL_MONITOR *handle = mon->handle; CONFIG_PARAMETER* params = (CONFIG_PARAMETER*)opt; - bool have_events = false; + bool have_events = false,script_error = false; if (handle != NULL) { @@ -140,6 +140,7 @@ startMonitor(void *arg,void* opt) } else { + script_error = true; if(access(params->value,F_OK) == 0) { skygw_log_write(LE, @@ -157,10 +158,19 @@ startMonitor(void *arg,void* opt) } else if(!strcmp(params->name,"events")) { - mon_parse_event_string(&handle->events,sizeof(handle->events),params->value); - have_events = true; + if(mon_parse_event_string((bool*)&handle->events,sizeof(handle->events),params->value) != 0) + script_error = true; + else + have_events = true; } params = params->next; + } + if(script_error) + { + skygw_log_write(LE,"Error: Errors were found in the script configuration parameters " + "for the monitor '%s'. The script will not be used.",mon->name); + free(handle->script); + handle->script = NULL; } /** If no specific events are given, enable them all */ if(!have_events) From c15469013e89d31b9dd5878792d4013304716f90 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 10 Jun 2015 15:05:39 +0300 Subject: [PATCH 40/54] Fixed non-SSL connections to SSL enabled services failing. --- server/modules/protocol/mysql_client.c | 83 +++++++++++++++----------- 1 file changed, 48 insertions(+), 35 deletions(-) diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index 15c572a0f..dd6be0de2 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -74,7 +74,7 @@ int gw_MySQLWrite_client_SSL(DCB *dcb, GWBUF *queue); int gw_write_client_event_SSL(DCB *dcb); int mysql_send_ok(DCB *dcb, int packet_number, int in_affected_rows, const char* mysql_message); int MySQLSendHandshake(DCB* dcb); -static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue); +static int gw_mysql_do_authentication(DCB *dcb, GWBUF **queue); static int route_by_statement(SESSION *, GWBUF **); extern char* get_username_from_auth(char* ptr, uint8_t* data); extern int check_db_name_after_auth(DCB *, char *, int); @@ -402,7 +402,8 @@ MySQLSendHandshake(DCB* dcb) * @note in case of failure, dcb->data is freed before returning. If succeed, * dcb->data is freed in session.c:session_free. */ -static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { +static int gw_mysql_do_authentication(DCB *dcb, GWBUF **buf) { + GWBUF* queue = *buf; MySQLProtocol *protocol = NULL; /* int compress = -1; */ int connect_with_db = -1; @@ -464,46 +465,58 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { &protocol->client_capabilities); */ - if(protocol->protocol_auth_state == MYSQL_AUTH_SSL_HANDSHAKE_DONE) - goto ssl_hs_done; - - ssl = protocol->client_capabilities & GW_MYSQL_CAPABILITIES_SSL; - - /** Client didn't requested SSL when SSL mode was required*/ - if(!ssl && protocol->owner_dcb->service->ssl_mode == SSL_REQUIRED) + /** Skip this if the SSL handshake is already done. + * If not, start the SSL handshake. */ + if(protocol->protocol_auth_state != MYSQL_AUTH_SSL_HANDSHAKE_DONE) { - LOGIF(LT,(skygw_log_write(LT,"User %s@%s connected to service '%s' without SSL when SSL was required.", - protocol->owner_dcb->user, - protocol->owner_dcb->remote, - protocol->owner_dcb->service->name))); - return MYSQL_FAILED_AUTH_SSL; - } - if(LOG_IS_ENABLED(LT) && ssl) - { - skygw_log_write(LT,"User %s@%s connected to service '%s' with SSL.", - protocol->owner_dcb->user, - protocol->owner_dcb->remote, - protocol->owner_dcb->service->name); - } + ssl = protocol->client_capabilities & GW_MYSQL_CAPABILITIES_SSL; - /** Do the SSL Handshake */ - if(ssl && protocol->owner_dcb->service->ssl_mode != SSL_DISABLED) - { - protocol->protocol_auth_state = MYSQL_AUTH_SSL_REQ; - - if(do_ssl_accept(protocol) < 0) + /** Client didn't requested SSL when SSL mode was required*/ + if(!ssl && protocol->owner_dcb->service->ssl_mode == SSL_REQUIRED) { - return MYSQL_FAILED_AUTH; + LOGIF(LT,(skygw_log_write(LT,"User %s@%s connected to service '%s' without SSL when SSL was required.", + protocol->owner_dcb->user, + protocol->owner_dcb->remote, + protocol->owner_dcb->service->name))); + return MYSQL_FAILED_AUTH_SSL; } - else + + if(LOG_IS_ENABLED(LT) && ssl) { - return 0; + skygw_log_write(LT,"User %s@%s connected to service '%s' with SSL.", + protocol->owner_dcb->user, + protocol->owner_dcb->remote, + protocol->owner_dcb->service->name); + } + + /** Do the SSL Handshake */ + if(ssl && protocol->owner_dcb->service->ssl_mode != SSL_DISABLED) + { + protocol->protocol_auth_state = MYSQL_AUTH_SSL_REQ; + + if(do_ssl_accept(protocol) < 0) + { + return MYSQL_FAILED_AUTH; + } + else + { + return 0; + } + } + else if(dcb->service->ssl_mode == SSL_ENABLED) + { + /** This is a non-SSL connection to a SSL enabled service + * and we need to read the rest of the packet from the socket for the username */ + int bytes = dcb_read(dcb,&queue); + queue = gwbuf_make_contiguous(queue); + client_auth_packet = GWBUF_DATA(queue); + client_auth_packet_size = gwbuf_length(queue); + *buf = queue; + LOGIF(LD,(skygw_log_write(LD,"%lu Read %d bytes from fd %d",pthread_self(),bytes,dcb->fd))); } } - ssl_hs_done: - username = get_username_from_auth(username, client_auth_packet); if (username == NULL) @@ -848,7 +861,7 @@ int gw_read_client_event( { int auth_val; - auth_val = gw_mysql_do_authentication(dcb, read_buffer); + auth_val = gw_mysql_do_authentication(dcb, &read_buffer); if(protocol->protocol_auth_state == MYSQL_AUTH_SSL_REQ || protocol->protocol_auth_state == MYSQL_AUTH_SSL_HANDSHAKE_ONGOING || @@ -955,7 +968,7 @@ int gw_read_client_event( { int auth_val; - auth_val = gw_mysql_do_authentication(dcb, read_buffer); + auth_val = gw_mysql_do_authentication(dcb, &read_buffer); if (auth_val == 0) From 8ac79cf2adaec88704140731f3b93eddf48e6836 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 11 Jun 2015 10:28:35 +0300 Subject: [PATCH 41/54] Fixed some hard-coded paths being used instead of CMake variable values. --- etc/postinst.in | 11 ++++++----- server/include/gwdirs.h.in | 8 ++++---- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/etc/postinst.in b/etc/postinst.in index 82886990f..1a2c68ca3 100755 --- a/etc/postinst.in +++ b/etc/postinst.in @@ -2,11 +2,12 @@ # Create directories -mkdir -p @MAXSCALE_LIBDIR@ -mkdir -p @MAXSCALE_BINDIR@ -mkdir -p @MAXSCALE_SHAREDIR@ -mkdir -p @MAXSCALE_DOCDIR@ -mkdir -p @MAXSCALE_CONFDIR@ +mkdir -p @CMAKE_INSTALL_PREFIX@/@MAXSCALE_LIBDIR@ +mkdir -p @CMAKE_INSTALL_PREFIX@/@MAXSCALE_BINDIR@ +mkdir -p @CMAKE_INSTALL_PREFIX@/@MAXSCALE_SHAREDIR@ +mkdir -p @CMAKE_INSTALL_PREFIX@/@MAXSCALE_DOCDIR@ + +# MAXSCALE_VARDIR is an absolute path to /var by default mkdir -p @MAXSCALE_VARDIR@/log/maxscale mkdir -p @MAXSCALE_VARDIR@/lib/maxscale mkdir -p @MAXSCALE_VARDIR@/cache/maxscale diff --git a/server/include/gwdirs.h.in b/server/include/gwdirs.h.in index b8044484b..cf1f47987 100644 --- a/server/include/gwdirs.h.in +++ b/server/include/gwdirs.h.in @@ -24,11 +24,11 @@ /** Default file locations, configured by CMake */ static const char* default_cnf_fname = "maxscale.cnf"; static const char* default_configdir = "/etc/"; -static const char* default_piddir = "/var/run/maxscale/"; -static const char* default_logdir = "/var/log/maxscale/"; -static const char* default_datadir = "/var/lib/maxscale/"; +static const char* default_piddir = "@MAXSCALE_VARDIR@/run/maxscale/"; +static const char* default_logdir = "@MAXSCALE_VARDIR@/log/maxscale/"; +static const char* default_datadir = "@MAXSCALE_VARDIR@/lib/maxscale/"; static const char* default_libdir = "@CMAKE_INSTALL_PREFIX@/@MAXSCALE_LIBDIR@"; -static const char* default_cachedir = "/var/cache/maxscale/"; +static const char* default_cachedir = "@MAXSCALE_VARDIR@/cache/maxscale/"; static const char* default_langdir = "@MAXSCALE_VARDIR@/lib/maxscale/"; static char* configdir = NULL; From bb427128a9f3a80f3545943e676d5422137770f6 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 11 Jun 2015 12:00:03 +0300 Subject: [PATCH 42/54] Fixed successful SSL_accept calls causing another call to SSL_accept. --- server/core/dcb.c | 34 ++++++++++------ server/core/gateway.c | 90 ++++++++++++++++++++++++++++--------------- 2 files changed, 81 insertions(+), 43 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 4c9dd0324..e7c74b6bc 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1046,7 +1046,7 @@ int dcb_read_SSL( int rc; int n; int nread = 0; - + int ssl_errno = 0; CHK_DCB(dcb); if (dcb->fd <= 0) @@ -1062,7 +1062,7 @@ int dcb_read_SSL( while (true) { int bufsize; - int ssl_errno = 0; + ssl_errno = 0; rc = ioctl(dcb->fd, FIONREAD, &b); pending = SSL_pending(dcb->ssl); if (rc == -1) @@ -1096,9 +1096,9 @@ int dcb_read_SSL( if(ssl_errno != SSL_ERROR_WANT_READ && ssl_errno != SSL_ERROR_WANT_WRITE && ssl_errno != SSL_ERROR_NONE) - { n = -1; - } + else + n = 0; goto return_n; } } @@ -1192,7 +1192,7 @@ int dcb_read_SSL( } } } - + n = -1; gwbuf_free(buffer); goto return_n; } @@ -1595,7 +1595,7 @@ dcb_write_SSL(DCB *dcb, GWBUF *queue) if (w < 0) { - int ssl_errno = ERR_get_error(); + int ssl_errno = SSL_get_error(dcb->ssl,w); if (LOG_IS_ENABLED(LOGFILE_DEBUG)) { @@ -1633,6 +1633,17 @@ dcb_write_SSL(DCB *dcb, GWBUF *queue) dcb, STRDCBSTATE(dcb->state), dcb->fd,ssl_errno))); + if(ssl_errno == SSL_ERROR_SSL || + ssl_errno == SSL_ERROR_SYSCALL) + { + while((ssl_errno = ERR_get_error()) != 0) + { + char errbuf[140]; + ERR_error_string(ssl_errno,errbuf); + skygw_log_write(LE,"%s",errbuf); + } + } + break; } } @@ -3016,7 +3027,7 @@ int dcb_accept_SSL(DCB* dcb) rval = 1; LOGIF(LD,(skygw_log_write_flush(LD,"[dcb_accept_SSL] SSL_accept done for %s", dcb->remote))); - break; + return rval; case -1: @@ -3035,10 +3046,10 @@ int dcb_accept_SSL(DCB* dcb) { rval = -1; skygw_log_write(LE, - "Error: Fatal error in SSL_accept for %s: (SSL error code: %d):%s", + "Error: Fatal error in SSL_accept for %s: (SSL version: %s SSL error code: %d)", dcb->remote, - errnum, - strerror(errno)); + SSL_get_version(dcb->ssl), + errnum); if(errnum == SSL_ERROR_SSL || errnum == SSL_ERROR_SYSCALL) { @@ -3104,8 +3115,7 @@ int dcb_connect_SSL(DCB* dcb) case -1: errnum = SSL_get_error(dcb->ssl,rval); - if(errnum == SSL_ERROR_WANT_READ || errnum == SSL_ERROR_WANT_WRITE || - errnum == SSL_ERROR_WANT_X509_LOOKUP) + if(errnum == SSL_ERROR_WANT_READ || errnum == SSL_ERROR_WANT_WRITE) { /** Not all of the data has been read. Go back to the poll queue and wait for more.*/ diff --git a/server/core/gateway.c b/server/core/gateway.c index 794ed5304..1805e483d 100644 --- a/server/core/gateway.c +++ b/server/core/gateway.c @@ -40,7 +40,16 @@ * @endverbatim */ #define _XOPEN_SOURCE 700 +#define OPENSSL_THREAD_DEFINES #include + + #include + #if defined(OPENSSL_THREADS) +#define HAVE_OPENSSL_THREADS 1 + #else +#define HAVE_OPENSSL_THREADS 0 + #endif + #include #include #include @@ -196,9 +205,46 @@ static bool resolve_maxscale_conf_fname( static char* check_dir_access(char* dirname,bool,bool); static int set_user(); -static void maxscale_ssl_lock(int mode,int n,const char* file, int line); -static unsigned long maxscale_ssl_id(); -static SPINLOCK* ssl_locks; + +/** SSL multi-threading functions and structures */ + +struct CRYPTO_dynlock_value +{ + SPINLOCK lock; +}; + +static struct CRYPTO_dynlock_value *ssl_create_dynlock(const char* file, int line) +{ + struct CRYPTO_dynlock_value* lock = malloc(sizeof(struct CRYPTO_dynlock_value)); + if(lock) + { + spinlock_init(&lock->lock); + } + return lock; +} + +static void ssl_lock_dynlock(int mode,struct CRYPTO_dynlock_value * n,const char* file, int line) +{ + if(mode & CRYPTO_LOCK) + { + spinlock_acquire(&n->lock); + } + else + { + spinlock_release(&n->lock); + } +} + +static void ssl_free_dynlock(struct CRYPTO_dynlock_value * n,const char* file, int line) +{ + free(n); +} + +static void maxscale_ssl_id(CRYPTO_THREADID* id) +{ + CRYPTO_THREADID_set_numeric(id,pthread_self()); +} + /** * Handler for SIGHUP signal. Reload the configuration for the * gateway. @@ -1374,23 +1420,21 @@ int main(int argc, char **argv) } /** OpenSSL initialization */ - - SSL_library_init(); - SSL_load_error_strings(); - OPENSSL_add_all_algorithms_noconf(); - - int n_locks = CRYPTO_num_locks(); - if((ssl_locks = malloc(n_locks*sizeof(SPINLOCK))) == NULL) + if(!HAVE_OPENSSL_THREADS) { + char* logerr = "OpenSSL library does not support multi-threading"; + print_log_n_stderr(true, true, logerr, logerr, eno); rc = MAXSCALE_INTERNALERROR; goto return_main; } + SSL_library_init(); + SSL_load_error_strings(); + OPENSSL_add_all_algorithms_noconf(); + CRYPTO_set_dynlock_create_callback(ssl_create_dynlock); + CRYPTO_set_dynlock_destroy_callback(ssl_free_dynlock); + CRYPTO_set_dynlock_lock_callback(ssl_lock_dynlock); + CRYPTO_THREADID_set_callback(maxscale_ssl_id); - for(i = 0;i Date: Thu, 11 Jun 2015 13:22:18 +0300 Subject: [PATCH 43/54] Added RSA key generator. --- server/core/service.c | 72 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 67 insertions(+), 5 deletions(-) diff --git a/server/core/service.c b/server/core/service.c index 826c79e66..fbffd27bd 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -69,6 +69,9 @@ extern int lm_enabled_logfiles_bitmask; extern size_t log_ses_count[]; extern __thread log_info_t tls_log_info; +static RSA *rsa_512 = NULL; +static RSA *rsa_1024 = NULL; + /** To be used with configuration type checks */ typedef struct typelib_st { int tl_nelems; @@ -418,6 +421,17 @@ serviceStart(SERVICE *service) SERV_PROTOCOL *port; int listeners = 0; +if(service->ssl_mode != SSL_DISABLED) +{ + if(serviceInitSSL(service) != 0) + { + LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR, + "%s: SSL initialization failed. Service not started.", + service->name))); + service->state = SERVICE_STATE_FAILED; + return 0; + } +} if ((service->router_instance = service->router->createInstance(service, service->routerOptions)) == NULL) { @@ -1839,9 +1853,44 @@ int *data; } +/** + * + * @param s + * @param is_export + * @param keylength + * @return + */ + RSA *tmp_rsa_callback(SSL *s, int is_export, int keylength) + { + RSA *rsa_tmp=NULL; + + switch (keylength) { + case 512: + if (rsa_512) + rsa_tmp = rsa_512; + else { /* generate on the fly, should not happen in this example */ + rsa_tmp = RSA_generate_key(keylength,RSA_F4,NULL,NULL); + rsa_512 = rsa_tmp; /* Remember for later reuse */ + } + break; + case 1024: + if (rsa_1024) + rsa_tmp=rsa_1024; + break; + default: + /* Generating a key on the fly is very costly, so use what is there */ + if (rsa_1024) + rsa_tmp=rsa_1024; + else + rsa_tmp=rsa_512; /* Use at least a shorter key */ + } + return(rsa_tmp); + } + int serviceInitSSL(SERVICE* service) { DH* dh; + RSA* rsa; if(!service->ssl_init_done) { @@ -1878,12 +1927,21 @@ int serviceInitSSL(SERVICE* service) service->ctx = SSL_CTX_new(service->method); - /** Enable the Diffie-Hellman algorithms */ - if((dh = ssl_get_dh2236()) != NULL) + if(rsa_512 == NULL) { - SSL_CTX_set_tmp_dh(service->ctx,dh); - DH_free(dh); + rsa_512 = RSA_generate_key(512,RSA_F4,NULL,NULL); + if (rsa_512 == NULL) + skygw_log_write(LE,"Error: 512-bit RSA key generation failed."); } + if(rsa_1024 == NULL) + { + rsa_1024 = RSA_generate_key(1024,RSA_F4,NULL,NULL); + if (rsa_1024 == NULL) + skygw_log_write(LE,"Error: 1024-bit RSA key generation failed."); + } + + if(rsa_512 != NULL && rsa_1024 != NULL) + SSL_CTX_set_tmp_rsa_callback(service->ctx,tmp_rsa_callback); if (SSL_CTX_use_certificate_file(service->ctx, service->ssl_cert, SSL_FILETYPE_PEM) <= 0) { skygw_log_write(LE,"Error: Failed to set server SSL certificate."); @@ -1919,6 +1977,10 @@ int serviceInitSSL(SERVICE* service) return 0; } +/** + * Generated by OpenSSL. + * @return + */ DH *ssl_get_dh2236() { static unsigned char dh2236_p[]={ @@ -1958,4 +2020,4 @@ DH *ssl_get_dh2236() if ((dh->p == NULL) || (dh->g == NULL)) { DH_free(dh); return(NULL); } return(dh); -} \ No newline at end of file +} From 3f34d237cae6a367ac20c448d91e8d307e79e998 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 11 Jun 2015 15:26:05 +0300 Subject: [PATCH 44/54] enabled all bug fixes for OpenSSL. --- server/core/service.c | 1 + 1 file changed, 1 insertion(+) diff --git a/server/core/service.c b/server/core/service.c index fbffd27bd..bd09f06ee 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -1926,6 +1926,7 @@ int serviceInitSSL(SERVICE* service) } service->ctx = SSL_CTX_new(service->method); + SSL_CTX_set_options(service->ctx,SSL_OP_ALL); if(rsa_512 == NULL) { From 1c68a9a8729d625e495a6bf133f6c194d9b8dfaf Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 11 Jun 2015 15:54:09 +0300 Subject: [PATCH 45/54] Fixed dcb_connect_SSL calling SSL_connect again after a successful connection was already made. --- server/core/dcb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index e7c74b6bc..51dbb412f 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -3110,7 +3110,7 @@ int dcb_connect_SSL(DCB* dcb) LOGIF(LD,(skygw_log_write_flush(LD,"SSL_connect done for %s@%s", dcb->user, dcb->remote))); - break; + return rval; case -1: errnum = SSL_get_error(dcb->ssl,rval); From f24da8712b4c4cbf5c1bf731a6133cae39d62245 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 11 Jun 2015 18:25:42 +0300 Subject: [PATCH 46/54] Fixed a segfault and disabled syslog by default. --- server/core/gateway.c | 46 +++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/server/core/gateway.c b/server/core/gateway.c index 9845713e1..9d0e6f908 100644 --- a/server/core/gateway.c +++ b/server/core/gateway.c @@ -935,7 +935,7 @@ int main(int argc, char **argv) char* tmp_var; int option_index; int logtofile = 0; /* Use shared memory or file */ - int syslog_enabled = 1; /** Log to syslog */ + int syslog_enabled = 0; /** Log to syslog */ int maxscalelog_enabled = 1; /** Log with MaxScale */ ssize_t log_flush_timeout_ms = 0; sigset_t sigset; @@ -1078,26 +1078,34 @@ int main(int argc, char **argv) } break; case 'S': - if(strstr(optarg,"=")) - { - strtok(optarg,"= "); - maxscalelog_enabled = config_truth_value(strtok(NULL,"= ")); - } - else - { - maxscalelog_enabled = config_truth_value(optarg); - } + { + char* tok = strstr(optarg,"="); + if(tok) + { + tok++; + if(tok) + maxscalelog_enabled = config_truth_value(tok); + } + else + { + maxscalelog_enabled = config_truth_value(optarg); + } + } break; case 's': - if(strstr(optarg,"=")) - { - strtok(optarg,"= "); - syslog_enabled = config_truth_value(strtok(NULL,"= ")); - } - else - { - syslog_enabled = config_truth_value(optarg); - } + { + char* tok = strstr(optarg,"="); + if(tok) + { + tok++; + if(tok) + syslog_enabled = config_truth_value(tok); + } + else + { + syslog_enabled = config_truth_value(optarg); + } + } break; case 'U': if(set_user(optarg) != 0) From 2b2e81feb26665b938eb4acb8a60adb124bb6fdc Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 11 Jun 2015 19:05:05 +0300 Subject: [PATCH 47/54] Fix to MXS-181: https://mariadb.atlassian.net/browse/MXS-181 Added TCP_NODELAY to socket options. --- server/modules/protocol/mysql_client.c | 4 ++++ server/modules/protocol/mysql_common.c | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index abdb4422c..ea0b05b79 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -46,6 +46,7 @@ #include #include #include +#include MODULE_INFO info = { MODULE_API_PROTOCOL, @@ -1064,6 +1065,9 @@ int gw_MySQLListener( LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR,"Error: Failed to set socket options. Error %d: %s",errno,strerror(errno)))); } + if((syseno = setsockopt(l_so, IPPROTO_TCP, TCP_NODELAY, (char *)&one, sizeof(one))) != 0){ + LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR,"Error: Failed to set socket options. Error %d: %s",errno,strerror(errno)))); + } // set NONBLOCKING mode setnonblocking(l_so); diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index ffd72c034..0a7f22f5e 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -44,6 +44,7 @@ #include #include #include +#include /** Defined in log_manager.cc */ extern int lm_enabled_logfiles_bitmask; @@ -812,6 +813,23 @@ int gw_do_connect_to_backend( goto close_so; } + int one = 1; + if(setsockopt(so, IPPROTO_TCP, TCP_NODELAY, &one, sizeof(one)) != 0) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error: Failed to set socket options " + "%s:%d failed.\n\t\t Socket configuration failed " + "due %d, %s.", + host, + port, + errno, + strerror(errno)))); + rv = -1; + /** Close socket */ + goto close_so; + } + /* set socket to as non-blocking here */ setnonblocking(so); rv = connect(so, (struct sockaddr *)&serv_addr, sizeof(serv_addr)); From 6f0e3937eba9e4466a9b4645deca9064c76cde47 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 11 Jun 2015 19:43:22 +0300 Subject: [PATCH 48/54] Added missing include to gwdirs.h. --- server/include/gwdirs.h.in | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/include/gwdirs.h.in b/server/include/gwdirs.h.in index cf1f47987..fe911c71b 100644 --- a/server/include/gwdirs.h.in +++ b/server/include/gwdirs.h.in @@ -18,9 +18,11 @@ * * Copyright MariaDB Corporation Ab 2015 */ - +#ifndef _GNU_SOURCE +#define _GNU_SOURCE 1 +#endif #include - +#include /** Default file locations, configured by CMake */ static const char* default_cnf_fname = "maxscale.cnf"; static const char* default_configdir = "/etc/"; From fe2062b5b09d18f4bfb078f646fd941bdd09338b Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 11 Jun 2015 19:50:51 +0300 Subject: [PATCH 49/54] Fixed a regression in mysql_mon.c which caused a memory leak --- server/modules/monitor/mysql_mon.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/modules/monitor/mysql_mon.c b/server/modules/monitor/mysql_mon.c index 2e16afc7b..e310e2ede 100644 --- a/server/modules/monitor/mysql_mon.c +++ b/server/modules/monitor/mysql_mon.c @@ -323,6 +323,8 @@ char *server_string; int read_timeout = mon->read_timeout; int write_timeout = mon->write_timeout; + if(database->con) + mysql_close(database->con); database->con = mysql_init(NULL); mysql_options(database->con, MYSQL_OPT_CONNECT_TIMEOUT, (void *)&connect_timeout); From 68d5054afe3977717af708834b1fa15d50c44963 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 11 Jun 2015 20:58:27 +0300 Subject: [PATCH 50/54] dcb_alloc now explicitly sets the server and service pointers to NULL. --- server/core/dcb.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 6717aea41..128ffed21 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -193,7 +193,8 @@ DCB *rval; rval->polloutbusy = 0; rval->writecheck = 0; rval->fd = DCBFD_CLOSED; - + rval->server = NULL; + rval->service = NULL; rval->evq.next = NULL; rval->evq.prev = NULL; rval->evq.pending_events = 0; From 521e1aaf3b9126a7c21c31219871f3eb25f959e8 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 12 Jun 2015 02:44:43 +0300 Subject: [PATCH 51/54] Added man page for maxscale. --- CMakeLists.txt | 1 + Documentation/maxscale.1 | 69 ++++++++++++++++++++++++++++++++++++++++ etc/postinst.in | 2 +- 3 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 Documentation/maxscale.1 diff --git a/CMakeLists.txt b/CMakeLists.txt index 4f35031d8..a21e9c745 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -159,6 +159,7 @@ install(FILES ${ERRMSG} DESTINATION ${MAXSCALE_VARDIR}/lib/maxscale install(FILES ${CMAKE_SOURCE_DIR}/COPYRIGHT DESTINATION ${MAXSCALE_SHAREDIR}) install(FILES ${CMAKE_SOURCE_DIR}/README DESTINATION ${MAXSCALE_SHAREDIR}) install(FILES ${CMAKE_SOURCE_DIR}/LICENSE DESTINATION ${MAXSCALE_SHAREDIR}) +install(FILES Documentation/maxscale.1 DESTINATION ${CMAKE_INSTALL_DATADIR}/man/man1) # Install startup scripts and ldconfig files if(WITH_SCRIPTS) diff --git a/Documentation/maxscale.1 b/Documentation/maxscale.1 new file mode 100644 index 000000000..104eaa235 --- /dev/null +++ b/Documentation/maxscale.1 @@ -0,0 +1,69 @@ +.TH maxscale 1 +.SH NAME +maxscale - The intelligent proxy +.SH SYNOPSIS +.B maxscale +[\fIOPTIONS...\fR] +.SH DESCRIPTION +The MariaDB Corporation MaxScale is an intelligent proxy that allows forwarding of +database statements to one or more database servers using complex rules, +a semantic understanding of the database statements and the roles of +the various servers within the backend cluster of databases. + +MaxScale is designed to provide load balancing and high availability +functionality transparently to the applications. In addition it provides +a highly scalable and flexible architecture, with plugin components to +support different protocols and routing decisions. + +.SH OPTIONS +.TP +.BR "-d, --nodaemon" +Run MaxScale in the terminal process +.TP +.BR -f " \fIFILE\fB, --config=\fIFILE\fR" +Relative or absolute pathname of MaxScale configuration file to load. +.TP +.BR -l "[\fIfile|shm\fB], --log=[\fIfile|shm\fB]" +Log trace and debug logs to file or shared memory. The debug and trace logs are disabled by default and if enabled, will log to shared memory. +.TP +.BR -L " \fIPATH\fB, --logdir=\fIPATH\fB" +Path to log file directory. +.TP +.BR -D " \fIPATH\fB, --datadir=\fIPATH\fB" +Path to data directory. This is where the embedded mysql tables are stored in addition to other MaxScale specific data. +.TP +.BR -C " \fIPATH\fB, --configdir=\fIPATH\fB" +Path to configuration file directory. MaxScale will look for the \fImaxscale.cnf\fR file from this folder. +.TP +.BR -B " \fIPATH\fB, --libdir=\fIPATH\fB" +Path to module directory. Modules are only searched from this folder. +.TP +.BR -A " \fIPATH\fB, --cachedir=\fIPATH\fB" +Path to cache directory. This is where MaxScale stores cached authentication data. +.TP +.BR -P " \fIPATH\fB, --piddir=\fIPATH\fB" +Location of MaxScale's PID file. +.TP +.BR -U " \fIUSER\fB, --user=\fIUSER\fB" +Run MaxScale as another user. The user ID and group ID of this user are used to run MaxScale. +.TP +.BR -s " [\fIyes\fB|\fIno\fB], --syslog=[\fIyes\fB|\fIno\fB]" +Log messages to syslog. +.TP +.BR -S " [\fIyes\fB|\fIno\fB], \fB--maxscalelog=[\fIyes\fB|\fIno\fB]" +Log messages to MaxScale's own log files. +.TP +.BR "-v, --version" +Print version information and exit. +.TP +.BR "-?, --help" +Show the help information for MaxScale and exit. + +.SH EXAMPLES +Tutorials on GitHub: +.UR https://github.com/mariadb-corporation/MaxScale/blob/master/Documentation/Documentation-Contents.md#tutorials +.UE +.SH SEE ALSO +The MaxScale documentation on GitHub: +.UR https://github.com/mariadb-corporation/MaxScale/blob/master/Documentation/Documentation-Contents.md +.UE diff --git a/etc/postinst.in b/etc/postinst.in index 1a2c68ca3..e1d3dc8ad 100755 --- a/etc/postinst.in +++ b/etc/postinst.in @@ -38,7 +38,7 @@ then cp @CMAKE_INSTALL_PREFIX@/@MAXSCALE_SHAREDIR@/maxscale.service /usr/lib/systemd/system fi /sbin/ldconfig - +mandb cat <& 2 ********** Notice: MaxScale 1.2 Changes ************** From d3cc9be52ec445602180abcfb78ccaf226ef4b5d Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 12 Jun 2015 10:05:50 +0300 Subject: [PATCH 52/54] Added libcurl-devel and pcre-devel to build dependencies. --- .../Getting-Started/Building-MaxScale-from-Source-Code.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/Getting-Started/Building-MaxScale-from-Source-Code.md b/Documentation/Getting-Started/Building-MaxScale-from-Source-Code.md index f83a6e3ae..95236ed7e 100644 --- a/Documentation/Getting-Started/Building-MaxScale-from-Source-Code.md +++ b/Documentation/Getting-Started/Building-MaxScale-from-Source-Code.md @@ -28,7 +28,7 @@ You will need to install all of the following packages for all versions of RHEL, ``` gcc gcc-c++ ncurses-devel bison glibc-devel cmake libgcc perl make libtool -openssl-devel libaio libaio-devel librabbitmq-devel +openssl-devel libaio libaio-devel librabbitmq-devel libcurl-devel pcre-devel ``` In addition, if you wish to to build an RPM package include: @@ -68,7 +68,7 @@ These packages are required on all versions of Ubuntu and Debian. ``` build-essential libssl-dev libaio-dev ncurses-dev bison - cmake perl libtool librabbitmq-dev + cmake perl libtool librabbitmq-dev libcurl-dev libpcre3-dev ``` If you want to build a DEB package, you will also need: From f602121459b021c4f597229add2dffbd2851aa34 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 12 Jun 2015 21:21:06 +0300 Subject: [PATCH 53/54] Added configurable SSL certificate verification depth and updated the documentation in the code. --- .../Getting-Started/Configuration-Guide.md | 9 ++ Documentation/Reference/MaxScale-and-SSL.md | 7 +- server/core/config.c | 11 ++ server/core/dcb.c | 13 +- server/core/gateway.c | 27 ++++ server/core/service.c | 120 ++++++++++-------- server/include/service.h | 13 +- server/modules/protocol/mysql_client.c | 53 ++++---- 8 files changed, 163 insertions(+), 90 deletions(-) diff --git a/Documentation/Getting-Started/Configuration-Guide.md b/Documentation/Getting-Started/Configuration-Guide.md index 23a9df70d..113e28c60 100644 --- a/Documentation/Getting-Started/Configuration-Guide.md +++ b/Documentation/Getting-Started/Configuration-Guide.md @@ -356,6 +356,15 @@ This parameter controls the level of encryption used. Accepted values are: * TLSv12 * MAX +### `ssl_cert_verification_depth` + +The maximum length of the certificate authority chain that will be accepted. Accepted values are positive integers. + +``` +# Example +ssl_cert_verification_depth=10 +``` + Example SSL enabled service configuration: ``` diff --git a/Documentation/Reference/MaxScale-and-SSL.md b/Documentation/Reference/MaxScale-and-SSL.md index ca61d52e2..d03a5af52 100644 --- a/Documentation/Reference/MaxScale-and-SSL.md +++ b/Documentation/Reference/MaxScale-and-SSL.md @@ -8,7 +8,8 @@ Here are the options which relate to SSL and certificates. Parameter|Values |Description ---------|-----------|-------- ssl | disabled, enabled, required |`disable` disables SSL, `enabled` enables SSL for client connections but still allows non-SSL connections and `required` requires SSL from all client connections. With the `required` option, client connections that do not use SSL will be rejected. -ssl_cert | |Path to server certificate -ssl_key | |Path to server private key -ssl_ca_cert | |Path to Certificate Authority file +ssl_cert | path to file |Path to server certificate +ssl_key | path to file |Path to server private key +ssl_ca_cert | path to file |Path to Certificate Authority file ssl_version|SSLV2,SSLV3,TLSV10,TLSV11,TLSV12,MAX| The SSL method level, defaults to highest available encryption level which is TLSv1.2 +ssl_cert_verify_depth|integer|Certificate authority certificate verification depth, default is 100. diff --git a/server/core/config.c b/server/core/config.c index 40be2c704..3e778d077 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -346,6 +346,7 @@ hashtable_memory_fns(monitorhash,strdup,NULL,free,NULL); char *version_string; char *subservices; char *ssl,*ssl_cert,*ssl_key,*ssl_ca_cert,*ssl_version; + char* ssl_cert_verify_depth; bool is_rwsplit = false; bool is_schemarouter = false; char *allow_localhost_match_wildcard_host; @@ -359,6 +360,7 @@ hashtable_memory_fns(monitorhash,strdup,NULL,free,NULL); ssl_key = config_get_value(obj->parameters, "ssl_key"); ssl_ca_cert = config_get_value(obj->parameters, "ssl_ca_cert"); ssl_version = config_get_value(obj->parameters, "ssl_version"); + ssl_cert_verify_depth = config_get_value(obj->parameters, "ssl_cert_verify_depth"); enable_root_user = config_get_value( obj->parameters, "enable_root_user"); @@ -514,6 +516,14 @@ hashtable_memory_fns(monitorhash,strdup,NULL,free,NULL); error_count++; } } + if(ssl_cert_verify_depth) + { + if(serviceSetSSLVerifyDepth(obj->element,atoi(ssl_cert_verify_depth)) != 0) + { + skygw_log_write(LE,"Error: Invalid parameter value for 'ssl_cert_verify_depth' for service '%s': %s",obj->object,ssl_cert_verify_depth); + error_count++; + } + } } } @@ -2005,6 +2015,7 @@ static char *service_params[] = "ssl", "ssl_key", "ssl_version", + "ssl_cert_verify_depth", NULL }; diff --git a/server/core/dcb.c b/server/core/dcb.c index 05114f48f..f600239ca 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -887,10 +887,13 @@ return_n: /** * General purpose read routine to read data from a socket in the * Descriptor Control Block and append it to a linked list of buffers. - * The list may be empty, in which case *head == NULL + * This function will read at most nbytes of data. + * + * The list may be empty, in which case *head == NULL. This * * @param dcb The DCB to read from * @param head Pointer to linked list to append data to + * @param nbytes Maximum number of bytes read * @return -1 on error, otherwise the number of read bytes on the last * iteration of while loop. 0 is returned if no data available. */ @@ -1835,7 +1838,8 @@ int above_water; /** * Drain the write queue of a DCB. This is called as part of the EPOLLOUT handling * of a socket and will try to send any buffered data from the write queue - * up until the point the write would block. + * up until the point the write would block. This function uses SSL encryption + * and the SSL handshake should have been completed prior to calling this function. * * @param dcb DCB to drain the write queue of * @return The number of bytes written @@ -2495,9 +2499,10 @@ static bool dcb_set_state_nomutex( } /** - * Write data to a DCB + * Write data to a socket through an SSL structure. The SSL structure is linked to a DCB's socket + * and all communication is encrypted and done via the SSL structure. * - * @param ssl The SSL to write the buffer to + * @param ssl The SSL structure to use for writing * @param buf Buffer to write * @param nbytes Number of bytes to write * @return Number of written bytes diff --git a/server/core/gateway.c b/server/core/gateway.c index 412714fbb..21c4472c0 100644 --- a/server/core/gateway.c +++ b/server/core/gateway.c @@ -208,11 +208,21 @@ static int set_user(); /** SSL multi-threading functions and structures */ +/** + * OpenSSL requires this struct to be defined in order to use dynamic locks + */ struct CRYPTO_dynlock_value { SPINLOCK lock; }; +/** + * Create a dynamic OpenSSL lock. The dynamic lock is just a wrapper structure + * around a SPINLOCK structure. + * @param file File name + * @param line Line number + * @return Pointer to new lock or NULL of an error occurred + */ static struct CRYPTO_dynlock_value *ssl_create_dynlock(const char* file, int line) { struct CRYPTO_dynlock_value* lock = malloc(sizeof(struct CRYPTO_dynlock_value)); @@ -223,6 +233,13 @@ static struct CRYPTO_dynlock_value *ssl_create_dynlock(const char* file, int lin return lock; } +/** + * Lock a dynamic lock for OpenSSL. + * @param mode + * @param n pointer to lock + * @param file File name + * @param line Line number + */ static void ssl_lock_dynlock(int mode,struct CRYPTO_dynlock_value * n,const char* file, int line) { if(mode & CRYPTO_LOCK) @@ -235,11 +252,21 @@ static void ssl_lock_dynlock(int mode,struct CRYPTO_dynlock_value * n,const char } } +/** + * Free a dynamic OpenSSL lock. + * @param n Lock to free + * @param file File name + * @param line Line number + */ static void ssl_free_dynlock(struct CRYPTO_dynlock_value * n,const char* file, int line) { free(n); } +/** + * The thread ID callback function for OpenSSL dynamic locks. + * @param id Id to modify + */ static void maxscale_ssl_id(CRYPTO_THREADID* id) { CRYPTO_THREADID_set_numeric(id,pthread_self()); diff --git a/server/core/service.c b/server/core/service.c index bd09f06ee..4215ce725 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -94,7 +94,7 @@ static SPINLOCK service_spin = SPINLOCK_INIT; static SERVICE *allServices = NULL; static int find_type(typelib_t* tl, const char* needle, int maxlen); -DH *ssl_get_dh2236(); + static void service_add_qualified_param( SERVICE* svc, CONFIG_PARAMETER* param); @@ -144,7 +144,8 @@ SERVICE *service; service->ssl_ca_cert = NULL; service->ssl_cert = NULL; service->ssl_key = NULL; - /** Use the highest possible SSL/TLS methods available */ + service->ssl_cert_verify_depth = DEFAULT_SSL_CERT_VERIFY_DEPTH; + /** Support the highest possible SSL/TLS methods available as the default */ service->ssl_method_type = SERVICE_SSL_TLS_MAX; if (service->name == NULL || service->routerModule == NULL) { @@ -875,6 +876,14 @@ serviceOptimizeWildcard(SERVICE *service, int action) return 1; } +/** + * Set the locations of the server's SSL certificate, server's private key and the CA + * certificate which both the client and the server should trust. + * @param service Service to configure + * @param cert SSL certificate + * @param key SSL private key + * @param ca_cert SSL CA certificate + */ void serviceSetCertificates(SERVICE *service, char* cert,char* key, char* ca_cert) { @@ -891,6 +900,12 @@ serviceSetCertificates(SERVICE *service, char* cert,char* key, char* ca_cert) service->ssl_ca_cert = strdup(ca_cert); } +/** + * Set the maximum SSL/TLS version the service will support + * @param service Service to configure + * @param version SSL/TLS version string + * @return 0 on success, -1 on invalid version string + */ int serviceSetSSLVersion(SERVICE *service, char* version) { @@ -909,7 +924,34 @@ serviceSetSSLVersion(SERVICE *service, char* version) else return -1; return 0; } -/** Enable or disable the service SSL capability*/ + +/** + * Set the service's SSL certificate verification depth. Depth of 0 means the peer + * certificate, 1 is the CA and 2 is a higher CA and so on. + * @param service Service to configure + * @param depth Certificate verification depth + * @return 0 on success, -1 on incorrect depth value + */ +int serviceSetSSLVerifyDepth(SERVICE* service, int depth) +{ + if(depth < 0) + return -1; + + service->ssl_cert_verify_depth = depth; + return 0; +} + +/** + * Enable or disable the service SSL capability of a service. + * The SSL mode string passed as a parameter should be one of required, enabled + * or disabled. Required requires all connections to use SSL encryption, enabled + * allows both SSL and non-SSL connections and disabled does not use SSL encryption. + * If the service SSL mode is set to enabled, then the client will decide whether + * SSL encryption is used. + * @param service Service to configure + * @param action Mode string. One of required, enabled or disabled. + * @return 0 on success, -1 on error + */ int serviceSetSSL(SERVICE *service, char* action) { @@ -1854,11 +1896,11 @@ int *data; /** - * - * @param s - * @param is_export - * @param keylength - * @return + * The RSA ket generation callback function for OpenSSL. + * @param s SSL structure + * @param is_export Not used + * @param keylength Length of the key + * @return Pointer to RSA structure */ RSA *tmp_rsa_callback(SSL *s, int is_export, int keylength) { @@ -1887,6 +1929,13 @@ int *data; return(rsa_tmp); } + /** + * Initialize the servce's SSL context. This sets up the generated RSA + * encryption keys, chooses the server encryption level and configures the server + * certificate, private key and certificate authority file. + * @param service + * @return + */ int serviceInitSSL(SERVICE* service) { DH* dh; @@ -1911,6 +1960,8 @@ int serviceInitSSL(SERVICE* service) case SERVICE_TLS12: service->method = (SSL_METHOD*)TLSv1_2_server_method(); break; + + /** Rest of these use the maximum available SSL/TLS methods */ case SERVICE_SSL_MAX: service->method = (SSL_METHOD*)SSLv23_server_method(); break; @@ -1926,8 +1977,11 @@ int serviceInitSSL(SERVICE* service) } service->ctx = SSL_CTX_new(service->method); + + /** Enable all OpenSSL bug fixes */ SSL_CTX_set_options(service->ctx,SSL_OP_ALL); + /** Generate the 512-bit and 1024-bit RSA keys */ if(rsa_512 == NULL) { rsa_512 = RSA_generate_key(512,RSA_F4,NULL,NULL); @@ -1944,6 +1998,7 @@ int serviceInitSSL(SERVICE* service) if(rsa_512 != NULL && rsa_1024 != NULL) SSL_CTX_set_tmp_rsa_callback(service->ctx,tmp_rsa_callback); + /** Load the server sertificate */ if (SSL_CTX_use_certificate_file(service->ctx, service->ssl_cert, SSL_FILETYPE_PEM) <= 0) { skygw_log_write(LE,"Error: Failed to set server SSL certificate."); return -1; @@ -1971,54 +2026,9 @@ int serviceInitSSL(SERVICE* service) /* Set to require peer (client) certificate verification */ SSL_CTX_set_verify(service->ctx,SSL_VERIFY_PEER,NULL); - /* Set the verification depth to 1 */ - SSL_CTX_set_verify_depth(service->ctx,1); + /* Set the verification depth */ + SSL_CTX_set_verify_depth(service->ctx,service->ssl_cert_verify_depth); service->ssl_init_done = true; } return 0; } - -/** - * Generated by OpenSSL. - * @return - */ -DH *ssl_get_dh2236() -{ - static unsigned char dh2236_p[]={ - 0x0B,0xC3,0xEC,0x3F,0xCB,0xD0,0x2E,0x43,0x7B,0x13,0xF9,0x0C, - 0x4D,0xE5,0xA3,0xA4,0xDB,0x68,0x13,0xBD,0xFC,0xD2,0x35,0x05, - 0xCB,0x62,0xA1,0x85,0x33,0x20,0xC4,0x88,0x3B,0x2B,0xD5,0x76, - 0x94,0xCD,0xEB,0x9C,0x5A,0xD1,0x16,0xDB,0x51,0x82,0x7A,0x1E, - 0xC6,0xC3,0xD9,0x52,0x8F,0x54,0x33,0xF4,0x50,0x96,0x01,0xF4, - 0x71,0xA1,0x8B,0x9B,0x43,0x85,0x9C,0x95,0xFF,0x53,0x1D,0x8D, - 0xDF,0xBC,0x60,0xEB,0x4D,0x96,0xD1,0x05,0x98,0x4A,0xEB,0xC9, - 0x33,0xF6,0xE9,0x74,0x73,0x29,0x27,0xCA,0x0D,0x6D,0xEA,0x36, - 0xB9,0x3B,0x54,0xF6,0x34,0x68,0x13,0xFA,0xAC,0x3B,0x57,0x55, - 0x76,0x41,0x67,0x48,0xEF,0x3C,0xE1,0xE1,0xAF,0x3C,0x68,0x05, - 0x9C,0x32,0xD9,0x14,0x8F,0xB2,0xEE,0xEE,0xBA,0x9F,0x0D,0x75, - 0xA7,0x33,0x1F,0x3A,0x0E,0xD1,0xA6,0x5A,0x29,0xC7,0x9B,0x5E, - 0x46,0xB1,0xA6,0xA5,0x1E,0x32,0xDB,0xAF,0x23,0x83,0x94,0x12, - 0x4F,0xE4,0xC2,0x8B,0x1B,0x2C,0x01,0x79,0x92,0x21,0xFF,0x01, - 0xED,0x46,0x27,0xF0,0x70,0x2A,0xA1,0xFD,0x5C,0x8F,0x8B,0x0C, - 0xC6,0x8F,0xFF,0x4C,0x99,0xAE,0x19,0xDB,0x58,0x4C,0xC0,0xE8, - 0x70,0xCC,0x7C,0x17,0xE8,0xBD,0x6B,0x19,0x93,0xB9,0x66,0xA9, - 0xD0,0x05,0x21,0x04,0x4C,0x7E,0x87,0x9F,0xF4,0xE9,0x23,0xE1, - 0x29,0x37,0xC5,0xE2,0x0A,0xC5,0xC1,0x92,0xC7,0x69,0xB4,0xFB, - 0x84,0x06,0xCE,0x0E,0xFC,0x65,0x70,0x2F,0xF6,0xB8,0x11,0xF9, - 0x0F,0x60,0x10,0xCA,0x94,0x29,0x44,0x5E,0x4A,0x05,0x46,0xE5, - 0xE6,0xA0,0xBD,0x14,0x45,0xA6,0xA7,0xCA,0x63,0x57,0xC6,0xB0, - 0x47,0xF9,0x71,0x24,0x19,0x75,0xD2,0x64,0x16,0xB1,0xBA,0x08, - 0xE9,0xE9,0xFB,0xF3, - }; - static unsigned char dh2236_g[]={ - 0x02, - }; - DH *dh; - - if ((dh=DH_new()) == NULL) return(NULL); - dh->p=BN_bin2bn(dh2236_p,sizeof(dh2236_p),NULL); - dh->g=BN_bin2bn(dh2236_g,sizeof(dh2236_g),NULL); - if ((dh->p == NULL) || (dh->g == NULL)) - { DH_free(dh); return(NULL); } - return(dh); -} diff --git a/server/include/service.h b/server/include/service.h index 936a73058..085c0c595 100644 --- a/server/include/service.h +++ b/server/include/service.h @@ -125,6 +125,8 @@ enum{ SERVICE_SSL_TLS_MAX }; +#define DEFAULT_SSL_CERT_VERIFY_DEPTH 100 /*< The default certificate verification depth */ + /** * Defines a service within the gateway. * @@ -173,14 +175,14 @@ typedef struct service { char *weightby; struct service *next; /**< The next service in the linked list */ SSL_CTX *ctx; - SSL *ssl; SSL_METHOD *method; /*< SSLv2/3 or TLSv1/2 methods * see: https://www.openssl.org/docs/ssl/SSL_CTX_new.html */ + int ssl_cert_verify_depth; /*< SSL certificate verification depth */ int ssl_method_type; /*< Which of the SSLv2/3 or TLS1.0/1.1/1.2 methods to use */ - char* ssl_cert; - char* ssl_key; - char* ssl_ca_cert; - bool ssl_init_done; + char* ssl_cert; /*< SSL certificate */ + char* ssl_key; /*< SSL private key */ + char* ssl_ca_cert; /*< SSL CA certificate */ + bool ssl_init_done; /*< If SSL has already been initialized for this service */ } SERVICE; @@ -212,6 +214,7 @@ extern void serviceSetFilters(SERVICE *, char *); extern int serviceSetSSL(SERVICE *service, char* action); extern int serviceInitSSL(SERVICE* service); extern int serviceSetSSLVersion(SERVICE *service, char* version); +extern int serviceSetSSLVerifyDepth(SERVICE* service, int depth); extern void serviceSetCertificates(SERVICE *service, char* cert,char* key, char* ca_cert); extern int serviceEnableRootUser(SERVICE *, int ); extern int serviceSetTimeout(SERVICE *, int ); diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index 3164f1642..c3e463139 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -389,15 +389,17 @@ MySQLSendHandshake(DCB* dcb) /** * gw_mysql_do_authentication * - * Performs the MySQL protocol 4.1 authentication, using data in GWBUF *queue + * Performs the MySQL protocol 4.1 authentication, using data in GWBUF **queue. * * (MYSQL_session*)client_data including: user, db, client_sha1 are copied into - * the dcb->data and later to dcb->session->data. - * - * client_capabilitiesa are copied into the dcb->protocol + * the dcb->data and later to dcb->session->data. client_capabilities are copied + * into the dcb->protocol. + * + * If SSL is enabled for the service, the SSL handshake will be done before the + * MySQL authentication. * * @param dcb Descriptor Control Block of the client - * @param queue The GWBUF with data from client + * @param queue Pointer to the location of the GWBUF with data from client * @return 0 If succeed, otherwise non-zero value * * @note in case of failure, dcb->data is freed before returning. If succeed, @@ -507,8 +509,11 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF **buf) { } else if(dcb->service->ssl_mode == SSL_ENABLED) { - /** This is a non-SSL connection to a SSL enabled service - * and we need to read the rest of the packet from the socket for the username */ + /** This is a non-SSL connection to a SSL enabled service. + * We have only read enough of the packet to know that the client + * is not requesting SSL and the rest of the auth packet is still + * waiting in the socket. We need to read the data from the socket + * to find out the username of the connecting client. */ int bytes = dcb_read(dcb,&queue); queue = gwbuf_make_contiguous(queue); client_auth_packet = GWBUF_DATA(queue); @@ -626,7 +631,8 @@ gw_MySQLWrite_client(DCB *dcb, GWBUF *queue) /** - * Write function for client DCB: writes data from MaxScale to Client + * Write function for client DCB: writes data from MaxScale to Client using SSL + * encryption. The SSH handshake must have already been done. * * @param dcb The DCB of the client * @param queue Queue of buffers to write @@ -671,6 +677,8 @@ int gw_read_client_event( #endif + /** SSL authentication is still going on, we need to call do_ssl_accept + * until it return 1 for success or -1 for error */ if(protocol->protocol_auth_state == MYSQL_AUTH_SSL_HANDSHAKE_ONGOING || protocol->protocol_auth_state == MYSQL_AUTH_SSL_REQ) { @@ -704,15 +712,21 @@ int gw_read_client_event( if(protocol->use_ssl) { + /** SSL handshake is done, communication is now encrypted with SSL */ rc = dcb_read_SSL(dcb, &read_buffer); } else if(dcb->service->ssl_mode != SSL_DISABLED && protocol->protocol_auth_state == MYSQL_AUTH_SENT) { + /** The service allows both SSL and non-SSL connections. + * read only enough of the auth packet to know if the client is + * requesting SSL. If the client is not requesting SSL the rest of + the auth packet will be read later. */ rc = dcb_read_n(dcb, &read_buffer,(4 + 4 + 4 + 1 + 23)); } else { + /** Normal non-SSL connection */ rc = dcb_read(dcb, &read_buffer); } @@ -869,6 +883,9 @@ int gw_read_client_event( protocol->protocol_auth_state == MYSQL_AUTH_SSL_HANDSHAKE_DONE || protocol->protocol_auth_state == MYSQL_AUTH_SSL_HANDSHAKE_FAILED) { + /** SSL was requested and the handshake is either done or + * still ongoing. After the handshake is done, the client + * will send another auth packet. */ break; } @@ -1249,22 +1266,12 @@ return_1: return 1; } -/////////////////////////////////////////////// -// client write event to Client triggered by EPOLLOUT -////////////////////////////////////////////// -/** - * @node Client's fd became writable, and EPOLLOUT event - * arrived. As a consequence, client input buffer (writeq) is flushed. - * - * Parameters: - * @param dcb - in, use - * client dcb - * +/** + * EPOLLOUT event arrived and as a consequence, client input buffer (writeq) is + * flushed. The data is encrypted and SSL is used. The SSL handshake must have + * been successfully completed prior to this function being called. + * @param client dcb * @return constantly 1 - * - * - * @details (write detailed description here) - * */ int gw_write_client_event_SSL(DCB *dcb) { From 9b0a5f13285902a7ef7cdfd121f8d837ef53cb1f Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 15 Jun 2015 16:16:48 +0300 Subject: [PATCH 54/54] Added more comments. --- server/core/dcb.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index f600239ca..7f3651953 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1032,9 +1032,10 @@ return_n: /** - * General purpose read routine to read data from a socket in the - * Descriptor Control Block and append it to a linked list of buffers. - * The list may be empty, in which case *head == NULL + * General purpose read routine to read data from a socket through the SSL + * structure lined with this DCB and append it to a linked list of buffers. + * The list may be empty, in which case *head == NULL. The SSL structure should + * be initialized and the SSL handshake should be done. * * @param dcb The DCB to read from * @param head Pointer to linked list to append data to