MDEV-7985: MySQL Users Break when Migrating to MariaDB, part 2

Gave priority to password field when using a native authentication
plugin.

Also, prevented a user from setting an invalid auth_string, when using
native authentication.

mysql-test/r/show_grants_with_plugin-7985.result

@@ -0,0 +1,197 @@
+call mtr.add_suppression("password and an authentication plugin");
+#
+# Create a user with mysql_native_password plugin.
+# The user has no password or auth_string set.
+#
+create user u1;
+GRANT SELECT ON mysql.* to u1 IDENTIFIED VIA mysql_native_password;
+select user, host, password, plugin, authentication_string from mysql.user where user = 'u1';
+user	host	password	plugin	authentication_string
+u1	%		mysql_native_password	
+#
+# The user's grants should show no password at all.
+#
+show grants for u1;
+Grants for u1@%
+GRANT USAGE ON *.* TO 'u1'@'%'
+GRANT SELECT ON `mysql`.* TO 'u1'@'%'
+#
+# Test to see if connecting with no password is succesful.
+#
+connect  con1, localhost, u1,,;
+show grants;
+Grants for u1@%
+GRANT USAGE ON *.* TO 'u1'@'%'
+GRANT SELECT ON `mysql`.* TO 'u1'@'%'
+disconnect con1;
+connection default;
+#
+# Test after flushing privileges.
+#
+flush privileges;
+connect  con1, localhost, u1,,;
+show grants;
+Grants for u1@%
+GRANT USAGE ON *.* TO 'u1'@'%'
+GRANT SELECT ON `mysql`.* TO 'u1'@'%'
+disconnect con1;
+connection default;
+#
+# Now add a mysql_native password string in authentication_string.
+#
+GRANT SELECT ON mysql.* to u1 IDENTIFIED VIA mysql_native_password
+USING '*7AFEFD08B6B720E781FB000CAA418F54FA662626';
+select user, host, password, plugin, authentication_string from mysql.user where user = 'u1';
+user	host	password	plugin	authentication_string
+u1	%		mysql_native_password	*7AFEFD08B6B720E781FB000CAA418F54FA662626
+#
+# Test to see if connecting with password is succesful.
+#
+connect  con1, localhost, u1,'SOMETHING',;
+show grants;
+Grants for u1@%
+GRANT USAGE ON *.* TO 'u1'@'%' IDENTIFIED BY PASSWORD '*7AFEFD08B6B720E781FB000CAA418F54FA662626'
+GRANT SELECT ON `mysql`.* TO 'u1'@'%'
+disconnect con1;
+connection default;
+#
+# Test after flushing privileges.
+#
+flush privileges;
+connect  con1, localhost, u1,'SOMETHING',;
+show grants;
+Grants for u1@%
+GRANT USAGE ON *.* TO 'u1'@'%' IDENTIFIED BY PASSWORD '*7AFEFD08B6B720E781FB000CAA418F54FA662626'
+GRANT SELECT ON `mysql`.* TO 'u1'@'%'
+disconnect con1;
+connection default;
+#
+# Now we also set a password for the user.
+#
+set password for u1 = PASSWORD('SOMETHINGELSE');
+select user, host, password, plugin, authentication_string from mysql.user where user = 'u1';
+user	host	password	plugin	authentication_string
+u1	%	*054B7BBD2B9A553DA560520DCD3F76DA2D81B7C6	mysql_native_password	*7AFEFD08B6B720E781FB000CAA418F54FA662626
+#
+# Here we should use the password field, as that primes over
+# the authentication_string field.
+#
+show grants for u1;
+Grants for u1@%
+GRANT USAGE ON *.* TO 'u1'@'%' IDENTIFIED BY PASSWORD '*054B7BBD2B9A553DA560520DCD3F76DA2D81B7C6'
+GRANT SELECT ON `mysql`.* TO 'u1'@'%'
+#
+# Logging in with the user's password should work.
+#
+connect  con1, localhost, u1,'SOMETHINGELSE',;
+show grants;
+Grants for u1@%
+GRANT USAGE ON *.* TO 'u1'@'%' IDENTIFIED BY PASSWORD '*054B7BBD2B9A553DA560520DCD3F76DA2D81B7C6'
+GRANT SELECT ON `mysql`.* TO 'u1'@'%'
+disconnect con1;
+connection default;
+#
+# Reload privileges and test logging in again.
+#
+flush privileges;
+show grants for u1;
+Grants for u1@%
+GRANT USAGE ON *.* TO 'u1'@'%' IDENTIFIED BY PASSWORD '*054B7BBD2B9A553DA560520DCD3F76DA2D81B7C6'
+GRANT SELECT ON `mysql`.* TO 'u1'@'%'
+#
+# Here we connect via the user's password again.
+#
+connect  con1, localhost, u1,'SOMETHINGELSE',;
+show grants;
+Grants for u1@%
+GRANT USAGE ON *.* TO 'u1'@'%' IDENTIFIED BY PASSWORD '*054B7BBD2B9A553DA560520DCD3F76DA2D81B7C6'
+GRANT SELECT ON `mysql`.* TO 'u1'@'%'
+disconnect con1;
+connection default;
+#
+# Now we remove the authentication plugin password, flush privileges and
+# try again.
+#
+update mysql.user set authentication_string = '' where user='u1';
+select user, host, password, plugin, authentication_string from mysql.user where user = 'u1';
+user	host	password	plugin	authentication_string
+u1	%	*054B7BBD2B9A553DA560520DCD3F76DA2D81B7C6	mysql_native_password	
+flush privileges;
+show grants for u1;
+Grants for u1@%
+GRANT USAGE ON *.* TO 'u1'@'%' IDENTIFIED BY PASSWORD '*054B7BBD2B9A553DA560520DCD3F76DA2D81B7C6'
+GRANT SELECT ON `mysql`.* TO 'u1'@'%'
+#
+# Here we connect via the user's password.
+#
+connect  con1, localhost, u1,'SOMETHINGELSE',;
+select user, host, password, plugin, authentication_string from mysql.user where user = 'u1';
+user	host	password	plugin	authentication_string
+u1	%	*054B7BBD2B9A553DA560520DCD3F76DA2D81B7C6	mysql_native_password	
+disconnect con1;
+connection default;
+#
+# Try and set a wrong auth_string password, with mysql_native_password.
+# Make sure it fails.
+#
+GRANT USAGE ON *.* TO u1 IDENTIFIED VIA mysql_native_password USING 'asd';
+ERROR HY000: Password hash should be a 41-digit hexadecimal number
+#
+# Now set a correct password.
+#
+GRANT SELECT ON mysql.* to u1 IDENTIFIED VIA mysql_native_password
+USING '*7AFEFD08B6B720E781FB000CAA418F54FA662626';
+show grants for u1;
+Grants for u1@%
+GRANT USAGE ON *.* TO 'u1'@'%' IDENTIFIED BY PASSWORD '*7AFEFD08B6B720E781FB000CAA418F54FA662626'
+GRANT SELECT ON `mysql`.* TO 'u1'@'%'
+#
+# Test if the user can now use that password instead.
+#
+connect  con1, localhost, u1,'SOMETHING',;
+show grants;
+Grants for u1@%
+GRANT USAGE ON *.* TO 'u1'@'%' IDENTIFIED BY PASSWORD '*7AFEFD08B6B720E781FB000CAA418F54FA662626'
+GRANT SELECT ON `mysql`.* TO 'u1'@'%'
+disconnect con1;
+#
+# Test if the user can now use that password instead, after flushing privileges;
+#
+connection default;
+flush privileges;
+connect  con1, localhost, u1,'SOMETHING',;
+show grants;
+Grants for u1@%
+GRANT USAGE ON *.* TO 'u1'@'%' IDENTIFIED BY PASSWORD '*7AFEFD08B6B720E781FB000CAA418F54FA662626'
+GRANT SELECT ON `mysql`.* TO 'u1'@'%'
+disconnect con1;
+connection default;
+#
+# Clear all passwords from the user.
+#
+GRANT SELECT ON mysql.* to u1 IDENTIFIED VIA mysql_native_password;
+select user, host, password, plugin, authentication_string from mysql.user where user = 'u1';
+user	host	password	plugin	authentication_string
+u1	%		mysql_native_password	
+#
+# Test no password connect.
+#
+connect  con1, localhost, u1,,;
+show grants;
+Grants for u1@%
+GRANT USAGE ON *.* TO 'u1'@'%'
+GRANT SELECT ON `mysql`.* TO 'u1'@'%'
+disconnect con1;
+connection default;
+#
+# Test no password connect, after flushing privileges.
+#
+flush privileges;
+connect  con1, localhost, u1,,;
+show grants;
+Grants for u1@%
+GRANT USAGE ON *.* TO 'u1'@'%'
+GRANT SELECT ON `mysql`.* TO 'u1'@'%'
+disconnect con1;
+connection default;
+drop user u1;

