Re: SE-PostgreSQL/Lite Review - Mailing list pgsql-hackers

Stephen Frost wrote:
> KaiGai,
> 
> * KaiGai Kohei (kaigai@ak.jp.nec.com) wrote:
>> (1) Whether the framework should host the default PG model, not only
>>     enhanced security features, or not?
>>   This patch tried to host both of the default PG model and SELinux.
>>   But, the default PG model does not have same origin with label-based
>>   mandatory access controls, such as SELinux, from a historical angle.
>>   So, this patch needed many of unobvious changes to the core routines.
> 
> I certainly understand what you're saying here.  However, I feel that
> these changes to the core of PG are good, and are taking PG in the right
> direction to have a much clearer and better implemented security
> infrastructure.  The fact that you've found a number of odd cases
> (duplicate checking, odd failure cases due to checks being done at
> various parts of the process, etc) is a testement that this is bringing
> the PG code forward even without the addition of SELinux support.
> 
> I realize that makes it more work for you and I don't envy you that.

The reason why I prefer MAC only framework is not a technical reason.
As long as we can review it and acceptable, I have no reason to avoid it.

>> (2) Whether the framework should be comprehensive, or not?
>>   This patch tried to replace all the default PG checks in the core
>>   routines from the begining. It required to review many of unobvious
>>   changes in the core routine. Because I've been repeatedly pointed out
>>   to keep changeset smaller, I'm frightened for the direction.
>>   The coverage of the latest SE-PgSQL/Lite patch is only databases,
>>   schemas, tables and columns. I think it is a good start to deploy
>>   security hooks on the routines related to these objects, rather than
>>   comprehensive support from the begining.
> 
> I don't believe we will get support to commit a framework which is
> intended to eventually support the PG model (following from my answer to
> #1) in a partial form.  Perhaps it would be good to split the patch up
> on a "per-object-type" basis, to make it simpler/easier to review (as
> in, patch #1: database_ac.c + changes to core for it; patch #2:
> table_ac.c + changes to core for it, etc; earlier patches assumed to be
> already done in later patches), but the assumption will almost certainly
> be that they will all be committed to wonderful CVS at the same time.

As Rober Haas already suggested in another message, my patch in the last
commit fest is too large. It tried to rework anything in a single patch.
The "per-object-type" basis make sense for me.

In my cosmetic preference, "ace_" is better than "ac_". The 'e' means
extendable, and "ace" feels like something cool. :-)
In X-window system, it calls its framework "XACE".

And I'd like to store these files in "backend/security/ace/*.c", because
"backend/security" will also store other security modules!

src/+ backend/   + security/      + ace/         ... access control framework      + sepgsql/     ... selinux specific
code     + smack/       ... (upcoming?) smack specific code      + solaristx/   ... (upcoming?) solaris-tx specific
code        :
 


>> (3) In the future, whether we should allow multiple enhanced securities
>>     at the same time, or not?
>>   It was not clear in the previous commit fest. But, in fact, Linux
>>   kernel does not support multiple security features in same time,
>>   because it just makes security label management complex. (It also
>>   have another reasons, but just now unlear.)
>>   I don't think we have any good reason to activate multiple enhanced
>>   securities at same time.
> 
> To be honest, I feel that this will just fall out once we get through
> doing #1.  I've just heard from Casey (author of the SMACK security
> manager, an alternative to SELinux) that the PGACE framework (which is
> what we suggested he look at) would be easily modified to support SMACK.
> I feel the same would be true of what we're talking about in #1.  If you
> disagree with that, please let me know.

Ahh, what I want to say is, whether we should allow "enhanced" securities.
If we host the default PG model on the framework, it is an exception of
the count.

The reason why I prefer the default PG check + one enhanced security at
most is simplification of the security label management.
If two label-based enhanced securities are enabled at same time,
we need two field on pg_class and others to store security context.
In addition, OS allows to choose one enhanced security at most eventually.

In my image, the hook should be as:
 Value * ac_database_create([arguments ...]) {     /*      * The default PG checks here.      * It is never disabled
 */
 
     /* "enhanced" security as follows */ #if defined(HAVE_SELINUX)     if (sepgsql_is_enabled())         return
sepgsql_database_create(...);#elif defined(HAVE_SMACK)     if (smack_is_enabled())         return
smack_database_create(...);#elif defined(HAVE_SOLARISTX)     if (soltx_is_enabled())         return
soltx_database_create(...);#endif     return NULL; }
 

We can share a common image image?

>> I think most of folks can agree with (2) and (3). But I expect opinion
>> is divided at (1).
>>
>> For example, if we host the default PG checks to create a new database,
>> all the existing checks shall be moved as follows:
>>
>>   http://code.google.com/p/sepgsql/source/browse/trunk/pgsec/src/backend/security/access_control.c?r=2273#1430
>>
>> In addition, it has to return the security context to be assigned on
>> the new database, when it hosts SELinux.
>> Even if this rework is limited to four kind of object classes, I'm
>> worry about it overs an acceptable complexity.
> 
> I feel that what was at issue is that the complexity of
> "access_control.c" was far too great- because *everything* was in that
> one file.  Splitting access_control.c into smaller files would make them
> more managable, and if we implement them all in a similar manner using a
> similar policy for function names, arguments, etc, then once the first
> object type is reviewed, the others will go more easily for the reviwer.

Yes, agreed.

> If the security framework does *not* support the SQL model, I think
> we'll get push back from the community that it clearly couldn't be used
> by any other security manager generically without alot of potential
> changes and additional hooks that could end up bringing us all the way
> to being capable of supporting the SQL model anyway.  I'm certainly
> willing to hear core indicate otherwise though.

OK, I seems to me it is reasonable reason to integrate DAC and MAC into
a common framework.

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