Re: Row-security on updatable s.b. views

On 03/06/2014 04:56 AM, Yeb Havinga wrote:
>>> It might be an idea to add the SELECT RLS clause for DML
>>> queries that contain a RETURNING clause.
>> That way lies madness: A DML statement that affects *a different set of
>> rows* depending on whether or not it has a RETURNING clause.
> If you state it like that, it sounds like a POLA violation. But the
> complete story is: "A user is allowed to UPDATE a set of rows from a
> table that is not a subsect of the set of rows he can SELECT from the
> table, iow he can UPDATE rows he is not allowed to SELECT. This can lead
> to unexpected results: When the user issues an UPDATE of the table
> without a returning clause, all rows the user may UPDATE are affected.
> When the user issues an UPDATE of the table with a returning clause, all
> rows the user may UPDATE and SELECT are affected."

Well, I think that's bizarre behaviour.

It doesn't make sense for RETURNING to affect the behaviour of the
command it's applied to. It never has before, and it's defined to return
the set of rows affected by the command. It shouldn't be changing that
set, it isn't a predicate.

> So the madness comes from the fact that it is allowed to define RLS that
> allow to modify rows you cannot select. Either prevent these conditions
> (i.e. proof that all DML RLS qual implies the SELECT qual, otherwise
> give an error on DML with a RETURNING clause)

Equivalence proofs in predicates are WAY outside what's going to be
reasonable to tackle in a feature like this. Especially since the row
security expression may be "my_c_function(the_row)", and all the detail
of behaviour is hidden behind some C function.

We need to treat row security expressions as pretty much opaque.

> or allow it without
> violating the RLS rules but accept that a DML with RETURNING is
> different from a DML only.

I don't think that's acceptable. Too many tools automatically add a
RETURNING clause, or use one in generated SQL. Notably, PgJDBC will
append a RETURNING clause to your query so it can return generated keys.

With regular permissions, we require that the user has SELECT rights if
they use a RETURNING clause. That works because it's a simple
permissions check. With row security, we'd be affecting a different set
of rows instead, and doing so silently. That's just ugly. (It also
creates execution inefficiencies where the SELECT and UPDATE/DELETE
predicates are the same).

Additionally, in PostgreSQL if you can supply a predicate for a row, you
can leak the row via a RAISE NOTICE or other tricks. So even without
RETURNING, allowing a user to update/delete a row permits them to
potentially see the row. (This is an issue with our current permissions
too; if you DELETE with a leaky predicate, you can see the rows you
deleted even without SELECT rights on a table).

That, IMO, is two good reasons not to differentiate between command
types for the purpose of which rows they can see in a scan.

We already have a mechanism for allowing users to do things that they
can't normally do under controlled and restricted circumstances:
SECURITY DEFINER functions.

I don't think we need to introduce bizarre and surprising behaviour in
DML when we have a viable mechanism for people who need to do this.

Is there a compelling use case for this? Where it really makes sense to
let users update/delete rows they cannot see via row security? We
support it in the table based permissions model, but it's possible to do
it with much saner semantics there. And with row security, it'll be
possible with security definer functions. I intend to add a "row
security exempt" flag for functions down the track, too.

-- Craig Ringer                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services