v6-0004-Restrict-power-granted-via-CREATEROLE.patch

From d949f35209b8de4723b0612ef823e0385ae6a338 Mon Sep 17 00:00:00 2001
From: Mark Dilger <mark.dilger@enterprisedb.com>
Date: Tue, 18 Jan 2022 10:24:55 -0800
Subject: [PATCH v6 4/5] Restrict power granted via CREATEROLE.

The CREATEROLE attribute no longer has anything to do with the power
to alter roles or to grant or revoke role membership, but merely the
ability to create new roles, as its name suggests.  The ability to
alter a role is based on role ownership; the ability to grant and
revoke role membership is based on having admin privilege on the
relevant role or alternatively on role ownership, as owners now
implicitly have admin privileges on roles they own.

A role must either be superuser or have the CREATEROLE attribute to
create roles.  This is unchanged from the prior behavior.  A new
principle is adopted, though, to make CREATEROLE less dangerous:  a
role may not create new roles with privileges that the creating role
lacks.  This new principle is intended to prevent privilege
escalation attacks stemming from giving CREATEROLE to a user.  This
is not backwards compatible.  The idea is to fix the CREATEROLE
privilege to not be pathway to gaining superuser, and no
non-breaking change to accomplish that is apparent.

SUPERUSER, REPLICATION, BYPASSRLS, CREATEDB, CREATEROLE and LOGIN
privilege can only be given to new roles by creators who have the
same privilege.  In the case of the CREATEROLE privilege, this is
trivially true, as the creator must necessarily have it or they
couldn't be creating the role to begin with.

The INHERIT attribute is not considered a privilege, and since a
user who belongs to a role may SET ROLE to that role and do anything
that role can do, it isn't clear that treating it as a privilege
would stop any privilege escalation attacks.

The CONNECTION LIMIT and VALID UNTIL attributes are also not
considered privileges, but this design choice is debatable.  One
could think of the ability to log in during a given window of time,
or up to a certain number of connections as a privilege, and
allowing such a restricted role to create a new role with unlimited
connections or no expiration as a privilege escalation which escapes
the intended restrictions.  However, it is just as easy to think of
these limitations as being used to guard against badly written
client programs connecting too many times, or connecting at a time
of day that is not intended.  Since it is unclear which design is
better, this commit is conservative and the handling of these
attributes is unchanged relative to prior behavior.

Since the grammar of the CREATE ROLE command allows specifying roles
into which the new role should be enrolled, and also lists of roles
which become members of the newly created role (as admin or not),
the CREATE ROLE command may now fail if the creating role has
insufficient privilege on the roles so listed.  Such failures were
not possible before, since the CREATEROLE privilege was always
sufficient.
---
 doc/src/sgml/ddl.sgml                     |  12 +--
 doc/src/sgml/ref/alter_role.sgml          |  20 ++--
 doc/src/sgml/ref/comment.sgml             |   8 +-
 doc/src/sgml/ref/create_role.sgml         |  26 +++--
 doc/src/sgml/ref/drop_role.sgml           |   3 +-
 doc/src/sgml/ref/dropuser.sgml            |   6 +-
 doc/src/sgml/ref/grant.sgml               |   4 +-
 doc/src/sgml/user-manag.sgml              |  44 +++++----
 src/backend/catalog/aclchk.c              | 111 ++++++++++++++++++++++
 src/backend/commands/user.c               |  60 +++++-------
 src/backend/utils/adt/acl.c               |  21 +---
 src/include/utils/acl.h                   |   5 +
 src/test/regress/expected/create_role.out |  63 +++++-------
 src/test/regress/sql/create_role.sql      |  36 +++----
 14 files changed, 250 insertions(+), 169 deletions(-)

diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 22f6c5c7ab..5596a359e3 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -3132,9 +3132,7 @@ REVOKE CREATE ON SCHEMA public FROM PUBLIC;
            doesn't preserve that DROP.
 
            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>
        Constrain ordinary users to user-private schemas.  To implement this,
        first issue <literal>REVOKE CREATE ON SCHEMA public FROM
@@ -3146,9 +3144,8 @@ REVOKE CREATE ON SCHEMA public FROM PUBLIC;
        pattern in a database where untrusted users had already logged in,
        consider auditing the public schema for objects named like objects in
        schema <literal>pg_catalog</literal>.  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.
+       usage pattern unless an untrusted user is the database owner, in which
+       case no secure schema usage pattern exists.
       </para>
       <para>
        If the database originated in an upgrade
@@ -3170,8 +3167,7 @@ REVOKE CREATE ON SCHEMA public FROM PUBLIC;
        schema <link linkend="typeconv-func">will be unsafe or
        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.
+       pattern, this is secure unless an untrusted user is the database owner.
       </para>
      </listitem>
 
diff --git a/doc/src/sgml/ref/alter_role.sgml b/doc/src/sgml/ref/alter_role.sgml
index 5aa5648ae7..fc9bea8072 100644
--- a/doc/src/sgml/ref/alter_role.sgml
+++ b/doc/src/sgml/ref/alter_role.sgml
@@ -70,18 +70,18 @@ ALTER ROLE { <replaceable class="parameter">role_specification</replaceable> | A
    <link linkend="sql-revoke"><command>REVOKE</command></link> for that.)
    Attributes not mentioned in the command retain their previous settings.
    Database superusers can change any of these settings for any role.
