Improving RLS qual pushdown
A while ago [1]/messages/by-id/CAEZATCWKcPfWiLoCnmfMSzKLgoaBz7AXKmGZ-Mk83Gd3JG8u1w@mail.gmail.com I proposed an enhancement to the way qual pushdown
safety is decided in RLS / security barrier views. Currently we just
test for the presence of leaky functions in the qual, but it is
possible to do better than that, by further testing if the leaky
function is actually being passed information that we don't want to be
leaked.
Attached is a patch that does that, allowing restriction clause quals
to be pushed down into subquery RTEs if they contain leaky functions,
provided that the arglists of those leaky functions contain no Vars
(such Vars would necessarily refer to output columns of the subquery,
which is the data that must not be leaked).
An example of the sort of query this will help optimise is:
SELECT * FROM table_with_rls WHERE created > now() - '1 hour'::interval;
where currently this qual cannot be pushed down because neither now()
nor timestamptz_mi_interval() are leakproof, but since they are not
being passed any data from the table, they can't actually leak
anything, so the qual can be safely pushed down, allowing indexes to
be used if available.
In fact the majority of builtin functions aren't marked leakproof, and
probably most user functions aren't either, so this could potentially
be useful in a wide range of real-world queries, where it is common to
write quals of the form <column> <operator> <expression>, and the
expression may contain leaky functions.
Regards,
Dean
[1]: /messages/by-id/CAEZATCWKcPfWiLoCnmfMSzKLgoaBz7AXKmGZ-Mk83Gd3JG8u1w@mail.gmail.com
Attachments:
rls-qual-pushdown.patchtext/x-diff; charset=US-ASCII; name=rls-qual-pushdown.patchDownload
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
new file mode 100644
index 58d78e6..4433468
*** a/src/backend/optimizer/path/allpaths.c
--- b/src/backend/optimizer/path/allpaths.c
*************** targetIsInAllPartitionLists(TargetEntry
*** 1982,1988 ****
* 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.
*
* 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).
--- 1982,1989 ----
* 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
! * that 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).
*************** qual_is_pushdown_safe(Query *subquery, I
*** 2009,2015 ****
/* Refuse leaky quals if told to (point 3) */
if (safetyInfo->unsafeLeaky &&
! contain_leaky_functions(qual))
return false;
/*
--- 2010,2016 ----
/* Refuse leaky quals if told to (point 3) */
if (safetyInfo->unsafeLeaky &&
! contain_leaked_vars(qual))
return false;
/*
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
new file mode 100644
index b340b01..7f82d54
*** a/src/backend/optimizer/util/clauses.c
--- b/src/backend/optimizer/util/clauses.c
*************** static bool contain_mutable_functions_wa
*** 97,103 ****
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 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);
--- 97,103 ----
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_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);
*************** contain_nonstrict_functions_walker(Node
*** 1318,1343 ****
}
/*****************************************************************************
! * Check clauses for non-leakproof functions
*****************************************************************************/
/*
! * contain_leaky_functions
! * Recursively search for leaky functions within a clause.
*
! * 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.
*/
bool
! contain_leaky_functions(Node *clause)
{
! return contain_leaky_functions_walker(clause, NULL);
}
static bool
! contain_leaky_functions_walker(Node *node, void *context)
{
if (node == NULL)
return false;
--- 1318,1347 ----
}
/*****************************************************************************
! * Check clauses for non-leakproof functions that might leak Vars
*****************************************************************************/
/*
! * 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 effects may be present in the
! * clause and that function's arguments contain any Var nodes of the current
! * query level. Qualifiers from outside a security_barrier view that leak
! * Var nodes in this way should not be pushed down into the view, lest the
! * contents of tuples intended to be filtered out be revealed via the side
! * effects.
*/
bool
! contain_leaked_vars(Node *clause)
{
! return contain_leaked_vars_walker(clause, NULL);
}
static bool
! contain_leaked_vars_walker(Node *node, void *context)
{
if (node == NULL)
return false;
*************** contain_leaky_functions_walker(Node *nod
*** 1369,1375 ****
{
FuncExpr *expr = (FuncExpr *) node;
! if (!get_func_leakproof(expr->funcid))
return true;
}
break;
--- 1373,1380 ----
{
FuncExpr *expr = (FuncExpr *) node;
! if (!get_func_leakproof(expr->funcid) &&
! contain_var_clause((Node *) expr->args))
return true;
}
break;
*************** contain_leaky_functions_walker(Node *nod
*** 1381,1387 ****
OpExpr *expr = (OpExpr *) node;
set_opfuncid(expr);
! if (!get_func_leakproof(expr->opfuncid))
return true;
}
break;
--- 1386,1393 ----
OpExpr *expr = (OpExpr *) node;
set_opfuncid(expr);
! if (!get_func_leakproof(expr->opfuncid) &&
! contain_var_clause((Node *) expr->args))
return true;
}
break;
*************** contain_leaky_functions_walker(Node *nod
*** 1391,1397 ****
ScalarArrayOpExpr *expr = (ScalarArrayOpExpr *) node;
set_sa_opfuncid(expr);
! if (!get_func_leakproof(expr->opfuncid))
return true;
}
break;
--- 1397,1404 ----
ScalarArrayOpExpr *expr = (ScalarArrayOpExpr *) node;
set_sa_opfuncid(expr);
! if (!get_func_leakproof(expr->opfuncid) &&
! contain_var_clause((Node *) expr->args))
return true;
}
break;
*************** contain_leaky_functions_walker(Node *nod
*** 1401,1415 ****
CoerceViaIO *expr = (CoerceViaIO *) node;
Oid funcid;
Oid ioparam;
bool varlena;
getTypeInputInfo(exprType((Node *) expr->arg),
&funcid, &ioparam);
! if (!get_func_leakproof(funcid))
! return true;
! getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
! if (!get_func_leakproof(funcid))
return true;
}
break;
--- 1408,1428 ----
CoerceViaIO *expr = (CoerceViaIO *) node;
Oid funcid;
Oid ioparam;
+ bool leaky;
bool varlena;
getTypeInputInfo(exprType((Node *) expr->arg),
&funcid, &ioparam);
! leaky = !get_func_leakproof(funcid);
! if (!leaky)
! {
! getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
! leaky = !get_func_leakproof(funcid);
! }
!
! if (leaky &&
! contain_var_clause((Node *) expr->arg))
return true;
}
break;
*************** contain_leaky_functions_walker(Node *nod
*** 1419,1432 ****
ArrayCoerceExpr *expr = (ArrayCoerceExpr *) node;
Oid funcid;
Oid ioparam;
bool varlena;
getTypeInputInfo(exprType((Node *) expr->arg),
&funcid, &ioparam);
! if (!get_func_leakproof(funcid))
! return true;
! getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
! if (!get_func_leakproof(funcid))
return true;
}
break;
--- 1432,1452 ----
ArrayCoerceExpr *expr = (ArrayCoerceExpr *) node;
Oid funcid;
Oid ioparam;
+ bool leaky;
bool varlena;
getTypeInputInfo(exprType((Node *) expr->arg),
&funcid, &ioparam);
! leaky = !get_func_leakproof(funcid);
!
! if (!leaky)
! {
! getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
! leaky = !get_func_leakproof(funcid);
! }
!
! if (leaky &&
! contain_var_clause((Node *) expr->arg))
return true;
}
break;
*************** contain_leaky_functions_walker(Node *nod
*** 1435,1446 ****
{
RowCompareExpr *rcexpr = (RowCompareExpr *) node;
ListCell *opid;
! foreach(opid, rcexpr->opnos)
{
Oid funcid = get_opcode(lfirst_oid(opid));
! if (!get_func_leakproof(funcid))
return true;
}
}
--- 1455,1472 ----
{
RowCompareExpr *rcexpr = (RowCompareExpr *) node;
ListCell *opid;
+ ListCell *larg;
+ ListCell *rarg;
! forthree(opid, rcexpr->opnos,
! larg, rcexpr->largs,
! rarg, rcexpr->rargs)
{
Oid funcid = get_opcode(lfirst_oid(opid));
! if (!get_func_leakproof(funcid) &&
! (contain_var_clause((Node *) lfirst(larg)) ||
! contain_var_clause((Node *) lfirst(rarg))))
return true;
}
}
*************** contain_leaky_functions_walker(Node *nod
*** 1455,1461 ****
*/
return true;
}
! return expression_tree_walker(node, contain_leaky_functions_walker,
context);
}
--- 1481,1487 ----
*/
return true;
}
! return expression_tree_walker(node, contain_leaked_vars_walker,
context);
}
diff --git a/src/include/optimizer/clauses.h b/src/include/optimizer/clauses.h
new file mode 100644
index 160045c..3d04ac2
*** a/src/include/optimizer/clauses.h
--- b/src/include/optimizer/clauses.h
*************** extern bool contain_mutable_functions(No
*** 63,69 ****
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 Relids find_nonnullable_rels(Node *clause);
extern List *find_nonnullable_vars(Node *clause);
--- 63,69 ----
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_leaked_vars(Node *clause);
extern Relids find_nonnullable_rels(Node *clause);
extern List *find_nonnullable_vars(Node *clause);
diff --git a/src/test/regress/expected/select_views.out b/src/test/regress/expected/select_views.out
new file mode 100644
index 82d510d..7f57526
*** a/src/test/regress/expected/select_views.out
--- b/src/test/regress/expected/select_views.out
*************** EXPLAIN (COSTS OFF) SELECT * FROM my_pro
*** 1349,1354 ****
--- 1349,1400 ----
(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
new file mode 100644
index ce22bfa..5275ef0
*** a/src/test/regress/expected/select_views_1.out
--- b/src/test/regress/expected/select_views_1.out
*************** EXPLAIN (COSTS OFF) SELECT * FROM my_pro
*** 1349,1354 ****
--- 1349,1400 ----
(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/select_views.sql b/src/test/regress/sql/select_views.sql
new file mode 100644
index da35623..3b74ab9
*** a/src/test/regress/sql/select_views.sql
--- b/src/test/regress/sql/select_views.sql
*************** SELECT * FROM my_property_secure WHERE f
*** 96,101 ****
--- 96,115 ----
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.
Dean,
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
A while ago [1] I proposed an enhancement to the way qual pushdown
safety is decided in RLS / security barrier views. Currently we just
test for the presence of leaky functions in the qual, but it is
possible to do better than that, by further testing if the leaky
function is actually being passed information that we don't want to be
leaked.
This certainly sounds reasonable to me.
In fact the majority of builtin functions aren't marked leakproof, and
probably most user functions aren't either, so this could potentially
be useful in a wide range of real-world queries, where it is common to
write quals of the form <column> <operator> <expression>, and the
expression may contain leaky functions.
Agreed.
Looks like you've already added it to the next commitfest, which is
great. I'm definitely interested but probably won't get to it right
away as I have a few other things to address.
Thanks!
Stephen
On Fri, Jan 9, 2015 at 7:54 AM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
A while ago [1] I proposed an enhancement to the way qual pushdown
safety is decided in RLS / security barrier views. Currently we just
test for the presence of leaky functions in the qual, but it is
possible to do better than that, by further testing if the leaky
function is actually being passed information that we don't want to be
leaked.Attached is a patch that does that, allowing restriction clause quals
to be pushed down into subquery RTEs if they contain leaky functions,
provided that the arglists of those leaky functions contain no Vars
(such Vars would necessarily refer to output columns of the subquery,
which is the data that must not be leaked).An example of the sort of query this will help optimise is:
SELECT * FROM table_with_rls WHERE created > now() - '1 hour'::interval;
where currently this qual cannot be pushed down because neither now()
nor timestamptz_mi_interval() are leakproof, but since they are not
being passed any data from the table, they can't actually leak
anything, so the qual can be safely pushed down, allowing indexes to
be used if available.
One thing they could still leak is the number of times they got
called, and thus possibly the number of unseen rows. Now if the
expressions get constant-folded away that won't be an issue, but a
clever user can probably avoid that.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 14 January 2015 at 13:29, Robert Haas <robertmhaas@gmail.com> wrote:
One thing they could still leak is the number of times they got
called, and thus possibly the number of unseen rows. Now if the
expressions get constant-folded away that won't be an issue, but a
clever user can probably avoid that.
Right now, EXPLAIN ANALYSE can be used to tell you the number of
unseen rows. Is that something that people are concerned about, and
are there any plans to change it?
Regards,
Dean
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Jan 14, 2015 at 9:22 AM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
On 14 January 2015 at 13:29, Robert Haas <robertmhaas@gmail.com> wrote:
One thing they could still leak is the number of times they got
called, and thus possibly the number of unseen rows. Now if the
expressions get constant-folded away that won't be an issue, but a
clever user can probably avoid that.Right now, EXPLAIN ANALYSE can be used to tell you the number of
unseen rows. Is that something that people are concerned about, and
are there any plans to change it?
Interesting question. I don't know.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas wrote:
On Wed, Jan 14, 2015 at 9:22 AM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
On 14 January 2015 at 13:29, Robert Haas <robertmhaas@gmail.com> wrote:
One thing they could still leak is the number of times they got
called, and thus possibly the number of unseen rows. Now if the
expressions get constant-folded away that won't be an issue, but a
clever user can probably avoid that.Right now, EXPLAIN ANALYSE can be used to tell you the number of
unseen rows. Is that something that people are concerned about, and
are there any plans to change it?Interesting question. I don't know.
Wasn't this part of the "covert channel" discussion that took place way
before RLS was committed? As I recall, it was argued that such covert
channels are acceptable as long as their bandwidth is low.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Alvaro Herrera (alvherre@2ndquadrant.com) wrote:
Robert Haas wrote:
On Wed, Jan 14, 2015 at 9:22 AM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
On 14 January 2015 at 13:29, Robert Haas <robertmhaas@gmail.com> wrote:
One thing they could still leak is the number of times they got
called, and thus possibly the number of unseen rows. Now if the
expressions get constant-folded away that won't be an issue, but a
clever user can probably avoid that.Right now, EXPLAIN ANALYSE can be used to tell you the number of
unseen rows. Is that something that people are concerned about, and
are there any plans to change it?Interesting question. I don't know.
Wasn't this part of the "covert channel" discussion that took place way
before RLS was committed? As I recall, it was argued that such covert
channels are acceptable as long as their bandwidth is low.
Yes, it was part of the discussion and no, there's no plans to try and
hide row counts in explain analyze, nor to deal with things like unique
constraint or foreign key reference violations.
There are other areas which need improvement which will help address
covert channel activity (better auditing, better control over what
actions are allowed to diffferent users when it comes to creating
objects, modifying permissions and policies, throttling, etc).
Thanks,
Stephen
Dean,
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
Attached is a patch that does that, allowing restriction clause quals
to be pushed down into subquery RTEs if they contain leaky functions,
provided that the arglists of those leaky functions contain no Vars
(such Vars would necessarily refer to output columns of the subquery,
which is the data that must not be leaked).
--- 1982,1989 ---- * 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 ! * that might reveal values from the subquery as side effects.
I'd probably extend this comment to note that the way we figure that out
is by looking for Vars being passed to non-leakproof functions.
--- 1318,1347 ---- }/*****************************************************************************
! * Check clauses for non-leakproof functions that might leak Vars
*****************************************************************************/
Might leak data in Vars (or somethhing along those lines...)
/*
! * 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 effects may be present in the
! * clause and that function's arguments contain any Var nodes of the current
! * query level. Qualifiers from outside a security_barrier view that leak
! * Var nodes in this way should not be pushed down into the view, lest the
! * contents of tuples intended to be filtered out be revealed via the side
! * effects.
*/
We don't actually know anything about the function and if it actually
has any side effects or not, so it might better to avoid discussing
that here. How about:
Returns true if any non-leakproof function is passed in data through a
Var node as that function might then leak see sensetive information.
Only leakproof functions are allowed to see data prior to the qualifiers
which are defined in the security_barrier view and therefore we can only
push down qualifiers if they are either leakproof or if they aren't
passed any Vars from this query level (and therefore they are not able
to see any of the data in the tuple, even if they are pushed down).
--- 1408,1428 ---- CoerceViaIO *expr = (CoerceViaIO *) node; Oid funcid; Oid ioparam; + bool leaky; bool varlena;getTypeInputInfo(exprType((Node *) expr->arg),
&funcid, &ioparam);
! leaky = !get_func_leakproof(funcid);! if (!leaky)
! {
! getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
! leaky = !get_func_leakproof(funcid);
! }
!
! if (leaky &&
! contain_var_clause((Node *) expr->arg))
return true;
}
break;
That seems slightly convoluted to me. Why not simply do:
bool in_leakproof, out_leakproof;
getTypeInputInfo(exprType((Node *) expr->arg), &funcid, &ioparam);
in_leakproof = get_func_leakproof(funcid);
getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
out_leakproof = get_func_leakproof(funcid);
if (!(in_leakproof && out_leakproof) && contain_var_clause((Node *)))
return true;
...
--- 1432,1452 ---- ArrayCoerceExpr *expr = (ArrayCoerceExpr *) node; Oid funcid; Oid ioparam; + bool leaky; bool varlena;getTypeInputInfo(exprType((Node *) expr->arg),
&funcid, &ioparam);
! leaky = !get_func_leakproof(funcid);
!
! if (!leaky)
! {
! getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
! leaky = !get_func_leakproof(funcid);
! }
!
! if (leaky &&
! contain_var_clause((Node *) expr->arg))
return true;
}
break;
Ditto here.
*************** contain_leaky_functions_walker(Node *nod
*** 1435,1446 ****
{
RowCompareExpr *rcexpr = (RowCompareExpr *) node;
ListCell *opid;! foreach(opid, rcexpr->opnos)
{
Oid funcid = get_opcode(lfirst_oid(opid));! if (!get_func_leakproof(funcid)) return true; } } --- 1455,1472 ---- { RowCompareExpr *rcexpr = (RowCompareExpr *) node; ListCell *opid; + ListCell *larg; + ListCell *rarg;! forthree(opid, rcexpr->opnos,
! larg, rcexpr->largs,
! rarg, rcexpr->rargs)
{
Oid funcid = get_opcode(lfirst_oid(opid));! if (!get_func_leakproof(funcid) &&
! (contain_var_clause((Node *) lfirst(larg)) ||
! contain_var_clause((Node *) lfirst(rarg))))
return true;
}
}
Might look a bit cleaner as:
/* Leakproof functions are ok */
if (get_func_leakproof(funcid))
continue;
/* Not leakproof, check if there are any Vars passed in */
if (contain_var_clause((Node *) lfirst(larg)) ||
contain_var_clause((Node *) lfirst(rarg)))
return true;
The rest looked good to me.
Thanks!
Stephen
Thanks for looking at this.
On 28 February 2015 at 04:25, Stephen Frost <sfrost@snowman.net> wrote:
Dean,
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
Attached is a patch that does that, allowing restriction clause quals
to be pushed down into subquery RTEs if they contain leaky functions,
provided that the arglists of those leaky functions contain no Vars
(such Vars would necessarily refer to output columns of the subquery,
which is the data that must not be leaked).--- 1982,1989 ---- * 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 ! * that might reveal values from the subquery as side effects.I'd probably extend this comment to note that the way we figure that out
is by looking for Vars being passed to non-leakproof functions.
OK fair enough.
--- 1318,1347 ---- }/*****************************************************************************
! * Check clauses for non-leakproof functions that might leak Vars
*****************************************************************************/Might leak data in Vars (or somethhing along those lines...)
Perhaps just "Check clauses for Vars passed to non-leakproof
functions". The more detailed explanation below is probably then
sufficient to cover why that's significant.
/*
! * 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 effects may be present in the
! * clause and that function's arguments contain any Var nodes of the current
! * query level. Qualifiers from outside a security_barrier view that leak
! * Var nodes in this way should not be pushed down into the view, lest the
! * contents of tuples intended to be filtered out be revealed via the side
! * effects.
*/We don't actually know anything about the function and if it actually
has any side effects or not, so it might better to avoid discussing
that here. How about:Returns true if any non-leakproof function is passed in data through a
Var node as that function might then leak see sensetive information.
Only leakproof functions are allowed to see data prior to the qualifiers
which are defined in the security_barrier view and therefore we can only
push down qualifiers if they are either leakproof or if they aren't
passed any Vars from this query level (and therefore they are not able
to see any of the data in the tuple, even if they are pushed down).
That sounds OK (except s/sensetive/sensitive).
--- 1408,1428 ---- CoerceViaIO *expr = (CoerceViaIO *) node; Oid funcid; Oid ioparam; + bool leaky; bool varlena;getTypeInputInfo(exprType((Node *) expr->arg),
&funcid, &ioparam);
! leaky = !get_func_leakproof(funcid);! if (!leaky)
! {
! getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
! leaky = !get_func_leakproof(funcid);
! }
!
! if (leaky &&
! contain_var_clause((Node *) expr->arg))
return true;
}
break;That seems slightly convoluted to me. Why not simply do:
bool in_leakproof, out_leakproof;
getTypeInputInfo(exprType((Node *) expr->arg), &funcid, &ioparam);
in_leakproof = get_func_leakproof(funcid);getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
out_leakproof = get_func_leakproof(funcid);if (!(in_leakproof && out_leakproof) && contain_var_clause((Node *)))
return true;
I'm not sure that's really much clearer, because of the 2
double-negatives in the final if-clause, and it's less efficient in
the case where the input function is leaky, when it's not nessecary to
check the output function (which involves 2 cache lookups).
...
--- 1432,1452 ---- ArrayCoerceExpr *expr = (ArrayCoerceExpr *) node; Oid funcid; Oid ioparam; + bool leaky; bool varlena;getTypeInputInfo(exprType((Node *) expr->arg),
&funcid, &ioparam);
! leaky = !get_func_leakproof(funcid);
!
! if (!leaky)
! {
! getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
! leaky = !get_func_leakproof(funcid);
! }
!
! if (leaky &&
! contain_var_clause((Node *) expr->arg))
return true;
}
break;Ditto here.
*************** contain_leaky_functions_walker(Node *nod
*** 1435,1446 ****
{
RowCompareExpr *rcexpr = (RowCompareExpr *) node;
ListCell *opid;! foreach(opid, rcexpr->opnos)
{
Oid funcid = get_opcode(lfirst_oid(opid));! if (!get_func_leakproof(funcid)) return true; } } --- 1455,1472 ---- { RowCompareExpr *rcexpr = (RowCompareExpr *) node; ListCell *opid; + ListCell *larg; + ListCell *rarg;! forthree(opid, rcexpr->opnos,
! larg, rcexpr->largs,
! rarg, rcexpr->rargs)
{
Oid funcid = get_opcode(lfirst_oid(opid));! if (!get_func_leakproof(funcid) &&
! (contain_var_clause((Node *) lfirst(larg)) ||
! contain_var_clause((Node *) lfirst(rarg))))
return true;
}
}Might look a bit cleaner as:
/* Leakproof functions are ok */
if (get_func_leakproof(funcid))
continue;/* Not leakproof, check if there are any Vars passed in */
if (contain_var_clause((Node *) lfirst(larg)) ||
contain_var_clause((Node *) lfirst(rarg)))
return true;
Shrug. Not sure it makes much difference either way.
The rest looked good to me.
Thanks. Do you want me to post an update, or are you going to hack on it?
Regards,
Dean
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Dean,
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
Thanks for looking at this.
Thanks for working on it. :)
On 28 February 2015 at 04:25, Stephen Frost <sfrost@snowman.net> wrote:
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
--- 1318,1347 ---- }/*****************************************************************************
! * Check clauses for non-leakproof functions that might leak Vars
*****************************************************************************/Might leak data in Vars (or somethhing along those lines...)
Perhaps just "Check clauses for Vars passed to non-leakproof
functions". The more detailed explanation below is probably then
sufficient to cover why that's significant.
Agreed, that looks fine to me.
That sounds OK (except s/sensetive/sensitive).
I'm horrible with spelling. :) Thanks for catching it.
I'm not sure that's really much clearer, because of the 2
double-negatives in the final if-clause, and it's less efficient in
the case where the input function is leaky, when it's not nessecary to
check the output function (which involves 2 cache lookups).
Yeah, it is a bit less performant.
Shrug. Not sure it makes much difference either way.
Alright.
The rest looked good to me.
Thanks. Do you want me to post an update, or are you going to hack on it?
Either works for me, though I'm happy to handle the modifications to
this if it means you have time to look at the other patches.. :)
Thanks!
Stephen
On 1 March 2015 at 18:23, Stephen Frost <sfrost@snowman.net> wrote:
Thanks. Do you want me to post an update, or are you going to hack on it?
Either works for me, though I'm happy to handle the modifications to
this if it means you have time to look at the other patches.. :)
OK, I'll continue testing the other patches. I want to think about RLS
and inheritance a bit more.
Regards,
Dean
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I took another look at this and came up with some alternate comment
rewording. I also added a couple of additional comments that should
hopefully clarify the code a bit.
Regards,
Dean
Attachments:
rls-qual-pushdown.patchtext/x-diff; charset=US-ASCII; name=rls-qual-pushdown.patchDownload
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
new file mode 100644
index 58d78e6..9e05fa8
*** a/src/backend/optimizer/path/allpaths.c
--- b/src/backend/optimizer/path/allpaths.c
*************** targetIsInAllPartitionLists(TargetEntry
*** 1982,1988 ****
* 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.
*
* 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).
--- 1982,1990 ----
* 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
! * 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).
*************** qual_is_pushdown_safe(Query *subquery, I
*** 2009,2015 ****
/* Refuse leaky quals if told to (point 3) */
if (safetyInfo->unsafeLeaky &&
! contain_leaky_functions(qual))
return false;
/*
--- 2011,2017 ----
/* Refuse leaky quals if told to (point 3) */
if (safetyInfo->unsafeLeaky &&
! contain_leaked_vars(qual))
return false;
/*
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
new file mode 100644
index 84d58ae..0098375
*** a/src/backend/optimizer/util/clauses.c
--- b/src/backend/optimizer/util/clauses.c
*************** static bool contain_mutable_functions_wa
*** 97,103 ****
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 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);
--- 97,103 ----
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_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);
*************** contain_nonstrict_functions_walker(Node
*** 1318,1343 ****
}
/*****************************************************************************
! * Check clauses for non-leakproof functions
*****************************************************************************/
/*
! * contain_leaky_functions
! * Recursively search for leaky functions within a clause.
*
! * 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.
*/
bool
! contain_leaky_functions(Node *clause)
{
! return contain_leaky_functions_walker(clause, NULL);
}
static bool
! contain_leaky_functions_walker(Node *node, void *context)
{
if (node == NULL)
return false;
--- 1318,1347 ----
}
/*****************************************************************************
! * Check clauses for Vars passed to non-leakproof functions
*****************************************************************************/
/*
! * 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 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_leaked_vars(Node *clause)
{
! return contain_leaked_vars_walker(clause, NULL);
}
static bool
! contain_leaked_vars_walker(Node *node, void *context)
{
if (node == NULL)
return false;
*************** contain_leaky_functions_walker(Node *nod
*** 1369,1375 ****
{
FuncExpr *expr = (FuncExpr *) node;
! if (!get_func_leakproof(expr->funcid))
return true;
}
break;
--- 1373,1380 ----
{
FuncExpr *expr = (FuncExpr *) node;
! if (!get_func_leakproof(expr->funcid) &&
! contain_var_clause((Node *) expr->args))
return true;
}
break;
*************** contain_leaky_functions_walker(Node *nod
*** 1381,1387 ****
OpExpr *expr = (OpExpr *) node;
set_opfuncid(expr);
! if (!get_func_leakproof(expr->opfuncid))
return true;
}
break;
--- 1386,1393 ----
OpExpr *expr = (OpExpr *) node;
set_opfuncid(expr);
! if (!get_func_leakproof(expr->opfuncid) &&
! contain_var_clause((Node *) expr->args))
return true;
}
break;
*************** contain_leaky_functions_walker(Node *nod
*** 1391,1397 ****
ScalarArrayOpExpr *expr = (ScalarArrayOpExpr *) node;
set_sa_opfuncid(expr);
! if (!get_func_leakproof(expr->opfuncid))
return true;
}
break;
--- 1397,1404 ----
ScalarArrayOpExpr *expr = (ScalarArrayOpExpr *) node;
set_sa_opfuncid(expr);
! if (!get_func_leakproof(expr->opfuncid) &&
! contain_var_clause((Node *) expr->args))
return true;
}
break;
*************** contain_leaky_functions_walker(Node *nod
*** 1401,1415 ****
CoerceViaIO *expr = (CoerceViaIO *) node;
Oid funcid;
Oid ioparam;
bool varlena;
getTypeInputInfo(exprType((Node *) expr->arg),
&funcid, &ioparam);
! if (!get_func_leakproof(funcid))
! return true;
! getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
! if (!get_func_leakproof(funcid))
return true;
}
break;
--- 1408,1432 ----
CoerceViaIO *expr = (CoerceViaIO *) node;
Oid funcid;
Oid ioparam;
+ bool leaky;
bool varlena;
+ /*
+ * Data may be leaked if either the input or the output
+ * function is leaky.
+ */
getTypeInputInfo(exprType((Node *) expr->arg),
&funcid, &ioparam);
! leaky = !get_func_leakproof(funcid);
! if (!leaky)
! {
! getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
! leaky = !get_func_leakproof(funcid);
! }
!
! if (leaky &&
! contain_var_clause((Node *) expr->arg))
return true;
}
break;
*************** contain_leaky_functions_walker(Node *nod
*** 1419,1432 ****
ArrayCoerceExpr *expr = (ArrayCoerceExpr *) node;
Oid funcid;
Oid ioparam;
bool varlena;
getTypeInputInfo(exprType((Node *) expr->arg),
&funcid, &ioparam);
! if (!get_func_leakproof(funcid))
! return true;
! getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
! if (!get_func_leakproof(funcid))
return true;
}
break;
--- 1436,1460 ----
ArrayCoerceExpr *expr = (ArrayCoerceExpr *) node;
Oid funcid;
Oid ioparam;
+ bool leaky;
bool varlena;
+ /*
+ * Data may be leaked if either the input or the output
+ * function is leaky.
+ */
getTypeInputInfo(exprType((Node *) expr->arg),
&funcid, &ioparam);
! leaky = !get_func_leakproof(funcid);
!
! if (!leaky)
! {
! getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
! leaky = !get_func_leakproof(funcid);
! }
!
! if (leaky &&
! contain_var_clause((Node *) expr->arg))
return true;
}
break;
*************** contain_leaky_functions_walker(Node *nod
*** 1435,1446 ****
{
RowCompareExpr *rcexpr = (RowCompareExpr *) node;
ListCell *opid;
! foreach(opid, rcexpr->opnos)
{
Oid funcid = get_opcode(lfirst_oid(opid));
! if (!get_func_leakproof(funcid))
return true;
}
}
--- 1463,1480 ----
{
RowCompareExpr *rcexpr = (RowCompareExpr *) node;
ListCell *opid;
+ ListCell *larg;
+ ListCell *rarg;
! forthree(opid, rcexpr->opnos,
! larg, rcexpr->largs,
! rarg, rcexpr->rargs)
{
Oid funcid = get_opcode(lfirst_oid(opid));
! if (!get_func_leakproof(funcid) &&
! (contain_var_clause((Node *) lfirst(larg)) ||
! contain_var_clause((Node *) lfirst(rarg))))
return true;
}
}
*************** contain_leaky_functions_walker(Node *nod
*** 1455,1461 ****
*/
return true;
}
! return expression_tree_walker(node, contain_leaky_functions_walker,
context);
}
--- 1489,1495 ----
*/
return true;
}
! return expression_tree_walker(node, contain_leaked_vars_walker,
context);
}
diff --git a/src/include/optimizer/clauses.h b/src/include/optimizer/clauses.h
new file mode 100644
index 160045c..3d04ac2
*** a/src/include/optimizer/clauses.h
--- b/src/include/optimizer/clauses.h
*************** extern bool contain_mutable_functions(No
*** 63,69 ****
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 Relids find_nonnullable_rels(Node *clause);
extern List *find_nonnullable_vars(Node *clause);
--- 63,69 ----
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_leaked_vars(Node *clause);
extern Relids find_nonnullable_rels(Node *clause);
extern List *find_nonnullable_vars(Node *clause);
diff --git a/src/test/regress/expected/select_views.out b/src/test/regress/expected/select_views.out
new file mode 100644
index 82d510d..7f57526
*** a/src/test/regress/expected/select_views.out
--- b/src/test/regress/expected/select_views.out
*************** EXPLAIN (COSTS OFF) SELECT * FROM my_pro
*** 1349,1354 ****
--- 1349,1400 ----
(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
new file mode 100644
index ce22bfa..5275ef0
*** a/src/test/regress/expected/select_views_1.out
--- b/src/test/regress/expected/select_views_1.out
*************** EXPLAIN (COSTS OFF) SELECT * FROM my_pro
*** 1349,1354 ****
--- 1349,1400 ----
(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/select_views.sql b/src/test/regress/sql/select_views.sql
new file mode 100644
index da35623..3b74ab9
*** a/src/test/regress/sql/select_views.sql
--- b/src/test/regress/sql/select_views.sql
*************** SELECT * FROM my_property_secure WHERE f
*** 96,101 ****
--- 96,115 ----
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.
Dean,
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
I took another look at this and came up with some alternate comment
rewording. I also added a couple of additional comments that should
hopefully clarify the code a bit.
Finally got back to this. Made a few additional changes that made
things look clearer, to me at least, and added documentation and
comments. Also added a bit to the regression tests (in particular, an
explicit check on the RLS side of this, not that it should be different,
but just in case things diverge in the future). Please review and let
me know if you see any issues.
Thanks!
Stephen
On 27 April 2015 at 17:33, Stephen Frost <sfrost@snowman.net> wrote:
Dean,
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
I took another look at this and came up with some alternate comment
rewording. I also added a couple of additional comments that should
hopefully clarify the code a bit.Finally got back to this. Made a few additional changes that made
things look clearer, to me at least, and added documentation and
comments. Also added a bit to the regression tests (in particular, an
explicit check on the RLS side of this, not that it should be different,
but just in case things diverge in the future). Please review and let
me know if you see any issues.
Thanks, that all looks very good.
Regards,
Dean
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers