1
0
mirror of https://github.com/postgres/postgres.git synced 2025-09-05 02:22:28 +03:00

Make standard maintenance operations (including VACUUM, ANALYZE, REINDEX,

and CLUSTER) execute as the table owner rather than the calling user, using
the same privilege-switching mechanism already used for SECURITY DEFINER
functions.  The purpose of this change is to ensure that user-defined
functions used in index definitions cannot acquire the privileges of a
superuser account that is performing routine maintenance.  While a function
used in an index is supposed to be IMMUTABLE and thus not able to do anything
very interesting, there are several easy ways around that restriction; and
even if we could plug them all, there would remain a risk of reading sensitive
information and broadcasting it through a covert channel such as CPU usage.

To prevent bypassing this security measure, execution of SET SESSION
AUTHORIZATION and SET ROLE is now forbidden within a SECURITY DEFINER context.

Thanks to Itagaki Takahiro for reporting this vulnerability.

Security: CVE-2007-6600
This commit is contained in:
Tom Lane
2008-01-03 21:24:26 +00:00
parent 8b1de3b515
commit 46cf9c260d
13 changed files with 210 additions and 108 deletions

View File

@@ -10,7 +10,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.215.2.3 2007/05/30 21:01:53 tgl Exp $
* $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.215.2.4 2008/01/03 21:24:26 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -120,7 +120,8 @@ typedef struct TransactionStateData
MemoryContext curTransactionContext; /* my xact-lifetime context */
ResourceOwner curTransactionOwner; /* my query resources */
List *childXids; /* subcommitted child XIDs */
Oid currentUser; /* subxact start current_user */
Oid prevUser; /* previous CurrentUserId setting */
bool prevSecDefCxt; /* previous SecurityDefinerContext setting */
bool prevXactReadOnly; /* entry-time xact r/o state */
struct TransactionStateData *parent; /* back link to parent */
} TransactionStateData;
@@ -152,7 +153,8 @@ static TransactionStateData TopTransactionStateData = {
NULL, /* cur transaction context */
NULL, /* cur transaction resource owner */
NIL, /* subcommitted child Xids */
0, /* entry-time current userid */
InvalidOid, /* previous CurrentUserId setting */
false, /* previous SecurityDefinerContext setting */
false, /* entry-time xact r/o state */
NULL /* link to parent state block */
};
@@ -1373,18 +1375,14 @@ StartTransaction(void)
/*
* initialize current transaction state fields
*
* note: prevXactReadOnly is not used at the outermost level
*/
s->nestingLevel = 1;
s->childXids = NIL;
/*
* You might expect to see "s->currentUser = GetUserId();" here, but you
* won't because it doesn't work during startup; the userid isn't set yet
* during a backend's first transaction start. We only use the
* currentUser field in sub-transaction state structs.
*
* prevXactReadOnly is also valid only in sub-transactions.
*/
GetUserIdAndContext(&s->prevUser, &s->prevSecDefCxt);
/* SecurityDefinerContext should never be set outside a transaction */
Assert(!s->prevSecDefCxt);
/*
* initialize other subsystems for new transaction
@@ -1866,17 +1864,16 @@ AbortTransaction(void)
AtAbort_ResourceOwner();
/*
* Reset user id which might have been changed transiently. We cannot use
* s->currentUser, since it may not be set yet; instead rely on internal
* state of miscinit.c.
* 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.
*
* (Note: it is not necessary to restore session authorization here
* because that can only be changed via GUC, and GUC will take care of
* rolling it back if need be. However, an error within a SECURITY
* DEFINER function could send control here with the wrong current
* userid.)
* (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.)
*/
AtAbort_UserId();
SetUserIdAndContext(s->prevUser, s->prevSecDefCxt);
/*
* do abort processing
@@ -3716,6 +3713,12 @@ AbortSubTransaction(void)
AtSubAbort_Memory();
AtSubAbort_ResourceOwner();
/*
* Reset user ID which might have been changed transiently. (See notes
* in AbortTransaction.)
*/
SetUserIdAndContext(s->prevUser, s->prevSecDefCxt);
/*
* We can skip all this stuff if the subxact failed before creating a
* ResourceOwner...
@@ -3768,20 +3771,6 @@ AbortSubTransaction(void)
AtEOSubXact_HashTables(false, s->nestingLevel);
}
/*
* Reset user id which might have been changed transiently. Here we want
* to restore to the userid that was current at subxact entry. (As in
* AbortTransaction, we need not worry about the session userid.)
*
* Must do this after AtEOXact_GUC to handle the case where we entered the
* subxact inside a SECURITY DEFINER function (hence current and session
* userids were different) and then session auth was changed inside the
* subxact. GUC will reset both current and session userids to the
* entry-time session userid. This is right in every other scenario so it
* seems simplest to let GUC do that and fix it here.
*/
SetUserId(s->currentUser);
/*
* Restore the upper transaction's read-only state, too. This should be
* redundant with GUC's cleanup but we may as well do it for consistency
@@ -3836,13 +3825,6 @@ PushTransaction(void)
{
TransactionState p = CurrentTransactionState;
TransactionState s;
Oid currentUser;
/*
* At present, GetUserId cannot fail, but let's not assume that. Get the
* ID before entering the critical code sequence.
*/
currentUser = GetUserId();
/*
* We keep subtransaction state nodes in TopTransactionContext.
@@ -3875,7 +3857,7 @@ PushTransaction(void)
s->savepointLevel = p->savepointLevel;
s->state = TRANS_DEFAULT;
s->blockState = TBLOCK_SUBBEGIN;
s->currentUser = currentUser;
GetUserIdAndContext(&s->prevUser, &s->prevSecDefCxt);
s->prevXactReadOnly = XactReadOnly;
CurrentTransactionState = s;

View File

@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.261.2.3 2007/11/08 23:23:07 tgl Exp $
* $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.261.2.4 2008/01/03 21:24:26 tgl Exp $
*
*
* INTERFACE ROUTINES
@@ -1361,6 +1361,8 @@ index_build(Relation heapRelation,
IndexInfo *indexInfo)
{
RegProcedure procedure;
Oid save_userid;
bool save_secdefcxt;
/*
* sanity checks
@@ -1371,6 +1373,13 @@ index_build(Relation heapRelation,
procedure = indexRelation->rd_am->ambuild;
Assert(RegProcedureIsValid(procedure));
/*
* Switch to the table owner's userid, so that any index functions are
* run as that user.
*/
GetUserIdAndContext(&save_userid, &save_secdefcxt);
SetUserIdAndContext(heapRelation->rd_rel->relowner, true);
/*
* Call the access method's build procedure
*/
@@ -1378,6 +1387,9 @@ index_build(Relation heapRelation,
PointerGetDatum(heapRelation),
PointerGetDatum(indexRelation),
PointerGetDatum(indexInfo));
/* Restore userid */
SetUserIdAndContext(save_userid, save_secdefcxt);
}

