maria-developers team mailing list archive

[Sergey Vojtovich <svoj@xxxxxxxxxxx>]

Hi Sergei,

overall it looks good. I want to double check the clean-up stuff, since I didn't
understand everything at the first glance. Some comments inline.

> diff --git a/include/mysql/plugin_password_validation.h b/include/mysql/plugin_password_validation.h
> new file mode 100644
> index 0000000..0bbb8f4
> --- /dev/null
> +++ b/include/mysql/plugin_password_validation.h
> @@ -0,0 +1,45 @@
> +/**
> +  Password validation plugin descriptor
> +*/
> +struct st_mysql_password_validation
> +{
> +  int interface_version;                        /**< version plugin uses */
> +  /**
> +    Function provided by the plugin which should perform password validation
> +    and return 0 if the password has passed the validation.
> +  */
> +  int (*validate_password)(MYSQL_LEX_STRING *username,
> +                           MYSQL_LEX_STRING *password);
> +};
> +#endif
> +
This differs from MySQL API, so we can't reuse their plugins.

Also they have this cool get_password_strength method, but that's something we
can expose via UDF.

OTOH we can compare password against username, and that's quite common password
misuse.

And what I always hated in this validation stuff is that you'll never come up
with a password that makes it happy. Of course unless you know requirements.
In our case requirements are exposed via system variables. This is more or less
acceptable. But I believe it would be nice if this stuff would be able to
generate strong passwords. Again, that's something that can be exposed as UDF.


> diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc
> index 529a795..cdf8c7b 100644
> --- a/sql/sql_plugin.cc
> +++ b/sql/sql_plugin.cc
> @@ -1051,7 +1055,8 @@ static bool plugin_add(MEM_ROOT *tmp_root,
>        continue; // invalid plugin type
>
>      if (plugin->type == MYSQL_UDF_PLUGIN ||
> -        (plugin->type == 8 && tmp.plugin_dl->mariaversion == 0))
> +        (plugin->type == MariaDB_PASSWORD_VALIDATION_INTERFACE_VERSION &&
> +         tmp.plugin_dl->mariaversion == 0))
>        continue; // unsupported plugin type
>
>      if (name->str && my_strnncoll(system_charset_info,
You compare type against version here. Why?

Regards,
Sergey