Re: API change advice: Passing plan invalidation info from the rewriter into the planner? - Mailing list pgsql-hackers

On Tue, Jun 10, 2014 at 7:18 PM, Craig Ringer <craig@2ndquadrant.com> wrote:
> On 06/11/2014 02:19 AM, Tom Lane wrote:
>> Hm ... I'm not following why we'd need a special case for superusers and
>> not anyone else?  Seems like any useful RLS scheme is going to require
>> more privilege levels than just superuser and not-superuser.
>
> What it really needs is to invalidate plans when switching between
> RLS-enabled and RLS-exempt users, yes. I'm sure we'll want an "RLS
> exempt" right or mode sooner rather than later, so I'm against tying
> this explicitly to superuser as such.
>
> I wouldn't be surprised to see
>
> SET ROW SECURITY ON|OFF
>
> down the track, with a right controlling whether you can or not. Or at
> least, a right that directly exempts a user from row security.

I'm really concerned about the security implications of this patch.  I
think we're setting ourselves up for a whole lot of hurt for somewhat
unclear gain.

In my view, commit 842faa714c0454d67e523f5a0b6df6500e9bc1a5 basically
*is* row-level security: instead of applying a row-level security
policy to a table, just create a security-barrier view over the table
and grant access to the view.  Forget that the table ever existed.
Done.

With this approach, there's a lot of stuff that we don't have to
reinvent.  We've talked a lot about whether row-level security should
only be concerned with the rows it scans, or whether it should also
restrict the new rows that can be created.  You can get either
behavior by choosing whether or not to use WITH CHECK OPTION.  And
then there's this question of who should be RLS-exempt; that's
basically a question of to whom you grant privileges on the underlying
table.  Note that this can be very fine-grained: for example, you can
allow someone to exempt themselves for selects but not for updates by
granting them SELECT privileges but not UPDATE privileges on the
underlying table.  And potentially-exempt users can choose whether
they want a particular access to actually be exempt by targeting the
view when they don't want to be exempt and the table when they do.
That's mighty useful for debugging, at least IMHO.  And, if you want
to have several row-level security policies for different classes of
users, just create more than one view and grant different privileges
on each.

By contrast, it seems to me that every design so far proposed for
something that is actually called row-level security - as opposed to
commit 842faa714c0454d67e523f5a0b6df6500e9bc1a5, which *really is*
row-level security, is extremely limited.  Look back at all the things
listed in the previous paragraph; can you do those things easily with
the designs that have been proposed?  As far as I can see, not really.
Your (Craig's) rls-9.4-upd-sb-views patch seems to have a rough
equivalent of WITH CHECK OPTION, probably because we've talked a lot
about that specific issue, but it doesn't line up exactly to what WITH
CHECK OPTION actually does.  There's no independently-grantable
RLS-exemption privilege - and even when we talk about that, it's
usually some kind of global bit that applies to all tables and all
operations equally - whereas with the above approach it can be
per-table and per-operation and doesn't require superuser intervention
to flip the bit.  There's no way for users who are RLS exempt to turn
off their exemption for testing purposes, let alone on a per-table
basis.  There's no way to have multiple RLS policies on a single
table.  All of those are things that we get "for free" in the
view-over-table model, and implementing formal RLS basically requires
us to either invent a new RLS-specific way of doing each of those
things, or suffer along with a subset of the functionality.  Yuck.

But what's really awful about this whole design is that it breaks the
invariant that reading from a table doesn't run anybody else's code.
It's already the case that users need to be awfully careful about
modifying tables, because that might fire triggers that do bad things.
But at least you can SELECT from a table and it will either work, or
it will fail with a permission denied error.  What it will not do is
unexpectedly run some code that you weren't expecting it to run.  You
can't be so blithe about selecting from views, but reading a plain
table is always OK.  Now, as soon as we introduce the concept that
selecting from a table might not really mean "read from the table" but
"read from the table after applying this owner-specified qual", we're
opening up a whole new set of attack surfaces.  Every pg_dump is an
opportunity to hack somebody else's account, or at least audit their
activity.  Protecting the superuser against everybody else is nice,
but I think it's just as important to protect non-superusers against
each other, and I think that's going to be hard -- because in the RLS
world, SELECT * FROM tab is now *fundamentally* ambiguous.  Maybe it's
reading from the table, and maybe it's really clandestinely reading
from a view over the table, and the user has no way of being really
clear about which behavior they want.  From a security point of view,
that seems very bad.

To recap:

1. Reinventing RLS-specific ways to do all of the things that can
already be done in the view-over-table model is a lot of work.
2. There's a danger that the functionality available in the two models
will diverge, so that certain things can only be done in one world or
the other.
3. On the whole, it seems likely that the RLS-specific world will
remain impoverished compared to the view-over-table model.
4. Making SELECT * FROM tab ambiguous seems likely to be a security minefield.

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