mysql-test/t/show_grants_with_plugin-7985.test

@@ -0,0 +1,161 @@
+--source include/not_embedded.inc
+--enable_connect_log
+call mtr.add_suppression("password and an authentication plugin");
+
+--echo #
+--echo # Create a user with mysql_native_password plugin.
+--echo # The user has no password or auth_string set.
+--echo #
+
+create user u1;
+GRANT SELECT ON mysql.* to u1 IDENTIFIED VIA mysql_native_password;
+select user, host, password, plugin, authentication_string from mysql.user where user = 'u1';
+
+--echo #
+--echo # The user's grants should show no password at all.
+--echo #
+show grants for u1;
+--echo #
+--echo # Test to see if connecting with no password is succesful.
+--echo #
+--connect (con1, localhost, u1,,)
+show grants;
+--disconnect con1
+
+--connection default
+--echo #
+--echo # Test after flushing privileges.
+--echo #
+flush privileges;
+--connect (con1, localhost, u1,,)
+show grants;
+--disconnect con1
+
+--connection default
+--echo #
+--echo # Now add a mysql_native password string in authentication_string.
+--echo #
+# Password string is SOMETHING
+GRANT SELECT ON mysql.* to u1 IDENTIFIED VIA mysql_native_password
+USING '*7AFEFD08B6B720E781FB000CAA418F54FA662626';
+select user, host, password, plugin, authentication_string from mysql.user where user = 'u1';
+--echo #
+--echo # Test to see if connecting with password is succesful.
+--echo #
+--connect (con1, localhost, u1,'SOMETHING',)
+show grants;
+--disconnect con1
+
+--connection default
+--echo #
+--echo # Test after flushing privileges.
+--echo #
+flush privileges;
+--connect (con1, localhost, u1,'SOMETHING',)
+show grants;
+--disconnect con1
+--connection default
+
+--echo #
+--echo # Now we also set a password for the user.
+--echo #
+set password for u1 = PASSWORD('SOMETHINGELSE');
+select user, host, password, plugin, authentication_string from mysql.user where user = 'u1';
+
+--echo #
+--echo # Here we should use the password field, as that primes over
+--echo # the authentication_string field.
+--echo #
+show grants for u1;
+
+--echo #
+--echo # Logging in with the user's password should work.
+--echo #
+--connect (con1, localhost, u1,'SOMETHINGELSE',)
+show grants;
+--disconnect con1
+--connection default
+--echo #
+--echo # Reload privileges and test logging in again.
+--echo #
+flush privileges;
+show grants for u1;
+--echo #
+--echo # Here we connect via the user's password again.
+--echo #
+--connect (con1, localhost, u1,'SOMETHINGELSE',)
+show grants;
+--disconnect con1
+--connection default
+
+--echo #
+--echo # Now we remove the authentication plugin password, flush privileges and
+--echo # try again.
+--echo #
+update mysql.user set authentication_string = '' where user='u1';
+select user, host, password, plugin, authentication_string from mysql.user where user = 'u1';
+flush privileges;
+show grants for u1;
+--echo #
+--echo # Here we connect via the user's password.
+--echo #
+--connect (con1, localhost, u1,'SOMETHINGELSE',)
+select user, host, password, plugin, authentication_string from mysql.user where user = 'u1';
+--disconnect con1
+--connection default
+
+--echo #
+--echo # Try and set a wrong auth_string password, with mysql_native_password.
+--echo # Make sure it fails.
+--echo #
+--error ER_PASSWD_LENGTH
+GRANT USAGE ON *.* TO u1 IDENTIFIED VIA mysql_native_password USING 'asd';
+--echo #
+--echo # Now set a correct password.
+--echo #
+GRANT SELECT ON mysql.* to u1 IDENTIFIED VIA mysql_native_password
+USING '*7AFEFD08B6B720E781FB000CAA418F54FA662626';
+show grants for u1;
+
+--echo #
+--echo # Test if the user can now use that password instead.
+--echo #
+--connect (con1, localhost, u1,'SOMETHING',)
+show grants;
+--disconnect con1
+
+--echo #
+--echo # Test if the user can now use that password instead, after flushing privileges;
+--echo #
+--connection default
+flush privileges;
+
+--connect (con1, localhost, u1,'SOMETHING',)
+show grants;
+--disconnect con1
+--connection default
+
+--echo #
+--echo # Clear all passwords from the user.
+--echo #
+GRANT SELECT ON mysql.* to u1 IDENTIFIED VIA mysql_native_password;
+select user, host, password, plugin, authentication_string from mysql.user where user = 'u1';
+
+--echo #
+--echo # Test no password connect.
+--echo #
+--connect (con1, localhost, u1,,)
+show grants;
+--disconnect con1
+--connection default
+
+--echo #
+--echo # Test no password connect, after flushing privileges.
+--echo #
+flush privileges;
+--connect (con1, localhost, u1,,)
+show grants;
+--disconnect con1
+--connection default
+
+drop user u1;

