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@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>

Am Donnerstag, 26. Juni 2008 schrieb KaiGai Kohei:

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

OK, I have quickly read through these patches. They look very nice, so I am
optimistic we can get through this.

First of all, now would be a good time if someone out there really wants to
object to this feature in general. It will probably always be a niche
feature. But all the code is hidden behind ifdefs or other constructs that a
compiler can easily hide away (or we can make it so, at least).

Here is a presentation from PGCon on SE-PostgreSQL:
http://www.pgcon.org/2008/schedule/events/77.en.html

Are there any comments yet from the (Trusted)Solaris people that wanted to
evaluate this approach for compatibility with their approach?

In general, are we OK with the syntax CONTEXT = '...'? I would rather see
something like SECURITY CONTEXT '...'. There are a lot of contexts, after
all.

This will also add a system column called security_context. I think that is
OK.

In the pg_dump patch:

spelling mistake "tuen on/off"

Evil coding style: if (strcmp(SELINUX_SYSATTR_NAME, security_sysattr_name)) --
compare the result with 0 please.

The above code appears to assume that security_sysattr_name never changes, but
then why do we need a GUC parameter to show it?

Might want to change the option name --enable-selinux to something
like --security-context.

In general, we might want to not name things selinux_* but instead
sepostgresql_* or security_* or security_context_*. Or maybe PGACE?

On the default policy:

Should this really be a contrib module? Considering that it would be a core
feature that is not really usable without a policy.

Please change all the sepgsql_* things to sepostgresql_*, considering that you
are using both already, so we shouldn't have both forms of names.

Documentation:

Looks good for a start, but we will probably want to write more later.

Peter,

Are there any comments yet from the (Trusted)Solaris people that wanted to
evaluate this approach for compatibility with their approach?

In case they're not paying attention, a month ago the Trusted Solaris
folks said the general approach was fine, but that they would likely
want to extend SEPostgres further (that is, add to it) for 8.5.

--Josh

Thanks for your reviewing.

Peter Eisentraut wrote:

Am Donnerstag, 26. Juni 2008 schrieb KaiGai Kohei:

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

OK, I have quickly read through these patches. They look very nice, so I am
optimistic we can get through this.

First of all, now would be a good time if someone out there really wants to
object to this feature in general. It will probably always be a niche
feature. But all the code is hidden behind ifdefs or other constructs that a
compiler can easily hide away (or we can make it so, at least).

Here is a presentation from PGCon on SE-PostgreSQL:
http://www.pgcon.org/2008/schedule/events/77.en.html

Are there any comments yet from the (Trusted)Solaris people that wanted to
evaluate this approach for compatibility with their approach?

In this April, I had a face-to-face meeting with Trusted Solaris people
to discuss SE-PostgreSQL and PGACE framework, and concluded that PGACE
framework provides enough facilities for both operating systems.

I modified several hooks from CommitFest:May, however, its fundamental
structures are unchanged.

In general, are we OK with the syntax CONTEXT = '...'? I would rather see
something like SECURITY CONTEXT '...'. There are a lot of contexts, after
all.

If we change it, I prefer SECURITY_CONTEXT = '...' style, because it enables
to represent the left hand with a single token and make PGACE hooks simpler.
I also agree the CONTEXT has widespread meanings and to be changed here.

This will also add a system column called security_context. I think that is
OK.

Thanks,

In the pg_dump patch:

spelling mistake "tuen on/off"

I'll fix it soon.

Evil coding style: if (strcmp(SELINUX_SYSATTR_NAME, security_sysattr_name)) --
compare the result with 0 please.

OK, I'll fix it and check my implementations in other place.

The above code appears to assume that security_sysattr_name never changes, but
then why do we need a GUC parameter to show it?

The purpose of the GUC parameter is to identify the kind of security feature
if enabled. It can be changed by other security features, and it will show us
different value.

Might want to change the option name --enable-selinux to something
like --security-context.

In general, we might want to not name things selinux_* but instead
sepostgresql_* or security_* or security_context_*. Or maybe PGACE?

The pgace_* scheme is an attractive idea, although the server process
has to provide a bit more hints (like the name of security system column
and the kind of objects exported with security attribute) pg_dump to
support various kind of security features with smallest implementation.

If not so, I prefer the combination of --security-context and sepostgresql_*.

On the default policy:

Should this really be a contrib module? Considering that it would be a core
feature that is not really usable without a policy.

It is correct, SE-PostgreSQL feature always need its security policy.
Do you think "src/backend/security/sepgsql/policy" is better?

Please change all the sepgsql_* things to sepostgresql_*, considering that you
are using both already, so we shouldn't have both forms of names.

We can convert them from sepostgresql_* to sepgsql_* easily, because the longer
forms are not used as a part of identifier in security context.
But we have a possible matter in changing from sepgsql_* to sepostgresql_*.

Here is a news from SELinux community:
http://marc.info/?l=selinux&amp;m=121501336024075&amp;w=2

It shows most part of the SE-PostgreSQL policy module got merged into
the upstreamed at selinux-policy-3.4.2 or later. It contains identifier
with sepgsql_* stuff, so it has a possible matter when users upgrade
his security policy.

If their database is labeled as sepostgresql_*, it will lose rules
defined in the policy when users upgrade selinux-policy package to
the latest one.

Documentation:

Looks good for a start, but we will probably want to write more later.

Do you think what kind of information should be added?
I intentionally omitted descriptions about SELinux itself,
because it is a documentation of PostgreSQL, not OS.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

KaiGai Kohei wrote:

Might want to change the option name --enable-selinux to something
like --security-context.

In general, we might want to not name things selinux_* but instead
sepostgresql_* or security_* or security_context_*. Or maybe PGACE?

The pgace_* scheme is an attractive idea, although the server process
has to provide a bit more hints (like the name of security system column
and the kind of objects exported with security attribute) pg_dump to
support various kind of security features with smallest implementation.

It might not be necessary to provide all the hints pg_dump to make queries.
The minimum required information is which security feature is running on
the server process, or nothing. And, pg_dump can add a security system
column within its queries to get security attribute, if required.

Now, I'm considering to add pgaceDumpSOMETHING() functions within pg_dump
for better modularity. What do you think?

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

