Re: pg_terminate_backend and pg_cancel_backend by not administrator user

On Wed, Jun 01, 2011 at 10:26:34PM -0400, Josh Kupershmidt wrote:
> On Wed, Jun 1, 2011 at 5:55 PM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> > On Sun, May 29, 2011 at 10:56:02AM -0400, Josh Kupershmidt wrote:
> >> Looking around, I see there were real problems[1] with sending SIGTERM
> >> to individual backends back in 2005 or so, and pg_terminate_backend()
> >> was only deemed safe enough to put in for 8.4 [2]. So expanding
> >> pg_terminate_backend() privileges does make me a tad nervous.
> >
> > The documentation for the CREATE USER flag would boil down to "omit this flag
> > only if you're worried about undiscovered PostgreSQL bugs in this area". ?I'd
> > echo Tom's sentiment from the first thread, "In any case I think we have to
> > solve it, not create new mechanisms to try to ignore it."
>
> I do agree with Tom's sentiment from that thread. But, if we are
> confident that pg_terminate_backend() is safe enough to relax
> permissions on, then I take it you agree we should plan to extend this
> power to all users?

Yes; that's what I was trying to say.

Having thought about this some more, I do now see a risk. Currently, a SECURITY
DEFINER function (actually any function, but that's where it matters) can trap
query_canceled. By doing so, the author can ensure that only superusers and
crashes may halt the function during a section protected in this way. One might
use it to guard a series of updates made over dblink. pg_terminate_backend()
breaks this protection. I've never designed something this way; it only
suffices when you merely sort-of-care about transactional integrity. Perhaps
it's an acceptable loss for this feature?

> And if so, is this patch a good first step on that path?

Yes.

> >> Reading through those old threads made me realize this patch would
> >> give database owners the ability to kill off autovacuum workers. Seems
> >> like we'd want to restrict that power to superusers.
> >
> > Would we? ?Any old user can already stifle VACUUM by holding a transaction open.
>
> This is true, though it's possible we might at some point want a
> backend process which really shouldn't be killable by non-superusers
> (if vacuum/autovacuum isn't one already.) Actually, I could easily
> imagine a superuser running an important query on a database getting
> peeved if a non-superuser were allowed to cancel/terminate his
> queries.

That's really a different level of user isolation than we have. If your
important query runs on a database owned by someone else, calls functions owned
by someone else, or reads tables owned by someone else, you're substantially at
the mercy of those object owners. That situation probably is unsatisfactory to
some folks. Adding the possibility that a database owner could cancel your
query seems like an extension of that codependency more than a new exposure.

Thanks,
nm