Re: [GENERAL] column-level update privs + lock table

Hi all,

I noticed that granting a user column-level update privileges doesn't
allow that user to issue LOCK TABLE with any mode other than Access
Share.

The documentation page for LOCK TABLE claims: "All other forms of LOCK
require at least one of UPDATE, DELETE, or TRUNCATE privileges.", and
I don't see a good reason why column-level privileges shouldn't be
enough to let the user use LOCK TABLE. Is this just an oversight?

Example below:

CREATE ROLE unpriv WITH LOGIN;
CREATE TABLE bar (id int primary key, comment text);
GRANT SELECT ON bar TO unpriv;
GRANT SELECT, UPDATE (comment) ON TABLE bar TO unpriv;

and then, as user "unpriv":

BEGIN;
LOCK TABLE bar IN ROW SHARE MODE;
COMMIT;

Josh

On Thu, Oct 7, 2010 at 7:43 PM, Josh Kupershmidt <schmiddy(at)gmail(dot)com> wrote:

> I noticed that granting a user column-level update privileges doesn't
> allow that user to issue LOCK TABLE with any mode other than Access
> Share.

Anyone think this could be added as a TODO?

Josh

On Mon, 2010-10-11 at 09:41 -0400, Josh Kupershmidt wrote:
> On Thu, Oct 7, 2010 at 7:43 PM, Josh Kupershmidt <schmiddy(at)gmail(dot)com> wrote:
>
> > I noticed that granting a user column-level update privileges doesn't
> > allow that user to issue LOCK TABLE with any mode other than Access
> > Share.
>
> Anyone think this could be added as a TODO?

Seems so to me, but you raise on Hackers.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Training and Services

[Moving to -hackers]

On Fri, Oct 15, 2010 at 3:43 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On Mon, 2010-10-11 at 09:41 -0400, Josh Kupershmidt wrote:
>> On Thu, Oct 7, 2010 at 7:43 PM, Josh Kupershmidt <schmiddy(at)gmail(dot)com> wrote:
>>
>> > I noticed that granting a user column-level update privileges doesn't
>> > allow that user to issue LOCK TABLE with any mode other than Access
>> > Share.
>>
>> Anyone think this could be added as a TODO?
>
> Seems so to me, but you raise on Hackers.

Thanks, Simon. Attached is a simple patch to let column-level UPDATE
privileges allow a user to LOCK TABLE in a mode higher than Access
Share. Small doc. update and regression test update are included as
well. Feedback is welcome.

Thanks
Josh

On Fri, Oct 15, 2010 at 3:49 PM, Josh Kupershmidt <schmiddy(at)gmail(dot)com> wrote:
> [Moving to -hackers]
>
> On Fri, Oct 15, 2010 at 3:43 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>> On Mon, 2010-10-11 at 09:41 -0400, Josh Kupershmidt wrote:
>>> On Thu, Oct 7, 2010 at 7:43 PM, Josh Kupershmidt <schmiddy(at)gmail(dot)com> wrote:
>>>
>>> > I noticed that granting a user column-level update privileges doesn't
>>> > allow that user to issue LOCK TABLE with any mode other than Access
>>> > Share.
>>>
>>> Anyone think this could be added as a TODO?
>>
>> Seems so to me, but you raise on Hackers.
>
> Thanks, Simon. Attached is a simple patch to let column-level UPDATE
> privileges allow a user to LOCK TABLE in a mode higher than Access
> Share. Small doc. update and regression test update are included as
> well. Feedback is welcome.

Please add this to https://commitfest.postgresql.org/action/commitfest_view/open

I want to look at this at some point, but we still have over a dozen
patches from the current CF to deal with.

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

On Mon, Oct 18, 2010 at 10:27 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> Please add this to https://commitfest.postgresql.org/action/commitfest_view/open
>
> I want to look at this at some point, but we still have over a dozen
> patches from the current CF to deal with.

Added at <https://commitfest.postgresql.org/action/patch_view?id=401>

Josh

(2010/10/16 4:49), Josh Kupershmidt wrote:
> [Moving to -hackers]
>
> On Fri, Oct 15, 2010 at 3:43 AM, Simon Riggs<simon(at)2ndquadrant(dot)com> wrote:
>> On Mon, 2010-10-11 at 09:41 -0400, Josh Kupershmidt wrote:
>>> On Thu, Oct 7, 2010 at 7:43 PM, Josh Kupershmidt<schmiddy(at)gmail(dot)com> wrote:
>>>
>>>> I noticed that granting a user column-level update privileges doesn't
>>>> allow that user to issue LOCK TABLE with any mode other than Access
>>>> Share.
>>>
>>> Anyone think this could be added as a TODO?
>>
>> Seems so to me, but you raise on Hackers.
>
> Thanks, Simon. Attached is a simple patch to let column-level UPDATE
> privileges allow a user to LOCK TABLE in a mode higher than Access
> Share. Small doc. update and regression test update are included as
> well. Feedback is welcome.
>

I checked your patch, then I'd like to mark it as "ready for committer".

The point of this patch is trying to solve an incompatible behavior
between SELECT ... FOR SHARE/UPDATE and LOCK command.

On ExecCheckRTEPerms(), it allows the required accesses when no columns
are explicitly specified in the query and the current user has necessary
privilege on one of columns within the target relation.
If we stand on the perspective that LOCK command should take same
privileges with the case when we use SELECT ... FOR SHARE/UPDATE without
specifying explicit columns, like COUNT(*), the existing LOCK command
seems to me odd.

I think this patch fixes the behavior as we expected.

BTW, how about backporting this patch?
It seems to me a bug fix, although it contains user visible changes.

Thanks,
--
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

2010/11/25 KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>:
> (2010/10/16 4:49), Josh Kupershmidt wrote:
>> [Moving to -hackers]
>>
>> On Fri, Oct 15, 2010 at 3:43 AM, Simon Riggs<simon(at)2ndquadrant(dot)com>  wrote:
>>> On Mon, 2010-10-11 at 09:41 -0400, Josh Kupershmidt wrote:
>>>> On Thu, Oct 7, 2010 at 7:43 PM, Josh Kupershmidt<schmiddy(at)gmail(dot)com>  wrote:
>>>>
>>>>> I noticed that granting a user column-level update privileges doesn't
>>>>> allow that user to issue LOCK TABLE with any mode other than Access
>>>>> Share.
>>>>
>>>> Anyone think this could be added as a TODO?
>>>
>>> Seems so to me, but you raise on Hackers.
>>
>> Thanks, Simon. Attached is a simple patch to let column-level UPDATE
>> privileges allow a user to LOCK TABLE in a mode higher than Access
>> Share. Small doc. update and regression test update are included as
>> well. Feedback is welcome.
>>
>
> I checked your patch, then I'd like to mark it as "ready for committer".
>
> The point of this patch is trying to solve an incompatible behavior
> between SELECT ... FOR SHARE/UPDATE and LOCK command.
>
> On ExecCheckRTEPerms(), it allows the required accesses when no columns
> are explicitly specified in the query and the current user has necessary
> privilege on one of columns within the target relation.
> If we stand on the perspective that LOCK command should take same
> privileges with the case when we use SELECT ... FOR SHARE/UPDATE without
> specifying explicit columns, like COUNT(*), the existing LOCK command
> seems to me odd.
>
> I think this patch fixes the behavior as we expected.

I'm not totally convinced that this is the correct behavior. It seems
a bit surprising that UPDATE privilege on a single column is enough to
lock out all SELECT activity from the table. It's actually a bit
surprising that even full-table UPDATE privileges are enough to do
this, but this change would allow people to block access to data they
can neither see nor modify. That seems counterintuitive, if not a
security hole.

> BTW, how about backporting this patch?
> It seems to me a bug fix, although it contains user visible changes.

I don't think it's a bug fix; and even if could be so construed, I
don't think it's important enough to back-patch.

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

On Fri, Nov 26, 2010 at 7:11 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> I'm not totally convinced that this is the correct behavior.  It seems
> a bit surprising that UPDATE privilege on a single column is enough to
> lock out all SELECT activity from the table.  It's actually a bit
> surprising that even full-table UPDATE privileges are enough to do
> this, but this change would allow people to block access to data they
> can neither see nor modify.  That seems counterintuitive, if not a
> security hole.

The way I see it, it's a Good Thing to encourage people to assign
UPDATE privileges on tables only as minimally as possible. The damage
that a poorly coded or malicious user can do with LOCK TABLE
privileges is insignificant next to the damage they can do with more
UPDATE privileges than they really need.

Right now, we're basically encouraging admins to grant full-table
update privileges when that's not really necessary.

If, in the future, Postgres supports the ability to LOCK TABLE only on
specific columns, I think we could refine this permissions check so
that column-level update privileges only allowed the user to lock
those columns. But I think this patch is a step in the right
direction.

Josh

On Fri, 2010-11-26 at 19:11 -0500, Robert Haas wrote:
> 2010/11/25 KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>:
> > (2010/10/16 4:49), Josh Kupershmidt wrote:
> >> [Moving to -hackers]
> >>
> >> On Fri, Oct 15, 2010 at 3:43 AM, Simon Riggs<simon(at)2ndquadrant(dot)com> wrote:
> >>> On Mon, 2010-10-11 at 09:41 -0400, Josh Kupershmidt wrote:
> >>>> On Thu, Oct 7, 2010 at 7:43 PM, Josh Kupershmidt<schmiddy(at)gmail(dot)com> wrote:
> >>>>
> >>>>> I noticed that granting a user column-level update privileges doesn't
> >>>>> allow that user to issue LOCK TABLE with any mode other than Access
> >>>>> Share.
> >>>>
> >>>> Anyone think this could be added as a TODO?
> >>>
> >>> Seems so to me, but you raise on Hackers.
> >>
> >> Thanks, Simon. Attached is a simple patch to let column-level UPDATE
> >> privileges allow a user to LOCK TABLE in a mode higher than Access
> >> Share. Small doc. update and regression test update are included as
> >> well. Feedback is welcome.
> >>
> >
> > I checked your patch, then I'd like to mark it as "ready for committer".
> >
> > The point of this patch is trying to solve an incompatible behavior
> > between SELECT ... FOR SHARE/UPDATE and LOCK command.
> >
> > On ExecCheckRTEPerms(), it allows the required accesses when no columns
> > are explicitly specified in the query and the current user has necessary
> > privilege on one of columns within the target relation.
> > If we stand on the perspective that LOCK command should take same
> > privileges with the case when we use SELECT ... FOR SHARE/UPDATE without
> > specifying explicit columns, like COUNT(*), the existing LOCK command
> > seems to me odd.
> >
> > I think this patch fixes the behavior as we expected.
>
> I'm not totally convinced that this is the correct behavior. It seems
> a bit surprising that UPDATE privilege on a single column is enough to
> lock out all SELECT activity from the table. It's actually a bit
> surprising that even full-table UPDATE privileges are enough to do
> this, but this change would allow people to block access to data they
> can neither see nor modify. That seems counterintuitive, if not a
> security hole.

This comment misses the point. A user can already lock every row of a
table, if they choose, by issuing SELECT ... FOR SHARE/UPDATE, if they
have update rights on a single column. So the patch does not increase
the rights of the user, it merely allows it to happen in a rational way
and in a way that makes SELECT and LOCK work the same.

--
Simon Riggs http://www.2ndQuadrant.com/books/
PostgreSQL Development, 24x7 Support, Training and Services

On Sun, Nov 28, 2010 at 11:35 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On Fri, 2010-11-26 at 19:11 -0500, Robert Haas wrote:
>> 2010/11/25 KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>:
>> > (2010/10/16 4:49), Josh Kupershmidt wrote:
>> >> [Moving to -hackers]
>> >>
>> >> On Fri, Oct 15, 2010 at 3:43 AM, Simon Riggs<simon(at)2ndquadrant(dot)com>  wrote:
>> >>> On Mon, 2010-10-11 at 09:41 -0400, Josh Kupershmidt wrote:
>> >>>> On Thu, Oct 7, 2010 at 7:43 PM, Josh Kupershmidt<schmiddy(at)gmail(dot)com>  wrote:
>> >>>>
>> >>>>> I noticed that granting a user column-level update privileges doesn't
>> >>>>> allow that user to issue LOCK TABLE with any mode other than Access
>> >>>>> Share.
>> >>>>
>> >>>> Anyone think this could be added as a TODO?
>> >>>
>> >>> Seems so to me, but you raise on Hackers.
>> >>
>> >> Thanks, Simon. Attached is a simple patch to let column-level UPDATE
>> >> privileges allow a user to LOCK TABLE in a mode higher than Access
>> >> Share. Small doc. update and regression test update are included as
>> >> well. Feedback is welcome.
>> >>
>> >
>> > I checked your patch, then I'd like to mark it as "ready for committer".
>> >
>> > The point of this patch is trying to solve an incompatible behavior
>> > between SELECT ... FOR SHARE/UPDATE and LOCK command.
>> >
>> > On ExecCheckRTEPerms(), it allows the required accesses when no columns
>> > are explicitly specified in the query and the current user has necessary
>> > privilege on one of columns within the target relation.
>> > If we stand on the perspective that LOCK command should take same
>> > privileges with the case when we use SELECT ... FOR SHARE/UPDATE without
>> > specifying explicit columns, like COUNT(*), the existing LOCK command
>> > seems to me odd.
>> >
>> > I think this patch fixes the behavior as we expected.
>>
>> I'm not totally convinced that this is the correct behavior.  It seems
>> a bit surprising that UPDATE privilege on a single column is enough to
>> lock out all SELECT activity from the table.  It's actually a bit
>> surprising that even full-table UPDATE privileges are enough to do
>> this, but this change would allow people to block access to data they
>> can neither see nor modify.  That seems counterintuitive, if not a
>> security hole.
>
> This comment misses the point. A user can already lock every row of a
> table, if they choose, by issuing SELECT ... FOR SHARE/UPDATE, if they
> have update rights on a single column. So the patch does not increase
> the rights of the user, it merely allows it to happen in a rational way
> and in a way that makes SELECT and LOCK work the same.

Locking every row of the table allows a user with UPDATE privileges to
block all current UPDATE and DELETE statements, but it won't
necessarily block INSERT statements (depending on unique indices) and
it certainly won't block SELECT statements. This patch proposes to
allow a user with update privileges on a single column to lock out ALL
concurrent activity, reads and writes. So it is not by any definition
making SELECT and LOCK work the same.

What it IS doing is making column-level update permissions and
table-level update permissions work the same way. After all, one
might argue, if full-table update permissions allow a user to take an
access exclusive lock, why not single-column update permissions? I
think, though, that there is a reasonable argument to be made that a
user who has been given UPDATE privileges on the entire table contents
is more trusted than one who has privileges only on certain columns.
The first user can presumably trash the entire table contents if he so
desires; the second one can't.

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

(2010/11/27 9:11), Robert Haas wrote:
> 2010/11/25 KaiGai Kohei<kaigai(at)ak(dot)jp(dot)nec(dot)com>:
>> (2010/10/16 4:49), Josh Kupershmidt wrote:
>>> [Moving to -hackers]
>>>
>>> On Fri, Oct 15, 2010 at 3:43 AM, Simon Riggs<simon(at)2ndquadrant(dot)com> wrote:
>>>> On Mon, 2010-10-11 at 09:41 -0400, Josh Kupershmidt wrote:
>>>>> On Thu, Oct 7, 2010 at 7:43 PM, Josh Kupershmidt<schmiddy(at)gmail(dot)com> wrote:
>>>>>
>>>>>> I noticed that granting a user column-level update privileges doesn't
>>>>>> allow that user to issue LOCK TABLE with any mode other than Access
>>>>>> Share.
>>>>>
>>>>> Anyone think this could be added as a TODO?
>>>>
>>>> Seems so to me, but you raise on Hackers.
>>>
>>> Thanks, Simon. Attached is a simple patch to let column-level UPDATE
>>> privileges allow a user to LOCK TABLE in a mode higher than Access
>>> Share. Small doc. update and regression test update are included as
>>> well. Feedback is welcome.
>>>
>>
>> I checked your patch, then I'd like to mark it as "ready for committer".
>>
>> The point of this patch is trying to solve an incompatible behavior
>> between SELECT ... FOR SHARE/UPDATE and LOCK command.
>>
>> On ExecCheckRTEPerms(), it allows the required accesses when no columns
>> are explicitly specified in the query and the current user has necessary
>> privilege on one of columns within the target relation.
>> If we stand on the perspective that LOCK command should take same
>> privileges with the case when we use SELECT ... FOR SHARE/UPDATE without
>> specifying explicit columns, like COUNT(*), the existing LOCK command
>> seems to me odd.
>>
>> I think this patch fixes the behavior as we expected.
>
> I'm not totally convinced that this is the correct behavior. It seems
> a bit surprising that UPDATE privilege on a single column is enough to
> lock out all SELECT activity from the table. It's actually a bit
> surprising that even full-table UPDATE privileges are enough to do
> this, but this change would allow people to block access to data they
> can neither see nor modify. That seems counterintuitive, if not a
> security hole.
>
In my understanding, UPDATE privilege on a single column also allows
lock out concurrent updating even if this query tries to update rows
partially.
Therefore, the current code considers UPDATE privilege on a single
column is enough to lock out the table. Right?

My comment was from a standpoint which wants consistent behavior
between SELECT ... FOR and LOCK command. If we concerned about this
behavior, ExecCheckRTEPerms() might be a place where we also should fix.

Thanks,
--
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

2010/11/28 KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>:
>> I'm not totally convinced that this is the correct behavior.  It seems
>> a bit surprising that UPDATE privilege on a single column is enough to
>> lock out all SELECT activity from the table.  It's actually a bit
>> surprising that even full-table UPDATE privileges are enough to do
>> this, but this change would allow people to block access to data they
>> can neither see nor modify.  That seems counterintuitive, if not a
>> security hole.
>>
> In my understanding, UPDATE privilege on a single column also allows
> lock out concurrent updating even if this query tries to update rows
> partially.
> Therefore, the current code considers UPDATE privilege on a single
> column is enough to lock out the table. Right?

Against concurrent updates and deletes, yes. Against inserts that
don't involve potential unique-index conflicts, and against selects,
no.

> My comment was from a standpoint which wants consistent behavior
> between SELECT ... FOR and LOCK command.

Again, nothing about this makes those consistent.

> If we concerned about this
> behavior, ExecCheckRTEPerms() might be a place where we also should fix.

I don't understand what you're getting at here.

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

(2010/11/29 10:43), Robert Haas wrote:
> 2010/11/28 KaiGai Kohei<kaigai(at)ak(dot)jp(dot)nec(dot)com>:
>>> I'm not totally convinced that this is the correct behavior. It seems
>>> a bit surprising that UPDATE privilege on a single column is enough to
>>> lock out all SELECT activity from the table. It's actually a bit
>>> surprising that even full-table UPDATE privileges are enough to do
>>> this, but this change would allow people to block access to data they
>>> can neither see nor modify. That seems counterintuitive, if not a
>>> security hole.
>>>
>> In my understanding, UPDATE privilege on a single column also allows
>> lock out concurrent updating even if this query tries to update rows
>> partially.
>> Therefore, the current code considers UPDATE privilege on a single
>> column is enough to lock out the table. Right?
>
> Against concurrent updates and deletes, yes. Against inserts that
> don't involve potential unique-index conflicts, and against selects,
> no.
>
>> My comment was from a standpoint which wants consistent behavior
>> between SELECT ... FOR and LOCK command.
>
> Again, nothing about this makes those consistent.
>
>> If we concerned about this
>> behavior, ExecCheckRTEPerms() might be a place where we also should fix.
>
> I don't understand what you're getting at here.
>
I thought the author concerned about inconsistency between them.
(Perhaps, I might misunderstood his motivation?)

What was the purpose that this patch tries to solve?
In the first message of this topic, he concerned as follows:

> I noticed that granting a user column-level update privileges doesn't
> allow that user to issue LOCK TABLE with any mode other than Access
> Share.

Do we need to answer: "Yes, it is a specification, so you need to grant
table level privileges, instead"?

Thanks
--
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

2010/11/28 KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>:
>>> My comment was from a standpoint which wants consistent behavior
>>> between SELECT ... FOR and LOCK command.
>>
>> Again, nothing about this makes those consistent.
>>
>>> If we concerned about this
>>> behavior, ExecCheckRTEPerms() might be a place where we also should fix.
>>
>> I don't understand what you're getting at here.
>>
> I thought the author concerned about inconsistency between them.
> (Perhaps, I might misunderstood his motivation?)

A user with single-column UPDATE privileges could obtain a ROW
EXCLUSIVE lock by issuing an UPDATE statement, but currently cannot
obtain the same lock using LOCK TABLE. It would be reasonable and
consistent to allow such a user to take a ROW SHARE or ROW EXCLUSIVE
lock using LOCK TABLE, but I'm not sure what the use case for that
would be.

It seems to me that if we're really worried about which locks users
are allowed to take (and so far all of the worrying seems to lack a
solid basis in any sort of usability argument) we'd need to invent
some special-purpose permissions, perhaps one for each lock level.
And we might also want custom permissions for ANALYZE and VACUUM and
each subcommand of ALTER TABLE. The question is, how much of that has
any real benefit? It's probably uncommon to want to dole out such
fine-grained permissions, and our current permissions-granting
infrastructure tops out at 16 individual permissions, so it would need
some rework - particularly, to minimize slowdown of the common case
where you DON'T care about any of these fiddly ultra-fine-grained user
rights.

For LOCK TABLE (or ANALYZE), it appears to be simple to allow users to
lock the table in any mode you like by providing an appropriate
SECURITY DEFINER function. So I think if people want a user who can
update a single column of the table and also take an
AccessExclusiveLock we can just recommend that they do it that way.
This also works for ANALYZE. If you need a user who doesn't own a
table to be able to VACUUM it, that's a bit trickier because VACUUM
can only be invoked as a top-level command, not from within a function
or already-open transaction. Perhaps we can fix this some day if we
implement autonomous transactions, but for now it doesn't really seem
worth losing a lot of sleep over. Just my opinion, of course...

> Do we need to answer: "Yes, it is a specification, so you need to grant
> table level privileges, instead"?

I think that's the most reasonable answer. My vote is to just update
the LOCK TABLE documentation to be more precise about what the rules
are, and move on.

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

On Mon, Nov 29, 2010 at 10:50 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> A user with single-column UPDATE privileges could obtain a ROW
> EXCLUSIVE lock by issuing an UPDATE statement, but currently cannot
> obtain the same lock using LOCK TABLE.  It would be reasonable and
> consistent to allow such a user to take a ROW SHARE or ROW EXCLUSIVE
> lock using LOCK TABLE, but I'm not sure what the use case for that
> would be.

Those limited privileges wouldn't be very useful for my purposes, at
least. I'll try to explain my use-case below.

> It seems to me that if we're really worried about which locks users
> are allowed to take (and so far all of the worrying seems to lack a
> solid basis in any sort of usability argument) we'd need to invent
> some special-purpose permissions, perhaps one for each lock level.

OK, so here's why I wanted column-level update + lock table
privileges. I put together a database application related to
table-synchronization -- basically performing remote table
comparisons. This application needed to update only a single column in
the source table (an updated timestamp), but it needed to be certain
that the source table wasn't changing underneath it.

I ended up just assigning full-table UPDATE privileges to this user,
despite knowing that it only needed to update a single column. I would
have liked to make this privilege restriction explicit in the database
schema, but I can't.

> And we might also want custom permissions for ANALYZE and VACUUM and
> each subcommand of ALTER TABLE.  The question is, how much of that has
> any real benefit?  It's probably uncommon to want to dole out such
> fine-grained permissions, and our current permissions-granting
> infrastructure tops out at 16 individual permissions, so it would need
> some rework - particularly, to minimize slowdown of the common case
> where you DON'T care about any of these fiddly ultra-fine-grained user
> rights.
>
> For LOCK TABLE (or ANALYZE), it appears to be simple to allow users to
> lock the table in any mode you like by providing an appropriate
> SECURITY DEFINER function.  So I think if people want a user who can
> update a single column of the table and also take an
> AccessExclusiveLock we can just recommend that they do it that way.

I actually hadn't thought of that, for some reason.

We used to similarly recommend that people handle TRUNCATE privileges
with a security definer function. That doesn't mean GRANT TRUNCATE
wasn't a sweet addition to 8.4.

> This also works for ANALYZE.  If you need a user who doesn't own a
> table to be able to VACUUM it, that's a bit trickier because VACUUM
> can only be invoked as a top-level command, not from within a function
> or already-open transaction.  Perhaps we can fix this some day if we
> implement autonomous transactions, but for now it doesn't really seem
> worth losing a lot of sleep over.  Just my opinion, of course...
>
>> Do we need to answer: "Yes, it is a specification, so you need to grant
>> table level privileges, instead"?
>
> I think that's the most reasonable answer.  My vote is to just update
> the LOCK TABLE documentation to be more precise about what the rules
> are, and move on.

