Re: 8.4 release planning - Mailing list pgsql-hackers

Joshua Brindle wrote:
> Tom Lane wrote:
>> Ron Mayer <rm_pg@cheapcomplexdevices.com> writes:
>>> It seems to me that there are two different standards to which this 
>>> feature
>>> might be held.
>>
>>> Is the goal
>>>   a) SEPostgres can provide useful rules to add security to some
>>>      specific applications so long as you're careful to avoid crafting
>>>      policies that produce bizarre behaviors (like avoiding restricing
>>>      access to foreign key data you might need).   On the other hand it
>>>      gives you enough rope to hang yourself and produce weird results
>>>      that don't make sense from a SQL standard point of view if you
>>>      aren't careful matching the SEPostgres rules with your apps.
>>
>>> or
>>>   b) SEPostgreSQL should only give enough rope that you can not
>>>      craft rules that produce unexpected behavior from a SQL point
>>>      of view; and that it would be bad if one can produce SEPostgres
>>>      policies that produce unexpected SQL behavior.
>>
>> With my other hat on (the red one) what I'm concerned about is whether
>> this patch will ever produce a feature that I could turn on in the
>> standard Red Hat/Fedora build of Postgres.  Right at the moment it seems
>> that the potential performance hit, for users who are *not using*
>> SEPostgres but merely have to use a build in which it is present,
>> might be bad enough to guarantee that that will never happen.
>>
> 
> According to the comments in security/sepgsql/core.c:
> 
> 
> /*
>  * sepgsqlIsEnabled
>  *
>  * This function returns the state of SE-PostgreSQL when PGACE hooks
>  * are invoked, to prevent to call sepgsqlXXXX() functions when
>  * SE-PostgreSQL is disabled.
>  *
>  * We can config the state of SE-PostgreSQL in $PGDATA/postgresql.conf.
>  * The GUC option "sepostgresql" can have the following four parameter.
>  *
>  * - default    : It always follows the in-kernel SELinux state. When it
>  *                works in Enforcing mode, SE-PostgreSQL also works in
>  *                Enforcing mode. Changes of in-kernel state are delivered
>  *                to userspace SE-PostgreSQL soon, and SELinux state
>  *                monitoring process updates it rapidly.
>  * - enforcing  : It always works in Enforcing mode. In-kernel SELinux
>  *                has to be enabled.
>  * - permissive : It always works in Permissive mode. In-kernel SELinux
>  *                has to be enabled.
>  * - disabled   : It disables SE-PostgreSQL feature. It works as if
>  *                original PostgreSQL
>  */
> 
> 
> and in the hooks there is a pgace_feature that turns off the checks:
> 
> void
> pgaceGramAlterRelation(Relation rel, HeapTuple tuple, DefElem *defel)
> {
>         switch (pgace_feature)
>         {
> #ifdef HAVE_SELINUX
>         case PGACE_FEATURE_SELINUX:
>                 if (sepgsqlIsEnabled())
>                 {
>                         sepgsqlGramAlterRelation(rel, tuple, defel);
>                         return;
>                 }
>                 break;
> #endif
>         default:
>                 break;
>         }
> 
>         if (defel)
>                 ereport(ERROR,
>                                 (errcode(ERRCODE_PGACE_ERROR),
>                                  errmsg("unable to set security 
> attribute of table "
>                                                 "via ALTER TABLE")));
> }
> 
> 
> So the pgace_feature turns off the backend call, there is an extra 
> function call, and a branch but that shouldn't cause the kind of 
> performance degradation you are talking about.

This hook is only available when an enhanced security feature is
enabled, so I injected ereport() at the tail.
(It is invoked on ALTER TABLE ... SECURITY_LABEL = '...';)

However, most of hooks do nothing or don't change existing behavior
when enhanced security is disabled.

So, I can't understand why it gives adverse affects in performances.

I can admit it needs additional steps to invoke empty functions at least.
However, using "static inline" was arguable in the previous discussion
due to the GCC dependency.

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