Re: Possibility to disable `ALTER SYSTEM`

From: Gabriele Bartolini <gabriele(dot)bartolini(at)enterprisedb(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Magnus Hagander <magnus(at)hagander(dot)net>, Martín Marqués <martin(dot)marques(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Possibility to disable `ALTER SYSTEM`
Date: 2023-09-11 15:56:01
Message-ID: CA+VUV5o6LO3XskPvKATRjFLxzX_xby8NkV-BFKiWPitM8r7ifQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Stephen,

On Mon, 11 Sept 2023 at 17:12, Stephen Frost <sfrost(at)snowman(dot)net> wrote:

> A lot of the objections seem to be on the grounds of returning a
> 'permission denied' kind of error and I generally agree with that being
> the wrong approach.
>
> As an alternative idea- what if we had something in postgresql.conf
> along the lines of:
>
> include_alter_system = true/false
>
> and use that to determine if the postgresql.auto.conf is included or
> not..?
>

That sounds like a very good idea. I had thought about that when writing
the PoC, as a SIGHUP controlled GUC. I had trouble finding an adequate GUC
category for that (ideas?), and thought it was a more intrusive patch
to trigger the conversation. But I am willing to explore that too (which
won't change by any inch the goal of the patch).

With the above, we could throw a WARNING or maybe just NOTICE when ALTER
> SYSTEM is run that 'include_alter_system is false and therefore these
> changes won't be included in the running configuration' or similar.
>

That's also an option I'd be willing to explore with folks here.

> What this does cause problems with is that pg_basebackup and other tools
> (eg: pgbackrest) write into postgresql.auto.conf currently and we'd want
> those to still work. That's an opportunity, imv, though, since I don't
> really think where ALTER SYSTEM writes to and where backup/restore
> tools are writing to should really be the same place anyway. Therefore,
> perhaps we add a 'postgresql.system.conf' or similar and maybe a
> corresponding option in postgresql.conf to include it or not.
>

Totally. We are for example in the same position with the CloudNativePG
operator, and it is something we are intending to fix (
https://github.com/cloudnative-pg/cloudnative-pg/issues/2727) I agree with
you that it is the wrong place to do it.

This is an issue if you're looking at it as a security thing. This
> isn't an issue if don't view it that way. Still, I do see some merit in
> making it so that you can't actually change the config that's loaded at
> system start from inside the data directory or as the PG superuser,
> which my proposal above would support- just configure in postgresql.conf
> to not include any of the alter-system or generated config. The actual
> postgresql.conf could be owned by root then too.
>

+1.

Thank you,
Gabriele
--
Gabriele Bartolini
Vice President, Cloud Native at EDB
enterprisedb.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Aleksander Alekseev 2023-09-11 16:04:30 Re: pg_rewind with cascade standby doesn't work well
Previous Message Aleksander Alekseev 2023-09-11 15:51:59 Re: How to add built-in func?