Author: Noah Misch Commit: Noah Misch Rollup of five patches, for easy testing. diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 60c7e11..8b3b37b 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -8925,7 +8925,7 @@ $d$; -- But creation of user mappings for non-superusers should fail CREATE USER MAPPING FOR public SERVER loopback_nopw; CREATE USER MAPPING FOR CURRENT_USER SERVER loopback_nopw; -CREATE FOREIGN TABLE ft1_nopw ( +CREATE FOREIGN TABLE pg_temp.ft1_nopw ( c1 int NOT NULL, c2 int NOT NULL, c3 text, diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 151f4f1..bf35097 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -2595,7 +2595,7 @@ $d$; CREATE USER MAPPING FOR public SERVER loopback_nopw; CREATE USER MAPPING FOR CURRENT_USER SERVER loopback_nopw; -CREATE FOREIGN TABLE ft1_nopw ( +CREATE FOREIGN TABLE pg_temp.ft1_nopw ( c1 int NOT NULL, c2 int NOT NULL, c3 text, diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index ea222c0..8080047 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -10105,6 +10105,9 @@ SCRAM-SHA-256$<iteration count>:&l pg_group + The view pg_group exists for backwards compatibility: it emulates a catalog that existed in diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 1e9a462..1b30a0d 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -2982,20 +2982,18 @@ SELECT 3 OPERATOR(pg_catalog.+) 4; By default, users cannot access any objects in schemas they do not own. To allow that, the owner of the schema must grant the - USAGE privilege on the schema. To allow users - to make use of the objects in the schema, additional privileges - might need to be granted, as appropriate for the object. + USAGE privilege on the schema. By default, everyone + has that privilege on the schema public. To allow + users to make use of the objects in a schema, additional privileges might + need to be granted, as appropriate for the object. - A user can also be allowed to create objects in someone else's - schema. To allow that, the CREATE privilege on - the schema needs to be granted. Note that by default, everyone - has CREATE and USAGE privileges on - the schema - public. This allows all users that are able to - connect to a given database to create objects in its - public schema. + A user can also be allowed to create objects in someone else's schema. To + allow that, the CREATE privilege on the schema needs to + be granted. In databases upgraded from + PostgreSQL 13 or earlier, everyone has that + privilege on the schema public. Some usage patterns call for revoking that privilege: @@ -3068,20 +3066,25 @@ REVOKE CREATE ON SCHEMA public FROM PUBLIC; database owner attack. --> Constrain ordinary users to user-private schemas. To implement this, - issue REVOKE CREATE ON SCHEMA public FROM PUBLIC, - and create a schema for each user with the same name as that user. - Recall that the default search path starts - with $user, which resolves to the user name. - Therefore, if each user has a separate schema, they access their own - schemas by default. After adopting this pattern in a database where - untrusted users had already logged in, consider auditing the public - schema for objects named like objects in + first issue REVOKE CREATE ON SCHEMA public FROM + PUBLIC. Then, for every user needing to create non-temporary + objects, create a schema with the same name as that user. Recall that + the default search path starts with $user, which + resolves to the user name. Therefore, if each user has a separate + schema, they access their own schemas by default. After adopting this + 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. + If the database originated in an upgrade + from PostgreSQL 13 or earlier, + the REVOKE is essential. Otherwise, the default + configuration follows this pattern; ordinary users can create only + temporary objects until a privileged user furnishes a schema. @@ -3090,10 +3093,10 @@ REVOKE CREATE ON SCHEMA public FROM PUBLIC; Remove the public schema from the default search path, by modifying postgresql.conf or by issuing ALTER ROLE ALL SET search_path = - "$user". Everyone retains the ability to create objects in - the public schema, but only qualified names will choose those objects. - While qualified table references are fine, calls to functions in the - public schema will be unsafe or + "$user". Then, grant privileges to create in the public + schema. Only qualified names will choose public schema objects. While + qualified table references are fine, calls to functions in the 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 @@ -3103,11 +3106,14 @@ REVOKE CREATE ON SCHEMA public FROM PUBLIC; - Keep the default. All users access the public schema implicitly. This + Keep the default search path, and grant privileges to create in the + public schema. All users access the public schema implicitly. This simulates the situation where schemas are not available at all, giving a smooth transition from the non-schema-aware world. However, this is never a secure pattern. It is acceptable only when the database has a - single user or a few mutually-trusting users. + single user or a few mutually-trusting users. In databases upgraded + from PostgreSQL 13 or earlier, this is the + default. diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml index cc08252..768a37f 100644 --- a/doc/src/sgml/user-manag.sgml +++ b/doc/src/sgml/user-manag.sgml @@ -540,6 +540,10 @@ DROP ROLE doomed_role; pg_stat_scan_tables. + pg_database_owner + None. Membership consists, implicitly, of the current database owner. + + pg_signal_backend Signal another backend to cancel a query or terminate its session. @@ -572,6 +576,18 @@ DROP ROLE doomed_role; + The pg_database_owner role has one implicit, + situation-dependent member, namely the owner of the current database. Like + any role, it can own objects or receive grants of access privileges. + Consequently, once pg_database_owner has rights within a + template database, each owner of a database instantiated from that template + will exercise those rights. pg_database_owner cannot be + a member of any role, and it cannot have non-implicit members. Initially, + this role owns the public schema, so each database owner + governs local use of the schema. + + + The pg_signal_backend role is intended to allow administrators to enable trusted, but non-superuser, roles to send signals to other backends. Currently this role enables sending of signals for @@ -617,8 +633,8 @@ GRANT pg_signal_backend TO admin_user; horse others with relative ease. The strongest protection is tight control over who can define objects. Where that is infeasible, write queries referring only to objects having trusted owners. Remove - from search_path the public schema and any other schemas - that permit untrusted users to create objects. + from search_path any schemas that permit untrusted users + to create objects. diff --git a/src/backend/catalog/information_schema.sql b/src/backend/catalog/information_schema.sql index 4907855..9a0169b 100644 --- a/src/backend/catalog/information_schema.sql +++ b/src/backend/catalog/information_schema.sql @@ -255,7 +255,14 @@ CREATE VIEW applicable_roles AS SELECT CAST(a.rolname AS sql_identifier) AS grantee, CAST(b.rolname AS sql_identifier) AS role_name, CAST(CASE WHEN m.admin_option THEN 'YES' ELSE 'NO' END AS yes_or_no) AS is_grantable - FROM pg_auth_members m + FROM (SELECT member, roleid, admin_option FROM pg_auth_members + -- This UNION could be UNION ALL, but UNION works even if we start + -- to allow explicit pg_database_owner membership. + UNION + SELECT datdba, pg_authid.oid, false + FROM pg_database, pg_authid + WHERE datname = current_database() AND rolname = 'pg_database_owner' + ) m JOIN pg_authid a ON (m.member = a.oid) JOIN pg_authid b ON (m.roleid = b.oid) WHERE pg_has_role(a.oid, 'USAGE'); diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index ed243e3..74be182 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -1497,6 +1497,18 @@ AddRoleMems(const char *rolename, Oid roleid, } /* + * The charter of pg_database_owner is to have exactly one, implicit, + * situation-dependent member. There's no technical need for this + * restriction. (One could lift it and take the further step of making + * pg_database_ownercheck() equivalent to has_privs_of_role(roleid, + * DEFAULT_ROLE_DATABASE_OWNER), in which case explicit, + * situation-independent members could act as the owner of any database.) + */ + if (roleid == DEFAULT_ROLE_DATABASE_OWNER) + ereport(ERROR, + errmsg("role \"%s\" cannot have explicit members", rolename)); + + /* * The role membership grantor of record has little significance at * present. Nonetheless, inasmuch as users might look to it for a crude * audit trail, let only superusers impute the grant to a third party. @@ -1525,6 +1537,22 @@ AddRoleMems(const char *rolename, Oid roleid, bool new_record_repl[Natts_pg_auth_members]; /* + * pg_database_owner is never a role member. Lifting this restriction + * would require a policy decision about membership loops. One could + * prevent loops, which would include making "ALTER DATABASE x OWNER + * TO proposed_datdba" fail if is_member_of_role(pg_database_owner, + * proposed_datdba). Hence, gaining a membership could reduce what a + * role could do. Alternately, one could allow these memberships to + * complete loops. A role could then have actual WITH ADMIN OPTION on + * itself, prompting a decision about is_admin_of_role() treatment of + * the case. + */ + if (memberid == DEFAULT_ROLE_DATABASE_OWNER) + ereport(ERROR, + errmsg("role \"%s\" cannot be a member of any role", + get_rolespec_name(memberRole))); + + /* * Refuse creation of membership loops, including the trivial case * where a role is made a member of itself. We do this by checking to * see if the target role is already a member of the proposed member diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c index c7f029e..34975ee 100644 --- a/src/backend/utils/adt/acl.c +++ b/src/backend/utils/adt/acl.c @@ -22,6 +22,7 @@ #include "catalog/pg_auth_members.h" #include "catalog/pg_authid.h" #include "catalog/pg_class.h" +#include "catalog/pg_database.h" #include "catalog/pg_type.h" #include "commands/dbcommands.h" #include "commands/proclang.h" @@ -50,32 +51,25 @@ typedef struct /* * We frequently need to test whether a given role is a member of some other * role. In most of these tests the "given role" is the same, namely the - * active current user. So we can optimize it by keeping a cached list of - * all the roles the "given role" is a member of, directly or indirectly. - * - * There are actually two caches, one computed under "has_privs" rules - * (do not recurse where rolinherit isn't true) and one computed under - * "is_member" rules (recurse regardless of rolinherit). + * active current user. So we can optimize it by keeping cached lists of all + * the roles the "given role" is a member of, directly or indirectly. * * Possibly this mechanism should be generalized to allow caching membership * info for multiple roles? * - * The has_privs cache is: - * cached_privs_role is the role OID the cache is for. - * cached_privs_roles is an OID list of roles that cached_privs_role - * has the privileges of (always including itself). - * The cache is valid if cached_privs_role is not InvalidOid. - * - * The is_member cache is similarly: - * cached_member_role is the role OID the cache is for. - * cached_membership_roles is an OID list of roles that cached_member_role - * is a member of (always including itself). - * The cache is valid if cached_member_role is not InvalidOid. + * Each element of cached_roles is an OID list of constituent roles for the + * corresponding element of cached_role (always including the cached_role + * itself). One cache has ROLERECURSE_PRIVS semantics, and the other has + * ROLERECURSE_MEMBERS semantics. */ -static Oid cached_privs_role = InvalidOid; -static List *cached_privs_roles = NIL; -static Oid cached_member_role = InvalidOid; -static List *cached_membership_roles = NIL; +enum RoleRecurseType +{ + ROLERECURSE_PRIVS = 0, /* recurse if rolinherit */ + ROLERECURSE_MEMBERS = 1 /* recurse unconditionally */ +}; +static Oid cached_role[] = {InvalidOid, InvalidOid}; +static List *cached_roles[] = {NIL, NIL}; +static uint32 cached_db_hash; static const char *getid(const char *s, char *n); @@ -4673,10 +4667,14 @@ initialize_acl(void) { if (!IsBootstrapProcessingMode()) { + cached_db_hash = + GetSysCacheHashValue1(DATABASEOID, + ObjectIdGetDatum(MyDatabaseId)); + /* * In normal mode, set a callback on any syscache invalidation of rows - * of pg_auth_members (for each AUTHMEM search in this file) or - * pg_authid (for has_rolinherit()) + * of pg_auth_members (for roles_is_member_of()), pg_authid (for + * has_rolinherit()), or pg_database (for roles_is_member_of()) */ CacheRegisterSyscacheCallback(AUTHMEMROLEMEM, RoleMembershipCacheCallback, @@ -4684,6 +4682,9 @@ initialize_acl(void) CacheRegisterSyscacheCallback(AUTHOID, RoleMembershipCacheCallback, (Datum) 0); + CacheRegisterSyscacheCallback(DATABASEOID, + RoleMembershipCacheCallback, + (Datum) 0); } } @@ -4694,9 +4695,16 @@ initialize_acl(void) static void RoleMembershipCacheCallback(Datum arg, int cacheid, uint32 hashvalue) { + if (cacheid == DATABASEOID && + hashvalue != cached_db_hash && + hashvalue != 0) + { + return; /* ignore pg_database changes for other DBs */ + } + /* Force membership caches to be recomputed on next use */ - cached_privs_role = InvalidOid; - cached_member_role = InvalidOid; + cached_role[ROLERECURSE_PRIVS] = InvalidOid; + cached_role[ROLERECURSE_MEMBERS] = InvalidOid; } @@ -4718,115 +4726,54 @@ has_rolinherit(Oid roleid) /* - * Get a list of roles that the specified roleid has the privileges of + * Get a list of roles that the specified roleid is a member of * - * This is defined not to recurse through roles that don't have rolinherit - * set; for such roles, membership implies the ability to do SET ROLE, but - * the privileges are not available until you've done so. + * Type ROLERECURSE_PRIVS recurses only through roles that have rolinherit + * set, while ROLERECURSE_MEMBERS recurses through all roles. This sets + * *is_admin==true if and only if role "roleid" has an ADMIN OPTION membership + * in role "admin_of". * * Since indirect membership testing is relatively expensive, we cache * a list of memberships. Hence, the result is only guaranteed good until - * the next call of roles_has_privs_of()! + * the next call of roles_is_member_of()! * * For the benefit of select_best_grantor, the result is defined to be * in breadth-first order, ie, closer relationships earlier. */ static List * -roles_has_privs_of(Oid roleid) +roles_is_member_of(Oid roleid, enum RoleRecurseType type, + Oid admin_of, bool *is_admin) { + Oid dba; List *roles_list; ListCell *l; - List *new_cached_privs_roles; + List *new_cached_roles; MemoryContext oldctx; - /* If cache is already valid, just return the list */ - if (OidIsValid(cached_privs_role) && cached_privs_role == roleid) - return cached_privs_roles; + /* If cache is valid and ADMIN OPTION not sought, just return the list */ + if (cached_role[type] == roleid && !OidIsValid(admin_of) && + OidIsValid(cached_role[type])) + return cached_roles[type]; /* - * Find all the roles that roleid is a member of, including multi-level - * recursion. The role itself will always be the first element of the - * resulting list. - * - * Each element of the list is scanned to see if it adds any indirect - * memberships. We can use a single list as both the record of - * already-found memberships and the agenda of roles yet to be scanned. - * This is a bit tricky but works because the foreach() macro doesn't - * fetch the next list element until the bottom of the loop. + * Role expansion happens in a non-database backend when guc.c checks + * DEFAULT_ROLE_READ_ALL_SETTINGS for a physical walsender SHOW command. + * In that case, no role gets pg_database_owner. */ - roles_list = list_make1_oid(roleid); - - foreach(l, roles_list) + if (!OidIsValid(MyDatabaseId)) + dba = InvalidOid; + else { - Oid memberid = lfirst_oid(l); - CatCList *memlist; - int i; - - /* Ignore non-inheriting roles */ - if (!has_rolinherit(memberid)) - continue; + HeapTuple dbtup; - /* Find roles that memberid is directly a member of */ - memlist = SearchSysCacheList1(AUTHMEMMEMROLE, - ObjectIdGetDatum(memberid)); - for (i = 0; i < memlist->n_members; i++) - { - HeapTuple tup = &memlist->members[i]->tuple; - Oid otherid = ((Form_pg_auth_members) GETSTRUCT(tup))->roleid; - - /* - * Even though there shouldn't be any loops in the membership - * graph, we must test for having already seen this role. It is - * legal for instance to have both A->B and A->C->B. - */ - roles_list = list_append_unique_oid(roles_list, otherid); - } - ReleaseSysCacheList(memlist); + dbtup = SearchSysCache1(DATABASEOID, ObjectIdGetDatum(MyDatabaseId)); + if (!HeapTupleIsValid(dbtup)) + elog(ERROR, "cache lookup failed for database %u", MyDatabaseId); + dba = ((Form_pg_database) GETSTRUCT(dbtup))->datdba; + ReleaseSysCache(dbtup); } /* - * Copy the completed list into TopMemoryContext so it will persist. - */ - oldctx = MemoryContextSwitchTo(TopMemoryContext); - new_cached_privs_roles = list_copy(roles_list); - MemoryContextSwitchTo(oldctx); - list_free(roles_list); - - /* - * Now safe to assign to state variable - */ - cached_privs_role = InvalidOid; /* just paranoia */ - list_free(cached_privs_roles); - cached_privs_roles = new_cached_privs_roles; - cached_privs_role = roleid; - - /* And now we can return the answer */ - return cached_privs_roles; -} - - -/* - * Get a list of roles that the specified roleid is a member of - * - * This is defined to recurse through roles regardless of rolinherit. - * - * Since indirect membership testing is relatively expensive, we cache - * a list of memberships. Hence, the result is only guaranteed good until - * the next call of roles_is_member_of()! - */ -static List * -roles_is_member_of(Oid roleid) -{ - List *roles_list; - ListCell *l; - List *new_cached_membership_roles; - MemoryContext oldctx; - - /* If cache is already valid, just return the list */ - if (OidIsValid(cached_member_role) && cached_member_role == roleid) - return cached_membership_roles; - - /* * Find all the roles that roleid is a member of, including multi-level * recursion. The role itself will always be the first element of the * resulting list. @@ -4845,6 +4792,9 @@ roles_is_member_of(Oid roleid) CatCList *memlist; int i; + if (type == ROLERECURSE_PRIVS && !has_rolinherit(memberid)) + continue; /* ignore non-inheriting roles */ + /* Find roles that memberid is directly a member of */ memlist = SearchSysCacheList1(AUTHMEMMEMROLE, ObjectIdGetDatum(memberid)); @@ -4854,6 +4804,15 @@ roles_is_member_of(Oid roleid) Oid otherid = ((Form_pg_auth_members) GETSTRUCT(tup))->roleid; /* + * While otherid==InvalidOid shouldn't appear in the catalog, the + * OidIsValid() avoids crashing if that arises. + */ + if (otherid == admin_of && + ((Form_pg_auth_members) GETSTRUCT(tup))->admin_option && + OidIsValid(admin_of)) + *is_admin = true; + + /* * Even though there shouldn't be any loops in the membership * graph, we must test for having already seen this role. It is * legal for instance to have both A->B and A->C->B. @@ -4861,26 +4820,31 @@ roles_is_member_of(Oid roleid) roles_list = list_append_unique_oid(roles_list, otherid); } ReleaseSysCacheList(memlist); + + /* implement pg_database_owner implicit membership */ + if (memberid == dba && OidIsValid(dba)) + roles_list = list_append_unique_oid(roles_list, + DEFAULT_ROLE_DATABASE_OWNER); } /* * Copy the completed list into TopMemoryContext so it will persist. */ oldctx = MemoryContextSwitchTo(TopMemoryContext); - new_cached_membership_roles = list_copy(roles_list); + new_cached_roles = list_copy(roles_list); MemoryContextSwitchTo(oldctx); list_free(roles_list); /* * Now safe to assign to state variable */ - cached_member_role = InvalidOid; /* just paranoia */ - list_free(cached_membership_roles); - cached_membership_roles = new_cached_membership_roles; - cached_member_role = roleid; + cached_role[type] = InvalidOid; /* just paranoia */ + list_free(cached_roles[type]); + cached_roles[type] = new_cached_roles; + cached_role[type] = roleid; /* And now we can return the answer */ - return cached_membership_roles; + return cached_roles[type]; } @@ -4906,7 +4870,9 @@ has_privs_of_role(Oid member, Oid role) * Find all the roles that member has the privileges of, including * multi-level recursion, then see if target role is any one of them. */ - return list_member_oid(roles_has_privs_of(member), role); + return list_member_oid(roles_is_member_of(member, ROLERECURSE_PRIVS, + InvalidOid, NULL), + role); } @@ -4930,7 +4896,9 @@ is_member_of_role(Oid member, Oid role) * Find all the roles that member is a member of, including multi-level * recursion, then see if target role is any one of them. */ - return list_member_oid(roles_is_member_of(member), role); + return list_member_oid(roles_is_member_of(member, ROLERECURSE_MEMBERS, + InvalidOid, NULL), + role); } /* @@ -4964,7 +4932,9 @@ is_member_of_role_nosuper(Oid member, Oid role) * Find all the roles that member is a member of, including multi-level * recursion, then see if target role is any one of them. */ - return list_member_oid(roles_is_member_of(member), role); + return list_member_oid(roles_is_member_of(member, ROLERECURSE_MEMBERS, + InvalidOid, NULL), + role); } @@ -4977,8 +4947,6 @@ bool is_admin_of_role(Oid member, Oid role) { bool result = false; - List *roles_list; - ListCell *l; if (superuser_arg(member)) return true; @@ -5016,44 +4984,7 @@ is_admin_of_role(Oid member, Oid role) return member == GetSessionUserId() && !InLocalUserIdChange() && !InSecurityRestrictedOperation(); - /* - * Find all the roles that member is a member of, including multi-level - * recursion. We build a list in the same way that is_member_of_role does - * to track visited and unvisited roles. - */ - roles_list = list_make1_oid(member); - - foreach(l, roles_list) - { - Oid memberid = lfirst_oid(l); - CatCList *memlist; - int i; - - /* Find roles that memberid is directly a member of */ - memlist = SearchSysCacheList1(AUTHMEMMEMROLE, - ObjectIdGetDatum(memberid)); - for (i = 0; i < memlist->n_members; i++) - { - HeapTuple tup = &memlist->members[i]->tuple; - Oid otherid = ((Form_pg_auth_members) GETSTRUCT(tup))->roleid; - - if (otherid == role && - ((Form_pg_auth_members) GETSTRUCT(tup))->admin_option) - { - /* Found what we came for, so can stop searching */ - result = true; - break; - } - - roles_list = list_append_unique_oid(roles_list, otherid); - } - ReleaseSysCacheList(memlist); - if (result) - break; - } - - list_free(roles_list); - + (void) roles_is_member_of(member, ROLERECURSE_MEMBERS, role, &result); return result; } @@ -5128,7 +5059,8 @@ select_best_grantor(Oid roleId, AclMode privileges, * roles_has_privs_of() throughout this loop, because aclmask_direct() * doesn't query any role memberships. */ - roles_list = roles_has_privs_of(roleId); + roles_list = roles_is_member_of(roleId, ROLERECURSE_PRIVS, + InvalidOid, NULL); /* initialize candidate result as default */ *grantorId = roleId; diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c index fa2b49c..19926f9 100644 --- a/src/backend/utils/cache/catcache.c +++ b/src/backend/utils/cache/catcache.c @@ -1076,8 +1076,9 @@ InitCatCachePhase2(CatCache *cache, bool touch_index) * criticalRelcachesBuilt), we don't have to worry anymore. * * Similarly, during backend startup we have to be able to use the - * pg_authid and pg_auth_members syscaches for authentication even if - * we don't yet have relcache entries for those catalogs' indexes. + * pg_authid, pg_auth_members and pg_database syscaches for + * authentication even if we don't yet have relcache entries for those + * catalogs' indexes. */ static bool IndexScanOK(CatCache *cache, ScanKey cur_skey) @@ -1110,6 +1111,7 @@ IndexScanOK(CatCache *cache, ScanKey cur_skey) case AUTHNAME: case AUTHOID: case AUTHMEMMEMROLE: + case DATABASEOID: /* * Protect authentication lookups occurring before relcache has diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 62540a1..3a72fbb 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -1696,8 +1696,7 @@ setup_privileges(FILE *cmdfd) CppAsString2(RELKIND_VIEW) ", " CppAsString2(RELKIND_MATVIEW) ", " CppAsString2(RELKIND_SEQUENCE) ")" " AND relacl IS NULL;\n\n", - "GRANT USAGE ON SCHEMA pg_catalog TO PUBLIC;\n\n", - "GRANT CREATE, USAGE ON SCHEMA public TO PUBLIC;\n\n", + "GRANT USAGE ON SCHEMA pg_catalog, public TO PUBLIC;\n\n", "REVOKE ALL ON pg_largeobject FROM PUBLIC;\n\n", "INSERT INTO pg_init_privs " " (objoid, classoid, objsubid, initprivs, privtype)" diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c index 60d306e..ea67e52 100644 --- a/src/bin/pg_dump/dumputils.c +++ b/src/bin/pg_dump/dumputils.c @@ -725,6 +725,7 @@ void buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery, PQExpBuffer init_acl_subquery, PQExpBuffer init_racl_subquery, const char *acl_column, const char *acl_owner, + const char *initprivs_expr, const char *obj_kind, bool binary_upgrade) { /* @@ -765,23 +766,25 @@ buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery, "WITH ORDINALITY AS perm(acl,row_n) " "WHERE NOT EXISTS ( " "SELECT 1 FROM " - "pg_catalog.unnest(coalesce(pip.initprivs,pg_catalog.acldefault(%s,%s))) " + "pg_catalog.unnest(coalesce(%s,pg_catalog.acldefault(%s,%s))) " "AS init(init_acl) WHERE acl = init_acl)) as foo)", acl_column, obj_kind, acl_owner, + initprivs_expr, obj_kind, acl_owner); printfPQExpBuffer(racl_subquery, "(SELECT pg_catalog.array_agg(acl ORDER BY row_n) FROM " "(SELECT acl, row_n FROM " - "pg_catalog.unnest(coalesce(pip.initprivs,pg_catalog.acldefault(%s,%s))) " + "pg_catalog.unnest(coalesce(%s,pg_catalog.acldefault(%s,%s))) " "WITH ORDINALITY AS initp(acl,row_n) " "WHERE NOT EXISTS ( " "SELECT 1 FROM " "pg_catalog.unnest(coalesce(%s,pg_catalog.acldefault(%s,%s))) " "AS permp(orig_acl) WHERE acl = orig_acl)) as foo)", + initprivs_expr, obj_kind, acl_owner, acl_column, @@ -807,12 +810,13 @@ buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery, printfPQExpBuffer(init_acl_subquery, "CASE WHEN privtype = 'e' THEN " "(SELECT pg_catalog.array_agg(acl ORDER BY row_n) FROM " - "(SELECT acl, row_n FROM pg_catalog.unnest(pip.initprivs) " + "(SELECT acl, row_n FROM pg_catalog.unnest(%s) " "WITH ORDINALITY AS initp(acl,row_n) " "WHERE NOT EXISTS ( " "SELECT 1 FROM " "pg_catalog.unnest(pg_catalog.acldefault(%s,%s)) " "AS privm(orig_acl) WHERE acl = orig_acl)) as foo) END", + initprivs_expr, obj_kind, acl_owner); @@ -823,10 +827,11 @@ buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery, "pg_catalog.unnest(pg_catalog.acldefault(%s,%s)) " "WITH ORDINALITY AS privp(acl,row_n) " "WHERE NOT EXISTS ( " - "SELECT 1 FROM pg_catalog.unnest(pip.initprivs) " + "SELECT 1 FROM pg_catalog.unnest(%s) " "AS initp(init_acl) WHERE acl = init_acl)) as foo) END", obj_kind, - acl_owner); + acl_owner, + initprivs_expr); } else { diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h index 6e97da7..f5465f1 100644 --- a/src/bin/pg_dump/dumputils.h +++ b/src/bin/pg_dump/dumputils.h @@ -54,6 +54,7 @@ extern void emitShSecLabels(PGconn *conn, PGresult *res, extern void buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery, PQExpBuffer init_acl_subquery, PQExpBuffer init_racl_subquery, const char *acl_column, const char *acl_owner, + const char *initprivs_expr, const char *obj_kind, bool binary_upgrade); extern bool variable_is_guc_list_quote(const char *name); diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 1f82c64..a16d149 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -3597,10 +3597,13 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData) * Actually print the definition. * * Really crude hack for suppressing AUTHORIZATION clause that old pg_dump - * versions put into CREATE SCHEMA. We have to do this when --no-owner - * mode is selected. This is ugly, but I see no other good way ... + * versions put into CREATE SCHEMA. Don't mutate the modern definition + * for schema "public", which consists of a comment. We have to do this + * when --no-owner mode is selected. This is ugly, but I see no other + * good way ... */ - if (ropt->noOwner && strcmp(te->desc, "SCHEMA") == 0) + if (ropt->noOwner && + strcmp(te->desc, "SCHEMA") == 0 && strncmp(te->defn, "--", 2) != 0) { ahprintf(AH, "CREATE SCHEMA %s;\n\n\n", fmtId(te->tag)); } @@ -3612,11 +3615,16 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData) /* * If we aren't using SET SESSION AUTH to determine ownership, we must - * instead issue an ALTER OWNER command. We assume that anything without - * a DROP command is not a separately ownable object. All the categories - * with DROP commands must appear in one list or the other. + * instead issue an ALTER OWNER command. Schema "public" is special; a + * dump never creates it, so we use ALTER OWNER even when using SET + * SESSION for all other objects. We assume that anything without a DROP + * command is not a separately ownable object. All the categories with + * DROP commands must appear in one list or the other. */ - if (!ropt->noOwner && !ropt->use_setsessauth && + if (!ropt->noOwner && + (!ropt->use_setsessauth || + (strcmp(te->tag, "public") == 0 + && strcmp(te->desc, "SCHEMA") == 0)) && te->owner && strlen(te->owner) > 0 && te->dropStmt && strlen(te->dropStmt) > 0) { diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index d99b61e..d59e063 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -44,6 +44,7 @@ #include "catalog/pg_aggregate_d.h" #include "catalog/pg_am_d.h" #include "catalog/pg_attribute_d.h" +#include "catalog/pg_authid_d.h" #include "catalog/pg_cast_d.h" #include "catalog/pg_class_d.h" #include "catalog/pg_collation_d.h" @@ -1564,13 +1565,15 @@ selectDumpableNamespace(NamespaceInfo *nsinfo, Archive *fout) { /* * The public schema is a strange beast that sits in a sort of - * no-mans-land between being a system object and a user object. We - * don't want to dump creation or comment commands for it, because - * that complicates matters for non-superuser use of pg_dump. But we - * should dump any ACL changes that have occurred for it, and of - * course we should dump contained objects. + * no-mans-land between being a system object and a user object. + * CREATE SCHEMA would fail, so its DUMP_COMPONENT_DEFINITION is just + * a comment and an indication of ownership. If the owner is the + * default, omit that superfluous DUMP_COMPONENT_DEFINITION. Before + * v14, the default owner was BOOTSTRAP_SUPERUSERID. */ - nsinfo->dobj.dump = DUMP_COMPONENT_ACL; + nsinfo->dobj.dump = DUMP_COMPONENT_ALL; + if (nsinfo->nspowner == DEFAULT_ROLE_DATABASE_OWNER) + nsinfo->dobj.dump &= ~DUMP_COMPONENT_DEFINITION; nsinfo->dobj.dump_contains = DUMP_COMPONENT_ALL; } else @@ -3352,8 +3355,8 @@ getBlobs(Archive *fout) PQExpBuffer init_racl_subquery = createPQExpBuffer(); buildACLQueries(acl_subquery, racl_subquery, init_acl_subquery, - init_racl_subquery, "l.lomacl", "l.lomowner", "'L'", - dopt->binary_upgrade); + init_racl_subquery, "l.lomacl", "l.lomowner", + "pip.initprivs", "'L'", dopt->binary_upgrade); appendPQExpBuffer(blobQry, "SELECT l.oid, (%s l.lomowner) AS rolname, " @@ -4754,6 +4757,7 @@ getNamespaces(Archive *fout, int *numNamespaces) int i_tableoid; int i_oid; int i_nspname; + int i_nspowner; int i_rolname; int i_nspacl; int i_rnspacl; @@ -4773,11 +4777,32 @@ getNamespaces(Archive *fout, int *numNamespaces) PQExpBuffer init_acl_subquery = createPQExpBuffer(); PQExpBuffer init_racl_subquery = createPQExpBuffer(); + /* + * Bypass pg_init_privs.initprivs for the public schema, for several + * reasons. First, dropping and recreating the schema detaches it + * from its pg_init_privs row, but an empty destination database + * starts with this ACL nonetheless. Second, we support dump/reload + * of public schema ownership changes. ALTER SCHEMA OWNER filters + * nspacl through aclnewowner(), but initprivs continues to reflect + * the initial owner. Hence, synthesize the value that nspacl will + * have after the restore's ALTER SCHEMA OWNER. Third, this makes the + * destination database match the source's ACL, even if the latter was + * an initdb-default ACL, which changed in v14. An upgrade pulls in + * changes to most system object ACLs that the DBA had not customized. + * We've made the public schema depart from that, because changing its + * ACL so easily breaks applications. + */ buildACLQueries(acl_subquery, racl_subquery, init_acl_subquery, - init_racl_subquery, "n.nspacl", "n.nspowner", "'n'", - dopt->binary_upgrade); + init_racl_subquery, "n.nspacl", "n.nspowner", + "CASE WHEN n.nspname = 'public' THEN array[" + " format('%s=UC/%s', " + " n.nspowner::regrole, n.nspowner::regrole)," + " format('=U/%s', n.nspowner::regrole)]::aclitem[] " + "ELSE pip.initprivs END", + "'n'", dopt->binary_upgrade); appendPQExpBuffer(query, "SELECT n.tableoid, n.oid, n.nspname, " + "n.nspowner, " "(%s nspowner) AS rolname, " "%s as nspacl, " "%s as rnspacl, " @@ -4802,7 +4827,7 @@ getNamespaces(Archive *fout, int *numNamespaces) destroyPQExpBuffer(init_racl_subquery); } else - appendPQExpBuffer(query, "SELECT tableoid, oid, nspname, " + appendPQExpBuffer(query, "SELECT tableoid, oid, nspname, nspowner, " "(%s nspowner) AS rolname, " "nspacl, NULL as rnspacl, " "NULL AS initnspacl, NULL as initrnspacl " @@ -4818,6 +4843,7 @@ getNamespaces(Archive *fout, int *numNamespaces) i_tableoid = PQfnumber(res, "tableoid"); i_oid = PQfnumber(res, "oid"); i_nspname = PQfnumber(res, "nspname"); + i_nspowner = PQfnumber(res, "nspowner"); i_rolname = PQfnumber(res, "rolname"); i_nspacl = PQfnumber(res, "nspacl"); i_rnspacl = PQfnumber(res, "rnspacl"); @@ -4831,6 +4857,7 @@ getNamespaces(Archive *fout, int *numNamespaces) nsinfo[i].dobj.catId.oid = atooid(PQgetvalue(res, i, i_oid)); AssignDumpId(&nsinfo[i].dobj); nsinfo[i].dobj.name = pg_strdup(PQgetvalue(res, i, i_nspname)); + nsinfo[i].nspowner = atooid(PQgetvalue(res, i, i_nspowner)); nsinfo[i].rolname = pg_strdup(PQgetvalue(res, i, i_rolname)); nsinfo[i].nspacl = pg_strdup(PQgetvalue(res, i, i_nspacl)); nsinfo[i].rnspacl = pg_strdup(PQgetvalue(res, i, i_rnspacl)); @@ -5022,8 +5049,8 @@ getTypes(Archive *fout, int *numTypes) PQExpBuffer initracl_subquery = createPQExpBuffer(); buildACLQueries(acl_subquery, racl_subquery, initacl_subquery, - initracl_subquery, "t.typacl", "t.typowner", "'T'", - dopt->binary_upgrade); + initracl_subquery, "t.typacl", "t.typowner", + "pip.initprivs", "'T'", dopt->binary_upgrade); appendPQExpBuffer(query, "SELECT t.tableoid, t.oid, t.typname, " "t.typnamespace, " @@ -5724,8 +5751,8 @@ getAggregates(Archive *fout, int *numAggs) const char *agg_check; buildACLQueries(acl_subquery, racl_subquery, initacl_subquery, - initracl_subquery, "p.proacl", "p.proowner", "'f'", - dopt->binary_upgrade); + initracl_subquery, "p.proacl", "p.proowner", + "pip.initprivs", "'f'", dopt->binary_upgrade); agg_check = (fout->remoteVersion >= 110000 ? "p.prokind = 'a'" : "p.proisagg"); @@ -5937,8 +5964,8 @@ getFuncs(Archive *fout, int *numFuncs) const char *not_agg_check; buildACLQueries(acl_subquery, racl_subquery, initacl_subquery, - initracl_subquery, "p.proacl", "p.proowner", "'f'", - dopt->binary_upgrade); + initracl_subquery, "p.proacl", "p.proowner", + "pip.initprivs", "'f'", dopt->binary_upgrade); not_agg_check = (fout->remoteVersion >= 110000 ? "p.prokind <> 'a'" : "NOT p.proisagg"); @@ -6234,13 +6261,14 @@ getTables(Archive *fout, int *numTables) buildACLQueries(acl_subquery, racl_subquery, initacl_subquery, initracl_subquery, "c.relacl", "c.relowner", + "pip.initprivs", "CASE WHEN c.relkind = " CppAsString2(RELKIND_SEQUENCE) " THEN 's' ELSE 'r' END::\"char\"", dopt->binary_upgrade); buildACLQueries(attacl_subquery, attracl_subquery, attinitacl_subquery, - attinitracl_subquery, "at.attacl", "c.relowner", "'c'", - dopt->binary_upgrade); + attinitracl_subquery, "at.attacl", "c.relowner", + "pip.initprivs", "'c'", dopt->binary_upgrade); appendPQExpBuffer(query, "SELECT c.tableoid, c.oid, c.relname, " @@ -8238,8 +8266,8 @@ getProcLangs(Archive *fout, int *numProcLangs) PQExpBuffer initracl_subquery = createPQExpBuffer(); buildACLQueries(acl_subquery, racl_subquery, initacl_subquery, - initracl_subquery, "l.lanacl", "l.lanowner", "'l'", - dopt->binary_upgrade); + initracl_subquery, "l.lanacl", "l.lanowner", + "pip.initprivs", "'l'", dopt->binary_upgrade); /* pg_language has a laninline column */ appendPQExpBuffer(query, "SELECT l.tableoid, l.oid, " @@ -9420,8 +9448,8 @@ getForeignDataWrappers(Archive *fout, int *numForeignDataWrappers) PQExpBuffer initracl_subquery = createPQExpBuffer(); buildACLQueries(acl_subquery, racl_subquery, initacl_subquery, - initracl_subquery, "f.fdwacl", "f.fdwowner", "'F'", - dopt->binary_upgrade); + initracl_subquery, "f.fdwacl", "f.fdwowner", + "pip.initprivs", "'F'", dopt->binary_upgrade); appendPQExpBuffer(query, "SELECT f.tableoid, f.oid, f.fdwname, " "(%s f.fdwowner) AS rolname, " @@ -9587,8 +9615,8 @@ getForeignServers(Archive *fout, int *numForeignServers) PQExpBuffer initracl_subquery = createPQExpBuffer(); buildACLQueries(acl_subquery, racl_subquery, initacl_subquery, - initracl_subquery, "f.srvacl", "f.srvowner", "'S'", - dopt->binary_upgrade); + initracl_subquery, "f.srvacl", "f.srvowner", + "pip.initprivs", "'S'", dopt->binary_upgrade); appendPQExpBuffer(query, "SELECT f.tableoid, f.oid, f.srvname, " "(%s f.srvowner) AS rolname, " @@ -9734,6 +9762,7 @@ getDefaultACLs(Archive *fout, int *numDefaultACLs) buildACLQueries(acl_subquery, racl_subquery, initacl_subquery, initracl_subquery, "defaclacl", "defaclrole", + "pip.initprivs", "CASE WHEN defaclobjtype = 'S' THEN 's' ELSE defaclobjtype END::\"char\"", dopt->binary_upgrade); @@ -9888,6 +9917,28 @@ dumpComment(Archive *fout, const char *type, const char *name, ncomments--; } + /* + * Skip dumping the initdb-provided public schema comment, which would + * complicate matters for non-superuser use of pg_dump. When the DBA has + * removed initdb's comment, replicate that. + */ + if (strcmp(type, "SCHEMA") == 0 && strcmp(name, "public") == 0) + { + static CommentItem empty_comment = {.descr = ""}; + + if (ncomments == 0) + { + comments = &empty_comment; + ncomments = 1; + } + else if (strcmp(comments->descr, (fout->remoteVersion >= 80300 ? + "standard public schema" : + "Standard public schema")) == 0) + { + ncomments = 0; + } + } + /* If a comment exists, build COMMENT ON statement */ if (ncomments > 0) { @@ -10351,9 +10402,19 @@ dumpNamespace(Archive *fout, NamespaceInfo *nspinfo) qnspname = pg_strdup(fmtId(nspinfo->dobj.name)); - appendPQExpBuffer(delq, "DROP SCHEMA %s;\n", qnspname); - - appendPQExpBuffer(q, "CREATE SCHEMA %s;\n", qnspname); + if (strcmp(nspinfo->dobj.name, "public") == 0) + { + /* see selectDumpableNamespace() */ + appendPQExpBufferStr(delq, + "-- *not* dropping schema, since initdb creates it\n"); + appendPQExpBufferStr(q, + "-- *not* creating schema, since initdb creates it\n"); + } + else + { + appendPQExpBuffer(delq, "DROP SCHEMA %s;\n", qnspname); + appendPQExpBuffer(q, "CREATE SCHEMA %s;\n", qnspname); + } if (dopt->binary_upgrade) binary_upgrade_extension_member(q, &nspinfo->dobj, @@ -15501,8 +15562,8 @@ dumpTable(Archive *fout, TableInfo *tbinfo) PQExpBuffer initracl_subquery = createPQExpBuffer(); buildACLQueries(acl_subquery, racl_subquery, initacl_subquery, - initracl_subquery, "at.attacl", "c.relowner", "'c'", - dopt->binary_upgrade); + initracl_subquery, "at.attacl", "c.relowner", + "pip.initprivs", "'c'", dopt->binary_upgrade); appendPQExpBuffer(query, "SELECT at.attname, " diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index 1290f96..51f5c03 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -142,6 +142,7 @@ typedef struct _dumpableObject typedef struct _namespaceInfo { DumpableObject dobj; + Oid nspowner; char *rolname; /* name of owner, or empty string */ char *nspacl; char *rnspacl; diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index 737e464..02b4418 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -125,6 +125,14 @@ my %pgdump_runs = ( 'regress_pg_dump_test', ], }, + defaults_public_owner => { + database => 'regress_public_owner', + dump_cmd => [ + 'pg_dump', '--no-sync', '-f', + "$tempdir/defaults_public_owner.sql", + 'regress_public_owner', + ], + }, # Do not use --no-sync to give test coverage for data sync. defaults_custom_format => { @@ -605,6 +613,26 @@ my %tests = ( unlike => { no_owner => 1, }, }, + 'ALTER SCHEMA public OWNER TO' => { + create_order => 15, + create_sql => + 'ALTER SCHEMA public OWNER TO "regress_quoted \"" role";', + regexp => qr/^ALTER SCHEMA public OWNER TO .+;/m, + like => { + %full_runs, section_pre_data => 1, + }, + unlike => { no_owner => 1, }, + }, + + 'ALTER SCHEMA public OWNER TO (w/o ACL changes)' => { + database => 'regress_public_owner', + create_order => 100, + create_sql => + 'ALTER SCHEMA public OWNER TO "regress_quoted \"" role";', + regexp => qr/^(GRANT|REVOKE)/m, + unlike => { defaults_public_owner => 1 }, + }, + 'ALTER SEQUENCE test_table_col1_seq' => { regexp => qr/^ \QALTER SEQUENCE dump_test.test_table_col1_seq OWNED BY dump_test.test_table.col1;\E @@ -940,6 +968,23 @@ my %tests = ( like => {}, }, + 'COMMENT ON SCHEMA public' => { + regexp => qr/^COMMENT ON SCHEMA public IS .+;/m, + # regress_public_owner emits this, due to create_sql of next test + like => { + pg_dumpall_dbprivs => 1, + pg_dumpall_exclude => 1, + }, + }, + + 'COMMENT ON SCHEMA public IS NULL' => { + database => 'regress_public_owner', + create_order => 100, + create_sql => 'COMMENT ON SCHEMA public IS NULL;', + regexp => qr/^COMMENT ON SCHEMA public IS '';/m, + like => { defaults_public_owner => 1 }, + }, + 'COMMENT ON TABLE dump_test.test_table' => { create_order => 36, create_sql => 'COMMENT ON TABLE dump_test.test_table @@ -1370,6 +1415,18 @@ my %tests = ( }, }, + 'CREATE ROLE regress_quoted...' => { + create_order => 1, + create_sql => 'CREATE ROLE "regress_quoted \"" role";', + regexp => qr/^\QCREATE ROLE "regress_quoted \"" role";\E/m, + like => { + pg_dumpall_dbprivs => 1, + pg_dumpall_exclude => 1, + pg_dumpall_globals => 1, + pg_dumpall_globals_clean => 1, + }, + }, + 'CREATE ACCESS METHOD gist2' => { create_order => 52, create_sql => @@ -3261,6 +3318,23 @@ my %tests = ( unlike => { no_privs => 1, }, }, + # With the exception of the public schema, we don't dump ownership changes + # for objects originating at initdb. Hence, any GRANT or REVOKE affecting + # owner privileges for those objects should reference the bootstrap + # superuser, not the dump-time owner. + 'REVOKE EXECUTE ON FUNCTION pg_stat_reset FROM regress_dump_test_role' => + { + create_order => 15, + create_sql => ' + ALTER FUNCTION pg_stat_reset OWNER TO regress_dump_test_role; + REVOKE EXECUTE ON FUNCTION pg_stat_reset + FROM regress_dump_test_role;', + regexp => qr/^[^-].*pg_stat_reset.* regress_dump_test_role/m, + + # this shouldn't ever get emitted + like => {}, + }, + 'REVOKE SELECT ON TABLE pg_proc FROM public' => { create_order => 45, create_sql => 'REVOKE SELECT ON TABLE pg_proc FROM public;', @@ -3270,12 +3344,12 @@ my %tests = ( unlike => { no_privs => 1, }, }, - 'REVOKE CREATE ON SCHEMA public FROM public' => { + 'REVOKE ALL ON SCHEMA public' => { create_order => 16, - create_sql => 'REVOKE CREATE ON SCHEMA public FROM public;', - regexp => qr/^ - \QREVOKE ALL ON SCHEMA public FROM PUBLIC;\E - \n\QGRANT USAGE ON SCHEMA public TO PUBLIC;\E + create_sql => + 'REVOKE ALL ON SCHEMA public FROM "regress_quoted \"" role";', + regexp => qr/^ + \QREVOKE ALL ON SCHEMA public FROM "regress_quoted \"" role";\E /xm, like => { %full_runs, section_pre_data => 1, }, unlike => { no_privs => 1, }, @@ -3376,8 +3450,9 @@ if ($collation_check_stderr !~ /ERROR: /) $collation_support = 1; } -# Create a second database for certain tests to work against +# Create additional databases for mutations of schema public $node->psql('postgres', 'create database regress_pg_dump_test;'); +$node->psql('postgres', 'create database regress_public_owner;'); # Start with number of command_fails_like()*2 tests below (each # command_fails_like is actually 2 tests) diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index 20af5a9..c8e64b1 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -3508,6 +3508,7 @@ describeRoles(const char *pattern, bool verbose, bool showSystem) printTableAddHeader(&cont, gettext_noop("Role name"), 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); if (verbose && pset.sversion >= 80200) diff --git a/src/include/catalog/pg_authid.dat b/src/include/catalog/pg_authid.dat index 87d917f..4c2bf97 100644 --- a/src/include/catalog/pg_authid.dat +++ b/src/include/catalog/pg_authid.dat @@ -24,6 +24,11 @@ rolcreaterole => 't', rolcreatedb => 't', rolcanlogin => 't', rolreplication => 't', rolbypassrls => 't', rolconnlimit => '-1', rolpassword => '_null_', rolvaliduntil => '_null_' }, +{ oid => '8778', oid_symbol => 'DEFAULT_ROLE_DATABASE_OWNER', + rolname => 'pg_database_owner', rolsuper => 'f', rolinherit => 't', + rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', + rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1', + rolpassword => '_null_', rolvaliduntil => '_null_' }, { oid => '3373', oid_symbol => 'DEFAULT_ROLE_MONITOR', rolname => 'pg_monitor', rolsuper => 'f', rolinherit => 't', rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', diff --git a/src/include/catalog/pg_namespace.dat b/src/include/catalog/pg_namespace.dat index 2ed136b..7932aa6 100644 --- a/src/include/catalog/pg_namespace.dat +++ b/src/include/catalog/pg_namespace.dat @@ -18,8 +18,9 @@ { oid => '99', oid_symbol => 'PG_TOAST_NAMESPACE', descr => 'reserved schema for TOAST tables', nspname => 'pg_toast', nspacl => '_null_' }, +# update dumpComment() if changing this descr { oid => '2200', oid_symbol => 'PG_PUBLIC_NAMESPACE', descr => 'standard public schema', - nspname => 'public', nspacl => '_null_' }, + nspname => 'public', nspowner => 'pg_database_owner', nspacl => '_null_' }, ] diff --git a/src/pl/plperl/expected/plperl_setup.out b/src/pl/plperl/expected/plperl_setup.out index a1a24df..5234feb 100644 --- a/src/pl/plperl/expected/plperl_setup.out +++ b/src/pl/plperl/expected/plperl_setup.out @@ -25,6 +25,9 @@ CREATE EXTENSION plperl; CREATE EXTENSION plperlu; -- fail ERROR: permission denied to create extension "plperlu" HINT: Must be superuser to create this extension. +CREATE SCHEMA plperl_setup_scratch; +SET search_path = plperl_setup_scratch; +GRANT ALL ON SCHEMA plperl_setup_scratch TO regress_user2; CREATE FUNCTION foo1() returns int language plperl as '1;'; SELECT foo1(); foo1 @@ -34,6 +37,7 @@ SELECT foo1(); -- Must reconnect to avoid failure with non-MULTIPLICITY Perl interpreters \c - +SET search_path = plperl_setup_scratch; SET ROLE regress_user1; -- Should be able to change privileges on the language revoke all on language plperl from public; diff --git a/src/pl/plperl/sql/plperl_setup.sql b/src/pl/plperl/sql/plperl_setup.sql index 7484478..a89cf56 100644 --- a/src/pl/plperl/sql/plperl_setup.sql +++ b/src/pl/plperl/sql/plperl_setup.sql @@ -27,12 +27,16 @@ SET ROLE regress_user1; CREATE EXTENSION plperl; CREATE EXTENSION plperlu; -- fail +CREATE SCHEMA plperl_setup_scratch; +SET search_path = plperl_setup_scratch; +GRANT ALL ON SCHEMA plperl_setup_scratch TO regress_user2; CREATE FUNCTION foo1() returns int language plperl as '1;'; SELECT foo1(); -- Must reconnect to avoid failure with non-MULTIPLICITY Perl interpreters \c - +SET search_path = plperl_setup_scratch; SET ROLE regress_user1; diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index 46a69fc..bad4eb0 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -1719,6 +1719,67 @@ SELECT * FROM pg_largeobject LIMIT 0; SET SESSION AUTHORIZATION regress_priv_user1; SELECT * FROM pg_largeobject LIMIT 0; -- to be denied ERROR: permission denied for table pg_largeobject +-- test pg_database_owner +RESET SESSION AUTHORIZATION; +GRANT pg_database_owner TO regress_priv_user1; +ERROR: role "pg_database_owner" cannot have explicit members +GRANT regress_priv_user1 TO pg_database_owner; +ERROR: role "pg_database_owner" cannot be a member of any role +CREATE TABLE datdba_only (); +ALTER TABLE datdba_only OWNER TO pg_database_owner; +REVOKE DELETE ON datdba_only FROM pg_database_owner; +SELECT + pg_has_role('regress_priv_user1', 'pg_database_owner', 'USAGE') as priv, + pg_has_role('regress_priv_user1', 'pg_database_owner', 'MEMBER') as mem, + pg_has_role('regress_priv_user1', 'pg_database_owner', + 'MEMBER WITH ADMIN OPTION') as admin; + priv | mem | admin +------+-----+------- + f | f | f +(1 row) + +BEGIN; +DO $$BEGIN EXECUTE format( + 'ALTER DATABASE %I OWNER TO regress_priv_group2', current_catalog); END$$; +SELECT + pg_has_role('regress_priv_user1', 'pg_database_owner', 'USAGE') as priv, + pg_has_role('regress_priv_user1', 'pg_database_owner', 'MEMBER') as mem, + pg_has_role('regress_priv_user1', 'pg_database_owner', + 'MEMBER WITH ADMIN OPTION') as admin; + priv | mem | admin +------+-----+------- + t | t | f +(1 row) + +SET SESSION AUTHORIZATION regress_priv_user1; +TABLE information_schema.enabled_roles; + role_name +--------------------- + pg_database_owner + regress_priv_user1 + regress_priv_group2 +(3 rows) + +TABLE information_schema.applicable_roles; + grantee | role_name | is_grantable +---------------------+---------------------+-------------- + regress_priv_group2 | pg_database_owner | NO + regress_priv_user1 | regress_priv_group2 | NO +(2 rows) + +INSERT INTO datdba_only DEFAULT VALUES; +SAVEPOINT q; DELETE FROM datdba_only; ROLLBACK TO q; +ERROR: permission denied for table datdba_only +SET SESSION AUTHORIZATION regress_priv_user2; +TABLE information_schema.enabled_roles; + role_name +-------------------- + regress_priv_user2 +(1 row) + +INSERT INTO datdba_only DEFAULT VALUES; +ERROR: permission denied for table datdba_only +ROLLBACK; -- test default ACLs \c - CREATE SCHEMA testns; diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source index c133e73..cb9774e 100644 --- a/src/test/regress/input/tablespace.source +++ b/src/test/regress/input/tablespace.source @@ -388,7 +388,7 @@ CREATE INDEX k ON testschema.tablespace_acl (c) TABLESPACE regress_tblspace; ALTER TABLE testschema.tablespace_acl OWNER TO regress_tablespace_user2; SET SESSION ROLE regress_tablespace_user2; -CREATE TABLE tablespace_table (i int) TABLESPACE regress_tblspace; -- fail +CREATE TEMP TABLE tablespace_table (i int) TABLESPACE regress_tblspace; -- fail ALTER TABLE testschema.tablespace_acl ALTER c TYPE bigint; REINDEX (TABLESPACE regress_tblspace) TABLE tablespace_table; -- fail REINDEX (TABLESPACE regress_tblspace, CONCURRENTLY) TABLE tablespace_table; -- fail @@ -409,3 +409,6 @@ DROP SCHEMA testschema CASCADE; DROP ROLE regress_tablespace_user1; DROP ROLE regress_tablespace_user2; + +-- Rest of this suite can use the public schema freely. +GRANT ALL ON SCHEMA public TO public; diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source index 1bbe7e0..e7629d4 100644 --- a/src/test/regress/output/tablespace.source +++ b/src/test/regress/output/tablespace.source @@ -908,7 +908,7 @@ CREATE TABLE testschema.tablespace_acl (c int); CREATE INDEX k ON testschema.tablespace_acl (c) TABLESPACE regress_tblspace; ALTER TABLE testschema.tablespace_acl OWNER TO regress_tablespace_user2; SET SESSION ROLE regress_tablespace_user2; -CREATE TABLE tablespace_table (i int) TABLESPACE regress_tblspace; -- fail +CREATE TEMP TABLE tablespace_table (i int) TABLESPACE regress_tblspace; -- fail ERROR: permission denied for tablespace regress_tblspace ALTER TABLE testschema.tablespace_acl ALTER c TYPE bigint; REINDEX (TABLESPACE regress_tblspace) TABLE tablespace_table; -- fail @@ -934,3 +934,5 @@ drop cascades to table testschema.atable drop cascades to table testschema.tablespace_acl DROP ROLE regress_tablespace_user1; DROP ROLE regress_tablespace_user2; +-- Rest of this suite can use the public schema freely. +GRANT ALL ON SCHEMA public TO public; diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index 6277140..b45719c 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -1034,6 +1034,37 @@ SELECT * FROM pg_largeobject LIMIT 0; SET SESSION AUTHORIZATION regress_priv_user1; SELECT * FROM pg_largeobject LIMIT 0; -- to be denied +-- test pg_database_owner +RESET SESSION AUTHORIZATION; +GRANT pg_database_owner TO regress_priv_user1; +GRANT regress_priv_user1 TO pg_database_owner; +CREATE TABLE datdba_only (); +ALTER TABLE datdba_only OWNER TO pg_database_owner; +REVOKE DELETE ON datdba_only FROM pg_database_owner; +SELECT + pg_has_role('regress_priv_user1', 'pg_database_owner', 'USAGE') as priv, + pg_has_role('regress_priv_user1', 'pg_database_owner', 'MEMBER') as mem, + pg_has_role('regress_priv_user1', 'pg_database_owner', + 'MEMBER WITH ADMIN OPTION') as admin; + +BEGIN; +DO $$BEGIN EXECUTE format( + 'ALTER DATABASE %I OWNER TO regress_priv_group2', current_catalog); END$$; +SELECT + pg_has_role('regress_priv_user1', 'pg_database_owner', 'USAGE') as priv, + pg_has_role('regress_priv_user1', 'pg_database_owner', 'MEMBER') as mem, + pg_has_role('regress_priv_user1', 'pg_database_owner', + 'MEMBER WITH ADMIN OPTION') as admin; +SET SESSION AUTHORIZATION regress_priv_user1; +TABLE information_schema.enabled_roles; +TABLE information_schema.applicable_roles; +INSERT INTO datdba_only DEFAULT VALUES; +SAVEPOINT q; DELETE FROM datdba_only; ROLLBACK TO q; +SET SESSION AUTHORIZATION regress_priv_user2; +TABLE information_schema.enabled_roles; +INSERT INTO datdba_only DEFAULT VALUES; +ROLLBACK; + -- test default ACLs \c -