]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Make relation-enumerating operations be security-restricted operations.
authorNoah Misch <noah@leadboat.com>
Mon, 9 May 2022 15:35:08 +0000 (08:35 -0700)
committerNoah Misch <noah@leadboat.com>
Mon, 9 May 2022 15:35:08 +0000 (08:35 -0700)
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
contrib/amcheck/sql/check_btree.sql
contrib/amcheck/verify_nbtree.c
src/backend/access/brin/brin.c
src/backend/catalog/index.c
src/backend/commands/cluster.c
src/backend/commands/indexcmds.c
src/backend/utils/init/miscinit.c
src/test/regress/expected/privileges.out
src/test/regress/sql/privileges.sql

index 5a3f1ef737cf89de4a33ca82c14626edd4c711dc..38791bbc1f4a49e49543f5db6d9ca39a4d0f932e 100644 (file)
@@ -177,11 +177,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;
index 97a3e1a20d5016c7b112cc332b2c20d0d8d54e66..033c04b4d057574995d53f33e62865582044c5d3 100644 (file)
@@ -115,11 +115,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;
index 70278c4f93242a1980c7b8d206c3ee11ed29aa4a..a8791000f871ef35f72b2546960dc611771fe4ca 100644 (file)
@@ -249,6 +249,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;
@@ -265,9 +268,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
@@ -326,6 +347,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
index 43660107688e86ab0dae7b84251765db30054f61..52f171772dc6d6f9d082df54243f764eacc6a18e 100644 (file)
@@ -1008,6 +1008,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())
@@ -1031,7 +1034,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;
 
@@ -1046,7 +1064,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));
 
@@ -1064,6 +1082,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);
 
@@ -1102,6 +1126,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))
index fd389c28d836d36700eff13a646773e8174fa0df..7539742c782887338219278eb05f22f265b43184 100644 (file)
@@ -1445,6 +1445,9 @@ index_concurrently_build(Oid heapRelationId,
                                                 Oid indexRelationId)
 {
        Relation        heapRel;
+       Oid                     save_userid;
+       int                     save_sec_context;
+       int                     save_nestlevel;
        Relation        indexRelation;
        IndexInfo  *indexInfo;
 
@@ -1454,7 +1457,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);
 
        /*
@@ -1470,6 +1482,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);
@@ -3299,7 +3317,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);
 
        /*
@@ -3312,16 +3340,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.
         */
@@ -3530,6 +3548,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;
@@ -3557,6 +3578,16 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
        if (!heapRelation)
                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(heapRelation->rd_rel->relowner,
+                                                  save_sec_context | SECURITY_RESTRICTED_OPERATION);
+       save_nestlevel = NewGUCNestLevel();
+
        if (progress)
        {
                const int       progress_cols[] = {
@@ -3775,12 +3806,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();
 }
 
 /*
index d8a6d43d959ef51e3c2ec5d6783d79d74bbd9296..cea2c8be805c145dc045a2474fc52bf2daa85cca 100644 (file)
@@ -310,6 +310,9 @@ void
 cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
 {
        Relation        OldHeap;
+       Oid                     save_userid;
+       int                     save_sec_context;
+       int                     save_nestlevel;
        bool            verbose = ((params->options & CLUOPT_VERBOSE) != 0);
        bool            recheck = ((params->options & CLUOPT_RECHECK) != 0);
 
@@ -339,6 +342,16 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
                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.
@@ -350,11 +363,10 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
        if (recheck)
        {
                /* 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;
                }
 
                /*
@@ -369,8 +381,7 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
                if (RELATION_IS_OTHER_TEMP(OldHeap))
                {
                        relation_close(OldHeap, AccessExclusiveLock);
-                       pgstat_progress_end_command();
-                       return;
+                       goto out;
                }
 
                if (OidIsValid(indexOid))
@@ -381,8 +392,7 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
                        if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(indexOid)))
                        {
                                relation_close(OldHeap, AccessExclusiveLock);
-                               pgstat_progress_end_command();
-                               return;
+                               goto out;
                        }
 
                        /*
@@ -393,8 +403,7 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
                                !get_index_isclustered(indexOid))
                        {
                                relation_close(OldHeap, AccessExclusiveLock);
-                               pgstat_progress_end_command();
-                               return;
+                               goto out;
                        }
                }
        }
@@ -447,8 +456,7 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
                !RelationIsPopulated(OldHeap))
        {
                relation_close(OldHeap, AccessExclusiveLock);
-               pgstat_progress_end_command();
-               return;
+               goto out;
        }
 
        Assert(OldHeap->rd_rel->relkind == RELKIND_RELATION ||
@@ -468,6 +476,13 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
 
        /* 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();
 }
 
index cd30f15eba694e0f8cc38f4b16671a3722334f4b..eac13ac0b73895791b83c461d0dfe04e47f661d0 100644 (file)
@@ -547,21 +547,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
@@ -640,6 +641,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 */
@@ -715,7 +725,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,
@@ -747,7 +757,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,
@@ -1138,15 +1148,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 */
@@ -1156,6 +1168,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,
@@ -1202,6 +1225,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;
                                AttrMap    *attmap;
@@ -1209,6 +1235,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
@@ -1224,6 +1256,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;
                                }
@@ -1295,6 +1330,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);
 
                                /*
@@ -1351,12 +1389,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,
@@ -1393,12 +1440,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 */
@@ -3524,6 +3576,9 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
                Oid                     newIndexId;
                Relation        indexRel;
                Relation        heapRel;
+               Oid                     save_userid;
+               int                     save_sec_context;
+               int                     save_nestlevel;
                Relation        newIndexRel;
                LockRelId  *lockrelid;
                Oid                     tablespaceid;
@@ -3532,6 +3587,16 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
                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();
+
                /* determine safety of this index for set_indexsafe_procflags */
                idx->safe = (indexRel->rd_indexprs == NIL &&
                                         indexRel->rd_indpred == NIL);
@@ -3607,6 +3672,13 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 
                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);
        }
 
index 30f0f19dd53e303045921daf1a6292d028e7b38d..6ed584e114313a74177c03d6bb096075f80f89ba 100644 (file)
@@ -567,15 +567,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
index 14bc497c21c022dba5626dd28b521e6d77d35630..4a3709e3151481a348355d60acafde635917a553 100644 (file)
@@ -1592,6 +1592,61 @@ 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;
+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';
index a26c0e17fcd9600269e4ba87a4a6011397096183..55df7f3b32e55ce3e0b9a77af941ed935ab44041 100644 (file)
@@ -1043,6 +1043,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';