Re: Extension pg_trgm, permissions and pg_dump order

On Sat, May 28, 2022 at 08:30:33PM -0700, Nathan Bossart wrote:
> On Fri, May 27, 2022 at 09:51:22PM -0700, Noah Misch wrote:
> > On Wed, May 25, 2022 at 10:50:47PM -0700, Noah Misch wrote:
> >> BEGIN;
> >> CREATE ROLE limitedrole;
> >> CREATE SCHEMA ext_trgm;
> >> CREATE EXTENSION pg_trgm SCHEMA ext_trgm;
> >> CREATE TABLE x(y text);
> >> ALTER TABLE x OWNER TO limitedrole;
> >> CREATE INDEX ON x USING gist(y ext_trgm.gist_trgm_ops);
> >> ROLLBACK;
> >
> >> Not too simple, no. The parts of DefineIndex() that execute user code
> >> (e.g. DefineIndex->ComputeIndexAttrs->CheckMutability) are interspersed with
> >> the parts that do permissions checks, like the one yielding $SUBJECT at
> >> DefineIndex->ComputeIndexAttrs->ResolveOpClass->LookupExplicitNamespace. My
> >> first candidate is to undo the userid switch before the ResolveOpClass() call
> >> and restore it after. My second candidate is to pass down the userid we want
> >> used for this sort of permissions check. Depending on the full list of call
> >> stacks reaching permissions checks, this could get hairy.
> >
> > While I'd value the opportunity to work on this, there only a 50% chance I
> > could get it done by 2022-08-01. I've set aside four hours on 2022-06-12 to
> > see how far I get. For a 95% chance, the date would be 2023-02-01. (I've
> > already canceled a 2022-07 vacation for the thing taking my time instead;
> > there's nothing clearly left to cut.) If anyone would like it done faster
> > than that, I welcome that person taking over.
>
> I'd like to help.

Thanks.

> I started looking at the problem and hacked together a
> proof-of-concept based on your first candidate that seems to fix the
> reported issue. However, from the upthread discussion, it is not clear
> whether there is agreement on the approach. IIUC there are still many
> other code paths that would require a similar treatment, so perhaps
> identifying all of those would be a good next step.

Agreed. To identify them, perhaps put an ereport(..., errbacktrace()) in
aclmask(), then write some index-creating DDL that refers to the
largest-possible number of objects.