Thread: SET variable - Permission issues

Hello all,
While working on an application I have been developing for about two 
years now, I have come across a base-limitation in PostgreSQL for three 
separate problems. I was talking with other members of the community 
recently and they agreed that the issue has some merit so I thought I 
would bring it up to the list.

First a bit about my environment to better understand the issue. My 
application allows users to connect directly to the database and issue 
commands as they see fit with their own database credentials, which 
operate under various permission restrictions. While I understand this 
is an unusual way of using the database, this is simply the design of my 
application. With that being said, I believe this issue is also 
important to address for other scenarios as well (like an organization 
that offers shared hosting of a PostgreSQL instance).

The actual issue is that many important variables within the PostgreSQL 
configuration file can be overridden by any (non superuser) account 
using SET.

The first instance of this I ran into was with statement_timeout, which 
I was hoping to use to limit the amount of time a query can run for 
before the query is cancelled. This was crucial to the architecture of 
my application to assure that infinite queries did not bog down the 
system and deny service to other users running their own queries. The 
problem is that regardless of what is set in the configuration, users of 
any privilege level can override the value in the postgresql.conf with 
the SET command.

This was frustrating, but not the end of the world. I solved it by using 
a perl script that monitors the running queries and kills any process 
that runs over the allotted time. This isn't perfect though, since a 
user could issue enough commands to prevent the perl script from being 
able to function correctly.

The second variable that I have recently come to realize exists causes a 
much more serious problem. This would be the work_mem setting. This 
variable allows any user to override the system default (again using 
SET) and issue some commands that may intentionally eat up huge chunks 
of RAM.

This one seems a lot more serious as I have no way of monitoring what 
this value is set to. Also, if the attacker acts fast enough (and uses 
the statement_timeout issue as well), they can completely DoS the server.

Finally, the third variable that has caused my application grief is the 
SEED value used by the RANDOM function that can also be set by any 
user.I realize I should be using a better pseudorandom number generator 
for anything important, but since I did not know that PostgreSQL's 
random is beyond a simple case of low-entropy and is actually completely 
deterministic for all my users (because they can each SET their own SEED 
values without restrictions), I now need to rework quite a bit of code 
to resolve this.

These are the three variables which have been problematic so far in my 
application, but I am sure there are others I simply have not noticed 
(or are harmless in my application, but may cause problems in another 
scenario). As far as a solution to the problem, some discussion was made 
about adding more variables for system maximums (like for 
statement_timeout and work_mem), but this does not cover all cases (like 
SEED) and adds unnecessary limitations to the system. Some back-end 
users may actually need to alter these variables beyond what is set as 
the maximum but cannot do so without altering the configuration file.

The other option that was discussed was to add privileges for variables 
(i.e. adding GRANT and REVOKE privileges for the SET command). This is 
obviously a bit more effort but I feel it would be the best option as it 
will add a lot more flexibility into the security of PostgreSQL.

If anybody has any other solutions for how to temporarily resolve these 
problems I would definitely love to hear it but I think it is a good
idea to discuss this problem and find a solution that everyone is 
comfortable with.

Thanks for taking the time to think this over,
-Josh

Josh <josh@schemaverse.com> writes:
> [ unhappy about users being able to freely adjust work_mem etc ]

Really, if you're letting users issue arbitrary SQL queries, there
simply isn't any way to prevent them from beating your server into
the ground.  I don't think that inserting a hack to prevent specific
configuration variables from being adjusted is going to help you
against an uncooperative user.  You'd be better off to rethink the
"let them issue SQL queries directly" part of your design.

The reason that the specific variables you mention (as well as some
others that bear on such things) are USERSET and not SUSET is precisely
that we are not trying to constrain the amount of resources an
uncooperative user can consume.  If we did try to do that, quite a
lot of design decisions would have to be revisited, and there would
be a number of unpleasant tradeoffs to be made.  GUC privilege levels
are just the tip of the iceberg.
        regards, tom lane

On Sat, Oct 8, 2011 at 12:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Josh <josh@schemaverse.com> writes:
>> [ unhappy about users being able to freely adjust work_mem etc ]
>
> Really, if you're letting users issue arbitrary SQL queries, there
> simply isn't any way to prevent them from beating your server into
> the ground.  I don't think that inserting a hack to prevent specific
> configuration variables from being adjusted is going to help you
> against an uncooperative user.  You'd be better off to rethink the
> "let them issue SQL queries directly" part of your design.
>
> The reason that the specific variables you mention (as well as some
> others that bear on such things) are USERSET and not SUSET is precisely
> that we are not trying to constrain the amount of resources an
> uncooperative user can consume.  If we did try to do that, quite a
> lot of design decisions would have to be revisited, and there would
> be a number of unpleasant tradeoffs to be made.  GUC privilege levels
> are just the tip of the iceberg.

Yeah.  For example, if somebody writes a complicated query against a
gigantic table that runs for a long time, is that because they're
trying to DOS the server, or because they have a legitimate need for
the results of that query, and it just so happens to be an expensive
query?  That's not really a question a computer can answer, and if you
restrict users to running only queries that are so short and simple
that they can't be used to DOS the box, you'll likely also be locking
out a significant percentage of legitimate user needs.

Having said that, I do think it might be useful to have ways of
controlling the values that users can set for GUC values, not so much
as a guard against an all-out assault (which is probably futile) but
as a way for DBAs to enforce system policy.  But even that seems like
a lot of work for a fairly marginal benefit....

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

Robert Haas <robertmhaas@gmail.com> writes:
> On Sat, Oct 8, 2011 at 12:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The reason that the specific variables you mention (as well as some
>> others that bear on such things) are USERSET and not SUSET is precisely
>> that we are not trying to constrain the amount of resources an
>> uncooperative user can consume. �If we did try to do that, quite a
>> lot of design decisions would have to be revisited, and there would
>> be a number of unpleasant tradeoffs to be made. �GUC privilege levels
>> are just the tip of the iceberg.

> Yeah.  For example, if somebody writes a complicated query against a
> gigantic table that runs for a long time, is that because they're
> trying to DOS the server, or because they have a legitimate need for
> the results of that query, and it just so happens to be an expensive
> query?  That's not really a question a computer can answer, and if you
> restrict users to running only queries that are so short and simple
> that they can't be used to DOS the box, you'll likely also be locking
> out a significant percentage of legitimate user needs.

Rereading the original message, I see that Josh raised an independent
point which I failed to see was independent.  Namely, that if one has
a SECURITY DEFINER function that relies on random() producing
unpredictable values, then an unprivileged user might be able to
compromise the behavior of the function by issuing SET SEED just before
calling the function.  That does seem like a potential security issue
--- not a very big one, but still undesirable.  And unlike other GUCs
that might affect query behavior, a SECURITY DEFINER function can't
protect itself against such a thing by adding SET clauses, because
setting the seed to a known value is exactly what it doesn't want.

I still don't think that a specialized GUC assignment privilege is worth
the trouble, but I could see an argument for just changing the seed GUC
from USERSET to SUSET.
        regards, tom lane

On 10/09/2011 09:09 PM, Robert Haas wrote:
> Having said that, I do think it might be useful to have ways of
> controlling the values that users can set for GUC values, not so much
> as a guard against an all-out assault (which is probably futile) but
> as a way for DBAs to enforce system policy.  But even that seems like
> a lot of work for a fairly marginal benefit....

I think the issues Josh raised are valid concerns for a number of use
cases. Even if you don't want to allow anyone on the Internet into your
database (as Josh does, since his application is a game and his attempt
is to set policies and privileges such that it is actually safe), there
are plenty of companies needing to run Postgres in a multi-tenant
environment.

Currently customer A canset work_mem = <some very large number>;
andset statement_timeout = 0;
and run a big query effectively DOS'ing customers B, C, and D. If these
two settings could be restricted by the DBA, there would be a much lower
chance of this happening. There are undoubtedly other holes to fill, but
it seems like a worthy cause.

Joe

-- 
Joe Conway
credativ LLC: http://www.credativ.us
Linux, PostgreSQL, and general Open Source
Training, Service, Consulting, & 24x7 Support

Gurjeet Singh <singh.gurjeet@gmail.com> writes:
> On Mon, Oct 10, 2011 at 1:06 PM, Joe Conway <mail@joeconway.com> wrote:
>> Currently customer A can
>> set work_mem = <some very large number>;
>> and
>> set statement_timeout = 0;
>> and run a big query effectively DOS'ing customers B, C, and D. If these
>> two settings could be restricted by the DBA, there would be a much lower
>> chance of this happening. There are undoubtedly other holes to fill, but
>> it seems like a worthy cause.

> Even in a controlled environment, say in a company where only legit apps
> developed in-house are run on the DB, a DBA would want peace of mind that
> the developers are not setting these GUCs at runtime (which is often even
> recommended in case of work_mem) to bypass a policy set by the DBA and are
> capable of bringing the DB down to its knees.

Any developer who can't think of six ways to DOS the server without
changing those settings should be fired on the spot for incompetence.

I could get interested in this if it weren't that the problem is so
open-ended as to be basically insoluble.  The only solution to the OP's
problem that's not got more holes than a wheel of Swiss cheese is to not
let untrustworthy people have direct SQL access to the server.  It *does
not improve security* to close a couple of obvious holes and leave a
bunch of other avenues to the same end open.  All it does is give you a
false sense of security.
        regards, tom lane

On Mon, Oct 10, 2011 at 2:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Any developer who can't think of six ways to DOS the server without
> changing those settings should be fired on the spot for incompetence.

No kidding.  But the point is that if the developer down the hall
maliciously destroys your database server, you can go through channels
and get him reprimanded or fired.  But if the developer down the hall
is a new hire who doesn't know beans about PostgreSQL and tries
setting work_mem to 10GB, well, oops, it was an accident.  And then
another developer gets hired three weeks later and does the same
thing.  And then three months later somebody does it again.  After a
while people no longer remember that in each case it was  a developer
to blame.  What they remember is that the DBA let the server go down
three times.

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

On 10/10/2011 01:52 PM, Gurjeet Singh wrote:

>On Mon, Oct 10, 2011 at 2:38 PM, Tom Lane wrote:
>> Any developer who can't think of six ways to DOS the server without
>> changing those settings should be fired on the spot for incompetence.

Perhaps, but I think our long term goal at least should be to make it
possible to prevent that -- not necessarily the default configuration,
but it should be at least *possible* for a sufficiently careful DBA to
harden their postgres instance.

I have multiple clients that either do run or would like to run postgres
multi-tenant, and at the moment that is somewhere between risky and
unacceptable.

> Would it be possible to make the SUSET/USERSET property of a GUC
> modifiable? So a DBA can do
> 
> ALTER USER novice SET CONTEXT OF work_mem TO 'superuser';
> 
> Or, maybe
> 
> ALTER USER novice SET MAX_VAL OF work_mem TO '1 MB';
> 
> and extend it to say
> 
> ALTER USER novice SET MIN_VAL OF statement_timeout TO '1';
> -- So that the user cannot turn off the timeout
> 
> ALTER DATABASE super_reliable SET ENUM_VALS OF synchronous_commit TO 'on';
> -- So that the user cannot change the synchronicity of transactions
> against this database.

I like this better than GRANT/REVOKE on SET.

Joe

-- 
Joe Conway
credativ LLC: http://www.credativ.us
Linux, PostgreSQL, and general Open Source
Training, Service, Consulting, & 24x7 Support

Joe Conway <mail@joeconway.com> wrote:
> On 10/10/2011 01:52 PM, Gurjeet Singh wrote:
>> ALTER USER novice SET MIN_VAL OF statement_timeout TO '1';
>> -- So that the user cannot turn off the timeout
>> 
>> ALTER DATABASE super_reliable SET ENUM_VALS OF synchronous_commit
>>   TO 'on';
>> -- So that the user cannot change the synchronicity of
>> transactions against this database.
> 
> I like this better than GRANT/REVOKE on SET.
+1
I would really like a way to prevent normal users from switching
from the default transaction isolation level I set.  This seems like
a good way to do that.  Putting sane bounds on some other settings,
more to protect against the accidental bad settings than malicious
mischief, would be a good thing, too.
-Kevin

Kevin Grittner wrote:
> Joe Conway <mail@joeconway.com> wrote:
> > On 10/10/2011 01:52 PM, Gurjeet Singh wrote:
>  
> >> ALTER USER novice SET MIN_VAL OF statement_timeout TO '1';
> >> -- So that the user cannot turn off the timeout
> >> 
> >> ALTER DATABASE super_reliable SET ENUM_VALS OF synchronous_commit
> >>   TO 'on';
> >> -- So that the user cannot change the synchronicity of
> >> transactions against this database.
> > 
> > I like this better than GRANT/REVOKE on SET.
>  
> +1
>  
> I would really like a way to prevent normal users from switching
> from the default transaction isolation level I set.  This seems like
> a good way to do that.  Putting sane bounds on some other settings,
> more to protect against the accidental bad settings than malicious
> mischief, would be a good thing, too.

Is this a TODO?  We might not want to make work_mem SUSET, but it would
allow administrators to control this.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + It's impossible for everything to be true. +

Bruce Momjian <bruce@momjian.us> wrote:
> Kevin Grittner wrote:
>> Joe Conway <mail@joeconway.com> wrote:
>>> On 10/10/2011 01:52 PM, Gurjeet Singh wrote:
>>  
>>>> ALTER USER novice SET MIN_VAL OF statement_timeout TO '1';
>>>> -- So that the user cannot turn off the timeout
>>>> 
>>>> ALTER DATABASE super_reliable SET ENUM_VALS OF
>>>>   synchronous_commit TO 'on';
>>>> -- So that the user cannot change the synchronicity of
>>>> transactions against this database.
>>> 
>>> I like this better than GRANT/REVOKE on SET.
>>  
>> +1
>>  
>> I would really like a way to prevent normal users from switching
>> from the default transaction isolation level I set.  This seems
>> like a good way to do that.  Putting sane bounds on some other
>> settings, more to protect against the accidental bad settings
>> than malicious mischief, would be a good thing, too.
> 
> Is this a TODO?  We might not want to make work_mem SUSET, but it
> would allow administrators to control this.
Well, we've identified a few people who like the idea, but I'm not
sure we have the degree of consensus we normally look for before
putting something on the TODO list.  After the discussion on this
thread, are there still any *objections* to allowing bounds or
subsets to be SUSET to limit GUC values more strictly than the
limits hard-coded in C?
-Kevin

