diff --git a/doc/src/sgml/ref/alter_role.sgml b/doc/src/sgml/ref/alter_role.sgml index fbb4612e707..ff2b88e9b60 100644 --- a/doc/src/sgml/ref/alter_role.sgml +++ b/doc/src/sgml/ref/alter_role.sgml @@ -70,11 +70,14 @@ ALTER ROLE { role_specification | A REVOKE 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 CREATEROLE privilege can change any of these - settings except SUPERUSER, REPLICATION, - and BYPASSRLS; but only for non-superuser and - non-replication roles for which they have been - granted ADMIN OPTION. + Non-superuser roles having CREATEROLE privilege can + change most of these properties, but only for non-superuser and + non-replication roles for which they have been granted + ADMIN OPTION. Non-superusers cannot change the + SUPERUSER property and can change the + CREATEDB, REPLICATION, and + BYPASSRLS properties only if they possess the + corresponding property themselves. Ordinary roles can only change their own password. diff --git a/doc/src/sgml/ref/create_role.sgml b/doc/src/sgml/ref/create_role.sgml index 7ce4e38b458..c101df6e2fd 100644 --- a/doc/src/sgml/ref/create_role.sgml +++ b/doc/src/sgml/ref/create_role.sgml @@ -109,6 +109,8 @@ in sync when changing the above synopsis! NOCREATEDB will deny a role the ability to create databases. If not specified, NOCREATEDB is the default. + Only superuser roles or roles with CREATEDB + can specify CREATEDB. @@ -188,8 +190,8 @@ in sync when changing the above synopsis! highly privileged role, and should only be used on roles actually used for replication. If not specified, NOREPLICATION is the default. - You must be a superuser to create a new role having the - REPLICATION attribute. + Only superuser roles or roles with REPLICATION + can specify REPLICATION. @@ -201,8 +203,8 @@ in sync when changing the above synopsis! These clauses determine whether a role bypasses every row-level security (RLS) policy. NOBYPASSRLS is the default. - You must be a superuser to create a new role having - the BYPASSRLS attribute. + Only superuser roles or roles with BYPASSRLS + can specify BYPASSRLS. @@ -390,19 +392,6 @@ in sync when changing the above synopsis! specified in the SQL standard. - - Be careful with the CREATEROLE privilege. There is no concept of - inheritance for the privileges of a CREATEROLE-role. That - means that even if a role does not have a certain privilege but is allowed - to create other roles, it can easily create another role with different - privileges than its own (except for creating roles with superuser - privileges). For example, if the role user has the - CREATEROLE privilege but not the CREATEDB privilege, - nonetheless it can create a new role with the CREATEDB - privilege. Therefore, regard roles that have the CREATEROLE - privilege as almost-superuser-roles. - - PostgreSQL includes a program that has diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index 518ffca09a0..1f4ce2fb9cf 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -121,7 +121,6 @@ static bool get_db_info(const char *name, LOCKMODE lockmode, Oid *dbTablespace, char **dbCollate, char **dbCtype, char **dbIculocale, char *dbLocProvider, char **dbCollversion); -static bool have_createdb_privilege(void); static void remove_dbtablespaces(Oid db_id); static bool check_db_file_conflict(Oid db_id); static int errdetail_busy_db(int notherbackends, int npreparedxacts); @@ -2742,7 +2741,7 @@ get_db_info(const char *name, LOCKMODE lockmode, } /* Check if current user has createdb privileges */ -static bool +bool have_createdb_privilege(void) { bool result = false; diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index 4d193a6f9a4..3a92e930c06 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -311,33 +311,28 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) bypassrls = boolVal(dbypassRLS->arg); /* Check some permissions first */ - if (issuper) + if (!superuser_arg(currentUserId)) { - if (!superuser()) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to create superusers"))); - } - else if (isreplication) - { - if (!superuser()) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to create replication users"))); - } - else if (bypassrls) - { - if (!superuser()) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to create bypassrls users"))); - } - else - { - if (!have_createrole_privilege()) + if (!has_createrole_privilege(currentUserId)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied to create role"))); + if (issuper) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to create superusers"))); + if (createdb && !have_createdb_privilege()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must have createdb permission to create createdb users"))); + if (isreplication && !has_rolreplication(currentUserId)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must have replication permission to create replication users"))); + if (bypassrls && !has_bypassrls_privilege(currentUserId)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must have bypassrls to create bypassrls users"))); } /* @@ -748,32 +743,11 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt) rolename = pstrdup(NameStr(authform->rolname)); roleid = authform->oid; - /* - * To mess with a superuser or replication role in any way you gotta be - * superuser. We also insist on superuser to change the BYPASSRLS - * property. - */ - if (authform->rolsuper || dissuper) - { - if (!superuser()) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to alter superuser roles or change superuser attribute"))); - } - else if (authform->rolreplication || disreplication) - { - if (!superuser()) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to alter replication roles or change replication attribute"))); - } - else if (dbypassRLS) - { - if (!superuser()) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to change bypassrls attribute"))); - } + /* To mess with a superuser in any way you gotta be superuser. */ + if (!superuser() && (authform->rolsuper || dissuper)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to alter superuser roles or change superuser attribute"))); /* * Most changes to a role require that you both have CREATEROLE privileges @@ -784,7 +758,7 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt) { /* things an unprivileged user certainly can't do */ if (dinherit || dcreaterole || dcreatedb || dcanlogin || dconnlimit || - dvalidUntil) + dvalidUntil || disreplication || dbypassRLS) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied"))); @@ -795,6 +769,26 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt) (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("must have CREATEROLE privilege to change another user's password"))); } + else if (!superuser()) + { + /* + * Even if you have both CREATEROLE and ADMIN OPTION on a role, you + * can only change the CREATEDB, REPLICATION, or BYPASSRLS attributes + * if they are set for your own role (or you are the superuser). + */ + if (dcreatedb && !have_createdb_privilege()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must have createdb privilege to change createdb attribute"))); + if (disreplication && !has_rolreplication(currentUserId)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must have replication privilege to change replication attribute"))); + if (dbypassRLS && !has_bypassrls_privilege(currentUserId)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must have bypassrls privilege to change bypassrls attribute"))); + } /* To add members to a role, you need ADMIN OPTION. */ if (drolemembers && !is_admin_of_role(currentUserId, roleid)) diff --git a/src/include/commands/dbcommands.h b/src/include/commands/dbcommands.h index 9b8818315ac..5fbc3ca7528 100644 --- a/src/include/commands/dbcommands.h +++ b/src/include/commands/dbcommands.h @@ -30,6 +30,7 @@ extern ObjectAddress AlterDatabaseOwner(const char *dbname, Oid newOwnerId); extern Oid get_database_oid(const char *dbname, bool missing_ok); extern char *get_database_name(Oid dbid); +extern bool have_createdb_privilege(void); extern void check_encoding_locale_matches(int encoding, const char *collate, const char *ctype); diff --git a/src/test/regress/expected/create_role.out b/src/test/regress/expected/create_role.out index cd49feabb3a..c8beb36bab9 100644 --- a/src/test/regress/expected/create_role.out +++ b/src/test/regress/expected/create_role.out @@ -2,19 +2,51 @@ CREATE ROLE regress_role_super SUPERUSER; CREATE ROLE regress_role_admin CREATEDB CREATEROLE REPLICATION BYPASSRLS; GRANT CREATE ON DATABASE regression TO regress_role_admin WITH GRANT OPTION; +CREATE ROLE regress_role_limited_admin CREATEROLE; CREATE ROLE regress_role_normal; --- fail, only superusers can create users with these privileges -SET SESSION AUTHORIZATION regress_role_admin; +-- fail, CREATEROLE user can't give away role attributes without having them +SET SESSION AUTHORIZATION regress_role_limited_admin; CREATE ROLE regress_nosuch_superuser SUPERUSER; ERROR: must be superuser to create superusers CREATE ROLE regress_nosuch_replication_bypassrls REPLICATION BYPASSRLS; -ERROR: must be superuser to create replication users +ERROR: must have replication permission to create replication users CREATE ROLE regress_nosuch_replication REPLICATION; -ERROR: must be superuser to create replication users +ERROR: must have replication permission to create replication users 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 +ERROR: must have bypassrls to create bypassrls users +CREATE ROLE regress_nosuch_createdb CREATEDB; +ERROR: must have createdb permission to create createdb users +-- ok, can create a role without any special attributes +CREATE ROLE regress_role_limited; +-- fail, can't give it in any of the restricted attributes +ALTER ROLE regress_role_limited SUPERUSER; +ERROR: must be superuser to alter superuser roles or change superuser attribute +ALTER ROLE regress_role_limited REPLICATION; +ERROR: must have replication privilege to change replication attribute +ALTER ROLE regress_role_limited CREATEDB; +ERROR: must have createdb privilege to change createdb attribute +ALTER ROLE regress_role_limited BYPASSRLS; +ERROR: must have bypassrls privilege to change bypassrls attribute +DROP ROLE regress_role_limited; +-- ok, can give away these role attributes if you have them +SET SESSION AUTHORIZATION regress_role_admin; +CREATE ROLE regress_replication_bypassrls REPLICATION BYPASSRLS; +CREATE ROLE regress_replication REPLICATION; +CREATE ROLE regress_bypassrls BYPASSRLS; CREATE ROLE regress_createdb CREATEDB; +-- ok, can toggle these role attributes off and on if you have them +ALTER ROLE regress_replication NOREPLICATION; +ALTER ROLE regress_replication REPLICATION; +ALTER ROLE regress_bypassrls NOBYPASSRLS; +ALTER ROLE regress_bypassrls BYPASSRLS; +ALTER ROLE regress_createdb NOCREATEDB; +ALTER ROLE regress_createdb CREATEDB; +-- fail, can't toggle SUPERUSER +ALTER ROLE regress_createdb SUPERUSER; +ERROR: must be superuser to alter superuser roles or change superuser attribute +ALTER ROLE regress_createdb NOSUPERUSER; +ERROR: must be superuser to alter superuser roles or change superuser attribute +-- ok, having CREATEROLE is enough to create users with these privileges CREATE ROLE regress_createrole CREATEROLE NOINHERIT; GRANT CREATE ON DATABASE regression TO regress_createrole WITH GRANT OPTION; CREATE ROLE regress_login LOGIN; @@ -53,9 +85,9 @@ 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 -CREATE ROLE regress_hasprivs CREATEDB CREATEROLE LOGIN INHERIT - CONNECTION LIMIT 5; +-- ok, roles with CREATEROLE can create new roles with different role +-- attributes, including CREATEROLE +CREATE ROLE regress_hasprivs 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; @@ -164,6 +196,9 @@ DROP ROLE regress_plainrole; -- must revoke privileges before dropping role REVOKE CREATE ON DATABASE regression FROM regress_createrole CASCADE; -- ok, should be able to drop non-superuser roles we created +DROP ROLE regress_replication_bypassrls; +DROP ROLE regress_replication; +DROP ROLE regress_bypassrls; DROP ROLE regress_createdb; DROP ROLE regress_createrole; DROP ROLE regress_login; diff --git a/src/test/regress/sql/create_role.sql b/src/test/regress/sql/create_role.sql index 6b90336da2e..19031b4612b 100644 --- a/src/test/regress/sql/create_role.sql +++ b/src/test/regress/sql/create_role.sql @@ -2,17 +2,47 @@ CREATE ROLE regress_role_super SUPERUSER; CREATE ROLE regress_role_admin CREATEDB CREATEROLE REPLICATION BYPASSRLS; GRANT CREATE ON DATABASE regression TO regress_role_admin WITH GRANT OPTION; +CREATE ROLE regress_role_limited_admin CREATEROLE; CREATE ROLE regress_role_normal; --- fail, only superusers can create users with these privileges -SET SESSION AUTHORIZATION regress_role_admin; +-- fail, CREATEROLE user can't give away role attributes without having them +SET SESSION AUTHORIZATION regress_role_limited_admin; CREATE ROLE regress_nosuch_superuser SUPERUSER; CREATE ROLE regress_nosuch_replication_bypassrls REPLICATION BYPASSRLS; CREATE ROLE regress_nosuch_replication REPLICATION; CREATE ROLE regress_nosuch_bypassrls BYPASSRLS; +CREATE ROLE regress_nosuch_createdb CREATEDB; + +-- ok, can create a role without any special attributes +CREATE ROLE regress_role_limited; + +-- fail, can't give it in any of the restricted attributes +ALTER ROLE regress_role_limited SUPERUSER; +ALTER ROLE regress_role_limited REPLICATION; +ALTER ROLE regress_role_limited CREATEDB; +ALTER ROLE regress_role_limited BYPASSRLS; +DROP ROLE regress_role_limited; + +-- ok, can give away these role attributes if you have them +SET SESSION AUTHORIZATION regress_role_admin; +CREATE ROLE regress_replication_bypassrls REPLICATION BYPASSRLS; +CREATE ROLE regress_replication REPLICATION; +CREATE ROLE regress_bypassrls BYPASSRLS; +CREATE ROLE regress_createdb CREATEDB; + +-- ok, can toggle these role attributes off and on if you have them +ALTER ROLE regress_replication NOREPLICATION; +ALTER ROLE regress_replication REPLICATION; +ALTER ROLE regress_bypassrls NOBYPASSRLS; +ALTER ROLE regress_bypassrls BYPASSRLS; +ALTER ROLE regress_createdb NOCREATEDB; +ALTER ROLE regress_createdb CREATEDB; + +-- fail, can't toggle SUPERUSER +ALTER ROLE regress_createdb SUPERUSER; +ALTER ROLE regress_createdb NOSUPERUSER; -- ok, having CREATEROLE is enough to create users with these privileges -CREATE ROLE regress_createdb CREATEDB; CREATE ROLE regress_createrole CREATEROLE NOINHERIT; GRANT CREATE ON DATABASE regression TO regress_createrole WITH GRANT OPTION; CREATE ROLE regress_login LOGIN; @@ -56,9 +86,9 @@ 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_hasprivs CREATEDB CREATEROLE LOGIN INHERIT - CONNECTION LIMIT 5; +-- ok, roles with CREATEROLE can create new roles with different role +-- attributes, including CREATEROLE +CREATE ROLE regress_hasprivs 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'; @@ -150,6 +180,9 @@ DROP ROLE regress_plainrole; REVOKE CREATE ON DATABASE regression FROM regress_createrole CASCADE; -- ok, should be able to drop non-superuser roles we created +DROP ROLE regress_replication_bypassrls; +DROP ROLE regress_replication; +DROP ROLE regress_bypassrls; DROP ROLE regress_createdb; DROP ROLE regress_createrole; DROP ROLE regress_login;