View File

@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.89.2.1 2005/11/22 18:23:06 momjian Exp $
* $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.89.2.2 2008/01/03 21:24:26 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -112,6 +112,8 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt)
double totalrows,
totaldeadrows;
HeapTuple *rows;
Oid save_userid;
bool save_secdefcxt;
if (vacstmt->verbose)
elevel = INFO;
@@ -199,6 +201,13 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt)
get_namespace_name(RelationGetNamespace(onerel)),
RelationGetRelationName(onerel))));
/*
* Switch to the table owner's userid, so that any index functions are
* run as that user.
*/
GetUserIdAndContext(&save_userid, &save_secdefcxt);
SetUserIdAndContext(onerel->rd_rel->relowner, true);
/*
* Determine which columns to analyze
*
@@ -318,9 +327,7 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt)
onerel->rd_rel->relisshared,
0, 0);
vac_close_indexes(nindexes, Irel, AccessShareLock);
relation_close(onerel, AccessShareLock);
return;
goto cleanup;
}
/*
@@ -439,6 +446,9 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt)
totalrows, totaldeadrows);
}
/* We skip to here if there were no analyzable columns */
cleanup:
/* Done with indexes */
vac_close_indexes(nindexes, Irel, NoLock);
@@ -448,6 +458,9 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt)
* we made in pg_statistic.)
*/
relation_close(onerel, NoLock);
/* Restore userid */
SetUserIdAndContext(save_userid, save_secdefcxt);
}
/*

View File

@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/commands/schemacmds.c,v 1.35 2005/10/15 02:49:15 momjian Exp $
* $PostgreSQL: pgsql/src/backend/commands/schemacmds.c,v 1.35.2.1 2008/01/03 21:24:26 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -44,9 +44,10 @@ CreateSchemaCommand(CreateSchemaStmt *stmt)
ListCell *parsetree_item;
Oid owner_uid;
Oid saved_uid;
bool saved_secdefcxt;
AclResult aclresult;
saved_uid = GetUserId();
GetUserIdAndContext(&saved_uid, &saved_secdefcxt);
/*
* Who is supposed to own the new schema?
@@ -82,11 +83,11 @@ CreateSchemaCommand(CreateSchemaStmt *stmt)
* temporarily set the current user so that the object(s) will be created
* with the correct ownership.
*
* (The setting will revert to session user on error or at the end of this
* routine.)
* (The setting will be restored at the end of this routine, or in case
* of error, transaction abort will clean things up.)
*/
if (saved_uid != owner_uid)
SetUserId(owner_uid);
SetUserIdAndContext(owner_uid, true);
/* Create the schema's namespace */
namespaceId = NamespaceCreate(schemaName, owner_uid);
@@ -138,7 +139,7 @@ CreateSchemaCommand(CreateSchemaStmt *stmt)
PopSpecialNamespace(namespaceId);
/* Reset current user */
SetUserId(saved_uid);
SetUserIdAndContext(saved_uid, saved_secdefcxt);
}

View File

@@ -13,7 +13,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.317.2.5 2007/06/14 13:54:40 alvherre Exp $
* $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.317.2.6 2008/01/03 21:24:26 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -961,6 +961,8 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind)
LockRelId onerelid;
Oid toast_relid;
bool result;
Oid save_userid;
bool save_secdefcxt;
/* Begin a transaction for vacuuming this relation */
StartTransactionCommand();
@@ -1071,6 +1073,14 @@ 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.)
*/
GetUserIdAndContext(&save_userid, &save_secdefcxt);
SetUserIdAndContext(onerel->rd_rel->relowner, true);
/*
* Do the actual work --- either FULL or "lazy" vacuum
*/
@@ -1081,6 +1091,9 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind)
result = true; /* did the vacuum */
/* Restore userid */
SetUserIdAndContext(save_userid, save_secdefcxt);
/* all done with this class, but hold lock until commit */
relation_close(onerel, NoLock);

View File

@@ -9,7 +9,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/commands/variable.c,v 1.114.2.2 2006/02/12 22:32:56 tgl Exp $
* $PostgreSQL: pgsql/src/backend/commands/variable.c,v 1.114.2.3 2008/01/03 21:24:26 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -623,6 +623,22 @@ assign_session_authorization(const char *value, bool doit, GucSource source)
/* not a saved ID, so look it up */
HeapTuple roleTup;
if (InSecurityDefinerContext())
{
/*
* Disallow SET SESSION AUTHORIZATION inside a security definer
* context. We need to do this because when we exit the context,
* GUC won't be notified, leaving things out of sync. Note that
* this test is positioned so that restoring a previously saved
* setting isn't prevented.
*/
if (source >= PGC_S_INTERACTIVE)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot set session authorization within security-definer function")));
return NULL;
}
if (!IsTransactionState())
{
/*
@@ -730,6 +746,25 @@ assign_role(const char *value, bool doit, GucSource source)
}
}
if (roleid == InvalidOid && InSecurityDefinerContext())
{
/*
* Disallow SET ROLE inside a security definer context. We need to do
* this because when we exit the context, GUC won't be notified,
* leaving things out of sync. Note that this test is arranged so
* that restoring a previously saved setting isn't prevented.
*
* XXX it would be nice to allow this case in future, with the
* behavior being that the SET ROLE's effects end when the security
* definer context is exited.
*/
if (source >= PGC_S_INTERACTIVE)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot set role within security-definer function")));
return NULL;
}
if (roleid == InvalidOid &&
strcmp(actual_rolename, "none") != 0)
{

View File

@@ -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.2 2007/08/15 19:16:04 tgl Exp $
* $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.82.2.3 2008/01/03 21:24:26 tgl Exp $
*
* ----------
*/
@@ -3003,7 +3003,8 @@ ri_PlanCheck(const char *querystr, int nargs, Oid *argtypes,
{
void *qplan;
Relation query_rel;
Oid save_uid;
Oid save_userid;
bool save_secdefcxt;
/*
* The query is always run against the FK table except when this is an
@@ -3017,8 +3018,8 @@ ri_PlanCheck(const char *querystr, int nargs, Oid *argtypes,
query_rel = fk_rel;
/* Switch to proper UID to perform check as */
save_uid = GetUserId();
SetUserId(RelationGetForm(query_rel)->relowner);
GetUserIdAndContext(&save_userid, &save_secdefcxt);
SetUserIdAndContext(RelationGetForm(query_rel)->relowner, true);
/* Create the plan */
qplan = SPI_prepare(querystr, nargs, argtypes);
@@ -3027,7 +3028,7 @@ ri_PlanCheck(const char *querystr, int nargs, Oid *argtypes,
elog(ERROR, "SPI_prepare returned %d for %s", SPI_result, querystr);
/* Restore UID */
SetUserId(save_uid);
SetUserIdAndContext(save_userid, save_secdefcxt);
/* Save the plan if requested */
if (cache_plan)
@@ -3056,7 +3057,8 @@ ri_PerformCheck(RI_QueryKey *qkey, void *qplan,
Snapshot crosscheck_snapshot;
int limit;
int spi_result;
Oid save_uid;
Oid save_userid;
bool save_secdefcxt;
Datum vals[RI_MAX_NUMKEYS * 2];
char nulls[RI_MAX_NUMKEYS * 2];
@@ -3134,8 +3136,8 @@ ri_PerformCheck(RI_QueryKey *qkey, void *qplan,
limit = (expect_OK == SPI_OK_SELECT) ? 1 : 0;
/* Switch to proper UID to perform check as */
save_uid = GetUserId();
SetUserId(RelationGetForm(query_rel)->relowner);
GetUserIdAndContext(&save_userid, &save_secdefcxt);
SetUserIdAndContext(RelationGetForm(query_rel)->relowner, true);
/* Finally we can run the query. */
spi_result = SPI_execute_snapshot(qplan,
@@ -3144,7 +3146,7 @@ ri_PerformCheck(RI_QueryKey *qkey, void *qplan,
false, false, limit);
/* Restore UID */
SetUserId(save_uid);
SetUserIdAndContext(save_userid, save_secdefcxt);
/* Check result */
if (spi_result < 0)

View File

@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/utils/fmgr/fmgr.c,v 1.97.2.2 2007/07/31 15:50:01 tgl Exp $
* $PostgreSQL: pgsql/src/backend/utils/fmgr/fmgr.c,v 1.97.2.3 2008/01/03 21:24:26 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -784,6 +784,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
FmgrInfo *save_flinfo;
struct fmgr_security_definer_cache *volatile fcache;
Oid save_userid;
bool save_secdefcxt;
HeapTuple tuple;
if (!fcinfo->flinfo->fn_extra)
@@ -809,26 +810,32 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
else
fcache = fcinfo->flinfo->fn_extra;
GetUserIdAndContext(&save_userid, &save_secdefcxt);
SetUserIdAndContext(fcache->userid, true);
/*
* We don't need to restore the userid settings on error, because the
* ensuing xact or subxact abort will do that. The PG_TRY block is only
* needed to clean up the flinfo link.
*/
save_flinfo = fcinfo->flinfo;
save_userid = GetUserId();
PG_TRY();
{
fcinfo->flinfo = &fcache->flinfo;
SetUserId(fcache->userid);
result = FunctionCallInvoke(fcinfo);
}
PG_CATCH();
{
fcinfo->flinfo = save_flinfo;
SetUserId(save_userid);
PG_RE_THROW();
}
PG_END_TRY();
fcinfo->flinfo = save_flinfo;
SetUserId(save_userid);
SetUserIdAndContext(save_userid, save_secdefcxt);
return result;
}

View File

@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/utils/init/miscinit.c,v 1.150.2.1 2005/11/22 18:23:24 momjian Exp $
* $PostgreSQL: pgsql/src/backend/utils/init/miscinit.c,v 1.150.2.2 2008/01/03 21:24:26 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -286,13 +286,15 @@ make_absolute_path(const char *path)
* OuterUserId is the current user ID in effect at the "outer level" (outside
* any transaction or function). This is initially the same as SessionUserId,
* but can be changed by SET ROLE to any role that SessionUserId is a
* member of. We store this mainly so that AtAbort_UserId knows what to
* reset CurrentUserId to.
* member of. (XXX rename to something like CurrentRoleId?)
*
* CurrentUserId is the current effective user ID; this is the one to use
* for all normal permissions-checking purposes. At outer level this will
* 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.
* ----------------------------------------------------------------
*/
static Oid AuthenticatedUserId = InvalidOid;
@@ -304,12 +306,16 @@ static Oid CurrentUserId = InvalidOid;
static bool AuthenticatedUserIsSuperuser = false;
static bool SessionUserIsSuperuser = false;
static bool SecurityDefinerContext = false;
/* We also remember if a SET ROLE is currently active */
static bool SetRoleIsActive = false;
/*
* GetUserId/SetUserId - get/set the current effective user ID.
* GetUserId - get the current effective user ID.
*
* Note: there's no SetUserId() anymore; use SetUserIdAndContext().
*/
Oid
GetUserId(void)
@@ -319,14 +325,6 @@ GetUserId(void)
}
void
SetUserId(Oid userid)
{
AssertArg(OidIsValid(userid));
CurrentUserId = userid;
}
/*
* GetOuterUserId/SetOuterUserId - get/set the outer-level user ID.
*/
@@ -341,6 +339,7 @@ GetOuterUserId(void)
static void
SetOuterUserId(Oid userid)
{
AssertState(!SecurityDefinerContext);
AssertArg(OidIsValid(userid));
OuterUserId = userid;
@@ -363,6 +362,7 @@ GetSessionUserId(void)
static void
SetSessionUserId(Oid userid, bool is_superuser)
{
AssertState(!SecurityDefinerContext);
AssertArg(OidIsValid(userid));
SessionUserId = userid;
SessionUserIsSuperuser = is_superuser;
@@ -374,6 +374,44 @@ SetSessionUserId(Oid userid, bool is_superuser)
}
/*
* GetUserIdAndContext/SetUserIdAndContext - get/set the current user ID
* and the SecurityDefinerContext flag.
*
* Unlike GetUserId, GetUserIdAndContext does *not* Assert that the current
* value of CurrentUserId is valid; nor does SetUserIdAndContext 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,
* and during the first transaction within a backend, the value to be saved
* and perhaps restored is indeed invalid. We have to be able to get
* through AbortTransaction without asserting in case InitPostgres fails.
*/
void
GetUserIdAndContext(Oid *userid, bool *sec_def_context)
{
*userid = CurrentUserId;
*sec_def_context = SecurityDefinerContext;
}
void
SetUserIdAndContext(Oid userid, bool sec_def_context)
{
CurrentUserId = userid;
SecurityDefinerContext = sec_def_context;
}
/*
* InSecurityDefinerContext - are we inside a SECURITY DEFINER context?
*/
bool
InSecurityDefinerContext(void)
{
return SecurityDefinerContext;
}
/*
* Initialize user identity during normal backend startup
*/
@@ -496,21 +534,6 @@ InitializeSessionUserIdStandalone(void)
}
/*
* Reset effective userid during AbortTransaction
*
* This is essentially SetUserId(GetOuterUserId()), but without the Asserts.
* The reason is that if a backend's InitPostgres transaction fails (eg,
* because an invalid user name was given), we have to be able to get through
* AbortTransaction without asserting.
*/
void
AtAbort_UserId(void)
{
CurrentUserId = OuterUserId;
}
/*
* Change session auth ID while running
*