diff --git a/doc/src/sgml/ref/grant.sgml b/doc/src/sgml/ref/grant.sgml
index f42d659518730529744e7b996b4c17a3196696b5..fc3552bc1de6f1fe78a7e157ed9236764924938b 100644
--- a/doc/src/sgml/ref/grant.sgml
+++ b/doc/src/sgml/ref/grant.sgml
@@ -395,11 +395,13 @@ GRANT <replaceable class="PARAMETER">role_name</replaceable> [, ...] TO <replace
<para>
If <literal>WITH ADMIN OPTION</literal> is specified, the member can
in turn grant membership in the role to others, and revoke membership
- in the role as well. Without the admin option, ordinary users cannot do
- that. However,
- database superusers can grant or revoke membership in any role to anyone.
- Roles having <literal>CREATEROLE</> privilege can grant or revoke
- membership in any role that is not a superuser.
+ in the role as well. Without the admin option, ordinary users cannot
+ do that. A role is not considered to hold <literal>WITH ADMIN
+ OPTION</literal> on itself, but it may grant or revoke membership in
+ itself from a database session where the session user matches the
+ role. Database superusers can grant or revoke membership in any role
+ to anyone. Roles having <literal>CREATEROLE</> privilege can grant
+ or revoke membership in any role that is not a superuser.
</para>
<para>
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index bcdc392a817525df11e5f7ae3182d53a1df7964c..7f5b8473d81c649916e4e094e8b73ad5dc20d8c7 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 10729d5e50627ef355ac2af32e61aac5dc7ae00d..dfac1243a40496e9e3df63298b04827d5eaa9116 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
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index fa574d744eeca92728d72eb6eb89cd78b9424f6f..1675075f6824e504e4274a62f3f95c7a5c69f610 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -32,7 +32,7 @@ ALTER GROUP regressgroup1 ADD USER regressuser4;
ALTER GROUP regressgroup2 ADD USER regressuser2; -- duplicate
NOTICE: role "regressuser2" is already a member of role "regressgroup2"
ALTER GROUP regressgroup2 DROP USER regressuser2;
-ALTER GROUP regressgroup2 ADD USER regressuser4;
+GRANT regressgroup2 TO regressuser4 WITH ADMIN OPTION;
-- test owner privileges
SET SESSION AUTHORIZATION regressuser1;
SELECT session_user, current_user;
@@ -948,6 +948,40 @@ SELECT has_table_privilege('regressuser1', 'atest4', 'SELECT WITH GRANT OPTION')
t
(1 row)
+-- Admin options
+SET SESSION AUTHORIZATION regressuser4;
+CREATE FUNCTION dogrant_ok() RETURNS void LANGUAGE sql SECURITY DEFINER AS
+ 'GRANT regressgroup2 TO regressuser5';
+GRANT regressgroup2 TO regressuser5; -- ok: had ADMIN OPTION
+SET ROLE regressgroup2;
+GRANT regressgroup2 TO regressuser5; -- fails: SET ROLE suspended privilege
+ERROR: must have admin option on role "regressgroup2"
+SET SESSION AUTHORIZATION regressuser1;
+GRANT regressgroup2 TO regressuser5; -- fails: no ADMIN OPTION
+ERROR: must have admin option on role "regressgroup2"
+SELECT dogrant_ok(); -- ok: SECURITY DEFINER conveys ADMIN
+NOTICE: role "regressuser5" is already a member of role "regressgroup2"
+CONTEXT: SQL function "dogrant_ok" statement 1
+ dogrant_ok
+------------
+
+(1 row)
+
+SET ROLE regressgroup2;
+GRANT regressgroup2 TO regressuser5; -- fails: SET ROLE did not help
+ERROR: must have admin option on role "regressgroup2"
+SET SESSION AUTHORIZATION regressgroup2;
+GRANT regressgroup2 TO regressuser5; -- ok: a role can self-admin
+NOTICE: role "regressuser5" is already a member of role "regressgroup2"
+CREATE FUNCTION dogrant_fails() RETURNS void LANGUAGE sql SECURITY DEFINER AS
+ 'GRANT regressgroup2 TO regressuser5';
+SELECT dogrant_fails(); -- fails: no self-admin in SECURITY DEFINER
+ERROR: must have admin option on role "regressgroup2"
+CONTEXT: SQL function "dogrant_fails" statement 1
+DROP FUNCTION dogrant_fails();
+SET SESSION AUTHORIZATION regressuser4;
+DROP FUNCTION dogrant_ok();
+REVOKE regressgroup2 FROM regressuser5;
-- has_sequence_privilege tests
\c -
CREATE SEQUENCE x_seq;
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index 38f8695475c48bc7275e418d9b72a38449fdb1b9..a0ff953c9041c904d3a1127976f4be81bdf6de6f 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -37,7 +37,7 @@ ALTER GROUP regressgroup1 ADD USER regressuser4;
ALTER GROUP regressgroup2 ADD USER regressuser2; -- duplicate
ALTER GROUP regressgroup2 DROP USER regressuser2;
-ALTER GROUP regressgroup2 ADD USER regressuser4;
+GRANT regressgroup2 TO regressuser4 WITH ADMIN OPTION;
-- test owner privileges
@@ -599,6 +599,33 @@ SELECT has_table_privilege('regressuser3', 'atest4', 'SELECT'); -- false
SELECT has_table_privilege('regressuser1', 'atest4', 'SELECT WITH GRANT OPTION'); -- true
+-- Admin options
+
+SET SESSION AUTHORIZATION regressuser4;
+CREATE FUNCTION dogrant_ok() RETURNS void LANGUAGE sql SECURITY DEFINER AS
+ 'GRANT regressgroup2 TO regressuser5';
+GRANT regressgroup2 TO regressuser5; -- ok: had ADMIN OPTION
+SET ROLE regressgroup2;
+GRANT regressgroup2 TO regressuser5; -- fails: SET ROLE suspended privilege
+
+SET SESSION AUTHORIZATION regressuser1;
+GRANT regressgroup2 TO regressuser5; -- fails: no ADMIN OPTION
+SELECT dogrant_ok(); -- ok: SECURITY DEFINER conveys ADMIN
+SET ROLE regressgroup2;
+GRANT regressgroup2 TO regressuser5; -- fails: SET ROLE did not help
+
+SET SESSION AUTHORIZATION regressgroup2;
+GRANT regressgroup2 TO regressuser5; -- ok: a role can self-admin
+CREATE FUNCTION dogrant_fails() RETURNS void LANGUAGE sql SECURITY DEFINER AS
+ 'GRANT regressgroup2 TO regressuser5';
+SELECT dogrant_fails(); -- fails: no self-admin in SECURITY DEFINER
+DROP FUNCTION dogrant_fails();
+
+SET SESSION AUTHORIZATION regressuser4;
+DROP FUNCTION dogrant_ok();
+REVOKE regressgroup2 FROM regressuser5;
+
+
-- has_sequence_privilege tests
\c -