Re: [HACKERS] RLS Design

2014-09-25 Thread Thom Brown
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

2014-09-25 Thread Stephen Frost
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

2014-09-25 Thread Thom Brown
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

2014-09-20 Thread Craig Ringer
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

2014-09-20 Thread Josh Berkus
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

2014-09-19 Thread Robert Haas
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

2014-09-19 Thread Andres Freund
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

2014-09-19 Thread Thom Brown
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

2014-09-19 Thread Stephen Frost
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

2014-09-19 Thread Stephen Frost
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

2014-09-19 Thread Thom Brown
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

2014-09-19 Thread Andres Freund
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

2014-09-19 Thread Robert Haas
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

2014-09-19 Thread Stephen Frost
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

2014-09-19 Thread Brightwell, Adam
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

2014-09-19 Thread Andrew Gierth
 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

2014-09-11 Thread Erik Rijkers
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

2014-09-11 Thread Stephen Frost
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

2014-09-11 Thread Robert Haas
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

2014-09-11 Thread Stephen Frost
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

2014-09-11 Thread Robert Haas
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

2014-09-11 Thread Stephen Frost
* 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

2014-09-03 Thread Robert Haas
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

2014-09-03 Thread Stephen Frost
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

2014-09-03 Thread Stephen Frost
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

2014-09-03 Thread Robert Haas
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

2014-08-31 Thread Stephen Frost
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

2014-07-21 Thread Robert Haas
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

2014-07-18 Thread Robert Haas
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

2014-07-18 Thread Brightwell, Adam

 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

2014-07-17 Thread Alvaro Herrera
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

2014-07-16 Thread Tom Lane
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

2014-07-16 Thread Stephen Frost
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

2014-07-16 Thread Brightwell, Adam
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

2014-07-16 Thread Brightwell, Adam
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

2014-07-16 Thread Stephen Frost
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

2014-07-11 Thread Stephen Frost
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

2014-07-11 Thread Robert Haas
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

2014-07-11 Thread Stephen Frost
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

2014-07-10 Thread Robert Haas
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

2014-07-09 Thread Stephen Frost
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

2014-07-09 Thread Stephen Frost
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

2014-07-09 Thread Stephen Frost
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 Thread Kohei KaiGai
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

2014-07-09 Thread Stephen Frost
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

2014-07-08 Thread Craig Ringer
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-08 Thread Kohei KaiGai
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

2014-07-07 Thread Robert Haas
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

2014-07-05 Thread Stephen Frost
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

2014-07-04 Thread Kouhei Kaigai
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

2014-07-04 Thread Stephen Frost
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

2014-07-04 Thread Kouhei Kaigai
 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

2014-07-02 Thread Yeb Havinga

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

2014-07-02 Thread Stephen Frost
* 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

2014-07-02 Thread Robert Haas
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

2014-07-02 Thread Stephen Frost
* 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

2014-07-02 Thread Robert Haas
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

2014-07-02 Thread Stephen Frost
* 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

2014-07-02 Thread Stephen Frost
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

2014-07-01 Thread Dean Rasheed
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

2014-07-01 Thread Robert Haas
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

2014-07-01 Thread Dean Rasheed
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

2014-07-01 Thread Robert Haas
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

2014-06-30 Thread Robert Haas
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

2014-06-30 Thread Stephen Frost
* 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

2014-06-30 Thread Robert Haas
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

2014-06-30 Thread Stephen Frost
* 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

2014-06-29 Thread Stephen Frost
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

2014-06-27 Thread Dean Rasheed
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

2014-06-26 Thread Robert Haas
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

2014-06-25 Thread Dean Rasheed
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

2014-06-25 Thread Stephen Frost
* 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?

2014-06-25 Thread Stephen Frost
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

2014-06-25 Thread Robert Haas
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

2014-06-25 Thread Stephen Frost
* 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

2014-06-25 Thread Dean Rasheed
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

2014-06-24 Thread Stephen Frost
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,