Restrict the privileges of CREATEROLE users.

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

4 files changed, 73 insertions, 48 deletions

diff --git a/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out b/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
index b2d898a7d1a..c57d4fd2df0 100644
--- a/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
+++ b/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
@@ -6,9 +6,11 @@ CREATE EXTENSION dummy_seclabel;
 SET client_min_messages TO 'warning';
 DROP ROLE IF EXISTS regress_dummy_seclabel_user1;
 DROP ROLE IF EXISTS regress_dummy_seclabel_user2;
+DROP ROLE IF EXISTS regress_dummy_seclabel_user3;
 RESET client_min_messages;
 CREATE USER regress_dummy_seclabel_user1 WITH CREATEROLE;
 CREATE USER regress_dummy_seclabel_user2;
+CREATE USER regress_dummy_seclabel_user3;
 CREATE TABLE dummy_seclabel_tbl1 (a int, b text);
 CREATE TABLE dummy_seclabel_tbl2 (x int, y text);
 CREATE VIEW dummy_seclabel_view1 AS SELECT * FROM dummy_seclabel_tbl2;
@@ -16,6 +18,8 @@ CREATE FUNCTION dummy_seclabel_four() RETURNS integer AS $$SELECT 4$$ language s
 CREATE DOMAIN dummy_seclabel_domain AS text;
 ALTER TABLE dummy_seclabel_tbl1 OWNER TO regress_dummy_seclabel_user1;
 ALTER TABLE dummy_seclabel_tbl2 OWNER TO regress_dummy_seclabel_user2;
+GRANT regress_dummy_seclabel_user2, regress_dummy_seclabel_user3
+  TO regress_dummy_seclabel_user1 WITH ADMIN TRUE, INHERIT FALSE, SET FALSE;
 --
 -- Test of SECURITY LABEL statement with a plugin
 --
@@ -43,16 +47,16 @@ SECURITY LABEL ON TABLE dummy_seclabel_tbl2 IS 'classified';			-- OK
 -- Test for shared database object
 --
 SET SESSION AUTHORIZATION regress_dummy_seclabel_user1;
-SECURITY LABEL ON ROLE regress_dummy_seclabel_user1 IS 'classified';			-- OK
-SECURITY LABEL ON ROLE regress_dummy_seclabel_user1 IS '...invalid label...';	-- fail
+SECURITY LABEL ON ROLE regress_dummy_seclabel_user3 IS 'classified';			-- OK
+SECURITY LABEL ON ROLE regress_dummy_seclabel_user3 IS '...invalid label...';	-- fail
 ERROR:  '...invalid label...' is not a valid security label
 SECURITY LABEL FOR 'dummy' ON ROLE regress_dummy_seclabel_user2 IS 'unclassified';	-- OK
 SECURITY LABEL FOR 'unknown_seclabel' ON ROLE regress_dummy_seclabel_user1 IS 'unclassified';	-- fail
 ERROR:  security label provider "unknown_seclabel" is not loaded
-SECURITY LABEL ON ROLE regress_dummy_seclabel_user1 IS 'secret';	-- fail (not superuser)
+SECURITY LABEL ON ROLE regress_dummy_seclabel_user3 IS 'secret';	-- fail (not superuser)
 ERROR:  only superuser can set 'secret' label
-SECURITY LABEL ON ROLE regress_dummy_seclabel_user3 IS 'unclassified';	-- fail (not found)
-ERROR:  role "regress_dummy_seclabel_user3" does not exist
+SECURITY LABEL ON ROLE regress_dummy_seclabel_user4 IS 'unclassified';	-- fail (not found)
+ERROR:  role "regress_dummy_seclabel_user4" does not exist
 SET SESSION AUTHORIZATION regress_dummy_seclabel_user2;
 SECURITY LABEL ON ROLE regress_dummy_seclabel_user2 IS 'unclassified';	-- fail (not privileged)
 ERROR:  must have CREATEROLE privilege
@@ -81,8 +85,8 @@ SELECT objtype, objname, provider, label FROM pg_seclabels
  domain       | dummy_seclabel_domain        | dummy    | classified
  function     | dummy_seclabel_four()        | dummy    | classified
  publication  | dummy_pub                    | dummy    | classified
