Re: SE-PostgreSQL/Lite Review

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(at)kaigai(dot)gr(dot)jp>