Skip to content

Commit f1358ca

Browse files
robertmhaasrobertmhaas
committed
Adjust interaction of CREATEROLE with role properties.
Previously, a CREATEROLE user without SUPERUSER could not alter REPLICATION users in any way, and could not set the BYPASSRLS attribute. However, they could manipulate the CREATEDB property even if they themselves did not possess it. With this change, a CREATEROLE user without SUPERUSER can set or clear the REPLICATION, BYPASSRLS, or CREATEDB property on a new role or a role that they have rights to manage if and only if that property is set for their own role. This implements the standard idea that you can't give permissions you don't have (but you can give the ones you do have). We might in the future want to provide more powerful ways to constrain what a CREATEROLE user can do - for example, to limit whether CONNECTION LIMIT can be set or the values to which it can be set - but that is left as future work. Patch by me, reviewed by Nathan Bossart, Tushar Ahuja, and Neha Sharma. Discussion: http://postgr.es/m/CA+TgmobX=LHg_J5aT=0pi9gJy=JdtrUVGAu0zhr-i5v5nNbJDg@mail.gmail.com
1 parent 6c6d6ba commit f1358ca

File tree

7 files changed

+137
-83
lines changed

7 files changed

+137
-83
lines changed

‎doc/src/sgml/ref/alter_role.sgml

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,14 @@ ALTER ROLE { <replaceable class="parameter">role_specification</replaceable> | A
7070
<link linkend="sql-revoke"><command>REVOKE</command></link> for that.)
7171
Attributes not mentioned in the command retain their previous settings.
7272
Database superusers can change any of these settings for any role.
73-
Roles having <literal>CREATEROLE</literal> privilege can change any of these
74-
settings except <literal>SUPERUSER</literal>, <literal>REPLICATION</literal>,
75-
and <literal>BYPASSRLS</literal>; but only for non-superuser and
76-
non-replication roles for which they have been
77-
granted <literal>ADMIN OPTION</literal>.
73+
Non-superuser roles having <literal>CREATEROLE</literal> privilege can
74+
change most of these properties, but only for non-superuser and
75+
non-replication roles for which they have been granted
76+
<literal>ADMIN OPTION</literal>. Non-superusers cannot change the
77+
<literal>SUPERUSER</literal> property and can change the
78+
<literal>CREATEDB</literal>, <literal>REPLICATION</literal>, and
79+
<literal>BYPASSRLS</literal> properties only if they possess the
80+
corresponding property themselves.
7881
Ordinary roles can only change their own password.
7982
</para>
8083

‎doc/src/sgml/ref/create_role.sgml

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,8 @@ in sync when changing the above synopsis!
109109
<literal>NOCREATEDB</literal> will deny a role the ability to
110110
create databases. If not specified,
111111
<literal>NOCREATEDB</literal> is the default.
112+
Only superuser roles or roles with <literal>CREATEDB</literal>
113+
can specify <literal>CREATEDB</literal>.
112114
</para>
113115
</listitem>
114116
</varlistentry>
@@ -188,8 +190,8 @@ in sync when changing the above synopsis!
188190
highly privileged role, and should only be used on roles actually
189191
used for replication. If not specified,
190192
<literal>NOREPLICATION</literal> is the default.
191-
You must be a superuser to create a new role having the
192-
<literal>REPLICATION</literal> attribute.
193+
Only superuser roles or roles with <literal>REPLICATION</literal>
194+
can specify <literal>REPLICATION</literal>.
193195
</para>
194196
</listitem>
195197
</varlistentry>
@@ -201,8 +203,8 @@ in sync when changing the above synopsis!
201203
<para>
202204
These clauses determine whether a role bypasses every row-level
203205
security (RLS) policy. <literal>NOBYPASSRLS</literal> is the default.
204-
You must be a superuser to create a new role having
205-
the <literal>BYPASSRLS</literal> attribute.
206+
Only superuser roles or roles with <literal>BYPASSRLS</literal>
207+
can specify <literal>BYPASSRLS</literal>.
206208
</para>
207209

208210
<para>
@@ -390,19 +392,6 @@ in sync when changing the above synopsis!
390392
specified in the SQL standard.
391393
</para>
392394

