Horton

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

Jeff Davis <pgsql@j-davis.com>

View thread View on postgr.es
From: Jeff Davis <pgsql@j-davis.com>
To: Michael Paquier <michael@paquier.xyz>, Noah Misch <noah@leadboat.com>
Cc: Nathan Bossart <nathandbossart@gmail.com>, pgsql-hackers@postgresql.org, Joe Conway <mail@joeconway.com>, Robert Haas <robertmhaas@gmail.com>
Date: 2024-07-10T00:47:36Z
Lists: pgsql-hackers

Attachments

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:
> > I've audited NewGUCNestLevel() calls that didn't get this
> > addition.  Among
> > those, these need the addition:
> > 
> > - Each in ComputeIndexAttrs() -- they arise when the caller is
> > DefineIndex()
> > - In DefineIndex(), after comment "changed a behavior-affecting
> > GUC"

Thank you for the report. Patch attached to address these missing call
sites.

> 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?

> > 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?

Regards,
	Jeff Davis

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.