- role         | regress_dummy_seclabel_user1 | dummy    | classified
  role         | regress_dummy_seclabel_user2 | dummy    | unclassified
+ role         | regress_dummy_seclabel_user3 | dummy    | classified
  schema       | dummy_seclabel_test          | dummy    | unclassified
  subscription | dummy_sub                    | dummy    | classified
  table        | dummy_seclabel_tbl1          | dummy    | top secret
@@ -115,3 +119,4 @@ DROP SUBSCRIPTION dummy_sub;
 DROP PUBLICATION dummy_pub;
 DROP ROLE regress_dummy_seclabel_user1;
 DROP ROLE regress_dummy_seclabel_user2;
+DROP ROLE regress_dummy_seclabel_user3;
diff --git a/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql b/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql
index 8c347b6a68b..649409757e8 100644
--- a/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql
+++ b/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql
@@ -8,11 +8,13 @@ SET client_min_messages TO 'warning';
 
 DROP ROLE IF EXISTS regress_dummy_seclabel_user1;
 DROP ROLE IF EXISTS regress_dummy_seclabel_user2;
+DROP ROLE IF EXISTS regress_dummy_seclabel_user3;
 
 RESET client_min_messages;
 
 CREATE USER regress_dummy_seclabel_user1 WITH CREATEROLE;
 CREATE USER regress_dummy_seclabel_user2;
+CREATE USER regress_dummy_seclabel_user3;
 
 CREATE TABLE dummy_seclabel_tbl1 (a int, b text);
 CREATE TABLE dummy_seclabel_tbl2 (x int, y text);
@@ -22,6 +24,8 @@ CREATE DOMAIN dummy_seclabel_domain AS text;
 
 ALTER TABLE dummy_seclabel_tbl1 OWNER TO regress_dummy_seclabel_user1;
 ALTER TABLE dummy_seclabel_tbl2 OWNER TO regress_dummy_seclabel_user2;
+GRANT regress_dummy_seclabel_user2, regress_dummy_seclabel_user3
+  TO regress_dummy_seclabel_user1 WITH ADMIN TRUE, INHERIT FALSE, SET FALSE;
 
 --
 -- Test of SECURITY LABEL statement with a plugin
@@ -47,12 +51,12 @@ SECURITY LABEL ON TABLE dummy_seclabel_tbl2 IS 'classified';			-- OK
 --
 SET SESSION AUTHORIZATION regress_dummy_seclabel_user1;
 
-SECURITY LABEL ON ROLE regress_dummy_seclabel_user1 IS 'classified';			-- OK
-SECURITY LABEL ON ROLE regress_dummy_seclabel_user1 IS '...invalid label...';	-- fail
+SECURITY LABEL ON ROLE regress_dummy_seclabel_user3 IS 'classified';			-- OK
+SECURITY LABEL ON ROLE regress_dummy_seclabel_user3 IS '...invalid label...';	-- fail
 SECURITY LABEL FOR 'dummy' ON ROLE regress_dummy_seclabel_user2 IS 'unclassified';	-- OK
 SECURITY LABEL FOR 'unknown_seclabel' ON ROLE regress_dummy_seclabel_user1 IS 'unclassified';	-- fail
-SECURITY LABEL ON ROLE regress_dummy_seclabel_user1 IS 'secret';	-- fail (not superuser)
-SECURITY LABEL ON ROLE regress_dummy_seclabel_user3 IS 'unclassified';	-- fail (not found)
+SECURITY LABEL ON ROLE regress_dummy_seclabel_user3 IS 'secret';	-- fail (not superuser)
+SECURITY LABEL ON ROLE regress_dummy_seclabel_user4 IS 'unclassified';	-- fail (not found)
 
 SET SESSION AUTHORIZATION regress_dummy_seclabel_user2;
 SECURITY LABEL ON ROLE regress_dummy_seclabel_user2 IS 'unclassified';	-- fail (not privileged)
@@ -113,3 +117,4 @@ DROP PUBLICATION dummy_pub;
 
 DROP ROLE regress_dummy_seclabel_user1;
 DROP ROLE regress_dummy_seclabel_user2;
