On 8 October 2015 at 15:05, Dean Rasheed <dean.a.rash...@gmail.com> wrote:
> Attached is a simple patch that appears to work, but it needs more
> testing (and some regression tests).
>

Here's an updated patch with an extra regression test case that
triggers the issue.

I've also updated the function comment for expand_security_quals() to
better explain the situations where it actually has work to do --
tables with RLS and updates to auto-updatable security barrier views,
but not SELECTs from security berrier views. This explains why this
bug doesn't affect security barrier views (UNION ALL views aren't
auto-updatable), so only 9.5 and HEAD need to be patched.

Regards,
Dean
diff --git a/src/backend/optimizer/prep/prepsecurity.c b/src/backend/optimizer/prep/prepsecurity.c
new file mode 100644
index c4b61df..ee1e1e4
--- a/src/backend/optimizer/prep/prepsecurity.c
+++ b/src/backend/optimizer/prep/prepsecurity.c
@@ -56,6 +56,12 @@ static bool security_barrier_replace_var
  * the others, providing protection against malicious user-defined security
  * barriers.  The first security barrier qual in the list will be used in the
  * innermost subquery.
+ *
+ * In practice, the only RTEs that will have security barrier quals are those
+ * that refer to tables with row-level security, or which are the target
+ * relation of an update to an auto-updatable security barrier view.  RTEs
+ * that read from a security barrier view will have already been expanded by
+ * the rewriter.
  */
 void
 expand_security_quals(PlannerInfo *root, List *tlist)
@@ -263,7 +269,8 @@ expand_security_qual(PlannerInfo *root,
 			 * Replace any variables in the outer query that refer to the
 			 * original relation RTE with references to columns that we will
 			 * expose in the new subquery, building the subquery's targetlist
-			 * as we go.
+			 * as we go.  Also replace any references in the translated_vars
+			 * lists of any appendrels.
 			 */
 			context.rt_index = rt_index;
 			context.sublevels_up = 0;
@@ -274,6 +281,8 @@ expand_security_qual(PlannerInfo *root,
 
 			security_barrier_replace_vars((Node *) parse, &context);
 			security_barrier_replace_vars((Node *) tlist, &context);
+			security_barrier_replace_vars((Node *) root->append_rel_list,
+										  &context);
 
 			heap_close(context.rel, NoLock);
 
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
new file mode 100644
index 6becf59..c844499
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -640,6 +640,26 @@ EXPLAIN (COSTS OFF) SELECT * FROM t1 WHE
                                  Filter: ((a % 2) = 0)
 (12 rows)
 
+-- union all query
+SELECT a, b, oid FROM t2 UNION ALL SELECT a, b, oid FROM t3;
+ a |  b  | oid 
+---+-----+-----
+ 1 | abc | 201
+ 3 | cde | 203
+ 1 | xxx | 301
+ 2 | yyy | 302
+ 3 | zzz | 303
+(5 rows)
+
+EXPLAIN (COSTS OFF) SELECT a, b, oid FROM t2 UNION ALL SELECT a, b, oid FROM t3;
+          QUERY PLAN           
+-------------------------------
+ Append
+   ->  Seq Scan on t2
+         Filter: ((a % 2) = 1)
+   ->  Seq Scan on t3
+(4 rows)
+
 -- superuser is allowed to bypass RLS checks
 RESET SESSION AUTHORIZATION;
 SET row_security TO OFF;
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
new file mode 100644
index 662f520..b230a0f
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -255,6 +255,10 @@ EXPLAIN (COSTS OFF) SELECT * FROM t1 FOR
 SELECT * FROM t1 WHERE f_leak(b) FOR SHARE;
 EXPLAIN (COSTS OFF) SELECT * FROM t1 WHERE f_leak(b) FOR SHARE;
 
+-- union all query
+SELECT a, b, oid FROM t2 UNION ALL SELECT a, b, oid FROM t3;
+EXPLAIN (COSTS OFF) SELECT a, b, oid FROM t2 UNION ALL SELECT a, b, oid FROM t3;
+
 -- superuser is allowed to bypass RLS checks
 RESET SESSION AUTHORIZATION;
 SET row_security TO OFF;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to