Re: [BUGS] 7.4 beta 1: SET log_statement=false

Non superusers can set log_statement to true but can't set it
back to false even if log_statement was false at the begining of a
connection.

I think lambda users should be able to revert log_statement to
false when false is the default setting.

--
%!PS
297.6 420.9 translate 90 rotate 0 setgray gsave 0 1 1{pop 0 180 moveto 100
180 170 100 170 -10 curveto 180 -9 180 -9 190 -10 curveto 190 100 100 180
0 180 curveto fill 180 rotate}for grestore/Bookman-LightItalic findfont
240 scalefont setfont -151.536392 -63.7998886 moveto (bp)show showpage

Bertrand Petit <pgsql-bugs(at)phoe(dot)frmug(dot)org> writes:
> Non superusers can set log_statement to true but can't set it
> back to false even if log_statement was false at the begining of a
> connection.

Yeah. I think that the restrictions for USERLIMIT variables ought to
compare against the reset_val, not the session_val.

regards, tom lane

[ Moved to hackers.]

Tom Lane wrote:
> Bertrand Petit <pgsql-bugs(at)phoe(dot)frmug(dot)org> writes:
> > Non superusers can set log_statement to true but can't set it
> > back to false even if log_statement was false at the begining of a
> > connection.
>
> Yeah. I think that the restrictions for USERLIMIT variables ought to
> compare against the reset_val, not the session_val.

I wish reset_val would fix it. This is the code being used:

/* Limit non-superuser changes */
if (record->context == PGC_USERLIMIT &&
source > PGC_S_UNPRIVILEGED &&
newval < conf->session_val &&
!superuser())
{
ereport(elevel,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied to set option \"%s\"",
name),
errhint("Must be superuser to change this value to false.")));
return false;
}

It basically says:

o is this a USERLIMIT variable (restrict non-super users)
o is the new value being set in a user-accessible way
o is the value being set to true from false
o is this person not a super-user

I tried adding this line:

record->session_source < PGC_S_UNPRIVILEGED &&

and it does allow you to set the variable to false if you have set it to
true in your session, but it also allows you to set it to false if it is
set to true in postgresql.conf. You do this by setting it to 'true'
_then_ to 'false' in your session. The reason this works is that there
is no history in the GUC code to record the value this variable was set
at various locations. Once you set it to 'true' in your session, the
system remembers that, and forgets it was also set to true in
postgresql.conf, and therefore allows you to set it to false.

The documentation is vague on this point. It says:

Only superusers can turn off this option if it is enabled by
the administrator.

I think I should remove the part about administrators, and just say:

Only superusers can turn off this option.

or is that worse? The administrator part makes it clear why you
shouldn't be able to set it to false.

Peter, do you have any ideas on this?

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

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> /* Limit non-superuser changes */
> if (record->context == PGC_USERLIMIT &&
> source > PGC_S_UNPRIVILEGED &&
> newval < conf->session_val &&
^^^^^^^^^^^

I had in mind s/session_val/reset_val/ right here. Why wouldn't that
work?

regards, tom lane

On Wed, Aug 13, 2003 at 06:05:46PM -0400, Bruce Momjian wrote:
> I tried adding this line:
>
> record->session_source < PGC_S_UNPRIVILEGED &&
>
> and it does allow you to set the variable to false if you have set it to
> true in your session, but it also allows you to set it to false if it is
> set to true in postgresql.conf. You do this by setting it to 'true'
> _then_ to 'false' in your session. The reason this works is that there
> is no history in the GUC code to record the value this variable was set
> at various locations. Once you set it to 'true' in your session, the
> system remembers that, and forgets it was also set to true in
> postgresql.conf, and therefore allows you to set it to false.

The following describes what I would do if I had the controls
on the postgres source code:

I would put the lambda-user protected configuration parameters
under the control of a system similar to what is used for the
{get|set}rlimit system calls. At the begining of a session, a snapshot
of the parameters is taken, it is kept appart from the live
parameters. The snapshot would be used by the parameters controller to
permit or deny a parameter change depending on the snaphot value, the
current value, the desired new value and a set of rules. A rule could
express things like theses:

* boolean values:

* deny change if the new value would transition from true to
false and the snapshot value is true;

* allow change if the new value would transition from true to
false and the snapshot value is false;

* and any the the negation and/or inverse of both of these rules.

* numerical values:

* deny the change if the new value would be less than the snapshot
value and the current value is larger than the snapshot value;

* deny the change if the new value would be greater than the
snapshot value and the current value is less than the snapshot
value;

* new textual values would be matched against a set of acceptable or
unacceptable user values, the status returned by the countroller
would be according the matched set.

A scheme like this would allow a tolerance from the user point
of view while fixing limits on what can be done by non-superuser
accounts.

It could also be interesting to tie the parameters set to
transactions: parameters changed while inside a transaction could be
reset to their default or pre-transactions values on a commit or
rollback. This could look like the behavior of the save/restore
postscript operators.

Regards.

--
%!PS
297.6 420.9 translate 90 rotate 0 setgray gsave 0 1 1{pop 0 180 moveto 100
180 170 100 170 -10 curveto 180 -9 180 -9 190 -10 curveto 190 100 100 180
0 180 curveto fill 180 rotate}for grestore/Bookman-LightItalic findfont
240 scalefont setfont -151.536392 -63.7998886 moveto (bp)show showpage

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > /* Limit non-superuser changes */
> > if (record->context == PGC_USERLIMIT &&
> > source > PGC_S_UNPRIVILEGED &&
> > newval < conf->session_val &&
> ^^^^^^^^^^^
>
> I had in mind s/session_val/reset_val/ right here. Why wouldn't that
> work?

Yes, I have been thinking of that. The big question is whether a
non-super user can control the reset value? I don't know the answer, so
I didn't take the chance. Does anyone know? I assume Peter does and
was hoping for his help on this before we go 7.4 final.

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

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Yes, I have been thinking of that. The big question is whether a
> non-super user can control the reset value?

He could (via PGOPTIONS) ... but since he can only increase it, there is
nothing to fear.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > Yes, I have been thinking of that. The big question is whether a
> > non-super user can control the reset value?
>
> He could (via PGOPTIONS) ... but since he can only increase it, there is
> nothing to fear.

I have followed your suggestion and applied the following patch to have
PGC_USERLIMIT track reset_val rather than session_val. I now see that
all sources set the default, except SET:

makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL$

typedef enum
{
PGC_S_DEFAULT, /* wired-in default */
PGC_S_ENV_VAR, /* postmaster environment variable */
PGC_S_FILE, /* postgresql.conf */
PGC_S_ARGV, /* postmaster command line */
PGC_S_UNPRIVILEGED, /* dividing line for USERLIMIT */
PGC_S_DATABASE, /* per-database setting */
PGC_S_USER, /* per-user setting */
PGC_S_CLIENT, /* from client connection request */
PGC_S_OVERRIDE, /* special case to forcibly set default$
PGC_S_SESSION /* SET command */
} GucSource;

This fixes the reported problem where log_statement couldn't be turned
on then off in a session by a non-super user.

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

On Thu, Sep 04, 2003 at 01:06:15AM -0400, Bruce Momjian wrote:
> I have followed your suggestion and applied the following patch to have
> PGC_USERLIMIT track reset_val rather than session_val. I now see that
> all sources set the default, except SET:

I just updated my CVS workign directory tree. Your patch as
expected.

Thanks!
Bertrand.

--
%!PS
297.6 420.9 translate 90 rotate 0 setgray gsave 0 1 1{pop 0 180 moveto 100
180 170 100 170 -10 curveto 180 -9 180 -9 190 -10 curveto 190 100 100 180
0 180 curveto fill 180 rotate}for grestore/Bookman-LightItalic findfont
240 scalefont setfont -151.536392 -63.7998886 moveto (bp)show showpage