From f602121459b021c4f597229add2dffbd2851aa34 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 12 Jun 2015 21:21:06 +0300 Subject: [PATCH] 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) {