Re: Infinite recursion in row-security based on updatable s.b. views

On 24 January 2014 09:04, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
> Hi all
>
> I've hit an interesting wrinkle and am interested in opinions. By
> integrating updatable security barrier view support with row-security,
> it has become a lot harder to detect and prevent infinite recursion
> (including mutual recursion) in row-security policy expansion.
>
> The code is attached, but it's not an independent patch so it's way
> easier to grab it from git:
>
> git(at)github(dot)com:ringerc/postgres.git
> branch rls-9.4-upd-sb-views (subject to rebase); or
> tag rls-9.4-upd-sb-views-v1
>
> (Only contains half the old row-security patch; I'll rebase the docs,
> tests, etc on top of this once it's working properly).
>
>
> I've integrated the updatable security barrier view support into RLS by
> injecting securityQuals in subquery_planner() just before
> preprocess_rowmarks . (I'm still thinking about some inheritance related
> aspects to that, but that's for later).
>
> The problem is that this causes infinite recursion - the securityQuals
> get expanded into a subquery over the original RTE that had the
> row-security policy on it. Then subquery_planner is re-entered when
> processing the subquery, a row-security qual is found on the target RTE,
> and ... *boom*.
>
> Fixing this is not as simple as suppressing expansion of row-security
> policy when processing a security barrier subquery created by a
> row-security policy, as it is desirable to respect the row-security
> policy of *other* tables that get referenced in the expanded
> row-security qual.
>
> If we just record the relid of the relation a subquery was expanded from
> and avoid expanding that inside the generated subquery we prevent simple
> linear recursion, but not mutual recursion between two or more rels with
> row-security policy.
>
> KaiGai's row-security patch handles this because it does its own
> recursive expansion, so (like the rewriter) it can keep a breadcrumb
> trail and detect when it is repeating a path. That's not so practical
> when row-security code tags RTEs with policy, then updatable s.b. views
> goes and expands them.
>
> So. Options.
>
> 1.Come up with a reasonable way to track recursive row-security
> expansion, detect infinite recursion, and bail out. Likely to involve
> a new global structure with planner/optimizer lifetime that gets
> checked and maintained by apply_row_security_rangetbl.
>
> 2.Solve the linear recursion case by storing a relid that should not get
> expanded for security quals when processing a subquery. Say "don't do
> that, expect stack exceeded crashes if you do" for the mutual
> recursion case. Seems user hostile, incomplete, and likely to break
> people's systems when they really don't expect it.
>
> 3.Disregard row-security policy on referenced tables when expanding
> row-security qualifiers. There's precendent here in foreign keys,
> which ignore row-security policy, but I don't think this is at all
> appealing.
>
> 4.Magic?
>

My first thought is to add a boolean flag to RangeTblEntry (similar to
the "inh" flag) to say whether RLS expansion is requested for that
RTE. Then set it to false on each RTE as you add new RLS quals to it's
securityQuals.

In addition, when adding RLS quals to an RTE, I think they should be
fully and recursively expanded immediately, before setting the new
flag to false and moving on --- think recursively calling the rewriter
to expand view references in the new RLS qual, and
expand_security_qual() to expand any additional RLS quals in the
securityQuals list --- with loop detection there. I'm not pretending
that's going to be easy, but there are a couple of existing pieces of
code to borrow ideas from. Doing it this way should make it possible
to do the loop detection without any global structures.

Regards,
Dean