Re: Reworks for Access Control facilities (r2363)

Heikki Linnakangas wrote:
> KaiGai Kohei wrote:
>> The purpose of this patch is to provide function entrypoints for the
>> upcoming SE-PostgreSQL feature, because I got a few comments that we
>> hesitate to put sepgsql_xxx() hooks on the main routines directly in
>> the first commit fest. In addition, I already tried to put SE-PG hooks
>> within pg_xxx_aclchecks() in this CF, but it was failed due to the
>> differences in the security models.
>
> Can you elaborate that? It might well be that you need to adapt the
> SE-PostgreSQL security model to the one that's there already. Putting
> SE-PG hooks into existing pg_xxx_aclcheck functions is the only
> low-impact way I can see to implement SE-PostgreSQL.

We can show several examples that pg_xxx_aclcheck() routines are not
suitable to implement SELinux's security model.
Please note that it is not a defect of the default PG's security model
needless to say. It is just a different in standpoints.

1) creation of a database object

In SELinux model, when a user tries to create a new object (not limited
to database object, like a file or socket), a default security context
is assigned on the new object, then SELinux checks whether the user has
privileges to create a new object labeled with the security context, or not.

When we create a new table, the default PG model checks ACL_CREATE privilege
on the namespace which is supposed to own the new table. DefineRelation()
invokes pg_namespace_aclcheck() with OID of the namespace, but we cannot
see any properties of the new table from inside of pg_namespace_aclcheck().
It checks permissions on the couple of a user and a namespace.

On the other hand, SE-PG model follows the above principle. When we create
a new table, SE-PG compute a default security context to be assigned on,
then it checks the security policy whether the user is allowed to create
a new table labeled with the context, or not.
It checks permissions on the couple of a user and a new table itself.

The caller does not provide enough information to the pg_xxx_aclcheck(),
so we decided to create an abstraction layer which can provide enough
informations to both of security models. Then, the ac_xxx_*() routines
were implemented.

2) AND-condition for all the privileges

When a certain action requires multiple permissions at one time,
the principle of SELinux is that all the permissions have to be checked.
If one of them is not allowed, it disallows the required action.
In other word, all the conditions are chained by AND.

This principle enables us to analyze the data flows between users and
resources with the security policy, without implementation details.
If a certain permission (e.g db_table:{select}) can override any other
permission (e.g db_column:{select}), it also implicitly means a possibility
of infotmation leaks/manipulations, even if the security policy said this
user cannot read a data from the column.

On the other hand, the default PG model allows to bypass checks on
certain objects. For example, column-level privileges are only checked
when a user does not have enough permissions on the target table.
If "SELECT a,b FROM t" is given, pg_attribute_aclcheck() may not invoked
when user has needed privileges on the table t.

3) superuser is not an exception of access control.

It is the similar issue to the 2).
The following code is a part of AlterFunctionOwner_internal().

----------------
/* Superusers can always do it */
if (!superuser())
{
/* Otherwise, must be owner of the existing object */
if (!pg_proc_ownercheck(procOid, GetUserId()))
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_PROC,
NameStr(procForm->proname));

/* Must be able to become new owner */
check_is_member_of_role(GetUserId(), newOwnerId);

/* New owner must have CREATE privilege on namespace */
aclresult = pg_namespace_aclcheck(procForm->pronamespace,
newOwnerId,
ACL_CREATE);
if (aclresult != ACLCHECK_OK)
aclcheck_error(aclresult, ACL_KIND_NAMESPACE,
get_namespace_name(procForm->pronamespace));
}
----------------

From perspective of the default PG model, this code perfectly correct.
Both of pg_proc_ownercheck() and pg_namespace_aclcheck() always returns
ACLCHECK_OK, so these invocations are bypassable.

However, if SE-PG's hooks are deployed on pg_xxx_aclcheck() routines,
it means that we cannot check correct MAC permissions when a client is
allowed to apply superuser privilege.
Please remind that SELinux requires AND-condition for all the privileges
required to a certain action. When a root user tries to read a certain
file without DAC permisions, it requires both of capability:{dac_override}
and file:{read} permissions in operating system.

These are a part of reasons why we had to design such a large patch.
If we stick around the common entrypoints of access controls, such kind of
reworks are necessary. The current pg_xxx_aclcheck() routines are designed
for the database ACL model. It works fine and correctly.
However, it is not suitable to host the SELinux's model within the hooks.

>>> * There are two special-purpose shims, ac_database_calculate_size and
>>> ac_tablespace_calculate_size, that got added for the benefit of
>>> utils/adt/dbsize.c. What if that code were still in contrib? How is it
>>> different from a lot of the code that is in contrib now, eg dblink or
>>> pgrowlocks, to say nothing of third-party modules? Presuming that the
>>> shim layer can know explicitly about each individual permission-checking
>>> requirement is a dead-end design.
>> Back to the definition of access controls (or reference monitor).
>> It prevents violated accesses launched by user's requests (SQL).
>> It is not a job to protect something from malicious internal modules.
>
> The issue isn't malicious modules, but modules that have pg_xxx_aclcheck
> calls in them and haven't been modified to do SE-pgsql checks like you
> modified all the backend code. As the patch stands, they would perform
> just the regular acl checks and bypass SE-pgsql.

OK, I can understand what he wanted to say.
In the kernel module cases, we can find out modules which provide its own
checks based on user/group identifier, so it also means using the loadable
module allows to bypass MAC checks. However, the loadable module is loaded
at first, and the administrator (or init script) is allowed to load such
a loadable module which bypasses MAC checks.
In other word, the security policy basically controls whole of the usage
of these modules. Needless to say, we can add security hooks if necessary.

If we look at the SE-PgSQL project on the greater scale, it also can be
considered as an efforts to add MAC checks on the module which applied
its own access controls, but bypassed MAC checks.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>