Skip to content
Permalink
Browse files

MDEV-14735 better matching order for grants

fixes
MDEV-14732 mysql.db privileges evaluated on order of grants rather than hierarchically
MDEV-8269 Correct fix for Bug #20181776 :- ACCESS CONTROL DOESN'T MATCH MOST SPECIFIC HOST WHEN IT CONTAINS WILDCARD

reimplement the old ad hoc get_sort() function to use a wildcard
pattern ordering logic, that works correctly in may be all practical cases.

get_sort() is renamed to catch merge errors at compilation time.
moved to a separate included file, because of a huge comment.
  • Loading branch information...
vuvova committed Jun 10, 2019
1 parent 6bc333d commit bce2a0e69a06b7bf985a1bbcbded4e27dc017890
Showing with 272 additions and 55 deletions.
  1. +25 −0 mysql-test/main/grant5.result
  2. +38 −0 mysql-test/main/grant5.test
  3. +18 −55 sql/sql_acl.cc
  4. +191 −0 sql/sql_acl_getsort.ic
@@ -102,3 +102,28 @@ u6 Y mysql_old_password 78a302dd267f6044
u7 Y mysql_old_password 78a302dd267f6044
u8 Y nonexistent 78a302dd267f6044
drop user u1@h, u2@h, u3@h, u4@h, u5@h, u6@h, u7@h, u8@h;
create database mysqltest_1;
create user twg@'%' identified by 'test';
create table mysqltest_1.t1(id int);
grant create, drop on `mysqltest_1%`.* to twg@'%';
grant all privileges on `mysqltest_1`.* to twg@'%';
connect conn1,localhost,twg,test,mysqltest_1;
insert into t1 values(1);
disconnect conn1;
connection default;
revoke all privileges, grant option from twg@'%';
grant create, drop on `mysqlt%`.* to twg@'%';
grant all privileges on `mysqlt%1`.* to twg@'%';
connect conn1,localhost,twg,test,mysqltest_1;
insert into t1 values(1);
disconnect conn1;
connection default;
revoke all privileges, grant option from twg@'%';
grant create, drop on `mysqlt%`.* to twg@'%';
grant all privileges on `%mysqltest_1`.* to twg@'%';
connect conn1,localhost,twg,test,mysqltest_1;
insert into t1 values(1);
disconnect conn1;
connection default;
drop database mysqltest_1;
drop user twg@'%';
@@ -86,3 +86,41 @@ select user,select_priv,plugin,authentication_string from mysql.user where user

# but they still can be dropped
drop user u1@h, u2@h, u3@h, u4@h, u5@h, u6@h, u7@h, u8@h;

#
# MDEV-14735 better matching order for grants
# MDEV-14732 mysql.db privileges evaluated on order of grants rather than hierarchically
# MDEV-8269 Correct fix for Bug #20181776 :- ACCESS CONTROL DOESN'T MATCH MOST SPECIFIC HOST WHEN IT CONTAINS WILDCARD
#
create database mysqltest_1;
create user twg@'%' identified by 'test';
create table mysqltest_1.t1(id int);

# MDEV-14732 test case
grant create, drop on `mysqltest_1%`.* to twg@'%';
grant all privileges on `mysqltest_1`.* to twg@'%';
connect conn1,localhost,twg,test,mysqltest_1;
insert into t1 values(1);
disconnect conn1;
connection default;

# prefix%suffix
revoke all privileges, grant option from twg@'%';
grant create, drop on `mysqlt%`.* to twg@'%';
grant all privileges on `mysqlt%1`.* to twg@'%';
connect conn1,localhost,twg,test,mysqltest_1;
insert into t1 values(1);
disconnect conn1;
connection default;

# more specific can even have a shorter prefix
revoke all privileges, grant option from twg@'%';
grant create, drop on `mysqlt%`.* to twg@'%';
grant all privileges on `%mysqltest_1`.* to twg@'%';
connect conn1,localhost,twg,test,mysqltest_1;
insert into t1 values(1);
disconnect conn1;
connection default;

drop database mysqltest_1;
drop user twg@'%';
@@ -62,6 +62,12 @@
bool mysql_user_table_is_in_short_password_format= false;
bool using_global_priv_table= true;

// set that from field length in acl_load?
const uint max_hostname_length= 60;
const uint max_dbname_length= 64;

#include "sql_acl_getsort.ic"

