Re: [v9.3] Row-Level Security

The revised patch fixes the problem that Daen pointed out.

In case when row-level security policy contains SubLink node,
it become to call the rewriter to expand views being contained
within SubLink, and also append qualifier of security policy
onto underlying tables. It enables to apply configured policy
on nested relations also, as if top-level ones.
In addition, I added a check for infinite recursion when two
different tables have row-level policy that references each
other.

So, for example, self-recursion RLS shall be prevented.
postgres=> ALTER TABLE foo SET ROW LEVEL SECURITY (a < (SELECT
max(a) FROM foo));
ALTER TABLE
postgres=> SELECT * FROM foo;
ERROR: infinite recursion detected for relation "foo"

It shows RLS policy is recursively applied.
postgres=> ALTER TABLE foo SET ROW LEVEL SECURITY (a > 0);
ALTER TABLE
postgres=> ALTER TABLE bar SET ROW LEVEL SECURITY
(EXISTS(SELECT 1 FROM foo WHERE foo.a = bar.a));
ALTER TABLE
postgres=> EXPLAIN SELECT * FROM bar;
QUERY PLAN
--------------------------------------------------------------------------------------------
Hash Join (cost=44.95..111.95 rows=1200 width=4)
Hash Cond: (bar.a = foo.a)
-> Seq Scan on bar (cost=0.00..34.00 rows=2400 width=4)
-> Hash (cost=42.45..42.45 rows=200 width=4)
-> HashAggregate (cost=40.45..42.45 rows=200 width=4)
-> Bitmap Heap Scan on foo (cost=10.45..30.45
rows=800 width=4)
Recheck Cond: (a > 0)
-> Bitmap Index Scan on foo_pkey
(cost=0.00..10.25 rows=800 width=0)
Index Cond: (a > 0)
(9 rows)

Even if RLS policy contains a view, it works fine.
postgres=> CREATE VIEW foo_v AS SELECT * FROM foo;
CREATE VIEW
postgres=> ALTER TABLE bar SET ROW LEVEL SECURITY
(a IN (SELECT * FROM foo_v));
ALTER TABLE
postgres=> EXPLAIN SELECT * FROM bar;
QUERY PLAN
--------------------------------------------------------------------------------------------
Hash Join (cost=44.95..111.95 rows=1200 width=4)
Hash Cond: (bar.a = foo.a)
-> Seq Scan on bar (cost=0.00..34.00 rows=2400 width=4)
-> Hash (cost=42.45..42.45 rows=200 width=4)
-> HashAggregate (cost=40.45..42.45 rows=200 width=4)
-> Bitmap Heap Scan on foo (cost=10.45..30.45
rows=800 width=4)
Recheck Cond: (a > 0)
-> Bitmap Index Scan on foo_pkey
(cost=0.00..10.25 rows=800 width=0)
Index Cond: (a > 0)
(9 rows)

Thanks,

2012/10/8 Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>:
> 2012/10/8 Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>:
>> On 8 October 2012 15:57, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>>> The attached patch is a refreshed version towards the latest master branch,
>>> to fix up patch conflicts.
>>> Here is no other difference from the previous revision.
>>>
>>> Thanks,
>>>
>>
>> I had another look at this over the weekend and I found couple of
>> additional problems (test cases attached):
>>
>> 1). It is possible to define a RLS qual that refers back to the table
>> that it's defined on, in such a way that causes infinite recursion in
>> the planner, giving "ERROR: stack depth limit exceeded". I think it
>> would be preferable to trap this and report a more meaningful error
>> back to the user, along similar lines to a self-referencing view.
>>
>> 2). In other cases it is possible to define a RLS qual that refers to
>> another table with a RLS qual in such a way that the second table's
>> RLS qual is not checked, thus allowing a user to bypass the security
>> check.
>>
>> 3). If a RLS qual refers to a view it errors, since the RLS quals are
>> added after rule expansion, and so the view is not rewritten.
>>
>> To me this suggests that perhaps the expansion of RLS quals should be
>> done in the rewriter. I've not thought that through in any detail, but
>> ISTM that a RIR rule could add a table with a RLS qual, and a RLS qual
>> could add a relation with a RIR rule that needs expanding, and so the
>> 2 need to be processed together. This could also make use of the
>> existing recursion-checking code in the rewriter.
>>
> Thanks for your checks. I missed some cases that you suggested.
>
> The reason why we need to put RLS expansion at planner stage is
> requirement towards plan cache invalidation. Due to special case
> handling for superuser, plan cache has to be invalidated if user-id
> to run executor was switched since planner stage. The planner shall
> be invoked again, but not rewritter, on its invalidation.
>
> Probably, it make sense to invoke rewriter's logic to solve RLS
> policy from planner stage (that allows plan-cache invalidation).
> Let me investigate the code of rewriter.
>
> Best regards,
> --
> KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

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