Re: ExecRTCheckPerms() and many prunable partitions (checkAsUser)
Justin Pryzby <pryzby@telsasoft.com>
From: Justin Pryzby <pryzby@telsasoft.com>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Ian Lawrence Barwick <barwick@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, David Rowley <dgrowleyml@gmail.com>, Greg Stark <stark@mit.edu>, Julien Rouhaud <rjuju123@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2022-12-10T20:17:53Z
Lists: pgsql-hackers
On Tue, Nov 29, 2022 at 10:37:56PM +0900, Amit Langote wrote:
> 0002 contains changes that has to do with changing how we access
> checkAsUser in some foreign table planning/execution code sites.
> Thought it might be better to describe it separately too.
This was committed as 599b33b94:
Stop accessing checkAsUser via RTE in some cases
Which does this in a couple places in selfuncs.c:
if (!vardata->acl_ok &&
root->append_rel_array != NULL)
{
AppendRelInfo *appinfo;
Index varno = index->rel->relid;
appinfo = root->append_rel_array[varno];
while (appinfo &&
planner_rt_fetch(appinfo->parent_relid,
root)->rtekind == RTE_RELATION)
{
varno = appinfo->parent_relid;
appinfo = root->append_rel_array[varno];
}
if (varno != index->rel->relid)
{
/* Repeat access check on this rel */
rte = planner_rt_fetch(varno, root);
Assert(rte->rtekind == RTE_RELATION);
- userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+ userid = OidIsValid(onerel->userid) ?
+ onerel->userid : GetUserId();
vardata->acl_ok =
rte->securityQuals == NIL &&
(pg_class_aclcheck(rte->relid,
userid,
ACL_SELECT) == ACLCHECK_OK);
}
}
The original code rechecks rte->checkAsUser with the rte of the parent
rel. The patch changed to access onerel instead, but that's not updated
after looping to find the parent.
Is that okay ? It doesn't seem intentional, since "userid" is still
being recomputed, but based on onerel, which hasn't changed. The
original intent (since 553d2ec27) is to recheck the parent's
"checkAsUser".
It seems like this would matter for partitioned tables, when the
partition isn't readable, but its parent is, and accessed via a view
owned by another user?
--
Justin
Commits
-
Add a test case for a316a3bc
- 054ff3b33a85 17.0 landed
- 0a14bca662a4 16.0 landed
-
Correctly set userid of subquery relations' child rels
- a316a3bc6d3f 16.0 landed
-
Fix buggy recursion in flatten_rtes_walker().
- c7468c73f7b6 16.0 landed
-
Remove some dead code in selfuncs.c
- 438e6b724090 16.0 landed
-
Update outdated comment in ApplyRetrieveRule
- 29861e228a47 16.0 landed
-
Rework query relation permission checking
- a61b1f74823c 16.0 landed
-
Generalize ri_RootToPartitionMap to use for non-partition children
- fb958b5da86d 16.0 landed
-
Stop accessing checkAsUser via RTE in some cases
- 599b33b9492d 16.0 landed
-
Add 'missing_ok' argument to build_attrmap_by_name
- ad86d159b6ab 16.0 landed