Robert, * Robert Haas (robertmh...@gmail.com) wrote: > On Sat, Sep 6, 2014 at 2:54 AM, Brightwell, Adam > <adam.brightw...@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
signature.asc
Description: Digital signature