1
0
mirror of https://github.com/postgres/postgres.git synced 2025-06-14 18:42:34 +03:00

Rework query relation permission checking

Currently, information about the permissions to be checked on relations
mentioned in a query is stored in their range table entries.  So the
executor must scan the entire range table looking for relations that
need to have permissions checked.  This can make the permission checking
part of the executor initialization needlessly expensive when many
inheritance children are present in the range range.  While the
permissions need not be checked on the individual child relations, the
executor still must visit every range table entry to filter them out.

This commit moves the permission checking information out of the range
table entries into a new plan node called RTEPermissionInfo.  Every
top-level (inheritance "root") RTE_RELATION entry in the range table
gets one and a list of those is maintained alongside the range table.
This new list is initialized by the parser when initializing the range
table.  The rewriter can add more entries to it as rules/views are
expanded.  Finally, the planner combines the lists of the individual
subqueries into one flat list that is passed to the executor for
checking.

To make it quick to find the RTEPermissionInfo entry belonging to a
given relation, RangeTblEntry gets a new Index field 'perminfoindex'
that stores the corresponding RTEPermissionInfo's index in the query's
list of the latter.

ExecutorCheckPerms_hook has gained another List * argument; the
signature is now:
typedef bool (*ExecutorCheckPerms_hook_type) (List *rangeTable,
					      List *rtePermInfos,
					      bool ereport_on_violation);
The first argument is no longer used by any in-core uses of the hook,
but we leave it in place because there may be other implementations that
do.  Implementations should likely scan the rtePermInfos list to
determine which operations to allow or deny.

Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CA+HiwqGjJDmUhDSfv-U2qhKJjt9ST7Xh9JXC_irsAQ1TAUsJYg@mail.gmail.com
This commit is contained in:
Alvaro Herrera
2022-12-06 16:09:24 +01:00
parent b5bbaf08ed
commit a61b1f7482
47 changed files with 954 additions and 534 deletions

View File

@ -54,6 +54,7 @@
#include "jit/jit.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
#include "parser/parse_relation.h"
#include "parser/parsetree.h"
#include "storage/bufmgr.h"
#include "storage/lmgr.h"
@ -74,7 +75,7 @@ ExecutorRun_hook_type ExecutorRun_hook = NULL;
ExecutorFinish_hook_type ExecutorFinish_hook = NULL;
ExecutorEnd_hook_type ExecutorEnd_hook = NULL;
/* Hook for plugin to get control in ExecCheckRTPerms() */
/* Hook for plugin to get control in ExecCheckPermissions() */
ExecutorCheckPerms_hook_type ExecutorCheckPerms_hook = NULL;
/* decls for local routines only used within this module */
@ -90,10 +91,10 @@ static void ExecutePlan(EState *estate, PlanState *planstate,
ScanDirection direction,
DestReceiver *dest,
bool execute_once);
static bool ExecCheckRTEPerms(RangeTblEntry *rte);
static bool ExecCheckRTEPermsModified(Oid relOid, Oid userid,
Bitmapset *modifiedCols,
AclMode requiredPerms);
static bool ExecCheckOneRelPerms(RTEPermissionInfo *perminfo);
static bool ExecCheckPermissionsModified(Oid relOid, Oid userid,
Bitmapset *modifiedCols,
AclMode requiredPerms);
static void ExecCheckXactReadOnly(PlannedStmt *plannedstmt);
static char *ExecBuildSlotValueDescription(Oid reloid,
TupleTableSlot *slot,
@ -554,8 +555,8 @@ ExecutorRewind(QueryDesc *queryDesc)
/*
* ExecCheckRTPerms
* Check access permissions for all relations listed in a range table.
* ExecCheckPermissions
* Check access permissions of relations mentioned in a query
*
* Returns true if permissions are adequate. Otherwise, throws an appropriate
* error if ereport_on_violation is true, or simply returns false otherwise.
@ -565,73 +566,65 @@ ExecutorRewind(QueryDesc *queryDesc)
* passing, then RLS also needs to be consulted (and check_enable_rls()).
*
* See rewrite/rowsecurity.c.
*
* NB: rangeTable is no longer used by us, but kept around for the hooks that
* might still want to look at the RTEs.
*/
bool
ExecCheckRTPerms(List *rangeTable, bool ereport_on_violation)
ExecCheckPermissions(List *rangeTable, List *rteperminfos,
bool ereport_on_violation)
{
ListCell *l;
bool result = true;
foreach(l, rangeTable)
foreach(l, rteperminfos)
{
RangeTblEntry *rte = (RangeTblEntry *) lfirst(l);
RTEPermissionInfo *perminfo = lfirst_node(RTEPermissionInfo, l);
result = ExecCheckRTEPerms(rte);
Assert(OidIsValid(perminfo->relid));
result = ExecCheckOneRelPerms(perminfo);
if (!result)
{
Assert(rte->rtekind == RTE_RELATION);
if (ereport_on_violation)
aclcheck_error(ACLCHECK_NO_PRIV, get_relkind_objtype(get_rel_relkind(rte->relid)),
get_rel_name(rte->relid));
aclcheck_error(ACLCHECK_NO_PRIV,
get_relkind_objtype(get_rel_relkind(perminfo->relid)),
get_rel_name(perminfo->relid));
return false;
}
}
if (ExecutorCheckPerms_hook)
result = (*ExecutorCheckPerms_hook) (rangeTable,
result = (*ExecutorCheckPerms_hook) (rangeTable, rteperminfos,
ereport_on_violation);
return result;
}
/*
* ExecCheckRTEPerms
* Check access permissions for a single RTE.
* ExecCheckOneRelPerms
* Check access permissions for a single relation.
*/
static bool
ExecCheckRTEPerms(RangeTblEntry *rte)
ExecCheckOneRelPerms(RTEPermissionInfo *perminfo)
{
AclMode requiredPerms;
AclMode relPerms;
AclMode remainingPerms;
Oid relOid;
Oid userid;
Oid relOid = perminfo->relid;
/*
* Only plain-relation RTEs need to be checked here. Function RTEs are
* checked when the function is prepared for execution. Join, subquery,
* and special RTEs need no checks.
*/
if (rte->rtekind != RTE_RELATION)
return true;
/*
* No work if requiredPerms is empty.
*/
requiredPerms = rte->requiredPerms;
if (requiredPerms == 0)
return true;
relOid = rte->relid;
requiredPerms = perminfo->requiredPerms;
Assert(requiredPerms != 0);
/*
* 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.
* calling it separately for each relation. If that stops being true, we
* could call it once in ExecCheckPermissions and pass the userid down
* from there. But for now, no need for the extra clutter.
*/
userid = OidIsValid(rte->checkAsUser) ? rte->checkAsUser : GetUserId();
userid = OidIsValid(perminfo->checkAsUser) ?
perminfo->checkAsUser : GetUserId();
/*
* We must have *all* the requiredPerms bits, but some of the bits can be
@ -665,14 +658,14 @@ ExecCheckRTEPerms(RangeTblEntry *rte)
* 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 (bms_is_empty(perminfo->selectedCols))
{
if (pg_attribute_aclcheck_all(relOid, userid, ACL_SELECT,
ACLMASK_ANY) != ACLCHECK_OK)
return false;
}
while ((col = bms_next_member(rte->selectedCols, col)) >= 0)
while ((col = bms_next_member(perminfo->selectedCols, col)) >= 0)
{
/* bit #s are offset by FirstLowInvalidHeapAttributeNumber */
AttrNumber attno = col + FirstLowInvalidHeapAttributeNumber;
@ -697,29 +690,31 @@ ExecCheckRTEPerms(RangeTblEntry *rte)
* Basically the same for the mod columns, for both INSERT and UPDATE
* privilege as specified by remainingPerms.
*/
if (remainingPerms & ACL_INSERT && !ExecCheckRTEPermsModified(relOid,
userid,
rte->insertedCols,
ACL_INSERT))
if (remainingPerms & ACL_INSERT &&
!ExecCheckPermissionsModified(relOid,
userid,
perminfo->insertedCols,
ACL_INSERT))
return false;
if (remainingPerms & ACL_UPDATE && !ExecCheckRTEPermsModified(relOid,
userid,
rte->updatedCols,
ACL_UPDATE))
if (remainingPerms & ACL_UPDATE &&
!ExecCheckPermissionsModified(relOid,
userid,
perminfo->updatedCols,
ACL_UPDATE))
return false;
}
return true;
}
/*
* ExecCheckRTEPermsModified
* Check INSERT or UPDATE access permissions for a single RTE (these
* ExecCheckPermissionsModified
* Check INSERT or UPDATE access permissions for a single relation (these
* are processed uniformly).
*/
static bool
ExecCheckRTEPermsModified(Oid relOid, Oid userid, Bitmapset *modifiedCols,
AclMode requiredPerms)
ExecCheckPermissionsModified(Oid relOid, Oid userid, Bitmapset *modifiedCols,
AclMode requiredPerms)
{
int col = -1;
@ -773,17 +768,14 @@ ExecCheckXactReadOnly(PlannedStmt *plannedstmt)
* Fail if write permissions are requested in parallel mode for table
* (temp or non-temp), otherwise fail for any non-temp table.
*/
foreach(l, plannedstmt->rtable)
foreach(l, plannedstmt->permInfos)
{
RangeTblEntry *rte = (RangeTblEntry *) lfirst(l);
RTEPermissionInfo *perminfo = lfirst_node(RTEPermissionInfo, l);
if (rte->rtekind != RTE_RELATION)
if ((perminfo->requiredPerms & (~ACL_SELECT)) == 0)
continue;
if ((rte->requiredPerms & (~ACL_SELECT)) == 0)
continue;
if (isTempNamespace(get_rel_namespace(rte->relid)))
if (isTempNamespace(get_rel_namespace(perminfo->relid)))
continue;
PreventCommandIfReadOnly(CreateCommandName((Node *) plannedstmt));
@ -815,9 +807,10 @@ InitPlan(QueryDesc *queryDesc, int eflags)
int i;
/*
* Do permissions checks
* Do permissions checks and save the list for later use.
*/
ExecCheckRTPerms(rangeTable, true);
ExecCheckPermissions(rangeTable, plannedstmt->permInfos, true);
estate->es_rteperminfos = plannedstmt->permInfos;
/*
* initialize the node's execution state