Re: SE-PgSQL patch review

From: KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>
To: David Fetter <david(at)fetter(dot)org>
Cc: Bruce Momjian <bruce(at)momjian(dot)us>, Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: SE-PgSQL patch review
Date: 2009-12-01 05:27:07
Message-ID: 4B14A92B.6010808@ak.jp.nec.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

David Fetter wrote:
> On Mon, Nov 30, 2009 at 11:03:08PM -0500, 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
>
> This patch addresses points 1-3 of Andrew Sullivan's post here:
> http://archives.postgresql.org/pgsql-hackers/2008-10/msg00388.php
>
> Left out is point 4, namely whether it's possible to map metadata
> access controls can do this completely, and if so, whether this patch
> implements such a mapping.

I'm not clear what means the metadata level access controls here.

If you talk about permissions when we create/alter/drop a certain database
obejct, the existing PG model also checks its permission. These operations
are equivalent to modify metadata of database objects.
And, SE-PgSQL also checks its permissions on modification of metadata.

We can refer the metadata of the database object using SELECT on the system
catalogs typically. In this case, we need row-level granularity because
individual tuples mean metadata of the database object. So, we don't implement
permission checks of references to metadata in this version.

I introduced it on the README file as follows:

| o getattr (not implemented yet)
|
| It is checked when a client read properties of database object.
|
| Typically, it is required on scanning database objects within system catalogs,
| such as "SELECT * FROM pg_class". Because this check requires row-level access
| control facilities, it is not implemented yet in this version.
|
| Note that no need to check this permission, unless fetched properties are
| consumed internally without returning to clients. For example, catcache stuff
| provides system internal stuff an easy way to refer system catalog. It is used
| to implement backend routines, and fetched properties are not returned to the
| client in normal way, so we don't check this permission here.

> This is totally separate from the really important question of whether
> SE-Linux has a future, and another about whether, if SE-Linux has a
> future, PostgreSQL needs to go there.
>
> All that aside, there is an excellent economic reason why a
> proprietary product might need to follow a dead-end technology, namely
> increasing shareholder value due to one or more large, long-term
> deals.
>
> PostgreSQL doesn't have this motive, although some of the proprietary
> forks may well.
>
> Can we see about Andrew Sullivan's point 4? Or is it more important
> to address the "do we want to" question first?
>
> Whatever order we take them in, we do need to address both.

As Bruce mentioned, our big motivation is improvement of web-system's
security. However, most of security burden tend to concentrate to the
quality of web applications, because it tends to share privileges on
OS/DB for all the user's requests.
In other word, correctness of the system depends on whether every
applications are bug-free, or not. But we know it is difficult to
keep them bug-free. :(

Nowadays, we can often see a web server which host multiple tenants
in a single daemon. For example, one is assigned to the division-X,
and the other is assigned to the division-Y, ...
In this case, it is worthful to prevent a (buggy) web-application to
access resources labeled as an other division independent from the
way to store information assets (either filesystem or database).

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Allan Morris Caras 2009-12-01 06:29:52 Recover deleted records using
Previous Message KaiGai Kohei 2009-12-01 04:37:36 Re: SE-PgSQL patch review