Thread: ALTER SYSTEM vs symlink

I don't know if this was discussed at the time ALTER SYSTEM was 
implemented, but I have just discovered that if postgresql.auto.conf is 
a symlink to a file elsewhere, ALTER SYSTEM will happily break that link 
and write its own local copy. That strikes me as rather unfriendly. Why 
not just truncate the file rather than unlink it as it's being 
rewritten? Even if we don't want to do that a warning in the docs might 
help users avoid the "mistake" I just made.

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:
> I don't know if this was discussed at the time ALTER SYSTEM was 
> implemented, but I have just discovered that if postgresql.auto.conf is 
> a symlink to a file elsewhere, ALTER SYSTEM will happily break that link 
> and write its own local copy. That strikes me as rather unfriendly. Why 
> not just truncate the file rather than unlink it as it's being 
> rewritten? Even if we don't want to do that a warning in the docs might 
> help users avoid the "mistake" I just made.

Frankly, that behavior strikes me as a good idea.  There is no situation,
IMV, where it's sane to try to put a symlink there.
        regards, tom lane

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
> > I don't know if this was discussed at the time ALTER SYSTEM was
> > implemented, but I have just discovered that if postgresql.auto.conf is
> > a symlink to a file elsewhere, ALTER SYSTEM will happily break that link
> > and write its own local copy. That strikes me as rather unfriendly. Why
> > not just truncate the file rather than unlink it as it's being
> > rewritten? Even if we don't want to do that a warning in the docs might
> > help users avoid the "mistake" I just made.
>
> Frankly, that behavior strikes me as a good idea.  There is no situation,
> IMV, where it's sane to try to put a symlink there.

I tend to agree with this.  Where a symlink makes sense for us are cases
where the configuration information is intentionally stored in the
correct directory structure (eg: /etc on Debian/Ubuntu and systems which
follow the FHS), and intended to be user-modifyable.  That's not the
case with ALTER SYSTEM.

What this request strikes me as asking for is the same as what I asked
for when this feature was originally going in- there should be a way to
disable it.  Allowing changes through ALTER SYSTEM which can result in
the system not being able to start is dangerous and confusing to many.

Thanks!

Stephen

On 11/02/2015 09:36 AM, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> I don't know if this was discussed at the time ALTER SYSTEM was
>> implemented, but I have just discovered that if postgresql.auto.conf is
>> a symlink to a file elsewhere, ALTER SYSTEM will happily break that link
>> and write its own local copy. That strikes me as rather unfriendly. Why
>> not just truncate the file rather than unlink it as it's being
>> rewritten? Even if we don't want to do that a warning in the docs might
>> help users avoid the "mistake" I just made.
> Frankly, that behavior strikes me as a good idea.  There is no situation,
> IMV, where it's sane to try to put a symlink there.
>
>             


OK. I was experimenting with some backup procedures, and I have since 
abandoned the approach I was following. However, it does seem like it's 
worth documenting the behaviour.

cheers

andrew

On 2015-11-02 09:43:02 -0500, Stephen Frost wrote:
> What this request strikes me as asking for is the same as what I asked
> for when this feature was originally going in- there should be a way to
> disable it.

You can just revoke permissions on the file if necessary. Results in the
expected
ERROR:  XX000: could not open file "../postgresql.auto.conf": Permission denied

* Andrew Dunstan (andrew@dunslane.net) wrote:
> On 11/02/2015 09:36 AM, Tom Lane wrote:
> >Andrew Dunstan <andrew@dunslane.net> writes:
> >>I don't know if this was discussed at the time ALTER SYSTEM was
> >>implemented, but I have just discovered that if postgresql.auto.conf is
> >>a symlink to a file elsewhere, ALTER SYSTEM will happily break that link
> >>and write its own local copy. That strikes me as rather unfriendly. Why
> >>not just truncate the file rather than unlink it as it's being
> >>rewritten? Even if we don't want to do that a warning in the docs might
> >>help users avoid the "mistake" I just made.
> >Frankly, that behavior strikes me as a good idea.  There is no situation,
> >IMV, where it's sane to try to put a symlink there.
>
> OK. I was experimenting with some backup procedures, and I have
> since abandoned the approach I was following. However, it does seem
> like it's worth documenting the behaviour.

