diff --git a/Documentation/Reference/MaxCtrl.md b/Documentation/Reference/MaxCtrl.md index 6816a8c09..e32d3607b 100644 --- a/Documentation/Reference/MaxCtrl.md +++ b/Documentation/Reference/MaxCtrl.md @@ -59,8 +59,9 @@ HTTPS/TLS Options: [boolean] [default: true] Options: - --version Show version number [boolean] - --help Show help [boolean] + --version Show version number [boolean] + --tls-passphrase Password for the TLS private key [string] + --help Show help [boolean] ``` ## list @@ -680,6 +681,11 @@ Usage: api Commands: get [path] Get raw JSON +API options: + --sum Calculate sum of API result. Only works for arrays of numbers e.g. `api + get --sum servers data[].attributes.statistics.connections`. + [boolean] [default: false] + ``` ### api get diff --git a/include/maxscale/protocol/mysql.h b/include/maxscale/protocol/mysql.h index 0282fcf2e..e7f54f9cf 100644 --- a/include/maxscale/protocol/mysql.h +++ b/include/maxscale/protocol/mysql.h @@ -486,8 +486,6 @@ int mysql_send_custom_error(DCB *dcb, int sequence, int affected_rows, const cha int mysql_send_standard_error(DCB *dcb, int sequence, int errnum, const char *msg); int mysql_send_auth_error(DCB *dcb, int sequence, int affected_rows, const char* msg); -GWBUF* gw_MySQL_get_next_packet(GWBUF** p_readbuf); -GWBUF* gw_MySQL_get_packets(GWBUF** p_readbuf, int* npackets); void protocol_add_srv_command(MySQLProtocol* p, mxs_mysql_cmd_t cmd); void protocol_remove_srv_command(MySQLProtocol* p); bool protocol_waits_response(MySQLProtocol* p); diff --git a/maxctrl/lib/core.js b/maxctrl/lib/core.js index 460a96fcb..84bc3ed61 100644 --- a/maxctrl/lib/core.js +++ b/maxctrl/lib/core.js @@ -74,6 +74,10 @@ program describe: 'Path to TLS public certificate', type: 'string' }) + .option('tls-passphrase', { + describe: 'Password for the TLS private key', + type: 'string' + }) .option('tls-ca-cert', { describe: 'Path to TLS CA certificate', type: 'string' diff --git a/maxscale-system-test/CMakeLists.txt b/maxscale-system-test/CMakeLists.txt index c1bad051e..0f0df8839 100644 --- a/maxscale-system-test/CMakeLists.txt +++ b/maxscale-system-test/CMakeLists.txt @@ -494,6 +494,10 @@ add_test_executable(mxs822_maxpasswd.cpp mxs822_maxpasswd maxpasswd LABELS maxsc # Do only SELECTS during time > wait_timeout and then do INSERT add_test_executable(mxs827_write_timeout.cpp mxs827_write_timeout mxs827_write_timeout LABELS readwritesplit REPL_BACKEND) +# MXS-872: MaxScale doesn't understand roles +# https://jira.mariadb.org/browse/MXS-872 +add_test_executable(mxs872_roles.cpp mxs872_roles replication LABELS REPL_BACKEND) + # Block and unblock first and second slaves and check that they are recovered add_test_executable(mxs874_slave_recovery.cpp mxs874_slave_recovery mxs874 LABELS readwritesplit REPL_BACKEND) diff --git a/maxscale-system-test/mxs1719.cpp b/maxscale-system-test/mxs1719.cpp index 9d60ca768..efb634ea3 100644 --- a/maxscale-system-test/mxs1719.cpp +++ b/maxscale-system-test/mxs1719.cpp @@ -75,8 +75,7 @@ int main(int argc, char* argv[]) { if (test.maxscales->start() == 0) { - // Give the monitor a few seconds to monitor the servers - sleep(5); + test.maxscales->wait_for_monitor(); if (test.maxscales->connect_rwsplit() == 0) { diff --git a/maxscale-system-test/mxs548_short_session_change_user.cpp b/maxscale-system-test/mxs548_short_session_change_user.cpp index d2b452beb..208575fcc 100644 --- a/maxscale-system-test/mxs548_short_session_change_user.cpp +++ b/maxscale-system-test/mxs548_short_session_change_user.cpp @@ -86,7 +86,7 @@ int main(int argc, char *argv[]) Test->repl->sync_slaves(); Test->tprintf("Creating user 'user' \n"); - execute_query(Test->maxscales->conn_rwsplit[0], (char *) "DROP USER user@'%%'"); + execute_query(Test->maxscales->conn_rwsplit[0], (char *) "DROP USER IF EXISTS user@'%%'"); execute_query(Test->maxscales->conn_rwsplit[0], (char *) "CREATE USER user@'%%' IDENTIFIED BY 'pass2'"); execute_query(Test->maxscales->conn_rwsplit[0], (char *) "GRANT SELECT ON test.* TO user@'%%'"); execute_query(Test->maxscales->conn_rwsplit[0], (char *) "DROP TABLE IF EXISTS test.t1"); diff --git a/maxscale-system-test/mxs872_roles.cpp b/maxscale-system-test/mxs872_roles.cpp new file mode 100644 index 000000000..23a583fea --- /dev/null +++ b/maxscale-system-test/mxs872_roles.cpp @@ -0,0 +1,60 @@ +/** + * MXS-872: MaxScale doesn't understand roles + * + * https://jira.mariadb.org/browse/MXS-872 + */ + +#include "testconnections.h" +#include + +using namespace std; + +int main(int argc, char** argv) +{ + TestConnections test(argc, argv); + + test.repl->connect(); + for (auto a : vector({"DROP DATABASE IF EXISTS my_db", + "CREATE DATABASE my_db", + "DROP ROLE IF EXISTS dba", + "CREATE ROLE dba", + "GRANT SELECT ON my_db.* TO dba", + "DROP USER IF EXISTS 'test'@'%'", + "DROP USER IF EXISTS 'test2'@'%'", + "CREATE USER 'test'@'%' IDENTIFIED BY 'test'", + "CREATE USER 'test2'@'%' IDENTIFIED BY 'test2'", + "GRANT dba TO 'test'@'%'", + "GRANT dba TO 'test2'@'%'", + "SET DEFAULT ROLE dba FOR 'test'@'%'"})) + { + test.try_query(test.repl->nodes[0], "%s", a.c_str()); + } + + // Wait for the users to replicate + test.repl->sync_slaves(); + + test.tprintf("Connect with a user that has a default role"); + MYSQL* conn = open_conn_db(test.maxscales->rwsplit_port[0], test.maxscales->IP[0], "my_db", "test", "test"); + test.assert(mysql_errno(conn) == 0, "Connection failed: %s", mysql_error(conn)); + char value[100] {}; + find_field(conn, "SELECT CURRENT_ROLE() AS role", "role", value); + test.assert(strcmp(value, "dba") == 0, "Current role should be 'dba' but is: %s", value); + mysql_close(conn); + + test.tprintf("Connect with a user that doesn't have a default role, expect failure"); + conn = open_conn_db(test.maxscales->rwsplit_port[0], test.maxscales->IP[0], "my_db", "test2", "test2"); + test.assert(mysql_errno(conn) != 0, "Connection should fail"); + mysql_close(conn); + + // Cleanup + for (auto a : vector({"DROP DATABASE IF EXISTS my_db", + "DROP ROLE IF EXISTS dba", + "DROP USER 'test'@'%'", + "DROP USER 'test2'@'%'"})) + { + execute_query_silent(test.repl->nodes[0], "%s", a.c_str()); + } + + test.repl->disconnect(); + return test.global_result; +} diff --git a/maxscale-system-test/testconnections.cpp b/maxscale-system-test/testconnections.cpp index 2b56fbe57..cb87e8206 100644 --- a/maxscale-system-test/testconnections.cpp +++ b/maxscale-system-test/testconnections.cpp @@ -643,7 +643,10 @@ void TestConnections::init_maxscale(int m) { const char * template_name = get_template_name(test_name); - stop_maxscale(m); + if (maxscale::start) + { + stop_maxscale(m); + } process_template(m, template_name, maxscales->access_homedir[m]); maxscales->ssh_node_f(m, true, diff --git a/server/modules/authenticator/MySQLAuth/dbusers.cc b/server/modules/authenticator/MySQLAuth/dbusers.cc index 61caff0de..e16377b15 100644 --- a/server/modules/authenticator/MySQLAuth/dbusers.cc +++ b/server/modules/authenticator/MySQLAuth/dbusers.cc @@ -50,14 +50,77 @@ FROM mysql.user AS u LEFT JOIN mysql.tables_priv AS t \ ON (u.user = t.user AND u.host = t.host) WHERE u.plugin IN ('', 'mysql_native_password') %s" + // Query used with MariaDB 10.1 and newer, supports roles +const char* mariadb_users_query = + // First, select all users + "SELECT t.user, t.host, t.db, t.select_priv, t.password FROM " + "( " + " SELECT u.user, u.host, d.db, u.select_priv, u.password AS password, u.is_role " + " FROM mysql.user AS u LEFT JOIN mysql.db AS d " + " ON (u.user = d.user AND u.host = d.host) " + " UNION " + " SELECT u.user, u.host, t.db, u.select_priv, u.password AS password, u.is_role " + " FROM mysql.user AS u LEFT JOIN mysql.tables_priv AS t " + " ON (u.user = t.user AND u.host = t.host) " + ") AS t " + // Discard any users that are roles + "WHERE t.is_role <> 'Y' %s " + "UNION " + // Then select all users again + "SELECT r.user, r.host, u.db, u.select_priv, t.password FROM " + "( " + " SELECT u.user, u.host, d.db, u.select_priv, u.password AS password, u.default_role " + " FROM mysql.user AS u LEFT JOIN mysql.db AS d " + " ON (u.user = d.user AND u.host = d.host) " + " UNION " + " SELECT u.user, u.host, t.db, u.select_priv, u.password AS password, u.default_role " + " FROM mysql.user AS u LEFT JOIN mysql.tables_priv AS t " + " ON (u.user = t.user AND u.host = t.host) " + ") AS t " + // Join it to the roles_mapping table to only have users with roles + "JOIN mysql.roles_mapping AS r " + "ON (r.user = t.user AND r.host = t.host) " + // Then join it into itself to get the privileges of the role with the name of the user + "JOIN " + "( " + " SELECT u.user, u.host, d.db, u.select_priv, u.password AS password, u.is_role " + " FROM mysql.user AS u LEFT JOIN mysql.db AS d " + " ON (u.user = d.user AND u.host = d.host) " + " UNION " + " SELECT u.user, u.host, t.db, u.select_priv, u.password AS password, u.is_role " + " FROM mysql.user AS u LEFT JOIN mysql.tables_priv AS t " + " ON (u.user = t.user AND u.host = t.host) " + ") AS u " + "ON (u.user = r.role AND u.is_role = 'Y') " + // We only care about users that have a default role assigned + "WHERE t.default_role = u.user %s;"; + static int get_users(SERV_LISTENER *listener, bool skip_local); static MYSQL *gw_mysql_init(void); static int gw_mysql_set_timeouts(MYSQL* handle); static char *mysql_format_user_entry(void *data); static bool get_hostname(DCB *dcb, char *client_hostname, size_t size); -static char* get_new_users_query(const char *server_version, bool include_root) +static char* get_mariadb_users_query(bool include_root) { + const char *root = include_root ? "" : " AND t.user NOT IN ('root')"; + + size_t n_bytes = snprintf(NULL, 0, mariadb_users_query, root, root); + char *rval = static_cast(MXS_MALLOC(n_bytes + 1)); + MXS_ABORT_IF_NULL(rval); + snprintf(rval, n_bytes + 1, mariadb_users_query, root, root); + + return rval; +} + +static char* get_users_query(const char *server_version, uint64_t version, bool include_root) +{ + if (version >= 100101) // 10.1.1 or newer, supports default roles + { + return get_mariadb_users_query(include_root); + } + + // Either an older MariaDB version or a MySQL variant, use the legacy query const char* password = strstr(server_version, "5.7.") || strstr(server_version, "8.0.") ? MYSQL57_PASSWORD : MYSQL_PASSWORD; const char *with_root = include_root ? "" : " AND u.user NOT IN ('root')"; @@ -738,7 +801,9 @@ int get_users_from_server(MYSQL *con, SERVER_REF *server_ref, SERVICE *service, mxs_mysql_set_server_version(con, server_ref->server); } - char *query = get_new_users_query(server_ref->server->version_string, service->enable_root); + char *query = get_users_query(server_ref->server->version_string, + server_ref->server->version, + service->enable_root); MYSQL_AUTH *instance = (MYSQL_AUTH*)listener->auth_instance; sqlite3* handle = get_handle(instance); bool anon_user = false; diff --git a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc index 269ca6014..61ddf9e14 100644 --- a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc +++ b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc @@ -1056,9 +1056,17 @@ gw_read_normal_data(DCB *dcb, GWBUF *read_buffer, int nbytes_read) return 0; } - // Update the current command, required by KILL command processing + /** + * Update the current command, required by KILL command processing. + * If a COM_CHANGE_USER is in progress, this must not be done as the client + * is sending authentication data that does not have the command byte. + */ MySQLProtocol *proto = (MySQLProtocol*)dcb->protocol; - proto->current_command = (mxs_mysql_cmd_t)mxs_mysql_get_command(read_buffer); + + if (!proto->changing_user) + { + proto->current_command = (mxs_mysql_cmd_t)mxs_mysql_get_command(read_buffer); + } char* message = handle_variables(session, &read_buffer); @@ -1624,6 +1632,17 @@ static bool reauthenticate_client(MXS_SESSION* session, GWBUF* packetbuf) return rval; } +// Helper function for debug assertions +static bool only_one_packet(GWBUF* buffer) +{ + ss_dassert(buffer); + uint8_t header[4] = {}; + gwbuf_copy_data(buffer, 0, MYSQL_HEADER_LEN, header); + size_t packet_len = gw_mysql_get_byte3(header); + size_t buffer_len = gwbuf_length(buffer); + return packet_len + MYSQL_HEADER_LEN == buffer_len; +} + /** * Detect if buffer includes partial mysql packet or multiple packets. * Store partial packet to dcb_readqueue. Send complete packets one by one @@ -1645,15 +1664,15 @@ static int route_by_statement(MXS_SESSION* session, uint64_t capabilities, GWBUF GWBUF* packetbuf; do { - /** - * Collect incoming bytes to a buffer until complete packet has - * arrived and then return the buffer. - */ - // TODO: This should be replaced with modutil_get_next_MySQL_packet. - packetbuf = gw_MySQL_get_next_packet(p_readbuf); + // Process client request one packet at a time + packetbuf = modutil_get_next_MySQL_packet(p_readbuf); + + // TODO: Do this only when RCAP_TYPE_CONTIGUOUS_INPUT is requested + packetbuf = gwbuf_make_contiguous(packetbuf); if (packetbuf != NULL) { + ss_dassert(only_one_packet(packetbuf)); CHK_GWBUF(packetbuf); MySQLProtocol* proto = (MySQLProtocol*)session->client_dcb->protocol; @@ -1667,25 +1686,9 @@ static int route_by_statement(MXS_SESSION* session, uint64_t capabilities, GWBUF if (rcap_type_required(capabilities, RCAP_TYPE_CONTIGUOUS_INPUT)) { - if (!GWBUF_IS_CONTIGUOUS(packetbuf)) - { - // TODO: As long as gw_MySQL_get_next_packet is used above, the buffer - // TODO: will be contiguous. That function should be replaced with - // TODO: modutil_get_next_MySQL_packet. - GWBUF* tmp = gwbuf_make_contiguous(packetbuf); - if (tmp) - { - packetbuf = tmp; - } - else - { - // TODO: A memory allocation failure. We should close the dcb - // TODO: and terminate the session. - rc = 0; - goto return_rc; - } - } + ss_dassert(GWBUF_IS_CONTIGUOUS(packetbuf)); SERVICE *service = session->client_dcb->service; + if (rcap_type_required(capabilities, RCAP_TYPE_TRANSACTION_TRACKING) && !service->session_track_trx_state) { if (session_trx_is_ending(session)) @@ -1752,6 +1755,7 @@ static int route_by_statement(MXS_SESSION* session, uint64_t capabilities, GWBUF } else if (proto->changing_user) { + ss_dassert(proto->current_command == MXS_COM_CHANGE_USER); proto->changing_user = false; bool ok = reauthenticate_client(session, packetbuf); gwbuf_free(packetbuf); diff --git a/server/modules/protocol/MySQL/mysql_common.cc b/server/modules/protocol/MySQL/mysql_common.cc index a8e85dd26..43d7863dc 100644 --- a/server/modules/protocol/MySQL/mysql_common.cc +++ b/server/modules/protocol/MySQL/mysql_common.cc @@ -475,100 +475,6 @@ int mysql_send_auth_error(DCB *dcb, return sizeof(mysql_packet_header) + mysql_payload_size; } - -/** - * Buffer contains at least one of the following: - * complete [complete] [partial] mysql packet - * - * @param p_readbuf Address of read buffer pointer - * - * @return pointer to gwbuf containing a complete packet or - * NULL if no complete packet was found. - */ -GWBUF* gw_MySQL_get_next_packet(GWBUF** p_readbuf) -{ - GWBUF* packetbuf; - GWBUF* readbuf; - size_t buflen; - size_t packetlen; - size_t totalbuflen; - uint8_t* data; - size_t nbytes_copied = 0; - uint8_t* target; - - readbuf = *p_readbuf; - - if (readbuf == NULL) - { - packetbuf = NULL; - goto return_packetbuf; - } - CHK_GWBUF(readbuf); - - if (GWBUF_EMPTY(readbuf)) - { - packetbuf = NULL; - goto return_packetbuf; - } - totalbuflen = gwbuf_length(readbuf); - data = (uint8_t *)GWBUF_DATA((readbuf)); - packetlen = MYSQL_GET_PAYLOAD_LEN(data) + 4; - - /** packet is incomplete */ - if (packetlen > totalbuflen) - { - packetbuf = NULL; - goto return_packetbuf; - } - - packetbuf = gwbuf_alloc(packetlen); - target = GWBUF_DATA(packetbuf); - packetbuf->gwbuf_type = readbuf->gwbuf_type; /*< Copy the type too */ - /** - * Copy first MySQL packet to packetbuf and leave posible other - * packets to read buffer. - */ - while (nbytes_copied < packetlen && totalbuflen > 0) - { - uint8_t* src = GWBUF_DATA((*p_readbuf)); - size_t bytestocopy; - - buflen = GWBUF_LENGTH((*p_readbuf)); - bytestocopy = buflen < (packetlen - nbytes_copied) ? buflen : packetlen - nbytes_copied; - - memcpy(target + nbytes_copied, src, bytestocopy); - *p_readbuf = gwbuf_consume((*p_readbuf), bytestocopy); - totalbuflen = gwbuf_length((*p_readbuf)); - nbytes_copied += bytestocopy; - } - ss_dassert(buflen == 0 || nbytes_copied == packetlen); - -return_packetbuf: - return packetbuf; -} - -/** - * Move from buffer pointed to by <*p_readbuf>. - * Appears to be unused 11 May 2016 (Martin) - */ -GWBUF* gw_MySQL_get_packets(GWBUF** p_srcbuf, - int* npackets) -{ - GWBUF* packetbuf; - GWBUF* targetbuf = NULL; - - while (*npackets > 0 && (packetbuf = gw_MySQL_get_next_packet(p_srcbuf)) != NULL) - { - targetbuf = gwbuf_append(targetbuf, packetbuf); - *npackets -= 1; - } - ss_dassert(*npackets < 128); - ss_dassert(*npackets >= 0); - - return targetbuf; -} - - static server_command_t* server_command_init(server_command_t* srvcmd, mxs_mysql_cmd_t cmd) { diff --git a/server/modules/routing/readwritesplit/rwsplitsession.cc b/server/modules/routing/readwritesplit/rwsplitsession.cc index 4a2d55658..a6f1316de 100644 --- a/server/modules/routing/readwritesplit/rwsplitsession.cc +++ b/server/modules/routing/readwritesplit/rwsplitsession.cc @@ -321,6 +321,22 @@ void RWSplitSession::correct_packet_sequence(GWBUF *buffer) } } +static bool connection_was_killed(GWBUF* buffer) +{ + bool rval = false; + + if (mxs_mysql_is_err_packet(buffer)) + { + uint8_t buf[2]; + // First two bytes after the 0xff byte are the error code + gwbuf_copy_data(buffer, MYSQL_HEADER_LEN + 1, 2, buf); + uint16_t errcode = gw_mysql_get_byte2(buf); + rval = errcode == ER_CONNECTION_KILLED; + } + + return rval; +} + static void log_unexpected_response(SRWBackend& backend, GWBUF* buffer, GWBUF* current_query) { if (mxs_mysql_is_err_packet(buffer)) @@ -333,18 +349,9 @@ static void log_unexpected_response(SRWBackend& backend, GWBUF* buffer, GWBUF* c uint16_t errcode = MYSQL_GET_ERRCODE(data); std::string errstr((char*)data + 7, (char*)data + 7 + len - 3); - if (errcode == ER_CONNECTION_KILLED) - { - MXS_INFO("Connection from '%s'@'%s' to '%s' was killed", - backend->dcb()->session->client_dcb->user, - backend->dcb()->session->client_dcb->remote, - backend->name()); - } - else - { - MXS_WARNING("Server '%s' sent an unexpected error: %hu, %s", - backend->name(), errcode, errstr.c_str()); - } + ss_dassert(errcode != ER_CONNECTION_KILLED); + MXS_WARNING("Server '%s' sent an unexpected error: %hu, %s", + backend->name(), errcode, errstr.c_str()); } else { @@ -449,11 +456,20 @@ void RWSplitSession::clientReply(GWBUF *writebuf, DCB *backend_dcb) if (backend->get_reply_state() == REPLY_STATE_DONE) { - /** If we receive an unexpected response from the server, the internal - * logic cannot handle this situation. Routing the reply straight to - * the client should be the safest thing to do at this point. */ - log_unexpected_response(backend, writebuf, m_current_query.get()); - MXS_SESSION_ROUTE_REPLY(backend_dcb->session, writebuf); + if (connection_was_killed(writebuf)) + { + // The connection was killed, we can safely ignore it. When the TCP connection is + // closed, the router's error handling will sort it out. + gwbuf_free(writebuf); + } + else + { + /** If we receive an unexpected response from the server, the internal + * logic cannot handle this situation. Routing the reply straight to + * the client should be the safest thing to do at this point. */ + log_unexpected_response(backend, writebuf, m_current_query.get()); + MXS_SESSION_ROUTE_REPLY(backend_dcb->session, writebuf); + } return; }