maria-developers team mailing list archive

[Sergei <serg@xxxxxxxxxxxx>]

Hi, Vicențiu!

On Jun 25, noreply@xxxxxxxxxxxxx wrote:
> ------------------------------------------------------------
> revno: 3766
> committer: Vicențiu Ciorbaru <cvicentiu@xxxxxxxxx>
> branch nick: trunk
> timestamp: Tue 2013-06-25 23:52:30 +0300
> message:
>   Added separation between roles and users in the mysql.user table
> modified:
>   sql/sql_acl.cc
> 
> === modified file 'sql/sql_acl.cc'
> --- sql/sql_acl.cc	2013-06-23 12:25:04 +0000
> +++ sql/sql_acl.cc	2013-06-25 20:52:30 +0000
> @@ -542,7 +542,16 @@
>  #endif /* HAVE_OPENSSL && !EMBEDDED_LIBRARY */
>  #define NORMAL_HANDSHAKE_SIZE   6
>  
> -static DYNAMIC_ARRAY acl_hosts, acl_users, acl_dbs, acl_proxy_users;
> +static DYNAMIC_ARRAY acl_hosts, acl_users, acl_dbs, acl_proxy_users, acl_roles;
> +/* XXX
> +   ***** Potential optimization *****
> +   role_grants could potentially be a HASH with keys as usernames and values
> +   as a DYNAMIC_ARRAY of pointers to granted roles
> +   XXX
> +   ***** Implementation choice for now *****
> +   An array representing mappings between acl_users and acl_roles;
> +*/

Agree about a hash. Hosts, users and dbs are stored in an array, because
they support wildcards in MariaDB, not only exact matches. And very
specific sort order, from most specific to least specific.

But we only need exact matches for role names, so you can put all roles
in a hash (see HASH or Hash_set).

> +static DYNAMIC_ARRAY role_grants;

Same. Do you need an array here? Is the order of entries important?
I don't think it is.

In fact, you probably may store role<->user relationships directly in
the ACL_USER structure. For a user - list of granted roles. For a role -
list of users it is granted to. Then it should be easy to check if a
given user can SET a given role.

>  static MEM_ROOT mem, memex;
>  static bool initialized=0;
>  static bool allow_all_hosts=1;
> @@ -876,11 +891,20 @@
>    while (!(read_record_info.read_record(&read_record_info)))
>    {
>      ACL_USER user;
> +    bool is_role= FALSE;
>      bzero(&user, sizeof(user));
>      update_hostname(&user.host, get_field(&mem, table->field[0]));
>      user.user= get_field(&mem, table->field[1]);
> -    DBUG_PRINT("info", ("user_entry %s", user.user));
> -    if (check_no_resolve && hostname_requires_resolving(user.host.hostname))
> +
> +    /* If the user entry is a role, skip password and hostname checks
> +       A user can not log in with a role so some checks are not necessary
> +     */
> +    if (!user.host.hostname) {
> +      is_role= TRUE;

I'm sorry, but it wouldn't do.
Apparently it is possible to create a user with the empty host name:

  MariaDB [test]> create user foo@'';
  Query OK, 0 rows affected (0.00 sec)

  MariaDB [test]> select user,host from mysql.user;
  +------+-----------+
  | user | host      |
  +------+-----------+
  | foo  |           |

Looking at compare_hostname() function, one can see that empty hostname
means "any hostname is ok", it's a wildcard.

It means, you cannot use empty hostnames to distinguish between roles
and users. Which is a pity, I'd also prefer to use an empty hostname for
that.

You can use a NULL hostname (you'd need ALTER TABLE to make the column
nullable) or a separate column, like, Is_role ENUM('N','Y').

> +    }
> +
> +    if (!is_role && check_no_resolve &&
> +        hostname_requires_resolving(user.host.hostname))

Regards,
Sergei