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:13 +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 ab8c29e0b221a8f8d07a92148b542d6d69da6579..48b559c7eeeba8aad2b5c5835da26d64d8b11197 100644 (file)
@@ -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;
index fdce3c3d47504787371f15483041fdb07b52af9c..2967cafc97e8c17e570a388215bb7cedd8cbd383 100644 (file)
@@ -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;
index f4864679909b3664ee9e893d55a66a40a8973c3c..31326ae8d2d9bd383fb58fecfa3cf7cbda427756 100644 (file)
@@ -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
index b7a662f57611bf55b4c292fbab2d703665663592..a39faf01003b011558828dbbb0f8a149e274073f 100644 (file)
@@ -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))
index 75ade78647679db1be7d5221b15f6ba09c4011a6..234bf7f8548ecf703161c6b5b7919aff7f20abe7 100644 (file)
@@ -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);
index ac7277e2dea5663823df511b9ef0588fdac0ef64..1aeafa38622db9c78fb37e081eccf31528970264 100644 (file)
@@ -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);
 }
 
 /*
index db83d20e68730bac3a5a23a8cac087ef6ea9fbdc..30dc0376ebabaa6580e1371ab5c00cb8e81ee7b2 100644 (file)
@@ -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);
index 407fe64271dc5ac60e49b9f19a8b6e4883119e7a..c172295ffd3dd1b8c48dfa06509f6e633563464d 100644 (file)
@@ -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
index 478c2cdf016622ad1cf9ae82eaf76aa62d5b2ba0..966d6ecfb0392bf4482bb45e66837729e5fa86ef 100644 (file)
@@ -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';
index b19e7b3888a6a0f06974fe8c0581da6148f29e8a..d5e838a5a933316cae59a46ff750b5f68dc7dd35 100644 (file)
@@ -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';