From 57e0282cdfe1862e234ba880b74540c8fd771157 Mon Sep 17 00:00:00 2001 From: Mark Dilger Date: Mon, 7 Mar 2022 20:12:09 -0800 Subject: [PATCH v1] WIP: Removing role self-administration --- src/backend/utils/adt/acl.c | 33 ------------------------ src/test/regress/expected/privileges.out | 4 +-- src/test/regress/sql/privileges.sql | 2 +- 3 files changed, 3 insertions(+), 36 deletions(-) diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c index 0a16f8156c..659f6db955 100644 --- a/src/backend/utils/adt/acl.c +++ b/src/backend/utils/adt/acl.c @@ -4935,39 +4935,6 @@ is_admin_of_role(Oid member, Oid role) if (superuser_arg(member)) return true; - if (member == role) - - /* - * A role can admin itself when it matches the session user and we're - * outside any security-restricted operation, SECURITY DEFINER or - * similar context. SQL-standard roles cannot self-admin. However, - * SQL-standard users are distinct from roles, and they are not - * grantable like roles: PostgreSQL's role-user duality extends the - * standard. Checking for a session user match has the effect of - * letting a role self-admin only when it's conspicuously behaving - * like a user. Note that allowing self-admin under a mere SET ROLE - * would make WITH ADMIN OPTION largely irrelevant; any member could - * SET ROLE to issue the otherwise-forbidden command. - * - * Withholding self-admin in a security-restricted operation prevents - * object owners from harnessing the session user identity during - * administrative maintenance. Suppose Alice owns a database, has - * issued "GRANT alice TO bob", and runs a daily ANALYZE. Bob creates - * an alice-owned SECURITY DEFINER function that issues "REVOKE alice - * FROM carol". If he creates an expression index calling that - * function, Alice will attempt the REVOKE during each ANALYZE. - * Checking InSecurityRestrictedOperation() thwarts that attack. - * - * Withholding self-admin in SECURITY DEFINER functions makes their - * behavior independent of the calling user. There's no security or - * SQL-standard-conformance need for that restriction, though. - * - * A role cannot have actual WITH ADMIN OPTION on itself, because that - * would imply a membership loop. Therefore, we're done either way. - */ - return member == GetSessionUserId() && - !InLocalUserIdChange() && !InSecurityRestrictedOperation(); - (void) roles_is_member_of(member, ROLERECURSE_MEMBERS, role, &result); return result; } diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index 291e21d7a6..73728eb23f 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -1555,8 +1555,8 @@ SET ROLE regress_priv_group2; GRANT regress_priv_group2 TO regress_priv_user5; -- fails: SET ROLE did not help ERROR: must have admin option on role "regress_priv_group2" SET SESSION AUTHORIZATION regress_priv_group2; -GRANT regress_priv_group2 TO regress_priv_user5; -- ok: a role can self-admin -NOTICE: role "regress_priv_user5" is already a member of role "regress_priv_group2" +GRANT regress_priv_group2 TO regress_priv_user5; -- fail: a role cannot self-admin +ERROR: must have admin option on role "regress_priv_group2" CREATE FUNCTION dogrant_fails() RETURNS void LANGUAGE sql SECURITY DEFINER AS 'GRANT regress_priv_group2 TO regress_priv_user5'; SELECT dogrant_fails(); -- fails: no self-admin in SECURITY DEFINER diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index c8c545b64c..2047be7d8b 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -981,7 +981,7 @@ SET ROLE regress_priv_group2; GRANT regress_priv_group2 TO regress_priv_user5; -- fails: SET ROLE did not help SET SESSION AUTHORIZATION regress_priv_group2; -GRANT regress_priv_group2 TO regress_priv_user5; -- ok: a role can self-admin +GRANT regress_priv_group2 TO regress_priv_user5; -- fail: a role cannot self-admin CREATE FUNCTION dogrant_fails() RETURNS void LANGUAGE sql SECURITY DEFINER AS 'GRANT regress_priv_group2 TO regress_priv_user5'; SELECT dogrant_fails(); -- fails: no self-admin in SECURITY DEFINER -- 2.21.1 (Apple Git-122.3)