[PATCH] Fix leaky VIEWs for RLS

The attached patch is a proof of concept.
It tries to fix the problem of leaky VIEWs for RLS.

* The scenario of leaky VIEWs for RLS
-------------------------------------
When a view contains any table joins and user gives a function that
takes arguments depending on only one-side table of the joins, the
planner tries to distribute the qualifier into least unit of scans.
It enables to minimize the scale of execution cost, but it means
user given function may be invoked earlier than other qualifiers
within a view, although row-level security is intended using views.
If the user given function has a side-effect (E.g, it may try to
insert given arguments into other tables), this behavior allows us
to leak contents of invisible tuples.

postgres=# SELECT * FROM v2 WHERE f_malicious(y);
NOTICE: f_malicious: xxx
NOTICE: f_malicious: yyy <-- leaky contents
NOTICE: f_malicious: zzz
a | b | x | y
---+-----+---+-----
1 | aaa | 1 | xxx
3 | ccc | 3 | zzz
(2 rows)

* Discussions in -hackers
-------------------------
Although it is a security problem, we will face unignorable performance
regression, if we disallow to distribute all user defined functions to
inside of joins come from views originally.

One idea is to distinguish a view into two types. The one is security
oriented view, and the other is regular view. Because only creator of
the view knows purpose of the view, an idea was suggested that takes
a hint on CREATE VIEW, like CREATE SECURITY VIEW.
(But I don't implement this feature in this patch yet.)

In addition, I was pointed out it may be harmless as long as
a person executes the query (not view's owner) has enough privileges
on underlaying tables, even if a view is marked as security view.

However, it was also pointed out the idea needs to check privileges
on planner stage, not execution stage. Is it really preferable?
It is not concluded yet. But inline_set_returning_function() checks
ACL_EXECUTE permission on planner stage, then it makes a FuncExpr
flatten prior to optimization. At least, it seems to me something
violation to the dogma.

Please point out any issues which I missed.

* About this patch
------------------
This patch mainly consists of two parts.

The first part is at pull_up_simple_subquery().
It checks privileges of underlaying tables in the subquery to be pulled
up, then it marks FromExpr->security_view as TRUE.

Note that it does not distinct a subquery come from a view and a pure
subquery in the original query. But it eventually causes access violation
error, if current user does not have privileges on a pure subquery.
So, no matter.

And, also note that it does not have statement support right now.
So, it assumes all the subqueries are possibly security views.

The second part is at distribute_qual_to_rels().
It computes what relations does the given clause depend on at first.
If the given clause depends on a part of relations to be joined
inside of the security view, it expands the dependency of the clause
into whole of the security view.
In the result, the clause is not distributed into inside of the join
loop.

(*) This patch uses check_relation_privileges() to check permissions
on DML execution, so please apply another my patch also on testing.

* Execution result
------------------

postgres=# CREATE TABLE t1 (a int primary key, b text);
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "t1_pkey" for table "t1"
CREATE TABLE
postgres=# CREATE TABLE t2 (x int primary key, y text);
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "t2_pkey" for table "t2"
CREATE TABLE
postgres=# CREATE TABLE t3 (s int primary key, t text);
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "t3_pkey" for table "t3"
CREATE TABLE
postgres=# CREATE VIEW v1 AS SELECT * FROM t1 JOIN t2 ON t1.a = t2.x;
CREATE VIEW
postgres=# CREATE VIEW v2 AS SELECT * FROM v1 JOIN t3 ON v1.a = t3.s;
CREATE VIEW
postgres=# CREATE USER tak;
CREATE ROLE
postgres=# GRANT SELECT ON v1, v2, t3 TO tak;
GRANT

The user: tak is allowed to select v1, v2 and t3.

postgres=# CREATE OR REPLACE FUNCTION f_malicious(text) RETURNS bool
AS 'BEGIN RAISE NOTICE ''f_malicious: %'', $1; RETURN true; END;'
LANGUAGE plpgsql;
CREATE FUNCTION

Because superuser can access t1 and t2 directly, f_malicious(y) shall be
distributed to Seq-scan on t2.

postgres=# EXPLAIN SELECT * FROM v1 WHERE f_malicious(y);
QUERY PLAN
-------------------------------------------------------------------
Hash Join (cost=334.93..365.94 rows=410 width=72)
Hash Cond: (t1.a = t2.x)
-> Seq Scan on t1 (cost=0.00..22.30 rows=1230 width=36)
-> Hash (cost=329.80..329.80 rows=410 width=36)
-> Seq Scan on t2 (cost=0.00..329.80 rows=410 width=36)
Filter: f_malicious(y)
(6 rows)

But tak cannot access t1 and t2 directly, f_malicious(y) shall be
distributed to outside of the join.

postgres=> EXPLAIN SELECT * FROM v1 WHERE f_malicious(y);
QUERY PLAN
-------------------------------------------------------------------
Hash Join (cost=37.67..384.39 rows=410 width=72)
Hash Cond: (t1.a = t2.x)
Join Filter: f_malicious(t2.y)
-> Seq Scan on t1 (cost=0.00..22.30 rows=1230 width=36)
-> Hash (cost=22.30..22.30 rows=1230 width=36)
-> Seq Scan on t2 (cost=0.00..22.30 rows=1230 width=36)
(6 rows)

In this path, it assumes operators are not malicious (is it a correct
assumption?), this restriction does not prevent indexing of scans.

postgres=> EXPLAIN SELECT * FROM v1 WHERE f_malicious(y) AND x = 100;
QUERY PLAN
-------------------------------------------------------------------------
Nested Loop (cost=0.00..16.80 rows=1 width=72)
Join Filter: f_malicious(t2.y)
-> Index Scan using t1_pkey on t1 (cost=0.00..8.27 rows=1 width=36)
Index Cond: (a = 100)
-> Index Scan using t2_pkey on t2 (cost=0.00..8.27 rows=1 width=36)
Index Cond: (t2.x = 100)
(6 rows)

* To do items

- Need to discuss whether we are on the right direction, or not.
- Statement support to provide a hint.
Such as CREATE SECURITY VIEW AS ... ?
- Need to tackle the matter of evaluation order of scan filter.
This patch does not care anything.

Thanks,
--
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>