Adjust behavior of row_security GUC to match the docs.

Some time back we agreed that row_security=off should not be a way to
bypass RLS entirely, but only a way to get an error if it was being
applied.  However, the code failed to act that way for table owners.
Per discussion, this is a must-fix bug for 9.5.0.

Adjust the logic in rls.c to behave as expected; also, modify the
error message to be more consistent with the new interpretation.
The regression tests need minor corrections as well.  Also update
the comments about row_security in ddl.sgml to be correct.  (The
official description of the GUC in config.sgml is already correct.)

I failed to resist the temptation to do some other very minor
cleanup as well, such as getting rid of a duplicate extern declaration.

Author: tglsfdc

doc/src/sgml/ddl.sgml

@@ -1572,11 +1572,7 @@ REVOKE ALL ON accounts FROM PUBLIC;
    bypass the row security system when accessing a table.  Table owners
    normally bypass row security as well, though a table owner can choose to
    be subject to row security with <link linkend="sql-altertable">ALTER
-   TABLE ... FORCE ROW LEVEL SECURITY</>.  Even in a table with that option
-   selected, the table owner will bypass row security if the
-   <xref linkend="guc-row-security"> configuration parameter is set
-   to <literal>off</>; this setting is typically used for purposes such as
-   backup and restore.
+   TABLE ... FORCE ROW LEVEL SECURITY</>.
   </para>
 
   <para>
@@ -1606,14 +1602,6 @@ REVOKE ALL ON accounts FROM PUBLIC;
    of all roles that they are a member of.
   </para>
 
-  <para>
-   Referential integrity checks, such as unique or primary key constraints
-   and foreign key references, always bypass row security to ensure that
-   data integrity is maintained.  Care must be taken when developing
-   schemas and row level policies to avoid <quote>covert channel</> leaks of
-   information through such referential integrity checks.
-  </para>
-
   <para>
    As a simple example, here is how to create a policy on
    the <literal>account</> relation to allow only members of
@@ -1773,6 +1761,26 @@ postgres=&gt; update passwd set pwhash = 'abc';
 UPDATE 1
 </programlisting>
 
+  <para>
+   Referential integrity checks, such as unique or primary key constraints
+   and foreign key references, always bypass row security to ensure that
+   data integrity is maintained.  Care must be taken when developing
+   schemas and row level policies to avoid <quote>covert channel</> leaks of
+   information through such referential integrity checks.
+  </para>
+
+  <para>
+   In some contexts it is important to be sure that row security is
+   not being applied.  For example, when taking a backup, it could be
+   disastrous if row security silently caused some rows to be omitted
+   from the backup.  In such a situation, you can set the
+   <xref linkend="guc-row-security"> configuration parameter
+   to <literal>off</>.  This does not in itself bypass row security;
+   what it does is throw an error if any query's results would get filtered
+   by a policy.  The reason for the error can then be investigated and
+   fixed.
+  </para>
+
   <para>
    For additional details see <xref linkend="sql-createpolicy">
    and <xref linkend="sql-altertable">.

src/backend/utils/misc/rls.c

@@ -17,18 +17,17 @@
 #include "access/htup.h"
 #include "access/htup_details.h"
 #include "access/transam.h"
-#include "catalog/pg_class.h"
 #include "catalog/namespace.h"
+#include "catalog/pg_class.h"
 #include "miscadmin.h"
 #include "utils/acl.h"
 #include "utils/builtins.h"
 #include "utils/elog.h"
+#include "utils/lsyscache.h"
 #include "utils/rls.h"
 #include "utils/syscache.h"
 
 
-extern int	check_enable_rls(Oid relid, Oid checkAsUser, bool noError);
-
 /*
  * check_enable_rls
  *
@@ -52,20 +51,21 @@ extern int	check_enable_rls(Oid relid, Oid checkAsUser, bool noError);
 int
 check_enable_rls(Oid relid, Oid checkAsUser, bool noError)
 {
+	Oid			user_id = checkAsUser ? checkAsUser : GetUserId();
 	HeapTuple	tuple;
 	Form_pg_class classform;
 	bool		relrowsecurity;
 	bool		relforcerowsecurity;
-	Oid			user_id = checkAsUser ? checkAsUser : GetUserId();
+	bool		amowner;
 
 	/* Nothing to do for built-in relations */
-	if (relid < FirstNormalObjectId)
+	if (relid < (Oid) FirstNormalObjectId)
 		return RLS_NONE;
 
+	/* Fetch relation's relrowsecurity and relforcerowsecurity flags */
 	tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
 	if (!HeapTupleIsValid(tuple))
 		return RLS_NONE;
-
 	classform = (Form_pg_class) GETSTRUCT(tuple);
 
 	relrowsecurity = classform->relrowsecurity;
@@ -88,41 +88,45 @@ check_enable_rls(Oid relid, Oid checkAsUser, bool noError)
 		return RLS_NONE_ENV;
 
 	/*
-	 * Table owners generally bypass RLS, except if row_security=true and the
-	 * table has been set (by an owner) to FORCE ROW SECURITY, and this is not
-	 * a referential integrity check.
+	 * Table owners generally bypass RLS, except if the table has been set (by
+	 * an owner) to FORCE ROW SECURITY, and this is not a referential
+	 * integrity check.
 	 *
 	 * Return RLS_NONE_ENV to indicate that this decision depends on the
 	 * environment (in this case, the user_id).
 	 */
-	if (pg_class_ownercheck(relid, user_id))
+	amowner = pg_class_ownercheck(relid, user_id);
+	if (amowner)
 	{
 		/*
-		 * If row_security=true and FORCE ROW LEVEL SECURITY has been set on
-		 * the relation then we return RLS_ENABLED to indicate that RLS should
-		 * still be applied.  If we are in a SECURITY_NOFORCE_RLS context or if
-		 * row_security=false then we return RLS_NONE_ENV.
+		 * If FORCE ROW LEVEL SECURITY has been set on the relation then we
+		 * should return RLS_ENABLED to indicate that RLS should be applied.
+		 * If not, or if we are in an InNoForceRLSOperation context, we return
+		 * RLS_NONE_ENV.
 		 *
-		 * The SECURITY_NOFORCE_RLS indicates that we should not apply RLS even
-		 * if the table has FORCE RLS set- IF the current user is the owner.
-		 * This is specifically to ensure that referential integrity checks are
-		 * able to still run correctly.
+		 * InNoForceRLSOperation indicates that we should not apply RLS even
+		 * if the table has FORCE RLS set - IF the current user is the owner.
+		 * This is specifically to ensure that referential integrity checks
+		 * are able to still run correctly.
 		 *
 		 * This is intentionally only done after we have checked that the user
 		 * is the table owner, which should always be the case for referential
 		 * integrity checks.
 		 */
-		if (row_security && relforcerowsecurity && !InNoForceRLSOperation())
-			return RLS_ENABLED;
-		else
+		if (!relforcerowsecurity || InNoForceRLSOperation())
 			return RLS_NONE_ENV;
 	}
 
-	/* row_security GUC says to bypass RLS, but user lacks permission */
+	/*
+	 * We should apply RLS.  However, the user may turn off the row_security
+	 * GUC to get a forced error instead.
+	 */
 	if (!row_security && !noError)
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("insufficient privilege to bypass row-level security")));
+				 errmsg("query would be affected by row-level security policy for table \"%s\"",
+						get_rel_name(relid)),
+				 amowner ? errhint("To disable the policy for the table's owner, use ALTER TABLE NO FORCE ROW LEVEL SECURITY.") : 0));
 
 	/* RLS should be fully enabled for this relation. */
 	return RLS_ENABLED;

src/test/regress/expected/rowsecurity.out

@@ -2728,8 +2728,8 @@ COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ',';
 -- Check COPY TO as user with permissions.
 SET SESSION AUTHORIZATION rls_regress_user1;
 SET row_security TO OFF;
-COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - insufficient to bypass rls
-ERROR:  insufficient privilege to bypass row-level security
+COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - would be affected by RLS
+ERROR:  query would be affected by row-level security policy for table "copy_t"
 SET row_security TO ON;
 COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --ok
 0,cfcd208495d565ef66e7dff9f98764da
