Robert Haas wrote:
On Wed, Jan 28, 2009 at 6:34 PM, Tom Lane <t...@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.
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 <kai...@ak.jp.nec.com>
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
