Re: [RFC] Interface of Row Level Security

On Jun5, 2012, at 15:07 , Kohei KaiGai wrote:
> 2012/6/5 Florian Pflug <fgp@phlo.org>:
>> On Jun5, 2012, at 11:43 , Kohei KaiGai wrote:
>>> I think it does not require to add a mechanism to invalidate
>>> prepared-statement, because all the checks are applied on
>>> executor stage. And these functions can be stable functions,
>>> so I believe some enhancements at planner will correctly wipe
>>> out prior to query execution at the next step.

>> That won't work, since the current role can change any time mid-query,
>> at least if you're using cursors. Here's an example

<snipped example>

>> Imagine that the current_user function call was actually part of the
>> RLS policy. Then the example above shows that *which* role we check
>> access against (the one active at the first fetch, or currently active
>> one) depends on which plan the optimizer happens to pick. This, IMHO,
>> is not acceptable for a security feature like RLS.

> Which is your expected behavior in this example?

I can live with any behaviour, as long as it doesn't depends on details
of the query plan. My vote would be for always using the role which was
active at statement creation time (i.e. at PREPARE/DECLARE time) for
RLS purposes though. That way, we could treat the role as being IMMUTABLE
I think.

> In case when the query is internally materialized, the current_user function
> is *already* executed prior to switch user-id, thus, the result set internally
> stored in was already filtered out using older user-id.

Exactly. But whether or not the query, some parts of it, or even only some
parts of the policy function are materialized is outside the control of the
user and may change at any time.

> It seems to me, caution of the problem is current_user is performing out
> of the snapshot controls. According to the definition of stable function,
> its result should not be changed during a particular scan.

Yeah, well, ISTM that our definition of STABLE doesn't really take functions
whose return value depends on GUCs into account.

> At least, it is not a fundamental problem of RLS implementation, although
> it needs to take an enhancement something like effective user-id per
> snapshot basis.

Right. We need something like "statement_role" which returns the role the
statement is to be executed as (However we define that, though as I said
above my vote is for it being the role which created the statement). To make
that work, however, the plan cache needs to store that role, since the
statement might be re-planned after its initial creation, but before it is
executed. At which point the amount of complexity saved by not implementing
RLSBYPASS is very nearly zero.

To reiterate, my point is that we won't get away with zero planner changes
even without RLSBYPASS. In fact, we'll be paying *most* of the complexity
costs of RLSBYPASS whether we actually add that feature or not. Which makes
not adding it look like a pretty bad deal to me.

best regards,
Florian Pflug