@@ -2769,8 +2769,8 @@ COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --ok
 -- Check COPY TO as user without permissions. SET row_security TO OFF;
 SET SESSION AUTHORIZATION rls_regress_user2;
 SET row_security TO OFF;
-COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - insufficient to bypass rls
-ERROR:  insufficient privilege to bypass row-level security
+COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - would be affected by RLS
+ERROR:  query would be affected by row-level security policy for table "copy_t"
 SET row_security TO ON;
 COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - permission denied
 ERROR:  permission denied for relation copy_t
@@ -2793,8 +2793,8 @@ COPY copy_rel_to TO STDOUT WITH DELIMITER ',';
 -- Check COPY TO as user with permissions.
 SET SESSION AUTHORIZATION rls_regress_user1;
 SET row_security TO OFF;
-COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - insufficient to bypass rls
-ERROR:  insufficient privilege to bypass row-level security
+COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - would be affected by RLS
+ERROR:  query would be affected by row-level security policy for table "copy_rel_to"
 SET row_security TO ON;
 COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok
 -- Check COPY TO as user with permissions and BYPASSRLS
@@ -2822,8 +2822,8 @@ COPY copy_t FROM STDIN; --ok
 -- Check COPY FROM as user with permissions.
 SET SESSION AUTHORIZATION rls_regress_user1;
 SET row_security TO OFF;
-COPY copy_t FROM STDIN; --fail - insufficient privilege to bypass rls.
-ERROR:  insufficient privilege to bypass row-level security
+COPY copy_t FROM STDIN; --fail - would be affected by RLS.
+ERROR:  query would be affected by row-level security policy for table "copy_t"
 SET row_security TO ON;
 COPY copy_t FROM STDIN; --fail - COPY FROM not supported by RLS.
 ERROR:  COPY FROM not supported with row-level security
@@ -3181,8 +3181,7 @@ SET SESSION AUTHORIZATION rls_regress_user0;
 DROP TABLE r1;
 DROP TABLE r2;
 --
--- FORCE ROW LEVEL SECURITY applies RLS to owners but
--- only when row_security = on
+-- FORCE ROW LEVEL SECURITY applies RLS to owners too
 --
 SET SESSION AUTHORIZATION rls_regress_user0;
 SET row_security = on;
@@ -3215,30 +3214,16 @@ TABLE r1;
 (0 rows)
 
 SET row_security = off;
--- Shows all rows
+-- these all fail, would be affected by RLS
 TABLE r1;
- a  
-----
- 10
- 20
-(2 rows)
-
--- Update all rows
+ERROR:  query would be affected by row-level security policy for table "r1"
+HINT:  To disable the policy for the table's owner, use ALTER TABLE NO FORCE ROW LEVEL SECURITY.
 UPDATE r1 SET a = 1;
-TABLE r1;
- a 
----
- 1
- 1
-(2 rows)
-
--- Delete all rows
+ERROR:  query would be affected by row-level security policy for table "r1"
+HINT:  To disable the policy for the table's owner, use ALTER TABLE NO FORCE ROW LEVEL SECURITY.
 DELETE FROM r1;
-TABLE r1;
- a 
----
-(0 rows)
-
+ERROR:  query would be affected by row-level security policy for table "r1"
+HINT:  To disable the policy for the table's owner, use ALTER TABLE NO FORCE ROW LEVEL SECURITY.
 DROP TABLE r1;
 --
 -- FORCE ROW LEVEL SECURITY does not break RI
@@ -3349,14 +3334,10 @@ TABLE r1;
 (0 rows)
 
 SET row_security = off;
--- Rows shown now
+-- fail, would be affected by RLS
 TABLE r1;
- a  
-----
- 10
- 20
-(2 rows)
-
+ERROR:  query would be affected by row-level security policy for table "r1"
+HINT:  To disable the policy for the table's owner, use ALTER TABLE NO FORCE ROW LEVEL SECURITY.
 SET row_security = on;
 -- Error
 INSERT INTO r1 VALUES (10), (20) RETURNING *;
