On 8 October 2015 at 15:05, Dean Rasheed <[email protected]> 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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers