Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review]) - Mailing list pgsql-hackers

Robert,

Was working on replying to this, but got distracted..

* Robert Haas (robertmhaas@gmail.com) wrote:
> To be honest, I think the argument that this is dangerous is pretty
> ridiculous.  AFAICS, the only argument anyone's advanced for this
> being dangerous is the theory that you might accidentally put
> something in your postgresql.conf file that makes the server not
> start.  However, the reality is that the superuser has MANY, MANY ways
> of killing the database cluster as things stand.  Consider the
> ever-popular "DELETE FROM pg_proc".

You will not find me argueing to allow that in normal operation, or most
direct-catalog hacking.  I'd be much happier if we had all the ALTER,
etc, options necessary to prevent any need to ever touch the catalogs.
Similairly, it'd be nice to have more permission options which reduce
the need for anyone to be superuser.

> Now, you can argue that people are more likely to render the database
> nonfunctional by changing GUC settings than they are to do it by
> corrupting the system catalogs, but I'm not sure I believe it.  We
> can, after all, validate that any setting a user supplies is a valid
> value for that setting before writing it out to the configuration
> file.  It might still make the system fail to start if - for example -
> it causes too many semaphores to be reserved, or something like that.
> But why should we think that such mistakes will be common?  If
> anything, it sounds less error-prone to me than hand-editing the
> configuration file, where typing something like on_exit_error=maybe
> will make the server fail to start.  We can eliminate that whole
> category of error, at least for people who choose to use the new
> tools.

That category of error is much more likely to get caught by the sysadmin
doing the restart after the update..  ALTER SYSTEM doesn't provide any
way to restart the DB to make sure it worked.  That, in turn, encourages
modification of the config parameters w/o a restart, which is a pretty
bad idea, imv.

> If you're using the term "dangerous" to refer to a security exposure,
> I concede there is some incremental exposure from allowing this by
> default.  But it's not a terribly large additional exposure.  It's
> already the case that if adminpack is available the super-user can do
> whatever he or she wants, because she or he can write to arbitrary
> files inside the data directory.

This is only true if -contrib is installed, which a *lot* of admins
refuse to do, specifically because of such insane modules as this.
Therefore, I would argue that it's not nearly as small a change wrt
exposure as you believe.  Strictly speaking, I don't believe it's a huge
security hole or something, but I do think it's going to surprise admins
who aren't following the lists.

> Even if not, for most intents and
> purposes, ALTER DATABASE my_main_database SET whatever = thing is
> functionally equivalent to modifying postgresql.conf.  Some settings
> can't be modified that way, but so what?  AFAICS, about the worst
> thing the user can do is ALTER SYSTEM SET shared_preload_libraries =
> 'rootkit'.

This is assuming a malicious user, and one who has access to the
filesystem, which are pretty big assumptions to be making.  I'm not
trying to make any argument about a malicious user and in general I
don't believe the users of this capability are people who have root
on the box.

> Huh?  The problem with adminpack is that it doesn't let you modify
> individual configuration settings.  All you can do is rewrite an
> entire file.  I guess somebody could write a specialized client that
> just uses that infrastructure to rewrite postgresql.conf.  For all I
> know, someone has.  Even if not, I don't think that you can use that
> to prove that people don't care about this feature.  If nobody cares,
> why are there 400 emails on this topic?!

Addressed this in my reply to Andres.

> > Why can't adminpack do that..?
>
> It could.  But it doesn't.  We could improve it to do that, and that
> might be worthwhile, but it still wouldn't be as nice as what's being
> proposed here.

This wouldn't be the first time we've said "do it externally and when
we figure out a better answer then we'll put it into core."

> 1. disabling the feature by default, and providing no way for it to be
> turned on remotely, and

Yes, that's quite intentional because it would make someone knowledgable
about the *overall system* be aware of this rather than having it sprung
on them when they discover some change has made the database unable to
start.

> 2. even when the feature is enabled, only allowing a subset of the
> settings to be changed with it, and
> 3. also changing the interpretation of the config files so that we
> have a first-wins rules instead of a last-wins rule.
>
> If we do either #1 or #2, this won't be a plausible substitute for the
> functionality that adminpack offers today.

While I appreciate that the original thrust behind this may be trying to
replace adminpack, I find that module to be utterly horrendous in what
it does and if we don't replicate all that it can do, so much the
better.  Consider a similar proposal as ALTER SYSTEM which simply
rewrites pg_hba.conf, ala how adminpack could.  Would we punt on the
hard questions around things like how to specify the order in which
entries in pg_hba.conf are maintained?  We could have an ALTER HBA today
which takes a line number to insert a string of text into, but is it a
good idea or design?  Certainly not.  My point here is simply that
focusing on replacing adminpack is not the basis for a feature or
design.

> #3 is a bad idea in my
> book - it would break some of my existing benchmarking scripts, which
> do initdb; cat >>$PGDATA/postgresql.conf <<EOM.  But even if it were a
> good idea, it isn't a necessary prerequisite for this patch.

Too often do we consider sloppy handling like this, which is great for a
dev or testing environment, to be appropriate for production systems. :(
Thanks,
    Stephen