From 3cb0a7e75aaa9a7826c769068970ce2200e61023 Mon Sep 17 00:00:00 2001
From: Noah Misch <noah@leadboat.com>
Date: Sat, 3 Oct 2015 20:19:57 -0400
Subject: [PATCH] Make BYPASSRLS behave like superuser RLS bypass.
Specifically, make its effect independent from the row_security GUC, and
make it affect permission checks pertinent to views the BYPASSRLS role
owns. The row_security GUC thereby ceases to change successful-query
behavior; it can only make a query fail with an error. Back-patch to
9.5, where BYPASSRLS was introduced.
---
doc/src/sgml/catalogs.sgml | 6 ++--
doc/src/sgml/config.sgml | 25 ++++++---------
doc/src/sgml/ddl.sgml | 19 ++++-------
doc/src/sgml/ref/create_role.sgml | 9 ++----
src/backend/utils/misc/rls.c | 39 +++++++----------------
src/include/catalog/pg_authid.h | 2 +-
src/test/regress/expected/rowsecurity.out | 14 ++++----
src/test/regress/sql/rowsecurity.sql | 6 ++--
8 files changed, 44 insertions(+), 76 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index e8bd3a11c8c..28bb480447e 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -1454,7 +1454,7 @@
<entry><structfield>rolbypassrls</structfield></entry>
<entry><type>bool</type></entry>
<entry>
- Role can bypass row level security policies, see
+ Role bypasses every row level security policy, see
<xref linkend="ddl-rowsecurity"> for more information.
</entry>
</row>
@@ -9413,7 +9413,7 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
<entry><type>bool</type></entry>
<entry></entry>
<entry>
- User can bypass row level security policies, see
+ User bypasses every row level security policy, see
<xref linkend="ddl-rowsecurity"> for more information.
</entry>
</row>
@@ -9888,7 +9888,7 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
<entry><structfield>usebypassrls</structfield></entry>
<entry><type>bool</type></entry>
<entry>
- User can bypass row level security policies, see
+ User bypasses every row level security policy, see
<xref linkend="ddl-rowsecurity"> for more information.
</entry>
</row>
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 696f286b760..c174d68dc13 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5591,22 +5591,15 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
</term>
<listitem>
<para>
- This variable controls if row security policies are to be applied
- 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. When set to
- <literal>off</>, queries will bypass row policies for the table, if
- possible, and error if not.
- </para>
-
- <para>
- For a user who is not a superuser and not the table owner to bypass
- row policies for the table, they must have the <literal>BYPASSRLS</>
- role attribute. If this is set to <literal>off</> and the user queries
- a table which has row policies enabled and the user does not have the
- right to bypass row policies then a permission denied error will be
- returned.
+ This variable controls whether to raise an error in lieu of applying a
+ row security policy. When set to <literal>on</>, policies apply
+ normally. When set to <literal>off</>, queries fail which would
+ otherwise apply at least one policy. The default is <literal>on</>.
+ Change to <literal>off</> where limited row visibility could cause
+ incorrect results; for example, <application>pg_dump</> makes that
+ change by default. This variable has no effect on roles which bypass
+ every row security policy, to wit, superusers and roles with
+ the <literal>BYPASSRLS</> attribute.
</para>
<para>
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 052cf25ef3b..a889ffc5472 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -1543,8 +1543,12 @@ REVOKE ALL ON accounts FROM PUBLIC;
Row security policies can be specific to commands, or to roles, or to
both. The commands available are <literal>ALL</literal>,
<literal>SELECT</>, <literal>INSERT</>, <literal>UPDATE</>, and
- <literal>DELETE</>. Multiple roles can be assigned to a given policy
- and normal role membership and inheritance rules apply.
+ <literal>DELETE</>. Multiple roles can be assigned to a given policy and
+ normal role membership and inheritance rules apply. Table owners,
+ superusers, and roles with the <literal>BYPASSRLS</> attribute bypass the
+ row security system when querying a table. Applications that expect to
+ bypass all row security through those mechanisms should
+ set <xref linkend="guc-row-security"> to <literal>off</>.
</para>
<para>
@@ -1574,17 +1578,6 @@ REVOKE ALL ON accounts FROM PUBLIC;
<xref linkend="sql-altertable"> command.
</para>
- <para>
- The table owners and superusers bypass the row security system when
- 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>
Each policy has a name and multiple policies can be defined for a
table. As policies are table-specific, each policy for a table must
diff --git a/doc/src/sgml/ref/create_role.sgml b/doc/src/sgml/ref/create_role.sgml
index f4a176bff3f..240c21ce85f 100644
--- a/doc/src/sgml/ref/create_role.sgml
+++ b/doc/src/sgml/ref/create_role.sgml
@@ -196,16 +196,13 @@ CREATE ROLE <replaceable class="PARAMETER">name</replaceable> [ [ WITH ] <replac
<term><literal>NOBYPASSRLS</literal></term>
<listitem>
<para>
- These clauses determine whether a role is allowed to bypass row-level security (RLS)
- policies. A role having the <literal>BYPASSRLS</literal> attribute will
- be allowed to bypass row-security policies by setting
- <literal>row_security</literal> to
- <literal>OFF</literal>. <literal>NOBYPASSRLS</literal> is the default.
+ These clauses determine whether a role bypasses every row-level
+ security (RLS) policy. <literal>NOBYPASSRLS</literal> is the default.
Note that pg_dump will set <literal>row_security</literal> to
<literal>OFF</literal> by default, to ensure all contents of a table are
dumped out. If the user running pg_dump does not have appropriate
permissions, an error will be returned. The superuser and owner of the
- table being dumped are considered to always have the right to bypass RLS.
+ table being dumped always bypass RLS.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/utils/misc/rls.c b/src/backend/utils/misc/rls.c
index c900c98848b..eaf9d6e66ff 100644
--- a/src/backend/utils/misc/rls.c
+++ b/src/backend/utils/misc/rls.c
@@ -40,10 +40,8 @@ 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). 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.
+ * Handle checking as another role via checkAsUser (for views, etc). Pass
+ * InvalidOid to check the current user.
*
* 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
@@ -78,32 +76,19 @@ check_enable_rls(Oid relid, Oid checkAsUser, bool noError)
return RLS_NONE;
/*
- * Check permissions
- *
- * 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).
+ * Table owners and BYPASSRLS users bypass RLS. Note that a superuser
+ * qualifies as both. 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))
+ if (pg_class_ownercheck(relid, user_id) ||
+ has_bypassrls_privilege(user_id))
return RLS_NONE_ENV;
- /*
- * 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 (!row_security && !checkAsUser)
- {
- if (has_bypassrls_privilege(user_id))
- /* OK to bypass */
- return RLS_NONE_ENV;
- else if (noError)
- return RLS_ENABLED;
- else
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("insufficient privilege to bypass row security.")));
- }
+ /* row_security GUC says to bypass RLS, but user lacks permission */
+ if (!row_security && !noError)
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("insufficient privilege to bypass row security.")));
/* RLS should be fully enabled for this relation. */
return RLS_ENABLED;
diff --git a/src/include/catalog/pg_authid.h b/src/include/catalog/pg_authid.h
index d5f19d6aabb..2c8565ea254 100644
--- a/src/include/catalog/pg_authid.h
+++ b/src/include/catalog/pg_authid.h
@@ -51,7 +51,7 @@ CATALOG(pg_authid,1260) BKI_SHARED_RELATION BKI_ROWTYPE_OID(2842) BKI_SCHEMA_MAC
bool rolcreatedb; /* allowed to create databases? */
bool rolcanlogin; /* allowed to log in as session user? */
bool rolreplication; /* role used for streaming replication */
- bool rolbypassrls; /* allowed to bypass row level security? */
+ bool rolbypassrls; /* bypasses row level security? */
int32 rolconnlimit; /* max connections allowed (-1=no limit) */
/* remaining fields may be null; use heap_getattr to read them! */
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 0cde8fd0e3f..0363dfd07ff 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -2584,10 +2584,15 @@ COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --ok
SET row_security TO ON;
COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --ok
0,cfcd208495d565ef66e7dff9f98764da
+1,c4ca4238a0b923820dcc509a6f75849b
2,c81e728d9d4c2f636f067f89cc14862c
+3,eccbc87e4b5ce2fe28308fd9f2a7baf3
4,a87ff679a2f3e71d9181a67b7542122c
+5,e4da3b7fbbce2345d7772b0674a318d5
6,1679091c5a880faf6fb5e6087eb1b2dc
+7,8f14e45fceea167a5a36dedd4bea2543
8,c9f0f895fb98ab9159f51fd0297e236d
+9,45c48cce2e2d7fbdea1afc51c7c6ad26
10,d3d9446802a44259755d38e6d163e820
-- Check COPY TO as user without permissions. SET row_security TO OFF;
SET SESSION AUTHORIZATION rls_regress_user2;
@@ -2627,6 +2632,7 @@ COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok
1,c4ca4238a0b923820dcc509a6f75849b
SET row_security TO ON;
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok
+1,c4ca4238a0b923820dcc509a6f75849b
-- Check COPY TO as user without permissions. SET row_security TO OFF;
SET SESSION AUTHORIZATION rls_regress_user2;
SET row_security TO OFF;
@@ -2650,14 +2656,10 @@ 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.
HINT: Use direct INSERT statements instead.
--- Check COPY TO as user with permissions and BYPASSRLS
+-- Check COPY FROM as user with permissions and BYPASSRLS
SET SESSION AUTHORIZATION rls_regress_exempt_user;
-SET row_security TO OFF;
-COPY copy_t FROM STDIN; --ok
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.
-HINT: Use direct INSERT statements instead.
+COPY copy_t FROM STDIN; --ok
-- Check COPY FROM as user without permissions.
SET SESSION AUTHORIZATION rls_regress_user2;
SET row_security TO OFF;
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index 6ed0daf345f..7f8772fa26c 100644
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -1070,17 +1070,15 @@ COPY copy_t FROM STDIN; --fail - insufficient privilege to bypass rls.
SET row_security TO ON;
COPY copy_t FROM STDIN; --fail - COPY FROM not supported by RLS.
--- Check COPY TO as user with permissions and BYPASSRLS
+-- Check COPY FROM as user with permissions and BYPASSRLS
SET SESSION AUTHORIZATION rls_regress_exempt_user;
-SET row_security TO OFF;
+SET row_security TO ON;
COPY copy_t FROM STDIN; --ok
1 abc
2 bcd
3 cde
4 def
\.
-SET row_security TO ON;
-COPY copy_t FROM STDIN; --fail - COPY FROM not supported by RLS.
-- Check COPY FROM as user without permissions.
SET SESSION AUTHORIZATION rls_regress_user2;
--
GitLab