Remove the row_security=force GUC value.

nmisch · nmisch · commit 537bd178c73b · 2015-09-20T20:45:41.000-04:00
Every query of a single ENABLE ROW SECURITY table has two meanings, with
the row_security GUC selecting between them.  With row_security=force
available, every function author would have been advised to either set
the GUC locally or test both meanings.  Non-compliance would have
threatened reliability and, for SECURITY DEFINER functions, security.
Authors already face an obligation to account for search_path, and we
should not mimic that example.  With this change, only BYPASSRLS roles
need exercise the aforementioned care.  Back-patch to 9.5, where the
row_security GUC was introduced.

Since this narrows the domain of pg_db_role_setting.setconfig and
pg_proc.proconfig, one might bump catversion.  A row_security=force
setting in one of those columns will elicit a clear message, so don't.
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
@@ -5549,10 +5549,8 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
         to queries which are run against tables that have row security enabled.
         The default is <literal>on</>.  When set to <literal>on</>, all users,
         except superusers and the owner of the table, will have the row
-        policies for the table applied to their queries. The table owner and
-        superuser can request that row policies be applied to their queries by
-        setting this to <literal>force</>. Lastly, this can also be set to
-        <literal>off</> which will bypass row policies for the table, if
+        policies for the table applied to their queries.  When set to
+        <literal>off</>, queries will bypass row policies for the table, if
         possible, and error if not.
        </para>
 
@@ -5565,13 +5563,6 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
         returned.
        </para>
 
-       <para>
-        The allowed values of <varname>row_security</> are
-        <literal>on</> (apply normally - not to superuser or table owner),
-        <literal>off</> (fail if row security would be applied), and
-        <literal>force</> (apply always - even to superuser and table owner).
-       </para>
-
        <para>
         For more information on row security policies,
         see <xref linkend="SQL-CREATEPOLICY">.
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
@@ -1585,16 +1585,13 @@ REVOKE ALL ON accounts FROM PUBLIC;
 
   <para>
    The table owners and superusers bypass the row security system when
-   querying a table, by default.  Row security can be enabled for
-   superusers and table owners by setting
-   <xref linkend="guc-row-security"> to <literal>force</literal>.  Any
-   user can request that row security be bypassed by setting
-   <xref linkend="guc-row-security"> to <literal>off</literal>.  If
-   the user does not have privileges to bypass row security when
-   querying a given table then an error will be returned instead.  Other
-   users can be granted the ability to bypass the row security system
-   with the <literal>BYPASSRLS</literal> role attribute.  This
-   attribute can only be set by a superuser.
+   querying a table.  Any user can request that row security be bypassed by
+   setting <xref linkend="guc-row-security"> to <literal>off</literal>.  If
+   the user does not have privileges to bypass row security when querying a
+   given table then an error will be returned instead.  Other users can be
+   granted the ability to bypass the row security system with
+   the <literal>BYPASSRLS</literal> role attribute.  This attribute can only
+   be set by a superuser.
   </para>
 
   <para>
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
@@ -379,23 +379,6 @@ static const struct config_enum_entry huge_pages_options[] = {
 	{NULL, 0, false}
 };
 
-/*
- * Although only "on", "off", and "force" are documented, we
- * accept all the likely variants of "on" and "off".
- */
-static const struct config_enum_entry row_security_options[] = {
-	{"on", ROW_SECURITY_ON, false},
-	{"off", ROW_SECURITY_OFF, false},
-	{"force", ROW_SECURITY_FORCE, false},
-	{"true", ROW_SECURITY_ON, true},
-	{"false", ROW_SECURITY_OFF, true},
-	{"yes", ROW_SECURITY_ON, true},
-	{"no", ROW_SECURITY_OFF, true},
-	{"1", ROW_SECURITY_ON, true},
-	{"0", ROW_SECURITY_OFF, true},
-	{NULL, 0, false}
-};
-
 /*
  * Options for enum values stored in other modules
  */
