Re: WIP patch (v2) for updatable security barrier views

On 30 January 2014 05:36, Craig Ringer <craig@2ndquadrant.com> wrote:
> On 01/29/2014 08:29 PM, Dean Rasheed wrote:
>> On 29 January 2014 11:34, Craig Ringer <craig@2ndquadrant.com> wrote:
>>> On 01/23/2014 06:06 PM, Dean Rasheed wrote:
>>>> On 21 January 2014 09:18, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>>>>> Yes, please review the patch from 09-Jan
>>>>> (http://www.postgresql.org/message-id/CAEZATCUiKxOg=vOOvjA2S6G-sixzzxg18ToTggP8zOBq6QnQHQ@mail.gmail.com).
>>>>>
>>>>
>>>> After further testing I found a bug --- it involves having a security
>>>> barrier view on top of a base relation that has a rule that rewrites
>>>> the query to have a different result relation, and possibly also a
>>>> different command type, so that the securityQuals are no longer on the
>>>> result relation, which is a code path not previously tested and the
>>>> rowmark handling was wrong. That's probably a pretty obscure case in
>>>> the context of security barrier views, but that code path would be
>>>> used much more commonly if RLS were built on top of this. Fortunately
>>>> the fix is trivial --- updated patch attached.
>>>
>>> This is the most recent patch I see, and the one I've been working on
>>> top of.
>>>
>>> Are there any known tests that this patch fails?
>>>
>>
>> None that I've been able to come up with.
>
> I've found an issue. I'm not sure if it can be triggered from SQL, but
> it affects in-code users who add their own securityQuals.
>
> expand_security_quals fails to update any rowMarks that may point to a
> relation being expanded. If the relation isn't the target relation, this
> causes a rowmark to refer to a RTE with no relid, and thus failure in
> the executor.
>
> Relative patch against updatable s.b. views attached (for easier
> reading), along with a new revision of updatable s.b. views that
> incorporates the patch.
>
> This isn't triggered by FOR SHARE / FOR UPDATE rowmark on a security
> barrier view because the flattening to securityQuals and re-expansion in
> the optimizer is only done for _target_ security barrier views. For
> target views, different rowmark handling applies, so they don't trigger
> it either.
>
> This is triggered by adding securityQuals to a non-target relation
> during row-security processing, though.
>

Hmm, looks like this is a pre-existing bug.

The first thing I tried was to reproduce it using SQL, so I used a
RULE to turn a DELETE into a SELECT FOR UPDATE. This is pretty dumb,
but it shows the problem:

CREATE TABLE foo (a int);
CREATE RULE foo_del_rule AS ON DELETE TO foo DO INSTEAD SELECT * FROM foo FOR UPDATE;
DELETE FROM foo;

ERROR:  no relation entry for relid 1

So I think this should be fixed independently of the updatable s.b. view code.

Regards,
Dean