-   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.
-   Ordinary roles can only change their own password.
+   Role owners can change any of these settings on roles they directly or
+   indirectly own except <literal>SUPERUSER</literal>,
+   <literal>REPLICATION</literal>, and <literal>BYPASSRLS</literal>; but only
+   for non-superuser and non-replication roles, and only if the role owner does
+   not alter the target role to have a privilege which the role owner itself
+   lacks.  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 can rename non-superuser roles they directly or indirectly own.
    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
@@ -114,9 +114,9 @@ ALTER ROLE { <replaceable class="parameter">role_specification</replaceable> | A
   </para>
 
   <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.
+   Superusers can change anyone's session defaults. Roles may change
+   privilege for non-superuser roles they directly or indirectly own. 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
index e07fc47fd3..1ba374f3a1 100644
--- a/doc/src/sgml/ref/comment.sgml
+++ b/doc/src/sgml/ref/comment.sgml
@@ -92,12 +92,8 @@ COMMENT ON
 
   <para>
    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.
-   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.
+   Access methods don't have owners; you must be superuser to comment on an
+   access method.  Of course, a superuser can comment on anything.
   </para>
 
   <para>
diff --git a/doc/src/sgml/ref/create_role.sgml b/doc/src/sgml/ref/create_role.sgml
index b6a4ea1f72..2e73102562 100644
--- a/doc/src/sgml/ref/create_role.sgml
+++ b/doc/src/sgml/ref/create_role.sgml
@@ -107,8 +107,10 @@ in sync when changing the above synopsis!
         <literal>CREATEDB</literal> is specified, the role being
         defined will be allowed to create new databases. Specifying
         <literal>NOCREATEDB</literal> will deny a role the ability to
-        create databases. If not specified,
-        <literal>NOCREATEDB</literal> is the default.
+        create databases. Only roles with the <literal>CREATEDB</literal>
+        attribute may create roles with the <literal>CREATEDB</literal>
+        attribute. If not specified, <literal>NOCREATEDB</literal> is the
+        default.
        </para>
       </listitem>
      </varlistentry>
@@ -120,8 +122,6 @@ in sync when changing the above synopsis!
        <para>
         These clauses determine whether a role will be permitted to
         create new roles (that is, execute <command>CREATE ROLE</command>).
-        A role with <literal>CREATEROLE</literal> privilege can also alter
-        and drop other roles.
         If not specified,
         <literal>NOCREATEROLE</literal> is the default.
        </para>
@@ -163,6 +163,8 @@ in sync when changing the above synopsis!
         <literal>NOLOGIN</literal> is the default, except when
         <command>CREATE ROLE</command> is invoked through its alternative spelling
         <link linkend="sql-createuser"><command>CREATE USER</command></link>.
+        You must have the <literal>LOGIN</literal> attribute to create a new role
+        with the <literal>LOGIN</literal> attribute.
        </para>
       </listitem>
      </varlistentry>
@@ -194,8 +196,8 @@ in sync when changing the above synopsis!
        <para>
         These clauses determine whether a role bypasses every row-level
         security (RLS) policy.  <literal>NOBYPASSRLS</literal> is the default.
-        You must be a superuser to create a new role having
-        the <literal>BYPASSRLS</literal> attribute.
+        You must have the <literal>BYPASSRLS</literal> attribute to create a
+        new role having the <literal>BYPASSRLS</literal> attribute.
        </para>
 
        <para>
@@ -281,6 +283,10 @@ in sync when changing the above synopsis!
         member.  (Note that there is no option to add the new role as an
         administrator; use a separate <command>GRANT</command> command to do that.)
        </para>
+       <para>
+        If not a superuser, the creating role must either own or have admin
+        privilege on each listed role.
+       </para>
       </listitem>
      </varlistentry>
 
@@ -301,6 +307,10 @@ in sync when changing the above synopsis!
         roles which are automatically added as members of the new role.
         (This in effect makes the new role a <quote>group</quote>.)
        </para>
+       <para>
+        If not a superuser, the creating role must either own or have admin
+        privilege on each listed role.
+       </para>
       </listitem>
      </varlistentry>
 
@@ -313,6 +323,10 @@ in sync when changing the above synopsis!
         OPTION</literal>, giving them the right to grant membership in this role
         to others.
        </para>
+       <para>
+        If not a superuser, the creating role must either own or have admin
+        privilege on each listed role.
+       </para>
       </listitem>
      </varlistentry>
 
diff --git a/doc/src/sgml/ref/drop_role.sgml b/doc/src/sgml/ref/drop_role.sgml
index 13dc1cc649..c3d57ee8db 100644
--- a/doc/src/sgml/ref/drop_role.sgml
+++ b/doc/src/sgml/ref/drop_role.sgml
@@ -31,8 +31,7 @@ DROP ROLE [ IF EXISTS ] <replaceable class="parameter">name</replaceable> [, ...
   <para>
    <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.
+   to drop non-superuser roles, you must own the target role.
   </para>
 
   <para>
diff --git a/doc/src/sgml/ref/dropuser.sgml b/doc/src/sgml/ref/dropuser.sgml
index 81580507e8..30a99eaf68 100644
--- a/doc/src/sgml/ref/dropuser.sgml
+++ b/doc/src/sgml/ref/dropuser.sgml
@@ -35,9 +35,9 @@ 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.)
+   A <productname>PostgreSQL</productname> user may only be removed by its
+   owner or by a superuser.  (To remove a superuser, you must yourself be a
+   superuser.)
   </para>
 
   <para>
diff --git a/doc/src/sgml/ref/grant.sgml b/doc/src/sgml/ref/grant.sgml
index a897712de2..86fc387af2 100644
--- a/doc/src/sgml/ref/grant.sgml
+++ b/doc/src/sgml/ref/grant.sgml
@@ -254,8 +254,8 @@ GRANT <replaceable class="parameter">role_name</replaceable> [, ...] TO <replace
    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</literal> privilege can grant
-   or revoke membership in any role that is not a superuser.
+   to anyone.  Roles can revoke membership in any role they own, and 
+   may grant membership in any role they own to any role they own.
   </para>
 
   <para>
diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml
index 9067be1d9c..e65b55a004 100644
--- a/doc/src/sgml/user-manag.sgml
+++ b/doc/src/sgml/user-manag.sgml
@@ -198,9 +198,10 @@ CREATE USER <replaceable>name</replaceable>;
         (except for superusers, since those bypass all permission
         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.
-        However, to create, alter, drop, or change membership of a
+        A role which creates a new role becomes the new role's owner, able to
+        alter or drop that new role, and sharing ownership of any additional
+        objects (including additional roles) that new role creates.
+        To create, alter, drop, or change membership of a
         superuser role, superuser status is required;
         <literal>CREATEROLE</literal> is insufficient for that.
        </para>
@@ -246,11 +247,14 @@ CREATE USER <replaceable>name</replaceable>;
 
   <tip>
    <para>
-    It is good practice to create a role that has the <literal>CREATEDB</literal>
-    and <literal>CREATEROLE</literal> privileges, but is not a superuser, and then
+    It is good practice to create a role that has the
+    <literal>CREATEDB</literal>, <literal>LOGIN</literal> and
+    <literal>CREATEROLE</literal> privileges, but is not a superuser, and then
     use this role for all routine management of databases and roles.  This
-    approach avoids the dangers of operating as a superuser for tasks that
-    do not really require it.
+    approach avoids the dangers of operating as a superuser for tasks that do
+    not really require it.  This role must also have
+    <literal>REPLICATION</literal> if it will create replication users, and
+    must have <literal>BYPASSRLS</literal> if it will create bypassrls users.
    </para>
   </tip>
 
@@ -387,15 +391,22 @@ RESET ROLE;
 
   <para>
    The role attributes <literal>LOGIN</literal>, <literal>SUPERUSER</literal>,
-   <literal>CREATEDB</literal>, and <literal>CREATEROLE</literal> can be thought of as
-   special privileges, but they are never inherited as ordinary privileges
-   on database objects are.  You must actually <command>SET ROLE</command> to a
-   specific role having one of these attributes in order to make use of
-   the attribute.  Continuing the above example, we might choose to
+   <literal>CREATEDB</literal>, <literal>REPLICATION</literal>,
+   <literal>BYPASSRLS</literal>, and <literal>CREATEROLE</literal> can be
+   thought of as special privileges, but they are never inherited as ordinary
+   privileges on database objects are.  You must actually <command>SET
+   ROLE</command> to a specific role having one of these attributes in order to
+   make use of the attribute.  Continuing the above example, we might choose to
    grant <literal>CREATEDB</literal> and <literal>CREATEROLE</literal> to the
-   <literal>admin</literal> role.  Then a session connecting as role <literal>joe</literal>
-   would not have these privileges immediately, only after doing
-   <command>SET ROLE admin</command>.
+   <literal>admin</literal> role.  Then a session connecting as role
+   <literal>joe</literal> would not have these privileges immediately, only
+   after doing <command>SET ROLE admin</command>.  Roles with these attributes
+   may only be created by roles which themselves have these attributes.
+   Superusers may always do so, but non-superuser roles with
+   <literal>CREATEROLE</literal> may only create new roles with
+   <literal>LOGIN</literal>, <literal>CREATEDB</literal>,
+   <literal>REPLICATION</literal>, or <literal>BYPASSRLS</literal> if they
+   themselves have the same attribute.
   </para>
 
   <para>
@@ -493,8 +504,7 @@ DROP ROLE doomed_role;
   <para>
    <productname>PostgreSQL</productname> provides a set of predefined roles
    that provide access to certain, commonly needed, privileged capabilities
-   and information.  Administrators (including roles that have the
-   <literal>CREATEROLE</literal> privilege) can <command>GRANT</command> these
+   and information.  Administrators can <command>GRANT</command> these
    roles to users and/or other roles in their environment, providing those
    users with access to the specified capabilities and information.
   </para>
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 1e0ee503e4..8327005404 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -5470,6 +5470,9 @@ has_createrole_privilege(Oid roleid)
 	return result;
 }
 
+/*
+ * Check whether specified role has BYPASSRLS privilege (or is a superuser)
+ */
 bool
 has_bypassrls_privilege(Oid roleid)
 {
@@ -5489,6 +5492,114 @@ has_bypassrls_privilege(Oid roleid)
 	return result;
 }
 
+/*
+ * Check whether specified role has INHERIT privilege (or is a superuser)
+ */
+bool
+has_inherit_privilege(Oid roleid)
+{
+	bool		result = false;
+	HeapTuple	utup;
+
+	/* Superusers bypass all permission checking. */
+	if (superuser_arg(roleid))
+		return true;
+
+	utup = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
+	if (HeapTupleIsValid(utup))
+	{
+		result = ((Form_pg_authid) GETSTRUCT(utup))->rolinherit;
+		ReleaseSysCache(utup);
+	}
+	return result;
+}
+
+/*
+ * Check whether specified role has CREATEDB privilege (or is a superuser)
+ */
+bool
+has_createdb_privilege(Oid roleid)
+{
+	bool		result = false;
+	HeapTuple	utup;
+
+	/* Superusers bypass all permission checking. */
+	if (superuser_arg(roleid))
+		return true;
+
+	utup = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
+	if (HeapTupleIsValid(utup))
+	{
+		result = ((Form_pg_authid) GETSTRUCT(utup))->rolcreatedb;
+		ReleaseSysCache(utup);
+	}
+	return result;
+}
+
+/*
+ * Check whether specified role has LOGIN privilege (or is a superuser)
+ */
+bool
+has_login_privilege(Oid roleid)
+{
+	bool		result = false;
+	HeapTuple	utup;
+
+	/* Superusers bypass all permission checking. */
+	if (superuser_arg(roleid))
+		return true;
+
+	utup = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
+	if (HeapTupleIsValid(utup))
+	{
+		result = ((Form_pg_authid) GETSTRUCT(utup))->rolcanlogin;
+		ReleaseSysCache(utup);
+	}
+	return result;
+}
+
+/*
+ * Check whether specified role has REPLICATION privilege (or is a superuser)
+ */
+bool
+has_replication_privilege(Oid roleid)
+{
+	bool		result = false;
+	HeapTuple	utup;
+
+	/* Superusers bypass all permission checking. */
+	if (superuser_arg(roleid))
+		return true;
+
+	utup = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
+	if (HeapTupleIsValid(utup))
+	{
+		result = ((Form_pg_authid) GETSTRUCT(utup))->rolreplication;
+		ReleaseSysCache(utup);
+	}
+	return result;
+}
+
+/*
+ * Get the connection limit for the specified role.
+ *
+ * Returns -1 if the role has no connection limit.
+ */
+int32
+role_connection_limit(Oid roleid)
+{
+	int32		result = -1;
+	HeapTuple	utup;
+
+	utup = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
+	if (HeapTupleIsValid(utup))
+	{
+		result = ((Form_pg_authid) GETSTRUCT(utup))->rolconnlimit;
+		ReleaseSysCache(utup);
+	}
+	return result;
+}
+
 /*
  * Fetch pg_default_acl entry for given role, namespace and object type
  * (object type must be given in pg_default_acl's encoding).
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 3a24934679..cc33277c20 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -58,15 +58,6 @@ static void DelRoleMems(const char *rolename, Oid roleid,
 static void AlterRoleOwner_internal(HeapTuple tup, Relation rel,
 									Oid newOwnerId);
 
-
-/* Check if current user has createrole privileges */
-static bool
-have_createrole_privilege(void)
-{
-	return has_createrole_privilege(GetUserId());
-}
-
-
 /*
  * CREATE ROLE
  */
@@ -276,24 +267,32 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
 	}
 	else if (isreplication)
 	{
-		if (!superuser())
+		if (!has_replication_privilege(GetUserId()))
 			ereport(ERROR,
 					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-					 errmsg("must be superuser to create replication users")));
+					 errmsg("must have replication privilege to create replication users")));
 	}
 	else if (bypassrls)
 	{
-		if (!superuser())
+		if (!has_bypassrls_privilege(GetUserId()))
 			ereport(ERROR,
 					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-					 errmsg("must be superuser to create bypassrls users")));
+					 errmsg("must have bypassrls privilege to create bypassrls users")));
 	}
-	else
+	else if (!superuser())
 	{
-		if (!have_createrole_privilege())
+		if (!has_createrole_privilege(GetUserId()))
 			ereport(ERROR,
 					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 					 errmsg("permission denied to create role")));
+		if (createdb && !has_createdb_privilege(GetUserId()))
+			ereport(ERROR,
+					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+					 errmsg("must have createdb privilege to create createdb users")));
+		if (canlogin && !has_login_privilege(GetUserId()))
+			ereport(ERROR,
+					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+					 errmsg("must have login privilege to create login users")));
 	}
 
 	/*
@@ -689,7 +688,7 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
 					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 					 errmsg("must be superuser to change bypassrls attribute")));
 	}
-	else if (!have_createrole_privilege())
+	else if (!superuser())
 	{
 		/* check the rest */
 		if (dinherit || dcreaterole || dcreatedb || dcanlogin || dconnlimit ||
@@ -888,7 +887,7 @@ AlterRoleSet(AlterRoleSetStmt *stmt)
 
 		/*
 		 * To mess with a superuser you gotta be superuser; else you need
-		 * createrole, or just want to change your own settings
+		 * to own the role, or just want to change your own settings
 		 */
 		if (roleform->rolsuper)
 		{
@@ -899,8 +898,7 @@ AlterRoleSet(AlterRoleSetStmt *stmt)
 		}
 		else
 		{
-			if (!have_createrole_privilege() &&
-				!pg_role_ownercheck(roleid, GetUserId()))
+			if (!pg_role_ownercheck(roleid, GetUserId()))
 				ereport(ERROR,
 						(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 						 errmsg("permission denied")));
@@ -1013,18 +1011,12 @@ DropRole(DropRoleStmt *stmt)
 					(errcode(ERRCODE_OBJECT_IN_USE),
 					 errmsg("session user cannot be dropped")));
 
-		/*
-		 * For safety's sake, we allow createrole holders to drop ordinary
-		 * roles but not superuser roles.  This is mainly to avoid the
-		 * scenario where you accidentally drop the last superuser.
-		 */
 		if (roleform->rolsuper && !superuser())
 			ereport(ERROR,
 					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 					 errmsg("must be superuser to drop superusers")));
 
-		if (!have_createrole_privilege() &&
-			!pg_role_ownercheck(roleid, GetUserId()))
+		if (!pg_role_ownercheck(roleid, GetUserId()))
 			ereport(ERROR,
 					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 					 errmsg("permission denied to drop role")));
@@ -1205,7 +1197,7 @@ RenameRole(const char *oldname, const char *newname)
 				 errmsg("role \"%s\" already exists", newname)));
 
 	/*
-	 * createrole is enough privilege unless you want to mess with a superuser
+	 * role ownership is enough privilege unless you want to mess with a superuser
 	 */
 	if (((Form_pg_authid) GETSTRUCT(oldtuple))->rolsuper)
 	{
@@ -1216,7 +1208,7 @@ RenameRole(const char *oldname, const char *newname)
 	}
 	else
 	{
-		if (!have_createrole_privilege())
+		if (!pg_role_ownercheck(roleid, GetUserId()))
 			ereport(ERROR,
 					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 					 errmsg("permission denied to rename role")));
@@ -1431,7 +1423,7 @@ AddRoleMems(const char *rolename, Oid roleid,
 		return;
 
 	/*
-	 * Check permissions: must have createrole or admin option on the role to
+	 * Check permissions: must be owner or have admin option on the role to
 	 * be changed.  To mess with a superuser role, you gotta be superuser.
 	 */
 	if (superuser_arg(roleid))
@@ -1441,9 +1433,9 @@ AddRoleMems(const char *rolename, Oid roleid,
 					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 					 errmsg("must be superuser to alter superusers")));
 	}
