Re: SE-PostgreSQL Specifications

Stephen Frost wrote:
>> 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.

Basically, I can agree this approach.
However, it is necessary to consider the idea deeply whether it is really
possible to implement the SELinux's model correctly, or not.

For example, ExecCheckRTEPerms() checks permissions on DML statement.
It allows users to access required table and columns, if he has appropriate
permissions on the table OR all the columns in use.
But SELinux requires the client needs to be allowed on the table AND all
the columns in use.

Please note that all we need to focus on is not pg_xxx_aclcheck() routines
in other words.

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

What I would like to say was how does the user defined function is used.
(However, it is the issue corresponding to the security model, not where
we should put the security hooks.)

For example, type input/output handlers are invoked without permission
checks on runtime. We never see an error due to the lack of permission
to execute textout() function. The native privilege model assumes type
input/output handlers can be trusted, because only superuser can set up
own types. (In fact, DefineType() checks superuser() at the head.)

So, rest of the routine does not check anything based on the assumption
of the superuser. See the #ifdef NOT_USER ... #endif block in DefineType().

It looks like the native database privilege mechanism does not check such
a permission, however, it implicitly assume ACL_EXECUTE permission is
always allowed for superuser.

Here, it seems to me ACL_EXECUTE has two different meanings here.
The one is obvious. It is a privilege a certain user to execute a certain
function in his query. The other is a privilege a certain user allows
everyone to execute a certain a part of the system internal stuff
(such as type input/output handlers).

The example is not dramatically different from the others, indeed.
But, this code implicitly assume the database superuser can do anyhting,
so the necessary checks are omitted from the code (because they always
return "allowed").

I think it is also necessary to follow these implicit permission check.

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

At this time, this approach seems to me reasonable.

I think what I should do on the next is ...
- To check up whether it is really possible to implement SELinux's model.
- To describe the list of the security functions in the new abstraction layer.
- To discuss the list of permission at:
http://wiki.postgresql.org/wiki/SEPostgreSQL_Development#Mandatory_access_controls

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