Re: RLS Design - Mailing list pgsql-hackers

* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
> On 25 June 2014 01:49, Stephen Frost <sfrost@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..  For that we may have to add "Oid rls_select_policy", etc, to
> > AclItem, which would be pretty painful.  Other thoughts?
> >
>
> Huh? Isn't it just another column in rls_policy_table to specify the
> action type?

I had been trying to fit it into the ACL structure somehow.  What would
it look like to have multiple action types then?  Here's one thought:

table1 using policy1 for INSERT: bob=rw/grantor
table1 using policy1 for SELECT: bob=r/grantor

Or how about:

table1|policy1/w: bob=rw/grantor
table1|policy1/r: bob=r/grantor

Another question is about showing what the actual quals are for a given
policy which is being applied to a table.  Would we want that to show up
in \d, \d+, or only be available through querying the catalog..?

> > This certainly feels like quite a bit to try and bite off for 9.5 and,
> > as mentioned, this would be a strict superset of the current approach,
> > which could be implemented under this structure as:
> >
> > CREATE POLICY t1_p1_policy;
> > ALTER TABLE t1 SET POLICY p1 TO t1_p1_quals;
> > GRANT (user's rights) ON t1 TO user USING policy1;
> >
> > Tha main downside here is that we'd have to create a policy for every
> > table in the system which had RLS applied, to avoid granting more than
> > should be.  Perhaps the 9.4 approach could include the 'CREATE POLICY'
> > and 'ALTER TABLE' bits, but not the GRANT parts, meaning that we would,
> > for the 9.5 -> 9.6 upgrade, pg_dump:
> >
> > GRANT (user's rights) ON t1 TO user USING policy1;
> >
> > We would still need the GUCs for "rls_enable = on/off" and perhaps the
> > role-level "bypass_rls" attribute, but those wouldn't change with this.
>
> Well I think you'd have to flesh out the alternatives to a similar
> level of detail to assess the relative effort involved, but I think
> it's encouraging to see this level of design this early in the 9.5
> cycle.

I'm not sure which other alternatives you're thinking about here- could
you be more specific..?  I can try to flesh them out but I had actually
been hoping that this would be a good compromise position among the
alternatives.

This provides the per-role policy granularity which has been mentioned a
few times, but doesn't allow the policies to overlap.  Overlapping
policies could be added to this general design, I believe, though we'd
have to make a few catalog changes and invent some new syntax to define
how the policies are to be combined (ANDs vs ORs, etc).  I had brought
up the idea of ordering/prioritizing policies, but I didn't particularly
like the suggestion when I made it and I don't recall anyone else
voicing interest in that approach.

For my part, I don't see the GUCs as really being "alternatives" so much
as pre-requisites.  Even with all the granularity and comprehensive set
of features which we're talking about here, we're going to need a way
for pg_dump to simply say "do not apply RLS to me and ERROR out if
that's an issue".  I agree that it's great to get these design
discussions happening now but I really do not want this to become a
behemoth patch by the last CF and ends up bounced because of that.

What I'd like to work through is the minimal set which would be accepted
and get that in, in a way that doesn't prevent further improvements, and
then see what can be done to get those improvements and refinements in
during the 9.5 cycle and what gets bounced to the next release.  To that
end, I've been trying to gauge interest in this and get some feel for
who is interested in helping push this forward- your help was
instrumental in getting updatable security barrier views into 9.4, would
you have time to help with this also..?
Thanks!
    Stephen