Re: [HACKERS] Review of Row Level Security
2013/3/25 Simon Riggs si...@2ndquadrant.com: On 19 March 2013 15:08, Kohei KaiGai kai...@kaigai.gr.jp wrote: It is much helpful if someone give me comments around these arguable portions from the standpoint with fresh eyes. My feeling at this point is that I don't think I should commit this and related patches without more work. I certainly have time and willingness to do that in the next release cycle, but I feel like we need substantially longer to confirm that this is as rock solid as it needs to be. With everybody's permission, I'd like to move this to the next CF, with apologies to KaiGai. I have to admit it will become time to move v9.4 development cycle soon, and row-level security patch still needs some more works to merge into the core. At least, it does not stand on 40km point at marathon. One thing we need to consider is, the current version of RLS patch has cut-down functionality about writer side on INSERT / UPDATE commands. So, we anyway needed to work on this feature on v9.4 development cycle. So, I can agree to move this patch to the v9.4 development cycle. Also, I'd like to have discussion for this feature in earlier half of v9.4 to keep time for the remaining features, such as check on writer-side, integration with selinux, and so on. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
I moved Row Level Security patch to the 2013-Next commitfest. 2013/3/27 Kohei KaiGai kai...@kaigai.gr.jp: 2013/3/25 Simon Riggs si...@2ndquadrant.com: On 19 March 2013 15:08, Kohei KaiGai kai...@kaigai.gr.jp wrote: It is much helpful if someone give me comments around these arguable portions from the standpoint with fresh eyes. My feeling at this point is that I don't think I should commit this and related patches without more work. I certainly have time and willingness to do that in the next release cycle, but I feel like we need substantially longer to confirm that this is as rock solid as it needs to be. With everybody's permission, I'd like to move this to the next CF, with apologies to KaiGai. I have to admit it will become time to move v9.4 development cycle soon, and row-level security patch still needs some more works to merge into the core. At least, it does not stand on 40km point at marathon. One thing we need to consider is, the current version of RLS patch has cut-down functionality about writer side on INSERT / UPDATE commands. So, we anyway needed to work on this feature on v9.4 development cycle. So, I can agree to move this patch to the v9.4 development cycle. Also, I'd like to have discussion for this feature in earlier half of v9.4 to keep time for the remaining features, such as check on writer-side, integration with selinux, and so on. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 27 March 2013 10:57, Kohei KaiGai kai...@kaigai.gr.jp wrote: 2013/3/25 Simon Riggs si...@2ndquadrant.com: On 19 March 2013 15:08, Kohei KaiGai kai...@kaigai.gr.jp wrote: It is much helpful if someone give me comments around these arguable portions from the standpoint with fresh eyes. My feeling at this point is that I don't think I should commit this and related patches without more work. I certainly have time and willingness to do that in the next release cycle, but I feel like we need substantially longer to confirm that this is as rock solid as it needs to be. With everybody's permission, I'd like to move this to the next CF, with apologies to KaiGai. I have to admit it will become time to move v9.4 development cycle soon, and row-level security patch still needs some more works to merge into the core. At least, it does not stand on 40km point at marathon. One thing we need to consider is, the current version of RLS patch has cut-down functionality about writer side on INSERT / UPDATE commands. So, we anyway needed to work on this feature on v9.4 development cycle. So, I can agree to move this patch to the v9.4 development cycle. Also, I'd like to have discussion for this feature in earlier half of v9.4 to keep time for the remaining features, such as check on writer-side, integration with selinux, and so on. Thanks for your hard work, and understanding. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 19 March 2013 15:08, Kohei KaiGai kai...@kaigai.gr.jp wrote: It is much helpful if someone give me comments around these arguable portions from the standpoint with fresh eyes. My feeling at this point is that I don't think I should commit this and related patches without more work. I certainly have time and willingness to do that in the next release cycle, but I feel like we need substantially longer to confirm that this is as rock solid as it needs to be. With everybody's permission, I'd like to move this to the next CF, with apologies to KaiGai. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security - call for testers/reviewers
On 01/16/2013 06:28 AM, Kohei KaiGai wrote: The attached patch is row-security v9. Has anyone had a chance to check this out? It's seen a lot of work over a long time and looks really valuable if it's solid enough. Kohei KaiGai, is there any chance you can go through Simon's review and offer an itemized response showing what changes you've made related to each review point? It might help encourage movement on this patch if it's easier to see what's changed since the last discussion and review. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
KaiGai, * Kohei KaiGai (kai...@kaigai.gr.jp) wrote: IMO, only parser should accept command types except for ALL but raise an error something like it is not supported yet to protect from syntax conflicts. Right, I agree with this. Right now, I plan to submit a revised patch with the syntax support above, and without support for INSERT or NEW of UPDATE checks, as minimum set of functionality for v9.3. Sounds good. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Review of Row Level Security
2012/12/31 Simon Riggs si...@2ndquadrant.com: On 23 December 2012 18:49, Simon Riggs si...@2ndquadrant.com wrote: Anyway, hope you can make call on 28th so we can discuss this and agree a way forwards you're happy with. Stephen, KaiGai and myself met by phone on 28th to discuss. 1. The actual default is not that important to any of us. We could go either way, or have no default at all. 2. What we do want is a declarative way of specifying row security, with options to support all use cases discussed/requested on list. We shouldn't support just one of those use cases and force everybody else to use triggers manually for the other cases. 3. We want to have the possibility of multiple row security expressions, defined for different privilege types (SELECT, UPDATE, INSERT, DELETE). (Note that this means you'd be able to specify that an update could read a row in one security mode by setting SELECT, then update that row to a new security mode by setting a clause on UPDATE - hence we refer to those as privileges not commands/events). The expressions should be separate so they can be pushed easily into query plans (exactly as in the current patch). Stephen has updated the Wiki with some ideas on how that can be structured https://wiki.postgresql.org/wiki/RLS 4. Supporting multiple expressions may not be possible for 9.3, but if not, we want to agree now what the syntax is to make sure we have a clear route for future development. If we can agree this quickly we increase the chances of KaiGai successfully implementing that. The syntax being discussed were below: ALTER TABLE relname SET ROW SECURITY FOR privilege TO (expression); ALTER TABLE relname RESET ROW SECURITY FOR privilege; privilege can be one of: ALL, SELECT, INSERT, UPDATE, DELETE The point in development towards v9.3 is, we only support ALL but we can add other command types in the future. IMO, only parser should accept command types except for ALL but raise an error something like it is not supported yet to protect from syntax conflicts. Right now, I plan to submit a revised patch with the syntax support above, and without support for INSERT or NEW of UPDATE checks, as minimum set of functionality for v9.3. Please give me some suggestions, if you have different opinion towards the overall direction, until 15th-Jan. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On Wed, Jan 02, 2013 at 05:35:13PM +0100, Kohei KaiGai wrote: 2012/12/31 Simon Riggs si...@2ndquadrant.com: On 23 December 2012 18:49, Simon Riggs si...@2ndquadrant.com wrote: Anyway, hope you can make call on 28th so we can discuss this and agree a way forwards you're happy with. Stephen, KaiGai and myself met by phone on 28th to discuss. 1. The actual default is not that important to any of us. We could go either way, or have no default at all. 2. What we do want is a declarative way of specifying row security, with options to support all use cases discussed/requested on list. We shouldn't support just one of those use cases and force everybody else to use triggers manually for the other cases. 3. We want to have the possibility of multiple row security expressions, defined for different privilege types (SELECT, UPDATE, INSERT, DELETE). (Note that this means you'd be able to specify that an update could read a row in one security mode by setting SELECT, then update that row to a new security mode by setting a clause on UPDATE - hence we refer to those as privileges not commands/events). The expressions should be separate so they can be pushed easily into query plans (exactly as in the current patch). Stephen has updated the Wiki with some ideas on how that can be structured https://wiki.postgresql.org/wiki/RLS 4. Supporting multiple expressions may not be possible for 9.3, but if not, we want to agree now what the syntax is to make sure we have a clear route for future development. If we can agree this quickly we increase the chances of KaiGai successfully implementing that. The syntax being discussed were below: ALTER TABLE relname SET ROW SECURITY FOR privilege TO (expression); ALTER TABLE relname RESET ROW SECURITY FOR privilege; privilege can be one of: ALL, SELECT, INSERT, UPDATE, DELETE The point in development towards v9.3 is, we only support ALL but we can add other command types in the future. IMO, only parser should accept command types except for ALL but raise an error something like it is not supported yet to protect from syntax conflicts. Great! Would COPY be covered separately? How about TRUNCATE? Also, is there any way to apply this to the catalog, or would that be too large a restructuring, given how catalog access actually works? Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 2 January 2013 17:19, David Fetter da...@fetter.org wrote: Would COPY be covered separately? How about TRUNCATE? COPY == INSERT TRUNCATE isn't covered at all since it doesn't look at rows. It has a separate privilege that can be granted to those that need it. Also, is there any way to apply this to the catalog, or would that be too large a restructuring, given how catalog access actually works? Doubt it. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On Wed, Jan 02, 2013 at 05:31:42PM +, Simon Riggs wrote: On 2 January 2013 17:19, David Fetter da...@fetter.org wrote: Would COPY be covered separately? How about TRUNCATE? COPY == INSERT Makes sense. The reason I mentioned it is that COPY is supposed to be a very fast bulk loading process, which implies a couple of things: 1. In the RLS (really should be RLAC, but let's not go there now) case, COPY makes it pretty simple to probe hugely many things at once for existence unless there's some kind of COPY pre-processor that throws away non-matching rows. Fortunately there's work being done to that end. 2. COPY, being intended to be very, very fast, should probably get some kind of notation, at least in the docs, about how it will slow down in the RLS case. TRUNCATE isn't covered at all since it doesn't look at rows. It has a separate privilege that can be granted to those that need it. OK Also, is there any way to apply this to the catalog, or would that be too large a restructuring, given how catalog access actually works? Doubt it. Somewhat related issue: Is there a worked example of setting up PostgreSQL to a default deny access policy as far as is possible today? This touches a lot of things, among them reading the catalog. Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 23 December 2012 18:49, Simon Riggs si...@2ndquadrant.com wrote: Anyway, hope you can make call on 28th so we can discuss this and agree a way forwards you're happy with. Stephen, KaiGai and myself met by phone on 28th to discuss. 1. The actual default is not that important to any of us. We could go either way, or have no default at all. 2. What we do want is a declarative way of specifying row security, with options to support all use cases discussed/requested on list. We shouldn't support just one of those use cases and force everybody else to use triggers manually for the other cases. 3. We want to have the possibility of multiple row security expressions, defined for different privilege types (SELECT, UPDATE, INSERT, DELETE). (Note that this means you'd be able to specify that an update could read a row in one security mode by setting SELECT, then update that row to a new security mode by setting a clause on UPDATE - hence we refer to those as privileges not commands/events). The expressions should be separate so they can be pushed easily into query plans (exactly as in the current patch). Stephen has updated the Wiki with some ideas on how that can be structured https://wiki.postgresql.org/wiki/RLS 4. Supporting multiple expressions may not be possible for 9.3, but if not, we want to agree now what the syntax is to make sure we have a clear route for future development. If we can agree this quickly we increase the chances of KaiGai successfully implementing that. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 22 December 2012 20:13, Kevin Grittner kgri...@mail.com wrote: I apologize again for coming in so late with strong opinions, but I thought I knew what row level security meant, and it was just a question of how to do it, but I can't reconcile what I thought the feature was about with the patch I'm seeing; perhaps it's just a lack of the hight level context that's making it difficult. Agreed, I think we're all feeling that. I'll do my best to accommodate all viewpoints. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
2012/12/22 Kevin Grittner kgri...@mail.com: Kohei KaiGai wrote: RLS entry of wiki has not been updated for long time, I'll try to update the entry for high-level design in a couple of days. Thanks, I think that is essential for a productive discussion of the issue. I tried to update http://wiki.postgresql.org/wiki/RLS I backed to the definition of feature for information security; that requires to ensure confidentiality, integrity and availability (C.I.A) of information asset managed by system. Access control contributes the first two elements. So, I'm inclined RLS feature eventually support reader-side and writer-side, to prevent unprivileged rows are read or written. If I could introduce the most conceptual stuff in one statement, it shall be: Overall, RLS prevents users to read and write rows that does not satisfies the row-security policy being configured on the table by the table owner. Reader-side ensures confidentiality of data, writer-side ensures integrity of data. Also note that, I believe this criteria never deny to have multiple (asymmetric) row-security policy for each command type, as long as we care about problematic scenario properly. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 21 December 2012 16:51, Kevin Grittner kgri...@mail.com wrote:
It would be easy enough to include read/write variable clauses by
using a function similar to TG_OP for triggers, e.g. StatementType.
That would make the security clause that applied only to reads into
something like this (StatementType('INSERT, UPDATE') OR other-quals).
In the case where the logic in encapsulated into a function, what
are the logical differences from a BEFORE EACH ROW trigger?
Logically it is broadly similar to a CHECK constraint, which is also
similar to a trigger in a few ways.
Implemented as a BEFORE EACH ROW trigger it would need to be a new
class of trigger that always executed after all other BEFORE EACH ROW
triggers had executed, in the same way that security barrier views act
last. The act last bit seems to be the most important thing here,
just as it was for security barriers and by analogy a string enough
reason to add specific syntax to support this case. (Syntax as yet
undecided...)
If
none, and this is strictly an optimization, what are the benchmarks
showing?
AFAIK its well known that a check constraint is much faster than a
trigger. The objection to using triggers is partially for that reason
and partially because of the admin overhead, as I already said.
Adding a trigger could be done automatically, just as the current
patch does with the check constraint approach. So if anyone has
benchmarks that show triggers are actually faster, then we could add
that automatically instead, I guess.
Anyway, hope you can make call on 28th so we can discuss this and
agree a way forwards you're happy with.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
Simon Riggs si...@2ndquadrant.com writes: On 21 December 2012 16:51, Kevin Grittner kgri...@mail.com wrote: If none, and this is strictly an optimization, what are the benchmarks showing? AFAIK its well known that a check constraint is much faster than a trigger. I don't believe that that's well known at all, at least not for apples-to-apples comparison cases. A C-coded BEFORE trigger doesn't have very much overhead; I suspect it's probably comparable to expression evaluation setup overhead. I think if you want to argue for this on performance grounds, you need to actually prove there's a significant performance advantage, not just assume there will be. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 23 December 2012 19:16, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: On 21 December 2012 16:51, Kevin Grittner kgri...@mail.com wrote: If none, and this is strictly an optimization, what are the benchmarks showing? AFAIK its well known that a check constraint is much faster than a trigger. I don't believe that that's well known at all, at least not for apples-to-apples comparison cases. A C-coded BEFORE trigger doesn't have very much overhead; I suspect it's probably comparable to expression evaluation setup overhead. I think if you want to argue for this on performance grounds, you need to actually prove there's a significant performance advantage, not just assume there will be. If you want to see some tests, I'm sure those can be arranged, no problem. But honestly, if its low enough, then which is the fastest will likely be moot in comparison with the cost of a non-C coded role-based security check. So I think our attention is best spent on providing a few likely C-coded security checks, so we're able to address the whole performance concern not just the constraint/trigger debate. That still leaves the points about ensuring the trigger/checks are executed last and also that they are added automatically, rather than requiring them to be added manually. As KaiGai points out, if they are added automatically, it doesn't really matter which we pick. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
Kohei KaiGai wrote: RLS entry of wiki has not been updated for long time, I'll try to update the entry for high-level design in a couple of days. Thanks, I think that is essential for a productive discussion of the issue. For me, it would help tremendously if you could provide a very short statement of the over-arching goal of the current development effort. As an example, I could summarize the SSI development as: Ensure that the result of executing any set of successfully committed serializable transactions is the same as having run those transactions one at a time, without introducing any new blocking. Proceeding from a general goal statement like that, to general principles of how it will be achieved before getting down to implementation details helps me put the details in proper context. I apologize again for coming in so late with strong opinions, but I thought I knew what row level security meant, and it was just a question of how to do it, but I can't reconcile what I thought the feature was about with the patch I'm seeing; perhaps it's just a lack of the hight level context that's making it difficult. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 20 December 2012 19:42, Stephen Frost sfr...@snowman.net wrote: Kevin, all, * Kevin Grittner (kgri...@mail.com) wrote: The more secure behavior is to allow entry of data which will not be visible by the person doing the entry. wrt this- I'm inclined to agree with Kevin. It's certainly common in certain environments that you can write to a higher level than you can read from. Granting those writers access to read the data later would be... difficult. What we're really arguing about here, afaict, is what the default should be. In line with Kevin's comments and Tom's reading of the spec (along with my own experience in these environments), I'd argue for the default to allow writing rows you're not allowed to read. It would certainly be ideal if we could support both options, on a per-relation basis, when we release the overall feature. It doesn't feel like it'd be a lot of work to do that, but I've not been able to follow this discussion up til now. Thankfully, I'm hopeful that I'm going to have more time now to keep up with PG. :) It is certainly possible to deliver a row security feature that covers all the cases discussed in 9.3. I want that. 1. The case of row security applies to all commands is simple to implement and document, since it presents no anomalies. 2. As KaiGai has explained, there are significant anomalies in the behaviour of row security applies only to reads. Those anomalies need to be analysed carefully. They also need to be explained to the user in documentation. It's unreasonable for people to demand a feature yet provide no guidance to the person trying (hard) to provide that feature in a sensible way. If people genuinely believe case (2) is worth pursuing, additional work and input is needed so that KaiGai can make changes in time for the 9.3 deadline. Please read what KaiGai has said and respond. Since there are so many people reading this thread and wanting (2), that seems reasonable to expect. What I have proposed is that I work on the review for case (1) and then if we solve (2) that can go in also. I don't think its reasonable to reject the whole feature because of unresolved difficulties around one use case, which is what will happen if this is seen as merely a debate about defaults. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 21 December 2012 08:56, Simon Riggs si...@2ndquadrant.com wrote: It's unreasonable for people to demand a feature yet provide no guidance to the person trying (hard) to provide that feature in a sensible way. If people genuinely believe case (2) is worth pursuing, additional work and input is needed so that KaiGai can make changes in time for the 9.3 deadline. Please read what KaiGai has said and respond. Since there are so many people reading this thread and wanting (2), that seems reasonable to expect. What I have proposed is that I work on the review for case (1) and then if we solve (2) that can go in also. I don't think its reasonable to reject the whole feature because of unresolved difficulties around one use case, which is what will happen if this is seen as merely a debate about defaults. One comment on the code itself -- I think it needs some locking of rows from the subquery to ensure correct concurrency behaviour when there are multiple transactions doing updates at the same time. Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 21 December 2012 09:29, Dean Rasheed dean.a.rash...@gmail.com wrote: On 21 December 2012 08:56, Simon Riggs si...@2ndquadrant.com wrote: It's unreasonable for people to demand a feature yet provide no guidance to the person trying (hard) to provide that feature in a sensible way. If people genuinely believe case (2) is worth pursuing, additional work and input is needed so that KaiGai can make changes in time for the 9.3 deadline. Please read what KaiGai has said and respond. Since there are so many people reading this thread and wanting (2), that seems reasonable to expect. What I have proposed is that I work on the review for case (1) and then if we solve (2) that can go in also. I don't think its reasonable to reject the whole feature because of unresolved difficulties around one use case, which is what will happen if this is seen as merely a debate about defaults. One comment on the code itself -- I think it needs some locking of rows from the subquery to ensure correct concurrency behaviour when there are multiple transactions doing updates at the same time. Another comment -- the use of the RangeTblEntry structure in the new code is a bit odd. It seems to be creating an RTE that is both an RTE_RELATION and an RTE_SUBQUERY at the same time. I think it would be cleaner to just add a new RTE for the subquery (cf. the trigger-updatable view code in ApplyRetrieveRule). Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On Thu, Dec 20, 2012 at 4:50 PM, Kevin Grittner kgri...@mail.com wrote: I don't think I like ALTER TABLE as a syntax for row level security. How about using existing GRANT syntax but allowing a WHERE clause? That seems more natural to me, and it would make it easy to apply the same conditions to multiple types of operations when desired, but use different expressions when desired. Without having spent a lot of time pondering it, I think that if row level SELECT permissions exist, they would need to be met on the OLD tuple to allow DELETE or UPDATE, and UPDATE row level permissions would be applied to the NEW tuple. This gets thorny if a role inherits from multiple roles each having a different RLS predicate. You can OR them together, but performance will likely suck. I initially thought of this as well, but I think it's just too ugly to live. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On Fri, Dec 21, 2012 at 3:56 AM, Simon Riggs si...@2ndquadrant.com wrote: It's unreasonable for people to demand a feature yet provide no guidance to the person trying (hard) to provide that feature in a sensible way. You've got it backwards. All the issues that are being discussed now on this thread have been discussed previously on prior threads about row-level security. For the most part, nobody other than KaiGai and I participated in those, and we had a consensus hammered out that was reflected in the patch that started this thread. The person insisting on an eleventh-hour design change is you. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 21 December 2012 14:19, Robert Haas robertmh...@gmail.com wrote: On Fri, Dec 21, 2012 at 3:56 AM, Simon Riggs si...@2ndquadrant.com wrote: It's unreasonable for people to demand a feature yet provide no guidance to the person trying (hard) to provide that feature in a sensible way. You've got it backwards. All the issues that are being discussed now on this thread have been discussed previously on prior threads about row-level security. For the most part, nobody other than KaiGai and I participated in those, and we had a consensus hammered out that was reflected in the patch that started this thread. The person insisting on an eleventh-hour design change is you. Forwards or backwards, the problems of that previous design still exist and need some attention before we can commit. If you can spend some time investigating the problems and documenting how it works in that mode, it would be a great help for this patch. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
Kohei KaiGai wrote: I don't think I like ALTER TABLE as a syntax for row level security. How about using existing GRANT syntax but allowing a WHERE clause? That seems more natural to me, and it would make it easy to apply the same conditions to multiple types of operations when desired, but use different expressions when desired. It seems to me this syntax gives an impression that row-security feature is tightly combined with role-mechanism, even though it does not need. For example, we can set row-security policy of primary key is even/odd number, independent from working role. Is there some high-level discussion of the concept of row level security that operates independently of roles? I'm having a little trouble getting my head around the idea, there is no README in the patch, and the Wiki page on RLS hasn't been updated for two and a half years and seems to be mainly discussing the possibility of adding non-leaky views (which we now have). If it doesn't control which roles have access to which rows, what does it do, conceptually? (A URL to a page explaining this would be fine.) -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 21 December 2012 14:48, Kevin Grittner kgri...@mail.com wrote:
Kohei KaiGai wrote:
I don't think I like ALTER TABLE as a syntax for row level
security. How about using existing GRANT syntax but allowing a
WHERE clause? That seems more natural to me, and it would make
it easy to apply the same conditions to multiple types of
operations when desired, but use different expressions when
desired.
It seems to me this syntax gives an impression that row-security
feature is tightly combined with role-mechanism, even though it
does not need. For example, we can set row-security policy of
primary key is even/odd number, independent from working role.
Is there some high-level discussion of the concept of row level
security that operates independently of roles? I'm having a little
trouble getting my head around the idea, there is no README in the
patch, and the Wiki page on RLS hasn't been updated for two and a
half years and seems to be mainly discussing the possibility of
adding non-leaky views (which we now have). If it doesn't control
which roles have access to which rows, what does it do,
conceptually? (A URL to a page explaining this would be fine.)
There's some docs there, but that needs improving.
Each table has a single security clause. The clause doesn't enforce
that it must contain something that depends on role, but that is the
most easily understood usage of it. We do that to ensure that you can
embed the intelligence into a function, say hasRowLevelAccess(), which
doesn't have any provable relationship on role, and maybe wouldn't do
anything in unit test, yet clearly in production it would do so.
It would be easy enough to include read/write variable clauses by
using a function similar to TG_OP for triggers, e.g. StatementType.
That would make the security clause that applied only to reads into
something like this (StatementType('INSERT, UPDATE') OR other-quals).
If you push for GRANT ... WHERE then you may as well just say you want
the patch cancelled in this release. There's no way to analyze, design
and code something like that in 3 weeks. As I've said, I single
table-level policy is much easier to manage anyway.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
Simon Riggs wrote:
Each table has a single security clause. The clause doesn't enforce
that it must contain something that depends on role, but that is the
most easily understood usage of it. We do that to ensure that you can
embed the intelligence into a function, say hasRowLevelAccess(), which
doesn't have any provable relationship on role, and maybe wouldn't do
anything in unit test, yet clearly in production it would do so.
It would be easy enough to include read/write variable clauses by
using a function similar to TG_OP for triggers, e.g. StatementType.
That would make the security clause that applied only to reads into
something like this (StatementType('INSERT, UPDATE') OR other-quals).
In the case where the logic in encapsulated into a function, what
are the logical differences from a BEFORE EACH ROW trigger? If
none, and this is strictly an optimization, what are the benchmarks
showing?
-Kevin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
2012/12/21 Kevin Grittner kgri...@mail.com:
Simon Riggs wrote:
Each table has a single security clause. The clause doesn't enforce
that it must contain something that depends on role, but that is the
most easily understood usage of it. We do that to ensure that you can
embed the intelligence into a function, say hasRowLevelAccess(), which
doesn't have any provable relationship on role, and maybe wouldn't do
anything in unit test, yet clearly in production it would do so.
It would be easy enough to include read/write variable clauses by
using a function similar to TG_OP for triggers, e.g. StatementType.
That would make the security clause that applied only to reads into
something like this (StatementType('INSERT, UPDATE') OR other-quals).
In the case where the logic in encapsulated into a function, what
are the logical differences from a BEFORE EACH ROW trigger? If
none, and this is strictly an optimization, what are the benchmarks
showing?
It seems to me we need some more discussion about design and
implementation on row-security checks of writer-side, to reach our
consensus.
On the other hand, we are standing next to the consensus about
reader-side; a unique row-security policy (so, first version does not
support per-command policy) shall be checked on table scanning
on select, update or delete commands.
I'd like to suggest these arguable stuff being postponed to v9.4,
and we like to focus on the minimum / core functionality towards
the deadline of v9.3.
I think it is the most productive way to enhance our features.
I can understand Simon's opinion; security feature should not be
defective item. However, please don't forget sepgsql started from
just a small functional module to check DML permissions only.
How about folk's opinion?
Thanks,
--
KaiGai Kohei kai...@kaigai.gr.jp
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
KaiGai, all,
* Kohei KaiGai (kai...@kaigai.gr.jp) wrote:
2012/12/21 Kevin Grittner kgri...@mail.com:
Simon Riggs wrote:
Each table has a single security clause. The clause doesn't enforce
that it must contain something that depends on role, but that is the
most easily understood usage of it. We do that to ensure that you can
embed the intelligence into a function, say hasRowLevelAccess(), which
doesn't have any provable relationship on role, and maybe wouldn't do
anything in unit test, yet clearly in production it would do so.
It would be easy enough to include read/write variable clauses by
using a function similar to TG_OP for triggers, e.g. StatementType.
That would make the security clause that applied only to reads into
something like this (StatementType('INSERT, UPDATE') OR other-quals).
In the case where the logic in encapsulated into a function, what
are the logical differences from a BEFORE EACH ROW trigger? If
none, and this is strictly an optimization, what are the benchmarks
showing?
I'm trying to understand this piece also. It sounds like all we're
doing at the moment is adding different syntax to define a trigger
function and that's hardly what I, at least, was expecting. If a
function has to be written to have RLS, then I think we've failed. Much
of the point of this is to provide an easy solution which gets all the
details right for users who aren't comfortable or savvy enough to just
write functions/security definer views/etc themselves.
It seems to me we need some more discussion about design and
implementation on row-security checks of writer-side, to reach our
consensus.
Again, I agree with Kevin on this- there should be a wiki or similar
which actually outlines the high-level design, syntax, etc. That would
help us understand and be able to meaningfully comment about the
approach.
On the other hand, we are standing next to the consensus about
reader-side; a unique row-security policy (so, first version does not
support per-command policy) shall be checked on table scanning
on select, update or delete commands.
I don't feel that we've really reached a consensus about the
'reader-side' implemented in this patch- rather, we've agreed (at a
pretty high level) what the default impact of RLS for SELECT queries is.
While I'm glad that we were able to do that, I'm rather dismayed that it
took a great deal of discussion to get to that point.
Thanks,
Stephen
signature.asc
Description: Digital signature
Re: [HACKERS] Review of Row Level Security
On 21 December 2012 22:01, Stephen Frost sfr...@snowman.net wrote: On the other hand, we are standing next to the consensus about reader-side; a unique row-security policy (so, first version does not support per-command policy) shall be checked on table scanning on select, update or delete commands. I don't feel that we've really reached a consensus about the 'reader-side' implemented in this patch- rather, we've agreed (at a pretty high level) what the default impact of RLS for SELECT queries is. While I'm glad that we were able to do that, I'm rather dismayed that it took a great deal of discussion to get to that point. Would anybody like to discuss this on a conference call on say 28th Dec, to see if we can agree a way forwards? I feel certain that we can work through any difficulties and agree a minimal subset for change. All comers welcome, just contact me offlist for details. I've got literally nothing riding on this, other than my desire for progress, so I don't see the need for going too many extra miles if nobody else does. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
Simon, * Simon Riggs (si...@2ndquadrant.com) wrote: Would anybody like to discuss this on a conference call on say 28th Dec, to see if we can agree a way forwards? I feel certain that we can work through any difficulties and agree a minimal subset for change. All comers welcome, just contact me offlist for details. Thank you for the offer, I agree that it's a good idea. I'd be happy to (and would like to) participate. I've got literally nothing riding on this, other than my desire for progress, so I don't see the need for going too many extra miles if nobody else does. I'd like to make progress also but let's make sure we're going in the right direction. :) Thanks again, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Review of Row Level Security
2012/12/21 Stephen Frost sfr...@snowman.net: It seems to me we need some more discussion about design and implementation on row-security checks of writer-side, to reach our consensus. Again, I agree with Kevin on this- there should be a wiki or similar which actually outlines the high-level design, syntax, etc. That would help us understand and be able to meaningfully comment about the approach. I also. RLS entry of wiki has not been updated for long time, I'll try to update the entry for high-level design in a couple of days. On the other hand, we are standing next to the consensus about reader-side; a unique row-security policy (so, first version does not support per-command policy) shall be checked on table scanning on select, update or delete commands. I don't feel that we've really reached a consensus about the 'reader-side' implemented in this patch- rather, we've agreed (at a pretty high level) what the default impact of RLS for SELECT queries is. While I'm glad that we were able to do that, I'm rather dismayed that it took a great deal of discussion to get to that point. Thanks, Stephen -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
2012/12/22 Simon Riggs si...@2ndquadrant.com: On 21 December 2012 22:01, Stephen Frost sfr...@snowman.net wrote: On the other hand, we are standing next to the consensus about reader-side; a unique row-security policy (so, first version does not support per-command policy) shall be checked on table scanning on select, update or delete commands. I don't feel that we've really reached a consensus about the 'reader-side' implemented in this patch- rather, we've agreed (at a pretty high level) what the default impact of RLS for SELECT queries is. While I'm glad that we were able to do that, I'm rather dismayed that it took a great deal of discussion to get to that point. Would anybody like to discuss this on a conference call on say 28th Dec, to see if we can agree a way forwards? I feel certain that we can work through any difficulties and agree a minimal subset for change. All comers welcome, just contact me offlist for details. Of course, I'll join the conference. Please give me the detail. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 20 December 2012 00:24, Robert Haas robertmh...@gmail.com wrote: On Wed, Dec 19, 2012 at 12:54 PM, Simon Riggs si...@2ndquadrant.com wrote: I can see a use case for not having security apply for users who have *only* INSERT privilege. This would allow people to run bulk loads of data into a table with row security. We should add that. That is not the common case, so with proper documentation that should be a useful feature without relaxing default security. Never applying security for INSERT and then forcing them to add BEFORE triggers if they want full security is neither secure nor performant. I think INSERT vs. not-INSERT is not the relevant distinction, because the question also arises for UPDATE. Not sure I understand you. You suggested it was a valid use case for a user to have only INSERT privilege and wish to bypass security checks. I agreed and suggested it could be special-cased. In the UPDATE case, the question is whether the RLS qual should be checked only against the OLD tuple (to make sure that we can see the tuple to modify it) or also against the NEW tuple (to make sure that we're not modifying it to a form that we can no longer see). In other words, the question is not do we support all of the commands? but rather do we check not only the tuple read but also the tuple written?. For INSERT, we only write a tuple, without reading. For SELECT and DELETE, we only read a tuple, without writing a new one. UPDATE does both a read and a write. I'm not sure what this comment adds to the discussion. What you say is understood. Previously, I suggested that we handle this by enforcing row-level security only on data read from the table - the OLD row, so to speak - and not on data written to the table - the NEW row, so to speak - because the latter case can be handled well enough by triggers. (The OLD case cannot, because not seeing the row is different from erroring out when you do see it.) There are other alternatives, like allowing the user to specify which behavior they want. But I think that simply decreeing that the policy will apply not only to rows read but also rows written in all cases will be less flexible than we will ultimately want to be. As discussed, we should add a security feature that is secure by default. Adding options to make it less secure can follow initial commit. We might even make it in this release if the review of the main feature goes well. Saying that something is or is not secure is not meaningful without defining what you want to be secure against. There's nothing insecure about checking only the tuples read; it's just a different (and useful) threat model. There are three main points * Applies to all commands should not be implemented via triggers. Complex, slow, unacceptable thing to force upon users. Doing that begs the question of why we would have the feature at all, since we already have triggers and barrier views. * the default for row security should be applies to all commands. Anything else may be useful in some cases, but is surprising to users and requires careful thought to determine if it is appropriate. * How to handle asymmetric row security policies? KaiGai has already begun discussing problems caused by a security policy that differs between reads/writes, on his latest patch post. That needs further analysis to check that it actually makes sense to allow it, since it is more complex. It would be better to fully analyse that situation and post solutions, rather than simply argue its OK. Kevin has made good arguments to show there could be value in such a setup; nobody has talked about banning it, but we do need analysis, suggested syntax/mechanisms and extensive documentation to explain it etc. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
Kevin, all, * Kevin Grittner (kgri...@mail.com) wrote: The more secure behavior is to allow entry of data which will not be visible by the person doing the entry. wrt this- I'm inclined to agree with Kevin. It's certainly common in certain environments that you can write to a higher level than you can read from. Granting those writers access to read the data later would be... difficult. What we're really arguing about here, afaict, is what the default should be. In line with Kevin's comments and Tom's reading of the spec (along with my own experience in these environments), I'd argue for the default to allow writing rows you're not allowed to read. It would certainly be ideal if we could support both options, on a per-relation basis, when we release the overall feature. It doesn't feel like it'd be a lot of work to do that, but I've not been able to follow this discussion up til now. Thankfully, I'm hopeful that I'm going to have more time now to keep up with PG. :) Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Review of Row Level Security
On Thu, Dec 20, 2012 at 4:35 AM, Simon Riggs si...@2ndquadrant.com wrote: Not sure I understand you. You suggested it was a valid use case for a user to have only INSERT privilege and wish to bypass security checks. I agreed and suggested it could be special-cased. That's not really what I intended to suggest. I view checking an inserted tuple and checking the new version of an updated tuple as of a piece. I would think we would check against the RLS quals in either both of those situations or neither, not one without the other. * Applies to all commands should not be implemented via triggers. Complex, slow, unacceptable thing to force upon users. Doing that begs the question of why we would have the feature at all, since we already have triggers and barrier views. I agree that it is questionable whether we need this feature given that we already have security barrier views. I don't agree that performing security checks via triggers is unacceptably slow or complex. Rather, I would say it is flexible and can meet a variety of needs, unlike this feature, which imposes much tighter constraints on what you can and cannot check and in which situations. * the default for row security should be applies to all commands. Anything else may be useful in some cases, but is surprising to users and requires careful thought to determine if it is appropriate. I (and several other people, it seems) do not agree. * How to handle asymmetric row security policies? KaiGai has already begun discussing problems caused by a security policy that differs between reads/writes, on his latest patch post. That needs further analysis to check that it actually makes sense to allow it, since it is more complex. It would be better to fully analyse that situation and post solutions, rather than simply argue its OK. Kevin has made good arguments to show there could be value in such a setup; nobody has talked about banning it, but we do need analysis, suggested syntax/mechanisms and extensive documentation to explain it etc. Frankly, in view of your comments above, I am starting to rethink whether we want this at all. I mean, if you've got security barrier views, you can check the data being read. If you've got triggers, you can check the data being written. So what's left? There's something notationally appealing about being able to apply a security policy to a table rather than creating a separate view and telling people to use the view in lieu of the table, but how much is that notational convenience worth? It has some value from the standpoint of compatibility with other database products ... but probably not a whole lot, since all the syntax we're inventing here is PostgreSQL-specific anyway. Your proposal to check both tuples read and tuples written might add some value ... but unless there's an as-yet-undemonstrated performance benefit, it is again mostly a notational benefit. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
2012/12/20 Stephen Frost sfr...@snowman.net: Kevin, all, * Kevin Grittner (kgri...@mail.com) wrote: The more secure behavior is to allow entry of data which will not be visible by the person doing the entry. wrt this- I'm inclined to agree with Kevin. It's certainly common in certain environments that you can write to a higher level than you can read from. Granting those writers access to read the data later would be... difficult. What we're really arguing about here, afaict, is what the default should be. In line with Kevin's comments and Tom's reading of the spec (along with my own experience in these environments), I'd argue for the default to allow writing rows you're not allowed to read. If system ensures writer's permission is always equivalent or more restrictive than reader's permission, it also eliminates the problem around asymmetric row-security policy between commands. The problematic scenario was that updatable but invisible rows are exposed; using update with returning clause for example. In case when updatable rows are always visible, here is no matter even if user shows it. Probably, we can implement it as ((row-security of select) AND (row-security of update)) that ensures always restrictive row-security policy. Thanks, It would certainly be ideal if we could support both options, on a per-relation basis, when we release the overall feature. It doesn't feel like it'd be a lot of work to do that, but I've not been able to follow this discussion up til now. Thankfully, I'm hopeful that I'm going to have more time now to keep up with PG. :) Thanks, Stephen -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
* Robert Haas (robertmh...@gmail.com) wrote: * Applies to all commands should not be implemented via triggers. Complex, slow, unacceptable thing to force upon users. Doing that begs the question of why we would have the feature at all, since we already have triggers and barrier views. I would rather neither requires writing custom triggers but rather both are supported through this feature. I agree that it is questionable whether we need this feature given that we already have security barrier views. This I don't agree with- the plan has long been to have PG-specific RLS first and then to support SELinux capabilities on top of it. We didn't want to have SELinux-specific functionality that couldn't be achieved without SELinux being involved, and I continue to agree with that. There are many situations, environments, and individuals that would view having to implement RLS through views and triggers as being far-and-away too painful and error-prone to rely on. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Review of Row Level Security
2012/12/20 Robert Haas robertmh...@gmail.com: On Thu, Dec 20, 2012 at 4:35 AM, Simon Riggs si...@2ndquadrant.com wrote: Not sure I understand you. You suggested it was a valid use case for a user to have only INSERT privilege and wish to bypass security checks. I agreed and suggested it could be special-cased. That's not really what I intended to suggest. I view checking an inserted tuple and checking the new version of an updated tuple as of a piece. I would think we would check against the RLS quals in either both of those situations or neither, not one without the other. * Applies to all commands should not be implemented via triggers. Complex, slow, unacceptable thing to force upon users. Doing that begs the question of why we would have the feature at all, since we already have triggers and barrier views. I agree that it is questionable whether we need this feature given that we already have security barrier views. I don't agree that performing security checks via triggers is unacceptably slow or complex. Rather, I would say it is flexible and can meet a variety of needs, unlike this feature, which imposes much tighter constraints on what you can and cannot check and in which situations. I'd like to ask Simon which point is more significant; performance penalty or complex operations by users. If later, FK constraint is a good example that automatically defines triggers that applies its checks on inserted tuple and newer version of updated tuple. Even though we need to consider how to handle dynamically added row-security policy by extension (e.g sepgsql), I don't think we need to enforce users to define triggers for each tables with row-security as long as system support it. * the default for row security should be applies to all commands. Anything else may be useful in some cases, but is surprising to users and requires careful thought to determine if it is appropriate. I (and several other people, it seems) do not agree. * How to handle asymmetric row security policies? KaiGai has already begun discussing problems caused by a security policy that differs between reads/writes, on his latest patch post. That needs further analysis to check that it actually makes sense to allow it, since it is more complex. It would be better to fully analyse that situation and post solutions, rather than simply argue its OK. Kevin has made good arguments to show there could be value in such a setup; nobody has talked about banning it, but we do need analysis, suggested syntax/mechanisms and extensive documentation to explain it etc. Frankly, in view of your comments above, I am starting to rethink whether we want this at all. I mean, if you've got security barrier views, you can check the data being read. If you've got triggers, you can check the data being written. So what's left? In some cases, it is not a reasonable choice to re-define kind of database objects or its name from what existing application assumes. It is a reason why we need adaptive security features on regular tables without or minimum application changes Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
Kohei KaiGai wrote: If system ensures writer's permission is always equivalent or more restrictive than reader's permission, it also eliminates the problem around asymmetric row-security policy between commands. I'm not sure we're understanding each other; so far people who favor asymmetric read and write predicates for row level security have been arguing that people should be able to write tuples that they can't read back. Of course you need to be able to read a tuple to update it, but the argument is that you should be able to configure security so that a role can (for example) update a row to set a sealed flag, and then no longer have rights to read that row (including for purposes of update). You can give away data which is yours, but you can't then take it back if it's not. The problematic scenario was that updatable but invisible rows are exposed; I have not seen anyone argue that such behavior should be allowed. Probably, we can implement it as ((row-security of select) AND (row-security of update)) that ensures always restrictive row-security policy. I hadn't been paying a lot of attention to this patch until I saw the question about whether a user with a particular role could write a row which would then not be visible to that role. I just took a look at the patch. I don't think I like ALTER TABLE as a syntax for row level security. How about using existing GRANT syntax but allowing a WHERE clause? That seems more natural to me, and it would make it easy to apply the same conditions to multiple types of operations when desired, but use different expressions when desired. Without having spent a lot of time pondering it, I think that if row level SELECT permissions exist, they would need to be met on the OLD tuple to allow DELETE or UPDATE, and UPDATE row level permissions would be applied to the NEW tuple. So, Simon's use-case could be met with: GRANT SELECT, INSERT, UPDATE, DELETE ON tabx TO org12user WHERE org = 12; ... and similar GRANTs. A user who should be able to access rows for a particular value of org would be granted the appropriate permission. These could be combined by granting a role to another role. To go back to a Wisconsin Courts example, staff in a county might be granted rights to access data for that county, but district roles could be set up and granted to court officials, who need to be able to access data for all counties in their judicial district, because judges fill in for each other across county lines, but only within their own district. My use-case could be met with: GRANT SELECT, INSERT, UPDATE, DELETE ON addr TO general_staff WHERE NOT sealed; GRANT SELECT, INSERT, UPDATE, DELETE ON addr TO sealed_addr_authority WHERE SEALED; GRANT general_staff TO sealed_addr_authority; Note that I think that if one has multiple roles with row level permissions on a table, access should be allowed if any of those roles allows it. I think that the above should be logically equivalent to (although perhaps slightly less efficient at run-time): GRANT SELECT, INSERT, UPDATE, DELETE ON addr TO general_staff WHERE NOT sealed; GRANT SELECT, INSERT, UPDATE, DELETE ON addr TO sealed_addr_authority; And just to round it out, that these could be applied to users with: GRANT general_staff TO bob; GRANT sealed_addr_authority TO supervisor; GRANT supervisor TO jean; I realize this is a major syntactic departure from the current patch, but it just seems a lot more natural and flexible. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 20 December 2012 21:50, Kevin Grittner kgri...@mail.com wrote: How about using existing GRANT syntax but allowing a WHERE clause? It's a nice feature, but a completely different thing to what is being discussed here. Row security adds the ability to enforce a single coherent policy at table level. It might be nice to have multiple, potentially overlapping policies, but that would require significantly different design and coding to what we have here. For me, enforcing a single policy at table level helps to make it secure by being coherent and understandable. So perhaps in later releases we might do the feature you suggest. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
2012/12/20 Kevin Grittner kgri...@mail.com: Kohei KaiGai wrote: If system ensures writer's permission is always equivalent or more restrictive than reader's permission, it also eliminates the problem around asymmetric row-security policy between commands. I'm not sure we're understanding each other; so far people who favor asymmetric read and write predicates for row level security have been arguing that people should be able to write tuples that they can't read back. Of course you need to be able to read a tuple to update it, but the argument is that you should be able to configure security so that a role can (for example) update a row to set a sealed flag, and then no longer have rights to read that row (including for purposes of update). You can give away data which is yours, but you can't then take it back if it's not. Hmm, for this example, the right behavior is to check rows with ((row-security of select) AND (row-security of update)) on scan- stage, but only (row-security of update) is checked on just-before row updates. In case of (row-security of select) is sealed = false, it is not visible thus not target row of the update, but user can turn on the flag to make it gone. Anyway, the row-security to be applied on scanning-stage of source of update/delete needs to be equivalent or more restrictive than rules on select. However, it might not be needed to ensure rules being restrictive on writer-stage. Probably, we can implement it as ((row-security of select) AND (row-security of update)) that ensures always restrictive row-security policy. I hadn't been paying a lot of attention to this patch until I saw the question about whether a user with a particular role could write a row which would then not be visible to that role. I just took a look at the patch. I don't think I like ALTER TABLE as a syntax for row level security. How about using existing GRANT syntax but allowing a WHERE clause? That seems more natural to me, and it would make it easy to apply the same conditions to multiple types of operations when desired, but use different expressions when desired. It seems to me this syntax gives an impression that row-security feature is tightly combined with role-mechanism, even though it does not need. For example, we can set row-security policy of primary key is even/odd number, independent from working role. Without having spent a lot of time pondering it, I think that if row level SELECT permissions exist, they would need to be met on the OLD tuple to allow DELETE or UPDATE, and UPDATE row level permissions would be applied to the NEW tuple. Yes, I agree. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On Tue, Dec 18, 2012 at 3:39 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: postgres= INSERT INTO t1 VALUES (4,'ddd'); INSERT 0 1 postgres= INSERT INTO t1 VALUES (5,'eee'); ERROR: new row for relation t1 violates row-secirity DETAIL: Failing row contains (5, eee). I've argued against this before - and maybe I should drop my objection, because a number of people seem to be on the other side. But I still think there will be some people who don't want this behavior. Right now, for example, you can give someone INSERT but not SELECT permission on a table, and they will then be able to put rows into the table that they cannot read back. Similarly, in the RLS case, it is not necessarily undesirable for a user to be able to insert a row that they can't read back; or for them to be able to update a row from a value that they can see to one that they cannot. Some people will want to prohibit that, while others will not. Previously, I suggested that we handle this by enforcing row-level security only on data read from the table - the OLD row, so to speak - and not on data written to the table - the NEW row, so to speak - because the latter case can be handled well enough by triggers. (The OLD case cannot, because not seeing the row is different from erroring out when you do see it.) There are other alternatives, like allowing the user to specify which behavior they want. But I think that simply decreeing that the policy will apply not only to rows read but also rows written in all cases will be less flexible than we will ultimately want to be. YMMV, of course. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 19 December 2012 17:25, Robert Haas robertmh...@gmail.com wrote: On Tue, Dec 18, 2012 at 3:39 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: postgres= INSERT INTO t1 VALUES (4,'ddd'); INSERT 0 1 postgres= INSERT INTO t1 VALUES (5,'eee'); ERROR: new row for relation t1 violates row-secirity DETAIL: Failing row contains (5, eee). I've argued against this before - and maybe I should drop my objection, because a number of people seem to be on the other side. But I still think there will be some people who don't want this behavior. Right now, for example, you can give someone INSERT but not SELECT permission on a table, and they will then be able to put rows into the table that they cannot read back. Similarly, in the RLS case, it is not necessarily undesirable for a user to be able to insert a row that they can't read back; or for them to be able to update a row from a value that they can see to one that they cannot. Some people will want to prohibit that, while others will not. I can see a use case for not having security apply for users who have *only* INSERT privilege. This would allow people to run bulk loads of data into a table with row security. We should add that. That is not the common case, so with proper documentation that should be a useful feature without relaxing default security. Never applying security for INSERT and then forcing them to add BEFORE triggers if they want full security is neither secure nor performant. Previously, I suggested that we handle this by enforcing row-level security only on data read from the table - the OLD row, so to speak - and not on data written to the table - the NEW row, so to speak - because the latter case can be handled well enough by triggers. (The OLD case cannot, because not seeing the row is different from erroring out when you do see it.) There are other alternatives, like allowing the user to specify which behavior they want. But I think that simply decreeing that the policy will apply not only to rows read but also rows written in all cases will be less flexible than we will ultimately want to be. As discussed, we should add a security feature that is secure by default. Adding options to make it less secure can follow initial commit. We might even make it in this release if the review of the main feature goes well. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
Robert Haas robertmh...@gmail.com writes: On Tue, Dec 18, 2012 at 3:39 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: postgres= INSERT INTO t1 VALUES (4,'ddd'); INSERT 0 1 postgres= INSERT INTO t1 VALUES (5,'eee'); ERROR: new row for relation t1 violates row-secirity DETAIL: Failing row contains (5, eee). I've argued against this before - and maybe I should drop my objection, because a number of people seem to be on the other side. But I still think there will be some people who don't want this behavior. Right now, for example, you can give someone INSERT but not SELECT permission on a table, and they will then be able to put rows into the table that they cannot read back. There is also precedent for your opinion in the spec-mandated behavior of updatable views: it is perfectly possible to INSERT a row that you can't read back via the view, or UPDATE it to a state you can't see via the view. The RLS patch's current behavior corresponds to a view created WITH CHECK OPTION --- which we don't support yet. Whether we add that feature soon or not, what seems important for the current debate is that the SQL spec authors chose not to make it the default behavior. This seems to weigh heavily against making it the default, much less only, behavior for RLS cases. I'd also suggest that throw an error is not the only response that people are likely to want for attempts to insert/update non-compliant rows, so hard-wiring that choice is insufficiently flexible even if you grant that local policy is to not allow such updates. (As an example, they might prefer to log the attempt and substitute some other value.) Previously, I suggested that we handle this by enforcing row-level security only on data read from the table - the OLD row, so to speak - and not on data written to the table - the NEW row, so to speak - because the latter case can be handled well enough by triggers. +1. I'm less than excited about RLS in the first place, so the less complexity we have to put into the core system for it the better IMO. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 19 December 2012 18:40, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Dec 18, 2012 at 3:39 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: postgres= INSERT INTO t1 VALUES (4,'ddd'); INSERT 0 1 postgres= INSERT INTO t1 VALUES (5,'eee'); ERROR: new row for relation t1 violates row-secirity DETAIL: Failing row contains (5, eee). I've argued against this before - and maybe I should drop my objection, because a number of people seem to be on the other side. But I still think there will be some people who don't want this behavior. Right now, for example, you can give someone INSERT but not SELECT permission on a table, and they will then be able to put rows into the table that they cannot read back. There is also precedent for your opinion in the spec-mandated behavior of updatable views: it is perfectly possible to INSERT a row that you can't read back via the view, or UPDATE it to a state you can't see via the view. The RLS patch's current behavior corresponds to a view created WITH CHECK OPTION --- which we don't support yet. Whether we add that feature soon or not, what seems important for the current debate is that the SQL spec authors chose not to make it the default behavior. This seems to weigh heavily against making it the default, much less only, behavior for RLS cases. This is security, not spec compliance. By default, we need full security. Nobody has argued that it should be the only behaviour, only that it is the most typically requested behaviour and the most secure, therefore the one we should do first. I'd also suggest that throw an error is not the only response that people are likely to want for attempts to insert/update non-compliant rows, so hard-wiring that choice is insufficiently flexible even if you grant that local policy is to not allow such updates. (As an example, they might prefer to log the attempt and substitute some other value.) Previously, I suggested that we handle this by enforcing row-level security only on data read from the table - the OLD row, so to speak - and not on data written to the table - the NEW row, so to speak - because the latter case can be handled well enough by triggers. +1. I'm less than excited about RLS in the first place, so the less complexity we have to put into the core system for it the better IMO. Agree with the need for less complexity, but that decision increases complexity for the typical user and does very little to the complexity of the patch. Treating a security rule as a check constraint is natural and obvious, so there are no core system problems here. If we don't enforce rules on INSERT the user has to specifically add a trigger, which makes things noticeably slower. There is more maintenance work for the average user, less performance and more mistakes to make. The way to do this is by adding an option to allow users to specify INSERT should be exempt from the security rule, which Kaigai and I agreed on list some weeks back should come after the initial patch, to no other comment. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
Simon Riggs wrote: This is security, not spec compliance. By default, we need full security. But you are arguing that users should not be able to make something secure if they, and everyone with the same permissions, could not later access it. I thought about situations where I've seen a need for something like this, and probably the best fit that I've seen is the ability of a judge to order that something is sealed. There are various levels where that can happen, but I'll focus on just one which Wisconsin Courts have implemented at the application level, but which would be nice to be able to support at the database level. Let's say we're talking about Milwaukee County, where hundreds of people work for the courts and related organizations with some rights to view the court data. Let's say a battered wife has moved to a new address she wants to keep secret for safety. She files a case with the court for a temporary restraining order, prohibiting the husband from coming near her. The court needs her address for the official record, but the judge will order the address sealed so that only people with a certain authority can see it. The authority is very limited, for obvious reasons. It is quite likely that the person initially entering the address and flagging it as sealed will not have authority to see the address if they go back and look up the case. Neither will the dozens of other people making the same kind of entries in the county. Obviously, if the person doing the initial entry is a friend of the husband, the data is compromised; but not allowing entry of the data in a state sealed by people without authority to look it up exposes the data to every other person with entry authority, with fairly obvious risks. The more secure behavior is to allow entry of data which will not be visible by the person doing the entry. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 19 December 2012 19:46, Kevin Grittner kgri...@mail.com wrote: But you are arguing that users should not be able to make something secure if they, and everyone with the same permissions, could not later access it. Not exactly, no. I've argued that row security should apply to ALL commands by default. Which is exactly the same default as Oracle, as well as being the obvious common sense understanding of row security, which does not cause a POLA violation. I have no objection to an option to allow row security to not apply to inserts, if people want that. I do object to the idea that row security for inserts/updates should only happen via triggers, which is an ugly and non-performant route, as well as complicating security. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 2012-12-19 14:46:18 -0500, Kevin Grittner wrote: Simon Riggs wrote: This is security, not spec compliance. By default, we need full security. But you are arguing that users should not be able to make something secure if they, and everyone with the same permissions, could not later access it. I thought about situations where I've seen a need for something like this, and probably the best fit that I've seen is the ability of a judge to order that something is sealed. There are various levels where that can happen, but I'll focus on just one which Wisconsin Courts have implemented at the application level, but which would be nice to be able to support at the database level. Let's say we're talking about Milwaukee County, where hundreds of people work for the courts and related organizations with some rights to view the court data. Let's say a battered wife has moved to a new address she wants to keep secret for safety. She files a case with the court for a temporary restraining order, prohibiting the husband from coming near her. The court needs her address for the official record, but the judge will order the address sealed so that only people with a certain authority can see it. The authority is very limited, for obvious reasons. It is quite likely that the person initially entering the address and flagging it as sealed will not have authority to see the address if they go back and look up the case. Neither will the dozens of other people making the same kind of entries in the county. Obviously, if the person doing the initial entry is a friend of the husband, the data is compromised; but not allowing entry of the data in a state sealed by people without authority to look it up exposes the data to every other person with entry authority, with fairly obvious risks. The more secure behavior is to allow entry of data which will not be visible by the person doing the entry. I don't think it is that simple. Allowing inserts without regard for row level restrictions makes it far easier to probe for data. E.g. by inserting rows and checking for unique violations. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
Simon Riggs wrote: I've argued that row security should apply to ALL commands by default. Which is exactly the same default as Oracle, as well as being the obvious common sense understanding of row security, which does not cause a POLA violation. I have no objection to an option to allow row security to not apply to inserts, if people want that. I do object to the idea that row security for inserts/updates should only happen via triggers, which is an ugly and non-performant route, as well as complicating security. In the software where I've seen this managed at an application level, an address (for example) can be sealed by an update to the sealed column or inserted with the sealed column set to true. I'm not sure why you would want to allow one and not the other. Now, I can envision a situation where it could make sense to use the same predicate for limiting what a role could read and what a role could write, but I'm not buying that that is more secure. In fact, I see cases where you cannot achieve decent security without the ability for both INSERT and UPDATE to write rows which security excludes on reads. Functionally, I don't see anything which can't be accomplished with just the read security and triggers. I do agree that it would be nice if there were an easy way to specify that the same predicate applies to both reads and writes. I hope we can leave the syntax for this feature open to such specification, even if the initial implementation only supports limiting reads. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
Andres Freund wrote: I don't think it is that simple. Allowing inserts without regard for row level restrictions makes it far easier to probe for data. E.g. by inserting rows and checking for unique violations. Unless you want to go to a military-style security level system where people at each security level have a separate version of the same data, primary keys (and I think other unique constraints) can leak. It seems clear enough that sensitive data should not be used for such constraints. That doesn't even require completely meaningless numeric keys. Court cases in Wisconsin have been numbered within county by year, case type, a sequential portion, and an optional apha suffix since before things were computerized -- you may know that there is a 2010 case in Dane County for mental commitment number 45, but that doesn't leak any sensitive data. In the sealed address example, if that were moved to the database layer, someone might be able to test whether addess number 5 existed on a case by seeing whether an insert succeeded; but if it did, there would be a record of that insert with their user ID that they could not retract in any way -- they would know very little, and be exposed as having done something inappropriate. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 19 December 2012 20:23, Kevin Grittner kgri...@mail.com wrote: I hope we can leave the syntax for this feature open to such specification, even if the initial implementation only supports limiting reads. Well, I hope the opposite: that we can support simple full security by default, while leaving syntax open. The basic model for this is complete separation of data between customers/people. They can't see my data, I can't see theirs. Simple privacy. Obvious. Sure, more complex applications exist, but forcing the simple/common usage to adopt triggers because of that is not a sensible way forwards. Simple basic functionality, with an option for more advanced cases is what we need. Setting a status flag so that the current user no longer sees the row is a good example of more complex workflows in secure applications, I agree, but its not the common case by any means. When we have these discussions about priority, it seems people think this means don't do it ever. It doesn't, it means do the most important things first and then do other stuff later. I always wish to do both, but circumstances teach me that hard cutoffs and deadlines mean we can't always have everything if debates overrun and decisions aren't forthcoming. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
The more secure behavior is to allow entry of data which will not be visible by the person doing the entry. I don't think it is that simple. Allowing inserts without regard for row level restrictions makes it far easier to probe for data. E.g. by inserting rows and checking for unique violations. So the PK column(s) are not as secure as, say, the address-related column. Vice-versa I may know that someone lives at a given address (because my attempt to place someone else there failed) but I would have no way of knowing who that other person is. My recourse would be to escalate the data-entry request to someone with higher security permissions who could read and write to the appropriate tables and resolve the conflict. In both cases the direct write-only situation necessitates that some level of exposure occurs. The work-around if that is unacceptable would be to accept all data but any entries that cannot be directly inserted into the table would remain in a staging area that someone with higher security would have to monitor and clear as needed. The same intervention is required but in the first situation you can at least avoid coding the special logic and instead trade security for ease-of-use. As a default level of security we could throw a generic secure DLL rejected for ROW(...) and not tell the user anything about the cause. If that person knows all unique indexes and constraints defined on the table they could use trial-and-error to discover information about stored records but even then if they get an error on two different columns they still have no way of knowing if those errors belong to the same record. Beyond that level you provide the user with some information as to the cause so that they have a reasonable chance to catch typos and other mistakes instead of escalating an benign issue. Lastly is the custom solution whereby the developers accept ALL data entered as being correct but saved to a staging table. A review process by someone with higher security clearances would then process and clear out that table as necessary. If the user is write-only then regardless of whether the entry succeeded or failed they are considered to be done with their task at that point and no meaningful results from the system can be supplied to them. None of these options disallows the presence of non-security related check constraints to be checked, enforced, and communicated to the user. I've probably lost sight of the bigger picture as my response to mostly informed by these last couple of messages. David J. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 19 December 2012 20:37, Kevin Grittner kgri...@mail.com wrote: Andres Freund wrote: I don't think it is that simple. Allowing inserts without regard for row level restrictions makes it far easier to probe for data. E.g. by inserting rows and checking for unique violations. Unless you want to go to a military-style security level system where people at each security level have a separate version of the same data, primary keys (and I think other unique constraints) can leak. It seems clear enough that sensitive data should not be used for such constraints. But there is the more obvious case where you shouldn't be able to insert medical history for a patient you have no responsibility for. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
Simon Riggs wrote: Kevin Grittner kgri...@mail.com wrote: I hope we can leave the syntax for this feature open to such specification, even if the initial implementation only supports limiting reads. Well, I hope the opposite: that we can support simple full security by default, while leaving syntax open. The basic model for this is complete separation of data between customers/people. They can't see my data, I can't see theirs. Simple privacy. Obvious. And something we already can handle several different ways. Inheritance, schemas, etc. Allowing data to be fully secured from prying eyes on entry, regardless of whether the role allowing the entry has rights to see the data does not yet have any built-in support. Sure, more complex applications exist, but forcing the simple/common usage to adopt triggers because of that is not a sensible way forwards. Simple basic functionality, with an option for more advanced cases is what we need. Setting a status flag so that the current user no longer sees the row is a good example of more complex workflows in secure applications, I agree, but its not the common case by any means. When we have these discussions about priority, it seems people think this means don't do it ever. It doesn't, it means do the most important things first and then do other stuff later. I always wish to do both, but circumstances teach me that hard cutoffs and deadlines mean we can't always have everything if debates overrun and decisions aren't forthcoming. Well, it seems we have different views of what is intuitively obvious here. What you suggest does not look to me to be more secure (making full security a misnomer), simpler, nor more important. Perhaps we can avoid divisive discussions on which is more important if we can manage both in the initial implementation. I guess the usual rule if we can't manage it is that a tie goes to the author, which is neither you nor I. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 2012-12-19 18:25, Robert Haas wrote: On Tue, Dec 18, 2012 at 3:39 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: postgres= INSERT INTO t1 VALUES (4,'ddd'); INSERT 0 1 postgres= INSERT INTO t1 VALUES (5,'eee'); ERROR: new row for relation t1 violates row-secirity DETAIL: Failing row contains (5, eee). I've argued against this before - and maybe I should drop my objection, because a number of people seem to be on the other side. But I still think there will be some people who don't want this behavior. Right now, for example, you can give someone INSERT but not SELECT permission on a table, and they will then be able to put rows into the table that they cannot read back. Similarly, in the RLS case, it is not necessarily undesirable for a user to be able to insert a row that they can't read back; or for them to be able to update a row from a value that they can see to one that they cannot. Some people will want to prohibit that, while others will not. Maybe it is an idea to provide different RLS expressions for read and write. I remember reading a scenario (it might be well known in security land) where it is possible to write to authorization levels = users level, and read levels = the users level. In this setup Kevin's address example is possible, a user could write to e.g. the highest level, but then not read it anymore if his own level was lower than the highest. This setup also shows that to implement it, one would need a different expression for read and write (or the rls expression should know the query's commandtype). regards, Yeb -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On Wed, Dec 19, 2012 at 12:54 PM, Simon Riggs si...@2ndquadrant.com wrote: I can see a use case for not having security apply for users who have *only* INSERT privilege. This would allow people to run bulk loads of data into a table with row security. We should add that. That is not the common case, so with proper documentation that should be a useful feature without relaxing default security. Never applying security for INSERT and then forcing them to add BEFORE triggers if they want full security is neither secure nor performant. I think INSERT vs. not-INSERT is not the relevant distinction, because the question also arises for UPDATE. In the UPDATE case, the question is whether the RLS qual should be checked only against the OLD tuple (to make sure that we can see the tuple to modify it) or also against the NEW tuple (to make sure that we're not modifying it to a form that we can no longer see). In other words, the question is not do we support all of the commands? but rather do we check not only the tuple read but also the tuple written?. For INSERT, we only write a tuple, without reading. For SELECT and DELETE, we only read a tuple, without writing a new one. UPDATE does both a read and a write. Previously, I suggested that we handle this by enforcing row-level security only on data read from the table - the OLD row, so to speak - and not on data written to the table - the NEW row, so to speak - because the latter case can be handled well enough by triggers. (The OLD case cannot, because not seeing the row is different from erroring out when you do see it.) There are other alternatives, like allowing the user to specify which behavior they want. But I think that simply decreeing that the policy will apply not only to rows read but also rows written in all cases will be less flexible than we will ultimately want to be. As discussed, we should add a security feature that is secure by default. Adding options to make it less secure can follow initial commit. We might even make it in this release if the review of the main feature goes well. Saying that something is or is not secure is not meaningful without defining what you want to be secure against. There's nothing insecure about checking only the tuples read; it's just a different (and useful) threat model. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On Wed, Dec 19, 2012 at 1:58 PM, Simon Riggs si...@2ndquadrant.com wrote: If we don't enforce rules on INSERT the user has to specifically add a trigger, which makes things noticeably slower. There is more maintenance work for the average user, less performance and more mistakes to make. Well, again, only if that's the behavior they want. Also, it's also worth noting that, even if we assume that it is in fact the behavior that users will want, the contention that it is faster than a trigger is thus far unsubstantiated by any actual benchmarks. It may indeed be faster ... but I don't know without testing whether it's slightly faster or a whole lot faster. That might be a good thing to find out, because if it is a whole lot faster, that would certainly strengthen the case for including a mode that works that way, whether or not we also provide other options. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 9 December 2012 06:21, Kohei KaiGai kai...@kaigai.gr.jp wrote: 2012/12/7 Simon Riggs si...@2ndquadrant.com: On 5 December 2012 11:16, Kohei KaiGai kai...@kaigai.gr.jp wrote: Oracle defaults to putting VPD on all event types: INSERT, UPDATE, DELETE, SELECT. ISTM we should be doing the same, not just say we can add an INSERT trigger if you want. Adding a trigger just begs the question as to why we are bothering in the first place, since this functionality could already be added by INSERT, UPDATE or DELETE triggers, if they are a full replacement for this feature. The only answer is ease of use We can easily add syntax like this [ROW SECURITY CHECK ( ) [ON [ ALL | INSERT, UPDATE, DELETE, SELECT [.., with the default being ALL I think it is flaw of Oracle. :-) Agreed In case when user can define leakable function, it enables to leak contents of invisible rows at the timing when executor fetch the rows, prior to modification stage, even if we allows to configure individual row-security policies for SELECT and DELETE or UPDATE commands. My preference is one policy on a particular table for all the commands. Yes, only one security policy allowed. Question is, should we offer the option to enforce it on a subset of command types. That isn't anything I can see a need for myself. It is not hard to support a feature not to apply security policy on particular command types, from implementation perspective. So, my preference is to support only the behavior corresponding to above ALL option, then support per commands basis when we got strong demands. How about your thought? Very much agree that ALL should be the default, and only option for first commit of this feature. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 9 December 2012 06:08, Kohei KaiGai kai...@kaigai.gr.jp wrote: 2012/12/7 Simon Riggs si...@2ndquadrant.com: On 5 December 2012 11:16, Kohei KaiGai kai...@kaigai.gr.jp wrote: * TRUNCATE works, and allows you to remove all rows of a table, even ones you can't see to run a DELETE on. Er... It was my oversight. My preference is to rewrite TRUNCATE command with DELETE statement in case when row-security policy is active on the target table. In this case, a NOTICE message may be helpful for users not to assume the table is always empty after the command. I think the default must be to throw an ERROR, since part of the contract with TRUNCATE is that it is fast and removes storage. OK. Does the default imply you are suggesting configurable behavior using GUC or something? I think both of the behaviors are reasonable from security point of view, as long as user cannot remove unprivileged rows. Hmm, its difficult one that. I guess this raises the question as to whether users know they are accessing a table with RLS enabled. If they don't and we want to keep it that way, then changing TRUNCATE into DELETE makes sense. To issue TRUNCATE you need the correct privilege, which is separate from DELETE. If they have TRUNCATE privilege they should be allowed to remove all rows, bypassing the row level security. If that behavious isn't wanted, then the table owner can create an INSTEAD OF TRUNCATE trigger that turns the action into a DELETE, which is then subject to RLS rules. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
2012/12/9 Simon Riggs si...@2ndquadrant.com: On 9 December 2012 06:08, Kohei KaiGai kai...@kaigai.gr.jp wrote: 2012/12/7 Simon Riggs si...@2ndquadrant.com: On 5 December 2012 11:16, Kohei KaiGai kai...@kaigai.gr.jp wrote: * TRUNCATE works, and allows you to remove all rows of a table, even ones you can't see to run a DELETE on. Er... It was my oversight. My preference is to rewrite TRUNCATE command with DELETE statement in case when row-security policy is active on the target table. In this case, a NOTICE message may be helpful for users not to assume the table is always empty after the command. I think the default must be to throw an ERROR, since part of the contract with TRUNCATE is that it is fast and removes storage. OK. Does the default imply you are suggesting configurable behavior using GUC or something? I think both of the behaviors are reasonable from security point of view, as long as user cannot remove unprivileged rows. Hmm, its difficult one that. I guess this raises the question as to whether users know they are accessing a table with RLS enabled. If they don't and we want to keep it that way, then changing TRUNCATE into DELETE makes sense. To issue TRUNCATE you need the correct privilege, which is separate from DELETE. If they have TRUNCATE privilege they should be allowed to remove all rows, bypassing the row level security. If that behavious isn't wanted, then the table owner can create an INSTEAD OF TRUNCATE trigger that turns the action into a DELETE, which is then subject to RLS rules. It seems to me make sense, also. Even though selinux does not define separated permissions for TRUNCATE, the later option will work well for me in case of row-level label based security is configured in the future version. So, I don't implement something special around TRUNCATE, except for paying mention at the documentation. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
2012/12/7 Simon Riggs si...@2ndquadrant.com: On 5 December 2012 11:16, Kohei KaiGai kai...@kaigai.gr.jp wrote: * TRUNCATE works, and allows you to remove all rows of a table, even ones you can't see to run a DELETE on. Er... It was my oversight. My preference is to rewrite TRUNCATE command with DELETE statement in case when row-security policy is active on the target table. In this case, a NOTICE message may be helpful for users not to assume the table is always empty after the command. I think the default must be to throw an ERROR, since part of the contract with TRUNCATE is that it is fast and removes storage. OK. Does the default imply you are suggesting configurable behavior using GUC or something? I think both of the behaviors are reasonable from security point of view, as long as user cannot remove unprivileged rows. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
2012/12/7 Simon Riggs si...@2ndquadrant.com: On 5 December 2012 11:16, Kohei KaiGai kai...@kaigai.gr.jp wrote: Oracle defaults to putting VPD on all event types: INSERT, UPDATE, DELETE, SELECT. ISTM we should be doing the same, not just say we can add an INSERT trigger if you want. Adding a trigger just begs the question as to why we are bothering in the first place, since this functionality could already be added by INSERT, UPDATE or DELETE triggers, if they are a full replacement for this feature. The only answer is ease of use We can easily add syntax like this [ROW SECURITY CHECK ( ) [ON [ ALL | INSERT, UPDATE, DELETE, SELECT [.., with the default being ALL I think it is flaw of Oracle. :-) Agreed In case when user can define leakable function, it enables to leak contents of invisible rows at the timing when executor fetch the rows, prior to modification stage, even if we allows to configure individual row-security policies for SELECT and DELETE or UPDATE commands. My preference is one policy on a particular table for all the commands. Yes, only one security policy allowed. Question is, should we offer the option to enforce it on a subset of command types. That isn't anything I can see a need for myself. It is not hard to support a feature not to apply security policy on particular command types, from implementation perspective. So, my preference is to support only the behavior corresponding to above ALL option, then support per commands basis when we got strong demands. How about your thought? Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 5 December 2012 11:16, Kohei KaiGai kai...@kaigai.gr.jp wrote: * TRUNCATE works, and allows you to remove all rows of a table, even ones you can't see to run a DELETE on. Er... It was my oversight. My preference is to rewrite TRUNCATE command with DELETE statement in case when row-security policy is active on the target table. In this case, a NOTICE message may be helpful for users not to assume the table is always empty after the command. I think the default must be to throw an ERROR, since part of the contract with TRUNCATE is that it is fast and removes storage. * Docs need work, but thats OK. I'd like to want some help with native English speakers. I'll help with that. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
On 5 December 2012 11:16, Kohei KaiGai kai...@kaigai.gr.jp wrote: Oracle defaults to putting VPD on all event types: INSERT, UPDATE, DELETE, SELECT. ISTM we should be doing the same, not just say we can add an INSERT trigger if you want. Adding a trigger just begs the question as to why we are bothering in the first place, since this functionality could already be added by INSERT, UPDATE or DELETE triggers, if they are a full replacement for this feature. The only answer is ease of use We can easily add syntax like this [ROW SECURITY CHECK ( ) [ON [ ALL | INSERT, UPDATE, DELETE, SELECT [.., with the default being ALL I think it is flaw of Oracle. :-) Agreed In case when user can define leakable function, it enables to leak contents of invisible rows at the timing when executor fetch the rows, prior to modification stage, even if we allows to configure individual row-security policies for SELECT and DELETE or UPDATE commands. My preference is one policy on a particular table for all the commands. Yes, only one security policy allowed. Question is, should we offer the option to enforce it on a subset of command types. That isn't anything I can see a need for myself. * psql \d support needed Are you suggesting to print out full qualifiers of row-security? Or, a mark to indicate whether row-security is configured, or not? One of those options, yes -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Row Level Security
Thanks for your reviewing in spite of large number of lines. My comments are below. 2012/12/4 Simon Riggs si...@2ndquadrant.com: Patch looks good and also like it will/can be ready for 9.3. I'm happy to put time into this as committer and/or reviewer and take further responsibility for it, unless others wish to. LIKES * It's pretty simple to understand and use * Check qual is stored in pre-parsed form. That is fast, and also secure, since changing search_path of the user doesn't change the security check at all. Nice. * Performance overhead is low, integration with indexing is clear and effective and it works with partitioning * It works, apart from design holes listed below, easily plugged IMHO DISLIKEs * Who gets to see stats on the underlying table? Are the stats protected by security? How does ANALYZE work? I think, ANALYZE should perform on the raw tables without row-security policy. Even though statistics are gray-zone, it is not a complete set of the raw table contents, so all we can do is just implying the original from processed statistical values. The situation is similar to iteration of probe using PK/FK violation. In general, it is called covert channel, and out of the scope in regular access control mechanism (including MAC). So, I don't think we have special protection on pg_stat even if row- security is configured. * INSERT ignores the SECURITY clause, on the ground that this has no meaning. So its possible to INSERT data you can't see. For example, you can insert medical records for *another* patient, or insert new top secret information. This also causes a security hole... since inserted rows can violate defined constraints, letting you know that other keys you can't see exist. Why don't we treat the SECURITY clause as a CHECK constraint? That makes intuitive sense to me. * UPDATE allows you to bypass the SECURITY clause, to produce new rows that you can't see. (Not good). But you can't get them back again, cos you can't see them. The above two comments seems me that you are suggesting to apply checks on both of scanning rows stage (UPDATE case) and modifying rows stage (INSERT and UPDATE), to prevent touchable rows getting gone anywhere. In the previous discussion, it was suggested we can implement this feature using before-row trigger. However, I love the idea to support same row-security policy integrated with CHECK constraint; that kills individual user's operation to define triggers. One problem is a case when row-security policy contains SubLink node. I expect it takes a special case handling, however, also guess not hard to implement so much. Let me investigate the code around here. * TRUNCATE works, and allows you to remove all rows of a table, even ones you can't see to run a DELETE on. Er... It was my oversight. My preference is to rewrite TRUNCATE command with DELETE statement in case when row-security policy is active on the target table. In this case, a NOTICE message may be helpful for users not to assume the table is always empty after the command. None of those things are cool, at all. Oracle defaults to putting VPD on all event types: INSERT, UPDATE, DELETE, SELECT. ISTM we should be doing the same, not just say we can add an INSERT trigger if you want. Adding a trigger just begs the question as to why we are bothering in the first place, since this functionality could already be added by INSERT, UPDATE or DELETE triggers, if they are a full replacement for this feature. The only answer is ease of use We can easily add syntax like this [ROW SECURITY CHECK ( ) [ON [ ALL | INSERT, UPDATE, DELETE, SELECT [.., with the default being ALL I think it is flaw of Oracle. :-) In case when user can define leakable function, it enables to leak contents of invisible rows at the timing when executor fetch the rows, prior to modification stage, even if we allows to configure individual row-security policies for SELECT and DELETE or UPDATE commands. My preference is one policy on a particular table for all the commands. * The design has nothing at all to do with SECURITY LABELs. Why did we create them, I wonder? I understood we would have row-level label security. Doesn't that require us to have a data type, such as reglabel or similar enum? Seems strange. Oracle has two features: Oracle Label Security and Row Level Security - I think it should be implemented on the next step. It additionally takes two independent features (1) functionality to inject a column to store security label at table definition. (2) functionality to assign a default security label when a new row is inserted. As Oracle constructs OLS on the top of VPD feature, the base row- security feature shall be upstreamed first. OTHER * The docs should explain a little better how to optimize using RLS. Specifically, the fact that indexable operators are marked leakproof and thus can be optimized ahead of the rlsquals. The docs say rls quals
