Horton

Thread

  1. ALTER USER SET log_* not allowed...

    Sean Chittenden <sean@chittenden.org> — 2004-11-09T08:23:52Z

    I've got a particular database account that connects/disconnects 
hundreds of times a second, literally (email delivery agent).  Being 
the ever ready logger that I am, I have a number of logging bits turned 
on to help me identify problems when they come up.  In this case, I'm 
not worried about a problem and want to turn off most logging for this 
particular user.  I can issue an ALTER USER [usr] SET log_* = 
false/none in all cases, except for log_duration.

db=# ALTER USER dbmail SET log_disconnections = false;
ERROR:  parameter "log_disconnections" cannot be set after connection 
start

:-/  I use disconnections as the way of noting the number of 
connections.  Regardless, I think I'm off to the races... BUT! not 
quite:

INFO:  permission denied to set parameter "log_statement"
HINT:  Must be superuser to increase this value.
INFO:  permission denied to set parameter "log_duration"
HINT:  Must be superuser to change this value to false.

doh!  So much for that idea.  There's no real way to have some 
useconfig variables run by the super user and some run by the session 
user.  My workaround is to have the program call a SECURITY DEFINER 
function, but I'd be nice to be able to remove that hack.  Anyway, 
something to consider next time the system catalogs are being tweaked.  
-sc

-- 
Sean Chittenden

  2. Re: ALTER USER SET log_* not allowed...

    Tom Lane <tgl@sss.pgh.pa.us> — 2004-11-09T15:03:38Z

    Sean Chittenden <sean@chittenden.org> writes:
> doh!  So much for that idea.  There's no real way to have some 
> useconfig variables run by the super user and some run by the session 
> user.  My workaround is to have the program call a SECURITY DEFINER 
> function, but I'd be nice to be able to remove that hack.

Yeah, the whole concept of USERLIMIT GUC variables is fairly dubious,
because of the fact that we have to be able to set these values at times
when we don't necessarily know whether the user is a superuser or not.

It occurs to me though that we might be able to handle this one case.
Variable values coming from ALTER USER/DATABASE SET ... are currently
treated as untrusted (because of the ordering of enum GucSource).
However, we *also* test whether the user executing the ALTER has
permissions:

regression=>  ALTER USER tgl SET log_duration = false;
ERROR:  permission denied to set parameter "log_duration"
HINT:  Must be superuser to change this value to false.

At first glance it seems like the ALTER-time test is a sufficient
security check and therefore PGC_S_DATABASE and PGC_S_USER sources
could be treated as privileged.

This doesn't entirely work, because the USERLIMIT checks are context
sensitive: if the current config-file setting of log_duration is FALSE
then I will be allowed to install the above per-user setting, and
without the time-of-use check it would override any change in the global
setting that the DBA tried to make.

It strikes me that a more useful definition would be to say that you
must be superuser, period, to install an ALTER USER/DATABASE value for
any USERLIMIT variable; and then we could treat these values as
privileged for USERLIMIT purposes.  This would lose the ability for
non-superusers to set allowable values for themselves this way, but
I think the case we'd gain is more useful.

Comments?

			regards, tom lane

  3. Re: ALTER USER SET log_* not allowed...

    Sean Chittenden <sean@chittenden.org> — 2004-11-09T18:08:05Z

    >> doh!  So much for that idea.  There's no real way to have some
>> useconfig variables run by the super user and some run by the session
>> user.  My workaround is to have the program call a SECURITY DEFINER
>> function, but I'd be nice to be able to remove that hack.
>
> Yeah, the whole concept of USERLIMIT GUC variables is fairly dubious,
> because of the fact that we have to be able to set these values at 
> times
> when we don't necessarily know whether the user is a superuser or not.
[snip]
> It strikes me that a more useful definition would be to say that you
> must be superuser, period, to install an ALTER USER/DATABASE value for
> any USERLIMIT variable; and then we could treat these values as
> privileged for USERLIMIT purposes.  This would lose the ability for
> non-superusers to set allowable values for themselves this way, but
> I think the case we'd gain is more useful.
>
> Comments?

Oh!  Please!  I thought about suggesting that but didn't think it'd 
pass your litmus test and figured a pg_shadow.useconfig and an 
pg_shadow.admconfig would be received better, but, absolutely!  The 
reason I hesitated to suggest such a change was SET search_path = foo;, 
which a user should be able to set on their own.  Sure it'd be easy to 
have it a one-off exception, but then it'd be a one-off exception and 
having an 'ALTER USER usr ADMIN SET guc = val' would skirt that issue 
completely.  That's the only concern that I have.

How about this, leave the existing system in place, but since useconfig 
is just a TEXT[], if an admin sets the value (ALTER USER usr ADMIN 
SET), prefix the guc name with an 'A:'.  As things currently stand, 
useconfig looks like, '{search_path=dbmail,log_statements=none}', but 
after, it'd look like: '{search_path=dbmail,A:log_statements=none}'.  
Then log_statements gets set with admin privs, where as search_path is 
set with user privs.  Pros:

*) Preserves backwards compatibility with existing databases and the 
GUC security infrastructure (no need to bump catalog version)
*) Allows GUC variables to be set with differing permission levels and 
still be set by the user
*) At ALTER USER time, a permission message can be returned that tells 
the user a GUC has already been set by the admin, go bug them to change 
this value

Cons:

*) Places a special value on the prefix of GUC variable names.
*) Requires adding a new keyword in the ALTER USER syntax.
*) Feels a tad like a "miscellaneous" column that is on the verge of 
being abused.

Then again, isn't it on the horizon to have GUC reworked?  Maybe this 
would be an acceptable addition.  *shrug*  Just an idea. -sc

-- 
Sean Chittenden

  4. Re: ALTER USER SET log_* not allowed...

    Tom Lane <tgl@sss.pgh.pa.us> — 2004-11-09T18:58:07Z

    Sean Chittenden <sean@chittenden.org> writes:
>> It strikes me that a more useful definition would be to say that you
>> must be superuser, period, to install an ALTER USER/DATABASE value for
>> any USERLIMIT variable; and then we could treat these values as
>> privileged for USERLIMIT purposes.  This would lose the ability for
>> non-superusers to set allowable values for themselves this way, but
>> I think the case we'd gain is more useful.

> Oh!  Please!  I thought about suggesting that but didn't think it'd 
> pass your litmus test and figured a pg_shadow.useconfig and an 
> pg_shadow.admconfig would be received better, but, absolutely!  The 
> reason I hesitated to suggest such a change was SET search_path = foo;, 
> which a user should be able to set on their own.

But search_path is not USERLIMIT.  The only variables that are in that
category are the ones that control logging.

Bruce and I were chatting about this on the phone today, and we were
seriously considering a more radical proposal: get rid of the whole
concept of USERLIMIT variables, and make the logging variables be plain
SUSET (ie, only superusers can change 'em).  This would eliminate the
current ability of a non-superuser to increase the logging verbosity
of his session, but it's not real clear that that's such a good idea
anyway.  (Cranking the log verbosity up far past what the DBA wants
could be seen as a primitive form of DOS attack; and anyway, if you are
not a superuser then you can't see what's in the log, so why should
you care what the verbosity is, much less be able to affect it?)  Given
the code complexity of the USERLIMIT stuff and the number of bugs
already found in it, getting rid of it seems awfully attractive.

Without USERLIMIT, we could easily change the rules to make ALTER USER
work the way you want: we'd say you have to be superuser to issue ALTER
USER or ALTER DATABASE with a SUSET variable (already true), and then
the value can be adopted at session start in all cases since we can then
treat pg_shadow and pg_database as secure sources.

			regards, tom lane

  5. Re: ALTER USER SET log_* not allowed...

    Sean Chittenden <sean@chittenden.org> — 2004-11-09T19:05:48Z

    > Without USERLIMIT, we could easily change the rules to make ALTER USER
> work the way you want: we'd say you have to be superuser to issue ALTER
> USER or ALTER DATABASE with a SUSET variable (already true), and then
> the value can be adopted at session start in all cases since we can 
> then
> treat pg_shadow and pg_database as secure sources.

Nevermind, you win.  That's far more elegant/easier...  is 8.0 looking 
like a good time to make this break from tradition?  -sc

-- 
Sean Chittenden

  6. Re: ALTER USER SET log_* not allowed...

    Andrew McMillan <andrew@catalyst.net.nz> — 2004-11-10T10:30:32Z

    On Tue, 2004-11-09 at 13:58 -0500, Tom Lane wrote:
> 
> Bruce and I were chatting about this on the phone today, and we were
> seriously considering a more radical proposal: get rid of the whole
> concept of USERLIMIT variables, and make the logging variables be plain
> SUSET (ie, only superusers can change 'em).  This would eliminate the
> current ability of a non-superuser to increase the logging verbosity
> of his session, but it's not real clear that that's such a good idea
> anyway.  (Cranking the log verbosity up far past what the DBA wants
> could be seen as a primitive form of DOS attack; and anyway, if you are
> not a superuser then you can't see what's in the log, so why should
> you care what the verbosity is, much less be able to affect it?)  Given
> the code complexity of the USERLIMIT stuff and the number of bugs
> already found in it, getting rid of it seems awfully attractive.

The current functionality could be useful inside particular code paths
of an application, where you want to increase the log verbosity in a
particular part of the code, when it (unpredictably) happens, without
nuking the logs entirely.

Of course you are superuser when you review such logs, but I wouldn't
usually want the db connection from the application to have to run as
superuser if I could help it...  especially not a web application.

Regards,
				Andrew McMillan.

-------------------------------------------------------------------------
Andrew @ Catalyst .Net .NZ  Ltd,  PO Box 11-053, Manners St,  Wellington
WEB: http://catalyst.net.nz/            PHYS: Level 2, 150-154 Willis St
DDI: +64(4)803-2201      MOB: +64(272)DEBIAN      OFFICE: +64(4)499-2267
              Survey for nothing with http://survey.net.nz/
-------------------------------------------------------------------------

  7. Re: ALTER USER SET log_* not allowed...

    Tom Lane <tgl@sss.pgh.pa.us> — 2004-11-10T16:41:35Z

    Andrew McMillan <andrew@catalyst.net.nz> writes:
> The current functionality could be useful inside particular code paths
> of an application, where you want to increase the log verbosity in a
> particular part of the code, when it (unpredictably) happens, without
> nuking the logs entirely.
> Of course you are superuser when you review such logs, but I wouldn't
> usually want the db connection from the application to have to run as
> superuser if I could help it...  especially not a web application.

Sure.  There is a workaround for that though, which is to provide a
SECURITY DEFINER function for the app to call that will adjust the
logging level for it, rather than trying to do the SET directly in
unprivileged code.

			regards, tom lane

  8. Re: ALTER USER SET log_* not allowed...

    Bruce Momjian <pgman@candle.pha.pa.us> — 2004-11-10T16:53:18Z

    Tom Lane wrote:
> Andrew McMillan <andrew@catalyst.net.nz> writes:
> > The current functionality could be useful inside particular code paths
> > of an application, where you want to increase the log verbosity in a
> > particular part of the code, when it (unpredictably) happens, without
> > nuking the logs entirely.
> > Of course you are superuser when you review such logs, but I wouldn't
> > usually want the db connection from the application to have to run as
> > superuser if I could help it...  especially not a web application.
> 
> Sure.  There is a workaround for that though, which is to provide a
> SECURITY DEFINER function for the app to call that will adjust the
> logging level for it, rather than trying to do the SET directly in
> unprivileged code.

But if they go that way can it done securely, turned on and off?

-- 
  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

  9. Re: ALTER USER SET log_* not allowed...

    Tom Lane <tgl@sss.pgh.pa.us> — 2004-11-10T17:55:47Z

    Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> Sure.  There is a workaround for that though, which is to provide a
>> SECURITY DEFINER function for the app to call that will adjust the
>> logging level for it, rather than trying to do the SET directly in
>> unprivileged code.

> But if they go that way can it done securely, turned on and off?

Why not?  You can put whatever restrictions you like in such a function.

It'd certainly be more "secure" than the existing USERLIMIT behavior,
because the DBA can decide exactly what policy he wants and code it
into the function he gives his users (maybe even multiple functions for
different users).  USERLIMIT effectively dictates to the DBA what will
be allowed.

			regards, tom lane

  10. Re: ALTER USER SET log_* not allowed...

    Bruce Momjian <pgman@candle.pha.pa.us> — 2004-11-10T18:17:16Z

    Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Tom Lane wrote:
> >> Sure.  There is a workaround for that though, which is to provide a
> >> SECURITY DEFINER function for the app to call that will adjust the
> >> logging level for it, rather than trying to do the SET directly in
> >> unprivileged code.
> 
> > But if they go that way can it done securely, turned on and off?
> 
> Why not?  You can put whatever restrictions you like in such a function.
> 
> It'd certainly be more "secure" than the existing USERLIMIT behavior,
> because the DBA can decide exactly what policy he wants and code it
> into the function he gives his users (maybe even multiple functions for
> different users).  USERLIMIT effectively dictates to the DBA what will
> be allowed.

But right now the userlimit codes allows a non-super user to turn
logging on only if it was off, and he can then turn it off again.

I assume to do this in a function you would have to create a temp table
to store the original setting?  Doesn't sound trivial, nor does it sound
like something someone is going to write just for a single debug session.

-- 
  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

  11. Re: ALTER USER SET log_* not allowed...

    Tom Lane <tgl@sss.pgh.pa.us> — 2004-11-10T18:19:38Z

    Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> Why not?  You can put whatever restrictions you like in such a function.

> I assume to do this in a function you would have to create a temp table
> to store the original setting?

Not at all.  You could use "RESET foo" to see what the "original" value
of foo was.

			regards, tom lane

  12. Re: ALTER USER SET log_* not allowed...

    Bruce Momjian <pgman@candle.pha.pa.us> — 2004-11-10T23:38:43Z

    Andrew McMillan wrote:
-- Start of PGP signed section.
> On Tue, 2004-11-09 at 13:58 -0500, Tom Lane wrote:
> > 
> > Bruce and I were chatting about this on the phone today, and we were
> > seriously considering a more radical proposal: get rid of the whole
> > concept of USERLIMIT variables, and make the logging variables be plain
> > SUSET (ie, only superusers can change 'em).  This would eliminate the
> > current ability of a non-superuser to increase the logging verbosity
> > of his session, but it's not real clear that that's such a good idea
> > anyway.  (Cranking the log verbosity up far past what the DBA wants
> > could be seen as a primitive form of DOS attack; and anyway, if you are
> > not a superuser then you can't see what's in the log, so why should
> > you care what the verbosity is, much less be able to affect it?)  Given
> > the code complexity of the USERLIMIT stuff and the number of bugs
> > already found in it, getting rid of it seems awfully attractive.
> 
> The current functionality could be useful inside particular code paths
> of an application, where you want to increase the log verbosity in a
> particular part of the code, when it (unpredictably) happens, without
> nuking the logs entirely.
> 
> Of course you are superuser when you review such logs, but I wouldn't
> usually want the db connection from the application to have to run as
> superuser if I could help it...  especially not a web application.

As much as I would like the URERLIMIT hacks removed for 8.0 I am thinking
we are too far along in release and don't have enough time to figure out
how to do the security definer function cleanly.  I am thinking we
should wait for 8.1 and maybe have the USERLIMIT capability integrated
intot a security definer capability function we ship with our code.

-- 
  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

  13. Re: ALTER USER SET log_* not allowed...

    Tom Lane <tgl@sss.pgh.pa.us> — 2004-11-10T23:44:02Z

    Bruce Momjian <pgman@candle.pha.pa.us> writes:
> As much as I would like the URERLIMIT hacks removed for 8.0 I am thinking
> we are too far along in release and don't have enough time to figure out
> how to do the security definer function cleanly.

What does that have to do with it?  We aren't the ones who would be
writing such a function.

			regards, tom lane

  14. Re: ALTER USER SET log_* not allowed...

    Bruce Momjian <pgman@candle.pha.pa.us> — 2004-11-11T02:24:59Z

    Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > As much as I would like the URERLIMIT hacks removed for 8.0 I am thinking
> > we are too far along in release and don't have enough time to figure out
> > how to do the security definer function cleanly.
> 
> What does that have to do with it?  We aren't the ones who would be
> writing such a function.

I am thinking it is too much to remove such functionality and require
others to write it.  They will not write it until they are debugging
something, and then they will not have time to do it.  I think we need
to provide such functionality in a cleaner way than just saying "write a
function".  I think it needs more thought and we don't have the time.

-- 
  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