Horton

Re: Possibility to disable `ALTER SYSTEM`

Gabriele Bartolini <gabriele.bartolini@enterprisedb.com>

View thread
From: Gabriele Bartolini <gabriele.bartolini@enterprisedb.com>
To: Stephen Frost <sfrost@snowman.net>
Cc: Magnus Hagander <magnus@hagander.net>, Martín Marqués <martin.marques@gmail.com>, pgsql-hackers@postgresql.org
Date: 2023-09-11T15:56:01Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add allow_alter_system GUC.

  2. Rename COMPAT_OPTIONS_CLIENT to COMPAT_OPTIONS_OTHER.

  3. Remove support for version-0 calling conventions.

Hi Stephen,

On Mon, 11 Sept 2023 at 17:12, Stephen Frost <sfrost@snowman.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