Message-ID: <52E74A01.7070307@2ndquadrant.com>
Date: Tue, 28 Jan 2014 14:11:13 +0800
From: Craig Ringer <craig@2ndquadrant.com>
Organization: 2nd Quadrant
User-Agent: Mozilla/5.0 (X11; Linux x86_64;
 rv:24.0) Gecko/20100101 Thunderbird/24.0
MIME-Version: 1.0
To: Dean Rasheed <dean.a.rasheed@gmail.com>
CC: Gregory Smith <gregsmithpgsql@gmail.com>, Greg Stark <stark@mit.edu>,
 Kohei KaiGai <kaigai@kaigai.gr.jp>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Infinite recursion in row-security based on updatable s.b. views
References: 
 <CADyhKSWkHrPqbRsFK0LjPMOOTsbKGWOuN75GqYJrxCDO3CKWtA@mail.gmail.com>
	<1398.1378305956@sss.pgh.pa.us>
	<CA+TgmoYTf+4RUOW8stVz1RE+3TD+QR5TNKzYuFDCehSkieL5OQ@mail.gmail.com>
	<1543.1378306249@sss.pgh.pa.us>	<2620.1378308159@sss.pgh.pa.us>
	<52775604.9090306@2ndquadrant.com>	<11101.1383575839@sss.pgh.pa.us>
	<CA+TgmoZS46fGTHCMo1FpNiBYDU7Wrx7R8PL+48Xc7UcVCsuaUA@mail.gmail.com>
	<52784DF9.9040205@2ndquadrant.com>
	<CA+TgmobvM9t8y6+BoG0DXMVA5jcNc9V6fzNt2tNSaVqsRmGpHA@mail.gmail.com>
	<5279E3CA.7050400@2ndquadrant.com>
	<CAM-w4HOJaWt2naKakW67KRZE9mBcEF-BQ3UA=7SHKcFv3=FmVQ@mail.gmail.com>
	<527C478D.4070901@2ndquadrant.com>	<52ABCF6C.4010409@gmail.com>
	<52ABE150.6050201@2ndquadrant.com>	<52D98436.2030508@gmail.com>
	<52DC82E2.5010801@2ndquadrant.com>	<52DCBEF1.3010004@2ndquadrant.com>
	<52E1CC04.8040907@2ndquadrant.com>	<52E1DAD8.30909@2ndquadrant.com>
	<52E22C8E.2020702@2ndquadrant.com>
 <CAEZATCVJPnUrLjsS626Pp+r_bdq-3ofRHWmV0fKx1OgpyLt5ww@mail.gmail.com>
In-Reply-To: 
 <CAEZATCVJPnUrLjsS626Pp+r_bdq-3ofRHWmV0fKx1OgpyLt5ww@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

On 01/24/2014 07:16 PM, Dean Rasheed wrote:

> My first thought is to add a boolean flag to RangeTblEntry (similar to
> the "inh" flag) to say whether RLS expansion is requested for that
> RTE. Then set it to false on each RTE as you add new RLS quals to it's
> securityQuals.

That's what I was getting at with adding flags to RangeTblEntry, yes.

Given the number of flags we're growing I wonder if they should be
consolodated into a bitmask, but I'll leave that problem for later.

I'll do this part first, since it seems you agree that a RangeTblEntry
flag is the appropriate path. That'll make row-security checking work
and make the patch testable.

It won't deal with recursive rules, they'll still crash the backend.
I'll deal with that as a further step.

> In addition, when adding RLS quals to an RTE, I think they should be
> fully and recursively expanded immediately, before setting the new
> flag to false and moving on --- think recursively calling the rewriter
> to expand view references in the new RLS qual, and
> expand_security_qual() to expand any additional RLS quals in the
> securityQuals list --- with loop detection there. I'm not pretending
> that's going to be easy, but there are a couple of existing pieces of
> code to borrow ideas from. Doing it this way should make it possible
> to do the loop detection without any global structures.

Ugh. I was really hoping to avoid *another* place where we do recursive
expansion and infinite recursion checking, especially when the rewriter
already does this job.

Unfortunately, so long as the rewriter doesn't know about inheritance,
it cannot fully solve this problem. A mutually recursive rule involving
inheritance wouldn't get detected by rewriter based checking.

The original RLS patch only detects direct recursion, btw, it looks like
it won't catch mutual recursion.

-- 
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers