diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index cec67016b5..f9a71760d3 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -10,7 +10,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.276 2009/11/23 09:58:36 heikki Exp $ + * $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.277 2009/12/09 21:57:50 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -137,7 +137,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; @@ -165,7 +165,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 */ }; @@ -1522,9 +1522,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 @@ -2014,13 +2014,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. + * 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 @@ -3860,7 +3860,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 @@ -4000,7 +4000,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; diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 6564c05c9e..5394d2557e 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.325 2009/12/07 05:22:21 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.326 2009/12/09 21:57:50 tgl Exp $ * * * INTERFACE ROUTINES @@ -58,6 +58,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" @@ -1481,7 +1482,8 @@ index_build(Relation heapRelation, RegProcedure procedure; IndexBuildResult *stats; Oid save_userid; - bool save_secdefcxt; + int save_sec_context; + int save_nestlevel; /* * sanity checks @@ -1494,10 +1496,13 @@ index_build(Relation heapRelation, /* * Switch to the table owner's userid, so that any index functions are run - * as that user. + * 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 @@ -1516,8 +1521,11 @@ index_build(Relation heapRelation, if (indexInfo->ii_ExclusionOps != NULL) IndexCheckExclusion(heapRelation, indexRelation, indexInfo); - /* 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 @@ -2126,7 +2134,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); @@ -2145,10 +2154,13 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot) /* * Switch to the table owner's userid, so that any index functions are run - * as that user. + * 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. @@ -2189,8 +2201,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); diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index ab1fe9bf0e..6d13eeb5ec 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.142 2009/11/16 21:32:06 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.143 2009/12/09 21:57:50 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -38,6 +38,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" @@ -133,7 +134,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->options & VACOPT_VERBOSE) elevel = INFO; @@ -235,10 +237,13 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt, /* * Switch to the table owner's userid, so that any index functions are run - * as that user. + * 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); @@ -548,8 +553,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); } /* diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c index 5680da3657..6bf09108d4 100644 --- a/src/backend/commands/schemacmds.c +++ b/src/backend/commands/schemacmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/schemacmds.c,v 1.53 2009/06/11 14:48:56 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/commands/schemacmds.c,v 1.54 2009/12/09 21:57:50 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) * 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); } diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index d10a7490b7..c9188b2d7c 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.307 2009/12/07 05:22:21 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.308 2009/12/09 21:57:50 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -378,6 +378,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, diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 83b8c18e8e..15648d5a39 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -13,7 +13,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.397 2009/12/07 05:22:21 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.398 2009/12/09 21:57:51 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -47,6 +47,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" @@ -1033,7 +1034,8 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound, LockRelId onerelid; Oid toast_relid; Oid save_userid; - bool save_secdefcxt; + int save_sec_context; + int save_nestlevel; bool heldoff; if (scanned_all) @@ -1192,10 +1194,14 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound, /* * 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.) + * 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 @@ -1205,8 +1211,11 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound, else heldoff = lazy_vacuum_rel(onerel, vacstmt, vac_strategy, scanned_all); - /* 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); diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 60ded334c6..9d7bdb777c 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -26,7 +26,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.335 2009/11/20 20:38:10 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.336 2009/12/09 21:57:51 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -2067,6 +2067,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 */ @@ -2075,6 +2080,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 */ diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 1a7640b350..59576a25d8 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -10,7 +10,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.321 2009/12/07 05:22:22 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.322 2009/12/09 21:57:51 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -225,6 +225,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 @@ -390,6 +409,7 @@ ProcessUtility(Node *parsetree, { ClosePortalStmt *stmt = (ClosePortalStmt *) parsetree; + CheckRestrictedOperation("CLOSE"); PerformPortalClose(stmt->portalname); } break; @@ -586,6 +606,7 @@ ProcessUtility(Node *parsetree, break; case T_PrepareStmt: + CheckRestrictedOperation("PREPARE"); PrepareQuery((PrepareStmt *) parsetree, queryString); break; @@ -595,6 +616,7 @@ ProcessUtility(Node *parsetree, break; case T_DeallocateStmt: + CheckRestrictedOperation("DEALLOCATE"); DeallocateQuery((DeallocateStmt *) parsetree); break; @@ -885,6 +907,7 @@ ProcessUtility(Node *parsetree, { ListenStmt *stmt = (ListenStmt *) parsetree; + CheckRestrictedOperation("LISTEN"); Async_Listen(stmt->conditionname); } break; @@ -893,6 +916,7 @@ ProcessUtility(Node *parsetree, { UnlistenStmt *stmt = (UnlistenStmt *) parsetree; + CheckRestrictedOperation("UNLISTEN"); if (stmt->conditionname) Async_Unlisten(stmt->conditionname); else @@ -936,6 +960,8 @@ ProcessUtility(Node *parsetree, break; case T_DiscardStmt: + /* should we allow DISCARD PLANS? */ + CheckRestrictedOperation("DISCARD"); DiscardCommand((DiscardStmt *) parsetree, isTopLevel); break; diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index d108d077db..1f6fd54f43 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -15,7 +15,7 @@ * * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group * - * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.115 2009/11/05 04:38:29 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.116 2009/12/09 21:57:51 tgl Exp $ * * ---------- */ @@ -3209,7 +3209,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 @@ -3223,8 +3223,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); @@ -3232,8 +3233,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) @@ -3263,7 +3264,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]; @@ -3343,8 +3344,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, @@ -3352,8 +3354,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) diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c index 1256e7c88d..43872f3c3d 100644 --- a/src/backend/utils/fmgr/fmgr.c +++ b/src/backend/utils/fmgr/fmgr.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/fmgr/fmgr.c,v 1.126 2009/06/11 14:49:05 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/utils/fmgr/fmgr.c,v 1.127 2009/12/09 21:57:51 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -880,7 +880,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; PgStat_FunctionCallUsage fcusage; @@ -926,15 +926,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) { @@ -981,7 +982,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; } diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c index f7a27b9e43..e47991ddb8 100644 --- a/src/backend/utils/init/miscinit.c +++ b/src/backend/utils/init/miscinit.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/init/miscinit.c,v 1.178 2009/10/07 22:14:22 alvherre Exp $ + * $PostgreSQL: pgsql/src/backend/utils/init/miscinit.c,v 1.179 2009/12/09 21:57:51 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -265,8 +265,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; @@ -278,7 +280,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; @@ -287,7 +289,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) @@ -311,7 +313,7 @@ GetOuterUserId(void) static void SetOuterUserId(Oid userid) { - AssertState(!SecurityDefinerContext); + AssertState(SecurityRestrictionContext == 0); AssertArg(OidIsValid(userid)); OuterUserId = userid; @@ -334,7 +336,7 @@ GetSessionUserId(void) static void SetSessionUserId(Oid userid, bool is_superuser) { - AssertState(!SecurityDefinerContext); + AssertState(SecurityRestrictionContext == 0); AssertArg(OidIsValid(userid)); SessionUserId = userid; SessionUserIsSuperuser = is_superuser; @@ -347,11 +349,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, @@ -360,27 +380,66 @@ SetSessionUserId(Oid userid, bool is_superuser) * through AbortTransaction without asserting in case InitPostgres fails. */ void +GetUserIdAndSecContext(Oid *userid, int *sec_context) +{ + *userid = CurrentUserId; + *sec_context = SecurityRestrictionContext; +} + +void +SetUserIdAndSecContext(Oid userid, int sec_context) +{ + CurrentUserId = userid; + SecurityRestrictionContext = sec_context; +} + + +/* + * InLocalUserIdChange - are we inside a local change of CurrentUserId? + */ +bool +InLocalUserIdChange(void) +{ + 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 = SecurityDefinerContext; + *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; - SecurityDefinerContext = sec_def_context; -} - - -/* - * InSecurityDefinerContext - are we inside a SECURITY DEFINER context? - */ -bool -InSecurityDefinerContext(void) -{ - return SecurityDefinerContext; + if (sec_def_context) + SecurityRestrictionContext |= SECURITY_LOCAL_USERID_CHANGE; + else + SecurityRestrictionContext &= ~SECURITY_LOCAL_USERID_CHANGE; } diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index f5db7783f1..edc5544f7f 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -10,7 +10,7 @@ * Written by Peter Eisentraut . * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.525 2009/12/02 04:54:10 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.526 2009/12/09 21:57:51 tgl Exp $ * *-------------------------------------------------------------------- */ @@ -2319,7 +2319,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 @@ -2330,7 +2330,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 @@ -4669,29 +4669,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; + } } /* diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index ae73f91edd..424af2e56a 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -13,7 +13,7 @@ * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/miscadmin.h,v 1.214 2009/09/01 00:09:42 tgl Exp $ + * $PostgreSQL: pgsql/src/include/miscadmin.h,v 1.215 2009/12/09 21:57:51 tgl Exp $ * * NOTES * some of the information in this file should be moved to other files. @@ -242,6 +242,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 */ @@ -251,9 +255,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); diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index 53704ec38b..5e2b52bf9a 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -7,7 +7,7 @@ * Copyright (c) 2000-2009, PostgreSQL Global Development Group * Written by Peter Eisentraut . * - * $PostgreSQL: pgsql/src/include/utils/guc.h,v 1.109 2009/11/28 23:38:08 tgl Exp $ + * $PostgreSQL: pgsql/src/include/utils/guc.h,v 1.110 2009/12/09 21:57:51 tgl Exp $ *-------------------------------------------------------------------- */ #ifndef GUC_H @@ -149,7 +149,7 @@ typedef enum #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 */ /* GUC vars that are actually declared in guc.c, rather than elsewhere */ extern bool log_duration;