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

Stephen Frost wrote:
> KaiGai,
> 
> * KaiGai Kohei (kaigai@ak.jp.nec.com) wrote:
>> For the recent a few days, I've worked to write and edit
>> the specification (partially copied from the draft of user
>> documentation) for the development purpose.
>>
>>  http://wiki.postgresql.org/wiki/SEPostgreSQL_Development
> 
> Thanks for doing this.  I've taken a quick glance through it and will
> review it more later today.  The next step, I believe, is to document
> the changes which need to be made to PG, at a high level, to support
> this specification.  

Thanks for your help.

I also think it is necessary to describe what changes are needed on
the core pgsql except for security routines.

It is just an example:
http://wiki.postgresql.org/wiki/SEPostgreSQL_Development#security_hooks_.28example.29

I believe such kind of specifications are necessary at the next.

> I think the initial goal should be to make changes mainly to aclchk.c to
> add the SELinux hooks.  I'd like to hear from some committers about this
> idea: I think we should also be looking to move more of the permissions
> checking which is done in other parts of the code (eg:
> ATExecChangeOwner()) into aclchk.c, which is where I would argue it
> belongs.  Doing so will make it much easier to add other hooks, should
> the need arise, and would minimize the amount of code 'touched' to add
> SELinux support.
> 
> 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.
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.

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.

Linux kernel is a good case in success.
Needless to say, most of kernel developer are not security experts.
But they defined an explicit specification of the security hooks called
LSM, and put these hooks on the strategic points in the kernel.
(The very long source code comments describes what is the purpose of this
security hook, what arguments should be given and so on for each security
hooks.)

> This initial round, again, should focus on just those
> controls/permissions which PostgreSQL already supports.  At this time,
> do not stress over finding every "if(superuser())" and moving it to
> aclchk.c.  Let's deal with the "clear" situations, such as tablecmds.c
> lines 6322-6342 (that entire block, including the if(!superuser()),
> should be ripped out and made a function in aclchk.c which is called
> with necessary args).  We can go back and "clean up" the other places
> where we have explicit superuser() checks later, if necessary.

At least, it works correctly as far as the native database privilege
mechanism. If the specification of the security hooks is clear, I think
the following manner is better than modifying DAC mechanism.
 if (!superuser()) {     if (pg_xxx_aclcheck(....) != ACLCHECK_OK)         aclcheck_error(....); }
+ sepgsqlCheckXXXX();

> I've finally had a chance to look through the last set of proposed
> patches some, and I've noticed some other issues that need to be
> addressed:
> 
> - Let's avoid the changes to heaptuple.c for now.  That's really for
>   much later down the road when we implement row-level security.

Yes, I can agree to postpone pg_security and corresponding facilities
until row-level security.

> - I would expect the dependency system to be used to handle deleting
>   things from pg_security, etc, when an object has been deleted (eg: a
>   table was dropped).

In the patch for v8.4.x series (with fullset functionality), it is
already implemented. Any entries within pg_security referenced by the
dropped table is reclaimed, when we drop a table.
The matter of orphan entries are already solved.

> - Conversly, when new entries need to be added to pg_security, they
>   should be done at the same level as other items being added to the
>   catalog.  In places like createdb(), where it's all one big function,
>   I would recommend splitting out the existing catalog update into a
>   separate function, which then makes it much clearer that the code is
>   doing: update pg_database table, update pg_security table, update
>   pg_dependency table.  That should be done as a separate patch, of
>   course.  Remember, we're trying to make small incremental changes that
>   make sense by themselves but at the same time will reduce the effort
>   required to add SELinux later.

Sorry, it's unclear for me what you would like to explain...

However, I can agree the step-by-step approach on development.
The design specification describes an "ideal image" of SE-PostgreSQL.
I understand several phases are necessary to come up to the stage.

> - In terms of cacheing the results, is there any way we could use
>   SysCache for this?  And just handle the cacheing in the hooks, etc,
>   from aclchk.c?  I dislike having the security labels added to
>   every tuple until we actually implement row level security.  It adds
>   alot of unnecessary complexity to the initial implementation.
>   Note that I'm referring to the results from the kernel/syscalls, I
>   realize you're using SysCache properly for the entries in pg_security
>   already, and that's good.

In the current fullset version, I uses SysCache facility to translate
between security identifier (Oid) and security context (text).
It should be implemented as a part of pg_security and corresponding
facilities.

What does the "cacheing the results" mean?
If you talked about the caches of access control decision, it is quite
different thing from SysCache mechanism. It intends to reduce the number
of kernel invocation, not the number of database lookups.

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