Re: ExecRTCheckPerms() and many prunable partitions (checkAsUser)

Поиск
Список
Период
Сортировка
От Justin Pryzby
Тема Re: ExecRTCheckPerms() and many prunable partitions (checkAsUser)
Дата
Msg-id 20221210201753.GA27893@telsasoft.com
обсуждение исходный текст
Ответ на Re: ExecRTCheckPerms() and many prunable partitions  (Amit Langote <amitlangote09@gmail.com>)
Ответы Re: ExecRTCheckPerms() and many prunable partitions (checkAsUser)  (Amit Langote <amitlangote09@gmail.com>)
Список 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



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Jeff Davis
Дата:
Сообщение: Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX
Следующее
От: Thomas Munro
Дата:
Сообщение: Re: -Wunreachable-code-generic-assoc warnings on elver