E57A Centralize DML permissions-checking logic. · postgres/postgres@b8c6c71 · GitHub
[go: up one dir, main page]
Skip to content

Commit b8c6c71

Browse files
robertmhaasrobertmhaas
committed
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.
1 parent 9f8cf32 commit b8c6c71

File tree

4 files changed

+89
-57
lines changed

4 files changed

+89
-57
lines changed

‎src/backend/commands/copy.c‎

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/copy.c,v 1.327 2010/04/28 16:10:41 heikki Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/copy.c,v 1.328 2010/07/22 00:47:52 rhaas Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -21,6 +21,7 @@
2121
#include <arpa/inet.h>
2222

2323
#include "access/heapam.h"
24+
#include "access/sysattr.h"
2425
#include "access/xact.h"
2526
#include "catalog/namespace.h"
2627
#include "catalog/pg_type.h"
@@ -726,8 +727,6 @@ DoCopy(const CopyStmt *stmt, const char *queryString)
726727
bool force_quote_all = false;
727728
bool format_specified = false;
728729
AclMode required_access = (is_from ? ACL_INSERT : ACL_SELECT);
729-
AclMode relPerms;
730-
AclMode remainingPerms;
731730
ListCell *option;
732731
TupleDesc tupDesc;
733732
int num_phys_attrs;
@@ -988,6 +987,10 @@ DoCopy(const CopyStmt *stmt, const char *queryString)
988987

989988
if (stmt->relation)
990989
{
990+
RangeTblEntry *rte;
991+
List *attnums;
992+
ListCell *cur;
993+
991994
Assert(!stmt->query);
992995
cstate->queryDesc = NULL;
993996

@@ -998,28 +1001,22 @@ DoCopy(const CopyStmt *stmt, const char *queryString)
9981001
tupDesc = RelationGetDescr(cstate->rel);
9991002

10001003
/* Check relation permissions. */
1001-
relPerms = pg_class_aclmask(RelationGetRelid(cstate->rel), GetUserId(),
1002-
required_access, ACLMASK_ALL);
1003-
B018 remainingPerms = required_access & ~relPerms;
1004-
if (remainingPerms != 0)
1005-
{
1006-
/* We don't have table permissions, check per-column permissions */
1007-
List *attnums;
1008-
ListCell *cur;
1004+
rte = makeNode(RangeTblEntry);
1005+
rte->rtekind = RTE_RELATION;
1006+
rte->relid = RelationGetRelid(cstate->rel);
1007+
rte->requiredPerms = required_access;
10091008

1010-
attnums = CopyGetAttnums(tupDesc, cstate->rel, attnamelist);
1011-
foreach(cur, attnums)
1012-
{
1013-
int attnum = lfirst_int(cur);
1009+
attnums = CopyGetAttnums(tupDesc, cstate->rel, attnamelist);
1010+
foreach (cur, attnums)
1011+
{
1012+
int attno = lfirst_int(cur) - FirstLowInvalidHeapAttributeNumber;
10141013

1015-
if (pg_attribute_aclcheck(RelationGetRelid(cstate->rel),
1016-
attnum,
1017-
GetUserId(),
1018-
remainingPerms) != ACLCHECK_OK)
1019-
aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
1020-
RelationGetRelationName(cstate->rel));
1021-
}
1014+
if (is_from)
1015+
rte->modifiedCols = bms_add_member(rte->modifiedCols, attno);
1016+
else
1017+
rte->selectedCols = bms_add_member(rte->selectedCols, attno);
10221018
}
1019+
ExecCheckRTPerms(list_make1(rte), true);
10231020

10241021
/* check read-only transaction */
10251022
if (XactReadOnly && is_from && !cstate->rel->rd_islocaltemp)

‎src/backend/executor/execMain.c‎

Lines changed: 37 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
*
2727
*
2828
* IDENTIFICATION
29-
* $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.351 2010/07/12 17:01:05 tgl Exp $
29+
* $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.352 2010/07/22 00:47:52 rhaas Exp $
3030
*
3131
*-------------------------------------------------------------------------
3232
*/
@@ -75,8 +75,7 @@ static void ExecutePlan(EState *estate, PlanState *planstate,
7575
long numberTuples,
7676
ScanDirection direction,
7777
DestReceiver *dest);
78-
static void ExecCheckRTPerms(List *rangeTable);
79-
static void ExecCheckRTEPerms(RangeTblEntry *rte);
78+
static bool ExecCheckRTEPerms(RangeTblEntry *rte);
8079
static void ExecCheckXactReadOnly(PlannedStmt *plannedstmt);
8180
static void EvalPlanQualStart(EPQState *epqstate, EState *parentestate,
8281
Plan *planTree);
@@ -409,26 +408,42 @@ ExecutorRewind(QueryDesc *queryDesc)
409408
/*
410409
* ExecCheckRTPerms
411410
* Check access permissions for all relations listed in a range table.
411+
*
412+
* Returns true if permissions are adequate. Otherwise, throws an appropriate
413+
* error if ereport_on_violation is true, or simply returns false otherwise.
412414
*/
413-
static void
414-
ExecCheckRTPerms(List *rangeTable)
415+
bool
416+
ExecCheckRTPerms(List *rangeTable, bool ereport_on_violation)
415417
{
416418
ListCell *l;
419+
bool result = true;
417420

418421
foreach(l, rangeTable)
419422
{
420-
ExecCheckRTEPerms((RangeTblEntry *) lfirst(l));
423+
RangeTblEntry *rte = (RangeTblEntry *) lfirst(l);
424+
425+
result = ExecCheckRTEPerms(rte);
426+
if (!result)
427+
{
428+
Assert(rte->rtekind == RTE_RELATION);
429+
if (ereport_on_violation)
430+
aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
431+
get_rel_name(rte->relid));
432+
return false;
433+
}
421434
}
422435

423436
if (ExecutorCheckPerms_hook)
424-
(*ExecutorCheckPerms_hook)(rangeTable);
437+
result = (*ExecutorCheckPerms_hook)(rangeTable,
438+
ereport_on_violation);
439+
return result;
425440
}
426441

