Re: [v9.4] row level security

2013/11/6 Craig Ringer <craig(at)2ndquadrant(dot)com>:
> On 11/05/2013 09:36 PM, Robert Haas wrote:
>> I haven't studied this patch in detail, but I see why there's some
>> unhappiness about that code: it's an RLS-specific kluge. Just
>> shooting from the hip here, maybe we should attack the problem of
>> making security-barrier views updatable first, as a separate patch.
>
> That's the approach I've been considering. There are a few wrinkles with
> it, though:
>
> (a) Updatable views are implemented in the rewriter, not the planner.
> The rewriter is not re-run when plans are invalidated or when the
> session authorization changes, etc. This means that we can't simply omit
> the RLS predicate for superuser because the same rewritten parse tree
> might get used for both superuser and non-superuser queries.
>
> Options:
>
> * Store the before-rewrite parse tree when RLS is discovered on one of
> the rels in the tree. Re-run the rewriter when re-planning. Ensure a
> change in current user always invalidates plans.
>
> * Declare that it's not legal to run a query originally parsed and
> rewritten as superuser as a non-superuser or vice versa. This would
> cause a great deal of pain with PL/PgSQL.
>
> * Always add the RLS predicate and solve the problem of reliably
> short-circuiting the user-supplied predicate for RLS-exempt users. We'd
> need a way to allow direct (not query-based) COPY TO for tables with RLS
> applied, preventing the rewriting of direct table access into subqueries
> for COPY, but since we never save plans for COPY that may be fine.
>
> * ... ?
>
How about an idea that uses two different type of rules: the existing one
is expanded prior to planner stage as we are doing now, and the newer
one is expanded on the head of planner stage.
The argument of planner() is still parse tree, so it seems to me here is
no serious problem to call rewriter again to handle second stage rules.
If we go on this approach, ALTER TABLE ... SET ROW SECURITY
will become a synonym to declare a rule with special attribute.

> (b) Inheritance is a problem when RLS is done in the rewriter. As I
> understood it from Kohei KaiGai's description to me earlier, there was a
> strong preference on -hackers to enforce RLS predicates for child and
> parent tables completely independently. That's how RLS currently works,
> but it might be hard to get the same effect when applying RLS in the
> rewriter. We'd need to solve that, or redefine RLS's behaviour so that
> the predicate on a parent table applies to any child tables too.
> Personally I'd prefer the latter.
>
I'm not certain whether it was a "strong preference", even though I followed
the consensus at that time. So, I think it makes sense to discuss how RLS
policy shall be enforced on the child tables.
As long as we can have consistent view on child tables even if it is referenced
without parent tables, I don't have any arguments to your preference.
Also, it makes implementation simple than the approach I tried to have; that
enforces RLS policy of tables individually, because of utilization of existing
rule mechanism.
It is not difficult to enforce parent's RLS policy on the child relation even if
it is referenced individually. All we need to do special is append RLS policy
of its parent, not only child's one, if referenced table has parent.

> (c) RLS might interact differently with rules declared on tables if
> implemented in the rewriter, so some investigation into that would be
> needed.
>
>> I
>> would think that if we make that work, this will also work without,
>> hopefully, any special hackery. And we'd get a separate,
>> independently useful feature out of it, too.
>
> I tend to agree. I'm just a bit concerned about dealing with the issues
> around RLS-exempt operations and users.
>
> --
> Craig Ringer http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services

--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>