All,

The pgace_* scheme is an attractive idea, although the server process
has to provide a bit more hints (like the name of security system column
and the kind of objects exported with security attribute) pg_dump to
support various kind of security features with smallest implementation.

If we have a choice, it should be pg_ace_*. We've told developers that they
can expect system stuff to be named pg_* ; let's stick to that whenever we
can.

--
Josh Berkus
PostgreSQL @ Sun
San Francisco

Hi,

I updated the series of patches, as follows:

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

[2/4] "--security-context" option of pg_dump/pg_dumpall
http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r953.patch

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

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

List of updates:
* "--enable-selinux" option of pg_dump/pg_dumpall are replaced
by "--security-context" option.
* A new GUC parameter of "pgace_security_feature" is added to show
what feature is worked on PGACE security framework.
* pg_ace_dumpXXXX() hooks are added to src/bin/pg_dump/pg_ace_dump.h
to abstract security attribute dumping effort.
* An extended syntax of CONTEXT = '...' is replaced by
SECURITY_CONTEXT = '...'.
* The sources of security policy module are moved from contrib/ to
src/backend/security/sepgsql/policy.
* The prefix of interfaces in the default security policy are changed
to sepgsql_*() from sepostgresql_*()
* Using integer value as a condition is replaced as follows:
if (!strcmp(..)) -> if (strcmp(...) == 0)
* Two potential bug fixes:
1. Unconditional Assert() in sepgsql_avc_reclaim().
2. relkind checks are lacked in sepgsqlSetDefaultContext().

The patch of core facilities is unchanged expect for the new GUC parameters
and above two minor bug fixes.

And I have a question. Is it legal to use a pointer value as a condition,
like `while (!pointer) ...' ?

Thanks for youe reviewing.

KaiGai Kohei wrote:

Thanks for your reviewing.

Peter Eisentraut wrote:

Am Donnerstag, 26. Juni 2008 schrieb KaiGai Kohei:

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

OK, I have quickly read through these patches. They look very nice,
so I am optimistic we can get through this.

First of all, now would be a good time if someone out there really
wants to object to this feature in general. It will probably always
be a niche feature. But all the code is hidden behind ifdefs or other
constructs that a compiler can easily hide away (or we can make it so,
at least).

Here is a presentation from PGCon on SE-PostgreSQL:
http://www.pgcon.org/2008/schedule/events/77.en.html

Are there any comments yet from the (Trusted)Solaris people that
wanted to evaluate this approach for compatibility with their approach?

In this April, I had a face-to-face meeting with Trusted Solaris people
to discuss SE-PostgreSQL and PGACE framework, and concluded that PGACE
framework provides enough facilities for both operating systems.

I modified several hooks from CommitFest:May, however, its fundamental
structures are unchanged.

In general, are we OK with the syntax CONTEXT = '...'? I would rather
see something like SECURITY CONTEXT '...'. There are a lot of
contexts, after all.

If we change it, I prefer SECURITY_CONTEXT = '...' style, because it
enables
to represent the left hand with a single token and make PGACE hooks
simpler.
I also agree the CONTEXT has widespread meanings and to be changed here.

This will also add a system column called security_context. I think
that is OK.

Thanks,

In the pg_dump patch:

spelling mistake "tuen on/off"

I'll fix it soon.

Evil coding style: if (strcmp(SELINUX_SYSATTR_NAME,
security_sysattr_name)) -- compare the result with 0 please.

OK, I'll fix it and check my implementations in other place.

The above code appears to assume that security_sysattr_name never
changes, but
then why do we need a GUC parameter to show it?

The purpose of the GUC parameter is to identify the kind of security
feature
if enabled. It can be changed by other security features, and it will
show us
different value.

Might want to change the option name --enable-selinux to something
like --security-context.

In general, we might want to not name things selinux_* but instead
sepostgresql_* or security_* or security_context_*. Or maybe PGACE?

The pgace_* scheme is an attractive idea, although the server process
has to provide a bit more hints (like the name of security system column
and the kind of objects exported with security attribute) pg_dump to
support various kind of security features with smallest implementation.

If not so, I prefer the combination of --security-context and
sepostgresql_*.

On the default policy:

Should this really be a contrib module? Considering that it would be
a core
feature that is not really usable without a policy.

It is correct, SE-PostgreSQL feature always need its security policy.
Do you think "src/backend/security/sepgsql/policy" is better?

Please change all the sepgsql_* things to sepostgresql_*, considering

that you

are using both already, so we shouldn't have both forms of names.

We can convert them from sepostgresql_* to sepgsql_* easily, because the
longer
forms are not used as a part of identifier in security context.
But we have a possible matter in changing from sepgsql_* to sepostgresql_*.

Here is a news from SELinux community:
http://marc.info/?l=selinux&amp;m=121501336024075&amp;w=2

It shows most part of the SE-PostgreSQL policy module got merged into
the upstreamed at selinux-policy-3.4.2 or later. It contains identifier
with sepgsql_* stuff, so it has a possible matter when users upgrade
his security policy.

If their database is labeled as sepostgresql_*, it will lose rules
defined in the policy when users upgrade selinux-policy package to
the latest one.

Documentation:

Looks good for a start, but we will probably want to write more later.

Do you think what kind of information should be added?
I intentionally omitted descriptions about SELinux itself,
because it is a documentation of PostgreSQL, not OS.

Thanks,

--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

At 2008-07-11 19:10:16 +0900, kaigai@ak.jp.nec.com wrote:

And I have a question. Is it legal to use a pointer value as a
condition, like `while (!pointer) ...' ?

Yes, it's fine.

-- ams

Hello,

The following patches are updated ones of SE-PostgreSQL toward the HEAD of CVS.

[1/4] http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r958.patch
[2/4] http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r958.patch
[3/4] http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r958.patch
[4/4] http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r958.patch

By the way,
http://wiki.postgresql.org/wiki/CommitFest:2008-07

| Peter says: checking with Solaris engineers about compatibility with
| Solaris TX; will continue review throughout August

Jon, do you have anything to comment about PGACE security framework?
(Show the src/include/security/pgace.h)

It has been reworked a bit from this April when we had an offline meeting,
but I think its impact is not significant for its portability.

