On Thu, Oct 22, 2020 at 8:25 PM Sergei Golubchik <[email protected]> wrote: > > Hi, Daniel! > > On Oct 22, Daniel Black wrote: > > SHOW CREATE USER; dilignently uses this value in its output > > generating the SQL: > > > > MariaDB [(none)]> show create user; > > > > > > +---------------------------------------------------------------------------------------------------+ > > | CREATE USER for dan@localhost > > | > > > > +---------------------------------------------------------------------------------------------------+ > > | CREATE USER `dan`@`localhost` IDENTIFIED VIA mysql_native_password > > USING 'invalid' OR unix_socket | > > > > +---------------------------------------------------------------------------------------------------+ > > > > Attempting to execute this before this patch resutls in: > > > > MariaDB [(none)]> CREATE USER `dan2`@`localhost` IDENTIFIED VIA > > mysql_native_password USING 'invalid' OR unix_socket; > > ERROR 1372 (HY000): Password hash should be a 41-digit hexadecimal number > > > > As such deep the the implementation of mysql_native_password we make > > "invalid" valid (pun intended) such that the above create user will > > succeed. > > > > native_password_get_salt is only called in the context of > > set_user_salt, so all setting of native passwords to hashed content of > > 'invalid', quite literally create an invalid password. > > > > So other forms of "invalid" are valid SQL in creating invalid passwords: > > > > MariaDB [(none)]> set password = 'invalid'; > > Query OK, 0 rows affected (0.001 sec) > > > > MariaDB [(none)]> alter user dan@localhost IDENTIFIED BY PASSWORD > > 'invalid'; > > Query OK, 0 rows affected (0.000 sec) > > > > diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc > > index d94016b7815..3cd7a67ae1a 100644 > > --- a/sql/sql_acl.cc > > +++ b/sql/sql_acl.cc > > @@ -14539,6 +14539,12 @@ static int native_password_get_salt(const char > > *hash, size_t hash_length, > > > > if (hash_length != SCRAMBLED_PASSWORD_CHAR_LENGTH) > > { > > + if (hash_length == 7 && strcmp(hash, "invalid") == 0) > > + { > > + memcpy(out, "invalid", 7); > > + *out_length= 7; > > + return 0; > > + } > > okay. After you said ASAN, I think I can see why this could be > problematic. > > You can, of course, pad it to SCRAMBLED_PASSWORD_CHAR_LENGTH, but then > you'll create a *valid* scramble that would correspond to some actual > password.
yikes. > One option would be to allow "invalid" literal in set_user_auth(), > before even any plugin checks. But I'm unsure of the implications. > Updated: bb-10.4-danielblack-MDEV-22974-mysql_native_password-make-invalid-valid with 2 commits: first addresses this review: https://github.com/MariaDB/server/commit/9a478b11de26fb43f8a7df4253f80d549cd41ab1 In native_password_get_salt we set a scramble of an invalid length. We check the length in native_password_authenticate before even looking at the contents of the scrambled password. This keeps the implementation confined to the native_auth implementation. second - d5ddbdcf61218a0a45228d2f22fe4dd77a7fe7c8 accepts the mysql invalid password forms (and becomes slightly more strict on our invalid forms (those not beginning with '*')) _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
