Re: [0/4] Proposal of SE-PostgreSQL patches - Mailing list pgsql-hackers

I wrote:
> I tried to do a bit of testing of this, but it does not work on current
> Fedora 8, because the policy module doesn't build:

I got past that by commenting out the last few lines of sepostgresql.te,
which apparently are intended for some SELinux feature that's not
shipped yet in F8.

After that, and a whole bunch of foolery with manually putting the right
security labels on files (because the contrib module is unhelpful for
test installations with nonstandard PREFIXes), I managed to get a
version that worked well enough to test.  What I found out:

* It does not come close to passing the regression tests.  I saw a lot of
! ERROR:  unrecognized node type: 903
which suggests that something's been screwed up about parse analysis
(903 = T_A_Const, which shouldn't get further than parse analysis),
and it dumps core in the TRUNCATE test.

* pgbench shows a relatively marginal speed difference, which is not
too surprising since it pushes only a few tuples around per query.
The worst case for sepostgres is likely to be bulk operations.
I tried "select count(*) from accounts" (at scale factor 20, so this
is 2000000 tuples) and got about 550 msec from CVS HEAD, about
1340 msec from HEAD plus patch.  That's with no actual useful policy
loaded, mind you.

So the overhead is indeed pretty bad, but it seems reasonable to think
that with some optimization effort it could be brought down to where
it'd be acceptable for people who really need the feature.

AFAICS the only thing left that really needs to be discussed more during
this commit-fest is the business about whether it's sane to be trying to
apply selinux restrictions in simple_heap_update and friends.  The more
I think about that the more I think it's simply a bad idea.  Those
functions are used primarily (if not solely) for system-initiated
catalog updates, and I fail to see any plausible reason to let
user-written policy be applied at that level.  For instance, it would
be a completely bad move to allow such policy to prevent autovacuum
from updating relfrozenxid values.  Furthermore, those functions are
much too low-level to have any idea of the reason for the change they
are being asked to make, so they can't apply any intelligence to the
allow-or-not decision.  Lastly, I don't see any use-case for policy there
that wouldn't be served as well or better by the standard SQL privilege
mechanism.  So I think that really all needs to be ripped out, and
do enforcement in the executor instead.
        regards, tom lane