It can provide its guest subsystem (like SE-PostgreSQL) a series of hooks to
make a decision and facilities to manage text represented security attribute
of database objects.
IIUC, we concluded these foundations are also enough to port SolarisTX feature.

If you find out something lacks/conflicts, please tell me.

Thanks,

KaiGai Kohei wrote:

Hi,

I updated the series of patches, as follows:

[1/4] Core facilities of PGACE/SE-PostgreSQL

http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r953.patch

[2/4] "--security-context" option of pg_dump/pg_dumpall

http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r953.patch

[3/4] Default security policy for SE-PostgreSQL

http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r953.patch

[4/4] Documentation updates

http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r953.patch

List of updates:
* "--enable-selinux" option of pg_dump/pg_dumpall are replaced
by "--security-context" option.
* A new GUC parameter of "pgace_security_feature" is added to show
what feature is worked on PGACE security framework.
* pg_ace_dumpXXXX() hooks are added to src/bin/pg_dump/pg_ace_dump.h
to abstract security attribute dumping effort.
* An extended syntax of CONTEXT = '...' is replaced by
SECURITY_CONTEXT = '...'.
* The sources of security policy module are moved from contrib/ to
src/backend/security/sepgsql/policy.
* The prefix of interfaces in the default security policy are changed
to sepgsql_*() from sepostgresql_*()
* Using integer value as a condition is replaced as follows:
if (!strcmp(..)) -> if (strcmp(...) == 0)
* Two potential bug fixes:
1. Unconditional Assert() in sepgsql_avc_reclaim().
2. relkind checks are lacked in sepgsqlSetDefaultContext().

The patch of core facilities is unchanged expect for the new GUC parameters
and above two minor bug fixes.

And I have a question. Is it legal to use a pointer value as a condition,
like `while (!pointer) ...' ?

Thanks for youe reviewing.

KaiGai Kohei wrote:

Thanks for your reviewing.

Peter Eisentraut wrote:

Am Donnerstag, 26. Juni 2008 schrieb KaiGai Kohei:

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

OK, I have quickly read through these patches. They look very nice,
so I am optimistic we can get through this.

First of all, now would be a good time if someone out there really
wants to object to this feature in general. It will probably always
be a niche feature. But all the code is hidden behind ifdefs or
other constructs that a compiler can easily hide away (or we can make
it so, at least).

Here is a presentation from PGCon on SE-PostgreSQL:
http://www.pgcon.org/2008/schedule/events/77.en.html

Are there any comments yet from the (Trusted)Solaris people that
wanted to evaluate this approach for compatibility with their approach?

In this April, I had a face-to-face meeting with Trusted Solaris people
to discuss SE-PostgreSQL and PGACE framework, and concluded that PGACE
framework provides enough facilities for both operating systems.

I modified several hooks from CommitFest:May, however, its fundamental
structures are unchanged.

In general, are we OK with the syntax CONTEXT = '...'? I would
rather see something like SECURITY CONTEXT '...'. There are a lot of
contexts, after all.

If we change it, I prefer SECURITY_CONTEXT = '...' style, because it
enables
to represent the left hand with a single token and make PGACE hooks
simpler.
I also agree the CONTEXT has widespread meanings and to be changed here.

This will also add a system column called security_context. I think
that is OK.

Thanks,

In the pg_dump patch:

spelling mistake "tuen on/off"

I'll fix it soon.

Evil coding style: if (strcmp(SELINUX_SYSATTR_NAME,
security_sysattr_name)) -- compare the result with 0 please.

OK, I'll fix it and check my implementations in other place.

The above code appears to assume that security_sysattr_name never
changes, but
then why do we need a GUC parameter to show it?

The purpose of the GUC parameter is to identify the kind of security
feature
if enabled. It can be changed by other security features, and it will
show us
different value.

Might want to change the option name --enable-selinux to something
like --security-context.

In general, we might want to not name things selinux_* but instead
sepostgresql_* or security_* or security_context_*. Or maybe PGACE?

The pgace_* scheme is an attractive idea, although the server process
has to provide a bit more hints (like the name of security system column
and the kind of objects exported with security attribute) pg_dump to
support various kind of security features with smallest implementation.

If not so, I prefer the combination of --security-context and
sepostgresql_*.

On the default policy:

Should this really be a contrib module? Considering that it would be
a core
feature that is not really usable without a policy.

It is correct, SE-PostgreSQL feature always need its security policy.
Do you think "src/backend/security/sepgsql/policy" is better?

Please change all the sepgsql_* things to sepostgresql_*,

considering that you

are using both already, so we shouldn't have both forms of names.

We can convert them from sepostgresql_* to sepgsql_* easily, because
the longer
forms are not used as a part of identifier in security context.
But we have a possible matter in changing from sepgsql_* to
sepostgresql_*.

Here is a news from SELinux community:
http://marc.info/?l=selinux&amp;m=121501336024075&amp;w=2

It shows most part of the SE-PostgreSQL policy module got merged into
the upstreamed at selinux-policy-3.4.2 or later. It contains identifier
with sepgsql_* stuff, so it has a possible matter when users upgrade
his security policy.

If their database is labeled as sepostgresql_*, it will lose rules
defined in the policy when users upgrade selinux-policy package to
the latest one.

Documentation:

Looks good for a start, but we will probably want to write more later.

Do you think what kind of information should be added?
I intentionally omitted descriptions about SELinux itself,
because it is a documentation of PostgreSQL, not OS.

Thanks,

--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

On the WiKi of CommitFest:Sep,
http://wiki.postgresql.org/wiki/CommitFest:2008-09

The entry of SE-PostgreSQL points a message when I submitted older version
of our patch set. But the latest ones are listed on another message.

Please add a link to the following message for our convenience:
http://archives.postgresql.org/pgsql-hackers/2008-07/msg01365.php

BTW, I can respond to reviewer's comments and update/rework our patches
on this August, although SE-PostgreSQL is moved to the CommitFest:Sep.
We welcome any early comments, if possible.

Thanks,

KaiGai Kohei wrote:

Hello,

The following patches are updated ones of SE-PostgreSQL toward the HEAD
of CVS.

[1/4]
http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r958.patch

[2/4]
http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r958.patch

[3/4]
http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r958.patch

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

By the way,
http://wiki.postgresql.org/wiki/CommitFest:2008-07

| Peter says: checking with Solaris engineers about compatibility with
| Solaris TX; will continue review throughout August

Jon, do you have anything to comment about PGACE security framework?
(Show the src/include/security/pgace.h)

It has been reworked a bit from this April when we had an offline meeting,
but I think its impact is not significant for its portability.

It can provide its guest subsystem (like SE-PostgreSQL) a series of
hooks to
make a decision and facilities to manage text represented security
attribute
of database objects.
IIUC, we concluded these foundations are also enough to port SolarisTX
feature.

If you find out something lacks/conflicts, please tell me.

Thanks,

KaiGai Kohei wrote:

Hi,

I updated the series of patches, as follows:

[1/4] Core facilities of PGACE/SE-PostgreSQL

http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r953.patch

[2/4] "--security-context" option of pg_dump/pg_dumpall

http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r953.patch

[3/4] Default security policy for SE-PostgreSQL

http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r953.patch

[4/4] Documentation updates

http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r953.patch

List of updates:
* "--enable-selinux" option of pg_dump/pg_dumpall are replaced
by "--security-context" option.
* A new GUC parameter of "pgace_security_feature" is added to show
what feature is worked on PGACE security framework.
* pg_ace_dumpXXXX() hooks are added to src/bin/pg_dump/pg_ace_dump.h
to abstract security attribute dumping effort.
* An extended syntax of CONTEXT = '...' is replaced by
SECURITY_CONTEXT = '...'.
* The sources of security policy module are moved from contrib/ to
src/backend/security/sepgsql/policy.
* The prefix of interfaces in the default security policy are changed
to sepgsql_*() from sepostgresql_*()
* Using integer value as a condition is replaced as follows:
if (!strcmp(..)) -> if (strcmp(...) == 0)
* Two potential bug fixes:
1. Unconditional Assert() in sepgsql_avc_reclaim().
2. relkind checks are lacked in sepgsqlSetDefaultContext().

The patch of core facilities is unchanged expect for the new GUC
parameters
and above two minor bug fixes.

And I have a question. Is it legal to use a pointer value as a condition,
like `while (!pointer) ...' ?

Thanks for youe reviewing.

KaiGai Kohei wrote:

Thanks for your reviewing.

Peter Eisentraut wrote:

Am Donnerstag, 26. Juni 2008 schrieb KaiGai Kohei:

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

OK, I have quickly read through these patches. They look very nice,
so I am optimistic we can get through this.

First of all, now would be a good time if someone out there really
wants to object to this feature in general. It will probably always
be a niche feature. But all the code is hidden behind ifdefs or
other constructs that a compiler can easily hide away (or we can
make it so, at least).

Here is a presentation from PGCon on SE-PostgreSQL:
http://www.pgcon.org/2008/schedule/events/77.en.html

Are there any comments yet from the (Trusted)Solaris people that
wanted to evaluate this approach for compatibility with their approach?

In this April, I had a face-to-face meeting with Trusted Solaris people
to discuss SE-PostgreSQL and PGACE framework, and concluded that PGACE
framework provides enough facilities for both operating systems.

I modified several hooks from CommitFest:May, however, its fundamental
structures are unchanged.

In general, are we OK with the syntax CONTEXT = '...'? I would
rather see something like SECURITY CONTEXT '...'. There are a lot
of contexts, after all.

If we change it, I prefer SECURITY_CONTEXT = '...' style, because it
enables
to represent the left hand with a single token and make PGACE hooks
simpler.
I also agree the CONTEXT has widespread meanings and to be changed here.

This will also add a system column called security_context. I think
that is OK.

Thanks,

In the pg_dump patch:

spelling mistake "tuen on/off"

I'll fix it soon.

Evil coding style: if (strcmp(SELINUX_SYSATTR_NAME,
security_sysattr_name)) -- compare the result with 0 please.

OK, I'll fix it and check my implementations in other place.

The above code appears to assume that security_sysattr_name never
changes, but
then why do we need a GUC parameter to show it?

The purpose of the GUC parameter is to identify the kind of security
feature
if enabled. It can be changed by other security features, and it will
show us
different value.

Might want to change the option name --enable-selinux to something
like --security-context.

In general, we might want to not name things selinux_* but instead
sepostgresql_* or security_* or security_context_*. Or maybe PGACE?

The pgace_* scheme is an attractive idea, although the server process
has to provide a bit more hints (like the name of security system column
and the kind of objects exported with security attribute) pg_dump to
support various kind of security features with smallest implementation.

If not so, I prefer the combination of --security-context and
sepostgresql_*.

On the default policy:

Should this really be a contrib module? Considering that it would
be a core
feature that is not really usable without a policy.

It is correct, SE-PostgreSQL feature always need its security policy.
Do you think "src/backend/security/sepgsql/policy" is better?

Please change all the sepgsql_* things to sepostgresql_*,

considering that you

are using both already, so we shouldn't have both forms of names.

We can convert them from sepostgresql_* to sepgsql_* easily, because
the longer
forms are not used as a part of identifier in security context.
But we have a possible matter in changing from sepgsql_* to
sepostgresql_*.

Here is a news from SELinux community:
http://marc.info/?l=selinux&amp;m=121501336024075&amp;w=2

It shows most part of the SE-PostgreSQL policy module got merged into
the upstreamed at selinux-policy-3.4.2 or later. It contains identifier
with sepgsql_* stuff, so it has a possible matter when users upgrade
his security policy.

If their database is labeled as sepostgresql_*, it will lose rules
defined in the policy when users upgrade selinux-policy package to
the latest one.

Documentation:

Looks good for a start, but we will probably want to write more later.

Do you think what kind of information should be added?
I intentionally omitted descriptions about SELinux itself,
because it is a documentation of PostgreSQL, not OS.

Thanks,

--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

KaiGai Kohei wrote:

On the WiKi of CommitFest:Sep,
http://wiki.postgresql.org/wiki/CommitFest:2008-09

The entry of SE-PostgreSQL points a message when I submitted older version
of our patch set. But the latest ones are listed on another message.

Please add a link to the following message for our convenience:
http://archives.postgresql.org/pgsql-hackers/2008-07/msg01365.php

Hey, given the amount of work still to go on this, don't you think you
should get a wiki account so that you can add comments yourself?

--Josh

Josh Berkus wrote:

KaiGai Kohei wrote:

On the WiKi of CommitFest:Sep,
http://wiki.postgresql.org/wiki/CommitFest:2008-09

The entry of SE-PostgreSQL points a message when I submitted older
version
of our patch set. But the latest ones are listed on another message.

Please add a link to the following message for our convenience:
http://archives.postgresql.org/pgsql-hackers/2008-07/msg01365.php

Hey, given the amount of work still to go on this, don't you think you
should get a wiki account so that you can add comments yourself?

Thanks for updating the wiki entry.
If possible, I want to apply my wiki account for a rapid status updating.

--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

The following SE-PostgreSQL patches are updated.

It contains rebasing to the lastest CVS and fixes at invocation of trusted
procedures via operators. Rest of parts are unchanged.

[1/4] http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r980.patch
[2/4] http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r980.patch
[3/4] http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r980.patch
[4/4] http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r980.patch

Please add them to the queue of CommitFest:Sep and update its wiki entry.

Thanks,
--
KaiGai Kohei <kaigai@kaigai.gr.jp>

Hello,

The latest SE-PostgreSQL patches are updated here:

[1/4] http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1005.patch
[2/4] http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r1005.patch
[3/4] http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1005.patch
[4/4] http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1005.patch

They contains rebasing to the CVS HEAD, because we cannot apply the previous ones
correctly due to progress in the base version.
Rest of changes are here:
- A new PGACE hook: pgaceIsAllowExecutorRunHook().
It enables to control overriding ExecutorRun_hook, because several important
hooks are invoked from standard_ExecutorRun().
- T_SEvalItem related equalXXXX() functions are added to nodes/equalfuncs.c.
# I've left for implement them.
- Fix typo in src/include/security/pgace.h

BTW, I thought I have to show the overview of the patch to help reviwers.
The main patch ([1/4]) is especially big and contains new concepts.

The following explanation shows several key concept of SE-PostgreSQL.
I'm happy if it can reduce the load of reviwers.

No need to say, please feel free to ask me anything. :-)

Thanks,

Security hooks
--------------
We called it as PostgreSQL Access Control Extention (PGACE).
The "src/include/security/pgace.h" newly added declares these hooks as
inline functions. If HAVE_SELINUX is available at build time, they have
a valid implementation to invoke functions to make access control decision.
When the SE-PostgreSQL feature is disabled at build time or run time,
it does not change any existing behavior.

These hooks have a prefix of "pgace", like pgaceHeapTupleInsert().
This hook is invoked just before inserting a new tuple into a relation,
and the SE-PostgreSQL subsystem can make its decision.

Its argument provides information to make a decision. The pgaceHeapTupleInsert()
has four arguments like the target Relation object and newly inserted HeapTuple.

Specifications of each hooks are described in the "src/include/security/pgace.h".

Security attribute management
-----------------------------
We need a security attribute of tuple to use it as a basic of access control
decision. SELinux calls it as "security context", and most of security aware
operating system has similar idea called as label.
It is represented as a text format like "system_u:object_r:etc_t:s0", and has
its characteristic that many objects tend to share a single security context.

We stores text represented security attribute into "pg_security" system catalog
and put an alternate key (oid of pg_security) on each tuples, because it is
unacceptable approach to put a raw string data on individual tuples.

The alternate key is stored in the tail of HeapTupleHeader, as "oid" doing.
This field is valid when t_infomask has HEAP_HASSECURITY bit.

    HeapTupleHeader
   +-----------------+
   |       :         |
   +-----------------+
   |  t_infomask     |
   +-----------------+                 pg_security system catalog
   |    t_hoff    o-------+      +-------+---------------------------------------------+
   +-----------------+    |      |  oid  |   seclabel                                  |
   |       :         |    |      +-------+---------------------------------------------+
   |       :         |    |      | 16389 | system_u:object_r:sepgsql_table_t:s0        |
   +-----------------+    |      | 16401 | system_u:object_r:sepgsql_proc_t:s0         |
   |*Oid security_id*|---------> | 16402 | system_u:object_r:sepgsql_secret_table_t:s0 |
   +-----------------+    |      |   :   |               :                             |
   | Oid object_id   |    |
   +-----------------+ <--+
   |   Data field    |
   |       :         |

The alternate key is just a internal representation, so we have to translate
it to/from text format when communicating to in-kernel SELinux, or export/import
them.

Note that the security attribute is also assigned to tuples within system
catalogs. A security attribute of a tuple within pg_attribute means column's
security attribute, and used to column-level access controls, for example.

The src/backend/security/pgaceCommon.c have functions to traslate them:

char *pgaceLookupSecurityLabel(Oid security_id);
Oid pgaceLookupSecurityId(char *security_label);

When a new security_label is given and not found on pg_security,
pgaceLookupSecurityId() tries to insert a new tuple into pg_security and
returns its object id as an alternate key.

Two more similar functions are also declared:
char *pgaceSidToSecurityLabel(Oid security_id)
Oid pgaceSecurityLabelToSid(char *security_label)
It also enables to translate between a cosmetic text format and an internal
security identifier.
An example of cosmetic format is:
unconfined_u:unconfined_r:unconfined_t:SystemLow-SystemHigh
^^^^^^^^^^^^^^^^^^^^
We have a case when `pg_security` system catalog is not available because
initdb does not complete to initialize system catalogs yet. In early phase,
this facility caches the pairs of identifier and text representations, then
it writes out to `pg_security` after initial set up completed.
earlySecurityLabelToSid() and earlySidToSecurityLabel() are used while we
are in bootstraping mode, and pgacePostBootstrapingMode() write out them.

Making access control decision
------------------------------
SE-PostgreSQL makes its decision with three steps in major path.

The first step picks up any database objects accessed with the given queries.
For example, the following UPDATE statement contains accesses to 6 objects.

UPDATE t SET a = 5, b = b * 2 WHERE c > 10;
- table t is updated and refered.
- column a is updated.
- column b is updated and refered.
- column c is refered without leaking its contents.
- function int4mul and int4gt are executed.

The sepgsqlProxyQuery() defined at "src/backend/security/sepgsql/proxy.c"
walks on the given Query trees to pick up database objects accessed, and
makes a list of them. This list is chained to Query->pgaceItem which is
a private field of security subsystem, and delivered to private member
of PlannedStmt structure.
In same time, it marks a kind of accesses on RangeTblEntry->pgaceTuplePerms.
This value is copied to Scan->pgaceTuplePerms, and sepgsqlExecScan() hook
refers to identify what is the purpose of this scan.

The second step is evaluations of permission for picked up database objects.
The sepgsqlVerifyQuery() is invoked at the head of ExecutorStart(), to check
whether the client has enough permission
It walk on the list chained to PlannedStmt->pgaceItem which is created at
the first step, and checks whether the client has enough permissions, or not.
If the security policy does not allow the required accesses, SE-PostgreSQL
makes an error to abort query execution.
Tuple-level access controls are not done in this step.

The last step is evaluations of permission for scanned tuples.
The sepgsqlExecScan() defined at "src/baskend/security/sepgsql/hooks.c" is
invoked from ExecScan(). This hook can return bool value, and modified
logic skips a tuple scanned if it returns 'false'.

Other topics
------------

* Default security context

The security policy also contains rules to decide a security context of
newly inserted tuples. If client does not specify any explicit one, the
default one is applied.
We can specify explicit security context with the following Enhanced SQL
statement or writable system column.

* Enhanced SQL statement

It helps to define a new table, column, function and database with specific
security context. We can use the facility as follows:

CREATE TABLE t (
a integer primary key,
b text,
) SECURITY_CONTEXT = 'system_u:object_r:sepgsql_ro_t:s0';
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The above token is represented as DefElem objects in gram.y, and saved on
private member of CreateStmt structure. Then, these are translated to List
of DefElem and delivered to heap_create_with_catalog() with its fifteenth
argument.
pgaceCreateRelationCommon() and pgaceCreateAttributeCommon() are invoked
just before inserting a new tuple into pg_class/pg_attribute. These are
defined at "src/backend/security/pgaceCommon.c", and they invokes PGACE
hooks if the security context of new objects are explicitly specified.

At the next, sepgsqlHeapTupleInsert() is invoked for the new tuple.
It skips to assign default security context, because explicit security
context is already set. Permissions to create table/column is evaluated
for explicitly specified one.

* Writable system column

Security contexts can be exported via "security_context" system column.
In the previous version, system column is read only, so we cannot specify
in an element of INSERT or UPDATE.
SE-PostgreSQL allows to set an explicit value to "security_context" system
column to utilize it as an interface to change security context of tuples.

An example)
UPDATE t SET security_context = security_context || ':c0' WHERE a = 10;

The TargetEntry->resjunk for "security_context" is always turned on for
INSERT, UPDATE and SELECT INTO statement. In the result, its new value
is computed but removed at ExecFilterJunk().
The patched ExecutePlan() fetches the value and stores it on the tail of
HeapTupleHeader with HeapTupleSetSecurity() macro.

This patch changes transformInsertStmt() and updateTargetListEntry()
to turn on TargetEntry->resjunk when attribute number is negative.
Its purpose is to enable writable system column.

Same logic can be applied to writable "oid" system column as a possibility.

* userspace Access vector cache

The "src/backend/security/sepgsql/avc.c" is an implementation of userspace
access vector cache. SE-PostgreSQL ask in-kernel SELinux to check whether
the given access should be allowed, or not.
However, it requires system call invocation which is a bit heavy step.
The userspace AVC caches recently asked access patterns, and enables to
reduce the number of system call invocation.

--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

Peter, Abhijit,

Could you tell me the current status of reviewing process in the CommitFest:Sep?
I can understand the patches contain several new concept and a bit large,
and it is a tough work to review them comprehensively.

I'm not unconcern anything even if reviwing comments are partial one.
However, I cannot improve anything without any comments. :(

Did the previous message which I posted help you to understand the patches?
If not, please feel free to ask me anything.

Thanks,

KaiGai Kohei wrote:

Hello,

The latest SE-PostgreSQL patches are updated here:

[1/4] http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1005.patch
[2/4] http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r1005.patch
[3/4] http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1005.patch
[4/4] http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1005.patch

They contains rebasing to the CVS HEAD, because we cannot apply the
previous ones
correctly due to progress in the base version.
Rest of changes are here:
- A new PGACE hook: pgaceIsAllowExecutorRunHook().
It enables to control overriding ExecutorRun_hook, because several
important
hooks are invoked from standard_ExecutorRun().
- T_SEvalItem related equalXXXX() functions are added to
nodes/equalfuncs.c.
# I've left for implement them.
- Fix typo in src/include/security/pgace.h

BTW, I thought I have to show the overview of the patch to help reviwers.
The main patch ([1/4]) is especially big and contains new concepts.

The following explanation shows several key concept of SE-PostgreSQL.
I'm happy if it can reduce the load of reviwers.

No need to say, please feel free to ask me anything. :-)

Thanks,

--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

KaiGai Kohei wrote:

Could you tell me the current status of reviewing process in the
CommitFest:Sep?

Well ... I have analyzed this patch several times now. The
implementation appears to be pretty straightforward, and when the time
comes, we can discuss some of the low-level details.

At this time, however, I would like to take a step or two back and
discuss with the rest of the community what we really want to achieve in
terms of enhancing "security" and access control. We need wider input
on this. The patch makes use of generic terms such as "security
enhanced" and "access control extensions" and appropriates them to this
particular implementation, and I am not really sure what direction we
want to take with this. I have also reviewed your PGCon presentation to
infer your goals behind this implementation.

At the core of it, I can see a few things that are being done here:

* System-wide consistency in access controls

The idea is, if we use some system and language to control operating
system permissions, it would be nice to use the same system and language
to control access permissions in the database system and elsewhere.

* Mandatory access control (MAC)

This defines a global security policy on top of the discretionary access
control currently in place. (Depending on how you interpret the terms
and the capabilities of SELinux, it also provides Type Enforcement and
Multilevel Security to some degree. These are related in some ways to MAC.)

* Row-level security

This defines a way to control access to rows, not only to columns.

* Additional privileges

The presented patches add ways to control permissions to alter tables,
modify large objects, and other things as well as column-level
privileges. Some of this is a moral prerequisite for a useful MAC setup.

Now these items are arguably useful and welcome features in their own
right. Unfortunately, this patch has chosen to provide these features
in a way that makes them accessible to the least amount of users. And
moreover, it bunches them all in one feature, while they should really
be available independently.

Let's review:

*) System-wide consistency in access controls could be nice to have in
some cases. But is it really achievable? In the typical three-tier web
application scenario, do you really have system-wide consistency? Can
you configure your application server using SELinux? I'm no expert on
these things, but I wonder, would it even work in a useful way, over the
network, with all the different threads, processes, and sessions going
on? Or how about a desktop, pgAdmin with several database connections,
can those be isolated from each other or whatever the security setup may be?

And is SELinux really the desirable interface for a system-wide access
control facility? Why not port chmod or POSIX ACLs to PostgreSQL, or
port SQL roles back to the operating system, or something else that
captures what more people are actually using in practice.

*) Mandatory access control could be a useful feature for some users.
But must we resort to SELinux as the configuration language and
implementation backend? Why couldn't we implement a MAC system in SQL
using the existing language?

Also, what I read is that role-based access control (RBAC) systems are
equally capable of providing the sort of stronger security that MAC
users are typically looking for. (In some ways, it appears to be the
newer thing.) PostgreSQL already has a pretty good role system, so we
could perhaps look into enhancing that to meet the necessary functional
criteria that may exist.

*) Row-level security, column-level security and so on should in my mind
first exist as a native SQL-level feature. It would be hard to explain
that PostgreSQL does not have column-level GRANT privileges but that you
can achieve the same if you go through SELinux. After all, the
SE-PostgreSQL patch only hooks in to several places to throw "permission
denied errors" when appropriate, so native SQL features should be able
to achieve the same. (Well, there are interesting SELinux-vs-AppArmor
type differences here, that may be worth considering separately.)

Ultimately, I see this patch as an interesting proof of concept -- it
got us on the NSA site anyway -- but I can't see more than three people
actually making use of it, and they are not going to maintain this code
for us in the long run. On the other hand, it provides useful features,
but with what I consider suboptimal interfaces. Considering that
SELinux configurations on Red Hat are now just slowly evolving from
annoying to usable after many years of work, I can't see us mustering
the resources to achieve a usable configuration of this in a reasonable
time, let alone the resources required to verify the whole thing so that
is actually provides some assurances rather than just another way to
fiddle about with the system.

The way I see this, the approach to be taken to achieve some kind of
government-grade security system would be:

1. Implement row-level security, column-level privileges, and all other
necessary permission bits as SQL features.

2. Enhance the role system to meet whatever the evaluation criteria may be.

3a. Add access control extension hooks.

3b. Implement SELinux on top of these hooks.

You have already done 3a and 3b, but I think 1 and 2 should be done first.

On Wed, 17 Sep 2008, Peter Eisentraut wrote:

System-wide consistency in access controls could be nice to have in some
cases. But is it really achievable? In the typical three-tier web
application scenario, do you really have system-wide consistency? Can
you configure your application server using SELinux?

Each of the tiers end up with mapping layer similar to the one implemented
here to map the SELinux permissions -> PostgreSQL. Java for example has a
whole JVM security manager component that makes it straighforward to do
such a mapping.
http://articles.techrepublic.com.com/5100-10878_11-6178805.html is a good
quick intro that shows how the call structure is similar to what the
SE-PostgreSQL code does.

And is SELinux really the desirable interface for a system-wide access
control facility? Why not port chmod or POSIX ACLs to PostgreSQL, or port
SQL roles back to the operating system, or something else that captures what
more people are actually using in practice.

The main feature of SELinux that this crowd likes is how it manages
privledge escalation risk. I'm not sure if POSIX ACLs for example are as
effective at limiting the damage an exploitable suid binary can cause.
As for what people are actually using, as someone who lives near the US
capital I can tell you that installs using SELinux are quite plentiful
around here--there really is no other UNIX-based technology for this
purpose that's gotten traction inside this government like SELinux has.

Anyway, even though I think picking SELinux as the primary security
mechanism to integrate with is a sensible choice and I'm confident that
the rest of the software stack isn't a problem, I do share your concern
that implementing row and column-level security would make more sense in a
general way first.

Ultimately, I see this patch as an interesting proof of concept -- it got us
on the NSA site anyway -- but I can't see more than three people actually
making use of it

I take it you've never seen how big the NSA fort^H^H^H^Hfacility is? I'm
not sure exactly how many orders of magnitude your estimate is off by, but
I know it's at least 2 just based on conversations I've been involved in
with companies around here. A lot of the US defense and homeland security
related companies are adopting open-source software stacks because they
can audit every level of the software, and there's a big void in that
stack waiting for a database with the right security model to fill. You
are right that getting code contributions back again is a challenge
though.

--
* Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD

Peter, thanks for your comments.

Let's review:

*) System-wide consistency in access controls could be nice to have in
some cases. But is it really achievable? In the typical three-tier web
application scenario, do you really have system-wide consistency? Can
you configure your application server using SELinux? I'm no expert on
these things, but I wonder, would it even work in a useful way, over the
network, with all the different threads, processes, and sessions going
on? Or how about a desktop, pgAdmin with several database connections,
can those be isolated from each other or whatever the security setup may
be?

It's a good question. Yes, it is possible no need to say. :)

We can configure Apache to kick its contents handler with a proper security
context. The contents handler is a sort of Apache module to handle various
kind of web contents like *.html, *.php, *.cgi and so on.
The existing module (mod_selinux) eanbles to invoke CGI program with a proper
security context based on HTTP authentication. In addition, the upcoming
Linux kernel got a feature to assign built-in scripts its security context.

SELinux applied its access controls based on the assigned security context
for various kind of objects like files, sockets, IPCs, tables, columns and
so on.

I can provide a demonstration, pelase wait for a while to set up.

