Re: RLS Design

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> On Thu, Sep 11, 2014 at 3:08 PM, Stephen Frost <sfrost(at)snowman(dot)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