Skip site navigation (1) Skip section navigation (2)

Re: How to get SE-PostgreSQL acceptable

From: KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: How to get SE-PostgreSQL acceptable
Date: 2009-01-29 02:49:06
Message-ID: 49811922.5050000@ak.jp.nec.com (view raw or flat)
Thread:
Lists: pgsql-hackers
Robert Haas wrote:
> On Wed, Jan 28, 2009 at 6:34 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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.

My concern is that superuser is allowed to modify system catalog
by hand, like:

   UPDATE pg_proc SET probin = '/tmp/malicious_library.so'
      WHERE oid = ...;

It is logically same as ALTER FUNCTION.

Even if I remove a hook from simple_heap_xxxx(), it is necessary
to check queries from clients.


>> 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.

 From the viewpoint of SELinux, it is not a matter because core
PostgreSQL does not change client's security context.
But, it is indeed controversial for Row-level ACLs.
(We have a time about this issue.)

>>> 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.

IIRC, 0racle or M$ has a patent to rewrite WHERE clause for security
purpose, so Tom suggested it should be implemented using a hook
deployed within executor.
At least, it also enables code more simple.

>> 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.

A wall between the data and _backend C code_ is not my intention.

Its purpose is a wall between the data and clients including superuser
via SQL queries. It is a basic assumption we cannot acquire any accesses
from system internal entities, as SELinux do nothing for kernel loadable
modules.

However, please note that making a decision at more hot point is more
good design, because it enables to reduce potential bypasses.
At the begining, I choosed simple_heap_xxxx() as a most hot point.
However, it can be replacable, if we can find any other place without
omission and consistent.

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

In response to

Responses

pgsql-hackers by date

Next:From: Stephen FrostDate: 2009-01-29 03:08:57
Subject: Re: How to get SE-PostgreSQL acceptable
Previous:From: Joshua BrindleDate: 2009-01-29 02:36:14
Subject: Re: How to get SE-PostgreSQL acceptable

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group