diff options
Diffstat (limited to 'src')
-rw-r--r-- | src/backend/optimizer/path/allpaths.c | 6 | ||||
-rw-r--r-- | src/backend/optimizer/util/clauses.c | 96 | ||||
-rw-r--r-- | src/include/optimizer/clauses.h | 2 | ||||
-rw-r--r-- | src/test/modules/test_rls_hooks/expected/test_rls_hooks.out | 12 | ||||
-rw-r--r-- | src/test/regress/expected/rowsecurity.out | 106 | ||||
-rw-r--r-- | src/test/regress/expected/select_views.out | 46 | ||||
-rw-r--r-- | src/test/regress/expected/select_views_1.out | 46 | ||||
-rw-r--r-- | src/test/regress/sql/rowsecurity.sql | 20 | ||||
-rw-r--r-- | src/test/regress/sql/select_views.sql | 14 |
9 files changed, 313 insertions, 35 deletions
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index c4b0c79fb17..9caca94f64b 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -1982,7 +1982,9 @@ targetIsInAllPartitionLists(TargetEntry *tle, Query *query) * 2. If unsafeVolatile is set, the qual must not contain any volatile * functions. * - * 3. If unsafeLeaky is set, the qual must not contain any leaky functions. + * 3. If unsafeLeaky is set, the qual must not contain any leaky functions + * that are passed Var nodes, and therefore might reveal values from the + * subquery as side effects. * * 4. The qual must not refer to the whole-row output of the subquery * (since there is no easy way to name that within the subquery itself). @@ -2009,7 +2011,7 @@ qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual, /* Refuse leaky quals if told to (point 3) */ if (safetyInfo->unsafeLeaky && - contain_leaky_functions(qual)) + contain_leaked_vars(qual)) return false; /* diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 84d58ae595e..480114d92b6 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -97,7 +97,7 @@ static bool contain_mutable_functions_walker(Node *node, void *context); static bool contain_volatile_functions_walker(Node *node, void *context); static bool contain_volatile_functions_not_nextval_walker(Node *node, void *context); static bool contain_nonstrict_functions_walker(Node *node, void *context); -static bool contain_leaky_functions_walker(Node *node, void *context); +static bool contain_leaked_vars_walker(Node *node, void *context); static Relids find_nonnullable_rels_walker(Node *node, bool top_level); static List *find_nonnullable_vars_walker(Node *node, bool top_level); static bool is_strict_saop(ScalarArrayOpExpr *expr, bool falseOK); @@ -1318,26 +1318,30 @@ contain_nonstrict_functions_walker(Node *node, void *context) } /***************************************************************************** - * Check clauses for non-leakproof functions + * Check clauses for Vars passed to non-leakproof functions *****************************************************************************/ /* - * contain_leaky_functions - * Recursively search for leaky functions within a clause. + * contain_leaked_vars + * Recursively scan a clause to discover whether it contains any Var + * nodes (of the current query level) that are passed as arguments to + * leaky functions. * - * Returns true if any function call with side-effect may be present in the - * clause. Qualifiers from outside the a security_barrier view should not - * be pushed down into the view, lest the contents of tuples intended to be - * filtered out be revealed via side effects. + * Returns true if the clause contains any non-leakproof functions that are + * passed Var nodes of the current query level, and which might therefore leak + * data. Qualifiers from outside a security_barrier view that might leak data + * in this way should not be pushed down into the view in case the contents of + * tuples intended to be filtered out by the view are revealed by the leaky + * functions. */ bool -contain_leaky_functions(Node *clause) +contain_leaked_vars(Node *clause) { - return contain_leaky_functions_walker(clause, NULL); + return contain_leaked_vars_walker(clause, NULL); } static bool -contain_leaky_functions_walker(Node *node, void *context) +contain_leaked_vars_walker(Node *node, void *context) { if (node == NULL) return false; @@ -1369,7 +1373,8 @@ contain_leaky_functions_walker(Node *node, void *context) { FuncExpr *expr = (FuncExpr *) node; - if (!get_func_leakproof(expr->funcid)) + if (!get_func_leakproof(expr->funcid) && + contain_var_clause((Node *) expr->args)) return true; } break; @@ -1381,7 +1386,8 @@ contain_leaky_functions_walker(Node *node, void *context) OpExpr *expr = (OpExpr *) node; set_opfuncid(expr); - if (!get_func_leakproof(expr->opfuncid)) + if (!get_func_leakproof(expr->opfuncid) && + contain_var_clause((Node *) expr->args)) return true; } break; @@ -1391,7 +1397,8 @@ contain_leaky_functions_walker(Node *node, void *context) ScalarArrayOpExpr *expr = (ScalarArrayOpExpr *) node; set_sa_opfuncid(expr); - if (!get_func_leakproof(expr->opfuncid)) + if (!get_func_leakproof(expr->opfuncid) && + contain_var_clause((Node *) expr->args)) return true; } break; @@ -1401,15 +1408,29 @@ contain_leaky_functions_walker(Node *node, void *context) CoerceViaIO *expr = (CoerceViaIO *) node; Oid funcid; Oid ioparam; + bool leakproof; bool varlena; + /* + * Data may be leaked if either the input or the output + * function is leaky. + */ getTypeInputInfo(exprType((Node *) expr->arg), &funcid, &ioparam); - if (!get_func_leakproof(funcid)) - return true; + leakproof = get_func_leakproof(funcid); + + /* + * If the input function is leakproof, then check the output + * function. + */ + if (leakproof) + { + getTypeOutputInfo(expr->resulttype, &funcid, &varlena); + leakproof = get_func_leakproof(funcid); + } - getTypeOutputInfo(expr->resulttype, &funcid, &varlena); - if (!get_func_leakproof(funcid)) + if (!leakproof && + contain_var_clause((Node *) expr->arg)) return true; } break; @@ -1419,14 +1440,29 @@ contain_leaky_functions_walker(Node *node, void *context) ArrayCoerceExpr *expr = (ArrayCoerceExpr *) node; Oid funcid; Oid ioparam; + bool leakproof; bool varlena; + /* + * Data may be leaked if either the input or the output + * function is leaky. + */ getTypeInputInfo(exprType((Node *) expr->arg), &funcid, &ioparam); - if (!get_func_leakproof(funcid)) - return true; - getTypeOutputInfo(expr->resulttype, &funcid, &varlena); - if (!get_func_leakproof(funcid)) + leakproof = get_func_leakproof(funcid); + + /* + * If the input function is leakproof, then check the output + * function. + */ + if (leakproof) + { + getTypeOutputInfo(expr->resulttype, &funcid, &varlena); + leakproof = get_func_leakproof(funcid); + } + + if (!leakproof && + contain_var_clause((Node *) expr->arg)) return true; } break; @@ -1435,12 +1471,22 @@ contain_leaky_functions_walker(Node *node, void *context) { RowCompareExpr *rcexpr = (RowCompareExpr *) node; ListCell *opid; + ListCell *larg; + ListCell *rarg; - foreach(opid, rcexpr->opnos) + /* + * Check the comparison function and arguments passed to it for + * each pair of row elements. + */ + forthree(opid, rcexpr->opnos, + larg, rcexpr->largs, + rarg, rcexpr->rargs) { Oid funcid = get_opcode(lfirst_oid(opid)); - if (!get_func_leakproof(funcid)) + if (!get_func_leakproof(funcid) && + (contain_var_clause((Node *) lfirst(larg)) || + contain_var_clause((Node *) lfirst(rarg)))) return true; } } @@ -1455,7 +1501,7 @@ contain_leaky_functions_walker(Node *node, void *context) */ return true; } - return expression_tree_walker(node, contain_leaky_functions_walker, + return expression_tree_walker(node, contain_leaked_vars_walker, context); } diff --git a/src/include/optimizer/clauses.h b/src/include/optimizer/clauses.h index 160045cf417..3d04ac2a98d 100644 --- a/src/include/optimizer/clauses.h +++ b/src/include/optimizer/clauses.h @@ -63,7 +63,7 @@ extern bool contain_mutable_functions(Node *clause); extern bool contain_volatile_functions(Node *clause); extern bool contain_volatile_functions_not_nextval(Node *clause); extern bool contain_nonstrict_functions(Node *clause); -extern bool contain_leaky_functions(Node *clause); +extern bool contain_leaked_vars(Node *clause); extern Relids find_nonnullable_rels(Node *clause); extern List *find_nonnullable_vars(Node *clause); diff --git a/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out b/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out index 9427a6fae80..3a7a4c329f3 100644 --- a/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out +++ b/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out @@ -85,13 +85,11 @@ SET ROLE s1; -- restrictive hook's policy is current_user = superuser -- combined with AND, results in nothing being allowed EXPLAIN (costs off) SELECT * FROM rls_test_both; - QUERY PLAN -------------------------------------------------------- - Subquery Scan on rls_test_both - Filter: ("current_user"() = rls_test_both.username) - -> Seq Scan on rls_test_both rls_test_both_1 - Filter: ("current_user"() = supervisor) -(4 rows) + QUERY PLAN +------------------------------------------------------------------------------- + Seq Scan on rls_test_both + Filter: ((supervisor = "current_user"()) AND (username = "current_user"())) +(2 rows) SELECT * FROM rls_test_both; username | supervisor | data diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index 1ea65a7d8a1..ad936321749 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -1914,6 +1914,112 @@ EXPLAIN (COSTS OFF) SELECT * FROM y2 WHERE f_leak(b); (4 rows) -- +-- Qual push-down of leaky functions, when not referring to table +-- +SELECT * FROM y2 WHERE f_leak('abc'); +NOTICE: f_leak => abc +NOTICE: f_leak => abc +NOTICE: f_leak => abc +NOTICE: f_leak => abc +NOTICE: f_leak => abc +NOTICE: f_leak => abc +NOTICE: f_leak => abc +NOTICE: f_leak => abc +NOTICE: f_leak => abc +NOTICE: f_leak => abc +NOTICE: f_leak => abc +NOTICE: f_leak => abc +NOTICE: f_leak => abc +NOTICE: f_leak => abc +NOTICE: f_leak => abc +NOTICE: f_leak => abc +NOTICE: f_leak => abc +NOTICE: f_leak => abc +NOTICE: f_leak => abc +NOTICE: f_leak => abc +NOTICE: f_leak => abc + a | b +----+---------------------------------- + 0 | cfcd208495d565ef66e7dff9f98764da + 2 | c81e728d9d4c2f636f067f89cc14862c + 3 | eccbc87e4b5ce2fe28308fd9f2a7baf3 + 4 | a87ff679a2f3e71d9181a67b7542122c + 6 | 1679091c5a880faf6fb5e6087eb1b2dc + 8 | c9f0f895fb98ab9159f51fd0297e236d + 9 | 45c48cce2e2d7fbdea1afc51c7c6ad26 + 10 | d3d9446802a44259755d38e6d163e820 + 12 | c20ad4d76fe97759aa27a0c99bff6710 + 14 | aab3238922bcc25a6f606eb525ffdc56 + 15 | 9bf31c7ff062936a96d3c8bd1f8f2ff3 + 16 | c74d97b01eae257e44aa9d5bade97baf + 18 | 6f4922f45568161a8cdf4ad2299f6d23 + 20 | 98f13708210194c475687be6106a3b84 +(14 rows) + +EXPLAIN (COSTS OFF) SELECT * FROM y2 WHERE f_leak('abc'); + QUERY PLAN +--------------------------------------------------------------------------------------- + Seq Scan on y2 + Filter: (f_leak('abc'::text) AND (((a % 4) = 0) OR ((a % 3) = 0) OR ((a % 2) = 0))) +(2 rows) + +CREATE TABLE test_qual_pushdown ( + abc text +); +INSERT INTO test_qual_pushdown VALUES ('abc'),('def'); +SELECT * FROM y2 JOIN test_qual_pushdown ON (b = abc) WHERE f_leak(abc); +NOTICE: f_leak => abc +NOTICE: f_leak => def + a | b | abc +---+---+----- +(0 rows) + +EXPLAIN (COSTS OFF) SELECT * FROM y2 JOIN test_qual_pushdown ON (b = abc) WHERE f_leak(abc); + QUERY PLAN +------------------------------------------------------------------------- + Hash Join + Hash Cond: (test_qual_pushdown.abc = y2.b) + -> Seq Scan on test_qual_pushdown + Filter: f_leak(abc) + -> Hash + -> Seq Scan on y2 + Filter: (((a % 4) = 0) OR ((a % 3) = 0) OR ((a % 2) = 0)) +(7 rows) + +SELECT * FROM y2 JOIN test_qual_pushdown ON (b = abc) WHERE f_leak(b); +NOTICE: f_leak => cfcd208495d565ef66e7dff9f98764da +NOTICE: f_leak => c81e728d9d4c2f636f067f89cc14862c +NOTICE: f_leak => eccbc87e4b5ce2fe28308fd9f2a7baf3 +NOTICE: f_leak => a87ff679a2f3e71d9181a67b7542122c +NOTICE: f_leak => 1679091c5a880faf6fb5e6087eb1b2dc +NOTICE: f_leak => c9f0f895fb98ab9159f51fd0297e236d +NOTICE: f_leak => 45c48cce2e2d7fbdea1afc51c7c6ad26 +NOTICE: f_leak => d3d9446802a44259755d38e6d163e820 +NOTICE: f_leak => c20ad4d76fe97759aa27a0c99bff6710 +NOTICE: f_leak => aab3238922bcc25a6f606eb525ffdc56 +NOTICE: f_leak => 9bf31c7ff062936a96d3c8bd1f8f2ff3 +NOTICE: f_leak => c74d97b01eae257e44aa9d5bade97baf +NOTICE: f_leak => 6f4922f45568161a8cdf4ad2299f6d23 +NOTICE: f_leak => 98f13708210194c475687be6106a3b84 + a | b | abc +---+---+----- +(0 rows) + +EXPLAIN (COSTS OFF) SELECT * FROM y2 JOIN test_qual_pushdown ON (b = abc) WHERE f_leak(b); + QUERY PLAN +------------------------------------------------------------------------------- + Hash Join + Hash Cond: (test_qual_pushdown.abc = y2.b) + -> Seq Scan on test_qual_pushdown + -> Hash + -> Subquery Scan on y2 + Filter: f_leak(y2.b) + -> Seq Scan on y2 y2_1 + Filter: (((a % 4) = 0) OR ((a % 3) = 0) OR ((a % 2) = 0)) +(8 rows) + +DROP TABLE test_qual_pushdown; +-- -- Plancache invalidate on user change. -- RESET SESSION AUTHORIZATION; diff --git a/src/test/regress/expected/select_views.out b/src/test/regress/expected/select_views.out index 82d510de806..7f575266c1b 100644 --- a/src/test/regress/expected/select_views.out +++ b/src/test/regress/expected/select_views.out @@ -1349,6 +1349,52 @@ EXPLAIN (COSTS OFF) SELECT * FROM my_property_secure WHERE f_leak(passwd); (4 rows) -- +-- scenario: qualifiers can be pushed down if they contain leaky functions, +-- provided they aren't passed data from inside the view. +-- +SELECT * FROM my_property_normal v + WHERE f_leak('passwd') AND f_leak(passwd); +NOTICE: f_leak => passwd +NOTICE: f_leak => passwd123 +NOTICE: f_leak => passwd +NOTICE: f_leak => beafsteak +NOTICE: f_leak => passwd +NOTICE: f_leak => hamburger + cid | name | tel | passwd +-----+---------------+------------------+----------- + 101 | regress_alice | +81-12-3456-7890 | passwd123 +(1 row) + +EXPLAIN (COSTS OFF) SELECT * FROM my_property_normal v + WHERE f_leak('passwd') AND f_leak(passwd); + QUERY PLAN +--------------------------------------------------------------------------------------------- + Seq Scan on customer + Filter: (f_leak('passwd'::text) AND f_leak(passwd) AND (name = ("current_user"())::text)) +(2 rows) + +SELECT * FROM my_property_secure v + WHERE f_leak('passwd') AND f_leak(passwd); +NOTICE: f_leak => passwd +NOTICE: f_leak => passwd123 +NOTICE: f_leak => passwd +NOTICE: f_leak => passwd + cid | name | tel | passwd +-----+---------------+------------------+----------- + 101 | regress_alice | +81-12-3456-7890 | passwd123 +(1 row) + +EXPLAIN (COSTS OFF) SELECT * FROM my_property_secure v + WHERE f_leak('passwd') AND f_leak(passwd); + QUERY PLAN +-------------------------------------------------------------------------------- + Subquery Scan on v + Filter: f_leak(v.passwd) + -> Seq Scan on customer + Filter: (f_leak('passwd'::text) AND (name = ("current_user"())::text)) +(4 rows) + +-- -- scenario: if a qualifier references only one-side of a particular join- -- tree, it shall be distributed to the most deep scan plan as -- possible as we can. diff --git a/src/test/regress/expected/select_views_1.out b/src/test/regress/expected/select_views_1.out index ce22bfabeca..5275ef0b2de 100644 --- a/src/test/regress/expected/select_views_1.out +++ b/src/test/regress/expected/select_views_1.out @@ -1349,6 +1349,52 @@ EXPLAIN (COSTS OFF) SELECT * FROM my_property_secure WHERE f_leak(passwd); (4 rows) -- +-- scenario: qualifiers can be pushed down if they contain leaky functions, +-- provided they aren't passed data from inside the view. +-- +SELECT * FROM my_property_normal v + WHERE f_leak('passwd') AND f_leak(passwd); +NOTICE: f_leak => passwd +NOTICE: f_leak => passwd123 +NOTICE: f_leak => passwd +NOTICE: f_leak => beafsteak +NOTICE: f_leak => passwd +NOTICE: f_leak => hamburger + cid | name | tel | passwd +-----+---------------+------------------+----------- + 101 | regress_alice | +81-12-3456-7890 | passwd123 +(1 row) + +EXPLAIN (COSTS OFF) SELECT * FROM my_property_normal v + WHERE f_leak('passwd') AND f_leak(passwd); + QUERY PLAN +--------------------------------------------------------------------------------------------- + Seq Scan on customer + Filter: (f_leak('passwd'::text) AND f_leak(passwd) AND (name = ("current_user"())::text)) +(2 rows) + +SELECT * FROM my_property_secure v + WHERE f_leak('passwd') AND f_leak(passwd); +NOTICE: f_leak => passwd +NOTICE: f_leak => passwd123 +NOTICE: f_leak => passwd +NOTICE: f_leak => passwd + cid | name | tel | passwd +-----+---------------+------------------+----------- + 101 | regress_alice | +81-12-3456-7890 | passwd123 +(1 row) + +EXPLAIN (COSTS OFF) SELECT * FROM my_property_secure v + WHERE f_leak('passwd') AND f_leak(passwd); + QUERY PLAN +-------------------------------------------------------------------------------- + Subquery Scan on v + Filter: f_leak(v.passwd) + -> Seq Scan on customer + Filter: (f_leak('passwd'::text) AND (name = ("current_user"())::text)) +(4 rows) + +-- -- scenario: if a qualifier references only one-side of a particular join- -- tree, it shall be distributed to the most deep scan plan as -- possible as we can. diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql index f38b4438fdf..7d12dd00a2f 100644 --- a/src/test/regress/sql/rowsecurity.sql +++ b/src/test/regress/sql/rowsecurity.sql @@ -680,6 +680,26 @@ SELECT * FROM y2 WHERE f_leak(b); EXPLAIN (COSTS OFF) SELECT * FROM y2 WHERE f_leak(b); -- +-- Qual push-down of leaky functions, when not referring to table +-- +SELECT * FROM y2 WHERE f_leak('abc'); +EXPLAIN (COSTS OFF) SELECT * FROM y2 WHERE f_leak('abc'); + +CREATE TABLE test_qual_pushdown ( + abc text +); + +INSERT INTO test_qual_pushdown VALUES ('abc'),('def'); + +SELECT * FROM y2 JOIN test_qual_pushdown ON (b = abc) WHERE f_leak(abc); +EXPLAIN (COSTS OFF) SELECT * FROM y2 JOIN test_qual_pushdown ON (b = abc) WHERE f_leak(abc); + +SELECT * FROM y2 JOIN test_qual_pushdown ON (b = abc) WHERE f_leak(b); +EXPLAIN (COSTS OFF) SELECT * FROM y2 JOIN test_qual_pushdown ON (b = abc) WHERE f_leak(b); + +DROP TABLE test_qual_pushdown; + +-- -- Plancache invalidate on user change. -- RESET SESSION AUTHORIZATION; diff --git a/src/test/regress/sql/select_views.sql b/src/test/regress/sql/select_views.sql index da356237eba..3b74ab9d80f 100644 --- a/src/test/regress/sql/select_views.sql +++ b/src/test/regress/sql/select_views.sql @@ -96,6 +96,20 @@ SELECT * FROM my_property_secure WHERE f_leak(passwd); EXPLAIN (COSTS OFF) SELECT * FROM my_property_secure WHERE f_leak(passwd); -- +-- scenario: qualifiers can be pushed down if they contain leaky functions, +-- provided they aren't passed data from inside the view. +-- +SELECT * FROM my_property_normal v + WHERE f_leak('passwd') AND f_leak(passwd); +EXPLAIN (COSTS OFF) SELECT * FROM my_property_normal v + WHERE f_leak('passwd') AND f_leak(passwd); + +SELECT * FROM my_property_secure v + WHERE f_leak('passwd') AND f_leak(passwd); +EXPLAIN (COSTS OFF) SELECT * FROM my_property_secure v + WHERE f_leak('passwd') AND f_leak(passwd); + +-- -- scenario: if a qualifier references only one-side of a particular join- -- tree, it shall be distributed to the most deep scan plan as -- possible as we can. |