From 48ca2904c11d4293ec0bed1625259b2e4ef550cc Mon Sep 17 00:00:00 2001 From: Noah Misch Date: Mon, 9 May 2022 08:35:08 -0700 Subject: [PATCH] Make relation-enumerating operations be security-restricted operations. When a feature enumerates relations and runs functions associated with all found relations, the feature's user shall not need to trust every user having permission to create objects. BRIN-specific functionality in autovacuum neglected to account for this, as did pg_amcheck and CLUSTER. An attacker having permission to create non-temp objects in at least one schema could execute arbitrary SQL functions under the identity of the bootstrap superuser. CREATE INDEX (not a relation-enumerating operation) and REINDEX protected themselves too late. This change extends to the non-enumerating amcheck interface. Back-patch to v10 (all supported versions). Sergey Shinderuk, reviewed (in earlier versions) by Alexander Lakhin. Reported by Alexander Lakhin. Security: CVE-2022-1552 --- contrib/amcheck/expected/check_btree.out | 23 +++++++ contrib/amcheck/sql/check_btree.sql | 21 +++++++ contrib/amcheck/verify_nbtree.c | 28 +++++++++ src/backend/access/brin/brin.c | 30 ++++++++- src/backend/catalog/index.c | 41 ++++++++---- src/backend/commands/cluster.c | 35 ++++++++--- src/backend/commands/indexcmds.c | 79 +++++++++++++++++++++++- src/backend/utils/init/miscinit.c | 24 ++++--- src/test/regress/expected/privileges.out | 63 +++++++++++++++++++ src/test/regress/sql/privileges.sql | 47 ++++++++++++++ 10 files changed, 361 insertions(+), 30 deletions(-) diff --git a/contrib/amcheck/expected/check_btree.out b/contrib/amcheck/expected/check_btree.out index ab8c29e0b2..48b559c7ee 100644 --- a/contrib/amcheck/expected/check_btree.out +++ b/contrib/amcheck/expected/check_btree.out @@ -167,11 +167,34 @@ SELECT bt_index_check('toasty', true); (1 row) +-- +-- Check that index expressions and predicates are run as the table's owner +-- +TRUNCATE bttest_a; +INSERT INTO bttest_a SELECT * FROM generate_series(1, 1000); +ALTER TABLE bttest_a OWNER TO regress_bttest_role; +-- A dummy index function checking current_user +CREATE FUNCTION ifun(int8) RETURNS int8 AS $$ +BEGIN + ASSERT current_user = 'regress_bttest_role', + format('ifun(%s) called by %s', $1, current_user); + RETURN $1; +END; +$$ LANGUAGE plpgsql IMMUTABLE; +CREATE INDEX bttest_a_expr_idx ON bttest_a ((ifun(id) + ifun(0))) + WHERE ifun(id + 10) > ifun(10); +SELECT bt_index_check('bttest_a_expr_idx', true); + bt_index_check +---------------- + +(1 row) + -- cleanup DROP TABLE bttest_a; DROP TABLE bttest_b; DROP TABLE bttest_multi; DROP TABLE delete_test_table; DROP TABLE toast_bug; +DROP FUNCTION ifun(int8); DROP OWNED BY regress_bttest_role; -- permissions DROP ROLE regress_bttest_role; diff --git a/contrib/amcheck/sql/check_btree.sql b/contrib/amcheck/sql/check_btree.sql index fdce3c3d47..2967cafc97 100644 --- a/contrib/amcheck/sql/check_btree.sql +++ b/contrib/amcheck/sql/check_btree.sql @@ -109,11 +109,32 @@ INSERT INTO toast_bug SELECT repeat('a', 2200); -- Should not get false positive report of corruption: SELECT bt_index_check('toasty', true); +-- +-- Check that index expressions and predicates are run as the table's owner +-- +TRUNCATE bttest_a; +INSERT INTO bttest_a SELECT * FROM generate_series(1, 1000); +ALTER TABLE bttest_a OWNER TO regress_bttest_role; +-- A dummy index function checking current_user +CREATE FUNCTION ifun(int8) RETURNS int8 AS $$ +BEGIN + ASSERT current_user = 'regress_bttest_role', + format('ifun(%s) called by %s', $1, current_user); + RETURN $1; +END; +$$ LANGUAGE plpgsql IMMUTABLE; + +CREATE INDEX bttest_a_expr_idx ON bttest_a ((ifun(id) + ifun(0))) + WHERE ifun(id + 10) > ifun(10); + +SELECT bt_index_check('bttest_a_expr_idx', true); + -- cleanup DROP TABLE bttest_a; DROP TABLE bttest_b; DROP TABLE bttest_multi; DROP TABLE delete_test_table; DROP TABLE toast_bug; +DROP FUNCTION ifun(int8); DROP OWNED BY regress_bttest_role; -- permissions DROP ROLE regress_bttest_role; diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c index f486467990..31326ae8d2 100644 --- a/contrib/amcheck/verify_nbtree.c +++ b/contrib/amcheck/verify_nbtree.c @@ -34,6 +34,7 @@ #include "miscadmin.h" #include "storage/lmgr.h" #include "storage/smgr.h" +#include "utils/guc.h" #include "utils/memutils.h" #include "utils/snapmgr.h" @@ -206,6 +207,9 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed) Relation indrel; Relation heaprel; LOCKMODE lockmode; + Oid save_userid; + int save_sec_context; + int save_nestlevel; if (parentcheck) lockmode = ShareLock; @@ -222,9 +226,27 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed) */ heapid = IndexGetRelation(indrelid, true); if (OidIsValid(heapid)) + { heaprel = heap_open(heapid, lockmode); + + /* + * Switch to the table owner's userid, so that any index functions are + * run as that user. Also lock down security-restricted operations + * and arrange to make GUC variable changes local to this command. + */ + GetUserIdAndSecContext(&save_userid, &save_sec_context); + SetUserIdAndSecContext(heaprel->rd_rel->relowner, + save_sec_context | SECURITY_RESTRICTED_OPERATION); + save_nestlevel = NewGUCNestLevel(); + } else + { heaprel = NULL; + /* for "gcc -Og" https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78394 */ + save_userid = InvalidOid; + save_sec_context = -1; + save_nestlevel = -1; + } /* * Open the target index relations separately (like relation_openrv(), but @@ -267,6 +289,12 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed) bt_check_every_level(indrel, heaprel, parentcheck, heapallindexed); } + /* Roll back any GUC changes executed by index functions */ + AtEOXact_GUC(false, save_nestlevel); + + /* Restore userid and security context */ + SetUserIdAndSecContext(save_userid, save_sec_context); + /* * Release locks early. That's ok here because nothing in the called * routines will trigger shared cache invalidations to be sent, so we can diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index b7a662f576..a39faf0100 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -30,6 +30,7 @@ #include "storage/bufmgr.h" #include "storage/freespace.h" #include "utils/builtins.h" +#include "utils/guc.h" #include "utils/index_selfuncs.h" #include "utils/memutils.h" #include "utils/rel.h" @@ -869,6 +870,9 @@ brin_summarize_range(PG_FUNCTION_ARGS) Oid heapoid; Relation indexRel; Relation heapRel; + Oid save_userid; + int save_sec_context; + int save_nestlevel; double numSummarized = 0; if (RecoveryInProgress()) @@ -895,7 +899,22 @@ brin_summarize_range(PG_FUNCTION_ARGS) */ heapoid = IndexGetRelation(indexoid, true); if (OidIsValid(heapoid)) + { heapRel = heap_open(heapoid, ShareUpdateExclusiveLock); + + /* + * Autovacuum calls us. For its benefit, switch to the table owner's + * userid, so that any index functions are run as that user. Also + * lock down security-restricted operations and arrange to make GUC + * variable changes local to this command. This is harmless, albeit + * unnecessary, when called from SQL, because we fail shortly if the + * user does not own the index. + */ + GetUserIdAndSecContext(&save_userid, &save_sec_context); + SetUserIdAndSecContext(heapRel->rd_rel->relowner, + save_sec_context | SECURITY_RESTRICTED_OPERATION); + save_nestlevel = NewGUCNestLevel(); + } else heapRel = NULL; @@ -910,7 +929,7 @@ brin_summarize_range(PG_FUNCTION_ARGS) RelationGetRelationName(indexRel)))); /* User must own the index (comparable to privileges needed for VACUUM) */ - if (!pg_class_ownercheck(indexoid, GetUserId())) + if (heapRel != NULL && !pg_class_ownercheck(indexoid, save_userid)) aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_INDEX, RelationGetRelationName(indexRel)); @@ -928,6 +947,12 @@ brin_summarize_range(PG_FUNCTION_ARGS) /* OK, do it */ brinsummarize(indexRel, heapRel, heapBlk, true, &numSummarized, NULL); + /* Roll back any GUC changes executed by index functions */ + AtEOXact_GUC(false, save_nestlevel); + + /* Restore userid and security context */ + SetUserIdAndSecContext(save_userid, save_sec_context); + relation_close(indexRel, ShareUpdateExclusiveLock); relation_close(heapRel, ShareUpdateExclusiveLock); @@ -969,6 +994,9 @@ brin_desummarize_range(PG_FUNCTION_ARGS) * passed indexoid isn't an index then IndexGetRelation() will fail. * Rather than emitting a not-very-helpful error message, postpone * complaining, expecting that the is-it-an-index test below will fail. + * + * Unlike brin_summarize_range(), autovacuum never calls this. Hence, we + * don't switch userid. */ heapoid = IndexGetRelation(indexoid, true); if (OidIsValid(heapoid)) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 75ade78647..234bf7f854 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -3271,7 +3271,17 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot) /* Open and lock the parent heap relation */ heapRelation = heap_open(heapId, ShareUpdateExclusiveLock); - /* And the target index relation */ + + /* + * Switch to the table owner's userid, so that any index functions are run + * as that user. Also lock down security-restricted operations and + * arrange to make GUC variable changes local to this command. + */ + GetUserIdAndSecContext(&save_userid, &save_sec_context); + SetUserIdAndSecContext(heapRelation->rd_rel->relowner, + save_sec_context | SECURITY_RESTRICTED_OPERATION); + save_nestlevel = NewGUCNestLevel(); + indexRelation = index_open(indexId, RowExclusiveLock); /* @@ -3284,16 +3294,6 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot) /* mark build is concurrent just for consistency */ indexInfo->ii_Concurrent = true; - /* - * Switch to the table owner's userid, so that any index functions are run - * as that user. Also lock down security-restricted operations and - * arrange to make GUC variable changes local to this command. - */ - GetUserIdAndSecContext(&save_userid, &save_sec_context); - SetUserIdAndSecContext(heapRelation->rd_rel->relowner, - save_sec_context | SECURITY_RESTRICTED_OPERATION); - save_nestlevel = NewGUCNestLevel(); - /* * Scan the index and gather up all the TIDs into a tuplesort object. */ @@ -3756,6 +3756,9 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, Relation iRel, heapRelation; Oid heapId; + Oid save_userid; + int save_sec_context; + int save_nestlevel; IndexInfo *indexInfo; volatile bool skipped_constraint = false; PGRUsage ru0; @@ -3769,6 +3772,16 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, heapId = IndexGetRelation(indexId, false); heapRelation = heap_open(heapId, ShareLock); + /* + * Switch to the table owner's userid, so that any index functions are run + * as that user. Also lock down security-restricted operations and + * arrange to make GUC variable changes local to this command. + */ + GetUserIdAndSecContext(&save_userid, &save_sec_context); + SetUserIdAndSecContext(heapRelation->rd_rel->relowner, + save_sec_context | SECURITY_RESTRICTED_OPERATION); + save_nestlevel = NewGUCNestLevel(); + /* * Open the target index relation and get an exclusive lock on it, to * ensure that no one else is touching this particular index. @@ -3918,6 +3931,12 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, errdetail_internal("%s", pg_rusage_show(&ru0)))); + /* Roll back any GUC changes executed by index functions */ + AtEOXact_GUC(false, save_nestlevel); + + /* Restore userid and security context */ + SetUserIdAndSecContext(save_userid, save_sec_context); + /* Close rels, but keep locks */ index_close(iRel, NoLock); heap_close(heapRelation, NoLock); diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index ac7277e2de..1aeafa3862 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -44,6 +44,7 @@ #include "storage/smgr.h" #include "utils/acl.h" #include "utils/fmgroids.h" +#include "utils/guc.h" #include "utils/inval.h" #include "utils/lsyscache.h" #include "utils/memutils.h" @@ -268,6 +269,9 @@ void cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose) { Relation OldHeap; + Oid save_userid; + int save_sec_context; + int save_nestlevel; /* Check for user-requested abort. */ CHECK_FOR_INTERRUPTS(); @@ -284,6 +288,16 @@ cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose) if (!OldHeap) return; + /* + * Switch to the table owner's userid, so that any index functions are run + * as that user. Also lock down security-restricted operations and + * arrange to make GUC variable changes local to this command. + */ + GetUserIdAndSecContext(&save_userid, &save_sec_context); + SetUserIdAndSecContext(OldHeap->rd_rel->relowner, + save_sec_context | SECURITY_RESTRICTED_OPERATION); + save_nestlevel = NewGUCNestLevel(); + /* * Since we may open a new transaction for each relation, we have to check * that the relation still is what we think it is. @@ -298,10 +312,10 @@ cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose) Form_pg_index indexForm; /* Check that the user still owns the relation */ - if (!pg_class_ownercheck(tableOid, GetUserId())) + if (!pg_class_ownercheck(tableOid, save_userid)) { relation_close(OldHeap, AccessExclusiveLock); - return; + goto out; } /* @@ -315,7 +329,7 @@ cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose) if (RELATION_IS_OTHER_TEMP(OldHeap)) { relation_close(OldHeap, AccessExclusiveLock); - return; + goto out; } if (OidIsValid(indexOid)) @@ -326,7 +340,7 @@ cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose) if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(indexOid))) { relation_close(OldHeap, AccessExclusiveLock); - return; + goto out; } /* @@ -336,14 +350,14 @@ cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose) if (!HeapTupleIsValid(tuple)) /* probably can't happen */ { relation_close(OldHeap, AccessExclusiveLock); - return; + goto out; } indexForm = (Form_pg_index) GETSTRUCT(tuple); if (!indexForm->indisclustered) { ReleaseSysCache(tuple); relation_close(OldHeap, AccessExclusiveLock); - return; + goto out; } ReleaseSysCache(tuple); } @@ -397,7 +411,7 @@ cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose) !RelationIsPopulated(OldHeap)) { relation_close(OldHeap, AccessExclusiveLock); - return; + goto out; } /* @@ -412,6 +426,13 @@ cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose) rebuild_relation(OldHeap, indexOid, verbose); /* NB: rebuild_relation does heap_close() on OldHeap */ + +out: + /* Roll back any GUC changes executed by index functions */ + AtEOXact_GUC(false, save_nestlevel); + + /* Restore userid and security context */ + SetUserIdAndSecContext(save_userid, save_sec_context); } /* diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index db83d20e68..30dc0376eb 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -55,6 +55,7 @@ #include "utils/acl.h" #include "utils/builtins.h" #include "utils/fmgroids.h" +#include "utils/guc.h" #include "utils/inval.h" #include "utils/lsyscache.h" #include "utils/memutils.h" @@ -371,8 +372,13 @@ DefineIndex(Oid relationId, LOCKTAG heaplocktag; LOCKMODE lockmode; Snapshot snapshot; + Oid root_save_userid; + int root_save_sec_context; + int root_save_nestlevel; int i; + root_save_nestlevel = NewGUCNestLevel(); + /* * Force non-concurrent build on temporary relations, even if CONCURRENTLY * was requested. Other backends can't access a temporary relation, so @@ -430,6 +436,15 @@ DefineIndex(Oid relationId, lockmode = concurrent ? ShareUpdateExclusiveLock : ShareLock; rel = heap_open(relationId, lockmode); + /* + * Switch to the table owner's userid, so that any index functions are run + * as that user. Also lock down security-restricted operations. We + * already arranged to make GUC variable changes local to this command. + */ + GetUserIdAndSecContext(&root_save_userid, &root_save_sec_context); + SetUserIdAndSecContext(rel->rd_rel->relowner, + root_save_sec_context | SECURITY_RESTRICTED_OPERATION); + relationId = RelationGetRelid(rel); namespaceId = RelationGetNamespace(rel); @@ -516,7 +531,7 @@ DefineIndex(Oid relationId, { AclResult aclresult; - aclresult = pg_namespace_aclcheck(namespaceId, GetUserId(), + aclresult = pg_namespace_aclcheck(namespaceId, root_save_userid, ACL_CREATE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_SCHEMA, @@ -543,7 +558,7 @@ DefineIndex(Oid relationId, { AclResult aclresult; - aclresult = pg_tablespace_aclcheck(tablespaceId, GetUserId(), + aclresult = pg_tablespace_aclcheck(tablespaceId, root_save_userid, ACL_CREATE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_TABLESPACE, @@ -939,10 +954,25 @@ DefineIndex(Oid relationId, if (!OidIsValid(indexRelationId)) { + /* Roll back any GUC changes executed by index functions. */ + AtEOXact_GUC(false, root_save_nestlevel); + + /* Restore userid and security context */ + SetUserIdAndSecContext(root_save_userid, root_save_sec_context); + heap_close(rel, NoLock); return address; } + /* + * Roll back any GUC changes executed by index functions, and keep + * subsequent changes local to this command. It's barely possible that + * some index function changed a behavior-affecting GUC, e.g. xmloption, + * that affects subsequent steps. + */ + AtEOXact_GUC(false, root_save_nestlevel); + root_save_nestlevel = NewGUCNestLevel(); + /* Add any requested comment */ if (stmt->idxcomment != NULL) CreateComments(indexRelationId, RelationRelationId, 0, @@ -986,6 +1016,9 @@ DefineIndex(Oid relationId, { Oid childRelid = part_oids[i]; Relation childrel; + Oid child_save_userid; + int child_save_sec_context; + int child_save_nestlevel; List *childidxs; ListCell *cell; AttrNumber *attmap; @@ -994,6 +1027,12 @@ DefineIndex(Oid relationId, childrel = heap_open(childRelid, lockmode); + GetUserIdAndSecContext(&child_save_userid, + &child_save_sec_context); + SetUserIdAndSecContext(childrel->rd_rel->relowner, + child_save_sec_context | SECURITY_RESTRICTED_OPERATION); + child_save_nestlevel = NewGUCNestLevel(); + /* * Don't try to create indexes on foreign tables, though. * Skip those if a regular index, or fail if trying to create @@ -1009,6 +1048,9 @@ DefineIndex(Oid relationId, errdetail("Table \"%s\" contains partitions that are foreign tables.", RelationGetRelationName(rel)))); + AtEOXact_GUC(false, child_save_nestlevel); + SetUserIdAndSecContext(child_save_userid, + child_save_sec_context); heap_close(childrel, lockmode); continue; } @@ -1081,6 +1123,9 @@ DefineIndex(Oid relationId, } list_free(childidxs); + AtEOXact_GUC(false, child_save_nestlevel); + SetUserIdAndSecContext(child_save_userid, + child_save_sec_context); heap_close(childrel, NoLock); /* @@ -1136,12 +1181,21 @@ DefineIndex(Oid relationId, if (found_whole_row) elog(ERROR, "cannot convert whole-row table reference"); + /* + * Recurse as the starting user ID. Callee will use that + * for permission checks, then switch again. + */ + Assert(GetUserId() == child_save_userid); + SetUserIdAndSecContext(root_save_userid, + root_save_sec_context); DefineIndex(childRelid, childStmt, InvalidOid, /* no predefined OID */ indexRelationId, /* this is our child */ createdConstraintId, is_alter_table, check_rights, check_not_in_use, skip_build, quiet); + SetUserIdAndSecContext(child_save_userid, + child_save_sec_context); } pfree(attmap); @@ -1176,10 +1230,15 @@ DefineIndex(Oid relationId, * Indexes on partitioned tables are not themselves built, so we're * done here. */ + AtEOXact_GUC(false, root_save_nestlevel); + SetUserIdAndSecContext(root_save_userid, root_save_sec_context); heap_close(rel, NoLock); return address; } + AtEOXact_GUC(false, root_save_nestlevel); + SetUserIdAndSecContext(root_save_userid, root_save_sec_context); + if (!concurrent) { /* Close the heap and we're done, in the non-concurrent case */ @@ -1258,6 +1317,16 @@ DefineIndex(Oid relationId, /* Open and lock the parent heap relation */ rel = heap_openrv(stmt->relation, ShareUpdateExclusiveLock); + /* + * Switch to the table owner's userid, so that any index functions are run + * as that user. Also lock down security-restricted operations and + * arrange to make GUC variable changes local to this command. + */ + GetUserIdAndSecContext(&root_save_userid, &root_save_sec_context); + SetUserIdAndSecContext(rel->rd_rel->relowner, + root_save_sec_context | SECURITY_RESTRICTED_OPERATION); + root_save_nestlevel = NewGUCNestLevel(); + /* And the target index relation */ indexRelation = index_open(indexRelationId, RowExclusiveLock); @@ -1273,6 +1342,12 @@ DefineIndex(Oid relationId, /* Now build the index */ index_build(rel, indexRelation, indexInfo, stmt->primary, false, true); + /* Roll back any GUC changes executed by index functions */ + AtEOXact_GUC(false, root_save_nestlevel); + + /* Restore userid and security context */ + SetUserIdAndSecContext(root_save_userid, root_save_sec_context); + /* Close both the relations, but keep the locks */ heap_close(rel, NoLock); index_close(indexRelation, NoLock); diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c index 407fe64271..c172295ffd 100644 --- a/src/backend/utils/init/miscinit.c +++ b/src/backend/utils/init/miscinit.c @@ -462,15 +462,21 @@ GetAuthenticatedUserId(void) * with guc.c's internal state, so SET ROLE has to be disallowed. * * SECURITY_RESTRICTED_OPERATION indicates that we are inside an operation - * that does not wish to trust called user-defined functions at all. This - * bit prevents not only SET ROLE, but various other changes of session state - * that normally is unprotected but might possibly be used to subvert the - * calling session later. An example is replacing an existing prepared - * statement with new code, which will then be executed with the outer - * session's permissions when the prepared statement is next used. Since - * these restrictions are fairly draconian, we apply them only in contexts - * where the called functions are really supposed to be side-effect-free - * anyway, such as VACUUM/ANALYZE/REINDEX. + * that does not wish to trust called user-defined functions at all. The + * policy is to use this before operations, e.g. autovacuum and REINDEX, that + * enumerate relations of a database or schema and run functions associated + * with each found relation. The relation owner is the new user ID. Set this + * as soon as possible after locking the relation. Restore the old user ID as + * late as possible before closing the relation; restoring it shortly after + * close is also tolerable. If a command has both relation-enumerating and + * non-enumerating modes, e.g. ANALYZE, both modes set this bit. This bit + * prevents not only SET ROLE, but various other changes of session state that + * normally is unprotected but might possibly be used to subvert the calling + * session later. An example is replacing an existing prepared statement with + * new code, which will then be executed with the outer session's permissions + * when the prepared statement is next used. These restrictions are fairly + * draconian, but the functions called in relation-enumerating operations are + * really supposed to be side-effect-free anyway. * * SECURITY_NOFORCE_RLS indicates that we are inside an operation which should * ignore the FORCE ROW LEVEL SECURITY per-table indication. This is used to diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index 478c2cdf01..966d6ecfb0 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -1336,6 +1336,69 @@ SELECT has_table_privilege('regress_priv_user1', 'atest4', 'SELECT WITH GRANT OP -- security-restricted operations \c - CREATE ROLE regress_sro_user; +-- Check that index expressions and predicates are run as the table's owner +-- A dummy index function checking current_user +CREATE FUNCTION sro_ifun(int) RETURNS int AS $$ +BEGIN + -- Below we set the table's owner to regress_sro_user + ASSERT current_user = 'regress_sro_user', + format('sro_ifun(%s) called by %s', $1, current_user); + RETURN $1; +END; +$$ LANGUAGE plpgsql IMMUTABLE; +-- Create a table owned by regress_sro_user +CREATE TABLE sro_tab (a int); +ALTER TABLE sro_tab OWNER TO regress_sro_user; +INSERT INTO sro_tab VALUES (1), (2), (3); +-- Create an expression index with a predicate +CREATE INDEX sro_idx ON sro_tab ((sro_ifun(a) + sro_ifun(0))) + WHERE sro_ifun(a + 10) > sro_ifun(10); +DROP INDEX sro_idx; +-- Do the same concurrently +CREATE INDEX CONCURRENTLY sro_idx ON sro_tab ((sro_ifun(a) + sro_ifun(0))) + WHERE sro_ifun(a + 10) > sro_ifun(10); +-- REINDEX +REINDEX TABLE sro_tab; +REINDEX INDEX sro_idx; +REINDEX TABLE CONCURRENTLY sro_tab; -- v12+ feature +ERROR: syntax error at or near "CONCURRENTLY" +LINE 1: REINDEX TABLE CONCURRENTLY sro_tab; + ^ +DROP INDEX sro_idx; +-- CLUSTER +CREATE INDEX sro_cluster_idx ON sro_tab ((sro_ifun(a) + sro_ifun(0))); +CLUSTER sro_tab USING sro_cluster_idx; +DROP INDEX sro_cluster_idx; +-- BRIN index +CREATE INDEX sro_brin ON sro_tab USING brin ((sro_ifun(a) + sro_ifun(0))); +SELECT brin_desummarize_range('sro_brin', 0); + brin_desummarize_range +------------------------ + +(1 row) + +SELECT brin_summarize_range('sro_brin', 0); + brin_summarize_range +---------------------- + 1 +(1 row) + +DROP TABLE sro_tab; +-- Check with a partitioned table +CREATE TABLE sro_ptab (a int) PARTITION BY RANGE (a); +ALTER TABLE sro_ptab OWNER TO regress_sro_user; +CREATE TABLE sro_part PARTITION OF sro_ptab FOR VALUES FROM (1) TO (10); +ALTER TABLE sro_part OWNER TO regress_sro_user; +INSERT INTO sro_ptab VALUES (1), (2), (3); +CREATE INDEX sro_pidx ON sro_ptab ((sro_ifun(a) + sro_ifun(0))) + WHERE sro_ifun(a + 10) > sro_ifun(10); +REINDEX TABLE sro_ptab; +WARNING: REINDEX of partitioned tables is not yet implemented, skipping "sro_ptab" +NOTICE: table "sro_ptab" has no indexes +REINDEX INDEX CONCURRENTLY sro_pidx; -- v12+ feature +ERROR: syntax error at or near "CONCURRENTLY" +LINE 1: REINDEX INDEX CONCURRENTLY sro_pidx; + ^ SET SESSION AUTHORIZATION regress_sro_user; CREATE FUNCTION unwanted_grant() RETURNS void LANGUAGE sql AS 'GRANT regress_priv_group2 TO regress_sro_user'; diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index b19e7b3888..d5e838a5a9 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -826,6 +826,53 @@ SELECT has_table_privilege('regress_priv_user1', 'atest4', 'SELECT WITH GRANT OP \c - CREATE ROLE regress_sro_user; +-- Check that index expressions and predicates are run as the table's owner + +-- A dummy index function checking current_user +CREATE FUNCTION sro_ifun(int) RETURNS int AS $$ +BEGIN + -- Below we set the table's owner to regress_sro_user + ASSERT current_user = 'regress_sro_user', + format('sro_ifun(%s) called by %s', $1, current_user); + RETURN $1; +END; +$$ LANGUAGE plpgsql IMMUTABLE; +-- Create a table owned by regress_sro_user +CREATE TABLE sro_tab (a int); +ALTER TABLE sro_tab OWNER TO regress_sro_user; +INSERT INTO sro_tab VALUES (1), (2), (3); +-- Create an expression index with a predicate +CREATE INDEX sro_idx ON sro_tab ((sro_ifun(a) + sro_ifun(0))) + WHERE sro_ifun(a + 10) > sro_ifun(10); +DROP INDEX sro_idx; +-- Do the same concurrently +CREATE INDEX CONCURRENTLY sro_idx ON sro_tab ((sro_ifun(a) + sro_ifun(0))) + WHERE sro_ifun(a + 10) > sro_ifun(10); +-- REINDEX +REINDEX TABLE sro_tab; +REINDEX INDEX sro_idx; +REINDEX TABLE CONCURRENTLY sro_tab; -- v12+ feature +DROP INDEX sro_idx; +-- CLUSTER +CREATE INDEX sro_cluster_idx ON sro_tab ((sro_ifun(a) + sro_ifun(0))); +CLUSTER sro_tab USING sro_cluster_idx; +DROP INDEX sro_cluster_idx; +-- BRIN index +CREATE INDEX sro_brin ON sro_tab USING brin ((sro_ifun(a) + sro_ifun(0))); +SELECT brin_desummarize_range('sro_brin', 0); +SELECT brin_summarize_range('sro_brin', 0); +DROP TABLE sro_tab; +-- Check with a partitioned table +CREATE TABLE sro_ptab (a int) PARTITION BY RANGE (a); +ALTER TABLE sro_ptab OWNER TO regress_sro_user; +CREATE TABLE sro_part PARTITION OF sro_ptab FOR VALUES FROM (1) TO (10); +ALTER TABLE sro_part OWNER TO regress_sro_user; +INSERT INTO sro_ptab VALUES (1), (2), (3); +CREATE INDEX sro_pidx ON sro_ptab ((sro_ifun(a) + sro_ifun(0))) + WHERE sro_ifun(a + 10) > sro_ifun(10); +REINDEX TABLE sro_ptab; +REINDEX INDEX CONCURRENTLY sro_pidx; -- v12+ feature + SET SESSION AUTHORIZATION regress_sro_user; CREATE FUNCTION unwanted_grant() RETURNS void LANGUAGE sql AS 'GRANT regress_priv_group2 TO regress_sro_user';