Re: WIP patch (v2) for updatable security barrier views - Mailing list pgsql-hackers

Dean,

Short version
-------------

Looks amazing overall. Very clever to zip up the s.b. quals, let the
rest of the rewriter and planer do their work normally, then unpack them
into subqueries inserted in the planner once inheritance appendrels are
expanded, etc.

My main concern is that the securityQuals appear to bypass all later
rewrite stages, inheritance expansion during planning, etc. I suspect
this might be hard to get around (because these are disembodied quals
which may have nonsense varnos), but I'm looking into it now.

There's also an assertion failure whenever a correlated subquery appears
as a security barrier view qual.  Again, looking at it.

Ideas on that issue?



Much longer version: My understanding of how it works
-----------------------------------------------------

My understanding from reading the patch is that this:

- Flattens target views in rewriteTargetView, as in current master. If
the target view is a security barrier view, the view quals are appended
to a list of security barrier quals on the new RTE, instead of appended
to the RTE's normal quals like for normal views.

After rewrite the views are fully flattened down to a RTE_RELATION,
which becomes the resultRelation. An unreferenced RTE for each view
that's been rewritten is preserved in the range-table for permissions
checking purposes only (same as current master).

- Inheritance expansion, tlist expansion, etc then occurrs as normal.

- In planning, in inheritance_planner, if any RTE has any stashed
security quals in its RangeTableEntry, expand_security_qual is invoked.
This iteratively wraps the base relation in a subquery with the saved
security barrier quals, creating nested subqueries around the original
RTE. At each pass resultRelation is changed to point to the new
outer-most subquery.


As a result of this approach everything looks normal to
preprocess_targetlist, row-marking, etc, because they're seeing a normal
RTE_RELATION as resultRelation. The security barrier quals are, at this
stage, stashed aside. If there's inheritance involved, RTEs copied
during appendrel expansion get copies of the security quals on in the
parent RTE.

Problem with inheritance, views, etc in s.b. quals
--------------------------------------------------

After inheritance expansion, tlist expansion, etc, the s.b. quals are
unpacked to create subqueries wrapping the original RTEs.


So, with:

CREATE TABLE t1 (x float, b integer, secret1 text, secret2 text);
CREATE TABLE t1child (z integer) INHERITS (t1);

INSERT INTO t1 (x, b, secret1, secret2)
VALUES
(0,0,'secret0', 'supersecret'),
(1,1,'secret1', 'supersecret'),
(2,2,'secret2', 'supersecret'),
(3,3,'secret3', 'supersecret'),
(4,4,'secret4', 'supersecret'),
(5,6,'secret5', 'supersecret');

INSERT INTO t1child (x, b, secret1, secret2, z)
VALUES
(8,8,'secret8', 'ss', 8),
(9,9,'secret8', 'ss', 9),
(10,10,'secret8', 'ss', 10);

CREATE VIEW v1
WITH (security_barrier)
AS
SELECT b AS b1, x AS x1, secret1
FROM t1 WHERE b % 2 = 0;

CREATE VIEW v2
WITH (security_barrier)
AS
SELECT b1 AS b2, x1 AS x2
FROM v1 WHERE b1 % 4 = 0;



then a statement like:

UPDATE v2
SET x2 = x2 + 32;

will be rewritten into something like (imaginary sql)

UPDATE t1 WITH SECURITY QUALS ((b % 2 == 0), (b % 4 == 0))
SET x = x + 32

inheritance-expanded and tlist-expanded into something like (imaginary SQL)


UPDATE(t1 WITH SECURITY QUALS ((b % 2 == 0), (b % 4 == 0)))UNION ALL(t1child WITH SECURITY QUALS ((b % 2 == 0), (b % 4
==0)))
 
SET x = x + 32;


after which security qual expansion occurs, giving us something like:


UPDATEt1, t1child <--- resultRelations(   SELECT v2.ctid, v2.*   FROM (     SELECT v1.ctid, v1.*     FROM (
SELECTt1.ctid, t1.*       FROM t1       WHERE b % 2 == 0     ) v1     WHERE b % 4 == 0   ) v2
 
   UNION ALL
   SELECT v2.ctid, v2.*   FROM (     SELECT v1.ctid, v1.*     FROM (       SELECT t1child.ctid, t1child.*       FROM
t1child      WHERE b % 2 == 0     ) v1     WHERE b % 4 == 0   ) v2
 
)
SET x = x + 32;


Giving a plan looking like:

EXPLAIN UPDATE v2 SET x2 = 32

                               QUERY PLAN
---------------------------------------------------------------------------Update on t1 t1_2  (cost=0.00..23.35 rows=2
width=76) ->  Subquery Scan on t1  (cost=0.00..2.18 rows=1 width=74)        ->  Subquery Scan on t1_3  (cost=0.00..2.17
rows=1width=74)              Filter: ((t1_3.b % 4) = 0)              ->  Seq Scan on t1 t1_4  (cost=0.00..2.16 rows=1
width=74)                   Filter: ((b % 2) = 0)  ->  Subquery Scan on t1_1  (cost=0.00..21.17 rows=1 width=78)
-> Subquery Scan on t1_5  (cost=0.00..21.16 rows=1 width=78)              Filter: ((t1_5.b % 4) = 0)              ->
SeqScan on t1child  (cost=0.00..21.10 rows=4 width=78)                    Filter: ((b % 2) = 0)
 
(11 rows)




So far this looks like a really clever approach. My only real concern is
that the security quals are currently hidden from rewrite and parsing
before during the period they're hidden away in the security quals RTI
field.

This means they aren't processed for things like inheritance expansion. e.g.

CREATE TABLE rowfilter (remainder integer, userid text);
CREATE TABLE rowfilterchild () INHERITS (rowfilter);
INSERT INTO rowfilterchild(remainder, userid) values (0, current_user);

a view with a security qual that refers to an inherited relation won't work:

CREATE VIEW sqv
WITH (security_barrier)
AS
SELECT x, b FROM t1 WHERE ( SELECT b % 4 = remainder FROM rowfilter WHERE userid = current_user OFFSET 0
);

This is a bit contrived to force the optimiser to treat the subquery as
correlated and thus make sure the ref to rowfilter gets into the
securityQuals list.

I expected zero results (a scan of rowfilter, but not rowfilterchild,
resulting in a null subquery return) but land up with an assertion
failure instead. The assertion triggers for any security qual containing
a correlated subquery, so it's crashing out before we can break due to
failure to expand inheritance.


This isn't just about inheritance. In general, we'd need a way to
process those securityQuals through any rewrite phases and through the
parts of planning before they get merged back in, so they're subject to
things like inheritance appendrel expansion.

Same if the security qual contains a view ref:

CREATE VIEW dumbview(zero)
AS SELECT 0;

CREATE VIEW sqv2
WITH (security_barrier)
AS
SELECT x, b FROM t1
WHERE (SELECT b % 2 = zero FROM dumbview OFFSET 0);


--
Craig Ringer
(Phew!)