This is not gonna do

Started by Tom Lanealmost 23 years ago5 messageshackers
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

The recent change to make log_min_messages SUSET provokes the following
behavior:

$ export PGOPTIONS="-d 5"
$ psql
psql: FATAL: 'log_min_messages': permission denied
$

Considering that I *am* superuser, this is quite unacceptable.
If you don't want to revert the change, propose another solution.

regards, tom lane

#2Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#1)
GUC --- prevent non-super user changes

Tom Lane wrote:

The recent change to make log_min_messages SUSET provokes the following
behavior:

$ export PGOPTIONS="-d 5"
$ psql
psql: FATAL: 'log_min_messages': permission denied
$

Considering that I *am* superuser, this is quite unacceptable.
If you don't want to revert the change, propose another solution.

Here is a proposed fix for the new SUSET of various variables. The
solution is to create a new GUC context called PGC_USERLIMIT, which
limits changes by non-super users. For example, non-super users can
turn on logging, but can't turn it off, and log_min_* logging can have
added output, but not less output.

The first part of the patch prevent client PGOPTIONS from lowering the
debug level. The second part adds this new GUC context, then allows it
to be set properly. The tests are in two parts --- the first prevents
non-super users from changing the value inappropriately, and the second
allows postgresql.conf changes to apply to existing backends, i.e. if
postgresql.conf turns logging off via SET, turning it on via
postgresql.conf should propogate to the client, because the client can't
turn something off that the admin wants turned on --- that is the tricky
part that we have to be able to handle the settings in any order.

Peter, how does this look? Is reset_val the proper value to test?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Attachments:

/pgpatches/userlimittext/plainDownload+170-98
#3Bruce Momjian
bruce@momjian.us
In reply to: Bruce Momjian (#2)
Re: GUC --- prevent non-super user changes

Sorry, I forgot --- this should have gone only to patches.

---------------------------------------------------------------------------

Bruce Momjian wrote:

Tom Lane wrote:

The recent change to make log_min_messages SUSET provokes the following
behavior:

$ export PGOPTIONS="-d 5"
$ psql
psql: FATAL: 'log_min_messages': permission denied
$

Considering that I *am* superuser, this is quite unacceptable.
If you don't want to revert the change, propose another solution.

Here is a proposed fix for the new SUSET of various variables. The
solution is to create a new GUC context called PGC_USERLIMIT, which
limits changes by non-super users. For example, non-super users can
turn on logging, but can't turn it off, and log_min_* logging can have
added output, but not less output.

The first part of the patch prevent client PGOPTIONS from lowering the
debug level. The second part adds this new GUC context, then allows it
to be set properly. The tests are in two parts --- the first prevents
non-super users from changing the value inappropriately, and the second
allows postgresql.conf changes to apply to existing backends, i.e. if
postgresql.conf turns logging off via SET, turning it on via
postgresql.conf should propogate to the client, because the client can't
turn something off that the admin wants turned on --- that is the tricky
part that we have to be able to handle the settings in any order.

Peter, how does this look? Is reset_val the proper value to test?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#4Robert Treat
xzilla@users.sourceforge.net
In reply to: Bruce Momjian (#2)
Re: [HACKERS] GUC --- prevent non-super user changes

On Wed, 2003-06-11 at 01:01, Bruce Momjian wrote:

Here is a proposed fix for the new SUSET of various variables. The
solution is to create a new GUC context called PGC_USERLIMIT, which
limits changes by non-super users. For example, non-super users can
turn on logging, but can't turn it off, and log_min_* logging can have
added output, but not less output.

Is there a danger here that users can crank logging up to the max and
either crash a server due to i/o load, or perhaps create enough "noise"
to cover tracks of something malicious?

Robert Treat
--
Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL

#5Bruce Momjian
bruce@momjian.us
In reply to: Robert Treat (#4)
Re: [HACKERS] GUC --- prevent non-super user changes

Robert Treat wrote:

On Wed, 2003-06-11 at 01:01, Bruce Momjian wrote:

Here is a proposed fix for the new SUSET of various variables. The
solution is to create a new GUC context called PGC_USERLIMIT, which
limits changes by non-super users. For example, non-super users can
turn on logging, but can't turn it off, and log_min_* logging can have
added output, but not less output.

Is there a danger here that users can crank logging up to the max and
either crash a server due to i/o load, or perhaps create enough "noise"
to cover tracks of something malicious?

There perhaps is, but if they can connect to the database, I would think
there would be other worse things they can do.

The base problem is that we don't know if the person is a super user
until we connect to pg_shadow, and this is much later than when we
process the flags and PGOPTIONS packet, so the patch seemed like the
cleanest way to go. We could have delayed the setting of those
variables, but there are some variables that have to be set _before_ we
connect to pg_shadow, so it would get tricky.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073