Restrict the privileges of CREATEROLE users.

robertmhaas · robertmhaas · commit cf5eb37c5ee0 · 2023-01-10T12:44:30.000-05:00
Previously, CREATEROLE users were permitted to make nearly arbitrary changes to roles that they didn't create, with certain exceptions, particularly superuser roles. Instead, allow CREATEROLE users to make such changes to roles for which they possess ADMIN OPTION, and to grant membership only in roles for which they possess ADMIN OPTION. When a CREATEROLE user who is not a superuser creates a role, grant ADMIN OPTION on the newly-created role to the creator, so that they can administer roles they create or for which they have been given privileges. With these changes, CREATEROLE users still have very significant powers that unprivileged users do not receive: they can alter, rename, drop, comment on, change the password for, and change security labels on roles. However, they can now do these things only for roles for which they possess appropriate privileges, rather than all non-superuser roles; moreover, they cannot grant a role such as pg_execute_server_program unless they themselves possess it. Patch by me, reviewed by Mark Dilger. Discussion: https://postgr.es/m/CA+TgmobN59ct+Emmz6ig1Nua2Q-_o=r6DSD98KfU53kctq_kQw@mail.gmail.com
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
@@ -3216,13 +3216,11 @@ REVOKE CREATE ON SCHEMA public FROM PUBLIC;
        name. Therefore, if each user has a separate schema, they access
        their own schemas by default.)  This pattern is a secure schema
        usage pattern unless an untrusted user is the database owner or
-       holds the <literal>CREATEROLE</literal> privilege, in which case no
-       secure schema usage pattern exists.
+       has been granted <literal>ADMIN OPTION</literal> on a relevant role,
+       in which case no secure schema usage pattern exists.
       </para>
       <!-- A database owner can attack the database's users via "CREATE SCHEMA
-           trojan; ALTER DATABASE $mydb SET search_path = trojan, public;".  A
-           CREATEROLE user can issue "GRANT $dbowner TO $me" and then use the
-           database owner attack. -->
+           trojan; ALTER DATABASE $mydb SET search_path = trojan, public;". -->
 
       <para>
        In <productname>PostgreSQL</productname> 15 and later, the default
@@ -3250,7 +3248,7 @@ REVOKE CREATE ON SCHEMA public FROM PUBLIC;
        unreliable</link>.  If you create functions or extensions in the public
        schema, use the first pattern instead.  Otherwise, like the first
        pattern, this is secure unless an untrusted user is the database owner
-       or holds the <literal>CREATEROLE</literal> privilege.
+       or has been granted <literal>ADMIN OPTION</literal> on a relevant role.
       </para>
      </listitem>
 
diff --git a/doc/src/sgml/ref/alter_role.sgml b/doc/src/sgml/ref/alter_role.sgml
@@ -73,15 +73,16 @@ ALTER ROLE { <replaceable class="parameter">role_specification</replaceable> | A
    Roles having <literal>CREATEROLE</literal> privilege can change any of these
    settings except <literal>SUPERUSER</literal>, <literal>REPLICATION</literal>,
    and <literal>BYPASSRLS</literal>; but only for non-superuser and
-   non-replication roles.
+   non-replication roles for which they have been
+   granted <literal>ADMIN OPTION</literal>.
    Ordinary roles can only change their own password.
   </para>
 
   <para>
    The second variant changes the name of the role.
    Database superusers can rename any role.
    Roles having <literal>CREATEROLE</literal> privilege can rename non-superuser
-   roles.
+   roles for which they have been granted <literal>ADMIN OPTION</literal>.
    The current session user cannot be renamed.
    (Connect as a different user if you need to do that.)
    Because <literal>MD5</literal>-encrypted passwords use the role name as
@@ -116,7 +117,8 @@ ALTER ROLE { <replaceable class="parameter">role_specification</replaceable> | A
   <para>
    Superusers can change anyone's session defaults. Roles having
    <literal>CREATEROLE</literal> privilege can change defaults for non-superuser
-   roles. Ordinary roles can only set defaults for themselves.
+   roles for which they have been granted <literal>ADMIN OPTION</literal>.
+   Ordinary roles can only set defaults for themselves.
    Certain configuration variables cannot be set this way, or can only be
    set if a superuser issues the command.  Only superusers can change a setting
    for all roles in all databases.
diff --git a/doc/src/sgml/ref/comment.sgml b/doc/src/sgml/ref/comment.sgml
@@ -99,7 +99,8 @@ COMMENT ON
    For most kinds of object, only the object's owner can set the comment.
    Roles don't have owners, so the rule for <literal>COMMENT ON ROLE</literal> is
    that you must be superuser to comment on a superuser role, or have the
-   <literal>CREATEROLE</literal> privilege to comment on non-superuser roles.
+   <literal>CREATEROLE</literal> privilege and have been granted
+   <literal>ADMIN OPTION</literal> on the target role.
    Likewise, access methods don't have owners either; you must be superuser
    to comment on an access method.
    Of course, a superuser can comment on anything.
diff --git a/doc/src/sgml/ref/create_role.sgml b/doc/src/sgml/ref/create_role.sgml
@@ -119,8 +119,8 @@ in sync when changing the above synopsis!
       <listitem>
        <para>
         These clauses determine whether a role will be permitted to
-        create, alter, drop, comment on, change the security label for,
-        and grant or revoke membership in other roles.
+        create, alter, drop, comment on, and change the security label for
+        other roles.
         See <xref linkend='role-creation' /> for more details about what
         capabilities are conferred by this privilege.
         If not specified, <literal>NOCREATEROLE</literal> is the default.
diff --git a/doc/src/sgml/ref/createuser.sgml b/doc/src/sgml/ref/createuser.sgml
@@ -252,8 +252,7 @@ PostgreSQL documentation
       <listitem>
        <para>
         The new user will be allowed to create, alter, drop, comment on,
-        change the security label for, and grant or revoke membership in
-        other roles; that is,
+        change the security label for other roles; that is,
         this user will have <literal>CREATEROLE</literal> privilege.
         See <xref linkend='role-creation' /> for more details about what
         capabilities are conferred by this privilege.
diff --git a/doc/src/sgml/ref/drop_role.sgml b/doc/src/sgml/ref/drop_role.sgml
@@ -32,7 +32,7 @@ DROP ROLE [ IF EXISTS ] <replaceable class="parameter">name</replaceable> [, ...
    <command>DROP ROLE</command> removes the specified role(s).
    To drop a superuser role, you must be a superuser yourself;
    to drop non-superuser roles, you must have <literal>CREATEROLE</literal>
-   privilege.
+   privilege and have been granted <literal>ADMIN OPTION</literal> on the role.
   </para>
 
   <para>
diff --git a/doc/src/sgml/ref/dropuser.sgml b/doc/src/sgml/ref/dropuser.sgml
@@ -35,9 +35,10 @@ PostgreSQL documentation
   <para>
    <application>dropuser</application> removes an existing
    <productname>PostgreSQL</productname> user.
-   Only superusers and users with the <literal>CREATEROLE</literal> privilege can
-   remove <productname>PostgreSQL</productname> users.  (To remove a
-   superuser, you must yourself be a superuser.)
+   Superusers can use this command to remove any role; otherwise, only
+   non-superuser roles can be removed, and only by a user who possesses
+   the <literal>CREATEROLE</literal> privilege and has been granted
+   <literal>ADMIN OPTION</literal> on the target role.
   </para>
 
   <para>
diff --git a/doc/src/sgml/ref/grant.sgml b/doc/src/sgml/ref/grant.sgml
@@ -271,9 +271,7 @@ GRANT <replaceable class="parameter">role_name</replaceable> [, ...] TO <replace
    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.  Database superusers can grant or revoke
-   membership in any role to anyone.  Roles having
-   <literal>CREATEROLE</literal> privilege can grant or revoke membership
-   in any role that is not a superuser. This option defaults to
+   membership in any role to anyone. This option defaults to
    <literal>FALSE</literal>.
   </para>
 
diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml
@@ -199,7 +199,12 @@ CREATE USER <replaceable>name</replaceable>;
         checks). To create such a role, use <literal>CREATE ROLE
         <replaceable>name</replaceable> CREATEROLE</literal>.
         A role with <literal>CREATEROLE</literal> privilege can alter and drop
-        other roles, too, as well as grant or revoke membership in them.
+        roles which have been granted to the <literal>CREATEROLE</literal>
+        user with the <literal>ADMIN</literal> option. Such a grant occurs
+        automatically when a <literal>CREATEROLE</literal> user that is not
+        a superuser creates a new role, so that by default, a
+        <literal>CREATEROLE</literal> user can alter and drop the roles
+        which they have created.
         Altering a role includes most changes that can be made using
         <literal>ALTER ROLE</literal>, including, for example, changing
         passwords.  It also includes modifications to a role that can
@@ -224,15 +229,6 @@ CREATE USER <replaceable>name</replaceable>;
         confer the ability to grant or revoke the <literal>BYPASSRLS</literal>
         privilege.
        </para>
-       <para>
-        Because the <literal>CREATEROLE</literal> privilege allows a user
-        to grant or revoke membership even in roles to which it does not (yet)
-        have any access, a <literal>CREATEROLE</literal> user can obtain access
-        to the capabilities of every predefined role in the system, including
-        highly privileged roles such as
-        <literal>pg_execute_server_program</literal> and
-        <literal>pg_write_server_files</literal>.
-       </para>
       </listitem>
      </varlistentry>
 
@@ -329,6 +325,34 @@ ALTER ROLE myname SET enable_indexscan TO off;
    <literal>LOGIN</literal> privilege are fairly useless, since they will never
    be invoked.
   </para>
+
+  <para>
+   When a non-superuser creates a role using the <literal>CREATEROLE</literal>
+   privilege, the created role is automatically granted back to the creating
+   user, just as if the bootstrap superuser had executed the command
+   <literal>GRANT created_user TO creating_user WITH ADMIN TRUE, SET FALSE,
+   INHERIT FALSE</literal>. Since a <literal>CREATEROLE</literal> user can
+   only exercise special privileges with regard to an existing role if they
+   have <literal>ADMIN OPTION</literal> on it, this grant is just sufficient
+   to allow a <literal>CREATEROLE</literal> user to administer the roles they
+   created. However, because it is created with <literal>INHERIT FALSE, SET
+   FALSE</literal>, the <literal>CREATEROLE</literal> user doesn't inherit the
+   privileges of the created role, nor can it access the privileges of that
+   role using <literal>SET ROLE</literal>. However, since any user who has
+   <literal>ADMIN OPTION</literal> on a role can grant membership in that
+   role to any other user, the <literal>CREATEROLE</literal> user can gain
+   access to the created role by simplying granting that role back to
+   themselves with the <literal>INHERIT</literal> and/or <literal>SET</literal>
+   options. Thus, the fact that privileges are not inherited by default nor
+   is <literal>SET ROLE</literal> granted by default is a safeguard against
+   accidents, not a security feature. Also note that, because this automatic
+   grant is granted by the bootstrap user, it cannot be removed or changed by
+   the <literal>CREATEROLE</literal> user; however, any superuser could
+   revoke it, modify it, and/or issue additional such grants to other
+   <literal>CREATEROLE</literal> users. Whichever <literal>CREATEROLE</literal>
+   users have <literal>ADMIN OPTION</literal> on a role at any given time
+   can administer it.
+  </para>
  </sect1>
 
  <sect1 id="role-membership">
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
@@ -2538,7 +2538,9 @@ check_object_ownership(Oid roleid, ObjectType objtype, ObjectAddress address,
 
 			/*
 			 * We treat roles as being "owned" by those with CREATEROLE priv,
-			 * except that superusers are only owned by superusers.
+			 * provided that they also have admin option on the role.
+			 *
+			 * However, superusers are only owned by superusers.
 			 */
 			if (superuser_arg(address.objectId))
 			{
@@ -2553,6 +2555,12 @@ check_object_ownership(Oid roleid, ObjectType objtype, ObjectAddress address,
 					ereport(ERROR,
 							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 							 errmsg("must have CREATEROLE privilege")));
+				if (!is_admin_of_role(roleid, address.objectId))
+					ereport(ERROR,
+							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+							 errmsg("must have admin option on role \"%s\"",
+									GetUserNameFromId(address.objectId,
+													  true))));
 			}
 			break;
 		case OBJECT_TSPARSER:
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
diff --git a/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out b/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
diff --git a/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql b/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql
diff --git a/src/test/regress/expected/create_role.out b/src/test/regress/expected/create_role.out
diff --git a/src/test/regress/sql/create_role.sql b/src/test/regress/sql/create_role.sql
