Re: [v9.2] Fix leaky-view problem, part 1 - Mailing list pgsql-hackers
From | Noah Misch |
---|---|
Subject | Re: [v9.2] Fix leaky-view problem, part 1 |
Date | |
Msg-id | 20110630102916.GA14019@tornado.leadboat.com Whole thread Raw |
In response to | Re: [v9.2] Fix leaky-view problem, part 1 (Kohei KaiGai <kaigai@kaigai.gr.jp>) |
Responses |
Re: [v9.2] Fix leaky-view problem, part 1
|
List | pgsql-hackers |
On Wed, Jun 29, 2011 at 05:05:22PM +0100, Kohei KaiGai wrote: > 2011/6/28 Noah Misch <noah@leadboat.com>: > > On Tue, Jun 28, 2011 at 10:11:59PM +0200, Kohei KaiGai wrote: > > CREATE VIEW a AS SELECT * FROM ta WHERE ac = 5; > > ALTER VIEW a OWNER TO alice; > > CREATE VIEW b AS SELECT * FROM tb WHERE bc = 6; > > ALTER VIEW b OWNER TO bob; > > SELECT * FROM a, b; > > > > Both the ac=5 and the bc=6 quals do run at the same depth despite enforcing > > security for different principals. ?I can't think of a way that one view owner > > could use this situation to subvert the security of the other view owner, but I > > wanted to throw it out. > > > Even if view owner set a trap in his view, we have no way to reference variables > come from outside of the view. In above example, even if I added f_leak() into > the definition of VIEW A, we cannot give argument to reference VIEW B. Good point. Yes, it should be rigorously safe on that account. > > I was referring to this paragraph: > > > > ?On the technical side, I am pretty doubtful that the approach of adding a > > ?nestlevel to FuncExpr and RelOptInfo is the right way to go. ?I believe we > > ?have existing code (to handle left joins) that prevents quals from being > > ?pushed down too far by fudging the set of relations that are supposedly needed > > ?to evaluate the qual. ?I suspect a similar approach would work here. > > > It seems to me the later half of this paragraph is talking about the problem of > unexpected qualifier pushing-down over the security barrier; I'm trying to solve > the problem with the part.2 patch. > The scenario the part.1 patch tries to solve is order to launch qualifiers, not > unexpected pushing-down. Okay, you're probably correct that it wasn't referring to the topic at hand. I'm still suspicious of the silent assumption about how quals can be assigned to plan nodes, but I don't have any concrete ideas for avoiding that. > >> In addition, implementation will become complex, if both of qualifiers pulled-up > >> from security barrier view and qualifiers pulled-up from regular views are mixed > >> within a single qualifier list. > > > > I only scanned the part 2 patch, but isn't the bookkeeping already happening for > > its purposes? ?How much more complexity would we get to apply the same strategy > > to the behavior of this patch? > > > If conditional, what criteria we should have to reorder the quelifier? > The current patch checks the depth at first, then it checks cost if same deptn. > It is quite simple rule. I have no idea of the criteria to order the > mixed qualifier > come from security-barrier views and regular views. Let's see. Every qual list will have some depth d such that all quals having depth >= d are security-relevant, and all others are not security-relevant. (This does not hold for all means of identifying security-relevant quals, but it does hold for the CREATE SECURITY VIEW/reloptions strategy proposed in your part 2 patch.) Suppose you track whether each Query node represents a security view, then only increment the qualifier depth for such Query nodes, rather than all Query nodes. The tracked depth then becomes a security partition depth. Keep the actual sorting algorithm the same. (Disclaimer: I haven't been thinking about this nearly as long as you have, so I may be missing something relatively obvious.) As it stands, the patch badly damages the performance of this example: CREATE FUNCTION expensive(int) RETURNS boolean LANGUAGE sqlAS 'SELECT pg_sleep(1); SELECT true' COST 1000000; CREATE TABLE t(c) AS SELECT * FROM generate_series(1,3); EXPLAIN ANALYZESELECT * FROM (SELECT * FROM t WHERE expensive(c)) t0 WHERE c = 2; That doesn't even use a view, let alone a security view. While I like the patch's current simplicity, we need to narrow its impact. Thanks, nm
pgsql-hackers by date: