From 08b59a68ed3a333bfb92de20f6ea74ccbc42b652 Mon Sep 17 00:00:00 2001 From: Mark Dilger Date: Tue, 25 Jan 2022 11:15:19 -0800 Subject: [PATCH v7 1/2] Add owners to roles All roles now have owners. By default, created roles belong to the role that created them, and initdb-time roles are owned by the bootstrap superuser. Role ownership is transitive; if role A owns role B, and role B owns role C, then role A can act as the owner of role C. Roles may drop or alter roles that they own, and have all privileges that any role they own has. --- doc/src/sgml/ref/create_role.sgml | 23 ++- src/backend/catalog/aclchk.c | 30 ++- src/backend/catalog/objectaddress.c | 22 +-- src/backend/catalog/pg_shdepend.c | 5 + src/backend/catalog/system_views.sql | 1 + src/backend/commands/alter.c | 3 + src/backend/commands/schemacmds.c | 10 +- src/backend/commands/user.c | 180 +++++++++++++++++- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/parser/gram.y | 19 ++ src/backend/utils/adt/acl.c | 118 ++++++++++++ src/bin/psql/describe.c | 12 ++ src/include/catalog/pg_authid.h | 1 + src/include/commands/user.h | 2 + src/include/nodes/parsenodes.h | 1 + src/include/utils/acl.h | 2 + .../expected/dummy_seclabel.out | 12 +- .../dummy_seclabel/sql/dummy_seclabel.sql | 12 +- .../unsafe_tests/expected/rolenames.out | 6 +- .../modules/unsafe_tests/sql/rolenames.sql | 3 +- src/test/regress/expected/create_role.out | 177 ++++++++++++++--- src/test/regress/expected/oidjoins.out | 1 + src/test/regress/expected/privileges.out | 9 +- src/test/regress/expected/rules.out | 1 + src/test/regress/sql/create_role.sql | 100 ++++++++-- src/test/regress/sql/privileges.sql | 10 +- 27 files changed, 657 insertions(+), 105 deletions(-) diff --git a/doc/src/sgml/ref/create_role.sgml b/doc/src/sgml/ref/create_role.sgml index b6a4ea1f72..1e9347b2ce 100644 --- a/doc/src/sgml/ref/create_role.sgml +++ b/doc/src/sgml/ref/create_role.sgml @@ -21,9 +21,16 @@ PostgreSQL documentation -CREATE ROLE name [ [ WITH ] option [ ... ] ] +CREATE ROLE name [ AUTHORIZATION role_specification ] [ [ WITH ] option [ ... ] ] -where option can be: +where role_specification can be: + + user_name + | CURRENT_ROLE + | CURRENT_USER + | SESSION_USER + +and option can be: SUPERUSER | NOSUPERUSER | CREATEDB | NOCREATEDB @@ -83,6 +90,18 @@ in sync when changing the above synopsis! + + user_name + + + The role name of the user who will own the new role. If omitted, + defaults to the user executing the command. To create a role + owned by another role, you must be a direct or indirect member of + that role, directly or indirectly own that role, or be a superuser. + + + + SUPERUSER NOSUPERUSER diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c index 1dd03a8e51..1e0ee503e4 100644 --- a/src/backend/catalog/aclchk.c +++ b/src/backend/catalog/aclchk.c @@ -3385,6 +3385,9 @@ aclcheck_error(AclResult aclerr, ObjectType objtype, case OBJECT_PUBLICATION: msg = gettext_noop("permission denied for publication %s"); break; + case OBJECT_ROLE: + msg = gettext_noop("permission denied for role %s"); + break; case OBJECT_ROUTINE: msg = gettext_noop("permission denied for routine %s"); break; @@ -3429,7 +3432,6 @@ aclcheck_error(AclResult aclerr, ObjectType objtype, case OBJECT_DOMCONSTRAINT: case OBJECT_PUBLICATION_NAMESPACE: case OBJECT_PUBLICATION_REL: - case OBJECT_ROLE: case OBJECT_RULE: case OBJECT_TABCONSTRAINT: case OBJECT_TRANSFORM: @@ -3511,6 +3513,9 @@ aclcheck_error(AclResult aclerr, ObjectType objtype, case OBJECT_PUBLICATION: msg = gettext_noop("must be owner of publication %s"); break; + case OBJECT_ROLE: + msg = gettext_noop("must be owner of role %s"); + break; case OBJECT_ROUTINE: msg = gettext_noop("must be owner of routine %s"); break; @@ -3569,7 +3574,6 @@ aclcheck_error(AclResult aclerr, ObjectType objtype, case OBJECT_DOMCONSTRAINT: case OBJECT_PUBLICATION_NAMESPACE: case OBJECT_PUBLICATION_REL: - case OBJECT_ROLE: case OBJECT_TRANSFORM: case OBJECT_TSPARSER: case OBJECT_TSTEMPLATE: @@ -5430,16 +5434,22 @@ pg_statistics_object_ownercheck(Oid stat_oid, Oid roleid) return has_privs_of_role(roleid, ownerId); } +/* + * Ownership check for a role (specified by OID) + */ +bool +pg_role_ownercheck(Oid owned_role_oid, Oid owner_roleid) +{ + /* Superusers bypass all permission checking. */ + if (superuser_arg(owner_roleid)) + return true; + + /* Otherwise, check the role ownership hierarchy */ + return is_owner_of_role_nosuper(owner_roleid, owned_role_oid); +} + /* * Check whether specified role has CREATEROLE privilege (or is a superuser) - * - * Note: roles do not have owners per se; instead we use this test in - * places where an ownership-like permissions test is needed for a role. - * Be sure to apply it to the role trying to do the operation, not the - * role being operated on! Also note that this generally should not be - * considered enough privilege if the target role is a superuser. - * (We don't handle that consideration here because we want to give a - * separate error message for such cases, so the caller has to deal with it.) */ bool has_createrole_privilege(Oid roleid) diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index f30c742d48..1a47f28829 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -2596,25 +2596,9 @@ check_object_ownership(Oid roleid, ObjectType objtype, ObjectAddress address, NameListToString(castNode(List, object))); break; case OBJECT_ROLE: - - /* - * We treat roles as being "owned" by those with CREATEROLE priv, - * except that superusers are only owned by superusers. - */ - if (superuser_arg(address.objectId)) - { - if (!superuser_arg(roleid)) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser"))); - } - else - { - if (!has_createrole_privilege(roleid)) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must have CREATEROLE privilege"))); - } + if (!pg_role_ownercheck(address.objectId, roleid)) + aclcheck_error(ACLCHECK_NOT_OWNER, objtype, + strVal(object)); break; case OBJECT_TSPARSER: case OBJECT_TSTEMPLATE: diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_shdepend.c index 3e8fa008b9..52f69ad76e 100644 --- a/src/backend/catalog/pg_shdepend.c +++ b/src/backend/catalog/pg_shdepend.c @@ -61,6 +61,7 @@ #include "commands/tablecmds.h" #include "commands/tablespace.h" #include "commands/typecmds.h" +#include "commands/user.h" #include "miscadmin.h" #include "storage/lmgr.h" #include "utils/acl.h" @@ -1578,6 +1579,10 @@ shdepReassignOwned(List *roleids, Oid newrole) AlterSubscriptionOwner_oid(sdepForm->objid, newrole); break; + case AuthIdRelationId: + AlterRoleOwner_oid(sdepForm->objid, newrole); + break; + /* Generic alter owner cases */ case CollationRelationId: case ConversionRelationId: diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 3cb69b1f87..057d17460b 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -17,6 +17,7 @@ CREATE VIEW pg_roles AS SELECT rolname, + pg_get_userbyid(rolowner) AS rolowner, rolsuper, rolinherit, rolcreaterole, diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c index 1f64c8aa51..e6c7c84c87 100644 --- a/src/backend/commands/alter.c +++ b/src/backend/commands/alter.c @@ -840,6 +840,9 @@ ExecAlterOwnerStmt(AlterOwnerStmt *stmt) case OBJECT_DATABASE: return AlterDatabaseOwner(strVal(stmt->object), newowner); + case OBJECT_ROLE: + return AlterRoleOwner(strVal(stmt->object), newowner); + case OBJECT_SCHEMA: return AlterSchemaOwner(strVal(stmt->object), newowner); diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c index 984000a5bc..320e8378c4 100644 --- a/src/backend/commands/schemacmds.c +++ b/src/backend/commands/schemacmds.c @@ -363,11 +363,11 @@ AlterSchemaOwner_internal(HeapTuple tup, Relation rel, Oid newOwnerId) /* * must have create-schema rights * - * NOTE: This is different from other alter-owner checks in that the - * current user is checked for create privileges instead of the - * destination owner. This is consistent with the CREATE case for - * schemas. Because superusers will always have this right, we need - * no special case for them. + * NOTE: alter-schema and alter-role are different from other + * alter-owner checks in that the current user is checked for create + * privileges instead of the destination owner. Alter-schema is + * consistent with the CREATE case for schemas. Because superusers + * will always have this right, we need no special case for them. */ aclresult = pg_database_aclcheck(MyDatabaseId, GetUserId(), ACL_CREATE); diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index f9d3c1246b..3726e40f36 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -55,6 +55,8 @@ static void AddRoleMems(const char *rolename, Oid roleid, static void DelRoleMems(const char *rolename, Oid roleid, List *memberSpecs, List *memberIds, bool admin_opt); +static void AlterRoleOwner_internal(HeapTuple tup, Relation rel, + Oid newOwnerId); /* Check if current user has createrole privileges */ @@ -77,6 +79,9 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) Datum new_record[Natts_pg_authid]; bool new_record_nulls[Natts_pg_authid]; Oid roleid; + Oid owner_uid; + Oid saved_uid; + int save_sec_context; ListCell *item; ListCell *option; char *password = NULL; /* user password */ @@ -108,6 +113,19 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) DefElem *dvalidUntil = NULL; DefElem *dbypassRLS = NULL; + GetUserIdAndSecContext(&saved_uid, &save_sec_context); + + /* + * Who is supposed to own the new role? + */ + if (stmt->authrole) + { + owner_uid = get_rolespec_oid(stmt->authrole, false); + check_is_member_of_role(saved_uid, owner_uid); + } + else + owner_uid = saved_uid; + /* The defaults can vary depending on the original statement type */ switch (stmt->stmt_type) { @@ -254,6 +272,10 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("must be superuser to create superusers"))); + if (!superuser_arg(owner_uid)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to own superusers"))); } else if (isreplication) { @@ -310,6 +332,19 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) errmsg("role \"%s\" already exists", stmt->role))); + /* + * If the requested authorization is different from the current user, + * temporarily set the current user so that the object(s) will be created + * with the correct ownership. + * + * (The setting will be restored at the end of this routine, or in case of + * error, transaction abort will clean things up.) + */ + if (saved_uid != owner_uid) + SetUserIdAndSecContext(owner_uid, + save_sec_context | SECURITY_LOCAL_USERID_CHANGE); + + /* Convert validuntil to internal form */ if (validUntil) { @@ -345,6 +380,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) DirectFunctionCall1(namein, CStringGetDatum(stmt->role)); new_record[Anum_pg_authid_rolsuper - 1] = BoolGetDatum(issuper); + new_record[Anum_pg_authid_rolowner - 1] = ObjectIdGetDatum(owner_uid); new_record[Anum_pg_authid_rolinherit - 1] = BoolGetDatum(inherit); new_record[Anum_pg_authid_rolcreaterole - 1] = BoolGetDatum(createrole); new_record[Anum_pg_authid_rolcreatedb - 1] = BoolGetDatum(createdb); @@ -422,6 +458,8 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) */ CatalogTupleInsert(pg_authid_rel, tuple); + recordDependencyOnOwner(AuthIdRelationId, roleid, owner_uid); + /* * Advance command counter so we can see new record; else tests in * AddRoleMems may fail. @@ -478,6 +516,9 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) */ table_close(pg_authid_rel, NoLock); + /* Reset current user and security context */ + SetUserIdAndSecContext(saved_uid, save_sec_context); + return roleid; } @@ -655,7 +696,8 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt) { /* check the rest */ if (dinherit || dcreaterole || dcreatedb || dcanlogin || dconnlimit || - drolemembers || dvalidUntil || !dpassword || roleid != GetUserId()) + drolemembers || dvalidUntil || !dpassword || + !pg_role_ownercheck(roleid, GetUserId())) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied"))); @@ -860,7 +902,8 @@ AlterRoleSet(AlterRoleSetStmt *stmt) } else { - if (!have_createrole_privilege() && roleid != GetUserId()) + if (!have_createrole_privilege() && + !pg_role_ownercheck(roleid, GetUserId())) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied"))); @@ -912,11 +955,6 @@ DropRole(DropRoleStmt *stmt) pg_auth_members_rel; ListCell *item; - if (!have_createrole_privilege()) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("permission denied to drop role"))); - /* * Scan the pg_authid relation to find the Oid of the role(s) to be * deleted. @@ -988,6 +1026,12 @@ DropRole(DropRoleStmt *stmt) (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("must be superuser to drop superusers"))); + if (!have_createrole_privilege() && + !pg_role_ownercheck(roleid, GetUserId())) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied to drop role"))); + /* DROP hook for the role being removed */ InvokeObjectDropHook(AuthIdRelationId, roleid, 0); @@ -1051,8 +1095,9 @@ DropRole(DropRoleStmt *stmt) systable_endscan(sscan); /* - * Remove any comments or security labels on this role. + * Remove any dependencies, comments or security labels on this role. */ + deleteSharedDependencyRecordsFor(AuthIdRelationId, roleid, 0); DeleteSharedComments(roleid, AuthIdRelationId); DeleteSharedSecurityLabel(roleid, AuthIdRelationId); @@ -1648,3 +1693,122 @@ DelRoleMems(const char *rolename, Oid roleid, */ table_close(pg_authmem_rel, NoLock); } + +/* + * Change role owner + */ +ObjectAddress +AlterRoleOwner(const char *name, Oid newOwnerId) +{ + Oid roleid; + HeapTuple tup; + Relation rel; + ObjectAddress address; + Form_pg_authid authform; + + rel = table_open(AuthIdRelationId, RowExclusiveLock); + + tup = SearchSysCache1(AUTHNAME, CStringGetDatum(name)); + if (!HeapTupleIsValid(tup)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("role \"%s\" does not exist", name))); + + authform = (Form_pg_authid) GETSTRUCT(tup); + roleid = authform->oid; + + AlterRoleOwner_internal(tup, rel, newOwnerId); + + ObjectAddressSet(address, AuthIdRelationId, roleid); + + ReleaseSysCache(tup); + + table_close(rel, RowExclusiveLock); + + return address; +} + +void +AlterRoleOwner_oid(Oid roleOid, Oid newOwnerId) +{ + HeapTuple tup; + Relation rel; + + rel = table_open(AuthIdRelationId, RowExclusiveLock); + + tup = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleOid)); + if (!HeapTupleIsValid(tup)) + elog(ERROR, "cache lookup failed for role %u", roleOid); + + AlterRoleOwner_internal(tup, rel, newOwnerId); + + ReleaseSysCache(tup); + + table_close(rel, RowExclusiveLock); +} + +static void +AlterRoleOwner_internal(HeapTuple tup, Relation rel, Oid newOwnerId) +{ + Form_pg_authid authForm; + + Assert(tup->t_tableOid == AuthIdRelationId); + Assert(RelationGetRelid(rel) == AuthIdRelationId); + + authForm = (Form_pg_authid) GETSTRUCT(tup); + + /* + * If the new owner is the same as the existing owner, consider the + * command to have succeeded. This is for dump restoration purposes. + */ + if (authForm->rolowner != newOwnerId) + { + /* Otherwise, must be owner of the existing object */ + if (!pg_role_ownercheck(authForm->oid, GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_ROLE, + NameStr(authForm->rolname)); + + /* Must be able to become new owner */ + check_is_member_of_role(GetUserId(), newOwnerId); + + /* + * must have CREATEROLE rights + * + * NOTE: Alter-role and alter-schema are different from other + * alter-owner checks in that the current user is checked for create + * privileges instead of the destination owner. Alter-role is + * consistent with the CREATE case for roles. Because superusers will + * always have this right, we need no special case for them. + */ + if (!have_createrole_privilege()) + aclcheck_error(ACLCHECK_NO_PRIV, OBJECT_ROLE, + NameStr(authForm->rolname)); + + /* Only the bootstrap superuser is allowed to own itself. */ + if (newOwnerId != BOOTSTRAP_SUPERUSERID && authForm->oid == newOwnerId) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("role may not own itself"))); + + /* + * Must not create cycles in the role ownership hierarchy. If this + * role owns (directly or indirectly) the proposed new owner, disallow + * the ownership transfer. + */ + if (is_owner_of_role_nosuper(authForm->oid, newOwnerId)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("role \"%s\" may not both own and be owned by role \"%s\"", + NameStr(authForm->rolname), + GetUserNameFromId(newOwnerId, false)))); + + authForm->rolowner = newOwnerId; + CatalogTupleUpdate(rel, &tup->t_self, tup); + + /* Update owner dependency reference */ + changeDependencyOnOwner(AuthIdRelationId, authForm->oid, newOwnerId); + } + + InvokeObjectPostAlterHook(AuthIdRelationId, + authForm->oid, 0); +} diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 90b5da51c9..f43c92a70f 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -4522,6 +4522,7 @@ _copyCreateRoleStmt(const CreateRoleStmt *from) COPY_SCALAR_FIELD(stmt_type); COPY_STRING_FIELD(role); + COPY_NODE_FIELD(authrole); COPY_NODE_FIELD(options); return newnode; diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 06345da3ba..328f155208 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -2128,6 +2128,7 @@ _equalCreateRoleStmt(const CreateRoleStmt *a, const CreateRoleStmt *b) { COMPARE_SCALAR_FIELD(stmt_type); COMPARE_STRING_FIELD(role); + COMPARE_NODE_FIELD(authrole); COMPARE_NODE_FIELD(options); return true; diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index b5966712ce..024d96470f 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -1077,9 +1077,20 @@ CreateRoleStmt: CreateRoleStmt *n = makeNode(CreateRoleStmt); n->stmt_type = ROLESTMT_ROLE; n->role = $3; + n->authrole = NULL; n->options = $5; $$ = (Node *)n; } + | + CREATE ROLE RoleId AUTHORIZATION RoleSpec opt_with OptRoleList + { + CreateRoleStmt *n = makeNode(CreateRoleStmt); + n->stmt_type = ROLESTMT_ROLE; + n->role = $3; + n->authrole = $5; + n->options = $7; + $$ = (Node *)n; + } ; @@ -9585,6 +9596,14 @@ AlterOwnerStmt: ALTER AGGREGATE aggregate_with_argtypes OWNER TO RoleSpec n->newowner = $6; $$ = (Node *)n; } + | ALTER ROLE name OWNER TO RoleSpec + { + AlterOwnerStmt *n = makeNode(AlterOwnerStmt); + n->objectType = OBJECT_ROLE; + n->object = (Node *) makeString($3); + n->newowner = $6; + $$ = (Node *)n; + } | ALTER ROUTINE function_with_argtypes OWNER TO RoleSpec { AlterOwnerStmt *n = makeNode(AlterOwnerStmt); diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c index 0a16f8156c..97336db058 100644 --- a/src/backend/utils/adt/acl.c +++ b/src/backend/utils/adt/acl.c @@ -4832,6 +4832,111 @@ roles_is_member_of(Oid roleid, enum RoleRecurseType type, } +/* + * Get a list of roles which own the given role, directly or indirectly. + * + * Each role has only one direct owner. The returned list contains the given + * role's owner, that role's owner, etc., up to the top of the ownership + * hierarchy, which is always the bootstrap superuser. + * + * Raises an error if any role ownership invariant is violated. Returns NIL if + * the given roleid is invalid. + */ +static List * +roles_is_owned_by(Oid roleid) +{ + List *owners_list = NIL; + Oid role_oid = roleid; + + /* + * Start with the current role and follow the ownership chain upwards until + * we reach the bootstrap superuser. To defend against getting into an + * infinite loop, we must check for ownership cycles. We choose to perform + * other corruption checks on the ownership structure while iterating, too. + */ + while (OidIsValid(role_oid)) + { + HeapTuple tuple; + Form_pg_authid authform; + Oid owner_oid; + + /* Find the owner of the current iteration's role */ + tuple = SearchSysCache1(AUTHOID, ObjectIdGetDatum(role_oid)); + if (!HeapTupleIsValid(tuple)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("role with OID %u does not exist", role_oid))); + + authform = (Form_pg_authid) GETSTRUCT(tuple); + owner_oid = authform->rolowner; + + /* + * Roles must necessarily have owners. Even the bootstrap user has an + * owner. (It owns itself). + */ + if (!OidIsValid(owner_oid)) + ereport(ERROR, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("role \"%s\" with OID %u has invalid owner", + NameStr(authform->rolname), authform->oid))); + + /* The bootstrap user must own itself */ + if (authform->oid == BOOTSTRAP_SUPERUSERID && + owner_oid != BOOTSTRAP_SUPERUSERID) + ereport(ERROR, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("role \"%s\" with OID %u owned by role with OID %u", + NameStr(authform->rolname), authform->oid, + authform->rolowner))); + + /* + * Roles other than the bootstrap user must not be their own direct + * owners. + */ + if (authform->oid != BOOTSTRAP_SUPERUSERID && + authform->oid == owner_oid) + ereport(ERROR, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("role \"%s\" with OID %u owns itself", + NameStr(authform->rolname), authform->oid))); + + ReleaseSysCache(tuple); + + /* If we have reached the bootstrap user, we're done. */ + if (role_oid == BOOTSTRAP_SUPERUSERID) + { + if (!owners_list) + owners_list = lappend_oid(owners_list, owner_oid); + break; + } + + /* + * For all other users, check they do not own themselves indirectly + * through an ownership cycle. + * + * Scanning the list each time through this loop results in overall + * quadratic work in the depth of the ownership chain, but we're + * not on a critical performance path, nor do we expect ownership + * hierarchies to be deep. + */ + if (owners_list && list_member_oid(owners_list, + ObjectIdGetDatum(owner_oid))) + ereport(ERROR, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("role \"%s\" with OID %u indirectly owns itself", + GetUserNameFromId(owner_oid, false), + owner_oid))); + + /* Done with sanity checks. Add this owner to the list. */ + owners_list = lappend_oid(owners_list, owner_oid); + + /* Otherwise, iterate on this iteration's owner_oid. */ + role_oid = owner_oid; + } + + return owners_list; +} + /* * Does member have the privileges of role (directly or indirectly)? * @@ -4850,6 +4955,10 @@ has_privs_of_role(Oid member, Oid role) if (superuser_arg(member)) return true; + /* Owners of roles have every privilege the owned role has */ + if (pg_role_ownercheck(role, member)) + return true; + /* * Find all the roles that member has the privileges of, including * multi-level recursion, then see if target role is any one of them. @@ -4921,6 +5030,15 @@ is_member_of_role_nosuper(Oid member, Oid role) role); } +/* + * Is owner a direct or indirect owner of the role, not considering + * superuserness? + */ +bool +is_owner_of_role_nosuper(Oid owner, Oid role) +{ + return list_member_oid(roles_is_owned_by(role), owner); +} /* * Is member an admin of role? That is, is member the role itself (subject to diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index 346cd92793..406138609d 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -3518,6 +3518,12 @@ describeRoles(const char *pattern, bool verbose, bool showSystem) appendPQExpBufferStr(&buf, "\n, r.rolbypassrls"); } + if (pset.sversion >= 150000) + { + appendPQExpBufferStr(&buf, "\n, r.rolowner"); + ncols++; + } + appendPQExpBufferStr(&buf, "\nFROM pg_catalog.pg_roles r\n"); if (!showSystem && !pattern) @@ -3538,6 +3544,8 @@ describeRoles(const char *pattern, bool verbose, bool showSystem) printTableInit(&cont, &myopt, _("List of roles"), ncols, nrows); printTableAddHeader(&cont, gettext_noop("Role name"), true, align); + if (pset.sversion >= 150000) + printTableAddHeader(&cont, gettext_noop("Owner"), true, align); printTableAddHeader(&cont, gettext_noop("Attributes"), true, align); /* ignores implicit memberships from superuser & pg_database_owner */ printTableAddHeader(&cont, gettext_noop("Member of"), true, align); @@ -3549,6 +3557,10 @@ describeRoles(const char *pattern, bool verbose, bool showSystem) { printTableAddCell(&cont, PQgetvalue(res, i, 0), false, false); + if (pset.sversion >= 150000) + printTableAddCell(&cont, PQgetvalue(res, i, (verbose ? 12 : 11)), + false, false); + resetPQExpBuffer(&buf); if (strcmp(PQgetvalue(res, i, 1), "t") == 0) add_role_attribute(&buf, _("Superuser")); diff --git a/src/include/catalog/pg_authid.h b/src/include/catalog/pg_authid.h index 4b65e39a1f..3af0f3908c 100644 --- a/src/include/catalog/pg_authid.h +++ b/src/include/catalog/pg_authid.h @@ -32,6 +32,7 @@ CATALOG(pg_authid,1260,AuthIdRelationId) BKI_SHARED_RELATION BKI_ROWTYPE_OID(284 { Oid oid; /* oid */ NameData rolname; /* name of role */ + Oid rolowner BKI_DEFAULT(POSTGRES) BKI_LOOKUP(pg_authid); /* owner of this role */ bool rolsuper; /* read this field via superuser() only! */ bool rolinherit; /* inherit privileges from other roles? */ bool rolcreaterole; /* allowed to create more roles? */ diff --git a/src/include/commands/user.h b/src/include/commands/user.h index 0b7a3cd65f..c32127e41e 100644 --- a/src/include/commands/user.h +++ b/src/include/commands/user.h @@ -33,5 +33,7 @@ extern ObjectAddress RenameRole(const char *oldname, const char *newname); extern void DropOwnedObjects(DropOwnedStmt *stmt); extern void ReassignOwnedObjects(ReassignOwnedStmt *stmt); extern List *roleSpecsToIds(List *memberNames); +extern ObjectAddress AlterRoleOwner(const char *name, Oid newOwnerId); +extern void AlterRoleOwner_oid(Oid roleOid, Oid newOwnerId); #endif /* USER_H */ diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 3e9bdc781f..b9d124d38f 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2623,6 +2623,7 @@ typedef struct CreateRoleStmt NodeTag type; RoleStmtType stmt_type; /* ROLE/USER/GROUP */ char *role; /* role name */ + RoleSpec *authrole; /* the owner of the created role */ List *options; /* List of DefElem nodes */ } CreateRoleStmt; diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h index 1ce4c5556e..572cae0f27 100644 --- a/src/include/utils/acl.h +++ b/src/include/utils/acl.h @@ -209,6 +209,7 @@ extern bool has_privs_of_role(Oid member, Oid role); extern bool is_member_of_role(Oid member, Oid role); extern bool is_member_of_role_nosuper(Oid member, Oid role); extern bool is_admin_of_role(Oid member, Oid role); +extern bool is_owner_of_role_nosuper(Oid owner, Oid role); extern void check_is_member_of_role(Oid member, Oid role); extern Oid get_role_oid(const char *rolename, bool missing_ok); extern Oid get_role_oid_or_public(const char *rolename); @@ -316,6 +317,7 @@ extern bool pg_extension_ownercheck(Oid ext_oid, Oid roleid); extern bool pg_publication_ownercheck(Oid pub_oid, Oid roleid); extern bool pg_subscription_ownercheck(Oid sub_oid, Oid roleid); 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); diff --git a/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out b/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out index b2d898a7d1..93cf82b750 100644 --- a/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out +++ b/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out @@ -7,8 +7,11 @@ SET client_min_messages TO 'warning'; DROP ROLE IF EXISTS regress_dummy_seclabel_user1; DROP ROLE IF EXISTS regress_dummy_seclabel_user2; RESET client_min_messages; -CREATE USER regress_dummy_seclabel_user1 WITH CREATEROLE; +CREATE USER regress_dummy_seclabel_user0 WITH CREATEROLE; +SET SESSION AUTHORIZATION regress_dummy_seclabel_user0; +CREATE USER regress_dummy_seclabel_user1; CREATE USER regress_dummy_seclabel_user2; +RESET SESSION AUTHORIZATION; 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; @@ -19,7 +22,7 @@ ALTER TABLE dummy_seclabel_tbl2 OWNER TO regress_dummy_seclabel_user2; -- -- Test of SECURITY LABEL statement with a plugin -- -SET SESSION AUTHORIZATION regress_dummy_seclabel_user1; +SET SESSION AUTHORIZATION regress_dummy_seclabel_user0; SECURITY LABEL ON TABLE dummy_seclabel_tbl1 IS 'classified'; -- OK SECURITY LABEL ON COLUMN dummy_seclabel_tbl1.a IS 'unclassified'; -- OK SECURITY LABEL ON COLUMN dummy_seclabel_tbl1 IS 'unclassified'; -- fail @@ -29,6 +32,7 @@ ERROR: '...invalid label...' is not a valid security label SECURITY LABEL FOR 'dummy' ON TABLE dummy_seclabel_tbl1 IS 'unclassified'; -- OK SECURITY LABEL FOR 'unknown_seclabel' ON TABLE dummy_seclabel_tbl1 IS 'classified'; -- fail ERROR: security label provider "unknown_seclabel" is not loaded +SET SESSION AUTHORIZATION regress_dummy_seclabel_user1; SECURITY LABEL ON TABLE dummy_seclabel_tbl2 IS 'unclassified'; -- fail (not owner) ERROR: must be owner of table dummy_seclabel_tbl2 SECURITY LABEL ON TABLE dummy_seclabel_tbl1 IS 'secret'; -- fail (not superuser) @@ -42,7 +46,7 @@ SECURITY LABEL ON TABLE dummy_seclabel_tbl2 IS 'classified'; -- OK -- -- Test for shared database object -- -SET SESSION AUTHORIZATION regress_dummy_seclabel_user1; +SET SESSION AUTHORIZATION regress_dummy_seclabel_user0; SECURITY LABEL ON ROLE regress_dummy_seclabel_user1 IS 'classified'; -- OK SECURITY LABEL ON ROLE regress_dummy_seclabel_user1 IS '...invalid label...'; -- fail ERROR: '...invalid label...' is not a valid security label @@ -55,7 +59,7 @@ SECURITY LABEL ON ROLE regress_dummy_seclabel_user3 IS 'unclassified'; -- fail ( ERROR: role "regress_dummy_seclabel_user3" 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 +ERROR: must be owner of role regress_dummy_seclabel_user2 RESET SESSION AUTHORIZATION; -- -- Test for various types of object diff --git a/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql b/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql index 8c347b6a68..bf575343cf 100644 --- a/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql +++ b/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql @@ -11,8 +11,12 @@ DROP ROLE IF EXISTS regress_dummy_seclabel_user2; RESET client_min_messages; -CREATE USER regress_dummy_seclabel_user1 WITH CREATEROLE; +CREATE USER regress_dummy_seclabel_user0 WITH CREATEROLE; + +SET SESSION AUTHORIZATION regress_dummy_seclabel_user0; +CREATE USER regress_dummy_seclabel_user1; CREATE USER regress_dummy_seclabel_user2; +RESET SESSION AUTHORIZATION; CREATE TABLE dummy_seclabel_tbl1 (a int, b text); CREATE TABLE dummy_seclabel_tbl2 (x int, y text); @@ -26,7 +30,7 @@ ALTER TABLE dummy_seclabel_tbl2 OWNER TO regress_dummy_seclabel_user2; -- -- Test of SECURITY LABEL statement with a plugin -- -SET SESSION AUTHORIZATION regress_dummy_seclabel_user1; +SET SESSION AUTHORIZATION regress_dummy_seclabel_user0; SECURITY LABEL ON TABLE dummy_seclabel_tbl1 IS 'classified'; -- OK SECURITY LABEL ON COLUMN dummy_seclabel_tbl1.a IS 'unclassified'; -- OK @@ -34,6 +38,8 @@ SECURITY LABEL ON COLUMN dummy_seclabel_tbl1 IS 'unclassified'; -- fail SECURITY LABEL ON TABLE dummy_seclabel_tbl1 IS '...invalid label...'; -- fail SECURITY LABEL FOR 'dummy' ON TABLE dummy_seclabel_tbl1 IS 'unclassified'; -- OK SECURITY LABEL FOR 'unknown_seclabel' ON TABLE dummy_seclabel_tbl1 IS 'classified'; -- fail + +SET SESSION AUTHORIZATION regress_dummy_seclabel_user1; SECURITY LABEL ON TABLE dummy_seclabel_tbl2 IS 'unclassified'; -- fail (not owner) SECURITY LABEL ON TABLE dummy_seclabel_tbl1 IS 'secret'; -- fail (not superuser) SECURITY LABEL ON TABLE dummy_seclabel_tbl3 IS 'unclassified'; -- fail (not found) @@ -45,7 +51,7 @@ SECURITY LABEL ON TABLE dummy_seclabel_tbl2 IS 'classified'; -- OK -- -- Test for shared database object -- -SET SESSION AUTHORIZATION regress_dummy_seclabel_user1; +SET SESSION AUTHORIZATION regress_dummy_seclabel_user0; SECURITY LABEL ON ROLE regress_dummy_seclabel_user1 IS 'classified'; -- OK SECURITY LABEL ON ROLE regress_dummy_seclabel_user1 IS '...invalid label...'; -- fail diff --git a/src/test/modules/unsafe_tests/expected/rolenames.out b/src/test/modules/unsafe_tests/expected/rolenames.out index eb608fdc2e..8b79a63b80 100644 --- a/src/test/modules/unsafe_tests/expected/rolenames.out +++ b/src/test/modules/unsafe_tests/expected/rolenames.out @@ -1086,6 +1086,10 @@ REVOKE pg_read_all_settings FROM regress_role_haspriv; \c DROP SCHEMA test_roles_schema; DROP OWNED BY regress_testrol0, "Public", "current_role", "current_user", regress_testrol1, regress_testrol2, regress_testrolx CASCADE; -DROP ROLE regress_testrol0, regress_testrol1, regress_testrol2, regress_testrolx; +DROP ROLE regress_testrol0, regress_testrol1, regress_testrol2, regress_testrolx; -- fails with owner of role regress_role_haspriv +ERROR: role "regress_testrol2" cannot be dropped because some objects depend on it +DETAIL: owner of role regress_role_haspriv +owner of role regress_role_nopriv DROP ROLE "Public", "None", "current_role", "current_user", "session_user", "user"; DROP ROLE regress_role_haspriv, regress_role_nopriv; +DROP ROLE regress_testrol0, regress_testrol1, regress_testrol2, regress_testrolx; -- ok now diff --git a/src/test/modules/unsafe_tests/sql/rolenames.sql b/src/test/modules/unsafe_tests/sql/rolenames.sql index adac36536d..95a54ce70d 100644 --- a/src/test/modules/unsafe_tests/sql/rolenames.sql +++ b/src/test/modules/unsafe_tests/sql/rolenames.sql @@ -499,6 +499,7 @@ REVOKE pg_read_all_settings FROM regress_role_haspriv; DROP SCHEMA test_roles_schema; DROP OWNED BY regress_testrol0, "Public", "current_role", "current_user", regress_testrol1, regress_testrol2, regress_testrolx CASCADE; -DROP ROLE regress_testrol0, regress_testrol1, regress_testrol2, regress_testrolx; +DROP ROLE regress_testrol0, regress_testrol1, regress_testrol2, regress_testrolx; -- fails with owner of role regress_role_haspriv DROP ROLE "Public", "None", "current_role", "current_user", "session_user", "user"; DROP ROLE regress_role_haspriv, regress_role_nopriv; +DROP ROLE regress_testrol0, regress_testrol1, regress_testrol2, regress_testrolx; -- ok now diff --git a/src/test/regress/expected/create_role.out b/src/test/regress/expected/create_role.out index 4e67d72760..4ad6fd4161 100644 --- a/src/test/regress/expected/create_role.out +++ b/src/test/regress/expected/create_role.out @@ -1,6 +1,8 @@ -- ok, superuser can create users with any set of privileges CREATE ROLE regress_role_super SUPERUSER; +CREATE ROLE regress_role_bystander; CREATE ROLE regress_role_admin CREATEDB CREATEROLE REPLICATION BYPASSRLS; +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; @@ -11,14 +13,108 @@ 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_superuser AUTHORIZATION regress_role_super SUPERUSER; +-- fail, can only create roles belonging to other roles that we belong to +SET SESSION AUTHORIZATION regress_role_admin; +CREATE ROLE regress_nosuch_alice AUTHORIZATION regress_role_super; +ERROR: must be member of role "regress_role_super" +CREATE ROLE regress_nosuch_bob AUTHORIZATION regress_superuser; +ERROR: must be member of role "regress_superuser" +CREATE ROLE regress_nosuch_charlie AUTHORIZATION regress_role_bystander; +ERROR: must be member of role "regress_role_bystander" +-- ok, superuser can create users with these privileges for normal role +RESET SESSION AUTHORIZATION; +CREATE ROLE regress_replication_bypassrls AUTHORIZATION regress_role_admin REPLICATION BYPASSRLS; +CREATE ROLE regress_replication AUTHORIZATION regress_role_admin REPLICATION; +CREATE ROLE regress_bypassrls AUTHORIZATION regress_role_admin BYPASSRLS; +\du+ regress_superuser + List of roles + Role name | Owner | Attributes | Member of | Description +-------------------+--------------------+-------------------------+-----------+------------- + regress_superuser | regress_role_super | Superuser, Cannot login | {} | + +\du+ regress_replication_bypassrls + List of roles + Role name | Owner | Attributes | Member of | Description +-------------------------------+--------------------+---------------------------------------+-----------+------------- + regress_replication_bypassrls | regress_role_admin | Cannot login, Replication, Bypass RLS | {} | + +\du+ regress_replication + List of roles + Role name | Owner | Attributes | Member of | Description +---------------------+--------------------+---------------------------+-----------+------------- + regress_replication | regress_role_admin | Cannot login, Replication | {} | + +\du+ regress_bypassrls + List of roles + Role name | Owner | Attributes | Member of | Description +-------------------+--------------------+--------------------------+-----------+------------- + regress_bypassrls | regress_role_admin | Cannot login, Bypass RLS | {} | + +-- fail, roles are not allowed to own themselves +ALTER ROLE regress_bypassrls OWNER TO regress_bypassrls; +ERROR: role may not own itself -- ok, having CREATEROLE is enough to create users with these privileges +SET SESSION AUTHORIZATION regress_role_admin; CREATE ROLE regress_createdb CREATEDB; CREATE ROLE regress_createrole CREATEROLE; CREATE ROLE regress_login LOGIN; CREATE ROLE regress_inherit INHERIT; CREATE ROLE regress_connection_limit CONNECTION LIMIT 5; -CREATE ROLE regress_encrypted_password ENCRYPTED PASSWORD 'foo'; -CREATE ROLE regress_password_null PASSWORD NULL; +CREATE ROLE regress_encrypted_password PASSWORD NULL; +CREATE ROLE regress_password_null + CREATEDB CREATEROLE INHERIT CONNECTION LIMIT 2 ENCRYPTED PASSWORD 'foo' + IN ROLE regress_createdb, regress_createrole; +COMMENT ON ROLE regress_password_null IS 'no login test role'; +\du+ regress_createdb + List of roles + Role name | Owner | Attributes | Member of | Description +------------------+--------------------+-------------------------+-----------+------------- + regress_createdb | regress_role_admin | Create DB, Cannot login | {} | + +\du+ regress_createrole + List of roles + Role name | Owner | Attributes | Member of | Description +--------------------+--------------------+---------------------------+-----------+------------- + 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 +-----------------+--------------------+--------------+-----------+------------- + regress_inherit | regress_role_admin | Cannot login | {} | + +\du+ regress_connection_limit + List of roles + Role name | Owner | Attributes | Member of | Description +--------------------------+--------------------+---------------+-----------+------------- + regress_connection_limit | regress_role_admin | Cannot login +| {} | + | | 5 connections | | + +\du+ regress_encrypted_password + List of roles + Role name | Owner | Attributes | Member of | Description +----------------------------+--------------------+--------------+-----------+------------- + regress_encrypted_password | regress_role_admin | Cannot login | {} | + +\du+ regress_password_null + List of roles + Role name | Owner | Attributes | Member of | Description +-----------------------+--------------------+--------------------------------------+---------------------------------------+-------------------- + regress_password_null | regress_role_admin | Create role, Create DB, Cannot login+| {regress_createdb,regress_createrole} | no login test role + | | 2 connections | | + -- ok, backwards compatible noise words should be ignored CREATE ROLE regress_noiseword SYSID 12345; NOTICE: SYSID can no longer be specified @@ -58,21 +154,18 @@ CREATE TABLE tenant_table (i integer); CREATE INDEX tenant_idx ON tenant_table(i); CREATE VIEW tenant_view AS SELECT * FROM pg_catalog.pg_class; REVOKE ALL PRIVILEGES ON tenant_table FROM PUBLIC; --- fail, these objects belonging to regress_tenant +-- ok, owning role can manage owned role's objects SET SESSION AUTHORIZATION regress_createrole; DROP INDEX tenant_idx; -ERROR: must be owner of index tenant_idx ALTER TABLE tenant_table ADD COLUMN t text; -ERROR: must be owner of table tenant_table DROP TABLE tenant_table; -ERROR: must be owner of table tenant_table +-- fail, not a member of target role ALTER VIEW tenant_view OWNER TO regress_role_admin; -ERROR: must be owner of view tenant_view +ERROR: must be member of role "regress_role_admin" +-- ok DROP VIEW tenant_view; -ERROR: must be owner of view tenant_view --- fail, cannot take ownership of these objects from regress_tenant +-- ok, can take ownership objects from owned roles 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 CREATE ROLE regress_read_all_data IN ROLE pg_read_all_data; CREATE ROLE regress_write_all_data IN ROLE pg_write_all_data; @@ -84,16 +177,24 @@ CREATE ROLE regress_read_server_files IN ROLE pg_read_server_files; 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, creation of these roles failed above so they do not now exist +-- fail, cannot create ownership cycles +RESET SESSION AUTHORIZATION; +REASSIGN OWNED BY regress_role_admin TO regress_tenant; +ERROR: role "regress_createrole" may not both own and be owned by role "regress_tenant" +ALTER ROLE regress_role_admin OWNER TO regress_tenant; +ERROR: role "regress_role_admin" may not both own and be owned by role "regress_tenant" +-- ok, can take ownership from owned roles +SET SESSION AUTHORIZATION regress_role_admin; +ALTER ROLE regress_plainrole OWNER TO regress_role_admin; +REASSIGN OWNED BY regress_plainrole TO regress_role_admin; +-- ok, superuser roles can drop superuser roles they own +SET SESSION AUTHORIZATION regress_role_super; +DROP ROLE regress_superuser; +-- ok, non-superuser roles can drop non-superuser roles they own SET SESSION AUTHORIZATION regress_role_admin; -DROP ROLE regress_nosuch_superuser; -ERROR: role "regress_nosuch_superuser" does not exist -DROP ROLE regress_nosuch_replication_bypassrls; -ERROR: role "regress_nosuch_replication_bypassrls" does not exist -DROP ROLE regress_nosuch_replication; -ERROR: role "regress_nosuch_replication" does not exist -DROP ROLE regress_nosuch_bypassrls; -ERROR: role "regress_nosuch_bypassrls" does not exist +DROP ROLE regress_replication_bypassrls; +DROP ROLE regress_replication; +DROP ROLE regress_bypassrls; DROP ROLE regress_nosuch_super; ERROR: role "regress_nosuch_super" does not exist DROP ROLE regress_nosuch_dbowner; @@ -103,9 +204,23 @@ ERROR: role "regress_nosuch_recursive" does not exist DROP ROLE regress_nosuch_admin_recursive; ERROR: role "regress_nosuch_admin_recursive" does not exist DROP ROLE regress_plainrole; --- ok, should be able to drop non-superuser roles we created -DROP ROLE regress_createdb; +-- fail, cannot drop roles that own other roles 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; @@ -115,6 +230,7 @@ DROP ROLE regress_noiseword; 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; @@ -125,21 +241,20 @@ 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 DROP ROLE regress_role_admin; ERROR: current user cannot be dropped --- ok +-- ok, no more owned roles remain +DROP ROLE regress_createrole; +-- fail, cannot drop role with remaining privileges RESET SESSION AUTHORIZATION; -DROP INDEX tenant_idx; -DROP TABLE tenant_table; -DROP VIEW tenant_view; -DROP ROLE regress_tenant; DROP ROLE regress_role_admin; +ERROR: role "regress_role_admin" cannot be dropped because some objects depend on it +DETAIL: privileges for database regression +-- ok, can drop role if we revoke privileges first +REVOKE CREATE ON DATABASE regression FROM regress_role_admin; +DROP ROLE regress_role_admin; +DROP ROLE regress_role_bystander; DROP ROLE regress_role_super; diff --git a/src/test/regress/expected/oidjoins.out b/src/test/regress/expected/oidjoins.out index 215eb899be..266a30a85b 100644 --- a/src/test/regress/expected/oidjoins.out +++ b/src/test/regress/expected/oidjoins.out @@ -194,6 +194,7 @@ NOTICE: checking pg_database {dattablespace} => pg_tablespace {oid} NOTICE: checking pg_db_role_setting {setdatabase} => pg_database {oid} NOTICE: checking pg_db_role_setting {setrole} => pg_authid {oid} NOTICE: checking pg_tablespace {spcowner} => pg_authid {oid} +NOTICE: checking pg_authid {rolowner} => pg_authid {oid} NOTICE: checking pg_auth_members {roleid} => pg_authid {oid} NOTICE: checking pg_auth_members {member} => pg_authid {oid} NOTICE: checking pg_auth_members {grantor} => pg_authid {oid} diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index 291e21d7a6..9ce619fd5f 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -27,8 +27,10 @@ CREATE USER regress_priv_user4; CREATE USER regress_priv_user5; CREATE USER regress_priv_user5; -- duplicate ERROR: role "regress_priv_user5" already exists -CREATE USER regress_priv_user6; +CREATE USER regress_priv_user6 CREATEROLE; +SET SESSION AUTHORIZATION regress_priv_user6; CREATE USER regress_priv_user7; +RESET SESSION AUTHORIZATION; CREATE USER regress_priv_user8; CREATE USER regress_priv_user9; CREATE USER regress_priv_user10; @@ -2356,7 +2358,12 @@ DROP USER regress_priv_user3; DROP USER regress_priv_user4; DROP USER regress_priv_user5; DROP USER regress_priv_user6; +ERROR: role "regress_priv_user6" cannot be dropped because some objects depend on it +DETAIL: owner of role regress_priv_user7 +SET SESSION AUTHORIZATION regress_priv_user6; DROP USER regress_priv_user7; +RESET SESSION AUTHORIZATION; +DROP USER regress_priv_user6; DROP USER regress_priv_user8; -- does not exist ERROR: role "regress_priv_user8" does not exist -- permissions with LOCK TABLE diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index d652f7b5fb..b6e3c508ba 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1482,6 +1482,7 @@ pg_replication_slots| SELECT l.slot_name, FROM (pg_get_replication_slots() l(slot_name, plugin, slot_type, datoid, temporary, active, active_pid, xmin, catalog_xmin, restart_lsn, confirmed_flush_lsn, wal_status, safe_wal_size, two_phase) LEFT JOIN pg_database d ON ((l.datoid = d.oid))); pg_roles| SELECT pg_authid.rolname, + pg_get_userbyid(pg_authid.rolowner) AS rolowner, pg_authid.rolsuper, pg_authid.rolinherit, pg_authid.rolcreaterole, diff --git a/src/test/regress/sql/create_role.sql b/src/test/regress/sql/create_role.sql index 292dc08797..6266586b8b 100644 --- a/src/test/regress/sql/create_role.sql +++ b/src/test/regress/sql/create_role.sql @@ -1,6 +1,8 @@ -- ok, superuser can create users with any set of privileges CREATE ROLE regress_role_super SUPERUSER; +CREATE ROLE regress_role_bystander; CREATE ROLE regress_role_admin CREATEDB CREATEROLE REPLICATION BYPASSRLS; +GRANT CREATE ON DATABASE regression TO regress_role_admin; -- fail, only superusers can create users with these privileges SET SESSION AUTHORIZATION regress_role_admin; @@ -9,14 +11,53 @@ CREATE ROLE regress_nosuch_replication_bypassrls REPLICATION BYPASSRLS; CREATE ROLE regress_nosuch_replication REPLICATION; CREATE ROLE regress_nosuch_bypassrls BYPASSRLS; +-- fail, only superusers can own superusers +RESET SESSION AUTHORIZATION; +CREATE ROLE regress_nosuch_superuser AUTHORIZATION regress_role_admin SUPERUSER; + +-- ok, superuser can create superusers belonging to other superusers +CREATE ROLE regress_superuser AUTHORIZATION regress_role_super SUPERUSER; + +-- fail, can only create roles belonging to other roles that we belong to +SET SESSION AUTHORIZATION regress_role_admin; +CREATE ROLE regress_nosuch_alice AUTHORIZATION regress_role_super; +CREATE ROLE regress_nosuch_bob AUTHORIZATION regress_superuser; +CREATE ROLE regress_nosuch_charlie AUTHORIZATION regress_role_bystander; + +-- ok, superuser can create users with these privileges for normal role +RESET SESSION AUTHORIZATION; +CREATE ROLE regress_replication_bypassrls AUTHORIZATION regress_role_admin REPLICATION BYPASSRLS; +CREATE ROLE regress_replication AUTHORIZATION regress_role_admin REPLICATION; +CREATE ROLE regress_bypassrls AUTHORIZATION regress_role_admin BYPASSRLS; + +\du+ regress_superuser +\du+ regress_replication_bypassrls +\du+ regress_replication +\du+ regress_bypassrls + +-- fail, roles are not allowed to own themselves +ALTER ROLE regress_bypassrls OWNER TO regress_bypassrls; + -- ok, having CREATEROLE is enough to create users with these privileges +SET SESSION AUTHORIZATION regress_role_admin; CREATE ROLE regress_createdb CREATEDB; CREATE ROLE regress_createrole CREATEROLE; CREATE ROLE regress_login LOGIN; CREATE ROLE regress_inherit INHERIT; CREATE ROLE regress_connection_limit CONNECTION LIMIT 5; -CREATE ROLE regress_encrypted_password ENCRYPTED PASSWORD 'foo'; -CREATE ROLE regress_password_null PASSWORD NULL; +CREATE ROLE regress_encrypted_password PASSWORD NULL; +CREATE ROLE regress_password_null + CREATEDB CREATEROLE INHERIT CONNECTION LIMIT 2 ENCRYPTED PASSWORD 'foo' + IN ROLE regress_createdb, regress_createrole; +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 +\du+ regress_password_null -- ok, backwards compatible noise words should be ignored CREATE ROLE regress_noiseword SYSID 12345; @@ -63,15 +104,19 @@ CREATE INDEX tenant_idx ON tenant_table(i); CREATE VIEW tenant_view AS SELECT * FROM pg_catalog.pg_class; REVOKE ALL PRIVILEGES ON tenant_table FROM PUBLIC; --- fail, these objects belonging to regress_tenant +-- ok, owning role can manage owned role's objects SET SESSION AUTHORIZATION regress_createrole; DROP INDEX tenant_idx; ALTER TABLE tenant_table ADD COLUMN t text; DROP TABLE tenant_table; + +-- fail, not a member of target role ALTER VIEW tenant_view OWNER TO regress_role_admin; + +-- ok DROP VIEW tenant_view; --- fail, cannot take ownership of these objects from regress_tenant +-- 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 @@ -86,21 +131,36 @@ 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, creation of these roles failed above so they do not now exist +-- fail, cannot create ownership cycles +RESET SESSION AUTHORIZATION; +REASSIGN OWNED BY regress_role_admin TO regress_tenant; +ALTER ROLE regress_role_admin OWNER TO regress_tenant; + +-- ok, can take ownership from owned roles SET SESSION AUTHORIZATION regress_role_admin; -DROP ROLE regress_nosuch_superuser; -DROP ROLE regress_nosuch_replication_bypassrls; -DROP ROLE regress_nosuch_replication; -DROP ROLE regress_nosuch_bypassrls; +ALTER ROLE regress_plainrole OWNER TO regress_role_admin; +REASSIGN OWNED BY regress_plainrole TO regress_role_admin; + +-- ok, superuser roles can drop superuser roles they own +SET SESSION AUTHORIZATION regress_role_super; +DROP ROLE regress_superuser; + +-- ok, non-superuser roles can drop non-superuser roles they own +SET SESSION AUTHORIZATION regress_role_admin; +DROP ROLE regress_replication_bypassrls; +DROP ROLE regress_replication; +DROP ROLE regress_bypassrls; DROP ROLE regress_nosuch_super; DROP ROLE regress_nosuch_dbowner; DROP ROLE regress_nosuch_recursive; DROP ROLE regress_nosuch_admin_recursive; DROP ROLE regress_plainrole; --- ok, should be able to drop non-superuser roles we created -DROP ROLE regress_createdb; +-- fail, cannot drop roles that own other roles 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; @@ -110,6 +170,7 @@ DROP ROLE regress_noiseword; 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; @@ -121,18 +182,19 @@ 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; DROP ROLE regress_role_admin; --- ok +-- ok, no more owned roles remain +DROP ROLE regress_createrole; + +-- fail, cannot drop role with remaining privileges RESET SESSION AUTHORIZATION; -DROP INDEX tenant_idx; -DROP TABLE tenant_table; -DROP VIEW tenant_view; -DROP ROLE regress_tenant; DROP ROLE regress_role_admin; + +-- ok, can drop role if we revoke privileges first +REVOKE CREATE ON DATABASE regression FROM regress_role_admin; +DROP ROLE regress_role_admin; +DROP ROLE regress_role_bystander; DROP ROLE regress_role_super; diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index c8c545b64c..dcf84f91fd 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -30,8 +30,10 @@ CREATE USER regress_priv_user3; CREATE USER regress_priv_user4; CREATE USER regress_priv_user5; CREATE USER regress_priv_user5; -- duplicate -CREATE USER regress_priv_user6; +CREATE USER regress_priv_user6 CREATEROLE; +SET SESSION AUTHORIZATION regress_priv_user6; CREATE USER regress_priv_user7; +RESET SESSION AUTHORIZATION; CREATE USER regress_priv_user8; CREATE USER regress_priv_user9; CREATE USER regress_priv_user10; @@ -1426,8 +1428,14 @@ DROP USER regress_priv_user2; DROP USER regress_priv_user3; DROP USER regress_priv_user4; DROP USER regress_priv_user5; + DROP USER regress_priv_user6; + +SET SESSION AUTHORIZATION regress_priv_user6; DROP USER regress_priv_user7; +RESET SESSION AUTHORIZATION; + +DROP USER regress_priv_user6; DROP USER regress_priv_user8; -- does not exist -- 2.21.1 (Apple Git-122.3)