Re: Proposal of SE-PostgreSQL patches [try#2] - Mailing list pgsql-hackers

On the WiKi of CommitFest:Sep,  http://wiki.postgresql.org/wiki/CommitFest:2008-09

The entry of SE-PostgreSQL points a message when I submitted older version
of our patch set. But the latest ones are listed on another message.

Please add a link to the following message for our convenience:
http://archives.postgresql.org/pgsql-hackers/2008-07/msg01365.php


BTW, I can respond to reviewer's comments and update/rework our patches
on this August, although SE-PostgreSQL is moved to the CommitFest:Sep.
We welcome any early comments, if possible.

Thanks,

KaiGai Kohei wrote:
> Hello,
> 
> The following patches are updated ones of SE-PostgreSQL toward the HEAD 
> of CVS.
> 
> [1/4] 
> http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r958.patch 
> 
> [2/4] 
> http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r958.patch 
> 
> [3/4] 
> http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r958.patch 
> 
> [4/4] 
> http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r958.patch
> 
> By the way,
> http://wiki.postgresql.org/wiki/CommitFest:2008-07
> 
> | Peter says: checking with Solaris engineers about compatibility with
> | Solaris TX; will continue review throughout August
> 
> 
> Jon, do you have anything to comment about PGACE security framework?
> (Show the src/include/security/pgace.h)
> 
> It has been reworked a bit from this April when we had an offline meeting,
> but I think its impact is not significant for its portability.
> 
> It can provide its guest subsystem (like SE-PostgreSQL) a series of 
> hooks to
> make a decision and facilities to manage text represented security 
> attribute
> of database objects.
> IIUC, we concluded these foundations are also enough to port SolarisTX 
> feature.
> 
> If you find out something lacks/conflicts, please tell me.
> 
> Thanks,
> 
> 
> KaiGai Kohei wrote:
>> Hi,
>>
>> I updated the series of patches, as follows:
>>
>> [1/4] Core facilities of PGACE/SE-PostgreSQL
>>     
>> http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r953.patch 
>>
>>
>> [2/4] "--security-context" option of pg_dump/pg_dumpall
>>     
>> http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r953.patch 
>>
>>
>> [3/4] Default security policy for SE-PostgreSQL
>>     
>> http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r953.patch 
>>
>>
>> [4/4] Documentation updates
>>     
>> http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r953.patch 
>>
>>
>> List of updates:
>> * "--enable-selinux" option of pg_dump/pg_dumpall are replaced
>>   by "--security-context" option.
>> * A new GUC parameter of "pgace_security_feature" is added to show
>>   what feature is worked on PGACE security framework.
>> * pg_ace_dumpXXXX() hooks are added to src/bin/pg_dump/pg_ace_dump.h
>>   to abstract security attribute dumping effort.
>> * An extended syntax of CONTEXT = '...' is replaced by
>>   SECURITY_CONTEXT = '...'.
>> * The sources of security policy module are moved from contrib/ to
>>   src/backend/security/sepgsql/policy.
>> * The prefix of interfaces in the default security policy are changed
>>   to sepgsql_*() from sepostgresql_*()
>> * Using integer value as a condition is replaced as follows:
>>     if (!strcmp(..)) -> if (strcmp(...) == 0)
>> * Two potential bug fixes:
>>    1. Unconditional Assert() in sepgsql_avc_reclaim().
>>    2. relkind checks are lacked in sepgsqlSetDefaultContext().
>>
>> The patch of core facilities is unchanged expect for the new GUC 
>> parameters
>> and above two minor bug fixes.
>>
>> And I have a question. Is it legal to use a pointer value as a condition,
>> like `while (!pointer) ...' ?
>>
>> Thanks for youe reviewing.
>>
>>
>> KaiGai Kohei wrote:
>>> Thanks for your reviewing.
>>>
>>> Peter Eisentraut wrote:
>>>> Am Donnerstag, 26. Juni 2008 schrieb KaiGai Kohei:
>>>>> The following patch set (r926) are updated one toward the latest 
>>>>> CVS head,
>>>>> and contains some fixes in security policy and documentation.
>>>>
>>>> OK, I have quickly read through these patches.  They look very nice, 
>>>> so I am optimistic we can get through this.
>>>>
>>>> First of all, now would be a good time if someone out there really 
>>>> wants to object to this feature in general.  It will probably always 
>>>> be a niche feature.  But all the code is hidden behind ifdefs or 
>>>> other constructs that a compiler can easily hide away (or we can 
>>>> make it so, at least).
>>>>
>>>> Here is a presentation from PGCon on SE-PostgreSQL: 
>>>> http://www.pgcon.org/2008/schedule/events/77.en.html
>>>>
>>>> Are there any comments yet from the (Trusted)Solaris people that 
>>>> wanted to evaluate this approach for compatibility with their approach?
>>>
>>> In this April, I had a face-to-face meeting with Trusted Solaris people
>>> to discuss SE-PostgreSQL and PGACE framework, and concluded that PGACE
>>> framework provides enough facilities for both operating systems.
>>>
>>> I modified several hooks from CommitFest:May, however, its fundamental
>>> structures are unchanged.
>>>
>>>> In general, are we OK with the syntax CONTEXT = '...'?  I would 
>>>> rather see something like SECURITY CONTEXT '...'.  There are a lot 
>>>> of contexts, after all.
>>>
>>> If we change it, I prefer SECURITY_CONTEXT = '...' style, because it 
>>> enables
>>> to represent the left hand with a single token and make PGACE hooks 
>>> simpler.
>>> I also agree the CONTEXT has widespread meanings and to be changed here.
>>>
>>>> This will also add a system column called security_context.  I think 
>>>> that is OK.
>>>
>>> Thanks,
>>>
>>>> In the pg_dump patch:
>>>>
>>>> spelling mistake "tuen on/off"
>>>
>>> I'll fix it soon.
>>>
>>>
>>>> Evil coding style: if (strcmp(SELINUX_SYSATTR_NAME, 
>>>> security_sysattr_name)) -- compare the result with 0 please.
>>>
>>> OK, I'll fix it and check my implementations in other place.
>>>
>>>
>>>> The above code appears to assume that security_sysattr_name never 
>>>> changes, but
>>>> then why do we need a GUC parameter to show it?
>>>
>>> The purpose of the GUC parameter is to identify the kind of security 
>>> feature
>>> if enabled. It can be changed by other security features, and it will 
>>> show us
>>> different value.
>>>
>>>
>>>> Might want to change the option name --enable-selinux to something 
>>>> like --security-context.
>>>>
>>>> In general, we might want to not name things selinux_* but instead
>>>> sepostgresql_* or security_* or security_context_*.  Or maybe PGACE?
>>>
>>> The pgace_* scheme is an attractive idea, although the server process
>>> has to provide a bit more hints (like the name of security system column
>>> and the kind of objects exported with security attribute) pg_dump to
>>> support various kind of security features with smallest implementation.
>>>
>>> If not so, I prefer the combination of --security-context and 
>>> sepostgresql_*.
>>>
>>>
>>>> On the default policy:
>>>>
>>>> Should this really be a contrib module?  Considering that it would 
>>>> be a core
>>>> feature that is not really usable without a policy.
>>>
>>> It is correct, SE-PostgreSQL feature always need its security policy.
>>> Do you think "src/backend/security/sepgsql/policy" is better?
>>>
>>>
>>>  > Please change all the sepgsql_* things to sepostgresql_*, 
>>> considering that you
>>>  > are using both already, so we shouldn't have both forms of names.
>>>
>>> We can convert them from sepostgresql_* to sepgsql_* easily, because 
>>> the longer
>>> forms are not used as a part of identifier in security context.
>>> But we have a possible matter in changing from sepgsql_* to 
>>> sepostgresql_*.
>>>
>>> Here is a news from SELinux community:
>>>   http://marc.info/?l=selinux&m=121501336024075&w=2
>>>
>>> It shows most part of the SE-PostgreSQL policy module got merged into
>>> the upstreamed at selinux-policy-3.4.2 or later. It contains identifier
>>> with sepgsql_* stuff, so it has a possible matter when users upgrade
>>> his security policy.
>>>
>>> If their database is labeled as sepostgresql_*, it will lose rules
>>> defined in the policy when users upgrade selinux-policy package to
>>> the latest one.
>>>
>>>
>>>> Documentation:
>>>>
>>>> Looks good for a start, but we will probably want to write more later.
>>>
>>> Do you think what kind of information should be added?
>>> I intentionally omitted descriptions about SELinux itself,
>>> because it is a documentation of PostgreSQL, not OS.
>>>
>>> Thanks,
>>
>>
> 
> 


-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>