Re: [RFC] Interface of Row Level Security

On Thu, May 24, 2012 at 6:11 AM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>> Perhaps when we see that RLS
>> applies, we should replace the reference to the original table with a
>> subquery RTE that has the security_barrier flag set - essentially
>> treating a table with RLS as if it were a security view.
>>
> I become to think it is a better approach than tracking origin of each
> qualifiers. One problem is case handling on update or delete statement.
>
> It may be possible to rewrite the update / delete query as follows:
>
> From:
>  UPDATE tbl SET X = X + 1 WHERE f_leak(Y)
> To:
>  UPDATE tbl SET X = X + 1 WHERE ctid = (
>      SELECT * FROM (
>          SELECT ctid FROM tbl WHERE uname = getpgusername()  <== (*)
> should have security-barrier
>      ) AS tbl_subqry WHERE f_leak(Y)
>  );
>
> Expanded sub-queries will have security-barrier flag, so it enforces
> the "uname = getpgusername()" being checked earlier than f_leak(Y).
> We may need to measure the performance impact due to the reform.

The problem with this is that it introduces an extra instance of tbl
into the query - there are now two rather than one. UPDATE .. FROM is
supposed to be a way to avoid this, but it's insufficiently general to
handle all the cases (e.g. UPDATE a LEFT JOIN b can't be written using
the existing syntax). Anyway we want to avoid inserting self-joins
for performance reasons if at all possible. It should be easy to do
that in the case of SELECT; UPDATE and DELETE may need a bit more
work.

> I think, this situation is similar to a case when we reference a view
> without privileges to underlying tables. If Bob set up a view with
> something "tricky" function, it allows Bob to reference credentials
> of users who reference the view.
> More or less, it might be a problem when a user try to invoke
> a user defined function declared by others.
> (Thus, sepgsql policy does not allow users to invoke a function
> declared by another one in different domain; without DBA's checks.)

This is true, but there are still some new threat models. For
example, currently, pg_dump isn't going to run any user-defined code
just because you do SELECT * FROM table, but that will change with
this patch. Note that pg_dump need not actually select from views,
only tables.

> I think it is a good idea not to apply RLS when current user has
> superuser privilege from perspective of security model consistency,
> but it is inconsistent to check privileges underlying tables.

Seems like a somewhat random wart, if it's just an exception for
superusers. I think we need to do better than that. For example, at
my last company, sales reps A and B were permitted to see all
customers of the company, but sales reps C, D, E, F, G, H, I, and J
were permitted to see only their own accounts. Those sorts of
policies need to be easy to implement.

>> Another idea is to set things up so that the RLS policy function isn't
>> applied to each row directly; instead, it's invoked once per query and
>> *returns* a WHERE clause.  This would be a lot more powerful than the
>> proposed design, because now the table owner can write a function that
>> imposes quals on some people but not others, which seems very useful.
>>
> Sorry, I don't favor this idea. Even if table owner set up a function to
> generate additional qualifiers, it also has no guarantee the qualifiers
> are invoked prior to user-given one.
> It seems to me this approach will have same problem...

It's not intended to solve the qual-ordering problem, just to allow
additional policy flexibility.

>> It's not clear to me that there is any need for built-in server
>> functionality here.  If the table owner wants to enforce some sort of
>> policy regarding INSERT or UPDATE or DELETE, they can already do that
>> today just by attaching a trigger to the table.  And they can enforce
>> whatever policy they like that way.  Before designing any new
>> mechanism, what's wrong with the existing one?
>>
> Yes, we don't need any new invent to check the value of new tuples.
> But it should be done after all the user-defined triggers. Existing
> trigger does not have a mechanism to enforce order to be invoked.
> So, what I really implement is a mechanism to inject some pseudo
> triggers "at tail of the Trigger array".

Start the trigger names with the letter "z".

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company