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

Stephen Frost wrote:
>> In my cosmetic preference, "ace_" is better than "ac_". The 'e' means
>> extendable, and "ace" feels like something cool. :-)
> 
> No complaints here..  I just hope this doesn't end up being *exactly*
> the same as your original PGACE patches..  I'd feel terrible if we
> weren't able to at least improve something with this extremely long and
> drawn our process!

Please forget old PGACE. We're talking about our future. :-)

>> And I'd like to store these files in "backend/security/ace/*.c", because
>> "backend/security" will also store other security modules!
> 
> This is perfectly reasonable in my view.  No complaints here.
> 
>> src/
>>  + backend/
>>     + security/
>>        + ace/         ... access control framework
>>        + sepgsql/     ... selinux specific code
>>        + smack/       ... (upcoming?) smack specific code
>>        + solaristx/   ... (upcoming?) solaris-tx specific code
> 
> Looks good to me.  Perhaps we'll have a smack/ patch showing up very
> shortly as well..

It's a welcome feature.

>> 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.
> 
> Ah, yes, I see your point must more clearly now.  This sounds reasonable
> to me.
> 
>> 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?
> 
> If all of these security modules make sense as "only-more-restrictive",
> then I have no problem with this approach.  Honestly, I'm fine with the
> initial hooks looking as above in any case, since it provides a clear
> way to deal with switching out the 'default PG checks' if someone
> desires it- you could #if around that block as well.  As it's the
> principle/primary, and I could see "only-more-restrictive" being more
> popular, I don't mind having that code in-line in the hook.  The check
> itself is still being brought out and into the security/ framework.

Ahh, indeed, #elif does not allow a single binary to provide multiple
options. You are right.

I'd like to rewrite it as follows:
 Value * ac_database_create([arguments ...]) {     Value *retval = NULL;
     /*      * The default PG checks here.      * It is never disabled      */
     switch (ace_feature)     { #ifdef HAVE_SELINUX     case ACE_SELINUX_SUPPORT:         if (sepgsql_is_enabled())
       retval = sepgsql_database_create(...);         break; #endif #ifdef HAVE_SMACK     case ACE_SMACK_SUPPORT:
 if (smack_is_enabled())             retval = smack_database_create(...);         break; #endif #ifdef HAVE_SOLARISTX
 case ACE_SOLARISTX_SUPPORT:         if (soltx_is_enabled())             retval = soltx_database_create(...);
break;#endif     default:         break;     }
 
     return retval; }

> I do think that, technically, there's no reason we couldn't allow for
> multiple "only-more-restrictive" models to be enabled and built in a
> single binary for systems which support it.  As such, I would make those
> just "#if defined()" rather than "#elif".  Let it be decided at runtime
> which are actually used, otherwise it becomes a much bigger problem for
> packagers too.

The reason why I don't want to support multiple enhanced securities at
same time is complexity of security label management, not access control
model.

Unlike X-server, RDBMS have to manage the security label of database
object persistently on the disk. It naturally needs enhancement of data
structure, such as pg_class.relsecon field in the latest SE-PgSQL/Lite.

If the third enhanced security does not use security label, basically,
I think it is feasible. In fact, the default PG check is a security
provider without security label, and it can coexist with SELinux.

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