Re: [HACKERS] [v9.4] row level security
On 01/24/2014 10:12 AM, Craig Ringer wrote: (Re-sending; I forgot to cc the list) On 01/20/2014 02:15 PM, Craig Ringer wrote: On 01/20/2014 09:58 AM, Craig Ringer wrote: As it is I'm spending today reworking the RLS patch on top of the new approach to updatable security barrier views. To get that rolling I've split the RLS patch up into chunks, so we can argue about the catalogs, ALTER syntax, and the actual row-filtering implementation separately ;-) It's currently on g...@github.com:ringerc/postgres.git in the branch rls-9.4-split, which is subject to rebasing. I'm still going through it making sure each chunk at least compiles and preferably passes make check. That branch is now pretty stable, and passes checks at every stage up to the new RLS regression tests. I've pushed a new version to branch rls-9.4-split. Further updates will rebase this branch. The tag rls-9.4-split-v5 identifies this particular push, and won't get rebased away. Pushed a new rebase to the main working branch, merging in the fixes I made to KaiGai's patch last time. Tagged rls-9.4-split-v6 I haven't bothered with a patchset for this one, I'll be replacing it again soon. This is just for anybody following along. -- 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] [v9.4] row level security
On 01/18/2014 03:27 AM, Gregory Smith wrote: With my advocacy hat on, I'd like to revisit this idea now that there's a viable updatable security barrier view submission. I thought the most serious showstopper feedback from the last CF's RLS submission was that this needed to be sorted out first. Reworking KaiGai's submission to merge against Dean's new one makes it viable again in my mind, and I'd like to continue toward re-reviewing it as part of this CF in that light. I had hoped to have this done weeks ago, but was blocked on getting a viable approach to updatable security barrier views in place. I really appreciate Dean, with his greater experience and skill in this area, looking into it. As it is I'm spending today reworking the RLS patch on top of the new approach to updatable security barrier views. Then it'll be a matter of tests, lots and lots of tests of every weird corner I can think of. Admittedly it's not ideal to try and do that at the same time the barrier view patch is being modified, but I see that as a normal CF merge of things based on other people's submissions. I tend to agree - and the whole idea of reworking RLS on top of updatable security barrier views is that it makes the patch for RLS's UI and catalogs largely independent from the mechanics of filtering rows. I mentioned advocacy because the budding new PostgreSQL test instances I'm seeing now will lose a lot of momentum if we end up with no user visible RLS features in 9.4. The pieces we have now can assemble into something that's useful, and I don't think that goal is unreasonably far away. If it's possible, getting _something_ into 9.4 would be great. I'm aware of multiple interested users who originally expected this in 9.3. That hasn't worked out, but it'd be great to make 9.4. -- 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] [v9.4] row level security
On 12/13/13 11:40 PM, Craig Ringer wrote: You may want to check out the updated writable security-barrier views patch. http://www.postgresql.org/message-id/52ab112b.6020...@2ndquadrant.com It may offer a path forward for the CF submission for RLS, letting us get rid of the var/attr fiddling that many here objected to. With my advocacy hat on, I'd like to revisit this idea now that there's a viable updatable security barrier view submission. I thought the most serious showstopper feedback from the last CF's RLS submission was that this needed to be sorted out first. Reworking KaiGai's submission to merge against Dean's new one makes it viable again in my mind, and I'd like to continue toward re-reviewing it as part of this CF in that light. Admittedly it's not ideal to try and do that at the same time the barrier view patch is being modified, but I see that as a normal CF merge of things based on other people's submissions. I mentioned advocacy because the budding new PostgreSQL test instances I'm seeing now will lose a lot of momentum if we end up with no user visible RLS features in 9.4. The pieces we have now can assemble into something that's useful, and I don't think that goal is unreasonably far away. -- Greg Smith greg.sm...@crunchydatasolutions.com Chief PostgreSQL Evangelist - http://crunchydatasolutions.com/ -- 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] [v9.4] row level security
On 2013-12-14 05:40, Craig Ringer wrote: I find the heirachical and non-heirachical label security model used in Teradata to be extremely interesting and worthy of study. The concept is that there are heirachical label policies (think classified, unclassified, etc) or non-heirachical (mutually exclusive labels). A user can see a row if they have all the policies used in a table, and each of the user's policy values is sufficient to see the row. For non-heiracical policies that means the user and row must have at least one label in common; for heiracical policies it means the user label must be greater than or equal to the row label. That model lets you quite flexibly and easily build complex security models. It'd work well with PostgreSQL, too: We could implement something like non-heirachical policies using a system varbit column that's added whenever a non-heirachical policy gets added to a table, and simply AND it with the varbit on the user's policy to see if they can access the row. Or use a compact fixed-width bitfield. Heirachical policies would use a 1 or 2 byte int system column to store the label. Is is necessary to fix the attribute type of the security label? The label model described above seems to restrictive to support e.g. the labels described on p18 of the 'healthcare classification system' (HCS) (http://www.hl7.org/documentcenter/public/wg/secure/3.%20HCS%20Guide%20Final%202013%200322%20JMD.pdf). The HCS security label is based on the NIST's FIPS188 standard / http://www.itl.nist.gov/fipspubs/fip188.htm regards, Yeb Havinga -- 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] [v9.4] row level security
On 12/14/2013 11:24 AM, Gregory Smith wrote: The RLS feature set available with the CF submission is good enough for those projects to continue exploring PostgreSQL You may want to check out the updated writable security-barrier views patch. http://www.postgresql.org/message-id/52ab112b.6020...@2ndquadrant.com It may offer a path forward for the CF submission for RLS, letting us get rid of the var/attr fiddling that many here objected to. There's some interesting bad news, like how we're going to have to completely refactor all the if (!superuser()) shortcuts into real permissions one day to make them happy. We'll need that for mandatory access control. Initially I'd probably want to do it through SEPostgreSQL role checks, but it'd be desirable to have a generic mechanism that'd work with any OS's role management eventually. I think we're going to need finer grained rights than we have now, as superuser or not isn't really good enough for some things. In particular, the fact that if plperl or plpython are installed the superuser (often accessible remotely) can trivially run arbitrary C-level functions and invoke commands as the PostgreSQL OS user isn't going to be acceptable in these environments, but they may still want the trusted PLs. Attached is a small demo piece built by my new co-worker Jeff McCormick that I've been hacking lightly on. The idea was to get something a step beyond trivial that maps into real-world need, well enough that we could get people to agree it was usefully representative. I really appreciate that. I suspect more work will be needed on the interface for the actual row-level security code, and work like this helps point in the direction of what's needed. One reason the users I've been talking to feel comfortable that they'll be able to build apps usefully with this feature is this interface. It feels more like a pluggable API for defining rules than a specific implementation. Congratulations to the usual community members who expect complete feature overkill in every design. There's a C-level API that's completely pluggable, for what it's worth. I'm actually unconvinced that the SQL-level API for row-security is appropriate - I'm persuaded by comments made on the list that the RLS syntax and security model needs to be easy enough to use to be useful if it's to add much on top of what we get with updatable s.b. views, security barrier, leakproof, etc. I don't think run some SQL for every row fits that. We have that at the C level in the RLS patch, and I wonder if it should stay there. I find the heirachical and non-heirachical label security model used in Teradata to be extremely interesting and worthy of study. The concept is that there are heirachical label policies (think classified, unclassified, etc) or non-heirachical (mutually exclusive labels). A user can see a row if they have all the policies used in a table, and each of the user's policy values is sufficient to see the row. For non-heiracical policies that means the user and row must have at least one label in common; for heiracical policies it means the user label must be greater than or equal to the row label. That model lets you quite flexibly and easily build complex security models. It'd work well with PostgreSQL, too: We could implement something like non-heirachical policies using a system varbit column that's added whenever a non-heirachical policy gets added to a table, and simply AND it with the varbit on the user's policy to see if they can access the row. Or use a compact fixed-width bitfield. Heirachical policies would use a 1 or 2 byte int system column to store the label. So we'd have one system column per policy that's been applied to each table. For users we'd need a catalog table to describe policies, and a join table mapping users to policies with a user policy value. Optimization by storing policy values for users directly in pg_role might be worthwhile. At the SQL level, an admin would then: - Define policies - Assign users labels in policies - Apply policies to tables PostgreSQL would be responsible for ensuring that rows written by a user were labeled with that user's non-heirachical labels, and with their current session level for their heirachical labels. It'd also be responsible for enforcing matching on reads. Internally, we can do all of this with the functoinality provided by security-barrier views once write support is added. The interface exposed to the user is made drastically easier to use, though. Your example would become two non-heirachical security policies: CREATE SECURITY POLICY colors AS (blue, red, purple); CREATE SECURITY POLICY shapes AS (triangle, circle, square); ALTER TABLE color_shapes ADD SECURITY POLICY colors; ALTER TABLE colors_shapes ADD SECURITY POLICY shapes; You'd assign users to the policies: ALTER USER user1 SET SECURITY LABEL FOR POLICY colors TO blue, purple; ALTER USER user2 SET SECURITY LABEL FOR POLICY colors
Re: [HACKERS] [v9.4] row level security
On 12/14/2013 11:24 AM, Gregory Smith wrote: Things I can already see to work on here are: -Fix the regression tests -Catch up to master again I've got much of that in the tree: https://github.com/ringerc/postgres/tree/rls-9.4 -- 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] [v9.4] row level security
On Wed, Nov 6, 2013 at 1:38 AM, Craig Ringer cr...@2ndquadrant.com wrote: (a) Updatable views are implemented in the rewriter, not the planner. The rewriter is not re-run when plans are invalidated or when the session authorization changes, etc. This means that we can't simply omit the RLS predicate for superuser because the same rewritten parse tree might get used for both superuser and non-superuser queries. My impression was that we had discussed this problem with respect to some earlier version of the RLS patch and that the conclusion of that discussion was that we needed to record in the cached plan whether the plan was one which is sensitive to the user ID and, if so, avoid using that plan with a different user ID. I am murky on the details; I believe the original discussion of this topic was a year or more ago. (b) Inheritance is a problem when RLS is done in the rewriter. As I understood it from Kohei KaiGai's description to me earlier, there was a strong preference on -hackers to enforce RLS predicates for child and parent tables completely independently. Not to put a too fine a point on it, but I think that's a really bad plan; and I don't remember any such discussion. That's how RLS currently works, but it might be hard to get the same effect when applying RLS in the rewriter. We'd need to solve that, or redefine RLS's behaviour so that the predicate on a parent table applies to any child tables too. Personally I'd prefer the latter. Yes, let's please redefine it. The goal here ought to be to make RLS work as smoothly as possible with the rest of the system, not to invent weird semantics that are both unlike what we do elsewhere - and difficult to implement, to boot. I thought the whole point of implementing security barrier views was that read-side RLS would work just the same way, not having randomly incompatible semantics. -- 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] [v9.4] row level security
On Thu, Nov 7, 2013 at 9:08 PM, Craig Ringer cr...@2ndquadrant.com wrote: On 11/07/2013 09:47 PM, Greg Stark wrote: Incidentally I still feel this is at root the problem with updateable views in general. I know it's a bit off to be tossing in concerns from the peanut gallery when I'm not actually offering to work on it and others are having putting in serious efforts in this area and having some success. So take this for what it's worth... Frankly, the peanut gallery input can be quite handy. It's easy to get so stuck in the way you've seen it thought about already that you don't see other ways to view it. Plus, sometimes the peanut gallery becomes the oh, I don't like this at all crowd when commit time is approaching, so early comments are better than no comments then last minute complaints. I think the right approach for updateable views would be to support a syntax like this in the planner: UPDATE (select * from tab1,tab2 ...) WHERE tab1.id http://tab1.id = .. SET ... I want to support that for rewritten parse trees, and therefore (because of recursive rewrite) in pre-rewrite parse trees. It's exactly what's needed to make this sane, IMO, and I think this is what Robert was suggesting with making UPDATE capable of dealing with operating directly on a subquery scan. I'm not at all convinced it should be exposed to the user and accepted by the parser as SQL, but I don't know if that's what you were suggesting. Robert? Is this what you meant? If so, any chance you can point a planner neophyte like me in vaguely the right direction? I haven't studied this issue well enough to know what's really needed here, but Dean Rasheed's approach sounded like a promising tack to me. -- 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] [v9.4] row level security
On 6 November 2013 19:12, Kohei KaiGai kai...@kaigai.gr.jp wrote: 2013/11/6 Craig Ringer cr...@2ndquadrant.com: On 11/05/2013 09:36 PM, Robert Haas wrote: I haven't studied this patch in detail, but I see why there's some unhappiness about that code: it's an RLS-specific kluge. Just shooting from the hip here, maybe we should attack the problem of making security-barrier views updatable first, as a separate patch. That's the approach I've been considering. There are a few wrinkles with it, though: (a) Updatable views are implemented in the rewriter, not the planner. The rewriter is not re-run when plans are invalidated or when the session authorization changes, etc. This means that we can't simply omit the RLS predicate for superuser because the same rewritten parse tree might get used for both superuser and non-superuser queries. Options: * Store the before-rewrite parse tree when RLS is discovered on one of the rels in the tree. Re-run the rewriter when re-planning. Ensure a change in current user always invalidates plans. * Declare that it's not legal to run a query originally parsed and rewritten as superuser as a non-superuser or vice versa. This would cause a great deal of pain with PL/PgSQL. * Always add the RLS predicate and solve the problem of reliably short-circuiting the user-supplied predicate for RLS-exempt users. We'd need a way to allow direct (not query-based) COPY TO for tables with RLS applied, preventing the rewriting of direct table access into subqueries for COPY, but since we never save plans for COPY that may be fine. * ... ? How about an idea that uses two different type of rules: the existing one is expanded prior to planner stage as we are doing now, and the newer one is expanded on the head of planner stage. The argument of planner() is still parse tree, so it seems to me here is no serious problem to call rewriter again to handle second stage rules. If we go on this approach, ALTER TABLE ... SET ROW SECURITY will become a synonym to declare a rule with special attribute. I don't really get this part of the discussion. Why would you want to make updatable SB views do any of that? With RLS on tables, there is only one object in play - the table itself. So I can see that there is this requirement for certain privileged users to be able to bypass the RLS quals, and hence the need to invalidate cached plans. With SB views, however, you can have multiple SB views on top of the same base table, each giving different users access to different subsets of the data, and controlled by suitable GRANTs, and suitably privileged users can be given direct access to the base table. This also gives greater flexibility than the superuser/non-superuser distinction being discussed here. I don't think a view should ever show different data to different users (unless it has been deliberately set up to do so) because that would likely lead to confusion. Is there some other use-case that I'm missing here? 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] [v9.4] row level security
On Wed, Nov 6, 2013 at 6:38 AM, Craig Ringer cr...@2ndquadrant.com wrote: That's the approach I've been considering. There are a few wrinkles with it, though: (a) Updatable views are implemented in the rewriter, not the planner. The rewriter is not re-run when plans are invalidated or when the session authorization changes, etc. This means that we can't simply omit the RLS predicate for superuser because the same rewritten parse tree might get used for both superuser and non-superuser queries. Incidentally I still feel this is at root the problem with updateable views in general. I know it's a bit off to be tossing in concerns from the peanut gallery when I'm not actually offering to work on it and others are having putting in serious efforts in this area and having some success. So take this for what it's worth... I think the right approach for updateable views would be to support a syntax like this in the planner: UPDATE (select * from tab1,tab2 ...) WHERE tab1.id = .. SET ... Then updateable views would just rewrite the query mechanically the way regular views work by substituting the view definition in place of the view name. Since all the work would be done in the planner it would have access to the same kinds of information that regular join planning etc have. I'm not sure if this solves all the problems with RLS but it would solve the concern about plan invalidations and I think it would make it simpler to reason about security rules that are otherwise happening at plan time. -- greg
Re: [HACKERS] [v9.4] row level security
On 11/07/2013 09:47 PM, Greg Stark wrote: Incidentally I still feel this is at root the problem with updateable views in general. I know it's a bit off to be tossing in concerns from the peanut gallery when I'm not actually offering to work on it and others are having putting in serious efforts in this area and having some success. So take this for what it's worth... Frankly, the peanut gallery input can be quite handy. It's easy to get so stuck in the way you've seen it thought about already that you don't see other ways to view it. Plus, sometimes the peanut gallery becomes the oh, I don't like this at all crowd when commit time is approaching, so early comments are better than no comments then last minute complaints. I think the right approach for updateable views would be to support a syntax like this in the planner: UPDATE (select * from tab1,tab2 ...) WHERE tab1.id http://tab1.id = .. SET ... I want to support that for rewritten parse trees, and therefore (because of recursive rewrite) in pre-rewrite parse trees. It's exactly what's needed to make this sane, IMO, and I think this is what Robert was suggesting with making UPDATE capable of dealing with operating directly on a subquery scan. I'm not at all convinced it should be exposed to the user and accepted by the parser as SQL, but I don't know if that's what you were suggesting. Robert? Is this what you meant? If so, any chance you can point a planner neophyte like me in vaguely the right direction? I'm not sure if this solves all the problems with RLS but it would solve the concern about plan invalidations and I think it would make it simpler to reason about security rules that are otherwise happening at plan time. I don't think it'd help with plan invalidations. View expansion happens at rewrite time, and we don't re-run the rewriter on the original parse tree when we invalidate plans and re-plan. The current patch entirely elides the RLS quals and subquery when running as superuser and that won't work with view expansion at the rewrite stage. So we still have that to deal with, and need to handle a few side issues with portals and user id changes etc, but the biggest issue is coming up with an acceptable way to update a security barrier view or subquery. -- 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] [v9.4] row level security
On 11/07/2013 06:11 PM, Dean Rasheed wrote: I don't really get this part of the discussion. Why would you want to make updatable SB views do any of that? I don't, especially. If we're going to use updatable SB views as the basis for RLS then we need the option to skip adding the qual for superuser. That might be done outside the SB code its self, by not invoking the SB view rewrite on the base rel when we see we're running as an RLS-exempt user. It's all about dealing with the re-plan problem really. With SB views, however, you can have multiple SB views on top of the same base table, each giving different users access to different subsets of the data, and controlled by suitable GRANTs, and suitably privileged users can be given direct access to the base table. This also gives greater flexibility than the superuser/non-superuser distinction being discussed here. That's a thought. Can we munge what the planner sees in pg_class instead? So when we see a ref to an RLS relation we transparently substitute the oid for a hidden SB view over the relation instead. For RLS exempt users we omit that substitution. Since the lookups and view subs are done in the rewrite phase it probably doesn't help us much, but it'd get rid of the need to play about with substituting a RTE_RELATION with an RTE_SUBQUERY dynamically during rewrite. I don't think a view should ever show different data to different users (unless it has been deliberately set up to do so) because that would likely lead to confusion. Is there some other use-case that I'm missing here? The main concern is pg_dump - it's important that dumps be able to take a complete copy without relying on hacks or bug-free user-written RLS quals. Highly privileged users should also be exempt from RLS so they don't invoke untrusted functions that're part of RLS quals written by lower-rights users. This isn't really superuser vs not superuser. In fact we'll want a new right that controls whether RLS can be bypassed, and another that controls the ability to set RLS rights on tables. Both of those would be superuser only by default, but could be delegated. (Note: It's important that table owners _not_ get the right to set RLS on tables by default for security reasons. I'll explain in more detail later.) -- 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] [v9.4] row level security
On 11/06/2013 05:02 PM, Dean Rasheed wrote: The basic idea is to have rewriteTargetView() collect up any quals from SB views in a new list on the target RTE, instead of adding them to the main query's predicates (it needs to be a list of SB quals, in case there are SB views on top of other SB views, in which case they need to be kept separate from one another). Then at the end of the rewriting process (after any views referenced in the SB quals have been expanded), a new piece of code kicks in to process any RTEs with SB quals, turning them into (possibly nested) subquery RTEs. That makes sense, though presumably you face the same problem that the existing RLS code does with references to system columns that don't normally exist in subqueries? Since this happens during query rewrite, what prevents the optimizer from pushing outer quals down into the subqueries? The complication is that the query's resultRelation RTE mustn't be a subquery. I think this is what Robert was alluding to earlier with his comments about join relations: Robert Haas wrote: I don't really see why. AIUI, the ModifyTable node just needs to get the right TIDs. It's not like that has to be stacked directly on top of a scan; indeed, in cases like UPDATE .. FROM and DELETE .. USING it already isn't. Maybe there's some reason why the intervening level can be a Join but not a SubqueryScan, but if so I'd expect we could find some way of lifting that limitation without suffering too much pain. (http://www.postgresql.org/message-id/ca+tgmoyr1phw3x9vnvuwdcfxkzk2p_jhtwc0fv2q58negcx...@mail.gmail.com) Maybe we just need to make a subquery scan a valid target for an update, so those fixups aren't required anymore? This is handled this in a similar way to the trigger-updatable views code, producing 2 RTEs --- the resultRelation RTE becomes a direct reference to the base relation, and a separate subquery RTE acts as the source of rows to update. Some of the complexity of the current RLS code is caused by the need to do similar fixups to handle the case where the input relation isn't the same as the target relation, but is a subquery over it instead. Anyway, feel free to do what you like with this. I wasn't planning on submitting it to the next commitfest myself, because my non-PG workload is too high, and I don't expect to get much time to hack on postgresql during the next couple of months. Thanks for sending what you have. It's informative, and it shows that some of the same issues must be solved for writable security barrier views and for RLS. -- 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] [v9.4] row level security
On 6 November 2013 09:23, Craig Ringer cr...@2ndquadrant.com wrote: On 11/06/2013 05:02 PM, Dean Rasheed wrote: The basic idea is to have rewriteTargetView() collect up any quals from SB views in a new list on the target RTE, instead of adding them to the main query's predicates (it needs to be a list of SB quals, in case there are SB views on top of other SB views, in which case they need to be kept separate from one another). Then at the end of the rewriting process (after any views referenced in the SB quals have been expanded), a new piece of code kicks in to process any RTEs with SB quals, turning them into (possibly nested) subquery RTEs. That makes sense, though presumably you face the same problem that the existing RLS code does with references to system columns that don't normally exist in subqueries? Yeah, that feels like an ugly hack. Since this happens during query rewrite, what prevents the optimizer from pushing outer quals down into the subqueries? The subquery RTE is marked with the security_barrier flag, which prevents quals from being pushed down in the presence of leaky functions (see set_subquery_pathlist). The complication is that the query's resultRelation RTE mustn't be a subquery. I think this is what Robert was alluding to earlier with his comments about join relations: Robert Haas wrote: I don't really see why. AIUI, the ModifyTable node just needs to get the right TIDs. It's not like that has to be stacked directly on top of a scan; indeed, in cases like UPDATE .. FROM and DELETE .. USING it already isn't. Maybe there's some reason why the intervening level can be a Join but not a SubqueryScan, but if so I'd expect we could find some way of lifting that limitation without suffering too much pain. (http://www.postgresql.org/message-id/ca+tgmoyr1phw3x9vnvuwdcfxkzk2p_jhtwc0fv2q58negcx...@mail.gmail.com) Yeah. We already do a similar thing for trigger-updatable views. So with this approach you end up with similar plans, for example: Update on base_tbl - Subquery Scan on base_tbl ... Maybe we just need to make a subquery scan a valid target for an update, so those fixups aren't required anymore? Possibly. That feels like it would require much more extensive surgery on the planner though. I've not explored that idea, but I suspect it would quickly turn into a whole new can of worms. This is handled this in a similar way to the trigger-updatable views code, producing 2 RTEs --- the resultRelation RTE becomes a direct reference to the base relation, and a separate subquery RTE acts as the source of rows to update. Some of the complexity of the current RLS code is caused by the need to do similar fixups to handle the case where the input relation isn't the same as the target relation, but is a subquery over it instead. Anyway, feel free to do what you like with this. I wasn't planning on submitting it to the next commitfest myself, because my non-PG workload is too high, and I don't expect to get much time to hack on postgresql during the next couple of months. Thanks for sending what you have. It's informative, and it shows that some of the same issues must be solved for writable security barrier views and for RLS. Agreed. I'm not sure what the best way to fix those issues is though. The currently proposed approach feels pretty ugly, but I can't see a better way at the moment. 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] [v9.4] row level security
All, Just a comment: I'm really glad to see the serious work on this. If RLS doesn't make it into 9.4, it'll be because the problems of RLS are fundamentally unsolvable, not because we didn't give it our best. Great work, all! -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- 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] [v9.4] row level security
2013/11/6 Craig Ringer cr...@2ndquadrant.com: On 11/05/2013 09:36 PM, Robert Haas wrote: I haven't studied this patch in detail, but I see why there's some unhappiness about that code: it's an RLS-specific kluge. Just shooting from the hip here, maybe we should attack the problem of making security-barrier views updatable first, as a separate patch. That's the approach I've been considering. There are a few wrinkles with it, though: (a) Updatable views are implemented in the rewriter, not the planner. The rewriter is not re-run when plans are invalidated or when the session authorization changes, etc. This means that we can't simply omit the RLS predicate for superuser because the same rewritten parse tree might get used for both superuser and non-superuser queries. Options: * Store the before-rewrite parse tree when RLS is discovered on one of the rels in the tree. Re-run the rewriter when re-planning. Ensure a change in current user always invalidates plans. * Declare that it's not legal to run a query originally parsed and rewritten as superuser as a non-superuser or vice versa. This would cause a great deal of pain with PL/PgSQL. * Always add the RLS predicate and solve the problem of reliably short-circuiting the user-supplied predicate for RLS-exempt users. We'd need a way to allow direct (not query-based) COPY TO for tables with RLS applied, preventing the rewriting of direct table access into subqueries for COPY, but since we never save plans for COPY that may be fine. * ... ? How about an idea that uses two different type of rules: the existing one is expanded prior to planner stage as we are doing now, and the newer one is expanded on the head of planner stage. The argument of planner() is still parse tree, so it seems to me here is no serious problem to call rewriter again to handle second stage rules. If we go on this approach, ALTER TABLE ... SET ROW SECURITY will become a synonym to declare a rule with special attribute. (b) Inheritance is a problem when RLS is done in the rewriter. As I understood it from Kohei KaiGai's description to me earlier, there was a strong preference on -hackers to enforce RLS predicates for child and parent tables completely independently. That's how RLS currently works, but it might be hard to get the same effect when applying RLS in the rewriter. We'd need to solve that, or redefine RLS's behaviour so that the predicate on a parent table applies to any child tables too. Personally I'd prefer the latter. I'm not certain whether it was a strong preference, even though I followed the consensus at that time. So, I think it makes sense to discuss how RLS policy shall be enforced on the child tables. As long as we can have consistent view on child tables even if it is referenced without parent tables, I don't have any arguments to your preference. Also, it makes implementation simple than the approach I tried to have; that enforces RLS policy of tables individually, because of utilization of existing rule mechanism. It is not difficult to enforce parent's RLS policy on the child relation even if it is referenced individually. All we need to do special is append RLS policy of its parent, not only child's one, if referenced table has parent. (c) RLS might interact differently with rules declared on tables if implemented in the rewriter, so some investigation into that would be needed. I would think that if we make that work, this will also work without, hopefully, any special hackery. And we'd get a separate, independently useful feature out of it, too. I tend to agree. I'm just a bit concerned about dealing with the issues around RLS-exempt operations and users. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] [v9.4] row level security
On Mon, Nov 4, 2013 at 8:46 PM, Craig Ringer cr...@2ndquadrant.com wrote: On 11/04/2013 11:17 PM, Robert Haas wrote: I'd still like to here what's wrong with what I said here: http://www.postgresql.org/message-id/ca+tgmoyr1phw3x9vnvuwdcfxkzk2p_jhtwc0fv2q58negcx...@mail.gmail.com For me, just my understanding. I'm still too new to the planner and rewriter to grasp that properly as written. I was responding to Tom's objection, and he makes a good point about CASE and optimisation. We have to be free to re-order and pre-evaluate where LEAKPROOF flags make it safe and permissible, without ever otherwise doing so. That makes the SECURITY BARRIER subquery look better, since the limited pull-up / push-down is already implemented there. Robert, any suggesitons on how to approach what you suggest? I'm pretty new to the planner's guts, but I know there've been some complaints about the way the current RLS code fiddles with Vars when it changes a direct scan of a rel into a subquery scan. The code in: https://github.com/ringerc/postgres/blob/rls-9.4/src/backend/optimizer/prep/prepunion.c#L1647 and https://github.com/ringerc/postgres/blob/rls-9.4/src/backend/optimizer/prep/prepunion.c#L1591 seems to be the one folks were complaining about earlier. I haven't studied this patch in detail, but I see why there's some unhappiness about that code: it's an RLS-specific kluge. Just shooting from the hip here, maybe we should attack the problem of making security-barrier views updatable first, as a separate patch. I would think that if we make that work, this will also work without, hopefully, any special hackery. And we'd get a separate, independently useful feature out of it, too. -- 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] [v9.4] row level security
On 11/05/2013 09:36 PM, Robert Haas wrote: I haven't studied this patch in detail, but I see why there's some unhappiness about that code: it's an RLS-specific kluge. Just shooting from the hip here, maybe we should attack the problem of making security-barrier views updatable first, as a separate patch. That's the approach I've been considering. There are a few wrinkles with it, though: (a) Updatable views are implemented in the rewriter, not the planner. The rewriter is not re-run when plans are invalidated or when the session authorization changes, etc. This means that we can't simply omit the RLS predicate for superuser because the same rewritten parse tree might get used for both superuser and non-superuser queries. Options: * Store the before-rewrite parse tree when RLS is discovered on one of the rels in the tree. Re-run the rewriter when re-planning. Ensure a change in current user always invalidates plans. * Declare that it's not legal to run a query originally parsed and rewritten as superuser as a non-superuser or vice versa. This would cause a great deal of pain with PL/PgSQL. * Always add the RLS predicate and solve the problem of reliably short-circuiting the user-supplied predicate for RLS-exempt users. We'd need a way to allow direct (not query-based) COPY TO for tables with RLS applied, preventing the rewriting of direct table access into subqueries for COPY, but since we never save plans for COPY that may be fine. * ... ? (b) Inheritance is a problem when RLS is done in the rewriter. As I understood it from Kohei KaiGai's description to me earlier, there was a strong preference on -hackers to enforce RLS predicates for child and parent tables completely independently. That's how RLS currently works, but it might be hard to get the same effect when applying RLS in the rewriter. We'd need to solve that, or redefine RLS's behaviour so that the predicate on a parent table applies to any child tables too. Personally I'd prefer the latter. (c) RLS might interact differently with rules declared on tables if implemented in the rewriter, so some investigation into that would be needed. I would think that if we make that work, this will also work without, hopefully, any special hackery. And we'd get a separate, independently useful feature out of it, too. I tend to agree. I'm just a bit concerned about dealing with the issues around RLS-exempt operations and users. -- 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] [v9.4] row level security
On 09/04/2013 11:22 PM, Tom Lane wrote: AFAICT, to deal with update/delete the RLS patch needs to constrain order of qual application without the crutch of having a separate level of subquery; and it's that behavior that I have zero confidence in, either as to whether it works as submitted or as to our odds of not breaking it in the future. Wouldn't CASE do that job, albeit in a somewhat ugly manner, and also protect against malicious RLS functions? For any subclause in a WHERE clause (a) OR (b) you can instead write a short-circuit OR version as: CASE WHEN (a) THEN true ELSE (b) END no? So, given a row security condition like (rowowner = current_user AND rls_malicious_leak()) on table test, this: SELECT * FROM test WHERE client_leak_fn(); could be rewritten by row security as: SELECT * FROM test WHERE CASE WHEN CASE WHEN is_superuser() THEN true ELSE (rowowner = current_user AND rls_malicious_leak()) END THEN client_leak_fn() END; in other words: if the user is the superuser, don't evaluate the RLS predicate, just assume they have the right to see the row. Otherwise evaluate the RLS predicate to determine whether they can see the row. In either case, if they have the right to see the row, run the original WHERE clause. My main concern is that it'd be relying on a guarantee that CASE is always completely ordered, and that might not be ideal. It's also hideously ugly, and future optimiser smarts could create unexpected issues. Unless you propose the creaton of a new short-circuit left-to-right order guaranteed OR operator, and think the planner/optimizer should be taught special case rules to prevent it from doing pull-up / push-down or pre-evaluating subclauses for it, I suspect the notion of using security barrier views or subqueries is still going to be the sanest way to do it. -- 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] [v9.4] row level security
Craig Ringer cr...@2ndquadrant.com writes: On 09/04/2013 11:22 PM, Tom Lane wrote: AFAICT, to deal with update/delete the RLS patch needs to constrain order of qual application without the crutch of having a separate level of subquery; and it's that behavior that I have zero confidence in, either as to whether it works as submitted or as to our odds of not breaking it in the future. Wouldn't CASE do that job, albeit in a somewhat ugly manner, and also protect against malicious RLS functions? You mean wrap all the query quals in a CASE? Sure, if you didn't mind totally destroying any optimization possibilities. If you did that, every table scan would become a seqscan and every join a nestloop. 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] [v9.4] row level security
On Mon, Nov 4, 2013 at 9:37 AM, Tom Lane t...@sss.pgh.pa.us wrote: Craig Ringer cr...@2ndquadrant.com writes: On 09/04/2013 11:22 PM, Tom Lane wrote: AFAICT, to deal with update/delete the RLS patch needs to constrain order of qual application without the crutch of having a separate level of subquery; and it's that behavior that I have zero confidence in, either as to whether it works as submitted or as to our odds of not breaking it in the future. Wouldn't CASE do that job, albeit in a somewhat ugly manner, and also protect against malicious RLS functions? You mean wrap all the query quals in a CASE? Sure, if you didn't mind totally destroying any optimization possibilities. If you did that, every table scan would become a seqscan and every join a nestloop. I'd still like to here what's wrong with what I said here: http://www.postgresql.org/message-id/ca+tgmoyr1phw3x9vnvuwdcfxkzk2p_jhtwc0fv2q58negcx...@mail.gmail.com -- 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] [v9.4] row level security
On 11/04/2013 11:17 PM, Robert Haas wrote: I'd still like to here what's wrong with what I said here: http://www.postgresql.org/message-id/ca+tgmoyr1phw3x9vnvuwdcfxkzk2p_jhtwc0fv2q58negcx...@mail.gmail.com For me, just my understanding. I'm still too new to the planner and rewriter to grasp that properly as written. I was responding to Tom's objection, and he makes a good point about CASE and optimisation. We have to be free to re-order and pre-evaluate where LEAKPROOF flags make it safe and permissible, without ever otherwise doing so. That makes the SECURITY BARRIER subquery look better, since the limited pull-up / push-down is already implemented there. Robert, any suggesitons on how to approach what you suggest? I'm pretty new to the planner's guts, but I know there've been some complaints about the way the current RLS code fiddles with Vars when it changes a direct scan of a rel into a subquery scan. The code in: https://github.com/ringerc/postgres/blob/rls-9.4/src/backend/optimizer/prep/prepunion.c#L1647 and https://github.com/ringerc/postgres/blob/rls-9.4/src/backend/optimizer/prep/prepunion.c#L1591 seems to be the one folks were complaining about earlier. -- 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] [v9.4] row level security
Because of CF-2nd end, it was moved to the next commit-fest. In my personal opinion, it probably needs a few groundwork to get RLS commitable, prior to RLS itself. One is a correct handling towards the scenario that Korotkov pointed out. (How was it concluded?) I think it is a problem we can fix with reasonable way. Likely, solution is to prohibit to show number of rows being filtered if plan node is underlying a sub- query with security-barrier. One other stuff I'm concerned about is, existing implementation assumes a certain rtable entry performs as a source relation, also result relation in same time on DELETE and UPDATE. We usually scan a regular relation to fetch ctid of the tuples to be modified, then ModifyTable deletes or updates the tuple being identified. Here has been no matter for long period, even if same rtable entry is used to point out a relation to be scanned and to be modified. It however makes RLS implementation complicated, because this feature tries to replace a rtable entry to relation with row-level security policy by a simple sub-query with security-barrier attribute. Our implementation does not assume a sub-query performs as a result relation, so I had to have some ad-hoc coding, like adjustment on varno/varattno of Var objects, to avoid problem. I think, solution is to separate a rtable entry of result-relation from rtable entry to be scanned. It makes easier to implement RLS feature because all we need to do is just replacement of rtable entry for scanning stage, and no need to care about ModifyTable operations towards sub-query. Is it a right direction to go? Thanks, 2013/10/10 Marc Munro m...@bloodnok.com: On Wed, 2013-09-04 at 14:35 +, Robert Haas wrote: On Fri, Aug 30, 2013 at 3:43 PM, Tom Lane t...@sss.pgh.pa.us wrote: I think it's entirely sensible to question whether we should reject (not hold up) RLS if it has major covert-channel problems. We've already had this argument before, about the security_barrier [ . . . ] Sorry for following up on this so late, I have just been trying to catch up with the mailing lists. I am the developer of Veil, which this thread mentioned a number of times. I wanted to state/confirm a number of things: Veil is not up to date wrt Postgres versions. I didn't release a new version for 9.2, and when no-one complained I figured no-one other than me was using it. I'll happily update it if anyone wants it. Veil makes no attempt to avoid covert channels. It can't. Veil is a low-level toolset designed for optimising queries about privileges. It allows you to build RLS with reasonable performance, but it is not in itself a solution for RLS. I wish the Postgres RLS project well and look forward to its release in Postgres 9.4. __ Marc -- 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] [v9.4] row level security
On Wed, 2013-09-04 at 14:35 +, Robert Haas wrote: On Fri, Aug 30, 2013 at 3:43 PM, Tom Lane t...@sss.pgh.pa.us wrote: I think it's entirely sensible to question whether we should reject (not hold up) RLS if it has major covert-channel problems. We've already had this argument before, about the security_barrier [ . . . ] Sorry for following up on this so late, I have just been trying to catch up with the mailing lists. I am the developer of Veil, which this thread mentioned a number of times. I wanted to state/confirm a number of things: Veil is not up to date wrt Postgres versions. I didn't release a new version for 9.2, and when no-one complained I figured no-one other than me was using it. I'll happily update it if anyone wants it. Veil makes no attempt to avoid covert channels. It can't. Veil is a low-level toolset designed for optimising queries about privileges. It allows you to build RLS with reasonable performance, but it is not in itself a solution for RLS. I wish the Postgres RLS project well and look forward to its release in Postgres 9.4. __ Marc signature.asc Description: This is a digitally signed message part
Re: [HACKERS] [v9.4] row level security
Now I'm trying to tackle the covert-channel problem that Korotkov pointed out at upthread. The attached patch works almost well. It prevents to print number of rows being filtered if the target plan node is under sub-query with security-barrier attribute; because row- level security feature is constructed on existing security-barrier view infrastructure. One remaining issue is that planner pulls-up underlying relation scan if top-level sub-query is enough simple; probably, in case when targetlist of upper sub-query is compatible with underlying relation scan. See, the example below: postgres=# CREATE TABLE t1 (a int primary key, b int); CREATE TABLE postgres=# INSERT INTO t1 VALUES (1,),(2,),(3,),(4,),(5,); INSERT 0 5 postgres=# CREATE VIEW v1 WITH(security_barrier) AS SELECT * FROM t1 WHERE a % 2 = 1; CREATE VIEW postgres=# SET enable_seqscan = off; SET postgres=# EXPLAIN ANALYZE SELECT 1 FROM v1 WHERE b BETWEEN 2000 AND 4000; QUERY PLAN -- Subquery Scan on v1 (cost=100.00..152.81 rows=1 width=0) (actual time=0.016..0.017 rows=1 loops=1) - Seq Scan on t1 (cost=100.00..152.80 rows=1 width=8) (actual time=0.014..0.015 rows=1 loops=1) Filter: ((b = 2000) AND (b = 4000) AND ((a % 2) = 1)) Total runtime: 0.067 ms (4 rows) According to the scenario that Korotkov pointed out, number of filtered rows shall be printed, thus, it allows to estimate particular value with log(N) times trials. However, this example hides number of rows being done within security-barrier sub- query as I expected. On the other hand, if planner pulled-up underlying relation scan, it does NOT work fine. postgres=# EXPLAIN ANALYZE SELECT * FROM v1 WHERE b BETWEEN 2000 AND 4000; QUERY PLAN Seq Scan on t1 (cost=100.00..152.80 rows=1 width=8) (actual time=0.019..0.021 rows=1 loops=1) Filter: ((b = 2000) AND (b = 4000) AND ((a % 2) = 1)) Rows Removed by Filter: 4 Total runtime: 0.055 ms (4 rows) It seems to me we need to prohibit to pull-up security-barrier sub-query here, or to mark underlying relation scan security-barrier attribute to solve this issue. (I'm still looking for which routine handles this pull-up job, however, I didn't find it yet.) How about opinion for this solution? Thanks, 2013/9/7 Peter Eisentraut pete...@gmx.net: On Wed, 2013-08-28 at 13:56 +0200, Kohei KaiGai wrote: The attached patch fixed the portion I was pointed out, so its overall design has not been changed so much. The documentation doesn't build: openjade:catalogs.sgml:237:28:X: reference to non-existent ID CATALOG-PG-ROWLEVELSEC -- KaiGai Kohei kai...@kaigai.gr.jp explain_hide_filtered.v0.patch Description: Binary data -- 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] [v9.4] row level security
On Wed, 2013-08-28 at 13:56 +0200, Kohei KaiGai wrote: The attached patch fixed the portion I was pointed out, so its overall design has not been changed so much. The documentation doesn't build: openjade:catalogs.sgml:237:28:X: reference to non-existent ID CATALOG-PG-ROWLEVELSEC -- 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] [v9.4] row level security
On Wed, Sep 4, 2013 at 10:50 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Wed, Sep 4, 2013 at 10:45 AM, Tom Lane t...@sss.pgh.pa.us wrote: Well, the security-barrier view stuff did not present itself as a 100% solution. But perhaps more to the point, it was conceptually simple to implement, ie don't flatten views if they have this bit set, and don't push down quals into such sub-selects unless they're marked leakproof. Right. IMHO, this new feature should be similarly simple: when an unprivileged user references a table, treat that as a reference to a leakproof view over the table, with the RLS qual injected into the view. And for insert/update/delete, we do what exactly? The same mechanism will prevent UPDATE and DELETE from seeing any rows the user shouldn't be able to touch. Simon and Greg are arguing that when an INSERT or UPDATE happens, we ought to also check that the NEW row matches the RLS qual. I don't find that to be terribly important because you can accomplish the same thing with a per-row trigger today; and I also don't think everyone will want that behavior. Some people will, I'm pretty sure, want to let users give away rows, either unconditionally or subject to defined restrictions. Perhaps it's worth having, but it's a separate feature, IMHO. -- 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] [v9.4] row level security
Robert Haas robertmh...@gmail.com writes: On Fri, Aug 30, 2013 at 3:43 PM, Tom Lane t...@sss.pgh.pa.us wrote: I think it's entirely sensible to question whether we should reject (not hold up) RLS if it has major covert-channel problems. We've already had this argument before, about the security_barrier view stuff, and that code got committed and is already released. So the horse is already out of the barn and no amount of wishing will put it back in. Well, the security-barrier view stuff did not present itself as a 100% solution. But perhaps more to the point, it was conceptually simple to implement, ie don't flatten views if they have this bit set, and don't push down quals into such sub-selects unless they're marked leakproof. I haven't reviewed this patch in a long time, but I would expect that it's basically just reusing that same infrastructure; in fact, I'd expect that it's little more than syntactic sugar around that infrastructure. I've not read it in great detail, but it isn't that. It's whacking the planner around in ways that I have no confidence in, and probably still wouldn't have any confidence in if they'd been adequately documented. 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] [v9.4] row level security
On Sun, Sep 1, 2013 at 11:47 PM, Greg Smith g...@2ndquadrant.com wrote: And if someone can INSERT values that they can't actually see once they're committed, that's a similarly bad we should describe. This is desirable in some cases but not others. If the goal is compartmentalization, then it's sensible to prevent this. But you might also have a drop-box environment - e.g. a student submits coursework to a professor, and can't access the submitted work after it's submitted. FWIW, my CS classes in college had a tool that worked just this way. Or maybe an analyst writes a report and is then permitted to give away the document to his boss for revisions. Once the ownership of the document has changed, the analyst can't see it any more, because he can only see the documents he owns. And maybe he's not permitted to give away documents to just anyone (polluting their sandbox), but he can give them to his boss (who expects to receive them). The point is that we should be in the business of providing mechanism, not policy. -- 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] [v9.4] row level security
2013/9/3 Bruce Momjian br...@momjian.us: On Sun, Sep 1, 2013 at 11:05:58AM -0700, Josh Berkus wrote: Security community also concludes it is not avoidable nature as long as human can observe system behavior and estimate something, thus, security evaluation criteria does not require eliminate covert-channels or does not pay attention about covert-channels for the products that is installed on the environment with basic robustness (that means, non-military, regular enterprise usage). To be completely blunt, the security community does not understand databases. At all. I'd think if anything had become clear through the course of work on SEPosgres, it would be that. Agreed. The security community realizes these covert channels exist, but doesn't really have any recommendations on how to avoid them. You could argue that avoiding them is too tied to specific database implementations, but there are general channels, like insert with a unique key, that should at least have well-defined solutions. The security community also provides an extreme solution, but I don't think it is suitable for flexible security policy and PostgreSQL wants it. Their extreme solution manipulate definition of PK that automatically become combined key that takes user-given key and security level being set mandatory. Thus, it does not conflict even if two different users with different security level tries to insert a row with same primary key. This technology is called polyinstantiation. http://en.wikipedia.org/wiki/Polyinstantiation However, of course, I'm not favor to port this technology to PostgreSQL world. Its side-effects are too much towards the problem to be solved. 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] [v9.4] row level security
On Fri, Aug 30, 2013 at 3:43 PM, Tom Lane t...@sss.pgh.pa.us wrote: I think it's entirely sensible to question whether we should reject (not hold up) RLS if it has major covert-channel problems. We've already had this argument before, about the security_barrier view stuff, and that code got committed and is already released. So the horse is already out of the barn and no amount of wishing will put it back in. I haven't reviewed this patch in a long time, but I would expect that it's basically just reusing that same infrastructure; in fact, I'd expect that it's little more than syntactic sugar around that infrastructure. (If it it's instead introducing a whole new mechanism, then I think that's reason enough to reject it right there.) My main question about this is whether that syntactic sugar is really worth having given that it doesn't add any real new functionality, not about the covert channel issues, which are already a done deal. And frankly, I'm with the group that says the covert channel issues are not really a big deal. In many real-world cases, the user can control only the values that get subbed into queries that get sent to the database, not the queries themselves, which eliminates a large category of attacks. Real-world example, from last job: sales reps only get to see their own accounts, not accounts of other sales reps. They could input new accounts (with sales_rep_id set to their ID) and they could query the list of accounts (limited to those where sales_rep_id matched their ID) - pulling either all of them or searching by account name, both through a web application. Yeah, a sales rep could have launched a timing attack through the web interface, and they could also have polled for the existence of accounts by trying to create accounts with names that might already exist in the system to see whether a duplicate got flagged. But neither attack had enough useful bandwidth to matter; a sales rep wishing to learn our full account list (so that he could try to poach them after leaving the company) could have learned a lot more a lot faster via social engineering, and with less risk of being caught doing something that would have resulted in his or her immediate termination. The point is, I don't think RLS needs to solve every problem. What it needs to do is solve one problem well, so that it can be used in combination with other techniques to achieve a certain set of security objectives. If, in a particular environment, EXPLAIN is an issue, then it can be blocked in that environment via a variety of well-understood techniques. That's not very hard to do even without core support, and if we want to add core support, fine, but that's a separate patch. This is a patch to add row-level security, and it deserves to be judged on how well or poorly it does that, not on whether it solves covert channel problems that represent a mostly orthogonal set of technical issues. -- 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] [v9.4] row level security
Robert Haas robertmh...@gmail.com writes: On Wed, Sep 4, 2013 at 10:50 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Right. IMHO, this new feature should be similarly simple: when an unprivileged user references a table, treat that as a reference to a leakproof view over the table, with the RLS qual injected into the view. And for insert/update/delete, we do what exactly? The same mechanism will prevent UPDATE and DELETE from seeing any rows the user shouldn't be able to touch. No, it won't, because we don't support direct update/delete on views (and if you look, you'll notice the auto-updatable-view stuff doesn't think a security-barrier view is auto-updatable). AFAICT, to deal with update/delete the RLS patch needs to constrain order of qual application without the crutch of having a separate level of subquery; and it's that behavior that I have zero confidence in, either as to whether it works as submitted or as to our odds of not breaking it in the future. 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] [v9.4] row level security
* Robert Haas (robertmh...@gmail.com) wrote: On Sun, Sep 1, 2013 at 11:47 PM, Greg Smith g...@2ndquadrant.com wrote: And if someone can INSERT values that they can't actually see once they're committed, that's a similarly bad we should describe. This is desirable in some cases but not others. If the goal is compartmentalization, then it's sensible to prevent this. But you might also have a drop-box environment - e.g. a student submits coursework to a professor, and can't access the submitted work after it's submitted. FWIW, my CS classes in college had a tool that worked just this way. Agreed, and part of the discussion that I had w/ KaiGai and Simon was that we should provide a way to let the user pick which they'd like. This is the concept around 'insert privileges' being different from 'select privileges' wrt RLS. The point is that we should be in the business of providing mechanism, not policy. ++ Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] [v9.4] row level security
Robert Haas robertmh...@gmail.com wrote: Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: IMHO, this new feature should be similarly simple: when an unprivileged user references a table, treat that as a reference to a leakproof view over the table, with the RLS qual injected into the view. And for insert/update/delete, we do what exactly? The same mechanism will prevent UPDATE and DELETE from seeing any rows the user shouldn't be able to touch. +1 Simon and Greg are arguing that when an INSERT or UPDATE happens, we ought to also check that the NEW row matches the RLS qual. I don't find that to be terribly important because you can accomplish the same thing with a per-row trigger today; and I also don't think everyone will want that behavior. As an example from my Wisconsin Courts days, source documents come in which need to be entered, which may contain a Social Security Number, and most of the Clerk of Courts staff should be authorized to enter that into the database. Once it is entered, most people should not be allowed to view it, including many of the people who were authorized to enter it initially. It's one thing for a line staff person to have a handful of documents come across their desk with SSN on a given day; it's quite another if they could dump a list of names, addresses, dates of birth, and SSNs for the entire database on demand. In a way this issue is similar to the covert channel issue -- volume matters. -- Kevin Grittner EDB: 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] [v9.4] row level security
Robert Haas robertmh...@gmail.com writes: On Wed, Sep 4, 2013 at 10:45 AM, Tom Lane t...@sss.pgh.pa.us wrote: Well, the security-barrier view stuff did not present itself as a 100% solution. But perhaps more to the point, it was conceptually simple to implement, ie don't flatten views if they have this bit set, and don't push down quals into such sub-selects unless they're marked leakproof. Right. IMHO, this new feature should be similarly simple: when an unprivileged user references a table, treat that as a reference to a leakproof view over the table, with the RLS qual injected into the view. And for insert/update/delete, we do what exactly? 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] [v9.4] row level security
2013/9/4 Tom Lane t...@sss.pgh.pa.us: Robert Haas robertmh...@gmail.com writes: On Wed, Sep 4, 2013 at 10:45 AM, Tom Lane t...@sss.pgh.pa.us wrote: Well, the security-barrier view stuff did not present itself as a 100% solution. But perhaps more to the point, it was conceptually simple to implement, ie don't flatten views if they have this bit set, and don't push down quals into such sub-selects unless they're marked leakproof. Right. IMHO, this new feature should be similarly simple: when an unprivileged user references a table, treat that as a reference to a leakproof view over the table, with the RLS qual injected into the view. And for insert/update/delete, we do what exactly? This patch does not care about insert, because it shall be done around the place where we usually put before-row-insert; that is not related to planner. Regarding to update/delete, this patch also enhanced to allow update or delete mechanism allows to take a sub-query on top of the table scan plan. So, its explain output shows as follows: postgres= EXPLAIN (costs off) UPDATE customer SET email = 'al...@example.com'; QUERY PLAN -- Update on customer - Subquery Scan on customer - Seq Scan on customer customer_1 Filter: (current_user() = uname) You can see this update has Subquery plan instead of regular relation scan. 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] [v9.4] row level security
2013/9/1 Greg Stark st...@mit.edu: On Sun, Sep 1, 2013 at 8:31 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: Or, any other criteria even though? My (current) preference is plan (c: we will be able to fix up *this* cover-channel with reasonable efforts on explain code. probably, we can close it if we don't print filtered rows under the sub-query with security-barrier attribute. I think the criteria being discussed in this thread are too strict. It may be the case that Postgres cannot make a strong general case that it protects against covert channels. However it may still be able to make the much weaker case that it is *possible* to arrange your database such that the covert channels are kept under control. Yes. I have to admit it is difficult to determine a strict and regular rule to handle covert-channel scenario. Sorry, the later half of this sentence is uncertain for me. Are you saying, even if we could have a strict rule, we may have many possible covert channel for information leakage?? So I think up above Tom is wrong about why it's ok that INSERT leaks keys when it reports a unique key violation. The reason why it's ok that there's a covert channel there is that the DBA can avoid that covert channel by being careful when creating unique constraints. He or she should be aware that creating a unique constraint implicitly provides a kind of limited access to data to users who have INSERT privilege even if they lack the real SELECT privilege. IIRC, we discussed and concluded that the above information leakage scenario shall be described in the documentation, and the way to avoid valuable information leakage using alternative keys, a few years before. Likewise, as long as the covert channels in RLS are things the DBA has even a modicum of control over I wouldn't be too worried. Afaict from skimming the thread it looks like creating any indexes on columns leaks what values of the index key exist in the table. Is it the case that non-indexed columns do not get leaked? According to the scenario reported by Korotkov, he could find number of rows being filtered by the given qualifier, thus it implies existence of the row with a value in a particular range. Its solution is that I noted above, and I'm working for it now. 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] [v9.4] row level security
On Wed, Sep 4, 2013 at 10:45 AM, Tom Lane t...@sss.pgh.pa.us wrote: Well, the security-barrier view stuff did not present itself as a 100% solution. But perhaps more to the point, it was conceptually simple to implement, ie don't flatten views if they have this bit set, and don't push down quals into such sub-selects unless they're marked leakproof. Right. IMHO, this new feature should be similarly simple: when an unprivileged user references a table, treat that as a reference to a leakproof view over the table, with the RLS qual injected into the view. I haven't reviewed this patch in a long time, but I would expect that it's basically just reusing that same infrastructure; in fact, I'd expect that it's little more than syntactic sugar around that infrastructure. I've not read it in great detail, but it isn't that. It's whacking the planner around in ways that I have no confidence in, and probably still wouldn't have any confidence in if they'd been adequately documented. If that's the case, then I agree that we should not accept it, at least in its present form. -- 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] [v9.4] row level security
2013/9/4 Kevin Grittner kgri...@ymail.com: Robert Haas robertmh...@gmail.com wrote: Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: IMHO, this new feature should be similarly simple: when an unprivileged user references a table, treat that as a reference to a leakproof view over the table, with the RLS qual injected into the view. And for insert/update/delete, we do what exactly? The same mechanism will prevent UPDATE and DELETE from seeing any rows the user shouldn't be able to touch. +1 Simon and Greg are arguing that when an INSERT or UPDATE happens, we ought to also check that the NEW row matches the RLS qual. I don't find that to be terribly important because you can accomplish the same thing with a per-row trigger today; and I also don't think everyone will want that behavior. As an example from my Wisconsin Courts days, source documents come in which need to be entered, which may contain a Social Security Number, and most of the Clerk of Courts staff should be authorized to enter that into the database. Once it is entered, most people should not be allowed to view it, including many of the people who were authorized to enter it initially. It's one thing for a line staff person to have a handful of documents come across their desk with SSN on a given day; it's quite another if they could dump a list of names, addresses, dates of birth, and SSNs for the entire database on demand. In a way this issue is similar to the covert channel issue -- volume matters. I think an important nature of this behavior is it is configurable. In case when both of reader and writer side need to have same security policy, it's good. One configuration allows to apply a consistent security policy to fetch a row from table, and to write a row to table. If they don't want to check security policy on writer side, all they need to do is setting a security policy for SELECT only, even though its functionality is not implemented yet. 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] [v9.4] row level security
2013/9/4 Tom Lane t...@sss.pgh.pa.us: Robert Haas robertmh...@gmail.com writes: On Wed, Sep 4, 2013 at 10:50 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Right. IMHO, this new feature should be similarly simple: when an unprivileged user references a table, treat that as a reference to a leakproof view over the table, with the RLS qual injected into the view. And for insert/update/delete, we do what exactly? The same mechanism will prevent UPDATE and DELETE from seeing any rows the user shouldn't be able to touch. No, it won't, because we don't support direct update/delete on views (and if you look, you'll notice the auto-updatable-view stuff doesn't think a security-barrier view is auto-updatable). AFAICT, to deal with update/delete the RLS patch needs to constrain order of qual application without the crutch of having a separate level of subquery; and it's that behavior that I have zero confidence in, either as to whether it works as submitted or as to our odds of not breaking it in the future. Are you suggesting to rewrite update / delete statement to filter out unprivileged rows from manipulation? Yes. I also thought it is a simple solution that does not need additional enhancement to allow update / delete to take sub-query on top of reader side plan. For example, if security policy is (t1.owner = current_user) and the given query was UPDATE t1 SET value = value || '_updated' WHERE value like '%abc%', this query may be able to rewritten as follows: UPDATE t1 SET value = value || '_updated' WHERE tid = ( SELECT tid FROM t1 WHERE t1.owner = current_user ) AND value like '%abc%'; This approach makes implementation simple, but it has to scan the relation twice, thus its performance it not ideal, according to the past discussion. 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] [v9.4] row level security
On Wed, Sep 4, 2013 at 11:22 AM, Tom Lane t...@sss.pgh.pa.us wrote: The same mechanism will prevent UPDATE and DELETE from seeing any rows the user shouldn't be able to touch. No, it won't, because we don't support direct update/delete on views (and if you look, you'll notice the auto-updatable-view stuff doesn't think a security-barrier view is auto-updatable). AFAICT, to deal with update/delete the RLS patch needs to constrain order of qual application without the crutch of having a separate level of subquery; and it's that behavior that I have zero confidence in, either as to whether it works as submitted or as to our odds of not breaking it in the future. I don't really see why. AIUI, the ModifyTable node just needs to get the right TIDs. It's not like that has to be stacked directly on top of a scan; indeed, in cases like UPDATE .. FROM and DELETE .. USING it already isn't. Maybe there's some reason why the intervening level can be a Join but not a SubqueryScan, but if so I'd expect we could find some way of lifting that limitation without suffering too much pain. -- 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] [v9.4] row level security
Kohei KaiGai kai...@kaigai.gr.jp writes: 2013/9/4 Tom Lane t...@sss.pgh.pa.us: And for insert/update/delete, we do what exactly? Regarding to update/delete, this patch also enhanced to allow update or delete mechanism allows to take a sub-query on top of the table scan plan. So, its explain output shows as follows: postgres= EXPLAIN (costs off) UPDATE customer SET email = 'al...@example.com'; QUERY PLAN -- Update on customer - Subquery Scan on customer - Seq Scan on customer customer_1 Filter: (current_user() = uname) You can see this update has Subquery plan instead of regular relation scan. Really? That wasn't apparent from reading the patch. (Have I mentioned it's desperately underdocumented? Aside from needing a lot more in-code comments than it's got, it would benefit from having an overview section added to optimizer/README explaining stuff at the level of this discussion.) I'm a bit surprised that setrefs.c doesn't eliminate the Subquery Scan as being a no-op, given that no quals end up getting applied at that level. You might look into why not, since if that plan node were eliminated at the end, it'd fix any efficiency complaints about this approach. 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] [v9.4] row level security
On Sun, Sep 1, 2013 at 11:05:58AM -0700, Josh Berkus wrote: Security community also concludes it is not avoidable nature as long as human can observe system behavior and estimate something, thus, security evaluation criteria does not require eliminate covert-channels or does not pay attention about covert-channels for the products that is installed on the environment with basic robustness (that means, non-military, regular enterprise usage). To be completely blunt, the security community does not understand databases. At all. I'd think if anything had become clear through the course of work on SEPosgres, it would be that. Agreed. The security community realizes these covert channels exist, but doesn't really have any recommendations on how to avoid them. You could argue that avoiding them is too tied to specific database implementations, but there are general channels, like insert with a unique key, that should at least have well-defined solutions. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] [v9.4] row level security
On Fri, Aug 30, 2013 at 04:24:06PM -0400, Stephen Frost wrote: The scenario I'm worried about is where somebody says hey, Postgres has RLS now, I can rely on that to hide my sooper sekrit data from other users in the same database, and later they have a security breach through some covert-channel attack. Are they going to blame themselves? No, they're gonna blame Postgres. Or consider the case where some bozo publishes a method for such an attack and uses it to badmouth us as insecure. In my experience, issues are discovered, patched, and released as security updates. Does adding RLS mean we might have more of those? Sure, as did column level privileges and as does *any* such increase in the granularity of what we can provide security-wise. Releasing a feature we think is perfect, and finding out later is isn't, and fixing it, is not the same as releasing a feature we _know_ isn't perfect, and having no idea how to fix it. From later discussions, it seems like we have to accept that we will never be able to avoid all convert channels, e.g. timing queries, but we can avoid the most obvious ones, e.g. EXPLAIN, and improve it as we go. Knowing there is no end to improving it does make some people feel uncomfortable, and I can't think of another feature we have added with such an open-ended nature. We do have open-ended performance features, but here is seems the value of the feature itself, security, is not attainable. Perhaps reasonable security is attainable. I am not saying we should reject this feature --- just that the calculus of how we decide to add this feature doesn't fit our normal analysis. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] [v9.4] row level security
I think there is another covert channel much more serious than constrains. You can gather information about hidden data by reading query plans. I think a lot of people would be quite happy to simply disallow EXPLAIN. Define a permission for it, grant it to public and newly created users/groups by default (for BC), and allow it to be revoked. To define what is/isn't reasonable in terms of covert channel leakage, I (reluctantly) suggest checking out the Common Criteria stuff. Yes, it's verbose and questionably useful, but it's something that already exists and that has lots of weight in government / large orgs. I've started reading into it but don't have enough info to comment on RLS specifically yet. Regarding unique keys and other constraints as leakage channels, I'm inclined to think that this is partly a documentation issue. The docs can explain what is/isn't protected against. Suggest creating keys incorporating security domain identifiers so that users in different security domains can't create conflicting values. Possibly provide a mechanism to enforce that so that users can't attempt to insert/update rows with a different security identifier. Or even make it transparently part of the system's operations, an implicit extra column in PRIMARY and FOREIGN keys on RLS-enabled tables. What I'm more worried about re the covert uniqueness issue is that any solution might run the risk of creating situations where changes in access rights can make previously valid data in tables invalid. If two users A and B can't see each other's data and create the same values for a key, then B is given the rights to see A's data ... those unique values now have duplicates. So: I do think we need to step back a little when it comes to covert channel attacks and define what we do/don't defend against. What about timing attacks - do we need to make sure we don't leak information about the number of RLS rows via scan durations? Make everything constant-time? Yick. Crypto systems have a hard enough time getting this stuff right, and I just think we say we don't defend against timing attacks. If someone wants to mess around with adding random sleeps in an executor hook, well, that's their pain to deal with. -- 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] [v9.4] row level security
On 30.08.2013 22:57, Josh Berkus wrote: Right now, the primary tool for doing row filtering for MTA is Veil, which has numerous and well-known limitations. If RLS has fewer limitations, or is easier to deploy, maintain, and/or understand, then it's a valuable feature for that user base, even if it doesn't address the covert channels we've brought up at all. That is, if RLS is your*second* level of defense, instead of your primary defense, covert channels are not a make-or-break issue. It just has to be better than what we had before. Note that I have NOT done an evaluation of Veil vs. RLS for MTA at this point. I'm hoping someone else will ;-) I'd also like to hear how Veil differs from RLS. From what I've understood this far, they are the same in terms of what you can and cannot do. To phrase it differently: We already have RLS. It's shipped as an extension called Veil. Now please explain what's wrong with that statement, if anything. - Heikki -- 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] [v9.4] row level security
On 2013-09-01 16:38:51 +0300, Heikki Linnakangas wrote: On 30.08.2013 22:57, Josh Berkus wrote: Right now, the primary tool for doing row filtering for MTA is Veil, which has numerous and well-known limitations. If RLS has fewer limitations, or is easier to deploy, maintain, and/or understand, then it's a valuable feature for that user base, even if it doesn't address the covert channels we've brought up at all. That is, if RLS is your*second* level of defense, instead of your primary defense, covert channels are not a make-or-break issue. It just has to be better than what we had before. Note that I have NOT done an evaluation of Veil vs. RLS for MTA at this point. I'm hoping someone else will ;-) I'd also like to hear how Veil differs from RLS. From what I've understood this far, they are the same in terms of what you can and cannot do. To phrase it differently: We already have RLS. It's shipped as an extension called Veil. Now please explain what's wrong with that statement, if anything. I don't think veil really is an argument for much in this discussion. I don't know who in this discussion has used it, I have. Admittedly a good bit ago, 8.2, 8.3 days I think. There hasn't been much fundamental development since though, if my quick look is right. Veil gives you a rather low level toolbox for developing an RLS and you're left with a *huge* amount of work needed ontop. It basically gives you a bunch of new datatypes (sets, bitmaps, ranges) and provides some support for sharing variables across backends (shared memory). That's nearly it. 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] [v9.4] row level security
On 9/1/13 9:38 AM, Heikki Linnakangas wrote: To phrase it differently: We already have RLS. It's shipped as an extension called Veil. Now please explain what's wrong with that statement, if anything. Veil was last updated for 9.1 to work against that version, so the first thing is that it's two versions back from being current. The main improvement for a few now core features, compared to their external/extension predecessors, is that they go through a real review process. I suspect a lot of the criticisms being lobbied against the core RLS feature would also hit Veil if it were evaluated to the same standard. Regardless, I'm seeing a few review themes pop up from this thread: -Comparison against the Veil feature set. -Competitive review against industry expectations, AKA checkbox compliance. -Confirm feature set is useful to government security clearance applications and multi-tenant applications. There's also a secured web application use case that's popped up a few times too; KaiGai has used secured Apache installs for example. -Summary of known covert channels, with documentation coverage. -Assess odds of this implementation's future issues turning into security bugs. My personal hotspot here is that I'd like minimal code exposure to people who don't use this feature at all. Are there parts here that should be compile time enabled? Of course those are all on top of the usual code quality review. Did I miss any big themes on that list? -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com -- 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] [v9.4] row level security
Kaigai, However, we have yet to talk about taking any such provisions with Postgres. If we commit this patch, arguably we'll have a row-level security feature which only protects data from well-behaved users, which seems counterproductive. The point we shouldn't forget is information leakage via covert-channel has less grade of threat than leakage via main-channel, because of much less bandwidth. That's an astonishingly weak argument, because the bandwidth you're talking about is still very high, as in *hundreds* or *thousands* of values per minute. It's one thing to make the bandwidth argument for things like monitoring power draw, where the leakage is one character per hour, but the covert channels we're talking about are several orders of magnitude faster than that. So, I repeat: if you continue to pursue the argument that the covert channels identified aren't significant because of bandwidth, you will doom this patch. The valid arguments are only two: a) clearly identified use case X doesn't care about covert channels for reason Y, and will find RLS extremely useful. b) we can't fix these covert channels now, but plan to work on them in the future, and have ideas for how to approach them. Security community also concludes it is not avoidable nature as long as human can observe system behavior and estimate something, thus, security evaluation criteria does not require eliminate covert-channels or does not pay attention about covert-channels for the products that is installed on the environment with basic robustness (that means, non-military, regular enterprise usage). To be completely blunt, the security community does not understand databases. At all. I'd think if anything had become clear through the course of work on SEPosgres, it would be that. So, determinative questions: 1) are the security mechanisms supplied by this patch superior in some way to approaches like Veil for multi-tenant applications? (this is a serious question, as multi-tenant applications are far less concerned about covert channels) Yes. This RLS implementation targets the environment that does not have requirement for a particular bandwidth upperbound on covert- channels. It allows to centralize the place where we have to care about access control on main-channel, so it well fits multi-tenant applications. Again, please abandon your bandwidth arguments. They are irrelevant to whether or not this patch gets accepted. So, please try again? 3) will accepting this patch allow our users in the Government space to more freely adopt PostgreSQL? Government has two spaces. Most of their environment requires similar requirement as enterprise grade system doing. On the other hand, military environment often requires upper-bound of covert channel, as a story I introduce on upthread, but they are minority and have budget to develop special purpose database system designed for security with first priority. I don't think I understood this answer. What I'm asking is: will government agencies be sigificantly more likely to adopt PostgreSQL if we have RLS, in this form? Or do we need military-grade before it makes a difference? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- 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] [v9.4] row level security
2013/8/31 Stephen Frost sfr...@snowman.net: KaiGai, * Kohei KaiGai (kai...@kaigai.gr.jp) wrote: The point we shouldn't forget is information leakage via covert-channel has less grade of threat than leakage via main-channel, because of much less bandwidth. While true, this argument can't be used to simply ignore any and all covert channels. There are covert channels which are *not* much less bandwidth, and the Filtered Rows one is one of those- it's simply too big to ignore. There are likely other which are equally large and until we clean those up our RLS implementation will be questioned by our users. This does not mean that we need to clean up all covert channels and things which are clearly intractable don't need to be addressed (eg: the unique constraint situation; we can't both guarantee uniqueness and hide the existance of an entry). Security community also concludes it is not avoidable nature as long as human can observe system behavior and estimate something, This particular case is actually beyond 'estimation'. Even if attacker has enough knowledge, the ratio they can estimate hidden values is very limited because of much less bandwidth of covert channels. You really need to back away from this argument in this case. The example shown isn't based on estimation and provides quite high bandwidth because it can be run by a computer. This argument can't be used for every situation where information is leaked. However, in general, it is impossible to eliminate anything in spite of less degree of threats because of smaller bandwidth. So, I don't think this is an issue to spent much efforts to solve. I don't see a lot of complexity required to fix this specific case. A great deal of effort will be involved in going through the rest of the code and various options and working to eliminate other similar cases, but that's a reality we need to deal with. Hopefully the cases found will not require a lot of additional code to deal with. We will need to be mindful of new code which adds leaks or changes behavior such that RLS doesn't function properly (hopefully, sufficient regression tests will help to address that as well). You're saying that we need to fix up cover-channels with unignorable threat degree, even though it does not mean elimination of all the covert-channels. I almost agree and feel it a reasonable stance. One thing we need to pay attention is, how large covert-channel is we need to fix-up and how small is we can ignore. The reason why I used the term of estimation, even heuristic or machinery way, is this covert-channel does not expose the raw data on the display. I (personally) used this criteria to decide whether the covert-channel is ignorable, or not. Thus, I tackled leaky-function problem on interaction with views for security purpose. It seems to me a rough watermark, rather than my criteria above, is necessary to determine which covert-channel is hit for us. Anyway, I try to investigate the scenario that Korotkov pointed out. It should be a common problem for views with security_barrier attribute, not only RLS feature, because it is designed on a common infrastructure. 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] [v9.4] row level security
Josh, * Josh Berkus (j...@agliodbs.com) wrote: That's an astonishingly weak argument, because the bandwidth you're talking about is still very high, as in *hundreds* or *thousands* of values per minute. I agree that, in this specific case, we should do something about it. It's one thing to make the bandwidth argument for things like monitoring power draw, where the leakage is one character per hour, but the covert channels we're talking about are several orders of magnitude faster than that. That's actually not an accurate representation of the bandwidth associated with power draw measurements, but even if I convinced you it was in the hundreds or thousands of values per minute, I doubt you'd change your mind regarding the Filtered Rows issue or claim that we should fix the power draw measurement issue. :) So, I repeat: if you continue to pursue the argument that the covert channels identified aren't significant because of bandwidth, you will doom this patch. The valid arguments are only two: There is actually another argument, which I mentioned up-thread.. There are cases where you can't guarantee all the promises which can be asked for. In the unique-value case, we can't guarantee both that the values are unique and that existing values can't be detected by an individual with insert access. We should make the user aware that allowing insert access (or perhaps 'explain') opens up these cases. a) clearly identified use case X doesn't care about covert channels for reason Y, and will find RLS extremely useful. These certainly exist and I'd argue that's part of why Veil exists.. Users of Veil (which I admit, I'm not) would likely be much happier with an in-core solution because it will be much easier to use and much more performant. b) we can't fix these covert channels now, but plan to work on them in the future, and have ideas for how to approach them. I expect we will find more covert chanels which need to be fixed. To be completely blunt, the security community does not understand databases. At all. I'd think if anything had become clear through the course of work on SEPosgres, it would be that. That's just false and it's poor of you to claim it. The security community is not one individual nor even some small group. They're not very obviously involved with PostgreSQL but that's no reason to claim that they do not understand databases. I don't think I understood this answer. What I'm asking is: will government agencies be sigificantly more likely to adopt PostgreSQL if we have RLS, in this form? Or do we need military-grade before it makes a difference? From my experience, the answer would be 'yes' to your first question. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] [v9.4] row level security
2013/9/1 Josh Berkus j...@agliodbs.com: Kaigai, However, we have yet to talk about taking any such provisions with Postgres. If we commit this patch, arguably we'll have a row-level security feature which only protects data from well-behaved users, which seems counterproductive. The point we shouldn't forget is information leakage via covert-channel has less grade of threat than leakage via main-channel, because of much less bandwidth. That's an astonishingly weak argument, because the bandwidth you're talking about is still very high, as in *hundreds* or *thousands* of values per minute. It's one thing to make the bandwidth argument for things like monitoring power draw, where the leakage is one character per hour, but the covert channels we're talking about are several orders of magnitude faster than that. So, I repeat: if you continue to pursue the argument that the covert channels identified aren't significant because of bandwidth, you will doom this patch. The valid arguments are only two: a) clearly identified use case X doesn't care about covert channels for reason Y, and will find RLS extremely useful. b) we can't fix these covert channels now, but plan to work on them in the future, and have ideas for how to approach them. An important point is, what covert-channel should be closed, and what other covert-channel can be ignored. Even though the scenario Korotkov reported would be enough to make us surprised, we may need to consider how wide covert channel should be closed soon or later. * A covert-channel that can expose raw-data directly, should be fixed up. = Thus, leaky-view problem needed to be fixed. * A covert-channel that can expose existence of a particular value with less than Log(N) order trials to the possible range of hidden value. = Thus, this scenario needs to be cared? Or, any other criteria even though? My (current) preference is plan (c: we will be able to fix up *this* cover-channel with reasonable efforts on explain code. probably, we can close it if we don't print filtered rows under the sub-query with security-barrier attribute. It does not mean we shall or can fix up any possible covert-channels being found in the future, however, I also agree that we shall tackle covert-channel scenario being enough serious; that's criteria may or may not be bandwidth of information leakage. Security community also concludes it is not avoidable nature as long as human can observe system behavior and estimate something, thus, security evaluation criteria does not require eliminate covert-channels or does not pay attention about covert-channels for the products that is installed on the environment with basic robustness (that means, non-military, regular enterprise usage). To be completely blunt, the security community does not understand databases. At all. I'd think if anything had become clear through the course of work on SEPosgres, it would be that. So, determinative questions: 1) are the security mechanisms supplied by this patch superior in some way to approaches like Veil for multi-tenant applications? (this is a serious question, as multi-tenant applications are far less concerned about covert channels) Yes. This RLS implementation targets the environment that does not have requirement for a particular bandwidth upperbound on covert- channels. It allows to centralize the place where we have to care about access control on main-channel, so it well fits multi-tenant applications. Again, please abandon your bandwidth arguments. They are irrelevant to whether or not this patch gets accepted. So, please try again? 3) will accepting this patch allow our users in the Government space to more freely adopt PostgreSQL? Government has two spaces. Most of their environment requires similar requirement as enterprise grade system doing. On the other hand, military environment often requires upper-bound of covert channel, as a story I introduce on upthread, but they are minority and have budget to develop special purpose database system designed for security with first priority. I don't think I understood this answer. What I'm asking is: will government agencies be sigificantly more likely to adopt PostgreSQL if we have RLS, in this form? Or do we need military-grade before it makes a difference? Even though I'm not a marketer for public sector, not a small number of Oracle virtual private database, that provide more simple functionality, has been accepted for public sectors also, I believe it encourages adoption of PostgreSQL on government agencies. However, military division often requires special treatment for security functionality but very small number of users are interested in, thus, it is not a good idea to focus on this grade here. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:
Re: [HACKERS] [v9.4] row level security
On Sun, Sep 1, 2013 at 8:31 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: Or, any other criteria even though? My (current) preference is plan (c: we will be able to fix up *this* cover-channel with reasonable efforts on explain code. probably, we can close it if we don't print filtered rows under the sub-query with security-barrier attribute. I think the criteria being discussed in this thread are too strict. It may be the case that Postgres cannot make a strong general case that it protects against covert channels. However it may still be able to make the much weaker case that it is *possible* to arrange your database such that the covert channels are kept under control. So I think up above Tom is wrong about why it's ok that INSERT leaks keys when it reports a unique key violation. The reason why it's ok that there's a covert channel there is that the DBA can avoid that covert channel by being careful when creating unique constraints. He or she should be aware that creating a unique constraint implicitly provides a kind of limited access to data to users who have INSERT privilege even if they lack the real SELECT privilege. Likewise, as long as the covert channels in RLS are things the DBA has even a modicum of control over I wouldn't be too worried. Afaict from skimming the thread it looks like creating any indexes on columns leaks what values of the index key exist in the table. Is it the case that non-indexed columns do not get leaked? -- greg -- 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] [v9.4] row level security
On 9/1/13 5:54 PM, Greg Stark wrote: So I think up above Tom is wrong about why it's ok that INSERT leaks keys when it reports a unique key violation. The reason why it's ok that there's a covert channel there is that the DBA can avoid that covert channel by being careful when creating unique constraints. He or she should be aware that creating a unique constraint implicitly provides a kind of limited access to data to users who have INSERT privilege even if they lack the real SELECT privilege. And if someone can INSERT values that they can't actually see once they're committed, that's a similarly bad we should describe. People should be dumping their trash in their neighbor's yard. I think eventually this needs to be wrestled to the ground in a robust way. I want to see if all unique violations might be changed to give less information in this sort of RLS context. One rough early idea is to create a new error condition that means you hit something protected by RLS, but doesn't leak any more information than that. Just a generic Security restriction operation that comes out of fishing for keys, inserting outside your area, etc. I want to think through some use cases and review the code to see whether that concept helps or not. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com -- 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] [v9.4] row level security
KaiGai, * Kohei KaiGai (kai...@kaigai.gr.jp) wrote: The point we shouldn't forget is information leakage via covert-channel has less grade of threat than leakage via main-channel, because of much less bandwidth. While true, this argument can't be used to simply ignore any and all covert channels. There are covert channels which are *not* much less bandwidth, and the Filtered Rows one is one of those- it's simply too big to ignore. There are likely other which are equally large and until we clean those up our RLS implementation will be questioned by our users. This does not mean that we need to clean up all covert channels and things which are clearly intractable don't need to be addressed (eg: the unique constraint situation; we can't both guarantee uniqueness and hide the existance of an entry). Security community also concludes it is not avoidable nature as long as human can observe system behavior and estimate something, This particular case is actually beyond 'estimation'. Even if attacker has enough knowledge, the ratio they can estimate hidden values is very limited because of much less bandwidth of covert channels. You really need to back away from this argument in this case. The example shown isn't based on estimation and provides quite high bandwidth because it can be run by a computer. This argument can't be used for every situation where information is leaked. However, in general, it is impossible to eliminate anything in spite of less degree of threats because of smaller bandwidth. So, I don't think this is an issue to spent much efforts to solve. I don't see a lot of complexity required to fix this specific case. A great deal of effort will be involved in going through the rest of the code and various options and working to eliminate other similar cases, but that's a reality we need to deal with. Hopefully the cases found will not require a lot of additional code to deal with. We will need to be mindful of new code which adds leaks or changes behavior such that RLS doesn't function properly (hopefully, sufficient regression tests will help to address that as well). Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] [v9.4] row level security
2013/8/29 Josh Berkus j...@agliodbs.com: Kaigai, Although I didn't touch this task by myself, my senior colleague said that we calculated some possible bandwidth of leakage as an evident when we ship supercomputer system with MAC feature for customer who worry about security. I'm not sure how to measure it. However, if we assume a human can run up to 5 query per seconds, he needs 2-3 seconds to identify a particular integer value less than 1, it means bandwidth of this covert channel is less than 5bps. I'm not sure whether enterprise-grade dbms has to care about these amount of covert channel. Why are you assuming a human needs to do it? Given the explain vector, I could write a rather simple python or perl script which would find values by EXPLAIN leakage, at 1000 explain plans per minute. It's one thing to day we can't solve this covert channel issue right now in this patch, but saying we don't plan to solve it at all is likely to doom the patch. I'm not sure what the solution would be, exactly. Deny permission for EXPLAIN on certain tables? Surely someone in the security community has discussed this? Security community considers covert channel is a hard to solve problem; nearly impossible to eliminate. Let's see the summary in wikipedia: http://en.wikipedia.org/wiki/Covert_channel It does not require countermeasure of covert channels in middle or entry class security evaluation; that is usually required for enterprise grade, even though it is required for the product being designed for military grade. The reason why its priority is relatively lower, is that degree of threats with information leakage via covert channel has limited bandwidth in comparison to main channel. I also follow this standpoint; that is enough reasonable between functionality and its strictness under limited resources. Even if we could close a certain channel, we never can all other channels, like a signal by namespace contention on table creation as covert channel. Also, I don't know major commercial dbms handles these scenario well. Of course, it should be described in the document for users not to apply these security features onto the region that overs our capability. 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] [v9.4] row level security
2013/8/29 David Fetter da...@fetter.org: On Thu, Aug 29, 2013 at 10:05:14AM -0400, Tom Lane wrote: Alexander Korotkov aekorot...@gmail.com writes: On Wed, Aug 28, 2013 at 4:17 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: It is out of scope for this feature. We usually calls this type of information leakage covert channel; that is not avoidable in principle. I think there is another covert channel much more serious than constrains. You can gather information about hidden data by reading query plans. I'm not convinced by this argument that covert channels are out of scope. That would be a fine justification for, say, a thesis topic. However, what we're talking about here is a real-world feature that will be of no real-world use if it can't stand up against rather obvious attack techniques. I'm not interested in carrying the maintenance and runtime overhead of a feature that's only of academic value. Looking at the real-world perspective, what covert channels do our competitors in the space currently claim to do anything about? I'm not sure whether minor dbms that is designed for extreme secure environment already got certified. (If they have such functionality, they should take certification for promotion.) Oracle lists some of their certified products: http://www.oracle.com/technetwork/topics/security/security-evaluations-099357.html However, these are based on protection profile for basic robustness that is designed for environment where we don't care about covert channel. This would represent the bar we need to clear at least as far as documenting what we do (do the access constraint before anything else, e.g.) or why we don't do things (disabling EXPLAIN, e.g.). +1. I'd like to add description about this scenario. 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] [v9.4] row level security
2013/8/29 Tom Lane t...@sss.pgh.pa.us: Josh Berkus j...@agliodbs.com writes: That would close only one covert channel. Others were already pointed out upthread, and I'll bet there are more ... Mind you, fundamentally this is no different from allowing INSERT permission on a table but denying SELECT, or denying SELECT on certain columns. In either case, covert channels for some data are available. Certainly. But INSERT's purpose in life is not to prevent people from inferring what data is in the table. What we have to ask here is whether a row level security feature that doesn't deal with these real-world attack techniques is worth having. I think, we should clearly note that row-level security feature does not have capability to control information leakage via covert channel but very limited bandwidth, even though it control information leakage and manipulation via main channel. It depends on user's environment and expectation. If they need rdbms with security feature for military grade, it is not recommendable. However, it is a recommended solution for regular enterprise grade environment. Anything depends on user's environment, threats and worth of values to be protected. 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] [v9.4] row level security
On 08/30/2013 03:05 AM, Kohei KaiGai wrote: Surely someone in the security community has discussed this? Security community considers covert channel is a hard to solve problem; nearly impossible to eliminate. Let's see the summary in wikipedia: http://en.wikipedia.org/wiki/Covert_channel Well, in each of the cases covered in that article, the given technology (OSI, TCP, etc.) takes specific provisions to limit the ability of attackers to discover information via the covert channel. However, we have yet to talk about taking any such provisions with Postgres. If we commit this patch, arguably we'll have a row-level security feature which only protects data from well-behaved users, which seems counterproductive. So, arguments in favor of this patch: a) it's as good as Oracle's security features, giving us check-box compliance. b) it allows securing individual rows against attackers with limited technical knowledge or limited database access, and could be very hardened in combination with intelligent access control. c) it is an improvement on techniques like Veil (is it)? d) we plan to continue improving it and closing covert channels, or limiting their bandwidth. Arguments against: m) covert channels are currently broad enough to make it trivially circumventable (are they?) n) overhead and code maintenance required is substantial So, determinative questions: 1) are the security mechanisms supplied by this patch superior in some way to approaches like Veil for multi-tenant applications? (this is a serious question, as multi-tenant applications are far less concerned about covert channels) 2) do we plan to reduce the accessibility of data via covert channels over successive releases? How? 3) will accepting this patch allow our users in the Government space to more freely adopt PostgreSQL? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- 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] [v9.4] row level security
Josh, * Josh Berkus (j...@agliodbs.com) wrote: On 08/30/2013 03:05 AM, Kohei KaiGai wrote: Security community considers covert channel is a hard to solve problem; nearly impossible to eliminate. While impossible to eliminate, we should certainly consider cases like this where we can do better and fix them. RLS certainly brings another level of consideration to the overall PG security environment by requiring we think about security on a row level rather than just a table or column level. We have issues with covert channels even without RLS though and holding up RLS because it doesn't fix all the covert channels isn't sensible. Column-level privleges have a similar problem, where you can read the default value for a column using the catalogs. Perhaps the default isn't sensetive (you'd certainly hope not), but it's still an issue. It wouldn't surprise me to find that there are ways to abuse a multi-column index which includes both a column you can manipulate and one you don't have access to read to determine something about the hidden column (maybe you have access to the 2nd field in the index and you can encourage an in-order index traversal and then look at filtered rows, or just work out a way to do timing attacks to determine the btree depth). Well, in each of the cases covered in that article, the given technology (OSI, TCP, etc.) takes specific provisions to limit the ability of attackers to discover information via the covert channel. The work we've done around secure views would lend credit to our attempts at taking specific provisions as well; sadly, PG is slightly more complicated than TCP. We do what we can and we've got a great community which will point out where we can do better- and we work on it and improve over time. Hell, when roles were first added we had a *massive* security hole because we didn't check to make sure we weren't overrunning the length of the GUC. It was a mistake and we should have done better, but that doesn't mean adding roles was the wrong decision. However, we have yet to talk about taking any such provisions with Postgres. If we commit this patch, arguably we'll have a row-level security feature which only protects data from well-behaved users, which seems counterproductive. I would argue both that we *have* been taking provisions to avoid obvious and big covert channels, and that this patch adds value even if it doesn't protect the system perfectly from malicious users. We're all certainly aware of the ability for an attacker to cause major problems to a PG system if they can issue arbitrary SQL and our permissions system doesn't do much to protect us. A single query which doesn't require any privileges could cause havok on the system (massive on-disk temp file, which could be shared with pg_xlog causing the system to PANIC, massive CPU load if they can execute multiple commands in parallel...). Not to mention the default installation of pl/pgsql and anonymous functions. I could see many a web app (things like LedgerSMB) which could benefit from having more fine-grained in-database control because they already authenticate to the database as the user and have a static or at least controlled set of queries which they run. Today, any of those kinds of systems have to implement their own RLS (though sometimes it's done through independent tables for each customer or similar, rather than as conditionals added to queries). a) it's as good as Oracle's security features, giving us check-box compliance. I'd argue that this is definitely much more than 'check-box' compliance. b) it allows securing individual rows against attackers with limited technical knowledge or limited database access, and could be very hardened in combination with intelligent access control. c) it is an improvement on techniques like Veil (is it)? d) we plan to continue improving it and closing covert channels, or limiting their bandwidth. Arguments against: m) covert channels are currently broad enough to make it trivially circumventable (are they?) There are some which are and likely deserve to be fixed. Do they all need to be addressed before this patch goes in? I'd argue 'no'. n) overhead and code maintenance required is substantial So, determinative questions: 1) are the security mechanisms supplied by this patch superior in some way to approaches like Veil for multi-tenant applications? (this is a serious question, as multi-tenant applications are far less concerned about covert channels) I'd argue 'yes' if just for the fact that it'd be simpler and easier to use, both because it's in core and because you're using an actual grammar instead of function calls- but this RLS does more than just that, it's going to cause us to improve things that Veil probably can't fix and simply ignores today. 2) do we plan to reduce the accessibility of data via covert channels over successive releases? How? By discovering them and fixing them as we
Re: [HACKERS] [v9.4] row level security
Stephen Frost sfr...@snowman.net writes: We have issues with covert channels even without RLS though and holding up RLS because it doesn't fix all the covert channels isn't sensible. I think it's entirely sensible to question whether we should reject (not hold up) RLS if it has major covert-channel problems. The scenario I'm worried about is where somebody says hey, Postgres has RLS now, I can rely on that to hide my sooper sekrit data from other users in the same database, and later they have a security breach through some covert-channel attack. Are they going to blame themselves? No, they're gonna blame Postgres. Or consider the case where some bozo publishes a method for such an attack and uses it to badmouth us as insecure. I don't think we need the headaches that will result from promising (or at least appearing to promise) something we can't deliver. Nor am I convinced that we're really doing users any favors by providing such a feature. They'd be *far* better advised to put their critical data in a separate database. In short, we can check some check-box is a really, really bad reason to accept a security-related feature. If we're going to put up with all the downsides of RLS, I want the end result to be something that's actually secure, not something that gives the illusion of security. And right now, I do not believe we can get past the illusion stage, ever (certainly not in a release or two). 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] [v9.4] row level security
On 08/30/2013 12:43 PM, Tom Lane wrote: In short, we can check some check-box is a really, really bad reason to accept a security-related feature. If we're going to put up with all the downsides of RLS, I want the end result to be something that's actually secure, not something that gives the illusion of security. And right now, I do not believe we can get past the illusion stage, ever (certainly not in a release or two). Can you be more explicit about all the downsides of RLS? I was just looking over the patch, which is less than 5000 lines. While it's not small, we have larger patches in the CF. So what's the specific downsides of this? The reason I brought up multi-tenant applications (MTA), BTW, is that this is the other major potential utility of RLS, and for such users the covert channel limitations are acceptable (as long as we publish them). That is, for a thing-as-a-service application, users are not assumed to have unlimited access to the SQL command line anyway; RLS is employed just to limit the damage they can do if they get access, and to limit the disclosure if some app programmer makes a mistake. Right now, the primary tool for doing row filtering for MTA is Veil, which has numerous and well-known limitations. If RLS has fewer limitations, or is easier to deploy, maintain, and/or understand, then it's a valuable feature for that user base, even if it doesn't address the covert channels we've brought up at all. That is, if RLS is your *second* level of defense, instead of your primary defense, covert channels are not a make-or-break issue. It just has to be better than what we had before. Note that I have NOT done an evaluation of Veil vs. RLS for MTA at this point. I'm hoping someone else will ;-) -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- 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] [v9.4] row level security
Josh Berkus j...@agliodbs.com writes: On 08/30/2013 12:43 PM, Tom Lane wrote: In short, we can check some check-box is a really, really bad reason to accept a security-related feature. If we're going to put up with all the downsides of RLS, I want the end result to be something that's actually secure, not something that gives the illusion of security. Can you be more explicit about all the downsides of RLS? I was just looking over the patch, which is less than 5000 lines. While it's not small, we have larger patches in the CF. So what's the specific downsides of this? I think it's going to be an ongoing maintenance headache and an endless source of security bugs, even disregarding covert-channel issues. I have pretty much zero faith in the planner changes, in particular, and would still not have a lot if they were adequately documented, which they absolutely are not. The whole thing depends on nowhere-clearly-stated assumptions that plan-time transformations will never allow an RLS check to be bypassed. I foresee future planner work breaking this in non-obvious ways on a regular basis (even granting the assumption that it's bulletproof today, which is at best unproven). The reason I brought up multi-tenant applications (MTA), BTW, is that this is the other major potential utility of RLS, and for such users the covert channel limitations are acceptable (as long as we publish them). [ shrug... ] You might've noticed I work for a multi-tenant shop now. I'm still not excited about this. That is, if RLS is your *second* level of defense, instead of your primary defense, covert channels are not a make-or-break issue. It just has to be better than what we had before. Yeah, that's a fair point. I'm just not convinced that it's enough better to justify the maintenance burden we'll be taking on. I'm not thrilled about the every bug is a security bug angle, either. 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] [v9.4] row level security
* Tom Lane (t...@sss.pgh.pa.us) wrote: Stephen Frost sfr...@snowman.net writes: We have issues with covert channels even without RLS though and holding up RLS because it doesn't fix all the covert channels isn't sensible. I think it's entirely sensible to question whether we should reject (not hold up) RLS if it has major covert-channel problems. Rejecting RLS because we've suddently realized that covert channels exist is foolishness. It's akin to rejecting the ability to add stored procedures because we don't protect prosrc from people who don't own or can't execute the function. The scenario I'm worried about is where somebody says hey, Postgres has RLS now, I can rely on that to hide my sooper sekrit data from other users in the same database, and later they have a security breach through some covert-channel attack. Are they going to blame themselves? No, they're gonna blame Postgres. Or consider the case where some bozo publishes a method for such an attack and uses it to badmouth us as insecure. In my experience, issues are discovered, patched, and released as security updates. Does adding RLS mean we might have more of those? Sure, as did column level privileges and as does *any* such increase in the granularity of what we can provide security-wise. I don't think we need the headaches that will result from promising (or at least appearing to promise) something we can't deliver. Nor am I convinced that we're really doing users any favors by providing such a feature. They'd be *far* better advised to put their critical data in a separate database. We've barely got cross-database queries with FDWs. The notion that adding such complexity into those as RLS, which each individual user will need to figure out how to do themselves and most will likely get far wrong and much worse than what we'd implement, is better for our users is just ridiculous. In short, we can check some check-box is a really, really bad reason to accept a security-related feature. If we're going to put up with all the downsides of RLS, I want the end result to be something that's actually secure, not something that gives the illusion of security. And right now, I do not believe we can get past the illusion stage, ever (certainly not in a release or two). I'm not argueing for this because it fulfills some check-box; the question about if it would help a given set of clients (ones which I no longer have any direct relationship with, as it turns out) adopt PG was asked and I answered it as best I could. I certainly think we need to get past the 'illusion' stage also. I'm certainly more optimistic about that than you are but I also understand it's not going to be perfect in the first release- but I do think it'll be better than the 'illusion' stage. It'll get there because we'll continue to discuss it, people will test it, etc; as one hopes happens with all new features, but this even more than others. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] [v9.4] row level security
2013/8/30 Josh Berkus j...@agliodbs.com: On 08/30/2013 03:05 AM, Kohei KaiGai wrote: Surely someone in the security community has discussed this? Security community considers covert channel is a hard to solve problem; nearly impossible to eliminate. Let's see the summary in wikipedia: http://en.wikipedia.org/wiki/Covert_channel Well, in each of the cases covered in that article, the given technology (OSI, TCP, etc.) takes specific provisions to limit the ability of attackers to discover information via the covert channel. However, we have yet to talk about taking any such provisions with Postgres. If we commit this patch, arguably we'll have a row-level security feature which only protects data from well-behaved users, which seems counterproductive. The point we shouldn't forget is information leakage via covert-channel has less grade of threat than leakage via main-channel, because of much less bandwidth. Security community also concludes it is not avoidable nature as long as human can observe system behavior and estimate something, thus, security evaluation criteria does not require eliminate covert-channels or does not pay attention about covert-channels for the products that is installed on the environment with basic robustness (that means, non-military, regular enterprise usage). I don't think PostgreSQL goes into military-grade secure database system. If so, it has to sacrifice many thing (like, performance, usability, nature of open source, ...) for security. It's not a fact. So, arguments in favor of this patch: a) it's as good as Oracle's security features, giving us check-box compliance. b) it allows securing individual rows against attackers with limited technical knowledge or limited database access, and could be very hardened in combination with intelligent access control. Even if attacker has enough knowledge, the ratio they can estimate hidden values is very limited because of much less bandwidth of covert channels. c) it is an improvement on techniques like Veil (is it)? d) we plan to continue improving it and closing covert channels, or limiting their bandwidth. Arguments against: m) covert channels are currently broad enough to make it trivially circumventable (are they?) n) overhead and code maintenance required is substantial So, determinative questions: 1) are the security mechanisms supplied by this patch superior in some way to approaches like Veil for multi-tenant applications? (this is a serious question, as multi-tenant applications are far less concerned about covert channels) Yes. This RLS implementation targets the environment that does not have requirement for a particular bandwidth upperbound on covert- channels. It allows to centralize the place where we have to care about access control on main-channel, so it well fits multi-tenant applications. 2) do we plan to reduce the accessibility of data via covert channels over successive releases? How? Less covert channels is better than massive, if we could close it with reasonable cost; that means run-time performance, code complexity and so on. However, in general, it is impossible to eliminate anything in spite of less degree of threats because of smaller bandwidth. So, I don't think this is an issue to spent much efforts to solve. 3) will accepting this patch allow our users in the Government space to more freely adopt PostgreSQL? Government has two spaces. Most of their environment requires similar requirement as enterprise grade system doing. On the other hand, military environment often requires upper-bound of covert channel, as a story I introduce on upthread, but they are minority and have budget to develop special purpose database system designed for security with first priority. 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] [v9.4] row level security
2013/8/30 Tom Lane t...@sss.pgh.pa.us: Josh Berkus j...@agliodbs.com writes: On 08/30/2013 12:43 PM, Tom Lane wrote: In short, we can check some check-box is a really, really bad reason to accept a security-related feature. If we're going to put up with all the downsides of RLS, I want the end result to be something that's actually secure, not something that gives the illusion of security. Can you be more explicit about all the downsides of RLS? I was just looking over the patch, which is less than 5000 lines. While it's not small, we have larger patches in the CF. So what's the specific downsides of this? I think it's going to be an ongoing maintenance headache and an endless source of security bugs, even disregarding covert-channel issues. I have pretty much zero faith in the planner changes, in particular, and would still not have a lot if they were adequately documented, which they absolutely are not. The whole thing depends on nowhere-clearly-stated assumptions that plan-time transformations will never allow an RLS check to be bypassed. I foresee future planner work breaking this in non-obvious ways on a regular basis (even granting the assumption that it's bulletproof today, which is at best unproven). In general, we will adopt / enhance features as long as PostgreSQL runs with evolution. It can never be free from bugs or maintenance, regardless of its nature. Later half seems to me a bit unfair because any features may conflict with some future works, not only RLS. Also, if you have some tangible planner enhancement plan, could you inform us which plan shall be in progress? At least, it is impossible to adjust my implementation because of abstract concern towards future conflict. 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] [v9.4] row level security
Any constraints could be covert channel. On Wed, Aug 28, 2013 at 4:17 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: 2013/8/28 Oleg Bartunov obartu...@gmail.com: btw, there is serious problem with row-level security and constraints. For example, user with low security level could use unique constraint to know about existence of a row with higher security. I don't know, what is the best practice to avoid this. It is out of scope for this feature. We usually calls this type of information leakage covert channel; that is not avoidable in principle. However, its significance is minor, because attacker must know identical data to be here, or must have proving for each possible values. Its solution is simple. DBA should not use value to be confidential as unique key. If needed, our recommendation is alternative key, instead of natural key, because its value itself does not have worth. I should add a note of caution onto the documentation according to the previous consensus, however, I noticed it had gone from the sgml files while I was unaware. So, let me add description on the documentation. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp
Re: [HACKERS] [v9.4] row level security
On Wed, Aug 28, 2013 at 4:17 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: 2013/8/28 Oleg Bartunov obartu...@gmail.com: btw, there is serious problem with row-level security and constraints. For example, user with low security level could use unique constraint to know about existence of a row with higher security. I don't know, what is the best practice to avoid this. It is out of scope for this feature. We usually calls this type of information leakage covert channel; that is not avoidable in principle. However, its significance is minor, because attacker must know identical data to be here, or must have proving for each possible values. Its solution is simple. DBA should not use value to be confidential as unique key. If needed, our recommendation is alternative key, instead of natural key, because its value itself does not have worth. I should add a note of caution onto the documentation according to the previous consensus, however, I noticed it had gone from the sgml files while I was unaware. So, let me add description on the documentation. I think there is another covert channel much more serious than constrains. You can gather information about hidden data by reading query plans. CREATE TABLE test (id SERIAL PRIMARY KEY, value INTEGER); INSERT INTO test (value) VALUES (1234), (4321), (2356), (6542); ALTER TABLE test SET ROW SECURITY FOR ALL TO (id % 2 = 1); CREATE INDEX test_idx ON test (value); User sees only 1 and 3 rows: postgres= select * from test; id | value +--- 1 | 1234 3 | 2356 (2 rows) But user can probe values column using explain command. postgres= set enable_seqscan = off; SET postgres= explain analyze select * from test where value between and ; QUERY PLAN --- Index Scan using test_idx on test (cost=0.13..8.16 rows=1 width=8) (actual time=0.021..0.024 rows=2 loops=1) Index Cond: ((value = ) AND (value = )) Filter: ((id % 2) = 1) Total runtime: 0.056 ms (4 rows) postgres= explain analyze select * from test where value between and ; QUERY PLAN --- Index Scan using test_idx on test (cost=0.13..8.16 rows=1 width=8) (actual time=0.020..0.024 rows=2 loops=1) Index Cond: ((value = ) AND (value = )) Filter: ((id % 2) = 1) Rows Removed by Filter: 1 Total runtime: 0.057 ms (5 rows) In given example user can realize that there is a hidden value in index between 3334 and . Using dichotomy he can find exact value. I didn't find if there was discussion about it. This example is only my first idea about using plans for probing hidden values. Probably, there are some other ways to do it. I don't think we can say that indexed data is not sensitive for leakage. Prohibiting push down of all operators which could be used for such kind of attacks also doesn't seem acceptable for me because of huge impact to performance. Another option I see is to hide some sensitive parts of plan from unprivileged user. There is still a room for timing attack, but it doesn't seem to be feasible in practice to apply. -- With best regards, Alexander Korotkov.
Re: [HACKERS] [v9.4] row level security
Alexander Korotkov aekorot...@gmail.com writes: On Wed, Aug 28, 2013 at 4:17 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: It is out of scope for this feature. We usually calls this type of information leakage covert channel; that is not avoidable in principle. I think there is another covert channel much more serious than constrains. You can gather information about hidden data by reading query plans. I'm not convinced by this argument that covert channels are out of scope. That would be a fine justification for, say, a thesis topic. However, what we're talking about here is a real-world feature that will be of no real-world use if it can't stand up against rather obvious attack techniques. I'm not interested in carrying the maintenance and runtime overhead of a feature that's only of academic value. 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] [v9.4] row level security
2013/8/29 Alexander Korotkov aekorot...@gmail.com: On Wed, Aug 28, 2013 at 4:17 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: 2013/8/28 Oleg Bartunov obartu...@gmail.com: btw, there is serious problem with row-level security and constraints. For example, user with low security level could use unique constraint to know about existence of a row with higher security. I don't know, what is the best practice to avoid this. It is out of scope for this feature. We usually calls this type of information leakage covert channel; that is not avoidable in principle. However, its significance is minor, because attacker must know identical data to be here, or must have proving for each possible values. Its solution is simple. DBA should not use value to be confidential as unique key. If needed, our recommendation is alternative key, instead of natural key, because its value itself does not have worth. I should add a note of caution onto the documentation according to the previous consensus, however, I noticed it had gone from the sgml files while I was unaware. So, let me add description on the documentation. I think there is another covert channel much more serious than constrains. You can gather information about hidden data by reading query plans. CREATE TABLE test (id SERIAL PRIMARY KEY, value INTEGER); INSERT INTO test (value) VALUES (1234), (4321), (2356), (6542); ALTER TABLE test SET ROW SECURITY FOR ALL TO (id % 2 = 1); CREATE INDEX test_idx ON test (value); User sees only 1 and 3 rows: postgres= select * from test; id | value +--- 1 | 1234 3 | 2356 (2 rows) But user can probe values column using explain command. postgres= set enable_seqscan = off; SET postgres= explain analyze select * from test where value between and ; QUERY PLAN --- Index Scan using test_idx on test (cost=0.13..8.16 rows=1 width=8) (actual time=0.021..0.024 rows=2 loops=1) Index Cond: ((value = ) AND (value = )) Filter: ((id % 2) = 1) Total runtime: 0.056 ms (4 rows) postgres= explain analyze select * from test where value between and ; QUERY PLAN --- Index Scan using test_idx on test (cost=0.13..8.16 rows=1 width=8) (actual time=0.020..0.024 rows=2 loops=1) Index Cond: ((value = ) AND (value = )) Filter: ((id % 2) = 1) Rows Removed by Filter: 1 Total runtime: 0.057 ms (5 rows) In given example user can realize that there is a hidden value in index between 3334 and . Using dichotomy he can find exact value. I didn't find if there was discussion about it. This example is only my first idea about using plans for probing hidden values. Probably, there are some other ways to do it. Hmm. It is a new scenario for me, even though its basis is similar to the scenario that abuses constraints because it still does not leak the hidden value itself and it requires estimation by human. If we tackle this issue, an easy solution is to hide rows removed by filter output if plan is underlying sub-query plan that was constructed by row-level security feature. I don't think we can say that indexed data is not sensitive for leakage. Prohibiting push down of all operators which could be used for such kind of attacks also doesn't seem acceptable for me because of huge impact to performance. Another option I see is to hide some sensitive parts of plan from unprivileged user. There is still a room for timing attack, but it doesn't seem to be feasible in practice to apply. A principle of this row-level security feature is, it prohibits to leak invisible datum itself, but might allow users to expect existence of records with a particular value. In fact, we never push down function that may leak the given argument, that does not have leakproof attribute, even if it can be utilized for index-scan. My opinion is, we should deal with it is a limitation of this feature, as long as it does not expose the raw data to be hidden. Estimation takes time to carry out much hidden data via covert channel, thus traditional secure operating system specification with MAC implementation says its degree of threat is not significant as long as bandwidth of covert channel is not so much. I think it is a reasonable standpoint. 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] [v9.4] row level security
On Thu, Aug 29, 2013 at 04:14:53PM +0200, Kohei KaiGai wrote: 2013/8/29 Alexander Korotkov aekorot...@gmail.com: On Wed, Aug 28, 2013 at 4:17 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: 2013/8/28 Oleg Bartunov obartu...@gmail.com: btw, there is serious problem with row-level security and constraints. For example, user with low security level could use unique constraint to know about existence of a row with higher security. I don't know, what is the best practice to avoid this. ... A principle of this row-level security feature is, it prohibits to leak invisible datum itself, but might allow users to expect existence of records with a particular value. In fact, we never push down function that may leak the given argument, that does not have leakproof attribute, even if it can be utilized for index-scan. My opinion is, we should deal with it is a limitation of this feature, as long as it does not expose the raw data to be hidden. Estimation takes time to carry out much hidden data via covert channel, thus traditional secure operating system specification with MAC implementation says its degree of threat is not significant as long as bandwidth of covert channel is not so much. I think it is a reasonable standpoint. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp Okay, given that argument, how would you monitor such attempts to access data through the covert channel and shut it down? Regards, Ken -- 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] [v9.4] row level security
2013/8/29 k...@rice.edu k...@rice.edu: On Thu, Aug 29, 2013 at 04:14:53PM +0200, Kohei KaiGai wrote: 2013/8/29 Alexander Korotkov aekorot...@gmail.com: On Wed, Aug 28, 2013 at 4:17 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: 2013/8/28 Oleg Bartunov obartu...@gmail.com: btw, there is serious problem with row-level security and constraints. For example, user with low security level could use unique constraint to know about existence of a row with higher security. I don't know, what is the best practice to avoid this. ... A principle of this row-level security feature is, it prohibits to leak invisible datum itself, but might allow users to expect existence of records with a particular value. In fact, we never push down function that may leak the given argument, that does not have leakproof attribute, even if it can be utilized for index-scan. My opinion is, we should deal with it is a limitation of this feature, as long as it does not expose the raw data to be hidden. Estimation takes time to carry out much hidden data via covert channel, thus traditional secure operating system specification with MAC implementation says its degree of threat is not significant as long as bandwidth of covert channel is not so much. I think it is a reasonable standpoint. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp Okay, given that argument, how would you monitor such attempts to access data through the covert channel and shut it down? Although I didn't touch this task by myself, my senior colleague said that we calculated some possible bandwidth of leakage as an evident when we ship supercomputer system with MAC feature for customer who worry about security. I'm not sure how to measure it. However, if we assume a human can run up to 5 query per seconds, he needs 2-3 seconds to identify a particular integer value less than 1, it means bandwidth of this covert channel is less than 5bps. I'm not sure whether enterprise-grade dbms has to care about these amount of covert channel. 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] [v9.4] row level security
Kaigai, Although I didn't touch this task by myself, my senior colleague said that we calculated some possible bandwidth of leakage as an evident when we ship supercomputer system with MAC feature for customer who worry about security. I'm not sure how to measure it. However, if we assume a human can run up to 5 query per seconds, he needs 2-3 seconds to identify a particular integer value less than 1, it means bandwidth of this covert channel is less than 5bps. I'm not sure whether enterprise-grade dbms has to care about these amount of covert channel. Why are you assuming a human needs to do it? Given the explain vector, I could write a rather simple python or perl script which would find values by EXPLAIN leakage, at 1000 explain plans per minute. It's one thing to day we can't solve this covert channel issue right now in this patch, but saying we don't plan to solve it at all is likely to doom the patch. I'm not sure what the solution would be, exactly. Deny permission for EXPLAIN on certain tables? Surely someone in the security community has discussed this? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- 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] [v9.4] row level security
* Kohei KaiGai (kai...@kaigai.gr.jp) wrote: Although I didn't touch this task by myself, my senior colleague said that we calculated some possible bandwidth of leakage as an evident when we ship supercomputer system with MAC feature for customer who worry about security. I'm not sure how to measure it. However, if we assume a human can run up to 5 query per seconds, he needs 2-3 seconds to identify a particular integer value less than 1, it means bandwidth of this covert channel is less than 5bps. I'm not sure whether enterprise-grade dbms has to care about these amount of covert channel. A human isn't necessary in this particular scenario- you're doing a simple binary search through the space, which computers can be awful good at. Using the type's bounds (eg: an 'integer' field is only 32bits) and forcing index scans, you could tell how many records exist and then break down how many exist in the first half, second half, and then split those.. Eventually, you could work out every value in the column. That could be applied to variable length values also, though it'd be more costly to get down to the exact values. I don't have a solution to these issues offhand except to suggest that, in such an environment, having a don't allow these users to run EXPLAIN and similar options would probably be welcome. Even then there are potential timing attacks, but that certainly increases the level of effort involved. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] [v9.4] row level security
Josh Berkus j...@agliodbs.com writes: It's one thing to day we can't solve this covert channel issue right now in this patch, but saying we don't plan to solve it at all is likely to doom the patch. I'm not sure what the solution would be, exactly. Deny permission for EXPLAIN on certain tables? That would close only one covert channel. Others were already pointed out upthread, and I'll bet there are more ... 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] [v9.4] row level security
I'm not sure what the solution would be, exactly. Deny permission for EXPLAIN on certain tables? That would close only one covert channel. Others were already pointed out upthread, and I'll bet there are more ... Mind you, fundamentally this is no different from allowing INSERT permission on a table but denying SELECT, or denying SELECT on certain columns. In either case, covert channels for some data are available. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- 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] [v9.4] row level security
Josh Berkus j...@agliodbs.com writes: That would close only one covert channel. Others were already pointed out upthread, and I'll bet there are more ... Mind you, fundamentally this is no different from allowing INSERT permission on a table but denying SELECT, or denying SELECT on certain columns. In either case, covert channels for some data are available. Certainly. But INSERT's purpose in life is not to prevent people from inferring what data is in the table. What we have to ask here is whether a row level security feature that doesn't deal with these real-world attack techniques is worth having. 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] [v9.4] row level security
On Thu, Aug 29, 2013 at 10:05:14AM -0400, Tom Lane wrote: Alexander Korotkov aekorot...@gmail.com writes: On Wed, Aug 28, 2013 at 4:17 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: It is out of scope for this feature. We usually calls this type of information leakage covert channel; that is not avoidable in principle. I think there is another covert channel much more serious than constrains. You can gather information about hidden data by reading query plans. I'm not convinced by this argument that covert channels are out of scope. That would be a fine justification for, say, a thesis topic. However, what we're talking about here is a real-world feature that will be of no real-world use if it can't stand up against rather obvious attack techniques. I'm not interested in carrying the maintenance and runtime overhead of a feature that's only of academic value. Looking at the real-world perspective, what covert channels do our competitors in the space currently claim to do anything about? This would represent the bar we need to clear at least as far as documenting what we do (do the access constraint before anything else, e.g.) or why we don't do things (disabling EXPLAIN, e.g.). 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] [v9.4] row level security
btw, there is serious problem with row-level security and constraints. For example, user with low security level could use unique constraint to know about existence of a row with higher security. I don't know, what is the best practice to avoid this. On Wed, Aug 28, 2013 at 1:37 AM, Greg Smith g...@2ndquadrant.com wrote: On 7/20/13 10:08 AM, Kohei KaiGai wrote: Hmm. I didn't have this idea. It seems to me fair enough and kills necessity to enhance RangeTblEntry and getrelid() indeed. I try to fix up this implementation according to your suggestion. How is that going? I'm going to do a serious review of this myself over the next few weeks. I have a good chunk of time set aside for it as part of a larger project. I'm hoping to get more people here involved in that effort too, starting in the November CF if that works out. I've been trying to catch up with your larger plan for this feature for 9.4. You made this comment earlier: 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 Is any of that code around yet? I see that you have split your submissions so that a smaller program can be reviewed today. I'd like to start taking a look at the next step too though. For the project I'm starting to work on here, getting the integration with labeling also done is a very important thing to target for 9.4. It would be nice to see how that fits together today, even if the code for it isn't being reviewed heavily yet. I don't quite understand yet what's missing on the writer side. If you could help explain what's missing there, I would like to read about that. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/**mailpref/pgsql-hackershttp://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.4] row level security
2013/8/28 Oleg Bartunov obartu...@gmail.com: btw, there is serious problem with row-level security and constraints. For example, user with low security level could use unique constraint to know about existence of a row with higher security. I don't know, what is the best practice to avoid this. It is out of scope for this feature. We usually calls this type of information leakage covert channel; that is not avoidable in principle. However, its significance is minor, because attacker must know identical data to be here, or must have proving for each possible values. Its solution is simple. DBA should not use value to be confidential as unique key. If needed, our recommendation is alternative key, instead of natural key, because its value itself does not have worth. I should add a note of caution onto the documentation according to the previous consensus, however, I noticed it had gone from the sgml files while I was unaware. So, let me add description on 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] [v9.4] row level security
On 7/20/13 10:08 AM, Kohei KaiGai wrote: Hmm. I didn't have this idea. It seems to me fair enough and kills necessity to enhance RangeTblEntry and getrelid() indeed. I try to fix up this implementation according to your suggestion. How is that going? I'm going to do a serious review of this myself over the next few weeks. I have a good chunk of time set aside for it as part of a larger project. I'm hoping to get more people here involved in that effort too, starting in the November CF if that works out. I've been trying to catch up with your larger plan for this feature for 9.4. You made this comment earlier: 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 Is any of that code around yet? I see that you have split your submissions so that a smaller program can be reviewed today. I'd like to start taking a look at the next step too though. For the project I'm starting to work on here, getting the integration with labeling also done is a very important thing to target for 9.4. It would be nice to see how that fits together today, even if the code for it isn't being reviewed heavily yet. I don't quite understand yet what's missing on the writer side. If you could help explain what's missing there, I would like to read about that. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com -- 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] [v9.4] row level security
On Tue, Jul 23, 2013 at 11:30:14AM -0700, Josh Berkus wrote: Greg, It's more than the available experienced reviewers are willing to chew on fully as volunteers. The reward for spending review time is pretty low right now. Short of paying for review time, I don't think we have another solution for getting the big patches reviewed, except to rely on the major contributors who are paid full-time to hack Postgres. You know as well as me that, as consultants, we can get clients to pay for 10% extra time for review in the course of developing a feature, but the kind of time which patches like Row Security, Changesets, or other big patches need nobody is going to pay for on a contract basis. And nobody who is doing this in their spare time has that kind of block. So I don't think there's any good solution for the big patches. Let me echo Josh's comments above --- in the early years, we had trouble creating new features that required more than 1-2 weekends of development. We now have enough full-time developers that this is not a problem, but now it seems features requiring more than a weekend to _review_ are a problem, so full-time folks are again required here. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] [v9.4] row level security
On 07/22/2013 01:27 PM, Greg Smith wrote: Anyone who would like to see RLS committed should consider how you can get resources to support a skilled PostgreSQL reviewer spending time on it. (This is a bit much to expect new reviewers to chew on usefully) I've been working on that here, but I don't have anything I can publicly commit to yet. Apparently it's a little much for experienced reviewers to chew on, too, since I've been trying to get someone to review it since the beginning of the Commitfest. While I understand the call for resources, this is a bit unfair to KaiGai, who has put in his time reviewing other people's patches. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- 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] [v9.4] row level security
On 7/23/13 12:10 PM, Josh Berkus wrote: Apparently it's a little much for experienced reviewers to chew on, too, since I've been trying to get someone to review it since the beginning of the Commitfest. It's more than the available experienced reviewers are willing to chew on fully as volunteers. The reward for spending review time is pretty low right now. While I understand the call for resources, this is a bit unfair to KaiGai, who has put in his time reviewing other people's patches. If you read Dean Rasheed's comments, it's obvious he put a useful amount of work into his review suggestions. It is not the case here that KaiGai worked on a review and got nothing in return. Unfortunately that has happened to this particular patch before, but the community did a little better this time. The goal of the CF is usually to reach one of these outcomes for every submission: -The submission is ready for commit -The submission needs improvement in X Review here went far enough to identify an X to be improved. It was a big enough issue that a rewrite is needed, commit at this time isn't possible, and now KaiGai has something we hope is productive he can continue working on. That's all we can really promise here--that if we say something isn't ready for commit yet, that there's some feedback as to why. I would have preferred to see multiple X issues identified here, given that we know there has to be more than just the one in a submission of this size. The rough fairness promises of the CommitFest seem satisfied to me though. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com -- 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] [v9.4] row level security
Greg, It's more than the available experienced reviewers are willing to chew on fully as volunteers. The reward for spending review time is pretty low right now. Short of paying for review time, I don't think we have another solution for getting the big patches reviewed, except to rely on the major contributors who are paid full-time to hack Postgres. You know as well as me that, as consultants, we can get clients to pay for 10% extra time for review in the course of developing a feature, but the kind of time which patches like Row Security, Changesets, or other big patches need nobody is going to pay for on a contract basis. And nobody who is doing this in their spare time has that kind of block. So I don't think there's any good solution for the big patches. I do think our project could do much more to recruit reviewers for the small-medium patches, to take workload off the core contributors in general. Historically, however, this project (and the contributors on this list) has made material decisions not to encourage or recruit new people as reviewers, and has repeatedly stated that reviewers are not important. Until that changes, we are not going to get more reviewers (and I'm not going to have much sympathy for existing contributors who say they have no time for review). If we want more reviewers and people spending more time on review, then we need to give reviewers the *same* respect and the *same* rewards that feature contributors get. Not something else, the exact same. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- 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] [v9.4] row level security
On 7/23/13 2:30 PM, Josh Berkus wrote: You know as well as me that, as consultants, we can get clients to pay for 10% extra time for review in the course of developing a feature Before this number gets quoted anywhere, I think it's on the low side. I've spent a good bit of time measuring how much time it takes to do a fair offsetting review--one where you put as much time in as it takes to review your submission--and I keep getting numbers more in the 20 to 25% range. The work involved to do a high quality review takes a while. I happen to think the review structure is one of the most important components to PostgreSQL release quality. It used to be a single level review with just the committers, now it's a two level structure. The reason the Postgres code is so good isn't that the submitted development is inherently any better than other projects. There's plenty of bogus material submitted here. It's the high standards for review and commit that are the key filter. The importance of the process to the result isn't weighed as heavily as I think it should be. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com -- 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] [v9.4] row level security
On 07/23/2013 03:34 PM, Greg Smith wrote: I happen to think the review structure is one of the most important components to PostgreSQL release quality. It used to be a single level review with just the committers, now it's a two level structure. The reason the Postgres code is so good isn't that the submitted development is inherently any better than other projects. There's plenty of bogus material submitted here. It's the high standards for review and commit that are the key filter. The importance of the process to the result isn't weighed as heavily as I think it should be. I think we're in violent agreement here. Now, we just need to convince everyone else on this list. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- 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] [v9.4] row level security
On 7/20/13 10:08 AM, Kohei KaiGai wrote: With that change to expand_targetlist(), the change to getrelid() may be unnecessary, and then also the new rowsec_relid field in RangeTblEntry may not be needed. Hmm. I didn't have this idea. It seems to me fair enough and kills necessity to enhance RangeTblEntry and getrelid() indeed. I try to fix up this implementation according to your suggestion. Great, there's one useful bit of feedback for you then, and that seems to address Tom's getrelid concern too. For the active CommitFest, I don't see any place we can go with this right now except for Returned with Feedback. We really need more reviewers willing to put a significant amount of time into going through this code. Anyone who would like to see RLS committed should consider how you can get resources to support a skilled PostgreSQL reviewer spending time on it. (This is a bit much to expect new reviewers to chew on usefully) I've been working on that here, but I don't have anything I can publicly commit to yet. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com -- 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] [v9.4] row level security
(This is a bit much to expect new reviewers to chew on usefully) I've been working on that here, but I don't have anything I can publicly commit to yet. True that. I spent some time on it, but couldn't come up with anything useful. Mike's extensive testing seems good enough for me, though. One thing I was a bit concerned about (I don't know if it has been resolved already, or if it isn't a concern) is that there was an issue about a security part being visible in the syntax (or something like that, my memory isn't too good today morning). Regards, Atri Regards, Atri l'apprenant -- 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] [v9.4] row level security
2013/7/19 Stephen Frost sfr...@snowman.net: * Greg Smith (g...@2ndquadrant.com) wrote: On 7/18/13 7:57 PM, Karol Trzcionka wrote: Current head 4cbe3ac3e86790d05c569de4585e5075a62a9b41 - patch applies correct (only change needed in parallel_schedule). However it fails on own regression tests (other tests pass). I got a rejected hunk in src/backend/nodes/nodeFuncs.c as well as that parallel_schedule issue. Maybe you didn't get the nodeFuncs change but didn't notice that? That might explain why the tests didn't work for you either. The nodeFuncs.c hunk seems likely to have been impacted by the patch I committed today (WITH CHECK OPTION), so I doubt that was the issue. Attached is an updated patch where I tried to only fix the two small hunks of bit rot. I get All 140 tests passed here, on a Mac no less. Thanks for updating the patch, I ran into the failed hunks too and expected to have to deal with them. :) Thanks for pointing out this problem. I synchronized my local master with the upstream one, then adjusted the row-security branch. I'll submit the patch soon, with fixing-up the portion that Tom pointed out. Thanks, -- 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] [v9.4] row level security
2013/7/19 Dean Rasheed dean.a.rash...@gmail.com: On 19 July 2013 04:09, Greg Smith g...@2ndquadrant.com wrote: On 7/18/13 11:03 PM, Stephen Frost wrote: Wasn't there a wiki page about this feature which might also help? Bigger question- is it correct for the latest version of the patch? https://wiki.postgresql.org/wiki/RLS has accumulated a lot of older debris that may or may not be useful here. This useful piece was just presented at PGCon: http://www.pgcon.org/2013/schedule/attachments/273_PGcon2013-kaigai-row-level-security.pdf That is very up to date intro to the big picture issues. Hi, I took a look at this patch too. I didn't read all the code in detail, but there was one area I wondered about --- is it still necessary to add the new field rowsec_relid to RangeTblEntry? As far as I can see, it is only used in the new function getrelid() which replaces the old macro of the same name, so that it can work if it is passed the index of the sourceRelation subquery RTE instead of the resultRelation. This seems to be encoding a specific assumption that a subquery RTE is always going to come from row-level security expansion. Is it the case that this can only happen from expand_targetlist(), in which case why not pass both source_relation and result_relation to expand_targetlist(), which I think will make that code neater. As it stands, what expand_targetlist() sees as result_relation is actually source_relation, which getrelid() then handles specially. Logically I think expand_targetlist() should pass the actual result_relation to getrelid(), opening the target table, and then pass source_relation to makeVar() when building new TLEs. With that change to expand_targetlist(), the change to getrelid() may be unnecessary, and then also the new rowsec_relid field in RangeTblEntry may not be needed. Hmm. I didn't have this idea. It seems to me fair enough and kills necessity to enhance RangeTblEntry and getrelid() indeed. I try to fix up this implementation according to your suggestion. 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] [v9.4] row level security
On 19 July 2013 04:09, Greg Smith g...@2ndquadrant.com wrote: On 7/18/13 11:03 PM, Stephen Frost wrote: Wasn't there a wiki page about this feature which might also help? Bigger question- is it correct for the latest version of the patch? https://wiki.postgresql.org/wiki/RLS has accumulated a lot of older debris that may or may not be useful here. This useful piece was just presented at PGCon: http://www.pgcon.org/2013/schedule/attachments/273_PGcon2013-kaigai-row-level-security.pdf That is very up to date intro to the big picture issues. Hi, I took a look at this patch too. I didn't read all the code in detail, but there was one area I wondered about --- is it still necessary to add the new field rowsec_relid to RangeTblEntry? As far as I can see, it is only used in the new function getrelid() which replaces the old macro of the same name, so that it can work if it is passed the index of the sourceRelation subquery RTE instead of the resultRelation. This seems to be encoding a specific assumption that a subquery RTE is always going to come from row-level security expansion. Is it the case that this can only happen from expand_targetlist(), in which case why not pass both source_relation and result_relation to expand_targetlist(), which I think will make that code neater. As it stands, what expand_targetlist() sees as result_relation is actually source_relation, which getrelid() then handles specially. Logically I think expand_targetlist() should pass the actual result_relation to getrelid(), opening the target table, and then pass source_relation to makeVar() when building new TLEs. With that change to expand_targetlist(), the change to getrelid() may be unnecessary, and then also the new rowsec_relid field in RangeTblEntry may not be needed. 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] [v9.4] row level security
Dean Rasheed dean.a.rash...@gmail.com writes: I took a look at this patch too. I didn't read all the code in detail, but there was one area I wondered about --- is it still necessary to add the new field rowsec_relid to RangeTblEntry? As far as I can see, it is only used in the new function getrelid() which replaces the old macro of the same name, so that it can work if it is passed the index of the sourceRelation subquery RTE instead of the resultRelation. This seems to be encoding a specific assumption that a subquery RTE is always going to come from row-level security expansion. I haven't read the patch at all, but I would opine that anything that's changing the behavior of getrelid() is broken by definition. Something is being done at the wrong level of abstraction if you think that you need to do that. 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] [v9.4] row level security
Current head 4cbe3ac3e86790d05c569de4585e5075a62a9b41 - patch applies correct (only change needed in parallel_schedule). However it fails on own regression tests (other tests pass). Regards, Karol -- 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] [v9.4] row level security
* Greg Smith (g...@2ndquadrant.com) wrote: On 7/18/13 7:57 PM, Karol Trzcionka wrote: Current head 4cbe3ac3e86790d05c569de4585e5075a62a9b41 - patch applies correct (only change needed in parallel_schedule). However it fails on own regression tests (other tests pass). I got a rejected hunk in src/backend/nodes/nodeFuncs.c as well as that parallel_schedule issue. Maybe you didn't get the nodeFuncs change but didn't notice that? That might explain why the tests didn't work for you either. The nodeFuncs.c hunk seems likely to have been impacted by the patch I committed today (WITH CHECK OPTION), so I doubt that was the issue. Attached is an updated patch where I tried to only fix the two small hunks of bit rot. I get All 140 tests passed here, on a Mac no less. Thanks for updating the patch, I ran into the failed hunks too and expected to have to deal with them. :) I did a brief code scan through the patch just to get a feel for how the feature is put together, and what you'd need to know for a deeper review. That would be extremely helpful.. Wasn't there a wiki page about this feature which might also help? Bigger question- is it correct for the latest version of the patch? (I'm trying to get customer time approved to work on this a lot more) The code was easier to follow than I expected. The way it completely avoids even getting into the security label integration yet seems like a successful design partitioning. This isn't nearly as scary as the SEPostgres patches. There are some useful looking utility functions that dump information about what's going on too. The bulk of the complexity is how the feature modifies query nodes to restrict what rows come through them. Some familiarity with that part of the code is what you'd need to take on reviewing this in detail. That and a week of time to spend trudging through it. If anyone is looking for an educational challenge on query execution, marching through all of these changes to validate they work as expected would do that. I'm hoping to find time this weekend to look into this patch myself, but the weekend is also filling up with other activities, so we'll see. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] [v9.4] row level security
On 7/18/13 11:03 PM, Stephen Frost wrote: Wasn't there a wiki page about this feature which might also help? Bigger question- is it correct for the latest version of the patch? https://wiki.postgresql.org/wiki/RLS has accumulated a lot of older debris that may or may not be useful here. This useful piece was just presented at PGCon: http://www.pgcon.org/2013/schedule/attachments/273_PGcon2013-kaigai-row-level-security.pdf That is very up to date intro to the big picture issues. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com -- 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] [v9.4] row level security
With the current HEAD and v3 patch I'm seeing the following error with make check. -- == creating temporary installation== == initializing database system == pg_regress: initdb failed Examine /usr/local/src/postgres.patched.v3/src/test/regress/log/initdb.log for the reason. Command was: /usr/local/src/postgres.patched.v3/src/test/regress/./tmp_check/install//usr/local/pgsql/bin/initdb -D /usr/local/src/postgres.patched.v3/src/test/regress/./tmp_check/data -L /usr/local/src/postgres.patched.v3/src/test/regress/./tmp_check/install//usr/local/pgsql/share --noclean --nosync /usr/local/src/postgres.patched.v3/src/test/regress/log/initdb.log 21 make[1]: *** [check] Error 2 make[1]: Leaving directory `/usr/local/src/postgres.patched.v3/src/test/regress' make: *** [check] Error 2 __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com http://www.rrdonnelley.com/ * mike.blackw...@rrd.com*
Re: [HACKERS] [v9.4] row level security -- needs a reviewer
Hackers, Please, oh please, won't someone review this? This patch has been 3 years in the making, so I suspect that it will NOT be a fast review. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
