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