Re: leaky views, yet again

On 19/07/10 20:08, Robert Haas wrote:
> 2010/7/8 KaiGai Kohei<kaigai(at)ak(dot)jp(dot)nec(dot)com>:
>> (2010/07/08 22:08), Robert Haas wrote:
>>> I think we pretty much have conceptual agreement on the shape of the
>>> solution to this problem: when a view is created with CREATE SECURITY
>>> VIEW, restrict functions and operators that might disclose tuple data
>>> from being pushed down into view (unless, perhaps, the user invoking
>>> the view has sufficient privileges to execute the underlying query
>>> anyhow, e.g. superuser or view owner). What we have not resolved is
>>> exactly how we're going to decide which functions and operators might
>>> do such a dastardly thing. I think the viable options are as follows:
>>>
>>> 1. Adopt Heikki's proposal of treating indexable operators as non-leaky.
>>>
>>> http://archives.postgresql.org/pgsql-hackers/2010-06/msg00291.php
>>>
>>> Or, perhaps, and even more restrictively, treat only
>>> hashable/mergeable operators as non-leaky.
>>>
>>> 2. Add an explicit flag to pg_proc, which can only be set by
>>> superusers (and which is cleared if a non-superuser modifies it in any
>>> way), allowing a function to be tagged as non-leaky. I believe that
>>> it would be reasonable to set this flag for all of our built-in
>>> indexable operators (though I'd read the code before doing it), but it
>>> would remove the need for the assumption that third-party operators
>>> are equally sane.
>>>
>>> CREATE OR REPLACE FUNCTION doesnt_leak() RETURNS integer AS $$SELECT
>>> 42$$ IMMUTABLE SEAWORTHY; -- doesn't leak
>>>
>>> This problem is not going away, so we should try to decide on something.
>>>
>> I'd like to vote the second option, because this approach will be also
>> applied on another aspect of leaky views.
>>
>> When leaky and non-leaky functions are chained within a WHERE clause,
>> it will be ordered by the cost of functions. So, we have possibility
>> that leaky functions are executed earlier than non-leaky functions.
>>
>> It will not be an easy work to mark built-in functions as either leaky
>> or non-leaky, but it seems to me the most straight forward solution.
>
> Does anyone else have an opinion on this?

I have a bad feeling that marking functions explicitly as seaworthy is
going to be hard to get right, and every time you get that wrong it's a
security issue. On the other hand, if it's enough from a performance
point of view to review and mark only a few built-in functions like
index operators, maybe it's ok.

It would be easier to assess this if we had a patch to play with that
contained all the planner changes to keep track of the seaworthiness of
functions and apply the quals in right order. You could then try out
different scenarios to see what the performance is like.

I guess what I'm saying is "write a patch, and I'll shoot it down when
you're done" :-/. But hopefully the planner changes don't depend much on
how we deduce which quals are leaky.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com