393-
<para>
394-
Be careful with the <literal>CREATEROLE</literal> privilege. There is no concept of
395-
inheritance for the privileges of a <literal>CREATEROLE</literal>-role. That
396-
means that even if a role does not have a certain privilege but is allowed
397-
to create other roles, it can easily create another role with different
398-
privileges than its own (except for creating roles with superuser
399-
privileges). For example, if the role <quote>user</quote> has the
400-
<literal>CREATEROLE</literal> privilege but not the <literal>CREATEDB</literal> privilege,
401-
nonetheless it can create a new role with the <literal>CREATEDB</literal>
402-
privilege. Therefore, regard roles that have the <literal>CREATEROLE</literal>
403-
privilege as almost-superuser-roles.
404-
</para>
405-
406395
<para>
407396
<productname>PostgreSQL</productname> includes a program <xref
408397
linkend="app-createuser"/> that has

‎src/backend/commands/dbcommands.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,6 @@ static bool get_db_info(const char *name, LOCKMODE lockmode,
121121
Oid *dbTablespace, char **dbCollate, char **dbCtype, char **dbIculocale,
122122
char *dbLocProvider,
123123
char **dbCollversion);
124-
static bool have_createdb_privilege(void);
125124
static void remove_dbtablespaces(Oid db_id);
126125
static bool check_db_file_conflict(Oid db_id);
127126
static int errdetail_busy_db(int notherbackends, int npreparedxacts);
@@ -2742,7 +2741,7 @@ get_db_info(const char *name, LOCKMODE lockmode,
27422741
}
27432742

27442743
/* Check if current user has createdb privileges */
2745-
static bool
2744+
bool
27462745
have_createdb_privilege(void)
27472746
{
27482747
bool result = false;

‎src/backend/commands/user.c

Lines changed: 38 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -311,33 +311,28 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
311311
bypassrls = boolVal(dbypassRLS->arg);
312312

313313
/* Check some permissions first */
314-
if (issuper)
314+
if (!superuser_arg(currentUserId))
315315
{
316-
if (!superuser())
316+
if (!has_createrole_privilege(currentUserId))
317+
ereport(ERROR,
318+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
319+
errmsg("permission denied to create role")));
320+
if (issuper)
317321
ereport(ERROR,
318322
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
319323
errmsg("must be superuser to create superusers")));
320-
}
321-
else if (isreplication)
322-
{
323-
if (!superuser())
324+
if (createdb && !have_createdb_privilege())
324325
ereport(ERROR,
325326
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
326-
errmsg("must be superuser to create replication users")));
327-
}
328-
else if (bypassrls)
329-
{
330-
if (!superuser())
327+
errmsg("must have createdb permission to create createdb users")));
328+
if (isreplication && !has_rolreplication(currentUserId))
331329
ereport(ERROR,
332330
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
333-
errmsg("must be superuser to create bypassrls users")));
334-
}
335-
else
336-
{
337-
if (!have_createrole_privilege())
331+
errmsg("must have replication permission to create replication users")));
332+
if (bypassrls && !has_bypassrls_privilege(currentUserId))
338333
ereport(ERROR,
339334
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
340-
errmsg("permission denied to create role")));
335+
errmsg("must have bypassrls to create bypassrls users")));
341336
}
342337

343338
/*
@@ -748,32 +743,11 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
748743
rolename = pstrdup(NameStr(authform->rolname));
749744
roleid = authform->oid;
750745

751-
/*
752-
* To mess with a superuser or replication role in any way you gotta be
753-
* superuser. We also insist on superuser to change the BYPASSRLS
754-
* property.
755-
*/
756-
if (authform->rolsuper || dissuper)
757-
{
758-
if (!superuser())
759-
ereport(ERROR,
760-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
761-
errmsg("must be superuser to alter superuser roles or change superuser attribute")));
762-
}
763-
else if (authform->rolreplication || disreplication)
764-
{
765-
if (!superuser())
766-
ereport(ERROR,
767-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
768-
errmsg("must be superuser to alter replication roles or change replication attribute")));
769-
}
770-
else if (dbypassRLS)
771-
{
772-
if (!superuser())
773-
ereport(ERROR,
774-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
775-
errmsg("must be superuser to change bypassrls attribute")));
776-
}
746+
/* To mess with a superuser in any way you gotta be superuser. */
747+
if (!superuser() && (authform->rolsuper || dissuper))
748+
ereport(ERROR,
749+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
750+
errmsg("must be superuser to alter superuser roles or change superuser attribute")));
777751

