Re: Updates of SE-PostgreSQL 8.4devel patches (r1704)

Stephen Frost wrote:
> * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
>> Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
>>> KaiGai Kohei wrote:
>>>> As I promised last week, SE-PostgreSQL patches are revised here:
>>> The patch adds permission checks to SET/SHOW. If that's useful
>>> functionality, it would be nice to see that as a separate patch, not
>>> requiring SE-Linux.
>> My goodness. This patch seems to be going FAR beyond what I thought
>> its charter was.
>
> I agree. I thought the idea was that the first round of SE-PostgreSQL
> additions would be to add SE hooks for permissions that PG already
> implements. Other permissions would then be implemented in a PG-way
> first, and SE hooks then added to those later.

This seems to be a recurring theme with this patch. We stripped
row-level permissions, now we have SET/SHOW and the "function
installation" permissions. And the read/write file permissions. To make
progress, we need to consider each new feature like that separately, as
separate patches.

KaiGei: Let's drop SET/SHOW permissions from the patch, I presume that's
not critical for the overall goal.

Dropping the "function installation" permissions would simplify the
patch a lot. It would make the patch as whole a lot easier to swallow.
Let's ask the same question as with the row-level permissions: If we
drop the function installation stuff, would the rest of the patch still
be useful? We can revisit that part in the future.

I have the same concern as Tom about trying to curtail what superusers
can do. We have not been concerned about what a superuser can and can't
do before, so there could be small loopholes all over the codebase that
we haven't thought about. Just as an example, you added checks to COPY
to prevent reads from/writes to files. That's restricted to superusers.
However, you left pg_read_file() in src/backend/utils/adt/genfile.c wide
open.

If we drop the goal of trying to restrict what a superuser can do, is
the patch still useful?

One idea is to add a single "is superuser" permission to sepgsql. That
can be checked in a single place: superuser_arg(). If SE-Linux says that
you don't have superuser permissions, then superuser() will return false
even if the current user is marked as a superuser in pg_role. It would
give the same level of protection as sprinkling those fine-grained
checks all over the code, just in a more coarse-grain fashion.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com