427442
/*
428443
* ExecCheckRTEPerms
429444
* Check access permissions for a single RTE.
430445
*/
431-
static void
446+
static bool
432447
ExecCheckRTEPerms(RangeTblEntry *rte)
433448
{
434449
AclMode requiredPerms;
@@ -445,14 +460,14 @@ ExecCheckRTEPerms(RangeTblEntry *rte)
445460
* Join, subquery, and special RTEs need no checks.
446461
*/
447462
if (rte->rtekind != RTE_RELATION)
448-
return;
463+
return true;
449464

450465
/*
451466
* No work if requiredPerms is empty.
452467
*/
453468
requiredPerms = rte->requiredPerms;
454469
if (requiredPerms == 0)
455-
return;
470+
return true;
456471

457472
relOid = rte->relid;
458473

@@ -480,8 +495,7 @@ ExecCheckRTEPerms(RangeTblEntry *rte)
480495
* we can fail straight away.
481496
*/
482497
if (remainingPerms & ~(ACL_SELECT | ACL_INSERT | ACL_UPDATE))
483-
aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
484-
get_rel_name(relOid));
498+
return false;
485499

486500
/*
487501
* Check to see if we have the needed privileges at column level.
@@ -501,8 +515,7 @@ ExecCheckRTEPerms(RangeTblEntry *rte)
501515
{
502516
if (pg_attribute_aclcheck_all(relOid, userid, ACL_SELECT,
503517
ACLMASK_ANY) != ACLCHECK_OK)
504-
aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
505-
get_rel_name(relOid));
518+
return false;
506519
}
507520

508521
tmpset = bms_copy(rte->selectedCols);
@@ -515,15 +528,13 @@ ExecCheckRTEPerms(RangeTblEntry *rte)
515528
/* Whole-row reference, must have priv on all cols */
516529
if (pg_attribute_aclcheck_all(relOid, userid, ACL_SELECT,
517530
ACLMASK_ALL) != ACLCHECK_OK)
518-
aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
519-
get_rel_name(relOid));
531+
return false;
520532
}
521533
else
522534
{
523-
if (pg_attribute_aclcheck(relOid, col, userid, ACL_SELECT)
524-
!= ACLCHECK_OK)
525-
aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
526-
get_rel_name(relOid));
535+
if (pg_attribute_aclcheck(relOid, col, userid,
536+
ACL_SELECT) != ACLCHECK_OK)
537+
return false;
527538
}
528539
}
529540
bms_free(tmpset);
@@ -546,8 +557,7 @@ ExecCheckRTEPerms(RangeTblEntry *rte)
546557
{
547558
if (pg_attribute_aclcheck_all(relOid, userid, remainingPerms,
548559
ACLMASK_ANY) != ACLCHECK_OK)
549-
aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
550-
get_rel_name(relOid));
560+
return false;
551561
}
552562

553563
tmpset = bms_copy(rte->modifiedCols);
@@ -562,15 +572,15 @@ ExecCheckRTEPerms(RangeTblEntry *rte)
562572
}
563573
else
564574
{
565-
if (pg_attribute_aclcheck(relOid, col, userid, remainingPerms)
566-
!= ACLCHECK_OK)
567-
aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
568-
get_rel_name(relOid));
575+
if (pg_attribute_aclcheck(relOid, col, userid,
576+
remainingPerms) != ACLCHECK_OK)
577+
return false;
569578
}
570579
}
571580
bms_free(tmpset);
572581
}
573582
}
583+
return true;
574584
}
575585

576586
/*
@@ -636,7 +646,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
636646
/*
637647
* Do permissions checks
638648
*/
639-
ExecCheckRTPerms(rangeTable);
649+
ExecCheckRTPerms(rangeTable, true);
640650

