Re: [0/4] Proposal of SE-PostgreSQL patches

From: KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org, KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>
Subject: Re: [0/4] Proposal of SE-PostgreSQL patches
Date: 2008-05-13 09:46:56
Message-ID: 48296390.5040201@ak.jp.nec.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

Tom Lane wrote:
> KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> writes:
>> Tom Lane wrote:
>>> Yeah, I remember those. What needs to be looked at here is *why* the
>>> output is changing. For a patch that allegedly does not touch the
>>> planner, it's fairly disturbing that you don't get the same results.
>
>> SE-PostgreSQL does not touch the planner, but it modifies given query
>> to filter violated tuples for the current user.
>
> Hmm. Is that really a good idea, compared to hard-wiring the checks
> into nodeSeqscan and friends? I didn't look at the query-rewriting
> portion of the patch in any detail, but I'd tend not to trust suchte
> a technique very far: getting it right is going to be quite complex
> and probably bug prone.

In the prior base version (8.2.x and 8.3.x), I tended to implement
these stuffs in the modular part as far as possible, because massive
patched hanks makes more difficult to follow the mainstreamed PostgreSQL. :-(

However, the hard-wides checks look like a good idea for me.
I tried to implement a prototype of the disign, and currently it works fine.
If we can replace the implementation of tuple-level access controls by this
design, it makes the implementation simpler.

Now, I add a hook into ExecScan(). It can return true or false, to decide
whether a given tuple is filtered or not.
Permissions to be evaluated are delivered via Scan structure. A variable
named as pgaceTuplePerms (uint32) is added to keep permission set for
tuple level access controls into Scan structure.
If the security module put a proper bitmask on pgaceTuplePerms of RangeTblEntry,
it is copied to related Scan structure later.

>>> Are you sure that the security_label type should not have an array type?
>
>> Yes, security_label type should not have an array type.
>
> You didn't provide one ounce of justification for making it not obey the
> expected behavior, so I'm not accepting this position. It doesn't seem
> to me to be all that unlikely that users would want to compute with
> arrays of security labels. As an example:
> select ... where security_label in ('foo', 'bar')
> which will become an = ANY(ARRAY[]) construct under the hood.

Ah.., I didn't intend such kind of usage, so security_label type does not
have operators to use it directly, not only array support.

I'll add it in the next patch set.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Gavin Sherry 2008-05-13 10:25:05 Arbitary file size limit in twophase.c
Previous Message Simon Riggs 2008-05-13 08:32:55 Re: odd output in restore mode

Browse pgsql-patches by date

  From Date Subject
Next Message Andrew Dunstan 2008-05-13 12:00:29 Re: odd output in restore mode
Previous Message Simon Riggs 2008-05-13 08:32:55 Re: odd output in restore mode