Re: SE-PgSQL patch review

Bruce Momjian wrote:
> KaiGai Kohei wrote:
>> In summary, it was similar approach with what I already proposed in the CF#2,
>> but rejected.
>>
>> During the first commit-fest of v8.5 development cycle, Stephen Frost suggested
>> to rework the default PG's access controls to host other optional security
>> features, not only the default one.
>> Then, I submitted a large patch titled as "Reworks for Access Controls",
>> but it contained 3.5KL of changeset on the core routines, and 4KL of new codes
>> into "src/backend/security/*" except for documentations and testcases.
>> Then, this approach was rejected (not "returned with feedback") due to the scale
>> and complexity.
>>
>> After the fest, we discussed the direction to implement SE-PgSQL.
>> Basically, it needs to keep the changeset small, and the rest of features (such
>> as row-level granurality, access control decision cache, ...) shoule be added
>> step-by-step consistently, according to the suggestion in the v8.4 development
>> cycle. Heikki Linnakangas also suggested we need developer documentation which
>> introduces SE-PgSQL compliant permission checks and specification of security
>> hooks, after the reworks are rejected.
>>
>> So, I boldly removed most of the features from SE-PgSQL except for its core
>> functionalities, and added developer documentation (README) and widespread
>> source code comments to introduce the implementations instead.
>> In the result, the current proposal is near to naked one.
>> - No access controls except for database, schema, table and column.
>> - No row-level granularity in access controls.
>> - No access control decision chache.
>> - No security OID within HeapTupleHeader.
>>
>> I believe the current patch is designed according to the past suggestions.
>
> Agreed. The patch is exactly what I was hoping to see:
>
> o only covers existing Postgres ACLs
> o has both user and developer documentation
> o includes regression tests
> o main code impact is minimal
>
> Now, if this is applied, we might then move forward with implementing
> SE-Linux specific features like mandatory access control (MAC) and
> row-level security.
>
> In terms of review, the patch is 13k lines, but most of that is
> documentation, se-linux-specific files, system catalog adjustments, and
> regression tests.
>
> Also, I attended KaiGai's talk in Tokyo where he explained that managing
> permission at the operating system level, the web server level (via
> .htaccess and htpasswd), and at the database level is confusing, and
> having a single permission system has benefits.
>
> The number of revisions and adjustments KaiGai has done since the
> original SE-PostgreSQL patch is amazing and certainly gives me
> confidence that he will be around to help in case there are any problems
> in the future.
>
> So, one big problem is that no one has agreed to review it, partly or
> probably because few developers understand the SE-Linux API, and many
> people who have used SE-Linux have been confused by it.
>
> I think I could review this if I could team up with someone to help me,
> ideally someone on instant message (IM) and perhaps using SE-Linux.

I think some of SELinux developers (including me) can help you to review
the code. They are maintaining both of the kernel and libraries (APIs).
I'll call for them to help reviewing.

BTW, I can use skype messanger in my home, and IRC may be available in office.

> I think the big question is whether this feature (mappming SE-Linux
> permissions to existing Postgres permissions) has an acceptable code
> impact. Of course we might be adding things later, but at this stage is
> this something we can apply?

It needs to deploy a set of hooks on the strategic points of the core
PostgreSQL codes, and rest of steps (such as computing a default security
context and making an access control decision) are done in the SE-PgSQL
specific files.
These hooks are designed not to prevent anything when SE-PgSQL is disabled.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>