From 60f19cbc9aa425ab5606383f81829d54643f4fd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vicen=C8=9Biu=20Ciorbaru?= Date: Fri, 18 Oct 2013 09:25:42 -0700 Subject: [PATCH] Added GRANT ROLE TO ... and REVOKE ROLE FROM ... functionality. TODO: Privilege checks are not done upon executing the command. --- mysql-test/r/acl_roles_show_grants.result | 4 +- mysql-test/t/acl_roles_show_grants.test | 5 + scripts/mysql_system_tables.sql | 3 +- sql/sql_acl.cc | 201 +++++++++++++++++++--- sql/sql_acl.h | 2 +- sql/sql_parse.cc | 4 +- sql/sql_yacc.yy | 4 +- 7 files changed, 196 insertions(+), 27 deletions(-) diff --git a/mysql-test/r/acl_roles_show_grants.result b/mysql-test/r/acl_roles_show_grants.result index 63080ed1b38..f615fba2ea4 100644 --- a/mysql-test/r/acl_roles_show_grants.result +++ b/mysql-test/r/acl_roles_show_grants.result @@ -31,8 +31,8 @@ select * from information_schema.applicable_roles; GRANTEE ROLE_NAME IS_GRANTABLE select * from information_schema.applicable_roles; GRANTEE ROLE_NAME IS_GRANTABLE -test_user@localhost test_role1 YES test_role1 test_role2 YES +test_user@localhost test_role1 YES test_user@localhost test_role2 YES show grants; Grants for test_user@localhost @@ -45,8 +45,8 @@ test_user@localhost NULL set role test_role1; select * from information_schema.enabled_roles; ROLE_NAME -test_role2 test_role1 +test_role2 select current_user(), current_role(); current_user() current_role() test_user@localhost test_role1 diff --git a/mysql-test/t/acl_roles_show_grants.test b/mysql-test/t/acl_roles_show_grants.test index b520745128e..7fb36f8c12d 100644 --- a/mysql-test/t/acl_roles_show_grants.test +++ b/mysql-test/t/acl_roles_show_grants.test @@ -24,21 +24,25 @@ select user, host from mysql.db; grant select on mysql.* to test_role2; flush privileges; +--sorted_result select * from information_schema.applicable_roles; change_user 'test_user'; +--sorted_result select * from information_schema.applicable_roles; --sorted_result show grants; select current_user(), current_role(); set role test_role1; +--sorted_result select * from information_schema.enabled_roles; select current_user(), current_role(); --sorted_result show grants; set role none; +--sorted_result select * from information_schema.enabled_roles; select current_user(), current_role(); --sorted_result @@ -60,6 +64,7 @@ show grants for CURRENT_ROLE; show grants for CURRENT_ROLE(); set role test_role2; +--sorted_result select * from information_schema.enabled_roles; select current_user(), current_role(); --sorted_result diff --git a/scripts/mysql_system_tables.sql b/scripts/mysql_system_tables.sql index 30bea5d9b0f..7f85cb409f3 100644 --- a/scripts/mysql_system_tables.sql +++ b/scripts/mysql_system_tables.sql @@ -41,8 +41,7 @@ CREATE TABLE IF NOT EXISTS user ( Host char(60) binary DEFAULT '' NOT NULL, Us -- Remember for later if user table already existed set @had_user_table= @@warning_count != 0; -CREATE TABLE IF NOT EXISTS roles_mapping (HostFk char(60) binary DEFAULT '' NOT NULL, UserFk char(16) binary DEFAULT '' NOT NULL, RoleFk char(16) binary DEFAULT '' NOT NULL); - +CREATE TABLE IF NOT EXISTS roles_mapping (HostFk char(60) binary DEFAULT '' NOT NULL, UserFk char(16) binary DEFAULT '' NOT NULL, RoleFk char(16) binary DEFAULT '' NOT NULL, unique index (HostFk, UserFk, RoleFk)); CREATE TABLE IF NOT EXISTS func ( name char(64) binary DEFAULT '' NOT NULL, ret tinyint(1) DEFAULT '0' NOT NULL, dl char(128) DEFAULT '' NOT NULL, type enum ('function','aggregate') COLLATE utf8_general_ci NOT NULL, PRIMARY KEY (name) ) engine=MyISAM CHARACTER SET utf8 COLLATE utf8_bin comment='User defined functions'; diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 42f03ed8b84..fdc6516ec9a 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -2741,6 +2741,62 @@ int add_role_user_mapping(ROLE_GRANT_PAIR *mapping) return result; } +int remove_role_user_mapping(ROLE_GRANT_PAIR *mapping) +{ + ACL_USER_BASE *user= find_user_no_anon((mapping->u_hname) ? mapping->u_hname: "", + (mapping->u_uname) ? mapping->u_uname: "", + TRUE); + ACL_ROLE *role= find_acl_role(mapping->r_uname ? mapping->r_uname: ""); + + + int result= 0; + uint idx_user, idx_role; + bool deleted_role= FALSE, deleted_user= FALSE; + + if (user == NULL || role == NULL) + { + /* There still exists the possibility that the user is actually a role */ + if (user == NULL && role && (!mapping->u_hname || !mapping->u_hname[0]) + && /* in this case the grantee is a role */ + ((user= find_acl_role(mapping->u_uname ? mapping->u_uname: "")))) + { + result= 1; + } + else + { + DBUG_PRINT("warning", ("Invalid remove_role_user_mapping '%s'@'%s' %s %p %p", + mapping->u_uname, mapping->u_hname, + mapping->r_uname, user, role)); + + return -1; + } + } + + /* scan both arrays to find and delete both links */ + for (idx_user=0; idx_user < user->role_grants.elements; idx_user++) + { + if (role == *dynamic_element(&user->role_grants, idx_user, ACL_ROLE**)) + { + delete_dynamic_element(&user->role_grants, idx_user); + deleted_user= TRUE; + } + } + + for (idx_role=0; idx_role < role->parent_grantee.elements; idx_role++) + { + if (user == *dynamic_element(&role->parent_grantee, idx_role, + ACL_USER_BASE**)) + { + delete_dynamic_element(&role->parent_grantee, idx_role); + deleted_role= TRUE; + } + } + + /* we should always get to delete from both arrays */ + DBUG_ASSERT(deleted_role && deleted_user); + return result; +} + /* Rebuild the role grants every time the acl_users is modified @@ -3695,6 +3751,60 @@ abort: DBUG_RETURN(-1); } +static int +replace_roles_mapping_table(TABLE *table, ROLE_GRANT_PAIR *pair, + bool revoke_grant) +{ + DBUG_ENTER("replace_roles_mapping_table"); + + uchar row_key[MAX_KEY_LENGTH]; + int error; + table->use_all_columns(); + table->field[0]->store(pair->u_hname, strlen(pair->u_hname), + system_charset_info); + table->field[1]->store(pair->u_uname, strlen(pair->u_uname), + system_charset_info); + table->field[2]->store(pair->r_uname, strlen(pair->r_uname), + system_charset_info); + key_copy(row_key, table->record[0], table->key_info, + table->key_info->key_length); + + if (table->file->ha_index_read_idx_map(table->record[0], 0, row_key, + HA_WHOLE_KEY, HA_READ_KEY_EXACT)) + { + /* No match */ + if (revoke_grant) + { + DBUG_RETURN(1); + } + } + if (revoke_grant) + { + if ((error= table->file->ha_delete_row(table->record[0]))) + { + DBUG_PRINT("info", ("error deleting row '%s' '%s' '%s'", + pair->u_hname, pair->u_uname, pair->r_uname)); + goto table_error; + } + } + else + { + if ((error= table->file->ha_write_row(table->record[0]))) + { + DBUG_PRINT("info", ("error inserting row '%s' '%s' '%s'", + pair->u_hname, pair->u_uname, pair->r_uname)); + goto table_error; + } + } + + /* all ok */ + DBUG_RETURN(0); + +table_error: + DBUG_PRINT("info", ("table error")); + table->file->print_error(error, MYF(0)); + DBUG_RETURN(1); +} static void acl_update_proxy_user(ACL_PROXY_USER *new_value, bool is_revoke) @@ -5183,7 +5293,7 @@ static void append_user(String *str, const char *u, const char *h, } -bool mysql_grant_role(THD *thd, List &list) +bool mysql_grant_role(THD *thd, List &list, bool revoke) { DBUG_ENTER("mysql_grant_role"); /* @@ -5215,16 +5325,29 @@ bool mysql_grant_role(THD *thd, List &list) rolename= granted_role->user.str; } + TABLE_LIST tables; + tables.init_one_table(C_STRING_WITH_LEN("mysql"), + C_STRING_WITH_LEN("roles_mapping"), + "roles_mapping", TL_WRITE); + mysql_rwlock_wrlock(&LOCK_grant); mysql_mutex_lock(&acl_cache->lock); if (!(role= find_acl_role(rolename))) { - my_error(ER_INVALID_ROLE, MYF(0), rolename); mysql_mutex_unlock(&acl_cache->lock); mysql_rwlock_unlock(&LOCK_grant); + my_error(ER_INVALID_ROLE, MYF(0), rolename); DBUG_RETURN(TRUE); } + if (open_and_lock_tables(thd, &tables, FALSE, MYSQL_LOCK_IGNORE_TIMEOUT)) + { // Should never happen + mysql_mutex_unlock(&acl_cache->lock); + mysql_rwlock_unlock(&LOCK_grant); + my_error(ER_NO_SUCH_TABLE, MYF(0), "mysql", "roles_mapping"); + DBUG_RETURN(TRUE); /* purecov: deadcode */ + } + while ((user= user_list++)) { role_as_user= NULL; @@ -5244,6 +5367,7 @@ bool mysql_grant_role(THD *thd, List &list) result= 1; continue; } + /* can not grant current_role to current_role */ if (granted_role->user.str == current_role.str) { @@ -5264,31 +5388,69 @@ bool mysql_grant_role(THD *thd, List &list) ROLE_GRANT_PAIR *mapping= (ROLE_GRANT_PAIR *) alloc_root(&mem, sizeof(ROLE_GRANT_PAIR)); - /* TODO write into roles_mapping table */ init_role_grant_pair(&mem, mapping, username, hostname, rolename); - int res= add_role_user_mapping(mapping); - if (res == -1) + + if (!revoke) { - append_user(&wrong_users, username, hostname, role_as_user != NULL); + int res= add_role_user_mapping(mapping); + /* role or user does not exist*/ + if (res == -1) + { + append_user(&wrong_users, username, hostname, role_as_user != NULL); + result= 1; + continue; + } + + /* + Check if this grant would cause a cycle. It only needs to be run + if we're granting a role to a role + */ + if (role_as_user && + traverse_role_graph(role, NULL, NULL, NULL, role_explore_detect_cycle, + NULL) == 2) + { + append_user(&wrong_users, username, hostname, TRUE); + result= 1; + /* need to rollback the mapping added previously */ + remove_role_user_mapping(mapping); + continue; + } + } + else + { + /* revoke a role grant */ + int res= remove_role_user_mapping(mapping); + if (res == -1) + { + append_user(&wrong_users, username, hostname, role_as_user != NULL); + result= 1; + continue; + } + } + + /* write into the roles_mapping table */ + if (replace_roles_mapping_table(tables.table, mapping, revoke)) + { + append_user(&wrong_users, username, hostname, TRUE); result= 1; + if (!revoke) + { + /* need to rollback the mapping added previously */ + remove_role_user_mapping(mapping); + } + else + { + /* need to rollback the mapping deleted previously */ + add_role_user_mapping(mapping); + } continue; } /* - Check if this grant would cause a cycle. It only needs to be run - if we're granting a role to a role - */ - if (role_as_user && - traverse_role_graph(role, NULL, NULL, NULL, role_explore_detect_cycle, - NULL) == 2) - { - append_user(&wrong_users, username, hostname, TRUE); - result= 1; - continue; - } - - /* only need to propagate grants when granting a role to a role */ + Only need to propagate grants when granting/revoking a role to/from + a role + */ if (role_as_user) { acl_update_role_entry(role_as_user, role_as_user->initial_role_access); @@ -5302,7 +5464,6 @@ bool mysql_grant_role(THD *thd, List &list) rolename, wrong_users.c_ptr_safe()); - DBUG_RETURN(result); } diff --git a/sql/sql_acl.h b/sql/sql_acl.h index 506a1fe4d40..91add84a066 100644 --- a/sql/sql_acl.h +++ b/sql/sql_acl.h @@ -204,7 +204,7 @@ int check_change_password(THD *thd, const char *host, const char *user, bool change_password(THD *thd, const char *host, const char *user, char *password); -bool mysql_grant_role(THD *thd, List &user_list); +bool mysql_grant_role(THD *thd, List &user_list, bool revoke); bool mysql_grant(THD *thd, const char *db, List &user_list, ulong rights, bool revoke, bool is_proxy); int mysql_table_grant(THD *thd, TABLE_LIST *table, List &user_list, diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 24b4fd0fc27..7aecdff761a 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -3894,7 +3894,9 @@ end_with_restore_list: if (thd->security_ctx->user) // If not replication { - if (!(res= mysql_grant_role(thd, lex->users_list))) + if (!(res= mysql_grant_role(thd, lex->users_list, + lex->sql_command == SQLCOM_GRANT_ROLE ? 0 : 1 + ))) my_ok(thd); } else diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index cd176ded1c0..756a6bf5ca4 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -14251,7 +14251,9 @@ revoke_command: { LEX *lex= Lex; lex->sql_command= SQLCOM_REVOKE_ROLE; - lex->type= 0; + /* The first role is the one that is revoked */ + if (Lex->users_list.push_front($1)) + MYSQL_YYABORT; } ;