On 10/11/2011 11:53 AM, Kevin Grittner wrote:
> Bruce Momjian <bruce@momjian.us> wrote:
>> Is this a TODO?  We might not want to make work_mem SUSET, but it 
>> would allow administrators to control this.
> 
> Well, we've identified a few people who like the idea, but I'm not 
> sure we have the degree of consensus we normally look for before 
> putting something on the TODO list.

That's pretty much what I was thinking.

> After the discussion on this thread, are there still any *objections*
> to allowing bounds or subsets to be SUSET to limit GUC values more
> strictly than the limits hard-coded in C?

No objections here.

Joe

-- 
Joe Conway
credativ LLC: http://www.credativ.us
Linux, PostgreSQL, and general Open Source
Training, Service, Consulting, & 24x7 Support

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Well, we've identified a few people who like the idea, but I'm not
> sure we have the degree of consensus we normally look for before
> putting something on the TODO list.  After the discussion on this
> thread, are there still any *objections* to allowing bounds or
> subsets to be SUSET to limit GUC values more strictly than the
> limits hard-coded in C?

No objection here, I like this whole idea.  Adding the information
visible at the right places is a fun project in itself, too :)

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Dimitri Fontaine <dimitri@2ndQuadrant.fr> wrote:
>> Adding the information visible at the right places is a fun
>> project in itself, too :)
> I was thinking a couple new columns in pg_settings (and what backs
> it) would be the main thing, but I haven't searched the source code
> yet.  Does something else leap to mind for you?

This isn't exactly a trivial matter.  What happens for instance if you
try to change the limit, and there are already active values outside the
limit in some processes?
        regards, tom lane

Tom Lane <tgl@sss.pgh.pa.us> wrote:
> This isn't exactly a trivial matter.  What happens for instance if
> you try to change the limit, and there are already active values
> outside the limit in some processes?
I would certainly vote for enforcing on the SET and not causing an
error on the attempt to change the limit.  (Maybe a notice?)  At the
time they set the GUC, they were allowed to do so.  It's a bit like
revoking a user's right to create a table in a schema -- what if
they've already done so?  You leave the table and you don't let them
create another.
What problems do you see with that?
-Kevin

On 10/11/2011 02:07 PM, Kevin Grittner wrote:
> Tom Lane <tgl@sss.pgh.pa.us> wrote:
>  
>> This isn't exactly a trivial matter.  What happens for instance if
>> you try to change the limit, and there are already active values
>> outside the limit in some processes?
>  
> I would certainly vote for enforcing on the SET and not causing an
> error on the attempt to change the limit.  (Maybe a notice?)  At the
> time they set the GUC, they were allowed to do so.  It's a bit like
> revoking a user's right to create a table in a schema -- what if
> they've already done so?  You leave the table and you don't let them
> create another.
>  
> What problems do you see with that?

Yeah, I don't know why it need be handled any different than say
 ALTER DATABASE foo SET config_param TO value
or ALTER ROLE foo SET config_param TO value

These cases do not effect already existing processes either.

Joe


-- 
Joe Conway
credativ LLC: http://www.credativ.us
Linux, PostgreSQL, and general Open Source
Training, Service, Consulting, & 24x7 Support

Joe Conway <mail@joeconway.com> writes:
> On 10/11/2011 02:07 PM, Kevin Grittner wrote:
>> I would certainly vote for enforcing on the SET and not causing an
>> error on the attempt to change the limit. ...
>> What problems do you see with that?

> Yeah, I don't know why it need be handled any different than say
>   ALTER DATABASE foo SET config_param TO value
> or
>   ALTER ROLE foo SET config_param TO value
> These cases do not effect already existing processes either.

It's not the same thing.  Those operations are documented as providing
the initial default value for subsequently-started sessions.  The
proposed change in limit values is different because the GUC range
limits have always before been immutable and continuously enforced
for the life of a database instance.

It may be that Kevin's proposal is adequate.  But I find that far
from obvious.  The trend of everything we've done with GUC for the last
ten years is to cause settings changes to apply immediately on-demand
and without "oh, but that's obvious if you know the implementation"
special cases.  I'm not real sure why this should get a free exemption
from that expectation ... or to put it more plainly, I *am* sure that
we'll be expected to fix it later, just like we had to fix the behavior
around removal of postgresql.conf entries, and some other things that
people didn't find as obvious as all that.
        regards, tom lane