Re: [v9.3] Row-Level Security - Mailing list pgsql-hackers

The attached patch is a revised version of row-level security feature.

I added a feature to invalidate plan cache if user-id was switched
between planner and optimizer. It enabled to generate more optimized
plan than the previous approach; that adds hardwired "OR superuser()".

Example)
postgres=# PREPARE p1(int) AS SELECT * FROM t1 WHERE x > $1 AND f_leak(y);
PREPARE
postgres=# EXPLAIN (costs off) EXECUTE p1(2);
            QUERY PLAN
-----------------------------------
 Seq Scan on t1
   Filter: (f_leak(y) AND (x > 2))
(2 rows)

postgres=# SET SESSION AUTHORIZATION alice;
SET
postgres=> EXPLAIN (costs off) EXECUTE p1(2);
                 QUERY PLAN
---------------------------------------------
 Subquery Scan on t1
   Filter: f_leak(t1.y)
   ->  Seq Scan on t1
         Filter: ((x > 2) AND ((x % 2) = 0))
(4 rows)

On the other hand, I removed support for UPDATE / DELETE commands
in this revision, because I'm still uncertain on the implementation that I
adopted in the previous patch. I believe it helps to keep patch size being
minimum reasonable.
Due to same reason, RLS is not supported on COPY TO command.

According to the Robert's comment, I revised the place to inject
applyRowLevelSecurity(). The reason why it needed to patch on
adjust_appendrel_attrs_mutator() was, we handled expansion from
regular relation to sub-query after expand_inherited_tables().
In this revision, it was moved to the head of sub-query planner.

Even though I added a syscache entry for pg_rowlevelsec catalog,
it was revised to read the catalog on construction of relcache, like
trigger descriptor, because it enables to reduce cost to parse an
expression tree in text format and memory consumption of hash
slot.

This revision adds support on pg_dump, and also adds support
to include SubLinks in the row-level security policy.

Thanks,

2012/7/1 Kohei KaiGai <kaigai@kaigai.gr.jp>:
> 2012/6/28 Tom Lane <tgl@sss.pgh.pa.us>:
>> Kohei KaiGai <kaigai@kaigai.gr.jp> writes:
>>> 2012/6/27 Florian Pflug <fgp@phlo.org>:
>>>> Hm, what happens if a SECURITY DEFINER functions returns a refcursor?
>>
>>> My impression is, here is no matter even if SECURITY DEFINER function
>>> returns refcursor.
>>
>> I think Florian has a point: it *should* work, but *will* it?
>>
>> I believe it works today, because the executor only applies permissions
>> checks during query startup.  So those checks are executed while still
>> within the SECURITY DEFINER context, and should behave as expected.
>> Subsequently, the cursor portal is returned to caller and caller can
>> execute it to completion, no problem.
>>
>> However, with RLS security-related checks will happen throughout the
>> execution of the portal.  They might do the wrong thing once the
>> SECURITY DEFINER function has been exited.
>>
> I tried the scenario that Florian pointed out.
>
> postgres=# CREATE OR REPLACE FUNCTION f_test(refcursor) RETURNS refcursor
> postgres-# SECURITY DEFINER LANGUAGE plpgsql
> postgres-# AS 'BEGIN OPEN $1 FOR SELECT current_user, * FROM t1;
> RETURN $1; END';
> CREATE FUNCTION
> postgres=# BEGIN;
> BEGIN
> postgres=# SELECT f_test('abc');
>  f_test
> --------
>  abc
> (1 row)
>
> postgres=# FETCH abc;
>  current_user | a |  b
> --------------+---+-----
>  kaigai       | 1 | aaa
> (1 row)
>
> postgres=# SET SESSION AUTHORIZATION alice;
> SET
> postgres=> FETCH abc;
>  current_user | a |  b
> --------------+---+-----
>  alice        | 2 | bbb
> (1 row)
>
> postgres=> SET SESSION AUTHORIZATION bob;
> SET
> postgres=> FETCH abc;
>  current_user | a |  b
> --------------+---+-----
>  bob          | 3 | ccc
> (1 row)
>
> Indeed, the output of "current_user" depends on the setting of session
> authorization, even though it was declared within security definer
> functions (thus, its security checks are applied on the privileges of
> function owner).
>
>> We might need to consider that a portal has a local value of
>> "current_user", which is kind of a pain, but probably doable.
>>
> It seems to me, it is an individual matter to be fixed independent
> from RLS feature. How about your opinion?
>
> If we go ahead, an idea to tackle this matter is switch user-id
> into saved one in the Portal at the beginning of PortanRun or
> PortalRunFetch.
>
> Thanks,
> --
> KaiGai Kohei <kaigai@kaigai.gr.jp>



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