+DROP ROLE regress_dummy_seclabel_user3;
diff --git a/src/test/regress/expected/create_role.out b/src/test/regress/expected/create_role.out
index 4e67d727603..f5f745504c2 100644
--- a/src/test/regress/expected/create_role.out
+++ b/src/test/regress/expected/create_role.out
@@ -1,6 +1,7 @@
 -- ok, superuser can create users with any set of privileges
 CREATE ROLE regress_role_super SUPERUSER;
 CREATE ROLE regress_role_admin CREATEDB CREATEROLE REPLICATION BYPASSRLS;
+CREATE ROLE regress_role_normal;
 -- fail, only superusers can create users with these privileges
 SET SESSION AUTHORIZATION regress_role_admin;
 CREATE ROLE regress_nosuch_superuser SUPERUSER;
@@ -13,7 +14,7 @@ CREATE ROLE regress_nosuch_bypassrls BYPASSRLS;
 ERROR:  must be superuser to create bypassrls users
 -- ok, having CREATEROLE is enough to create users with these privileges
 CREATE ROLE regress_createdb CREATEDB;
-CREATE ROLE regress_createrole CREATEROLE;
+CREATE ROLE regress_createrole CREATEROLE NOINHERIT;
 CREATE ROLE regress_login LOGIN;
 CREATE ROLE regress_inherit INHERIT;
 CREATE ROLE regress_connection_limit CONNECTION LIMIT 5;
@@ -51,7 +52,19 @@ 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
-CREATE ROLE regress_tenant CREATEDB CREATEROLE LOGIN INHERIT CONNECTION LIMIT 5;
+CREATE ROLE regress_hasprivs CREATEDB CREATEROLE LOGIN INHERIT
+	CONNECTION LIMIT 5;
+-- ok, we should be able to modify a role we created
+COMMENT ON ROLE regress_hasprivs IS 'some comment';
+ALTER ROLE regress_hasprivs RENAME TO regress_tenant;
+ALTER ROLE regress_tenant NOINHERIT NOLOGIN CONNECTION LIMIT 7;
+-- fail, we should be unable to modify a role we did not create
+COMMENT ON ROLE regress_role_normal IS 'some comment';
+ERROR:  must have admin option on role "regress_role_normal"
+ALTER ROLE regress_role_normal RENAME TO regress_role_abnormal;
+ERROR:  permission denied to rename role
+ALTER ROLE regress_role_normal NOINHERIT NOLOGIN CONNECTION LIMIT 7;
+ERROR:  permission denied
 -- ok, regress_tenant can create objects within the database
 SET SESSION AUTHORIZATION regress_tenant;
 CREATE TABLE tenant_table (i integer);
@@ -70,20 +83,35 @@ ALTER VIEW tenant_view OWNER TO regress_role_admin;
 ERROR:  must be owner of view tenant_view
 DROP VIEW tenant_view;
 ERROR:  must be owner of view tenant_view
--- fail, cannot take ownership of these objects from regress_tenant
+-- fail, we don't inherit permissions from regress_tenant
 REASSIGN OWNED BY regress_tenant TO regress_createrole;
 ERROR:  permission denied to reassign objects
--- ok, having CREATEROLE is enough to create roles in privileged roles
+-- fail, 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, role still owns database objects
+DROP ROLE regress_tenant;
+ERROR:  role "regress_tenant" cannot be dropped because some objects depend on it
+DETAIL:  owner of table tenant_table
+owner of view tenant_view
 -- fail, creation of these roles failed above so they do not now exist
 SET SESSION AUTHORIZATION regress_role_admin;
 DROP ROLE regress_nosuch_superuser;
@@ -114,22 +142,6 @@ DROP ROLE regress_password_null;
 DROP ROLE regress_noiseword;
 DROP ROLE regress_inroles;
 DROP ROLE regress_adminroles;
-DROP ROLE regress_rolecreator;
-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, role still owns database objects
-DROP ROLE regress_tenant;
-ERROR:  role "regress_tenant" cannot be dropped because some objects depend on it
-DETAIL:  owner of table tenant_table
-owner of view tenant_view
 -- fail, cannot drop ourself nor superusers
 DROP ROLE regress_role_super;
 ERROR:  must be superuser to drop superusers
@@ -143,3 +155,4 @@ DROP VIEW tenant_view;
 DROP ROLE regress_tenant;
 DROP ROLE regress_role_admin;
 DROP ROLE regress_role_super;
