Merge branch 'mysql-8.0-bug30600321' into mysql-8.0

Author: jdduncan

mysql-test/suite/ndb/r/stored_grants_error_handling.result

@@ -2,4 +2,17 @@ CREATE USER "mcmd"@"localhost";
 GRANT ALL PRIVILEGES ON *.* to "mcmd"@"localhost";
 CALL mtr.add_suppression("NDB: Error 626, Tuple did not exist");
 GRANT ALL PRIVILEGES ON *.* to "mcmd"@"localhost";
+Expect 33
+max_questions
+33
+Expect 0
+max_questions
+0
+Expect 55
+max_questions
+55
+Expect 0
+max_questions
+0
 DROP USER mcmd@localhost;
+DROP USER lu1@a;

mysql-test/suite/ndb/t/stored_grants_error_handling.test

@@ -1,7 +1,14 @@
 --source include/have_ndb.inc
 --source suite/ndb/include/ndb_find_tools.inc
 
+#
+# Open connections
+#
+connect(mysqld2,127.0.0.1,root,,test,$MASTER_MYPORT1);
+
+#
 # Bug#30556487 MYSQLD HANG IN NDB_STORED_GRANTS CODE ON CREATE USER
+#
 CREATE USER "mcmd"@"localhost";
 GRANT ALL PRIVILEGES ON *.* to "mcmd"@"localhost";
 
@@ -13,6 +20,60 @@ GRANT ALL PRIVILEGES ON *.* to "mcmd"@"localhost";
 CALL mtr.add_suppression("NDB: Error 626, Tuple did not exist");
 GRANT ALL PRIVILEGES ON *.* to "mcmd"@"localhost";
 
+#
+# Bug#30600321 MAX_QUERIES_PER_HOUR 0 ONLY REMOVES THE LIMIT ON 1 MYSQLD
+#
+--enable_result_log
+--disable_query_log
+
+# Bug#30600321 Test (1): Alter resource limit of stored user
+ALTER USER "mcmd"@"localhost" WITH MAX_QUERIES_PER_HOUR 33;
+
+connection mysqld2;
+echo Expect 33;
+SELECT max_questions from mysql.user where user = 'mcmd';
+
+connection default;
+ALTER USER "mcmd"@"localhost" WITH MAX_QUERIES_PER_HOUR 0;
+
+connection mysqld2;
+let $max= `SELECT max_questions from mysql.user where user = 'mcmd'`;
+if ($max != 0)
+{
+  die Test failed -- max_questions was not zero;
+}
+
+# Bug#30600321 Test (2): Alter both resource limit and password of stored user
+connection default;
+ALTER USER "mcmd"@"localhost" WITH MAX_QUERIES_PER_HOUR 44;
+ALTER USER "mcmd"@"localhost" IDENTIFIED BY "Garb_farb_earb" WITH MAX_QUERIES_PER_HOUR 0;
+
+connection mysqld2;
+echo Expect 0;
+SELECT max_questions from mysql.user where user = 'mcmd';
+
+# Bug#30600321 Test (3): Alter resource limit of stored and non-stored users
+connection default;
+CREATE USER "lu1"@"a";
+ALTER USER "lu1"@"a", "mcmd"@"localhost" WITH MAX_QUERIES_PER_HOUR 55;
+
+connection mysqld2;
+echo Expect 55;
+SELECT max_questions from mysql.user where user = 'mcmd';
+
+connection default;
+ALTER USER "lu1"@"a", "mcmd"@"localhost" WITH MAX_QUERIES_PER_HOUR 0;
+
+connection mysqld2;
+echo Expect 0;
+SELECT max_questions from mysql.user where user = 'mcmd';
+
+--enable_query_log
+
+#
 # Cleanup
+#
+connection default;
 DROP USER mcmd@localhost;
+DROP USER lu1@a;
 

sql/auth/acl_change_notification.h

@@ -27,6 +27,8 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301  USA
 #include "my_sqlcommand.h"  // enum_sql_command
 #include "sql/table.h"      // LEX_USER, LEX_CSTRING, List
 
+class Rewrite_params;  // forward declaration
+
 class Acl_change_notification {
  public:
   struct User {
@@ -39,23 +41,24 @@ class Acl_change_notification {
 
   Acl_change_notification(class THD *thd, enum_sql_command op,
                           const List<LEX_USER> *users,
+                          Rewrite_params *rewrite_params,
                           const List<LEX_CSTRING> *dynamic_privs);
 
  private:
   enum_sql_command operation;
   std::string db;
-  std::string query;
   std::vector<User> user_list;
   std::vector<std::string> dynamic_privilege_list;
+  Rewrite_params *rewrite_params;
 
  public:
   enum_sql_command get_operation() const { return operation; }
   const std::string &get_db() const { return db; }
-  const std::string &get_query_for_logging() const { return query; }
   const std::vector<User> &get_user_list() const { return user_list; }
   const std::vector<std::string> &get_dynamic_privilege_list() const {
     return dynamic_privilege_list;
   }
+  Rewrite_params *get_rewrite_params() const { return rewrite_params; }
 };
 
 #endif

sql/auth/auth_internal.h

@@ -209,6 +209,7 @@ bool log_and_commit_acl_ddl(THD *thd, bool transactional_tables,
                             bool log_to_binlog = true);
 void acl_notify_htons(THD *thd, enum_sql_command operation,
                       const List<LEX_USER> *users,
+                      std::set<LEX_USER *> *rewrite_users = nullptr,
                       const List<LEX_CSTRING> *dynamic_privs = nullptr);
 
 /* sql_authorization */

sql/auth/sql_authorization.cc

@@ -3592,7 +3592,7 @@ bool mysql_grant(THD *thd, const char *db, List<LEX_USER> &list, ulong rights,
     my_ok(thd);
     /* Notify storage engines */
     acl_notify_htons(thd, revoke_grant ? SQLCOM_REVOKE : SQLCOM_GRANT, &list,
-                     granted_dynamic_privs);
+                     nullptr, granted_dynamic_privs);
   }
 
   return error;

sql/auth/sql_user.cc

@@ -1578,11 +1578,11 @@ bool change_password(THD *thd, LEX_USER *lex_user, const char *new_password,
         authentication_plugin.c_str(), is_role, nullptr, nullptr);
   } /* Critical section */
 
-  /* Notify storage engines */
+  /* Notify storage engines (including rewrite list) */
   if (!(result || commit_result)) {
     List<LEX_USER> user_list;
     user_list.push_back(lex_user);
-    acl_notify_htons(thd, SQLCOM_SET_PASSWORD, &user_list);
+    acl_notify_htons(thd, SQLCOM_SET_PASSWORD, &user_list, &users);
   }
 
   return result || commit_result;
@@ -2819,8 +2819,8 @@ bool mysql_alter_user(THD *thd, List<LEX_USER> &list, bool if_exists) {
         reset_mqh(thd, extra_user, false);
       }
     }
