From: Nathan Bossart <nathandbossart@gmail.com>
To: Jeff Davis <pgsql@j-davis.com>
Cc: pgsql-hackers@postgresql.org, Joe Conway <mail@joeconway.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Subject: Re: MAINTAIN privilege -- what do we need to un-revert it?
Date: Wed, 28 Feb 2024 10:55:23 -0600
Message-ID: <20240228165523.GA2435539@nathanxps13> (raw)
In-Reply-To: <196d1c86d43a147e777bb74159774f255dbe73e3.camel@j-davis.com>
References: <d4ccaf3658cb3c281ec88c851a09733cd9482f22.camel@j-davis.com>
<20240214190226.GA373350@nathanxps13>
<4d497d82e4516037dc1082d5a62d199ea00d62e7.camel@j-davis.com>
<20240223213055.GA1805646@nathanxps13>
<196d1c86d43a147e777bb74159774f255dbe73e3.camel@j-davis.com>
On Tue, Feb 27, 2024 at 04:22:34PM -0800, Jeff Davis wrote:
> Do we need a documentation update here? If so, where would be a good
> place?
I'm afraid I don't have a better idea than adding a short note in each
affected commands's page.
> On Fri, 2024-02-23 at 15:30 -0600, Nathan Bossart wrote:
>> I wonder if it's worth using PGC_S_INTERACTIVE or introducing a new
>> value
>> for these.
>
> Did you have a particular concern about PGC_S_SESSION?
My only concern is that it could obscure the source of the search_path
change, which in turn might cause confusion when things fail.
> If it's less than PGC_S_SESSION, it won't work, because the caller's
> SET command will override it, and the same manipulation is possible.
>
> And I don't think we want it higher than PGC_S_SESSION, otherwise the
> function can't set its own search_path, if needed.
Yeah, we would have to make it equivalent in priority to PGC_S_SESSION,
which would likely require a bunch of special logic. I don't know if this
is worth it, and this seems like something that could pretty easily be
added in the future if it became necessary.
>> > +#define GUC_SAFE_SEARCH_PATH "pg_catalog, pg_temp"
>>
>> Is including pg_temp actually safe? I worry that a user could use
>> their
>> temporary schema to inject objects that would take the place of
>> non-schema-qualified stuff in functions.
>
> pg_temp cannot (currently) be excluded. If it is omitted from the
> string, it will be placed *first* in the search_path, which is more
> dangerous.
>
> pg_temp does not take part in function or operator resolution, which
> makes it safer than it first appears. There are potentially some risks
> around tables, but it's not typical to access a table in a function
> called as part of an index expression.
>
> If we determine that pg_temp is actually unsafe to include, we need to
> do something like what I proposed here:
>
> https://www.postgresql.org/message-id/a6865db287596c9c6ea12bdd9de87216cb5e7902.camel@j-davis.com
I don't doubt anything you've said, but I can't help but think that we
might as well handle the pg_temp risk, too.
Furthermore, I see that we use "" as a safe search_path for autovacuum and
fe_utils/connect.h. Is there any way to unite these? IIUC it might be
possible to combine the autovacuum and maintenance command cases (i.e.,
"!pg_temp"), but we might need to keep pg_temp for the frontend case. I
think it's worth trying to add comments about why this setting is safe for
some cases but not others, too.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
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: nathandbossart@gmail.com, pgsql@j-davis.com, mail@joeconway.com, robertmhaas@gmail.com
Subject: Re: MAINTAIN privilege -- what do we need to un-revert it?
In-Reply-To: <20240228165523.GA2435539@nathanxps13>
* 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