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 makes reduce 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(at)kaigai(dot)gr(dot)jp>