Re: SE-PostgreSQL/Lite Review

I'd like to summary about the framework.

* Functionalities

The ACE framework hosts both of the default PG checks and upcoming
enhanced securities. We can build a binary with multiple enhanced
security features, but user can choose one from them at most due
to the security label management.

So, it basically has the following structure:

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
default:
break;
}
return retval;
}

* Namespace

The framework is stored in the src/backend/security/ace.
As a general rule, Each source file has this naming convension:
src/backend/security/ace/<object_class>_ace.c

Uncategolized hooks (such as cascaded-deletion called from dependency
system) are stored in ace/sysobj_ace.c. It is an exception.

All the framework functions (except for static) have "ace_" prefix
to distinguish from any other internal routines.

The security providers (SELinux, upcoming SMACK, ...) specific files
are stored in a certain directory in src/backend/security/ace/.
SE-PgSQL shall use "sepgsql" directory.

* Documentation

I don't plan to provide SGML documentation about ACE itself, except for
internal section, because it is an internal infrastructure.
For developers, src/backend/security/ace/README will provide a brief
overview of the ACE framework, and every security hooks have source
code comments to introduce the specification of itself.

* Patches

As a general rule, a patch is organized for each object classes, except
for very tiny object classes.
e.g)
pgsql-ace-02-database.patch
pgsql-ace-03-schema.patch
:

I'll try to submit a set of patches related to the following object classes
and functionalities by the 12/16.

* pgsql-ace-01-common.patch
It contains common part of ACE and uncategolized ones, such as cascaded
deletion from dependency, and so on.
* pgsql-ace-02-database.patch
It contains security hooks related to pg_database.
* pgsql-ace-03-schema.patch
It contains security hooks related to pg_namespace.
* pgsql-ace-04-relation.patch
It contains security hooks related to pg_class and pg_attribtue
* pgace-ace-05-label.patch
It contains security label support in SQL statement.

I'd like to conclude arguable things earlier than whole of reworks.

Then, we shall be able to see the rest of patches within this year.

Thanks,

KaiGai Kohei wrote:
> 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>