diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 36070484ca3..55a46cef668 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.215.2.5 2009/11/23 09:59:21 heikki Exp $ + * $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.215.2.6 2009/12/09 21:58:41 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -116,12 +116,13 @@ typedef struct TransactionStateData int savepointLevel; /* savepoint level */ TransState state; /* low-level state */ TBlockState blockState; /* high-level state */ - int nestingLevel; /* nest depth */ + int nestingLevel; /* transaction nesting depth */ + int gucNestLevel; /* GUC context nesting depth */ MemoryContext curTransactionContext; /* my xact-lifetime context */ ResourceOwner curTransactionOwner; /* my query resources */ List *childXids; /* subcommitted child XIDs */ 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; @@ -149,12 +150,13 @@ static TransactionStateData TopTransactionStateData = { TRANS_DEFAULT, /* transaction state */ TBLOCK_DEFAULT, /* transaction block state from the client * perspective */ - 0, /* nesting level */ + 0, /* transaction nesting depth */ + 0, /* GUC context nesting depth */ NULL, /* cur transaction context */ NULL, /* cur transaction resource owner */ NIL, /* subcommitted child Xids */ InvalidOid, /* previous CurrentUserId setting */ - false, /* previous SecurityDefinerContext setting */ + 0, /* previous SecurityRestrictionContext */ false, /* entry-time xact r/o state */ NULL /* link to parent state block */ }; @@ -1379,14 +1381,16 @@ StartTransaction(void) * note: prevXactReadOnly is not used at the outermost level */ s->nestingLevel = 1; + s->gucNestLevel = 1; s->childXids = NIL; - 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 */ + AtStart_GUC(); AtStart_Inval(); AtStart_Cache(); AfterTriggerBeginXact(); @@ -1570,7 +1574,7 @@ CommitTransaction(void) /* Check we've released all catcache entries */ AtEOXact_CatCache(true); - AtEOXact_GUC(true, false); + AtEOXact_GUC(true, 1); AtEOXact_SPI(true); AtEOXact_on_commit_actions(true); AtEOXact_Namespace(true); @@ -1590,6 +1594,7 @@ CommitTransaction(void) s->transactionId = InvalidTransactionId; s->subTransactionId = InvalidSubTransactionId; s->nestingLevel = 0; + s->gucNestLevel = 0; s->childXids = NIL; /* @@ -1786,7 +1791,7 @@ PrepareTransaction(void) AtEOXact_CatCache(true); /* PREPARE acts the same as COMMIT as far as GUC is concerned */ - AtEOXact_GUC(true, false); + AtEOXact_GUC(true, 1); AtEOXact_SPI(true); AtEOXact_on_commit_actions(true); AtEOXact_Namespace(true); @@ -1805,6 +1810,7 @@ PrepareTransaction(void) s->transactionId = InvalidTransactionId; s->subTransactionId = InvalidSubTransactionId; s->nestingLevel = 0; + s->gucNestLevel = 0; s->childXids = NIL; /* @@ -1868,13 +1874,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 @@ -1934,7 +1940,7 @@ AbortTransaction(void) false, true); AtEOXact_CatCache(false); - AtEOXact_GUC(false, false); + AtEOXact_GUC(false, 1); AtEOXact_SPI(false); AtEOXact_on_commit_actions(false); AtEOXact_Namespace(false); @@ -1981,6 +1987,7 @@ CleanupTransaction(void) s->transactionId = InvalidTransactionId; s->subTransactionId = InvalidSubTransactionId; s->nestingLevel = 0; + s->gucNestLevel = 0; s->childXids = NIL; /* @@ -3645,7 +3652,7 @@ CommitSubTransaction(void) RESOURCE_RELEASE_AFTER_LOCKS, true, false); - AtEOXact_GUC(true, true); + AtEOXact_GUC(true, s->gucNestLevel); AtEOSubXact_SPI(true, s->subTransactionId); AtEOSubXact_on_commit_actions(true, s->subTransactionId, s->parent->subTransactionId); @@ -3718,7 +3725,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 @@ -3761,7 +3768,7 @@ AbortSubTransaction(void) RESOURCE_RELEASE_AFTER_LOCKS, false, false); - AtEOXact_GUC(false, true); + AtEOXact_GUC(false, s->gucNestLevel); AtEOSubXact_SPI(false, s->subTransactionId); AtEOSubXact_on_commit_actions(false, s->subTransactionId, s->parent->subTransactionId); @@ -3855,10 +3862,11 @@ PushTransaction(void) s->subTransactionId = currentSubTransactionId; s->parent = p; s->nestingLevel = p->nestingLevel + 1; + s->gucNestLevel = NewGUCNestLevel(); 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 343779cfad1..557dd82985d 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.261.2.5 2008/05/27 21:13:39 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.261.2.6 2009/12/09 21:58:42 tgl Exp $ * * * INTERFACE ROUTINES @@ -45,6 +45,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" @@ -1364,7 +1365,8 @@ index_build(Relation heapRelation, { RegProcedure procedure; Oid save_userid; - bool save_secdefcxt; + int save_sec_context; + int save_nestlevel; /* * sanity checks @@ -1376,11 +1378,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 @@ -1390,8 +1395,11 @@ index_build(Relation heapRelation, PointerGetDatum(indexRelation), PointerGetDatum(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); } diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 04ac6b92c84..9291b03c29f 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.89.2.3 2009/05/19 08:30:24 heikki Exp $ + * $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.89.2.4 2009/12/09 21:58:42 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -34,6 +34,7 @@ #include "utils/builtins.h" #include "utils/datum.h" #include "utils/fmgroids.h" +#include "utils/guc.h" #include "utils/lsyscache.h" #include "utils/memutils.h" #include "utils/syscache.h" @@ -113,7 +114,8 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt) totaldeadrows; HeapTuple *rows; Oid save_userid; - bool save_secdefcxt; + int save_sec_context; + int save_nestlevel; if (vacstmt->verbose) elevel = INFO; @@ -202,11 +204,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(); /* * Determine which columns to analyze @@ -447,8 +452,11 @@ cleanup: */ relation_close(onerel, NoLock); - /* 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 957d50d7fa7..a7636c922a5 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.35.2.1 2008/01/03 21:24:26 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/schemacmds.c,v 1.35.2.2 2009/12/09 21:58:42 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -44,10 +44,10 @@ CreateSchemaCommand(CreateSchemaStmt *stmt) 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? @@ -87,7 +87,8 @@ CreateSchemaCommand(CreateSchemaStmt *stmt) * 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); @@ -138,8 +139,8 @@ CreateSchemaCommand(CreateSchemaStmt *stmt) /* Reset search path to normal state */ PopSpecialNamespace(namespaceId); - /* 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 f54902f5daa..e4bcff6c38a 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.174.2.8 2008/10/07 11:16:01 heikki Exp $ + * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.174.2.9 2009/12/09 21:58:42 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -295,6 +295,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 f9fce596a36..eb0ba4afef2 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.317.2.8 2009/11/10 18:01:11 alvherre Exp $ + * $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.317.2.9 2009/12/09 21:58:42 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -44,6 +44,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" @@ -962,7 +963,8 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind) Oid toast_relid; bool result; Oid save_userid; - bool save_secdefcxt; + int save_sec_context; + int save_nestlevel; bool heldoff; /* Begin a transaction for vacuuming this relation */ @@ -1075,12 +1077,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 @@ -1092,8 +1097,11 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind) result = true; /* did the vacuum */ - /* 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 643714a948c..923ac2c7d05 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.256.2.7 2008/08/08 17:01:34 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.256.2.8 2009/12/09 21:58:42 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -763,6 +763,17 @@ InitPlan(QueryDesc *queryDesc, bool explainOnly) Oid intoRelationId; TupleDesc tupdesc; + /* + * 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 (parseTree->into->istemp && InSecurityRestrictedOperation()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("cannot create temporary table within security-restricted operation"))); + /* * find namespace to create in, check permissions */ diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 8e3c900053c..38452672a14 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.245 2005/10/15 02:49:27 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.245.2.1 2009/12/09 21:58:42 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -330,6 +330,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 @@ -474,6 +493,7 @@ ProcessUtility(Node *parsetree, { ClosePortalStmt *stmt = (ClosePortalStmt *) parsetree; + CheckRestrictedOperation("CLOSE"); PerformPortalClose(stmt->portalname); } break; @@ -606,6 +626,7 @@ ProcessUtility(Node *parsetree, break; case T_PrepareStmt: + CheckRestrictedOperation("PREPARE"); PrepareQuery((PrepareStmt *) parsetree); break; @@ -614,6 +635,7 @@ ProcessUtility(Node *parsetree, break; case T_DeallocateStmt: + CheckRestrictedOperation("DEALLOCATE"); DeallocateQuery((DeallocateStmt *) parsetree); break; @@ -820,6 +842,7 @@ ProcessUtility(Node *parsetree, { ListenStmt *stmt = (ListenStmt *) parsetree; + CheckRestrictedOperation("LISTEN"); Async_Listen(stmt->relation->relname); } break; @@ -828,6 +851,7 @@ ProcessUtility(Node *parsetree, { UnlistenStmt *stmt = (UnlistenStmt *) parsetree; + CheckRestrictedOperation("UNLISTEN"); Async_Unlisten(stmt->relation->relname); } break; diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 7cbac5eab82..45f19eaf561 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -17,7 +17,7 @@ * * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * - * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.82.2.3 2008/01/03 21:24:26 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.82.2.4 2009/12/09 21:58:42 tgl Exp $ * * ---------- */ @@ -3004,7 +3004,7 @@ ri_PlanCheck(const char *querystr, int nargs, Oid *argtypes, void *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 @@ -3018,8 +3018,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); @@ -3027,8 +3028,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) @@ -3058,7 +3059,7 @@ ri_PerformCheck(RI_QueryKey *qkey, void *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]; @@ -3136,8 +3137,9 @@ ri_PerformCheck(RI_QueryKey *qkey, void *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, @@ -3145,8 +3147,8 @@ ri_PerformCheck(RI_QueryKey *qkey, void *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 48a5b0bf654..1c85102002b 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.97.2.3 2008/01/03 21:24:26 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/fmgr/fmgr.c,v 1.97.2.4 2009/12/09 21:58:42 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -784,7 +784,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS) FmgrInfo *save_flinfo; struct fmgr_security_definer_cache *volatile fcache; Oid save_userid; - bool save_secdefcxt; + int save_sec_context; HeapTuple tuple; if (!fcinfo->flinfo->fn_extra) @@ -810,8 +810,9 @@ fmgr_security_definer(PG_FUNCTION_ARGS) else fcache = fcinfo->flinfo->fn_extra; - GetUserIdAndContext(&save_userid, &save_secdefcxt); - SetUserIdAndContext(fcache->userid, true); + GetUserIdAndSecContext(&save_userid, &save_sec_context); + SetUserIdAndSecContext(fcache->userid, + save_sec_context | SECURITY_LOCAL_USERID_CHANGE); /* * We don't need to restore the userid settings on error, because the @@ -835,7 +836,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS) fcinfo->flinfo = save_flinfo; - 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 7a81fabb8fb..34dfe670e43 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.150.2.2 2008/01/03 21:24:26 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/init/miscinit.c,v 1.150.2.3 2009/12/09 21:58:43 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -293,8 +293,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; @@ -306,7 +308,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; @@ -315,7 +317,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) @@ -339,7 +341,7 @@ GetOuterUserId(void) static void SetOuterUserId(Oid userid) { - AssertState(!SecurityDefinerContext); + AssertState(SecurityRestrictionContext == 0); AssertArg(OidIsValid(userid)); OuterUserId = userid; @@ -362,7 +364,7 @@ GetSessionUserId(void) static void SetSessionUserId(Oid userid, bool is_superuser) { - AssertState(!SecurityDefinerContext); + AssertState(SecurityRestrictionContext == 0); AssertArg(OidIsValid(userid)); SessionUserId = userid; SessionUserIsSuperuser = is_superuser; @@ -375,11 +377,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, @@ -388,27 +408,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 06dce7c4053..751e7921512 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.299.2.5 2009/09/03 22:08:45 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.299.2.6 2009/12/09 21:58:43 tgl Exp $ * *-------------------------------------------------------------------- */ @@ -1936,7 +1936,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 @@ -1947,7 +1947,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 @@ -2162,6 +2162,8 @@ static bool guc_dirty; /* TRUE if need to do commit/abort work */ static bool reporting_enabled; /* TRUE to enable GUC_REPORT */ +static int GUCNestLevel = 0; /* 1 when in main transaction */ + static int guc_var_compare(const void *a, const void *b); static int guc_name_compare(const char *namea, const char *nameb); @@ -3000,17 +3002,16 @@ ResetAllOptions(void) static void push_old_value(struct config_generic * gconf) { - int my_level = GetCurrentTransactionNestLevel(); GucStack *stack; /* If we're not inside a transaction, do nothing */ - if (my_level == 0) + if (GUCNestLevel == 0) return; for (;;) { /* Done if we already pushed it at this nesting depth */ - if (gconf->stack && gconf->stack->nest_level >= my_level) + if (gconf->stack && gconf->stack->nest_level >= GUCNestLevel) return; /* @@ -3069,20 +3070,53 @@ push_old_value(struct config_generic * gconf) } /* - * Do GUC processing at transaction or subtransaction commit or abort. + * Do GUC processing at main transaction start. */ void -AtEOXact_GUC(bool isCommit, bool isSubXact) +AtStart_GUC(void) +{ + /* + * The nest level should be 0 between transactions; if it isn't, + * somebody didn't call AtEOXact_GUC, or called it with the wrong + * nestLevel. We throw a warning but make no other effort to clean up. + */ + if (GUCNestLevel != 0) + elog(WARNING, "GUC nest level = %d at transaction start", + GUCNestLevel); + GUCNestLevel = 1; +} + +/* + * Enter a new nesting level for GUC values. This is called at subtransaction + * start and when entering a function that has proconfig settings. NOTE that + * we must not risk error here, else subtransaction start will be unhappy. + */ +int +NewGUCNestLevel(void) +{ + return ++GUCNestLevel; +} + +/* + * Do GUC processing at transaction or subtransaction commit or abort, or + * when exiting a function that has proconfig settings. (The name is thus + * a bit of a misnomer; perhaps it should be ExitGUCNestLevel or some such.) + * During abort, we discard all GUC settings that were applied at nesting + * levels >= nestLevel. nestLevel == 1 corresponds to the main transaction. + */ +void +AtEOXact_GUC(bool isCommit, int nestLevel) { - int my_level; int i; + Assert(nestLevel > 0 && nestLevel <= GUCNestLevel); + /* Quick exit if nothing's changed in this transaction */ if (!guc_dirty) + { + GUCNestLevel = nestLevel - 1; return; - - my_level = GetCurrentTransactionNestLevel(); - Assert(isSubXact ? (my_level > 1) : (my_level == 1)); + } for (i = 0; i < num_guc_variables; i++) { @@ -3103,9 +3137,9 @@ AtEOXact_GUC(bool isCommit, bool isSubXact) /* Assert that we stacked old value before changing it */ Assert(stack != NULL && (my_status & GUC_HAVE_STACK)); /* However, the last change may have been at an outer xact level */ - if (stack->nest_level < my_level) + if (stack->nest_level < nestLevel) continue; - Assert(stack->nest_level == my_level); + Assert(stack->nest_level == nestLevel); /* * We will pop the stack entry. Start by restoring outer xact status @@ -3289,7 +3323,7 @@ AtEOXact_GUC(bool isCommit, bool isSubXact) set_string_field(conf, &stack->tentative_val.stringval, NULL); /* Don't store tentative value separately after commit */ - if (!isSubXact) + if (nestLevel == 1) set_string_field(conf, &conf->tentative_val, NULL); break; } @@ -3303,7 +3337,7 @@ AtEOXact_GUC(bool isCommit, bool isSubXact) * If we're now out of all xact levels, forget TENTATIVE status bit; * there's nothing tentative about the value anymore. */ - if (!isSubXact) + if (nestLevel == 1) { Assert(gconf->stack == NULL); gconf->status = 0; @@ -3320,8 +3354,11 @@ AtEOXact_GUC(bool isCommit, bool isSubXact) * that all outer transaction levels will have stacked values to deal * with.) */ - if (!isSubXact) + if (nestLevel == 1) guc_dirty = false; + + /* Update nesting level */ + GUCNestLevel = nestLevel - 1; } @@ -3666,29 +3703,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 7a9a5690265..6414427cc3d 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -13,7 +13,7 @@ * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/miscadmin.h,v 1.183.2.2 2008/01/03 21:24:26 tgl Exp $ + * $PostgreSQL: pgsql/src/include/miscadmin.h,v 1.183.2.3 2009/12/09 21:58:44 tgl Exp $ * * NOTES * some of the information in this file should be moved to other files. @@ -222,6 +222,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 */ @@ -231,9 +235,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 dfeb2bc2644..fd1b226a754 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -7,7 +7,7 @@ * Copyright (c) 2000-2005, PostgreSQL Global Development Group * Written by Peter Eisentraut . * - * $PostgreSQL: pgsql/src/include/utils/guc.h,v 1.63.2.1 2006/05/21 20:11:02 tgl Exp $ + * $PostgreSQL: pgsql/src/include/utils/guc.h,v 1.63.2.2 2009/12/09 21:58:44 tgl Exp $ *-------------------------------------------------------------------- */ #ifndef GUC_H @@ -189,7 +189,9 @@ extern void ProcessConfigFile(GucContext context); extern void InitializeGUCOptions(void); extern bool SelectConfigFiles(const char *userDoption, const char *progname); extern void ResetAllOptions(void); -extern void AtEOXact_GUC(bool isCommit, bool isSubXact); +extern void AtStart_GUC(void); +extern int NewGUCNestLevel(void); +extern void AtEOXact_GUC(bool isCommit, int nestLevel); extern void BeginReportingGUCOptions(void); extern void ParseLongOption(const char *string, char **name, char **value); extern bool set_config_option(const char *name, const char *value, diff --git a/src/include/utils/guc_tables.h b/src/include/utils/guc_tables.h index 05b1dc48cf2..7d41103749b 100644 --- a/src/include/utils/guc_tables.h +++ b/src/include/utils/guc_tables.h @@ -7,7 +7,7 @@ * * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * - * $PostgreSQL: pgsql/src/include/utils/guc_tables.h,v 1.20.2.2 2009/09/03 22:08:45 tgl Exp $ + * $PostgreSQL: pgsql/src/include/utils/guc_tables.h,v 1.20.2.3 2009/12/09 21:58:44 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -128,7 +128,7 @@ struct config_generic #define GUC_SUPERUSER_ONLY 0x0100 /* show only to superusers */ #define GUC_IS_NAME 0x0200 /* limit string to NAMEDATALEN-1 */ -#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_HAVE_TENTATIVE 0x0001 /* tentative value is defined */