Re: Review of Row Level Security - Mailing list pgsql-hackers

The attached patch is rebased one towards the latest master branch, with
some documentation / source code updates. Except for these cosmetic
changes, nothing has been changed.

What this patch tries to do is simple; that replaces reference to tables with
pre-configured row-security policy by a sub-query that simply scans this
table with row-security policy and security-barrier flag.
I expect nobody has strong arguments towards this basic design.

However, some detailed implementations has not been reviewed so much
deeply. For example, when system-column or whole-row of the table with
row-security policy are referenced, rowsecurity.c adds the row-security
subquery target-entries to reference underlying system-columns or
whole-row reference, then it also fixed up Var->varattno that originally
referenced these columns because subquery has no system columns
and subquery's record type is not compatible with underlying table.
If table has inherited children, it also needs to have additional fixup
at prep/preptlist.c or /prep/prepunion.c.

It is much helpful if someone give me comments around these
arguable portions from the standpoint with fresh eyes.

Thanks,

2013/1/15 Kohei KaiGai <kaigai@kaigai.gr.jp>:
> The attached patch is row-security v9.
>
> According to the upthread discussion, I adjusted the syntax as follows:
>
>   ALTER TABLE <table> SET ROW SECURITY FOR <cmd> TO (<expression>);
>   ALTER TABLE <table> RESET ROW SECURITY FOR <cmd>;
>
> It seems to me "FOR <cmd>" might be omissible as synonym of "FOR ALL".
> User needs to input this clause everytime, so they may feel it troublesome.
>
> As previous version doing, references to a table with row-security policy is
> replaced by a simple sub-query that scans the target table with configured
> row-security policy.
>
> postgres=> ALTER TABLE t1 SET ROW SECURITY FOR ALL TO (a % 2 = 0);
> ALTER TABLE
> postgres=> ALTER TABLE t2 SET ROW SECURITY FOR ALL TO (a % 2 = 1);
> ALTER TABLE
> postgres=> EXPLAIN SELECT * FROM t1 WHERE f_leak(b);
>                                 QUERY PLAN
> ---------------------------------------------------------------------------
>  Result  (cost=0.00..50.82 rows=413 width=36)
>    ->  Append  (cost=0.00..50.82 rows=413 width=36)
>          ->  Subquery Scan on t1  (cost=0.00..0.01 rows=1 width=36)
>                Filter: f_leak(t1.b)
>                ->  Seq Scan on t1 t1_1  (cost=0.00..0.00 rows=1 width=36)
>                      Filter: ((a % 2) = 0)
>          ->  Subquery Scan on t2  (cost=0.00..28.51 rows=2 width=36)
>                Filter: f_leak(t2.b)
>                ->  Seq Scan on t2 t2_1  (cost=0.00..28.45 rows=6 width=36)
>                      Filter: ((a % 2) = 1)
>          ->  Seq Scan on t3  (cost=0.00..22.30 rows=410 width=36)
>                Filter: f_leak(b)
> (12 rows)
>
> In case of UPDATE or DELETE, row-security also prevent to modify
> rows that does not satisfy the configured security policy.
>
> postgres=> EXPLAIN UPDATE t1 SET b=b || '_update' WHERE b like '%abc%';
>                              QUERY PLAN
> ---------------------------------------------------------------------
>  Update on t1  (cost=0.00..54.04 rows=51 width=42)
>    ->  Subquery Scan on t1_1  (cost=0.00..0.02 rows=1 width=42)
>          Filter: (t1_1.b ~~ '%abc%'::text)
>          ->  Seq Scan on t1 t1_2  (cost=0.00..0.00 rows=1 width=42)
>                Filter: ((a % 2) = 0)
>    ->  Subquery Scan on t2  (cost=0.00..28.53 rows=1 width=42)
>          Filter: (t2.b ~~ '%abc%'::text)
>          ->  Seq Scan on t2 t2_1  (cost=0.00..28.45 rows=6 width=42)
>                Filter: ((a % 2) = 1)
>    ->  Seq Scan on t3  (cost=0.00..25.50 rows=49 width=42)
>          Filter: (b ~~ '%abc%'::text)
> (11 rows)
>
> One significant change to the planner is, planner had to accept cases
> that result relation is not identical with source relation being replaced
> to row-security subquery. E.g, constructed plan for UPDATE may scans
> tuples from a sub-query with rtindex=5 then update the relation with
> rtindex=1. Some existing code assumes result relation is also source
> relation, so it was my headache during the development.
> Even though the current implementation is working for all the test cases
> in regression test as I expected, I'm not 100% certain whether this
> implementation is the best way. So, it's welcome if we can have better
> and stable implementation than my proposition.
>
> Thanks,
> --
> KaiGai Kohei <kaigai@kaigai.gr.jp>



--
KaiGai Kohei <kaigai@kaigai.gr.jp>