[Maria-developers] MDEV-6431 review
![](https://secure.gravatar.com/avatar/32b3f5fe52f3e988b4cd6460e7f68d91.jpg?s=120&d=mm&r=g)
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
![](https://secure.gravatar.com/avatar/39b623a1559cf9c69ac3d9d4fb44e7fe.jpg?s=120&d=mm&r=g)
Hi, Sergey! On Nov 27, Sergey Vojtovich wrote:
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.
In simple validation plugins requirements are, well, simple and one can look at system variables. In the cracklib case the warning reports the exact cracklib error, like in: Warning 1819 cracklib: it does not contain enough DIFFERENT characters or Warning 1819 cracklib: it is based on your username or Warning 1819 cracklib: it is based on a dictionary word I can add a similar warning for the simple plugin too, if needed (I didn't think it was needed, though).
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?
Typo, sorry. Fixed. Regards, Sergei
participants (2)
-
Sergei Golubchik
-
Sergey Vojtovich