Re: RLS restrictive hook policies - Mailing list pgsql-hackers

Dean,

* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
> In get_row_security_policies():
>
>     /*
>      * If the only built-in policy is the default-deny one, and hook policies
>      * exist, then use the hook policies only and do not apply the
>      * default-deny policy.  Otherwise, we will apply both sets below.
>      */
>     if (defaultDeny &&
>         (hook_policies_restrictive != NIL || hook_policies_permissive != NIL))
>     {
>         rowsec_expr = NULL;
>         rowsec_with_check_expr = NULL;
>     }
>
> So if the only policies that exist are restrictive policies from an
> extension, the default-deny policy is not applied and the restrictive
> policies are applied instead, which is effectively a "default-allow"
> situation with each restrictive policy further limiting access to the
> table.

Right, the idea there being that applying the default-deny would render
the restrictive policies pointless, since nothing would ever be visible,
however..

> I think this is potentially very dangerous when both kinds of policy
> exist. Consider for example a situation where initially multiple
> permissive policies are set up for different users allowing them
> access to different subsets of the table, and users not covered by any
> of those permissive policies have no access. Then suppose that later a
> restrictive policy is added, applying to all users -- the intention
> being to prevent any user from accessing some particularly sensitive
> data. The result of adding that restrictive policy would be that all
> the users who previously had no access at all would suddenly have
> access to everything not prohibited by the restrictive policy.
>
> That goes against everything I would expect from a restrictive policy
> -- adding more restrictive policies should only ever reduce the number
> of rows visible, not ever open up more. So I think the test above
> should only disable the default-deny policy if there are any
> permissive policies from the extension.
>
> Of course that will make life a little harder for people who only want
> to use restrictive policies, since they will be forced to first add a
> permissive policy, possibly just one that's always true, but I think
> it's the safest approach.

Requiring a permissive policy which allows the records first, to avoid
the default-deny, makes a ton of sense.

> Possibly if/when we add proper SQL support for defining restrictive
> policies, we might also add an option to ALTER TABLE ... ENABLE ROW
> LEVEL SECURITY to allow a table to have RLS enabled in default-allow
> mode, for users who know they only want to add restrictive policies.

Perhaps...  I'm not sure that's really better than simply requiring a
'create policy p1 on t1 using (true);' to be done.

> However, I think that should be something the user has to explicitly
> ask for, not an automatic decision based on the existence of
> restrictive policies.

Agreed.  I'm happy to commit that change and back-patch it to 9.5,
barring objections.  Given that the only way to have restrictive
policies currently is using hooks, I don't believe there's any
documentation update required either; of course, I'll update the comment
to reflect this discussion/decision and make any necessary changes to
test_rls_hooks.
Thanks!
    Stephen