From 592e3d60754161665bedba44be8d4eeea83bef89 Mon Sep 17 00:00:00 2001 From: Anju Bharti <66729219+anju15bharti@users.noreply.github.com> Date: Mon, 26 Aug 2024 18:10:36 +0530 Subject: [PATCH] Restrict DROP USER/ROLE from non-dbo user (#2859) (#2864) (#2882) Earlier, a user was able to drop user/role that belonged to another database. With this commit, a user can only drop the role/user that belongs to the same database with sufficient privileges. Issues Resolved Task: BABEL-5173 Signed-off-by: Shalini Lohia lshalini@amazon.com --- contrib/babelfishpg_tsql/src/pl_handler.c | 36 +- test/JDBC/expected/BABEL-3844-vu-cleanup.out | 2 +- .../JDBC/expected/restrict_drop_user_role.out | 413 ++++++++++++++++++ test/JDBC/input/restrict_drop_user_role.mix | 279 ++++++++++++ 4 files changed, 728 insertions(+), 2 deletions(-) create mode 100644 test/JDBC/expected/restrict_drop_user_role.out create mode 100644 test/JDBC/input/restrict_drop_user_role.mix diff --git a/contrib/babelfishpg_tsql/src/pl_handler.c b/contrib/babelfishpg_tsql/src/pl_handler.c index cbbd4d62bd..3d199a7f5c 100644 --- a/contrib/babelfishpg_tsql/src/pl_handler.c +++ b/contrib/babelfishpg_tsql/src/pl_handler.c @@ -3187,15 +3187,49 @@ bbf_ProcessUtility(PlannedStmt *pstmt, { RoleSpec *rolspec = lfirst(item); char *user_name; + const char *db_principal_type = drop_user ? "user" : "role"; + const char *db_owner_name; + int role_oid; + int rolename_len; + bool is_tsql_db_principal = false; + bool is_psql_db_principal = false; + Oid dbowner; user_name = get_physical_user_name(db_name, rolspec->rolename); + + db_owner_name = get_db_owner_name(db_name); + dbowner = get_role_oid(db_owner_name, false); + role_oid = get_role_oid(user_name, true); + rolename_len = strlen(rolspec->rolename); + is_tsql_db_principal = OidIsValid(role_oid) && + ((drop_user && is_user(role_oid)) || + (drop_role && is_role(role_oid))); + is_psql_db_principal = OidIsValid(role_oid) && !is_tsql_db_principal; + + /* If user is dbo or role is db_owner, restrict dropping */ + if ((drop_user && rolename_len == 3 && strncmp(rolspec->rolename, "dbo", 3) == 0) || + (drop_role && rolename_len == 8 && strncmp(rolspec->rolename, "db_owner", 8) == 0)) + ereport(ERROR, + (errcode(ERRCODE_CHECK_VIOLATION), + errmsg("Cannot drop the %s '%s'.", db_principal_type, rolspec->rolename))); + /* + * Check for current_user's privileges + * must be database owner to drop user/role + */ + if ((!stmt->missing_ok && !is_tsql_db_principal) || + !is_member_of_role(GetUserId(), dbowner) || + (is_tsql_db_principal && !is_member_of_role(dbowner, role_oid)) || is_psql_db_principal) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("Cannot drop the %s '%s', because it does not exist or you do not have permission.", db_principal_type, rolspec->rolename))); + /* * If a role has members, do not drop it. * Note that here we don't handle invalid * roles. */ - if (drop_role && !is_empty_role(get_role_oid(user_name, true))) + if (drop_role && !is_empty_role(role_oid)) ereport(ERROR, (errcode(ERRCODE_CHECK_VIOLATION), errmsg("The role has members. It must be empty before it can be dropped."))); diff --git a/test/JDBC/expected/BABEL-3844-vu-cleanup.out b/test/JDBC/expected/BABEL-3844-vu-cleanup.out index 2f51052245..9dd45e6462 100644 --- a/test/JDBC/expected/BABEL-3844-vu-cleanup.out +++ b/test/JDBC/expected/BABEL-3844-vu-cleanup.out @@ -6,7 +6,7 @@ drop user test_user; GO ~~ERROR (Code: 33557097)~~ -~~ERROR (Message: role "master_test_user" does not exist)~~ +~~ERROR (Message: Cannot drop the user 'test_user', because it does not exist or you do not have permission.)~~ drop login [babel\aduser1]; diff --git a/test/JDBC/expected/restrict_drop_user_role.out b/test/JDBC/expected/restrict_drop_user_role.out new file mode 100644 index 0000000000..f3d8f79a97 --- /dev/null +++ b/test/JDBC/expected/restrict_drop_user_role.out @@ -0,0 +1,413 @@ +-- tsql +create login no_priv_login1 with password = '12345678' +go + +create role dont_drop_role +go + +create login no_priv_login2 with password = '12345678' +go + +create user no_priv_user2 for login no_priv_login2 +go + +create login no_priv_login3 with password = '12345678' +go + +create user no_priv_user3 for login no_priv_login3 +go + +create role no_priv_role3; +go + +create database restrict_user_db1 +go + +use restrict_user_db1 +go + +create role dont_drop_role +go + +create user no_priv_user1 for login no_priv_login1 +go + +use master +go + +-- tsql user=no_priv_login1 password=12345678 +-- case-1 - when current user is guest +-- permission denied +select current_user,db_name() +go +~~START~~ +varchar#!#nvarchar +guest#!#master +~~END~~ + + +drop role dont_drop_role +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the role 'dont_drop_role', because it does not exist or you do not have permission.)~~ + + +drop user no_priv_user2 +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the user 'no_priv_user2', because it does not exist or you do not have permission.)~~ + + +-- tsql +select current_user,db_name() +go +~~START~~ +varchar#!#nvarchar +dbo#!#master +~~END~~ + + +create user no_priv_user1 for login no_priv_login1 +go + +-- tsql user=no_priv_login1 password=12345678 +-- case-2 - when current user has no privilege +-- permission denied +use master +go + +select current_user +go +~~START~~ +varchar +no_priv_user1 +~~END~~ + + +drop role dont_drop_role +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the role 'dont_drop_role', because it does not exist or you do not have permission.)~~ + + +drop user no_priv_user2 +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the user 'no_priv_user2', because it does not exist or you do not have permission.)~~ + + +-- tsql + +-- case-3 - when current login has privilege +-- 3.1 - when login is database owner +Alter authorization on database::restrict_user_db1 to no_priv_login2 +go + +-- tsql user=no_priv_login2 password=12345678 database=restrict_user_db1 +-- allowed +select current_user +go +~~START~~ +varchar +dbo +~~END~~ + + +drop role dont_drop_role +go + +drop user no_priv_user1 +go + +-- error +-- try to drop non existing role/user +drop role fake_role +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the role 'fake_role', because it does not exist or you do not have permission.)~~ + + +drop user fake_user; +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the user 'fake_user', because it does not exist or you do not have permission.)~~ + + +drop user db +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the user 'db', because it does not exist or you do not have permission.)~~ + + +drop role db_own +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the role 'db_own', because it does not exist or you do not have permission.)~~ + + +drop user dbo_u1 +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the user 'dbo_u1', because it does not exist or you do not have permission.)~~ + + +drop role db_owner_r1 +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the role 'db_owner_r1', because it does not exist or you do not have permission.)~~ + + +-- should deny +-- try to drop dbo user, db_owner role +drop role db_owner +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the role 'db_owner'.)~~ + + +drop user dbo +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the user 'dbo'.)~~ + + + +create role dont_drop_role +go + +create user no_priv_user1 for login no_priv_login1 +go + +-- tsql +-- 3.2 - when login is sysadmin +-- allowed +drop user no_priv_user1 +go + +drop user no_priv_user2 +go + +drop role dont_drop_role +go + +-- 3.3 - when drop user/role has IF EXISTS clause +-- Should not throw error for the case where IF EXISTS clause is included +drop role if exists fake_role; +go + +drop user if exists fake_user; +go + +-- both of the following statements should be executed successfully since user/role exists +drop role if exists no_priv_user3; +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the role 'no_priv_user3', because it does not exist or you do not have permission.)~~ + + +drop user if exists no_priv_user3; +go + +drop user if exists no_priv_role3; +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the user 'no_priv_role3', because it does not exist or you do not have permission.)~~ + + +drop role if exists no_priv_role3; +go + +select count(*) from sys.database_principals where name like '%no_priv_%3'; +go +~~START~~ +int +0 +~~END~~ + + +-- psql +-- 3.4 - Try dropping role created from PG role +create role master_pguser1; +go + +-- tsql +drop user pguser1; +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the user 'pguser1', because it does not exist or you do not have permission.)~~ + + +drop role pguser1; +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the role 'pguser1', because it does not exist or you do not have permission.)~~ + + +-- it will still try to drop it, but permission denied will be generated since +-- bbf_role_admin won't have privilege to drop non-babelfish roles +drop user if exists pguser1; +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the user 'pguser1', because it does not exist or you do not have permission.)~~ + + +drop role if exists pguser1; +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the role 'pguser1', because it does not exist or you do not have permission.)~~ + + +-- psql +drop role master_pguser1; +go + +-- Need to terminate active session before cleaning up the login +SELECT pg_terminate_backend(pid) FROM pg_stat_get_activity(NULL) +WHERE sys.suser_name(usesysid) = 'no_priv_login1' AND backend_type = 'client backend' AND usesysid IS NOT NULL; +go +~~START~~ +bool +t +~~END~~ + + + +-- tsql +drop login no_priv_login1 +go + +-- psql +-- Need to terminate active session before cleaning up the login +SELECT pg_terminate_backend(pid) FROM pg_stat_get_activity(NULL) +WHERE sys.suser_name(usesysid) = 'no_priv_login2' AND backend_type = 'client backend' AND usesysid IS NOT NULL; +go +~~START~~ +bool +t +~~END~~ + + +-- tsql +drop login no_priv_login2 +go + +drop login no_priv_login3 +go + +drop database restrict_user_db1 +go + +-- psql +-- 3.5 - Try dropping user created for another database +ALTER SYSTEM SET babelfishpg_tsql.migration_mode = 'multi-db'; +GO +SELECT pg_reload_conf(); +GO +~~START~~ +bool +t +~~END~~ + +-- Wait to sync with another session +SELECT pg_sleep(1); +GO +~~START~~ +void + +~~END~~ + + +-- tsql +create database restrict_user_db1; +go + +create database restrict_user_db1_restrict_user_db1; +go + +create login restrict_user_l1 with password = '12345678' +go + +use restrict_user_db1 +go + +create user restrict_user_db1_user1 for login restrict_user_l1; +go + +use restrict_user_db1_restrict_user_db1 +go + +drop user user1; +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the user 'user1', because it does not exist or you do not have permission.)~~ + + +drop user if exists user1; +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the user 'user1', because it does not exist or you do not have permission.)~~ + + +use master +go + +-- psql +-- Need to terminate active session before cleaning up the login +SELECT pg_terminate_backend(pid) FROM pg_stat_get_activity(NULL) +WHERE sys.suser_name(usesysid) = 'restrict_user_l1' AND backend_type = 'client backend' AND usesysid IS NOT NULL; +go +~~START~~ +bool +~~END~~ + + +-- tsql +use restrict_user_db1 +go + +drop user restrict_user_db1_user1; +go + +use master +go + +drop login restrict_user_l1; +go + +drop database restrict_user_db1; +go + +drop database restrict_user_db1_restrict_user_db1; +go + +-- psql +ALTER SYSTEM SET babelfishpg_tsql.migration_mode = 'single-db'; +GO +-- Wait to sync with another session +SELECT pg_sleep(1); +GO +~~START~~ +void + +~~END~~ + diff --git a/test/JDBC/input/restrict_drop_user_role.mix b/test/JDBC/input/restrict_drop_user_role.mix new file mode 100644 index 0000000000..a013061551 --- /dev/null +++ b/test/JDBC/input/restrict_drop_user_role.mix @@ -0,0 +1,279 @@ +-- tsql +create login no_priv_login1 with password = '12345678' +go + +create role dont_drop_role +go + +create login no_priv_login2 with password = '12345678' +go + +create user no_priv_user2 for login no_priv_login2 +go + +create login no_priv_login3 with password = '12345678' +go + +create user no_priv_user3 for login no_priv_login3 +go + +create role no_priv_role3; +go + +create database restrict_user_db1 +go + +use restrict_user_db1 +go + +create role dont_drop_role +go + +create user no_priv_user1 for login no_priv_login1 +go + +use master +go + +-- case-1 - when current user is guest +-- permission denied +-- tsql user=no_priv_login1 password=12345678 +select current_user,db_name() +go + +drop role dont_drop_role +go + +drop user no_priv_user2 +go + +-- tsql +select current_user,db_name() +go + +create user no_priv_user1 for login no_priv_login1 +go + +-- case-2 - when current user has no privilege +-- permission denied +-- tsql user=no_priv_login1 password=12345678 +use master +go + +select current_user +go + +drop role dont_drop_role +go + +drop user no_priv_user2 +go + +-- case-3 - when current login has privilege +-- tsql + +-- 3.1 - when login is database owner +Alter authorization on database::restrict_user_db1 to no_priv_login2 +go + +-- allowed +-- tsql user=no_priv_login2 password=12345678 database=restrict_user_db1 +select current_user +go + +drop role dont_drop_role +go + +drop user no_priv_user1 +go + +-- error +-- try to drop non existing role/user +drop role fake_role +go + +drop user fake_user; +go + +drop user db +go + +drop role db_own +go + +drop user dbo_u1 +go + +drop role db_owner_r1 +go + +-- should deny +-- try to drop dbo user, db_owner role +drop role db_owner +go + +drop user dbo +go + + +create role dont_drop_role +go + +create user no_priv_user1 for login no_priv_login1 +go + +-- 3.2 - when login is sysadmin +-- allowed +-- tsql +drop user no_priv_user1 +go + +drop user no_priv_user2 +go + +drop role dont_drop_role +go + +-- 3.3 - when drop user/role has IF EXISTS clause +-- Should not throw error for the case where IF EXISTS clause is included +drop role if exists fake_role; +go + +drop user if exists fake_user; +go + +-- both of the following statements should be executed successfully since user/role exists +drop role if exists no_priv_user3; +go + +drop user if exists no_priv_user3; +go + +drop user if exists no_priv_role3; +go + +drop role if exists no_priv_role3; +go + +select count(*) from sys.database_principals where name like '%no_priv_%3'; +go + +-- 3.4 - Try dropping role created from PG role +-- psql +create role master_pguser1; +go + +-- tsql +drop user pguser1; +go + +drop role pguser1; +go + +-- it will still try to drop it, but permission denied will be generated since +-- bbf_role_admin won't have privilege to drop non-babelfish roles +drop user if exists pguser1; +go + +drop role if exists pguser1; +go + +-- psql +drop role master_pguser1; +go + +-- Need to terminate active session before cleaning up the login +SELECT pg_terminate_backend(pid) FROM pg_stat_get_activity(NULL) +WHERE sys.suser_name(usesysid) = 'no_priv_login1' AND backend_type = 'client backend' AND usesysid IS NOT NULL; +go + + +-- tsql +drop login no_priv_login1 +go + +-- psql +-- Need to terminate active session before cleaning up the login +SELECT pg_terminate_backend(pid) FROM pg_stat_get_activity(NULL) +WHERE sys.suser_name(usesysid) = 'no_priv_login2' AND backend_type = 'client backend' AND usesysid IS NOT NULL; +go + +-- tsql +drop login no_priv_login2 +go + +drop login no_priv_login3 +go + +drop database restrict_user_db1 +go + +-- 3.5 - Try dropping user created for another database +-- psql +ALTER SYSTEM SET babelfishpg_tsql.migration_mode = 'multi-db'; +GO +SELECT pg_reload_conf(); +GO +-- Wait to sync with another session +SELECT pg_sleep(1); +GO + +-- tsql +create database restrict_user_db1; +go + +create database restrict_user_db1_restrict_user_db1; +go + +create login restrict_user_l1 with password = '12345678' +go + +use restrict_user_db1 +go + +create user restrict_user_db1_user1 for login restrict_user_l1; +go + +use restrict_user_db1_restrict_user_db1 +go + +drop user user1; +go + +drop user if exists user1; +go + +use master +go + +-- psql +-- Need to terminate active session before cleaning up the login +SELECT pg_terminate_backend(pid) FROM pg_stat_get_activity(NULL) +WHERE sys.suser_name(usesysid) = 'restrict_user_l1' AND backend_type = 'client backend' AND usesysid IS NOT NULL; +go + +-- tsql +use restrict_user_db1 +go + +drop user restrict_user_db1_user1; +go + +use master +go + +drop login restrict_user_l1; +go + +drop database restrict_user_db1; +go + +drop database restrict_user_db1_restrict_user_db1; +go + +-- psql +ALTER SYSTEM SET babelfishpg_tsql.migration_mode = 'single-db'; +GO +-- Wait to sync with another session +SELECT pg_sleep(1); +GO \ No newline at end of file