Re: b17308e254a: MDEV-34854 Parsec sends garbage when using an empty password
Hi, Nikita, On Oct 07, Nikita Malyavin wrote:
revision-id: b17308e254a (mariadb-11.6.1-10-gb17308e254a) parent(s): e8021aaf28e author: Nikita Malyavin committer: Nikita Malyavin timestamp: 2024-10-04 01:33:47 +0200 message:
MDEV-34854 Parsec sends garbage when using an empty password
When an empty password is set, the server doesn't call st_mysql_auth::hash_password and leaves MYSQL_SERVER_AUTH_INFO::auth_string empty.
Fix: generate hashes for empty passwords as well. This breaks some auth plugins, so we increment interface version and do it only from Auth V. 2.03.
Some empty passwords could be already stored with no though. The user
"with no though" ?
will have to call SET PASSWORD once again, anyway the authentication wouldn't have worked for such password.
ok, I presume you mean ed25519 only, because mysql_native_password worked with an empty password and it generates an empty hash for it.
diff --git a/mysql-test/suite/plugins/r/parsec.result b/mysql-test/suite/plugins/r/parsec.result index 512c066e2d7..b7e3537af29 100644 --- a/mysql-test/suite/plugins/r/parsec.result +++ b/mysql-test/suite/plugins/r/parsec.result ... let's add ed25519 test too, for completeness.
diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 2722ea2ea19..ba05a5656c5 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -2402,7 +2402,10 @@ static int set_user_auth(THD *thd, const LEX_CSTRING &user, res= ER_NOT_VALID_PASSWORD; goto end; } - if (pwtext.length) + + // Starting from version 2.03 we also generate hash for empty passwords. + if ((info->interface_version >= MYSQL_AUTH_INTERFACE_VERSION_2_03
I don't understand this MYSQL_AUTH_INTERFACE_VERSION_2_03 thing. First, again, that's not how a version is supposed to work. Second, this empty-password change isn't a change in the API. You can simply start calling hash_password() for empty passwords and it'll just work. I've tried :)
+ && pwtext.str) || pwtext.length) { if (info->hash_password) {
Regards, Sergei Chief Architect, MariaDB Server and security@mariadb.org
On Mon, 7 Oct 2024 at 21:08, Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Nikita,
On Oct 07, Nikita Malyavin wrote:
revision-id: b17308e254a (mariadb-11.6.1-10-gb17308e254a) parent(s): e8021aaf28e author: Nikita Malyavin committer: Nikita Malyavin timestamp: 2024-10-04 01:33:47 +0200 message:
MDEV-34854 Parsec sends garbage when using an empty password
When an empty password is set, the server doesn't call st_mysql_auth::hash_password and leaves MYSQL_SERVER_AUTH_INFO::auth_string empty.
Fix: generate hashes for empty passwords as well. This breaks some auth plugins, so we increment interface version and do it only from Auth V. 2.03.
Some empty passwords could be already stored with no though. The user
"with no though" ?
with none. All empty passwords. All empty passwords have no ext-salt stored.
will have to call SET PASSWORD once again, anyway the authentication wouldn't have worked for such password.
ok, I presume you mean ed25519 only, because mysql_native_password worked with an empty password and it generates an empty hash for it.
No, this is only about those users, who could set an empty password for PARSEC plugin within the last two months.
diff --git a/mysql-test/suite/plugins/r/parsec.result b/mysql-test/suite/plugins/r/parsec.result index 512c066e2d7..b7e3537af29 100644 --- a/mysql-test/suite/plugins/r/parsec.result +++ b/mysql-test/suite/plugins/r/parsec.result ... let's add ed25519 test too, for completeness.
I guess it will not work with empty password, just as before? I'll check anyway.
diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 2722ea2ea19..ba05a5656c5 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -2402,7 +2402,10 @@ static int set_user_auth(THD *thd, const LEX_CSTRING &user, res= ER_NOT_VALID_PASSWORD; goto end; } - if (pwtext.length) + + // Starting from version 2.03 we also generate hash for empty passwords. + if ((info->interface_version >= MYSQL_AUTH_INTERFACE_VERSION_2_03
I don't understand this MYSQL_AUTH_INTERFACE_VERSION_2_03 thing. First, again, that's not how a version is supposed to work. Second, this empty-password change isn't a change in the API.
You can simply start calling hash_password() for empty passwords and it'll just work. I've tried :)
I've tried, and I had main.set_password failing, exactly with mysql_old_password . This is why I decided to simply alter the version and leave mysql_old_password as it is, without figuring out the problem.
+ && pwtext.str) || pwtext.length) { if (info->hash_password) {
Regards, Sergei Chief Architect, MariaDB Server and security@mariadb.org
-- Yours truly, Nikita Malyavin
Hi, Nikita, On Oct 07, Nikita Malyavin wrote:
On Mon, 7 Oct 2024 at 21:08, Sergei Golubchik <serg@mariadb.org> wrote:
--- a/mysql-test/suite/plugins/r/parsec.result +++ b/mysql-test/suite/plugins/r/parsec.result ... let's add ed25519 test too, for completeness.
I guess it will not work with empty password, just as before? I'll check anyway.
It will not, of course, its hash_password callback starts from if (*dlen < PASSWORD_LEN || pwlen == 0) return 1; I mean, perhaps it'd make sense to remove this "|| pwlen == 0" part and allow it to run with empty passwords?
diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 2722ea2ea19..ba05a5656c5 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -2402,7 +2402,10 @@ static int set_user_auth(THD *thd, const LEX_CSTRING &user, res= ER_NOT_VALID_PASSWORD; goto end; } - if (pwtext.length) + + // Starting from version 2.03 we also generate hash for empty passwords. + if ((info->interface_version >= MYSQL_AUTH_INTERFACE_VERSION_2_03
I don't understand this MYSQL_AUTH_INTERFACE_VERSION_2_03 thing. First, again, that's not how a version is supposed to work. Second, this empty-password change isn't a change in the API.
You can simply start calling hash_password() for empty passwords and it'll just work. I've tried :)
I've tried, and I had main.set_password failing, exactly with mysql_old_password . This is why I decided to simply alter the version and leave mysql_old_password as it is, without figuring out the problem.
I see. It was failing if you simply remove pwtext.length check, because you called hash_password() when password hash was already provided. This is how to enable hash_password() for empty passwords: --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -2341,7 +2341,7 @@ static int set_user_auth(THD *thd, const LEX_CSTRING &user, res= ER_NOT_VALID_PASSWORD; goto end; } - if (pwtext.length) + if (!auth->auth_string.length) { if (info->hash_password) { @@ -2356,7 +2356,7 @@ static int set_user_auth(THD *thd, const LEX_CSTRING &user, auth->auth_string.str= (char*)memdup_root(&acl_memroot, buf, len+1); auth->auth_string.length= len; } - else + else if (pwtext.length) { res= ER_SET_PASSWORD_AUTH_PLUGIN; goto end; Regards, Sergei Chief Architect, MariaDB Server and security@mariadb.org
Hello, Sergei! On Mon, 7 Oct 2024 at 22:52, Sergei Golubchik <serg@mariadb.org> wrote:
This is how to enable hash_password() for empty passwords:
--- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -2341,7 +2341,7 @@ static int set_user_auth(THD *thd, const LEX_CSTRING &user, res= ER_NOT_VALID_PASSWORD; goto end; } - if (pwtext.length) + if (!auth->auth_string.length) { if (info->hash_password) { @@ -2356,7 +2356,7 @@ static int set_user_auth(THD *thd, const LEX_CSTRING &user, auth->auth_string.str= (char*)memdup_root(&acl_memroot, buf, len+1); auth->auth_string.length= len; } - else + else if (pwtext.length) { res= ER_SET_PASSWORD_AUTH_PLUGIN; goto end;
Indeed. Fixing it this way resolves all the problems. Please see commits c3e28ca <https://github.com/MariaDB/server/commit/c3e28ca1178a8006b8dac955a27fba8bcea57c7a> and 06e4583 <https://github.com/MariaDB/server/commit/06e4583a2e0dc990b37af134573698ce4f8e5132> with the new updates. -- Yours truly, Nikita Malyavin
participants (2)
-
Nikita Malyavin -
Sergei Golubchik