RLS: Keep deny policy when only restrictive exist

Only remove the default deny policy when a permissive policy exists
(either from the hook or defined by the user).  If only restrictive
policies exist then no rows will be visible, as restrictive policies
shouldn't make rows visible.  To address this requirement, a single
"USING (true)" permissive policy can be created.

Update the test_rls_hooks regression tests to create the necessary
"USING (true)" permissive policy.

Back-patch to 9.5 where RLS was added.

Per discussion with Dean.

diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
index 562dbc90e9fdf7f1d0a29227873ce4ff75dc1471..5a81db3618c06a03aeedf12a88e2191421538818 100644 (file)
--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -225,12 +225,18 @@ get_row_security_policies(Query *root, CmdType commandType, RangeTblEntry *rte,
        }
 
        /*
-        * 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
+        * If the only built-in policy is the default-deny one, and permissive hook
+        * policies exist, then use the hook policies only and do not apply the
         * default-deny policy.  Otherwise, we will apply both sets below.
+        *
+        * Note that we do not remove the defaultDeny policy if only *restrictive*
+        * policies exist as restrictive policies should only ever be reducing what
+        * is visible.  Therefore, at least one permissive policy must exist which
+        * allows records to be seen before restrictive policies can remove rows
+        * from that set.  A single "true" policy can be created to address this
+        * requirement, if necessary.
         */
-       if (defaultDeny &&
-               (hook_policies_restrictive != NIL || hook_policies_permissive != NIL))
+       if (defaultDeny && hook_policies_permissive != NIL)
        {
                rowsec_expr = NULL;
                rowsec_with_check_expr = NULL;

diff --git a/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out b/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
index 3a7a4c329f3adaedb4be5b0c152f3c9ad8689d0a..4587eb014b7fa4ada1fa0842d5f2c3fbb5f09a59 100644 (file)
--- a/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
+++ b/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
@@ -13,6 +13,11 @@ CREATE TABLE rls_test_restrictive (
     supervisor      name,
     data            integer
 );
+-- At least one permissive policy must exist, otherwise
+-- the default deny policy will be applied.  For
+-- testing the only-restrictive-policies from the hook,
+-- create a simple 'allow all' policy.
+CREATE POLICY p1 ON rls_test_restrictive USING (true);
 -- initial test data
 INSERT INTO rls_test_restrictive VALUES ('r1','s1',1);
 INSERT INTO rls_test_restrictive VALUES ('r2','s2',2);
@@ -109,6 +114,8 @@ RESET ROLE;
 -- Create "internal" policies, to check that the policies from
 -- the hooks are combined correctly.
 CREATE POLICY p1 ON rls_test_permissive USING (data % 2 = 0);
+-- Remove the original allow-all policy
+DROP POLICY p1 ON rls_test_restrictive;
 CREATE POLICY p1 ON rls_test_restrictive USING (data % 2 = 0);
 CREATE POLICY p1 ON rls_test_both USING (data % 2 = 0);
 SET ROLE r1;

diff --git a/src/test/modules/test_rls_hooks/sql/test_rls_hooks.sql b/src/test/modules/test_rls_hooks/sql/test_rls_hooks.sql
index ece4ab9dc94822cf2c65b07bd3ac55691d29ae70..3071213732f79eb7f34e381554fe37b548881898 100644 (file)
--- a/src/test/modules/test_rls_hooks/sql/test_rls_hooks.sql
+++ b/src/test/modules/test_rls_hooks/sql/test_rls_hooks.sql
@@ -17,6 +17,12 @@ CREATE TABLE rls_test_restrictive (
     data            integer
 );
 
+-- At least one permissive policy must exist, otherwise
+-- the default deny policy will be applied.  For
+-- testing the only-restrictive-policies from the hook,
+-- create a simple 'allow all' policy.
+CREATE POLICY p1 ON rls_test_restrictive USING (true);
+
 -- initial test data
 INSERT INTO rls_test_restrictive VALUES ('r1','s1',1);
 INSERT INTO rls_test_restrictive VALUES ('r2','s2',2);
@@ -101,6 +107,8 @@ RESET ROLE;
 -- the hooks are combined correctly.
 CREATE POLICY p1 ON rls_test_permissive USING (data % 2 = 0);
 
+-- Remove the original allow-all policy
+DROP POLICY p1 ON rls_test_restrictive;
 CREATE POLICY p1 ON rls_test_restrictive USING (data % 2 = 0);
 
 CREATE POLICY p1 ON rls_test_both USING (data % 2 = 0);

diff --git a/src/test/modules/test_rls_hooks/test_rls_hooks.c b/src/test/modules/test_rls_hooks/test_rls_hooks.c
index d76b17ae46a3d05ed0581fe7a5b1f02cc00a1624..b96dbff9548c6380ce92e94938fe202360a204be 100644 (file)
--- a/src/test/modules/test_rls_hooks/test_rls_hooks.c
+++ b/src/test/modules/test_rls_hooks/test_rls_hooks.c
@@ -119,6 +119,11 @@ test_rls_hooks_permissive(CmdType cmdtype, Relation relation)
 
 /*
  * Return restrictive policies to be added
+ *
+ * Note that a permissive policy must exist or the default-deny policy
+ * will be included and nothing will be visible.  If no filtering should
+ * be done except for the restrictive policy, then a single "USING (true)"
+ * permissive policy can be used; see the regression tests.
  */
 List *
 test_rls_hooks_restrictive(CmdType cmdtype, Relation relation)