MDEV-8598 : Failed MySQL DDL commands and Galera replication

RENAME TABLE, unlike other DDLs, was getting replicated before
the access check was performed. As a result, the command could
get get replicated and thus executed on other nodes, even if it
fails on the originating node due to permission issues. Fixed by
moving the logic to check user privileges before replicating the
command.

mysql-test/suite/galera/r/rename.result

+#
+# MDEV-8598 : Failed MySQL DDL commands and Galera replication
+#
+# On node 1
+USE test;
+DROP TABLE IF EXISTS t1, t2;
+CREATE TABLE t1(i INT) ENGINE=INNODB;
+INSERT INTO t1 VALUE(1);
+SELECT * FROM t1;
+i
+1
+# Create a new user 'foo' with limited privileges
+GRANT SELECT on test.* TO foo@localhost;
+# Open connection to the 1st node using 'test_user1' user.
+SELECT * FROM t1;
+i
+1
+# Following RENAME should not replicate to other node.
+RENAME TABLE t1 TO t2;
+ERROR 42000: DROP,ALTER command denied to user 'foo'@'localhost' for table 't1'
+# On node 2
+USE test;
+SELECT * FROM t1;
+i
+1
+# On node_1
+RENAME TABLE t1 TO t2;
+SHOW TABLES;
+Tables_in_test
+t2
+# On node 2
+USE test;
+SELECT * FROM t2;
+i
+1
+DROP USER foo@localhost;
+DROP TABLE t2;
+# End of tests

mysql-test/suite/galera/t/rename.test

+--source include/galera_cluster.inc
+--source include/have_innodb.inc
+
+--echo #
+--echo # MDEV-8598 : Failed MySQL DDL commands and Galera replication
+--echo #
+--echo # On node 1
+--connection node_1
+USE test;
+--disable_warnings
+DROP TABLE IF EXISTS t1, t2;
+--enable_warnings
+
+CREATE TABLE t1(i INT) ENGINE=INNODB;
+INSERT INTO t1 VALUE(1);
+SELECT * FROM t1;
+
+--echo # Create a new user 'foo' with limited privileges
+GRANT SELECT on test.* TO foo@localhost;
+
+--echo # Open connection to the 1st node using 'test_user1' user.
+--let $port_1= \$NODE_MYPORT_1
+--connect(foo_node_1,localhost,foo,,test,$port_1,)
+
+--connection foo_node_1
+SELECT * FROM t1;
+--echo # Following RENAME should not replicate to other node.
+--error ER_TABLEACCESS_DENIED_ERROR
+RENAME TABLE t1 TO t2;
+
+--echo # On node 2
+--connection node_2
+USE test;
+SELECT * FROM t1;
+
+--echo # On node_1
+--connection node_1
+RENAME TABLE t1 TO t2;
+SHOW TABLES;
+
+--echo # On node 2
+--connection node_2
+USE test;
+SELECT * FROM t2;
+
+# Cleanup
+--connection node_1
+DROP USER foo@localhost;
+DROP TABLE t2;
+
+--echo # End of tests
+

sql/sql_parse.cc

@@ -52,7 +52,7 @@
 #include "sql_connect.h"      // decrease_user_connections,
                               // check_mqh,
                               // reset_mqh
-#include "sql_rename.h"       // mysql_rename_table
+#include "sql_rename.h"       // mysql_rename_tables
 #include "sql_tablespace.h"   // mysql_alter_tablespace
 #include "hostname.h"         // hostname_cache_refresh
 #include "sql_acl.h"          // *_ACL, check_grant, is_acl_user,
@@ -127,7 +127,7 @@ static bool execute_sqlcom_select(THD *thd, TABLE_LIST *all_tables);
 static void sql_kill(THD *thd, ulong id, killed_state state);
 static void sql_kill_user(THD *thd, LEX_USER *user, killed_state state);
 static bool execute_show_status(THD *, TABLE_LIST *);
-static bool execute_rename_table(THD *, TABLE_LIST *, TABLE_LIST *);
+static bool check_rename_table(THD *, TABLE_LIST *, TABLE_LIST *);
 
 const char *any_db="*any*";	// Special symbol for check_access
 
@@ -3055,8 +3055,12 @@ case SQLCOM_PREPARE:
 
   case SQLCOM_RENAME_TABLE:
   {
+    if (check_rename_table(thd, first_table, all_tables))
+      goto error;
+
     WSREP_TO_ISOLATION_BEGIN(0, 0, first_table)
-    if (execute_rename_table(thd, first_table, all_tables))
+
+    if (mysql_rename_tables(thd, first_table, 0))
       goto error;
     break;
   }
@@ -5226,8 +5230,8 @@ static bool execute_show_status(THD *thd, TABLE_LIST *all_tables)
 }
 
 
-static bool execute_rename_table(THD *thd, TABLE_LIST *first_table,
-                                 TABLE_LIST *all_tables)
+static bool check_rename_table(THD *thd, TABLE_LIST *first_table,
+                               TABLE_LIST *all_tables)
 {
   DBUG_ASSERT(first_table == all_tables && first_table != 0);
   TABLE_LIST *table;
@@ -5241,7 +5245,7 @@ static bool execute_rename_table(THD *thd, TABLE_LIST *first_table,
                      &table->next_local->grant.privilege,
                      &table->next_local->grant.m_internal,
                      0, 0))
-      return 1;
+      return true;
     TABLE_LIST old_list, new_list;
     /*
       we do not need initialize old_list and new_list because we will
@@ -5254,10 +5258,10 @@ static bool execute_rename_table(THD *thd, TABLE_LIST *first_table,
                        INSERT_ACL | CREATE_ACL) &&
         check_grant(thd, INSERT_ACL | CREATE_ACL, &new_list, FALSE, 1,
                     FALSE)))
-      return 1;
+      return true;
   }
 
-  return mysql_rename_tables(thd, first_table, 0);
+  return false;
 }