2011/7/7 Noah Misch <n...@2ndquadrant.com>:
> On Sun, Jul 03, 2011 at 11:41:47AM +0200, Kohei KaiGai wrote:
>> The simplified version of fix-leaky-view patch. The part of reloptions
>> for views got splitted out
>> into the part-0 patch, so it needs to be applied prior to this patch.
>> Rest of logic to prevent unexpected pushing down across security
>> barrier is not changed.
>>
>> Thanks,
>>
>> 2011/6/6 Kohei Kaigai <kohei.kai...@emea.nec.com>:
>> > This patch enables to fix up leaky-view problem using qualifiers that
>> > reference only one-side of join-loop inside of view definition.
>> >
>> > The point of this scenario is criteria to distribute qualifiers of
>> > scanning-plan distributed in distribute_qual_to_rels(). If and when a
>> > qualifiers that reference only one-side of join-loop, the optimizer may
>> > distribute this qualifier into inside of the join-loop, even if it goes
>> > over the boundary of a subquery expanded from a view for row-level
>> > security.
>> > This behavior allows us to reference whole of one-side of join-loop using
>> > functions with side-effects.
>> > The solution is quite simple; it prohibits to distribute qualifiers over
>> > the boundary of subquery, however, performance cost is unignorable,
>> > because it also disables to utilize obviously indexable qualifiers such as
>> > (id=123), so this patch requires users a hint whether a particular view is
>> > for row-level security, or not.
>> >
>> > This patch newly adds "CREATE SECURITY VIEW" statement that marks a flag
>> > to show this view was defined for row-level security purpose. This flag
>> > shall be stored as reloptions.
>> > If this flag was set, the optimizer does not distribute qualifiers over
>> > the boundary of subqueries expanded from security views, except for
>> > obviously safe qualifiers.
>> > (Right now, we consider built-in indexable operators are safe, but it
>> > might be arguable.)
>
> I took a moderately-detailed look at this patch. This jumped out:
>
>> --- a/src/backend/optimizer/util/clauses.c
>> +++ b/src/backend/optimizer/util/clauses.c
>
>> +static bool
>> +contain_leakable_functions_walker(Node *node, void *context)
>> +{
>> + if (node == NULL)
>> + return false;
>> +
>> + if (IsA(node, FuncExpr))
>> + {
>> + /*
>> + * Right now, we have no way to distinguish safe functions with
>> + * leakable ones, so, we treat all the function call possibly
>> + * leakable.
>> + */
>> + return true;
>> + }
>> + else if (IsA(node, OpExpr))
>> + {
>> + OpExpr *expr = (OpExpr *) node;
>> +
>> + /*
>> + * Right now, we assume operators implemented by built-in
>> functions
>> + * are not leakable, so it does not need to prevent
>> optimization.
>> + */
>> + set_opfuncid(expr);
>> + if (get_func_lang(expr->opfuncid) != INTERNALlanguageId)
>> + return true;
>> + /* else fall through to check args */
>> + }
>
> Any user can do this:
>
> CREATE OPERATOR !-! (PROCEDURE = int4in, RIGHTARG = cstring);
> SELECT !-! 'foo';
>
As I mentioned at the source code comments, this ad-hoc assumption was
come from we have no way to distinguish a non-leaky function from others.
So, I definitely love the approach (2), because only trusted function creator
can determine whether it is possible leaky or not.
> Making a distinction based simply on the call being an operator vs. a function
> is a dead end. I see these options:
>
> 1. The user defining a security view can be assumed to trust the operator
> class
> members of indexes defined on the tables he references. Keep track of which
> those are and treat only them as non-leakable. This covers many interesting
> cases, but it's probably tricky to implement and/or costly at runtime.
>
It requires DBA massive amount of detailed knowledge about functions underlying
operators used in a view. I don't think it is a realistic assumption.
> 2. Add a pg_proc flag indicating whether the function is known leak-free.
> Simple, but tedious and perhaps error-prone.
>
+1
> 3. Trust operators owned by PGUID. This is simple and probably covers the
> essential cases, but it's an ugly hack.
>
Some of built-in functions are also leaky. For example, int4div raise an error
when we try to divid a particular value by zero.
> 4. Trust nothing as leak-free. Simple; performance will be unattractive.
>
-1, Because of performance perspective.
Thanks,
--
KaiGai Kohei <kai...@kaigai.gr.jp>
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