641651
/*
642652
* initialize the node's execution state

‎src/backend/utils/adt/ri_triggers.c‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*
1616
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
1717
*
18-
* $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.118 2010/02/14 18:42:16 rhaas Exp $
18+
* $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.119 2010/07/22 00:47:52 rhaas Exp $
1919
*
2020
* ----------
2121
*/
@@ -31,10 +31,12 @@
3131
#include "postgres.h"
3232

3333
#include "access/xact.h"
34+
#include "access/sysattr.h"
3435
#include "catalog/pg_constraint.h"
3536
#include "catalog/pg_operator.h"
3637
#include "catalog/pg_type.h"
3738
#include "commands/trigger.h"
39+
#include "executor/executor.h"
3840
#include "executor/spi.h"
3941
#include "parser/parse_coerce.h"
4042
#include "parser/parse_relation.h"
@@ -2624,26 +2626,48 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
26242626
char fkrelname[MAX_QUOTED_REL_NAME_LEN];
26252627
char pkattname[MAX_QUOTED_NAME_LEN + 3];
26262628
char fkattname[MAX_QUOTED_NAME_LEN + 3];
2629+
RangeTblEntry *pkrte;
2630+
RangeTblEntry *fkrte;
26272631
const char *sep;
26282632
int i;
26292633
int old_work_mem;
26302634
char workmembuf[32];
26312635
int spi_result;
26322636
SPIPlanPtr qplan;
26332637

2638+
/* Fetch constraint info. */
2639+
ri_FetchConstraintInfo(&riinfo, trigger, fk_rel, false);
2640+
26342641
/*
26352642
* Check to make sure current user has enough permissions to do the test
26362643
* query. (If not, caller can fall back to the trigger method, which
26372644
* works because it changes user IDs on the fly.)
26382645
*
26392646
* XXX are there any other show-stopper conditions to check?
26402647
*/
2641-
if (pg_class_aclcheck(RelationGetRelid(fk_rel), GetUserId(), ACL_SELECT) != ACLCHECK_OK)
2642-
return false;
2643-
if (pg_class_aclcheck(RelationGetRelid(pk_rel), GetUserId(), ACL_SELECT) != ACLCHECK_OK)
2644-
return false;
2648+
pkrte = makeNode(RangeTblEntry);
2649+
pkrte->rtekind = RTE_RELATION;
2650+
pkrte->relid = RelationGetRelid(pk_rel);
2651+
pkrte->requiredPerms = ACL_SELECT;
26452652

2646-
ri_FetchConstraintInfo(&riinfo, trigger, fk_rel, false);
2653+
fkrte = makeNode(RangeTblEntry);
2654+
fkrte->rtekind = RTE_RELATION;
2655+
fkrte->relid = RelationGetRelid(fk_rel);
2656+
fkrte->requiredPerms = ACL_SELECT;
2657+
2658+
for (i = 0; i < riinfo.nkeys; i++)
2659+
{
2660+
int attno;
2661+
2662+
attno = riinfo.pk_attnums[i] - FirstLowInvalidHeapAttributeNumber;
2663+
pkrte->selectedCols = bms_add_member(pkrte->selectedCols, attno);
2664+
2665+
attno = riinfo.fk_attnums[i] - FirstLowInvalidHeapAttributeNumber;
2666+
fkrte->selectedCols = bms_add_member(fkrte->selectedCols, attno);
2667+
}
2668+
2669+
if (!ExecCheckRTPerms(list_make2(fkrte, pkrte), false))
2670+
return false;
26472671

26482672
/*----------
26492673
* The query string built is:

‎src/include/executor/executor.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.170 2010/07/12 17:01:06 tgl Exp $
10+
* $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.171 2010/07/22 00:47:59 rhaas Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -75,7 +75,7 @@ typedef void (*ExecutorEnd_hook_type) (QueryDesc *queryDesc);
7575
extern PGDLLIMPORT ExecutorEnd_hook_type ExecutorEnd_hook;
7676

7777
/* Hook for plugins to get control in ExecCheckRTPerms() */
78-
typedef void (*ExecutorCheckPerms_hook_type) (List *);
78+
typedef bool (*ExecutorCheckPerms_hook_type) (List *, bool);
7979
extern PGDLLIMPORT ExecutorCheckPerms_hook_type ExecutorCheckPerms_hook;
8080

8181

@@ -161,6 +161,7 @@ extern void standard_ExecutorRun(QueryDesc *queryDesc,
161161
extern void ExecutorEnd(QueryDesc *queryDesc);
162162
extern void standard_ExecutorEnd(QueryDesc *queryDesc);
163163
extern void ExecutorRewind(QueryDesc *queryDesc);
164+
extern bool ExecCheckRTPerms(List *rangeTable, bool ereport_on_violation);
164165
extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
165166
Relation resultRelationDesc,
166167
Index resultRelationIndex,

0 commit comments

Comments
 (0)
0