Re: SE-PostgreSQL Specifications - Mailing list pgsql-hackers

KaiGai,

* KaiGai Kohei (kaigai@kaigai.gr.jp) wrote:
> It seems to me your suggestion is similar to the idea of PGACE framework.

It is, but it's being done as incremental changes to the existing
structures, and working with them, instead of ignoring that they exist.

> Let's consider the matter more using a few examples.
>
> Example 1) ALTER TABLE xxx
> The native database privilege mechanism checks the ownership of the target
> table to control the ALTER TABLE. On the other hand, SE-PgSQL checks the
> db_table:{setattr} permission on the target table to control the ALTER TABLE.
>
> I believe you can notice both of checks have same purpose but differences are
> on the criterions. It is the way to make their decisions in other word.
>
> The pg_xxx_aclcheck() interfaces provide the way to make its access control
> decision. The core PG code calls pg_xxx_aclcheck() to achieve its purpose
> which is to apply access controls on ALTER TABLE.
>
> Accordingly, if we avoid to put SE-PgSQL's security hooks on the tablecmd.c
> directly, what we should do is to inject an abstraction layer between tablecmd.c
> and aclchk.c.
>
> For example:
>   void pg_security_alter_table(Oid relid)
>   {
>       if (!pg_class_ownercheck(relid, GetUserId())
>           aclcheck_error(...);
>
>       if (!sepgsqlCheckTableSetattr(relid))
>           selinux_error(...);
>   }

Right, something along these lines, where the function lives in aclchk.c
and is part of its overall API.  This also shows pretty clearly how, if
we needed to add other hooks into the permissions for this operation, we
could do so.  It also shows how we could *first* build:

void pg_security_alter_table(Oid relid)
{if (!pg_class_ownercheck(relid, GetUserId())    aclcheck_error(...);
}

and make the corresponding changes in tablecmds.c, etc, and do that as a
separate patch.  Once that's done, we can review it, test it, etc, as
just an incremental change to PG.  This should be much easier/faster to
review as a patch as well, and to convince ourselves that we havn't
broken PG's current security (which is an extremely critical thing).
Additionally, when we then come back and add SELinux hooks, they will be
done in an isolated area and be additions to a system which has been
made to support such an extension.


> Example 2) DROP TABLE xxx
>
> The native database privilege mechanism also checks the ownership of the target
> table to be dropped (except for cascaded deletion).
> However, SELinux want to apply db_table:{drop} permission, instead of db_table:{setattr}.
> It is obvious that pg_class_ownercheck() cannot be a caller of the SELinux's
> security hook.
>
> It should be:
>   void pg_security_drop_table(Oid relid)
>   {
>       if (!pg_class_ownercheck(relid, GetUserId())
>           ackcheck_error(...);
>
>       if (!sepgsqlCheckTableDrop(relid))
>           selinux_error(...);
>   }

Right.  Similar to above.

> Example 3) ACL_EXECUTE
>
> The ACL_EXECUTE permission is checked on the runtime and setup time.
> The meaning of runtime is obvious. It should be checked when user's query tries to
> execute a function.
> For example, CreateConversionCommand() checks ACL_EXECUTE when user tries to create
> a new conversion. It is available for all the users.
> However, SELinux want to apply different permissions for runtime and installation time.
>
> On the runtime of the function, it applies db_procedure:{execute}.
> On the installation time, it applies db_procedure:{install}.
> Because a user defined function is installed as a part of system internals,
> it is available for every users. SELinux considers it is fundamentally different
> from a user can run a function defined myself.

I'm not sure I follow how this is dramatically different from the other
examples.  We have checks in place in PG already at both runtime of the
function (user has 'execute' privilege) and at creation time (functions
are created in schemas, after all).  If there are checks that PG doesn't
do today but which SELinux wants, those can be added too..  But as we've
discussed, that should be postponed until we get this initial structure
in place to allow PG to be extensible in this way.

> Because the topic is a bit abstract, it is not clear whether I can introduce
> what I want to say correctly, or not.
> Please feel free to ask me, if something unclear.

Yeah, I don't entirely get what you're trying to say here.  Perhaps you
could try and rephrase it?

> It is described at:
>   http://wiki.postgresql.org/wiki/SEPostgreSQL_Development#Common_object_behavior
>
> When we create a new table, SELinux's mode requires all the following
> permissions individually.
>  - db_schema:{add_name}, because a table is created under a certain schema
>  - db_table:{create} due to the common object behavior
>  - db_column:{create} for each columns, due to the common object behavior
>
> Note that the newly created object does not exist when SE-PgSQL checks its
> db_xxx:{create} permission.
> Accordingly, it computes a default security context (if user gives nothing)
> to be assigned on the new object, and it checks db_xxx:{create} permission
> toward the context. Then, it returns the security context to the caller.

That sounds like it should all be possible to do through the mechanism
I'm suggesting..

> As I mentioned above, if (enhanced) PG's access control mechanism can
> contain all the needed SELinux hooks, I can agree to put security hooks
> inside the PG's security.
> However, note that I have a legitimate reason that we cannot put SELinux
> hooks inside the *current* pg_xxx_aclcheck() routines, to implement the
> security model of SELinux correctly.

Sure, and that's fine.  I think the problem that you keep running into
is this:  You don't want to touch the PG code too much for fear of the
patch being too big, but at the same time, what you need really should
be implemented by making those changes to the core code.

We're not afraid of making large scale changes to core.  It just needs
to be done incrementally and done *first*, before adding features and
code which depends on it.  We have to fix PG *first*, in this regard,
before we can even start to look at SELinux hooks, etc.
Thanks,
    Stephen