Re: Updates of SE-PostgreSQL 8.4devel patches (r1168)

Bruce Momjian wrote:
> KaiGai Kohei wrote:
>>> I just looked over the patch. This new version with row-level SQL
>>> security has certainly reduced the SE-Linux-specific part, which is
>>> good.
>>>
>>> It was interesting how you implemented SQL-level column-level
>>> permissions:
>>>
>>> CREATE TABLE customer (
>>> cid integer primary key,
>>> cname varchar(32),
>>> credit varchar(32) SECURITY_CONTEXT = 'system_u:object_r:sepgsql_secret_table_t'
>>> );
>>>
>>> I am unclear how that will behave with the column-level permissions
>>> patch someone is working on. I am wondering if your approach is clearer
>>> than the other patch because it gives a consistent right policy for rows
>>> and columns.
>> The column-level permissions in SE-PostgreSQL works independently and
>> orthogonally from the upcoming column-level permissions by Stephen Frost.
>> When the SE-PostgreSQL is enabled, both of facilities have to allow the
>> client to access required columns.
>>
>> In the above case, the "credit" column has "sepgsql_secret_table_t" type,
>> but rest of columns inherits the type of "customer" table which allows
>> non-administrative users to access in the default security policy.
>> If the given query contains the "credit" column, SE-PostgreSQL checks
>> privileges of client to access columns labeled as "sepgsql_secret_table_t",
>> then it raises an error to abort the current transaction if the security
>> policy does not allow it.
>>
>> There is a possibility that column-level ACLs are set via newer GRANT/REVOKE
>> statement. In this case, the core PostgreSQL checks them, and raises an error
>> if violated.
>
> OK. I am wondering if we _want_ two ways to set column permisions,
> especially since I think there will be only one way to set row-level
> permissions.

I think we should not see the feature from only the viewpoint of granularity
in access controls. The both of new security features (sepgsql and rowacl)
are enhanced security features, but the Stephen's efforts is one of the core
features based on SQL-standard and enabled in the default.
Please pay mention that any given queries have to be checked by the core
facility, and can be checked by the enhanced one if enabled.

The PGACE security framework enables us to implement various kind of enhanced
security features, and has two guest facilities now. They can have its own
security model and granularities as a part of its design.
The one has its granularities with some of overlaps on tables/columns/functions,
and the other also has its granularity without overlaps because its purpose is
supplement of the core security facilities.

So, it is not a strange design there is only one way to set row-level permissions,
because the current SQL-standard does not have its specifications and no core
facilities are here.
If the future version of PostgreSQL got a newer row-level permissions defined
within SQL-standard, I think there should be two ways to set row-level ones
for both of the core and enhanced.

>>> I was wondering why you mention the NSA (U.S. National Security Agency)
>>> in the patch?
>>>
>>> +# NSA SELinux support
>> The original author of SELinux is NSA.
>> There is no more meanings than a caption of the option.
>> I'll fix it, if necessary.
>
> Yes, please remove; the "NSA" suggests to me that this is an NSA-only
> feature, which it is not; it was just originally designed for them.

OK, I modified the caption from "NSA SELinux" to "SELinux".

http://code.google.com/p/sepgsql/source/detail?r=1170

>>> The size of the patch is still larger but I don't see any way to reduce it:
>>>
>>> 1275 sepostgresql-docs-8.4devel-3-r1168.patch
>>> 625 sepostgresql-pg_dump-8.4devel-3-r1168.patch
>>> 829 sepostgresql-policy-8.4devel-3-r1168.patch
>>> 1736 sepostgresql-row_acl-8.4devel-3-r1168.patch
>>> 10847 sepostgresql-sepgsql-8.4devel-3-r1168.patch
>>> 1567 sepostgresql-tests-8.4devel-3-r1168.patch
>>> 16879 total
>> I thought the "sepostgresql-docs" can be replaced by the pointing to the wiki
>> page, how do you think the idea?
>
> No, I docs for using the tarball should be in the main documentation,
> even if they are not compile-enabled by default. The new patch affects
> the main Postgres backend code much less, which is a great improvement.

OK, I could understand it.

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