From 2fced9e7b643d0c101d0ffde33f51e0a38f7017e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vicen=C8=9Biu=20Ciorbaru?= Date: Sat, 16 Dec 2017 11:56:16 +0200 Subject: [PATCH] MDEV-13655: Set role does not properly grant privileges. When granting a role to another role, DB privileges get propagated. If the grantee had no previous DB privileges, an extra ACL_DB entry is created to house those "indirectly received" privileges. If, afterwards, DB privileges are granted to the grantee directly, we must make sure to not create a duplicate ACL_DB entry. --- mysql-test/suite/roles/set_role-13655.result | 50 ++++++++++++++++++++ mysql-test/suite/roles/set_role-13655.test | 49 +++++++++++++++++++ sql/sql_acl.cc | 47 ++++++++++++------ sql/structs.h | 2 +- 4 files changed, 132 insertions(+), 16 deletions(-) create mode 100644 mysql-test/suite/roles/set_role-13655.result create mode 100644 mysql-test/suite/roles/set_role-13655.test diff --git a/mysql-test/suite/roles/set_role-13655.result b/mysql-test/suite/roles/set_role-13655.result new file mode 100644 index 00000000000..1c4841c8afd --- /dev/null +++ b/mysql-test/suite/roles/set_role-13655.result @@ -0,0 +1,50 @@ +# +# MDEV-13655: SET ROLE does not properly grant privileges. +# +# We must test that if aditional db privileges get granted to a role +# which previously inherited privileges from another granted role +# keep the internal memory structures intact. +# +create role simple; +# +# First we create an entry with privileges for databases for the simple role. +# +grant select, insert, update, delete, lock tables, execute on t.* to simple; +create role admin; +# +# Now we grant the simple role to admin. This means that db privileges +# should propagate to admin. +# +grant simple to admin; +show grants for admin; +Grants for admin +GRANT simple TO 'admin' +GRANT USAGE ON *.* TO 'admin' +GRANT USAGE ON *.* TO 'simple' +GRANT SELECT, INSERT, UPDATE, DELETE, LOCK TABLES, EXECUTE ON `t`.* TO 'simple' +# +# Finally, we give the admin all the available privileges for the db. +# +grant all on t.* to admin; +# +# Create a user to test out the new roles; +# +create user foo; +grant admin to foo; +create database t; +ERROR 42000: Access denied for user 'foo'@'%' to database 't' +set role admin; +show grants; +Grants for foo@% +GRANT admin TO 'foo'@'%' +GRANT USAGE ON *.* TO 'foo'@'%' +GRANT simple TO 'admin' +GRANT USAGE ON *.* TO 'admin' +GRANT ALL PRIVILEGES ON `t`.* TO 'admin' +GRANT USAGE ON *.* TO 'simple' +GRANT SELECT, INSERT, UPDATE, DELETE, LOCK TABLES, EXECUTE ON `t`.* TO 'simple' +create database t; +drop database t; +drop role simple; +drop role admin; +drop user foo; diff --git a/mysql-test/suite/roles/set_role-13655.test b/mysql-test/suite/roles/set_role-13655.test new file mode 100644 index 00000000000..97a82109276 --- /dev/null +++ b/mysql-test/suite/roles/set_role-13655.test @@ -0,0 +1,49 @@ +source include/not_embedded.inc; + +--echo # +--echo # MDEV-13655: SET ROLE does not properly grant privileges. +--echo # +--echo # We must test that if aditional db privileges get granted to a role +--echo # which previously inherited privileges from another granted role +--echo # keep the internal memory structures intact. +--echo # + +create role simple; + +--echo # +--echo # First we create an entry with privileges for databases for the simple role. +--echo # +grant select, insert, update, delete, lock tables, execute on t.* to simple; +create role admin; + +--echo # +--echo # Now we grant the simple role to admin. This means that db privileges +--echo # should propagate to admin. +--echo # +grant simple to admin; +show grants for admin; + +--echo # +--echo # Finally, we give the admin all the available privileges for the db. +--echo # +grant all on t.* to admin; + +--echo # +--echo # Create a user to test out the new roles; +--echo # +create user foo; +grant admin to foo; + +connect (foo,localhost,foo,,,,,); +--error ER_DBACCESS_DENIED_ERROR +create database t; +set role admin; +show grants; +create database t; +drop database t; + +connection default; + +drop role simple; +drop role admin; +drop user foo; diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 177b4517904..1b25871b664 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -2129,37 +2129,42 @@ static void acl_insert_user(const char *user, const char *host, } -static void acl_update_db(const char *user, const char *host, const char *db, +static bool acl_update_db(const char *user, const char *host, const char *db, ulong privileges) { mysql_mutex_assert_owner(&acl_cache->lock); + bool updated= false; + for (uint i=0 ; i < acl_dbs.elements ; i++) { ACL_DB *acl_db=dynamic_element(&acl_dbs,i,ACL_DB*); if ((!acl_db->user && !user[0]) || - (acl_db->user && - !strcmp(user,acl_db->user))) + (acl_db->user && + !strcmp(user,acl_db->user))) { if ((!acl_db->host.hostname && !host[0]) || - (acl_db->host.hostname && - !strcmp(host, acl_db->host.hostname))) + (acl_db->host.hostname && + !strcmp(host, acl_db->host.hostname))) { - if ((!acl_db->db && !db[0]) || - (acl_db->db && !strcmp(db,acl_db->db))) + if ((!acl_db->db && !db[0]) || + (acl_db->db && !strcmp(db,acl_db->db))) - { - if (privileges) + { + if (privileges) { acl_db->access= privileges; acl_db->initial_access= acl_db->access; } - else - delete_dynamic_element(&acl_dbs,i); - } + else + delete_dynamic_element(&acl_dbs,i); + updated= true; + } } } } + + return updated; } @@ -3428,9 +3433,21 @@ static int replace_db_table(TABLE *table, const char *db, acl_cache->clear(1); // Clear privilege cache if (old_row_exists) acl_update_db(combo.user.str,combo.host.str,db,rights); - else - if (rights) - acl_insert_db(combo.user.str,combo.host.str,db,rights); + else if (rights) + { + /* + If we did not have an already existing row, for users, we must always + insert an ACL_DB entry. For roles however, it is possible that one was + already created when DB privileges were propagated from other granted + roles onto the current role. For this case, first try to update the + existing entry, otherwise insert a new one. + */ + if (!combo.is_role() || + !acl_update_db(combo.user.str, combo.host.str, db, rights)) + { + acl_insert_db(combo.user.str,combo.host.str,db,rights); + } + } DBUG_RETURN(0); /* This could only happen if the grant tables got corrupted */ diff --git a/sql/structs.h b/sql/structs.h index ee61b8d3b3a..e141a8d7668 100644 --- a/sql/structs.h +++ b/sql/structs.h @@ -191,7 +191,7 @@ typedef int *(*update_var)(THD *, struct st_mysql_show_var *); typedef struct st_lex_user { LEX_STRING user, host, password, plugin, auth; - bool is_role() { return user.str[0] && !host.str[0]; } + bool is_role() const { return user.str[0] && !host.str[0]; } void set_lex_string(LEX_STRING *l, char *buf) { if (is_role())