sql/sql_acl.cc

@@ -863,32 +863,30 @@ static char *fix_plugin_ptr(char *name)
 }
 
 /**
-  Fix ACL::plugin pointer to point to a hard-coded string, if appropriate
+  Fix a LEX_STRING *plugin pointer to point to a hard-coded string,
+  if appropriate
 
   Make sure that if ACL_USER's plugin is a built-in, then it points
   to a hard coded string, not to an allocated copy. Run-time, for
   authentication, we want to be able to detect built-ins by comparing
   pointers, not strings.
 
-  Additionally - update the salt if the plugin is built-in.
-
   @retval 0 the pointers were fixed
   @retval 1 this ACL_USER uses a not built-in plugin
 */
-static bool fix_user_plugin_ptr(ACL_USER *user)
+static bool fix_user_plugin_ptr(LEX_STRING *plugin_ptr)
 {
-  if (my_strcasecmp(system_charset_info, user->plugin.str,
+  DBUG_ASSERT(plugin_ptr);
+  if (my_strcasecmp(system_charset_info, plugin_ptr->str,
                     native_password_plugin_name.str) == 0)
-    user->plugin= native_password_plugin_name;
+    *plugin_ptr= native_password_plugin_name;
   else
-  if (my_strcasecmp(system_charset_info, user->plugin.str,
+  if (my_strcasecmp(system_charset_info, plugin_ptr->str,
                     old_password_plugin_name.str) == 0)
-    user->plugin= old_password_plugin_name;
+    *plugin_ptr= old_password_plugin_name;
   else
     return true;
 
-  if (user->auth_string.length)
-    set_user_salt(user, user->auth_string.str, user->auth_string.length);
   return false;
 }
 
@@ -966,6 +964,23 @@ my_bool acl_init(bool dont_read_acl_tables)
   DBUG_RETURN(return_val);
 }
 
+/**
+  Check if the password length provided matches supported native formats.
+*/
+static bool password_length_valid(int password_length)
+{
+  switch (password_length)
+  {
+  case 0: /* no password */
+  case SCRAMBLED_PASSWORD_CHAR_LENGTH:
+    return TRUE;
+  case SCRAMBLED_PASSWORD_CHAR_LENGTH_323:
+    return TRUE;
+  default:
+    return FALSE;
+  }
+}
+
 /**
   Choose from either native or old password plugins when assigning a password
 */
@@ -1257,22 +1272,43 @@ static my_bool acl_load(THD *thd, TABLE_LIST *tables)
           char *tmpstr= get_field(&acl_memroot, table->field[next_field++]);
           if (tmpstr)
           {
+            LEX_STRING auth;
+            auth.str = safe_str(get_field(&acl_memroot, table->field[next_field++]));
+            auth.length = strlen(auth.str);
             user.plugin.str= tmpstr;
             user.plugin.length= strlen(user.plugin.str);
-            user.auth_string.str=
+            if (fix_user_plugin_ptr(&user.plugin)) // Non native plugin.
-              safe_str(get_field(&acl_memroot, table->field[next_field++]));
-            user.auth_string.length= strlen(user.auth_string.str);
-
-            if (user.auth_string.length && password_len)
             {
-              sql_print_warning("'user' entry '%s@%s' has both a password "
+              user.auth_string= auth;
-                                "and an authentication plugin specified. The "
+              if (password_len)
-                                "password will be ignored.",
+              {
-                                safe_str(user.user.str),
+                sql_print_warning("'user' entry '%s@%s' has both a password "
-                                safe_str(user.host.hostname));
+                    "and an authentication plugin specified. The "
+                    "password will be ignored.",
+                    safe_str(user.user.str),
+                    safe_str(user.host.hostname));
+              }
+            }
+            else // Native plugin.
+            {
+              /*
+                Password field, if not empty, has precedence over
+                authentication_string field, only for native plugins.
+                See MDEV-6253 and MDEV-7985 for reasoning.
+              */
+              if (!password_len)
+              {
+                user.auth_string = auth;
+                if (!password_length_valid(auth.length))
+                {
+                  sql_print_warning("Found invalid password for user: '%s@%s';"
+                                    " Ignoring user", safe_str(user.user.str),
+                                    safe_str(user.host.hostname));
+                  continue;
+                }
+                set_user_salt(&user, auth.str, auth.length);
+              }
             }
-
-            fix_user_plugin_ptr(&user);
           }
         }
       }
@@ -1971,8 +2007,13 @@ static void acl_update_user(const char *user, const char *host,
         acl_user->auth_string.str= auth->str ?
           strmake_root(&acl_memroot, auth->str, auth->length) : const_cast<char*>("");
         acl_user->auth_string.length= auth->length;
-        if (fix_user_plugin_ptr(acl_user))
+        if (acl_user->plugin.str != native_password_plugin_name.str &&
+            acl_user->plugin.str != old_password_plugin_name.str)
           acl_user->plugin.str= strmake_root(&acl_memroot, plugin->str, plugin->length);
+        else
+          set_user_salt(acl_user, acl_user->auth_string.str,
+                                  acl_user->auth_string.length);
+
       }
       else
         if (password[0])
@@ -2047,8 +2088,12 @@ static void acl_insert_user(const char *user, const char *host,
     acl_user.auth_string.str= auth->str ?
       strmake_root(&acl_memroot, auth->str, auth->length) : const_cast<char*>("");
     acl_user.auth_string.length= auth->length;
-    if (fix_user_plugin_ptr(&acl_user))
+    if (acl_user.plugin.str != native_password_plugin_name.str &&
+        acl_user.plugin.str != old_password_plugin_name.str)
       acl_user.plugin.str= strmake_root(&acl_memroot, plugin->str, plugin->length);
+    else
+      set_user_salt(&acl_user, acl_user.auth_string.str,
+                               acl_user.auth_string.length);
   }
   else
   {
@@ -3014,17 +3059,20 @@ static int replace_user_table(THD *thd, TABLE *table, LEX_USER &combo,
 
   mysql_mutex_assert_owner(&acl_cache->lock);
 
+  size_t length_to_check = 0;
+  combo.password = combo.password.str ? combo.password : empty_lex_str;
   if (combo.password.str && combo.password.str[0])
+    length_to_check = combo.password.length;
+  else if (!fix_user_plugin_ptr(&combo.plugin))
+  {
+    length_to_check = combo.auth.length;
+  }
+
+  if (!password_length_valid(length_to_check))
   {
-    if (combo.password.length != SCRAMBLED_PASSWORD_CHAR_LENGTH &&
-        combo.password.length != SCRAMBLED_PASSWORD_CHAR_LENGTH_323)
-    {
       my_error(ER_PASSWD_LENGTH, MYF(0), SCRAMBLED_PASSWORD_CHAR_LENGTH);
       DBUG_RETURN(-1);
-    }
   }
-  else
-    combo.password= empty_lex_str;
 
   /* if the user table is not up to date, we can't handle role updates */
   if (table->s->fields <= ROLE_ASSIGN_COLUMN_IDX && handle_as_role)