maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Oleksandr!

you forgot to check in the result file

On Sep 07, Oleksandr Byelkin wrote:
> revision-id: ca4ef7185da (mariadb-10.6.1-79-gca4ef7185da)
> parent(s): 271afbc88e4
> author: Oleksandr Byelkin
> committer: Oleksandr Byelkin
> timestamp: 2021-09-07 14:35:05 +0200
> message:
> 
> MDEV-9245 password "reuse prevention" validation plugin
> 
> diff --git a/plugin/reuse_password_check/CMakeLists.txt b/plugin/reuse_password_check/CMakeLists.txt
> --- /dev/null
> +++ b/plugin/reuse_password_check/CMakeLists.txt
> @@ -0,0 +1,4 @@
> +
> +ADD_DEFINITIONS(-DMYSQL_SERVER)

as I wrote to HF, this should not be needed.

> +
> +MYSQL_ADD_PLUGIN(reuse_password_check reuse_password_check.c MODULE_ONLY)

Unless you have a reason why this must be "MODULE_ONLY", please remove
MODULE_ONLY and try to compile and test with -DPLUGIN_REUSE_PASSWORD_CHECK=STATIC

also, I'd suggest "password_reuse_check" not "reuse_password_check".

> diff --git a/plugin/reuse_password_check/reuse_password_check.c b/plugin/reuse_password_check/reuse_password_check.c
> --- /dev/null
> +++ b/plugin/reuse_password_check/reuse_password_check.c
> @@ -0,0 +1,236 @@
> +/* Copyright (c) 2021, Oleksandr Byelkin and MariaDB
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; version 2 of the License.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program; if not, write to the Free Software
> +   Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1335  USA */
> +
> +#include <my_config.h>
> +#include <my_global.h>
> +#include <my_base.h>
> +#include <mysql/plugin_password_validation.h>
> +#include <mysql.h>

only plugin_password_validation.h should be needed here.

> +
> +
> +#include <mysqld_error.h>

this might be needed all right

> +#include <mysql/service_sha2.h>

but this isn't needed either

> +
> +#define HISTORY_DB_NAME "reuse_password_check_history"
> +
> +#define SQL_BUFF_LEN 2048
> +
> +// 0 - unlimited, otherwise number of days to check
> +static unsigned interval= 0;
> +
> +// helping string for bin_to_hex512
> +static char digits[]= "0123456789ABCDEF";
> +
> +
> +/**
> +  Convert string of 512 bits (64 bytes) to hex representation
> +
> +  @param to              pointer to the result puffer
> +                         (should be at least 64*2 bytes)
> +  @param str             pointer to 512 bits (64 bytes string)
> +*/
> +
> +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[((uchar) *str) >> 4];
> +    *to++= digits[((uchar) *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, "[%d] %s", ME_WARNING,
> +                  mysql_errno(mysql), mysql_error(mysql));

may be include plugin name here?

> +}
> +
> +
> +/**
> +  Create the history of passwords table for this plugin.
> +
> +  @param mysql           Connection handler
> +
> +  @retval 1 - Error
> +  @retval 0 - OK
> +*/
> +
> +static int create_table(MYSQL *mysql)
> +{
> +  if (mysql_real_query(mysql,
> +        // 512/8 = 64
> +        STRING_WITH_LEN("CREATE TABLE mysql." HISTORY_DB_NAME
> +                        " ( hash binary(64),"
> +                        " time timestamp,"

make it "default current_timestamp", just to be explicit.
even though it's implicitly default current_timestamp already.

> +                        " primary key (hash), index tm (time) )"
> +                        " ENGINE=Aria")))
> +  {
> +    report_sql_error(mysql);
> +    return 1;
> +  }
> +  return 0;
> +}
> +
> +
> +/**
> +  Run this query and create table if needed
> +
> +  @param mysql           Connection handler
> +  @param query           The query to run
> +  @param len             length of the query text
> +
> +  @retval 1 - Error
> +  @retval 0 - OK
> +*/
> +
> +static int run_query_with_table_creation(MYSQL *mysql, const char *query,
> +                                         size_t len)
> +{
> +  if (unlikely(mysql_real_query(mysql, query, len)))
> +  {
> +    unsigned int rc= mysql_errno(mysql);
> +    if (rc != ER_NO_SUCH_TABLE)
> +    {
> +      // suppress this error in case of try to add the same password twice
> +      if (rc != ER_DUP_ENTRY)
> +        report_sql_error(mysql);
> +      return 1;
> +    }
> +    if (create_table(mysql))
> +      return 1;
> +    if (unlikely(mysql_real_query(mysql, query, len)))
> +    {
> +      report_sql_error(mysql);
> +      return 1;
> +    }
> +  }
> +  return 0;
> +}
> +
> +
> +/**
> +  Password validator
> +
> +  @param username        User name (part of whole login name)
> +  @param password        Password to validate
> +  @param hostname        Host name (part of whole login name)
> +
> +  @retval 1 - Password is not OK or an error happened
> +  @retval 0 - Password is OK
> +*/
> +
> +static int validate(const MYSQL_CONST_LEX_STRING *username,
> +                    const MYSQL_CONST_LEX_STRING *password,
> +                    const MYSQL_CONST_LEX_STRING *hostname)
> +{
> +  MYSQL *mysql= NULL;
> +  size_t key_len= username->length + password->length + hostname->length;
> +  size_t buff_len= (key_len > SQL_BUFF_LEN ? key_len : SQL_BUFF_LEN);
> +  size_t len;
> +  char *buff= malloc(buff_len);
> +  unsigned char hash[512/8];
> +  char escaped_hash[512/8*2 + 1];
> +  if (!buff)
> +    return 1;
> +
> +  mysql= mysql_init(NULL);
> +  if (!mysql)
> +  {
> +    free(buff);
> +    return 1;
> +  }
> +
> +  memcpy(buff, hostname->str, hostname->length);
> +  memcpy(buff + hostname->length, username->str, username->length);
> +  memcpy(buff + hostname->length + username->length, password->str,
> +          password->length);
> +  buff[key_len]= 0;
> +  bzero(hash, sizeof(hash));
> +  my_sha512(hash, buff, key_len);
> +  if (mysql_real_connect_local(mysql, NULL, NULL, NULL, 0) == NULL)
> +    goto sql_error;
> +
> +  if (interval)
> +  {
> +    // trim the table
> +    len= snprintf(buff, buff_len,
> +                  "DELETE FROM mysql." HISTORY_DB_NAME
> +                  " WHERE time < DATE_SUB(NOW(), interval %d day)",
> +                  interval);
> +    if (unlikely(run_query_with_table_creation(mysql, buff, len)))
> +      goto sql_error;
> +  }
> +
> +  bin_to_hex512(escaped_hash, hash);
> +  escaped_hash[512/8*2]= '\0';
> +  len= snprintf(buff, buff_len,
> +                "INSERT INTO mysql." HISTORY_DB_NAME "(hash) "
> +                "values (x'%s')",
> +                escaped_hash);
> +  if (unlikely(run_query_with_table_creation(mysql, buff, len)))
> +    goto sql_error;
> +
> +  free(buff);
> +  mysql_close(mysql);
> +  return 0; // OK
> +
> +sql_error:
> +  free(buff);
> +  if (mysql)
> +    mysql_close(mysql);
> +  return 1; // Error
> +}
> +
> +static MYSQL_SYSVAR_UINT(interval, interval, PLUGIN_VAR_RQCMDARG,
> +  "How old (in days) passwords to check (0 means unlimited)", NULL, NULL,

better "Password history retention period in days (0 means unlimited)"

>
> +  0, 0, 365*100, 1);
> +
> +
> +static struct st_mysql_sys_var* sysvars[]= {
> +  MYSQL_SYSVAR(interval),
> +  NULL
> +};
> +
> +static struct st_mariadb_password_validation info=
> +{
> +  MariaDB_PASSWORD_VALIDATION_INTERFACE_VERSION,
> +  validate
> +};
> +
> +maria_declare_plugin(simple_password_check)

^^^ must be the same name as in CMakeLists.txt,
otherwise static linking won't work.

> +{
> +  MariaDB_PASSWORD_VALIDATION_PLUGIN,
> +  &info,
> +  "reuse_password_check",
> +  "Oleksandr Byelkin",
> +  "Reusage of password check",

Better "Password reuse check"
or, as it's a description, "Prevent password reuse"

> +  PLUGIN_LICENSE_GPL,
> +  NULL,
> +  NULL,
> +  0x0100,
> +  NULL,
> +  sysvars,
> +  "1.0",
> +  MariaDB_PLUGIN_MATURITY_ALPHA
> +}
> +maria_declare_plugin_end;
> 
> diff --git a/mysql-test/suite/plugins/t/reuse_password_check.test b/mysql-test/suite/plugins/t/reuse_password_check.test
> --- /dev/null
> +++ b/mysql-test/suite/plugins/t/reuse_password_check.test
> @@ -0,0 +1,58 @@
> +--source include/not_embedded.inc
> +
> +if (!$REUSE_PASSWORD_CHECK_SO) {
> +  skip No REUSE_PASSWORD_CHECK plugin;
> +}
> +
> +install soname "reuse_password_check";
> +
> +set @save_reuse_password_check_interval= @@reuse_password_check_interval;
> +
> +set global reuse_password_check_interval= 0;
> +
> +--echo # Default value (sould be unlimited i.e. 0)
> +SHOW GLOBAL VARIABLES like "reuse_password_check%";
> +
> +--echo # insert user
> +grant select on *.* to user_name@localhost identified by 'test_pwd';

what about sql mode that doesn't allow to create users with grant?
I don't see where you've disabled that.

> +
> +#--error ER_NOT_VALID_PASSWORD
> +#grant select on *.* to user_name@localhost identified by 'test_pwd';
> +#show warnings;

why is that commented out?

> +
> +--error ER_CANNOT_USER
> +alter user user_name@localhost identified by 'test_pwd';
> +show warnings;
> +
> +# Plugin does not work for it
> +#--error ER_NOT_VALID_PASSWORD
> +#SET PASSWORD FOR user_name@localhost = PASSWORD('test_pwd');
> +
> +--echo # check exparation
> +
> +set global reuse_password_check_interval= 10;
> +
> +--error ER_CANNOT_USER
> +alter user user_name@localhost identified by 'test_pwd';
> +show warnings;
> +select hex(hash) from mysql.reuse_password_check_history;
> +
> +--echo # emulate old password
> +update mysql.reuse_password_check_history set time= date_sub(now(), interval
> +11 day);
> +
> +alter user user_name@localhost identified by 'test_pwd';
> +show warnings;
> +
> +drop user user_name@localhost;
> +
> +show create table mysql.reuse_password_check_history;
> +select count(*) from mysql.reuse_password_check_history;
> +
> +drop table mysql.reuse_password_check_history;
> +
> +set @save_reuse_password_check_interval= @@reuse_password_check_interval;
> +
> +set global reuse_password_check_interval= @save_reuse_password_check_interval;
> +
> +uninstall plugin reuse_password_check;

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