778752
/*
779753
* Most changes to a role require that you both have CREATEROLE privileges
@@ -784,7 +758,7 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
784758
{
785759
/* things an unprivileged user certainly can't do */
786760
if (dinherit || dcreaterole || dcreatedb || dcanlogin || dconnlimit ||
787-
dvalidUntil)
761+
dvalidUntil || disreplication || dbypassRLS)
788762
ereport(ERROR,
789763
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
790764
errmsg("permission denied")));
@@ -795,6 +769,26 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
795769
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
796770
errmsg("must have CREATEROLE privilege to change another user's password")));
797771
}
772+
else if (!superuser())
773+
{
774+
/*
775+
* Even if you have both CREATEROLE and ADMIN OPTION on a role, you
776+
* can only change the CREATEDB, REPLICATION, or BYPASSRLS attributes
777+
* if they are set for your own role (or you are the superuser).
778+
*/
779+
if (dcreatedb && !have_createdb_privilege())
780+
ereport(ERROR,
781+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
782+
errmsg("must have createdb privilege to change createdb attribute")));
783+
if (disreplication && !has_rolreplication(currentUserId))
784+
ereport(ERROR,
785+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
786+
errmsg("must have replication privilege to change replication attribute")));
787+
if (dbypassRLS && !has_bypassrls_privilege(currentUserId))
788+
ereport(ERROR,
789+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
790+
errmsg("must have bypassrls privilege to change bypassrls attribute")));
791+
}
798792

799793
/* To add members to a role, you need ADMIN OPTION. */
800794
if (drolemembers && !is_admin_of_role(currentUserId, roleid))

‎src/include/commands/dbcommands.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ extern ObjectAddress AlterDatabaseOwner(const char *dbname, Oid newOwnerId);
3030

3131
extern Oid get_database_oid(const char *dbname, bool missing_ok);
3232
extern char *get_database_name(Oid dbid);
33+
extern bool have_createdb_privilege(void);
3334

3435
extern void check_encoding_locale_matches(int encoding, const char *collate, const char *ctype);
3536

