Re: [v9.4] row level security - Mailing list pgsql-hackers

From Dean Rasheed
Subject Re: [v9.4] row level security
Date
Msg-id CAEZATCV+12de7XCFv09oC7Lbb6x1Nwjd_RLAHiH4CKThU72LQA@mail.gmail.com
Whole thread Raw
In response to Re: [v9.4] row level security  (Craig Ringer <craig@2ndquadrant.com>)
List pgsql-hackers
On 6 November 2013 09:23, Craig Ringer <craig@2ndquadrant.com> wrote:
> On 11/06/2013 05:02 PM, Dean Rasheed wrote:
>
>> The basic idea is to have rewriteTargetView() collect up any quals
>> from SB views in a new list on the target RTE, instead of adding them
>> to the main query's predicates (it needs to be a list of SB quals, in
>> case there are SB views on top of other SB views, in which case they
>> need to be kept separate from one another). Then at the end of the
>> rewriting process (after any views referenced in the SB quals have
>> been expanded), a new piece of code kicks in to process any RTEs with
>> SB quals, turning them into (possibly nested) subquery RTEs.
>
> That makes sense, though presumably you face the same problem that the
> existing RLS code does with references to system columns that don't
> normally exist in subqueries?
>

Yeah, that feels like an ugly hack.


> Since this happens during query rewrite, what prevents the optimizer
> from pushing outer quals down into the subqueries?
>

The subquery RTE is marked with the security_barrier flag, which
prevents quals from being pushed down in the presence of leaky
functions (see set_subquery_pathlist).


>> The complication is that the query's resultRelation RTE mustn't be a
>> subquery.
>
> I think this is what Robert was alluding to earlier with his comments
> about join relations:
>
> ____
> Robert Haas wrote:
>> I don't really see why.  AIUI, the ModifyTable node just needs to get
>> the right TIDs.  It's not like that has to be stacked directly on top
>> of a scan; indeed, in cases like UPDATE .. FROM and DELETE .. USING it
>> already isn't.  Maybe there's some reason why the intervening level
>> can be a Join but not a  SubqueryScan, but if so I'd expect we could
>> find some way of lifting that limitation without suffering too much
>> pain.
> (http://www.postgresql.org/message-id/CA+TgmoYr1PHw3X9vnVuWDcfXkzK2p_jhtWc0fV2Q58NEgcxyTA@mail.gmail.com)
> ____
>

Yeah. We already do a similar thing for trigger-updatable views. So
with this approach you end up with similar plans, for example:

Update on base_tbl ->  Subquery Scan on base_tbl ...


>
> Maybe we just need to make a subquery scan a valid target for an update,
> so those fixups aren't required anymore?
>

Possibly. That feels like it would require much more extensive surgery
on the planner though. I've not explored that idea, but I suspect it
would quickly turn into a whole new can of worms.


>> This is handled this in a similar way to the
>> trigger-updatable views code, producing 2 RTEs --- the resultRelation
>> RTE becomes a direct reference to the base relation, and a separate
>> subquery RTE acts as the source of rows to update.
>
> Some of the complexity of the current RLS code is caused by the need to
> do similar fixups to handle the case where the input relation isn't the
> same as the target relation, but is a subquery over it instead.
>
>> Anyway, feel free to do what you like with this. I wasn't planning on
>> submitting it to the next commitfest myself, because my non-PG
>> workload is too high, and I don't expect to get much time to hack on
>> postgresql during the next couple of months.
>
> Thanks for sending what you have. It's informative, and it shows that
> some of the same issues must be solved for writable security barrier
> views and for RLS.
>

Agreed. I'm not sure what the best way to fix those issues is though.
The currently proposed approach feels pretty ugly, but I can't see a
better way at the moment.

Regards,
Dean



pgsql-hackers by date:

Previous
From: Craig Ringer
Date:
Subject: Re: [v9.4] row level security
Next
From: Kevin Grittner
Date:
Subject: Re: missing locking in at least INSERT INTO view WITH CHECK