Re: RLS Design

Robert,

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Sat, Sep 6, 2014 at 2:54 AM, Brightwell, Adam
> <adam.brightwell@crunchydatasolutions.com> wrote:
> > * Add ALTER TABLE <name> { ENABLE | DISABLE } ROW LEVEL SECURITY - set flag
> > on table to allow for a default-deny capability.  If RLS is enabled on a
> > table and has no policies, then a default-deny policy is automatically
> > applied.  If RLS is disabled on table and the table still has policies on it
> > then then an error is raised.  Though if DISABLE is accompanied with
> > CASCADE, then all policies will be removed and no error is raised.
>
> This text doesn't make it clear that all of the cases have been
> covered; in particular, you didn't specify whether an error is thrown
> if you try to add a policy to a table with DISABLE ROW LEVEL SECURITY
> in effect.  Backing up a bit, I think there are two sensible designs
> here:

Ah, yeah, the text could certainly be clearer.

> 1. Row level security policies can't exist for a table with DISABLE
> ROW LEVEL SECURITY in effect.  It sounds like this is what you have
> implemented, modulo any hypothetical bugs.  You can't add policies
> without enabling RLS, and you can't disable RLS without dropping them
> all.

Right, this was the approach we were taking.  Specifically, adding
policies would implicitly enable RLS for the relation.

> 2. Row level security policies can exist for a table with DISABLE ROW
> LEVEL SECURITY in effect, but they don't do anything until RLS is
> enabled.  A possible advantage of this approach is that you could
> *temporarily* shut off RLS for a table without having to drop all of
> your policies and put them back.  I kind of like this approach; we
> have something similar for triggers, and I think it could be useful to
> people.

I like the idea of being able to turn them off without dropping them.
We have that with row_security = off, but that would only work for the
owner or a superuser (or a user with bypassrls).  This would allow
disabling RLS temporairly for everything accessing the table.

The one thing I'm wondering about with this design is- what happens when
a policy is initially added?  Currently, we automatically turn on RLS
for the table when that happens.  I'm not thrilled with the idea that
you have to add policies AND turn on RLS explicitly- someone might add
policies but then forget to turn RLS on..

> If you stick with approach #1, make sure pg_dump is guaranteed to
> enable RLS before applying the policies.

Currently, adding a policy automatically turns on RLS, so we don't have
any issue with pg_dump from that perspective.  Handling cases where RLS
is disabled but policies exist would get more complicated for pg_dump if
we keep the current idea that adding policies implicitly turns on RLS-
it'd essentially have to go back and turn it off after the policies are
added.  Not a big fan of that either.

> And either way, you should
> that pg_dump behaves sanely in the case where there are circular
> dependencies, like you have two table A and B, and each has a RLS
> policy that manages to use the other table's row-type.  (You probably
> also want to check that DROP and DROP .. CASCADE on either policy or
> either table does the right thing in that situation, but that's
> probably easier to get right.)

Agreed, we'll double-check that this is working.  As these are
attributes of the table which get added later on by pg_dump, similar to
permissions, I'd think it'd all work fine, but good to make sure (and
ditto with DROP/DROP CASCADE..  We have some checks for that, but good
to make sure it works in a circular-dependency case too).

If we want to be able to disable RLS w/o dropping the policies, then I
think we have to completely de-couple the two and users would then have
both add policies AND turn on RLS to have RLS actually be enabled for a
given table.  I'm on the fence about that.

Thoughts?
Thanks!
    Stephen