+DROP ROLE regress_role_normal;
diff --git a/src/test/regress/sql/create_role.sql b/src/test/regress/sql/create_role.sql
index 292dc087975..ddc80578d90 100644
--- a/src/test/regress/sql/create_role.sql
+++ b/src/test/regress/sql/create_role.sql
@@ -1,6 +1,7 @@
 -- ok, superuser can create users with any set of privileges
 CREATE ROLE regress_role_super SUPERUSER;
 CREATE ROLE regress_role_admin CREATEDB CREATEROLE REPLICATION BYPASSRLS;
+CREATE ROLE regress_role_normal;
 
 -- fail, only superusers can create users with these privileges
 SET SESSION AUTHORIZATION regress_role_admin;
@@ -11,7 +12,7 @@ CREATE ROLE regress_nosuch_bypassrls BYPASSRLS;
 
 -- ok, having CREATEROLE is enough to create users with these privileges
 CREATE ROLE regress_createdb CREATEDB;
-CREATE ROLE regress_createrole CREATEROLE;
+CREATE ROLE regress_createrole CREATEROLE NOINHERIT;
 CREATE ROLE regress_login LOGIN;
 CREATE ROLE regress_inherit INHERIT;
 CREATE ROLE regress_connection_limit CONNECTION LIMIT 5;
@@ -54,7 +55,18 @@ CREATE ROLE regress_plainrole;
 CREATE ROLE regress_rolecreator CREATEROLE;
 
 -- ok, roles with CREATEROLE can create new roles with privilege they lack
-CREATE ROLE regress_tenant CREATEDB CREATEROLE LOGIN INHERIT CONNECTION LIMIT 5;
+CREATE ROLE regress_hasprivs CREATEDB CREATEROLE LOGIN INHERIT
+	CONNECTION LIMIT 5;
+
+-- ok, we should be able to modify a role we created
+COMMENT ON ROLE regress_hasprivs IS 'some comment';
+ALTER ROLE regress_hasprivs RENAME TO regress_tenant;
+ALTER ROLE regress_tenant NOINHERIT NOLOGIN CONNECTION LIMIT 7;
+
+-- fail, we should be unable to modify a role we did not create
+COMMENT ON ROLE regress_role_normal IS 'some comment';
+ALTER ROLE regress_role_normal RENAME TO regress_role_abnormal;
+ALTER ROLE regress_role_normal NOINHERIT NOLOGIN CONNECTION LIMIT 7;
 
 -- ok, regress_tenant can create objects within the database
 SET SESSION AUTHORIZATION regress_tenant;
@@ -71,10 +83,10 @@ DROP TABLE tenant_table;
 ALTER VIEW tenant_view OWNER TO regress_role_admin;
 DROP VIEW tenant_view;
 
--- fail, cannot take ownership of these objects from regress_tenant
+-- fail, we don't inherit permissions from regress_tenant
 REASSIGN OWNED BY regress_tenant TO regress_createrole;
 
--- ok, having CREATEROLE is enough to create roles in privileged roles
+-- fail, 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;
@@ -86,6 +98,9 @@ CREATE ROLE regress_write_server_files IN ROLE pg_write_server_files;
 CREATE ROLE regress_execute_server_program IN ROLE pg_execute_server_program;
 CREATE ROLE regress_signal_backend IN ROLE pg_signal_backend;
 
+-- fail, role still owns database objects
+DROP ROLE regress_tenant;
+
 -- fail, creation of these roles failed above so they do not now exist
 SET SESSION AUTHORIZATION regress_role_admin;
 DROP ROLE regress_nosuch_superuser;
@@ -109,20 +124,6 @@ DROP ROLE regress_password_null;
 DROP ROLE regress_noiseword;
 DROP ROLE regress_inroles;
 DROP ROLE regress_adminroles;
-DROP ROLE regress_rolecreator;
-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, role still owns database objects
-DROP ROLE regress_tenant;
 
 -- fail, cannot drop ourself nor superusers
 DROP ROLE regress_role_super;
@@ -136,3 +137,4 @@ DROP VIEW tenant_view;
 DROP ROLE regress_tenant;
 DROP ROLE regress_role_admin;
 DROP ROLE regress_role_super;
+DROP ROLE regress_role_normal;