Re: [RFC] Interface of Row Level Security - Mailing list pgsql-hackers

I'd like to summarize the current design being discussed.

syntax: ALTER TABLE <tblname> WITH ROW LEVEL SECURITY     ( <condition clause> ) [FOR (SELECT | UPDATE | DELETE)];
ALTERTABLE <tblname> WITHOUT ROW LEVEL SECURITY; 

I tried to patch the parser/gram.y, but here was syntax conflicts
on ADD / DROP sub-command.
And, I noticed "ROW LEVEL SECURITY" allows to implement
without adding new keyword, unlike "SECURITY POLICY".

As we discussed, it causes a problem with approach to append
additional qualifiers to where clause implicitly, because it does
not solve the matter corresponding to the order to execute
qualifiers. So, I'm inclined to the approach to replace reference
to tables with security policy by sub-queries with security barrier
flag.
For example, if "tbl" has security policy, this query shall be rewritten
internally, as follows:

original) SELECT * FROM tbl WHERE X > 20 AND f_leak(Y);

rewritten) SELECT * FROM (   SELECT * FROM tbl WHERE uname = getpgusername() ) AS tbl_subqry WHERE X > 20 AND
f_leak(Y);

The sub-query shall have security-barrier flag, so f_leak() is never
pushed down but X > 20 will be pushed down because of leakproof
attribute of the function.

It is a bit complex at UPDATE or DELETE statement, but
what I try to do is same.
 original)   UPDATE tbl SET X = X + 1 WHERE X > 20 AND f_leak(Y);
 rewritten)   UPDATE tbl SET X = X + 1 WHERE ctid = (       SELECT ctid FROM (           SELECT ctid, * FROM uname =
getpgusername()      ) AS tbl_subqry WHERE X > 20 AND f_leak(Y)   ); 

That guarantees the security policy ("uname = getpgusername()")
is evaluated prior to user given conditions.

One thing still I'm thinking is whether the security policy should
be provided as a function or a clause. Enough simple sql function
is inlined at simplify_function(), so here is no meaningful difference.
I was afraid of code complexity, but all we should do is to append
configured clause on the where clause of sub-query inside.

|  ALTER TABLE <tblname> WITH ROW LEVEL SECURITY
|      ( <condition clause> ) [FOR (SELECT | UPDATE | DELETE)];

So, I tried to put "condition clause" instead of a function, right now.


Regarding to FK constraints, I don't think it is a situation to
apply row-level security policy towards internal queries.
So, I plan to disable during FK checks.


One other issue we didn't have discussed is table inheritance.
In case when a table TBLP has a child table TBLC and only
TBLC has its security policy, what security policy should be
applied when we run "SELECT * FROM TBLP".
My preference is, the security policy is only applied to scan on
TBLC, not TBLP. It is not desirable behavior that visible tuples
are different from way to reference a certain table.
In addition, if and when TBLP and TBLC have their own policy
individually, what is a desirable behavior?
I think, the security policy of both TBLP and TBLC should be
applied on TBLC; in other words, it applies the security policy
of all the parent tables to scan on child table.

Any comments please. Thanks,

2012/5/24 Kohei KaiGai <kaigai@kaigai.gr.jp>:
> 2012/5/24 Florian Pflug <fgp@phlo.org>:
>> On May24, 2012, at 16:19 , Kohei KaiGai wrote:
>>> So, the proposed interface might be revised as follows:
>>>  ALTER TABLE <tblname> ADD SECURITY POLICY
>>>      <func_name>(<args>, ...) [FOR SELECT |
>>>                                                         INSERT |
>>>                                                         [BEFORE|AFTER] UPDATE |
>>>                                                         DELETE];
>>>
>>> In case of INSERT or AFTER UPDATE, I assume the check shall be applied
>>> on the tail of before-row triggers.
>>
>> I'd go with just "SELECT, UPDATE, DELETE", and leave the INSERT and BEFORE
>> UPDATE case to regular triggers, for two reasons
>>
>> First, it's conceptually much simpler, since the policy always just adds
>> an implicit WHERE clause, period. This of course assumes that DELETE and
>> (BEFORE) UPDATE simply skips rows for which the policy function returns false,
>> instead of reporting 'permission denied' or something. But that's the most
>> reasonable behaviour anyway, I think, because otherwise you'd make batch
>> UPDATEs and DELETEs pretty much unusable, 'cause there'd always be the risk
>> of tripping over some invisible row and getting and error.
>>
> I definitely agree with starting a new feature from simple implementation.
>
> Although I'm inclined to the approach to replace references to tables with
> security policy by sub-queries with security barrier flag, instead of adding
> qualifiers of where clause to avoid the leaky-view scenario, it will make its
> implementation mush simpler.
>
>> Another issue, BTW, are FOREIGN KEY constraints. Should you be allowed to
>> created references to rows which are invisible to you, or should FOREIGN KEY
>> constraints be exempt from security policies? I'd say they shouldn't be, i.e.
>> the policy WHERE clause should be added to constraint checking queries like
>> usual. But maybe I'm missing some reason why that'd be undesirable…
>>
> I agree. The row level security policy should not be applied during FK checks
> (or other internal stuff; to be harmless). At the previous discussion, it was
> issued that iteration of FK/PK proving enables malicious one to estimate
> existence of invisible tuple and its key value, although they cannot see the
> actual values. It is well documented limitation, thus, user should not use row-
> level security (or should not use natural key) if they cannot accept
> this limitation.
>
> Thanks,
> --
> KaiGai Kohei <kaigai@kaigai.gr.jp>



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