From d3a3adb8337ae530cbcb75719f55889d1352b98f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vicen=C8=9Biu=20Ciorbaru?= Date: Tue, 5 May 2015 21:19:53 +0300 Subject: [PATCH] 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. --- .../r/show_grants_with_plugin-7985.result | 197 ++++++++++++++++++ .../t/show_grants_with_plugin-7985.test | 161 ++++++++++++++ sql/sql_acl.cc | 108 +++++++--- 3 files changed, 436 insertions(+), 30 deletions(-) create mode 100644 mysql-test/r/show_grants_with_plugin-7985.result create mode 100644 mysql-test/t/show_grants_with_plugin-7985.test diff --git a/mysql-test/r/show_grants_with_plugin-7985.result b/mysql-test/r/show_grants_with_plugin-7985.result new file mode 100644 index 00000000000..5accda75383 --- /dev/null +++ b/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; diff --git a/mysql-test/t/show_grants_with_plugin-7985.test b/mysql-test/t/show_grants_with_plugin-7985.test new file mode 100644 index 00000000000..339c75c7c49 --- /dev/null +++ b/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; diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index f46a923bddf..9cc9efae6f8 100644 --- a/sql/sql_acl.cc +++ b/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= - 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) + if (fix_user_plugin_ptr(&user.plugin)) // Non native plugin. { - sql_print_warning("'user' entry '%s@%s' has both a password " - "and an authentication plugin specified. The " - "password will be ignored.", - safe_str(user.user.str), - safe_str(user.host.hostname)); + user.auth_string= auth; + if (password_len) + { + sql_print_warning("'user' entry '%s@%s' has both a password " + "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(""); 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(""); 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)