-    /* Notify storage engines */
-    acl_notify_htons(thd, SQLCOM_ALTER_USER, &list);
+    /* Notify storage engines (including rewrite list) */
+    acl_notify_htons(thd, SQLCOM_ALTER_USER, &list, &extra_users);
   }
 
   if (result == 0) {

sql/auth/sql_user_table.cc

@@ -558,13 +558,10 @@ ulong get_access(TABLE *form, uint fieldnr, uint *next_field) {
 */
 Acl_change_notification::Acl_change_notification(
     THD *thd, enum_sql_command op, const List<LEX_USER> *users,
-    const List<LEX_CSTRING> *dynamic_privs)
-    : operation(op), db(thd->db().str, thd->db().length) {
-  if (thd->rewritten_query().length()) {
-    query.assign(thd->rewritten_query().ptr(), thd->rewritten_query().length());
-  } else {
-    query.assign(thd->query().str, thd->query().length);
-  }
+    Rewrite_params *rewrite, const List<LEX_CSTRING> *dynamic_privs)
+    : operation(op),
+      db(thd->db().str, thd->db().length),
+      rewrite_params(rewrite) {
   if (users) {
     /* Copy data out of List<LEX_USER> */
     user_list.reserve(users->size());
@@ -581,11 +578,12 @@ Acl_change_notification::Acl_change_notification(
   }
 }
 
-void acl_notify_htons(THD *thd MY_ATTRIBUTE((unused)),
-                      enum_sql_command operation MY_ATTRIBUTE((unused)),
-                      const List<LEX_USER> *users MY_ATTRIBUTE((unused)),
-                      const List<LEX_CSTRING> *dynamic_privs
-                          MY_ATTRIBUTE((unused))) {
+void acl_notify_htons(
+    THD *thd MY_ATTRIBUTE((unused)),
+    enum_sql_command operation MY_ATTRIBUTE((unused)),
+    const List<LEX_USER> *users MY_ATTRIBUTE((unused)),
+    std::set<LEX_USER *> *rewrite_users MY_ATTRIBUTE((unused)),
+    const List<LEX_CSTRING> *dynamic_privs MY_ATTRIBUTE((unused))) {
   DBUG_TRACE;
   DBUG_PRINT("enter", ("db: %s query: '%s'", thd->db().str, thd->query().str));
 #ifdef WITH_NDBCLUSTER_STORAGE_ENGINE
@@ -594,7 +592,9 @@ void acl_notify_htons(THD *thd MY_ATTRIBUTE((unused)),
     So, instantiate it and send a notification only if the Server is
     built with ndbcluster SE.
   */
-  Acl_change_notification notice(thd, operation, users, dynamic_privs);
+  User_params rewrite_user_params(rewrite_users);
+  User_params *rewrite = rewrite_users ? &rewrite_user_params : nullptr;
+  Acl_change_notification notice(thd, operation, users, rewrite, dynamic_privs);
   ha_acl_notify(thd, &notice);
 #endif
 }

storage/ndb/plugin/ha_ndbcluster_binlog.cc

@@ -40,6 +40,7 @@
 #include "sql/rpl_injector.h"
 #include "sql/rpl_slave.h"
 #include "sql/sql_lex.h"
+#include "sql/sql_rewrite.h"
 #include "sql/sql_table.h"  // build_table_filename
 #include "sql/sql_thd_internal_api.h"
 #include "sql/thd_raii.h"
@@ -596,8 +597,17 @@ static void ndbcluster_acl_notify(THD *thd,
     return;
   }
 
-  const std::string &query = notice->get_query_for_logging();
+  /* Obtain the query in a form suitable for writing to the error log.
+     The password is replaced with the string "<secret>".
+  */
+  std::string query;
+  if (thd->rewritten_query().length())
+    query.assign(thd->rewritten_query().ptr(), thd->rewritten_query().length());
+  else
+    query.assign(thd->query().str, thd->query().length);
+  DBUG_ASSERT(query.length());
   ndb_log_verbose(9, "ACL considering: %s", query.c_str());
+
   std::string user_list;
   bool dist_use_db = false;   // Prepend "use [db];" to statement
   bool dist_refresh = false;  // All participants must refresh their caches
@@ -632,6 +642,19 @@ static void ndbcluster_acl_notify(THD *thd,
 
   DBUG_ASSERT(strategy == Ndb_stored_grants::Strategy::STATEMENT);
   ndb_log_verbose(9, "ACL change distribution: STATEMENT");
+
+  /* If the notice contains rewrite_params, query is an ALTER USER or SET
+     PASSWORD statement and must be rewritten again, as if for the binlog,
+     replacing a plaintext password with a crytpographic hash.
+  */
+  if (notice->get_rewrite_params()) {
+    String rewritten_query;
+    mysql_rewrite_acl_query(thd, rewritten_query, Consumer_type::BINLOG,
+                            notice->get_rewrite_params(), false);
+    query.assign(rewritten_query.c_ptr_safe(), rewritten_query.length());
+    DBUG_ASSERT(query.length());
+  }
+
   if (!schema_dist_client.acl_notify(
           dist_use_db ? notice->get_db().c_str() : nullptr, query.c_str(),
           query.length(), dist_refresh))

storage/ndb/plugin/ndb_stored_grants.cc

@@ -617,16 +617,17 @@ int ThreadContext::drop_users(ChangeNotice *notice,
    from SHOW CREATE USER, so its exact format is known. For idempotence, it
    must be rewritten as several statements. The final result is:
       CREATE USER IF NOT EXISTS user@host;
-      ALTER USER user@host ... ;
       REVOKE ALL ON *.* FROM user@host;
-      GRANT ... TO user@host;
-      GRANT ... TO user@host;
-      ALTER USER user@host DEFAULT ROLE r;
+      ALTER USER user@host ...;   [CLEAR RESOURCE LIMITS]
+      ALTER USER user@host ...;   [SET VALUES FROM SHOW CREATE USER]
 */
 void ThreadContext::create_user(std::string &name, std::string &statement) {
   const std::string create_user("CREATE USER IF NOT EXISTS ");
   const std::string alter_user("ALTER USER ");
   const std::string revoke_all("REVOKE ALL ON *.* FROM ");
+  const std::string set_resource_defaults(
+      " WITH MAX_QUERIES_PER_HOUR 0 MAX_UPDATES_PER_HOUR 0 "
+      " MAX_CONNECTIONS_PER_HOUR 0 MAX_USER_CONNECTIONS 0");
 
   /* Run statement CREATE USER IF NOT EXISTS */
   if (!get_local_user(name)) {
@@ -635,6 +636,12 @@ void ThreadContext::create_user(std::string &name, std::string &statement) {
     run_acl_statement(create_user + name);
   }
 
+  /* Revoke any privileges the user may have had prior to this snapshot. */
+  run_acl_statement(revoke_all + name);
+
+  /* Clear resource limits (this is not included in SHOW CREATE USER) */
+  run_acl_statement(alter_user + name + set_resource_defaults);
+
   /* Rewrite CREATE to ALTER */
   statement = statement.replace(0, 6, "ALTER");
 
@@ -658,9 +665,6 @@ void ThreadContext::create_user(std::string &name, std::string &statement) {
 
   /* Run the rest of the statement. */
   run_acl_statement(statement.erase(default_role_pos, role_clause_len));
-
-  /* Revoke any privileges the user may have had prior to this snapshot. */
-  run_acl_statement(revoke_all + name);
 }
 
 /* Apply the snapshot in m_current_rows,
@@ -849,10 +853,6 @@ Ndb_stored_grants::Strategy ThreadContext::handle_change(ChangeNotice *notice) {
     /* ALTER USER, SET PASSWORD, or GRANT or REVOKE of misc. privileges */
     rebuild_local_cache = false;
     update_list = &m_intersection;
-    /* Distribute ALTER USER and SET PASSWORD as snapshot refreshes
-       in order to avoid transmitting plaintext passwords. */
-    if (operation == SQLCOM_ALTER_USER || operation == SQLCOM_SET_PASSWORD)
-      dist_as_snapshot = true;
   }
 
   /* drop_users() will DROP USER or REVOKE NDB_STORED_USER, as appropriate */