‎src/test/regress/expected/create_role.out

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,51 @@
22
CREATE ROLE regress_role_super SUPERUSER;
33
CREATE ROLE regress_role_admin CREATEDB CREATEROLE REPLICATION BYPASSRLS;
44
GRANT CREATE ON DATABASE regression TO regress_role_admin WITH GRANT OPTION;
5+
CREATE ROLE regress_role_limited_admin CREATEROLE;
56
CREATE ROLE regress_role_normal;
6-
-- fail, only superusers can create users with these privileges
7-
SET SESSION AUTHORIZATION regress_role_admin;
7+
-- fail, CREATEROLE user can't give away role attributes without having them
8+
SET SESSION AUTHORIZATION regress_role_limited_admin;
89
CREATE ROLE regress_nosuch_superuser SUPERUSER;
910
ERROR: must be superuser to create superusers
1011
CREATE ROLE regress_nosuch_replication_bypassrls REPLICATION BYPASSRLS;
11-
ERROR: must be superuser to create replication users
12+
ERROR: must have replication permission to create replication users
1213
CREATE ROLE regress_nosuch_replication REPLICATION;
13-
ERROR: must be superuser to create replication users
14+
ERROR: must have replication permission to create replication users
1415
CREATE ROLE regress_nosuch_bypassrls BYPASSRLS;
15-
ERROR: must be superuser to create bypassrls users
16-
-- ok, having CREATEROLE is enough to create users with these privileges
16+
ERROR: must have bypassrls to create bypassrls users
17+
CREATE ROLE regress_nosuch_createdb CREATEDB;
18+
ERROR: must have createdb permission to create createdb users
19+
-- ok, can create a role without any special attributes
20+
CREATE ROLE regress_role_limited;
21+
-- fail, can't give it in any of the restricted attributes
22+
ALTER ROLE regress_role_limited SUPERUSER;
23+
ERROR: must be superuser to alter superuser roles or change superuser attribute
24+
ALTER ROLE regress_role_limited REPLICATION;
25+
ERROR: must have replication privilege to change replication attribute
26+
ALTER ROLE regress_role_limited CREATEDB;
27+
ERROR: must have createdb privilege to change createdb attribute
28+
ALTER ROLE regress_role_limited BYPASSRLS;
29+
ERROR: must have bypassrls privilege to change bypassrls attribute
30+
DROP ROLE regress_role_limited;
31+
-- ok, can give away these role attributes if you have them
32+
SET SESSION AUTHORIZATION regress_role_admin;
33+
CREATE ROLE regress_replication_bypassrls REPLICATION BYPASSRLS;
34+
CREATE ROLE regress_replication REPLICATION;
35+
CREATE ROLE regress_bypassrls BYPASSRLS;
1736
CREATE ROLE regress_createdb CREATEDB;
37+
-- ok, can toggle these role attributes off and on if you have them
38+
ALTER ROLE regress_replication NOREPLICATION;
39+
ALTER ROLE regress_replication REPLICATION;
40+
ALTER ROLE regress_bypassrls NOBYPASSRLS;
41+
ALTER ROLE regress_bypassrls BYPASSRLS;
42+
ALTER ROLE regress_createdb NOCREATEDB;
43+
ALTER ROLE regress_createdb CREATEDB;
44+
-- fail, can't toggle SUPERUSER
45+
ALTER ROLE regress_createdb SUPERUSER;
46+
ERROR: must be superuser to alter superuser roles or change superuser attribute
47+
ALTER ROLE regress_createdb NOSUPERUSER;
48+
ERROR: must be superuser to alter superuser roles or change superuser attribute
49+
-- ok, having CREATEROLE is enough to create users with these privileges
1850
CREATE ROLE regress_createrole CREATEROLE NOINHERIT;
1951
GRANT CREATE ON DATABASE regression TO regress_createrole WITH GRANT OPTION;
2052
CREATE ROLE regress_login LOGIN;
@@ -53,9 +85,9 @@ ERROR: permission denied to create database
5385
CREATE ROLE regress_plainrole;
5486
-- ok, roles with CREATEROLE can create new roles with it
5587
CREATE ROLE regress_rolecreator CREATEROLE;
56-
-- ok, roles with CREATEROLE can create new roles with privilege they lack
57-
CREATE ROLE regress_hasprivs CREATEDB CREATEROLE LOGIN INHERIT
58-
CONNECTION LIMIT 5;
88+
-- ok, roles with CREATEROLE can create new roles with different role
89+
-- attributes, including CREATEROLE
90+
CREATE ROLE regress_hasprivs CREATEROLE LOGIN INHERIT CONNECTION LIMIT 5;
5991
-- ok, we should be able to modify a role we created
6092
COMMENT ON ROLE regress_hasprivs IS 'some comment';
6193
ALTER ROLE regress_hasprivs RENAME TO regress_tenant;
@@ -164,6 +196,9 @@ DROP ROLE regress_plainrole;
164196
-- must revoke privileges before dropping role
165197
REVOKE CREATE ON DATABASE regression FROM regress_createrole CASCADE;
166198
-- ok, should be able to drop non-superuser roles we created
199+
DROP ROLE regress_replication_bypassrls;
200+
DROP ROLE regress_replication;
201+
DROP ROLE regress_bypassrls;
167202
DROP ROLE regress_createdb;
168203
DROP ROLE regress_createrole;
169204
DROP ROLE regress_login;

‎src/test/regress/sql/create_role.sql

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,47 @@
22
CREATE ROLE regress_role_super SUPERUSER;
33
CREATE ROLE regress_role_admin CREATEDB CREATEROLE REPLICATION BYPASSRLS;
44
GRANT CREATE ON DATABASE regression TO regress_role_admin WITH GRANT OPTION;
5+
CREATE ROLE regress_role_limited_admin CREATEROLE;
56
CREATE ROLE regress_role_normal;
67

7-
-- fail, only superusers can create users with these privileges
8-
SET SESSION AUTHORIZATION regress_role_admin;
8+
-- fail, CREATEROLE user can't give away role attributes without having them
9+
SET SESSION AUTHORIZATION regress_role_limited_admin;
910
CREATE ROLE regress_nosuch_superuser SUPERUSER;
1011
CREATE ROLE regress_nosuch_replication_bypassrls REPLICATION BYPASSRLS;
1112
CREATE ROLE regress_nosuch_replication REPLICATION;
1213
CREATE ROLE regress_nosuch_bypassrls BYPASSRLS;
14+
CREATE ROLE regress_nosuch_createdb CREATEDB;
1315

14-
-- ok, having CREATEROLE is enough to create users with these privileges
16+
-- ok, can create a role without any special attributes
17+
CREATE ROLE regress_role_limited;
18+
19+
-- fail, can't give it in any of the restricted attributes
20+
ALTER ROLE regress_role_limited SUPERUSER;
21+
ALTER ROLE regress_role_limited REPLICATION;
22+
ALTER ROLE regress_role_limited CREATEDB;
23+
ALTER ROLE regress_role_limited BYPASSRLS;
24+
DROP ROLE regress_role_limited;
25+
26+
-- ok, can give away these role attributes if you have them
27+
SET SESSION AUTHORIZATION regress_role_admin;
28+
CREATE ROLE regress_replication_bypassrls REPLICATION BYPASSRLS;
29+
CREATE ROLE regress_replication REPLICATION;
30+
CREATE ROLE regress_bypassrls BYPASSRLS;
1531
CREATE ROLE regress_createdb CREATEDB;
32+
33+
-- ok, can toggle these role attributes off and on if you have them
34+
ALTER ROLE regress_replication NOREPLICATION;
35+
ALTER ROLE regress_replication REPLICATION;
36+
ALTER ROLE regress_bypassrls NOBYPASSRLS;
37+
ALTER ROLE regress_bypassrls BYPASSRLS;
38+
ALTER ROLE regress_createdb NOCREATEDB;
39+
ALTER ROLE regress_createdb CREATEDB;
40+
41+
-- fail, can't toggle SUPERUSER
42+
ALTER ROLE regress_createdb SUPERUSER;
43+
ALTER ROLE regress_createdb NOSUPERUSER;
44+
45+
-- ok, having CREATEROLE is enough to create users with these privileges
1646
CREATE ROLE regress_createrole CREATEROLE NOINHERIT;
1747
GRANT CREATE ON DATABASE regression TO regress_createrole WITH GRANT OPTION;
1848
CREATE ROLE regress_login LOGIN;
@@ -56,9 +86,9 @@ CREATE ROLE regress_plainrole;
5686
-- ok, roles with CREATEROLE can create new roles with it
5787
CREATE ROLE regress_rolecreator CREATEROLE;
5888

59-
-- ok, roles with CREATEROLE can create new roles with privilege they lack
60-
CREATE ROLE regress_hasprivs CREATEDB CREATEROLE LOGIN INHERIT
61-
CONNECTION LIMIT 5;
89+
-- ok, roles with CREATEROLE can create new roles with different role
90+
-- attributes, including CREATEROLE
91+
CREATE ROLE regress_hasprivs CREATEROLE LOGIN INHERIT CONNECTION LIMIT 5;
6292

6393
-- ok, we should be able to modify a role we created
6494
COMMENT ON ROLE regress_hasprivs IS 'some comment';
@@ -150,6 +180,9 @@ DROP ROLE regress_plainrole;
150180
REVOKE CREATE ON DATABASE regression FROM regress_createrole CASCADE;
151181

152182
-- ok, should be able to drop non-superuser roles we created
183+
DROP ROLE regress_replication_bypassrls;
184+
DROP ROLE regress_replication;
185+
DROP ROLE regress_bypassrls;
153186
DROP ROLE regress_createdb;
154187
DROP ROLE regress_createrole;
155188
DROP ROLE regress_login;

0 commit comments

Comments
 (0)