Message-ID: <fff566293c9165c69bb4c555da1ac02c63660664.camel@j-davis.com>
Subject: Re: pgsql: Fix search_path to a safe value during maintenance
 operations.
From: Jeff Davis <pgsql@j-davis.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>, Noah Misch <noah@leadboat.com>,
	"pgsql-hackers@postgresql.org"
	 <pgsql-hackers@postgresql.org>, Nathan Bossart <nathandbossart@gmail.com>
Date: Fri, 30 Jun 2023 00:41:02 -0700
In-Reply-To: <1659699.1688086436@sss.pgh.pa.us>
References: <E1q7j7Y-000z1H-Hr@gemulon.postgresql.org>
	 <89a415b86d5421941e4281ba5bf6288934078624.camel@j-davis.com>
	 <d20864b929ff769a0ebf772359719f05bb6982cd.camel@j-davis.com>
	 <573014.1686375211@sss.pgh.pa.us>
	 <20230612170510.GA201972@gust.leadboat.com>
	 <f53df2dbe11bb42764bcdd003a54aa29c9bb6137.camel@j-davis.com>
	 <20230613182920.GA259321@gust.leadboat.com>
	 <1799839.1686687804@sss.pgh.pa.us>
	 <4df537f5a0d8c0ceac3eec3809d1144880b8e76d.camel@j-davis.com>
	 <CA+TgmoYtJU0KknWr11W--wK=4eJtdHOgupyT=beZVPC8P=o+YA@mail.gmail.com>
	 <578fb4be80247570e6a05924908765a0b345971e.camel@j-davis.com>
	 <CA+TgmoZVCHERUkXhAMT2Er-sKBc5C6_iX+TpxxivBevDHzq2TQ@mail.gmail.com>
	 <43bbc9f7318057f51552d5986830908cb018dadf.camel@j-davis.com>
	 <1659699.1688086436@sss.pgh.pa.us>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4-0ubuntu1 
MIME-Version: 1.0
Archived-At: 
 <https://www.postgresql.org/message-id/fff566293c9165c69bb4c555da1ac02c63660664.camel%40j-davis.com>
Precedence: bulk

On Thu, 2023-06-29 at 20:53 -0400, Tom Lane wrote:
> I think that's a seriously awful kluge.=C2=A0 It will mean that things
> behave
> differently for the owner than for MAINTAIN grantees, which pretty
> much
> destroys the use-case for that privilege, as well as being very
> confusing
> and hard to debug.

In version 15, try this:

  CREATE USER foo;
  CREATE SCHEMA foo AUTHORIZATION foo;
  CREATE USER bar;
  CREATE SCHEMA bar AUTHORIZATION bar;
  \c - foo
  CREATE FUNCTION foo.mod10(INT) RETURNS INT IMMUTABLE
    LANGUAGE plpgsql AS $$ BEGIN RETURN mod($1,10); END; $$;
  CREATE TABLE t(i INT);
  -- units digit must be unique
  CREATE UNIQUE INDEX t_idx ON t (foo.mod10(i));
  INSERT INTO t VALUES(7); -- success
  INSERT INTO t VALUES(17); -- fails
  GRANT USAGE ON SCHEMA foo TO bar;
  GRANT INSERT ON t TO bar;
  \c - bar
  CREATE FUNCTION bar.mod(INT, INT) RETURNS INT IMMUTABLE
    LANGUAGE plpgsql AS $$ BEGIN RETURN $1 + 1000000; END; $$;
  SET search_path =3D bar, pg_catalog;
  INSERT INTO foo.t VALUES(7); -- succeeds
  \c - foo
  SELECT * FROM t;
   i=20
  ---
   7
   7
  (2 rows)


I'm not sure that everyone in this thread realizes just how broken it
is to depend on search_path in a functional index at all. And doubly so
if it depends on a schema other than pg_catalog in the search_path.

Let's also not forget that logical replication always uses
search_path=3Dpg_catalog, so if you depend on a different search_path for
any function attached to the table (not just functional indexes, also
functions inside expressions or trigger functions), then those are
already broken in version 15. And if a superuser is executing
maintenance commands, there's little reason to think they'll have the
same search path as the user that created the table.

At some point in the very near future (though I realize that point may
come after version 16), we need to lock down the search path in a lot
of cases (not just maintenance commands), and I don't see any way
around that.


Regards,
	Jeff Davis