Thread: Re: [GENERAL] column-level update privs + lock table
[Moving to -hackers] On Fri, Oct 15, 2010 at 3:43 AM, Simon Riggs <simon@2ndquadrant.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@gmail.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
Attachment
On Fri, Oct 15, 2010 at 3:49 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote: > [Moving to -hackers] > > On Fri, Oct 15, 2010 at 3:43 AM, Simon Riggs <simon@2ndquadrant.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@gmail.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@gmail.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@2ndquadrant.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@gmail.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@ak.jp.nec.com>
2010/11/25 KaiGai Kohei <kaigai@ak.jp.nec.com>: > (2010/10/16 4:49), Josh Kupershmidt wrote: >> [Moving to -hackers] >> >> On Fri, Oct 15, 2010 at 3:43 AM, Simon Riggs<simon@2ndquadrant.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@gmail.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@gmail.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@ak.jp.nec.com>: > > (2010/10/16 4:49), Josh Kupershmidt wrote: > >> [Moving to -hackers] > >> > >> On Fri, Oct 15, 2010 at 3:43 AM, Simon Riggs<simon@2ndquadrant.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@gmail.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@2ndquadrant.com> wrote: > On Fri, 2010-11-26 at 19:11 -0500, Robert Haas wrote: >> 2010/11/25 KaiGai Kohei <kaigai@ak.jp.nec.com>: >> > (2010/10/16 4:49), Josh Kupershmidt wrote: >> >> [Moving to -hackers] >> >> >> >> On Fri, Oct 15, 2010 at 3:43 AM, Simon Riggs<simon@2ndquadrant.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@gmail.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@ak.jp.nec.com>: >> (2010/10/16 4:49), Josh Kupershmidt wrote: >>> [Moving to -hackers] >>> >>> On Fri, Oct 15, 2010 at 3:43 AM, Simon Riggs<simon@2ndquadrant.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@gmail.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@ak.jp.nec.com>
2010/11/28 KaiGai Kohei <kaigai@ak.jp.nec.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@ak.jp.nec.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@ak.jp.nec.com>
2010/11/28 KaiGai Kohei <kaigai@ak.jp.nec.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@gmail.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@gmail.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, Nov 29, 2010 at 10:06 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Mon, Nov 29, 2010 at 9:37 PM, Josh Kupershmidt <schmiddy@gmail.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@gmail.com> wrote: > On Mon, Nov 29, 2010 at 10:06 PM, Robert Haas <robertmhaas@gmail.com> wrote: >> On Mon, Nov 29, 2010 at 9:37 PM, Josh Kupershmidt <schmiddy@gmail.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 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 Tue, Nov 30, 2010 at 7:26 AM, Simon Riggs <simon@2ndquadrant.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@gmail.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@ak.jp.nec.com>
On Tue, Nov 30, 2010 at 1:48 PM, Tom Lane <tgl@sss.pgh.pa.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