maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Daniel!

On Oct 22, Daniel Black wrote:
> revision-id: 7807d9b6939 (mariadb-10.5.4-62-g7807d9b6939)
> parent(s): 054f10365c4
> author: Daniel Black <daniel@xxxxxxxxxxx>
> committer: Daniel Black <daniel@xxxxxxxxxxx>
> timestamp: 2020-07-16 17:06:36 +1000
> message:
> 
> MDEV-22974: mysql_native_password make "invalid" valid
> 
> Per b9f3f06857ac, mysql_system_tables_data.sql creates
> a mysql_native_password with a salted hash of "invalid" so that `set password`
> will detect a native password can be pplied:.

"applied"

> 
> 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.

One option would be to allow "invalid" literal in set_user_auth(),
before even any plugin checks. But I'm unsure of the implications.

>      my_error(ER_PASSWD_LENGTH, MYF(0), SCRAMBLED_PASSWORD_CHAR_LENGTH);
>      return 1;
>    }
> 
Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx