Stephen, thanks for comments.
The attached three patches are the revised and divided ones.
A: add makeRangeTblEntry()
B: major reworks of DML permission checks
C: add an ESP hook on the DML permission checks
(2010/05/27 0:09), Stephen Frost wrote:
> KaiGai,
>
> * KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
>> The attached patch is a revised one for DML permission checks.
>
> This is certainly alot better.
>
>> ToDo:
>> - makeRangeTblEntry() stuff to allocate a RTE node with given parameter
>> is not yet.
>
> I'd certainly like to see the above done, or to understand why it can't
> be if that turns out to be the case.
The patch-A tries to implement makeRangeTblEntry() which takes only rtekind
as argument right now.
Other fields are initialized to zero, using makeNode().
> A couple of other comments, all pretty minor things:
>
> - I'd still rather see the hook itself in another patch, but given that
> we've determined that none of this is going to go into 9.0, it's not
> as big a deal.
OK, I divided the ESP hook part into the patch-C.
> - The hook definition in aclchk.c should really be at the top of that
> file. We've been pretty consistant about putting hooks at the top of
> files instead of deep down in the file, this should also follow that
> scheme.
OK, I moved it.
> - Some of the comments at the top of chkpriv_rte_perms probably make
> sense to move up to where it's called from execMain.c. Specifically,
> the comments about the other RTE types (function, join, subquery).
> I'd probably change the comment in chkpriv_rte_perms to be simpler-
> "This is only used for checking plain relation permissions, nothing
> else is checked here", and also have that same comment around
> chkpriv_relation_perms, both in aclchk.c and in acl.h.
OK, I edited the comment as follows:
| /*
| * Do permissions checks. The check_relation_privileges() checks access
| * permissions for all relations listed in a range table, but does not
| * check anything for other RTE types (function, join, subquery, ...).
| * Function RTEs are checked by init_fcache when the function is prepared
| * for execution. Join, subquery, and special RTEs need no checks.
| */
> - I'd move chkpriv_relation_perms above chkpriv_rte_perms, it's what we
> expect people to use, after all.
OK, I reordered it.
> - Don't particularly like the function names. How about
> relation_privilege_check? Or rangetbl_privilege_check? We don't use
> 'perms' much (uh, at all?) in function names, and even if we did, it'd
> be redundant and not really help someone understand what the function
> is doing.
IIRC, Robert suggested that a verb should lead the function name.
So, I renamed it into check_relation_privileges() and check_rte_privileges().
> - I don't really like having 'abort' as the variable name for the 2nd
> argument. I'm not finding an obvious convention right now, but maybe
> something like "error_on_failure" instead?
The 'failure' may make an impression of generic errors not only permission
denied.
How about 'error_on_violation'?
> - In DoCopy, some comments about what you're doing there to set up for
> calling chkpriv_relation_perms would be good (like the comment you
> removed- /* We don't have table permissions, check per-column
> permissions */, updated to for something like "build an RTE with the
> columns referenced marked to check for necessary privileges").
> Additionally, it might be worth considering if having an RTE built
> farther up in DoCopy would make sense and would then be usable for
> other bits in DoCopy.
I edited the comments as follows:
| /*
| * Check relation permissions.
| * We built an RTE with the relation and columns to be accessed
| * to check for necessary privileges in the common way.
| */
> - In RI_Initial_Check, why not build up an actual list of RTEs and just
> call chkpriv_relation_perms once? Also, you should add comments
> there, again, about what you're doing and why. If you can use another
> function to build the actual RTE, this will probably fall out more
> sensibly too.
Good catch! I fixed the invocation of checker function with list_make2().
And, I edited the comments as follows:
| /*
| * We built a pair of RTEs of FK/PK relations and columns referenced
| * in the test query to check necessary permission in the common way.
| */
> - Have you checked if there are any bad side-effects from calling
> ri_FetchConstraintInfo before doing the permissions checking?
The ri_FetchConstraintInfo() only references SysCaches to set up given
local variable without any other locks except for ones acquired by syscache.c.
> - The hook in acl.h should be separated out and brought to the top and
> documented independently as to exactly where the hook is and what it
> can be used for, along with what the arguments mean, etc. Similairly,
> chkpriv_relation_perms should really have a short comment for it about
> what it's for. Something more than 'security checker function'.
OK, at the patch-C, I moved the definition of the hook into the first half
of acl.h, but it needs to be declared after the AclResult definition.
BTW, I wonder whether acl.h is a correct place to explain about the hook,
although I added comments for the hook.
I think we should add a series of explanation about ESP hooks in the internal
section of the documentation, when the number of hooks reaches a dozen for
example.
Thanks,
--
KaiGai Kohei <kai...@ak.jp.nec.com>
*** a/src/backend/catalog/aclchk.c
--- b/src/backend/catalog/aclchk.c
***************
*** 135,140 **** static AclMode pg_aclmask(AclObjectKind objkind, Oid table_oid, AttrNumber attnu
--- 135,144 ----
static AclResult check_rte_privileges(RangeTblEntry *rte, bool error_on_violation);
+ /*
+ * External security provider hooks
+ */
+ check_relation_privileges_hook_type check_relation_privileges_hook = NULL;
#ifdef ACLDEBUG
static void
***************
*** 4730,4735 **** get_user_default_acl(GrantObjectType objtype, Oid ownerId, Oid nsp_oid)
--- 4734,4742 ----
* It checks access permissions for all relations listed in a range table.
* If violated, it raises an error or returns false depending on the 'abort'
* argument.
+ * It also invokes an external security provide to check the permissions.
+ * If it is available, both of the default PG checks and external checks
+ * have to allow the required accesses for the relations.
*/
AclResult
check_relation_privileges(List *rangeTable, bool error_on_violation)
***************
*** 4746,4751 **** check_relation_privileges(List *rangeTable, bool error_on_violation)
--- 4753,4763 ----
if (retval != ACLCHECK_OK)
return retval;
}
+
+ /* External security provider invocation */
+ if (check_relation_privileges_hook)
+ retval = (*check_relation_privileges_hook)(rangeTable,
+ error_on_violation);
return retval;
}
*** a/src/backend/executor/execMain.c
--- b/src/backend/executor/execMain.c
***************
*** 466,471 **** InitPlan(QueryDesc *queryDesc, int eflags)
--- 466,475 ----
* check anything for other RTE types (function, join, subquery, ...).
* Function RTEs are checked by init_fcache when the function is prepared
* for execution. Join, subquery, and special RTEs need no checks.
+ *
+ * If available, it also invokes an external security provider. In this
+ * case, both of the default PG checks and the external checks have to
+ * allow the required accesses on the relation
*/
check_relation_privileges(rangeTable, true);
*** a/src/include/utils/acl.h
--- b/src/include/utils/acl.h
***************
*** 196,201 **** typedef enum AclObjectKind
--- 196,216 ----
MAX_ACL_KIND /* MUST BE LAST */
} AclObjectKind;
+ /*
+ * External security provider hooks
+ */
+
+ /*
+ * check_relation_privileges_hook
+ * It allows an ESP to get control at check_relation_privileges().
+ * A list of RangeTblEntry to be referenced and a flag to inform preferable
+ * bahavior on access violations.
+ * The ESP shall return ACLCHECK_OK if it allows the required access.
+ * Elsewhere, it raises an error or return other AclResult status depending
+ * on the given flag.
+ */
+ typedef AclResult (*check_relation_privileges_hook_type)(List *, bool);
+ extern PGDLLIMPORT check_relation_privileges_hook_type check_relation_privileges_hook;
/*
* routines used internally
*** a/src/backend/nodes/makefuncs.c
--- b/src/backend/nodes/makefuncs.c
***************
*** 262,267 **** makeRelabelType(Expr *arg, Oid rtype, int32 rtypmod, CoercionForm rformat)
--- 262,281 ----
}
/*
+ * makeRangeTblEntry
+ * creates a RangeTblEntry node
+ */
+ RangeTblEntry *
+ makeRangeTblEntry(RTEKind rtekind)
+ {
+ RangeTblEntry *r = makeNode(RangeTblEntry);
+
+ r->rtekind = rtekind;
+
+ return r;
+ }
+
+ /*
* makeRangeVar -
* creates a RangeVar node (rather oversimplified case)
*/
*** a/src/backend/parser/parse_relation.c
--- b/src/backend/parser/parse_relation.c
***************
*** 877,888 **** addRangeTableEntry(ParseState *pstate,
bool inh,
bool inFromCl)
{
! RangeTblEntry *rte = makeNode(RangeTblEntry);
char *refname = alias ? alias->aliasname : relation->relname;
LOCKMODE lockmode;
Relation rel;
- rte->rtekind = RTE_RELATION;
rte->alias = alias;
/*
--- 877,887 ----
bool inh,
bool inFromCl)
{
! RangeTblEntry *rte = makeRangeTblEntry(RTE_RELATION);
char *refname = alias ? alias->aliasname : relation->relname;
LOCKMODE lockmode;
Relation rel;
rte->alias = alias;
/*
***************
*** 950,959 **** addRangeTableEntryForRelation(ParseState *pstate,
bool inh,
bool inFromCl)
{
! RangeTblEntry *rte = makeNode(RangeTblEntry);
char *refname = alias ? alias->aliasname : RelationGetRelationName(rel);
- rte->rtekind = RTE_RELATION;
rte->alias = alias;
rte->relid = RelationGetRelid(rel);
--- 949,957 ----
bool inh,
bool inFromCl)
{
! RangeTblEntry *rte = makeRangeTblEntry(RTE_RELATION);
char *refname = alias ? alias->aliasname : RelationGetRelationName(rel);
rte->alias = alias;
rte->relid = RelationGetRelid(rel);
***************
*** 1004,1017 **** addRangeTableEntryForSubquery(ParseState *pstate,
Alias *alias,
bool inFromCl)
{
! RangeTblEntry *rte = makeNode(RangeTblEntry);
char *refname = alias->aliasname;
Alias *eref;
int numaliases;
int varattno;
ListCell *tlistitem;
- rte->rtekind = RTE_SUBQUERY;
rte->relid = InvalidOid;
rte->subquery = subquery;
rte->alias = alias;
--- 1002,1014 ----
Alias *alias,
bool inFromCl)
{
! RangeTblEntry *rte = makeRangeTblEntry(RTE_SUBQUERY);
char *refname = alias->aliasname;
Alias *eref;
int numaliases;
int varattno;
ListCell *tlistitem;
rte->relid = InvalidOid;
rte->subquery = subquery;
rte->alias = alias;
***************
*** 1084,1090 **** addRangeTableEntryForFunction(ParseState *pstate,
RangeFunction *rangefunc,
bool inFromCl)
{
! RangeTblEntry *rte = makeNode(RangeTblEntry);
TypeFuncClass functypclass;
Oid funcrettype;
TupleDesc tupdesc;
--- 1081,1087 ----
RangeFunction *rangefunc,
bool inFromCl)
{
! RangeTblEntry *rte = makeRangeTblEntry(RTE_FUNCTION);
TypeFuncClass functypclass;
Oid funcrettype;
TupleDesc tupdesc;
***************
*** 1092,1098 **** addRangeTableEntryForFunction(ParseState *pstate,
List *coldeflist = rangefunc->coldeflist;
Alias *eref;
- rte->rtekind = RTE_FUNCTION;
rte->relid = InvalidOid;
rte->subquery = NULL;
rte->funcexpr = funcexpr;
--- 1089,1094 ----
***************
*** 1217,1229 **** addRangeTableEntryForValues(ParseState *pstate,
Alias *alias,
bool inFromCl)
{
! RangeTblEntry *rte = makeNode(RangeTblEntry);
char *refname = alias ? alias->aliasname : pstrdup("*VALUES*");
Alias *eref;
int numaliases;
int numcolumns;
- rte->rtekind = RTE_VALUES;
rte->relid = InvalidOid;
rte->subquery = NULL;
rte->values_lists = exprs;
--- 1213,1224 ----
Alias *alias,
bool inFromCl)
{
! RangeTblEntry *rte = makeRangeTblEntry(RTE_VALUES);
char *refname = alias ? alias->aliasname : pstrdup("*VALUES*");
Alias *eref;
int numaliases;
int numcolumns;
rte->relid = InvalidOid;
rte->subquery = NULL;
rte->values_lists = exprs;
***************
*** 1291,1297 **** addRangeTableEntryForJoin(ParseState *pstate,
Alias *alias,
bool inFromCl)
{
! RangeTblEntry *rte = makeNode(RangeTblEntry);
Alias *eref;
int numaliases;
--- 1286,1292 ----
Alias *alias,
bool inFromCl)
{
! RangeTblEntry *rte = makeRangeTblEntry(RTE_JOIN);
Alias *eref;
int numaliases;
***************
*** 1305,1311 **** addRangeTableEntryForJoin(ParseState *pstate,
errmsg("joins can have at most %d columns",
MaxAttrNumber)));
- rte->rtekind = RTE_JOIN;
rte->relid = InvalidOid;
rte->subquery = NULL;
rte->jointype = jointype;
--- 1300,1305 ----
***************
*** 1361,1374 **** addRangeTableEntryForCTE(ParseState *pstate,
Alias *alias,
bool inFromCl)
{
! RangeTblEntry *rte = makeNode(RangeTblEntry);
char *refname = alias ? alias->aliasname : cte->ctename;
Alias *eref;
int numaliases;
int varattno;
ListCell *lc;
- rte->rtekind = RTE_CTE;
rte->ctename = cte->ctename;
rte->ctelevelsup = levelsup;
--- 1355,1367 ----
Alias *alias,
bool inFromCl)
{
! RangeTblEntry *rte = makeRangeTblEntry(RTE_CTE);
char *refname = alias ? alias->aliasname : cte->ctename;
Alias *eref;
int numaliases;
int varattno;
ListCell *lc;
rte->ctename = cte->ctename;
rte->ctelevelsup = levelsup;
*** a/src/include/nodes/makefuncs.h
--- b/src/include/nodes/makefuncs.h
***************
*** 56,61 **** extern Alias *makeAlias(const char *aliasname, List *colnames);
--- 56,62 ----
extern RelabelType *makeRelabelType(Expr *arg, Oid rtype, int32 rtypmod,
CoercionForm rformat);
+ extern RangeTblEntry *makeRangeTblEntry(RTEKind rtekind);
extern RangeVar *makeRangeVar(char *schemaname, char *relname, int location);
extern TypeName *makeTypeName(char *typnam);
*** a/src/backend/catalog/aclchk.c
--- b/src/backend/catalog/aclchk.c
***************
*** 133,138 **** static AclMode restrict_and_check_grant(bool is_grant, AclMode avail_goptions,
--- 133,140 ----
static AclMode pg_aclmask(AclObjectKind objkind, Oid table_oid, AttrNumber attnum,
Oid roleid, AclMode mask, AclMaskHow how);
+ static AclResult check_rte_privileges(RangeTblEntry *rte, bool error_on_violation);
+
#ifdef ACLDEBUG
static void
***************
*** 4722,4724 **** get_user_default_acl(GrantObjectType objtype, Oid ownerId, Oid nsp_oid)
--- 4724,4923 ----
return result;
}
+
+ /*
+ * check_relation_privileges
+ * It checks access permissions for all relations listed in a range table.
+ * If violated, it raises an error or returns false depending on the 'abort'
+ * argument.
+ */
+ AclResult
+ check_relation_privileges(List *rangeTable, bool error_on_violation)
+ {
+ ListCell *l;
+ RangeTblEntry *rte;
+ AclResult retval = ACLCHECK_OK;
+
+ foreach(l, rangeTable)
+ {
+ rte = (RangeTblEntry *) lfirst(l);
+
+ retval = check_rte_privileges(rte, error_on_violation);
+ if (retval != ACLCHECK_OK)
+ return retval;
+ }
+ return retval;
+ }
+
+ /*
+ * check_rte_privileges
+ * This is only used for checking plain relation permissions, by
+ * check_relation_privileges(), nothing else shall be checked here.
+ */
+ static AclResult
+ check_rte_privileges(RangeTblEntry *rte, bool error_on_violation)
+ {
+ AclMode requiredPerms;
+ AclMode relPerms;
+ AclMode remainingPerms;
+ Oid relOid;
+ Oid userid;
+ Bitmapset *tmpset;
+ int col;
+
+ /*
+ * Only plain-relation RTEs need to be checked here.
+ */
+ if (rte->rtekind != RTE_RELATION)
+ return ACLCHECK_OK;
+
+ /*
+ * No work if requiredPerms is empty.
+ */
+ requiredPerms = rte->requiredPerms;
+ if (requiredPerms == 0)
+ return ACLCHECK_OK;
+
+ relOid = rte->relid;
+
+ /*
+ * userid to check as: current user unless we have a setuid indication.
+ *
+ * Note: GetUserId() is presently fast enough that there's no harm in
+ * calling it separately for each RTE. If that stops being true, we could
+ * call it once in chkpriv_relation_perms and pass the userid down from
+ * there. But for now, no need for the extra clutter.
+ */
+ userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+
+ /*
+ * We must have *all* the requiredPerms bits, but some of the bits can be
+ * satisfied from column-level rather than relation-level permissions.
+ * First, remove any bits that are satisfied by relation permissions.
+ */
+ relPerms = pg_class_aclmask(relOid, userid, requiredPerms, ACLMASK_ALL);
+ remainingPerms = requiredPerms & ~relPerms;
+ if (remainingPerms != 0)
+ {
+ /*
+ * If we lack any permissions that exist only as relation permissions,
+ * we can fail straight away.
+ */
+ if (remainingPerms & ~(ACL_SELECT | ACL_INSERT | ACL_UPDATE))
+ {
+ if (error_on_violation)
+ aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
+ get_rel_name(relOid));
+ return ACLCHECK_NO_PRIV;
+ }
+
+ /*
+ * Check to see if we have the needed privileges at column level.
+ *
+ * Note: failures just report a table-level error; it would be nicer
+ * to report a column-level error if we have some but not all of the
+ * column privileges.
+ */
+ if (remainingPerms & ACL_SELECT)
+ {
+ /*
+ * When the query doesn't explicitly reference any columns (for
+ * example, SELECT COUNT(*) FROM table), allow the query if we
+ * have SELECT on any column of the rel, as per SQL spec.
+ */
+ if (bms_is_empty(rte->selectedCols))
+ {
+ if (pg_attribute_aclcheck_all(relOid, userid, ACL_SELECT,
+ ACLMASK_ANY) != ACLCHECK_OK)
+ {
+ if (error_on_violation)
+ aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
+ get_rel_name(relOid));
+ return ACLCHECK_NO_PRIV;
+ }
+ }
+
+ tmpset = bms_copy(rte->selectedCols);
+ while ((col = bms_first_member(tmpset)) >= 0)
+ {
+ /* remove the column number offset */
+ col += FirstLowInvalidHeapAttributeNumber;
+ if (col == InvalidAttrNumber)
+ {
+ /* Whole-row reference, must have priv on all cols */
+ if (pg_attribute_aclcheck_all(relOid, userid, ACL_SELECT,
+ ACLMASK_ALL) != ACLCHECK_OK)
+ {
+ if (error_on_violation)
+ aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
+ get_rel_name(relOid));
+ return ACLCHECK_NO_PRIV;
+ }
+ }
+ else
+ {
+ if (pg_attribute_aclcheck(relOid, col, userid,
+ ACL_SELECT) != ACLCHECK_OK)
+ {
+ if (error_on_violation)
+ aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
+ get_rel_name(relOid));
+ return ACLCHECK_NO_PRIV;
+ }
+ }
+ }
+ bms_free(tmpset);
+ }
+
+ /*
+ * Basically the same for the mod columns, with either INSERT or
+ * UPDATE privilege as specified by remainingPerms.
+ */
+ remainingPerms &= ~ACL_SELECT;
+ if (remainingPerms != 0)
+ {
+ /*
+ * When the query doesn't explicitly change any columns, allow the
+ * query if we have permission on any column of the rel. This is
+ * to handle SELECT FOR UPDATE as well as possible corner cases in
+ * INSERT and UPDATE.
+ */
+ if (bms_is_empty(rte->modifiedCols))
+ {
+ if (pg_attribute_aclcheck_all(relOid, userid, remainingPerms,
+ ACLMASK_ANY) != ACLCHECK_OK)
+ {
+ if (error_on_violation)
+ aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
+ get_rel_name(relOid));
+ return ACLCHECK_NO_PRIV;
+ }
+ }
+
+ tmpset = bms_copy(rte->modifiedCols);
+ while ((col = bms_first_member(tmpset)) >= 0)
+ {
+ /* remove the column number offset */
+ col += FirstLowInvalidHeapAttributeNumber;
+ if (col == InvalidAttrNumber)
+ {
+ /* whole-row reference can't happen here */
+ elog(ERROR, "whole-row update is not implemented");
+ }
+ else
+ {
+ if (pg_attribute_aclcheck(relOid, col, userid,
+ remainingPerms) != ACLCHECK_OK)
+ {
+ if (error_on_violation)
+ aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
+ get_rel_name(relOid));
+ return ACLCHECK_NO_PRIV;
+ }
+ }
+ }
+ bms_free(tmpset);
+ }
+ }
+ return ACLCHECK_OK;
+ }
*** a/src/backend/commands/copy.c
--- b/src/backend/commands/copy.c
***************
*** 21,26 ****
--- 21,27 ----
#include <arpa/inet.h>
#include "access/heapam.h"
+ #include "access/sysattr.h"
#include "access/xact.h"
#include "catalog/namespace.h"
#include "catalog/pg_type.h"
***************
*** 726,733 **** 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;
--- 727,732 ----
***************
*** 988,993 **** DoCopy(const CopyStmt *stmt, const char *queryString)
--- 987,996 ----
if (stmt->relation)
{
+ RangeTblEntry *rte;
+ List *attnums;
+ ListCell *cur;
+
Assert(!stmt->query);
cstate->queryDesc = NULL;
***************
*** 997,1025 **** 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;
! attnums = CopyGetAttnums(tupDesc, cstate->rel, attnamelist);
! foreach(cur, attnums)
! {
! int attnum = lfirst_int(cur);
! if (pg_attribute_aclcheck(RelationGetRelid(cstate->rel),
! attnum,
! GetUserId(),
! remainingPerms) != ACLCHECK_OK)
! aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
! RelationGetRelationName(cstate->rel));
! }
}
/* check read-only transaction */
if (XactReadOnly && is_from && !cstate->rel->rd_islocaltemp)
--- 1000,1025 ----
tupDesc = RelationGetDescr(cstate->rel);
! /*
! * Check relation permissions.
! * We built an RTE with the relation and columns to be accessed
! * to check for necessary privileges in the common way.
! */
! rte = makeRangeTblEntry(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) - FirstLowInvalidHeapAttributeNumber;
! if (is_from)
! rte->modifiedCols = bms_add_member(rte->modifiedCols, attnum);
! else
! rte->selectedCols = bms_add_member(rte->selectedCols, attnum);
}
+ check_relation_privileges(list_make1(rte), true);
/* check read-only transaction */
if (XactReadOnly && is_from && !cstate->rel->rd_islocaltemp)
*** a/src/backend/executor/execMain.c
--- b/src/backend/executor/execMain.c
***************
*** 72,79 **** static void ExecutePlan(EState *estate, PlanState *planstate,
long numberTuples,
ScanDirection direction,
DestReceiver *dest);
- static void ExecCheckRTPerms(List *rangeTable);
- static void ExecCheckRTEPerms(RangeTblEntry *rte);
static void ExecCheckXactReadOnly(PlannedStmt *plannedstmt);
static void EvalPlanQualStart(EPQState *epqstate, EState *parentestate,
Plan *planTree);
--- 72,77 ----
***************
*** 402,572 **** ExecutorRewind(QueryDesc *queryDesc)
MemoryContextSwitchTo(oldcontext);
}
-
- /*
- * ExecCheckRTPerms
- * Check access permissions for all relations listed in a range table.
- */
- static void
- ExecCheckRTPerms(List *rangeTable)
- {
- ListCell *l;
-
- foreach(l, rangeTable)
- {
- ExecCheckRTEPerms((RangeTblEntry *) lfirst(l));
- }
- }
-
- /*
- * ExecCheckRTEPerms
- * Check access permissions for a single RTE.
- */
- static void
- ExecCheckRTEPerms(RangeTblEntry *rte)
- {
- AclMode requiredPerms;
- AclMode relPerms;
- AclMode remainingPerms;
- Oid relOid;
- Oid userid;
- Bitmapset *tmpset;
- int col;
-
- /*
- * Only plain-relation RTEs need to be checked here. Function RTEs are
- * checked by init_fcache when the function is prepared for execution.
- * Join, subquery, and special RTEs need no checks.
- */
- if (rte->rtekind != RTE_RELATION)
- return;
-
- /*
- * No work if requiredPerms is empty.
- */
- requiredPerms = rte->requiredPerms;
- if (requiredPerms == 0)
- return;
-
- relOid = rte->relid;
-
- /*
- * userid to check as: current user unless we have a setuid indication.
- *
- * Note: GetUserId() is presently fast enough that there's no harm in
- * calling it separately for each RTE. If that stops being true, we could
- * call it once in ExecCheckRTPerms and pass the userid down from there.
- * But for now, no need for the extra clutter.
- */
- userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
-
- /*
- * We must have *all* the requiredPerms bits, but some of the bits can be
- * satisfied from column-level rather than relation-level permissions.
- * First, remove any bits that are satisfied by relation permissions.
- */
- relPerms = pg_class_aclmask(relOid, userid, requiredPerms, ACLMASK_ALL);
- remainingPerms = requiredPerms & ~relPerms;
- if (remainingPerms != 0)
- {
- /*
- * If we lack any permissions that exist only as relation permissions,
- * 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));
-
- /*
- * Check to see if we have the needed privileges at column level.
- *
- * Note: failures just report a table-level error; it would be nicer
- * to report a column-level error if we have some but not all of the
- * column privileges.
- */
- if (remainingPerms & ACL_SELECT)
- {
- /*
- * When the query doesn't explicitly reference any columns (for
- * example, SELECT COUNT(*) FROM table), allow the query if we
- * have SELECT on any column of the rel, as per SQL spec.
- */
- if (bms_is_empty(rte->selectedCols))
- {
- 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));
- }
-
- tmpset = bms_copy(rte->selectedCols);
- while ((col = bms_first_member(tmpset)) >= 0)
- {
- /* remove the column number offset */
- col += FirstLowInvalidHeapAttributeNumber;
- if (col == InvalidAttrNumber)
- {
- /* 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));
- }
- else
- {
- if (pg_attribute_aclcheck(relOid, col, userid, ACL_SELECT)
- != ACLCHECK_OK)
- aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
- get_rel_name(relOid));
- }
- }
- bms_free(tmpset);
- }
-
- /*
- * Basically the same for the mod columns, with either INSERT or
- * UPDATE privilege as specified by remainingPerms.
- */
- remainingPerms &= ~ACL_SELECT;
- if (remainingPerms != 0)
- {
- /*
- * When the query doesn't explicitly change any columns, allow the
- * query if we have permission on any column of the rel. This is
- * to handle SELECT FOR UPDATE as well as possible corner cases in
- * INSERT and UPDATE.
- */
- if (bms_is_empty(rte->modifiedCols))
- {
- if (pg_attribute_aclcheck_all(relOid, userid, remainingPerms,
- ACLMASK_ANY) != ACLCHECK_OK)
- aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
- get_rel_name(relOid));
- }
-
- tmpset = bms_copy(rte->modifiedCols);
- while ((col = bms_first_member(tmpset)) >= 0)
- {
- /* remove the column number offset */
- col += FirstLowInvalidHeapAttributeNumber;
- if (col == InvalidAttrNumber)
- {
- /* whole-row reference can't happen here */
- elog(ERROR, "whole-row update is not implemented");
- }
- else
- {
- if (pg_attribute_aclcheck(relOid, col, userid, remainingPerms)
- != ACLCHECK_OK)
- aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
- get_rel_name(relOid));
- }
- }
- bms_free(tmpset);
- }
- }
- }
-
/*
* Check that the query does not imply any writes to non-temp tables.
*
--- 400,405 ----
***************
*** 628,636 **** InitPlan(QueryDesc *queryDesc, int eflags)
int i;
/*
! * Do permissions checks
*/
! ExecCheckRTPerms(rangeTable);
/*
* initialize the node's execution state
--- 461,473 ----
int i;
/*
! * Do permissions checks. The check_relation_privileges() checks access
! * permissions for all relations listed in a range table, but does not
! * check anything for other RTE types (function, join, subquery, ...).
! * Function RTEs are checked by init_fcache when the function is prepared
! * for execution. Join, subquery, and special RTEs need no checks.
*/
! check_relation_privileges(rangeTable, true);
/*
* initialize the node's execution state
*** a/src/backend/utils/adt/ri_triggers.c
--- b/src/backend/utils/adt/ri_triggers.c
***************
*** 30,35 ****
--- 30,36 ----
#include "postgres.h"
+ #include "access/sysattr.h"
#include "access/xact.h"
#include "catalog/pg_constraint.h"
#include "catalog/pg_operator.h"
***************
*** 39,44 ****
--- 40,46 ----
#include "parser/parse_coerce.h"
#include "parser/parse_relation.h"
#include "miscadmin.h"
+ #include "nodes/makefuncs.h"
#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
***************
*** 2624,2629 **** RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
--- 2626,2633 ----
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;
***************
*** 2632,2649 **** RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
SPIPlanPtr qplan;
/*
* 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
* works because it changes user IDs on the fly.)
*
* 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;
! ri_FetchConstraintInfo(&riinfo, trigger, fk_rel, false);
/*----------
* The query string built is:
--- 2636,2678 ----
SPIPlanPtr qplan;
/*
+ * Set up RI_ConstraintInfo
+ */
+ 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
* works because it changes user IDs on the fly.)
*
* XXX are there any other show-stopper conditions to check?
*/
! /*
! * We built a pair of RTEs of FK/PK relations and columns referenced
! * in the test query to check necessary permission in the common way.
! */
! pkrte = makeRangeTblEntry(RTE_RELATION);
! pkrte->relid = RelationGetRelid(pk_rel);
! pkrte->requiredPerms = ACL_SELECT;
!
! fkrte = makeRangeTblEntry(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 (check_relation_privileges(list_make2(fkrte, pkrte), false) != ACLCHECK_OK)
! return false;
/*----------
* The query string built is:
*** a/src/include/utils/acl.h
--- b/src/include/utils/acl.h
***************
*** 311,314 **** extern bool pg_ts_dict_ownercheck(Oid dict_oid, Oid roleid);
--- 311,319 ----
extern bool pg_ts_config_ownercheck(Oid cfg_oid, Oid roleid);
extern bool pg_foreign_server_ownercheck(Oid srv_oid, Oid roleid);
+ /*
+ * security checker functions
+ */
+ extern AclResult check_relation_privileges(List *rangeTable, bool abort);
+
#endif /* ACL_H */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
