Re: [PATCH] SE-PgSQL/lite rev.2163

Robert Haas wrote:
>> If so, I can postpone some of permission checks outside of the
>> pg_xxx_aclcheck(). However, SELinux's security model often
>> requires different criteria to make its decision.
>> (Please note that I never say either of them is better or worse.)
>> Thus, we will need to add security hooks outside of the pg_xxx_aclcheck()
>> in the future, although it can be done step-by-step.
>
> Yes: to repeat what has been said multiple times previously, you
> should postpone everything that isn't a mirror of the current security
> model: there should only be permission checks in places where there
> are permissions checks now, and they should be mirror images of the
> current DAC checks.

OK, it seems to me reasonable.

> As for the rest, I think you've likely got it backwards: we shouldn't
> start by not cluttering the entire code-base with sepgsql permission
> checks, and then go and do it later after the basic functionality is
> in. We shouldn't EVER clutter the whole code-base with sepgsql
> permission checks. If we want to expose other sepgsql functionality,
> we should do that by putting in more kinds of pg_xxx_aclcheck()-type
> calls in other places, and the sepgsql can leverage those call sites
> too. But never mind that, because it's irrelevant for right now: what
> you need to do is strip this down to the minimal feature set without
> worrying about what will or won't happen later. Otherwise, nothing is
> going to happen at all.

I might call them pgace_xxx....
Anyway, I can agree to begin and focus on the minimal features which
can be implemented on the existing pg_xxx_aclcheck().

>> I guess the following permissions can be checked at the first version.
>> - db_database:{access} ... equivalent to ACL_CONNECT on pg_database
>> - db_procedure:{execute} ... equivalent to ACL_EXECUTE on pg_proc
>> - db_schema:{search} ... equivalent to ACL_USAGE on pg_namespace
>
> Why not db_database:{connect} and db_schema:{usage}? It seems to me
> that there is zero value in inventing new names for the same things.

We don't have any specific reason for db_database:{connect}.
In the early phase, SELinux's upstream security policy merged definitions
of object classes (such as db_table) and corresponding permissions,
so I uses its naming scheme.

However, security policy maintainer also said it is acceptable
if PostgreSQL folks merge SE-PgSQL with new permissions.
I don't mind to change db_database:{access} to {connect}.
(In actually, we need to add {connect} newly and mark {access} as
obsolete due to the compatibility issue.)

On the other hand, db_schema class was designed as an analogy to
directoty in filesystems. SELinux defines several permissions on
"dir" object class, such as "add_name", "remove_name" and "search".

dir:{search} is checked when we lookup filesystem object under
a certain directory, it seems like what schema object performs.
So, I would not like to change this naming scheme, if possible.

FYI, I summarized the list of SE-PgSQL permissions in the fullset
functionality here:
http://wiki.postgresql.org/wiki/SEPostgreSQL_References#Object_classes_and_access_vector

>> - db_database:{superuser} to be called from superuser_arg().
>> Should it be postponed to the next phase?
>
> No, I don't see any reason to postpone that. That seems analagous to
> what we do now, and should be included in the first version, I would
> think.

OK,

>> BTW, SELinux needs objects to be labeled correctly on its creation time.
>> At least, we have to put hooks to provide security labels to be assigned
>> on the DefineRelation() and so on, although it does not check permission
>> to create the objects.
>
> That seems very reasonable to me.

OK,

I'll update my patch set within this week.
Please wait for SE-PgSQL/tiny.
^^^^
Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>