diff --git a/maxscale-system-test/change_user.cpp b/maxscale-system-test/change_user.cpp index a12515c5a..770551c0b 100644 --- a/maxscale-system-test/change_user.cpp +++ b/maxscale-system-test/change_user.cpp @@ -31,6 +31,10 @@ int main(int argc, char *argv[]) Test->try_query(Test->maxscales->conn_rwsplit[0], (char *) "DROP TABLE IF EXISTS t1"); Test->try_query(Test->maxscales->conn_rwsplit[0], (char *) "CREATE TABLE t1 (x1 int, fl int)"); + Test->maxscales->restart_maxscale(); + sleep(2); + Test->maxscales->connect_maxscale(0); + Test->tprintf("Changing user... \n"); Test->add_result(mysql_change_user(Test->maxscales->conn_rwsplit[0], (char *) "user", (char *) "pass2", (char *) "test") , "changing user failed \n"); @@ -60,9 +64,9 @@ int main(int argc, char *argv[]) Test->add_result(1, "There is no proper error message\n"); } - Test->tprintf("Trying INSERT again (expecting success - use change should fail)... \n"); - Test->try_query(Test->maxscales->conn_rwsplit[0], (char *) "INSERT INTO t1 VALUES (77, 13);"); - + Test->tprintf("Trying INSERT again (expecting failure - change user should have failed)..."); + Test->add_result(!execute_query(Test->maxscales->conn_rwsplit[0], (char *) "INSERT INTO t1 VALUES (77, 13);"), + "Query should fail, MaxScale should disconnect on auth failure"); Test->tprintf("Changing user with wrong password using ReadConn \n"); if (mysql_change_user(Test->maxscales->conn_slave[0], (char *) "user", (char *) "wrong_pass2", (char *) "test") == 0) @@ -79,6 +83,7 @@ int main(int argc, char *argv[]) Test->add_result(mysql_change_user(Test->maxscales->conn_slave[0], (char *) "user", (char *) "pass2", (char *) "test") , "changing user failed \n"); + Test->maxscales->connect_maxscale(0); Test->try_query(Test->maxscales->conn_rwsplit[0], (char *) "DROP USER user@'%%';"); execute_query_silent(Test->maxscales->conn_rwsplit[0], "DROP TABLE test.t1"); diff --git a/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.c b/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.c index 8218ba049..6e57d475f 100644 --- a/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.c +++ b/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.c @@ -859,6 +859,14 @@ gw_read_and_write(DCB *dcb) } else { + /** + * The client protocol always requests an authentication method + * switch to the same plugin to be compatible with most connectors. + * + * To prevent packet sequence number mismatch, always return a sequence + * of 3 for the final response to a COM_CHANGE_USER. + */ + GWBUF_DATA(read_buffer)[3] = 0x3; proto->changing_user = false; } } diff --git a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc index befd35008..62374ffc9 100644 --- a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc +++ b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc @@ -1513,19 +1513,36 @@ static bool reauthenticate_client(MXS_SESSION* session, GWBUF* packetbuf) if (session->client_dcb->authfunc.reauthenticate) { MySQLProtocol* proto = (MySQLProtocol*)session->client_dcb->protocol; - MYSQL_session* data = (MYSQL_session*)session->client_dcb->data; uint8_t client_sha1[MYSQL_SCRAMBLE_LEN] = {}; uint8_t payload[gwbuf_length(packetbuf) - MYSQL_HEADER_LEN]; gwbuf_copy_data(packetbuf, MYSQL_HEADER_LEN, sizeof(payload), payload); + // Will contains extra data but the username is null-terminated + char user[gwbuf_length(proto->stored_query) - MYSQL_HEADER_LEN - 1]; + gwbuf_copy_data(proto->stored_query, MYSQL_HEADER_LEN + 1, + sizeof(user), (uint8_t*)user); + + // Copy the new username to the session data + MYSQL_session* data = (MYSQL_session*)session->client_dcb->data; + strcpy(data->user, user); + int rc = session->client_dcb->authfunc.reauthenticate(session->client_dcb, data->user, payload, sizeof(payload), proto->scramble, sizeof(proto->scramble), client_sha1, sizeof(client_sha1)); - if (!(rval = rc == MXS_AUTH_SUCCEEDED)) + if (rc == MXS_AUTH_SUCCEEDED) + { + // Re-authentication successful, route the original COM_CHANGE_USER + rval = true; + } + else { /** + * Authentication failed. To prevent the COM_CHANGE_USER from reaching + * the backend servers (and possibly causing problems) the client + * connection will be closed. + * * First packet is COM_CHANGE_USER, the second is AuthSwitchRequest, * third is the response and the fourth is the following error. */ @@ -1656,14 +1673,34 @@ static int route_by_statement(MXS_SESSION* session, uint64_t capabilities, GWBUF { changed_user = true; send_auth_switch_request_packet(session->client_dcb); + + // Store the original COM_CHANGE_USER for later + proto->stored_query = packetbuf; + packetbuf = NULL; + } + else if (proto->changing_user) + { + proto->changing_user = false; + bool ok = reauthenticate_client(session, packetbuf); + gwbuf_free(packetbuf); + packetbuf = proto->stored_query; + proto->stored_query = NULL; + + if (ok) + { + // Authentication was successful, route the original COM_CHANGE_USER + rc = 1; + } + else + { + // Authentication failed, close the connection + rc = 0; + gwbuf_free(packetbuf); + packetbuf = NULL; + } } - if (proto->changing_user) - { - rc = reauthenticate_client(session, packetbuf) ? 1 : 0; - gwbuf_free(packetbuf); - } - else + if (packetbuf) { /** Route query */ rc = MXS_SESSION_ROUTE_QUERY(session, packetbuf);