pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
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: Fri, 23 Feb 2024 15:30:55 -0600
Message-ID: <20240223213055.GA1805646@nathanxps13> (raw)
In-Reply-To: <4d497d82e4516037dc1082d5a62d199ea00d62e7.camel@j-davis.com>
References: <d4ccaf3658cb3c281ec88c851a09733cd9482f22.camel@j-davis.com>
	<20240214190226.GA373350@nathanxps13>
	<4d497d82e4516037dc1082d5a62d199ea00d62e7.camel@j-davis.com>

(Apologies in advance for anything I'm bringing up that we've already
covered somewhere else.)

On Fri, Feb 16, 2024 at 04:03:55PM -0800, Jeff Davis wrote:
> Note the changes in amcheck. It's creating functions and calling those
> functions from the comparators, and so the comparators need to set the
> search_path. I don't think that's terribly common, but does represent a
> behavior change and could break something.

Why is this change needed?  Is the idea to make amcheck follow the same
rules as maintenance commands to encourage folks to set up index functions
correctly?  Or is amcheck similarly affected by search_path tricks?

>  void
>  InitializeSearchPath(void)
>  {
> +	/* Make the context we'll keep search path cache hashtable in */
> +	SearchPathCacheContext = AllocSetContextCreate(TopMemoryContext,
> +												   "search_path processing cache",
> +												   ALLOCSET_DEFAULT_SIZES);
> +
>  	if (IsBootstrapProcessingMode())
>  	{
>  		/*
> @@ -4739,11 +4744,6 @@ InitializeSearchPath(void)
>  	}
>  	else
>  	{
> -		/* Make the context we'll keep search path cache hashtable in */
> -		SearchPathCacheContext = AllocSetContextCreate(TopMemoryContext,
> -													   "search_path processing cache",
> -													   ALLOCSET_DEFAULT_SIZES);
> -

What is the purpose of this change?

> +	SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
> +					PGC_S_SESSION);

I wonder if it's worth using PGC_S_INTERACTIVE or introducing a new value
for these.

> +/*
> + * Safe search path when executing code as the table owner, such as during
> + * maintenance operations.
> + */
> +#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.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com





view thread (30+ messages)  latest in thread

Message-ID: <20240223213055.GA1805646@nathanxps13>
Permalink:  ../20240223213055.GA1805646@nathanxps13/
Also on:    postgresql.org/message-id/20240223213055.GA1805646@nathanxps13

 · 

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: 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: <20240223213055.GA1805646@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