- For overing networks,
The current SELinux provides the Labeled Networking feature. It enables to
deliver a peer's security context to other peer side. We can fetch delivered
one with getpeercon() API. SE-PostgreSQL also uses this API to obtain the
security context of client process.
(*) Note that SE-PostgreSQL does not depend on Database authentication.

- For different threads,
The upcoming Linux kernel got a feature to assign individual security context
for a thread with a constraint to prevent information boundary violation.
http://marc.info/?l=selinux&amp;m=121992782231903

- For sessions,
We can set a proper security context using session variable before invoking
web applications, in same way as I noted above.
However, I've not provide an implementation yet.

And is SELinux really the desirable interface for a system-wide access
control facility? Why not port chmod or POSIX ACLs to PostgreSQL, or
port SQL roles back to the operating system, or something else that
captures what more people are actually using in practice.

Yes, SELinux is one of the suitable facilities.

It enables to describe its security policy using abstracted attributes
called as "security context", independent from a sort of subsystems.
For example, the POSIX ACL is an accomplished access control stuff, but
it is specific for filesystem. We have no way to apply it on network
access controls.

In addition, it enables to reduce privilege escalation risk as Greg Smith
mentioned.

*) Mandatory access control could be a useful feature for some users.
But must we resort to SELinux as the configuration language and
implementation backend? Why couldn't we implement a MAC system in SQL
using the existing language?

Also, what I read is that role-based access control (RBAC) systems are
equally capable of providing the sort of stronger security that MAC
users are typically looking for. (In some ways, it appears to be the
newer thing.) PostgreSQL already has a pretty good role system, so we
could perhaps look into enhancing that to meet the necessary functional
criteria that may exist.

I want you to understand the proposd design *NEVER* denies mandatory access
controls based on other security engine. The PGACE hooks enables to swap
the access control subsystem by the configure option.

I also agree that PostgreSQL already has a pretty good role and access
control mechanism. However, it is hard to describe mandatory access control
policy in common forms between OS and RDBMS.

I don't think dual-checks are matter. The operating system applies its DAC
policy known as filesystem permission and MAC policy provided by SELinux
(or other facilities).

*) Row-level security, column-level security and so on should in my mind
first exist as a native SQL-level feature. It would be hard to explain
that PostgreSQL does not have column-level GRANT privileges but that you
can achieve the same if you go through SELinux. After all, the
SE-PostgreSQL patch only hooks in to several places to throw "permission
denied errors" when appropriate, so native SQL features should be able
to achieve the same. (Well, there are interesting SELinux-vs-AppArmor
type differences here, that may be worth considering separately.)

I cannot understand the reason why these native/newer access control
facilities should have same access control guranualities.
These are provided by different subsystems, so they have different
guranualities and results. What is the matter?

The relationship of them is designed by reference to DAC and MAC on operating
system. As I noted above, it checks user's request twice based on different
security policy.

In addition, SELinux defines 20 kind of permissions for regular files,
being finer than filesystem permissions. It is not a curious behavior.

The way I see this, the approach to be taken to achieve some kind of
government-grade security system would be:

1. Implement row-level security, column-level privileges, and all other
necessary permission bits as SQL features.

2. Enhance the role system to meet whatever the evaluation criteria may be.

3a. Add access control extension hooks.

3b. Implement SELinux on top of these hooks.

You have already done 3a and 3b, but I think 1 and 2 should be done first.

It seems to me there is no dependencies.
We can make progress them in parallel.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

Greg Smith wrote:

On Wed, 17 Sep 2008, Peter Eisentraut wrote:

System-wide consistency in access controls could be nice to have in
some cases. But is it really achievable? In the typical three-tier
web application scenario, do you really have system-wide consistency?
Can you configure your application server using SELinux?

Each of the tiers end up with mapping layer similar to the one
implemented here to map the SELinux permissions -> PostgreSQL. Java for
example has a whole JVM security manager component that makes it
straighforward to do such a mapping.
http://articles.techrepublic.com.com/5100-10878_11-6178805.html is a
good quick intro that shows how the call structure is similar to what
the SE-PostgreSQL code does.

I guess these security architectures have same origin.

The reference monitor concept requres all accesses to data objects to be
checked by a tamperproof, always-invoked module based on its policy.
http://en.wikipedia.org/wiki/Reference_monitor

SE-PostgreSQL uses in-kernel SELinux as a reference monitor to check
all accesses to database object via SQL.

And is SELinux really the desirable interface for a system-wide access
control facility? Why not port chmod or POSIX ACLs to PostgreSQL, or
port SQL roles back to the operating system, or something else that
captures what more people are actually using in practice.

The main feature of SELinux that this crowd likes is how it manages
privledge escalation risk. I'm not sure if POSIX ACLs for example are
as effective at limiting the damage an exploitable suid binary can
cause. As for what people are actually using, as someone who lives near
the US capital I can tell you that installs using SELinux are quite
plentiful around here--there really is no other UNIX-based technology
for this purpose that's gotten traction inside this government like
SELinux has.

Anyway, even though I think picking SELinux as the primary security
mechanism to integrate with is a sensible choice and I'm confident that
the rest of the software stack isn't a problem, I do share your concern
that implementing row and column-level security would make more sense in
a general way first.

Thanks for your explanation.

The PGACE security framework can mount a OS independent fine
grained access control feature, like Oracle Label Security.
However, one concern is we have only one CommitFest remained.

As I mentioned at the previous message, I think it is not
a strange behavior that different security subsystems make
different decisions on individual gulanualities.

Ultimately, I see this patch as an interesting proof of concept -- it
got us on the NSA site anyway -- but I can't see more than three
people actually making use of it

I take it you've never seen how big the NSA fort^H^H^H^Hfacility is?
I'm not sure exactly how many orders of magnitude your estimate is off
by, but I know it's at least 2 just based on conversations I've been
involved in with companies around here. A lot of the US defense and
homeland security related companies are adopting open-source software
stacks because they can audit every level of the software, and there's a
big void in that stack waiting for a database with the right security
model to fill. You are right that getting code contributions back again
is a challenge though.

I don't have statistically reliable information. :)
However, I believe there is potentially strong demand for secure database
due to responses from audiences when I had presentations about SE-PostgreSQL
in various opportunities.

IIRC, Josh also said similar things.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>