Re: custom variable classes

One thing I want to look at for 8.3 is improving custom variable
classes. Right now these are all user settable, which makes them quite
inappropriate for security related settings (such as which perl modules
to load for use by trusted plperl). I'm wondering if we should perhaps
allow something like:

custom_variable_classes = 'foo'
foo:<security_level>.bar = 'blurfl'

and providing some mechanism whereby we could ascertain that the value
comes from a permitted source.

I know I am not the only person who has noticed that we are a bit
lacking in this area.

As far as plperl goes, I guess I could instead use a db table to store a
set of module names for plperl to load, but then I would have to do some
fairly comprehensive permission tests.

Another possibility would be to provide somewhere in the catalog to
store such info. per db might be nicer, though.

Thoughts?

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> One thing I want to look at for 8.3 is improving custom variable
> classes. Right now these are all user settable, which makes them quite
> inappropriate for security related settings (such as which perl modules
> to load for use by trusted plperl). I'm wondering if we should perhaps
> allow something like:

> custom_variable_classes = 'foo'
> foo:<security_level>.bar = 'blurfl'

This seems really ugly --- for one thing, what if the DBA gets it wrong?

The value won't mean anything until the code that uses it gets loaded,
at which time the correct GucContext for the variable will be supplied.
How about we just compare that to the current definition source of the
value, and if we see it's been set improperly, revert the value to
default?

It might also be a good idea to disallow ALTER USER/DATABASE SET for a
custom variable that's a placeholder, since we don't at that time know
if the value should be SUSET or not; and there seems no pressing need to
allow these ALTERs to be done without having loaded the defining module.

[ thinks for a bit... ] With that provision in place, I think it would
be safe to revert to the "reset" value instead of the wired-in default,
which would be marginally nicer.

regards, tom lane

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> One thing I want to look at for 8.3 is improving custom variable
>> classes. Right now these are all user settable, which makes them quite
>> inappropriate for security related settings (such as which perl modules
>> to load for use by trusted plperl). I'm wondering if we should perhaps
>> allow something like:
>>
>
>
>> custom_variable_classes = 'foo'
>> foo:<security_level>.bar = 'blurfl'
>>
>
> This seems really ugly --- for one thing, what if the DBA gets it wrong?
>
>

We have no defence against them setting the wrong value anyway. What I
want is a defence against it being set in the wrong context.

> The value won't mean anything until the code that uses it gets loaded,
> at which time the correct GucContext for the variable will be supplied.
> How about we just compare that to the current definition source of the
> value, and if we see it's been set improperly, revert the value to
> default?
>

Sounds interesting - can you expand on this a bit?

> It might also be a good idea to disallow ALTER USER/DATABASE SET for a
> custom variable that's a placeholder, since we don't at that time know
> if the value should be SUSET or not; and there seems no pressing need to
> allow these ALTERs to be done without having loaded the defining module.
>
> [ thinks for a bit... ] With that provision in place, I think it would
> be safe to revert to the "reset" value instead of the wired-in default,
> which would be marginally nicer.
>
>
>

Where is the wired-in default for a custom GUC var? This whole thing
needs some documentation, ISTM.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Tom Lane wrote:
>> How about we just compare that to the current definition source of the
>> value, and if we see it's been set improperly, revert the value to
>> default?

> Sounds interesting - can you expand on this a bit?

define_custom_variable() shouldn't just blindly accept the current
setting, but should check to see where it came from, and dig down to the
reset setting if the current source wouldn't have been allowed to set
the variable according to the now-known context. (As noted in the
comments, that routine is a crock anyway, since it only tries to
transfer the "current" setting, not any stacked or reset settings.)

> Where is the wired-in default for a custom GUC var? This whole thing
> needs some documentation, ISTM.

It appears to be whatever is the current content of the physical
variable at the instant DefineCustomFooVariable is called. The whole
thing looks pretty poorly thought through, as well as undocumented...