@@ -421,6 +404,7 @@ bool		log_statement_stats = false;		/* this is sort of all three
 bool		log_btree_build_stats = false;
 char	   *event_source;
 
+bool		row_security;
 bool		check_function_bodies = true;
 bool		default_with_oids = false;
 bool		SQL_inheritance = true;
@@ -452,8 +436,6 @@ int			tcp_keepalives_idle;
 int			tcp_keepalives_interval;
 int			tcp_keepalives_count;
 
-int			row_security;
-
 /*
  * This really belongs in pg_shmem.c, but is defined here so that it doesn't
  * need to be duplicated in all the different implementations of pg_shmem.c.
@@ -1373,6 +1355,15 @@ static struct config_bool ConfigureNamesBool[] =
 		false,
 		check_transaction_deferrable, NULL, NULL
 	},
+	{
+		{"row_security", PGC_USERSET, CONN_AUTH_SECURITY,
+			gettext_noop("Enable row security."),
+			gettext_noop("When enabled, row security will be applied to all users.")
+		},
+		&row_security,
+		true,
+		NULL, NULL, NULL
+	},
 	{
 		{"check_function_bodies", PGC_USERSET, CLIENT_CONN_STATEMENT,
 			gettext_noop("Check function bodies during CREATE FUNCTION."),
@@ -3630,16 +3621,6 @@ static struct config_enum ConfigureNamesEnum[] =
 		NULL, NULL, NULL
 	},
 
-	{
-		{"row_security", PGC_USERSET, CONN_AUTH_SECURITY,
-			gettext_noop("Enable row security."),
-			gettext_noop("When enabled, row security will be applied to all users.")
-		},
-		&row_security,
-		ROW_SECURITY_ON, row_security_options,
-		NULL, NULL, NULL
-	},
-
 	/* End-of-list marker */
 	{
 		{NULL, 0, 0, NULL, NULL}, NULL, 0, NULL, NULL, NULL, NULL
diff --git a/src/backend/utils/misc/rls.c b/src/backend/utils/misc/rls.c
@@ -87,32 +87,19 @@ check_enable_rls(Oid relid, Oid checkAsUser, bool noError)
 	/*
 	 * Check permissions
 	 *
-	 * If the relation has row level security enabled and the row_security GUC
-	 * is off, then check if the user has rights to bypass RLS for this
-	 * relation.  Table owners can always bypass, as can any role with the
-	 * BYPASSRLS capability.
-	 *
-	 * If the role is the table owner, then we bypass RLS unless row_security
-	 * is set to 'force'.  Note that superuser is always considered an owner.
-	 *
-	 * Return RLS_NONE_ENV to indicate that this decision depends on the
-	 * environment (in this case, what the current values of user_id and
-	 * row_security are).
+	 * Table owners always bypass RLS.  Note that superuser is always
+	 * considered an owner.  Return RLS_NONE_ENV to indicate that this
+	 * decision depends on the environment (in this case, the user_id).
 	 */
-	if (row_security != ROW_SECURITY_FORCE
-		&& (pg_class_ownercheck(relid, user_id)))
+	if (pg_class_ownercheck(relid, user_id))
 		return RLS_NONE_ENV;
 
 	/*
-	 * If the row_security GUC is 'off' then check if the user has permission
-	 * to bypass it.  Note that we have already handled the case where the
-	 * user is the table owner above.
-	 *
-	 * Note that row_security is always considered 'on' when querying through
-	 * a view or other cases where checkAsUser is true, so skip this if
-	 * checkAsUser is in use.
+	 * If the row_security GUC is 'off', check if the user has permission to
+	 * bypass RLS.  row_security is always considered 'on' when querying
+	 * through a view or other cases where checkAsUser is valid.
 	 */
-	if (!checkAsUser && row_security == ROW_SECURITY_OFF)
+	if (!row_security && !checkAsUser)
 	{
 		if (has_bypassrls_privilege(user_id))
 			/* OK to bypass */
diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
@@ -110,7 +110,7 @@ typedef struct CachedPlanSource
 	double		total_custom_cost;		/* total cost of custom plans so far */
 	int			num_custom_plans;		/* number of plans included in total */
 	bool		hasRowSecurity; /* planned with row security? */
-	int			row_security_env;		/* row security setting when planned */
+	bool		row_security_env;		/* row security setting when planned */
 	bool		rowSecurityDisabled;	/* is row security disabled? */
 } CachedPlanSource;
 
diff --git a/src/include/utils/rls.h b/src/include/utils/rls.h
@@ -14,23 +14,15 @@
 #define RLS_H
 
 /* GUC variable */
-extern int	row_security;
-
-/* Possible values for row_security GUC */
-typedef enum RowSecurityConfigType
-{
-	ROW_SECURITY_OFF,			/* RLS never applied- error thrown if no priv */
-	ROW_SECURITY_ON,			/* normal case, RLS applied for regular users */
-	ROW_SECURITY_FORCE			/* RLS applied for superusers and table owners */
-}	RowSecurityConfigType;
+extern bool row_security;
 
 /*
  * Used by callers of check_enable_rls.
  *
  * RLS could be completely disabled on the tables involved in the query,
  * which is the simple case, or it may depend on the current environment
  * (the role which is running the query or the value of the row_security
- * GUC- on, off, or force), or it might be simply enabled as usual.
+ * GUC), or it might be simply enabled as usual.
  *
  * If RLS isn't on the table involved then RLS_NONE is returned to indicate
  * that we don't need to worry about invalidating the query plan for RLS
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
