Re: Review of Row Level Security

On 19 December 2012 18:40, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Tue, Dec 18, 2012 at 3:39 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>>> postgres=> INSERT INTO t1 VALUES (4,'ddd');
>>> INSERT 0 1
>>> postgres=> INSERT INTO t1 VALUES (5,'eee');
>>> ERROR: new row for relation "t1" violates row-secirity
>>> DETAIL: Failing row contains (5, eee).
>
>> I've argued against this before - and maybe I should drop my
>> objection, because a number of people seem to be on the other side.
>> But I still think there will be some people who don't want this
>> behavior. Right now, for example, you can give someone INSERT but not
>> SELECT permission on a table, and they will then be able to put rows
>> into the table that they cannot read back.
>
> There is also precedent for your opinion in the spec-mandated behavior
> of updatable views: it is perfectly possible to INSERT a row that you
> can't read back via the view, or UPDATE it to a state you can't see
> via the view. The RLS patch's current behavior corresponds to a view
> created WITH CHECK OPTION --- which we don't support yet. Whether
> we add that feature soon or not, what seems important for the current
> debate is that the SQL spec authors chose not to make it the default
> behavior. This seems to weigh heavily against making it the default,
> much less only, behavior for RLS cases.

This is security, not spec compliance. By default, we need full security.

Nobody has argued that it should be the only behaviour, only that it
is the most typically requested behaviour and the most secure,
therefore the one we should do first.

> I'd also suggest that "throw an error" is not the only response that
> people are likely to want for attempts to insert/update non-compliant
> rows, so hard-wiring that choice is insufficiently flexible even if you
> grant that local policy is to not allow such updates. (As an example,
> they might prefer to log the attempt and substitute some other value.)
>
>> Previously, I suggested that we handle this by enforcing row-level
>> security only on data read from the table - the OLD row, so to speak -
>> and not on data written to the table - the NEW row, so to speak -
>> because the latter case can be handled well enough by triggers.
>
> +1. I'm less than excited about RLS in the first place, so the less
> complexity we have to put into the core system for it the better IMO.

Agree with the need for less complexity, but that decision increases
complexity for the typical user and does very little to the complexity
of the patch. Treating a security rule as a check constraint is
natural and obvious, so there are no core system problems here.

If we don't enforce rules on INSERT the user has to specifically add a
trigger, which makes things noticeably slower. There is more
maintenance work for the average user, less performance and more
mistakes to make.

The way to do this is by adding an option to allow users to specify
INSERT should be exempt from the security rule, which Kaigai and I
agreed on list some weeks back should come after the initial patch, to
no other comment.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services