I still see little reason to make LOCK TABLE permissions different for
column-level vs. table-level UPDATE privileges, but oh well.

Josh

On Mon, Nov 29, 2010 at 9:37 PM, Josh Kupershmidt <schmiddy(at)gmail(dot)com> wrote:
>> It seems to me that if we're really worried about which locks users
>> are allowed to take (and so far all of the worrying seems to lack a
>> solid basis in any sort of usability argument) we'd need to invent
>> some special-purpose permissions, perhaps one for each lock level.
>
> OK, so here's why I wanted column-level update + lock table
> privileges. I put together a database application related to
> table-synchronization -- basically performing remote table
> comparisons. This application needed to update only a single column in
> the source table (an updated timestamp), but it needed to be certain
> that the source table wasn't changing underneath it.

Reasonable... but it doesn't seem unimaginable that someone could want
the opposite behavior, either, for the reasons I stated upthread.

>> And we might also want custom permissions for ANALYZE and VACUUM and
>> each subcommand of ALTER TABLE.  The question is, how much of that has
>> any real benefit?  It's probably uncommon to want to dole out such
>> fine-grained permissions, and our current permissions-granting
>> infrastructure tops out at 16 individual permissions, so it would need
>> some rework - particularly, to minimize slowdown of the common case
>> where you DON'T care about any of these fiddly ultra-fine-grained user
>> rights.
>>
>> For LOCK TABLE (or ANALYZE), it appears to be simple to allow users to
>> lock the table in any mode you like by providing an appropriate
>> SECURITY DEFINER function.  So I think if people want a user who can
>> update a single column of the table and also take an
>> AccessExclusiveLock we can just recommend that they do it that way.
>
> I actually hadn't thought of that, for some reason.
>
> We used to similarly recommend that people handle TRUNCATE privileges
> with a security definer function. That doesn't mean GRANT TRUNCATE
> wasn't a sweet addition to 8.4.

Hmm, glad you like it (I wrote that). I'm just asking how far we
should go before we decide we catering to use cases that are too
narrow to warrant an extension of the permissions system.

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

On Mon, 2010-11-29 at 21:37 -0500, Josh Kupershmidt wrote:

> I still see little reason to make LOCK TABLE permissions different for
> column-level vs. table-level UPDATE privileges

Agreed.

This is the crux of the debate. Why should this inconsistency be allowed
to continue?

Are there covert channel issues here, KaiGai?

--
Simon Riggs http://www.2ndQuadrant.com/books/
PostgreSQL Development, 24x7 Support, Training and Services

On Mon, Nov 29, 2010 at 10:06 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Mon, Nov 29, 2010 at 9:37 PM, Josh Kupershmidt <schmiddy(at)gmail(dot)com> wrote:
>> I actually hadn't thought of that, for some reason.
>>
>> We used to similarly recommend that people handle TRUNCATE privileges
>> with a security definer function. That doesn't mean GRANT TRUNCATE
>> wasn't a sweet addition to 8.4.
>
> Hmm, glad you like it (I wrote that).  I'm just asking how far we
> should go before we decide we catering to use cases that are too
> narrow to warrant an extension of the permissions system.

I am slightly opposed to adding GRANTs for LOCK TABLE, ANALYZE,
VACUUM, etc. The GRANT help page is long enough already, and I doubt
many users would use them, even though I might use GRANT LOCK TABLE
myself.

Josh

On Tue, Nov 30, 2010 at 9:07 AM, Josh Kupershmidt <schmiddy(at)gmail(dot)com> wrote:
> On Mon, Nov 29, 2010 at 10:06 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Mon, Nov 29, 2010 at 9:37 PM, Josh Kupershmidt <schmiddy(at)gmail(dot)com> wrote:
>>> I actually hadn't thought of that, for some reason.
>>>
>>> We used to similarly recommend that people handle TRUNCATE privileges
>>> with a security definer function. That doesn't mean GRANT TRUNCATE
>>> wasn't a sweet addition to 8.4.
>>
>> Hmm, glad you like it (I wrote that).  I'm just asking how far we
>> should go before we decide we catering to use cases that are too
>> narrow to warrant an extension of the permissions system.
>
> I am slightly opposed to adding GRANTs for LOCK TABLE, ANALYZE,
> VACUUM, etc. The GRANT help page is long enough already, and I doubt
> many users would use them, even though I might use GRANT LOCK TABLE
> myself.

