Re: How to get SE-PostgreSQL acceptable - Mailing list pgsql-hackers

On Wed, Jan 28, 2009 at 6:34 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> As an example, the present patch imagines that it will have adequate
> control over inserts by putting hooks into simple_heap_insert and the
> (rather small number of) places that call heap_insert directly.  But
> there are dozens of calls of simple_heap_insert and no way for the
> function itself to know why it is being called or on whose behalf.
> The patch's hook function tries to work around the fact that it hasn't
> got that information by means of heuristics.  Aside from the question of
> whether there are bugs in those heuristics today (I'm certain there
> are), every time we accept a patch that adds another call of
> simple_heap_insert, we're going to have to revisit the hook to see
> if it needs to be twiddled.

I agree that that's no good.

> Another problem is that since the hook only knows the parameters to
> simple_heap_insert plus global state (such as current_user), it can't
> cope very well with security-related context changes.  We have already
> heard that situations involving views are simply broken in the patch as
> it stands --- row-level permissions are checked against current_user
> and not the view owner, and there's no good way to fix that.

I thought that was intentional, and I sort of think that it's the
right decision.

>> It seems to me that the crucial decision is "Where are we
>> trying to erect the security wall?".  If we're trying to put it
>> between the client and the postgres backend, then maybe the right
>> thing to do is apply some sort of processing step to each query that
>> is submitted, essentially rewriting "SELECT * FROM a, b" as "SELECT *
>> FROM a, b WHERE you_can_see(a.securityid) AND
>> you_can_see(b.securityid)".  Maybe you (Tom) still won't like this
>> because it breaks SQL semantics, but it seems like it would at least
>> centralize the code.
>
> Well, it wouldn't break SQL semantics from the point of view of the
> planner, so that's one demerit taken off the books.  However, I seem
> to recall that exactly that approach was taken in a older version of
> the patch (many moons ago) and we found fatal problems in it too.

Can you (or someone) provide a pointer to the archives?  I can't
immediately see any reason why that problem wouldn't be fixable.

> The "where's the wall" point is a good one.  I think part of the issue
> is that the patch is to some extent trying to erect a security wall
> that's between the data and large parts of the backend C code.  Which is
> an interesting idea and maybe could have been made to work if it'd been
> designed in from the beginning, but to retrofit it today seems pretty
> impractical.

Agreed.  With all respect to Kaigai-san, I am suspicious that some of
this may be unintentional.  It may be that cleaning this up and
putting the wall in one well-defined spot will allay a number of your
concerns.

...Robert