Re: RLS Design

Craig,

* Craig Ringer (craig(at)2ndquadrant(dot)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 about it first- and we'll need to figure out how to address
that when it comes to both psql and the 'rlsenabled' flag.

> - 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.

I'm really not interested in contrib modules with this first go around..
We can work to address their requests later on. I don't think many
contrib authors will be very happy with the low-level support which
we'll provide in 9.5 anyway and it'd probably be better off for everyone
if we hold off on adding hooks, etc, for them until we have a better
idea about how this will be used and it will work.

> - 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.

Yes, we'll need this.

> - 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;

Agreed. Adam has a patch for this already, more or less.

> Plus a number of fixes:
>
> - Fixing the security barrier view isssue with row level lock pushdown
> that's breaking the row security regression tests;

No- this is not the responsibility of this particular patch or
functionality. I agree that we will want to address it at some point,
but it's very complicated and not required at this time.

> - Enhancing plan cache invalidation so that row-security exempt-ness
> of a user is part of the plancache key;

We need to ensure that the plan cache is hanlded correctly. I'm not
convinced, at this point, that we actually need to inclue the user as
part of the key for looking up a plan cache. It might come to that, but
I'm not quite convinced it's necessary yet.

> - 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

Yeah, we need to consider this and how it *should* behave. Have we
really thought about and documented that, ideally as regression tests?
We need to do so, to ensure that we have the correct behavior in this
case.

> 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.

Yeah, not sure how we want to handle these. At this point, I'm open to
simply throwing an ERROR in cases which are not well defined or which do
not work as expected. Ideally we can do better than that, but throwing
an ERROR for cases which don't exist today and which are not yet
supported is reasonable, imv.

> It's already a decent sized amount of work on top of the existing row
> security patch.

Indeed.

> If we start adding policy groups, etc, this will never get done.

Agreed!

Thanks!

Stephen