Re: Proposal of SE-PostgreSQL patches [try#2]

Hi,

The following patch set (r926) are updated one toward the latest CVS head,
and contains some fixes in security policy and documentation.

I want to push them for the reviewing queue of CommitFest:Jul.

[1/4] Core facilities of PGACE/SE-PostgreSQL
http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r926.patch

[2/4] "--enable-selinux" option of pg_dump/pg_dumpall
http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r926.patch

[3/4] Default security policy for SE-PostgreSQL
http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r926.patch

[4/4] Documentation updates
http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r926.patch

Thanks,

KaiGai Kohei wrote:
> Hi,
>
> The following patch set is our second proposals of SE-PostgreSQL.
>
> It contains many of fixes and improvements from the previous version.
> Please add them a reviwing queue of the next commit fest.
>
> Thanks,
>
> List of Patches
> ===============
>
> [1/4] Core facilities of PGACE/SE-PostgreSQL
> http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r914.patch
>
> [2/4] "--enable-selinux" option of pg_dump/pg_dumpall
> http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r914.patch
>
> [3/4] Default security policy for SE-PostgreSQL
> http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r914.patch
>
> [4/4] Documentation updates
> http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r914.patch
>
> We can provide a quick overview for SE-PostgreSQL at:
> http://code.google.com/p/sepgsql/wiki/WhatIsSEPostgreSQL
> http://sepgsql.googlecode.com/files/PGCON20080523.pdf
>
> Compile and Installation
> ========================
>
> The following items are requirements of SE-PostgreSQL.
> - Fedora 8 or later system
> - SELinux is enabled, and working
> - kernel-2.6.23 or later
> - selinux-policy and selinux-policy-devel v3.0.8 or later
> - libselinux, policycoreutils, checkpolicy
>
> The followings are step by step installation.
>
> $ cvs -z3 -d :pserver:anoncvs(at)anoncvs(dot)postgresql(dot)org:/projects/cvsroot \
> export -r HEAD -d pgsql
> $ cd pgsql
> $ patch -p1 < sepostgresql-sepgsql-8.4devel-3-r914.patch
> $ patch -p1 < sepostgresql-pg_dump-8.4devel-3-r914.patch
> $ patch -p1 < sepostgresql-policy-8.4devel-3-r914.patch
> $ patch -p1 < sepostgresql-docs-8.4devel-3-r914.patch
> $ ./configure --enable-selinux
> $ make
> $ make -C ./contrib/sepgsql_policy
>
> $ su
> # /usr/sbin/semodule -i ./contrib/sepgsql_policy/sepostgresql.pp ... [1]
> # make install
> # /sbin/restorecon -R /usr/local/pgsql
>
> $ mkdir -p $PGDATA
> $ chcon -t postgresql_db_t -R $PGDATA
> $ initdb
> $ pg_ctl start
>
> [1] If "selinux-policy-3.4.2" or later is installed on your system,
> install "sepostgresql-devel.pp" instead.
> In this version, most of SE-PostgreSQL's policy are got mainlined.
>
>
> Updates from the previous version
> =================================
>
> o A new type of "security_label" has gone
>
> In the previous one, "security_context" system column is declared as
> security_label type. This type had its input handler, and it translated
> a given text representation into an internal Oid value with looking up
> pg_security system catalog. If it's not found, the input handler inserts
> a new entry automatically.
>
> The following query can show the reason why this design is problematic.
>
> SELECT 'system_u:object_r:sepgsql_db_t'::security_label;
>
> This query seems to us read-only, but it has a possibility to insert
> a new tuple into pg_security implicitly.
>
> In this version, "security_context" system column is re-defined as a TEXT
> type attribute, and a given text representation is translated into internal
> identifier (Oid) just before insert or update a tuple. This design change
> enables to make sure pg_security is not modified in read-only queries.
>
>
> o Query modification has gone.
>
> In the previous one, SE-PostgreSQL modified WHERE/JOIN ON clause to apply
> tuple-level access controls, but its implementation is a bit complicated.
> In addition, this design had a possibility to conflict with a new MS patent.
>
> Now we put a hook on ExecScan to apply tuple-level access controls.
> It enables to reduce code complexity and avoid patent conflicts.
>
>
> o Scanning with SnapshotSelf has gone.
>
> In the previous one, we had to scan some system catalogs with SnapshotSelf
> mode at three points (pg_class, pg_largeobject and pg_security).
>
> * When we defines a relation on heap_create_with_catalog(), a tuple of
> pg_class and several tuples of pg_attribute are inserted within same
> command id.
> A tuple of pg_class has to be refered just before inserting tuples of
> pg_attribute, because a new column inherits the security context of its
> parent relation in the default. But we cannot find it because these are
> inserted within same command id and SnapshotNow scanning mode ignores
> these tuples. We cannot invoke CommandIdIncrement() here, because it
> finally checks integrity of relation cache, but the relation is not
> constructed yet.
>
> We can apply two option here. One is preserving the security context
> of parent table and applying it later without looking up pg_class.
> The other is to insert a temporary entry into SysCache until it is
> invalidated.
> The later approach can also be applied on the next situation, so we
> now put InsertSysCache() withing heap_create_with_catalog() to refer
> the new tuple before next CommandIdIncrement() which is invoked just
> after hecp_create_with_catalog().
>
> * When a user gives a security context in text representation, it is
> translated into an internal identifier which indicates the oid of
> pg_security system catalog. If it was not found, PGACE/SE-PostgreSQL
> inserts a new tuple and applies its oid as an internal identifier.
> If a same new security context is given within same currentCommandId
> twice or more, it tries to insert a new tuple into pg_security twice
> or more. However, it violates uniqueness constraint at oid of pg_security.
>
> Thus, we had to look up pg_security with SnapshotSelf scanning mode
> as a fallback when SearchSysCache() returns invalid tuple. But we can
> apply same approach here. So, InsertSysCache() is invoked to keep
> a newly inserted security context until next CommandIdIncrement().
>
> * When a user inserts or deletes a tuple within pg_largeobject directly,
> it can also means create a new larageobject, or drop ones.
> In SE-PostgreSQL model, it requires 'create' or 'drop' permission,
> so we had to check whether the tuple is the first/last one, or not.
>
> In this case, we assumes inserting a tuple into pg_largeobject directly
> has a possibility to create new largeobject, and 'create' permission
> should be always evaluated, not only 'write'.
> This assumption kills to scan pg_largeobject for each insertion/deletion.
>
> If requests come from lowrite() or lo_create(), we can distinguish its
> meanings, so proper permissions are applied in the most cases.
>
> I guess the InsertSysCache() will be an arguable interface, but it can resolve
> our problem with minimum impact and utilizing existing facilities, so it is
> better than any other solutuions.
>
>
> o A new guc parameter to enable/disable SE-PostgreSQL
>
> It can take four options, as follows:
> sepostgresql = [ default | enforcing | permissive | disabled ]
> - default: always follows kernel setting (default)
> - enforcing: works in enforcing mode (MAC and audit enabled).
> - permissive: works in permissive mode (audit log only).
> - disabled: disables SE-PostgreSQL feature.
>
>
> o PGACE hooks are inlined
>
> The contains of src/backend/security/pgaceHooks.c are moved to
> src/include/security/pgace.h and inlined.
> It enables to reduce cost to invoke empty function when this
> feature is disabled.
>
>
> o Generic writable system column
>
> SystemAttributeIsWritable() can abstract what system attribute is writable.
> (Currently, the security system catalog is the only one writable.)
> If it returns true on the target of INSERT, UPDATE or SELECT INTO, these
> TargetEntries are marked as junk, and we can fetch its value on ExecutorRun()
> separated from any other regular attribute.
>
>
> o early security design
>
> In the previous one, we stores a relationship between security id and
> text representation on bootstraping mode, because pg_security system
> catalog is not constructed yet in this time.
> The current version holds them in-memory cache and writes out on the tail
> of the bootstraping process.
>
>
> o Documentation updates
>
> The doc/src/sgml/security.sgml gives us a short description of SE-PostgreSQL
> and PGACE security framework. In addition, we added source code comments for
> most of functions, including PGACE hooks.
>
>
> o Miscellaneous updates
> * Two separated patches (pgace and sepgsql) are integrated into one.
> * Copyrights are changed to "PostgreSQL Global Development Group".
> * Several PGACE hooks are added, redefined and removed.
> * Now, we can run regression test without any problem, except for two
> tests which can be explained reasonably.
> * SELinux state monitoring process is implemented using an existing
> facilities provided by postmaster.c.
> * Coding styles are fixed.
> * A definition of LWlock is moved to storage/lwlock.h
> * Definitions of SEvalItemXXXX are moved to nodes/nodes.h
> * Compiler warnings come from SE-PostgreSQL are killed.
> * Some error codes are added within 'SE' class, and elog()s which can
> report user facing messages are replaced by ereport().
> * Shell function is removed from genbki.sh.
> * Default security context of files consider --prefix setting in
> configure script.
>
>
> Regression Tests
> ================
>
> Now we remain two test fails, but these can be explained reasonably.
>
> The first fail (create_function_1) means that SE-PostgreSQL detects
> a client attempt to load an invalid file before core PostgreSQL doing,
> and generates its error message.
>
> The later one (sanity_check) means the regression test can detect
> an increation of system catalogs correctly.
>
> (*) Turn on "sepgsql_regression_test_mode" boolean of SELinux before
> regression test. It enables you to load shared library modules
> installed under user's home directory.
>
> # /usr/sbin/setsebool sepgsql_regression_test_mode on
>
> $ make check
> :
> (snip)
> :
> ========================
> 2 of 115 tests failed.
> ========================
>
>
> [kaigai(at)saba pgsql]$ less src/test/regress/regression.diffs
> *** ./expected/create_function_1.out Fri Jun 20 14:55:12 2008
> --- ./results/create_function_1.out Fri Jun 20 14:55:28 2008
> ***************
> *** 72,78 ****
> ERROR: only one AS item needed for language "sql"
> CREATE FUNCTION test1 (int) RETURNS int LANGUAGE C
> AS 'nosuchfile';
> ! ERROR: could not access file "nosuchfile": No such file or directory
> CREATE FUNCTION test1 (int) RETURNS int LANGUAGE C
> AS '/home/kaigai/tmp/e/pgsql/src/test/regress/regress.so', 'nosuchsymbol';
> ERROR: could not find function "nosuchsymbol" in file "/home/kaigai/tmp/e/pgsql/src/test/regress/regress.so
> "
> --- 72,78 ----
> ERROR: only one AS item needed for language "sql"
> CREATE FUNCTION test1 (int) RETURNS int LANGUAGE C
> AS 'nosuchfile';
> ! ERROR: SELinux: could not get context of nosuchfile
> CREATE FUNCTION test1 (int) RETURNS int LANGUAGE C
> AS '/home/kaigai/tmp/e/pgsql/src/test/regress/regress.so', 'nosuchsymbol';
> ERROR: could not find function "nosuchsymbol" in file "/home/kaigai/tmp/e/pgsql/src/test/regress/regress.so
> "
>
> ======================================================================
>
> *** ./expected/sanity_check.out Sun Nov 25 12:49:12 2007
> --- ./results/sanity_check.out Fri Jun 20 14:55:31 2008
> ***************
> *** 111,116 ****
> --- 111,117 ----
> pg_pltemplate | t
> pg_proc | t
> pg_rewrite | t
> + pg_security | t
> pg_shdepend | t
> pg_shdescription | t
> pg_statistic | t
> ***************
> *** 149,155 ****
> timetz_tbl | f
> tinterval_tbl | f
> varchar_tbl | f
> ! (138 rows)
>
> --
> -- another sanity check: every system catalog that has OIDs should have
> --- 150,156 ----
> timetz_tbl | f
> tinterval_tbl | f
> varchar_tbl | f
> ! (139 rows)
>
> --
> -- another sanity check: every system catalog that has OIDs should have
>

--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>