diff --git a/src/backend/commands/resgroupcmds.c b/src/backend/commands/resgroupcmds.c index 384675edb7f..3b2d5afba6b 100644 --- a/src/backend/commands/resgroupcmds.c +++ b/src/backend/commands/resgroupcmds.c @@ -34,6 +34,7 @@ #include "commands/resgroupcmds.h" #include "miscadmin.h" #include "nodes/pg_list.h" +#include "utils/acl.h" #include "utils/builtins.h" #include "utils/datetime.h" #include "utils/fmgroids.h" @@ -102,12 +103,14 @@ CreateResourceGroup(CreateResourceGroupStmt *stmt) ResGroupCaps caps; int nResGroups; MemoryContext oldContext; + Oid role; - /* Permission check - only superuser can create groups. */ - if (!superuser()) + /* Permission check - only superuser or mdb_admin can create groups. */ + role = get_role_oid("mdb_admin", true); + if (!is_member_of_role(GetUserId(), role)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to create resource groups"))); + errmsg("must be mdb_admin to create resource groups"))); /* * Check for an illegal name ('none' is used to signify no group in ALTER ROLE). @@ -268,12 +271,14 @@ DropResourceGroup(DropResourceGroupStmt *stmt) SysScanDesc sscan; Oid groupid; ResourceGroupCallbackContext *callbackCtx; + Oid role; - /* Permission check - only superuser can drop resource groups. */ - if (!superuser()) + /* Permission check - only superuser or mdb_admin can drop resource groups. */ + role = get_role_oid("mdb_admin", true); + if (!is_member_of_role(GetUserId(), role)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to drop resource groups"))); + errmsg("must be mdb_admin to drop resource groups"))); /* * Check the pg_resgroup relation to be certain the resource group already @@ -302,6 +307,13 @@ DropResourceGroup(DropResourceGroupStmt *stmt) */ groupid = ((Form_pg_resgroup) GETSTRUCT(tuple))->oid; + /* Permission check - only superuser can drop the admin/system resource groups. */ + if (!superuser() && (groupid == ADMINRESGROUP_OID || groupid == SYSTEMRESGROUP_OID)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to drop resource group \"%s\"", + stmt->name))); + /* cannot DROP default resource groups */ if (groupid == DEFAULTRESGROUP_OID || groupid == ADMINRESGROUP_OID @@ -374,12 +386,27 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt) char *io_limit = NULL; ResourceGroupCallbackContext *callbackCtx; MemoryContext oldContext; + Oid role; + + /* Permission check - only mdb_admin can alter resource groups. */ + role = get_role_oid("mdb_admin", true); + if (!is_member_of_role(GetUserId(), role)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be mdb_admin to alter resource groups"))); + + /* + * Check the pg_resgroup relation to be certain the resource group already + * exists. + */ + groupid = get_resgroup_oid(stmt->name, false); - /* Permission check - only superuser can alter resource groups. */ - if (!superuser()) + /* Permission check - only superuser can alter the admin/system resource groups. */ + if (!superuser() && (groupid == ADMINRESGROUP_OID || groupid == SYSTEMRESGROUP_OID)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to alter resource groups"))); + errmsg("must be superuser to alter resource group \"%s\"", + stmt->name))); /* Currently we only support to ALTER one limit at one time */ Assert(list_length(stmt->options) == 1); @@ -406,12 +433,6 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt) checkResgroupCapLimit(limitType, value); } - /* - * Check the pg_resgroup relation to be certain the resource group already - * exists. - */ - groupid = get_resgroup_oid(stmt->name, false); - if (limitType == RESGROUP_LIMIT_TYPE_CONCURRENCY && value == 0 && groupid == ADMINRESGROUP_OID) @@ -500,7 +521,7 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt) RESGROUP_DEFAULT_CPU_WEIGHT, ""); updateResgroupCapabilityEntry(pg_resgroupcapability_rel, - groupid, RESGROUP_LIMIT_TYPE_CPUSET, + groupid, RESGROUP_LIMIT_TYPE_CPUSET, 0, caps.cpuset); } else if (limitType == RESGROUP_LIMIT_TYPE_CPU) @@ -1007,7 +1028,7 @@ parseStmtOptions(CreateResourceGroupStmt *stmt, ResGroupCaps *caps) else mask |= 1 << type; - if (type == RESGROUP_LIMIT_TYPE_CPUSET) + if (type == RESGROUP_LIMIT_TYPE_CPUSET) { const char *cpuset = defGetString(defel); strlcpy(caps->cpuset, cpuset, sizeof(caps->cpuset)); @@ -1611,7 +1632,7 @@ checkCpuSetByRole(const char *cpuset) * ex: * cpuset = "1;4" * then we should assign '1' to corrdinator and '4' to segment - * + * * cpuset = "1" * assign '1' to both coordinator and segment */ diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index f7dcff72494..ed7cb0b94b1 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -100,6 +100,42 @@ have_createrole_privilege(void) return has_createrole_privilege(GetUserId()); } +/* + * is_mdb_protected_rolename + * + * mdb_admin / mdb_superuser are privilege roles resolved by name at runtime + * (see acl.c). Since the gate is keyed on the name, only superusers may manage + * these roles. + */ +static bool +is_mdb_protected_rolename(const char *rolename) +{ + return rolename != NULL && + (strcmp(rolename, "mdb_admin") == 0 || + strcmp(rolename, "mdb_superuser") == 0); +} + + +/* + * may_manage_mdb_role_membership + * + * Membership in mdb_admin / mdb_superuser is the gate that confers their + * privileges (see acl.c), so it must not be self-granted by an arbitrary + * CREATEROLE holder. Permit it only for superusers (the control plane) and + * for members of mdb_superuser, who are trusted to delegate the gate. + */ + static bool + may_manage_mdb_role_membership(void) + { + Oid mdb_superuser_roleoid; + + if (superuser()) + return true; + + mdb_superuser_roleoid = get_role_oid("mdb_superuser", true); + return OidIsValid(mdb_superuser_roleoid) && + is_member_of_role(GetUserId(), mdb_superuser_roleoid); + } /* * CREATE ROLE @@ -505,6 +541,16 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) stmt->role), errdetail("Role names starting with \"pg_\" are reserved."))); + /* + * Only superusers may create the mdb_admin / mdb_superuser privilege roles, + * so a CREATEROLE holder cannot create and own the gate. + */ + if (is_mdb_protected_rolename(stmt->role) && !superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to create role \"%s\"", + stmt->role))); + /* * If built with appropriate switch, whine when regression-testing * conventions for role names are violated. @@ -1283,6 +1329,15 @@ AlterRole(AlterRoleStmt *stmt) bWas_super = ((Form_pg_authid) GETSTRUCT(tuple))->rolsuper; + /* + * Only superusers may alter the mdb_admin / mdb_superuser privilege roles + * (e.g. reset their password), so a CREATEROLE holder cannot take them over. + */ + if (is_mdb_protected_rolename(rolename) && !superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to alter role \"%s\"", rolename))); + if (authform->rolsuper || issuper >= 0) { if (!superuser()) @@ -2087,6 +2142,15 @@ DropRole(DropRoleStmt *stmt) (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("must be superuser to drop superusers"))); + /* + * Likewise, only superusers may drop the mdb_admin / mdb_superuser + * privilege roles, so the gate cannot be removed and re-created. + */ + if (is_mdb_protected_rolename(role) && !superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to drop role \"%s\"", role))); + /* DROP hook for the role being removed */ InvokeObjectDropHook(AuthIdRelationId, roleid, 0); @@ -2182,7 +2246,7 @@ DropRole(DropRoleStmt *stmt) */ DeleteSharedComments(roleid, AuthIdRelationId); DeleteSharedSecurityLabel(roleid, AuthIdRelationId); - + /* * Delete any tag description and associated dependencies. */ @@ -2330,6 +2394,18 @@ RenameRole(const char *oldname, const char *newname) errmsg("permission denied to rename role"))); } + /* + * Only superusers may rename the mdb_admin / mdb_superuser privilege roles, + * in either direction, so a CREATEROLE holder cannot move the real role out + * of the way or claim the protected name for a role they control. + */ + if ((is_mdb_protected_rolename(oldname) || + is_mdb_protected_rolename(newname)) && !superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to rename role \"%s\"", + is_mdb_protected_rolename(oldname) ? oldname : newname))); + /* OK, construct the modified tuple */ for (i = 0; i < Natts_pg_authid; i++) repl_repl[i] = false; @@ -2598,6 +2674,18 @@ AddRoleMems(const char *rolename, Oid roleid, } else { + /* + * Membership in the mdb_admin / mdb_superuser privilege roles may only + * be granted by a superuser (the control plane) or a member of + * mdb_superuser; otherwise any CREATEROLE holder could self-grant the + * gate. + */ + if (is_mdb_protected_rolename(rolename) && !may_manage_mdb_role_membership()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser or a member of mdb_superuser to grant membership in role \"%s\"", + rolename))); + if (!have_createrole_privilege() && !is_admin_of_role(grantorId, roleid)) ereport(ERROR, @@ -3069,6 +3157,19 @@ DelRoleMems(const char *rolename, Oid roleid, } else { + + /* + * Membership in the mdb_admin / mdb_superuser privilege roles may only + * be revoked by a superuser (the control plane) or a member of + * mdb_superuser; otherwise a CREATEROLE holder could lock the legitimate + * admin out of the gate. + */ + if (is_mdb_protected_rolename(rolename) && !may_manage_mdb_role_membership()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser or a member of mdb_superuser to revoke membership in role \"%s\"", + rolename))); + if (!have_createrole_privilege() && !is_admin_of_role(GetUserId(), roleid)) ereport(ERROR, diff --git a/src/backend/utils/resgroup/resgroup_helper.c b/src/backend/utils/resgroup/resgroup_helper.c index 00aaded168d..4df203e3101 100644 --- a/src/backend/utils/resgroup/resgroup_helper.c +++ b/src/backend/utils/resgroup/resgroup_helper.c @@ -21,6 +21,7 @@ #include "cdb/cdbvars.h" #include "commands/resgroupcmds.h" #include "storage/procarray.h" +#include "utils/acl.h" #include "utils/builtins.h" #include "utils/datetime.h" #include "utils/resgroup.h" @@ -458,16 +459,18 @@ pg_resgroup_move_query(PG_FUNCTION_ARGS) int sessionId; Oid groupId; const char *groupName; + Oid role; if (!IsResGroupEnabled()) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), (errmsg("resource group is not enabled")))); - if (!superuser()) + role = get_role_oid("mdb_admin", true); + if (!is_member_of_role(GetUserId(), role)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - (errmsg("must be superuser to move query")))); + (errmsg("must be mdb_admin to move query")))); if (Gp_role == GP_ROLE_DISPATCH) { diff --git a/src/test/isolation2/expected/resgroup/resgroup_mdb_admin.out b/src/test/isolation2/expected/resgroup/resgroup_mdb_admin.out new file mode 100644 index 00000000000..c8fd232faea --- /dev/null +++ b/src/test/isolation2/expected/resgroup/resgroup_mdb_admin.out @@ -0,0 +1,123 @@ +-- Tests permission checks for the mdb_admin role with +-- resource groups enabled. + +-- start_matchsubs +-- m/ERROR: cannot find process: \d+/ +-- s/\d+/XXX/g +-- end_matchsubs + +DROP ROLE IF EXISTS role_rg_admin; +DROP +DROP ROLE IF EXISTS role_rg_noadmin; +DROP +DROP ROLE IF EXISTS mdb_admin; +DROP +-- start_ignore +DROP RESOURCE GROUP rg_perm_admin1; +DROP RESOURCE GROUP rg_perm_admin2; +DROP RESOURCE GROUP rg_perm_revoke1; +DROP RESOURCE GROUP rg_perm_revoke2; +DROP RESOURCE GROUP rg_perm_test; +-- end_ignore + +-- --------------------------------------------------------------------- +-- Setup. The mdb_admin role is not predefined in the catalog; it is +-- created here the same way the control plane provisions it at runtime. +-- --------------------------------------------------------------------- +CREATE RESOURCE GROUP rg_perm_test WITH (concurrency=2, cpu_max_percent=10); +CREATE +CREATE ROLE mdb_admin; +CREATE +CREATE ROLE role_rg_admin RESOURCE GROUP rg_perm_test; +CREATE +CREATE ROLE role_rg_noadmin RESOURCE GROUP rg_perm_test; +CREATE +GRANT mdb_admin TO role_rg_admin; +GRANT + +-- --------------------------------------------------------------------- +-- 1. Member of mdb_admin can CREATE/ALTER/DROP resource groups +-- (statements are dispatched to segments). +-- --------------------------------------------------------------------- +1: SET ROLE role_rg_admin; +SET +1: CREATE RESOURCE GROUP rg_perm_admin1 WITH (concurrency=1, cpu_max_percent=5); +CREATE +1: ALTER RESOURCE GROUP rg_perm_admin1 SET cpu_max_percent 6; +ALTER +1: DROP RESOURCE GROUP rg_perm_admin1; +DROP + +-- 2. Even a member cannot ALTER or DROP the system admin_group. +1: ALTER RESOURCE GROUP admin_group SET cpu_max_percent 99; +ERROR: must be superuser to alter resource group "admin_group" +1: DROP RESOURCE GROUP admin_group; +ERROR: must be superuser to drop resource group "admin_group" +1q: ... + +-- --------------------------------------------------------------------- +-- 3. A non-member is rejected on every entry point. +-- --------------------------------------------------------------------- +2: SET ROLE role_rg_noadmin; +SET +2: CREATE RESOURCE GROUP rg_perm_admin2 WITH (concurrency=1, cpu_max_percent=5); +ERROR: must be mdb_admin to create resource groups +2: ALTER RESOURCE GROUP rg_perm_test SET cpu_max_percent 7; +ERROR: must be mdb_admin to alter resource groups +2: DROP RESOURCE GROUP rg_perm_test; +ERROR: must be mdb_admin to drop resource groups +2q: ... + +-- --------------------------------------------------------------------- +-- 4. pg_resgroup_move_query() honours the same permission check. +-- The first call (non-member) must fail with "must be mdb_admin". +-- The second call (member) gets past the permission gate and +-- fails on the pid lookup (masked by start_matchsubs above). +-- --------------------------------------------------------------------- +3: SET ROLE role_rg_noadmin; +SET +3: SELECT pg_resgroup_move_query(999999999, 'admin_group'); +ERROR: must be mdb_admin to move query +3: RESET ROLE; +RESET +3: SET ROLE role_rg_admin; +SET +3: SELECT pg_resgroup_move_query(999999999, 'admin_group'); +ERROR: cannot find process: XXX +3q: ... + +-- --------------------------------------------------------------------- +-- 5. Cross-session REVOKE takes effect on the granted session's +-- next statement (the privilege is re-checked per command, not +-- cached at SET ROLE time). +-- --------------------------------------------------------------------- +4: SET ROLE role_rg_admin; +SET +4: CREATE RESOURCE GROUP rg_perm_revoke1 WITH (concurrency=1, cpu_max_percent=5); +CREATE +5: REVOKE mdb_admin FROM role_rg_admin; +REVOKE +4: CREATE RESOURCE GROUP rg_perm_revoke2 WITH (concurrency=1, cpu_max_percent=5); +ERROR: must be mdb_admin to create resource groups +4: DROP RESOURCE GROUP rg_perm_revoke1; +ERROR: must be mdb_admin to drop resource groups +4q: ... +5q: ... + +-- --------------------------------------------------------------------- +-- Cleanup. Roles must be dropped before the resource group they +-- reference, otherwise DROP RESOURCE GROUP fails with +-- "resource group is used by at least one role". +-- --------------------------------------------------------------------- +RESET ROLE; +RESET +DROP ROLE role_rg_admin; +DROP +DROP ROLE role_rg_noadmin; +DROP +DROP ROLE mdb_admin; +DROP +DROP RESOURCE GROUP rg_perm_revoke1; +DROP +DROP RESOURCE GROUP rg_perm_test; +DROP diff --git a/src/test/isolation2/isolation2_resgroup_v1_schedule b/src/test/isolation2/isolation2_resgroup_v1_schedule index f5dea2d4012..eccfe0210ef 100644 --- a/src/test/isolation2/isolation2_resgroup_v1_schedule +++ b/src/test/isolation2/isolation2_resgroup_v1_schedule @@ -34,6 +34,7 @@ test: resgroup/resgroup_move_query # regression tests test: resgroup/resgroup_recreate test: resgroup/resgroup_functions +test: resgroup/resgroup_mdb_admin # dump info test: resgroup/resgroup_dumpinfo diff --git a/src/test/isolation2/isolation2_resgroup_v2_schedule b/src/test/isolation2/isolation2_resgroup_v2_schedule index 0190efcd3d8..fa448b3c913 100644 --- a/src/test/isolation2/isolation2_resgroup_v2_schedule +++ b/src/test/isolation2/isolation2_resgroup_v2_schedule @@ -35,6 +35,7 @@ test: resgroup/resgroup_io_limit # regression tests test: resgroup/resgroup_recreate test: resgroup/resgroup_functions +test: resgroup/resgroup_mdb_admin # parallel tests #test: resgroup/restore_default_resgroup diff --git a/src/test/isolation2/sql/resgroup/resgroup_mdb_admin.sql b/src/test/isolation2/sql/resgroup/resgroup_mdb_admin.sql new file mode 100644 index 00000000000..3eedba5bc4e --- /dev/null +++ b/src/test/isolation2/sql/resgroup/resgroup_mdb_admin.sql @@ -0,0 +1,89 @@ +-- Tests permission checks for the mdb_admin role with +-- resource groups enabled. + +-- start_matchsubs +-- m/ERROR: cannot find process: \d+/ +-- s/\d+/XXX/g +-- end_matchsubs + +DROP ROLE IF EXISTS role_rg_admin; +DROP ROLE IF EXISTS role_rg_noadmin; +DROP ROLE IF EXISTS mdb_admin; +-- start_ignore +DROP RESOURCE GROUP rg_perm_admin1; +DROP RESOURCE GROUP rg_perm_admin2; +DROP RESOURCE GROUP rg_perm_revoke1; +DROP RESOURCE GROUP rg_perm_revoke2; +DROP RESOURCE GROUP rg_perm_test; +-- end_ignore + +-- --------------------------------------------------------------------- +-- Setup. The mdb_admin role is not predefined in the catalog; it is +-- created here the same way the control plane provisions it at runtime. +-- --------------------------------------------------------------------- +CREATE RESOURCE GROUP rg_perm_test WITH (concurrency=2, cpu_max_percent=10); +CREATE ROLE mdb_admin; +CREATE ROLE role_rg_admin RESOURCE GROUP rg_perm_test; +CREATE ROLE role_rg_noadmin RESOURCE GROUP rg_perm_test; +GRANT mdb_admin TO role_rg_admin; + +-- --------------------------------------------------------------------- +-- 1. Member of mdb_admin can CREATE/ALTER/DROP resource groups +-- (statements are dispatched to segments). +-- --------------------------------------------------------------------- +1: SET ROLE role_rg_admin; +1: CREATE RESOURCE GROUP rg_perm_admin1 WITH (concurrency=1, cpu_max_percent=5); +1: ALTER RESOURCE GROUP rg_perm_admin1 SET cpu_max_percent 6; +1: DROP RESOURCE GROUP rg_perm_admin1; + +-- 2. Even a member cannot ALTER or DROP the system admin_group. +1: ALTER RESOURCE GROUP admin_group SET cpu_max_percent 99; +1: DROP RESOURCE GROUP admin_group; +1q: + +-- --------------------------------------------------------------------- +-- 3. A non-member is rejected on every entry point. +-- --------------------------------------------------------------------- +2: SET ROLE role_rg_noadmin; +2: CREATE RESOURCE GROUP rg_perm_admin2 WITH (concurrency=1, cpu_max_percent=5); +2: ALTER RESOURCE GROUP rg_perm_test SET cpu_max_percent 7; +2: DROP RESOURCE GROUP rg_perm_test; +2q: + +-- --------------------------------------------------------------------- +-- 4. pg_resgroup_move_query() honours the same permission check. +-- The first call (non-member) must fail with "must be mdb_admin". +-- The second call (member) gets past the permission gate and +-- fails on the pid lookup (masked by start_matchsubs above). +-- --------------------------------------------------------------------- +3: SET ROLE role_rg_noadmin; +3: SELECT pg_resgroup_move_query(999999999, 'admin_group'); +3: RESET ROLE; +3: SET ROLE role_rg_admin; +3: SELECT pg_resgroup_move_query(999999999, 'admin_group'); +3q: + +-- --------------------------------------------------------------------- +-- 5. Cross-session REVOKE takes effect on the granted session's +-- next statement (the privilege is re-checked per command, not +-- cached at SET ROLE time). +-- --------------------------------------------------------------------- +4: SET ROLE role_rg_admin; +4: CREATE RESOURCE GROUP rg_perm_revoke1 WITH (concurrency=1, cpu_max_percent=5); +5: REVOKE mdb_admin FROM role_rg_admin; +4: CREATE RESOURCE GROUP rg_perm_revoke2 WITH (concurrency=1, cpu_max_percent=5); +4: DROP RESOURCE GROUP rg_perm_revoke1; +4q: +5q: + +-- --------------------------------------------------------------------- +-- Cleanup. Roles must be dropped before the resource group they +-- reference, otherwise DROP RESOURCE GROUP fails with +-- "resource group is used by at least one role". +-- --------------------------------------------------------------------- +RESET ROLE; +DROP ROLE role_rg_admin; +DROP ROLE role_rg_noadmin; +DROP ROLE mdb_admin; +DROP RESOURCE GROUP rg_perm_revoke1; +DROP RESOURCE GROUP rg_perm_test; diff --git a/src/test/regress/expected/mdb_superuser.out b/src/test/regress/expected/mdb_superuser.out index 21bafb1011b..330f6dc2e25 100644 --- a/src/test/regress/expected/mdb_superuser.out +++ b/src/test/regress/expected/mdb_superuser.out @@ -68,9 +68,9 @@ ERROR: permission denied to create database CREATE ROLE regress_role_fail; ERROR: permission denied to create role ALTER ROLE mdb_superuser WITH CREATEROLE; -ERROR: permission denied +ERROR: must be superuser to alter role "mdb_superuser" ALTER ROLE mdb_superuser WITH CREATEDB; -ERROR: permission denied +ERROR: must be superuser to alter role "mdb_superuser" ALTER ROLE regress_mdb_superuser_user2 WITH CREATEROLE; ERROR: permission denied ALTER ROLE regress_mdb_superuser_user2 WITH CREATEDB; diff --git a/src/test/regress/expected/resource_group.out b/src/test/regress/expected/resource_group.out index 0bcf2b14474..a7700f97423 100644 --- a/src/test/regress/expected/resource_group.out +++ b/src/test/regress/expected/resource_group.out @@ -4,9 +4,9 @@ -- this test creates resource group objects and roles associated with -- resource groups so pg_dumpall/pg_upgrade can dump those objects at -- the end of ICW. --- +-- -- NOTE: please always put this test in the end of this file and do not --- drop them. +-- drop the rg_dump_test* objects. -- start_ignore DROP ROLE IF EXISTS role_dump_test1; NOTICE: role "role_dump_test1" does not exist, skipping @@ -14,12 +14,28 @@ DROP ROLE IF EXISTS role_dump_test2; NOTICE: role "role_dump_test2" does not exist, skipping DROP ROLE IF EXISTS role_dump_test3; NOTICE: role "role_dump_test3" does not exist, skipping +DROP ROLE IF EXISTS rg_admin_test; +NOTICE: role "rg_admin_test" does not exist, skipping +DROP ROLE IF EXISTS rg_noadmin_test; +NOTICE: role "rg_noadmin_test" does not exist, skipping +DROP ROLE IF EXISTS rg_admin_grp; +NOTICE: role "rg_admin_grp" does not exist, skipping +DROP ROLE IF EXISTS rg_attacker; +NOTICE: role "rg_attacker" does not exist, skipping +DROP ROLE IF EXISTS mdb_admin; +NOTICE: role "mdb_admin" does not exist, skipping +DROP ROLE IF EXISTS mdb_superuser; +NOTICE: role "mdb_superuser" does not exist, skipping DROP RESOURCE GROUP rg_dump_test1; ERROR: resource group "rg_dump_test1" does not exist DROP RESOURCE GROUP rg_dump_test2; ERROR: resource group "rg_dump_test2" does not exist DROP RESOURCE GROUP rg_dump_test3; ERROR: resource group "rg_dump_test3" does not exist +DROP RESOURCE GROUP rg_admin_test1; +ERROR: resource group "rg_admin_test1" does not exist +DROP RESOURCE GROUP rg_admin_test2; +ERROR: resource group "rg_admin_test2" does not exist -- end_ignore CREATE RESOURCE GROUP rg_dump_test1 WITH (concurrency=2, cpu_max_percent=5); WARNING: resource group is disabled @@ -42,3 +58,87 @@ CREATE ROLE role_dump_test3 RESOURCE GROUP rg_dump_test3; NOTICE: resource queue required -- using default resource queue "pg_default" WARNING: resource group is disabled HINT: To enable set gp_resource_manager=group +-- Test that members of the mdb_admin role can manage resource groups +-- without superuser, but cannot touch the system admin_group. The role +-- is not predefined in the catalog; it is created here the same way the +-- control plane provisions it at runtime. +CREATE ROLE mdb_admin; +CREATE ROLE rg_admin_test RESOURCE GROUP rg_dump_test1; +NOTICE: resource queue required -- using default resource queue "pg_default" +WARNING: resource group is disabled +HINT: To enable set gp_resource_manager=group +CREATE ROLE rg_noadmin_test RESOURCE GROUP rg_dump_test1; +NOTICE: resource queue required -- using default resource queue "pg_default" +WARNING: resource group is disabled +HINT: To enable set gp_resource_manager=group +GRANT mdb_admin TO rg_admin_test; +SET ROLE rg_admin_test; +CREATE RESOURCE GROUP rg_admin_test1 WITH (concurrency=2, cpu_max_percent=5, memory_quota=5); +WARNING: resource group is disabled +HINT: To enable set gp_resource_manager=group +CREATE RESOURCE GROUP rg_admin_test2 WITH (concurrency=2, cpu_max_percent=5, memory_quota=5); +WARNING: resource group is disabled +HINT: To enable set gp_resource_manager=group +ALTER RESOURCE GROUP rg_admin_test1 SET cpu_max_percent 2; +DROP RESOURCE GROUP rg_admin_test1; +-- mdb_admin cannot ALTER or DROP the built-in resource groups. +ALTER RESOURCE GROUP admin_group SET cpu_max_percent 2; +ERROR: must be superuser to alter resource group "admin_group" +ALTER RESOURCE GROUP system_group SET cpu_max_percent 2; +ERROR: must be superuser to alter resource group "system_group" +DROP RESOURCE GROUP admin_group; +ERROR: must be superuser to drop resource group "admin_group" +DROP RESOURCE GROUP system_group; +ERROR: must be superuser to drop resource group "system_group" +DROP RESOURCE GROUP default_group; +ERROR: cannot drop default resource group "default_group" +SET ROLE rg_noadmin_test; +CREATE RESOURCE GROUP rg_admin_test1 WITH (concurrency=2, cpu_max_percent=5, memory_quota=5); +ERROR: must be mdb_admin to create resource groups +ALTER RESOURCE GROUP rg_admin_test2 SET cpu_max_percent 2; +ERROR: must be mdb_admin to alter resource groups +DROP RESOURCE GROUP rg_admin_test2; +ERROR: must be mdb_admin to drop resource groups +DROP RESOURCE GROUP admin_group; +ERROR: must be mdb_admin to drop resource groups +-- After REVOKE the role loses its privileges immediately. +RESET ROLE; +REVOKE mdb_admin FROM rg_admin_test; +SET ROLE rg_admin_test; +CREATE RESOURCE GROUP rg_admin_test3 WITH (concurrency=2, cpu_max_percent=5, memory_quota=5); +ERROR: must be mdb_admin to create resource groups +ALTER RESOURCE GROUP rg_admin_test2 SET cpu_max_percent 3; +ERROR: must be mdb_admin to alter resource groups +DROP RESOURCE GROUP rg_admin_test2; +ERROR: must be mdb_admin to drop resource groups +-- Transitive membership through an intermediate role must also work. +RESET ROLE; +CREATE ROLE rg_admin_grp; +GRANT mdb_admin TO rg_admin_grp; +GRANT rg_admin_grp TO rg_admin_test; +SET ROLE rg_admin_test; +ALTER RESOURCE GROUP rg_admin_test2 SET cpu_max_percent 4; +DROP RESOURCE GROUP rg_admin_test2; +-- A CREATEROLE user must not be able to hijack the mdb_admin role. +RESET ROLE; +CREATE ROLE rg_attacker CREATEROLE; +SET ROLE rg_attacker; +GRANT mdb_admin TO rg_attacker; -- self-grant membership +ERROR: must be superuser or a member of mdb_superuser to grant membership in role "mdb_admin" +DROP ROLE mdb_admin; -- drop the gate +ERROR: must be superuser to drop role "mdb_admin" +ALTER ROLE mdb_admin RENAME TO rg_attacker_admin; -- move the real role aside +ERROR: must be superuser to rename role "mdb_admin" +ALTER ROLE mdb_admin PASSWORD 'hijack'; -- take it over via password reset +ERROR: must be superuser to alter role "mdb_admin" +CREATE ROLE mdb_superuser; -- create a protected role +ERROR: must be superuser to create role "mdb_superuser" +RESET ROLE; +DROP ROLE rg_attacker; +RESET ROLE; +REVOKE rg_admin_grp FROM rg_admin_test; +REVOKE mdb_admin FROM rg_admin_grp; +DROP ROLE rg_admin_grp; +DROP ROLE rg_admin_test; +DROP ROLE rg_noadmin_test; +DROP ROLE mdb_admin; diff --git a/src/test/regress/sql/resource_group.sql b/src/test/regress/sql/resource_group.sql index 1411eb0d61f..f64bf6e5bb0 100644 --- a/src/test/regress/sql/resource_group.sql +++ b/src/test/regress/sql/resource_group.sql @@ -4,18 +4,26 @@ -- this test creates resource group objects and roles associated with -- resource groups so pg_dumpall/pg_upgrade can dump those objects at -- the end of ICW. --- +-- -- NOTE: please always put this test in the end of this file and do not --- drop them. +-- drop the rg_dump_test* objects. -- start_ignore DROP ROLE IF EXISTS role_dump_test1; DROP ROLE IF EXISTS role_dump_test2; DROP ROLE IF EXISTS role_dump_test3; +DROP ROLE IF EXISTS rg_admin_test; +DROP ROLE IF EXISTS rg_noadmin_test; +DROP ROLE IF EXISTS rg_admin_grp; +DROP ROLE IF EXISTS rg_attacker; +DROP ROLE IF EXISTS mdb_admin; +DROP ROLE IF EXISTS mdb_superuser; DROP RESOURCE GROUP rg_dump_test1; DROP RESOURCE GROUP rg_dump_test2; DROP RESOURCE GROUP rg_dump_test3; +DROP RESOURCE GROUP rg_admin_test1; +DROP RESOURCE GROUP rg_admin_test2; -- end_ignore CREATE RESOURCE GROUP rg_dump_test1 WITH (concurrency=2, cpu_max_percent=5); @@ -25,3 +33,69 @@ CREATE RESOURCE GROUP rg_dump_test3 WITH (concurrency=2, cpu_max_percent=5); CREATE ROLE role_dump_test1 RESOURCE GROUP rg_dump_test1; CREATE ROLE role_dump_test2 RESOURCE GROUP rg_dump_test2; CREATE ROLE role_dump_test3 RESOURCE GROUP rg_dump_test3; + +-- Test that members of the mdb_admin role can manage resource groups +-- without superuser, but cannot touch the system admin_group. The role +-- is not predefined in the catalog; it is created here the same way the +-- control plane provisions it at runtime. +CREATE ROLE mdb_admin; +CREATE ROLE rg_admin_test RESOURCE GROUP rg_dump_test1; +CREATE ROLE rg_noadmin_test RESOURCE GROUP rg_dump_test1; +GRANT mdb_admin TO rg_admin_test; + +SET ROLE rg_admin_test; + +CREATE RESOURCE GROUP rg_admin_test1 WITH (concurrency=2, cpu_max_percent=5, memory_quota=5); +CREATE RESOURCE GROUP rg_admin_test2 WITH (concurrency=2, cpu_max_percent=5, memory_quota=5); +ALTER RESOURCE GROUP rg_admin_test1 SET cpu_max_percent 2; +DROP RESOURCE GROUP rg_admin_test1; +-- mdb_admin cannot ALTER or DROP the built-in resource groups. +ALTER RESOURCE GROUP admin_group SET cpu_max_percent 2; +ALTER RESOURCE GROUP system_group SET cpu_max_percent 2; +DROP RESOURCE GROUP admin_group; +DROP RESOURCE GROUP system_group; +DROP RESOURCE GROUP default_group; + +SET ROLE rg_noadmin_test; + +CREATE RESOURCE GROUP rg_admin_test1 WITH (concurrency=2, cpu_max_percent=5, memory_quota=5); +ALTER RESOURCE GROUP rg_admin_test2 SET cpu_max_percent 2; +DROP RESOURCE GROUP rg_admin_test2; +DROP RESOURCE GROUP admin_group; + +-- After REVOKE the role loses its privileges immediately. +RESET ROLE; +REVOKE mdb_admin FROM rg_admin_test; +SET ROLE rg_admin_test; +CREATE RESOURCE GROUP rg_admin_test3 WITH (concurrency=2, cpu_max_percent=5, memory_quota=5); +ALTER RESOURCE GROUP rg_admin_test2 SET cpu_max_percent 3; +DROP RESOURCE GROUP rg_admin_test2; + +-- Transitive membership through an intermediate role must also work. +RESET ROLE; +CREATE ROLE rg_admin_grp; +GRANT mdb_admin TO rg_admin_grp; +GRANT rg_admin_grp TO rg_admin_test; +SET ROLE rg_admin_test; +ALTER RESOURCE GROUP rg_admin_test2 SET cpu_max_percent 4; +DROP RESOURCE GROUP rg_admin_test2; + +-- A CREATEROLE user must not be able to hijack the mdb_admin role. +RESET ROLE; +CREATE ROLE rg_attacker CREATEROLE; +SET ROLE rg_attacker; +GRANT mdb_admin TO rg_attacker; -- self-grant membership +DROP ROLE mdb_admin; -- drop the gate +ALTER ROLE mdb_admin RENAME TO rg_attacker_admin; -- move the real role aside +ALTER ROLE mdb_admin PASSWORD 'hijack'; -- take it over via password reset +CREATE ROLE mdb_superuser; -- create a protected role +RESET ROLE; +DROP ROLE rg_attacker; + +RESET ROLE; +REVOKE rg_admin_grp FROM rg_admin_test; +REVOKE mdb_admin FROM rg_admin_grp; +DROP ROLE rg_admin_grp; +DROP ROLE rg_admin_test; +DROP ROLE rg_noadmin_test; +DROP ROLE mdb_admin;