Date: Thu, 11 Jul 2024 05:52:07 -0700
From: Noah Misch <noah@leadboat.com>
To: Jeff Davis <pgsql@j-davis.com>
Cc: Michael Paquier <michael@paquier.xyz>,
	Nathan Bossart <nathandbossart@gmail.com>,
	pgsql-hackers@postgresql.org, Joe Conway <mail@joeconway.com>,
	Robert Haas <robertmhaas@gmail.com>
Subject: Re: MAINTAIN privilege -- what do we need to un-revert it?
Message-ID: <20240711125207.98.nmisch@google.com>
References: <20240214190226.GA373350@nathanxps13>
 <4d497d82e4516037dc1082d5a62d199ea00d62e7.camel@j-davis.com>
 <20240223213055.GA1805646@nathanxps13>
 <196d1c86d43a147e777bb74159774f255dbe73e3.camel@j-davis.com>
 <20240228165523.GA2435539@nathanxps13>
 <31ae1bf73e8a1dae3ee7ef2dd43a9bbda1e1ab86.camel@j-davis.com>
 <fe5d572b02d07d8e9940f9b518d900708e0b0166.camel@j-davis.com>
 <20240630222344.db.nmisch@google.com>
 <ZozWqg0XCkiPCnop@paquier.xyz>
 <23fac3b2a722373a52b92a12ee0f500821450f97.camel@j-davis.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <23fac3b2a722373a52b92a12ee0f500821450f97.camel@j-davis.com>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: 
 <https://www.postgresql.org/message-id/20240711125207.98.nmisch%40google.com>
Precedence: bulk

On Tue, Jul 09, 2024 at 05:47:36PM -0700, Jeff Davis wrote:
> On Tue, 2024-07-09 at 15:20 +0900, Michael Paquier wrote:
> > On Sun, Jun 30, 2024 at 03:23:44PM -0700, Noah Misch wrote:

> > Hmm.  Is RestrictSearchPath() something that we should advertise more
> > strongly, thinking here about extensions that call NewGUCNestLevel()?
> > That would be really easy to miss, and it could have bad
> > consequences.
> > I know that this is not something that's published in the release
> > notes, but it looks like something sensible to have, though.
> 
> The pattern also involves SetUserIdAndSecContext(). Perhaps we could
> come up with a wrapper function to better encapsulate the general
> pattern?

Worth a look.  usercontext.c has an existing wrapper for a superuser process
switching to an untrusted user.  It could become the home for another wrapper
targeting MAINTAIN-relevant callers.

> > > While "not necessary for security", ExecCreateTableAs() should do
> > > it for the
> > > same reason it calls NewGUCNestLevel().
> > 
> > +1.
> 
> Do you have a suggestion about how that should be done?
> 
> It's not trivial, because the both creates the table and populates it
> in ExecutorRun. For table creation, we need to use the original
> search_path, but we need to use the restricted search_path when
> populating it.
> 
> I could try to refactor it into two statements and execute them
> separately, or I could try to rewrite the statement to use a fully-
> qualified destination table before execution. Thoughts?

Those sound fine.  Also fine: just adding a comment on why creation namespace
considerations led to not doing it there.