maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Oleksandr!

Looks very good. See comments/questions below.
The main thing - please add some tests for SQL errors in the plugin.

On Sep 11, Oleksandr Byelkin wrote:
> revision-id: 0e09bc41cba (mariadb-10.6.1-78-g0e09bc41cba)
> parent(s): 67cd5da8c34
> author: Oleksandr Byelkin
> committer: Oleksandr Byelkin
> timestamp: 2021-09-10 13:47:07 +0200
> message:
> 
> MDEV-9245 password "reuse prevention" validation plugin
> 
> diff --git a/mysql-test/suite/plugins/t/password_reuse_check.test b/mysql-test/suite/plugins/t/password_reuse_check.test
> --- /dev/null
> +++ b/mysql-test/suite/plugins/t/password_reuse_check.test
> @@ -0,0 +1,58 @@
> +--source include/not_embedded.inc
> +
> +if (!$PASSWORD_REUSE_CHECK_SO) {
> +  skip No PASSWORD_REUSE_CHECK plugin;
> +}
> +
> +install soname "password_reuse_check";
> +
> +set @save_password_reuse_check_interval= @@password_reuse_check_interval;

this is kind of pointless, if you uninstall the plugin anyway.
it's not harmless, just useless.

> +
> +set global password_reuse_check_interval= 0;
...
> diff --git a/plugin/password_reuse_check/CMakeLists.txt b/plugin/password_reuse_check/CMakeLists.txt
> --- /dev/null
> +++ b/plugin/password_reuse_check/CMakeLists.txt
> @@ -0,0 +1,2 @@
> +
> +MYSQL_ADD_PLUGIN(password_reuse_check password_reuse_check.c)

did you try to compile it statically?

> diff --git a/plugin/password_reuse_check/password_reuse_check.c b/plugin/password_reuse_check/password_reuse_check.c
> --- /dev/null
> +++ b/plugin/password_reuse_check/password_reuse_check.c
...
> +static void bin_to_hex512(char *to, const unsigned char *str)
> +{
> +  const unsigned char *str_end= str + (512/8);
> +  for (; str != str_end; ++str)
> +  {
> +    *to++= digits[((unsigned char) *str) >> 4];
> +    *to++= digits[((unsigned char) *str) & 0x0F];
> +  }
> +}
> +
> +
> +/**
> +  Send SQL error as ER_UNKNOWN_ERROR for information
> +
> +  @param mysql           Connection handler
> +*/
> +
> +static void report_sql_error(MYSQL *mysql)
> +{
> +  my_printf_error(ER_UNKNOWN_ERROR, "password_reuse_check:[%d] %s", ME_WARNING,
> +                  mysql_errno(mysql), mysql_error(mysql));
> +}

No tests for that?

You can, for example, drop the table, and create a non-insertable view
with the same name.

  CREATE VIEW mysql.password_reuse_check_history AS SELECT 1;

That should make the plugin to fail, right?

> +
> +
> +/**
> +  Create the history of passwords table for this plugin.
> +
> +  @param mysql           Connection handler
> +
> +  @retval 1 - Error
> +  @retval 0 - OK
> +*/

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx