]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Prevent indirect security attacks via changing session-local state within
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 9 Dec 2009 21:58:17 +0000 (21:58 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 9 Dec 2009 21:58:17 +0000 (21:58 +0000)
an allegedly immutable index function.  It was previously recognized that
we had to prevent such a function from executing SET/RESET ROLE/SESSION
AUTHORIZATION, or it could trivially obtain the privileges of the session
user.  However, since there is in general no privilege checking for changes
of session-local state, it is also possible for such a function to change
settings in a way that might subvert later operations in the same session.
Examples include changing search_path to cause an unexpected function to
be called, or replacing an existing prepared statement with another one
that will execute a function of the attacker's choosing.

The present patch secures VACUUM, ANALYZE, and CREATE INDEX/REINDEX against
these threats, which are the same places previously deemed to need protection
against the SET ROLE issue.  GUC changes are still allowed, since there are
many useful cases for that, but we prevent security problems by forcing a
rollback of any GUC change after completing the operation.  Other cases are
handled by throwing an error if any change is attempted; these include temp
table creation, closing a cursor, and creating or deleting a prepared
statement.  (In 7.4, the infrastructure to roll back GUC changes doesn't
exist, so we settle for rejecting changes of "search_path" in these contexts.)

Original report and patch by Gurjeet Singh, additional analysis by
Tom Lane.

Security: CVE-2009-4136

14 files changed:
src/backend/access/transam/xact.c
src/backend/catalog/index.c
src/backend/commands/analyze.c
src/backend/commands/schemacmds.c
src/backend/commands/tablecmds.c
src/backend/commands/vacuum.c
src/backend/executor/execMain.c
src/backend/tcop/utility.c
src/backend/utils/adt/ri_triggers.c
src/backend/utils/fmgr/fmgr.c
src/backend/utils/init/miscinit.c
src/backend/utils/misc/guc.c
src/include/miscadmin.h
src/include/utils/guc_tables.h

index c2d9101108b70c660e7f5fd3a4b1ae94aefe33f3..2c440ca1faca0d152ec494fd91fee4dcecc4c50a 100644 (file)
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.257.2.4 2009/11/23 09:59:00 heikki Exp $
+ *       $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.257.2.5 2009/12/09 21:58:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -135,7 +135,7 @@ typedef struct TransactionStateData
        int                     nChildXids;             /* # of subcommitted child XIDs */
        int                     maxChildXids;   /* allocated size of childXids[] */
        Oid                     prevUser;               /* previous CurrentUserId setting */
-       bool            prevSecDefCxt;  /* previous SecurityDefinerContext setting */
+       int                     prevSecContext; /* previous SecurityRestrictionContext */
        bool            prevXactReadOnly;               /* entry-time xact r/o state */
        struct TransactionStateData *parent;            /* back link to parent */
 } TransactionStateData;
@@ -163,7 +163,7 @@ static TransactionStateData TopTransactionStateData = {
        0,                                                      /* # of subcommitted child Xids */
        0,                                                      /* allocated size of childXids[] */
        InvalidOid,                                     /* previous CurrentUserId setting */
-       false,                                          /* previous SecurityDefinerContext setting */
+       0,                                                      /* previous SecurityRestrictionContext */
        false,                                          /* entry-time xact r/o state */
        NULL                                            /* link to parent state block */
 };
@@ -1570,9 +1570,9 @@ StartTransaction(void)
        s->childXids = NULL;
        s->nChildXids = 0;
        s->maxChildXids = 0;
-       GetUserIdAndContext(&s->prevUser, &s->prevSecDefCxt);
-       /* SecurityDefinerContext should never be set outside a transaction */
-       Assert(!s->prevSecDefCxt);
+       GetUserIdAndSecContext(&s->prevUser, &s->prevSecContext);
+       /* SecurityRestrictionContext should never be set outside a transaction */
+       Assert(s->prevSecContext == 0);
 
        /*
         * initialize other subsystems for new transaction
@@ -2063,13 +2063,13 @@ AbortTransaction(void)
         * Reset user ID which might have been changed transiently.  We need this
         * to clean up in case control escaped out of a SECURITY DEFINER function
         * or other local change of CurrentUserId; therefore, the prior value
-        * of SecurityDefinerContext also needs to be restored.
+        * of SecurityRestrictionContext also needs to be restored.
         *
         * (Note: it is not necessary to restore session authorization or role
         * settings here because those can only be changed via GUC, and GUC will
         * take care of rolling them back if need be.)
         */
-       SetUserIdAndContext(s->prevUser, s->prevSecDefCxt);
+       SetUserIdAndSecContext(s->prevUser, s->prevSecContext);
 
        /*
         * do abort processing
@@ -3909,7 +3909,7 @@ AbortSubTransaction(void)
         * Reset user ID which might have been changed transiently.  (See notes
         * in AbortTransaction.)
         */
-       SetUserIdAndContext(s->prevUser, s->prevSecDefCxt);
+       SetUserIdAndSecContext(s->prevUser, s->prevSecContext);
 
        /*
         * We can skip all this stuff if the subxact failed before creating a
@@ -4051,7 +4051,7 @@ PushTransaction(void)
        s->savepointLevel = p->savepointLevel;
        s->state = TRANS_DEFAULT;
        s->blockState = TBLOCK_SUBBEGIN;
-       GetUserIdAndContext(&s->prevUser, &s->prevSecDefCxt);
+       GetUserIdAndSecContext(&s->prevUser, &s->prevSecContext);
        s->prevXactReadOnly = XactReadOnly;
 
        CurrentTransactionState = s;
index 85c368e4408b0bff0a32cd064d25d65daded6d7e..983262f0e7d43d74dda38e9d566e29df612710e8 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.292.2.2 2008/11/13 17:42:18 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.292.2.3 2009/12/09 21:58:16 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -49,6 +49,7 @@
 #include "storage/smgr.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"
@@ -1342,7 +1343,8 @@ index_build(Relation heapRelation,
        RegProcedure procedure;
        IndexBuildResult *stats;
        Oid                     save_userid;
-       bool            save_secdefcxt;
+       int                     save_sec_context;
+       int                     save_nestlevel;
 
        /*
         * sanity checks
@@ -1354,11 +1356,14 @@ index_build(Relation heapRelation,
        Assert(RegProcedureIsValid(procedure));
 
        /*
-        * Switch to the table owner's userid, so that any index functions are
-        * run as that user.
+        * 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.
         */
-       GetUserIdAndContext(&save_userid, &save_secdefcxt);
-       SetUserIdAndContext(heapRelation->rd_rel->relowner, true);
+       GetUserIdAndSecContext(&save_userid, &save_sec_context);
+       SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
+                                                  save_sec_context | SECURITY_RESTRICTED_OPERATION);
+       save_nestlevel = NewGUCNestLevel();
 
        /*
         * Call the access method's build procedure
@@ -1370,8 +1375,11 @@ index_build(Relation heapRelation,
                                                                                 PointerGetDatum(indexInfo)));
        Assert(PointerIsValid(stats));
 
-       /* Restore userid */
-       SetUserIdAndContext(save_userid, save_secdefcxt);
+       /* 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);
 
        /*
         * If we found any potentially broken HOT chains, mark the index as not
@@ -1875,7 +1883,8 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
        IndexVacuumInfo ivinfo;
        v_i_state       state;
        Oid                     save_userid;
-       bool            save_secdefcxt;
+       int                     save_sec_context;
+       int                     save_nestlevel;
 
        /* Open and lock the parent heap relation */
        heapRelation = heap_open(heapId, ShareUpdateExclusiveLock);
@@ -1893,11 +1902,14 @@ 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.
+        * 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.
         */
-       GetUserIdAndContext(&save_userid, &save_secdefcxt);
-       SetUserIdAndContext(heapRelation->rd_rel->relowner, true);
+       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.
@@ -1936,8 +1948,11 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
                 "validate_index found %.0f heap tuples, %.0f index tuples; inserted %.0f missing tuples",
                 state.htups, state.itups, state.tups_inserted);
 
-       /* Restore userid */
-       SetUserIdAndContext(save_userid, save_secdefcxt);
+       /* 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(indexRelation, NoLock);
index f442bf653609b9a56f1b394b1d5ecd37568f05d5..38f2e1f0b3708825688c0c801e6ca20beeeed76a 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.114.2.3 2009/08/12 18:24:03 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.114.2.4 2009/12/09 21:58:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -36,6 +36,7 @@
 #include "storage/procarray.h"
 #include "utils/acl.h"
 #include "utils/datum.h"
+#include "utils/guc.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
 #include "utils/pg_rusage.h"
@@ -122,7 +123,8 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt,
        PGRUsage        ru0;
        TimestampTz starttime = 0;
        Oid                     save_userid;
-       bool            save_secdefcxt;
+       int                     save_sec_context;
+       int                     save_nestlevel;
 
        if (vacstmt->verbose)
                elevel = INFO;
@@ -212,11 +214,14 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt,
                                        RelationGetRelationName(onerel))));
 
        /*
-        * Switch to the table owner's userid, so that any index functions are
-        * run as that user.
+        * 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.
         */
-       GetUserIdAndContext(&save_userid, &save_secdefcxt);
-       SetUserIdAndContext(onerel->rd_rel->relowner, true);
+       GetUserIdAndSecContext(&save_userid, &save_sec_context);
+       SetUserIdAndSecContext(onerel->rd_rel->relowner,
+                                                  save_sec_context | SECURITY_RESTRICTED_OPERATION);
+       save_nestlevel = NewGUCNestLevel();
 
        /* let others know what I'm doing */
        LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
@@ -497,8 +502,11 @@ cleanup:
        MyProc->vacuumFlags &= ~PROC_IN_ANALYZE;
        LWLockRelease(ProcArrayLock);
 
-       /* Restore userid */
-       SetUserIdAndContext(save_userid, save_secdefcxt);
+       /* 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 d77294ccc225913ea9e58673ba65dbbe41f4a037..a4c34493cddfdd27a612e5f351b4ff9ef83235d2 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/schemacmds.c,v 1.49 2008/01/03 21:23:15 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/schemacmds.c,v 1.49.2.1 2009/12/09 21:58:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -48,10 +48,10 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString)
        ListCell   *parsetree_item;
        Oid                     owner_uid;
        Oid                     saved_uid;
-       bool            saved_secdefcxt;
+       int                     save_sec_context;
        AclResult       aclresult;
 
-       GetUserIdAndContext(&saved_uid, &saved_secdefcxt);
+       GetUserIdAndSecContext(&saved_uid, &save_sec_context);
 
        /*
         * Who is supposed to own the new schema?
@@ -91,7 +91,8 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString)
         * of error, transaction abort will clean things up.)
         */
        if (saved_uid != owner_uid)
-               SetUserIdAndContext(owner_uid, true);
+               SetUserIdAndSecContext(owner_uid,
+                                                          save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
 
        /* Create the schema's namespace */
        namespaceId = NamespaceCreate(schemaName, owner_uid);
@@ -142,8 +143,8 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString)
        /* Reset search path to normal state */
        PopOverrideSearchPath();
 
-       /* Reset current user */
-       SetUserIdAndContext(saved_uid, saved_secdefcxt);
+       /* Reset current user and security context */
+       SetUserIdAndSecContext(saved_uid, save_sec_context);
 }
 
 
index 1bad88326a56b2d1f8f1cd0bf04b51354b269d5b..5897619f777ab480b8d391ffcfa444448160d07c 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.242.2.4 2008/10/07 11:15:48 heikki Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.242.2.5 2009/12/09 21:58:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -302,6 +302,16 @@ DefineRelation(CreateStmt *stmt, char relkind)
                                (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
                                 errmsg("ON COMMIT can only be used on temporary tables")));
 
+       /*
+        * Security check: disallow creating temp tables from security-restricted
+        * code.  This is needed because calling code might not expect untrusted
+        * tables to appear in pg_temp at the front of its search path.
+        */
+       if (stmt->relation->istemp && InSecurityRestrictedOperation())
+               ereport(ERROR,
+                               (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                                errmsg("cannot create temporary table within security-restricted operation")));
+
        /*
         * Look up the namespace in which we are supposed to create the relation.
         * Check we have permission to create there. Skip check if bootstrapping,
index b83299172dddc6e90335899437dd9b5616219a25..b2dfa870875b939fc0196fc3153588d117c2d09e 100644 (file)
@@ -13,7 +13,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.364.2.3 2009/11/10 18:00:44 alvherre Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.364.2.4 2009/12/09 21:58:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -42,6 +42,7 @@
 #include "utils/builtins.h"
 #include "utils/flatfiles.h"
 #include "utils/fmgroids.h"
+#include "utils/guc.h"
 #include "utils/inval.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
@@ -975,7 +976,8 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind,
        LockRelId       onerelid;
        Oid                     toast_relid;
        Oid                     save_userid;
-       bool            save_secdefcxt;
+       int                     save_sec_context;
+       int                     save_nestlevel;
        bool            heldoff;
 
        /* Begin a transaction for vacuuming this relation */
@@ -1111,12 +1113,15 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind,
        toast_relid = onerel->rd_rel->reltoastrelid;
 
        /*
-        * Switch to the table owner's userid, so that any index functions are
-        * run as that user.  (This is unnecessary, but harmless, for lazy
-        * VACUUM.)
+        * 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 unnecessary, but harmless, for lazy VACUUM.)
         */
-       GetUserIdAndContext(&save_userid, &save_secdefcxt);
-       SetUserIdAndContext(onerel->rd_rel->relowner, true);
+       GetUserIdAndSecContext(&save_userid, &save_sec_context);
+       SetUserIdAndSecContext(onerel->rd_rel->relowner,
+                                                  save_sec_context | SECURITY_RESTRICTED_OPERATION);
+       save_nestlevel = NewGUCNestLevel();
 
        /*
         * Do the actual work --- either FULL or "lazy" vacuum
@@ -1126,8 +1131,11 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind,
        else
                heldoff = lazy_vacuum_rel(onerel, vacstmt, vac_strategy);
 
-       /* Restore userid */
-       SetUserIdAndContext(save_userid, save_secdefcxt);
+       /* 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);
 
        /* all done with this class, but hold lock until commit */
        relation_close(onerel, NoLock);
index d6cf42c0f39812c876161d27d547a9beacf3d2c7..10a4b2ae87608fde73925522d5554a0a9af2e073 100644 (file)
@@ -26,7 +26,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.303.2.2 2008/08/08 17:01:18 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.303.2.3 2009/12/09 21:58:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2658,6 +2658,11 @@ OpenIntoRel(QueryDesc *queryDesc)
 
        Assert(into);
 
+       /*
+        * XXX This code needs to be kept in sync with DefineRelation().
+        * Maybe we should try to use that function instead.
+        */
+
        /*
         * Check consistency of arguments
         */
@@ -2666,6 +2671,16 @@ OpenIntoRel(QueryDesc *queryDesc)
                                (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
                                 errmsg("ON COMMIT can only be used on temporary tables")));
 
+       /*
+        * Security check: disallow creating temp tables from security-restricted
+        * code.  This is needed because calling code might not expect untrusted
+        * tables to appear in pg_temp at the front of its search path.
+        */
+       if (into->rel->istemp && InSecurityRestrictedOperation())
+               ereport(ERROR,
+                               (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                                errmsg("cannot create temporary table within security-restricted operation")));
+
        /*
         * Find namespace to create in, check its permissions
         */
index c88b8ca662ccf9df648e3469679d4c47f331585b..733ccb7f5cbd1f62fc81458d492287aae3da11eb 100644 (file)
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.289.2.2 2008/10/10 13:48:12 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.289.2.3 2009/12/09 21:58:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -368,6 +368,25 @@ check_xact_readonly(Node *parsetree)
 }
 
 
+/*
+ * CheckRestrictedOperation: throw error for hazardous command if we're
+ * inside a security restriction context.
+ *
+ * This is needed to protect session-local state for which there is not any
+ * better-defined protection mechanism, such as ownership.
+ */
+static void
+CheckRestrictedOperation(const char *cmdname)
+{
+       if (InSecurityRestrictedOperation())
+               ereport(ERROR,
+                               (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                                /* translator: %s is name of a SQL command, eg PREPARE */
+                                errmsg("cannot execute %s within security-restricted operation",
+                                               cmdname)));
+}
+
+
 /*
  * ProcessUtility
  *             general utility function invoker
@@ -527,6 +546,7 @@ ProcessUtility(Node *parsetree,
                        {
                                ClosePortalStmt *stmt = (ClosePortalStmt *) parsetree;
 
+                               CheckRestrictedOperation("CLOSE");
                                PerformPortalClose(stmt->portalname);
                        }
                        break;
@@ -717,6 +737,7 @@ ProcessUtility(Node *parsetree,
                        break;
 
                case T_PrepareStmt:
+                       CheckRestrictedOperation("PREPARE");
                        PrepareQuery((PrepareStmt *) parsetree, queryString);
                        break;
 
@@ -726,6 +747,7 @@ ProcessUtility(Node *parsetree,
                        break;
 
                case T_DeallocateStmt:
+                       CheckRestrictedOperation("DEALLOCATE");
                        DeallocateQuery((DeallocateStmt *) parsetree);
                        break;
 
@@ -1005,6 +1027,7 @@ ProcessUtility(Node *parsetree,
                        {
                                ListenStmt *stmt = (ListenStmt *) parsetree;
 
+                               CheckRestrictedOperation("LISTEN");
                                Async_Listen(stmt->relation->relname);
                        }
                        break;
@@ -1013,6 +1036,7 @@ ProcessUtility(Node *parsetree,
                        {
                                UnlistenStmt *stmt = (UnlistenStmt *) parsetree;
 
+                               CheckRestrictedOperation("UNLISTEN");
                                Async_Unlisten(stmt->relation->relname);
                        }
                        break;
@@ -1052,6 +1076,8 @@ ProcessUtility(Node *parsetree,
                        break;
 
                case T_DiscardStmt:
+                       /* should we allow DISCARD PLANS? */
+                       CheckRestrictedOperation("DISCARD");
                        DiscardCommand((DiscardStmt *) parsetree, isTopLevel);
                        break;
 
index 874d6464a3106e22c0cdc06d6b379c652e30cc3a..9792bfa2b4fea282ffb39eeec93b8bb880afda15 100644 (file)
@@ -15,7 +15,7 @@
  *
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.103.2.3 2008/09/15 23:37:49 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.103.2.4 2009/12/09 21:58:16 tgl Exp $
  *
  * ----------
  */
@@ -3201,7 +3201,7 @@ ri_PlanCheck(const char *querystr, int nargs, Oid *argtypes,
        SPIPlanPtr      qplan;
        Relation        query_rel;
        Oid                     save_userid;
-       bool            save_secdefcxt;
+       int                     save_sec_context;
 
        /*
         * The query is always run against the FK table except when this is an
@@ -3215,8 +3215,9 @@ ri_PlanCheck(const char *querystr, int nargs, Oid *argtypes,
                query_rel = fk_rel;
 
        /* Switch to proper UID to perform check as */
-       GetUserIdAndContext(&save_userid, &save_secdefcxt);
-       SetUserIdAndContext(RelationGetForm(query_rel)->relowner, true);
+       GetUserIdAndSecContext(&save_userid, &save_sec_context);
+       SetUserIdAndSecContext(RelationGetForm(query_rel)->relowner,
+                                                  save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
 
        /* Create the plan */
        qplan = SPI_prepare(querystr, nargs, argtypes);
@@ -3224,8 +3225,8 @@ ri_PlanCheck(const char *querystr, int nargs, Oid *argtypes,
        if (qplan == NULL)
                elog(ERROR, "SPI_prepare returned %d for %s", SPI_result, querystr);
 
-       /* Restore UID */
-       SetUserIdAndContext(save_userid, save_secdefcxt);
+       /* Restore UID and security context */
+       SetUserIdAndSecContext(save_userid, save_sec_context);
 
        /* Save the plan if requested */
        if (cache_plan)
@@ -3255,7 +3256,7 @@ ri_PerformCheck(RI_QueryKey *qkey, SPIPlanPtr qplan,
        int                     limit;
        int                     spi_result;
        Oid                     save_userid;
-       bool            save_secdefcxt;
+       int                     save_sec_context;
        Datum           vals[RI_MAX_NUMKEYS * 2];
        char            nulls[RI_MAX_NUMKEYS * 2];
 
@@ -3333,8 +3334,9 @@ ri_PerformCheck(RI_QueryKey *qkey, SPIPlanPtr qplan,
        limit = (expect_OK == SPI_OK_SELECT) ? 1 : 0;
 
        /* Switch to proper UID to perform check as */
-       GetUserIdAndContext(&save_userid, &save_secdefcxt);
-       SetUserIdAndContext(RelationGetForm(query_rel)->relowner, true);
+       GetUserIdAndSecContext(&save_userid, &save_sec_context);
+       SetUserIdAndSecContext(RelationGetForm(query_rel)->relowner,
+                                                  save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
 
        /* Finally we can run the query. */
        spi_result = SPI_execute_snapshot(qplan,
@@ -3342,8 +3344,8 @@ ri_PerformCheck(RI_QueryKey *qkey, SPIPlanPtr qplan,
                                                                          test_snapshot, crosscheck_snapshot,
                                                                          false, false, limit);
 
-       /* Restore UID */
-       SetUserIdAndContext(save_userid, save_secdefcxt);
+       /* Restore UID and security context */
+       SetUserIdAndSecContext(save_userid, save_sec_context);
 
        /* Check result */
        if (spi_result < 0)
index 62fdfba294ef512a2c998f5d9559577dfbbc9290..9876767f425b7b863df11863a8d25b51474521d8 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/fmgr/fmgr.c,v 1.113.2.1 2009/01/07 20:39:05 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/fmgr/fmgr.c,v 1.113.2.2 2009/12/09 21:58:17 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -865,7 +865,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
        struct fmgr_security_definer_cache *volatile fcache;
        FmgrInfo   *save_flinfo;
        Oid                     save_userid;
-       bool            save_secdefcxt;
+       int                     save_sec_context;
        volatile int save_nestlevel;
 
        if (!fcinfo->flinfo->fn_extra)
@@ -910,15 +910,16 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
        else
                fcache = fcinfo->flinfo->fn_extra;
 
-       /* GetUserIdAndContext is cheap enough that no harm in a wasted call */
-       GetUserIdAndContext(&save_userid, &save_secdefcxt);
+       /* GetUserIdAndSecContext is cheap enough that no harm in a wasted call */
+       GetUserIdAndSecContext(&save_userid, &save_sec_context);
        if (fcache->proconfig)          /* Need a new GUC nesting level */
                save_nestlevel = NewGUCNestLevel();
        else
                save_nestlevel = 0;             /* keep compiler quiet */
 
        if (OidIsValid(fcache->userid))
-               SetUserIdAndContext(fcache->userid, true);
+               SetUserIdAndSecContext(fcache->userid,
+                                                          save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
 
        if (fcache->proconfig)
        {
@@ -953,7 +954,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
        if (fcache->proconfig)
                AtEOXact_GUC(true, save_nestlevel);
        if (OidIsValid(fcache->userid))
-               SetUserIdAndContext(save_userid, save_secdefcxt);
+               SetUserIdAndSecContext(save_userid, save_sec_context);
 
        return result;
 }
index 5a42a8953a7a3e64fbe6790cbf2721e45f952213..441b808d0cd97bcb50f8d40af1f08015c62704af 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/init/miscinit.c,v 1.166 2008/01/03 21:23:15 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/init/miscinit.c,v 1.166.2.1 2009/12/09 21:58:17 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -271,8 +271,10 @@ make_absolute_path(const char *path)
  * be the same as OuterUserId, but it changes during calls to SECURITY
  * DEFINER functions, as well as locally in some specialized commands.
  *
- * SecurityDefinerContext is TRUE if we are within a SECURITY DEFINER function
- * or another context that temporarily changes CurrentUserId.
+ * SecurityRestrictionContext holds flags indicating reason(s) for changing
+ * CurrentUserId.  In some cases we need to lock down operations that are
+ * not directly controlled by privilege settings, and this provides a
+ * convenient way to do it.
  * ----------------------------------------------------------------
  */
 static Oid     AuthenticatedUserId = InvalidOid;
@@ -284,7 +286,7 @@ static Oid  CurrentUserId = InvalidOid;
 static bool AuthenticatedUserIsSuperuser = false;
 static bool SessionUserIsSuperuser = false;
 
-static bool SecurityDefinerContext = false;
+static int     SecurityRestrictionContext = 0;
 
 /* We also remember if a SET ROLE is currently active */
 static bool SetRoleIsActive = false;
@@ -293,7 +295,7 @@ static bool SetRoleIsActive = false;
 /*
  * GetUserId - get the current effective user ID.
  *
- * Note: there's no SetUserId() anymore; use SetUserIdAndContext().
+ * Note: there's no SetUserId() anymore; use SetUserIdAndSecContext().
  */
 Oid
 GetUserId(void)
@@ -317,7 +319,7 @@ GetOuterUserId(void)
 static void
 SetOuterUserId(Oid userid)
 {
-       AssertState(!SecurityDefinerContext);
+       AssertState(SecurityRestrictionContext == 0);
        AssertArg(OidIsValid(userid));
        OuterUserId = userid;
 
@@ -340,7 +342,7 @@ GetSessionUserId(void)
 static void
 SetSessionUserId(Oid userid, bool is_superuser)
 {
-       AssertState(!SecurityDefinerContext);
+       AssertState(SecurityRestrictionContext == 0);
        AssertArg(OidIsValid(userid));
        SessionUserId = userid;
        SessionUserIsSuperuser = is_superuser;
@@ -353,11 +355,29 @@ SetSessionUserId(Oid userid, bool is_superuser)
 
 
 /*
- * GetUserIdAndContext/SetUserIdAndContext - get/set the current user ID
- * and the SecurityDefinerContext flag.
+ * GetUserIdAndSecContext/SetUserIdAndSecContext - get/set the current user ID
+ * and the SecurityRestrictionContext flags.
  *
- * Unlike GetUserId, GetUserIdAndContext does *not* Assert that the current
- * value of CurrentUserId is valid; nor does SetUserIdAndContext require
+ * Currently there are two valid bits in SecurityRestrictionContext:
+ *
+ * SECURITY_LOCAL_USERID_CHANGE indicates that we are inside an operation
+ * that is temporarily changing CurrentUserId via these functions.  This is
+ * needed to indicate that the actual value of CurrentUserId is not in sync
+ * 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.
+ *
+ * Unlike GetUserId, GetUserIdAndSecContext does *not* Assert that the current
+ * value of CurrentUserId is valid; nor does SetUserIdAndSecContext require
  * the new value to be valid.  In fact, these routines had better not
  * ever throw any kind of error.  This is because they are used by
  * StartTransaction and AbortTransaction to save/restore the settings,
@@ -366,27 +386,66 @@ SetSessionUserId(Oid userid, bool is_superuser)
  * through AbortTransaction without asserting in case InitPostgres fails.
  */
 void
-GetUserIdAndContext(Oid *userid, bool *sec_def_context)
+GetUserIdAndSecContext(Oid *userid, int *sec_context)
 {
        *userid = CurrentUserId;
-       *sec_def_context = SecurityDefinerContext;
+       *sec_context = SecurityRestrictionContext;
 }
 
 void
-SetUserIdAndContext(Oid userid, bool sec_def_context)
+SetUserIdAndSecContext(Oid userid, int sec_context)
 {
        CurrentUserId = userid;
-       SecurityDefinerContext = sec_def_context;
+       SecurityRestrictionContext = sec_context;
 }
 
 
 /*
- * InSecurityDefinerContext - are we inside a SECURITY DEFINER context?
+ * InLocalUserIdChange - are we inside a local change of CurrentUserId?
  */
 bool
-InSecurityDefinerContext(void)
+InLocalUserIdChange(void)
 {
-       return SecurityDefinerContext;
+       return (SecurityRestrictionContext & SECURITY_LOCAL_USERID_CHANGE) != 0;
+}
+
+/*
+ * InSecurityRestrictedOperation - are we inside a security-restricted command?
+ */
+bool
+InSecurityRestrictedOperation(void)
+{
+       return (SecurityRestrictionContext & SECURITY_RESTRICTED_OPERATION) != 0;
+}
+
+
+/*
+ * These are obsolete versions of Get/SetUserIdAndSecContext that are
+ * only provided for bug-compatibility with some rather dubious code in
+ * pljava.  We allow the userid to be set, but only when not inside a
+ * security restriction context.
+ */
+void
+GetUserIdAndContext(Oid *userid, bool *sec_def_context)
+{
+       *userid = CurrentUserId;
+       *sec_def_context = InLocalUserIdChange();
+}
+
+void
+SetUserIdAndContext(Oid userid, bool sec_def_context)
+{
+       /* We throw the same error SET ROLE would. */
+       if (InSecurityRestrictedOperation())
+               ereport(ERROR,
+                               (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                                errmsg("cannot set parameter \"%s\" within security-restricted operation",
+                                               "role")));
+       CurrentUserId = userid;
+       if (sec_def_context)
+               SecurityRestrictionContext |= SECURITY_LOCAL_USERID_CHANGE;
+       else
+               SecurityRestrictionContext &= ~SECURITY_LOCAL_USERID_CHANGE;
 }
 
 
index c65e5af55b150cdc6bafc13f328a6133fc58c2b8..df52fb8b098c0f1379203196b0bbb239f916292c 100644 (file)
@@ -10,7 +10,7 @@
  * Written by Peter Eisentraut <peter_e@gmx.net>.
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.432.2.4 2009/09/03 22:08:22 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.432.2.5 2009/12/09 21:58:17 tgl Exp $
  *
  *--------------------------------------------------------------------
  */
@@ -2224,7 +2224,7 @@ static struct config_string ConfigureNamesString[] =
                {"role", PGC_USERSET, UNGROUPED,
                        gettext_noop("Sets the current role."),
                        NULL,
-                       GUC_IS_NAME | GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_NOT_WHILE_SEC_DEF
+                       GUC_IS_NAME | GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_NOT_WHILE_SEC_REST
                },
                &role_string,
                "none", assign_role, show_role
@@ -2235,7 +2235,7 @@ static struct config_string ConfigureNamesString[] =
                {"session_authorization", PGC_USERSET, UNGROUPED,
                        gettext_noop("Sets the session user name."),
                        NULL,
-                       GUC_IS_NAME | GUC_REPORT | GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_NOT_WHILE_SEC_DEF
+                       GUC_IS_NAME | GUC_REPORT | GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_NOT_WHILE_SEC_REST
                },
                &session_authorization_string,
                NULL, assign_session_authorization, show_session_authorization
@@ -4319,29 +4319,45 @@ set_config_option(const char *name, const char *value,
        }
 
        /*
-        * Disallow changing GUC_NOT_WHILE_SEC_DEF values if we are inside a
-        * security-definer function.  We can reject this regardless of
-        * the context or source, mainly because sources that it might be
+        * Disallow changing GUC_NOT_WHILE_SEC_REST values if we are inside a
+        * security restriction context.  We can reject this regardless of
+        * the GUC context or source, mainly because sources that it might be
         * reasonable to override for won't be seen while inside a function.
         *
-        * Note: variables marked GUC_NOT_WHILE_SEC_DEF should probably be marked
+        * Note: variables marked GUC_NOT_WHILE_SEC_REST should usually be marked
         * GUC_NO_RESET_ALL as well, because ResetAllOptions() doesn't check this.
+        * An exception might be made if the reset value is assumed to be "safe".
         *
         * Note: this flag is currently used for "session_authorization" and
-        * "role".  We need to prohibit this because when we exit the sec-def
-        * context, GUC won't be notified, leaving things out of sync.
-        *
-        * XXX it would be nice to allow these cases in future, with the behavior
-        * being that the SET's effects end when the security definer context is
-        * exited.
+        * "role".  We need to prohibit changing these inside a local userid
+        * context because when we exit it, GUC won't be notified, leaving things
+        * out of sync.  (This could be fixed by forcing a new GUC nesting level,
+        * but that would change behavior in possibly-undesirable ways.)  Also,
+        * we prohibit changing these in a security-restricted operation because
+        * otherwise RESET could be used to regain the session user's privileges.
         */
-       if ((record->flags & GUC_NOT_WHILE_SEC_DEF) && InSecurityDefinerContext())
+       if (record->flags & GUC_NOT_WHILE_SEC_REST)
        {
-               ereport(elevel,
-                               (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-                                errmsg("cannot set parameter \"%s\" within security-definer function",
-                                               name)));
-               return false;
+               if (InLocalUserIdChange())
+               {
+                       /*
+                        * Phrasing of this error message is historical, but it's the
+                        * most common case.
+                        */
+                       ereport(elevel,
+                                       (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                                        errmsg("cannot set parameter \"%s\" within security-definer function",
+                                                       name)));
+                       return false;
+               }
+               if (InSecurityRestrictedOperation())
+               {
+                       ereport(elevel,
+                                       (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                                        errmsg("cannot set parameter \"%s\" within security-restricted operation",
+                                                       name)));
+                       return false;
+               }
        }
 
        /*
index 6ded4c9bc4c539b5a963bc83c74eaa75eb00a4c0..a859da28f78f32a6333d9fd71da7c3711960d1a1 100644 (file)
@@ -13,7 +13,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/miscadmin.h,v 1.199 2008/01/03 21:23:15 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/miscadmin.h,v 1.199.2.1 2009/12/09 21:58:17 tgl Exp $
  *
  * NOTES
  *       some of the information in this file should be moved to other files.
@@ -227,6 +227,10 @@ extern void check_stack_depth(void);
  *                     POSTGRES directory path definitions.                                                     *
  *****************************************************************************/
 
+/* flags to be OR'd to form sec_context */
+#define SECURITY_LOCAL_USERID_CHANGE   0x0001
+#define SECURITY_RESTRICTED_OPERATION  0x0002
+
 extern char *DatabasePath;
 
 /* now in utils/init/miscinit.c */
@@ -236,9 +240,12 @@ extern char *GetUserNameFromId(Oid roleid);
 extern Oid     GetUserId(void);
 extern Oid     GetOuterUserId(void);
 extern Oid     GetSessionUserId(void);
+extern void GetUserIdAndSecContext(Oid *userid, int *sec_context);
+extern void SetUserIdAndSecContext(Oid userid, int sec_context);
+extern bool InLocalUserIdChange(void);
+extern bool InSecurityRestrictedOperation(void);
 extern void GetUserIdAndContext(Oid *userid, bool *sec_def_context);
 extern void SetUserIdAndContext(Oid userid, bool sec_def_context);
-extern bool InSecurityDefinerContext(void);
 extern void InitializeSessionUserId(const char *rolename);
 extern void InitializeSessionUserIdStandalone(void);
 extern void SetSessionAuthorization(Oid userid, bool is_superuser);
index 0578ab05a8612c18c0dfd340941b85eaf621d5f6..6bdc6721bf5ad7a888ef3ae2bc1eda9135d2404d 100644 (file)
@@ -7,7 +7,7 @@
  *
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  *
- *       $PostgreSQL: pgsql/src/include/utils/guc_tables.h,v 1.38.2.1 2009/09/03 22:08:23 tgl Exp $
+ *       $PostgreSQL: pgsql/src/include/utils/guc_tables.h,v 1.38.2.2 2009/12/09 21:58:17 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -148,7 +148,7 @@ struct config_generic
 #define GUC_UNIT_MIN                   0x4000  /* value is in minutes */
 #define GUC_UNIT_TIME                  0x7000  /* mask for MS, S, MIN */
 
-#define GUC_NOT_WHILE_SEC_DEF  0x8000  /* can't change inside sec-def func */
+#define GUC_NOT_WHILE_SEC_REST 0x8000  /* can't set if security restricted */
 
 /* bit values in status field */
 #define GUC_IS_IN_FILE         0x0001          /* found it in config file */