pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Noah Misch <noah@leadboat.com>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-bugs@lists.postgresql.org, Färber, Franz-Josef (StMUK) <Franz-Josef.Faerber@stmuk.bayern.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Subject: Re: Extension pg_trgm, permissions and pg_dump order
Date: Tue, 21 Jun 2022 20:37:04 -0700
Message-ID: <20220622033704.GA4167527@rfd.leadboat.com> (raw)
In-Reply-To: <20220621175616.GA93985@nathanxps13>
References: <20220529033033.GA1973706@nathanxps13>
	<20220529043403.GB3265235@rfd.leadboat.com>
	<YpWZ02YEN9pipA6M@paquier.xyz>
	<20220531152855.GA2236210@nathanxps13>
	<20220603181724.GA2672350@nathanxps13>
	<20220606063056.GA3527347@rfd.leadboat.com>
	<20220609051708.GA3615673@rfd.leadboat.com>
	<20220613034551.GB3728437@rfd.leadboat.com>
	<20220616054218.GA3980475@rfd.leadboat.com>
	<20220621175616.GA93985@nathanxps13>

On Tue, Jun 21, 2022 at 10:56:16AM -0700, Nathan Bossart wrote:
> On Wed, Jun 15, 2022 at 10:42:18PM -0700, Noah Misch wrote:
> > -REGRESS = citext citext_utf8
> > +REGRESS = create_index_acl citext citext_utf8
> 
> It's a little unfortunate that these tests are located within the citext
> module, but I can't claim to have a better idea.

A little, yes.  I did consider creating an operator class in the main
regression suite, then testing that.  If I had used that approach, it probably
would have been a my_text that behaves as much like text as possible.  There's
little difference in value between a contrib/ test and an equivalent
src/test/regress/ test, so I picked this for the increase in realism and the
decrease in test code bulk.

> > +		 * Identify the opclass to use.  Use of ddl_userid is necessary due to
> > +		 * ACL checks therein.  This is safe despite opclasses containing
> > +		 * opaque expressions (specifically, functions), because only
> > +		 * superusers can define opclasses.
> 
> It's not clear to me why the fact that only superusers can define opclasses
> makes this safe.

		classOidP[attn] = ResolveOpClass(attribute->opclass,
										 atttype,
										 accessMethodName,
										 accessMethodId);

To write the comment, I pondered how those four arguments could conceivably
lead ResolveOpClass() to locate Trojan code.  Since only superusers can define
opclasses, we can assume the catalog entries of an opclass do not point to
Trojan code.  (The superuser could just do the mischief directly, rather than
going to extra trouble to set a trap for later.)  If you see a hole in that
thinking, please do share.

> Overall, the patch seems reasonable to me.

Thanks for reviewing.  I'll push it sometime in the next seven days.





view thread (21+ messages)  latest in thread

Message-ID: <20220622033704.GA4167527@rfd.leadboat.com>
Permalink:  ../20220622033704.GA4167527@rfd.leadboat.com/
Also on:    postgresql.org/message-id/20220622033704.GA4167527@rfd.leadboat.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-bugs@postgresql.org
  Cc: noah@leadboat.com, nathandbossart@gmail.com, michael@paquier.xyz, Franz-Josef.Faerber@stmuk.bayern.de, tgl@sss.pgh.pa.us
  Subject: Re: Extension pg_trgm, permissions and pg_dump order
  In-Reply-To: <20220622033704.GA4167527@rfd.leadboat.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