Re: Updates of SE-PostgreSQL 8.4devel patches (r1668)

KaiGai Kohei wrote:
> Heikki, Thanks for your comments.
> 
> Heikki Linnakangas wrote:
>> Ok, I've taken a quick look at this too. My first impression is that 
>> this is actually not a very big patch. Much much smaller than I was 
>> afraid of. It seems that dropping the row-level security and the other 
>> change you've already done have helped a great deal.
>>
>> My first question is, why does the patch need the walker 
>> implementation to gather all the accessed tables and columns? Can't 
>> you hook into the usual pg_xxx_aclcheck() functions? In fact, Peter 
>> asked that same question here: 
>> http://archives.postgresql.org/pgsql-hackers/2009-01/msg02295.php 
>> (among other things). Many things have changed since, but I don't 
>> think that question has been adequately answered. Different handling 
>> of permissions on views was mentioned, but I think that could be 
>> handled with just a few extra checks in the rewriter or executor.
> 
> Yes, one major reason is to handle views. SE-PostgreSQL need to check
> permissions on after it is extracted.   :
> I'll check some of corner cases, such as inherited tables, COPY
> statement, trigger invocations and others, to consider whether
> your suggestion is possible, or not.
> Please wait for a while to fix my attitude.

Heikki, I now feel tempted by an idea to utilize the facilities
of table/column-level privileges.

One matter was "use" permission, but I can agree to integrate
it into "select" permission as the original design did.

The other is view. When we use a view in the query, it is extracted
as a subquery and its query tree is fetched from pg_rewrite.ev_action
which is already parsed. It means we need to ensure the parsed
representation is not manipulated. The simplest solution is to prevent
updating the pg_rewrite.ev_action by hand when SE-PostgreSQL is enabled.

I think smaller hard-wired rules are better, but it is a very corner-case
and its benefit cannot be ignorable. - It enables to reduce the "walker" code from sepgsql/checker.c.   (I guess it
makesreduce a few hundreds lines.) - It helps to maintain code to pick up what tables/columns are   accessed.
 

If nobody disagree it, I'll integrate "use" permission into "select" and
remove the "walker" code from sepgsql/checker.c due to the next Monday.
It affects on sepgsql/checker.c, but I expect little changes on others.
I'm happy, if you don't stop reviewing patches except for checker.c.

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