advisory locks and permissions
One good thing about advisory locks having been in contrib was that they
didn't affect anyone who didn't actually install the module. Now that
we've put those functions in core, I wonder whether we don't need to
face up to the possibility of malicious use. For instance, it's not
very hard to create a DoS situation by running the system out of shared
lock table space:
regression=# select pg_advisory_lock(x) from generate_series(1,1000000) x;
WARNING: out of shared memory
ERROR: out of shared memory
HINT: You may need to increase max_locks_per_transaction.
regression=#
and once you've done that about the only fix is to quit your backend :-(
The brute force answer is to make those functions superuser-only, but I
wonder if there is a better way. Perhaps we could just deny public
execute access on them by default, and let admins grant the privilege to
whom they trust.
Or we could try to do something about limiting the number of such locks
that can be granted, but that seems nontrivial to tackle at such a late
stage of the devel cycle.
Thoughts?
regards, tom lane
On Wed, Sep 20, 2006 at 07:52:33PM -0400, Tom Lane wrote:
face up to the possibility of malicious use. For instance, it's not
very hard to create a DoS situation by running the system out of shared
lock table space:
Didn't you just say we don't try and protect against DoS? ;P
The brute force answer is to make those functions superuser-only, but I
wonder if there is a better way. Perhaps we could just deny public
execute access on them by default, and let admins grant the privilege to
whom they trust.Or we could try to do something about limiting the number of such locks
that can be granted, but that seems nontrivial to tackle at such a late
stage of the devel cycle.
ISTM that just restricting default access still leaves a pretty big
foot-gun laying around... perhaps the best compromise would be to do
that for this release and add some kind of a limit in the next release.
--
Jim Nasby jim@nasby.net
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)
On 9/21/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
One good thing about advisory locks having been in contrib was that they
didn't affect anyone who didn't actually install the module. Now that
we've put those functions in core, I wonder whether we don't need to
face up to the possibility of malicious use. For instance, it's not
very hard to create a DoS situation by running the system out of shared
lock table space:
It's much more likely to accidentally bork yourself.
regression=# select pg_advisory_lock(x) from generate_series(1,1000000) x;
WARNING: out of shared memory
ERROR: out of shared memory
HINT: You may need to increase max_locks_per_transaction.
regression=#and once you've done that about the only fix is to quit your backend :-(
The brute force answer is to make those functions superuser-only, but I
wonder if there is a better way. Perhaps we could just deny public
execute access on them by default, and let admins grant the privilege to
whom they trust.Or we could try to do something about limiting the number of such locks
that can be granted, but that seems nontrivial to tackle at such a late
stage of the devel cycle.
I vote for locking down to superuser access (lets be frank here: I
would estimate 90%+ database installatons run with the application as
root) so we are not losing much.
I honestly think exhausting the lock space should raise a fatal, not
an error. This gives a chance for recovery without a hard reset. Any
other limitation would be fine, now or later, just please don't slow
them down! (advisory locks are extremely fast).
merlin
All,
I vote for locking down to superuser access (lets be frank here: I
would estimate 90%+ database installatons run with the application as
root) so we are not losing much.
Not in my experience. Note that making them superuser-only pretty much puts
them out of the hands of hosted applications.
How simple would it be to limit the number of advisory locks available to a
single request? That would at least make the DOS non-trivial. Or to put in
a handle (GUC?) that allows turning advisory locks off?
Hmmm ... I'll bet I could come up with other ways to use generate_series in a
DOS, even without advisory locks ...
--
Josh Berkus
PostgreSQL @ Sun
San Francisco
Doesn't creating many temp tables in a transaction do the same thing?
---------------------------------------------------------------------------
Josh Berkus wrote:
All,
I vote for locking down to superuser access (lets be frank here: I
would estimate 90%+ database installatons run with the application as
root) so we are not losing much.Not in my experience. Note that making them superuser-only pretty much puts
them out of the hands of hosted applications.How simple would it be to limit the number of advisory locks available to a
single request? That would at least make the DOS non-trivial. Or to put in
a handle (GUC?) that allows turning advisory locks off?Hmmm ... I'll bet I could come up with other ways to use generate_series in a
DOS, even without advisory locks ...--
Josh Berkus
PostgreSQL @ Sun
San Francisco---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend
--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
On Wed, 20 Sep 2006, Bruce Momjian wrote:
Doesn't creating many temp tables in a transaction do the same thing?
---------------------------------------------------------------------------
Like this?
jeremyd=# CREATE OR REPLACE FUNCTION testy(n integer) returns integer as $$
BEGIN
EXECUTE 'CREATE TEMP TABLE testy_' || n::text || ' (a integer, b text);';
RETURN n;
END;
$$ LANGUAGE plpgsql VOLATILE STRICT;
CREATE FUNCTION
jeremyd=# select testy(n) from generate_series(1,1000000) n;
WARNING: out of shared memory
CONTEXT: SQL statement "CREATE TEMP TABLE testy_3323 (a integer, b
text);" PL/pgSQL function "testy" line 2 at execute statement
ERROR: out of shared memory
HINT: You may need to increase max_locks_per_transaction.
CONTEXT: SQL statement "CREATE TEMP TABLE testy_3323 (a integer, b
text);"
PL/pgSQL function "testy" line 2 at execute statement
Josh Berkus wrote:
All,
I vote for locking down to superuser access (lets be frank here: I
would estimate 90%+ database installatons run with the application as
root) so we are not losing much.Not in my experience. Note that making them superuser-only pretty much puts
them out of the hands of hosted applications.How simple would it be to limit the number of advisory locks available to a
single request? That would at least make the DOS non-trivial. Or to put in
a handle (GUC?) that allows turning advisory locks off?Hmmm ... I'll bet I could come up with other ways to use generate_series in a
DOS, even without advisory locks ...--
Josh Berkus
PostgreSQL @ Sun
San Francisco---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend
--
Two percent of zero is almost nothing.
Bruce Momjian <bruce@momjian.us> writes:
Doesn't creating many temp tables in a transaction do the same thing?
True, but it's a tad harder/less likely that you'd accidentally cause
a problem that way.
I'm not sure if I'm crying wolf or whether there's a serious issue.
Certainly, if you have SQL-command access there are plenty of ways
to cause DoS situations of varying levels of severity.
An admin who is concerned about this can revoke public access on the
functions for himself ... but should that be the default out-of-the-box
configuration? I feel more comfortable with saying "you have to turn
on this potentially-dangerous feature" than with saying you have to turn
it off.
Another reason for restricting access to the advisory-lock functions
is that an uninformed application might take the wrong locks, and
bollix up your intended usage accidentally.
regards, tom lane
Le jeudi 21 septembre 2006 01:52, Tom Lane a écrit :
Or we could try to do something about limiting the number of such locks
that can be granted, but that seems nontrivial to tackle at such a late
stage of the devel cycle.Thoughts?
What about reserving some amount of shared_buffers out of those locks?
(For example ext2 preserve some disk space for root in case of emergency)
Don't know anything about how easily (error prone) this can be done, though.
Le jeudi 21 septembre 2006 16:22, Tom Lane a écrit :
Another reason for restricting access to the advisory-lock functions
is that an uninformed application might take the wrong locks, and
bollix up your intended usage accidentally.
This sounds like one more attempt to protect against idiots, which universe
tend to produce on a pretty quick rate :)
My 2¢,
--
Dimitri Fontaine
Directeur Technique
Tel: 06 74 15 56 53
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
An admin who is concerned about this can revoke public access on the
functions for himself ... but should that be the default out-of-the-box
configuration? I feel more comfortable with saying "you have to turn
on this potentially-dangerous feature" than with saying you have to turn
it off.
I agree with having it turned off by default, at least in 8.2. Once
it's out there and in use we can review that decision for 8.3 and
possibly implement appropriate safeguards based on the usage.
Another reason for restricting access to the advisory-lock functions
is that an uninformed application might take the wrong locks, and
bollix up your intended usage accidentally.
This begs for a mechanism to define who can take what locks, etc..
Which seems to be an 8.3 issue.
Thanks,
Stephen
Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
Doesn't creating many temp tables in a transaction do the same thing?
True, but it's a tad harder/less likely that you'd accidentally cause
a problem that way.
Then why not use a GUC (that only an administrator can set) to control
the maximum number of advisory locks a given backend can take at any
one time? Then it becomes the DBA's problem (and solution) if someone
manages to run the database out of shared memory through this
mechanism.
--
Kevin Brown kevin@sysexperts.com
Stephen Frost <sfrost@snowman.net> writes:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
An admin who is concerned about this can revoke public access on the
functions for himself ... but should that be the default out-of-the-box
configuration? I feel more comfortable with saying "you have to turn
on this potentially-dangerous feature" than with saying you have to turn
it off.
I agree with having it turned off by default, at least in 8.2.
Do we have a consensus to do this for 8.2? Or are we going to leave it
as is? Those are the only two realistic short-term options ...
regards, tom lane
On 9/22/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Stephen Frost <sfrost@snowman.net> writes:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
An admin who is concerned about this can revoke public access on the
functions for himself ... but should that be the default out-of-the-box
configuration? I feel more comfortable with saying "you have to turn
on this potentially-dangerous feature" than with saying you have to turn
it off.I agree with having it turned off by default, at least in 8.2.
Do we have a consensus to do this for 8.2? Or are we going to leave it
as is? Those are the only two realistic short-term options ...
there are plenty of other potentially nasty things (like
generate_series and the ! operator). why are advisory_locks handled
specially? the way it stands right now is a user with command access
can DoS a server after five minutes of research on the web.
however, if we decide to lock them, it should be documented as such.
advisory locks still show up as 'userlock' in the pg_locks view. does
this matter?
merlin
"Merlin Moncure" <mmoncure@gmail.com> writes:
advisory locks still show up as 'userlock' in the pg_locks view. does
this matter?
I'm disinclined to change that, because it would probably break existing
client-side code for little gain.
regards, tom lane
there are plenty of other potentially nasty things (like
generate_series and the ! operator). why are advisory_locks handled
specially? the way it stands right now is a user with command access
can DoS a server after five minutes of research on the web.
You don't even have to do any research, just fire off ab.
Using a DOS to attack *any* database server via the web is a 3 second
command.
Joshua D. Drake
--
=== The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240
Providing the most comprehensive PostgreSQL solutions since 1997
http://www.commandprompt.com/
On Sep 22, 2006, at 11:26 , Merlin Moncure wrote:
On 9/22/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Stephen Frost <sfrost@snowman.net> writes:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
An admin who is concerned about this can revoke public access
on the
functions for himself ... but should that be the default out-of-
the-box
configuration? I feel more comfortable with saying "you have
to turn
on this potentially-dangerous feature" than with saying you
have to turn
it off.
I agree with having it turned off by default, at least in 8.2.
Do we have a consensus to do this for 8.2? Or are we going to
leave it
as is? Those are the only two realistic short-term options ...there are plenty of other potentially nasty things (like
generate_series and the ! operator). why are advisory_locks handled
specially? the way it stands right now is a user with command access
can DoS a server after five minutes of research on the web.however, if we decide to lock them, it should be documented as such.
advisory locks still show up as 'userlock' in the pg_locks view. does
this matter?
I would be more worried about accidental collisions between
applications. The lock ranges will now need to be in their respective
application's configuration file in case of collision with another
app once developers really start using locks for IPC. Ideally, the
user-level lock functions would take strings instead of integers and
hash them appropriately, no? Otherwise, someone will end up
maintaining a registry of lock numbers in use. LISTEN doesn't use
integers.
-M
On Fri, Sep 22, 2006 at 12:03:46PM -0400, AgentM wrote:
On Sep 22, 2006, at 11:26 , Merlin Moncure wrote:
On 9/22/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Stephen Frost <sfrost@snowman.net> writes:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
An admin who is concerned about this can revoke public access
on the
functions for himself ... but should that be the default out-of-
the-box
configuration? I feel more comfortable with saying "you have
to turn
on this potentially-dangerous feature" than with saying you
have to turn
it off.
I agree with having it turned off by default, at least in 8.2.
Do we have a consensus to do this for 8.2? Or are we going to
leave it
as is? Those are the only two realistic short-term options ...there are plenty of other potentially nasty things (like
generate_series and the ! operator). why are advisory_locks handled
specially? the way it stands right now is a user with command access
can DoS a server after five minutes of research on the web.however, if we decide to lock them, it should be documented as such.
advisory locks still show up as 'userlock' in the pg_locks view. does
this matter?I would be more worried about accidental collisions between
applications. The lock ranges will now need to be in their respective
application's configuration file in case of collision with another
app once developers really start using locks for IPC. Ideally, the
user-level lock functions would take strings instead of integers and
hash them appropriately, no? Otherwise, someone will end up
maintaining a registry of lock numbers in use. LISTEN doesn't use
integers.
This is why I suggested we set aside some range of numbers that should
not be used. Doing so would allow adding a better-managed
numbering/naming scheme in the future.
--
Jim Nasby jim@nasby.net
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)
On 9/22/06, AgentM <agentm@themactionfaction.com> wrote:
I would be more worried about accidental collisions between
applications. The lock ranges will now need to be in their respective
i dont think this argument has merit because the lock is scoped to the
current database. this would only be a problem if two applications
used the same database...not likely.
the old userlocks had 48 bits of lock space and now we have 64, im not
complaining.
application's configuration file in case of collision with another
app once developers really start using locks for IPC. Ideally, the
user-level lock functions would take strings instead of integers and
hash them appropriately, no? Otherwise, someone will end up
maintaining a registry of lock numbers in use. LISTEN doesn't use
integers.
application can translate the locks to strings via a very simple
translation table. there is no downside to this besides a index
lookup on a small table, which is more or less what the listen/notify
does internally.
advisory locks work off of the internal lock system which is an
integer only system. the whole point is to get at these locks while
bypassing the transaction system. you are suggesting something which
does not fit into the current lock system.
merlin
On Sep 22, 2006, at 12:46 , Merlin Moncure wrote:
On 9/22/06, AgentM <agentm@themactionfaction.com> wrote:
I would be more worried about accidental collisions between
applications. The lock ranges will now need to be in their respectivei dont think this argument has merit because the lock is scoped to the
current database. this would only be a problem if two applications
used the same database...not likely.
Since we have schemas, I have several applications running in one
database- sometimes several versions of the same application. This
makes it easier to shuffle data around.
application's configuration file in case of collision with another
app once developers really start using locks for IPC. Ideally, the
user-level lock functions would take strings instead of integers and
hash them appropriately, no? Otherwise, someone will end up
maintaining a registry of lock numbers in use. LISTEN doesn't use
integers.application can translate the locks to strings via a very simple
translation table. there is no downside to this besides a index
lookup on a small table, which is more or less what the listen/notify
does internally.advisory locks work off of the internal lock system which is an
integer only system. the whole point is to get at these locks while
bypassing the transaction system. you are suggesting something which
does not fit into the current lock system.
I didn't suggest using lookup tables; I suggested that the lock
functions should perform the string hashing itself- the applications
will write wrappers for this anyway to prevent collisions or they
will have to provide some configuration element to change the lock
range in case of collision- which will be extraordinarily difficult
to debug in the first place.
-M
On 9/22/06, Jim C. Nasby <jim@nasby.net> wrote:
This is why I suggested we set aside some range of numbers that should
not be used. Doing so would allow adding a better-managed
numbering/naming scheme in the future.
the whole point about advisory locks is that the provided lock space
is unmanaged. for example, in the ISAM system I wrote which hooked
into the acucobol virtual file system interface, I used a global
sequence for row level pessimistic locking but reserved the 48th bit
for table level locks. This system was extremely effective. on the
current system I'm working on I use them to lock sequence oid's plus a
high bit indicator for what i am doing. in short, advisory locks are
application-defined in concept.
merlin
On Fri, Sep 22, 2006 at 12:56:37PM -0400, Merlin Moncure wrote:
On 9/22/06, Jim C. Nasby <jim@nasby.net> wrote:
This is why I suggested we set aside some range of numbers that should
not be used. Doing so would allow adding a better-managed
numbering/naming scheme in the future.the whole point about advisory locks is that the provided lock space
is unmanaged. for example, in the ISAM system I wrote which hooked
into the acucobol virtual file system interface, I used a global
sequence for row level pessimistic locking but reserved the 48th bit
for table level locks. This system was extremely effective. on the
current system I'm working on I use them to lock sequence oid's plus a
high bit indicator for what i am doing. in short, advisory locks are
application-defined in concept.
Yes, but if you get two pieces of code written by different people using
them in the same database, you can get hosed. As PostgreSQL becomes more
popular and more people start developing software for it, this is more
likely to occur.
--
Jim Nasby jim@nasby.net
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)