Re: pgsql: Fix search_path to a safe value during maintenance operations.

2024-03-05 Thread Jeff Davis
On Tue, 2024-03-05 at 17:19 +0100, Alvaro Herrera wrote: > This appears to have upset the sepgsql tests.  In buildfarm member > rhinoceros there's now a bunch of errors like this Thank you, pushed, and it appears to have fixed the problem. Regards, Jeff Davis

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2024-03-05 Thread Alvaro Herrera
On 2024-Mar-05, Jeff Davis wrote: > Fix search_path to a safe value during maintenance operations. > > While executing maintenance operations (ANALYZE, CLUSTER, REFRESH > MATERIALIZED VIEW, REINDEX, or VACUUM), set search_path to > 'pg_catalog, pg_temp' to prevent inconsistent behavior. > >

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-08-01 Thread Jeff Davis
On Tue, 2023-08-01 at 14:47 -0700, David G. Johnston wrote: > The overall point stands, it just requires defining a similar "FROM > SESSION" to allow for explicitly specifying the current default > (missing) behavior. That sounds useful as a way to future-proof function definitions that intend to

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-08-01 Thread Jeff Davis
On Tue, 2023-08-01 at 13:41 -0400, Robert Haas wrote: > In functions and procedures, except for the new > BEGIN ATOMIC stuff, we just store the statements as a string and they > get parsed at execution time. ... > I think that a lot of people would like it if we moved more in the > direction of

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-08-01 Thread David G. Johnston
On Tue, Aug 1, 2023 at 2:38 PM Jeff Davis wrote: > On Tue, 2023-08-01 at 11:16 -0700, David G. Johnston wrote: > > They can use ALTER FUNCTION and the existing "FROM CURRENT" > > specification to get back to current behavior if desired. > > The current behavior is that the search_path comes from

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-08-01 Thread Jeff Davis
On Tue, 2023-08-01 at 11:16 -0700, David G. Johnston wrote: > They can use ALTER FUNCTION and the existing "FROM CURRENT" > specification to get back to current behavior if desired. The current behavior is that the search_path comes from the environment each execution. FROM CURRENT saves the

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-08-01 Thread David G. Johnston
On Tue, Aug 1, 2023 at 10:42 AM Robert Haas wrote: > Now, if we don't go in the direction of resolving everything at parse > time, then I think capturing search_path is probably the next best > thing, or at least the next best thing that I've thought up so far. I'd much rather strongly

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-08-01 Thread Robert Haas
On Mon, Jul 31, 2023 at 6:10 PM Jeff Davis wrote: > Capturing the environment is not ideal either, in my opinion. It makes > it easy to carelessly depend on a schema that others might not have > USAGE privileges on, which would then create a runtime problem for > other callers. Also, I don't

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-08-01 Thread Robert Haas
On Mon, Jul 31, 2023 at 5:15 PM Jeff Davis wrote: > > ERROR: role "rhaas" should not execute arbitrary code provided by > > role "jconway" > > HINT: If this should be allowed, use the TRUST command to permit it. > > +1, though I'm not sure we need an extensive trust mechanism beyond > what we

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-07-31 Thread Jeff Davis
On Mon, 2023-07-31 at 13:17 -0400, Joe Conway wrote: > But the analysis of the issue needs to go one step further. Even if > the > search_path does not change from the originally intended one, a newly > created function can shadow the intended one based on argument > coercion > rules. There are

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-07-31 Thread Jeff Davis
On Mon, 2023-07-31 at 12:53 -0400, Robert Haas wrote: > I agree. I think there are actually two interrelated problems here. > > One is that virtually all code needs to run with the originally > intended search_path rather than some search_path chosen at another > time and maybe by a different

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-07-31 Thread Jeff Davis
On Mon, 2023-07-31 at 16:06 -0400, Robert Haas wrote: > if you > include in your search_path a schema to which some other user can > write, you are pretty much agreeing to execute code provided by that > user. Agreed on all counts here. I don't think it's reasonable for us to try to make such a

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-07-31 Thread Robert Haas
On Mon, Jul 31, 2023 at 1:18 PM Joe Conway wrote: > But the analysis of the issue needs to go one step further. Even if the > search_path does not change from the originally intended one, a newly > created function can shadow the intended one based on argument coercion > rules. Yeah, this is a

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-07-31 Thread Joe Conway
On 7/31/23 12:53, Robert Haas wrote: On Fri, Jun 30, 2023 at 3:41 AM Jeff Davis wrote: 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

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-07-31 Thread Robert Haas
On Fri, Jun 30, 2023 at 3:41 AM Jeff Davis wrote: > 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

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-07-07 Thread Nathan Bossart
On Fri, Jul 07, 2023 at 09:57:10AM -0700, Nathan Bossart wrote: > Yeah, I guess I should just revert it in both. Given your fix will > hopefully be committed soon, I was hoping to avoid reverting and > un-reverting in quick succession to prevent affecting git-blame too much. > > I found an

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-07-07 Thread Nathan Bossart
On Fri, Jul 07, 2023 at 09:22:22AM -0700, Jeff Davis wrote: > On Thu, 2023-07-06 at 22:14 -0700, Nathan Bossart wrote: >> Since we are only reverting from v16, the REL_16_STABLE catversion >> will be >> bumped ahead of the one on master. > > I don't object to you doing it this way, but FWIW, I'd

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-07-07 Thread Jeff Davis
On Thu, 2023-07-06 at 22:14 -0700, Nathan Bossart wrote: > Since we are only reverting from v16, the REL_16_STABLE catversion > will be > bumped ahead of the one on master. I don't object to you doing it this way, but FWIW, I'd just revert in both branches to avoid this kind of weirdness. Also

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-07-06 Thread Nathan Bossart
Here is a new version of the patch that I think is ready for commit (except it still needs a catversion bump). The only real difference from v1 is in AdjustUpgrade.pm. From my cross-version pg_upgrade testing, I believe we can remove the other "privilege-set discrepancy" rule as well. Since

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-07-06 Thread Nathan Bossart
On Thu, Jul 06, 2023 at 12:55:14AM -0700, Jeff Davis wrote: > It was difficult to review standalone, so I tried a quick version > myself and ended up with very similar results. Thanks for taking a look. > The only substantial > difference was that I put back: > > > + if

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-07-06 Thread Jeff Davis
On Thu, 2023-06-29 at 22:09 -0700, Nathan Bossart wrote: > On Thu, Jun 29, 2023 at 08:53:56PM -0400, Tom Lane wrote: > > I'm leaning to Robert's thought that we need to revert this for > > now, > > and think harder about how to make it work cleanly and safely. > > Since it sounds like this is

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-07-03 Thread Noah Misch
On Mon, Jul 03, 2023 at 11:19:14AM -0700, Nathan Bossart wrote: > On Sun, Jul 02, 2023 at 08:57:31PM -0700, Noah Misch wrote: > > Another dimension of compromise could be to make MAINTAIN affect fewer > > commands in v16. Un-revert the part of commit 05e1737 affecting just the > > commands it

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-07-03 Thread Nathan Bossart
On Sun, Jul 02, 2023 at 08:57:31PM -0700, Noah Misch wrote: > Another dimension of compromise could be to make MAINTAIN affect fewer > commands in v16. Un-revert the part of commit 05e1737 affecting just the > commands it still affects. For example, limit MAINTAIN and the 05e1737 > behavior

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-07-02 Thread Noah Misch
On Fri, Jun 30, 2023 at 11:35:46AM -0700, Nathan Bossart wrote: > On Thu, Jun 29, 2023 at 10:09:21PM -0700, Nathan Bossart wrote: > > On Thu, Jun 29, 2023 at 08:53:56PM -0400, Tom Lane wrote: > >> I'm leaning to Robert's thought that we need to revert this for now, > >> and think harder about how

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-06-30 Thread Nathan Bossart
On Thu, Jun 29, 2023 at 10:09:21PM -0700, Nathan Bossart wrote: > On Thu, Jun 29, 2023 at 08:53:56PM -0400, Tom Lane wrote: >> I'm leaning to Robert's thought that we need to revert this for now, >> and think harder about how to make it work cleanly and safely. > > Since it sounds like this is

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-06-30 Thread Jeff Davis
On Thu, 2023-06-29 at 20:53 -0400, Tom Lane wrote: > I think that's a seriously awful kluge.  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

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-06-29 Thread Nathan Bossart
On Thu, Jun 29, 2023 at 08:53:56PM -0400, Tom Lane wrote: > I'm leaning to Robert's thought that we need to revert this for now, > and think harder about how to make it work cleanly and safely. Since it sounds like this is headed towards a revert, here's a patch for removing MAINTAIN and

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-06-29 Thread Tom Lane
Jeff Davis writes: > On Thu, 2023-06-29 at 11:19 -0400, Robert Haas wrote: >> We shouldn't ship a new feature with a built-in >> security hole like that. > Let's take David's suggestion[1] then, and only restrict the search > path for those without owner privileges on the object. I think that's

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-06-29 Thread Jeff Davis
On Thu, 2023-06-29 at 11:19 -0400, Robert Haas wrote: > Yeah. I mean, as things stand, it seems like giving someone the > MAINTAIN privilege will be sufficient to allow them to escalate to > the > table owner if there are any expression indexes involved. That seems > like a real problem. We

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-06-29 Thread Nathan Bossart
On Thu, Jun 29, 2023 at 11:19:38AM -0400, Robert Haas wrote: > [ emerges from hibernation ] Welcome back. > If we're not going to fix the feature so that it doesn't break the > security model, we should probably just revert it. I don't understand > at all the idea of shipping something that we

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-06-29 Thread Andrew Dunstan
On 2023-06-29 Th 11:19, Robert Haas wrote: Now we're proposing to ship a brand-new feature with a hole that we definitely already know exists. I can't understand that at all. Should we just go file the CVE against ourselves right now, then? Seriously, what are we doing? If we're not going to

Re: pgsql: Fix search_path to a safe value during maintenance operations.

2023-06-29 Thread Robert Haas
[ emerges from hibernation ] On Mon, Jun 19, 2023 at 6:58 PM Jeff Davis wrote: > On Mon, 2023-06-19 at 16:03 -0400, Robert Haas wrote: > > I'm inclined to think that this is a real security issue and am not > > Can you expand on that a bit? You mean a practical security issue for > the intended