Re: [v9.2] Fix Leaky View Problem - Mailing list pgsql-hackers

2011/12/8 Robert Haas <robertmhaas@gmail.com>:
> On Sat, Dec 3, 2011 at 3:19 AM, Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
>> I rebased my patch set. New functions in pg_proc.h prevented to apply
>> previous revision cleanly. Here is no functional changes.
>
> I was thinking that my version of this (attached to an email from
> earlier today) might be about ready to commit.  But while I was
> trolling through the archives on this problem trying to figure out who
> to credit, I found an old complaint of Tom's that we never fixed, and
> which represents a security exposure for this patch:
>
> rhaas=# create table foo (a integer);
> CREATE TABLE
> rhaas=# insert into foo select generate_series(1,10);
> INSERT 0 10
> rhaas=# insert into foo values (1);
> INSERT 0 1
> rhaas=# analyze foo;
> ANALYZE
> rhaas=# create view safe_foo with (security_barrier) as select * from
> foo where a > 5;
> CREATE VIEW
> rhaas=# grant select on safe_foo to bob;
> GRANT
>
> Secure in the knowledge that Bob will only be able to see rows where a
> is 6 or higher, we go to bed.  But Bob finds a way to outsmart us:
>
> rhaas=> create or replace function leak(integer,integer) returns
> boolean as $$begin raise notice 'leak % %', $1, $2; return false;
> end$$ language plpgsql;
> CREATE FUNCTION
> rhaas=> create operator !! (procedure = leak, leftarg = integer,
> rightarg = integer, restrict = eqsel);
> CREATE OPERATOR
> rhaas=> explain select * from safe_foo where a !! 0;
> NOTICE:  leak 1 0
>                         QUERY PLAN
> -------------------------------------------------------------
>  Subquery Scan on safe_foo  (cost=0.00..2.70 rows=1 width=4)
>   Filter: (safe_foo.a !! 0)
>   ->  Seq Scan on foo  (cost=0.00..1.14 rows=6 width=4)
>         Filter: (a > 5)
> (4 rows)
>
> OOPS.  The *executor* has been persuaded not to apply the
> possibly-nefarious operator !! to the data until after applying the
> security-critical qual "a > 5".  But the *planner* has no such
> compunctions, and has cheerfully leaked the most common value in the
> table, which the user wasn't supposed to see.  I guess it's hopeless
> to suppose that we're going to completely conceal the list of MCVs
> from the user, since it might change the plan - and even if
> ProcessUtility_hook or somesuch is used to disable EXPLAIN, the user
> can still try to ferret out the MCVs via a timing attack.  That having
> been said, the above behavior doesn't sit well with me: letting the
> user probe for MCVs via a timing attack or a plan change is one thing;
> printing them out on request is a little bit too convenient for my
> taste.  :-(
>
Sorry, I missed this scenario, and have not investigated this code path
in detail yet.

My first impression remind me an idea that I proposed before, even
though it got negative response due to user visible changes.
It requires superuser privilege to create new operators, since we
assume superuser does not set up harmful configuration.

I still think it is an idea. Or, maybe, we can adopt a bit weaker restriction;
functions being used to operators must have "leakproof" property.

Is it worthful to have a discussion again?

Thanks,
--
KaiGai Kohei <kaigai@kaigai.gr.jp>