On 2014-03-05 04:02, Craig Ringer wrote:
> On 03/04/2014 09:41 PM, Yeb Havinga wrote:
>> On 04/03/14 02:36, Craig Ringer wrote:
>>> I've pushed an update to the branch with the fix for varno handling.
>>> Thanks. It's tagged rls-9.4-upd-sb-views-v8 .
>>>
>>> I've almost run out of time to spend on row security for this
>>> commitfest, unfortunately. I'm putting a blog together with a current
>>> status update. Frustrating, as it's coming together now.
>>>
>>> Open issues include:
>>>
>>> - Passing plan inval items from rewriter into planner
>>> - COPY support pending
>>> - Clear syntax in DDL
>>>
>>> Most of the rest are solved; it's actually looking pretty good.
>> Hi Craig,
>>
>> I've tested the results from the minirim.sql that was posted earlier,
>> and the v8 gives the same results as v4 :-)
> Good to hear.
>
> The main known issue remaining is plan invalidation. Row security needs
> to create PlanInvalItems during _rewrite_ and also needs to set a
> PlannerGlobal field for the user ID the plan depends on. If it fails to
> do this then the superuser will sometimes run queries and have
> row-security applied, non-superusers might skip row security when it
> should be applied. This seems to be the cause of foreign key check
> issues I've observed, too.
>
> That's not trivial, because right now the rewriter only returns a Query
> node. It doesn't have anywhere to stash information that's global across
> the whole query, and adding fields to Query for the purpose doesn't seem
> ideal, since it's also used for subqueries, and in the planner.
I looked up the Query structure and steps of e.g. exec_simple_query(),
but ISTM that Query would be the place to store a used id. After all it
is meta data about the query. Duplication of this information in the
presence of subqueries seems less ugly to me than trying to evade
duplication by wrapping a structure around a query list.
Maybe a naive thought, but shouldn't all plans that include a table with
an RLS clause be invalidated when the session role switches, regardless
of which users from and to?
> I've also got some concerns about the user visible API; I'm not sure it
> makes sense to set row security policy for row reads per-command in
> PostgreSQL, since we have the RETURNING clause. Read-side policy should
> just be "FOR READ".
Hmm but FOR READ would mean new keywords, and SELECT is also a concept
known to users. I didn't find the api problematic to understand, on the
contrary. It might be an idea to add the SELECT RLS clause for DML
queries that contain a RETURNING clause.
regards,
Yeb