diff options
Diffstat (limited to 'src/backend')
| -rw-r--r-- | src/backend/commands/user.c | 11 | ||||
| -rw-r--r-- | src/backend/utils/adt/acl.c | 50 |
2 files changed, 50 insertions, 11 deletions
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index bcdc392a817..7f5b8473d81 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -1366,7 +1366,16 @@ AddRoleMems(const char *rolename, Oid roleid, rolename))); } - /* XXX not sure about this check */ + /* + * The role membership grantor of record has little significance at + * present. Nonetheless, inasmuch as users might look to it for a crude + * audit trail, let only superusers impute the grant to a third party. + * + * Before lifting this restriction, give the member == role case of + * is_admin_of_role() a fresh look. Ensure that the current role cannot + * use an explicit grantor specification to take advantage of the session + * user's self-admin right. + */ if (grantorId != GetUserId() && !superuser()) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c index 10729d5e506..dfac1243a40 100644 --- a/src/backend/utils/adt/acl.c +++ b/src/backend/utils/adt/acl.c @@ -4582,6 +4582,11 @@ pg_role_aclcheck(Oid role_oid, Oid roleid, AclMode mode) { if (mode & ACL_GRANT_OPTION_FOR(ACL_CREATE)) { + /* + * XXX For roleid == role_oid, is_admin_of_role() also examines the + * session and call stack. That suits two-argument pg_has_role(), but + * it gives the three-argument version a lamentable whimsy. + */ if (is_admin_of_role(roleid, role_oid)) return ACLCHECK_OK; } @@ -4899,11 +4904,9 @@ is_member_of_role_nosuper(Oid member, Oid role) /* - * Is member an admin of role (directly or indirectly)? That is, is it - * a member WITH ADMIN OPTION? - * - * We could cache the result as for is_member_of_role, but currently this - * is not used in any performance-critical paths, so we don't. + * Is member an admin of role? That is, is member the role itself (subject to + * restrictions below), a member (directly or indirectly) WITH ADMIN OPTION, + * or a superuser? */ bool is_admin_of_role(Oid member, Oid role) @@ -4912,14 +4915,41 @@ is_admin_of_role(Oid member, Oid role) List *roles_list; ListCell *l; - /* Fast path for simple case */ - if (member == role) - return true; - - /* Superusers have every privilege, so are part of every 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(); + /* * Find all the roles that member is a member of, including multi-level * recursion. We build a list in the same way that is_member_of_role does |