@@ -3377,7 +3358,7 @@ ALTER TABLE r1 FORCE ROW LEVEL SECURITY;
 -- Works fine
 UPDATE r1 SET a = 30;
 -- Show updated rows
-SET row_security = off;
+ALTER TABLE r1 NO FORCE ROW LEVEL SECURITY;
 TABLE r1;
  a  
 ----
@@ -3393,7 +3374,7 @@ TABLE r1;
  10
 (1 row)
 
-SET row_security = on;
+ALTER TABLE r1 FORCE ROW LEVEL SECURITY;
 -- Error
 UPDATE r1 SET a = 30 RETURNING *;
 ERROR:  new row violates row-level security policy for table "r1"

src/test/regress/sql/rowsecurity.sql

@@ -1014,7 +1014,7 @@ COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ',';
 -- Check COPY TO as user with permissions.
 SET SESSION AUTHORIZATION rls_regress_user1;
 SET row_security TO OFF;
-COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - insufficient to bypass rls
+COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - would be affected by RLS
 SET row_security TO ON;
 COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --ok
 
@@ -1028,7 +1028,7 @@ COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --ok
 -- Check COPY TO as user without permissions. SET row_security TO OFF;
 SET SESSION AUTHORIZATION rls_regress_user2;
 SET row_security TO OFF;
-COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - insufficient to bypass rls
+COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - would be affected by RLS
 SET row_security TO ON;
 COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - permission denied
 
@@ -1054,7 +1054,7 @@ COPY copy_rel_to TO STDOUT WITH DELIMITER ',';
 -- Check COPY TO as user with permissions.
 SET SESSION AUTHORIZATION rls_regress_user1;
 SET row_security TO OFF;
-COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - insufficient to bypass rls
+COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - would be affected by RLS
 SET row_security TO ON;
 COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok
 
@@ -1092,7 +1092,7 @@ COPY copy_t FROM STDIN; --ok
 -- Check COPY FROM as user with permissions.
 SET SESSION AUTHORIZATION rls_regress_user1;
 SET row_security TO OFF;
-COPY copy_t FROM STDIN; --fail - insufficient privilege to bypass rls.
+COPY copy_t FROM STDIN; --fail - would be affected by RLS.
 SET row_security TO ON;
 COPY copy_t FROM STDIN; --fail - COPY FROM not supported by RLS.
 
@@ -1315,8 +1315,7 @@ DROP TABLE r1;
 DROP TABLE r2;
 
 --
--- FORCE ROW LEVEL SECURITY applies RLS to owners but
--- only when row_security = on
+-- FORCE ROW LEVEL SECURITY applies RLS to owners too
 --
 SET SESSION AUTHORIZATION rls_regress_user0;
 SET row_security = on;
@@ -1342,16 +1341,10 @@ DELETE FROM r1;
 TABLE r1;
 
 SET row_security = off;
--- Shows all rows
+-- these all fail, would be affected by RLS
 TABLE r1;
-
--- Update all rows
 UPDATE r1 SET a = 1;
-TABLE r1;
-
--- Delete all rows
 DELETE FROM r1;
-TABLE r1;
 
 DROP TABLE r1;
 
@@ -1469,7 +1462,7 @@ INSERT INTO r1 VALUES (10), (20);
 TABLE r1;
 
 SET row_security = off;
--- Rows shown now
+-- fail, would be affected by RLS
 TABLE r1;
 
 SET row_security = on;
@@ -1497,15 +1490,15 @@ ALTER TABLE r1 FORCE ROW LEVEL SECURITY;
 UPDATE r1 SET a = 30;
 
 -- Show updated rows
-SET row_security = off;
+ALTER TABLE r1 NO FORCE ROW LEVEL SECURITY;
 TABLE r1;
 -- reset value in r1 for test with RETURNING
 UPDATE r1 SET a = 10;
 
 -- Verify row reset
 TABLE r1;
 
-SET row_security = on;
+ALTER TABLE r1 FORCE ROW LEVEL SECURITY;
 
 -- Error
 UPDATE r1 SET a = 30 RETURNING *;