Re: [HACKERS] RLS Design
On 19 September 2014 17:54, Stephen Frost sfr...@snowman.net wrote: Thom, * Thom Brown (t...@linux.com) wrote: On 19 September 2014 17:32, Stephen Frost sfr...@snowman.net wrote: * Thom Brown (t...@linux.com) wrote: On 14 September 2014 16:38, Stephen Frost sfr...@snowman.net wrote: # create policy visible_colours on colours for all to joe using (visible = true); CREATE POLICY [...] insert into colours (name, visible) values ('transparent',false); ERROR: new row violates WITH CHECK OPTION for colours DETAIL: Failing row contains (7, transparent, f). select * from pg_policies ; policyname| tablename | roles | cmd | qual | with_check -+---+---+-+--+ visible_colours | colours | {joe} | ALL | (visible = true) | (1 row) There was no WITH CHECK OPTION. As I hope is clear if you look at the documentation- if the WITH CHECK clause is omitted, then the USING clause is used for both filtering and checking new records, otherwise you'd be able to add records which aren't visible to you. I can see that now, although I do find the error message somewhat confusing. Firstly, it looks like OPTION is part of the parameter name, which it isn't. Hmm, the notion of 'with check option' is from the SQL standard, which is why I felt the error message was appropriate as-is.. Also, I seem to get an error message with the following: # create policy nice_colours ON colours for all to joe using (visible = true) with check (name in ('blue','green','yellow')); CREATE POLICY \c - joe insert into colours (name, visible) values ('blue',false); ERROR: function with OID 0 does not exist Now *that* one is interesting and I'll definitely go take a look at it. We added quite a few regression tests to try and make sure these things work. And if this did work, but I only violated the USING clause, would this still say the WITH CHECK clause was the cause? WITH CHECK applies for INSERT and UPDATE for the new records going into the table. You can't actually violate the USING clause for an INSERT as USING is for filtering records, not checking that records being added to the table are valid. To try and clarify- by explicitly setting both USING and WITH CHECK, you *are* able to INSERT records which are not visible to you. We felt that was an important capability to support. I find it a bit of a limitation that I can't specify both INSERT and UPDATE for a policy. I'd want to be able to specify something like this: CREATE POLICY no_greys_allowed ON colours FOR INSERT, UPDATE WITH CHECK (name NOT IN ('grey','gray')); I would expect this to be rather common to prevent certain values making their way into a table. Instead I'd have to create 2 policies as it stands. In order to debug issues with accessing table data, perhaps it would be useful to output the name of the policy that was violated. If a table had 20 policies on, it could become time-consuming to debug. I keep getting tripped up by overlapping policies. On the one hand, I created a policy to ensure rows being added or selected have a visible column set to true. On the other hand, I have a policy that ensures that the name of a colour doesn't appear in a list. Policy 1 is violated until policy 2 is added: (using the table I created in a previous post on this thread...) # create policy must_be_visible ON colours for all to joe using (visible = true) with check (visible = true); CREATE POLICY \c - joe insert into colours (name, visible) values ('pink',false); ERROR: new row violates WITH CHECK OPTION for colours DETAIL: Failing row contains (28, pink, f). \c - thom # create policy no_greys_allowed on colours for insert with check (name not in ('grey','gray')); CREATE POLICY \c - joe # insert into colours (name, visible) values ('pink',false); INSERT 0 1 I expected this to still trigger an error due to the first policy. Am I to infer from this that the policy model is permissive rather than restrictive? I've also attached a few corrections for the docs. Thom diff --git a/doc/src/sgml/ref/alter_policy.sgml b/doc/src/sgml/ref/alter_policy.sgml index 37615fc..ab717f3 100644 --- a/doc/src/sgml/ref/alter_policy.sgml +++ b/doc/src/sgml/ref/alter_policy.sgml @@ -94,7 +94,7 @@ ALTER POLICY replaceable class=parametername/replaceable ON replaceable c security-barrier qualification to queries which use the table automatically. If multiple policies are being applied for a given table then they are all combined and added using OR. The USING - expression applies to records which are being retrived from the table. + expression applies to records which are being retrieved from the table. /para /listitem /varlistentry diff --git a/doc/src/sgml/ref/create_policy.sgml
Re: [HACKERS] RLS Design
Thom, * Thom Brown (t...@linux.com) wrote: I find it a bit of a limitation that I can't specify both INSERT and UPDATE for a policy. I'd want to be able to specify something like this: CREATE POLICY no_greys_allowed ON colours FOR INSERT, UPDATE WITH CHECK (name NOT IN ('grey','gray')); I would expect this to be rather common to prevent certain values making their way into a table. Instead I'd have to create 2 policies as it stands. That's not actually the case... CREATE POLICY no_greys_allowed ON colours FOR ALL USING (true) -- assuming this is what you intended WITH CHECK (name NOT IN ('grey','gray')); Right? That said, I'm not against the idea of supporting mulitple commands with one policy (similar to how ALL is done). It wouldn't be difficult or much of a change- make the 'cmd' a bitfield instead. If others feel the same then I'll look at doing that. In order to debug issues with accessing table data, perhaps it would be useful to output the name of the policy that was violated. If a table had 20 policies on, it could become time-consuming to debug. Good point. That'll involve a bit more as I'll need to look at the existing with check options structure, but I believe it's just adding the field to the structure, populating it when adding the WCO entries, and then checking for it in the ereport() call. The policy name is already stashed in the relcache entry, so it's already pretty easily available. I keep getting tripped up by overlapping policies. On the one hand, I created a policy to ensure rows being added or selected have a visible column set to true. On the other hand, I have a policy that ensures that the name of a colour doesn't appear in a list. Policy 1 is violated until policy 2 is added: (using the table I created in a previous post on this thread...) # create policy must_be_visible ON colours for all to joe using (visible = true) with check (visible = true); CREATE POLICY \c - joe insert into colours (name, visible) values ('pink',false); ERROR: new row violates WITH CHECK OPTION for colours DETAIL: Failing row contains (28, pink, f). \c - thom # create policy no_greys_allowed on colours for insert with check (name not in ('grey','gray')); CREATE POLICY \c - joe # insert into colours (name, visible) values ('pink',false); INSERT 0 1 I expected this to still trigger an error due to the first policy. Am I to infer from this that the policy model is permissive rather than restrictive? That's correct and I believe pretty clear in the documentation- policies are OR'd together, just the same as how roles are handled. As a logged-in user, you have the rights of all of the roles you are a member of (subject to inheiritance rules, of course), and similairly, you are able to view and add all rows which match any policy which applies to you (either through role membership or through different policies). I've also attached a few corrections for the docs. Thanks! I'll plan to include these with a few other typos and the fix for the bug that Andres pointed out, once I finish testing (and doing another CLOBBER_CACHE_ALWAYS run..). Thanks again, Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
On 25 September 2014 15:26, Stephen Frost sfr...@snowman.net wrote: I expected this to still trigger an error due to the first policy. Am I to infer from this that the policy model is permissive rather than restrictive? That's correct and I believe pretty clear in the documentation- policies are OR'd together, just the same as how roles are handled. As a logged-in user, you have the rights of all of the roles you are a member of (subject to inheiritance rules, of course), and similairly, you are able to view and add all rows which match any policy which applies to you (either through role membership or through different policies). Okay, I see now. This is a mindset issue for me as I'm looking at them like constraints rather than permissions. Thanks for the explanation. Thom -- 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] RLS Design
On 09/20/2014 12:38 AM, Stephen Frost wrote: I would not (nor do I feel that I did..) have committed it over a specific request to not do so from another committer. I had been hoping that there would be another review coming from somewhere, but there is always a trade-off between waiting longer to get a review ahead of a commit and having it committed and then available more easily for others to work with, review, and generally moving forward. Y'know what helps with that? Publishing clean git branches for non-trivial work, rather than just lobbing patches around. I'm finding the reliance on a patch based workflow increasingly frustrating for complex work, and wonder if it's time to revisit introducing a git repo+ref to the commitfest app. I find the need to find the latest patch on the list, apply it, and fix it up really frustrating. git am --3way helps a lot, but only if the patch is created with git format-patch. Perhaps it's time to look at whether git can do more to help us with the testing and review process. -- 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] RLS Design
On 09/20/2014 12:23 AM, Craig Ringer wrote: On 09/20/2014 12:38 AM, Stephen Frost wrote: I would not (nor do I feel that I did..) have committed it over a specific request to not do so from another committer. I had been hoping that there would be another review coming from somewhere, but there is always a trade-off between waiting longer to get a review ahead of a commit and having it committed and then available more easily for others to work with, review, and generally moving forward. Y'know what helps with that? Publishing clean git branches for non-trivial work, rather than just lobbing patches around. I'm finding the reliance on a patch based workflow increasingly frustrating for complex work, and wonder if it's time to revisit introducing a git repo+ref to the commitfest app. I find the need to find the latest patch on the list, apply it, and fix it up really frustrating. git am --3way helps a lot, but only if the patch is created with git format-patch. Perhaps it's time to look at whether git can do more to help us with the testing and review process. We discussed this at the last developer meeting, without coming up with a written procedure. Your ideas can help ... -- 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] RLS Design
On Sun, Sep 14, 2014 at 11:38 AM, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: On Thu, Sep 11, 2014 at 3:08 PM, Stephen Frost sfr...@snowman.net wrote: If we want to be able to disable RLS w/o dropping the policies, then I think we have to completely de-couple the two and users would then have both add policies AND turn on RLS to have RLS actually be enabled for a given table. I'm on the fence about that. Thoughts? A strong +1 for doing just that. Alright, updated patch attached which does just that (thanks to Adam for the updates for this and testing pg_dump- I just reviewed it and added some documentation updates and other minor improvements), and rebased to master. Also removed the catversion bump, so it should apply cleanly for people, for a while anyway. I specifically asked you to hold off on committing this until there was adequate opportunity for review, and explained my reasoning. You committed it anyway. I wonder if I am equally free to commit my own patches without properly observing the CommitFest process, because it would be a whole lot faster. My pg_background patches have been pending since before the start of the August CommitFest and I accepted that I would have to wait an extra two months to commit those because of a *clerical error*, namely my failure to actually add them to the CommitFest. This patch, on the other hand, was massively revised after the start of the CommitFest after many months of inactivity and committed with no thorough review by anyone who was truly independent of the development effort. It was then committed with no warning over a specific request, from another committer, that more time be allowed for review. I'm really disappointed by that. I feel I'm essentially getting punished for trying to follow what I understand to the process, which has involved me doing huge amounts of review of other people's patches and waiting a very long time to get my own stuff committed, while you bull ahead with your own patches. -- 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] RLS Design
On 2014-09-19 11:53:06 -0400, Robert Haas wrote: On Sun, Sep 14, 2014 at 11:38 AM, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: On Thu, Sep 11, 2014 at 3:08 PM, Stephen Frost sfr...@snowman.net wrote: If we want to be able to disable RLS w/o dropping the policies, then I think we have to completely de-couple the two and users would then have both add policies AND turn on RLS to have RLS actually be enabled for a given table. I'm on the fence about that. Thoughts? A strong +1 for doing just that. Alright, updated patch attached which does just that (thanks to Adam for the updates for this and testing pg_dump- I just reviewed it and added some documentation updates and other minor improvements), and rebased to master. Also removed the catversion bump, so it should apply cleanly for people, for a while anyway. I specifically asked you to hold off on committing this until there was adequate opportunity for review, and explained my reasoning. You committed it anyway. I was also rather surprised by the push. I wanted to write something about it, but: This patch, on the other hand, was massively revised after the start of the CommitFest after many months of inactivity and committed with no thorough review by anyone who was truly independent of the development effort. It was then committed with no warning over a specific request, from another committer, that more time be allowed for review. says it better. I think that's generally the case, but doubly so with sensitive stuff like this. I wonder if I am equally free to commit my own patches without properly observing the CommitFest process, because it would be a whole lot faster. My pg_background patches have been pending since before the start of the August CommitFest and I accepted that I would have to wait an extra two months to commit those because of a *clerical error*, namely my failure to actually add them to the CommitFest. FWIW, I think if a patch has been sent in time and has gotten a decent amount of review *and* agreement it's fair for a committer to push forward. That doesn't apply to this thread, but sometimes does for others. 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] RLS Design
On 14 September 2014 16:38, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: On Thu, Sep 11, 2014 at 3:08 PM, Stephen Frost sfr...@snowman.net wrote: If we want to be able to disable RLS w/o dropping the policies, then I think we have to completely de-couple the two and users would then have both add policies AND turn on RLS to have RLS actually be enabled for a given table. I'm on the fence about that. Thoughts? A strong +1 for doing just that. Alright, updated patch attached which does just that (thanks to Adam for the updates for this and testing pg_dump- I just reviewed it and added some documentation updates and other minor improvements), and rebased to master. Also removed the catversion bump, so it should apply cleanly for people, for a while anyway. This is testing what has been committed: # create table colours (id serial, name text, visible boolean); CREATE TABLE # insert into colours (name, visible) values ('blue',true),('yellow',true),('ultraviolet',false),('green',true),('infrared',false); INSERT 0 5 # create policy visible_colours on colours for all to joe using (visible = true); CREATE POLICY # grant all on colours to public; GRANT # grant all on sequence colours_id_seq to public; GRANT # alter table colours enable row level security ; ALTER TABLE \c - joe select * from colours; id | name | visible ++- 1 | blue | t 2 | yellow | t 4 | green | t (3 rows) insert into colours (name, visible) values ('purple',true); INSERT 0 1 insert into colours (name, visible) values ('transparent',false); ERROR: new row violates WITH CHECK OPTION for colours DETAIL: Failing row contains (7, transparent, f). select * from pg_policies ; policyname| tablename | roles | cmd | qual | with_check -+---+---+-+--+ visible_colours | colours | {joe} | ALL | (visible = true) | (1 row) There was no WITH CHECK OPTION. -- Thom
Re: [HACKERS] RLS Design
Thom, Thanks! * Thom Brown (t...@linux.com) wrote: On 14 September 2014 16:38, Stephen Frost sfr...@snowman.net wrote: # create policy visible_colours on colours for all to joe using (visible = true); CREATE POLICY [...] insert into colours (name, visible) values ('transparent',false); ERROR: new row violates WITH CHECK OPTION for colours DETAIL: Failing row contains (7, transparent, f). select * from pg_policies ; policyname| tablename | roles | cmd | qual | with_check -+---+---+-+--+ visible_colours | colours | {joe} | ALL | (visible = true) | (1 row) There was no WITH CHECK OPTION. As I hope is clear if you look at the documentation- if the WITH CHECK clause is omitted, then the USING clause is used for both filtering and checking new records, otherwise you'd be able to add records which aren't visible to you. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
Robert, * Robert Haas (robertmh...@gmail.com) wrote: On Sun, Sep 14, 2014 at 11:38 AM, Stephen Frost sfr...@snowman.net wrote: Alright, updated patch attached which does just that (thanks to Adam for the updates for this and testing pg_dump- I just reviewed it and added some documentation updates and other minor improvements), and rebased to master. Also removed the catversion bump, so it should apply cleanly for people, for a while anyway. I specifically asked you to hold off on committing this until there was adequate opportunity for review, and explained my reasoning. You committed it anyway. Hum- my apologies, I honestly don't recall you specifically asking for it to be held off indefinitely. :( There was discussion back and forth, quite a bit of it with you, and I thank you for your help with that and certainly welcome any additional comments. This patch, on the other hand, was massively revised after the start of the CommitFest after many months of inactivity and committed with no thorough review by anyone who was truly independent of the development effort. It was then committed with no warning over a specific request, from another committer, that more time be allowed for review. I would not (nor do I feel that I did..) have committed it over a specific request to not do so from another committer. I had been hoping that there would be another review coming from somewhere, but there is always a trade-off between waiting longer to get a review ahead of a commit and having it committed and then available more easily for others to work with, review, and generally moving forward. I'm really disappointed by that. I feel I'm essentially getting punished for trying to follow what I understand to the process, which has involved me doing huge amounts of review of other people's patches and waiting a very long time to get my own stuff committed, while you bull ahead with your own patches. While I wasn't public about it, I actually specifically discussed this question with others, a few times even, to try and make sure that I wasn't stepping out of line by moving forward. That said, I do see that Andres feels similairly. It certainly wasn't my intent to surprise anyone by it but simply to continue to move forward- in part, to allow me to properly break from it and work on other things, including reviewing other patches in the commitfest. I fear I've simply been overly focused on it these past few weeks, for a variety of reasons that would likely best be discussed at the pub. All-in-all, I feel appropriately chastised and certainly don't wish to be surprising fellow committers. Perhaps we can discuss at the dev meeting. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
On 19 September 2014 17:32, Stephen Frost sfr...@snowman.net wrote: Thom, Thanks! * Thom Brown (t...@linux.com) wrote: On 14 September 2014 16:38, Stephen Frost sfr...@snowman.net wrote: # create policy visible_colours on colours for all to joe using (visible = true); CREATE POLICY [...] insert into colours (name, visible) values ('transparent',false); ERROR: new row violates WITH CHECK OPTION for colours DETAIL: Failing row contains (7, transparent, f). select * from pg_policies ; policyname| tablename | roles | cmd | qual | with_check -+---+---+-+--+ visible_colours | colours | {joe} | ALL | (visible = true) | (1 row) There was no WITH CHECK OPTION. As I hope is clear if you look at the documentation- if the WITH CHECK clause is omitted, then the USING clause is used for both filtering and checking new records, otherwise you'd be able to add records which aren't visible to you. I can see that now, although I do find the error message somewhat confusing. Firstly, it looks like OPTION is part of the parameter name, which it isn't. Also, I seem to get an error message with the following: # create policy nice_colours ON colours for all to joe using (visible = true) with check (name in ('blue','green','yellow')); CREATE POLICY \c - joe insert into colours (name, visible) values ('blue',false); ERROR: function with OID 0 does not exist And if this did work, but I only violated the USING clause, would this still say the WITH CHECK clause was the cause? Thom
Re: [HACKERS] RLS Design
On 2014-09-19 12:38:39 -0400, Stephen Frost wrote: I would not (nor do I feel that I did..) have committed it over a specific request to not do so from another committer. I had been hoping that there would be another review coming from somewhere, but there is always a trade-off between waiting longer to get a review ahead of a commit and having it committed and then available more easily for others to work with, review, and generally moving forward. Sure, there is such a tradeoff. But others have to wait months to get enough review. The first revision of the patch in the form you committed was sent 2014-08-19, the first marked *ready for review* (not my words) is from 2014-08-30. 19 days really isn't very far along the tradeoff from waiting for a review to uselessly waiting. 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] RLS Design
On Fri, Sep 19, 2014 at 12:38 PM, Stephen Frost sfr...@snowman.net wrote: This patch, on the other hand, was massively revised after the start of the CommitFest after many months of inactivity and committed with no thorough review by anyone who was truly independent of the development effort. It was then committed with no warning over a specific request, from another committer, that more time be allowed for review. I would not (nor do I feel that I did..) have committed it over a specific request to not do so from another committer. Well, you're wrong. How could this email possibly have been any more clear? http://www.postgresql.org/message-id/CA+TgmoYA=uixxmn390sfgfqgvmll-as5bjal0om7yrppvwn...@mail.gmail.com You can hardly tell me you didn't see that email when you incorporated the technical content into the next patch version. While I wasn't public about it, I actually specifically discussed this question with others, a few times even, to try and make sure that I wasn't stepping out of line by moving forward. And yet you completely ignored the only public commentary on the issue, which was from me. I *should not have had* to object to this patch going in. It was clearly untimely for the August CommitFest, and as a long-time community member, you ought to know full well that any such patch should be resubmitted to a later CommitFest. This patch sat on the shelf for 4 months because you were too busy to work on it, and was committed 5 days from the last posted version, which version had zero review comments. If you didn't have time to work on it for 4 months, you can hardly expect everyone else who has an opinion to comment within 5 days. But, you know, because I could tell that you were fixated on pushing this patch through to commit quickly, I took the time to send you a message on that specific point, even though you should have known full well. In fact I took the time to send TWO. Here's the other one: http://www.postgresql.org/message-id/ca+tgmobqo0z87eivfdewjcac1dc4ahh5wcvoqoxrsateu1t...@mail.gmail.com All-in-all, I feel appropriately chastised and certainly don't wish to be surprising fellow committers. Perhaps we can discuss at the dev meeting. No, I think we should discuss it right now, not nine months from now when the issue has faded from everyone's mind. -- 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] RLS Design
Thom, * Thom Brown (t...@linux.com) wrote: On 19 September 2014 17:32, Stephen Frost sfr...@snowman.net wrote: * Thom Brown (t...@linux.com) wrote: On 14 September 2014 16:38, Stephen Frost sfr...@snowman.net wrote: # create policy visible_colours on colours for all to joe using (visible = true); CREATE POLICY [...] insert into colours (name, visible) values ('transparent',false); ERROR: new row violates WITH CHECK OPTION for colours DETAIL: Failing row contains (7, transparent, f). select * from pg_policies ; policyname| tablename | roles | cmd | qual | with_check -+---+---+-+--+ visible_colours | colours | {joe} | ALL | (visible = true) | (1 row) There was no WITH CHECK OPTION. As I hope is clear if you look at the documentation- if the WITH CHECK clause is omitted, then the USING clause is used for both filtering and checking new records, otherwise you'd be able to add records which aren't visible to you. I can see that now, although I do find the error message somewhat confusing. Firstly, it looks like OPTION is part of the parameter name, which it isn't. Hmm, the notion of 'with check option' is from the SQL standard, which is why I felt the error message was appropriate as-is.. Also, I seem to get an error message with the following: # create policy nice_colours ON colours for all to joe using (visible = true) with check (name in ('blue','green','yellow')); CREATE POLICY \c - joe insert into colours (name, visible) values ('blue',false); ERROR: function with OID 0 does not exist Now *that* one is interesting and I'll definitely go take a look at it. We added quite a few regression tests to try and make sure these things work. And if this did work, but I only violated the USING clause, would this still say the WITH CHECK clause was the cause? WITH CHECK applies for INSERT and UPDATE for the new records going into the table. You can't actually violate the USING clause for an INSERT as USING is for filtering records, not checking that records being added to the table are valid. To try and clarify- by explicitly setting both USING and WITH CHECK, you *are* able to INSERT records which are not visible to you. We felt that was an important capability to support. Thanks for taking a look at it! Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
Thom, Also, I seem to get an error message with the following: # create policy nice_colours ON colours for all to joe using (visible = true) with check (name in ('blue','green','yellow')); CREATE POLICY \c - joe insert into colours (name, visible) values ('blue',false); ERROR: function with OID 0 does not exist And if this did work, but I only violated the USING clause, would this still say the WITH CHECK clause was the cause? Since RLS is built on top of the same mechanisms used for Security Barrier Views, I figured I would check this case against that and, for the heck of it, regular VIEWs as well. The result is the same error in both cases (below and attached). I also verified that this issue exists for 9.4beta2 and the current REL9_4_STABLE branch. If this isn't the expected behavior (I can't imagine that it is), I am certainly willing to dive into it further and see what I can determine for a solution/recommendation. At any rate, this appears to be a previously existing issue with WITH CHECK OPTION. Thoughts? postgres=# DROP TABLE IF EXISTS colors CASCADE; NOTICE: table colors does not exist, skipping DROP TABLE postgres=# DROP ROLE IF EXISTS joe; DROP ROLE postgres=# CREATE ROLE joe LOGIN; CREATE ROLE postgres=# CREATE TABLE colors (name text, visible bool); CREATE TABLE postgres=# CREATE OR REPLACE VIEW v_colors_1 WITH (security_barrier) AS postgres-# SELECT * FROM colors WHERE (name in ('blue', 'green', 'yellow')) postgres-# WITH CHECK OPTION; CREATE VIEW postgres=# CREATE OR REPLACE VIEW v_colors_2 AS postgres-# SELECT * FROM colors WHERE (name in ('blue', 'green', 'yellow')) postgres-# WITH CHECK OPTION; CREATE VIEW postgres=# GRANT ALL ON v_colors_1, v_colors_2 TO joe; GRANT postgres=# \c - joe You are now connected to database postgres as user joe. postgres= INSERT INTO v_colors_1 (name, visible) VALUES ('blue', false); ERROR: function with OID 0 does not exist postgres= INSERT INTO v_colors_2 (name, visible) VALUES ('blue', false); ERROR: function with OID 0 does not exist Thanks, Adam -- Adam Brightwell - adam.brightw...@crunchydatasolutions.com Database Engineer - www.crunchydatasolutions.com with_check_error.sql Description: application/sql -- 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] RLS Design
Adam == Brightwell, Adam adam.brightw...@crunchydatasolutions.com writes: Adam At any rate, this appears to be a previously existing issue Adam with WITH CHECK OPTION. Thoughts? It's definitely an existing issue; you can reproduce it more simply, no need to mess with different users. The issue as far as I can tell is that the withCheckOption exprs are not being processed anywhere in setrefs.c, so it only works at all by pure fluke: for most operators, the opfuncid is also filled in by eval_const_expressions, but for whatever reason SAOPs escape this treatment. Same goes for other similar cases: create table colors (name text); create view vc1 as select * from colors where name is distinct from 'grue' with check option; create view vc2 as select * from colors where name in ('red','green','blue') with check option; create view vc3 as select * from colors where nullif(name,'grue') is null with check option; insert into vc1 values ('red'); -- fails insert into vc2 values ('red'); -- fails insert into vc3 values ('red'); -- fails (Also, commands/policy.c has two instances of const char as a function return type, which is a compiler warning since the const is meaningless.) -- Andrew (irc:RhodiumToad) -- 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] RLS Design
On Wed, September 10, 2014 23:50, Stephen Frost wrote: [rls_9-10-2014.patch] I can't get this to apply; I attach the complaints of patch. Erik Rijkers -- git clone git://git.postgresql.org/git/postgresql.git master Cloning into 'master'... -- git branch * master patching file doc/src/sgml/catalogs.sgml patching file doc/src/sgml/config.sgml Hunk #1 succeeded at 5411 (offset 114 lines). patching file doc/src/sgml/keywords.sgml patching file doc/src/sgml/ref/allfiles.sgml patching file doc/src/sgml/ref/alter_policy.sgml patching file doc/src/sgml/ref/alter_role.sgml patching file doc/src/sgml/ref/create_policy.sgml patching file doc/src/sgml/ref/create_role.sgml patching file doc/src/sgml/ref/drop_policy.sgml patching file doc/src/sgml/reference.sgml patching file src/backend/catalog/Makefile patching file src/backend/catalog/aclchk.c patching file src/backend/catalog/dependency.c patching file src/backend/catalog/heap.c patching file src/backend/catalog/objectaddress.c patching file src/backend/catalog/system_views.sql patching file src/backend/commands/Makefile patching file src/backend/commands/alter.c patching file src/backend/commands/copy.c patching file src/backend/commands/event_trigger.c patching file src/backend/commands/policy.c patching file src/backend/commands/tablecmds.c patching file src/backend/commands/user.c patching file src/backend/nodes/copyfuncs.c patching file src/backend/nodes/equalfuncs.c patching file src/backend/nodes/outfuncs.c patching file src/backend/nodes/readfuncs.c patching file src/backend/optimizer/plan/planner.c patching file src/backend/optimizer/plan/setrefs.c patching file src/backend/optimizer/prep/prepsecurity.c patching file src/backend/parser/gram.y patching file src/backend/parser/parse_agg.c patching file src/backend/parser/parse_expr.c patching file src/backend/rewrite/Makefile patching file src/backend/rewrite/rewriteHandler.c patching file src/backend/rewrite/rowsecurity.c patching file src/backend/tcop/utility.c Hunk #2 succeeded at 833 (offset -4 lines). patching file src/backend/utils/adt/acl.c patching file src/backend/utils/adt/ri_triggers.c patching file src/backend/utils/cache/plancache.c patching file src/backend/utils/cache/relcache.c patching file src/backend/utils/misc/guc.c patching file src/backend/utils/misc/postgresql.conf.sample patching file src/bin/pg_dump/common.c patching file src/bin/pg_dump/pg_backup.h patching file src/bin/pg_dump/pg_backup_archiver.c patching file src/bin/pg_dump/pg_dump.c patching file src/bin/pg_dump/pg_dump.h patching file src/bin/pg_dump/pg_dump_sort.c patching file src/bin/pg_dump/pg_dumpall.c patching file src/bin/pg_dump/pg_restore.c patching file src/bin/psql/describe.c patching file src/include/catalog/catversion.h Hunk #1 FAILED at 53. 1 out of 1 hunk FAILED -- saving rejects to file src/include/catalog/catversion.h.rej patching file src/include/catalog/dependency.h patching file src/include/catalog/indexing.h patching file src/include/catalog/pg_authid.h patching file src/include/catalog/pg_class.h patching file src/include/catalog/pg_rowsecurity.h patching file src/include/commands/policy.h patching file src/include/miscadmin.h patching file src/include/nodes/nodes.h patching file src/include/nodes/parsenodes.h patching file src/include/nodes/plannodes.h patching file src/include/nodes/relation.h patching file src/include/optimizer/planmain.h patching file src/include/parser/kwlist.h patching file src/include/parser/parse_node.h patching file src/include/rewrite/rowsecurity.h patching file src/include/utils/acl.h patching file src/include/utils/plancache.h patching file src/include/utils/rel.h patching file src/test/regress/expected/dependency.out patching file src/test/regress/expected/privileges.out patching file src/test/regress/expected/rowsecurity.out patching file src/test/regress/expected/rules.out patching file src/test/regress/expected/sanity_check.out patching file src/test/regress/parallel_schedule patching file src/test/regress/serial_schedule patching file src/test/regress/sql/rowsecurity.sql -- 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] RLS Design
Erik, * Erik Rijkers (e...@xs4all.nl) wrote: On Wed, September 10, 2014 23:50, Stephen Frost wrote: [rls_9-10-2014.patch] I can't get this to apply; I attach the complaints of patch. Thanks for taking a look at this! [...] patching file src/include/catalog/catversion.h Hunk #1 FAILED at 53. 1 out of 1 hunk FAILED -- saving rejects to file src/include/catalog/catversion.h.rej That's just the catversion bump- you can simply ignore it and everything should be fine. Look forward to hearing how it works for you! Thanks again, Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
On Sat, Sep 6, 2014 at 2:54 AM, Brightwell, Adam adam.brightw...@crunchydatasolutions.com wrote: * Add ALTER TABLE name { ENABLE | DISABLE } ROW LEVEL SECURITY - set flag on table to allow for a default-deny capability. If RLS is enabled on a table and has no policies, then a default-deny policy is automatically applied. If RLS is disabled on table and the table still has policies on it then then an error is raised. Though if DISABLE is accompanied with CASCADE, then all policies will be removed and no error is raised. This text doesn't make it clear that all of the cases have been covered; in particular, you didn't specify whether an error is thrown if you try to add a policy to a table with DISABLE ROW LEVEL SECURITY in effect. Backing up a bit, I think there are two sensible designs here: 1. Row level security policies can't exist for a table with DISABLE ROW LEVEL SECURITY in effect. It sounds like this is what you have implemented, modulo any hypothetical bugs. You can't add policies without enabling RLS, and you can't disable RLS without dropping them all. 2. Row level security policies can exist for a table with DISABLE ROW LEVEL SECURITY in effect, but they don't do anything until RLS is enabled. A possible advantage of this approach is that you could *temporarily* shut off RLS for a table without having to drop all of your policies and put them back. I kind of like this approach; we have something similar for triggers, and I think it could be useful to people. If you stick with approach #1, make sure pg_dump is guaranteed to enable RLS before applying the policies. And either way, you should that pg_dump behaves sanely in the case where there are circular dependencies, like you have two table A and B, and each has a RLS policy that manages to use the other table's row-type. (You probably also want to check that DROP and DROP .. CASCADE on either policy or either table does the right thing in that situation, but that's probably easier to get right.) -- 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] RLS Design
Robert, * Robert Haas (robertmh...@gmail.com) wrote: On Sat, Sep 6, 2014 at 2:54 AM, Brightwell, Adam adam.brightw...@crunchydatasolutions.com wrote: * Add ALTER TABLE name { ENABLE | DISABLE } ROW LEVEL SECURITY - set flag on table to allow for a default-deny capability. If RLS is enabled on a table and has no policies, then a default-deny policy is automatically applied. If RLS is disabled on table and the table still has policies on it then then an error is raised. Though if DISABLE is accompanied with CASCADE, then all policies will be removed and no error is raised. This text doesn't make it clear that all of the cases have been covered; in particular, you didn't specify whether an error is thrown if you try to add a policy to a table with DISABLE ROW LEVEL SECURITY in effect. Backing up a bit, I think there are two sensible designs here: Ah, yeah, the text could certainly be clearer. 1. Row level security policies can't exist for a table with DISABLE ROW LEVEL SECURITY in effect. It sounds like this is what you have implemented, modulo any hypothetical bugs. You can't add policies without enabling RLS, and you can't disable RLS without dropping them all. Right, this was the approach we were taking. Specifically, adding policies would implicitly enable RLS for the relation. 2. Row level security policies can exist for a table with DISABLE ROW LEVEL SECURITY in effect, but they don't do anything until RLS is enabled. A possible advantage of this approach is that you could *temporarily* shut off RLS for a table without having to drop all of your policies and put them back. I kind of like this approach; we have something similar for triggers, and I think it could be useful to people. I like the idea of being able to turn them off without dropping them. We have that with row_security = off, but that would only work for the owner or a superuser (or a user with bypassrls). This would allow disabling RLS temporairly for everything accessing the table. The one thing I'm wondering about with this design is- what happens when a policy is initially added? Currently, we automatically turn on RLS for the table when that happens. I'm not thrilled with the idea that you have to add policies AND turn on RLS explicitly- someone might add policies but then forget to turn RLS on.. If you stick with approach #1, make sure pg_dump is guaranteed to enable RLS before applying the policies. Currently, adding a policy automatically turns on RLS, so we don't have any issue with pg_dump from that perspective. Handling cases where RLS is disabled but policies exist would get more complicated for pg_dump if we keep the current idea that adding policies implicitly turns on RLS- it'd essentially have to go back and turn it off after the policies are added. Not a big fan of that either. And either way, you should that pg_dump behaves sanely in the case where there are circular dependencies, like you have two table A and B, and each has a RLS policy that manages to use the other table's row-type. (You probably also want to check that DROP and DROP .. CASCADE on either policy or either table does the right thing in that situation, but that's probably easier to get right.) Agreed, we'll double-check that this is working. As these are attributes of the table which get added later on by pg_dump, similar to permissions, I'd think it'd all work fine, but good to make sure (and ditto with DROP/DROP CASCADE.. We have some checks for that, but good to make sure it works in a circular-dependency case too). If we want to be able to disable RLS w/o dropping the policies, then I think we have to completely de-couple the two and users would then have both add policies AND turn on RLS to have RLS actually be enabled for a given table. I'm on the fence about that. Thoughts? Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
On Thu, Sep 11, 2014 at 3:08 PM, Stephen Frost sfr...@snowman.net wrote: 2. Row level security policies can exist for a table with DISABLE ROW LEVEL SECURITY in effect, but they don't do anything until RLS is enabled. A possible advantage of this approach is that you could *temporarily* shut off RLS for a table without having to drop all of your policies and put them back. I kind of like this approach; we have something similar for triggers, and I think it could be useful to people. I like the idea of being able to turn them off without dropping them. We have that with row_security = off, but that would only work for the owner or a superuser (or a user with bypassrls). This would allow disabling RLS temporairly for everything accessing the table. The one thing I'm wondering about with this design is- what happens when a policy is initially added? Currently, we automatically turn on RLS for the table when that happens. I'm not thrilled with the idea that you have to add policies AND turn on RLS explicitly- someone might add policies but then forget to turn RLS on.. Whoa. I think that's a bad idea. I think the default value for RLS should be disabled, and users should have to turn it on explicitly if they want to get it. It's arguable whether the behavior if you try to create a policy beforehand should be (1) outright failure or (2) command accepted but no effect, but I think (3) automagically enable the feature is a POLA violation. When somebody adds a policy and then drops it again, they will expect to be back in the same state they started out in, and for good reason. If we want to be able to disable RLS w/o dropping the policies, then I think we have to completely de-couple the two and users would then have both add policies AND turn on RLS to have RLS actually be enabled for a given table. I'm on the fence about that. Thoughts? A strong +1 for doing just that. Look, anybody who is going to use row-level security but isn't careful enough to verify that it's actually working as desired after configuring it is a lost cause anyway. That is the moral equivalent of a locksmith who comes out and replaces a lock for you and at no point while he's there does he ever close the door and verify that it latches and won't reopen. I'm sure somebody has done that, but if a security breach results, surely everybody would agree that the locksmith is at fault, not the lock manufacturer. Personally, I have to test every GRANT and REVOKE I issue, because there's no error for granting a privilege that the target already has or revoking one they don't, and with group membership and PUBLIC it's quite easy to have not done what you thought you did. Fixing that might be worthwhile but it doesn't take away from the fact that, like any other configuration change you make, security-relevant changes need to be tested. There is another possible advantage of the explicit-enable approach as well, which is that you might want to create several policies and then turn them all on at once. With what you have now, creating the first policy will enable RLS on the table and then everyone who wasn't the beneficiary of that initial policy is locked out. Now, granted, you can probably get around that by doing all of the operations in one transaction, so it's a minor point. But it's still nice to think about being able to add several policies and then flip them on. If it doesn't work out, flip them off, adjust, and flip them back on again. Now, again, the core design issue, IMHO, is that the switch from default-allow to default-deny should be explicit and unmistakable, so the rest of this is just tinkering around the edges. But we might as well make those edges as nice as possible, and the usability of this approach feels good 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] RLS Design
* Robert Haas (robertmh...@gmail.com) wrote: On Thu, Sep 11, 2014 at 3:08 PM, Stephen Frost sfr...@snowman.net wrote: The one thing I'm wondering about with this design is- what happens when a policy is initially added? Currently, we automatically turn on RLS for the table when that happens. I'm not thrilled with the idea that you have to add policies AND turn on RLS explicitly- someone might add policies but then forget to turn RLS on.. Whoa. I think that's a bad idea. I think the default value for RLS should be disabled, and users should have to turn it on explicitly if they want to get it. It's arguable whether the behavior if you try to create a policy beforehand should be (1) outright failure or (2) command accepted but no effect, but I think (3) automagically enable the feature is a POLA violation. When somebody adds a policy and then drops it again, they will expect to be back in the same state they started out in, and for good reason. Yeah, that I can agree with. Prior to adding the ability to explicitly enable RLS, that's what they got, but that's changed now that we've made the ability to turn on/off RLS half-way independent of policies. Also.. If we want to be able to disable RLS w/o dropping the policies, then I think we have to completely de-couple the two and users would then have both add policies AND turn on RLS to have RLS actually be enabled for a given table. I'm on the fence about that. A strong +1 for doing just that. Look, anybody who is going to use row-level security but isn't careful enough to verify that it's actually working as desired after configuring it is a lost cause anyway. I had been thinking the same, which is why I was on the fence about if it was really an issue or not. This all amounts to actually making the patch smaller also, which isn't a bad thing. Personally, I have to test every GRANT and REVOKE I issue, because there's no error for granting a privilege that the target already has or revoking one they don't, and with group membership and PUBLIC it's quite easy to have not done what you thought you did. Fixing that might be worthwhile but it doesn't take away from the fact that, like any other configuration change you make, security-relevant changes need to be tested. Hmm, pretty sure that'd end up going against the spec too, but that's a whole different discussion anyway. There is another possible advantage of the explicit-enable approach as well, which is that you might want to create several policies and then turn them all on at once. With what you have now, creating the first policy will enable RLS on the table and then everyone who wasn't the beneficiary of that initial policy is locked out. Now, granted, you can probably get around that by doing all of the operations in one transaction, so it's a minor point. But it's still nice to think about being able to add several policies and then flip them on. If it doesn't work out, flip them off, adjust, and flip them back on again. Now, again, the core design issue, IMHO, is that the switch from default-allow to default-deny should be explicit and unmistakable, so the rest of this is just tinkering around the edges. But we might as well make those edges as nice as possible, and the usability of this approach feels good to me. Fair enough. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
On Fri, Aug 29, 2014 at 8:16 PM, Brightwell, Adam adam.brightw...@crunchydatasolutions.com wrote: Attached is a patch for RLS that was create against master at 01363beae52700c7425cb2d2452177133dad3e93 and is ready for review. Overview: This patch provides the capability to create multiple named row level security policies for a table on a per command basis and assign them to be applied to specific roles/users. It contains the following changes: * Syntax: CREATE POLICY name ON table [ FOR { ALL | SELECT | INSERT | UPDATE | DELETE } ] [ TO { PUBLIC | role [, role ] } ] USING (condition) Creates a RLS policy named name on table. Specifying a command is optional, but the default is ALL. Specifying a role is options, but the default is PUBLIC. If PUBLIC and other roles are specified, ONLY PUBLIC is applied and a warning is raised. ALTER POLICY name ON table [ FOR { ALL | SELECT | INSERT | UPDATE | DELETE } ] [ TO { PUBLIC | role [, role ] } ] USING (condition) Alter a RLS policy named name on table. Specifying a command is optional, if provided then the policy's command is changed otherwise it is left as-is. Specifying a role is optional, if provided then the policy's role is changed otherwise it is left as-is. The condition must always be provided and is therefore always replaced. This is not a full review of this patch; as we're mid-CommitFest, I assume this will get added to the next CommitFest. In earlier discussions, it was proposed (and I thought the proposal was viewed favorably) that when enabling row-level security for a table (i.e. before doing CREATE POLICY), you'd have to first flip the table to a default-deny mode: ALTER TABLE name ENABLE ROW LEVEL SECURITY; In this design, I'm not sure what happens when there are policies for some but not all users or some but not all actions. Does creating a INSERT policy for one particular user cause a default-deny policy to be turned on for all other users and all other operations? That might be OK, but at the very least it should be documented more clearly. Does dropping the very last policy then instantaneously flip the table back to default-allow? As far as I can tell from the patch, and that's not too far since I've only looked at briefly, there's a default-deny policy only if there is at least 1 policy that applies to your user ID for this operation. As far as making it easy to create a watertight combination of policies, that seems like a bad plan. + elog(ERROR, Table \%s\ already has a policy named \%s\. + Use a different name for the policy or to modify this policy + use ALTER POLICY %s ON %s USING (qual), + RelationGetRelationName(target_table), stmt-policy_name, + RelationGetRelationName(target_table), stmt-policy_name); + That needs to be an ereport, be capitalized properly, and the hint, if it's to be included at all, needs to go into errhint(). + errhint(all roles are considered members of public))); Wrong message style for a hint. Also, not sure that's actually appropriate for a hint. + case EXPR_KIND_ROW_SECURITY: + return ROW SECURITY; This is quite simply bizarre. That's not the SQL syntax of anything. + | ROW SECURITY row_security_option + { + VariableSetStmt *n = makeNode(VariableSetStmt); + n-kind = VAR_SET_VALUE; + n-name = row_security; + n-args = list_make1(makeStringConst($3, @3)); + $$ = n; + } I object to this. There's no reason that we should bloat the parser to allow SET ROW SECURITY in lieu of SET row_security unless this is a standard-mandated syntax with standard-mandated semantics, which I bet it isn't. /* + * Although only on andoff are documented, we accept all likely variants of + * on and off. + */ + static const struct config_enum_entry row_security_options[] = { + {off, ROW_SECURITY_OFF, false}, + {on, ROW_SECURITY_ON, false}, + {true, ROW_SECURITY_ON, true}, + {false, ROW_SECURITY_OFF, true}, + {yes, ROW_SECURITY_ON, true}, + {no, ROW_SECURITY_OFF, true}, + {1, ROW_SECURITY_ON, true}, + {0, ROW_SECURITY_OFF, true}, + {NULL, 0, false} + }; Just make it a bool and you get all this for free. + /* + * is_rls_enabled - + * determines if row-security is enabled by checking the value of the system + * configuration row_security. + */ + bool + is_rls_enabled() + { + char const *rls_option; + + rls_option = GetConfigOption(row_security, true, false); + + return (strcmp(rls_option, on) == 0); + } Words fail me. + if (AuthenticatedUserIsSuperuser) + SetConfigOption(row_security, off, PGC_INTERNAL, PGC_S_OVERRIDE); Injecting this kind of magic into InitializeSessionUserId(), SetSessionAuthorization(), and SetCurrentRoleId() seems
Re: [HACKERS] RLS Design
Hey Robert, On my phone at the moment but wanted to reply. I'm working through a few of these issues already actually (noticed as I've been going over it with Adam), but certainly appreciate the additional review. We've not posted another update quite yet but plan to shortly. Thanks! Stephen
Re: [HACKERS] RLS Design
Robert, Alright, I can't help it so I'll try and reply from my phone for a couple of these. :) On Wednesday, September 3, 2014, Robert Haas robertmh...@gmail.com wrote: On Fri, Aug 29, 2014 at 8:16 PM, Brightwell, Adam adam.brightw...@crunchydatasolutions.com javascript:; wrote: Attached is a patch for RLS that was create against master at 01363beae52700c7425cb2d2452177133dad3e93 and is ready for review. Overview: This patch provides the capability to create multiple named row level security policies for a table on a per command basis and assign them to be applied to specific roles/users. It contains the following changes: * Syntax: CREATE POLICY name ON table [ FOR { ALL | SELECT | INSERT | UPDATE | DELETE } ] [ TO { PUBLIC | role [, role ] } ] USING (condition) Creates a RLS policy named name on table. Specifying a command is optional, but the default is ALL. Specifying a role is options, but the default is PUBLIC. If PUBLIC and other roles are specified, ONLY PUBLIC is applied and a warning is raised. ALTER POLICY name ON table [ FOR { ALL | SELECT | INSERT | UPDATE | DELETE } ] [ TO { PUBLIC | role [, role ] } ] USING (condition) Alter a RLS policy named name on table. Specifying a command is optional, if provided then the policy's command is changed otherwise it is left as-is. Specifying a role is optional, if provided then the policy's role is changed otherwise it is left as-is. The condition must always be provided and is therefore always replaced. This is not a full review of this patch; as we're mid-CommitFest, I assume this will get added to the next CommitFest. As per usual, the expectation is that the patch is reviewed and updated during the commitfest. Given that the commitfest isn't even over according to the calendar it seems a bit premature to talk about the next one, but certainly if it's not up to a commitable level before the end of this commitfest then it'll be submitted for the next. In earlier discussions, it was proposed (and I thought the proposal was viewed favorably) that when enabling row-level security for a table (i.e. before doing CREATE POLICY), you'd have to first flip the table to a default-deny mode: I do recall that (now that you remind me- clearly it had been lost during the subsequent discussion, from my point of view at least) and agree that it'd be useful. I don't believe it'll be difficult to address. ALTER TABLE name ENABLE ROW LEVEL SECURITY; Sounds reasonable to me. + elog(ERROR, Table \%s\ already has a policy named \%s\. + Use a different name for the policy or to modify this policy + use ALTER POLICY %s ON %s USING (qual), + RelationGetRelationName(target_table), stmt-policy_name, + RelationGetRelationName(target_table), stmt-policy_name); + That needs to be an ereport, be capitalized properly, and the hint, if it's to be included at all, needs to go into errhint(). Already addressed. + errhint(all roles are considered members of public))); Wrong message style for a hint. Also, not sure that's actually appropriate for a hint. Fair enough. Will address. + case EXPR_KIND_ROW_SECURITY: + return ROW SECURITY; This is quite simply bizarre. That's not the SQL syntax of anything. Will address. + | ROW SECURITY row_security_option + { + VariableSetStmt *n = makeNode(VariableSetStmt); + n-kind = VAR_SET_VALUE; + n-name = row_security; + n-args = list_make1(makeStringConst($3, @3)); + $$ = n; + } I object to this. There's no reason that we should bloat the parser to allow SET ROW SECURITY in lieu of SET row_security unless this is a standard-mandated syntax with standard-mandated semantics, which I bet it isn't. Agreed. Seemed like a nice idea but it's not necessary. /* + * Although only on andoff are documented, we accept all likely variants of + * on and off. + */ + static const struct config_enum_entry row_security_options[] = { + {off, ROW_SECURITY_OFF, false}, + {on, ROW_SECURITY_ON, false}, + {true, ROW_SECURITY_ON, true}, + {false, ROW_SECURITY_OFF, true}, + {yes, ROW_SECURITY_ON, true}, + {no, ROW_SECURITY_OFF, true}, + {1, ROW_SECURITY_ON, true}, + {0, ROW_SECURITY_OFF, true}, + {NULL, 0, false} + }; Just make it a bool and you get all this for free. Right- holdover from an earlier attempt to make it more complicated but now we've simplified it and so it should just be a bool. + if (AuthenticatedUserIsSuperuser) + SetConfigOption(row_security, off, PGC_INTERNAL, PGC_S_OVERRIDE); Injecting this kind of magic into InitializeSessionUserId(), SetSessionAuthorization(),
Re: [HACKERS] RLS Design
On Wed, Sep 3, 2014 at 10:40 AM, Stephen Frost sfr...@snowman.net wrote: This is not a full review of this patch; as we're mid-CommitFest, I assume this will get added to the next CommitFest. As per usual, the expectation is that the patch is reviewed and updated during the commitfest. Given that the commitfest isn't even over according to the calendar it seems a bit premature to talk about the next one, but certainly if it's not up to a commitable level before the end of this commitfest then it'll be submitted for the next. The first version of this patch that was described as ready for review was submitted on August 29th. The previous revision was submitted on August 18th. Both of those dates are after the CommitFest deadline of August 15th. So from where I sit this is not timely submitted for this CommitFest. The last version before August was submitted in April (there's a link to a version supposedly submitted in June in the CommitFest application, but it doesn't point to an email with a patch attached). I don't want to (and don't feel I should have to) decide between dropping everything to review an untimely-submitted patch and having it get committed with no review from anyone who wasn't involved in writing it. + if (AuthenticatedUserIsSuperuser) + SetConfigOption(row_security, off, PGC_INTERNAL, PGC_S_OVERRIDE); Injecting this kind of magic into InitializeSessionUserId(), SetSessionAuthorization(), and SetCurrentRoleId() seems 100% unacceptable to me. I was struggling with the right way to address this and welcome suggestions. The primary issue is that I really want to support a superuser turning it on, so we can't simply have it disabled for all superusers all the time. The requirement that it not be enabled by default for superusers makes sense, but how far does that extend and how do we address upgrades? In particular, can we simply set row_security=off as a custom GUC setting when superusers are created or roles altered to be made superusers? Would we do that in pg_upgrade? I think you need to have the GUC have one default value, not one default for superusers and another default for everybody else. I previously proposed making the GUC on/off/force, with on meaning apply row-level security unless we have permission to bypass it, either because we are the table owner or the superuser, off meaning error out if we would be forced to apply row-level security, and force meaning always apply row-level security even if we have permission to bypass it. I still think that's a good proposal. There may be other reasonable alternatives as well, but making changes to one GUC magically change other GUCs under the hood isn't one of them. -- 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] RLS Design
Adam, all, * Brightwell, Adam (adam.brightw...@crunchydatasolutions.com) wrote: Attached is a patch for RLS that was create against master at 01363beae52700c7425cb2d2452177133dad3e93 and is ready for review. Many thanks for posting this. As others may realize already, I've reviewed and modified this patch already, working with Adam to get it ready. I'm continuing to review and test it, but in general I'm quite happy with how it's shaping up- additional review, testing, and comments are always appreciated though. Alter a RLS policy named name on table. Specifying a command is optional, if provided then the policy's command is changed otherwise it is left as-is. Specifying a role is optional, if provided then the policy's role is changed otherwise it is left as-is. The condition must always be provided and is therefore always replaced. I'm pretty sure the condition is also optional in this patch (that was a late change that I made), but the documentation needs to be updated. * Plancache Invalidation: If a relation has a row-security policy and row-security is enabled then the invalidation will occur when either the row_security GUC is changed OR when a the current user changes. This invalidation ONLY takes place for cached plans where the target relation has a row security policy. I know there was a lot of discussion about this previously, but I'm fine with the initial version simply invalidating plans which involve RLS-enabled relations and role changes. This patch doesn't create any regressions for individuals who are not using RLS. We can certainly look into improving this in the future to have per-role plan caches but it's a fair bit of additional non-trivial code that can be added independently. * Security Qual Expression: All row-security policies are OR'ed together. This was also a point of much discussion, but I continue to feel this is the right approach for the initial version. We can add flexability here later, if necessary, but OR'ing these together is in-line with how role membership works today (you have right for all roles you are a member of, persuant to inherit/noinherit status, of course). * row_security GUC - enable/disable row level security. Note that, as discussed, pg_dump will set row_security off, unless specifically asked to enable it. row_security will also be set to off when the user logging in is a superuser or does a 'set role' to a superuser. Currently, if a user logging in is *not* a superuser, or a 'set role' is done to a non-superuser, row_security gets re-set to enabled. This is one aspect of the patch that I think we should change (which is a matter of removing just a few lines of code and then updating the regression tests to do 'set row_security = on;' before running), because if you log in as a superuser and then 'set role' to a non-superuser, it occurs to me now (it didn't really when I wrote this originally) as a bit surprising that row_security gets set to 'on' when doing a 'set role'. One thing that I really like about this approach is that a superuser can explicitly set 'row_security' to on and be able to see what happens. Clearly, in an environment of untrusted users, that could be dangerous, but it can also be an extremely useful way of testing things, particularly in development environments where everyone is a superuser. This deserves a bit more documentation also. * BYPASSRLS and NOBYPASSRLS role attribute - allows user to bypass RLS if row_security GUC is set to OFF. If a user sets row_security to OFF and does not have this attribute, then an error is raised when attempting to query a relation with a RLS policy. (note that the superuser is always considered to have the bypassrls attribute) * psql \d table support: psql describe support for listing policy information per table. This works pretty well for me, but we may want to add some indication that RLS is on a table in the \dp listing. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
On Fri, Jul 18, 2014 at 7:01 PM, Brightwell, Adam adam.brightw...@crunchydatasolutions.com wrote: I think we do want a way to modify policies. However, we tend to avoid syntax that involves unnatural word order, as this certainly does. Maybe it's better to follow the example of CREATE RULE and CREATE TRIGGER and do something this instead: CREATE POLICY policy_name ON table_name USING quals; ALTER POLICY policy_name ON table_name USING quals; DROP POLICY policy_name ON table_name; The advantage of this is that you can regard policy_name ON table_name as the identifier for the policy throughout the system. You need some kind of identifier of that sort anyway to support COMMENT ON, SECURITY LABEL, and ALTER EXTENSION ADD/DROP for policies. Sounds good. I certainly think it makes a lot of sense to include the ALTER functionality, if for no other reason than ease of use. Another item to consider, though I believe it can come later, is per-action policies. Following the above suggested syntax, perhaps that might look like the following? CREATE POLICY policy_name ON table_name FOR action USING quals; ALTER POLICY policy_name ON table_name FOR action USING quals; DROP POLICY policy_name ON table_name FOR action; That seems reasonable. You need to give some thought to what happens if the user types: CREATE POLICY pol1 ON tab1 FOR SELECT USING q1; ALTER POLICY pol1 ON tab1 FOR INSERT USING q2; I guess you end up with q1 as the SELECT policy and q2 as the INSERT policy. Similarly, had you typed: CREATE POLICY pol1 ON tab1 USING q1; ALTER POLICY pol1 ON tab1 FOR INSERT USING q2; ...then I guess you end up with q2 for INSERTs and q1 for everything else. I'm wondering if it might be better, though, not to allow the quals to be specified in CREATE POLICY, or else to allow multiple actions. Otherwise, getting pg_dump to DTRT might be complicated. Perhaps: CREATE POLICY pol1 ON tab1 ( [ [ FOR operation [ OR operation ] ... ] USING quals ] ... ); where operation = SELECT | INSERT | UPDATE | DELETE So that you can write things like: CREATE POLICY pol1 ON tab1 (USING a = 1); CREATE POLICY pol2 ON tab2 (FOR INSERT USING a = 1, FOR UPDATE USING b = 1, FOR DELETE USING c = 1); And then, for ALTER, just allow one change at a time, syntax as you proposed. That way each policy can be dumped as a single CREATE statement. I was also giving some thought to the use of POLICY, perhaps I am wrong, but it does seem it could be at risk of becoming ambiguous down the road. I can't think of any specific examples at the moment, but my concern is what happens if we wanted to add another type of policy, whatever that might be, later? Would it make more sense to go ahead and qualify this a little more with ROW SECURITY POLICY? I think that's probably over-engineering. I'm not aware of anything else we might add that would be likely to be called a policy, and if we did add something we could probably call it something else instead. And long command names are annoying. -- 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] RLS Design
On Wed, Jul 16, 2014 at 10:04 PM, Brightwell, Adam adam.brightw...@crunchydatasolutions.com wrote: Yes, I just tested it and the following would work from a grammar perspective: ALTER TABLE table_name POLICY ADD policy_name (policy_quals) ALTER TABLE table_name POLICY DROP policy_name Though, it would obviously require the addition of POLICY to the list of unreserved keywords. I don't suspect that would be a concern, as it is not reserved, but thought I would point it out just in case. Another thought I had was, would we also want the following, so that policies could be modified? ALTER TABLE table_name POLICY ALTER policy_name (policy_quals) I think we do want a way to modify policies. However, we tend to avoid syntax that involves unnatural word order, as this certainly does. Maybe it's better to follow the example of CREATE RULE and CREATE TRIGGER and do something this instead: CREATE POLICY policy_name ON table_name USING quals; ALTER POLICY policy_name ON table_name USING quals; DROP POLICY policy_name ON table_name; The advantage of this is that you can regard policy_name ON table_name as the identifier for the policy throughout the system. You need some kind of identifier of that sort anyway to support COMMENT ON, SECURITY LABEL, and ALTER EXTENSION ADD/DROP for policies. -- 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] RLS Design
I think we do want a way to modify policies. However, we tend to avoid syntax that involves unnatural word order, as this certainly does. Maybe it's better to follow the example of CREATE RULE and CREATE TRIGGER and do something this instead: CREATE POLICY policy_name ON table_name USING quals; ALTER POLICY policy_name ON table_name USING quals; DROP POLICY policy_name ON table_name; The advantage of this is that you can regard policy_name ON table_name as the identifier for the policy throughout the system. You need some kind of identifier of that sort anyway to support COMMENT ON, SECURITY LABEL, and ALTER EXTENSION ADD/DROP for policies. Sounds good. I certainly think it makes a lot of sense to include the ALTER functionality, if for no other reason than ease of use. Another item to consider, though I believe it can come later, is per-action policies. Following the above suggested syntax, perhaps that might look like the following? CREATE POLICY policy_name ON table_name FOR action USING quals; ALTER POLICY policy_name ON table_name FOR action USING quals; DROP POLICY policy_name ON table_name FOR action; I was also giving some thought to the use of POLICY, perhaps I am wrong, but it does seem it could be at risk of becoming ambiguous down the road. I can't think of any specific examples at the moment, but my concern is what happens if we wanted to add another type of policy, whatever that might be, later? Would it make more sense to go ahead and qualify this a little more with ROW SECURITY POLICY? Thanks, Adam -- Adam Brightwell - adam.brightw...@crunchydatasolutions.com Database Engineer - www.crunchydatasolutions.com
Re: [HACKERS] RLS Design
Tom Lane wrote: 20MB messages to the list aren't that friendly. Please don't do that again, unless asked to. FWIW the message was not distributed to the list. I got a note from Adam and dropped it from the moderation queue. -- Álvaro Herrerahttp://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] RLS Design
Brightwell, Adam adam.brightw...@crunchydatasolutions.com writes: You could do: ALTER TABLE table_name ADD POLICY policy_name (quals); ALTER TABLE table_name POLICY FOR role_name IS policy_name; ALTER TABLE table_name DROP POLICY policy_name; I am attempting to modify the grammar to support the above syntax. Unfortunately, I am encountering quite a number (280) shift/reduce errors/conflicts in bison. I have reviewed the bison documentation as well as the wiki page on resolving such conflicts. However, I am not entirely certain on the direction I should take in order to resolve these conflicts. I attempted to create a more redundant production like the wiki described, but unfortunately that was not successful. I have attached both the patch and bison report. Any help, recommendations or suggestions would be greatly appreciated. 20MB messages to the list aren't that friendly. Please don't do that again, unless asked to. FWIW, the above syntax is a nonstarter, at least unless we're willing to make POLICY a reserved word (hint: we're not). The reason is that the ADD/DROP COLUMN forms consider COLUMN to be optional, meaning that the column name could directly follow ADD; and the column type name, which could also be just a plain identifier, would directly follow that. So there's no way to resolve the ambiguity with one token of lookahead. This actually isn't just bison being stupid: in fact, you simply cannot tell whether ALTER TABLE tab ADD POLICY varchar(42); is an attempt to add a column named policy of type varchar(42), or an attempt to add a policy named varchar with quals 42. Pick a different syntax. 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] RLS Design
Adam, * Tom Lane (t...@sss.pgh.pa.us) wrote: Brightwell, Adam adam.brightw...@crunchydatasolutions.com writes: ALTER TABLE table_name ADD POLICY policy_name (quals); ALTER TABLE table_name POLICY FOR role_name IS policy_name; ALTER TABLE table_name DROP POLICY policy_name; [...] This actually isn't just bison being stupid: in fact, you simply cannot tell whether ALTER TABLE tab ADD POLICY varchar(42); is an attempt to add a column named policy of type varchar(42), or an attempt to add a policy named varchar with quals 42. Pick a different syntax. Yeah, now that we're trying to bake this into ALTER TABLE we need to be a bit more cautious. I'd think: ALTER TABLE tab POLICY ADD ... Would work though? (note: haven't looked/tested myself) Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
Tom, Thanks for the feedback. 20MB messages to the list aren't that friendly. Please don't do that again, unless asked to. Apologies, I didn't realize it was so large until after it was sent. At any rate, it won't happen again. FWIW, the above syntax is a nonstarter, at least unless we're willing to make POLICY a reserved word (hint: we're not). The reason is that the ADD/DROP COLUMN forms consider COLUMN to be optional, meaning that the column name could directly follow ADD; and the column type name, which could also be just a plain identifier, would directly follow that. So there's no way to resolve the ambiguity with one token of lookahead. This actually isn't just bison being stupid: in fact, you simply cannot tell whether ALTER TABLE tab ADD POLICY varchar(42); is an attempt to add a column named policy of type varchar(42), or an attempt to add a policy named varchar with quals 42. Ok. Make sense and I was afraid that was the case. Thanks, Adam -- Adam Brightwell - adam.brightw...@crunchydatasolutions.com Database Engineer - www.crunchydatasolutions.com
Re: [HACKERS] RLS Design
Stephen, Yeah, now that we're trying to bake this into ALTER TABLE we need to be a bit more cautious. I'd think: ALTER TABLE tab POLICY ADD ... Would work though? (note: haven't looked/tested myself) Yes, I just tested it and the following would work from a grammar perspective: ALTER TABLE table_name POLICY ADD policy_name (policy_quals) ALTER TABLE table_name POLICY DROP policy_name Though, it would obviously require the addition of POLICY to the list of unreserved keywords. I don't suspect that would be a concern, as it is not reserved, but thought I would point it out just in case. Another thought I had was, would we also want the following, so that policies could be modified? ALTER TABLE table_name POLICY ALTER policy_name (policy_quals) Thanks, Adam -- Adam Brightwell - adam.brightw...@crunchydatasolutions.com Database Engineer - www.crunchydatasolutions.com
Re: [HACKERS] RLS Design
Adam, * Brightwell, Adam (adam.brightw...@crunchydatasolutions.com) wrote: Yeah, now that we're trying to bake this into ALTER TABLE we need to be a bit more cautious. I'd think: ALTER TABLE tab POLICY ADD ... Would work though? (note: haven't looked/tested myself) Yes, I just tested it and the following would work from a grammar perspective: ALTER TABLE table_name POLICY ADD policy_name (policy_quals) ALTER TABLE table_name POLICY DROP policy_name Excellent, glad to hear it. Though, it would obviously require the addition of POLICY to the list of unreserved keywords. I don't suspect that would be a concern, as it is not reserved, but thought I would point it out just in case. Right, I don't anticipate anyone complaining too loudly about that.. Another thought I had was, would we also want the following, so that policies could be modified? ALTER TABLE table_name POLICY ALTER policy_name (policy_quals) Sounds like a good idea to me. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
On Thursday, July 10, 2014, Robert Haas robertmh...@gmail.com wrote: On Wed, Jul 9, 2014 at 2:13 AM, Stephen Frost sfr...@snowman.net javascript:; wrote: Yes, this would be possible (and is nearly identical to the original patch, except that this includes per-role considerations), however, my thinking is that it'd be simpler to work with policy names rather than sets of quals, to use when mapping to roles, and they would potentially be useful later for other things (eg: for setting up which policies should be applied when, or which should be OR' or ANDd with other policies, or having groups of policies, etc). Hmm. I guess that's reasonable. Should the policy be a per-table object (like rules, constraints, etc.) instead of a global object? You could do: ALTER TABLE table_name ADD POLICY policy_name (quals); ALTER TABLE table_name POLICY FOR role_name IS policy_name; ALTER TABLE table_name DROP POLICY policy_name; Right, I was thinking they would be per table as they would specifically provide a name for a set of quals, and quals are naturally table-specific. I don't see a need to have them be global- that had been brought up before with the notion of applications picking their policy, but we could also add that later through another term (eg: contexts) which would then map to policies or similar. We could even extend policies to be global by mapping existing per-table ones to be global if we really needed to... My feeling at the moment is that having them be per-table makes sense and we'd still have flexibility to change later if we had some compelling reason to do so. Thanks! Stephen
Re: [HACKERS] RLS Design
On Fri, Jul 11, 2014 at 4:55 AM, Stephen Frost sfr...@snowman.net wrote: On Thursday, July 10, 2014, Robert Haas robertmh...@gmail.com wrote: On Wed, Jul 9, 2014 at 2:13 AM, Stephen Frost sfr...@snowman.net wrote: Yes, this would be possible (and is nearly identical to the original patch, except that this includes per-role considerations), however, my thinking is that it'd be simpler to work with policy names rather than sets of quals, to use when mapping to roles, and they would potentially be useful later for other things (eg: for setting up which policies should be applied when, or which should be OR' or ANDd with other policies, or having groups of policies, etc). Hmm. I guess that's reasonable. Should the policy be a per-table object (like rules, constraints, etc.) instead of a global object? You could do: ALTER TABLE table_name ADD POLICY policy_name (quals); ALTER TABLE table_name POLICY FOR role_name IS policy_name; ALTER TABLE table_name DROP POLICY policy_name; Right, I was thinking they would be per table as they would specifically provide a name for a set of quals, and quals are naturally table-specific. I don't see a need to have them be global- that had been brought up before with the notion of applications picking their policy, but we could also add that later through another term (eg: contexts) which would then map to policies or similar. We could even extend policies to be global by mapping existing per-table ones to be global if we really needed to... My feeling at the moment is that having them be per-table makes sense and we'd still have flexibility to change later if we had some compelling reason to do so. I don't think you can really change it later. If policies are per-table, then you could have a policy p1 on table t1 and also on table t2; if they become global objects, then you can't have p1 in two places. I hope I'm not beating a dead horse here, but changing syntax after it's been released is very, very hard. But that's not an argument against doing it this way; I think per-table policies are probably simpler and better here. It means, for example, that policies need not have their own permissions and ownership structure - they're part of the table, just like a constraint, trigger, or rule, and the table owner's permissions control. I like that, and I think our users will, 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] RLS Design
Robert, On Friday, July 11, 2014, Robert Haas robertmh...@gmail.com wrote: On Fri, Jul 11, 2014 at 4:55 AM, Stephen Frost sfr...@snowman.net javascript:; wrote: My feeling at the moment is that having them be per-table makes sense and we'd still have flexibility to change later if we had some compelling reason to do so. I don't think you can really change it later. If policies are per-table, then you could have a policy p1 on table t1 and also on table t2; if they become global objects, then you can't have p1 in two places. I hope I'm not beating a dead horse here, but changing syntax after it's been released is very, very hard. Fair enough. My thinking was we'd come up with a way to map them (eg: table_policy), but I do agree that changing it later would really suck and having them be per-table makes a lot of sense. But that's not an argument against doing it this way; I think per-table policies are probably simpler and better here. It means, for example, that policies need not have their own permissions and ownership structure - they're part of the table, just like a constraint, trigger, or rule, and the table owner's permissions control. I like that, and I think our users will, too. Agreed and I believe this is more-or-less what I had proposed up-thread (not at a computer at the moment). I hope to have a chance to review and update the design and flush out the catalog definition this weekend. Thanks! Stephen
Re: [HACKERS] RLS Design
On Wed, Jul 9, 2014 at 2:13 AM, Stephen Frost sfr...@snowman.net wrote: Robert, * Robert Haas (robertmh...@gmail.com) wrote: If you're going to have predicates be table-level and access grants be table-level, then what's the value in having policies? You could just do: ALTER TABLE table_name GRANT ROW ACCESS TO role_name USING quals; Yes, this would be possible (and is nearly identical to the original patch, except that this includes per-role considerations), however, my thinking is that it'd be simpler to work with policy names rather than sets of quals, to use when mapping to roles, and they would potentially be useful later for other things (eg: for setting up which policies should be applied when, or which should be OR' or ANDd with other policies, or having groups of policies, etc). Hmm. I guess that's reasonable. Should the policy be a per-table object (like rules, constraints, etc.) instead of a global object? You could do: ALTER TABLE table_name ADD POLICY policy_name (quals); ALTER TABLE table_name POLICY FOR role_name IS policy_name; ALTER TABLE table_name DROP POLICY policy_name; -- 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] RLS Design
Craig, * Craig Ringer (cr...@2ndquadrant.com) wrote: I was jotting notes about this last sleepless night, and was really glad to see the suggestion of enabling RLS on a table being a requirement for OR-style quals suggested in the thread when I woke. Thanks for your thoughts and input! The only sane way to do OR-ing of multiple rules is to require that tables be switched to deny-by-default before RLS quals can be added to then selectively enable access. Right. The next step is DENY rules that override ALLOW rules, and are also ORed, so any DENY rule overrides any ALLOW rule. Like in ACLs. But that can be a later - I just think room for it should be left in any catalog definition. I'm not convinced regarding DENY rules, and I've seen very little of their use in practice.. The expectation is generally a deny-by-default setups with access granted explicity. My concern with the talk of policies, etc, is with making it possible to impliment this for 9.5. I'd really like to see a robust declarative row-security framework with access policies - but I'm not sure sure it's a good idea to try to assemble policies directly out of low level row security predicates. +1000%- we really need to solidify what should go into 9.5 and get that committed, then work out if there is more we can do in this release cycle. I'm fine with a simple approach to begin with, provided we can build on it moving forward without causing upgrade headaches, provided we can get to where we want to go, of course. Tying things into a policy model that isn't tried or tested might create more problems than it solves unless we implement multiple real-world test cases on top of the model to show it works. To this I would say- the original single-policy-per-table approach has been vetted by actual users to be valuable in their environments. It does not solve all cases, certainly, but it's simple and usable as-is and is the minimum which I would like to see in 9.5. Ideally, we can do better than that, but lets not throw out that win because we insist on a complete solution before it goes into core- because then we'll never get there. For how I think we should be pursuing this in the long run, take a look at how TeraData does it, with heirachical and non-heirachical rules - basically bitmaps or thresholds - that get grouped into access policies. It's a very good way to abstract the low level stuff. If we have low level table predicate filters, we can build this sort of thing on top. I keep thinking that a bitmap or similar might make sense here.. Consider a set of policies where we assign them numbers-per-table, a we can then build a bitmap of them, and then store what bitmap is applied to a given query. That then allows us to compare those bitmaps during plan cache checking to make sure that the policies applied last time are the same which we would be applying now, and therefore the existing cached plan is sufficient. It gets a bit more complicated when you allow AND-vs-OR and groups or hierarchies of policies, of course, but I'd like to think we can come up with a sensible way to represent that to allow for a quick check during plan cache lookup. For 9.5, unless the basics turn out to be way easier than they look and it's all done soon in the release process, surely we should be sticking to just getting the basics of row security in place? Leaving room for enhancement, sure, but sticking to the core feature which to my mind is: Agreed.. - A row security on/off flag for a table; Yes; I like this approach in general. - Room in the catalogs for multiple row security rules per table and a type flag for them. The initial type flag, for ALLOW rules, specifies that all ALLOW rules be ORed together. Works for me. I'm open to a per-table toggle which says AND instead of OR, provided we could implement that sanely and simply. - Syntax for creating and dropping row security predicates. If there can be multiple ones per table they'll need names, like we have with triggers, indexes, etc. Agreed. To Robert's question about having policy names at all, rather than just quals, I feel like we'll need them eventually anyway and having them earlier will simplify things. Additionally, it's simpler to reason about and to manage- one can expect a one-to-many relationship between policies and roles, making it simpler to work with the policy name when associating it it to a role rather than having to remember all of the quals involved. - psql support for listing row security predicates on a table if running as superuser or if you've been explicitly GRANTed access to the catalog table listing row security quals. We need psql support to list the RLS policies.. I don't wish to get into the question about what kind of access that requires though. At least initially, I wouldn't try to limit access to the policies or quals in the catalog... Perhaps we need that but I'd like a bit more discussion
Re: [HACKERS] RLS Design
KaiGai, * Kohei KaiGai (kai...@kaigai.gr.jp) wrote: What I'd like to implement is adjustment of query like: SELECT * FROM t1 WHERE (x like '%abc%') AND (quals by built-in RLS) AND (quals by extension-1) AND ... AND (quals by extension-N); I never mind even if qualifiers in the second block are connected with OR'd manner, however, I want RLS infrastructure to accept additional security models provided by extensions. Would having a table-level 'AND'-vs-'OR' modifier for the RLS policies on that table be sufficient for what you're looking for? That seems a simple enough addition which would still allow more complex groups to be developed later on... Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
Robert, * Robert Haas (robertmh...@gmail.com) wrote: If you're going to have predicates be table-level and access grants be table-level, then what's the value in having policies? You could just do: ALTER TABLE table_name GRANT ROW ACCESS TO role_name USING quals; Yes, this would be possible (and is nearly identical to the original patch, except that this includes per-role considerations), however, my thinking is that it'd be simpler to work with policy names rather than sets of quals, to use when mapping to roles, and they would potentially be useful later for other things (eg: for setting up which policies should be applied when, or which should be OR' or ANDd with other policies, or having groups of policies, etc). As I see it, the only value in having policies as separate objects is that you can then, by granting access to the policy, give a particular user a bundle of rights rather than having to grant each right individually. But with this design, you've got to create the policy, then add the quals to it for each table, and then you still have to give access individually for every row, table combination, so what value is the policy object itself providing? To clarify this part- the idea is that you would simply declare a policy name to be a set of quals for a particular table, so you declare them and then map a policy to roles for which it should be used. In this arrangement, you don't declare the policy explicitly before setting the quals, those are done at the same time. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
2014-07-09 15:07 GMT+09:00 Stephen Frost sfr...@snowman.net: KaiGai, * Kohei KaiGai (kai...@kaigai.gr.jp) wrote: What I'd like to implement is adjustment of query like: SELECT * FROM t1 WHERE (x like '%abc%') AND (quals by built-in RLS) AND (quals by extension-1) AND ... AND (quals by extension-N); I never mind even if qualifiers in the second block are connected with OR'd manner, however, I want RLS infrastructure to accept additional security models provided by extensions. Would having a table-level 'AND'-vs-'OR' modifier for the RLS policies on that table be sufficient for what you're looking for? That seems a simple enough addition which would still allow more complex groups to be developed later on... Probably, things I'm considering is more simple. If a table has multiple built-in RLS policies, its expression node will be represented as a BoolExpr with OR_EXPR and every policies are linked to its args field, isn't it? We assume the built-in RLS model merges multiple policies by OR manner. In case when an extension want to apply additional security model on top of RLS infrastructure, a straightforward way is to add its own rules in addition to the built-in rules. If extension can get control to modify the above expression node and RLS infrastructure works well on the modified expression node, I think it's sufficient to implement multiple security models on the RLS 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] RLS Design
KaiGai, * Kohei KaiGai (kai...@kaigai.gr.jp) wrote: 2014-07-09 15:07 GMT+09:00 Stephen Frost sfr...@snowman.net: * Kohei KaiGai (kai...@kaigai.gr.jp) wrote: What I'd like to implement is adjustment of query like: SELECT * FROM t1 WHERE (x like '%abc%') AND (quals by built-in RLS) AND (quals by extension-1) AND ... AND (quals by extension-N); I never mind even if qualifiers in the second block are connected with OR'd manner, however, I want RLS infrastructure to accept additional security models provided by extensions. Would having a table-level 'AND'-vs-'OR' modifier for the RLS policies on that table be sufficient for what you're looking for? That seems a simple enough addition which would still allow more complex groups to be developed later on... Probably, things I'm considering is more simple. If a table has multiple built-in RLS policies, its expression node will be represented as a BoolExpr with OR_EXPR and every policies are linked to its args field, isn't it? We assume the built-in RLS model merges multiple policies by OR manner. In case when an extension want to apply additional security model on top of RLS infrastructure, a straightforward way is to add its own rules in addition to the built-in rules. If extension can get control to modify the above expression node and RLS infrastructure works well on the modified expression node, I think it's sufficient to implement multiple security models on the RLS infrastructure. Another way would be to have a single RLS policy which extensions can modify, sure. That was actually along the lines of the originally proposed patch.. That approach would work if we OR'd multiple policies together too, provided the user took care to only have one policy implemented.. Not sure how easy that would be to work with for extension authors though. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
Hi all I was jotting notes about this last sleepless night, and was really glad to see the suggestion of enabling RLS on a table being a requirement for OR-style quals suggested in the thread when I woke. The only sane way to do OR-ing of multiple rules is to require that tables be switched to deny-by-default before RLS quals can be added to then selectively enable access. The next step is DENY rules that override ALLOW rules, and are also ORed, so any DENY rule overrides any ALLOW rule. Like in ACLs. But that can be a later - I just think room for it should be left in any catalog definition. My concern with the talk of policies, etc, is with making it possible to impliment this for 9.5. I'd really like to see a robust declarative row-security framework with access policies - but I'm not sure sure it's a good idea to try to assemble policies directly out of low level row security predicates. Tying things into a policy model that isn't tried or tested might create more problems than it solves unless we implement multiple real-world test cases on top of the model to show it works. For how I think we should be pursuing this in the long run, take a look at how TeraData does it, with heirachical and non-heirachical rules - basically bitmaps or thresholds - that get grouped into access policies. It's a very good way to abstract the low level stuff. If we have low level table predicate filters, we can build this sort of thing on top. For 9.5, unless the basics turn out to be way easier than they look and it's all done soon in the release process, surely we should be sticking to just getting the basics of row security in place? Leaving room for enhancement, sure, but sticking to the core feature which to my mind is: - A row security on/off flag for a table; - Room in the catalogs for multiple row security rules per table and a type flag for them. The initial type flag, for ALLOW rules, specifies that all ALLOW rules be ORed together. - Syntax for creating and dropping row security predicates. If there can be multiple ones per table they'll need names, like we have with triggers, indexes, etc. - psql support for listing row security predicates on a table if running as superuser or if you've been explicitly GRANTed access to the catalog table listing row security quals. - The hooks for contribs to inject their own row security rules. The API will need a tweak - right now it assumes these rules are ANDed with any row security predicates in the catalogs, but we'd want the option of treating them as ALLOW or DENY rules to get ORed with the rest of the set *or* as a pre-filter predicate like currently. - A row-security-exempt right, at the user-level, to assuage the concerns about malicious predicates. I maintain that in the first rev this should be simple: superuser is row security exempt. I don't think I'm going to win that one though, so a user/role attribute that makes the role ignore row security seems like the next simplest option. - A way to test whether the current user is row-security exempt so pg_dump can complain unless explicitly told it's allowed to do a selective dump via a cmdline option; Plus a number of fixes: - Fixing the security barrier view isssue with row level lock pushdown that's breaking the row security regression tests; - Enhancing plan cache invalidation so that row-security exempt-ness of a user is part of the plancache key; - Adding session state like current_user to portals, so security_barrier functions returning refcursor, and cursors created before SET SESSION AUTHORIZATION or SET ROLE, get the correct values when they use session information like current_user Note that this doesn't even consider the with check option style write-filtering side of row security and the corresponding challenges with the semantics around RETURNING. It's already a decent sized amount of work on top of the existing row security patch. If we start adding policy groups, etc, this will never get done. -- 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] RLS Design
2014-07-06 14:19 GMT+09:00 Stephen Frost sfr...@snowman.net: Kaigai, * Kouhei Kaigai (kai...@ak.jp.nec.com) wrote: Can you clarify where this is coming from..? It sounds like you're referring to an existing implementation and, if so, it'd be good to get more information on how that works exactly. Oracle VPD - Multiple Policies for Each Table, View, or Synonym http://docs.oracle.com/cd/B19306_01/network.102/b14266/apdvpoli.htm#i1008351 It says - Note that all policies applied to a table are enforced with AND syntax. While I'm not against using this as an example to consider, it's much more complex than what we're talking about here- and it supports application contexts which allow groups of RLS rights to be applied or not applied; essentially it allows both AND and OR for sets of RLS policies, along with default policies which are applied no matter what. Not only Oracle VPD, it fits attitude of defense in depth. Please assume a system that installs network firewall, unix permissions and selinux. If somebody wants to reference an information asset within a file, he has to connect the server from the network address being allowed by the firewall configuration AND both of DAC and MAC has to allow his access. These are not independent systems and your argument would apply to our GRANT system also, which I hope it's agreed would make it far less useful. Note also that SELinux brings in another complexity- it needs to make system calls out to check the access. Usually, we have to pass all the access control to reference the target information, not one of the access control stuffs being installed. This is true in some cases, and not in others. Only one role you are a member of needs to have access to a relation, not all of them. There are other examples of 'OR'-style security policies, this is merely one. I'm simply not convinced that it applies in the specific case we're talking about. In the end, I expect that either way people will be upset because they won't be able to specify fully which should be AND vs. which should be OR with the kind of flexibility other systems provide. What I'm trying to get to is an initial implementation which is generally useful and is able to add such support later. This is similar to how roles work- your overall access includes all access granted to any roles you are a member of. You don't need SELECT rights granted to every role you are a member of to select from the table. Additionally, if an admin wants to AND the quals together then they can simply create a policy which does that rather than have 2 policies. It seems to me a pain on database administration, if we have to pay attention not to conflict each RLS-policy. This notion of 'conflict' doesn't make much sense to me. What is 'conflicting' here? Each policy would simply need to stand on its own for the role which it's being applied to. That's very simple and straight-forward. I expect 90% of RLS-policy will be configured to PUBLIC user, to apply everybody same rules on access. In this case, DBA has to ensure the target table has no policy or existing policy does not conflict with the new policy to be set. I don't think it is a good idea to enforce DBA these checks. If the DBA only uses PUBLIC then they have to ensure that each policy they set up for PUBLIC can stand on its own- though, really, I expect if they go that route they'd end up with just one policy that calls a stored procedure... You are suggesting instead that if application 2 sets up policies on the table and then application 1 adds another policy that it should reduce what application 2's users can see? That doesn't make any sense to me. I'd actually expect these applications to at least use different roles anyway, which means they could each have a single role specific policy which only returns what that application is allowed to see. I don't think this assumption is reasonable. Please expect two applications: app-X that is a database security product to apply access control based on remote ip-address of the client for any table accesses by any database roles. app-Y that is a usual enterprise package for daily business data, with RLS-policy. What is the expected behavior in this case? That the DBA manage the rights on the tables. I expect that will be required for quite a while with PG. It's nice to think of these application products that will manage all access for users by setting up their own policies, but we have yet to even discuss how they would have appropriate rights on the table to be able to do so (and to not interfere with each other..). Let's at least get something which is generally useful in. I'm all for trying to plan out how to get there and would welcome suggestions you have which are specific to PG on what we could do here (I'm not keen on just trying to mimic another product completely...), but at the level
Re: [HACKERS] RLS Design
On Thu, Jul 3, 2014 at 1:14 AM, Stephen Frost sfr...@snowman.net wrote: Alright, apologies for it being a bit later than intended, but here's what I've come up with thus far. -- policies defined at a table scope -- allows using the same policy name for different tables -- with quals appropriate for each table ALTER TABLE t1 ADD POLICY p1 USING p1_quals; ALTER TABLE t1 ADD POLICY p2 USING p2_quals; -- used to drop a policy definition from a table ALTER TABLE t1 DROP POLICY p1; -- cascade required when references exist for the policy -- from roles ALTER TABLE t1 DROP POLICY p1 CASCADE; ALTER TABLE t1 ALTER POLICY p1 USING new_quals; -- Controls if any RLS is applied to this table or not -- If enabled, all users must access through some policy ALTER TABLE table_name [ NO ] ROW LEVEL SECURITY; -- Associates roles to policies ALTER TABLE table_name GRANT ROW ACCESS TO role_name USING p1; ALTER TABLE table_name REVOKE ROW ACCESS FROM role_name USING p1; If you're going to have predicates be table-level and access grants be table-level, then what's the value in having policies? You could just do: ALTER TABLE table_name GRANT ROW ACCESS TO role_name USING quals; As I see it, the only value in having policies as separate objects is that you can then, by granting access to the policy, give a particular user a bundle of rights rather than having to grant each right individually. But with this design, you've got to create the policy, then add the quals to it for each table, and then you still have to give access individually for every row, table combination, so what value is the policy object itself providing? -- 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] RLS Design
Kaigai, * Kouhei Kaigai (kai...@ak.jp.nec.com) wrote: Can you clarify where this is coming from..? It sounds like you're referring to an existing implementation and, if so, it'd be good to get more information on how that works exactly. Oracle VPD - Multiple Policies for Each Table, View, or Synonym http://docs.oracle.com/cd/B19306_01/network.102/b14266/apdvpoli.htm#i1008351 It says - Note that all policies applied to a table are enforced with AND syntax. While I'm not against using this as an example to consider, it's much more complex than what we're talking about here- and it supports application contexts which allow groups of RLS rights to be applied or not applied; essentially it allows both AND and OR for sets of RLS policies, along with default policies which are applied no matter what. Not only Oracle VPD, it fits attitude of defense in depth. Please assume a system that installs network firewall, unix permissions and selinux. If somebody wants to reference an information asset within a file, he has to connect the server from the network address being allowed by the firewall configuration AND both of DAC and MAC has to allow his access. These are not independent systems and your argument would apply to our GRANT system also, which I hope it's agreed would make it far less useful. Note also that SELinux brings in another complexity- it needs to make system calls out to check the access. Usually, we have to pass all the access control to reference the target information, not one of the access control stuffs being installed. This is true in some cases, and not in others. Only one role you are a member of needs to have access to a relation, not all of them. There are other examples of 'OR'-style security policies, this is merely one. I'm simply not convinced that it applies in the specific case we're talking about. In the end, I expect that either way people will be upset because they won't be able to specify fully which should be AND vs. which should be OR with the kind of flexibility other systems provide. What I'm trying to get to is an initial implementation which is generally useful and is able to add such support later. This is similar to how roles work- your overall access includes all access granted to any roles you are a member of. You don't need SELECT rights granted to every role you are a member of to select from the table. Additionally, if an admin wants to AND the quals together then they can simply create a policy which does that rather than have 2 policies. It seems to me a pain on database administration, if we have to pay attention not to conflict each RLS-policy. This notion of 'conflict' doesn't make much sense to me. What is 'conflicting' here? Each policy would simply need to stand on its own for the role which it's being applied to. That's very simple and straight-forward. I expect 90% of RLS-policy will be configured to PUBLIC user, to apply everybody same rules on access. In this case, DBA has to ensure the target table has no policy or existing policy does not conflict with the new policy to be set. I don't think it is a good idea to enforce DBA these checks. If the DBA only uses PUBLIC then they have to ensure that each policy they set up for PUBLIC can stand on its own- though, really, I expect if they go that route they'd end up with just one policy that calls a stored procedure... You are suggesting instead that if application 2 sets up policies on the table and then application 1 adds another policy that it should reduce what application 2's users can see? That doesn't make any sense to me. I'd actually expect these applications to at least use different roles anyway, which means they could each have a single role specific policy which only returns what that application is allowed to see. I don't think this assumption is reasonable. Please expect two applications: app-X that is a database security product to apply access control based on remote ip-address of the client for any table accesses by any database roles. app-Y that is a usual enterprise package for daily business data, with RLS-policy. What is the expected behavior in this case? That the DBA manage the rights on the tables. I expect that will be required for quite a while with PG. It's nice to think of these application products that will manage all access for users by setting up their own policies, but we have yet to even discuss how they would have appropriate rights on the table to be able to do so (and to not interfere with each other..). Let's at least get something which is generally useful in. I'm all for trying to plan out how to get there and would welcome suggestions you have which are specific to PG on what we could do here (I'm not keen on just trying to mimic another product completely...), but at the level we're talking about (either AND them all or OR them all), I don't think we'd actually solve the
Re: [HACKERS] RLS Design
Sorry for my late responding, now I'm catching up the discussion. * Robert Haas (robertmh...@gmail.com) wrote: On Tue, Jul 1, 2014 at 3:20 PM, Dean Rasheed dean.a.rash...@gmail.com wrote: If RLS quals are instead regarded as constraints on access, and multiple policies apply, then it seems that the quals should now be combined with AND rather than OR, right? I do feel that RLS quals are constraints on access, but I don't see how it follows that multiple quals should be AND'd together because of that. I view the RLS policies on each table as being independent and standing alone regarding what can be seen. If you have access to a table today through policy A, and then later policy B is added, using AND would mean that the set of rows returned is less than if only policy A existed. That doesn't seem correct to me. It seems to me direction of the constraints (RLS-policy) works to is reverse. In case when we have no RLS-policy, 100% of rows are visible isn't it? Addition of a constraint usually reduces the number of rows being visible, or same number of rows at least. Constraint shall never work to the direction to increase the number of rows being visible. If multiple RLS-policies are connected with OR-operator, the first policy works to the direction to reduce number of visible rows, but the second policy works to the reverse direction. If we would have OR'd RLS-policy, how does it merged with user given qualifiers with? For example, if RLS-policy of t1 is (t1.credential get_user_credential) and user's query is: SELECT * FROM t1 WHERE t1.x = t1.x; Do you think RLS-policy shall be merged with OR'd form? Yeah, maybe. I intuitively feel that OR would be more useful, so it would be nice to find a design where that makes sense. But it depends a lot, in my view, on what syntax we end up with. For example, suppose we add just one command: ALTER TABLE table_name FILTER [ role_name | PUBLIC ] USING qual; If the given role inherits from multiple roles that have different filters, I think the user will naturally expect all of the filters to be applied. Agreed. But you could do it other ways. For example: ALTER TABLE table_name [ NO ] ROW LEVEL SECURITY; ALTER TABLE table_name GRANT ROW ACCESS TO role_name USING qual; If a table is set to NO ROW LEVEL SECURITY then it behaves just like it does now: anyone who accesses it sees all the rows, restricted to those columns for which they have permission. If the table is set to ROW LEVEL SECURITY then the default is to show no rows. The second command then allows access to a subset of the rows for a give role name. In this case, it is probably logical for access to be combined via OR. I can see value is having a table-level option to indicate if RLS is applied for that table or not, but I had been thinking we'd just automatically manage that. That is to say that once you define an RLS policy for a table, we go look and see what policy should be applied in each case. With the user able to control that, what happens if they say row security on the table and there are no policies? All access would show the table as empty? What if policies exist and they decide to 'turn off' RLS for the table- suddenly everyone can see all the rows? My answers to the above (which are making me like the idea more, actually...) would be: Yes, if they turn on RLS for the table and there aren't any policies, then the table appears empty for anyone with normal SELECT rights (table owner and superusers would still see everything). If policies exist and the user asks to turn off RLS, I'd throw an ERROR as there is a security risk there. We could support a CASCADE option which would go and drop the policies from the table first. Hmm... This approach starts from the empty permission then adds permission to reference a particular range of the configured table. It's one attitude. However, I think it has a dark side we cannot ignore. Usually, the purpose of security mechanism is to ensure which is readable/writable according to the rules. Once multiple RLS-policies are merged with OR'd form, its results are unpredicatable. Please assume here are two individual applications that use RLS on table-X. Even if application-1 want only rows being public become visible, it may expose credential or secret rows by interaction of orthogonal policy configured by application-2 (that may configure the policy according to the source ip-address). It seems to me application-2 partially invalidated the RLS-policy configured by application-1. I think, an important characteristic is things to be invisible is invisible even though multiple rules are configured. Otherwise, I'm generally liking Dean's thoughts in http://www.postgresql.org/message-id/CAEZATCVftksFH=X+9mVmBNMZo5KsUP+R k0kb4oro92jofjo...@mail.gmail.com along with the table-level enable RLS option. Are we getting to a point where there is
Re: [HACKERS] RLS Design
Kaigai, On Thursday, July 3, 2014, Kouhei Kaigai kai...@ak.jp.nec.com wrote: Sorry for my late responding, now I'm catching up the discussion. * Robert Haas (robertmh...@gmail.com javascript:;) wrote: On Tue, Jul 1, 2014 at 3:20 PM, Dean Rasheed dean.a.rash...@gmail.com javascript:; wrote: If RLS quals are instead regarded as constraints on access, and multiple policies apply, then it seems that the quals should now be combined with AND rather than OR, right? I do feel that RLS quals are constraints on access, but I don't see how it follows that multiple quals should be AND'd together because of that. I view the RLS policies on each table as being independent and standing alone regarding what can be seen. If you have access to a table today through policy A, and then later policy B is added, using AND would mean that the set of rows returned is less than if only policy A existed. That doesn't seem correct to me. It seems to me direction of the constraints (RLS-policy) works to is reverse. In case when we have no RLS-policy, 100% of rows are visible isn't it? No, as outlined later, the table would appear empty if no policies exist and RLS is enabled for the table. Addition of a constraint usually reduces the number of rows being visible, or same number of rows at least. Constraint shall never work to the direction to increase the number of rows being visible. Can you clarify where this is coming from..? It sounds like you're referring to an existing implementation and, if so, it'd be good to get more information on how that works exactly. If multiple RLS-policies are connected with OR-operator, the first policy works to the direction to reduce number of visible rows, but the second policy works to the reverse direction. This isn't accurate, as mentioned. Each policy stands alone to define what is visible through it and if no policy exists then no rows are visible. If we would have OR'd RLS-policy, how does it merged with user given qualifiers with? The RLS quals are all applied together with OR's and the result is AND'd with any user quals provided. This is only when multiple policies are being applied for a given query and seems pretty straight forward to me. For example, if RLS-policy of t1 is (t1.credential get_user_credential) and user's query is: SELECT * FROM t1 WHERE t1.x = t1.x; Do you think RLS-policy shall be merged with OR'd form? Only the RLS policies are OR'd together, not user provided quals. The above would result in: Where t1.x = t1.x and (t1.credential get_user_credential) If another policy also applies for this query, such as t1.cred2 get_user_credential then we would have: Where t1.x = t1.x and (t1.credential get_user_credential OR t1.cred2 get_user_credential) This is similar to how roles work- your overall access includes all access granted to any roles you are a member of. You don't need SELECT rights granted to every role you are a member of to select from the table. Additionally, if an admin wants to AND the quals together then they can simply create a policy which does that rather than have 2 policies. Yeah, maybe. I intuitively feel that OR would be more useful, so it would be nice to find a design where that makes sense. But it depends a lot, in my view, on what syntax we end up with. For example, suppose we add just one command: ALTER TABLE table_name FILTER [ role_name | PUBLIC ] USING qual; If the given role inherits from multiple roles that have different filters, I think the user will naturally expect all of the filters to be applied. Agreed. But you could do it other ways. For example: ALTER TABLE table_name [ NO ] ROW LEVEL SECURITY; ALTER TABLE table_name GRANT ROW ACCESS TO role_name USING qual; If a table is set to NO ROW LEVEL SECURITY then it behaves just like it does now: anyone who accesses it sees all the rows, restricted to those columns for which they have permission. If the table is set to ROW LEVEL SECURITY then the default is to show no rows. The second command then allows access to a subset of the rows for a give role name. In this case, it is probably logical for access to be combined via OR. I can see value is having a table-level option to indicate if RLS is applied for that table or not, but I had been thinking we'd just automatically manage that. That is to say that once you define an RLS policy for a table, we go look and see what policy should be applied in each case. With the user able to control that, what happens if they say row security on the table and there are no policies? All access would show the table as empty? What if policies exist and they decide to 'turn off' RLS for the table- suddenly everyone can see all the rows? My answers to the above (which are making me like the idea more, actually...) would be: Yes, if they turn on RLS for the
Re: [HACKERS] RLS Design
Kaigai, On Thursday, July 3, 2014, Kouhei Kaigai kai...@ak.jp.nec.com wrote: Sorry for my late responding, now I'm catching up the discussion. * Robert Haas (robertmh...@gmail.com javascript:; ) wrote: On Tue, Jul 1, 2014 at 3:20 PM, Dean Rasheed dean.a.rash...@gmail.com javascript:; wrote: If RLS quals are instead regarded as constraints on access, and multiple policies apply, then it seems that the quals should now be combined with AND rather than OR, right? I do feel that RLS quals are constraints on access, but I don't see how it follows that multiple quals should be AND'd together because of that. I view the RLS policies on each table as being independent and standing alone regarding what can be seen. If you have access to a table today through policy A, and then later policy B is added, using AND would mean that the set of rows returned is less than if only policy A existed. That doesn't seem correct to me. It seems to me direction of the constraints (RLS-policy) works to is reverse. In case when we have no RLS-policy, 100% of rows are visible isn't it? No, as outlined later, the table would appear empty if no policies exist and RLS is enabled for the table. Addition of a constraint usually reduces the number of rows being visible, or same number of rows at least. Constraint shall never work to the direction to increase the number of rows being visible. Can you clarify where this is coming from..? It sounds like you're referring to an existing implementation and, if so, it'd be good to get more information on how that works exactly. Oracle VPD - Multiple Policies for Each Table, View, or Synonym http://docs.oracle.com/cd/B19306_01/network.102/b14266/apdvpoli.htm#i1008351 It says - Note that all policies applied to a table are enforced with AND syntax. Not only Oracle VPD, it fits attitude of defense in depth. Please assume a system that installs network firewall, unix permissions and selinux. If somebody wants to reference an information asset within a file, he has to connect the server from the network address being allowed by the firewall configuration AND both of DAC and MAC has to allow his access. Usually, we have to pass all the access control to reference the target information, not one of the access control stuffs being installed. For example, if RLS-policy of t1 is (t1.credential get_user_credential) and user's query is: SELECT * FROM t1 WHERE t1.x = t1.x; Do you think RLS-policy shall be merged with OR'd form? Only the RLS policies are OR'd together, not user provided quals. The above would result in: Where t1.x = t1.x and (t1.credential get_user_credential) If another policy also applies for this query, such as t1.cred2 get_user_credential then we would have: Where t1.x = t1.x and (t1.credential get_user_credential OR t1.cred2 get_user_credential) This is similar to how roles work- your overall access includes all access granted to any roles you are a member of. You don't need SELECT rights granted to every role you are a member of to select from the table. Additionally, if an admin wants to AND the quals together then they can simply create a policy which does that rather than have 2 policies. It seems to me a pain on database administration, if we have to pay attention not to conflict each RLS-policy. I expect 90% of RLS-policy will be configured to PUBLIC user, to apply everybody same rules on access. In this case, DBA has to ensure the target table has no policy or existing policy does not conflict with the new policy to be set. I don't think it is a good idea to enforce DBA these checks. Please assume here are two individual applications that use RLS on table-X. Even if application-1 want only rows being public become visible, it may expose credential or secret rows by interaction of orthogonal policy configured by application-2 (that may configure the policy according to the source ip-address). It seems to me application-2 partially invalidated the RLS-policy configured by application-1. You are suggesting instead that if application 2 sets up policies on the table and then application 1 adds another policy that it should reduce what application 2's users can see? That doesn't make any sense to me. I'd actually expect these applications to at least use different roles anyway, which means they could each have a single role specific policy which only returns what that application is allowed to see. I don't think this assumption is reasonable. Please expect two applications: app-X that is a database security product to apply access control based on remote ip-address of the client for any table accesses by any database roles. app-Y that is a
Re: [HACKERS] RLS Design
On 01/07/14 21:51, Robert Haas wrote: On Tue, Jul 1, 2014 at 3:20 PM, Dean Rasheed dean.a.rash...@gmail.com wrote: That seems like a pretty strong argument. If RLS quals are instead regarded as constraints on access, and multiple policies apply, then it seems that the quals should now be combined with AND rather than OR, right? Yeah, maybe. I intuitively feel that OR would be more useful, so it would be nice to find a design where that makes sense. Looking at the use cases we described earlier in http://www.postgresql.org/message-id/attachment/32196/mini-rim.sql I see more OR than AND, for instance 'if the row is sensitive then the user must be related to the row' which translates to (NOT sensitive) OR the user is related. An addition to that rule could be a breakglass method or other reasons to access, e.g. (NOT sensitive) OR user is related OR break glass OR legally required access. But it depends a lot, in my view, on what syntax we end up with. For example, suppose we add just one command: ALTER TABLE table_name FILTER [ role_name | PUBLIC ] USING qual; If the given role inherits from multiple roles that have different filters, I think the user will naturally expect all of the filters to be applied. Suppose a building administrator gives a single person that has multiple roles multiple key cards to access appropriate rooms in a building. You could draw a venn diagram of the rooms those key cards open, and the intuition here probably is that the person can enter any room if one of the key cards matches, not all cards. But you could do it other ways. For example: ALTER TABLE table_name [ NO ] ROW LEVEL SECURITY; ALTER TABLE table_name GRANT ROW ACCESS TO role_name USING qual; If a table is set to NO ROW LEVEL SECURITY then it behaves just like it does now: anyone who accesses it sees all the rows, restricted to those columns for which they have permission. If the table is set to ROW LEVEL SECURITY then the default is to show no rows. The second command then allows access to a subset of the rows for a give role name. In this case, it is probably logical for access to be combined via OR. 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] RLS Design
* Robert Haas (robertmh...@gmail.com) wrote: On Tue, Jul 1, 2014 at 3:20 PM, Dean Rasheed dean.a.rash...@gmail.com wrote: If RLS quals are instead regarded as constraints on access, and multiple policies apply, then it seems that the quals should now be combined with AND rather than OR, right? I do feel that RLS quals are constraints on access, but I don't see how it follows that multiple quals should be AND'd together because of that. I view the RLS policies on each table as being independent and standing alone regarding what can be seen. If you have access to a table today through policy A, and then later policy B is added, using AND would mean that the set of rows returned is less than if only policy A existed. That doesn't seem correct to me. Yeah, maybe. I intuitively feel that OR would be more useful, so it would be nice to find a design where that makes sense. But it depends a lot, in my view, on what syntax we end up with. For example, suppose we add just one command: ALTER TABLE table_name FILTER [ role_name | PUBLIC ] USING qual; If the given role inherits from multiple roles that have different filters, I think the user will naturally expect all of the filters to be applied. Agreed. But you could do it other ways. For example: ALTER TABLE table_name [ NO ] ROW LEVEL SECURITY; ALTER TABLE table_name GRANT ROW ACCESS TO role_name USING qual; If a table is set to NO ROW LEVEL SECURITY then it behaves just like it does now: anyone who accesses it sees all the rows, restricted to those columns for which they have permission. If the table is set to ROW LEVEL SECURITY then the default is to show no rows. The second command then allows access to a subset of the rows for a give role name. In this case, it is probably logical for access to be combined via OR. I can see value is having a table-level option to indicate if RLS is applied for that table or not, but I had been thinking we'd just automatically manage that. That is to say that once you define an RLS policy for a table, we go look and see what policy should be applied in each case. With the user able to control that, what happens if they say row security on the table and there are no policies? All access would show the table as empty? What if policies exist and they decide to 'turn off' RLS for the table- suddenly everyone can see all the rows? My answers to the above (which are making me like the idea more, actually...) would be: Yes, if they turn on RLS for the table and there aren't any policies, then the table appears empty for anyone with normal SELECT rights (table owner and superusers would still see everything). If policies exist and the user asks to turn off RLS, I'd throw an ERROR as there is a security risk there. We could support a CASCADE option which would go and drop the policies from the table first. Otherwise, I'm generally liking Dean's thoughts in http://www.postgresql.org/message-id/CAEZATCVftksFH=x+9mvmbnmzo5ksup+rk0kb4oro92jofjo...@mail.gmail.com along with the table-level enable RLS option. Are we getting to a point where there is sufficient agreement that it'd be worthwhile to really start implementing this? I'd suggest that we either forgo or at least table the notion of per-column policy definitions- RLS controls whole rows and so I don't feel that per-column policies really make sense. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
On Wed, Jul 2, 2014 at 9:47 AM, Stephen Frost sfr...@snowman.net wrote: But you could do it other ways. For example: ALTER TABLE table_name [ NO ] ROW LEVEL SECURITY; ALTER TABLE table_name GRANT ROW ACCESS TO role_name USING qual; If a table is set to NO ROW LEVEL SECURITY then it behaves just like it does now: anyone who accesses it sees all the rows, restricted to those columns for which they have permission. If the table is set to ROW LEVEL SECURITY then the default is to show no rows. The second command then allows access to a subset of the rows for a give role name. In this case, it is probably logical for access to be combined via OR. I can see value is having a table-level option to indicate if RLS is applied for that table or not, but I had been thinking we'd just automatically manage that. That is to say that once you define an RLS policy for a table, we go look and see what policy should be applied in each case. With the user able to control that, what happens if they say row security on the table and there are no policies? All access would show the table as empty? I said the same thing in the text you quoted immediately above this reply. What if policies exist and they decide to 'turn off' RLS for the table- suddenly everyone can see all the rows? That'd be my vote. Sorta like disabling triggers. Are we getting to a point where there is sufficient agreement that it'd be worthwhile to really start implementing this? I think we're converging, but it might be a good idea to summarize a specific proposal before you start implementing. -- 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] RLS Design
* Robert Haas (robertmh...@gmail.com) wrote: On Wed, Jul 2, 2014 at 9:47 AM, Stephen Frost sfr...@snowman.net wrote: But you could do it other ways. For example: ALTER TABLE table_name [ NO ] ROW LEVEL SECURITY; ALTER TABLE table_name GRANT ROW ACCESS TO role_name USING qual; If a table is set to NO ROW LEVEL SECURITY then it behaves just like it does now: anyone who accesses it sees all the rows, restricted to those columns for which they have permission. If the table is set to ROW LEVEL SECURITY then the default is to show no rows. The second command then allows access to a subset of the rows for a give role name. In this case, it is probably logical for access to be combined via OR. I can see value is having a table-level option to indicate if RLS is applied for that table or not, but I had been thinking we'd just automatically manage that. That is to say that once you define an RLS policy for a table, we go look and see what policy should be applied in each case. With the user able to control that, what happens if they say row security on the table and there are no policies? All access would show the table as empty? I said the same thing in the text you quoted immediately above this reply. huh. Somehow I managed to only read the first sentence in that paragraph. Clearly I need to go get (more) coffee. Still- sounds like agreement. :) What if policies exist and they decide to 'turn off' RLS for the table- suddenly everyone can see all the rows? That'd be my vote. Sorta like disabling triggers. Hmm. Ok- how would you feel about at least spitting out a WARNING if there are still policies on the table in that case..? Just makes me a bit nervous to have a case where policies can be defined on a table but are not actually being enforced.. Are we getting to a point where there is sufficient agreement that it'd be worthwhile to really start implementing this? I think we're converging, but it might be a good idea to summarize a specific proposal before you start implementing. Right- will do so later today. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
On Wed, Jul 2, 2014 at 11:42 AM, Stephen Frost sfr...@snowman.net wrote: What if policies exist and they decide to 'turn off' RLS for the table- suddenly everyone can see all the rows? That'd be my vote. Sorta like disabling triggers. Hmm. Ok- how would you feel about at least spitting out a WARNING if there are still policies on the table in that case..? Just makes me a bit nervous to have a case where policies can be defined on a table but are not actually being enforced.. Sounds like nanny-ism 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] RLS Design
* Robert Haas (robertmh...@gmail.com) wrote: On Wed, Jul 2, 2014 at 11:42 AM, Stephen Frost sfr...@snowman.net wrote: What if policies exist and they decide to 'turn off' RLS for the table- suddenly everyone can see all the rows? That'd be my vote. Sorta like disabling triggers. Hmm. Ok- how would you feel about at least spitting out a WARNING if there are still policies on the table in that case..? Just makes me a bit nervous to have a case where policies can be defined on a table but are not actually being enforced.. Sounds like nanny-ism to me. Alright, fair enough. Clearly, the individual changing the RLS on the table will have to have appropriate rights to do so. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
Robert, all, * Robert Haas (robertmh...@gmail.com) wrote: I think we're converging, but it might be a good idea to summarize a specific proposal before you start implementing. Alright, apologies for it being a bit later than intended, but here's what I've come up with thus far. -- policies defined at a table scope -- allows using the same policy name for different tables -- with quals appropriate for each table ALTER TABLE t1 ADD POLICY p1 USING p1_quals; ALTER TABLE t1 ADD POLICY p2 USING p2_quals; -- used to drop a policy definition from a table ALTER TABLE t1 DROP POLICY p1; -- cascade required when references exist for the policy -- from roles ALTER TABLE t1 DROP POLICY p1 CASCADE; ALTER TABLE t1 ALTER POLICY p1 USING new_quals; -- Controls if any RLS is applied to this table or not -- If enabled, all users must access through some policy ALTER TABLE table_name [ NO ] ROW LEVEL SECURITY; -- Associates roles to policies ALTER TABLE table_name GRANT ROW ACCESS TO role_name USING p1; ALTER TABLE table_name REVOKE ROW ACCESS FROM role_name USING p1; -- all provides a policy which equates to full access (eg: 'true' or -- 'direct' access). Used to explicitly state when RLS can be bypassed -- and therefore a GUC can be set which says bypass-RLS-or-error and -- not have an error if this policy is granted to the role. ALTER TABLE table_name GRANT ROW ACCESS TO role_name USING all; -- Per-command-type control ALTER TABLE table_name GRANT SELECT ROW ACCESS TO role_name USING all; ALTER TABLE table_name GRANT UPDATE ROW ACCESS TO role_name USING all; Policies for a table are checked against pg_has_role() and all which apply are OR'd together. Added to pg_class: relrlsenabled boolean pg_rowsecurity oid oid rlsrel oid rlspol name rlsquals text rlsacls aclitem[]..? cmdtype(s) + role If relrlsenabled then scan pg_rowsecurity for the policies associated with the table, testing each to see if any apply for the current role based on pg_has_role() against the aclitem array. Any which apply are added and OR'd together. Thoughts? Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
On 29 June 2014 20:42, Stephen Frost sfr...@snowman.net wrote: To try and clarify what this distinction is- Dean's approach with GRANT allows specifying the policy to be used when a given role queries a given table. Through this mechanism, one role might have access to many different tables, possibly with a different policy granting that access for each table. Robert's approach defines a policy for a user and that policy is used for all tables that user accesses. This ties the policy very closely to the role. Actually I think they were both originally Robert's ideas in one form or another, but at this point I'm losing track a bit :-) With either approach, I wonder how we are going to address the role membership question. Do you inherit policies through role membership? What happens on 'set role'? Robert points out that we should be using OR for these situations of overlapping policies and I tend to agree. Therefore, we would look at the RLS policies for a table and extract out all of them for all of the roles which the current user is a member of, OR them together and that would be the set of quals used. Yes I think that's right. I had hoped to avoid overlapping policies, but maybe they're more-or-less inevitable and we should just allow them. It seems justifiable in terms of GRANTs --- one GRANT gives you permission to access one set of rows from a table, another GRANT gives you permission to access another set of rows, so in the end you have access to the union of both sets. I'm leaning towards Dean's approach. With Robert's approach, one could emulate Dean's approach but I suspect it would devolve quickly into one policy per user with that policy simply being a proxy for the role instead of being useful on its own. With Dean's approach though, I don't think there's a need for a policy to be a stand-alone object. The policy is simply a proxy for the set of quals to be added and therefore the policy could really live as a per-table object. Yes I think that's right too. I had thought that stand-alone policies would be useful for selecting which policies to apply to each role, but maybe that's not necessary if you rely entirely on GRANTs to decide which policies apply. That means the syntax I proposed earlier is wrong/misleading. Instead of GRANT SELECT ON TABLE tbl TO role USING polname; it should really be GRANT SELECT USING polname ON TABLE tbl TO role; This would work, though the 'polname' could be a per-table object, no? Right. This could even be: GRANT SELECT USING (sec_level=manager) ON TABLE tbl TO role; Maybe. The important thing is that it's granting the role access to a {table,command,policy} set or equivalently a {table,command,quals} set --- i.e., the right to access a sub-set of the table's rows with a particular command. Let's explore this further to see where it leads. In some ways, I think it has ended up even simpler than I thought. To setup RLS, you would just need to do 2 things: 1). Add a bunch of RLS policies to your tables (not connected to any particular commands, since that is done using GRANTs). This could use Robert's earlier syntax: ALTER TABLE t1 ADD POLICY p1 WHERE p1_quals; ALTER TABLE t1 ADD POLICY p2 WHERE p2_quals; ... (or some similar syntax) where the policy names p1 and p2 need only be unique within the table. For maintenance purposes you'd also need to be able to do ALTER TABLE t1 DROP POLICY pol; and maybe in the future we'd support ALTER TABLE t1 ALTER POLICY pol TO new_quals; 2). Once each table has the required set of policies, grant each role permissions, specifying the allowed commands and policies together: GRANT SELECT USING p1 ON TABLE t1 TO role1; GRANT SELECT USING p2 ON TABLE t1 TO role1; GRANT UPDATE USING p3 ON TABLE t1 TO role1; ... (or some similar syntax) So in this example, if role1 SELECTed from t1, the system would automatically apply the combined quals (p1_quals OR p2_quals), whereas when role1 UPDATEd t1, it would apply p3_quals. So that takes care of the different-quals-for-different-commands requirement without even needing any special syntax for it in ALTER TABLE. A straight GRANT SELECT ON TABLE .. TO .. would grant access to the whole table without any RLS quals, as it always has done, which is good because it means nothing changes for users who aren't interested in RLS. Finally, pg_dump would require a GUC to ensure that RLS was not in effect. Perhaps something like SET require_direct_table_access = true, which would cause an error to be thrown if the user hadn't been granted straight select permissions on the tables in question. That all seems relatively easy to understand, whilst giving a lot of flexibility. An annoying complication, however, is how this interacts with column privileges. Right now GRANT SELECT(col1) ON t1 TO role1 gives role1 access to every row in col1, and I think that has to remain the case, since GRANTs only ever give you more access. But that
Re: [HACKERS] RLS Design
On Tue, Jul 1, 2014 at 3:33 AM, Dean Rasheed dean.a.rash...@gmail.com wrote: An annoying complication, however, is how this interacts with column privileges. Right now GRANT SELECT(col1) ON t1 TO role1 gives role1 access to every row in col1, and I think that has to remain the case, since GRANTs only ever give you more access. But that leads to a situation where the RLS quals applied would depend on the columns selected. Wow, that seems pretty horrible to me. That means that if I do: SELECT a FROM tab; and then: SELECT a, b FROM tab; ...the second one might return fewer rows than the first one. I think there's a good argument that RLS is unlike other grantable privileges, and that it really ought to be defined as something which is imposed rather than a kind of access grant. If RLS is merely a modifier to an access grant, then every access grant has to make sure to include that modifier, or you have a security hole. But if it's a separate constrain on access, then you just do it once, and exempt people from it only as needed. That seems less error-prone to me -- it's sort of a default-deny policy, which is generally viewed as good for security -- and it avoids weird cases like the above, which I think could easily break application logic. -- 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] RLS Design
On 1 July 2014 17:42, Robert Haas robertmh...@gmail.com wrote: On Tue, Jul 1, 2014 at 3:33 AM, Dean Rasheed dean.a.rash...@gmail.com wrote: An annoying complication, however, is how this interacts with column privileges. Right now GRANT SELECT(col1) ON t1 TO role1 gives role1 access to every row in col1, and I think that has to remain the case, since GRANTs only ever give you more access. But that leads to a situation where the RLS quals applied would depend on the columns selected. Wow, that seems pretty horrible to me. That means that if I do: SELECT a FROM tab; and then: SELECT a, b FROM tab; ...the second one might return fewer rows than the first one. I think there's a good argument that RLS is unlike other grantable privileges, and that it really ought to be defined as something which is imposed rather than a kind of access grant. If RLS is merely a modifier to an access grant, then every access grant has to make sure to include that modifier, or you have a security hole. But if it's a separate constrain on access, then you just do it once, and exempt people from it only as needed. That seems less error-prone to me -- it's sort of a default-deny policy, which is generally viewed as good for security -- and it avoids weird cases like the above, which I think could easily break application logic. That seems like a pretty strong argument. If RLS quals are instead regarded as constraints on access, and multiple policies apply, then it seems that the quals should now be combined with AND rather than OR, right? 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] RLS Design
On Tue, Jul 1, 2014 at 3:20 PM, Dean Rasheed dean.a.rash...@gmail.com wrote: On 1 July 2014 17:42, Robert Haas robertmh...@gmail.com wrote: On Tue, Jul 1, 2014 at 3:33 AM, Dean Rasheed dean.a.rash...@gmail.com wrote: An annoying complication, however, is how this interacts with column privileges. Right now GRANT SELECT(col1) ON t1 TO role1 gives role1 access to every row in col1, and I think that has to remain the case, since GRANTs only ever give you more access. But that leads to a situation where the RLS quals applied would depend on the columns selected. Wow, that seems pretty horrible to me. That means that if I do: SELECT a FROM tab; and then: SELECT a, b FROM tab; ...the second one might return fewer rows than the first one. I think there's a good argument that RLS is unlike other grantable privileges, and that it really ought to be defined as something which is imposed rather than a kind of access grant. If RLS is merely a modifier to an access grant, then every access grant has to make sure to include that modifier, or you have a security hole. But if it's a separate constrain on access, then you just do it once, and exempt people from it only as needed. That seems less error-prone to me -- it's sort of a default-deny policy, which is generally viewed as good for security -- and it avoids weird cases like the above, which I think could easily break application logic. That seems like a pretty strong argument. If RLS quals are instead regarded as constraints on access, and multiple policies apply, then it seems that the quals should now be combined with AND rather than OR, right? Yeah, maybe. I intuitively feel that OR would be more useful, so it would be nice to find a design where that makes sense. But it depends a lot, in my view, on what syntax we end up with. For example, suppose we add just one command: ALTER TABLE table_name FILTER [ role_name | PUBLIC ] USING qual; If the given role inherits from multiple roles that have different filters, I think the user will naturally expect all of the filters to be applied. But you could do it other ways. For example: ALTER TABLE table_name [ NO ] ROW LEVEL SECURITY; ALTER TABLE table_name GRANT ROW ACCESS TO role_name USING qual; If a table is set to NO ROW LEVEL SECURITY then it behaves just like it does now: anyone who accesses it sees all the rows, restricted to those columns for which they have permission. If the table is set to ROW LEVEL SECURITY then the default is to show no rows. The second command then allows access to a subset of the rows for a give role name. In this case, it is probably logical for access to be combined via OR. -- 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] RLS Design
On Sun, Jun 29, 2014 at 3:42 PM, Stephen Frost sfr...@snowman.net wrote: An interesting question we haven't much considered is: who can set up policies and add then to users? Maybe we should flip this around, and instead of adding users to policies, we should exempt users from policies. CREATE POLICY p1; And then, if they own p1 and t1, they can do: ALTER TABLE t1 SET POLICY p1 TO t1_p1_quals; (or maybe we should associate it to the policy instead of the table: ALTER POLICY p1 SET TABLE t1 TO t1_p1_quals) And then the policy applies to everyone who doesn't have the grantable EXEMPT privilege on the policy. The policy owner and superuser have that privilege by default and it can be handed out to others like this: GRANT EXEMPT ON POLICY p1 TO snowden; Then users who have row_level_security=on will bypass RLS if possible, and otherwise it will be applied. Users who have row_level_security=off will bypass RLS if possible, and otherwise error. And users who have row_level_security=force will apply RLS even if they are entitled to bypass it. That's interesting. I need to think some more about what that means. I'm not a fan of the EXEMPT approach.. Just out of curiosity, why not? -- 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] RLS Design
* Robert Haas (robertmh...@gmail.com) wrote: On Sun, Jun 29, 2014 at 3:42 PM, Stephen Frost sfr...@snowman.net wrote: An interesting question we haven't much considered is: who can set up policies and add then to users? Maybe we should flip this around, and instead of adding users to policies, we should exempt users from policies. CREATE POLICY p1; And then, if they own p1 and t1, they can do: ALTER TABLE t1 SET POLICY p1 TO t1_p1_quals; (or maybe we should associate it to the policy instead of the table: ALTER POLICY p1 SET TABLE t1 TO t1_p1_quals) And then the policy applies to everyone who doesn't have the grantable EXEMPT privilege on the policy. The policy owner and superuser have that privilege by default and it can be handed out to others like this: GRANT EXEMPT ON POLICY p1 TO snowden; Then users who have row_level_security=on will bypass RLS if possible, and otherwise it will be applied. Users who have row_level_security=off will bypass RLS if possible, and otherwise error. And users who have row_level_security=force will apply RLS even if they are entitled to bypass it. That's interesting. I need to think some more about what that means. I'm not a fan of the EXEMPT approach.. Just out of curiosity, why not? I don't see it as really solving the flexibility need and it feels quite a bit more complicated to reason about. Would someone who is EXEMPT from one policy on a given table still have other policies on that table applied to them? Would a user be able to be EXEMPT from multiple policies? I feel like that's what you're suggesting with this approach, otherwise I don't see it as really different from the 'DIRECT SELECT' privilege discussed previously.. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
On Mon, Jun 30, 2014 at 9:42 AM, Stephen Frost sfr...@snowman.net wrote: I'm not a fan of the EXEMPT approach.. Just out of curiosity, why not? I don't see it as really solving the flexibility need and it feels quite a bit more complicated to reason about. Would someone who is EXEMPT from one policy on a given table still have other policies on that table applied to them? Yes; otherwise, EXEMPT couldn't be granted by non-superusers, and the whole point of that proposal was to come up with something that would be clearly safe for ordinary users to use. Would a user be able to be EXEMPT from multiple policies? Yes, clearly. It would be a privilege on the policy object, so different objects can have different privileges. I feel like that's what you're suggesting with this approach, otherwise I don't see it as really different from the 'DIRECT SELECT' privilege discussed previously.. Right. If you took that away, it wouldn't be different. The number of possible approaches here has expanded beyond what I can keep in my head; I'm assuming you are planning to think this over and propose something comprehensive, or maybe Dean or someone else will do that. But I'm not sure that all the approaches proposed would make it safe for non-superusers to use RLS, and I think it would be good if they could. -- 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] RLS Design
* Robert Haas (robertmh...@gmail.com) wrote: On Mon, Jun 30, 2014 at 9:42 AM, Stephen Frost sfr...@snowman.net wrote: I don't see it as really solving the flexibility need and it feels quite a bit more complicated to reason about. Would someone who is EXEMPT from one policy on a given table still have other policies on that table applied to them? Yes; otherwise, EXEMPT couldn't be granted by non-superusers, and the whole point of that proposal was to come up with something that would be clearly safe for ordinary users to use. I'm confused on this part- granting EXEMPT and/or DIRECT SELECT would definitely need to be supported by a non-superuser, though someone who had the appropriate rights on the object involved (either the policy or the table, depending on where we feel that definition should go). Would a user be able to be EXEMPT from multiple policies? Yes, clearly. It would be a privilege on the policy object, so different objects can have different privileges. Ok.. then I'm not entirely sure how this is different from Dean's proposal except that it's a way of defining the inverse, which we don't do anywhere else in the system today.. I feel like that's what you're suggesting with this approach, otherwise I don't see it as really different from the 'DIRECT SELECT' privilege discussed previously.. Right. If you took that away, it wouldn't be different. Ok. The number of possible approaches here has expanded beyond what I can keep in my head; I'm assuming you are planning to think this over and propose something comprehensive, or maybe Dean or someone else will do that. But I'm not sure that all the approaches proposed would make it safe for non-superusers to use RLS, and I think it would be good if they could. I've been thinking about it quite a bit over the past few days (weeks?) and trying to continue to outline the proposals as they've changed. I'll try and work up another comprehensive email which covers the options currently under discussion as I understand them. Allowing non-superuser to use RLS is absolutely key to this in any case- it'd be great if you could voice any specific concerns you see there. We've already been working through the GUCs previously discussed, as I feel those will be necessary for any of these approaches (in particular the bypass RLS-or-error GUC which pg_dump will enable by default). Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
Robert, Dean, * Dean Rasheed (dean.a.rash...@gmail.com) wrote: On 26 June 2014 18:04, Robert Haas robertmh...@gmail.com wrote: ALTER TABLE t1 SET POLICY p1 ON SELECT TO t1_p1_sel_quals; GRANT SELECT ON TABLE t1 TO role1 USING p1; As I see it, the downside of this is that it gets a lot more complex. We have to revise the ACL representation, which is already pretty darn complicated, to keep track not only of the grantee, grantor, and permissions, but also the policies qualifying those permissions. The changes to GRANT will need to propagate into GRANT ON ALL TABLES IN SCHEMA and AFTER DEFAULT PRIVILEGES. No, it can be done without any changes to the permissions code by storing the ACLs on the catalog entries where the RLS quals are held, rather than modifying the ACL items on the table. I.e., instead of thinking of USING polname as a modifier to the grant, think of it as as an additional qualifier on the thing being granted. Yeah, I agree that we could do it without changing the existing ACL structure by using another table and having a flag in pg_class which indicates if there are RLS policies on the table or not. Regarding the concerns about users not using the RLS capabilities correctly- I find that concern to be much more appropriate for the current permissions system rather than RLS. If a user is going to the level of even looking at RLS then, I'd hope at least, they'll be able to understand and make good use of RLS to implement what they need and they would appreciate the flexibility. To try and clarify what this distinction is- Dean's approach with GRANT allows specifying the policy to be used when a given role queries a given table. Through this mechanism, one role might have access to many different tables, possibly with a different policy granting that access for each table. Robert's approach defines a policy for a user and that policy is used for all tables that user accesses. This ties the policy very closely to the role. With either approach, I wonder how we are going to address the role membership question. Do you inherit policies through role membership? What happens on 'set role'? Robert points out that we should be using OR for these situations of overlapping policies and I tend to agree. Therefore, we would look at the RLS policies for a table and extract out all of them for all of the roles which the current user is a member of, OR them together and that would be the set of quals used. I'm leaning towards Dean's approach. With Robert's approach, one could emulate Dean's approach but I suspect it would devolve quickly into one policy per user with that policy simply being a proxy for the role instead of being useful on its own. With Dean's approach though, I don't think there's a need for a policy to be a stand-alone object. The policy is simply a proxy for the set of quals to be added and therefore the policy could really live as a per-table object. That means the syntax I proposed earlier is wrong/misleading. Instead of GRANT SELECT ON TABLE tbl TO role USING polname; it should really be GRANT SELECT USING polname ON TABLE tbl TO role; This would work, though the 'polname' could be a per-table object, no? This could even be: GRANT SELECT USING (sec_level=manager) ON TABLE tbl TO role; There is administrative complexity as well, because if you want to policy-protect an additional table, you've got to add the table to the policy and then update all the grants as well. I think what will happen in practice is that people will grant to PUBLIC all rights on the policy, and then do all the access control through the GRANT statements. I agree that if you want to policy protect a table that you'll need to set the policies on the table (that's required either way) and that, with Dean's approach, you'd have to modify the GRANTs done to that table as well. I don't follow what you're suggesting with granting to PUBLIC all rights on the policy though..? With your approach though, if you have a policy which covers all managers and one which covers all VPs and then you have one VP whose access should be different, you'd have to create a new policy just for that VP and then modify all of the tables which have manager/VP access to also have that new VP's policy too, or something along those lines, no? If you assume that most users will only have one policy through which they can access any given table, then there is no more administrative overhead than we have right now. Right now you have to grant each user permissions on each table you define. The only difference is that now you throw in a USING polname. We could also simplify administration by supporting GRANT SELECT USING polname ON ALL TABLES IN SCHEMA sch TO role; The important distinction is that this is only granting permissions on tables that exist now, not on tables that might be created later. Sure, that's the same as it is now.. Robert's correct, imv,
Re: [HACKERS] RLS Design
On 26 June 2014 18:04, Robert Haas robertmh...@gmail.com wrote: ALTER TABLE t1 SET POLICY p1 ON SELECT TO t1_p1_sel_quals; GRANT SELECT ON TABLE t1 TO role1 USING p1; As I see it, the downside of this is that it gets a lot more complex. We have to revise the ACL representation, which is already pretty darn complicated, to keep track not only of the grantee, grantor, and permissions, but also the policies qualifying those permissions. The changes to GRANT will need to propagate into GRANT ON ALL TABLES IN SCHEMA and AFTER DEFAULT PRIVILEGES. No, it can be done without any changes to the permissions code by storing the ACLs on the catalog entries where the RLS quals are held, rather than modifying the ACL items on the table. I.e., instead of thinking of USING polname as a modifier to the grant, think of it as as an additional qualifier on the thing being granted. That means the syntax I proposed earlier is wrong/misleading. Instead of GRANT SELECT ON TABLE tbl TO role USING polname; it should really be GRANT SELECT USING polname ON TABLE tbl TO role; There is administrative complexity as well, because if you want to policy-protect an additional table, you've got to add the table to the policy and then update all the grants as well. I think what will happen in practice is that people will grant to PUBLIC all rights on the policy, and then do all the access control through the GRANT statements. If you assume that most users will only have one policy through which they can access any given table, then there is no more administrative overhead than we have right now. Right now you have to grant each user permissions on each table you define. The only difference is that now you throw in a USING polname. We could also simplify administration by supporting GRANT SELECT USING polname ON ALL TABLES IN SCHEMA sch TO role; The important distinction is that this is only granting permissions on tables that exist now, not on tables that might be created later. An interesting question we haven't much considered is: who can set up policies and add then to users? Maybe we should flip this around, and instead of adding users to policies, we should exempt users from policies. CREATE POLICY p1; And then, if they own p1 and t1, they can do: ALTER TABLE t1 SET POLICY p1 TO t1_p1_quals; (or maybe we should associate it to the policy instead of the table: ALTER POLICY p1 SET TABLE t1 TO t1_p1_quals) And then the policy applies to everyone who doesn't have the grantable EXEMPT privilege on the policy. The policy owner and superuser have that privilege by default and it can be handed out to others like this: GRANT EXEMPT ON POLICY p1 TO snowden; Then users who have row_level_security=on will bypass RLS if possible, and otherwise it will be applied. Users who have row_level_security=off will bypass RLS if possible, and otherwise error. And users who have row_level_security=force will apply RLS even if they are entitled to bypass it. That's interesting. I need to think some more about what that means. 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] RLS Design
On Wed, Jun 25, 2014 at 4:48 PM, Dean Rasheed dean.a.rash...@gmail.com wrote: Instead of doing it this way, we could instead do: ALTER ROLE role1 ADD POLICY p1; ALTER ROLE role2 ADD POLICY p2; We could possibly allow multiple policies to be set for the same user, but given an error (or OR the quals together) if there are conflicting policies for the same table. A user with no policies would see everything to which they've been granted access. I'm a bit uneasy about allowing overlapping policies like this, because I think it is more likely to lead to unintended consequences than solve real use cases. For example, suppose you define policies p1 and p2 and set them up on table t1, and you grant role1 permissions on t1 and allow role1 the use of policy p1. Then you set up policy p2 on another table t2, and decide you want to allow role1 access to t2 using this policy. The only way to do it is to add p2 to role1, but doing so also then gives role1 access to t1 using p2, which might not have been what you intended. I guess that's true but it just seems like a configuration error. I have it in mind that most people will define policies for non-overlapping sets of tables and then apply those policies as appropriate to each user. Whether that's true or not, I don't see it as being materially different from granting membership in a role - you could easily give the user permission to do stuff they shouldn't be able to do, but if you don't carefully examine the bundle of privileges that come with that GRANT before executing on it, that's your fault, not the system's. To support different policies on different operations, you could have something like: ALTER TABLE t1 SET POLICY p1 ON INSERT TO t1_p1_quals; Without the ON clause, it would establish the given policy for all operations. Yes, that makes sense. But as I was arguing above, I think the ACLs should be attached to the specific RLS policy identified uniquely by (table, policy, command). So, for example, if you did ALTER TABLE t1 SET POLICY p1 ON SELECT TO t1_p1_sel_quals; ALTER TABLE t1 SET POLICY p1 ON UPDATE TO t1_p1_upd_quals; you could also do GRANT SELECT ON TABLE t1 TO role1 USING p1; GRANT UPDATE ON TABLE t1 TO role1 USING p1; but it would be an error to do GRANT DELETE ON TABLE t1 TO role1 USING p1; As I see it, the downside of this is that it gets a lot more complex. We have to revise the ACL representation, which is already pretty darn complicated, to keep track not only of the grantee, grantor, and permissions, but also the policies qualifying those permissions. The changes to GRANT will need to propagate into GRANT ON ALL TABLES IN SCHEMA and AFTER DEFAULT PRIVILEGES. There is administrative complexity as well, because if you want to policy-protect an additional table, you've got to add the table to the policy and then update all the grants as well. I think what will happen in practice is that people will grant to PUBLIC all rights on the policy, and then do all the access control through the GRANT statements. An interesting question we haven't much considered is: who can set up policies and add then to users? Maybe we should flip this around, and instead of adding users to policies, we should exempt users from policies. CREATE POLICY p1; And then, if they own p1 and t1, they can do: ALTER TABLE t1 SET POLICY p1 TO t1_p1_quals; (or maybe we should associate it to the policy instead of the table: ALTER POLICY p1 SET TABLE t1 TO t1_p1_quals) And then the policy applies to everyone who doesn't have the grantable EXEMPT privilege on the policy. The policy owner and superuser have that privilege by default and it can be handed out to others like this: GRANT EXEMPT ON POLICY p1 TO snowden; Then users who have row_level_security=on will bypass RLS if possible, and otherwise it will be applied. Users who have row_level_security=off will bypass RLS if possible, and otherwise error. And users who have row_level_security=force will apply RLS even if they are entitled to bypass it. -- 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] RLS Design
On 25 June 2014 01:49, Stephen Frost sfr...@snowman.net wrote: Dean, all, Changing the subject of this thread (though keeping it threaded) as we've really moved on to a much broader discussion. * Dean Rasheed (dean.a.rash...@gmail.com) wrote: On 24 June 2014 17:27, Stephen Frost sfr...@snowman.net wrote: Single policy vs Multiple, Overlapping policies vs Multiple, Non-overlapping policies What I was describing upthread was multiple non-overlapping policies. Ok. I disagree that this will be more complicated to use. It's a strict superset of the single policy functionality, so if you want to do it all using a single policy then you can. But I think that once the ACLs reach a certain level of complexity, you probably will want to break it up into multiple policies, and I think doing so will make things simpler, not more complicated. If we keep it explicitly to per-role only, with only one policy ever being applied, then perhaps it would be, but I'm not convinced.. Taking a specific, simplistic example, suppose you had 2 groups of users - some are normal users who should only be able to access their own records. For these users, you might have a policy like WHERE person_id = current_user which would be highly selective, and probably use an index scan. Then there might be another group of users who are managers with access to the records of, say, everyone in their department. This might then be a more complex qual along the lines of WHERE person_id IN (SELECT ... FROM person_department WHERE mgr_id = current_user AND ...) which might end up being a hash or merge join, depending on any user-supplied quals. Certainly my experience with such a setup is that it includes at least 4 levels (self, manager, director, officer). Now, officer you could perhaps exclude as being simply RLS-exempt but with such a structure I would think we'd just make that a special kind of policy (and not chew up those last 4 bits). As for this example, it's quite naturally done with a recursive query as it's a tree structure, but if you want to keep the qual simple and fast, you'd materialize the results of such a query and simply have: WHERE EXISTS (SELECT 1 from org_chart WHERE current_user = emp_id AND person_id = org_chart.id) You _could_ combine those into a single policy, but I think it would be much better to have 2 distinct policies, since they're 2 very different queries, for different use cases. Normal users would only be granted permission to use the normal_user_policy. Managers might be granted permission to use either the normal_user_policy or the manager_policy (but not both at the same time). I can't recall a system where managers have to request access to their manager role. Having another way of changing the permissions which are applied to a session (the existing one being 'set role') doesn't strike me as a great idea either. Actually I think it's quite common to build applications where more privileged users might want to initially log in with normal privileges, and then only escalate to a higher privilege level if needed (much like only being root on a machine when absolutely necessary). But as you say, that can be done through 'set role' so I don't think being able to choose between policies is as important as being able to define different policies for different roles. That's a very simplified example. In more realistic situations there are likely to be many more classes of users, and trying to enforce all the logic in a single WHERE clause is likely to get unmanageable, or inefficient if it involves lots of logic hidden away in functions. Functions and external security systems are exactly the real-world use-case which users I've talked to are looking for. All of this discussion is completely orthogonal to their requirements. I understand that there are simpler use-cases than those and we may be able to provide an approach which performs better for those. OK. Allowing multiple, non-overlapping policies allows the problem to be broken up into more manageable pieces, which also makes the planner's job easier, since only a single, simpler policy is in effect in any given query. Let's try to outline what this would look like then. Taking your approach, we'd have: CREATE POLICY p1; CREATE POLICY p2; ALTER TABLE t1 SET POLICY p1 TO t1_p1_quals; ALTER TABLE t1 SET POLICY p2 TO t1_p2_quals; GRANT SELECT ON TABLE t1 TO role1 USING p1; GRANT SELECT ON TABLE t1 TO role2 USING p2; I'm guessing we would need to further support: GRANT INSERT ON TABLE t1 TO role1 USING p2; as we've already discussed being able to support per-action (SELECT, INSERT, UPDATE, DELETE) policies. I'm not quite sure how to address that though. Further, as you mention, users would be able to do: SET rls_policy = whatever; and things would appear fine, until they tried to
Re: [HACKERS] RLS Design
* Dean Rasheed (dean.a.rash...@gmail.com) wrote: On 25 June 2014 01:49, Stephen Frost sfr...@snowman.net wrote: I can't recall a system where managers have to request access to their manager role. Having another way of changing the permissions which are applied to a session (the existing one being 'set role') doesn't strike me as a great idea either. Actually I think it's quite common to build applications where more privileged users might want to initially log in with normal privileges, and then only escalate to a higher privilege level if needed (much like only being root on a machine when absolutely necessary). But as you say, that can be done through 'set role' so I don't think being able to choose between policies is as important as being able to define different policies for different roles. For those kinds of applications (eg: sudo), yes. I was, perhaps, looking at your example a bit too literally- I was thinking of HR management type systems (timecard systems, etc). You mention: GRANT SELECT (col1, col2), UPDATE (col1) ON t1 TO bob USING policy1; but, to be clear, there would be no option for policies to be column-specific, right? The policy would apply to the whole row and just the SELECT/UPDATE privileges would be on the specific columns (as exists today). I think that would be OK for the first release. It could be extended in a future release to support column-specific policy ACLs, as long as we don't preclude that in the syntax we choose now. The syntax GRANT command [,command] ON table TO role USING policy works because columns can be added to it later. What would per-column RLS policies mean..? Would we have to work out which columns are being updated vs. select'd on before being able to choose the policy/quals to include? Seems like that's probably workable but I've not thought about it very hard. From this what I'm gathering is that we'd need catalog tables along these lines: rls_policy oid, polname name, polowner oid, polnamespace oid, polacl aclitme[] (oid, policy name, policy owner, policy namespace, ACL, eg: usage?) rls_policy_table ptblpolid oid, ptblrelid oid, ptblquals text(?), ptblacl aclitem[]? (policy oid, table/relation oid, quals, ACL) pg_class relhasrls boolean ? Seems about right. An extension to the existing ACLs which are for GRANT to include a policy OID, eg: typedef struct AclItem { Oid ai_grantee; Oid ai_grantor; AclMode ai_privs; Oid rls_policy; } Alternatively, use the ACLs on rls_policy_table - i.e., to SELECT from a table using a particular policy, you would need to have the SELECT bit assigned to you in the corresponding rls_policy_table entry's ACLs. That seems like it would be a less invasive change, but I don't know if there are other problems with that approach. Ah, that's a good thought. My original thinking for that column was some kind of privilege structure around who is allowed to modify the quals for a given policy+table, but using that as the definition of who has what policies does make sense and means we can leave AclItem more-or-less alone, which is very nice. The relhasrls boolean would allow us to only query that catalog in cases where a policy exists, hopefully minimizing the impact for users who are not using RLS. and further: role1=r|p1/postgres role2=r|p2/postgres Or just table1: role1=rw/grantor table1 using policy1: role2=rw/grantor to avoid changing the privilege display pattern. That's also more in keeping with the model of storing the per-policy ACLs in rls_policy_table. I like that output, but do we expect any pushback from users who parse out that field? Admittedly, they really shouldn't be doing that, but I'm sure most actually do, and table1 using policy1 isn't terribly nice to parse. or even: bob=|policy1/postgres with no table-level privileges and only column-level privileges granted to role3 for this table. I don't get that last one. If there are no table-level privileges, would it not just be empty? No, as there could be column-level privileges. Note that table-level privileges get you access to all columns, and column level privileges (as defined by SQL spec) give you access even if you don't have any table-level privileges. As I was trying to exclude the notion of column-level policies, I figured policies would always show up in the table level ACL fields, but if there aren't any table-level rights, what would that look like? With your proposal, it'd be: table1 using policy1: bob=/grantor ? The plan cache would include what policy OID a given plan was run under (with InvalidOid indicating an everything-allowed policy). This doesn't address the concern raised about having different policies depending on the action type (SELECT, INSERT, etc) though, as mentioned above..
Re: [HACKERS] RLS Design the rewriter into the planner?
Robert, all, Changing the thread topic to match the other one, and adding Dean in explicitly since we're talking about the design discussed with him. * Robert Haas (robertmh...@gmail.com) wrote: I think role is good enough. That's the primary identifier for all access-control related decisions, so it should be good enough here, too. Alright. That works for me. I don't really understand your concerns about overlapping policies being complex. If you've got a couple of WHERE clauses, combining them with OR is not hard. Now, the query optimizer may have trouble with it, but on the whole I expect to win more than we lose, by entirely excluding some branches of an OR for users for whom entirely policies can be excluded. On the thread with Dean we're proposing some specific catalog designs and part of that included (fleshing it out a bit more) something like: CREATE TABLE pg_relrlspolicy (-- relation RLS policy table ptblrelid oid, -- Relation/table ptblaction text,-- SELECT, INSERT, UPDATE, DELETE ptblpolid oid, -- Policy ptblquals text,-- Quals to add ptblaclaclitem[], -- Rights to use this policy on the table primary key (ptblrelid, ptblaction) ); And note that I had expected aclitem to only include one entry per role. To support overlapping policies, we could add 'ptblpolid' into the primary key and then simply extract out all of the entries for the relation and action that we're currently running and step through each to find which of the policies apply to the current_role...? If a role has policyA with 'INSERT' rights, but no rights to SELECT, but they also have an entry for policyB with 'SELECT' rights, we would use only policyB for a SELECT query? Does that approach mean we don't need 'ptblaction' after all? I'm thinking this approach would also toss out the pick your policy concept that Dean had proposed up-thread. How would these interact with the existing table-level rights? For column-level rights, if you have access to SELECT the column then you don't need any table-level rights (and the table-level rights mean you can SELECT from any column), are we thinking the same would apply here, such that having 'USING POLICY' rights means you can SELECT from the table and the table-level rights end up being the 'DIRECT' rights which had been discussed up-thread? Not sure that I like that approach, though I understand some others might find it appealing.. As we're integrating this with the GRANT command, perhaps it'd be alright. Overall, while I'm interested in defining where this is going in a way which allows us implement an initial RLS capability while avoiding future upgrade issues, I am perfectly happy to say that the 9.5 RLS implementation may not be exactly syntax-compatible with 9.6 or 10.0. Again, I think that's completely non-viable. Are you going to tell people they can't pg_upgrade, and they can't dump-and-reload either, without manual fiddling? There's no way that's going to be accepted. I don't understand what you're getting at here. We dump the catalog using the newer version of pg_dump for pg_upgrade and we could handle any *syntax* change required during that process to ensure that the same access is granted in the new cluster as existed in the old cluster. We do the exact same thing every time we add a new reserved keyword- anything which used that keyword before ends up getting double-quoted by the new version of pg_dump and both pg_dump and pg_upgrade work just fine. We routinly break some syntax compatibility between major versions, address those changes in the newer version of pg_dump, and move on. I am not proposing that users won't be able to upgrade from 9.5 to 9.6 if they have RLS and agree that it'd be a non-starter. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
On Tue, Jun 24, 2014 at 8:49 PM, Stephen Frost sfr...@snowman.net wrote: Let's try to outline what this would look like then. Taking your approach, we'd have: CREATE POLICY p1; CREATE POLICY p2; ALTER TABLE t1 SET POLICY p1 TO t1_p1_quals; ALTER TABLE t1 SET POLICY p2 TO t1_p2_quals; This seems like a very nice, flexible framework. GRANT SELECT ON TABLE t1 TO role1 USING p1; GRANT SELECT ON TABLE t1 TO role2 USING p2; Instead of doing it this way, we could instead do: ALTER ROLE role1 ADD POLICY p1; ALTER ROLE role2 ADD POLICY p2; We could possibly allow multiple policies to be set for the same user, but given an error (or OR the quals together) if there are conflicting policies for the same table. A user with no policies would see everything to which they've been granted access. To support different policies on different operations, you could have something like: ALTER TABLE t1 SET POLICY p1 ON INSERT TO t1_p1_quals; Without the ON clause, it would establish the given policy for all operations. -- 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] RLS Design
* Robert Haas (robertmh...@gmail.com) wrote: On Tue, Jun 24, 2014 at 8:49 PM, Stephen Frost sfr...@snowman.net wrote: Let's try to outline what this would look like then. Taking your approach, we'd have: CREATE POLICY p1; CREATE POLICY p2; ALTER TABLE t1 SET POLICY p1 TO t1_p1_quals; ALTER TABLE t1 SET POLICY p2 TO t1_p2_quals; This seems like a very nice, flexible framework. GRANT SELECT ON TABLE t1 TO role1 USING p1; GRANT SELECT ON TABLE t1 TO role2 USING p2; Instead of doing it this way, we could instead do: ALTER ROLE role1 ADD POLICY p1; ALTER ROLE role2 ADD POLICY p2; We could possibly allow multiple policies to be set for the same user, but given an error (or OR the quals together) if there are conflicting policies for the same table. A user with no policies would see everything to which they've been granted access. Ok, I like that as it means that normal GRANTs, etc, remain the same and are just constrained by RLS when there is an RLS policy on the table, which I believe is really the right approach. To support different policies on different operations, you could have something like: ALTER TABLE t1 SET POLICY p1 ON INSERT TO t1_p1_quals; Without the ON clause, it would establish the given policy for all operations. Right, this makes sense also and is similar to what we were angling towards initially. I'll think further on this and propose a catalog structure and try to delve into the semantics of query operations, etc. One issue that occurs to me is trying to think through how to address the plancache invalidation, such that we are not invalidating constantly but also setting the correct quals for the current query. We had gone down a road where we saved a plan for each role under which a query was run but then ripped that out because the RLS policy would handle the per-role issues (modulo whether RLS should be enabled or not). This approach means that we'd have to bring back the notion of per-role plan cacheing. I'm not against that, just making note of it. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
On 25 June 2014 16:44, Robert Haas robertmh...@gmail.com wrote: On Tue, Jun 24, 2014 at 8:49 PM, Stephen Frost sfr...@snowman.net wrote: Let's try to outline what this would look like then. Taking your approach, we'd have: CREATE POLICY p1; CREATE POLICY p2; ALTER TABLE t1 SET POLICY p1 TO t1_p1_quals; ALTER TABLE t1 SET POLICY p2 TO t1_p2_quals; This seems like a very nice, flexible framework. GRANT SELECT ON TABLE t1 TO role1 USING p1; GRANT SELECT ON TABLE t1 TO role2 USING p2; Instead of doing it this way, we could instead do: ALTER ROLE role1 ADD POLICY p1; ALTER ROLE role2 ADD POLICY p2; We could possibly allow multiple policies to be set for the same user, but given an error (or OR the quals together) if there are conflicting policies for the same table. A user with no policies would see everything to which they've been granted access. I'm a bit uneasy about allowing overlapping policies like this, because I think it is more likely to lead to unintended consequences than solve real use cases. For example, suppose you define policies p1 and p2 and set them up on table t1, and you grant role1 permissions on t1 and allow role1 the use of policy p1. Then you set up policy p2 on another table t2, and decide you want to allow role1 access to t2 using this policy. The only way to do it is to add p2 to role1, but doing so also then gives role1 access to t1 using p2, which might not have been what you intended. With the GRANT ... USING policy syntax, you have greater flexibility to pick and choose which policies each user has permission to use with each table. To me at least, that seems much less error prone, since you are being much more explicit about exactly what privileges you are granting. The ALTER ROLE ... ADD POLICY syntax is potentially adding a whole bunch of extra privileges to the role, and you have to work quite hard to see exactly what it's adding. To support different policies on different operations, you could have something like: ALTER TABLE t1 SET POLICY p1 ON INSERT TO t1_p1_quals; Without the ON clause, it would establish the given policy for all operations. Yes, that makes sense. But as I was arguing above, I think the ACLs should be attached to the specific RLS policy identified uniquely by (table, policy, command). So, for example, if you did ALTER TABLE t1 SET POLICY p1 ON SELECT TO t1_p1_sel_quals; ALTER TABLE t1 SET POLICY p1 ON UPDATE TO t1_p1_upd_quals; you could also do GRANT SELECT ON TABLE t1 TO role1 USING p1; GRANT UPDATE ON TABLE t1 TO role1 USING p1; but it would be an error to do GRANT DELETE ON TABLE t1 TO role1 USING p1; because there is no p1 delete policy for t1; 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
[HACKERS] RLS Design
Dean, all, Changing the subject of this thread (though keeping it threaded) as we've really moved on to a much broader discussion. * Dean Rasheed (dean.a.rash...@gmail.com) wrote: On 24 June 2014 17:27, Stephen Frost sfr...@snowman.net wrote: Single policy vs Multiple, Overlapping policies vs Multiple, Non-overlapping policies What I was describing upthread was multiple non-overlapping policies. Ok. I disagree that this will be more complicated to use. It's a strict superset of the single policy functionality, so if you want to do it all using a single policy then you can. But I think that once the ACLs reach a certain level of complexity, you probably will want to break it up into multiple policies, and I think doing so will make things simpler, not more complicated. If we keep it explicitly to per-role only, with only one policy ever being applied, then perhaps it would be, but I'm not convinced.. Taking a specific, simplistic example, suppose you had 2 groups of users - some are normal users who should only be able to access their own records. For these users, you might have a policy like WHERE person_id = current_user which would be highly selective, and probably use an index scan. Then there might be another group of users who are managers with access to the records of, say, everyone in their department. This might then be a more complex qual along the lines of WHERE person_id IN (SELECT ... FROM person_department WHERE mgr_id = current_user AND ...) which might end up being a hash or merge join, depending on any user-supplied quals. Certainly my experience with such a setup is that it includes at least 4 levels (self, manager, director, officer). Now, officer you could perhaps exclude as being simply RLS-exempt but with such a structure I would think we'd just make that a special kind of policy (and not chew up those last 4 bits). As for this example, it's quite naturally done with a recursive query as it's a tree structure, but if you want to keep the qual simple and fast, you'd materialize the results of such a query and simply have: WHERE EXISTS (SELECT 1 from org_chart WHERE current_user = emp_id AND person_id = org_chart.id) You _could_ combine those into a single policy, but I think it would be much better to have 2 distinct policies, since they're 2 very different queries, for different use cases. Normal users would only be granted permission to use the normal_user_policy. Managers might be granted permission to use either the normal_user_policy or the manager_policy (but not both at the same time). I can't recall a system where managers have to request access to their manager role. Having another way of changing the permissions which are applied to a session (the existing one being 'set role') doesn't strike me as a great idea either. That's a very simplified example. In more realistic situations there are likely to be many more classes of users, and trying to enforce all the logic in a single WHERE clause is likely to get unmanageable, or inefficient if it involves lots of logic hidden away in functions. Functions and external security systems are exactly the real-world use-case which users I've talked to are looking for. All of this discussion is completely orthogonal to their requirements. I understand that there are simpler use-cases than those and we may be able to provide an approach which performs better for those. Allowing multiple, non-overlapping policies allows the problem to be broken up into more manageable pieces, which also makes the planner's job easier, since only a single, simpler policy is in effect in any given query. Let's try to outline what this would look like then. Taking your approach, we'd have: CREATE POLICY p1; CREATE POLICY p2; ALTER TABLE t1 SET POLICY p1 TO t1_p1_quals; ALTER TABLE t1 SET POLICY p2 TO t1_p2_quals; GRANT SELECT ON TABLE t1 TO role1 USING p1; GRANT SELECT ON TABLE t1 TO role2 USING p2; I'm guessing we would need to further support: GRANT INSERT ON TABLE t1 TO role1 USING p2; as we've already discussed being able to support per-action (SELECT, INSERT, UPDATE, DELETE) policies. I'm not quite sure how to address that though. Further, as you mention, users would be able to do: SET rls_policy = whatever; and things would appear fine, until they tried to access a table to which they didn't have that policy for, at which point they'd get an error. You mention: GRANT SELECT (col1, col2), UPDATE (col1) ON t1 TO bob USING policy1; but, to be clear, there would be no option for policies to be column-specific, right? The policy would apply to the whole row and just the SELECT/UPDATE privileges would be on the specific columns (as exists today). From this what I'm gathering is that we'd need catalog tables along these lines: rls_policy oid, polname name, polowner oid, polnamespace oid, polacl aclitme[] (oid,