aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNoah Misch <noah@leadboat.com>2022-05-09 08:35:08 -0700
committerNoah Misch <noah@leadboat.com>2022-05-09 08:35:12 -0700
commit7f098f7b53edfe8340c6e031bf497308a2e65e2e (patch)
tree3a4aac76f5669b94faee6741819e3ae6f56393ae
parentd387588601cdf303a28e71c2ec6f337a3b08ab64 (diff)
downloadpostgresql-7f098f7b53edfe8340c6e031bf497308a2e65e2e.tar.gz
postgresql-7f098f7b53edfe8340c6e031bf497308a2e65e2e.zip
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
-rw-r--r--contrib/amcheck/expected/check_btree.out23
-rw-r--r--contrib/amcheck/sql/check_btree.sql21
-rw-r--r--contrib/amcheck/verify_nbtree.c27
-rw-r--r--src/backend/access/brin/brin.c29
-rw-r--r--src/backend/catalog/index.c65
-rw-r--r--src/backend/commands/cluster.c40
-rw-r--r--src/backend/commands/indexcmds.c98
-rw-r--r--src/backend/utils/init/miscinit.c24
-rw-r--r--src/test/regress/expected/privileges.out58
-rw-r--r--src/test/regress/sql/privileges.sql47
10 files changed, 382 insertions, 50 deletions
diff --git a/contrib/amcheck/expected/check_btree.out b/contrib/amcheck/expected/check_btree.out
index 59a805d6a38..0fd6ea09f1c 100644
--- a/contrib/amcheck/expected/check_btree.out
+++ b/contrib/amcheck/expected/check_btree.out
@@ -168,11 +168,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 99acbc816f0..324818774d2 100644
--- a/contrib/amcheck/sql/check_btree.sql
+++ b/contrib/amcheck/sql/check_btree.sql
@@ -110,11 +110,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 700a02ffe41..cb6475d41f0 100644
--- a/contrib/amcheck/verify_nbtree.c
+++ b/contrib/amcheck/verify_nbtree.c
@@ -228,6 +228,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;
@@ -244,9 +247,27 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
*/
heapid = IndexGetRelation(indrelid, true);
if (OidIsValid(heapid))
+ {
heaprel = table_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
@@ -293,6 +314,12 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
heapallindexed, rootdescend);
}
+ /* 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 c7b403bff15..781cac25d82 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -873,6 +873,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())
@@ -899,7 +902,22 @@ brin_summarize_range(PG_FUNCTION_ARGS)
*/
heapoid = IndexGetRelation(indexoid, true);
if (OidIsValid(heapoid))
+ {
heapRel = table_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;
@@ -914,7 +932,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));
@@ -932,6 +950,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);
@@ -973,6 +997,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 5368023de32..0403fc086ab 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -1400,6 +1400,9 @@ index_concurrently_build(Oid heapRelationId,
Oid indexRelationId)
{
Relation heapRel;
+ Oid save_userid;
+ int save_sec_context;
+ int save_nestlevel;
Relation indexRelation;
IndexInfo *indexInfo;
@@ -1409,7 +1412,16 @@ index_concurrently_build(Oid heapRelationId,
/* Open and lock the parent heap relation */
heapRel = table_open(heapRelationId, 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(heapRel->rd_rel->relowner,
+ save_sec_context | SECURITY_RESTRICTED_OPERATION);
+ save_nestlevel = NewGUCNestLevel();
+
indexRelation = index_open(indexRelationId, RowExclusiveLock);
/*
@@ -1425,6 +1437,12 @@ index_concurrently_build(Oid heapRelationId,
/* Now build the index */
index_build(heapRel, indexRelation, indexInfo, false, true);
+ /* 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 both the relations, but keep the locks */
table_close(heapRel, NoLock);
index_close(indexRelation, NoLock);
@@ -3282,7 +3300,17 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
/* Open and lock the parent heap relation */
heapRelation = table_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);
/*
@@ -3296,16 +3324,6 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
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.
*/
ivinfo.index = indexRelation;
@@ -3508,6 +3526,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;
@@ -3522,6 +3543,16 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
heapId = IndexGetRelation(indexId, false);
heapRelation = table_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();
+
if (progress)
{
pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
@@ -3695,12 +3726,18 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
errdetail_internal("%s",
pg_rusage_show(&ru0))));
- if (progress)
- pgstat_progress_end_command();
+ /* 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);
table_close(heapRelation, NoLock);
+
+ if (progress)
+ pgstat_progress_end_command();
}
/*
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index bd6f408bcf5..f6219755535 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -266,6 +266,9 @@ void
cluster_rel(Oid tableOid, Oid indexOid, int options)
{
Relation OldHeap;
+ Oid save_userid;
+ int save_sec_context;
+ int save_nestlevel;
bool verbose = ((options & CLUOPT_VERBOSE) != 0);
bool recheck = ((options & CLUOPT_RECHECK) != 0);
@@ -296,6 +299,16 @@ cluster_rel(Oid tableOid, Oid indexOid, int options)
}
/*
+ * 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.
*
@@ -309,11 +322,10 @@ cluster_rel(Oid tableOid, Oid indexOid, int options)
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);
- pgstat_progress_end_command();
- return;
+ goto out;
}
/*
@@ -327,8 +339,7 @@ cluster_rel(Oid tableOid, Oid indexOid, int options)
if (RELATION_IS_OTHER_TEMP(OldHeap))
{
relation_close(OldHeap, AccessExclusiveLock);
- pgstat_progress_end_command();
- return;
+ goto out;
}
if (OidIsValid(indexOid))
@@ -339,8 +350,7 @@ cluster_rel(Oid tableOid, Oid indexOid, int options)
if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(indexOid)))
{
relation_close(OldHeap, AccessExclusiveLock);
- pgstat_progress_end_command();
- return;
+ goto out;
}
/*
@@ -350,16 +360,14 @@ cluster_rel(Oid tableOid, Oid indexOid, int options)
if (!HeapTupleIsValid(tuple)) /* probably can't happen */
{
relation_close(OldHeap, AccessExclusiveLock);
- pgstat_progress_end_command();
- return;
+ goto out;
}
indexForm = (Form_pg_index) GETSTRUCT(tuple);
if (!indexForm->indisclustered)
{
ReleaseSysCache(tuple);
relation_close(OldHeap, AccessExclusiveLock);
- pgstat_progress_end_command();
- return;
+ goto out;
}
ReleaseSysCache(tuple);
}
@@ -413,8 +421,7 @@ cluster_rel(Oid tableOid, Oid indexOid, int options)
!RelationIsPopulated(OldHeap))
{
relation_close(OldHeap, AccessExclusiveLock);
- pgstat_progress_end_command();
- return;
+ goto out;
}
/*
@@ -430,6 +437,13 @@ cluster_rel(Oid tableOid, Oid indexOid, int options)
/* NB: rebuild_relation does table_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);
+
pgstat_progress_end_command();
}
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index be1cf8c73d7..8ff25b34e87 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -470,21 +470,22 @@ DefineIndex(Oid relationId,
LOCKTAG heaplocktag;
LOCKMODE lockmode;
Snapshot snapshot;
- int save_nestlevel = -1;
+ Oid root_save_userid;
+ int root_save_sec_context;
+ int root_save_nestlevel;
int i;
+ root_save_nestlevel = NewGUCNestLevel();
+
/*
* Some callers need us to run with an empty default_tablespace; this is a
* necessary hack to be able to reproduce catalog state accurately when
* recreating indexes after table-rewriting ALTER TABLE.
*/
if (stmt->reset_default_tblspc)
- {
- save_nestlevel = NewGUCNestLevel();
(void) set_config_option("default_tablespace", "",
PGC_USERSET, PGC_S_SESSION,
GUC_ACTION_SAVE, true, 0, false);
- }
/*
* Force non-concurrent build on temporary relations, even if CONCURRENTLY
@@ -563,6 +564,15 @@ DefineIndex(Oid relationId,
lockmode = concurrent ? ShareUpdateExclusiveLock : ShareLock;
rel = table_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);
+
namespaceId = RelationGetNamespace(rel);
/* Ensure that it makes sense to index this kind of relation */
@@ -648,7 +658,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,
@@ -680,7 +690,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,
@@ -1066,15 +1076,17 @@ DefineIndex(Oid relationId,
ObjectAddressSet(address, RelationRelationId, indexRelationId);
- /*
- * Revert to original default_tablespace. Must do this before any return
- * from this function, but after index_create, so this is a good time.
- */
- if (save_nestlevel >= 0)
- AtEOXact_GUC(true, save_nestlevel);
-
if (!OidIsValid(indexRelationId))
{
+ /*
+ * Roll back any GUC changes executed by index functions. Also revert
+ * to original default_tablespace if we changed it above.
+ */
+ AtEOXact_GUC(false, root_save_nestlevel);
+
+ /* Restore userid and security context */
+ SetUserIdAndSecContext(root_save_userid, root_save_sec_context);
+
table_close(rel, NoLock);
/* If this is the top-level index, we're done */
@@ -1084,6 +1096,17 @@ DefineIndex(Oid relationId,
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. This improves bug-compatibility with
+ * older PostgreSQL versions. They did the AtEOXact_GUC() here for the
+ * purpose of clearing the above default_tablespace change.
+ */
+ AtEOXact_GUC(false, root_save_nestlevel);
+ root_save_nestlevel = NewGUCNestLevel();
+
/* Add any requested comment */
if (stmt->idxcomment != NULL)
CreateComments(indexRelationId, RelationRelationId, 0,
@@ -1130,6 +1153,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;
@@ -1138,6 +1164,12 @@ DefineIndex(Oid relationId,
childrel = table_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 a
@@ -1153,6 +1185,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);
table_close(childrel, lockmode);
continue;
}
@@ -1226,6 +1261,9 @@ DefineIndex(Oid relationId,
}
list_free(childidxs);
+ AtEOXact_GUC(false, child_save_nestlevel);
+ SetUserIdAndSecContext(child_save_userid,
+ child_save_sec_context);
table_close(childrel, NoLock);
/*
@@ -1280,12 +1318,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);
}
pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
@@ -1322,12 +1369,17 @@ 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);
table_close(rel, NoLock);
if (!OidIsValid(parentIndexId))
pgstat_progress_end_command();
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 */
@@ -3040,6 +3092,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid newIndexId;
Relation indexRel;
Relation heapRel;
+ Oid save_userid;
+ int save_sec_context;
+ int save_nestlevel;
Relation newIndexRel;
LockRelId *lockrelid;
@@ -3047,6 +3102,16 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
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(&save_userid, &save_sec_context);
+ SetUserIdAndSecContext(heapRel->rd_rel->relowner,
+ save_sec_context | SECURITY_RESTRICTED_OPERATION);
+ save_nestlevel = NewGUCNestLevel();
+
/* This function shouldn't be called for temporary relations. */
if (indexRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP)
elog(ERROR, "cannot reindex a temporary table concurrently");
@@ -3101,6 +3166,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
index_close(indexRel, NoLock);
index_close(newIndexRel, NoLock);
+
+ /* 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);
+
table_close(heapRel, NoLock);
}
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index c30fb986962..f6c1e7491dd 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -463,15 +463,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 186d2fbf4a3..d08e748e529 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -1336,6 +1336,64 @@ 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;
+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 to reindex
+REINDEX INDEX CONCURRENTLY sro_pidx;
+ERROR: REINDEX is not yet implemented for partitioned indexes
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 34fbf0e5cbb..bbb771d8851 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;
+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;
+
SET SESSION AUTHORIZATION regress_sro_user;
CREATE FUNCTION unwanted_grant() RETURNS void LANGUAGE sql AS
'GRANT regress_priv_group2 TO regress_sro_user';