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

Stephen Frost wrote:
> KaiGai,
> 
> * KaiGai Kohei (kaigai@kaigai.gr.jp) wrote:
>> Stephen Frost wrote:
>>> Strategy for code changes:
>>>   Patch #1: Move permissions checks currently implemented in other parts
>>>             of the code (eg: tablecmds.c:ATExecChangeOwner()) into
>>>             aclchk.c.
>>>   Patch #2: Change the aclchk.c function APIs, if necessary, to add
>>>             additional information required for SELinux (eg: the 'old'
>>>             owner).
>>>   Patch #3: Add SELinux hooks into aclchk.c functions.
>> Thanks for your idea. However, it seems to me this approach eventually
>> requires bigger changeset than what the current security hooks doing.
> 
> Yes, it almost certainly will.  However, this is really the approach
> that I think we need to take.  I think what we're beginning to
> understand, collectivly, is something that some of the committers, etc,
> have been saying all along- implementing SELinux in PG requires alot
> more changes to PG than just sprinkling hooks all over the code.  This
> is because we want to do it right, for PG, and for our users.
> 
> What this means is that we need to improve the PG code first.  Then,
> when we're happy with those changes, we can add the SELinux hooks and be
> comfortable that they will operate correctly, do what they're intended
> to, and also allow us to extend the system further for the "next big
> privileges thing", if that happens some day.

When I scraped the idea of PGACE framework, someone (IIRC, Robert Haas) asked
me whether it is possible to implement the native database privilege mechanism
on the PGACE, or not.
It seems to me your suggestion is similar to the idea of PGACE framework.
But, note that it does not mean I criticize your idea.

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(...); }

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(...); }

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.


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.


>> In addition, it cannot solve the differences in behavior due to the
>> security model on which they stand.
>>
>> For example, when we create a new table, the native database privilege
>> mechanism checks its permission on the schema object which the new
>> table blongs to. On the other hand, SELinux requires to check
>> "db_table:{create}" permission on the newly created table itself.
> 
> That's fine.  We implement a function in aclchk.c which is essentially
> "pg_createTablePriv(schema,newtable)".  The logic for doing the check is
> moved out of tablecmds.c (or where ever) and into that function.  We can
> then easily add to that function the SELinux hook.  Is there some issue
> here which I'm not seeing?  If so, can you please clarify?
> 
> Also, I don't see "db_table:{create}" in the documentation anywhere.
> There is a "db_schema:{add_name}", is that what you're referring to?
> How would you check the permissions on an object (and how or why would
> they be different than the default, unless that's also being handled at
> the creation time) which is in the process of being created?

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

>> In my understanding, the purpose of the design specification is to make
>> clear for database folks what the security hooks doin and why the security
>> hooks are deployed here.
> 
> I don't think we're going to have much buy-in or success sprinkling
> these hooks all over the PG source code.  It would make maintenance a
> real problem for the PG folks and for those of us trying to work with
> SE.  The SELinux hooks really need to be folded into PG's existing
> authentication system, and if that system needs to be improved or
> expanded, then that's what it requires.

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.

Anyway, if we put security hooks inside from the common strategic points,
we need one more abstraction layer for access controls.

If we reconstruct the current PG's access control, it is necessary.

Thanks,
-- 
KaiGai Kohei <kaigai@kaigai.gr.jp>