Re: Reworks for Access Control facilities (r2363)

KaiGai Kohei wrote:
> 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.

I don't think I buy that argument. Can't we simply decide that in
PostgreSQL, the granularity is different, and you can only create
policies governing creation of objects on the basis of schema+user
combination, not on the properties of the new object. AFAICS it wouldn't
violate the principle of Mandatory Access Control.

> 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.

Hmm, I see. Yes, it does seem like we'd need to change such permission
checks to accommodate both models.

> 3) superuser is not an exception of access control.
>
> It is the similar issue to the 2).

Yeah.

> 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.

We need to ask ourselves, is that a realistic goal, given how widespread
such "if (superuser())" calls are? And more imporantly, unless you
sprinkle additional fine-grained permission checks to all the places
that currently just check "if (superuser())", it will be possible to
circumvent the system with LOAD or any of the other commands that are
inherently dangerous. We don't want such additional fine-grained
permissions, not for now at least.

Seems a lot simpler and also easier to understand if there's a single
superuser privilege that trumps all other permission checks.

> 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.

Yeah, it seems like any external modules need to be modified or at least
verified to comply with the MAC requirements. Your point 2) about
whether permissions are ANDed or ORred together seem to be the key here.

This raises an important point: We need *developer documentation* on how
to write SE-Pgsql compliant permission checks. Not only for authors of
3rd party modules but for developers of PostgreSQL itself. Point 2)
above needs to be emphasized, it's a big change in the way permission
checks have to be programmed. One that I hadn't realized before. I
haven't been paying much attention, but neither is most other
developers, so we need clear documentation.

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