diff --git a/Documentation/Filters/Masking.md b/Documentation/Filters/Masking.md index 4b68df3a3..d87151c6c 100644 --- a/Documentation/Filters/Masking.md +++ b/Documentation/Filters/Masking.md @@ -37,25 +37,14 @@ the _ssn_ would be masked, as in ## Security -Note that he masking filter alone is *not* sufficient for preventing -access to a particular column. As the masking filter works on the column -name alone a query like -``` -> SELECT name, concat(ssn) FROM person; -``` -will reveal the value. Also, executing a query like -``` -> SELECT name FROM person WHERE ssn = ...; -``` -a sufficient number of times with different _ssn_ values, will, eventually, -reveal the social security number of all persons in the database. +From MaxScale 2.3 onwards, the masking filter will reject statements +that use functions in conjunction with columns that should be masked. +Allowing function usage provides a way for circumventing the masking, +unless a firewall filter is separately configured and installed. -For a secure solution, the masking filter *must* be combined with the -firewall filter to prevent the use of functions using which the masking -can be bypassed. - -In a future release, the combined use of the masking filter and the -database firewall filter will be simplified. +Please see the configuration parameter +[prevent_function_usage](#prevent_function_usage) +for how to change the default behaviour. ## Limitations diff --git a/maxscale-system-test/cnf/maxscale.cnf.template.mxs359_read_only b/maxscale-system-test/cnf/maxscale.cnf.template.mxs359_read_only index 45249cce3..cc1816d43 100644 --- a/maxscale-system-test/cnf/maxscale.cnf.template.mxs359_read_only +++ b/maxscale-system-test/cnf/maxscale.cnf.template.mxs359_read_only @@ -8,6 +8,7 @@ servers=server1,server2,server3,server4 user=maxskysql password=skysql monitor_interval=1000 +failcount=1 [RW-Split-Router] type=service diff --git a/maxscale-system-test/mxs1776_ps_exec_hang.cpp b/maxscale-system-test/mxs1776_ps_exec_hang.cpp index 09c41799c..e8d0c0d9e 100644 --- a/maxscale-system-test/mxs1776_ps_exec_hang.cpp +++ b/maxscale-system-test/mxs1776_ps_exec_hang.cpp @@ -50,8 +50,9 @@ void run_test(TestConnections& test, TestCase test_case) cout << test_case.name << endl; test.expect(test_case.func(test.maxscales->conn_rwsplit[0], stmt, bind), - "Test '%s' failed", - test_case.name.c_str()); + "Test '%s' failed: %s %s", test_case.name.c_str(), + mysql_error(test.maxscales->conn_rwsplit[0]), + mysql_stmt_error(stmt)); mysql_stmt_close(stmt); diff --git a/maxscale-system-test/mxs359_read_only.cpp b/maxscale-system-test/mxs359_read_only.cpp index 34af9a794..7e45da89a 100644 --- a/maxscale-system-test/mxs359_read_only.cpp +++ b/maxscale-system-test/mxs359_read_only.cpp @@ -65,7 +65,7 @@ void test_new_master(TestConnections& test, std::ostream& out) test.try_query(test.maxscales->conn_rwsplit[0], "SELECT * FROM test.t1"); change_master(1, 0); - test.maxscales->wait_for_monitor(); + test.maxscales->wait_for_monitor(2); out << "Both reads and writes after master change should work" << endl; test.try_query(test.maxscales->conn_rwsplit[0], "INSERT INTO test.t1 VALUES (2)"); diff --git a/maxscale-system-test/testconnections.cpp b/maxscale-system-test/testconnections.cpp index 8611658a4..510f7456c 100644 --- a/maxscale-system-test/testconnections.cpp +++ b/maxscale-system-test/testconnections.cpp @@ -395,6 +395,11 @@ TestConnections::~TestConnections() delete galera; } + if (maxscale::multiple_maxscales) + { + maxscales->stop_all(); + } + if (global_result) { // This causes the test to fail if a core dump is found diff --git a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc index 66c5356f1..23850111d 100644 --- a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc +++ b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc @@ -34,7 +34,7 @@ #include #include #include -#include +#include #include #include #include @@ -424,6 +424,7 @@ int MySQLSendHandshake(DCB* dcb) // writing data in the Client buffer queue dcb->func.write(dcb, buf); + protocol->protocol_auth_state = MXS_AUTH_STATE_MESSAGE_READ; return sizeof(mysql_packet_header) + mysql_payload_size; } @@ -1422,25 +1423,6 @@ int gw_MySQLAccept(DCB* listener) static void gw_process_one_new_client(DCB* client_dcb) { - MySQLProtocol* protocol; - - protocol = mysql_protocol_init(client_dcb, client_dcb->fd); - - if (protocol == NULL) - { - /** delete client_dcb */ - dcb_close(client_dcb); - MXS_ERROR("Failed to create protocol object for client connection."); - return; - } - client_dcb->protocol = protocol; - - // send handshake to the client_dcb - MySQLSendHandshake(client_dcb); - - // client protocol state change - protocol->protocol_auth_state = MXS_AUTH_STATE_MESSAGE_READ; - /** * Set new descriptor to event set. At the same time, * change state to DCB_STATE_POLLING so that @@ -1466,6 +1448,15 @@ static void gw_process_one_new_client(DCB* client_dcb) } else { + // Move the rest of the initialization process to the owning worker + mxs::RoutingWorker* worker = static_cast(client_dcb->poll.owner); + + worker->execute([=](){ + client_dcb->protocol = mysql_protocol_init(client_dcb, client_dcb->fd); + MXS_ABORT_IF_NULL(client_dcb->protocol); + MySQLSendHandshake(client_dcb); + }, mxs::RoutingWorker::EXECUTE_AUTO); + MXS_DEBUG("Added dcb %p for fd %d to epoll set.", client_dcb, client_dcb->fd); diff --git a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc index d673de23d..a8802cc7e 100644 --- a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc +++ b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc @@ -106,6 +106,12 @@ void replace_binary_ps_id(GWBUF* buffer, uint32_t id) uint8_t* ptr = GWBUF_DATA(buffer) + MYSQL_PS_ID_OFFSET; gw_mysql_set_byte4(ptr, id); } + +uint32_t extract_binary_ps_id(GWBUF* buffer) +{ + uint8_t* ptr = GWBUF_DATA(buffer) + MYSQL_PS_ID_OFFSET; + return gw_mysql_get_byte4(ptr); +} } bool RWSplitSession::have_connected_slaves() const @@ -178,14 +184,6 @@ bool RWSplitSession::route_single_stmt(GWBUF* querybuf) uint8_t command = info.command(); uint32_t qtype = info.type_mask(); route_target_t route_target = info.target(); - bool not_locked_to_master = !is_locked_to_master(); - - if (not_locked_to_master && mxs_mysql_is_ps_command(command) && !m_qc.large_query()) - { - /** Replace the client statement ID with our internal one only if the - * target node is not the current master */ - replace_binary_ps_id(querybuf, stmt_id); - } SRWBackend target; @@ -312,7 +310,7 @@ bool RWSplitSession::route_single_stmt(GWBUF* querybuf) // Target server was found and is in the correct state succp = handle_got_target(querybuf, target, store_stmt); - if (succp && command == MXS_COM_STMT_EXECUTE && not_locked_to_master) + if (succp && command == MXS_COM_STMT_EXECUTE && !is_locked_to_master()) { /** Track the targets of the COM_STMT_EXECUTE statements. This * information is used to route all COM_STMT_FETCH commands @@ -1106,6 +1104,16 @@ bool RWSplitSession::handle_got_target(GWBUF* querybuf, SRWBackend& target, bool */ mxb_assert(target->get_reply_state() == REPLY_STATE_DONE || m_qc.large_query()); + uint32_t orig_id = 0; + + if (!is_locked_to_master() && mxs_mysql_is_ps_command(cmd) && !m_qc.large_query()) + { + // Store the original ID in case routing fails + orig_id = extract_binary_ps_id(querybuf); + // Replace the ID with our internal one, the backends will replace it with their own ID + replace_binary_ps_id(querybuf, m_qc.current_route_info().stmt_id()); + } + /** * If we are starting a new query, we use RWBackend::write, otherwise we use * RWBackend::continue_write to continue an ongoing query. RWBackend::write @@ -1156,11 +1164,17 @@ bool RWSplitSession::handle_got_target(GWBUF* querybuf, SRWBackend& target, bool { m_target_node.reset(); } - return true; } else { + if (orig_id) + { + // Put the original ID back in case we try to route the query again + replace_binary_ps_id(querybuf, orig_id); + } + MXS_ERROR("Routing query failed."); - return false; } + + return success; }