From d949f35209b8de4723b0612ef823e0385ae6a338 Mon Sep 17 00:00:00 2001 From: Mark Dilger 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;". --> Constrain ordinary users to user-private schemas. To implement this, first issue 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 pg_catalog. This pattern is a secure schema - usage pattern unless an untrusted user is the database owner or holds - the CREATEROLE 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. If the database originated in an upgrade @@ -3170,8 +3167,7 @@ REVOKE CREATE ON SCHEMA public FROM PUBLIC; schema will be unsafe or unreliable. 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 CREATEROLE privilege. + pattern, this is secure unless an untrusted user is the database owner. 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 { 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. - Ordinary roles can only change their own password. + Role owners can change any of these settings on roles they directly or + indirectly own except SUPERUSER, + REPLICATION, and BYPASSRLS; 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. The second variant changes the name of the role. Database superusers can rename any role. - Roles having CREATEROLE 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 MD5-encrypted passwords use the role name as @@ -114,9 +114,9 @@ ALTER ROLE { role_specification | A - Superusers can change anyone's session defaults. Roles having - CREATEROLE 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 For most kinds of object, only the object's owner can set the comment. - Roles don't have owners, so the rule for COMMENT ON ROLE is - that you must be superuser to comment on a superuser role, or have the - CREATEROLE 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. 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! CREATEDB is specified, the role being defined will be allowed to create new databases. Specifying NOCREATEDB will deny a role the ability to - create databases. If not specified, - NOCREATEDB is the default. + create databases. Only roles with the CREATEDB + attribute may create roles with the CREATEDB + attribute. If not specified, NOCREATEDB is the + default. @@ -120,8 +122,6 @@ in sync when changing the above synopsis! These clauses determine whether a role will be permitted to create new roles (that is, execute CREATE ROLE). - A role with CREATEROLE privilege can also alter - and drop other roles. If not specified, NOCREATEROLE is the default. @@ -163,6 +163,8 @@ in sync when changing the above synopsis! NOLOGIN is the default, except when CREATE ROLE is invoked through its alternative spelling CREATE USER. + You must have the LOGIN attribute to create a new role + with the LOGIN attribute. @@ -194,8 +196,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. + You must have the BYPASSRLS attribute to create a + new role having the BYPASSRLS attribute. @@ -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 GRANT command to do that.) + + If not a superuser, the creating role must either own or have admin + privilege on each listed role. + @@ -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 group.) + + If not a superuser, the creating role must either own or have admin + privilege on each listed role. + @@ -313,6 +323,10 @@ in sync when changing the above synopsis! OPTION, giving them the right to grant membership in this role to others. + + If not a superuser, the creating role must either own or have admin + privilege on each listed role. + 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 ] name [, ... DROP ROLE removes the specified role(s). To drop a superuser role, you must be a superuser yourself; - to drop non-superuser roles, you must have CREATEROLE - privilege. + to drop non-superuser roles, you must own the target role. 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 dropuser removes an existing PostgreSQL user. - Only superusers and users with the CREATEROLE privilege can - remove PostgreSQL users. (To remove a - superuser, you must yourself be a superuser.) + A PostgreSQL user may only be removed by its + owner or by a superuser. (To remove a superuser, you must yourself be a + superuser.) 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 role_name [, ...] TO 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 CREATEROLE 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. 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 name; (except for superusers, since those bypass all permission checks). To create such a role, use CREATE ROLE name CREATEROLE. - A role with CREATEROLE 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; CREATEROLE is insufficient for that. @@ -246,11 +247,14 @@ CREATE USER name; - It is good practice to create a role that has the CREATEDB - and CREATEROLE privileges, but is not a superuser, and then + It is good practice to create a role that has the + CREATEDB, LOGIN and + CREATEROLE 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 + REPLICATION if it will create replication users, and + must have BYPASSRLS if it will create bypassrls users. @@ -387,15 +391,22 @@ RESET ROLE; The role attributes LOGIN, SUPERUSER, - CREATEDB, and CREATEROLE can be thought of as - special privileges, but they are never inherited as ordinary privileges - on database objects are. You must actually SET ROLE 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 + CREATEDB, REPLICATION, + BYPASSRLS, and CREATEROLE can be + thought of as special privileges, but they are never inherited as ordinary + privileges on database objects are. You must actually SET + ROLE 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 CREATEDB and CREATEROLE to the - admin role. Then a session connecting as role joe - would not have these privileges immediately, only after doing - SET ROLE admin. + admin role. Then a session connecting as role + joe would not have these privileges immediately, only + after doing SET ROLE admin. 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 + CREATEROLE may only create new roles with + LOGIN, CREATEDB, + REPLICATION, or BYPASSRLS if they + themselves have the same attribute. @@ -493,8 +504,7 @@ DROP ROLE doomed_role; PostgreSQL provides a set of predefined roles that provide access to certain, commonly needed, privileged capabilities - and information. Administrators (including roles that have the - CREATEROLE privilege) can GRANT these + and information. Administrators can GRANT these roles to users and/or other roles in their environment, providing those users with access to the specified capabilities and information. 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)