You'd really probably want GRANT LOCK TABLE (SHARE), GRANT LOCK TABLE
(EXCLUSIVE), ...

It'd be sort of cool, but it doesn't seem worth the complexity.

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

On Tue, Nov 30, 2010 at 7:26 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On Mon, 2010-11-29 at 21:37 -0500, Josh Kupershmidt wrote:
>
>> I still see little reason to make LOCK TABLE permissions different for
>> column-level vs. table-level UPDATE privileges
>
> Agreed.
>
> This is the crux of the debate. Why should this inconsistency be allowed
> to continue?

Well, a user with full-table UPDATE privileges can trash the whole
thing, so, from a security perspective, letting them lock is only
allowing them to deny access to data they could have just as easily
trashed. A user with only single-column UPDATE privileges cannot
trash the whole table, though, so you are allowing them to deny read
access to data they may not themselves have permission either to read
or to update.

Admittedly, this seems a bit more rickety in light of your point that
they can still lock all the rows... but then that only stops writes,
not reads. I'm less convinced that I'm right about this than I was 3
days ago. But I'm still not convinced that the above argument is
wrong, either.

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

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> Well, a user with full-table UPDATE privileges can trash the whole
> thing, so, from a security perspective, letting them lock is only
> allowing them to deny access to data they could have just as easily
> trashed. A user with only single-column UPDATE privileges cannot
> trash the whole table, though, so you are allowing them to deny read
> access to data they may not themselves have permission either to read
> or to update.

> Admittedly, this seems a bit more rickety in light of your point that
> they can still lock all the rows... but then that only stops writes,
> not reads. I'm less convinced that I'm right about this than I was 3
> days ago. But I'm still not convinced that the above argument is
> wrong, either.

I think what your complaint really boils down to is that LOCK TABLE
is granting excessive permissions to someone who only has table-level
UPDATE privilege. If we were designing that from scratch today, I am
sure we'd have made it tighter, say that UPDATE alone wouldn't give you
more than RowExclusive lock. However, given the lack of complaints
about this from the field, I can't get very excited about a
non-backward-compatible change to tighten LOCK's privilege checking.

I find the argument that column-level update should give weaker locking
privileges than full-table update to be pretty thin. That isn't the
case at the row level; why should it be true at the table level?

However, the other side of the "lack of complaints" argument is that
few people seem to care about whether LOCK TABLE responds to column
level privileges, either. AFAICS this patch is not in response to any
user request but just because Josh thought things were inconsistent.
Which they are, but at a deeper level than this. If we apply this
patch, then we'll be expanding the set of cases where LOCK is granting
privilege too freely, and thus creating more not less
backwards-compatibility problem if we are ever to make it saner.

On the whole I agree with Robert --- let's just adjust the
documentation, and not enlarge privileges in a way we might regret
later.

regards, tom lane

(2010/11/30 21:26), Simon Riggs wrote:
> On Mon, 2010-11-29 at 21:37 -0500, Josh Kupershmidt wrote:
>
>> I still see little reason to make LOCK TABLE permissions different for
>> column-level vs. table-level UPDATE privileges
>
> Agreed.
>
> This is the crux of the debate. Why should this inconsistency be allowed
> to continue?
>
> Are there covert channel issues here, KaiGai?
>
Existing database privilege mechanism (and SELinux, etc...) is not designed
to handle covert channel attacks, basically.
For example, if a user session with column-level UPDATE privilege tries
to update a certain column for each seconds depending on the contents of
other table X, other session can probably know the contents of table X
using iteration of LOCK command without SELECT permission.
It is a typical timing channel attack, but it is not a problem that we
should try to tackle, is it?

Sorry, I don't have a credible idea to solve this inconsistency right now.

Thanks,
--
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

On Tue, Nov 30, 2010 at 1:48 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> On the whole I agree with Robert --- let's just adjust the
> documentation, and not enlarge privileges in a way we might regret
> later.

Done.

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