Updates of SE-PostgreSQL 8.4devel patches (r1704)

As I promised last week, SE-PostgreSQL patches are revised here:

[1/5] http://sepgsql.googlecode.com/files/sepgsql-core-8.4devel-r1704.patch
[2/5] http://sepgsql.googlecode.com/files/sepgsql-utils-8.4devel-r1704.patch
[3/5] http://sepgsql.googlecode.com/files/sepgsql-policy-8.4devel-r1704.patch
[4/5] http://sepgsql.googlecode.com/files/sepgsql-docs-8.4devel-r1704.patch
[5/5] http://sepgsql.googlecode.com/files/sepgsql-tests-8.4devel-r1704.patch

* List of updates:
- It is rebased to the latest CVS HEAD.

- The two reader permission ("select" and "use") are integrated into
a single one ("select") as the original design did two year's ago.
(It also enables to pick up read columns from rte->selectedCols.)

- The 'walker' code in sepgsql/checker.c is removed.
In the previous revision, SE-PostgreSQL walked on the given query
tree to pick up all the appeared tables/columns. The reason why
we needed separated walker phase is SE-PostgreSQL wanted to apply
two kind of "reader" permissions, but these are integrated into one.
(In addition, column-level privileges are not available when I
started to develop SE-PostgreSQL. :-))
In the currect version, SE-PostgreSQL knows what tables/columns
are appeared in the given query, from relid, selectedCols and
modifiedCols in RangeTblEntry. Then, it makes access controls
decision communicating with in-kernel SELinux.
After the existing DAC are checked, SE-PostgreSQL also checks
client's privileges on the appeared tables/columns as DAC doing.
Required privilges are follows these rules:
* If ACL_SELECT is set on rte->requiredPerms, client need to have
db_table:{select} and db_column:{select} for the tables/columns.
* If ACL_INSERT is set on rte->requiredPerms, client need to have
db_table:{insert} and db_column:{insert} for the tables/columns.
* If ACL_UPDATE is set on rte->requiredPerms, client need to have
db_table:{update} and db_column:{update} for the tables/columns.
* If ACL_DELETE is set on rte->requiredPerms, client need to have
db_table:{delete} for the tables.
This design change gives us a great benefit in code maintenance.
A trade-off is hardwired rules to modify "pg_rewrite" system catalog.
The correctness access controls depends on this catalog is protected
from unexpected manipulation by hand, because it stores a parsed
query tree of views.

- T_SelinuxItem is removed from include/node/nodes.h, and the codes
related to the node type is eliminated from copyfuncs.c ant others.
It was used to store all appeared tables/columns in the walker phase,
but now it is unnecessary.

- Several functions are moved to appropriate files:
- The codes related to permission bits are consolidated to
'sepgsql/perms.c' as its filename means.
(It was placed at 'avc.c' due to historical reason.)
- A few hooks (such as sepgsqlHeapTupleInsert) are moved to 'checker.c',
and rest of simple hooks are kept in 'hooks.c'.

- The scale of patches become a bit slim more.
<rev.1668> <rev.1704>
security/Makefile | 11 -> | 11
security/sepgsql/Makefile | 16 -> | 16
security/sepgsql/avc.c | 1157 +++++++ -> | 837 +++++
security/sepgsql/checker.c | 902 +++++ -> | 538 +++
security/sepgsql/core.c | 235 + -> | 247 +
security/sepgsql/dummy.c | 37 -> | 43
security/sepgsql/hooks.c | 748 ++++ -> | 621 +++
security/sepgsql/label.c | 360 ++ -> | 360 ++
security/sepgsql/perms.c | 295 + -> | 400 ++

[kaigai(at)saba ~]$ diffstat sepgsql-core-8.4devel-r1668.patch
:
64 files changed, 4770 insertions(+), 11 deletions(-), 4947 modifications(!)

[kaigai(at)saba ~]$ diffstat sepgsql-core-8.4devel-r1704.patch
:
60 files changed, 4048 insertions(+), 11 deletions(-), 4944 modifications(!)

About 700 lines can be reduced in total!

I believe this revision can reduce the burden of reviewers.
Please any comments!

* An issue:
I found a new issue in this revision.

- ACL_SELECT_FOR_UPDATE and ACL_UPDATE have identical value.

In this version, SE-PostgreSQL knows what permission should be checked
via RangeTblEntry::requiredPerms, and it applies its access control
policy with translating them into SELinux's permissions.

But we have a trouble in the following query.
----------------
[kaigai(at)saba ~]$ psql postgres
psql (8.4devel)
Type "help" for help.

postgres=# CREATE TABLE t1 (a int, b text)
postgres-# SECURITY_LABEL = 'system_u:object_r:sepgsql_ro_table_t:s0';
CREATE TABLE
-- NOTE: "sepgsql_ro_table_t" means read-only table from unpriv clients.
postgres=# INSERT INTO t1 VALUES (1, 'aaa'), (2, 'bbb');
INSERT 0 2
postgres=# \q
[kaigai(at)saba ~]$ runcon -t sepgsql_test_t -- psql postgres
psql (8.4devel)
Type "help" for help.
-- NOTE: "sepgsql_test_t" means unpriv client.
postgres=# SELECT * FROM t1;
a | b
---+-----
1 | aaa
2 | bbb
(2 rows)

postgres=# SELECT * FROM t1 FOR SHARE OF t1;
ERROR: SELinux: denied { update } scontext=unconfined_u:unconfined_r:sepgsql_test_t:s0-s0:c0.c63 tcontext=system_u:object_r:sepgsql_ro_table_t:s0 tclass=db_table name=t1
----------------

It raises an error with the client does not have db_table:{update} permission
on the table labeled as "sepgsql_ro_table_t", although this query does not
modify the table: "t1".

Our desirable behavior is SE-PostgreSQL checks db_table:{select lock} and
db_column:{select} permissions for the tables/columns.
(*) db_table:{lock} means permission to lock a table explicitly.

The reason why the ACL_UPDATE is set on RangeTblEntry::requiredPerms is
ACL_SELECT_FOR_UPDATE is defined as same value with ACL_UPDATE, so we
cannot distinguish which permission is required, or both of them.

I want these permissions have different values without any impacts
for user interfaces. One possible idea is ACL_UPDATE_CHR ('w') to be
extracted into (ACL_UPDATE | ACL_SELECT_FOR_UPDATE) bits, not a single
bit for each character, and it is granted by the token of "update" in
GRANT/REVOKE statement.

Basically, I don't want go back to the "walker" approach.
The ACL_SELECT_FOR_UPDATE without identical value is much desirable
for me. What is your opinion?

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