static LEX_CSTRING native_password_plugin_name= {
STRING_WITH_LEN("mysql_native_password")
};
@@ -117,7 +123,7 @@ static bool compare_hostname(const acl_host_and_ip *, const char *, const char *

class ACL_ACCESS {
public:
ulong sort;
ulonglong sort;
ulong access;
ACL_ACCESS()
:sort(0), access(0)
@@ -284,7 +290,6 @@ ulong role_global_merges= 0, role_db_merges= 0, role_table_merges= 0,

#ifndef NO_EMBEDDED_ACCESS_CHECKS
static void update_hostname(acl_host_and_ip *host, const char *hostname);
static ulong get_sort(uint count,...);
static bool show_proxy_grants (THD *, const char *, const char *,
char *, size_t);
static bool show_role_grants(THD *, const char *, const char *,
@@ -332,7 +337,8 @@ class ACL_PROXY_USER :public ACL_ACCESS
(proxied_host_arg && *proxied_host_arg) ?
proxied_host_arg : NULL);
with_grant= with_grant_arg;
sort= get_sort(4, host.hostname, user, proxied_host.hostname, proxied_user);
sort= get_magic_sort("huhu", host.hostname, user, proxied_host.hostname,
proxied_user);
}

void init(MEM_ROOT *mem, const char *host_arg, const char *user_arg,
@@ -2344,7 +2350,7 @@ static bool acl_load(THD *thd, const Grant_tables& tables)
}
host.access= host_table.get_access();
host.access= fix_rights_for_db(host.access);
host.sort= get_sort(2, host.host.hostname, host.db);
host.sort= get_magic_sort("hd", host.host.hostname, host.db);
if (check_no_resolve && hostname_requires_resolving(host.host.hostname))
{
sql_print_warning("'host' entry '%s|%s' "
@@ -2386,7 +2392,7 @@ static bool acl_load(THD *thd, const Grant_tables& tables)

user.access= user_table.get_access();

user.sort= get_sort(2, user.host.hostname, user.user.str);
user.sort= get_magic_sort("hu", user.host.hostname, user.user.str);
user.hostname_length= safe_strlen(user.host.hostname);

my_init_dynamic_array(&user.role_grants, sizeof(ACL_ROLE *), 0, 8, MYF(0));
@@ -2505,7 +2511,7 @@ static bool acl_load(THD *thd, const Grant_tables& tables)
db.db, db.user, safe_str(db.host.hostname));
}
}
db.sort=get_sort(3,db.host.hostname,db.db,db.user);
db.sort=get_magic_sort("hdu", db.host.hostname, db.db, db.user);
#ifndef TO_BE_REMOVED
if (db_table.num_fields() <= 9)
{ // Without grant
@@ -2753,49 +2759,6 @@ static ulong get_access(TABLE *form, uint fieldnr, uint *next_field)
}


/*
Return a number which, if sorted 'desc', puts strings in this order:
no wildcards
wildcards
empty string
*/

static ulong get_sort(uint count,...)
{
va_list args;
va_start(args,count);
ulong sort=0;

/* Should not use this function with more than 4 arguments for compare. */
DBUG_ASSERT(count <= 4);

while (count--)
{
char *start, *str= va_arg(args,char*);
uint chars= 0;
uint wild_pos= 0; /* first wildcard position */

if ((start= str))
{
for (; *str ; str++)
{
if (*str == wild_prefix && str[1])
str++;
else if (*str == wild_many || *str == wild_one)
{
wild_pos= (uint) (str - start) + 1;
break;
}
chars= 128; // Marker that chars existed
}
}
sort= (sort << 8) + (wild_pos ? MY_MIN(wild_pos, 127U) : chars);
}
va_end(args);
return sort;
}


static int acl_compare(const ACL_ACCESS *a, const ACL_ACCESS *b)
{
if (a->sort > b->sort)
@@ -3157,7 +3120,7 @@ ACL_USER::ACL_USER(THD *thd, const LEX_USER &combo,
user= safe_lexcstrdup_root(&acl_memroot, combo.user);
update_hostname(&host, safe_strdup_root(&acl_memroot, combo.host.str));
hostname_length= combo.host.length;
sort= get_sort(2, host.hostname, user.str);
sort= get_magic_sort("hu", host.hostname, user.str);
password_last_changed= thd->query_start();
password_lifetime= -1;
my_init_dynamic_array(&role_grants, sizeof(ACL_USER *), 0, 8, MYF(0));
@@ -3314,7 +3277,7 @@ static void acl_insert_db(const char *user, const char *host, const char *db,
update_hostname(&acl_db.host, safe_strdup_root(&acl_memroot, host));
acl_db.db=strdup_root(&acl_memroot,db);
acl_db.initial_access= acl_db.access= privileges;
acl_db.sort=get_sort(3,acl_db.host.hostname,acl_db.db,acl_db.user);
acl_db.sort=get_magic_sort("hdu", acl_db.host.hostname, acl_db.db, acl_db.user);
acl_dbs.push(acl_db);
rebuild_acl_dbs();
}
@@ -5033,7 +4996,7 @@ class GRANT_NAME :public Sql_alloc
char *db, *user, *tname, *hash_key;
ulong privs;
ulong init_privs; /* privileges found in physical table */
ulong sort;
ulonglong sort;
size_t key_length;
GRANT_NAME(const char *h, const char *d,const char *u,
const char *t, ulong p, bool is_routine);
@@ -5079,7 +5042,7 @@ void GRANT_NAME::set_user_details(const char *h, const char *d,
my_casedn_str(files_charset_info, db);
}
user = strdup_root(&grant_memroot,u);
sort= get_sort(3,host.hostname,db,user);
sort= get_magic_sort("hdu", host.hostname, db, user);
if (tname != t)
{
tname= strdup_root(&grant_memroot, t);
@@ -5121,7 +5084,7 @@ GRANT_NAME::GRANT_NAME(TABLE *form, bool is_routine)
update_hostname(&host, hostname);

db= get_field(&grant_memroot,form->field[1]);
sort= get_sort(3, host.hostname, db, user);
sort= get_magic_sort("hdu", host.hostname, db, user);
tname= get_field(&grant_memroot,form->field[3]);
if (!db || !tname)
{
@@ -6214,7 +6177,7 @@ static int update_role_db(int merged, int first, ulong access,
acl_db.db= acl_dbs.at(first).db;
acl_db.access= access;
acl_db.initial_access= 0;
acl_db.sort=get_sort(3, "", acl_db.db, role);
acl_db.sort= get_magic_sort("hdu", "", acl_db.db, role);
acl_dbs.push(acl_db);
return 2;
}

0 comments on commit bce2a0e

Please sign in to comment.
You can’t perform that action at this time.