From: Robert Haas Date: Thu, 22 Jul 2010 00:47:59 +0000 (+0000) Subject: Centralize DML permissions-checking logic. X-Git-Tag: REL9_1_ALPHA1~193 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b8c6c71d1c513391975fc107a95f104aeeac3117;p=postgresql Centralize DML permissions-checking logic. Remove bespoke code in DoCopy and RI_Initial_Check, which now instead fabricate call ExecCheckRTPerms with a manufactured RangeTblEntry. This is intended to make it feasible for an enhanced security provider to actually make use of ExecutorCheckPerms_hook, but also has the advantage that RI_Initial_Check can allow use of the fast-path when column-level but not table-level permissions are present. KaiGai Kohei. Reviewed (in an earlier version) by Stephen Frost, and by me. Some further changes to the comments by me. --- diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 4e95a8315c..681a7aaa92 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/copy.c,v 1.327 2010/04/28 16:10:41 heikki Exp $ + * $PostgreSQL: pgsql/src/backend/commands/copy.c,v 1.328 2010/07/22 00:47:52 rhaas Exp $ * *------------------------------------------------------------------------- */ @@ -21,6 +21,7 @@ #include #include "access/heapam.h" +#include "access/sysattr.h" #include "access/xact.h" #include "catalog/namespace.h" #include "catalog/pg_type.h" @@ -726,8 +727,6 @@ DoCopy(const CopyStmt *stmt, const char *queryString) bool force_quote_all = false; bool format_specified = false; AclMode required_access = (is_from ? ACL_INSERT : ACL_SELECT); - AclMode relPerms; - AclMode remainingPerms; ListCell *option; TupleDesc tupDesc; int num_phys_attrs; @@ -988,6 +987,10 @@ DoCopy(const CopyStmt *stmt, const char *queryString) if (stmt->relation) { + RangeTblEntry *rte; + List *attnums; + ListCell *cur; + Assert(!stmt->query); cstate->queryDesc = NULL; @@ -998,28 +1001,22 @@ DoCopy(const CopyStmt *stmt, const char *queryString) tupDesc = RelationGetDescr(cstate->rel); /* Check relation permissions. */ - relPerms = pg_class_aclmask(RelationGetRelid(cstate->rel), GetUserId(), - required_access, ACLMASK_ALL); - remainingPerms = required_access & ~relPerms; - if (remainingPerms != 0) - { - /* We don't have table permissions, check per-column permissions */ - List *attnums; - ListCell *cur; + rte = makeNode(RangeTblEntry); + rte->rtekind = RTE_RELATION; + rte->relid = RelationGetRelid(cstate->rel); + rte->requiredPerms = required_access; - attnums = CopyGetAttnums(tupDesc, cstate->rel, attnamelist); - foreach(cur, attnums) - { - int attnum = lfirst_int(cur); + attnums = CopyGetAttnums(tupDesc, cstate->rel, attnamelist); + foreach (cur, attnums) + { + int attno = lfirst_int(cur) - FirstLowInvalidHeapAttributeNumber; - if (pg_attribute_aclcheck(RelationGetRelid(cstate->rel), - attnum, - GetUserId(), - remainingPerms) != ACLCHECK_OK) - aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, - RelationGetRelationName(cstate->rel)); - } + if (is_from) + rte->modifiedCols = bms_add_member(rte->modifiedCols, attno); + else + rte->selectedCols = bms_add_member(rte->selectedCols, attno); } + ExecCheckRTPerms(list_make1(rte), true); /* check read-only transaction */ if (XactReadOnly && is_from && !cstate->rel->rd_islocaltemp) diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 9eb765378f..87ca2a1b4b 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.351 2010/07/12 17:01:05 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.352 2010/07/22 00:47:52 rhaas Exp $ * *------------------------------------------------------------------------- */ @@ -75,8 +75,7 @@ static void ExecutePlan(EState *estate, PlanState *planstate, long numberTuples, ScanDirection direction, DestReceiver *dest); -static void ExecCheckRTPerms(List *rangeTable); -static void ExecCheckRTEPerms(RangeTblEntry *rte); +static bool ExecCheckRTEPerms(RangeTblEntry *rte); static void ExecCheckXactReadOnly(PlannedStmt *plannedstmt); static void EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree); @@ -409,26 +408,42 @@ ExecutorRewind(QueryDesc *queryDesc) /* * ExecCheckRTPerms * Check access permissions for all relations listed in a range table. + * + * Returns true if permissions are adequate. Otherwise, throws an appropriate + * error if ereport_on_violation is true, or simply returns false otherwise. */ -static void -ExecCheckRTPerms(List *rangeTable) +bool +ExecCheckRTPerms(List *rangeTable, bool ereport_on_violation) { ListCell *l; + bool result = true; foreach(l, rangeTable) { - ExecCheckRTEPerms((RangeTblEntry *) lfirst(l)); + RangeTblEntry *rte = (RangeTblEntry *) lfirst(l); + + result = ExecCheckRTEPerms(rte); + if (!result) + { + Assert(rte->rtekind == RTE_RELATION); + if (ereport_on_violation) + aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, + get_rel_name(rte->relid)); + return false; + } } if (ExecutorCheckPerms_hook) - (*ExecutorCheckPerms_hook)(rangeTable); + result = (*ExecutorCheckPerms_hook)(rangeTable, + ereport_on_violation); + return result; } /* * ExecCheckRTEPerms * Check access permissions for a single RTE. */ -static void +static bool ExecCheckRTEPerms(RangeTblEntry *rte) { AclMode requiredPerms; @@ -445,14 +460,14 @@ ExecCheckRTEPerms(RangeTblEntry *rte) * Join, subquery, and special RTEs need no checks. */ if (rte->rtekind != RTE_RELATION) - return; + return true; /* * No work if requiredPerms is empty. */ requiredPerms = rte->requiredPerms; if (requiredPerms == 0) - return; + return true; relOid = rte->relid; @@ -480,8 +495,7 @@ ExecCheckRTEPerms(RangeTblEntry *rte) * we can fail straight away. */ if (remainingPerms & ~(ACL_SELECT | ACL_INSERT | ACL_UPDATE)) - aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, - get_rel_name(relOid)); + return false; /* * Check to see if we have the needed privileges at column level. @@ -501,8 +515,7 @@ ExecCheckRTEPerms(RangeTblEntry *rte) { if (pg_attribute_aclcheck_all(relOid, userid, ACL_SELECT, ACLMASK_ANY) != ACLCHECK_OK) - aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, - get_rel_name(relOid)); + return false; } tmpset = bms_copy(rte->selectedCols); @@ -515,15 +528,13 @@ ExecCheckRTEPerms(RangeTblEntry *rte) /* Whole-row reference, must have priv on all cols */ if (pg_attribute_aclcheck_all(relOid, userid, ACL_SELECT, ACLMASK_ALL) != ACLCHECK_OK) - aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, - get_rel_name(relOid)); + return false; } else { - if (pg_attribute_aclcheck(relOid, col, userid, ACL_SELECT) - != ACLCHECK_OK) - aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, - get_rel_name(relOid)); + if (pg_attribute_aclcheck(relOid, col, userid, + ACL_SELECT) != ACLCHECK_OK) + return false; } } bms_free(tmpset); @@ -546,8 +557,7 @@ ExecCheckRTEPerms(RangeTblEntry *rte) { if (pg_attribute_aclcheck_all(relOid, userid, remainingPerms, ACLMASK_ANY) != ACLCHECK_OK) - aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, - get_rel_name(relOid)); + return false; } tmpset = bms_copy(rte->modifiedCols); @@ -562,15 +572,15 @@ ExecCheckRTEPerms(RangeTblEntry *rte) } else { - if (pg_attribute_aclcheck(relOid, col, userid, remainingPerms) - != ACLCHECK_OK) - aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, - get_rel_name(relOid)); + if (pg_attribute_aclcheck(relOid, col, userid, + remainingPerms) != ACLCHECK_OK) + return false; } } bms_free(tmpset); } } + return true; } /* @@ -636,7 +646,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) /* * Do permissions checks */ - ExecCheckRTPerms(rangeTable); + ExecCheckRTPerms(rangeTable, true); /* * initialize the node's execution state diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 2d0ab44b7d..080cb9c1ac 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-2010, PostgreSQL Global Development Group * - * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.118 2010/02/14 18:42:16 rhaas Exp $ + * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.119 2010/07/22 00:47:52 rhaas Exp $ * * ---------- */ @@ -31,10 +31,12 @@ #include "postgres.h" #include "access/xact.h" +#include "access/sysattr.h" #include "catalog/pg_constraint.h" #include "catalog/pg_operator.h" #include "catalog/pg_type.h" #include "commands/trigger.h" +#include "executor/executor.h" #include "executor/spi.h" #include "parser/parse_coerce.h" #include "parser/parse_relation.h" @@ -2624,6 +2626,8 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel) char fkrelname[MAX_QUOTED_REL_NAME_LEN]; char pkattname[MAX_QUOTED_NAME_LEN + 3]; char fkattname[MAX_QUOTED_NAME_LEN + 3]; + RangeTblEntry *pkrte; + RangeTblEntry *fkrte; const char *sep; int i; int old_work_mem; @@ -2631,6 +2635,9 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel) int spi_result; SPIPlanPtr qplan; + /* Fetch constraint info. */ + ri_FetchConstraintInfo(&riinfo, trigger, fk_rel, false); + /* * Check to make sure current user has enough permissions to do the test * query. (If not, caller can fall back to the trigger method, which @@ -2638,12 +2645,29 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel) * * XXX are there any other show-stopper conditions to check? */ - if (pg_class_aclcheck(RelationGetRelid(fk_rel), GetUserId(), ACL_SELECT) != ACLCHECK_OK) - return false; - if (pg_class_aclcheck(RelationGetRelid(pk_rel), GetUserId(), ACL_SELECT) != ACLCHECK_OK) - return false; + pkrte = makeNode(RangeTblEntry); + pkrte->rtekind = RTE_RELATION; + pkrte->relid = RelationGetRelid(pk_rel); + pkrte->requiredPerms = ACL_SELECT; - ri_FetchConstraintInfo(&riinfo, trigger, fk_rel, false); + fkrte = makeNode(RangeTblEntry); + fkrte->rtekind = RTE_RELATION; + fkrte->relid = RelationGetRelid(fk_rel); + fkrte->requiredPerms = ACL_SELECT; + + for (i = 0; i < riinfo.nkeys; i++) + { + int attno; + + attno = riinfo.pk_attnums[i] - FirstLowInvalidHeapAttributeNumber; + pkrte->selectedCols = bms_add_member(pkrte->selectedCols, attno); + + attno = riinfo.fk_attnums[i] - FirstLowInvalidHeapAttributeNumber; + fkrte->selectedCols = bms_add_member(fkrte->selectedCols, attno); + } + + if (!ExecCheckRTPerms(list_make2(fkrte, pkrte), false)) + return false; /*---------- * The query string built is: diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index ecaaab5f75..4cfbf7ad4d 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.170 2010/07/12 17:01:06 tgl Exp $ + * $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.171 2010/07/22 00:47:59 rhaas Exp $ * *------------------------------------------------------------------------- */ @@ -75,7 +75,7 @@ typedef void (*ExecutorEnd_hook_type) (QueryDesc *queryDesc); extern PGDLLIMPORT ExecutorEnd_hook_type ExecutorEnd_hook; /* Hook for plugins to get control in ExecCheckRTPerms() */ -typedef void (*ExecutorCheckPerms_hook_type) (List *); +typedef bool (*ExecutorCheckPerms_hook_type) (List *, bool); extern PGDLLIMPORT ExecutorCheckPerms_hook_type ExecutorCheckPerms_hook; @@ -161,6 +161,7 @@ extern void standard_ExecutorRun(QueryDesc *queryDesc, extern void ExecutorEnd(QueryDesc *queryDesc); extern void standard_ExecutorEnd(QueryDesc *queryDesc); extern void ExecutorRewind(QueryDesc *queryDesc); +extern bool ExecCheckRTPerms(List *rangeTable, bool ereport_on_violation); extern void InitResultRelInfo(ResultRelInfo *resultRelInfo, Relation resultRelationDesc, Index resultRelationIndex,