Re: [HACKERS] [v9.2] Fix leaky-view problem, part 2
On Wed, Jul 20, 2011 at 04:23:10PM +0200, Yeb Havinga wrote: > On 2011-07-20 16:15, Yeb Havinga wrote: >> On 2011-07-20 16:06, Noah Misch wrote: >>> >>> The SQL-level semantics of the view define the access rules in >>> question. How >>> would you translate that into tests to apply at a lower level? >> I assumed the leaky view thread was about row level security, not >> about access rules to views, since it was mentioned at the RLS wiki >> page for se-pgsql. Sorry for the confusion. > Had to digg a bit for the wiki, it was this one : > http://wiki.postgresql.org/wiki/RLS#Issue:_A_leaky_VIEWs_for_RLS It is about row-level security, broadly. These patches close the hazard described in the latter half of this page: http://www.postgresql.org/docs/9.0/static/rules-privileges.html In the example given there, "phone NOT LIKE '412%'" is the (row-level) access rule that needs to apply before any possibly-leaky function sees the tuple. -- Noah Mischhttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] [v9.2] Fix leaky-view problem, part 2
On 2011-07-20 16:15, Yeb Havinga wrote: On 2011-07-20 16:06, Noah Misch wrote: The SQL-level semantics of the view define the access rules in question. How would you translate that into tests to apply at a lower level? I assumed the leaky view thread was about row level security, not about access rules to views, since it was mentioned at the RLS wiki page for se-pgsql. Sorry for the confusion. Had to digg a bit for the wiki, it was this one : http://wiki.postgresql.org/wiki/RLS#Issue:_A_leaky_VIEWs_for_RLS regards, Yeb -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.2] Fix leaky-view problem, part 2
On 2011-07-20 16:06, Noah Misch wrote: The SQL-level semantics of the view define the access rules in question. How would you translate that into tests to apply at a lower level? I assumed the leaky view thread was about row level security, not about access rules to views, since it was mentioned at the RLS wiki page for se-pgsql. Sorry for the confusion. regards, Yeb -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.2] Fix leaky-view problem, part 2
On Wed, Jul 20, 2011 at 09:02:59AM +0200, Yeb Havinga wrote: > On 2011-07-09 09:14, Kohei KaiGai wrote: >> OK, I'll try to modify the patch according to the flag of pg_proc design. >> As long as the default of user-defined function is off, and we provide >> built-in functions >> with appropriate configurations, it seems to me the burden of DBA is >> quite limited. > > A different solution to the leaky view problem could be to check access > to a tuple at or near the heaptuple visibility level, in addition to > adding tuple access filter conditions to the query. This would have both > the possible performance benefits of the query rewriting solution, as > the everything is filtered before further processing at the heaptuple > visibility level. Fixing leaky views is not needed because they don't > exist in this case, the code is straightforward, and there's less change > of future security bugs by either misconfiguration of leakproof > functions or code that might introduce another leak path. The SQL-level semantics of the view define the access rules in question. How would you translate that into tests to apply at a lower level? -- Noah Mischhttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] [v9.2] Fix leaky-view problem, part 2
2011/7/20 Yeb Havinga : > On 2011-07-09 09:14, Kohei KaiGai wrote: >> >> OK, I'll try to modify the patch according to the flag of pg_proc design. >> As long as the default of user-defined function is off, and we provide >> built-in functions >> with appropriate configurations, it seems to me the burden of DBA is >> quite limited. > > A different solution to the leaky view problem could be to check access to a > tuple at or near the heaptuple visibility level, in addition to adding tuple > access filter conditions to the query. This would have both the possible > performance benefits of the query rewriting solution, as the everything is > filtered before further processing at the heaptuple visibility level. Fixing > leaky views is not needed because they don't exist in this case, the code is > straightforward, and there's less change of future security bugs by either > misconfiguration of leakproof functions or code that might introduce another > leak path. > I'm not fun with this approach. The harderst one to find out a solution is a way to distinguish qualifiers of security policy and others. Leaky functions looks like a harmless function, them the optimizer will distribute them onto particular scan plans. If it was executed on the visibility check of tuples, same problem will be reproduced. So, I'm still fun with a flag of pg_proc catalog and idea of security barrier. Thanks, -- KaiGai Kohei -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.2] Fix leaky-view problem, part 2
On 2011-07-09 09:14, Kohei KaiGai wrote: OK, I'll try to modify the patch according to the flag of pg_proc design. As long as the default of user-defined function is off, and we provide built-in functions with appropriate configurations, it seems to me the burden of DBA is quite limited. A different solution to the leaky view problem could be to check access to a tuple at or near the heaptuple visibility level, in addition to adding tuple access filter conditions to the query. This would have both the possible performance benefits of the query rewriting solution, as the everything is filtered before further processing at the heaptuple visibility level. Fixing leaky views is not needed because they don't exist in this case, the code is straightforward, and there's less change of future security bugs by either misconfiguration of leakproof functions or code that might introduce another leak path. regards, -- Yeb Havinga http://www.mgrid.net/ Mastering Medical Data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.2] Fix leaky-view problem, part 2
2011/7/9 Robert Haas : > On Fri, Jul 8, 2011 at 4:57 PM, Noah Misch wrote: >> Note that it does not matter whether we're actually doing an index scan -- a >> seq >> scan with a filter using only leakproof operators is equally acceptable. >> What I >> had in mind was to enumerate all operators in operator classes of indexes >> below >> each security view. Those become the leak-free operators for that security >> view. If the operator for an OpExpr is considered leak-free by all sources >> of >> its operands, then we may push it down. That's purely a high-level sketch: I >> haven't considered implementation concerns in any detail. The resulting >> behavior could be surprising: adding an index may change a plan without the >> new >> plan actually using the index. >> >> I lean toward favoring the pg_proc flag. Functions like "texteq" will be >> taken >> as leakproof even if no involved table has an index on a text column. It >> works >> for functions that will never take a place in an operator class, like >> length(text). When a user reports a qualifier not getting pushed down, the >> answer is much more satisfying: "Run 'CREATE OR REPLACE FUNCTION >> ... I_DONT_LEAK' as a superuser." Compare to "Define an operator class that >> includes the function, if needed, and create an otherwise-useless index." >> The >> main disadvantage I see is the loss of policy locality. Only a superuser (or >> maybe database owner?) can create or modify declared-leakproof functions, and >> that decision applies throughout the database. However, I think the other >> advantages clearly outweigh that loss. > > This strikes me as a fairly compelling refutation of Heikki's proposed > approach. > OK, I'll try to modify the patch according to the flag of pg_proc design. As long as the default of user-defined function is off, and we provide built-in functions with appropriate configurations, it seems to me the burden of DBA is quite limited. Thanks, -- KaiGai Kohei -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.2] Fix leaky-view problem, part 2
On Fri, Jul 8, 2011 at 4:57 PM, Noah Misch wrote: > Note that it does not matter whether we're actually doing an index scan -- a > seq > scan with a filter using only leakproof operators is equally acceptable. > What I > had in mind was to enumerate all operators in operator classes of indexes > below > each security view. Those become the leak-free operators for that security > view. If the operator for an OpExpr is considered leak-free by all sources of > its operands, then we may push it down. That's purely a high-level sketch: I > haven't considered implementation concerns in any detail. The resulting > behavior could be surprising: adding an index may change a plan without the > new > plan actually using the index. > > I lean toward favoring the pg_proc flag. Functions like "texteq" will be > taken > as leakproof even if no involved table has an index on a text column. It > works > for functions that will never take a place in an operator class, like > length(text). When a user reports a qualifier not getting pushed down, the > answer is much more satisfying: "Run 'CREATE OR REPLACE FUNCTION > ... I_DONT_LEAK' as a superuser." Compare to "Define an operator class that > includes the function, if needed, and create an otherwise-useless index." The > main disadvantage I see is the loss of policy locality. Only a superuser (or > maybe database owner?) can create or modify declared-leakproof functions, and > that decision applies throughout the database. However, I think the other > advantages clearly outweigh that loss. This strikes me as a fairly compelling refutation of Heikki's proposed approach. -- 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
Re: [HACKERS] [v9.2] Fix leaky-view problem, part 2
On Fri, Jul 08, 2011 at 10:09:54AM +0100, Kohei KaiGai wrote: > 2011/7/8 Heikki Linnakangas : > > On 08.07.2011 11:03, Kohei KaiGai wrote: > >> > >> 2011/7/7 Noah Misch: > >>> > >>> 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 > > > > IMHO the situation from DBA's point of view is exactly opposite. Option two > > requires deep knowledge of this leaky views issue. The DBA needs to inspect > > any function he wants to mark as leak-free closely, and understand that > > innocent-looking things like casts can cause leaks. That is not feasible in > > practice. Option 1, however, requires no such knowledge. Operators used in > > indexes are already expected to not throw errors, or you would get errors > > when inserting certain values to the table, for example. > > > I might misread his description at first. > Hmm. If we introduce DBA the scenario and the condition to push down > qualifiers, > it may be possible to explain more simply. > > A challenge of this approach is to determine what qualifier shall be > used to index > accesses in the stage of distribute_qual_to_rels(); prior to the > optimizer's selection > of access methods. > Do you have any good idea, or suggestion? Note that it does not matter whether we're actually doing an index scan -- a seq scan with a filter using only leakproof operators is equally acceptable. What I had in mind was to enumerate all operators in operator classes of indexes below each security view. Those become the leak-free operators for that security view. If the operator for an OpExpr is considered leak-free by all sources of its operands, then we may push it down. That's purely a high-level sketch: I haven't considered implementation concerns in any detail. The resulting behavior could be surprising: adding an index may change a plan without the new plan actually using the index. I lean toward favoring the pg_proc flag. Functions like "texteq" will be taken as leakproof even if no involved table has an index on a text column. It works for functions that will never take a place in an operator class, like length(text). When a user reports a qualifier not getting pushed down, the answer is much more satisfying: "Run 'CREATE OR REPLACE FUNCTION ... I_DONT_LEAK' as a superuser." Compare to "Define an operator class that includes the function, if needed, and create an otherwise-useless index." The main disadvantage I see is the loss of policy locality. Only a superuser (or maybe database owner?) can create or modify declared-leakproof functions, and that decision applies throughout the database. However, I think the other advantages clearly outweigh that loss. Incidentally, whichever policy we choose here can also loosen the constraints on qualifier order (part 1 of your original submission). -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.2] Fix leaky-view problem, part 2
On Fri, Jul 8, 2011 at 4:18 AM, Heikki Linnakangas wrote: > IMHO the situation from DBA's point of view is exactly opposite. Option two > requires deep knowledge of this leaky views issue. The DBA needs to inspect > any function he wants to mark as leak-free closely, and understand that > innocent-looking things like casts can cause leaks. That is not feasible in > practice. Option 1, however, requires no such knowledge. Operators used in > indexes are already expected to not throw errors, or you would get errors > when inserting certain values to the table, for example. But, IMHO, the chance of the DBA wanting to set this flag is miniscule. I think that 99.9% of DBAs will be perfectly happy to just use whatever set we mark as built-ins. And an explicit pg_proc flag gives us a lot more flexibility. -- 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
Re: [HACKERS] [v9.2] Fix leaky-view problem, part 2
2011/7/8 Heikki Linnakangas : > On 08.07.2011 11:03, Kohei KaiGai wrote: >> >> 2011/7/7 Noah Misch: >>> >>> 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 > > IMHO the situation from DBA's point of view is exactly opposite. Option two > requires deep knowledge of this leaky views issue. The DBA needs to inspect > any function he wants to mark as leak-free closely, and understand that > innocent-looking things like casts can cause leaks. That is not feasible in > practice. Option 1, however, requires no such knowledge. Operators used in > indexes are already expected to not throw errors, or you would get errors > when inserting certain values to the table, for example. > I might misread his description at first. Hmm. If we introduce DBA the scenario and the condition to push down qualifiers, it may be possible to explain more simply. A challenge of this approach is to determine what qualifier shall be used to index accesses in the stage of distribute_qual_to_rels(); prior to the optimizer's selection of access methods. Do you have any good idea, or suggestion? Thanks, -- KaiGai Kohei -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.2] Fix leaky-view problem, part 2
On 08.07.2011 11:03, Kohei KaiGai wrote: 2011/7/7 Noah Misch: 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 IMHO the situation from DBA's point of view is exactly opposite. Option two requires deep knowledge of this leaky views issue. The DBA needs to inspect any function he wants to mark as leak-free closely, and understand that innocent-looking things like casts can cause leaks. That is not feasible in practice. Option 1, however, requires no such knowledge. Operators used in indexes are already expected to not throw errors, or you would get errors when inserting certain values to the table, for example. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.2] Fix leaky-view problem, part 2
2011/7/7 Noah Misch :
> 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 :
>> > 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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.2] Fix leaky-view problem, part 2
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 :
> > 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';
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.
2. Add a pg_proc flag indicating whether the function is known leak-free.
Simple, but tedious and perhaps error-prone.
3. Trust operators owned by PGUID. This is simple and probably covers the
essential cases, but it's an ugly hack.
4. Trust nothing as leak-free. Simple; performance will be unattractive.
There are probably others.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.2] Fix leaky-view problem, part 2
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 : > 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.) > > It fixes up the scenario [2] in the bellow descriprions. > > > The background of the leaky-view problem is well summarized at: > http://wiki.postgresql.org/wiki/RLS > > We had discussed several scenarios in v9.1 development cycle, and the last > developer meeting. We almost concluded the following criteria to characterize > whether a leak-view scenario is problematic to be fixed, or not. > * If unprived user can directly reference contents of invisible tuples, it > is a problem to be fixed. > * As long as contents of invisible tuples are consumed by internal stuff > (eg, index-access method), it is not a problem to be fixed. > > Thus, the scenario [1] and [2] are problematic to be fixed, but [3] and [4] > are not. So, I'll try to fix up these two scenario with the patch part-1 amd > part-2. > > [1] unexpected reorder of functions with tiny-cost and side-effects > > Qualifiers of WHERE or JOIN ... IN clause shall be sorted by estimated cost, > not depth of nest level. Thus, this logic can make order reversal when > user-given qualifier has smaller cost than qualifiers to perform as security > policy inside of view. > In the result, these qualifiers can reference both of visible and invisible > tuples prior to the filtering by row-level security policy of the view. Thus, > this behavior can be used to leak contents of invisible tuples. > > > [2] unexpected push-down of functions with side-effect into join-loop > > If arguments of qualifier being appended on outside of join-loop references > only one-side of the join-loop, it is a good strategy to distribute this > qualifier into inside of the join-loop to minimize number of tuples to be > joined, from the viewpoint of performance. > However, it also makes order reversal when the join-loop is a part of view > definition that should perform row-level security policy. Then, these > exogenetic qualifiers may be executed prior to the filtering by row-level > security policy of the view. Thus, this behavior can be used to leak contents > of invisible tuple. > > > [3] estimation of hidden value using iteration of PK/FK proves > > Due to the nature of PK/FK constraints, we can infer existence of key values > being stored within invisible tuple, even if we never allows users to > reference contents of invisible tuples. > We commonly call this type of information leaks "covert-channel", and it is > basically impossible to prevent according to the previous security research, > however, its risk is also relatively small because of slow bandwidth to leak. > We already made consensus this scenario is not a problem to be fixed. > > > [4] estimation of hidden value using statistics > > One example was selectivity-estimator function; that may reference > statistical information delivered from the tables have invisible tuples for > optimization. Here are two points to be considered. The one is purely > internal stuff may be able to reference invisible tuples, however, it is not > a problem as long as it does not leak them into end-users; such as index > access methods. The second is statistical or other form of date delivered > from invisible tuples. We can set up a table that contains data delivered > from invisible tuples using row-level triggers, however, i
