Re: RLS Design

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Thu, Sep 11, 2014 at 3:08 PM, Stephen Frost <sfrost@snowman.net> wrote:
> > 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..
>
> Whoa.  I think that's a bad idea.  I think the default value for RLS
> should be disabled, and users should have to turn it on explicitly if
> they want to get it.  It's arguable whether the behavior if you try to
> create a policy beforehand should be (1) outright failure or (2)
> command accepted but no effect, but I think (3) automagically enable
> the feature is a POLA violation.  When somebody adds a policy and then
> drops it again, they will expect to be back in the same state they
> started out in, and for good reason.

Yeah, that I can agree with.  Prior to adding the ability to explicitly
enable RLS, that's what they got, but that's changed now that we've made
the ability to turn on/off RLS half-way independent of policies.  Also..

> > 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.
>
> A strong +1 for doing just that.  Look, anybody who is going to use
> row-level security but isn't careful enough to verify that it's
> actually working as desired after configuring it is a lost cause
> anyway.

I had been thinking the same, which is why I was on the fence about if
it was really an issue or not.  This all amounts to actually making the
patch smaller also, which isn't a bad thing.

> Personally, I have to test every GRANT and REVOKE I
> issue, because there's no error for granting a privilege that the
> target already has or revoking one they don't, and with group
> membership and PUBLIC it's quite easy to have not done what you
> thought you did.  Fixing that might be worthwhile but it doesn't take
> away from the fact that, like any other configuration change you make,
> security-relevant changes need to be tested.

Hmm, pretty sure that'd end up going against the spec too, but that's
a whole different discussion anyway.

> There is another possible advantage of the explicit-enable approach as
> well, which is that you might want to create several policies and then
> turn them all on at once.  With what you have now, creating the first
> policy will enable RLS on the table and then everyone who wasn't the
> beneficiary of that initial policy is locked out.  Now, granted, you
> can probably get around that by doing all of the operations in one
> transaction, so it's a minor point.  But it's still nice to think
> about being able to add several policies and then flip them on.  If it
> doesn't work out, flip them off, adjust, and flip them back on again.
> Now, again, the core design issue, IMHO, is that the switch from
> default-allow to default-deny should be explicit and unmistakable, so
> the rest of this is just tinkering around the edges.  But we might as
> well make those edges as nice as possible, and the usability of this
> approach feels good to me.

Fair enough.
Thanks!
    Stephen