-	else
+	else if (!superuser())
 	{
-		if (!have_createrole_privilege() &&
+		if (!pg_role_ownercheck(roleid, grantorId) &&
 			!is_admin_of_role(grantorId, roleid))
 			ereport(ERROR,
 					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
@@ -1619,9 +1611,9 @@ DelRoleMems(const char *rolename, Oid roleid,
 					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 					 errmsg("must be superuser to alter superusers")));
 	}
-	else
+	else if (!superuser())
 	{
-		if (!have_createrole_privilege() &&
+		if (!pg_role_ownercheck(roleid, GetUserId()) &&
 			!is_admin_of_role(GetUserId(), roleid))
 			ereport(ERROR,
 					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
@@ -1777,7 +1769,7 @@ AlterRoleOwner_internal(HeapTuple tup, Relation rel, Oid newOwnerId)
 		 * roles.  Because superusers will always have this right, we need no
 		 * special case for them.
 		 */
-		if (!have_createrole_privilege())
+		if (!has_createrole_privilege(GetUserId()))
 			aclcheck_error(ACLCHECK_NO_PRIV, OBJECT_ROLE,
 						   NameStr(authForm->rolname));
 
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 97336db058..cdcce9c569 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -4656,7 +4656,7 @@ initialize_acl(void)
 		/*
 		 * In normal mode, set a callback on any syscache invalidation of rows
 		 * of pg_auth_members (for roles_is_member_of()), pg_authid (for
-		 * has_rolinherit()), or pg_database (for roles_is_member_of())
+		 * has_inherit_privilege()), or pg_database (for roles_is_member_of())
 		 */
 		CacheRegisterSyscacheCallback(AUTHMEMROLEMEM,
 									  RoleMembershipCacheCallback,
@@ -4690,23 +4690,6 @@ RoleMembershipCacheCallback(Datum arg, int cacheid, uint32 hashvalue)
 }
 
 
-/* Check if specified role has rolinherit set */
-static bool
-has_rolinherit(Oid roleid)
-{
-	bool		result = false;
-	HeapTuple	utup;
-
-	utup = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
-	if (HeapTupleIsValid(utup))
-	{
-		result = ((Form_pg_authid) GETSTRUCT(utup))->rolinherit;
-		ReleaseSysCache(utup);
-	}
-	return result;
-}
-
-
 /*
  * Get a list of roles that the specified roleid is a member of
  *
@@ -4776,7 +4759,7 @@ roles_is_member_of(Oid roleid, enum RoleRecurseType type,
 		CatCList   *memlist;
 		int			i;
 
-		if (type == ROLERECURSE_PRIVS && !has_rolinherit(memberid))
+		if (type == ROLERECURSE_PRIVS && !has_inherit_privilege(memberid))
 			continue;			/* ignore non-inheriting roles */
 
 		/* Find roles that memberid is directly a member of */
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index 572cae0f27..7a6640dfc9 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -320,5 +320,10 @@ extern bool pg_statistics_object_ownercheck(Oid stat_oid, Oid roleid);
 extern bool pg_role_ownercheck(Oid owned_role_oid, Oid owner_roleid);
 extern bool has_createrole_privilege(Oid roleid);
 extern bool has_bypassrls_privilege(Oid roleid);
+extern bool has_inherit_privilege(Oid roleid);
+extern bool has_createdb_privilege(Oid roleid);
+extern bool has_login_privilege(Oid roleid);
+extern bool has_replication_privilege(Oid roleid);
+extern int32 role_connection_limit(Oid roleid);
 
 #endif							/* ACL_H */
diff --git a/src/test/regress/expected/create_role.out b/src/test/regress/expected/create_role.out
index 2c42c8ad8d..26d2ef43d9 100644
--- a/src/test/regress/expected/create_role.out
+++ b/src/test/regress/expected/create_role.out
@@ -6,22 +6,16 @@ GRANT CREATE ON DATABASE regression TO regress_role_admin;
 SET SESSION AUTHORIZATION regress_role_admin;
 CREATE ROLE regress_nosuch_superuser SUPERUSER;
 ERROR:  must be superuser to create superusers
+-- ok, can assign privileges the creator has
 CREATE ROLE regress_nosuch_replication_bypassrls REPLICATION BYPASSRLS;
-ERROR:  must be superuser to create replication users
 CREATE ROLE regress_nosuch_replication REPLICATION;
-ERROR:  must be superuser to create replication users
 CREATE ROLE regress_nosuch_bypassrls BYPASSRLS;
-ERROR:  must be superuser to create bypassrls users
 -- fail, only superusers can own superusers
 RESET SESSION AUTHORIZATION;
 CREATE ROLE regress_nosuch_superuser AUTHORIZATION regress_role_admin SUPERUSER;
 ERROR:  must be superuser to own superusers
 -- ok, superuser can create superusers belonging to other superusers
 CREATE ROLE regress_nosuch_superuser AUTHORIZATION regress_role_super SUPERUSER;
--- ok, superuser can create users with these privileges for normal role
-CREATE ROLE regress_nosuch_replication_bypassrls AUTHORIZATION regress_role_admin REPLICATION BYPASSRLS;
-CREATE ROLE regress_nosuch_replication AUTHORIZATION regress_role_admin REPLICATION;
-CREATE ROLE regress_nosuch_bypassrls AUTHORIZATION regress_role_admin BYPASSRLS;
 \du+ regress_nosuch_superuser
                                            List of roles
         Role name         |       Owner        |       Attributes        | Member of | Description 
@@ -53,7 +47,10 @@ ERROR:  role may not own itself
 SET SESSION AUTHORIZATION regress_role_admin;
 CREATE ROLE regress_createdb CREATEDB;
 CREATE ROLE regress_createrole CREATEROLE;
+-- fail, cannot assign LOGIN privilege that creator lacks
 CREATE ROLE regress_login LOGIN;
+ERROR:  must have login privilege to create login users
+-- ok, having CREATEROLE is enough for these
 CREATE ROLE regress_inherit INHERIT;
 CREATE ROLE regress_connection_limit CONNECTION LIMIT 5;
 CREATE ROLE regress_encrypted_password PASSWORD NULL;
@@ -73,12 +70,6 @@ COMMENT ON ROLE regress_password_null IS 'no login test role';
 --------------------+--------------------+---------------------------+-----------+-------------
  regress_createrole | regress_role_admin | Create role, Cannot login | {}        | 
 
-\du+ regress_login
-                               List of roles
-   Role name   |       Owner        | Attributes | Member of | Description 
----------------+--------------------+------------+-----------+-------------
- regress_login | regress_role_admin |            | {}        | 
-
 \du+ regress_inherit
                                  List of roles
     Role name    |       Owner        |  Attributes  | Member of | Description 
@@ -111,19 +102,19 @@ NOTICE:  SYSID can no longer be specified
 -- fail, cannot grant membership in superuser role
 CREATE ROLE regress_nosuch_super IN ROLE regress_role_super;
 ERROR:  must be superuser to alter superusers
--- fail, database owner cannot have members
+-- fail, do not have ADMIN privilege on database owner
 CREATE ROLE regress_nosuch_dbowner IN ROLE pg_database_owner;
-ERROR:  role "pg_database_owner" cannot have explicit members
+ERROR:  must have admin option on role "pg_database_owner"
 -- ok, can grant other users into a role
 CREATE ROLE regress_inroles ROLE
-	regress_role_super, regress_createdb, regress_createrole, regress_login,
+	regress_role_super, regress_createdb, regress_createrole,
 	regress_inherit, regress_connection_limit, regress_encrypted_password, regress_password_null;
 -- fail, cannot grant a role into itself
 CREATE ROLE regress_nosuch_recursive ROLE regress_nosuch_recursive;
 ERROR:  role "regress_nosuch_recursive" is a member of role "regress_nosuch_recursive"
 -- ok, can grant other users into a role with admin option
 CREATE ROLE regress_adminroles ADMIN
-	regress_role_super, regress_createdb, regress_createrole, regress_login,
+	regress_role_super, regress_createdb, regress_createrole,
 	regress_inherit, regress_connection_limit, regress_encrypted_password, regress_password_null;
 -- fail, cannot grant a role into itself with admin option
 CREATE ROLE regress_nosuch_admin_recursive ADMIN regress_nosuch_admin_recursive;
@@ -136,8 +127,11 @@ ERROR:  permission denied to create database
 CREATE ROLE regress_plainrole;
 -- ok, roles with CREATEROLE can create new roles with it
 CREATE ROLE regress_rolecreator CREATEROLE;
--- ok, roles with CREATEROLE can create new roles with privilege they lack
+-- fail, roles with CREATEROLE cannot create new roles with privilege they lack
 CREATE ROLE regress_tenant CREATEDB CREATEROLE LOGIN INHERIT CONNECTION LIMIT 5;
+ERROR:  must have createdb privilege to create createdb users
+-- ok, roles with CREATEROLE can create new roles with privilege they have
+CREATE ROLE regress_tenant CREATEROLE INHERIT CONNECTION LIMIT 5;
 -- ok, regress_tenant can create objects within the database
 SET SESSION AUTHORIZATION regress_tenant;
 CREATE TABLE tenant_table (i integer);
@@ -156,17 +150,27 @@ ERROR:  must be member of role "regress_role_admin"
 DROP VIEW tenant_view;
 -- ok, can take ownership objects from owned roles
 REASSIGN OWNED BY regress_tenant TO regress_createrole;
--- ok, having CREATEROLE is enough to create roles in privileged roles
+-- fail, having CREATEROLE is not enough to create roles in privileged roles
 CREATE ROLE regress_read_all_data IN ROLE pg_read_all_data;
+ERROR:  must have admin option on role "pg_read_all_data"
 CREATE ROLE regress_write_all_data IN ROLE pg_write_all_data;
+ERROR:  must have admin option on role "pg_write_all_data"
 CREATE ROLE regress_monitor IN ROLE pg_monitor;
+ERROR:  must have admin option on role "pg_monitor"
 CREATE ROLE regress_read_all_settings IN ROLE pg_read_all_settings;
+ERROR:  must have admin option on role "pg_read_all_settings"
 CREATE ROLE regress_read_all_stats IN ROLE pg_read_all_stats;
+ERROR:  must have admin option on role "pg_read_all_stats"
 CREATE ROLE regress_stat_scan_tables IN ROLE pg_stat_scan_tables;
+ERROR:  must have admin option on role "pg_stat_scan_tables"
 CREATE ROLE regress_read_server_files IN ROLE pg_read_server_files;
+ERROR:  must have admin option on role "pg_read_server_files"
 CREATE ROLE regress_write_server_files IN ROLE pg_write_server_files;
+ERROR:  must have admin option on role "pg_write_server_files"
 CREATE ROLE regress_execute_server_program IN ROLE pg_execute_server_program;
+ERROR:  must have admin option on role "pg_execute_server_program"
 CREATE ROLE regress_signal_backend IN ROLE pg_signal_backend;
+ERROR:  must have admin option on role "pg_signal_backend"
 -- fail, cannot create ownership cycles
 RESET SESSION AUTHORIZATION;
 REASSIGN OWNED BY regress_role_admin TO regress_tenant;
@@ -191,19 +195,8 @@ DROP ROLE regress_createrole;
 ERROR:  role "regress_createrole" cannot be dropped because some objects depend on it
 DETAIL:  owner of role regress_rolecreator
 owner of role regress_tenant
-owner of role regress_read_all_data
-owner of role regress_write_all_data
-owner of role regress_monitor
-owner of role regress_read_all_settings
-owner of role regress_read_all_stats
-owner of role regress_stat_scan_tables
-owner of role regress_read_server_files
-owner of role regress_write_server_files
-owner of role regress_execute_server_program
-owner of role regress_signal_backend
 -- ok, should be able to drop these non-superuser roles
 DROP ROLE regress_createdb;
-DROP ROLE regress_login;
 DROP ROLE regress_inherit;
 DROP ROLE regress_connection_limit;
 DROP ROLE regress_encrypted_password;
@@ -213,16 +206,6 @@ DROP ROLE regress_inroles;
 DROP ROLE regress_adminroles;
 DROP ROLE regress_rolecreator;
 DROP ROLE regress_tenant;
-DROP ROLE regress_read_all_data;
-DROP ROLE regress_write_all_data;
-DROP ROLE regress_monitor;
-DROP ROLE regress_read_all_settings;
-DROP ROLE regress_read_all_stats;
-DROP ROLE regress_stat_scan_tables;
-DROP ROLE regress_read_server_files;
-DROP ROLE regress_write_server_files;
-DROP ROLE regress_execute_server_program;
-DROP ROLE regress_signal_backend;
 -- fail, cannot drop ourself nor superusers
 DROP ROLE regress_role_super;
 ERROR:  must be superuser to drop superusers
diff --git a/src/test/regress/sql/create_role.sql b/src/test/regress/sql/create_role.sql
index 00e63b7d93..c484d77aa7 100644
--- a/src/test/regress/sql/create_role.sql
+++ b/src/test/regress/sql/create_role.sql
@@ -6,6 +6,8 @@ GRANT CREATE ON DATABASE regression TO regress_role_admin;
 -- fail, only superusers can create users with these privileges
 SET SESSION AUTHORIZATION regress_role_admin;
 CREATE ROLE regress_nosuch_superuser SUPERUSER;
+
+-- ok, can assign privileges the creator has
 CREATE ROLE regress_nosuch_replication_bypassrls REPLICATION BYPASSRLS;
 CREATE ROLE regress_nosuch_replication REPLICATION;
 CREATE ROLE regress_nosuch_bypassrls BYPASSRLS;
@@ -17,11 +19,6 @@ CREATE ROLE regress_nosuch_superuser AUTHORIZATION regress_role_admin SUPERUSER;
 -- ok, superuser can create superusers belonging to other superusers
 CREATE ROLE regress_nosuch_superuser AUTHORIZATION regress_role_super SUPERUSER;
 
--- ok, superuser can create users with these privileges for normal role
-CREATE ROLE regress_nosuch_replication_bypassrls AUTHORIZATION regress_role_admin REPLICATION BYPASSRLS;
-CREATE ROLE regress_nosuch_replication AUTHORIZATION regress_role_admin REPLICATION;
-CREATE ROLE regress_nosuch_bypassrls AUTHORIZATION regress_role_admin BYPASSRLS;
-
 \du+ regress_nosuch_superuser
 \du+ regress_nosuch_replication_bypassrls
 \du+ regress_nosuch_replication
@@ -34,7 +31,11 @@ ALTER ROLE regress_nosuch_bypassrls OWNER TO regress_nosuch_bypassrls;
 SET SESSION AUTHORIZATION regress_role_admin;
 CREATE ROLE regress_createdb CREATEDB;
 CREATE ROLE regress_createrole CREATEROLE;
+
+-- fail, cannot assign LOGIN privilege that creator lacks
 CREATE ROLE regress_login LOGIN;
+
+-- ok, having CREATEROLE is enough for these
 CREATE ROLE regress_inherit INHERIT;
 CREATE ROLE regress_connection_limit CONNECTION LIMIT 5;
 CREATE ROLE regress_encrypted_password PASSWORD NULL;
@@ -45,7 +46,6 @@ COMMENT ON ROLE regress_password_null IS 'no login test role';
 
 \du+ regress_createdb
 \du+ regress_createrole
-\du+ regress_login
 \du+ regress_inherit
 \du+ regress_connection_limit
 \du+ regress_encrypted_password
@@ -57,12 +57,12 @@ CREATE ROLE regress_noiseword SYSID 12345;
 -- fail, cannot grant membership in superuser role
 CREATE ROLE regress_nosuch_super IN ROLE regress_role_super;
 
--- fail, database owner cannot have members
+-- fail, do not have ADMIN privilege on database owner
 CREATE ROLE regress_nosuch_dbowner IN ROLE pg_database_owner;
 
 -- ok, can grant other users into a role
 CREATE ROLE regress_inroles ROLE
-	regress_role_super, regress_createdb, regress_createrole, regress_login,
+	regress_role_super, regress_createdb, regress_createrole,
 	regress_inherit, regress_connection_limit, regress_encrypted_password, regress_password_null;
 
 -- fail, cannot grant a role into itself
@@ -70,7 +70,7 @@ CREATE ROLE regress_nosuch_recursive ROLE regress_nosuch_recursive;
 
 -- ok, can grant other users into a role with admin option
 CREATE ROLE regress_adminroles ADMIN
-	regress_role_super, regress_createdb, regress_createrole, regress_login,
+	regress_role_super, regress_createdb, regress_createrole,
 	regress_inherit, regress_connection_limit, regress_encrypted_password, regress_password_null;
 
 -- fail, cannot grant a role into itself with admin option
@@ -86,9 +86,12 @@ CREATE ROLE regress_plainrole;
 -- ok, roles with CREATEROLE can create new roles with it
 CREATE ROLE regress_rolecreator CREATEROLE;
 
--- ok, roles with CREATEROLE can create new roles with privilege they lack
+-- fail, roles with CREATEROLE cannot create new roles with privilege they lack
 CREATE ROLE regress_tenant CREATEDB CREATEROLE LOGIN INHERIT CONNECTION LIMIT 5;
 
+-- ok, roles with CREATEROLE can create new roles with privilege they have
+CREATE ROLE regress_tenant CREATEROLE INHERIT CONNECTION LIMIT 5;
+
 -- ok, regress_tenant can create objects within the database
 SET SESSION AUTHORIZATION regress_tenant;
 CREATE TABLE tenant_table (i integer);
@@ -111,7 +114,7 @@ DROP VIEW tenant_view;
 -- ok, can take ownership objects from owned roles
 REASSIGN OWNED BY regress_tenant TO regress_createrole;
 
--- ok, having CREATEROLE is enough to create roles in privileged roles
+-- fail, having CREATEROLE is not enough to create roles in privileged roles
 CREATE ROLE regress_read_all_data IN ROLE pg_read_all_data;
 CREATE ROLE regress_write_all_data IN ROLE pg_write_all_data;
 CREATE ROLE regress_monitor IN ROLE pg_monitor;
@@ -149,7 +152,6 @@ DROP ROLE regress_createrole;
 
 -- ok, should be able to drop these non-superuser roles
 DROP ROLE regress_createdb;
-DROP ROLE regress_login;
 DROP ROLE regress_inherit;
 DROP ROLE regress_connection_limit;
 DROP ROLE regress_encrypted_password;
@@ -159,16 +161,6 @@ DROP ROLE regress_inroles;
 DROP ROLE regress_adminroles;
 DROP ROLE regress_rolecreator;
 DROP ROLE regress_tenant;
-DROP ROLE regress_read_all_data;
-DROP ROLE regress_write_all_data;
-DROP ROLE regress_monitor;
-DROP ROLE regress_read_all_settings;
-DROP ROLE regress_read_all_stats;
-DROP ROLE regress_stat_scan_tables;
-DROP ROLE regress_read_server_files;
-DROP ROLE regress_write_server_files;
-DROP ROLE regress_execute_server_program;
-DROP ROLE regress_signal_backend;
 
 -- fail, cannot drop ourself nor superusers
 DROP ROLE regress_role_super;
-- 
2.21.1 (Apple Git-122.3)