No problem with that, certainly.

Thanks!

Stephen

* Andres Freund (andres@anarazel.de) wrote:
> On 2015-11-02 09:43:02 -0500, Stephen Frost wrote:
> > What this request strikes me as asking for is the same as what I asked
> > for when this feature was originally going in- there should be a way to
> > disable it.
>
> You can just revoke permissions on the file if necessary. Results in the
> expected
> ERROR:  XX000: could not open file "../postgresql.auto.conf": Permission denied

Yes, I know, but that's a really grotty way of offering a way to disable
ALTER SYSTEM.  It's also not exactly intuitive to someone reading the
release notes or working on upgrading their existing postgresql.conf.

Thanks!

Stephen

Stephen Frost <sfrost@snowman.net> writes:
> * Andres Freund (andres@anarazel.de) wrote:
>> You can just revoke permissions on the file if necessary. Results in the
>> expected
>> ERROR:  XX000: could not open file "../postgresql.auto.conf": Permission denied

> Yes, I know, but that's a really grotty way of offering a way to disable
> ALTER SYSTEM.  It's also not exactly intuitive to someone reading the
> release notes or working on upgrading their existing postgresql.conf.

While I won't stand in the way if someone is dead set on providing a
disable switch for ALTER SYSTEM, I fail to see the point of one.  It's
a superuser-only feature to begin with, and if you are handing out
superuser on production-critical installations to people you don't trust
completely, you need to have your head examined.

As a directly comparable example, I note that you yourself were in favor
of getting rid of rolcatupdate, which was the only mechanism we ever had
that could prevent a superuser from destroying the catalogs entirely
with a mistyped update --- consider "DELETE FROM pg_proc", for example,
which unlike ALTER SYSTEM there is simply no way to recover from.

How is it that we don't need rolcatupdate but we do need a way to shut
off ALTER SYSTEM?  Doesn't compute, IMO.
        regards, tom lane

On Mon, Nov 2, 2015 at 10:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Stephen Frost <sfrost@snowman.net> writes:
>> * Andres Freund (andres@anarazel.de) wrote:
>>> You can just revoke permissions on the file if necessary. Results in the
>>> expected
>>> ERROR:  XX000: could not open file "../postgresql.auto.conf": Permission denied
>
>> Yes, I know, but that's a really grotty way of offering a way to disable
>> ALTER SYSTEM.  It's also not exactly intuitive to someone reading the
>> release notes or working on upgrading their existing postgresql.conf.
>
> While I won't stand in the way if someone is dead set on providing a
> disable switch for ALTER SYSTEM, I fail to see the point of one.  It's
> a superuser-only feature to begin with, and if you are handing out
> superuser on production-critical installations to people you don't trust
> completely, you need to have your head examined.
>
> As a directly comparable example, I note that you yourself were in favor
> of getting rid of rolcatupdate, which was the only mechanism we ever had
> that could prevent a superuser from destroying the catalogs entirely
> with a mistyped update --- consider "DELETE FROM pg_proc", for example,
> which unlike ALTER SYSTEM there is simply no way to recover from.
>
> How is it that we don't need rolcatupdate but we do need a way to shut
> off ALTER SYSTEM?  Doesn't compute, IMO.

Yeah, I quite agree.  I think ALTER SYSTEM is a heck of a lot safer
than some things we are currently allowing superusers to do.  I'd
really like there to be some kind of GUC like allow_me_to_hose_myself.
If that's not set, direct catalog modifications should fail, even as
superuser.  I don't think rolcatupdate has ever prevented that, and I
think a GUC would be better than a user-level setting, because you
might easily want to turn it off on a per-session basis.

I have not seen much evidence that the problem with ALTER SYSTEM is
more than hypothetical.  I agree that you COULD configure it so that
the database server doesn't start, but the same is true of manually
editing postgresql.conf - but now that we're mostly out from under
operating system limits on shared memory, there aren't actually that
many ways to do that.  And, unlike vi $PGDATA/postgresql.conf, ALTER
SYSTEM actually has a pretty healthy amount of sanity checking built
in:

rhaas=# alter system set wal_level = arcive;
ERROR:  invalid value for parameter "wal_level": "arcive"
HINT:  Available values: minimal, archive, hot_standby, logical.

I would be willing to wager that a lot more people will hose their
systems by avoiding ALTER SYSTEM than will do so by using it.  Has
anyone ever run across a case where this actually happened?  What GUC
did the user set to cause the problem, and to what value?  I was able
to make it happen on my MacBook by setting max_connections to 100,000,
but that's unlikely to come up much in the real world, and 50,000
worked.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Nov 2, 2015 at 10:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> While I won't stand in the way if someone is dead set on providing a
>> disable switch for ALTER SYSTEM, I fail to see the point of one.

> I have not seen much evidence that the problem with ALTER SYSTEM is
> more than hypothetical.

Yeah, that's an independent line of argument that I also agree with.
Part of the reason why I was happy to throw rolcatupdate overboard was
that it had sat there in the code for twenty years without anyone ever
getting interested enough to turn it into a real feature.  And that
was because we hardly ever heard any reports of anyone actually doing
"DELETE FROM pg_proc" or whatever.  Just as Unix has never really grown
any protections against root doing "rm -rf /", I'm skeptical that we
need superuser training wheels of this ilk.

> I would be willing to wager that a lot more people will hose their
> systems by avoiding ALTER SYSTEM than will do so by using it.

Well, mumble --- the subtext I thought I was hearing from Stephen was
that he'd not give his DBAs write access on postgresql.conf either.
But yes, pushing people away from ALTER SYSTEM and towards manual editing
of postgresql.conf would be a foolish way of "improving safety".
        regards, tom lane

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > * Andres Freund (andres@anarazel.de) wrote:
> >> You can just revoke permissions on the file if necessary. Results in the
> >> expected
> >> ERROR:  XX000: could not open file "../postgresql.auto.conf": Permission denied
>
> > Yes, I know, but that's a really grotty way of offering a way to disable
> > ALTER SYSTEM.  It's also not exactly intuitive to someone reading the
> > release notes or working on upgrading their existing postgresql.conf.
>
> While I won't stand in the way if someone is dead set on providing a
> disable switch for ALTER SYSTEM, I fail to see the point of one.  It's
> a superuser-only feature to begin with, and if you are handing out
> superuser on production-critical installations to people you don't trust
> completely, you need to have your head examined.

There's a few different pieces to this.

The first is that there is no superuser without a running database and
there's a difference between the perfect admin and reality.  A DBA may
be trusted to be a superuser and to connect to the database to perform
superuser operations without fully understanding the shared_buffers
parameter or the requirements it places on the system.  Perhaps that
implies that they shouldn't be a superuser, but it's far different to
say "ALTER SYSTEM SET shared_buffers = X;" and "DELETE FROM pg_proc;" to
even minimally experienced DBAs, though both can lead to a PG instance
which can't be started.  As I argued on the ALTER SYSTEM thread, there
may be an organizational role distinction between the roles which are
responsible for starting the database and making sure that it's running
and the set of individuals who have superuser rights.  This is quite
common in environments where there is a central management (CM) system
where everything in /etc is really under the OPs perview, as is making
sure that things are running.  We made things more difficult for them
when we added ALTER SYSTEM because it's no longer the case that the GUCs
which can only be set through postgresql.conf can, well, only be set
through postgresql.conf.  Even a superuser can't control what
postgresql.conf is used to start or to make changes directly to the
postgresql.conf if it's owned by root or another user.

The second is that through shared_preload_libraries and the various
hooks which are available, it's possible to load libraries which do
reduce the capabilities of the superuser in PG.  If the superuser can
still use ALTER SYSTEM and remove those libraries from
shared_preload_libraries then that approach to limiting what actions can
be performed doesn't, ultimately, help.  Such a library could hook into
standard_ProcessUtility to deal with that, but a way to configure
postgresql.conf would be much cleaner.  A deeper hook in the
ALTER SYSTEM might be another approach, or one in the GUC system to
allow external libraries to control what various GUCs can be set to
(either via ALTER SYSTEM or through regular SET calls).

Lastly, I've long been a strong advocate of reducing the set of actions
only a superuser can perform by allowing non-superusers access to
more, by extending our privilege system to be more fine-grained.  Adding
more superuser-only operations goes against the grain a bit.  I'm not
suggesting that it would make sense for ALTER SYSTEM to be non-superuser
or that we shouldn't add superuser-only options, just that I'd like us
to offer more control during initial configuration of the system, to the
point where it's very clear what actions are allowed during ongoing
operations.

> As a directly comparable example, I note that you yourself were in favor
> of getting rid of rolcatupdate, which was the only mechanism we ever had
> that could prevent a superuser from destroying the catalogs entirely
> with a mistyped update --- consider "DELETE FROM pg_proc", for example,
> which unlike ALTER SYSTEM there is simply no way to recover from.

I was an advocate of getting rid of rolcatupdate because it wasn't
fully implemented and that didn't appear likely to change.  Further, I
can see cases where that could end up being more a problem than a help-
where the catalogs have been corrupted in some part and need to be
fixed.

> How is it that we don't need rolcatupdate but we do need a way to shut
> off ALTER SYSTEM?  Doesn't compute, IMO.

I'd like the ability to control all of the above, ultimately.  I don't
believe that we should be allowing the superuser to always modify the
catalog directly- and things like the sepgsql module can actually
address that and limit when the superuser is allowed to with better
granularity then what rolcatupdate provided (or was ever likely to
provide, being a single boolean role attribute).

Thanks!

Stephen

Robert, Tom,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > I would be willing to wager that a lot more people will hose their
> > systems by avoiding ALTER SYSTEM than will do so by using it.
>
> Well, mumble --- the subtext I thought I was hearing from Stephen was
> that he'd not give his DBAs write access on postgresql.conf either.
> But yes, pushing people away from ALTER SYSTEM and towards manual editing
> of postgresql.conf would be a foolish way of "improving safety".

This is all very environment specific.  Changes to postgresql.conf, in
many environments, go through a serious of tests before being deployed
by a CM system.  How do we accomplish the same kind of tests before
deploying a change with ALTER SYSTEM?  We provide no mechanism to do
that today.

What the whole ALTER SYSTEM discussion lacks is an appreciation of the
good CM practices which exist in many environments.  If I set up my CM
correctly, then I deploy new changes to the system via puppet or chef
only after those changes have been applied to the pre-production
environments which have identical system configurations.  Today, a
helpful DBA may make changes in production that make later changes by
the CM to postgresql.conf completely ineffective, leading to problems
and possibly even failures.

Suggesting that we get rid of superuser accounts or minimize them
further than already done is ineffective because we simply don't have
the fine grained controls which are needed to allow that.  I'm hopeful
that we'll get there and will continue to work towards it.

Thanks!

Stephen

Stephen Frost <sfrost@snowman.net> writes:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>> How is it that we don't need rolcatupdate but we do need a way to shut
>> off ALTER SYSTEM?  Doesn't compute, IMO.

> I'd like the ability to control all of the above, ultimately.  I don't
> believe that we should be allowing the superuser to always modify the
> catalog directly- and things like the sepgsql module can actually
> address that and limit when the superuser is allowed to with better
> granularity then what rolcatupdate provided (or was ever likely to
> provide, being a single boolean role attribute).

Mumble.  I have no objection to sepgsql deciding to disallow ALTER SYSTEM
--- after all, the entire point of that module is to enforce arbitrary
annoying restrictions ;-).  But I am not convinced that we need any other
way to turn it off.  As Robert points out, it's far *less* dangerous than
most other superuser-only features.

Also, disallowing ALTER SYSTEM altogether strikes me as an extremely
brute-force solution to any of the specific issues you mention.  If you're
worried about locking down shared_preload_libraries, for example, it would
be far better to lock down just that one variable.
        regards, tom lane

Stephen Frost <sfrost@snowman.net> writes:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>> Well, mumble --- the subtext I thought I was hearing from Stephen was
>> that he'd not give his DBAs write access on postgresql.conf either.
>> But yes, pushing people away from ALTER SYSTEM and towards manual editing
>> of postgresql.conf would be a foolish way of "improving safety".

> This is all very environment specific.  Changes to postgresql.conf, in
> many environments, go through a serious of tests before being deployed
> by a CM system.  How do we accomplish the same kind of tests before
> deploying a change with ALTER SYSTEM?  We provide no mechanism to do
> that today.

Sure, so if you have such a process, you tell your DBAs not to use ALTER
SYSTEM.  End of problem --- or if it isn't end of problem, you have HR
issues that the database cannot fix for you.

The core point here is that if you're handing people superuser and
expecting that they can't possibly circumvent any training-wheel-type
restrictions you then put on that, you're wrong.  In the end you'd
better trust that your DBAs know the process they're supposed to follow
and follow it.

It may be that, at some point in the future, we'll have this sliced and
diced fine enough that it's safe to allow some part of ALTER SYSTEM
functionality to be accessible to people you don't want to give full
superuser to.  But there's no such thing as "partial superuser", and
personally I believe that it would be a tremendous waste of time to
try to build such a feature.
        regards, tom lane

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > * Tom Lane (tgl@sss.pgh.pa.us) wrote:
> >> Well, mumble --- the subtext I thought I was hearing from Stephen was
> >> that he'd not give his DBAs write access on postgresql.conf either.
> >> But yes, pushing people away from ALTER SYSTEM and towards manual editing
> >> of postgresql.conf would be a foolish way of "improving safety".
>
> > This is all very environment specific.  Changes to postgresql.conf, in
> > many environments, go through a serious of tests before being deployed
> > by a CM system.  How do we accomplish the same kind of tests before
> > deploying a change with ALTER SYSTEM?  We provide no mechanism to do
> > that today.
>
> Sure, so if you have such a process, you tell your DBAs not to use ALTER
> SYSTEM.  End of problem --- or if it isn't end of problem, you have HR
> issues that the database cannot fix for you.

It's not that simple though, as you point out- ALTER SYSTEM for *some*
operations is just fine, but it isn't for other changes.  That's one of
the difficulties with it.

Further, there's a level of cooperation required between the
postgresql.conf and the system, at a level which, in my mind anyway,
falls under the purview of the system adminsitration team instead of the
DBA team.  In larger organizations, those can be quite distinct sets.
If the DBA team is changing values via ALTER SYSTEM then, again, you can
run into problems when the sysadmin team or the CM system makes a
change.

Consider cases such as listen_addresses or, more generally, startup-time
options.  I brought all of this up on that thread and pointed out that
there are a bunch of postgresql.conf configuration options where system
level changes have to be made at the same time, which is why they make
sense to be put under a CM system which controls both the
postgresql.conf and the system configuration (what the IP address of the
system is, for example).

> The core point here is that if you're handing people superuser and
> expecting that they can't possibly circumvent any training-wheel-type
> restrictions you then put on that, you're wrong.  In the end you'd
> better trust that your DBAs know the process they're supposed to follow
> and follow it.

I don't view this as being about circumvention or training-wheel-type
restrictions.

> It may be that, at some point in the future, we'll have this sliced and
> diced fine enough that it's safe to allow some part of ALTER SYSTEM
> functionality to be accessible to people you don't want to give full
> superuser to.  But there's no such thing as "partial superuser", and
> personally I believe that it would be a tremendous waste of time to
> try to build such a feature.

I certainly look forward to having more fine grained control, to the
point where I'd like to be able to run a system reasonably without an
active superuser login.  Having superusers logging into production
running databases is extremely dangerous.  What I have seen happening,
in multiple organizations, is a move to proxy everything going to the
database through some other system which attempts to vet and verify that
the action is acceptable (this also happens to offer up much better
auditing than what we have today).  I feel confident that we can provide
a better solution than those proxy-based approaches.

Thanks!

Stephen

On 11/02/2015 09:24 AM, Stephen Frost wrote:
> I certainly look forward to having more fine grained control, to the
> point where I'd like to be able to run a system reasonably without an
> active superuser login.  Having superusers logging into production
> running databases is extremely dangerous.  What I have seen happening,
> in multiple organizations, is a move to proxy everything going to the
> database through some other system which attempts to vet and verify that
> the action is acceptable (this also happens to offer up much better
> auditing than what we have today).

I've seen this *repeatedly* in the past few years as well.

> I feel confident that we can provide a better solution than those proxy-based approaches.

+1

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

On 11/02/2015 08:33 AM, Stephen Frost wrote:
> A deeper hook in the ALTER SYSTEM might be another approach, or one
> in the GUC system to allow external libraries to control what various
> GUCs can be set to (either via ALTER SYSTEM or through regular SET
> calls).


I have run into multiple situations where having finer grained control
over the setting of GUCs would have been very useful, and spoken to many
others in the community with the same wish.


> I'd like the ability to control all of the above, ultimately.  I
> don't believe that we should be allowing the superuser to always
> modify the catalog directly

+10

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

On Mon, Nov 2, 2015 at 11:39 AM, Stephen Frost <sfrost@snowman.net> wrote:
> This is all very environment specific.  Changes to postgresql.conf, in
> many environments, go through a serious of tests before being deployed
> by a CM system.  How do we accomplish the same kind of tests before
> deploying a change with ALTER SYSTEM?  We provide no mechanism to do
> that today.

We provide no mechanism to put the changes to put postgresql.conf
changes through a series of tests before being deployed by a CM
system, either.  But you can do that if you want.

Two different methods of restricting ALTER SYSTEM have already been
discussed on this thread: one using file permissions, and the other
using ProcessUtility_hook.  I personally think that's good enough.
It's true that you could have a separate GUC for it, but then somebody
could lock themselves out by turning the GUC on using ALTER SYSTEM, so
now you've made things easier for one group of users while creating a
new pitfall for another group of users.  I'm not sure we really come
out ahead, there.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 11/02/2015 06:36 AM, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> I don't know if this was discussed at the time ALTER SYSTEM was 
>> implemented, but I have just discovered that if postgresql.auto.conf is 
>> a symlink to a file elsewhere, ALTER SYSTEM will happily break that link 
>> and write its own local copy. That strikes me as rather unfriendly. Why 
>> not just truncate the file rather than unlink it as it's being 
>> rewritten? Even if we don't want to do that a warning in the docs might 
>> help users avoid the "mistake" I just made.
> 
> Frankly, that behavior strikes me as a good idea.  There is no situation,
> IMV, where it's sane to try to put a symlink there.

So, just a doc patch then?

Since we have both include files and config_dir, I really don't
understand why anyone symlinks conf files anymore.  Anyone care to
enlighten me?

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Mon, Nov 2, 2015 at 11:39 AM, Stephen Frost <sfrost@snowman.net> wrote:
> > This is all very environment specific.  Changes to postgresql.conf, in
> > many environments, go through a serious of tests before being deployed
> > by a CM system.  How do we accomplish the same kind of tests before
> > deploying a change with ALTER SYSTEM?  We provide no mechanism to do
> > that today.
>
> We provide no mechanism to put the changes to put postgresql.conf
> changes through a series of tests before being deployed by a CM
> system, either.  But you can do that if you want.

I'm trying to understand what you're getting at above and how it is
actually an argument against my point, and I'm not able to do so.

I wasn't suggesting that we need to provide a way for users to vet the
changes to postgresql.conf but was rather saying that having ALTER
SYSTEM means that if a user already has such a system, it can end up
being defeated, as it were, by a user using ALTER SYSTEM.

> Two different methods of restricting ALTER SYSTEM have already been
> discussed on this thread: one using file permissions, and the other
> using ProcessUtility_hook.  I personally think that's good enough.

The issue which I have with these suggestions is that one requires users
to install an as-yet-unwritten module and the other is to hack with
permissions in the data directory.  As we've all seen, people playing in
$PGDATA is generally a bad idea.

> It's true that you could have a separate GUC for it, but then somebody
> could lock themselves out by turning the GUC on using ALTER SYSTEM, so
> now you've made things easier for one group of users while creating a
> new pitfall for another group of users.  I'm not sure we really come
> out ahead, there.

We wouldn't have to make it a GUC.  If we decided to anyway, we could
certainly disable the ability to modify it through ALTER SYSTEM, or
ignore it if we find it in postgresql.auto.conf as it surely wouldn't
make any sense to have it there.

The original idea here was to add an include line for .auto.conf.  That
would certainly have made me happy.  Unfortunately, by not doing that,
we've painted ourselves into a corner that's rather ugly to get out of,
since we can't now say that such an include line is required for
ALTER SYSTEM to work.

It wasn't my intent to get into such a discussion regarding ALTER SYSTEM
at this time- there are more important activities at hand, and the ship
has largely sailed on what I had been asking for originally.  Perhaps we
can discuss it again in the future but I'm certainly happy to drop the
it for now.

Thanks!

Stephen

On Mon, Nov 2, 2015 at 3:41 PM, Stephen Frost <sfrost@snowman.net> wrote:
>> Two different methods of restricting ALTER SYSTEM have already been
>> discussed on this thread: one using file permissions, and the other
>> using ProcessUtility_hook.  I personally think that's good enough.
>
> The issue which I have with these suggestions is that one requires users
> to install an as-yet-unwritten module and the other is to hack with
> permissions in the data directory.  As we've all seen, people playing in
> $PGDATA is generally a bad idea.

Well, fair enough.  I think somebody could write that module in about
an hour, though.  All you have to do is latch onto ProcessUtility_hook
and throw an error if you've got yourself an AlterSystemStmt.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Mon, Nov 2, 2015 at 10:13 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> I think that is the sensible way to deal with this and any other such
> parameters.  We already have a way to disallow setting of individual
> parameters (GUC_DISALLOW_IN_AUTO_FILE) via Alter System.
> Currently we disallow to set data_directory via this mechanism and I think
> we can do the same for other parameters if required.  Do you think we
> should do some investigation/analysis about un-safe parameters rather
> then doing it in retail fashion?

-1.

This was discussed before, and I feel about it now the same way I felt
about it then: disallowing all GUCs that could potentially cause the
server not to start would make ALTER SYSTEM a whole lot less useful.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Nov 2, 2015 at 10:13 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> I think that is the sensible way to deal with this and any other such
>> parameters.  We already have a way to disallow setting of individual
>> parameters (GUC_DISALLOW_IN_AUTO_FILE) via Alter System.
>> Currently we disallow to set data_directory via this mechanism and I think
>> we can do the same for other parameters if required.  Do you think we
>> should do some investigation/analysis about un-safe parameters rather
>> then doing it in retail fashion?

> -1.

> This was discussed before, and I feel about it now the same way I felt
> about it then: disallowing all GUCs that could potentially cause the
> server not to start would make ALTER SYSTEM a whole lot less useful.

I definitely agree that unconditionally disallowing "unsafe" parameters
in ALTER SYSTEM would be counterproductive.  data_directory is disallowed
there only because it's nonsensical: you can't even find the auto.conf
file until you've settled on the data_directory.

However, where I thought this discussion was going was to allow admins
to selectively disable particular parameters from being set that way.
Whether a particular installation finds locking down
shared_preload_libraries to be more useful than allowing it to be set
doesn't seem to me to be a one-size-fits-all question.  So at least in
principle I'd be okay with a feature to control that.  But maybe it's
sufficient to say "you can use sepgsql to restrict that".  Not every
feature needs to be in core.
        regards, tom lane

On Mon, Nov 2, 2015 at 10:40 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Mon, Nov 2, 2015 at 10:13 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>>> I think that is the sensible way to deal with this and any other such
>>> parameters.  We already have a way to disallow setting of individual
>>> parameters (GUC_DISALLOW_IN_AUTO_FILE) via Alter System.
>>> Currently we disallow to set data_directory via this mechanism and I think
>>> we can do the same for other parameters if required.  Do you think we
>>> should do some investigation/analysis about un-safe parameters rather
>>> then doing it in retail fashion?
>
>> -1.
>
>> This was discussed before, and I feel about it now the same way I felt
>> about it then: disallowing all GUCs that could potentially cause the
>> server not to start would make ALTER SYSTEM a whole lot less useful.
>
> I definitely agree that unconditionally disallowing "unsafe" parameters
> in ALTER SYSTEM would be counterproductive.  data_directory is disallowed
> there only because it's nonsensical: you can't even find the auto.conf
> file until you've settled on the data_directory.
>
> However, where I thought this discussion was going was to allow admins
> to selectively disable particular parameters from being set that way.
> Whether a particular installation finds locking down
> shared_preload_libraries to be more useful than allowing it to be set
> doesn't seem to me to be a one-size-fits-all question.  So at least in
> principle I'd be okay with a feature to control that.  But maybe it's
> sufficient to say "you can use sepgsql to restrict that".  Not every
> feature needs to be in core.

I really think this is a fine use of a custom contrib module.  If it's
important, we can even include that contrib module in the core
distribution as an example of how to restrict access to specific
commands or subcommands that a user may want to prevent in their
environment.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Tue, Nov 3, 2015 at 6:08 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Mon, Nov 2, 2015 at 3:41 PM, Stephen Frost <sfrost@snowman.net> wrote:
>>> Two different methods of restricting ALTER SYSTEM have already been
>>> discussed on this thread: one using file permissions, and the other
>>> using ProcessUtility_hook.  I personally think that's good enough.
>>
>> The issue which I have with these suggestions is that one requires users
>> to install an as-yet-unwritten module and the other is to hack with
>> permissions in the data directory.  As we've all seen, people playing in
>> $PGDATA is generally a bad idea.
>
> Well, fair enough.  I think somebody could write that module in about
> an hour, though.  All you have to do is latch onto ProcessUtility_hook
> and throw an error if you've got yourself an AlterSystemStmt.

BTW, I wrote that module 9 month before for pleasure.
https://github.com/MasaoFujii/pg_disallow_utility

If we want to prevent superuser from modifying the configuration file,
not only ALTER SYSTEM but also COPY PROGRAM should be restricted.
Otherwise, superuser can execute arbitrary OS command via COPY PROGRAM
and easily modify any file.

Regards,

-- 
Fujii Masao