Plug RLS related information leak in pg_stats view.

The pg_stats view is supposed to be restricted to only show rows
about tables the user can read. However, it sometimes can leak
information which could not otherwise be seen when row level security
is enabled. Fix that by not showing pg_stats rows to users that would
be subject to RLS on the table the row is related to. This is done
by creating/using the newly introduced SQL visible function,
row_security_active().

Along the way, clean up three call sites of check_enable_rls(). The second
argument of that function should only be specified as other than
InvalidOid when we are checking as a different user than the current one,
as in when querying through a view. These sites were passing GetUserId()
instead of InvalidOid, which can cause the function to return incorrect
results if the current user has the BYPASSRLS privilege and row_security
has been set to OFF.

Additionally fix a bug causing RI Trigger error messages to unintentionally
leak information when RLS is enabled, and other minor cleanup and
improvements. Also add WITH (security_barrier) to the definition of pg_stats.

Bumped CATVERSION due to new SQL functions and pg_stats view definition.

Back-patch to 9.5 where RLS was introduced. Reported by Yaroslav.
Patch by Joe Conway and Dean Rasheed with review and input by
Michael Paquier and Stephen Frost.

8 files changed, 78 insertions, 26 deletions

diff --git a/src/backend/access/index/genam.c b/src/backend/access/index/genam.c
index 1043362f914..aa5b28c61a0 100644
--- a/src/backend/access/index/genam.c
+++ b/src/backend/access/index/genam.c
@@ -204,7 +204,7 @@ BuildIndexValueDescription(Relation indexRelation,
 	Assert(indexrelid == idxrec->indexrelid);
 
 	/* RLS check- if RLS is enabled then we don't return anything. */
-	if (check_enable_rls(indrelid, GetUserId(), true) == RLS_ENABLED)
+	if (check_enable_rls(indrelid, InvalidOid, true) == RLS_ENABLED)
 	{
 		ReleaseSysCache(ht_idx);
 		return NULL;
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index e82a53aee93..c0bd6fa96b7 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -150,7 +150,7 @@ CREATE VIEW pg_indexes AS
          LEFT JOIN pg_tablespace T ON (T.oid = I.reltablespace)
     WHERE C.relkind IN ('r', 'm') AND I.relkind = 'i';
 
-CREATE VIEW pg_stats AS
+CREATE VIEW pg_stats WITH (security_barrier) AS
     SELECT
         nspname AS schemaname,
         relname AS tablename,
@@ -211,7 +211,9 @@ CREATE VIEW pg_stats AS
     FROM pg_statistic s JOIN pg_class c ON (c.oid = s.starelid)
          JOIN pg_attribute a ON (c.oid = attrelid AND attnum = s.staattnum)
          LEFT JOIN pg_namespace n ON (n.oid = c.relnamespace)
-    WHERE NOT attisdropped AND has_column_privilege(c.oid, a.attnum, 'select');
+    WHERE NOT attisdropped
+    AND has_column_privilege(c.oid, a.attnum, 'select')
+    AND (c.relrowsecurity = false OR NOT row_security_active(c.oid));
 
 REVOKE ALL on pg_statistic FROM public;
 
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index a1561ce0cc0..2c65a901d94 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1874,7 +1874,7 @@ ExecBuildSlotValueDescription(Oid reloid,
 	 * then don't return anything.  Otherwise, go through normal permission
 	 * checks.
 	 */
-	if (check_enable_rls(reloid, GetUserId(), true) == RLS_ENABLED)
+	if (check_enable_rls(reloid, InvalidOid, true) == RLS_ENABLED)
 		return NULL;
 
 	initStringInfo(&buf);
diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
index aaf0061164b..2386cf016fb 100644
--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -107,7 +107,6 @@ get_row_security_policies(Query *root, CmdType commandType, RangeTblEntry *rte,
 
 	Relation	rel;
 	Oid			user_id;
-	int			sec_context;
 	int			rls_status;
 	bool		defaultDeny = false;
 
@@ -117,22 +116,13 @@ get_row_security_policies(Query *root, CmdType commandType, RangeTblEntry *rte,
 	*hasRowSecurity = false;
 	*hasSubLinks = false;
 
-	/* This is just to get the security context */
-	GetUserIdAndSecContext(&user_id, &sec_context);
+	/* If this is not a normal relation, just return immediately */
+	if (rte->relkind != RELKIND_RELATION)
+		return;
 
 	/* Switch to checkAsUser if it's set */
 	user_id = rte->checkAsUser ? rte->checkAsUser : GetUserId();
 
-	/*
-	 * If this is not a normal relation, or we have been told to explicitly
-	 * skip RLS (perhaps because this is an FK check) then just return
-	 * immediately.
-	 */
-	if (rte->relid < FirstNormalObjectId
-		|| rte->relkind != RELKIND_RELATION
-		|| (sec_context & SECURITY_ROW_LEVEL_DISABLED))
-		return;
-
 	/* Determine the state of RLS for this, pass checkAsUser explicitly */
 	rls_status = check_enable_rls(rte->relid, rte->checkAsUser, false);
 
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 88dd3faf2d9..61edde9c5d3 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -3243,7 +3243,7 @@ ri_ReportViolation(const RI_ConstraintInfo *riinfo,
 	 * privileges.
 	 */
 
-	if (check_enable_rls(rel_oid, GetUserId(), true) != RLS_ENABLED)
+	if (check_enable_rls(rel_oid, InvalidOid, true) != RLS_ENABLED)
 	{
 		aclresult = pg_class_aclcheck(rel_oid, GetUserId(), ACL_SELECT);
 		if (aclresult != ACLCHECK_OK)
@@ -3264,6 +3264,8 @@ ri_ReportViolation(const RI_ConstraintInfo *riinfo,
 			}
 		}
 	}
+	else
+		has_perm = false;
 
 	if (has_perm)
 	{
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index e6808e75763..525794fb644 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -153,8 +153,6 @@ CreateCachedPlan(Node *raw_parse_tree,
 	CachedPlanSource *plansource;
 	MemoryContext source_context;
 	MemoryContext oldcxt;
-	Oid			user_id;
-	int			security_context;
 
 	Assert(query_string != NULL);		/* required as of 8.4 */
 
@@ -177,8 +175,6 @@ CreateCachedPlan(Node *raw_parse_tree,
 	 */
 	oldcxt = MemoryContextSwitchTo(source_context);
 
-	GetUserIdAndSecContext(&user_id, &security_context);
-
 	plansource = (CachedPlanSource *) palloc0(sizeof(CachedPlanSource));
 	plansource->magic = CACHEDPLANSOURCE_MAGIC;
 	plansource->raw_parse_tree = copyObject(raw_parse_tree);
@@ -208,8 +204,7 @@ CreateCachedPlan(Node *raw_parse_tree,
 	plansource->total_custom_cost = 0;
 	plansource->num_custom_plans = 0;
 	plansource->hasRowSecurity = false;
-	plansource->rowSecurityDisabled
-		= (security_context & SECURITY_ROW_LEVEL_DISABLED) != 0;
+	plansource->rowSecurityDisabled = InRowLevelSecurityDisabled();
 	plansource->row_security_env = row_security;
 	plansource->planUserId = InvalidOid;
 
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index acc4752015b..ac3e764e8b8 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -341,7 +341,7 @@ GetAuthenticatedUserId(void)
  * GetUserIdAndSecContext/SetUserIdAndSecContext - get/set the current user ID
  * and the SecurityRestrictionContext flags.
  *
- * Currently there are two valid bits in SecurityRestrictionContext:
+ * Currently there are three valid bits in SecurityRestrictionContext:
  *
  * SECURITY_LOCAL_USERID_CHANGE indicates that we are inside an operation
  * that is temporarily changing CurrentUserId via these functions.  This is
@@ -359,6 +359,9 @@ GetAuthenticatedUserId(void)
  * where the called functions are really supposed to be side-effect-free
  * anyway, such as VACUUM/ANALYZE/REINDEX.
  *
+ * SECURITY_ROW_LEVEL_DISABLED indicates that we are inside an operation that
+ * needs to bypass row level security checks, for example FK checks.
+ *
  * Unlike GetUserId, GetUserIdAndSecContext does *not* Assert that the current
  * value of CurrentUserId is valid; nor does SetUserIdAndSecContext require
  * the new value to be valid.  In fact, these routines had better not
@@ -401,6 +404,15 @@ InSecurityRestrictedOperation(void)
 	return (SecurityRestrictionContext & SECURITY_RESTRICTED_OPERATION) != 0;
 }
 
+/*
+ * InRowLevelSecurityDisabled - are we inside a RLS-disabled operation?
+ */
+bool
+InRowLevelSecurityDisabled(void)
+{
+	return (SecurityRestrictionContext & SECURITY_ROW_LEVEL_DISABLED) != 0;
+}
+
 
 /*
  * These are obsolete versions of Get/SetUserIdAndSecContext that are
diff --git a/src/backend/utils/misc/rls.c b/src/backend/utils/misc/rls.c
index 44cb3743034..7b8d51d956f 100644
--- a/src/backend/utils/misc/rls.c
+++ b/src/backend/utils/misc/rls.c
@@ -16,9 +16,12 @@
 
 #include "access/htup.h"
 #include "access/htup_details.h"
+#include "access/transam.h"
 #include "catalog/pg_class.h"
+#include "catalog/namespace.h"
 #include "miscadmin.h"
 #include "utils/acl.h"
+#include "utils/builtins.h"
 #include "utils/elog.h"
 #include "utils/rls.h"
 #include "utils/syscache.h"
@@ -37,7 +40,10 @@ extern int	check_enable_rls(Oid relid, Oid checkAsUser, bool noError);
  * for the table and the plan cache needs to be invalidated if the environment
  * changes.
  *
- * Handle checking as another role via checkAsUser (for views, etc).
+ * Handle checking as another role via checkAsUser (for views, etc). Note that
+ * if *not* checking as another role, the caller should pass InvalidOid rather
+ * than GetUserId(). Otherwise the check for row_security = OFF is skipped, and
+ * so we may falsely report that RLS is active when the user has bypassed it.
  *
  * If noError is set to 'true' then we just return RLS_ENABLED instead of doing
  * an ereport() if the user has attempted to bypass RLS and they are not
@@ -53,6 +59,17 @@ check_enable_rls(Oid relid, Oid checkAsUser, bool noError)
 	bool		relrowsecurity;
 	Oid			user_id = checkAsUser ? checkAsUser : GetUserId();
 
+	/* Nothing to do for built-in relations */
+	if (relid < FirstNormalObjectId)
+		return RLS_NONE;
+
+	/*
+	 * Check if we have been told to explicitly skip RLS (perhaps because this
+	 * is a foreign key check)
+	 */
+	if (InRowLevelSecurityDisabled())
+		return RLS_NONE;
+
 	tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
 	if (!HeapTupleIsValid(tuple))
 		return RLS_NONE;
@@ -111,3 +128,37 @@ check_enable_rls(Oid relid, Oid checkAsUser, bool noError)
 	/* RLS should be fully enabled for this relation. */
 	return RLS_ENABLED;
 }
+
+/*
+ * row_security_active
+ *
+ * check_enable_rls wrapped as a SQL callable function except
+ * RLS_NONE_ENV and RLS_NONE are the same for this purpose.
+ */
+Datum
+row_security_active(PG_FUNCTION_ARGS)
+{
+	/* By OID */
+	Oid			tableoid = PG_GETARG_OID(0);
+	int			rls_status;
+
+	rls_status = check_enable_rls(tableoid, InvalidOid, true);
+	PG_RETURN_BOOL(rls_status == RLS_ENABLED);
+}
+
+Datum
+row_security_active_name(PG_FUNCTION_ARGS)
+{
+	/* By qualified name */
+	text	   *tablename = PG_GETARG_TEXT_P(0);
+	RangeVar   *tablerel;
+	Oid			tableoid;
+	int			rls_status;
+
+	/* Look up table name.  Can't lock it - we might not have privileges. */
+	tablerel = makeRangeVarFromNameList(textToQualifiedNameList(tablename));
+	tableoid = RangeVarGetRelid(tablerel, NoLock, false);
+
+	rls_status = check_enable_rls(tableoid, InvalidOid, true);
+	PG_RETURN_BOOL(rls_status == RLS_ENABLED);
+}