Horton

Re: MAINTAIN privilege -- what do we need to un-revert it?

Nathan Bossart <nathandbossart@gmail.com>

View thread View on postgr.es
From: Nathan Bossart <nathandbossart@gmail.com>
To: Jeff Davis <pgsql@j-davis.com>
Cc: pgsql-hackers@postgresql.org, Joe Conway <mail@joeconway.com>, Robert Haas <robertmhaas@gmail.com>
Date: 2024-02-23T21:30:55Z
Lists: pgsql-hackers 
(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

Commits

  1. doc: Note that CREATE MATERIALIZED VIEW restricts search_path.

  2. Small refactoring around ExecCreateTableAs().

  3. Add is_create parameter to RefreshMatviewByOid().

  4. Remove unused ParamListInfo argument from ExecRefreshMatView.

  5. When creating materialized views, use REFRESH to load data.

  6. Add missing RestrictSearchPath() calls.

  7. Fix search_path to a safe value during maintenance operations.

  8. Revert MAINTAIN privilege and pg_maintain predefined role.

  9. Avoid repeated name lookups during table and index DDL.

  10. Add a materialized view relations.