← Back to team overview

maria-developers team mailing list archive

Re: 9ec0b36ee8b: MDEV-28838 password_reuse_check plugin mixes username and password

 

Hi, Oleksandr,

On Jul 04, Oleksandr Byelkin wrote:
> revision-id: 9ec0b36ee8b (mariadb-10.7.4-27-g9ec0b36ee8b)
> parent(s): ac0af4ec4ab
> author: Oleksandr Byelkin
> committer: Oleksandr Byelkin
> timestamp: 2022-06-29 17:25:44 +0200
> message:
> 
> MDEV-28838 password_reuse_check plugin mixes username and password
> 
> To prevent the problem of mixing user name and password and
> host name and user name we add length of the hostname and user name
> to the hash.
> 
> diff --git a/plugin/password_reuse_check/password_reuse_check.c b/plugin/password_reuse_check/password_reuse_check.c
> index ff0364ce007..4d07cda878a 100644
> --- a/plugin/password_reuse_check/password_reuse_check.c
> +++ b/plugin/password_reuse_check/password_reuse_check.c
> @@ -165,9 +166,11 @@ static int validate(const MYSQL_CONST_LEX_STRING *username,
>      return 1;
>    }
>  
> -  memcpy(buff, hostname->str, hostname->length);
> -  memcpy(buff + hostname->length, username->str, username->length);
> -  memcpy(buff + hostname->length + username->length, password->str,
> +  int2store(buff, hostname->length);
> +  int2store(buff + 2, username->length);
> +  memcpy(buff + 4, hostname->str, hostname->length);
> +  memcpy(buff + 4 + hostname->length, username->str, username->length);
> +  memcpy(buff + 4 + hostname->length + username->length, password->str,
>            password->length);

why? let's use the standard length,string format, why risk inventing
something unusual here for no benefits at all? I'd do like:

  static char *store_str(char *to, MYSQL_CONST_LEX_STRING from)
  {
    int2store(to, from.length);
    memcpy(to+2, from.str, from.length);
    return to+2+from.length;
  }

  and

  buff= store_str(buff, hostname);
  buff= store_str(buff, username);
  buff= store_str(buff, password);

>    buff[key_len]= 0;
>    memset(hash, 0, sizeof(hash));
> @@ -235,7 +238,7 @@ maria_declare_plugin(password_reuse_check)
>    0x0100,

^^^ this should be 0x0101 (or 0x0200 as I suggest below)

>    NULL,
>    sysvars,
> -  "1.0",
> +  "1.1",

make it 2.0 please, this is an incompatible change

>    MariaDB_PLUGIN_MATURITY_GAMMA
>  }
>  maria_declare_plugin_end;
> 
Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx