From 78799e505ff276c30a7af55bea9586179babdcbf Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Fri, 28 Feb 2014 11:29:50 +0100 Subject: [PATCH] Fix for bug 127 and 345 as part of it Fix for bug 127 and 345 as part of it http://bugs.skysql.com/show_bug.cgi?id=127 http://bugs.skysql.com/show_bug.cgi?id=345 --- server/core/dbusers.c | 138 ++++++++++++++++-- server/core/service.c | 48 ++++++ server/core/users.c | 2 +- server/include/dbusers.h | 16 +- server/include/service.h | 47 ++++-- server/include/users.h | 15 +- .../include/mysql_client_server_protocol.h | 12 +- server/modules/protocol/mysql_backend.c | 14 +- server/modules/protocol/mysql_client.c | 12 ++ server/modules/protocol/mysql_common.c | 36 ++--- 10 files changed, 283 insertions(+), 57 deletions(-) diff --git a/server/core/dbusers.c b/server/core/dbusers.c index 48ecee160..d6c512513 100644 --- a/server/core/dbusers.c +++ b/server/core/dbusers.c @@ -26,6 +26,7 @@ * 24/06/2013 Massimiliano Pinto Initial implementation * 08/08/2013 Massimiliano Pinto Fixed bug for invalid memory access in row[1]+1 when row[1] is "" * 06/02/2014 Massimiliano Pinto Mysql user root selected based on configuration flag + * 26/02/2014 Massimiliano Pinto Addd: replace_mysql_users() routine may replace users' table based on a checksum * * @endverbatim */ @@ -36,12 +37,15 @@ #include #include #include +#include #include #include #include #define USERS_QUERY_NO_ROOT " WHERE user NOT IN ('root')" -#define LOAD_MYSQL_USERS_QUERY "SELECT user, password FROM mysql.user" +#define LOAD_MYSQL_USERS_QUERY "SELECT user, password, concat(user,host,password) AS userdata FROM mysql.user" + +#define MYSQL_USERS_COUNT "SELECT COUNT(1) AS nusers FROM mysql.user" extern int lm_enabled_logfiles_bitmask; @@ -85,6 +89,57 @@ struct users *newusers, *oldusers; return i; } +/** + * Replace the user/passwd form mysql.user table into the service users' hashtable + * environment. + * The replacement is succesful only if the users' table checksums differ + * + * @param service The current service + * @return -1 on any error or the number of users inserted (0 means no users at all) + */ +int +replace_mysql_users(SERVICE *service) +{ +int i; +struct users *newusers, *oldusers; + + if ((newusers = users_alloc()) == NULL) + return -1; + + i = getUsers(service, newusers); + + if (i <= 0) + return i; + + spinlock_acquire(&service->spin); + oldusers = service->users; + + if (memcmp(oldusers->cksum, newusers->cksum, SHA_DIGEST_LENGTH) == 0) { + /* same data, nothing to do */ + LOGIF(LD, (skygw_log_write_flush( + LOGFILE_DEBUG, + "%lu [replace_mysql_users] users' tables not switched, checksum is the same", + pthread_self()))); + /* free the new table */ + users_free(newusers); + i = 0; + } else { + /* replace the service with effective new data */ + LOGIF(LD, (skygw_log_write_flush( + LOGFILE_DEBUG, + "%lu [replace_mysql_users] users' tables replaced, checksum differs", + pthread_self()))); + service->users = newusers; + } + + spinlock_release(&service->spin); + + if (i) + users_free(oldusers); + + return i; +} + /** * Load the user/passwd form mysql.user table into the service users' hashtable * environment. @@ -96,16 +151,20 @@ struct users *newusers, *oldusers; static int getUsers(SERVICE *service, struct users *users) { - MYSQL *con = NULL; - MYSQL_ROW row; - MYSQL_RES *result = NULL; - int num_fields = 0; - char *service_user = NULL; - char *service_passwd = NULL; - char *dpwd; - int total_users = 0; - SERVER *server; - char *users_query; + MYSQL *con = NULL; + MYSQL_ROW row; + MYSQL_RES *result = NULL; + int num_fields = 0; + char *service_user = NULL; + char *service_passwd = NULL; + char *dpwd; + int total_users = 0; + SERVER *server; + char *users_query; + unsigned char hash[SHA_DIGEST_LENGTH]=""; + char *users_data = NULL; + int nusers = 0; + int users_data_row_len = MYSQL_USER_MAXLEN + MYSQL_HOST_MAXLEN + MYSQL_PASSWORD_LEN + 1; if(service->enable_root) users_query = LOAD_MYSQL_USERS_QUERY; @@ -169,6 +228,44 @@ getUsers(SERVICE *service, struct users *users) return -1; } + if (mysql_query(con, MYSQL_USERS_COUNT)) { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Loading users for service %s encountered " + "error: %s.", + service->name, + mysql_error(con)))); + mysql_close(con); + return -1; + } + result = mysql_store_result(con); + + if (result == NULL) { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Loading users for service %s encountered " + "error: %s.", + service->name, + mysql_error(con)))); + mysql_close(con); + return -1; + } + num_fields = mysql_num_fields(result); + row = mysql_fetch_row(result); + + nusers = atoi(row[0]); + + mysql_free_result(result); + + if (!nusers) { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Counting users for service %s returned 0", + service->name))); + mysql_close(con); + return -1; + } + if (mysql_query(con, users_query)) { LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, @@ -179,6 +276,7 @@ getUsers(SERVICE *service, struct users *users) mysql_close(con); return -1; } + result = mysql_store_result(con); if (result == NULL) { @@ -192,7 +290,12 @@ getUsers(SERVICE *service, struct users *users) return -1; } num_fields = mysql_num_fields(result); - + + users_data = (char *)calloc(nusers, users_data_row_len * sizeof(char)); + + if(users_data == NULL) + return -1; + while ((row = mysql_fetch_row(result))) { /** * Two fields should be returned. @@ -200,10 +303,21 @@ getUsers(SERVICE *service, struct users *users) * added to hashtable. */ users_add(users, row[0], strlen(row[1]) ? row[1]+1 : row[1]); + + strncat(users_data, row[2], users_data_row_len); + total_users++; } + + SHA1((const unsigned char *) users_data, strlen(users_data), hash); + + memcpy(users->cksum, hash, SHA_DIGEST_LENGTH); + + free(users_data); + mysql_free_result(result); mysql_close(con); mysql_thread_end(); + return total_users; } diff --git a/server/core/service.c b/server/core/service.c index f3a065441..8f8f3df63 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -26,6 +26,8 @@ * 18/06/13 Mark Riddoch Initial implementation * 24/06/13 Massimiliano Pinto Added: Loading users from mysql backend in serviceStart * 06/02/14 Massimiliano Pinto Added: serviceEnableRootUser routine + * 25/02/14 Massimiliano Pinto Added: service refresh limit feature + * * @endverbatim */ #include @@ -82,6 +84,8 @@ SERVICE *service; service->enable_root = 0; service->routerOptions = NULL; service->databases = NULL; + memset(&service->rate_limit, 0, sizeof(SERVICE_REFRESH_RATE)); + spinlock_init(&service->users_table_spin); spinlock_init(&service->spin); spinlock_acquire(&service_spin); @@ -691,3 +695,47 @@ void *router_obj; serviceSetUser(service, user, auth); } } + + +int service_refresh_users(SERVICE *service) { + int ret = 1; + /* check for another running getUsers request */ + if (! spinlock_acquire_nowait(&service->users_table_spin)) { + LOGIF(LD, (skygw_log_write_flush( + LOGFILE_DEBUG, + "%lu [service_refresh_users] failed to get get lock for loading new users' table: another thread is loading users", + pthread_self()))); + + return 1; + } + + + /* check if refresh rate limit has exceeded */ + if ( (time(NULL) < (service->rate_limit.last + USERS_REFRESH_TIME)) || (service->rate_limit.nloads > USERS_REFRESH_MAX_PER_TIME)) { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "%lu [service_refresh_users] refresh rate limit exceeded loading new users' table", + pthread_self()))); + + spinlock_release(&service->users_table_spin); + return 1; + } + + service->rate_limit.nloads++; + + /* update time and counter */ + if (service->rate_limit.nloads > USERS_REFRESH_MAX_PER_TIME) { + service->rate_limit.nloads = 1; + service->rate_limit.last = time(NULL); + } + + ret = replace_mysql_users(service); + + /* remove lock */ + spinlock_release(&service->users_table_spin); + + if (ret >= 0) + return 0; + else + return 1; +} diff --git a/server/core/users.c b/server/core/users.c index ff0f8fa88..538327b84 100644 --- a/server/core/users.c +++ b/server/core/users.c @@ -61,7 +61,7 @@ USERS *rval; if ((rval = calloc(1, sizeof(USERS))) == NULL) return NULL; - if ((rval->data = hashtable_alloc(52, user_hash, strcmp)) == NULL) + if ((rval->data = hashtable_alloc(USERS_HASHTABLE_DEFAULT_SIZE, user_hash, strcmp)) == NULL) { free(rval); return NULL; diff --git a/server/include/dbusers.h b/server/include/dbusers.h index 3b5e55f9d..949177d19 100644 --- a/server/include/dbusers.h +++ b/server/include/dbusers.h @@ -25,12 +25,24 @@ * @verbatim * Revision History * - * Date Who Description - * 25/06/13 Mark Riddoch Initial implementation + * Date Who Description + * 25/06/13 Mark Riddoch Initial implementation + * 25/02/13 Massimiliano Pinto Added users table refresh rate default values * * @endverbatim */ +/* Refresh rate limits for load users from database */ +#define USERS_REFRESH_TIME 30 /* Allowed time interval (in seconds) after last update*/ +#define USERS_REFRESH_MAX_PER_TIME 4 /* Max number of load calls within the time interval */ + +/* Max length of fields in the mysql.user table */ +#define MYSQL_USER_MAXLEN 128 +#define MYSQL_PASSWORD_LEN 41 +#define MYSQL_HOST_MAXLEN 60 +#define MYSQL_DATABASE_MAXLEN 128 + extern int load_mysql_users(SERVICE *service); extern int reload_mysql_users(SERVICE *service); +extern int replace_mysql_users(SERVICE *service); #endif diff --git a/server/include/service.h b/server/include/service.h index 630bfdd8b..d52a7eccf 100644 --- a/server/include/service.h +++ b/server/include/service.h @@ -37,6 +37,7 @@ * prototypes * 23/06/13 Mark Riddoch Added service user and users * 06/02/14 Massimiliano Pinto Added service flag for root user access + * 25/02/14 Massimiliano Pinto Added service refresh limit feature * * @endverbatim */ @@ -79,6 +80,15 @@ typedef struct { char *authdata; /**< The authentication data requied */ } SERVICE_USER; +/** + * The service refresh rate hols the counter and last load timet for this service to + * load users data from the backend database + */ +typedef struct { + int nloads; + time_t last; +} SERVICE_REFRESH_RATE; + /** * Defines a service within the gateway. * @@ -87,24 +97,28 @@ typedef struct { * to the service. */ typedef struct service { - char *name; /**< The service name */ - int state; /**< The service state */ - SERV_PROTOCOL *ports; /**< Linked list of ports and protocols - * that this service will listen on. - */ - char *routerModule; /**< Name of router module to use */ - char **routerOptions;/**< Router specific option strings */ + char *name; /**< The service name */ + int state; /**< The service state */ + SERV_PROTOCOL *ports; /**< Linked list of ports and protocols + * that this service will listen on. + */ + char *routerModule; /**< Name of router module to use */ + char **routerOptions; /**< Router specific option strings */ struct router_object - *router; /**< The router we are using */ + *router; /**< The router we are using */ void *router_instance; - /**< The router instance for this service */ - struct server *databases; /**< The set of servers in the backend */ - SERVICE_USER credentials; /**< The cedentials of the service user */ - SPINLOCK spin; /**< The service spinlock */ - SERVICE_STATS stats; /**< The service statistics */ - struct users *users; /**< The user data for this service */ - int enable_root; /**< Allow root user access */ - struct service *next; /**< The next service in the linked list */ + /**< The router instance for this service */ + struct server *databases; /**< The set of servers in the backend */ + SERVICE_USER credentials; /**< The cedentials of the service user */ + SPINLOCK spin; /**< The service spinlock */ + SERVICE_STATS stats; /**< The service statistics */ + struct users *users; /**< The user data for this service */ + int enable_root; /**< Allow root user access */ + SPINLOCK + users_table_spin; /**< The spinlock for users data refresh */ + SERVICE_REFRESH_RATE + rate_limit; /**< The refresh rate limit for users table */ + struct service *next; /**< The next service in the linked list */ } SERVICE; #define SERVICE_STATE_ALLOC 1 /**< The service has been allocated */ @@ -128,6 +142,7 @@ extern int serviceSetUser(SERVICE *, char *, char *); extern int serviceGetUser(SERVICE *, char **, char **); extern int serviceEnableRootUser(SERVICE *, int ); extern void service_update(SERVICE *, char *, char *, char *); +extern int service_refresh_users(SERVICE *); extern void printService(SERVICE *); extern void printAllServices(); extern void dprintAllServices(DCB *); diff --git a/server/include/users.h b/server/include/users.h index 11dc2f60a..824a12818 100644 --- a/server/include/users.h +++ b/server/include/users.h @@ -19,6 +19,7 @@ */ #include #include +#include /** * @file users.h The functions to manipulate the table of users maintained @@ -27,12 +28,16 @@ * @verbatim * Revision History * - * Date Who Description - * 23/06/13 Mark Riddoch Initial implementation + * Date Who Description + * 23/06/13 Mark Riddoch Initial implementation + * 26/02/14 Massimiliano Pinto Added checksum to users' table with SHA1 + * 27/02/14 Massimiliano Pinto Added USERS_HASHTABLE_DEFAULT_SIZE * * @endverbatim */ +#define USERS_HASHTABLE_DEFAULT_SIZE 52 + /** * The users table statistics structure */ @@ -48,8 +53,10 @@ typedef struct { * for the authentication implementation within the gateway. */ typedef struct users { - HASHTABLE *data; /**< The hashtable containing the actual data */ - USERS_STATS stats; /**< The statistics for the users table */ + HASHTABLE *data; /**< The hashtable containing the actual data */ + USERS_STATS stats; /**< The statistics for the users table */ + unsigned char + cksum[SHA_DIGEST_LENGTH]; /**< The users' table ckecksum */ } USERS; extern USERS *users_alloc(); /**< Allocate a users table */ diff --git a/server/modules/include/mysql_client_server_protocol.h b/server/modules/include/mysql_client_server_protocol.h index 84872092d..d6b5413fb 100644 --- a/server/modules/include/mysql_client_server_protocol.h +++ b/server/modules/include/mysql_client_server_protocol.h @@ -29,6 +29,12 @@ * Added authentication reply status * 12-07-2013 Massimiliano Pinto Added routines for change_user * 14-02-2014 Massimiliano Pinto setipaddress returns int + * 25-02-2014 Massimiliano Pinto Added dcb parameter to gw_find_mysql_user_password_sha1() + * and repository to gw_check_mysql_scramble_data() + * It's now possible to specify a different users' table than + * dcb->service->users default + * 26-02-2014 Massimiliano Pinto Removed previouvsly added parameters to gw_check_mysql_scramble_data() and + * gw_find_mysql_user_password_sha1() * */ @@ -51,6 +57,7 @@ #include #include #include +#include #include #define GW_MYSQL_VERSION "MaxScale " MAXSCALE_VERSION @@ -71,9 +78,6 @@ #define MYSQL_SCRAMBLE_LEN GW_MYSQL_SCRAMBLE_SIZE #endif -#define MYSQL_USER_MAXLEN 128 -#define MYSQL_DATABASE_MAXLEN 128 - #define GW_NOINTR_CALL(A) do { errno = 0; A; } while (errno == EINTR) // network buffer is 32K #define MAX_BUFFER_SIZE 32768 @@ -264,7 +268,7 @@ int gw_send_change_user_to_backend( int gw_find_mysql_user_password_sha1( char *username, uint8_t *gateway_password, - void *repository); + DCB *dcb); int gw_check_mysql_scramble_data( DCB *dcb, uint8_t *token, diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index a031648c7..f11117923 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -243,7 +243,7 @@ static int gw_read_backend_event(DCB *dcb) { switch (receive_rc) { case -1: backend_protocol->state = MYSQL_AUTH_FAILED; - + LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, "Error : backend server didn't " @@ -300,6 +300,9 @@ static int gw_read_backend_event(DCB *dcb) { gwbuf_length(dcb->delayq)); } + /* try reload users' table for next connection */ + service_refresh_users(dcb->session->client->service); + while (session->state != SESSION_STATE_ROUTER_READY) { ss_dassert( @@ -882,6 +885,14 @@ static int gw_change_user(DCB *backend, SERVER *server, SESSION *in_session, GWB // Note: if auth_token_len == 0 && auth_token == NULL, user is without password auth_ret = gw_check_mysql_scramble_data(backend->session->client, auth_token, auth_token_len, client_protocol->scramble, sizeof(client_protocol->scramble), username, client_sha1); + if (auth_ret != 0) { + if (!service_refresh_users(backend->session->client->service)) { + /* Try authentication again with new repository data */ + /* Note: if no auth client authentication will fail */ + auth_ret = gw_check_mysql_scramble_data(backend->session->client, auth_token, auth_token_len, client_protocol->scramble, sizeof(client_protocol->scramble), username, client_sha1); + } + } + // let's free the auth_token now if (auth_token) free(auth_token); @@ -891,6 +902,7 @@ static int gw_change_user(DCB *backend, SERVER *server, SESSION *in_session, GWB // send the error packet mysql_send_auth_error(backend->session->client, 1, 0, "Authorization failed on change_user"); + rv = 1; } else { // get db name diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index a8cde7337..2ecc616c5 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -29,6 +29,9 @@ * 24/06/2013 Massimiliano Pinto Added: fetch passwords from service users' hashtable * 02/09/2013 Massimiliano Pinto Added: session refcount * 16/12/2013 Massimiliano Pinto Added: client closed socket detection with recv(..., MSG_PEEK) + * 24/02/2014 Massimiliano Pinto Added: on failed authentication a new users' table is loaded with time and frequency limitations + * If current user is authenticated the new users' table will replace the old one + * */ #include @@ -444,6 +447,15 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { username, stage1_hash); + /* On failed auth try to load users' table from backend database */ + if (auth_ret != 0) { + if (!service_refresh_users(dcb->service)) { + /* Try authentication again with new repository data */ + /* Note: if no auth client authentication will fail */ + auth_ret = gw_check_mysql_scramble_data(dcb, auth_token, auth_token_len, protocol->scramble, sizeof(protocol->scramble), username, stage1_hash); + } + } + /* let's free the auth_token now */ if (auth_token) free(auth_token); diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index 8e1bb3257..aaafe127d 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -406,7 +406,7 @@ int gw_send_authentication_to_backend( uint8_t client_capabilities[4]; uint32_t server_capabilities; uint32_t final_capabilities; - char dbpass[129]=""; + char dbpass[MYSQL_USER_MAXLEN + 1]=""; GWBUF *buffer; DCB *dcb; @@ -817,7 +817,7 @@ int gw_send_change_user_to_backend(char *dbname, char *user, uint8_t *passwd, My uint8_t client_capabilities[4]; uint32_t server_capabilities; uint32_t final_capabilities; - char dbpass[129]=""; + char dbpass[MYSQL_USER_MAXLEN + 1]=""; GWBUF *buffer; DCB *dcb; @@ -984,12 +984,12 @@ int gw_send_change_user_to_backend(char *dbname, char *user, uint8_t *passwd, My * Check authentication token received against stage1_hash and scramble * * @param dcb The current dcb - * @param token The token sent by the client in the authentication request - * @param token_len The token size in bytes - * @param scramble The scramble data sent by the server during handshake - * @param scramble_len The scrable size in bytes - * @param username The current username in the authentication request - * @param stage1_hash The SHA1(candidate_password) decoded by this routine + * @param token The token sent by the client in the authentication request + * @param token_len The token size in bytes + * @param scramble The scramble data sent by the server during handshake + * @param scramble_len The scrable size in bytes + * @param username The current username in the authentication request + * @param stage1_hash The SHA1(candidate_password) decoded by this routine * @return 0 on succesful check or != 0 on failure * */ @@ -1010,7 +1010,7 @@ int gw_check_mysql_scramble_data(DCB *dcb, uint8_t *token, unsigned int token_le * please note 'real_password' is unknown! */ - ret_val = gw_find_mysql_user_password_sha1(username, password, (DCB *) dcb); + ret_val = gw_find_mysql_user_password_sha1(username, password, dcb); if (ret_val) { return 1; @@ -1090,24 +1090,26 @@ int gw_check_mysql_scramble_data(DCB *dcb, uint8_t *token, unsigned int token_le /** * gw_find_mysql_user_password_sha1 * - * The routine fetches look for an user int he Gateway users' tableg - * If found the HEX passwotd, representing sha1(sha1(password)), is converted in binary data and + * The routine fetches look for an user int he Gateway users' table + * The users' table is dcb->service->users or a different one specified with void *repository + * + * If found the HEX password, representing sha1(sha1(password)), is converted in binary data and * copied into gateway_password * - * @param username The user to look for - * @param gateway_password The related SHA1(SHA1(password)), the pointer must be preallocated - * @param repository The pointer to users' table data, passed as void * + * @param username The user to look for + * @param gateway_password The related SHA1(SHA1(password)), the pointer must be preallocated + * @param dcb Current DCB * @return 1 if user is not found or 0 if the user exists * */ -int gw_find_mysql_user_password_sha1(char *username, uint8_t *gateway_password, void *repository) { +int gw_find_mysql_user_password_sha1(char *username, uint8_t *gateway_password, DCB *dcb) { SERVICE *service = NULL; char *user_password = NULL; - service = (SERVICE *) ((DCB *)repository)->service; + service = (SERVICE *) dcb->service; - user_password = (char *)users_fetch(service->users, username); + user_password = (char *)users_fetch(service->users, username); if (!user_password) { return 1;