Re: Prohibit row-security + inheritance in 9.4?

On Mon, Feb 3, 2014 at 10:23 PM, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
>>>> Then during expansion of the range table, no code is needed to
>>>> ignore child rls quals and copy parent rels to child rels.
>>> This is what's already implemented and isn't a huge amount of code to
>>> begin with, so I don't see this as being an argument against having the
>>> flexibility.
>>
>> It would seem to me that any additional logic that can be avoided during
>> planning is a good thing. Also, the argument that something is already
>> implemented, does not itself make it good to commit.
>
> The implementation with the minimum of required logic and complexity is
> "apply the parent's row-security quals to the children, don't check
> children for quals".
>
> Any special handling of child rels creates exciting challenges because
> inheritance expansion happens _after_ a bunch of the query planner and
> optimizer has run. Injecting new expression trees at this point is
> messy, especially if you want those expression trees to in turn contain
> row-security qualified tables, inheritance, etc.
>
> As previously discussed, applying the parent qual to children ensures
> that what's visible when querying a relation that has children is
> consistent with its row-security qual.
>
> If the parent qual is applied only on the parent rel directly, not
> children, then querying the parent could emit rows not permitted by the
> parent's row-security qual. I'm not happy with that, and as Stephen
> poined out upthread, it isn't really consistent with how permissions
> work with inheritance otherwise.
>
> If instead the parent qual is applied to the parent and all children,
> and you *also* add the quals of each child, you get a complex, hard to
> debug, hard to optimize mess. You also run back into the problem
> mentioned above, that adding quals after inhertiance expansion is messy
> and problematic. It's also really ugly in what's going to be the most
> common case, where the child quals are the same as the parent quals, as
> you'll get nested identical quals.
>
> Despite the challenges with it, I think that's the least insane way to
> respect child quals on parents. It has pretty much zero chance of
> happening in 9.4, though; the discussed approach of building
> row-security on top of updatable security barrier views doesn't play
> well with adding inheritance on inheritance-expanded children.
>
> One answer for that would be to keep inheritance as it is for 9.4 (if I
> can get the remaining issues sorted out) and in 9.5, if possible, allow
> the addition of a row-security qual that, if it appears on a child rel
> during inheritance expansion, _is_ expanded. At least, if it proves
> necessary, which I'm not entirely convinced of.

Me, neither. When you first described the scheme you're currently
pursuing, I thought it sounded kind of nuts. But as I've thought
about it more, it's grown on me. Today, the way to do row-level
security is:

1. Create a security barrier view over the table.
2. Grant rights to the view instead of the table, and tell people to
go through that.

Now, if you did that, as far as I can see, it would have exactly the
same semantics as what you're proposing to do here, with the sole
exception that you wouldn't need to tell people to access the view
rather the table. So yeah it's kind of funky but I have a feeling any
rule we come up with here will seem odd in some scenarios, and this
one at least has the virtue of being relatively easy to implement and
consistent with how similar things work today. I can't knock that.

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