Re: Proposal of SE-PostgreSQL patches [try#2] - Mailing list pgsql-hackers

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@anoncvs.postgresql.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@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@kaigai.gr.jp>