diff options
author | Brandon Nesterenko <brandon.nesterenko@mariadb.com> | 2022-04-26 19:51:42 -0600 |
---|---|---|
committer | Brandon Nesterenko <brandon.nesterenko@mariadb.com> | 2022-04-29 17:01:05 -0600 |
commit | 960223da39e6e7704b3b0e2de9fda404cc5f8d51 (patch) | |
tree | 101c69694eac4ebd4a306131cdadcdf66392bf93 | |
parent | 388032e99057449219d4a943b4407e36c42ec4af (diff) | |
download | mariadb-git-10.2-MDEV-28294-pre-exec.tar.gz |
MDEV-28294: set default role bypasses Replicate_Wild_Ignore_Table: mysql.%10.2-MDEV-28294-pre-exec
Problem:
========
When replicating SET DEFAULT ROLE, the pre-update check (i.e. that
in set_var_default_role::check()) tries to validate the existence of
the given rules/user even when the targeted tables are ignored. When
previously issued CREATE USER/ROLE commands are ignored by the
replica because of the replication filtering rules, this results in
an error because the targeted data does not exist.
Solution:
========
Before checking that the given rules/user exist of a SET DEFAULT
ROLE command, first ensure that the mysql.user and
mysql.roles_mapping tables are not excluded by replication filters.
Reviewed By
===========
Andrei Elkin <andrei.elkin@mariadb.com>
-rw-r--r-- | mysql-test/suite/rpl/r/rpl_filter_set_var_missing_data.result | 69 | ||||
-rw-r--r-- | mysql-test/suite/rpl/t/rpl_filter_set_var_missing_data.test | 100 | ||||
-rw-r--r-- | sql/set_var.cc | 16 | ||||
-rw-r--r-- | sql/set_var.h | 28 | ||||
-rw-r--r-- | sql/sql_acl.cc | 59 | ||||
-rw-r--r-- | sql/sql_acl.h | 2 | ||||
-rw-r--r-- | sql/sql_parse.cc | 32 |
7 files changed, 304 insertions, 2 deletions
diff --git a/mysql-test/suite/rpl/r/rpl_filter_set_var_missing_data.result b/mysql-test/suite/rpl/r/rpl_filter_set_var_missing_data.result new file mode 100644 index 00000000000..96b854ad83a --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_filter_set_var_missing_data.result @@ -0,0 +1,69 @@ +include/master-slave.inc +[connection master] +# +# Set replica to ignore system mysql tables +connection slave; +set @save_dbug=@@GLOBAL.debug_dbug; +set @@GLOBAL.debug_dbug="+d,sync_set_var_rpl_filtered"; +include/stop_slave.inc +SET @@GLOBAL.replicate_wild_ignore_table="mysql.%"; +include/start_slave.inc +# +# Execute grant-based commands on primary which modify mysql system +# tables +connection master; +CREATE ROLE journalist; +CREATE USER testuser@localhost IDENTIFIED by ''; +GRANT journalist to testuser@localhost; +# +# Execute SET commands which use the previous user/role data +SET DEFAULT ROLE journalist for testuser@localhost; +SET PASSWORD for testuser@localhost= PASSWORD('123'); +include/save_master_gtid.inc +# +# Verify primary's grant tables have the correct user/role data +select count(*)=1 from mysql.user where User='testuser'; +count(*)=1 +1 +select count(*)=1 from mysql.roles_mapping where User='testuser'; +count(*)=1 +1 +connection slave; +# +# This first synchronization validates that the rpl_filter will ignore +# the SET DEFAULT ROLE command +SET DEBUG_SYNC='now WAIT_FOR ignoring_event'; +SET DEBUG_SYNC='now SIGNAL ack_event_ignored'; +# +# This second synchronization validates that the rpl_filter will ignore +# the SET PASSWORD command +SET DEBUG_SYNC='now WAIT_FOR ignoring_event'; +SET DEBUG_SYNC='now SIGNAL ack_event_ignored'; +# +# Ensure that the replica receives all of the primary's events without +# error +include/sync_with_master_gtid.inc +Last_SQL_Error = +Last_SQL_Errno = 0 +# +# Verify that the replica did not execute the master's commands +select count(*)=0 from mysql.user where User='testuser'; +count(*)=0 +1 +select count(*)=0 from mysql.roles_mapping where User='testuser'; +count(*)=0 +1 +# +# Clean up +connection master; +DROP ROLE journalist; +DROP USER testuser@localhost; +include/save_master_gtid.inc +connection slave; +include/sync_with_master_gtid.inc +SET @@GLOBAL.debug_dbug=@save_dbug; +SET DEBUG_SYNC='RESET'; +include/stop_slave.inc +SET @@GLOBAL.replicate_wild_ignore_table=""; +include/start_slave.inc +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_filter_set_var_missing_data.test b/mysql-test/suite/rpl/t/rpl_filter_set_var_missing_data.test new file mode 100644 index 00000000000..3142e967192 --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_filter_set_var_missing_data.test @@ -0,0 +1,100 @@ +# +# Purpose: +# This test ensures that the SET DEFAULT ROLE and SET PASSWORD commands can +# be ignored by replica filter rules. MDEV-28294 exposed a bug in which +# SET DEFAULT ROLE would check for the existence of the given rules/user even +# when the targeted tables are ignored, resulting in errors if the targeted +# data does not exist. +# +# Methodology: +# Using a replica configured with replicate_wild_ignore_table="mysql.%", +# execute SET DEFAULT ROLE and SET PASSWORD on the primary and ensure that the +# replica neither errors nor executes the commands which the primary sends. +# DEBUG_SYNC points are used to validate that the rpl_filter ignores the +# commands at the expected point. +# +# References: +# MDEV-28294: set default role bypasses Replicate_Wild_Ignore_Table: mysql.% +# + +source include/master-slave.inc; +source include/have_binlog_format_mixed.inc; +source include/have_debug.inc; +source include/have_debug_sync.inc; + +--echo # +--echo # Set replica to ignore system mysql tables +connection slave; +set @save_dbug=@@GLOBAL.debug_dbug; +set @@GLOBAL.debug_dbug="+d,sync_set_var_rpl_filtered"; +let $old_filter= query_get_value(SHOW SLAVE STATUS, Replicate_Wild_Ignore_Table, 1); +source include/stop_slave.inc; +SET @@GLOBAL.replicate_wild_ignore_table="mysql.%"; +source include/start_slave.inc; + +--echo # +--echo # Execute grant-based commands on primary which modify mysql system +--echo # tables +connection master; +CREATE ROLE journalist; +CREATE USER testuser@localhost IDENTIFIED by ''; +GRANT journalist to testuser@localhost; + +--echo # +--echo # Execute SET commands which use the previous user/role data +SET DEFAULT ROLE journalist for testuser@localhost; +SET PASSWORD for testuser@localhost= PASSWORD('123'); +--source include/save_master_gtid.inc + +--echo # +--echo # Verify primary's grant tables have the correct user/role data +select count(*)=1 from mysql.user where User='testuser'; +select count(*)=1 from mysql.roles_mapping where User='testuser'; + +connection slave; +--echo # +--echo # This first synchronization validates that the rpl_filter will ignore +--echo # the SET DEFAULT ROLE command +SET DEBUG_SYNC='now WAIT_FOR ignoring_event'; +SET DEBUG_SYNC='now SIGNAL ack_event_ignored'; + +--echo # +--echo # This second synchronization validates that the rpl_filter will ignore +--echo # the SET PASSWORD command +SET DEBUG_SYNC='now WAIT_FOR ignoring_event'; +SET DEBUG_SYNC='now SIGNAL ack_event_ignored'; + +--echo # +--echo # Ensure that the replica receives all of the primary's events without +--echo # error +--source include/sync_with_master_gtid.inc +let $error= query_get_value(SHOW SLAVE STATUS, Last_SQL_Error, 1); +--echo Last_SQL_Error = $error +let $errno= query_get_value(SHOW SLAVE STATUS, Last_SQL_Errno, 1); +--echo Last_SQL_Errno = $errno + +--echo # +--echo # Verify that the replica did not execute the master's commands +select count(*)=0 from mysql.user where User='testuser'; +select count(*)=0 from mysql.roles_mapping where User='testuser'; + +--echo # +--echo # Clean up + +# The master has to drop the role/user combination while the slave still has +# its filters active; otherwise, the slave would try to drop users/roles that +# were never replicated. +--connection master +DROP ROLE journalist; +DROP USER testuser@localhost; +--source include/save_master_gtid.inc + +--connection slave +--source include/sync_with_master_gtid.inc +SET @@GLOBAL.debug_dbug=@save_dbug; +SET DEBUG_SYNC='RESET'; +source include/stop_slave.inc; +--eval SET @@GLOBAL.replicate_wild_ignore_table="$old_filter" +source include/start_slave.inc; + +--source include/rpl_end.inc diff --git a/sql/set_var.cc b/sql/set_var.cc index 569362fb7bd..60ed4c8dc5c 100644 --- a/sql/set_var.cc +++ b/sql/set_var.cc @@ -936,6 +936,14 @@ int set_var_password::update(THD *thd) #endif } +void set_var_password::get_modified_tables(TABLE_LIST **tables) +{ + size_t n_tables; + acl_get_tables_set_password(&user_table, &n_tables); + DBUG_ASSERT(n_tables == 1); + *tables= &user_table; +} + /***************************************************************************** Functions to handle SET ROLE *****************************************************************************/ @@ -1001,6 +1009,14 @@ int set_var_default_role::update(THD *thd) #endif } +void set_var_default_role::get_modified_tables(TABLE_LIST **tables) +{ + size_t n_tables; + acl_get_tables_set_default_role(&roles_mapping_table, &n_tables); + DBUG_ASSERT(n_tables == 1); + *tables= &roles_mapping_table; +} + /***************************************************************************** Functions to handle SET NAMES and SET CHARACTER SET *****************************************************************************/ diff --git a/sql/set_var.h b/sql/set_var.h index b43e8f96c59..1bdefab0003 100644 --- a/sql/set_var.h +++ b/sql/set_var.h @@ -274,6 +274,21 @@ public: virtual int update(THD *thd)=0; /* To set the value */ virtual int light_check(THD *thd) { return check(thd); } /* for PS */ virtual bool is_system() { return FALSE; } + + /* + Output any tables that will be modified during the update process. This is + used for rpl_filter validation to ignore SET commands which should not be + replicated. + + @param tables [out] The address of the pointer which should be updated to + reference the list of modified tables. Will be NULL if no + tables will be modified. + + */ + virtual void get_modified_tables(TABLE_LIST **tables) + { + *tables= NULL; + } }; @@ -325,11 +340,15 @@ public: class set_var_password: public set_var_base { LEX_USER *user; + TABLE_LIST user_table; public: set_var_password(LEX_USER *user_arg) :user(user_arg) - {} + { + user_table.next_local= user_table.next_global= NULL; + } int check(THD *thd); int update(THD *thd); + void get_modified_tables(TABLE_LIST **tables); }; /* For SET ROLE */ @@ -351,11 +370,16 @@ class set_var_default_role: public set_var_base LEX_USER *user, *real_user; LEX_STRING role; const char *real_role; + TABLE_LIST roles_mapping_table; public: set_var_default_role(LEX_USER *user_arg, LEX_STRING role_arg) : - user(user_arg), role(role_arg) {} + user(user_arg), role(role_arg) + { + roles_mapping_table.next_local= roles_mapping_table.next_global= NULL; + } int check(THD *thd); int update(THD *thd); + void get_modified_tables(TABLE_LIST **tables); }; /* For SET NAMES and SET CHARACTER SET */ diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index f62dd5471eb..403c296ce10 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -1428,6 +1428,50 @@ class Grant_tables return m_roles_mapping_table; } + /* + Outputs the TABLE_LIST corresponding to the table specification of + which_tables. + + @param which_tables [in] Bit map specifying which tables to include in the + output. See enum_acl_tables + + @param lock_type [in] Lock type for the tables + + @param tbls_out [out] Empty TABLE_LIST which will be populated using + the specification from which_tables + + @param n_tables [out] The number of tables output in tbls_out + */ + static void get_table_list(uint which_tables, enum thr_lock_type lock_type, + TABLE_LIST *tbls_out, size_t *n_tables) + { + DBUG_ENTER("Grant_tables::get_table_list"); + Grant_tables tables(which_tables, lock_type); + TABLE_LIST *my_tl= &(tables.first_table_in_list->tl); + *n_tables= 0; + DBUG_ASSERT(which_tables < ( 1 << TABLES_MAX)); + while (which_tables) + { + if (which_tables & 1) + { + tbls_out->init_one_table( + my_tl->db, my_tl->db_length, my_tl->table_name, + my_tl->table_name_length, my_tl->alias, my_tl->lock_type); + *n_tables+= 1; + + /* + Stop if this was the last table to populate + */ + if (!(tbls_out->next_local)) + break; + + tbls_out= tbls_out->next_local; + } + which_tables >>= 1; + } + DBUG_VOID_RETURN; + } + private: User_table m_user_table; Db_table m_db_table; @@ -3558,6 +3602,21 @@ WSREP_ERROR_LABEL: DBUG_RETURN(result); } +void acl_get_tables_set_password(TABLE_LIST *tables, size_t *n_tables) +{ + DBUG_ENTER("acl_get_tables_set_password"); + Grant_tables::get_table_list(Table_user, TL_WRITE, tables, n_tables); + DBUG_VOID_RETURN; +} + +void acl_get_tables_set_default_role(TABLE_LIST *tables, size_t *n_tables) +{ + DBUG_ENTER("acl_get_tables_set_default_role"); + Grant_tables::get_table_list(Table_roles_mapping, TL_WRITE, tables, + n_tables); + DBUG_VOID_RETURN; +} + int acl_check_set_default_role(THD *thd, const char *host, const char *user, const char *role) { diff --git a/sql/sql_acl.h b/sql/sql_acl.h index e9063673b8e..677884f04fc 100644 --- a/sql/sql_acl.h +++ b/sql/sql_acl.h @@ -412,6 +412,8 @@ int acl_check_setrole(THD *thd, char *rolename, ulonglong *access); int acl_check_set_default_role(THD *thd, const char *host, const char *user, const char *role); int acl_set_default_role(THD *thd, const char *host, const char *user, const char *rolename); +void acl_get_tables_set_password(TABLE_LIST *out, size_t *n_tables); +void acl_get_tables_set_default_role(TABLE_LIST *out, size_t *n_tables); extern SHOW_VAR acl_statistics[]; diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 989ca0c8803..38df31ae16c 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -3312,6 +3312,38 @@ mysql_execute_command(THD *thd) MYF(0)); DBUG_RETURN(0); } + + /* + Check if the SET command will modify a table that should be ignored in + replication. + */ + if (lex->sql_command == SQLCOM_SET_OPTION) + { + List<set_var_base> *lex_var_list= &lex->var_list; + List_iterator_fast<set_var_base> it(*lex_var_list); + set_var_base *set_var; + while ((set_var=it++)) + { + TABLE_LIST *tables; + set_var->get_modified_tables(&tables); + if (tables && all_tables_not_ok(thd, tables)) + { + /* + Used in MTR to prove that the event was ignored _here_ + */ + DBUG_EXECUTE_IF( + "sync_set_var_rpl_filtered", + DBUG_ASSERT(!debug_sync_set_action( + thd, STRING_WITH_LEN("now SIGNAL ignoring_event WAIT_FOR " + "ack_event_ignored")));); + /* warn the slave SQL thread */ + my_message(ER_SLAVE_IGNORED_TABLE, + ER_THD(thd, ER_SLAVE_IGNORED_TABLE), MYF(0)); + DBUG_RETURN(0); + } + } + } + /* Execute deferred events first */ |