Fix bug in mysql_client.c (over optimisation of protocol setting); various clarifications and improvements re code review.
This commit is contained in:
@ -133,14 +133,6 @@ static char *service_params[] =
|
||||
"version_string",
|
||||
"filters",
|
||||
"weightby",
|
||||
/* These should no longer be required
|
||||
"ssl_cert",
|
||||
"ssl_ca_cert",
|
||||
"ssl",
|
||||
"ssl_key",
|
||||
"ssl_version",
|
||||
"ssl_cert_verify_depth",
|
||||
* */
|
||||
"ignore_databases",
|
||||
"ignore_databases_regex",
|
||||
"log_auth_warnings",
|
||||
@ -1094,7 +1086,13 @@ make_ssl_structure (CONFIG_CONTEXT *obj, bool require_cert, int *error_count)
|
||||
local_errors++;
|
||||
}
|
||||
}
|
||||
else new_ssl->ssl_cert_verify_depth = 9;
|
||||
else
|
||||
{
|
||||
/**
|
||||
* Default of 9 as per Linux man page
|
||||
*/
|
||||
new_ssl->ssl_cert_verify_depth = 9;
|
||||
}
|
||||
|
||||
listener_set_certificates(new_ssl, ssl_cert, ssl_key, ssl_ca_cert);
|
||||
|
||||
|
@ -2957,38 +2957,32 @@ int dcb_connect_SSL(DCB* dcb)
|
||||
case SSL_ERROR_NONE:
|
||||
MXS_DEBUG("SSL_connect done for %s", dcb->remote);
|
||||
return 1;
|
||||
break;
|
||||
|
||||
case SSL_ERROR_WANT_READ:
|
||||
MXS_DEBUG("SSL_connect ongoing want read for %s", dcb->remote);
|
||||
return 0;
|
||||
break;
|
||||
|
||||
case SSL_ERROR_WANT_WRITE:
|
||||
MXS_DEBUG("SSL_connect ongoing want write for %s", dcb->remote);
|
||||
return 0;
|
||||
break;
|
||||
|
||||
case SSL_ERROR_ZERO_RETURN:
|
||||
MXS_DEBUG("SSL error, shut down cleanly during SSL connect %s", dcb->remote);
|
||||
dcb_log_errors_SSL(dcb, __func__, 0);
|
||||
poll_fake_hangup_event(dcb);
|
||||
return 0;
|
||||
break;
|
||||
|
||||
case SSL_ERROR_SYSCALL:
|
||||
MXS_DEBUG("SSL connection shut down with SSL_ERROR_SYSCALL during SSL connect %s", dcb->remote);
|
||||
dcb_log_errors_SSL(dcb, __func__, ssl_rval);
|
||||
poll_fake_hangup_event(dcb);
|
||||
return -1;
|
||||
break;
|
||||
|
||||
default:
|
||||
MXS_DEBUG("SSL connection shut down with error during SSL connect %s", dcb->remote);
|
||||
dcb_log_errors_SSL(dcb, __func__, 0);
|
||||
poll_fake_hangup_event(dcb);
|
||||
return -1;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -52,9 +52,8 @@
|
||||
* @param is_capable Indicates if the client can handle SSL
|
||||
* @return 0 if ok, >0 if a problem - see return codes defined in gw_ssl.h
|
||||
*/
|
||||
int ssl_authenticate_client(DCB *dcb, bool is_capable)
|
||||
int ssl_authenticate_client(DCB *dcb, const char *user, bool is_capable)
|
||||
{
|
||||
char *user = dcb->user ? dcb->user : "";
|
||||
char *remote = dcb->remote ? dcb->remote : "";
|
||||
char *service = (dcb->service && dcb->service->name) ? dcb->service->name : "";
|
||||
|
||||
@ -68,7 +67,7 @@ int ssl_authenticate_client(DCB *dcb, bool is_capable)
|
||||
{
|
||||
/* Should be SSL, but client is not SSL capable */
|
||||
MXS_INFO("User %s@%s connected to service '%s' without SSL when SSL was required.",
|
||||
user ? user : "", remote ? remote : "", service ? service : "");
|
||||
user, remote, service);
|
||||
return SSL_ERROR_CLIENT_NOT_SSL;
|
||||
}
|
||||
/* Now we know SSL is required and client is capable */
|
||||
|
@ -45,6 +45,32 @@ static RSA *rsa_1024 = NULL;
|
||||
|
||||
static RSA *tmp_rsa_callback(SSL *s, int is_export, int keylength);
|
||||
|
||||
/**
|
||||
* Create a new listener structure
|
||||
*
|
||||
* @param protocol The name of the protocol module
|
||||
* @param address The address to listen with
|
||||
* @param port The port to listen on
|
||||
* @param authenticator Name of the authenticator to be used
|
||||
* @param ssl SSL configuration
|
||||
* @return New listener object or NULL if unable to allocate
|
||||
*/
|
||||
SERV_LISTENER *
|
||||
alloc_listener(char *protocol, char *address, unsigned short port, char *authenticator, SSL_LISTENER *ssl)
|
||||
{
|
||||
SERV_LISTENER *proto = NULL;
|
||||
if ((proto = (SERV_LISTENER *)malloc(sizeof(SERV_LISTENER))) != NULL)
|
||||
{
|
||||
proto->listener = NULL;
|
||||
proto->protocol = strdup(protocol);
|
||||
proto->address = address ? strdup(address) : NULL;
|
||||
proto->port = port;
|
||||
proto->authenticator = authenticator ? strdup(authenticator) : NULL;
|
||||
proto->ssl = ssl;
|
||||
}
|
||||
return proto;
|
||||
}
|
||||
|
||||
/**
|
||||
* Set the maximum SSL/TLS version the listener will support
|
||||
* @param ssl_listener Listener data to configure
|
||||
@ -93,32 +119,15 @@ listener_set_ssl_version(SSL_LISTENER *ssl_listener, char* version)
|
||||
*/
|
||||
void
|
||||
listener_set_certificates(SSL_LISTENER *ssl_listener, char* cert, char* key, char* ca_cert)
|
||||
{
|
||||
if (NULL != cert)
|
||||
{
|
||||
if (ssl_listener->ssl_cert)
|
||||
{
|
||||
free(ssl_listener->ssl_cert);
|
||||
}
|
||||
ssl_listener->ssl_cert = strdup(cert);
|
||||
}
|
||||
else ssl_listener->ssl_cert = NULL;
|
||||
ssl_listener->ssl_cert = cert ? strdup(cert) : NULL;
|
||||
|
||||
if (NULL != key)
|
||||
{
|
||||
if (ssl_listener->ssl_key)
|
||||
{
|
||||
free(ssl_listener->ssl_key);
|
||||
}
|
||||
ssl_listener->ssl_key = strdup(key);
|
||||
}
|
||||
else ssl_listener->ssl_key = NULL;
|
||||
ssl_listener->ssl_key = key ? strdup(key) : NULL;
|
||||
|
||||
if (ssl_listener->ssl_ca_cert)
|
||||
{
|
||||
free(ssl_listener->ssl_ca_cert);
|
||||
}
|
||||
ssl_listener->ssl_ca_cert = strdup(ca_cert);
|
||||
ssl_listener->ssl_ca_cert = ca_cert ? strdup(ca_cert) : NULL;
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -680,6 +680,8 @@ service_free(SERVICE *service)
|
||||
* @param protocol The name of the protocol module
|
||||
* @param address The address to listen with
|
||||
* @param port The port to listen on
|
||||
* @param authenticator Name of the authenticator to be used
|
||||
* @param ssl SSL configuration
|
||||
* @return TRUE if the protocol/port could be added
|
||||
*/
|
||||
int
|
||||
@ -687,24 +689,18 @@ serviceAddProtocol(SERVICE *service, char *protocol, char *address, unsigned sho
|
||||
{
|
||||
SERV_LISTENER *proto;
|
||||
|
||||
if ((proto = (SERV_LISTENER *)malloc(sizeof(SERV_LISTENER))) == NULL)
|
||||
if ((proto = alloc_listener(protocol, address, port, authenticator, ssl)) != NULL)
|
||||
{
|
||||
return 0;
|
||||
}
|
||||
proto->listener = NULL;
|
||||
proto->protocol = strdup(protocol);
|
||||
proto->address = address ? strdup(address) : NULL;
|
||||
proto->port = port;
|
||||
proto->authenticator = authenticator ? strdup(authenticator) : NULL;
|
||||
proto->ssl = ssl;
|
||||
spinlock_acquire(&service->spin);
|
||||
proto->next = service->ports;
|
||||
service->ports = proto;
|
||||
spinlock_release(&service->spin);
|
||||
|
||||
return 1;
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a protocol/port pair is part of the service
|
||||
*
|
||||
|
@ -33,10 +33,6 @@
|
||||
*/
|
||||
|
||||
#include <buffer.h>
|
||||
#include <openssl/crypto.h>
|
||||
#include <openssl/ssl.h>
|
||||
#include <openssl/err.h>
|
||||
#include <openssl/dh.h>
|
||||
|
||||
struct dcb;
|
||||
struct server;
|
||||
|
@ -73,7 +73,7 @@ typedef struct ssl_listener
|
||||
bool ssl_init_done; /*< If SSL has already been initialized for this service */
|
||||
} SSL_LISTENER;
|
||||
|
||||
int ssl_authenticate_client(struct dcb *dcb, bool is_capable);
|
||||
int ssl_authenticate_client(struct dcb *dcb, const char *user, bool is_capable);
|
||||
bool ssl_is_connection_healthy(struct dcb *dcb);
|
||||
bool ssl_check_data_to_process(struct dcb *dcb);
|
||||
bool ssl_required_by_dcb(struct dcb *dcb);
|
||||
|
@ -53,6 +53,7 @@ typedef struct servlistener
|
||||
struct servlistener *next; /**< Next service protocol */
|
||||
} SERV_LISTENER;
|
||||
|
||||
SERV_LISTENER *alloc_listener(char *protocol, char *address, unsigned short port, char *authenticator, SSL_LISTENER *ssl);
|
||||
int listener_set_ssl_version(SSL_LISTENER *ssl_listener, char* version);
|
||||
void listener_set_certificates(SSL_LISTENER *ssl_listener, char* cert, char* key, char* ca_cert);
|
||||
int listener_init_SSL(SSL_LISTENER *ssl_listener);
|
||||
|
@ -69,7 +69,7 @@ mysql_auth_authenticate(DCB *dcb, GWBUF **buffer)
|
||||
MYSQL_session *client_data = (MYSQL_session *)dcb->data;
|
||||
int auth_ret, ssl_ret;
|
||||
|
||||
if (0 != (ssl_ret = ssl_authenticate_client(dcb, mysql_auth_is_client_ssl_capable(dcb))))
|
||||
if (0 != (ssl_ret = ssl_authenticate_client(dcb, client_data->user, mysql_auth_is_client_ssl_capable(dcb))))
|
||||
{
|
||||
auth_ret = (SSL_ERROR_CLIENT_NOT_SSL == ssl_ret) ? MYSQL_FAILED_AUTH_SSL : MYSQL_FAILED_AUTH;
|
||||
}
|
||||
@ -144,6 +144,7 @@ mysql_auth_authenticate(DCB *dcb, GWBUF **buffer)
|
||||
* @param buffer Pointer to pointer to buffer containing data from client
|
||||
* @return Authentication status
|
||||
* @note Authentication status codes are defined in mysql_client_server_protocol.h
|
||||
* @see https://dev.mysql.com/doc/internals/en/client-server-protocol.html
|
||||
*/
|
||||
int
|
||||
mysql_auth_set_protocol_data(DCB *dcb, GWBUF *buf)
|
||||
@ -184,6 +185,7 @@ mysql_auth_set_protocol_data(DCB *dcb, GWBUF *buf)
|
||||
* string[23] reserved (all [0])
|
||||
* ...
|
||||
* ...
|
||||
* Note that the fixed elements add up to 36
|
||||
*/
|
||||
|
||||
/* Detect now if there are enough bytes to continue */
|
||||
@ -210,6 +212,7 @@ mysql_auth_set_protocol_data(DCB *dcb, GWBUF *buf)
|
||||
* @param client_auth_packet size An integer giving the size of the data
|
||||
* @return Authentication status
|
||||
* @note Authentication status codes are defined in mysql_client_server_protocol.h
|
||||
* @see https://dev.mysql.com/doc/internals/en/client-server-protocol.html
|
||||
*/
|
||||
static int
|
||||
mysql_auth_set_client_data(
|
||||
@ -218,6 +221,7 @@ mysql_auth_set_client_data(
|
||||
uint8_t *client_auth_packet,
|
||||
int client_auth_packet_size)
|
||||
{
|
||||
/* The numbers are the fixed elements in the client handshake packet */
|
||||
int auth_packet_base_size = 4 + 4 + 4 + 1 + 23;
|
||||
int packet_length_used = 0;
|
||||
|
||||
|
@ -472,12 +472,21 @@ int gw_read_client_event(DCB* dcb)
|
||||
*/
|
||||
case MYSQL_AUTH_SENT:
|
||||
{
|
||||
MySQLProtocol *protocol;
|
||||
/* int compress = -1; */
|
||||
int auth_val, packet_number;
|
||||
MySQLProtocol *protocol = DCB_PROTOCOL(dcb, MySQLProtocol);
|
||||
|
||||
packet_number = ssl_required_by_dcb(dcb) ? 3 : 2;
|
||||
|
||||
/**
|
||||
* The first step in the authentication process is to extract the
|
||||
* relevant information from the buffer supplied and place it
|
||||
* into a data structure pointed to by the DCB. The "success"
|
||||
* result is not final, it implies only that the process is so
|
||||
* far successful, not that authentication has completed. If the
|
||||
* data extraction succeeds, then a call is made to
|
||||
* mysql_auth_authenticate to carry out the actual user checks.
|
||||
*/
|
||||
if (MYSQL_AUTH_SUCCEEDED == (
|
||||
auth_val = mysql_auth_set_protocol_data(dcb, read_buffer)))
|
||||
{
|
||||
@ -489,11 +498,18 @@ int gw_read_client_event(DCB* dcb)
|
||||
auth_val = mysql_auth_authenticate(dcb, &read_buffer);
|
||||
}
|
||||
|
||||
/**
|
||||
* At this point, if the auth_val return code indicates success
|
||||
* the user authentication has been successfully completed.
|
||||
* But in order to have a working connection, a session has to
|
||||
* be created. Provided that is successful (indicated by a
|
||||
* non-null session) then the whole process has succeeded. In all
|
||||
* other cases an error return is made.
|
||||
*/
|
||||
if (MYSQL_AUTH_SUCCEEDED == auth_val)
|
||||
{
|
||||
SESSION *session;
|
||||
|
||||
protocol = DCB_PROTOCOL(dcb, MySQLProtocol);
|
||||
protocol->protocol_auth_state = MYSQL_AUTH_RECV;
|
||||
/**
|
||||
* Create session, and a router session for it.
|
||||
|
Reference in New Issue
Block a user