regards, tom lane

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> Tom Lane wrote:
>>
>>> How about we just compare that to the current definition source of the
>>> value, and if we see it's been set improperly, revert the value to
>>> default?
>>>
>
>
>> Sounds interesting - can you expand on this a bit?
>>
>
> define_custom_variable() shouldn't just blindly accept the current
> setting, but should check to see where it came from, and dig down to the
> reset setting if the current source wouldn't have been allowed to set
> the variable according to the now-known context. (As noted in the
> comments, that routine is a crock anyway, since it only tries to
> transfer the "current" setting, not any stacked or reset settings.)
>
>

I think the first hurdle we have to get over is that supplying any
context other than PGC_USERSET seems to fail, even if the var is
actually set in the corrresponding place (e.g. postgresql.conf) - I
recall I had this trouble with plperl.use_strict, and Andrew(at)Supernews
tells me he has had similar issues.

cheers

andrew

On 2006-11-29, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
> Tom Lane wrote:
>> define_custom_variable() shouldn't just blindly accept the current
>> setting, but should check to see where it came from, and dig down to the
>> reset setting if the current source wouldn't have been allowed to set
>> the variable according to the now-known context. (As noted in the
>> comments, that routine is a crock anyway, since it only tries to
>> transfer the "current" setting, not any stacked or reset settings.)
>
> I think the first hurdle we have to get over is that supplying any
> context other than PGC_USERSET seems to fail, even if the var is
> actually set in the corrresponding place (e.g. postgresql.conf) - I
> recall I had this trouble with plperl.use_strict, and Andrew(at)Supernews
> tells me he has had similar issues.

This is pretty much what Tom is getting at, I think. Currently, the reason
why only PGC_USERSET works is that all placeholders are created as
PGC_USERSET, and no attempt is made to record whether the user that set the
value was a superuser or not. Thus, when the module that uses the variable
tries to define it properly, the newly-defined variable is assigned the
value from the placeholder using PGC_USERSET, which obviously fails if the
module tried to define some more restrictive type.

Another potential pitfall is that all the initialization code for backend
arguments (from the command line or the startup packet) relies on knowing
in advance whether options are PGC_SUSET or not - the PGC_SUSET options
are set after InitPostgres(), whereas all other options are set before.
(We presumably don't actually know until after InitPostgres whether or
not we are actually a superuser...)

This is particularly annoying for me because what I wanted was to have a
superuser-only option that could be set in the startup packet. (I got the
effect of a superuser-only option by preloading my module and putting a
check for superuserness in the variable's 'set' hook, but that doesn't
work at backend startup time.)

I also ran into an issue with a boolean config var causing errors like
"invalid value: 1" at definition time, but I've not tracked this down.

--
Andrew, Supernews
http://www.supernews.com - individual and corporate NNTP services

Added to TODO:

* Allow custom variable classes that can restrict who can set the values

http://archives.postgresql.org/pgsql-hackers/2006-11/msg00911.php

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

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> > One thing I want to look at for 8.3 is improving custom variable
> > classes. Right now these are all user settable, which makes them quite
> > inappropriate for security related settings (such as which perl modules
> > to load for use by trusted plperl). I'm wondering if we should perhaps
> > allow something like:
>
> > custom_variable_classes = 'foo'
> > foo:<security_level>.bar = 'blurfl'
>
> This seems really ugly --- for one thing, what if the DBA gets it wrong?
>
> The value won't mean anything until the code that uses it gets loaded,
> at which time the correct GucContext for the variable will be supplied.
> How about we just compare that to the current definition source of the
> value, and if we see it's been set improperly, revert the value to
> default?
>
> It might also be a good idea to disallow ALTER USER/DATABASE SET for a
> custom variable that's a placeholder, since we don't at that time know
> if the value should be SUSET or not; and there seems no pressing need to
> allow these ALTERs to be done without having loaded the defining module.
>
> [ thinks for a bit... ] With that provision in place, I think it would
> be safe to revert to the "reset" value instead of the wired-in default,
> which would be marginally nicer.
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: You can help support the PostgreSQL project by donating at
>
> http://www.postgresql.org/about/donate

--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +