Re: pg_cancel_backend by non-superuser

This patch would appear(?) to have languished:

https://commitfest.postgresql.org/action/patch_view?id=541

I'd really like to see it included. In the last comments of the
review, there seem to be problems in *terminate* backend, but even
just pg_cancel_backend as non-superuser would be just a huge
improvement. What are the things blocking non-superuser
pg_cancel_backend from being accepted?

--
fdr

Daniel Farina <daniel(at)heroku(dot)com> writes:
> This patch would appear(?) to have languished:
> https://commitfest.postgresql.org/action/patch_view?id=541

> I'd really like to see it included. In the last comments of the
> review, there seem to be problems in *terminate* backend, but even
> just pg_cancel_backend as non-superuser would be just a huge
> improvement. What are the things blocking non-superuser
> pg_cancel_backend from being accepted?

I think the reason the patch stalled is that we have not got consensus
on how far to extend the conditions under which these operations should
be allowed. For instance, in the last comment attached to that
commitfest entry, Noah alleges that a non-superuser database owner
should be allowed to kill a superuser's session, if it's connected
to his database. My reaction to that is somewhere between "no" and
"hell no"; IMO superusers can mess up non-superusers, never vice versa.
If I recall the discussion correctly, there were other points of
contention too.

I don't think we need more coding right now ... we need somebody to
write a spec that everyone can agree to.

ISTM it would be reasonably non-controversial to allow users to issue
pg_cancel_backend against other sessions logged in as the same userID.
The question is whether to go further than that, and if so how much.

regards, tom lane

2011/10/1 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Daniel Farina <daniel(at)heroku(dot)com> writes:
>> This patch would appear(?) to have languished:
>> https://commitfest.postgresql.org/action/patch_view?id=541
>
>> I'd really like to see it included.  In the last comments of the
>> review, there seem to be problems in *terminate* backend, but even
>> just pg_cancel_backend as non-superuser would be just a huge
>> improvement.  What are the things blocking non-superuser
>> pg_cancel_backend from being accepted?
>
> I think the reason the patch stalled is that we have not got consensus
> on how far to extend the conditions under which these operations should
> be allowed.  For instance, in the last comment attached to that
> commitfest entry, Noah alleges that a non-superuser database owner
> should be allowed to kill a superuser's session, if it's connected
> to his database.  My reaction to that is somewhere between "no" and
> "hell no"; IMO superusers can mess up non-superusers, never vice versa.
> If I recall the discussion correctly, there were other points of
> contention too.
>

Hi,

the original patch allow only for the DB Owner to kill sessions owner
by other users.
This because in real world I have some production database where I'm
not the DBA, but only the DB owner.

I think that is not a good idea that a normal users is able to kill
session from the same user because, unfortunally,
in some real environment there are a lots of application that need to
access to the same database and the same user is used.
I know that is not a good practise but it is on the field ....

For this reason I suppose that allow only to DB onwer to kill other
sessions it is a good compromize between functionality and security,
but is my personal opinion ...

> I don't think we need more coding right now ... we need somebody to
> write a spec that everyone can agree to.
>
> ISTM it would be reasonably non-controversial to allow users to issue
> pg_cancel_backend against other sessions logged in as the same userID.
> The question is whether to go further than that, and if so how much.
>
>                        regards, tom lane
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

On Fri, Sep 30, 2011 at 9:30 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> ISTM it would be reasonably non-controversial to allow users to issue
> pg_cancel_backend against other sessions logged in as the same userID.
> The question is whether to go further than that, and if so how much.

In *every* case -- and there are many -- where we've had people
express pain, this would have sufficed. Usually the problem is a
large index creation gone awry, or an automated backup process
blocking a schema change that has taken half the locks it needs, or
something like that -- all by the same role that is under control of
the folks feeling distress. If this minimal set is uncontroversial, I
would like to see that much committed and then spend some time
hand-wringing on whether to extend it.

If one does want to extend it, I think role inheritance makes the most
sense: a child role should be able to cancel its parent role's
queries, and not vice-versa. Since one can use SET ROLE in this case
anyway to basically act on behalf on that role, I think that, too,
should be uncontroversial.

--
fdr

"""
In *every* case -- and there are many -- where we've had people
express pain, this would have sufficed. Usually the problem is a
large index creation gone awry, or an automated backup process
blocking a schema change that has taken half the locks it needs, or
something like that -- all by the same role that is under control of
the folks feeling distress. If this minimal set is uncontroversial, I
would like to see that much committed and then spend some time
hand-wringing on whether to extend it.

If one does want to extend it, I think role inheritance makes the most
sense: a child role should be able to cancel its parent role's
queries, and not vice-versa. Since one can use SET ROLE in this case
anyway to basically act on behalf on that role, I think that, too,
should be uncontroversial.
"""

I would be a step in the right direction if the DB owner would see all queries
to the DB in pg_stat_activity.

- Anssi

On Sat, Oct 1, 2011 at 3:47 PM, Kääriäinen Anssi
<anssi(dot)kaariainen(at)thl(dot)fi> wrote:
> I would be a step in the right direction if the DB owner would see all queries
> to the DB in pg_stat_activity.

"All," including that of the superuser? I'd like to pass on that one, please.

In general, I feel there is this problem that one cannot hand over a
non-superuser but powerful role to someone else, and allowing them to
make new roles with strictly less power than what they were granted
(the opposite of role inheritance, whereby children have as much or
more power). Right now I get the feeling that I'd rather fix that
problem in the role system then overloading what it means to be a
database owner. If anything, to me being a database owner means the
ability to run ALTER DATABASE, and not much else.

--
fdr

On 01-10-2011 17:44, Daniel Farina wrote:
> On Fri, Sep 30, 2011 at 9:30 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> ISTM it would be reasonably non-controversial to allow users to issue
>> pg_cancel_backend against other sessions logged in as the same userID.
>> The question is whether to go further than that, and if so how much.
>
> In *every* case -- and there are many -- where we've had people
> express pain, this would have sufficed.
>
I see. What about passing this decision to DBA? I mean a GUC
can_cancel_session = user, dbowner (default is '' -- only superuser). You can
select one or both options. This GUC can only be changed by superuser.

--
Euler Taveira de Oliveira - Timbira http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

On Sat, Oct 1, 2011 at 10:11 PM, Euler Taveira de Oliveira
<euler(at)timbira(dot)com> wrote:
> On 01-10-2011 17:44, Daniel Farina wrote:
>>
>> On Fri, Sep 30, 2011 at 9:30 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us>  wrote:
>>>
>>> ISTM it would be reasonably non-controversial to allow users to issue
>>> pg_cancel_backend against other sessions logged in as the same userID.
>>> The question is whether to go further than that, and if so how much.
>>
>> In *every* case -- and there are many -- where we've had people
>> express pain, this would have sufficed.
>>
> I see. What about passing this decision to DBA? I mean a GUC
> can_cancel_session = user, dbowner (default is '' -- only superuser). You
> can select one or both options. This GUC can only be changed by superuser.

Or how about making it a grantable database-level privilege?

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

I like this idea

+1
Il giorno 02/ott/2011 12:56, "Robert Haas" <robertmhaas(at)gmail(dot)com> ha
scritto:
> On Sat, Oct 1, 2011 at 10:11 PM, Euler Taveira de Oliveira
> <euler(at)timbira(dot)com> wrote:
>> On 01-10-2011 17:44, Daniel Farina wrote:
>>>
>>> On Fri, Sep 30, 2011 at 9:30 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>>
>>>> ISTM it would be reasonably non-controversial to allow users to issue
>>>> pg_cancel_backend against other sessions logged in as the same userID.
>>>> The question is whether to go further than that, and if so how much.
>>>
>>> In *every* case -- and there are many -- where we've had people
>>> express pain, this would have sufficed.
>>>
>> I see. What about passing this decision to DBA? I mean a GUC
>> can_cancel_session = user, dbowner (default is '' -- only superuser). You
>> can select one or both options. This GUC can only be changed by
superuser.
>
> Or how about making it a grantable database-level privilege?
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

On Sun, Oct 02, 2011 at 06:55:51AM -0400, Robert Haas wrote:
> On Sat, Oct 1, 2011 at 10:11 PM, Euler Taveira de Oliveira
> <euler(at)timbira(dot)com> wrote:
> > On 01-10-2011 17:44, Daniel Farina wrote:
> >> On Fri, Sep 30, 2011 at 9:30 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> ?wrote:
> >>> ISTM it would be reasonably non-controversial to allow users to issue
> >>> pg_cancel_backend against other sessions logged in as the same userID.
> >>> The question is whether to go further than that, and if so how much.
> >>
> >> In *every* case -- and there are many -- where we've had people
> >> express pain, this would have sufficed.

+1 for allowing that unconditionally.

> > I see. What about passing this decision to DBA? I mean a GUC
> > can_cancel_session = user, dbowner (default is '' -- only superuser). You
> > can select one or both options. This GUC can only be changed by superuser.
>
> Or how about making it a grantable database-level privilege?

I think either is overkill. You can implement any policy by interposing a
SECURITY DEFINER wrapper around pg_cancel_backend().

nm

Noah Misch <noah(at)leadboat(dot)com> writes:
>> >> On Fri, Sep 30, 2011 at 9:30 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> ?wrote:
>> >>> ISTM it would be reasonably non-controversial to allow users to issue
>> >>> pg_cancel_backend against other sessions logged in as the same userID.
>> >>> The question is whether to go further than that, and if so how much.
>> >>
>> >> In *every* case -- and there are many -- where we've had people
>> >> express pain, this would have sufficed.
>
> +1 for allowing that unconditionally.

+1

>> Or how about making it a grantable database-level privilege?
>
> I think either is overkill. You can implement any policy by interposing a
> SECURITY DEFINER wrapper around pg_cancel_backend().

I still like the idea of grant cancel and grant terminate. For another
patch.

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

Noah Misch <noah(at)leadboat(dot)com> writes:
> On Sun, Oct 02, 2011 at 06:55:51AM -0400, Robert Haas wrote:
>> On Sat, Oct 1, 2011 at 10:11 PM, Euler Taveira de Oliveira
>> <euler(at)timbira(dot)com> wrote:
>>> I see. What about passing this decision to DBA? I mean a GUC
>>> can_cancel_session = user, dbowner (default is '' -- only superuser). You
>>> can select one or both options. This GUC can only be changed by superuser.

>> Or how about making it a grantable database-level privilege?

> I think either is overkill. You can implement any policy by interposing a
> SECURITY DEFINER wrapper around pg_cancel_backend().

I'm with Noah on this. If allowing same-user cancels is enough to solve
95% or 99% of the real-world use cases, let's just do that. There's no
very good reason to suppose that a GUC or some more ad-hoc privileges
will solve a large enough fraction of the rest of the cases to be worth
their maintenance effort. In particular, I think both of the above
proposals assume way too much about the DBA's specific administrative
requirements.

regards, tom lane

On Sun, Oct 2, 2011 at 23:32, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Noah Misch <noah(at)leadboat(dot)com> writes:
>> On Sun, Oct 02, 2011 at 06:55:51AM -0400, Robert Haas wrote:
>>> On Sat, Oct 1, 2011 at 10:11 PM, Euler Taveira de Oliveira
>>> <euler(at)timbira(dot)com> wrote:
>>>> I see. What about passing this decision to DBA? I mean a GUC
>>>> can_cancel_session = user, dbowner (default is '' -- only superuser). You
>>>> can select one or both options. This GUC can only be changed by superuser.
>
>>> Or how about making it a grantable database-level privilege?
>
>> I think either is overkill.  You can implement any policy by interposing a
>> SECURITY DEFINER wrapper around pg_cancel_backend().
>
> I'm with Noah on this.  If allowing same-user cancels is enough to solve
> 95% or 99% of the real-world use cases, let's just do that.  There's no
> very good reason to suppose that a GUC or some more ad-hoc privileges
> will solve a large enough fraction of the rest of the cases to be worth
> their maintenance effort.  In particular, I think both of the above
> proposals assume way too much about the DBA's specific administrative
> requirements.

+1.

Torello, are you up for updating your patch to do this, for now? If
not, I'll be happy to create an updated patch that does just this, but
since you got started on it...

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

On 10/02/2011 05:32 PM, Tom Lane wrote:
> I'm with Noah on this. If allowing same-user cancels is enough to solve
> 95% or 99% of the real-world use cases, let's just do that.

And we're back full circle. This is basically where Josh Kuperschmidt
started in early 2010:
http://archives.postgresql.org/message-id/4ec1cf761002051455i6e702999y7cf4699b4eb48242@mail.gmail.com

Then Torello's patch initially more ambitious patch got pruned down to
the same sort of feature set.

Next, the day after the November CommitFest started (so it got kind of
lost), Edward Muller took a shot at coding exactly this too, which he
tells me happened without even knowing the other two were already
floating around:
http://archives.postgresql.org/message-id/CABm0hDX+xUc3dsNCnb2Z2mErtw3Crcc5KjMVh6KWHB7JNixpHg@mail.gmail.com

The picture of what people really want here is pretty clear now, after
different people wanted same-user cancels (or more) badly enough to
submit a patch adding it, in three cases now.

--
Greg Smith 2ndQuadrant US greg(at)2ndQuadrant(dot)com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us

2011/12/6 Magnus Hagander <magnus(at)hagander(dot)net>:
> On Sun, Oct 2, 2011 at 23:32, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Noah Misch <noah(at)leadboat(dot)com> writes:
>>> On Sun, Oct 02, 2011 at 06:55:51AM -0400, Robert Haas wrote:
>>>> On Sat, Oct 1, 2011 at 10:11 PM, Euler Taveira de Oliveira
>>>> <euler(at)timbira(dot)com> wrote:
>>>>> I see. What about passing this decision to DBA? I mean a GUC
>>>>> can_cancel_session = user, dbowner (default is '' -- only superuser). You
>>>>> can select one or both options. This GUC can only be changed by superuser.
>>
>>>> Or how about making it a grantable database-level privilege?
>>
>>> I think either is overkill.  You can implement any policy by interposing a
>>> SECURITY DEFINER wrapper around pg_cancel_backend().
>>
>> I'm with Noah on this.  If allowing same-user cancels is enough to solve
>> 95% or 99% of the real-world use cases, let's just do that.  There's no
>> very good reason to suppose that a GUC or some more ad-hoc privileges
>> will solve a large enough fraction of the rest of the cases to be worth
>> their maintenance effort.  In particular, I think both of the above
>> proposals assume way too much about the DBA's specific administrative
>> requirements.
>
> +1.
>
> Torello, are you up for updating your patch to do this, for now? If
> not, I'll be happy to create an updated patch that does just this, but
> since you got started on it...
>

Sorry for the long delay.

I will try to adjust the patch and submit for the next Commit Fest if
this is ok for you.

> --
>  Magnus Hagander
>  Me: http://www.hagander.net/
>  Work: http://www.redpill-linpro.com/
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

On 12/11/2011 05:29 PM, Torello Querci wrote:
> I will try to adjust the patch and submit for the next Commit Fest if
> this is ok for you.
>

I don't think we'll need this, it will take a bit to explain why though.

First, thanks for returning this topic to discussion and keeping up with
all the controversy around it. You said back in February this was your
first post here, and I doubt you expected that 10 months later this
would still be active and argued over. The fact that you're still here
and everyone knows your name now is itself an accomplishment, many
people just give up on their submission ideas under far less negative
feedback.

I just took a long look at all three of the submissions in this area
we've gotten. The central idea that made yours different was making the
database owner the person allowed to cancel things. That hadn't been
suggested as a cancellation requisite before that I know of, and this
code may wander in that direction one day. It's just a bit too much to
accept right now. You seem to need that specific feature for your
environment. If that's the case, you might want to develop something
that works that way, but handles the concerns raised here. The fact
that it's not acceptable for a database owner to cancel a superuser
query is the biggest objection, there were some others too. Ultimately
it may take a reworking of database permissions to really make this
acceptable, which is a larger job than I think you were trying to get
involved with.

Unfortunately, when I look at the new spec we have now, I don't see
anything from what you did that we can re-use. It's too specific to the
owner-oriented idea. The two other patches that have been submitted
both are closer to what we've decided we want now. What I'm going to do
here is mark your submission "returned with feedback".

Rather than wait for something new from you, I'm going to review and
rework the other two submissions. That I can start on right now. It's
taken so long to reach this point that I don't want to wait much longer
for another submission here, certainly not until over a month from now
when the next CF starts. We need to get the arguments around a new
version started earlier than that. Thanks for offering to work on this
more, and I hope there's been something about this long wandering
discussion that's been helpful to you. As I said, you did at least make
a good first impression, and that is worth something when it comes to
this group.

--
Greg Smith 2ndQuadrant US greg(at)2ndQuadrant(dot)com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us

Hi Greg

2011/12/13 Greg Smith <greg(at)2ndquadrant(dot)com>:
> On 12/11/2011 05:29 PM, Torello Querci wrote:
>>
>> I will try to adjust the patch and submit for the next Commit Fest if
>> this is ok for you.
>>
>
>
> I don't think we'll need this, it will take a bit to explain why though.
>
> First, thanks for returning this topic to discussion and keeping up with all
> the controversy around it.  You said back in February this was your first
> post here, and I doubt you expected that 10 months later this would still be
> active and argued over.  The fact that you're still here and everyone knows
> your name now is itself an accomplishment, many people just give up on their
> submission ideas under far less negative feedback.
>
Why. I need one feature, can spend some time to try to get it and I try.
This is only way to lean that I know.

> I just took a long look at all three of the submissions in this area we've
> gotten.  The central idea that made yours different was making the database
> owner the person allowed to cancel things.  That hadn't been suggested as a
> cancellation requisite before that I know of, and this code may wander in
> that direction one day.  It's just a bit too much to accept right now.  You
> seem to need that specific feature for your environment.  If that's the
> case, you might want to develop something that works that way, but handles
> the concerns raised here.  The fact that it's not acceptable for a database
> owner to cancel a superuser query is the biggest objection, there were some
> others too.  Ultimately it may take a reworking of database permissions to
> really make this acceptable, which is a larger job than I think you were
> trying to get involved with.
>
Probably you have right :(

> Unfortunately, when I look at the new spec we have now, I don't see anything
> from what you did that we can re-use.  It's too specific to the
> owner-oriented idea.  The two other patches that have been submitted both
> are closer to what we've decided we want now.  What I'm going to do here is
> mark your submission "returned with feedback".
>
Again no problem.
The only thing that I need (not only me obviusly) is give the
permission to one or more users
to kill session and query owned by other users.
Have a kind of ACL where is specify who can kill and which is the right way.

My problem is related to production environment where an application
server access the database server and I am the database owner, not the
DBA.
So I need to kill the application server sessions (again I not the
root of application server so I not able to stop and restart it).
I hope to explain my scenario if not before.

> Rather than wait for something new from you, I'm going to review and rework
> the other two submissions.  That I can start on right now.  It's taken so
> long to reach this point that I don't want to wait much longer for another
> submission here, certainly not until over a month from now when the next CF
> starts.  We need to get the arguments around a new version started earlier
> than that.  Thanks for offering to work on this more, and I hope there's
> been something about this long wandering discussion that's been helpful to
> you.  As I said, you did at least make a good first impression, and that is
> worth something when it comes to this group.
>
Thanks Greg.
I hope to meet you at Fosdem if you wil go there.

Best Regards, Torello