pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Justin Pryzby <pryzby@telsasoft.com>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Ian Lawrence Barwick <barwick@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: David Rowley <dgrowleyml@gmail.com>
Cc: Greg Stark <stark@mit.edu>
Cc: Julien Rouhaud <rjuju123@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: ExecRTCheckPerms() and many prunable partitions (checkAsUser)
Date: Sat, 10 Dec 2022 14:17:53 -0600
Message-ID: <20221210201753.GA27893@telsasoft.com> (raw)
In-Reply-To: <CA+HiwqGFCs2uq7VRKi7g+FFKbP6Ea_2_HkgZb2HPhUfaAKT3ng@mail.gmail.com>
References: <CA+HiwqHQeooqvXy+FSXYOq0wajGtC5-TBj0=ZW1=qVq5Pm2ycw@mail.gmail.com>
	<20221129092708.uooopfzpiwrhgedl@alvherre.pgsql>
	<CA+HiwqGFCs2uq7VRKi7g+FFKbP6Ea_2_HkgZb2HPhUfaAKT3ng@mail.gmail.com>

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





view thread (97+ messages)  latest in thread

Message-ID: <20221210201753.GA27893@telsasoft.com>
Permalink:  ../20221210201753.GA27893@telsasoft.com/
Also on:    postgresql.org/message-id/20221210201753.GA27893@telsasoft.com

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: pryzby@telsasoft.com, amitlangote09@gmail.com, alvherre@alvh.no-ip.org, barwick@gmail.com, tgl@sss.pgh.pa.us, dgrowleyml@gmail.com, stark@mit.edu, rjuju123@gmail.com, pgsql-hackers@lists.postgresql.org
  Subject: Re: ExecRTCheckPerms() and many prunable partitions (checkAsUser)
  In-Reply-To: <20221210201753.GA27893@telsasoft.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox