Re: Possibility to disable `ALTER SYSTEM`

On Fri, Mar 15, 2024 at 7:09 AM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
> On Fri, 15 Mar 2024 at 11:08, Daniel Gustafsson <daniel@yesql.se> wrote:
> > Another quirk for the documentation of this: if I disable ALTER SYSTEM I would
> > assume that postgresql.auto.conf is no longer consumed, but it still is (and
> > still need to be), so maybe "enable/disable" is the wrong choice of words?
>
> Updated the docs to reflect this quirk. But I kept the same name for
> the GUC for now, because I couldn't come up with a better name myself.
> If someone suggests a better name, I'm happy to change it though.

Hmm. So in this patch, we have a whole new kind of GUC - guard rails -
of which enable_alter_system is the first member. Is that what we
want? I would have been somewhat inclined to find an existing section
of postgresql.auto.conf for this parameter, perhaps "platform and
version compatibility". But if we're going to add a bunch of similar
GUCs, maybe grouping them all together is the way to go.

Even if that is what we're going to do, do we want to call them "guard
rails"? I'm not sure I'd find that name terribly clear, as a user. We
know what we mean right now because we're having a very active
discussion about this topic, but it might not seem as clear to someone
coming at it fresh.

On balance, I'm disinclined to add a new category for this. If we get
to a point where we have several of these and we want to break them
out into a new category, we can do it then. Maybe by that time the
naming will seem more clear, too.

I also don't think it's good enough to just say that this isn't a
security feature. Talk is cheap. I think we need to say why it's not a
security feature. So my proposal is something like this, taking a
bunch of text from Jelte's patch and some inspiration from Magnus's
earlier remarks:

==
When <literal>enable_alter_system</literal> is set to
<literal>off</literal>, an error is returned if the <command>ALTER
SYSTEM</command> command is used. This parameter can only be set in
the <filename>postgresql.conf</filename> file or on the server command
line. The default value is <literal>on</literal>.

Note that this setting cannot be regarded as a security feature. It
only disables the <literal>ALTER SYSTEM</literal> command. It does not
prevent a superuser from changing the configuration remotely using
other means. A superuser has many ways of executing shell commands at
the operating system level, and can therefore modify
<literal>postgresql.auto.conf</literal> regardless of the value of
this setting. The purpose of the setting is to prevent
<emphasis>accidental</emphasis> modifications via <literal>ALTER
SYSTEM</literal> in environments where <literal>PostgreSQL</literal>'s
configuration is managed by some outside mechanism. In such
environments, using <command>ALTER SYSTEM</command> to make
configuration changes might appear to work, but then may be discarded
at some point in the future when that outside mechanism updates the
configuration. Setting this parameter to <literal>false</literal> can
help to avoid such mistakes.
==

I agree with Daniel's comment that Tom's concerns about people filing
CVEs are not without merit; indeed, I said the same thing in my first
post to this thread. However, I also believe that's not a sufficient
reason for rejecting a feature that many people seem to want. I think
the root of this problem is that our documentation is totally unclear
about the fact that we don't intend for there to be privilege
separation between the operating system user and the PostgreSQL
superuser; people want there to be a distinction, and think there is.
Hence CVE-2019-9193, for example. Several people, including me, wrote
blog posts about how that's not a security vulnerability, but while I
was researching mine, I went looking for where in the documentation we
actually SAY that there's no privilege separation between the OS user
and the superuser. The only mention I found at the time was the
PL/perlu documentation, which said this:

"The writer of a PL/PerlU function must take care that the function
cannot be used to do anything unwanted, since it will be able to do
anything that could be done by a user logged in as the database
administrator."

That statement, from the official documentation, in my mind at least,
DOES confirm that we don't intend privilege separation, but it's
really oblique. You have to think through the fact that the superuser
has to be the one to install plperlu, and that plperlu functions can
usurp the OS user; since both of those things are documented to be the
case, it follows that we know and expect that the superuser can usurp
the OS user. But someone who is wondering how PostgreSQL's security
model works is not going to read the plperlu documentation and make
the inferences I just described. It's crazy to me that a principle
frequently cited as gospel on this mailing list and others is nearly
undocumented. Obviously, even if we did document it clearly, people
could still get confused (or just disagree with our position) and file
CVEs anyway, but we're not helping our case by having nothing to cite.

A difficulty is where to PUT such a mention in the documentation.
There's not a single section title in the top-level documentation
index that includes the word "security". Perhaps figuring out how to
document this is best left to a separate thread, and there's also the
question of whether a new section that talks about this also ought to
talk about anything else. But I feel like we're way overdue to do
something about this.

--
Robert Haas
EDB: http://www.enterprisedb.com