Thread: [PATCH] SE-PostgreSQL for v8.5 development (r1769)

[PATCH] SE-PostgreSQL for v8.5 development (r1769)

From
KaiGai Kohei
Date:
The following list of patches are the initial revision of SE-PostgreSQL
on the v8.5 development cycle.
These are separated into several functional components to help review
and commit in earlier phase. Every patches (except for the core) have
abour 1KL scales. It is far smaller than them in a year ago. :-)
 http://sepgsql.googlecode.com/files/sepgsql-01-sysatt-8.4devel-r1769.patch
http://sepgsql.googlecode.com/files/sepgsql-02-core-8.4devel-r1769.patch
http://sepgsql.googlecode.com/files/sepgsql-03-writable-8.4devel-r1769.patch
http://sepgsql.googlecode.com/files/sepgsql-04-rowlevel-8.4devel-r1769.patch
http://sepgsql.googlecode.com/files/sepgsql-05-perms-8.4devel-r1769.patch
http://sepgsql.googlecode.com/files/sepgsql-06-utils-8.4devel-r1769.patch
http://sepgsql.googlecode.com/files/sepgsql-07-tests-8.4devel-r1769.patch
http://sepgsql.googlecode.com/files/sepgsql-08-docs-8.4devel-r1769.patch

Needless to say, it is now designed on 8.4devel tree, so anyone who want
to build/install SE-PostgreSQL can apply these patches by hand.
I'll also update and fix them with the progress of v8.4 development.
Before you apply them, please confirm whether they are the latest, or not.

Bruice,
| KaiGai-san, the only option I can offer is perhaps to list a URL for
| your SE-PostgreSQL patch to be applied by people who want to use SE-PG.

Does it mean I need to submit a patch to add an introduction under doc/ ?
If so, I'll submit it as soon as possible.

Thanks,


01) Security system attribute support   scale: 38 files changed, 853 insertions(+), 1 deletion(-), 113 modifications(!)
Thispatch adds a new system catalog "pg_security" and enables to store security identifier associated to a text
representationwithin padding area of HeapTupleHeader, as object identifier doing. It is a foundation of any other
facilities.

02) Core facilities of SE-PostgreSQL   scale: 55 files changed, 3588 insertions(+), 10 deletions(-), 736
modifications(!)This patch adds a mandatory access control feature collaborating with SELinux in table, column,
procedurelevel granurality. Most of this patch is same as I proposed in the v8.4 development cycle, except for it is
designedon the basis of security system attribute support.
 

03) Writable system column support   scale: 7 files changed, 298 insertions(+), 199 modifications(!) This patch enables
usersto update/insert on system columns ("security_label" and "security_acl") with explicit values. This feature is
necessaryto provide a user interface for row-level access controls.
 

04) Row-level access controls support   scale: 31 files changed, 1101 insertions(+), 231 modifications(!) This patch
enablesto apply mandatory/discretionary access control in row-level granularity also.
 

05) Advanced permission checks support   scale: 18 files changed, 858 insertions(+), 3 deletions(-), 43
modifications(!)This patch add some of advanced permission checks:  - file:{read write} on server side filesystem
accesses - db_procedure:{install} on user defined functions as system internal ones  - db_database:{load_module
install_module}on binary shared library files In the v8.4 development, these are suggested to separate from the core.
 

06) Security options in utilities   scale: 4 files changed, 95 insertions(+), 116 modifications(!) This patch adds
optionson utilities  - "--enable-selinux" option for initdb  - "--security-label" option for pg_dump and pg_dumpall
 

07) Testcases of SE-PostgreSQL   scale: 18 files changed, 1819 insertions(+), 2 modifications(!) This patch adds
testcasesfor SE-PostgreSQL.
 

08) Documentation of SE-PostgreSQL   scale: 16 files changed, 1595 insertions(+), 42 modifications(!) This patch adds
documentationsfor SE-PostgreSQL
 

0X) Upcoming patches The following patches are upcoming now. * Reclaim of unused entries in pg_security   I have a plan
toimplement it based on the idea from Robert Haas in:
http://archives.postgresql.org/message-id/603c8f070901281818u3e1fa70brd28e1bfac7adfea9@mail.gmail.com
 * System audit integration with SE-PostgreSQL   Linux has system audit stuff which is used by in-kernel SELinux and
itsuserspace facilities can output audit messages here.   Now SE-PostgreSQL writes out audit messages into PostgreSQL
logs,  but it is more desirable to write it on system audit.
 

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


[PATCH] SE-PostgreSQL for v8.5 development (r1819)

From
KaiGai Kohei
Date:
The following list of patches are the latest SE-PostgreSQL (r1819).

http://sepgsql.googlecode.com/files/sepgsql-01-sysatt-8.4beta1-r1819.patch
http://sepgsql.googlecode.com/files/sepgsql-02-core-8.4beta1-r1819.patch
http://sepgsql.googlecode.com/files/sepgsql-03-writable-8.4beta1-r1819.patch
http://sepgsql.googlecode.com/files/sepgsql-04-rowlevel-8.4beta1-r1819.patch
http://sepgsql.googlecode.com/files/sepgsql-05-perms-8.4beta1-r1819.patch
http://sepgsql.googlecode.com/files/sepgsql-06-utils-8.4beta1-r1819.patch
http://sepgsql.googlecode.com/files/sepgsql-07-tests-8.4beta1-r1819.patch
http://sepgsql.googlecode.com/files/sepgsql-08-docs-8.4beta1-r1819.patch

List of updates:
* The base version was updated to the latest CVS HEAD.
* The code to receice notifications from the kernelspace via netlink socket was simplified using the new
avc_netlink_xxx()APIs.
 
* It enables to handle permissive domain on the upcoming linux-2.6.31.
* It enables to handle undefined permissions in the policy correctly.

The purpose of every patches are not changed.

Thanks,

KaiGai Kohei wrote:
> The following list of patches are the initial revision of SE-PostgreSQL
> on the v8.5 development cycle.
> These are separated into several functional components to help review
> and commit in earlier phase. Every patches (except for the core) have
> abour 1KL scales. It is far smaller than them in a year ago. :-)
> 
>   http://sepgsql.googlecode.com/files/sepgsql-01-sysatt-8.4devel-r1769.patch
>   http://sepgsql.googlecode.com/files/sepgsql-02-core-8.4devel-r1769.patch
>   http://sepgsql.googlecode.com/files/sepgsql-03-writable-8.4devel-r1769.patch
>   http://sepgsql.googlecode.com/files/sepgsql-04-rowlevel-8.4devel-r1769.patch
>   http://sepgsql.googlecode.com/files/sepgsql-05-perms-8.4devel-r1769.patch
>   http://sepgsql.googlecode.com/files/sepgsql-06-utils-8.4devel-r1769.patch
>   http://sepgsql.googlecode.com/files/sepgsql-07-tests-8.4devel-r1769.patch
>   http://sepgsql.googlecode.com/files/sepgsql-08-docs-8.4devel-r1769.patch
> 
> Needless to say, it is now designed on 8.4devel tree, so anyone who want
> to build/install SE-PostgreSQL can apply these patches by hand.
> I'll also update and fix them with the progress of v8.4 development.
> Before you apply them, please confirm whether they are the latest, or not.
> 
> Bruice,
> | KaiGai-san, the only option I can offer is perhaps to list a URL for
> | your SE-PostgreSQL patch to be applied by people who want to use SE-PG.
> 
> Does it mean I need to submit a patch to add an introduction under doc/ ?
> If so, I'll submit it as soon as possible.
> 
> Thanks,
> 
> 
> 01) Security system attribute support
>     scale: 38 files changed, 853 insertions(+), 1 deletion(-), 113 modifications(!)
>   This patch adds a new system catalog "pg_security" and enables to store
>   security identifier associated to a text representation within padding
>   area of HeapTupleHeader, as object identifier doing.
>   It is a foundation of any other facilities.
> 
> 02) Core facilities of SE-PostgreSQL
>     scale: 55 files changed, 3588 insertions(+), 10 deletions(-), 736 modifications(!)
>   This patch adds a mandatory access control feature collaborating with
>   SELinux in table, column, procedure level granurality. Most of this
>   patch is same as I proposed in the v8.4 development cycle, except for
>   it is designed on the basis of security system attribute support.
> 
> 03) Writable system column support
>     scale: 7 files changed, 298 insertions(+), 199 modifications(!)
>   This patch enables users to update/insert on system columns ("security_label"
>   and "security_acl") with explicit values. This feature is necessary to provide
>   a user interface for row-level access controls.
> 
> 04) Row-level access controls support
>     scale: 31 files changed, 1101 insertions(+), 231 modifications(!)
>   This patch enables to apply mandatory/discretionary access control in row-level
>   granularity also.
> 
> 05) Advanced permission checks support
>     scale: 18 files changed, 858 insertions(+), 3 deletions(-), 43 modifications(!)
>   This patch add some of advanced permission checks:
>    - file:{read write} on server side filesystem accesses
>    - db_procedure:{install} on user defined functions as system internal ones
>    - db_database:{load_module install_module} on binary shared library files
>   In the v8.4 development, these are suggested to separate from the core.
> 
> 06) Security options in utilities
>     scale: 4 files changed, 95 insertions(+), 116 modifications(!)
>   This patch adds options on utilities
>    - "--enable-selinux" option for initdb
>    - "--security-label" option for pg_dump and pg_dumpall
> 
> 07) Testcases of SE-PostgreSQL
>     scale: 18 files changed, 1819 insertions(+), 2 modifications(!)
>   This patch adds testcases for SE-PostgreSQL.
> 
> 08) Documentation of SE-PostgreSQL
>     scale: 16 files changed, 1595 insertions(+), 42 modifications(!)
>   This patch adds documentations for SE-PostgreSQL
> 
> 0X) Upcoming patches
>   The following patches are upcoming now.
>   * Reclaim of unused entries in pg_security
>     I have a plan to implement it based on the idea from Robert Haas in:
>       http://archives.postgresql.org/message-id/603c8f070901281818u3e1fa70brd28e1bfac7adfea9@mail.gmail.com
> 
>   * System audit integration with SE-PostgreSQL
>     Linux has system audit stuff which is used by in-kernel SELinux and
>     its userspace facilities can output audit messages here.
>     Now SE-PostgreSQL writes out audit messages into PostgreSQL logs,
>     but it is more desirable to write it on system audit.
> 


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


Re: [PATCH] SE-PostgreSQL for v8.5 development (r1819)

From
Bruce Momjian
Date:
Kohei-san, what URL do you want me to list in the 8.4 release notes for
the SE-Linux patches?

---------------------------------------------------------------------------

KaiGai Kohei wrote:
> The following list of patches are the latest SE-PostgreSQL (r1819).
> 
> http://sepgsql.googlecode.com/files/sepgsql-01-sysatt-8.4beta1-r1819.patch
> http://sepgsql.googlecode.com/files/sepgsql-02-core-8.4beta1-r1819.patch
> http://sepgsql.googlecode.com/files/sepgsql-03-writable-8.4beta1-r1819.patch
> http://sepgsql.googlecode.com/files/sepgsql-04-rowlevel-8.4beta1-r1819.patch
> http://sepgsql.googlecode.com/files/sepgsql-05-perms-8.4beta1-r1819.patch
> http://sepgsql.googlecode.com/files/sepgsql-06-utils-8.4beta1-r1819.patch
> http://sepgsql.googlecode.com/files/sepgsql-07-tests-8.4beta1-r1819.patch
> http://sepgsql.googlecode.com/files/sepgsql-08-docs-8.4beta1-r1819.patch
> 
> List of updates:
> * The base version was updated to the latest CVS HEAD.
> * The code to receice notifications from the kernelspace via netlink
>   socket was simplified using the new avc_netlink_xxx() APIs.
> * It enables to handle permissive domain on the upcoming linux-2.6.31.
> * It enables to handle undefined permissions in the policy correctly.
> 
> The purpose of every patches are not changed.
> 
> Thanks,
> 
> KaiGai Kohei wrote:
> > The following list of patches are the initial revision of SE-PostgreSQL
> > on the v8.5 development cycle.
> > These are separated into several functional components to help review
> > and commit in earlier phase. Every patches (except for the core) have
> > abour 1KL scales. It is far smaller than them in a year ago. :-)
> > 
> >   http://sepgsql.googlecode.com/files/sepgsql-01-sysatt-8.4devel-r1769.patch
> >   http://sepgsql.googlecode.com/files/sepgsql-02-core-8.4devel-r1769.patch
> >   http://sepgsql.googlecode.com/files/sepgsql-03-writable-8.4devel-r1769.patch
> >   http://sepgsql.googlecode.com/files/sepgsql-04-rowlevel-8.4devel-r1769.patch
> >   http://sepgsql.googlecode.com/files/sepgsql-05-perms-8.4devel-r1769.patch
> >   http://sepgsql.googlecode.com/files/sepgsql-06-utils-8.4devel-r1769.patch
> >   http://sepgsql.googlecode.com/files/sepgsql-07-tests-8.4devel-r1769.patch
> >   http://sepgsql.googlecode.com/files/sepgsql-08-docs-8.4devel-r1769.patch
> > 
> > Needless to say, it is now designed on 8.4devel tree, so anyone who want
> > to build/install SE-PostgreSQL can apply these patches by hand.
> > I'll also update and fix them with the progress of v8.4 development.
> > Before you apply them, please confirm whether they are the latest, or not.
> > 
> > Bruice,
> > | KaiGai-san, the only option I can offer is perhaps to list a URL for
> > | your SE-PostgreSQL patch to be applied by people who want to use SE-PG.
> > 
> > Does it mean I need to submit a patch to add an introduction under doc/ ?
> > If so, I'll submit it as soon as possible.
> > 
> > Thanks,
> > 
> > 
> > 01) Security system attribute support
> >     scale: 38 files changed, 853 insertions(+), 1 deletion(-), 113 modifications(!)
> >   This patch adds a new system catalog "pg_security" and enables to store
> >   security identifier associated to a text representation within padding
> >   area of HeapTupleHeader, as object identifier doing.
> >   It is a foundation of any other facilities.
> > 
> > 02) Core facilities of SE-PostgreSQL
> >     scale: 55 files changed, 3588 insertions(+), 10 deletions(-), 736 modifications(!)
> >   This patch adds a mandatory access control feature collaborating with
> >   SELinux in table, column, procedure level granurality. Most of this
> >   patch is same as I proposed in the v8.4 development cycle, except for
> >   it is designed on the basis of security system attribute support.
> > 
> > 03) Writable system column support
> >     scale: 7 files changed, 298 insertions(+), 199 modifications(!)
> >   This patch enables users to update/insert on system columns ("security_label"
> >   and "security_acl") with explicit values. This feature is necessary to provide
> >   a user interface for row-level access controls.
> > 
> > 04) Row-level access controls support
> >     scale: 31 files changed, 1101 insertions(+), 231 modifications(!)
> >   This patch enables to apply mandatory/discretionary access control in row-level
> >   granularity also.
> > 
> > 05) Advanced permission checks support
> >     scale: 18 files changed, 858 insertions(+), 3 deletions(-), 43 modifications(!)
> >   This patch add some of advanced permission checks:
> >    - file:{read write} on server side filesystem accesses
> >    - db_procedure:{install} on user defined functions as system internal ones
> >    - db_database:{load_module install_module} on binary shared library files
> >   In the v8.4 development, these are suggested to separate from the core.
> > 
> > 06) Security options in utilities
> >     scale: 4 files changed, 95 insertions(+), 116 modifications(!)
> >   This patch adds options on utilities
> >    - "--enable-selinux" option for initdb
> >    - "--security-label" option for pg_dump and pg_dumpall
> > 
> > 07) Testcases of SE-PostgreSQL
> >     scale: 18 files changed, 1819 insertions(+), 2 modifications(!)
> >   This patch adds testcases for SE-PostgreSQL.
> > 
> > 08) Documentation of SE-PostgreSQL
> >     scale: 16 files changed, 1595 insertions(+), 42 modifications(!)
> >   This patch adds documentations for SE-PostgreSQL
> > 
> > 0X) Upcoming patches
> >   The following patches are upcoming now.
> >   * Reclaim of unused entries in pg_security
> >     I have a plan to implement it based on the idea from Robert Haas in:
> >       http://archives.postgresql.org/message-id/603c8f070901281818u3e1fa70brd28e1bfac7adfea9@mail.gmail.com
> > 
> >   * System audit integration with SE-PostgreSQL
> >     Linux has system audit stuff which is used by in-kernel SELinux and
> >     its userspace facilities can output audit messages here.
> >     Now SE-PostgreSQL writes out audit messages into PostgreSQL logs,
> >     but it is more desirable to write it on system audit.
> > 
> 
> 
> -- 
> OSS Platform Development Division, NEC
> KaiGai Kohei <kaigai@ak.jp.nec.com>
> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +


Re: [PATCH] SE-PostgreSQL for v8.5 development (r1819)

From
Tom Lane
Date:
Bruce Momjian <bruce@momjian.us> writes:
> Kohei-san, what URL do you want me to list in the 8.4 release notes for
> the SE-Linux patches?

What?  Why would there be anything in the 8.4 release notes about
SEPostgres?
        regards, tom lane


Re: [PATCH] SE-PostgreSQL for v8.5 development (r1819)

From
KaiGai Kohei
Date:
Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
>> Kohei-san, what URL do you want me to list in the 8.4 release notes for
>> the SE-Linux patches?

If desirable, I'll prepare a wiki entry to point the list of patches
and introduce the way to set up SE-PostgreSQL on the v8.4 due to the
stable release.

> What?  Why would there be anything in the 8.4 release notes about
> SEPostgres?

However, I also wonder whether the release note should include it, or not.
In the v8.4, it seems to me more appropriate that the "Appendix G. External
Projects" points to a few external (but not mainlined yet) projects such as
SE-PostgreSQL.

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


Re: [PATCH] SE-PostgreSQL for v8.5 development (r1819)

From
Bruce Momjian
Date:
Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Kohei-san, what URL do you want me to list in the 8.4 release notes for
> > the SE-Linux patches?
> 
> What?  Why would there be anything in the 8.4 release notes about
> SEPostgres?

I suggested it here and no one objected:
http://archives.postgresql.org/message-id/200903160256.n2G2uJS19900@momjian.us

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +


Re: [PATCH] SE-PostgreSQL for v8.5 development (r1819)

From
Bruce Momjian
Date:
KaiGai Kohei wrote:
> Tom Lane wrote:
> > Bruce Momjian <bruce@momjian.us> writes:
> >> Kohei-san, what URL do you want me to list in the 8.4 release notes for
> >> the SE-Linux patches?
> 
> If desirable, I'll prepare a wiki entry to point the list of patches
> and introduce the way to set up SE-PostgreSQL on the v8.4 due to the
> stable release.
> 
> > What?  Why would there be anything in the 8.4 release notes about
> > SEPostgres?
> 
> However, I also wonder whether the release note should include it, or not.
> In the v8.4, it seems to me more appropriate that the "Appendix G. External
> Projects" points to a few external (but not mainlined yet) projects such as
> SE-PostgreSQL.

Ah, very good idea.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +


[PATCH] SE-PostgreSQL for v8.5 development (r1891)

From
KaiGai Kohei
Date:
The SE-PostgreSQL patches (for v8.5) are updated:

http://sepgsql.googlecode.com/files/sepgsql-01-sysatt-8.4beta1-r1891.patch
http://sepgsql.googlecode.com/files/sepgsql-02-core-8.4beta1-r1891.patch
http://sepgsql.googlecode.com/files/sepgsql-03-writable-8.4beta1-r1891.patch
http://sepgsql.googlecode.com/files/sepgsql-04-rowlevel-8.4beta1-r1891.patch
http://sepgsql.googlecode.com/files/sepgsql-05-perms-8.4beta1-r1891.patch
http://sepgsql.googlecode.com/files/sepgsql-06-utils-8.4beta1-r1891.patch
http://sepgsql.googlecode.com/files/sepgsql-07-tests-8.4beta1-r1891.patch
http://sepgsql.googlecode.com/files/sepgsql-08-docs-8.4beta1-r1891.patch

List of updates:
* The base version was upgraded to the latest CVS HEAD.
* db_schema, db_sequence object classes were added. These classes provide analogy of ACL_USAGE and so on.
* db_table/db_column:{reference} permission was added.
* Some of bug were fixed.


BTW, Bruce,

> > In the v8.4, it seems to me more appropriate that the "Appendix G. External
> > > Projects" points to a few external (but not mainlined yet) projects such as
> > > SE-PostgreSQL.
>
> Ah, very good idea.

I made a wiki entry at: http://wiki.postgresql.org/wiki/SEPostgreSQLv8.4

It notes the step to apply the SE-PostgreSQL patches to v8.4 series, and
will introduce the list of patches for each v8.4 series.
(I'll start to provide them from v8.4beta2.)
Is it similar to what you imagined?
If OK, I'll send a documentation patch for the appendix.

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


[PATCH][v8.5] SE-PostgreSQL Patch Updates (r2016)

From
KaiGai Kohei
Date:
The SE-PostgreSQL patches are updated as follows:

1) http://sepgsql.googlecode.com/files/sepgsql-01-sysatt-8.4beta2-r2016.patch
2) http://sepgsql.googlecode.com/files/sepgsql-02-core-8.4beta2-r2016.patch
3) http://sepgsql.googlecode.com/files/sepgsql-03-writable-8.4beta2-r2016.patch
4) http://sepgsql.googlecode.com/files/sepgsql-04-rowlevel-8.4beta2-r2016.patch
5) http://sepgsql.googlecode.com/files/sepgsql-05-perms-8.4beta2-r2016.patch
6) http://sepgsql.googlecode.com/files/sepgsql-06-utils-8.4beta2-r2016.patch
7) http://sepgsql.googlecode.com/files/sepgsql-07-tests-8.4beta2-r2016.patch
8) http://sepgsql.googlecode.com/files/sepgsql-08-docs-8.4beta2-r2016.patch
9) http://sepgsql.googlecode.com/files/sepgsql-09-extra-8.4beta2-r2016.patch

The SE-PostgreSQL online documentation: http://wiki.postgresql.org/wiki/SEPostgreSQL

List of updates:
* Its base version was updated to the latest CVS HEAD.
* Add a feature to reclaim orphan pg_security entries. - See below.
* Add a new guc parameter: sepostgresql_mcstrans - It turnd on/off mcstrans support when we import/export security
context.
* Some of bugfixes
* Code cleanups
* Documentation updates - Ths wiki article was updated corresponding to the latest design.

A significant change is a feature to reclaim orphan pg_security entries.
The definition of the pg_security was changed, and a 'relid' field was
added to indicate the table refering the entry.
An administrative purpose function: security_reclaim_label() removes
entries within pg_security, which are not refered by the table identified
by pg_security.relid.
We assume the frequency to be reclaimed is less enough, so it is not
automatically as if autovacuume. If necessary, cron script can invoke
a script to reclaim orphan entries once per month or bimonth.
On the DROP TABLE, orphan entries are also reclaimed automatically.

-- Example ------------------------------------------
postgres=# CREATE TABLE t1 (a int, b text);
CREATE TABLE
postgres=# INSERT INTO t1 VALUES (1, 'aaa'), (2, 'bbb'), (3, 'ccc');
INSERT 0 3
postgres=# UPDATE t1 SET security_label = sepgsql_set_range(security_label, 's0:c' || a);
UPDATE 3
postgres=# UPDATE t1 SET security_label = sepgsql_set_user(security_label, 'system_u');
UPDATE 3
postgres=# SELECT security_label, * FROM t1;            security_label              | a |  b
-----------------------------------------+---+-----system_u:object_r:sepgsql_table_t:s0:c1 | 1 |
aaasystem_u:object_r:sepgsql_table_t:s0:c2| 2 | bbbsystem_u:object_r:sepgsql_table_t:s0:c3 | 3 | ccc
 
(3 rows)

postgres=# SELECT security_reclaim_label('t1');
NOTICE:  secattr="unconfined_u:object_r:sepgsql_table_t:s0", secid=16433 on public.t1 was reclaimed
NOTICE:  secattr="unconfined_u:object_r:sepgsql_table_t:s0:c1", secid=16434 on public.t1 was reclaimed
NOTICE:  secattr="unconfined_u:object_r:sepgsql_table_t:s0:c2", secid=16435 on public.t1 was reclaimed
NOTICE:  secattr="unconfined_u:object_r:sepgsql_table_t:s0:c3", secid=16436 on public.t1 was
reclaimedsecurity_reclaim_label
------------------------                     4
(1 row)

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


Re: [PATCH][v8.5] SE-PostgreSQL Patch Updates (r2016)

From
"David E. Wheeler"
Date:
On Jun 10, 2009, at 11:06 PM, KaiGai Kohei wrote:

> The SE-PostgreSQL patches are updated as follows:

Kohei-San,

I've no idea whether SE-PostgreSQL will ever make it into the core  
distribution, and have no way to really determine whether or not it's  
a good idea, but I did want to say:

I admire your persistence and hard work on this project. In the face  
of many criticisms and requests for changes, you keep coming back with  
more. It's an amazing amount of work to keep up with it all, and to  
send patches again and again, and I'm really impressed with your  
determination.

In short: respek!

Best,

David



[PATCH] SE-PostgreSQL Updates rev.2096

From
KaiGai Kohei
Date:
The SE-PostgreSQL patches are updated as follows:

01) http://sepgsql.googlecode.com/files/sepgsql-01-sysatt-8.4-r2096.patch
02) http://sepgsql.googlecode.com/files/sepgsql-02-core-8.4-r2096.patch
03) http://sepgsql.googlecode.com/files/sepgsql-03-gram-8.4-r2096.patch
04) http://sepgsql.googlecode.com/files/sepgsql-04-writable-8.4-r2096.patch
05) http://sepgsql.googlecode.com/files/sepgsql-05-rowlevel-8.4-r2096.patch
06) http://sepgsql.googlecode.com/files/sepgsql-06-perms-8.4-r2096.patch
07) http://sepgsql.googlecode.com/files/sepgsql-07-extra-8.4-r2096.patch
08) http://sepgsql.googlecode.com/files/sepgsql-08-utils-8.4-r2096.patch
09) http://sepgsql.googlecode.com/files/sepgsql-09-tests-8.4-r2096.patch
10) http://sepgsql.googlecode.com/files/sepgsql-10-docs-8.4-r2096.patch

The SE-PostgreSQL online documentation: http://wiki.postgresql.org/wiki/SEPostgreSQL

The purpose of every patches are introduced at: http://wiki.postgresql.org/wiki/SEPostgreSQL_Development#Patches

List of updates:
* Its base version was updated to the latest CVS HEAD.
* The third gram patch is a separated part from the second core patch to reduce the scale of the patch. It provides
extensionsin SQL grammer, such as SECURITY_LABEL = 'xxx'.
 
* bugfix: it didn't prevent accesses when user tries to select pg_toast.pg_toast_%u by hand.

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


Re: [PATCH] SE-PostgreSQL Updates rev.2096

From
Robert Haas
Date:
2009/6/30 KaiGai Kohei <kaigai@ak.jp.nec.com>:
> The SE-PostgreSQL patches are updated as follows:
>
> 01) http://sepgsql.googlecode.com/files/sepgsql-01-sysatt-8.4-r2096.patch
> 02) http://sepgsql.googlecode.com/files/sepgsql-02-core-8.4-r2096.patch
> 03) http://sepgsql.googlecode.com/files/sepgsql-03-gram-8.4-r2096.patch
> 04) http://sepgsql.googlecode.com/files/sepgsql-04-writable-8.4-r2096.patch
> 05) http://sepgsql.googlecode.com/files/sepgsql-05-rowlevel-8.4-r2096.patch
> 06) http://sepgsql.googlecode.com/files/sepgsql-06-perms-8.4-r2096.patch
> 07) http://sepgsql.googlecode.com/files/sepgsql-07-extra-8.4-r2096.patch
> 08) http://sepgsql.googlecode.com/files/sepgsql-08-utils-8.4-r2096.patch
> 09) http://sepgsql.googlecode.com/files/sepgsql-09-tests-8.4-r2096.patch
> 10) http://sepgsql.googlecode.com/files/sepgsql-10-docs-8.4-r2096.patch

KaiGai,

It appears that some things that you were previously asked to remove
for the first version, like row-level security, have crept back in
here.  I think that there is a clear consensus that what you have here
is too ambitious for a single commit.

http://archives.postgresql.org/message-id/20090311135413.GG4009@alvh.no-ip.org
http://archives.postgresql.org/message-id/336.1236792143@sss.pgh.pa.us

To put some numbers around this:

$ cat *.patch | diffstat | tail -1219 files changed, 11819 insertions(+), 29 deletions(-), 857 modifications(!)

For comparison:

Infrastructure Changes for Recovery8 files changed, 912 insertions(+), 183 deletions(-)
Window Functions92 files changed, 6631 insertions(+), 232 deletions(-)
WITH/WITH RECURSIVE77 files changed, 5826 insertions(+), 246 deletions(-)

Based on that, it looks to me like you should really aim to have a
patch set that changes AT MOST 5-6K lines, and less would be improve
your odds of having something accepted.  You can always submit
follow-on patches once the basic implementation is in, but I think I'm
reflecting the shared view of those who have looked at this in the
past when I say that it's never going to get committed in this form.

Just to be clear, the fact that you have split this up into multiple,
separate patch FILES is of no value at all, because they are
interdependent.  For example, I just took a quick look at the "01"
patch and it obviously includes code that is only necessary for
row-level security.  Nothing is going to get committed here unless it
is a self-contained change that does not presume that there will be
further commits in the future.

Unless this is resubmitted in a form that complies with the changes
that were requested the last time it was reviewed, there is really no
point in reviewing it again.

Thanks,

...Robert


Re: [PATCH] SE-PostgreSQL Updates rev.2096

From
KaiGai Kohei
Date:
Robert Haas wrote:
> 2009/6/30 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>> The SE-PostgreSQL patches are updated as follows:
>>
>> 01) http://sepgsql.googlecode.com/files/sepgsql-01-sysatt-8.4-r2096.patch
>> 02) http://sepgsql.googlecode.com/files/sepgsql-02-core-8.4-r2096.patch
>> 03) http://sepgsql.googlecode.com/files/sepgsql-03-gram-8.4-r2096.patch
>> 04) http://sepgsql.googlecode.com/files/sepgsql-04-writable-8.4-r2096.patch
>> 05) http://sepgsql.googlecode.com/files/sepgsql-05-rowlevel-8.4-r2096.patch
>> 06) http://sepgsql.googlecode.com/files/sepgsql-06-perms-8.4-r2096.patch
>> 07) http://sepgsql.googlecode.com/files/sepgsql-07-extra-8.4-r2096.patch
>> 08) http://sepgsql.googlecode.com/files/sepgsql-08-utils-8.4-r2096.patch
>> 09) http://sepgsql.googlecode.com/files/sepgsql-09-tests-8.4-r2096.patch
>> 10) http://sepgsql.googlecode.com/files/sepgsql-10-docs-8.4-r2096.patch
> 
> KaiGai,
> 
> It appears that some things that you were previously asked to remove
> for the first version, like row-level security, have crept back in
> here.  I think that there is a clear consensus that what you have here
> is too ambitious for a single commit.

Robert,

The row-level security is added at the fifth patch.
If you apply only the first three patches, it performs without row-level
security version.

I don't believe all the 10 patches get commited at the first commit fest.
If preferable, I can agree to forcus on the first three patches on the first
commit fest, according to the step-by-step approach, previously suggested.
 01 ... It provides a facility to manage security labels of database obejcts. 02 ... It provides a core access control
stuffincluding communication with kernel. 03 ... It provides SECURITY_LABEL option in some of CREATE/ALTER statement.
 

> http://archives.postgresql.org/message-id/20090311135413.GG4009@alvh.no-ip.org
> http://archives.postgresql.org/message-id/336.1236792143@sss.pgh.pa.us
> 
> To put some numbers around this:
> 
> $ cat *.patch | diffstat | tail -1
>  219 files changed, 11819 insertions(+), 29 deletions(-), 857 modifications(!)

Some of files are parched by multiple patches.

It is more correct estimation.  $ diffstat sepgsql-00-full-8.4.0-r2096.patch.gz  165 files changed, 11788
insertions(+),2 deletions(-), 657 modifications(!)
 

> For comparison:
> 
> Infrastructure Changes for Recovery
>  8 files changed, 912 insertions(+), 183 deletions(-)
> Window Functions
>  92 files changed, 6631 insertions(+), 232 deletions(-)
> WITH/WITH RECURSIVE
>  77 files changed, 5826 insertions(+), 246 deletions(-)
> 
> Based on that, it looks to me like you should really aim to have a
> patch set that changes AT MOST 5-6K lines, and less would be improve
> your odds of having something accepted.  You can always submit
> follow-on patches once the basic implementation is in, but I think I'm
> reflecting the shared view of those who have looked at this in the
> past when I say that it's never going to get committed in this form.

Scale of the first three patches is as follows:
$ diffstat sepgsql-01-sysatt-8.4.0-r2095.patch \           sepgsql-02-core-8.4.0-r2095.patch \
sepgsql-03-gram-8.4.0-r2095.patch
110 files changed, 5162 insertions(+), 2 deletions(-), 308 modifications(!)

The SE-PostgreSQL without row-level security version changes about 5-6K lines.
I can agree it may be a maximum scale in a single commit fest.

> Just to be clear, the fact that you have split this up into multiple,
> separate patch FILES is of no value at all, because they are
> interdependent.  For example, I just took a quick look at the "01"
> patch and it obviously includes code that is only necessary for
> row-level security.  Nothing is going to get committed here unless it
> is a self-contained change that does not presume that there will be
> further commits in the future.

I considered the separated patches reflects the step-by-step approach
previously suggested. At least, the minimum SE-PostgreSQL can works
with the first two patches.
The purpose of 01 patch is to provides a facility to manage security
labels of any database objects, including tables, columns and so on.
However, indeed, I could find a part only necessary for row-level stuff,
such as definitions of "security_labels" and "security_acl".
I can agree to move them to the later patch.

> Unless this is resubmitted in a form that complies with the changes
> that were requested the last time it was reviewed, there is really no
> point in reviewing it again.

Right now, I think it is preferable to focus on the first three patches
at the first commit fest.

What's your opinion?
-- 
KaiGai Kohei <kaigai@kaigai.gr.jp>


Re: [PATCH] SE-PostgreSQL Updates rev.2096

From
Robert Haas
Date:
On Thu, Jul 2, 2009 at 10:45 PM, KaiGai Kohei<kaigai@kaigai.gr.jp> wrote:
> Right now, I think it is preferable to focus on the first three patches
> at the first commit fest.
>
> What's your opinion?

If the first three patches are the least amount of code that makes a
complete feature, then that's the right way to go.  Please remove
everything from those patches that isn't part of the basic feature set
(e.g. row-level security) and repost the updated versions of just
those patches, so that it is clear which code we are reviewing.  Maybe
call it SE-pgsql lite, or whatever.

Thanks,

...Robert


Re: [PATCH] SE-PostgreSQL Updates rev.2096

From
KaiGai Kohei
Date:
Robert Haas wrote:
> On Thu, Jul 2, 2009 at 10:45 PM, KaiGai Kohei<kaigai@kaigai.gr.jp> wrote:
>> Right now, I think it is preferable to focus on the first three patches
>> at the first commit fest.
>>
>> What's your opinion?
> 
> If the first three patches are the least amount of code that makes a
> complete feature, then that's the right way to go.  Please remove
> everything from those patches that isn't part of the basic feature set
> (e.g. row-level security) and repost the updated versions of just
> those patches, so that it is clear which code we are reviewing.  Maybe
> call it SE-pgsql lite, or whatever.

OK, I'll re-organize my patch set.
Please wait for a few days.

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


Re: [PATCH] SE-PostgreSQL Updates rev.2096

From
Robert Haas
Date:
On Fri, Jul 3, 2009 at 1:18 AM, KaiGai Kohei<kaigai@kaigai.gr.jp> wrote:
> OK, I'll re-organize my patch set.
> Please wait for a few days.

Note that your patch will need to include docs and regression test
updates, and those things need to be coherent with the rest of the
patch.  You can't submit a subset of the patches that implement 25% of
the feature set you ultimately want to have, and docs that describe
the entire feature set as if it's already all committed.

...Robert


[PATCH] SE-PgSQL/lite rev.2163

From
KaiGai Kohei
Date:
The SE-PostgreSQL patches are updated as follows:

[1/5] http://sepgsql.googlecode.com/files/sepgsql-01-sysatt-8.5devel-r2163.patch
[2/5] http://sepgsql.googlecode.com/files/sepgsql-02-core-8.5devel-r2163.patch
[3/5] http://sepgsql.googlecode.com/files/sepgsql-03-gram-8.5devel-r2163.patch
[4/5] http://sepgsql.googlecode.com/files/sepgsql-04-tests-8.5devel-r2163.patch
[5/5] http://sepgsql.googlecode.com/files/sepgsql-05-docs-8.5devel-r2163.patch

List of updates:
* Patch set was organized to a few ones which provides only core features.
* Code base was upgraded to the latest CVS HEAD.
* Some of features in the fullset edition were separated, to focus on the core feature of SE-PostgreSQL at the first
commitfest.
 

The full functional SE-PostgreSQL consists of ten patches sequentially
numbered. The patch with smaller number provide more fundamental features.
Robert Haas suggested we should focus on a part of patches on the first
commit fest, because all the patch set of SE-PostgreSQL is a bit large
to review within a single commit fest. I agreed and reorganized my patches.
Some of advanced features (such as row-level controls) are separated
from the features to be focused on the 1st commit fest.
I decided to call the small functional SE-PostgreSQL as SE-PgSQL/lite
to make clear what we discuss on.

The SE-PgSQL/lite contains the following features.
* Management of the security labels (1st patch)SELinux's security model requires all the subjects and objects are
labeled.Itenables to assign a certain security label on several kinds of databaseobjects. The security label in text
formis stored within the new systemcatalog (pg_security), and the catalog/pg_security.c provides a facilityto translate
itand the security identifier.
 

* Core facility to communicate with in-kernel SELinux and to make its decision on various kind of database objcets.
(2ndpatch)The second patch provides the core functionality to perform with SELinux.It deploys security hooks on the
strategicpoints of PostgreSQL.The hooks invoke SE-PostgreSQL routines, when it is enabled. The routinesmakes its
decisionbased on the system's security policy.The userspace access vector cache
(src/backend/security/sepgsql/avc.c)minimizesthe number of kernel space invocations, and enables to makea decision
(previouslyasked) without context switching.The routines of security hooks (hooks.c and checker.c) pulls the
securitylabelof given database objects like a table, and asks the userspace AVCwhether the required accesses to be
allowed,or not.If denied, it returns an error status or raises error using ereport().
 

* SQL Extentions (3rd patch)When we create a database object, a default security label shall be givenbased on the
securitypolicy. But we can give an explicit security labelfor a new object, as far as user is allowed to create it with
thegivensecurity label.This patch provide SECURITY_LABEL = '...' option for several kinds ofCREATE or ALTER statement.
Itallows users to create database, schemas,tables, columns and procedures with a specified security label.
 

* Documentation patch (current 4th patch)It patches src/doc/sgml/*. Any descriptions corresponding to the
row-levelaccesscontrols and other upcoming features were separated.
 

* Test cases patch (current 5th patch)It provides test cases for SE-PgSQL/lite.

The SE-PgSQL/lite does NOT contain the following features, currently.

The row-level access controls provided by the 5th patch was separated from
the SE-PgSQL/lite. In addition, the writable system column support needed
by row-level controls provided by the 4th patch was also separated.
Some persons complained deployment of security hooks seem like row level
controls, such as sepgsqlHeapTupleInsert() from simple_heap_insert().
It was also separated from the SE-PgSQL/lite, and it checks permissions
outside of the simple_heap_insert(). For example, SE-PgSQL/lite put its
hook (sepgsqlCheckTableCreate()) on the DefineRelation() next to the DAC
permission checks. We can also keep completeness of the access controls
as far as security hooks checks all the routes users to create/alter/drop
tables and so on. However, it needed to apply a hardwired policy to prevent
users to modify system catalog by hand, instead of the design changes.

The advanced permission checks (in the 6th patch) were also separated
from the SE-PgSQL/lite. It includes file permission checks on COPY TO/FROM
statements, largeobjects accesses, installation of binary modules.

The functionality to reclaim orphan security labels (in the 7th patch)
was also separated.

Thanks,

-------------------------
FYI, scale of the patches

- sepgsql-01-sysatt-8.5devel-r2163.patch 34 files changed, 723 insertions(+), 69 modifications(!)
- sepgsql-02-core-8.5devel-r2163.patch 54 files changed, 4074 insertions(+), 128 modifications(!) (*) 88% of changesets
arenewlines at backend/security/sepgsql/*     or its header.
 
- sepgsql-03-gram-8.5devel-r2163.patch 25 files changed, 709 insertions(+), 87 modifications(!)
- sepgsql-04-tests-8.5devel-r2163.patch 12 files changed, 1039 insertions(+), 2 modifications(!)
- sepgsql-05-docs-8.5devel-r2163.patch 17 files changed, 1126 insertions(+), 4 modifications(!)

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


Re: [PATCH] SE-PgSQL/lite rev.2163

From
Robert Haas
Date:
2009/7/10 KaiGai Kohei <kaigai@ak.jp.nec.com>:
> The SE-PostgreSQL patches are updated as follows:
>
> [1/5] http://sepgsql.googlecode.com/files/sepgsql-01-sysatt-8.5devel-r2163.patch
> [2/5] http://sepgsql.googlecode.com/files/sepgsql-02-core-8.5devel-r2163.patch
> [3/5] http://sepgsql.googlecode.com/files/sepgsql-03-gram-8.5devel-r2163.patch
> [4/5] http://sepgsql.googlecode.com/files/sepgsql-04-tests-8.5devel-r2163.patch
> [5/5] http://sepgsql.googlecode.com/files/sepgsql-05-docs-8.5devel-r2163.patch
>
> List of updates:
> * Patch set was organized to a few ones which provides only core features.
> * Code base was upgraded to the latest CVS HEAD.
> * Some of features in the fullset edition were separated, to focus on
>  the core feature of SE-PostgreSQL at the first commit fest.

I took a look at this a little bit.  It looks as if you are still
treating the Security Label as a special attribute of the tuple, which
seems completely unnecessary given that this patch set is not
attempting to implement row-level security.  It seems to me that all
you should need is regular old columns pg_class.relseclabel,
pg_attribute.attseclabel, etc; it also seems to me that would simplify
the code.

As a general comment, I don't have much confidence that your original
design for row-level security is a good one.  I think that needs to be
thought about very carefully before anything is implemented.  I note
that your original design called for a system catalog lookup on each
row returned, which is basically saying that all security-label
filtering will be implemented as a nested loop with inner index scan.
It seems to me that if we ever implement row-level security (which is
far from a sure thing) we're certainly going to want to allow the
planner to make other decisions, such as sorting the tuples by
security label and performing a merge join against the pg_security
catalog, or hashing the pg_security catalog and performing a hash
join, or using an index on the security label column to perform a
bitmap index scan, or any other technique that the planner already
knows how to consider.  I say all this not to encourage you to spend
more time on row-level security now (because I think that would be
premature) but to encourage you to abandon all the design decisions in
this patch that are presuming a particular design for row-level
security and focus on making the features that are actually in this
patch set as lean and robust as possible.

Another problem that I have with this patch set is that it STILL
doesn't have parity with the DAC permissions scheme (despite previous
requests to make it have parity).  For example, you're checking
privileges like db_column:{drop}, which have no analogue in our
current permissions scheme.  I think this has been discussed several
times before, and it seems that you still haven't chosen to fully take
that advice, which I suspect is going to be an absolute bar to getting
this committed.  (I am not a committer, of course, so take whatever I
say with a grain of salt, but that's my opinion for what it's worth.)
It seems to me that if you have REAL parity with the existing
permissions scheme, it shouldn't be necessary to add additional,
separate sepgsql checks in every place currently has a standard
permissions check.  Instead, you should be able to put all of the
logic into functions like pg_class_aclmask().  This will be:

- Less code.
- Easier to maintain.

With the current design, every time someone makes a change to how DAC
permissions are checked, they have to think about the proper sepgsql
treatment as well.  That seems like a recipe for security bugs.

...Robert


Re: [PATCH] SE-PgSQL/lite rev.2163

From
KaiGai Kohei
Date:
Robert, thanks for your comments.

Robert Haas wrote:
> 2009/7/10 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>> The SE-PostgreSQL patches are updated as follows:
>>
>> [1/5] http://sepgsql.googlecode.com/files/sepgsql-01-sysatt-8.5devel-r2163.patch
>> [2/5] http://sepgsql.googlecode.com/files/sepgsql-02-core-8.5devel-r2163.patch
>> [3/5] http://sepgsql.googlecode.com/files/sepgsql-03-gram-8.5devel-r2163.patch
>> [4/5] http://sepgsql.googlecode.com/files/sepgsql-04-tests-8.5devel-r2163.patch
>> [5/5] http://sepgsql.googlecode.com/files/sepgsql-05-docs-8.5devel-r2163.patch
>>
>> List of updates:
>> * Patch set was organized to a few ones which provides only core features.
>> * Code base was upgraded to the latest CVS HEAD.
>> * Some of features in the fullset edition were separated, to focus on
>>  the core feature of SE-PostgreSQL at the first commit fest.
> 
> I took a look at this a little bit.  It looks as if you are still
> treating the Security Label as a special attribute of the tuple, which
> seems completely unnecessary given that this patch set is not
> attempting to implement row-level security.  It seems to me that all
> you should need is regular old columns pg_class.relseclabel,
> pg_attribute.attseclabel, etc; it also seems to me that would simplify
> the code.

Are you saying that whole of the pg_security mechanism also should be
postponed to the second commit fest, not only system column's definitions?

> As a general comment, I don't have much confidence that your original
> design for row-level security is a good one.  I think that needs to be
> thought about very carefully before anything is implemented.  I note
> that your original design called for a system catalog lookup on each
> row returned, which is basically saying that all security-label
> filtering will be implemented as a nested loop with inner index scan.
> It seems to me that if we ever implement row-level security (which is
> far from a sure thing) we're certainly going to want to allow the
> planner to make other decisions, such as sorting the tuples by
> security label and performing a merge join against the pg_security
> catalog, or hashing the pg_security catalog and performing a hash
> join, or using an index on the security label column to perform a
> bitmap index scan, or any other technique that the planner already
> knows how to consider.  I say all this not to encourage you to spend
> more time on row-level security now (because I think that would be
> premature) but to encourage you to abandon all the design decisions in
> this patch that are presuming a particular design for row-level
> security and focus on making the features that are actually in this
> patch set as lean and robust as possible.

I'm confusing a bit for the comments.
The row-level access control stuff (which is managed in my repository
now) does not need to look up the pg_security system catalog every time.
I guess you believe SE-PgSQL looks up the system catalog to fetch security
label in text form, and calls in-kernel SELinux to make its decision for
every tuples. However, it is incorrect.

The userspace avc (security/sepgsql/avc.c) routines enable to cache access
control decisions recently used, and return its result for the required
pair of security identifier (not a text form) and type of actions (like
db_table:{select}), if it hits a cache entries.
It means SE-PgSQL can return its decisions using only security identifier
in most cases.

I think what you suggested is useful, if SE-PgSQL needs security label
in text form on making its decision. However, it seems to me your comments
bases on incorrect assumption. Could you point to me, if I incorrectly
understood the intentions of your comments.

> Another problem that I have with this patch set is that it STILL
> doesn't have parity with the DAC permissions scheme (despite previous
> requests to make it have parity).  For example, you're checking
> privileges like db_column:{drop}, which have no analogue in our
> current permissions scheme.  I think this has been discussed several
> times before, and it seems that you still haven't chosen to fully take
> that advice, which I suspect is going to be an absolute bar to getting
> this committed.  (I am not a committer, of course, so take whatever I
> say with a grain of salt, but that's my opinion for what it's worth.)
> It seems to me that if you have REAL parity with the existing
> permissions scheme, it shouldn't be necessary to add additional,
> separate sepgsql checks in every place currently has a standard
> permissions check.  Instead, you should be able to put all of the
> logic into functions like pg_class_aclmask().  This will be:

I moved several security hooks to the pg_xxx_aclmask() because these
permissions to be checked in same places.
However, both of security models also have different definitions.
For example, when we create a new table, dac checks ACL_CREATE on
the namespace (it may be equivalent to db_schema:{add_object}),
but MAC checks db_table:{create} permission on the new table itself
labeled with default or user given one.

For the security hooks we can move to pg_xxx_aclmask(), I can agree
to move them as possible as we can, such as sepgsqlCheclProcedureExecute()
invoked from pg_proc_aclmask(). But, I concluded rest of security hooks
are hard to integrate with pg_xxxx_aclmask() mechanism.

Thanks,

> - Less code.
> - Easier to maintain.
> 
> With the current design, every time someone makes a change to how DAC
> permissions are checked, they have to think about the proper sepgsql
> treatment as well.  That seems like a recipe for security bugs.
> 
> ...Robert

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


Re: [PATCH] SE-PgSQL/lite rev.2163

From
Robert Haas
Date:
2009/7/12 KaiGai Kohei <kaigai@ak.jp.nec.com>:
> Robert, thanks for your comments.
>
> Robert Haas wrote:
>> 2009/7/10 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>>> The SE-PostgreSQL patches are updated as follows:
>>>
>>> [1/5] http://sepgsql.googlecode.com/files/sepgsql-01-sysatt-8.5devel-r2163.patch
>>> [2/5] http://sepgsql.googlecode.com/files/sepgsql-02-core-8.5devel-r2163.patch
>>> [3/5] http://sepgsql.googlecode.com/files/sepgsql-03-gram-8.5devel-r2163.patch
>>> [4/5] http://sepgsql.googlecode.com/files/sepgsql-04-tests-8.5devel-r2163.patch
>>> [5/5] http://sepgsql.googlecode.com/files/sepgsql-05-docs-8.5devel-r2163.patch
>>>
>>> List of updates:
>>> * Patch set was organized to a few ones which provides only core features.
>>> * Code base was upgraded to the latest CVS HEAD.
>>> * Some of features in the fullset edition were separated, to focus on
>>>  the core feature of SE-PostgreSQL at the first commit fest.
>>
>> I took a look at this a little bit.  It looks as if you are still
>> treating the Security Label as a special attribute of the tuple, which
>> seems completely unnecessary given that this patch set is not
>> attempting to implement row-level security.  It seems to me that all
>> you should need is regular old columns pg_class.relseclabel,
>> pg_attribute.attseclabel, etc; it also seems to me that would simplify
>> the code.
>
> Are you saying that whole of the pg_security mechanism also should be
> postponed to the second commit fest, not only system column's definitions?

I'm not sure what you mean by "the whole of the pg_security
mechanism".  But I believe there was negative feedback previously
about the idea of sandwhiching the security label into the tuple
header instead of making it an ordinary (non-system) column in a
system catalog table.

>> As a general comment, I don't have much confidence that your original
>> design for row-level security is a good one.  I think that needs to be
>> thought about very carefully before anything is implemented.  I note
>> that your original design called for a system catalog lookup on each
>> row returned, which is basically saying that all security-label
>> filtering will be implemented as a nested loop with inner index scan.
>> It seems to me that if we ever implement row-level security (which is
>> far from a sure thing) we're certainly going to want to allow the
>> planner to make other decisions, such as sorting the tuples by
>> security label and performing a merge join against the pg_security
>> catalog, or hashing the pg_security catalog and performing a hash
>> join, or using an index on the security label column to perform a
>> bitmap index scan, or any other technique that the planner already
>> knows how to consider.  I say all this not to encourage you to spend
>> more time on row-level security now (because I think that would be
>> premature) but to encourage you to abandon all the design decisions in
>> this patch that are presuming a particular design for row-level
>> security and focus on making the features that are actually in this
>> patch set as lean and robust as possible.
>
> I'm confusing a bit for the comments.
> The row-level access control stuff (which is managed in my repository
> now) does not need to look up the pg_security system catalog every time.
> I guess you believe SE-PgSQL looks up the system catalog to fetch security
> label in text form, and calls in-kernel SELinux to make its decision for
> every tuples. However, it is incorrect.
>
> The userspace avc (security/sepgsql/avc.c) routines enable to cache access
> control decisions recently used, and return its result for the required
> pair of security identifier (not a text form) and type of actions (like
> db_table:{select}), if it hits a cache entries.
> It means SE-PgSQL can return its decisions using only security identifier
> in most cases.

Perhaps you're not making a kernel call for each row (good!), but I
think you are still assuming that you're going to fetch the rows
first, without any sepgsql-specific decision making, and then perform
an operation of some kind on each row to see whether to filter it.  If
so, what I'm saying is that's bad.

Suppose we have two security contexts A and B, and two users Alice and
Bob.  Alice can see only tuples in security context A, and Bob can see
only tuples in security context B.  If I create an index on the
security ID of a table with row-level security, (a) then will it work?
and (b) if one of the users issues a command like "select * for
table", will the planner consider a bitmap-index-scan using that index
rather than a sequential scan of the entire table?

> I think what you suggested is useful, if SE-PgSQL needs security label
> in text form on making its decision. However, it seems to me your comments
> bases on incorrect assumption. Could you point to me, if I incorrectly
> understood the intentions of your comments.
>
>> Another problem that I have with this patch set is that it STILL
>> doesn't have parity with the DAC permissions scheme (despite previous
>> requests to make it have parity).  For example, you're checking
>> privileges like db_column:{drop}, which have no analogue in our
>> current permissions scheme.  I think this has been discussed several
>> times before, and it seems that you still haven't chosen to fully take
>> that advice, which I suspect is going to be an absolute bar to getting
>> this committed.  (I am not a committer, of course, so take whatever I
>> say with a grain of salt, but that's my opinion for what it's worth.)
>> It seems to me that if you have REAL parity with the existing
>> permissions scheme, it shouldn't be necessary to add additional,
>> separate sepgsql checks in every place currently has a standard
>> permissions check.  Instead, you should be able to put all of the
>> logic into functions like pg_class_aclmask().  This will be:
>
> I moved several security hooks to the pg_xxx_aclmask() because these
> permissions to be checked in same places.
> However, both of security models also have different definitions.
> For example, when we create a new table, dac checks ACL_CREATE on
> the namespace (it may be equivalent to db_schema:{add_object}),
> but MAC checks db_table:{create} permission on the new table itself
> labeled with default or user given one.

So why do it differently?  I don't see why you have to invent a whole
new paradigm here.

> For the security hooks we can move to pg_xxx_aclmask(), I can agree
> to move them as possible as we can, such as sepgsqlCheclProcedureExecute()
> invoked from pg_proc_aclmask(). But, I concluded rest of security hooks
> are hard to integrate with pg_xxxx_aclmask() mechanism.

...Robert


Re: [PATCH] SE-PgSQL/lite rev.2163

From
KaiGai Kohei
Date:
Robert Haas wrote:
> 2009/7/12 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>> Robert, thanks for your comments.
>>
>> Robert Haas wrote:
>>> 2009/7/10 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>>>> The SE-PostgreSQL patches are updated as follows:
>>>>
>>>> [1/5] http://sepgsql.googlecode.com/files/sepgsql-01-sysatt-8.5devel-r2163.patch
>>>> [2/5] http://sepgsql.googlecode.com/files/sepgsql-02-core-8.5devel-r2163.patch
>>>> [3/5] http://sepgsql.googlecode.com/files/sepgsql-03-gram-8.5devel-r2163.patch
>>>> [4/5] http://sepgsql.googlecode.com/files/sepgsql-04-tests-8.5devel-r2163.patch
>>>> [5/5] http://sepgsql.googlecode.com/files/sepgsql-05-docs-8.5devel-r2163.patch
>>>>
>>>> List of updates:
>>>> * Patch set was organized to a few ones which provides only core features.
>>>> * Code base was upgraded to the latest CVS HEAD.
>>>> * Some of features in the fullset edition were separated, to focus on
>>>>  the core feature of SE-PostgreSQL at the first commit fest.
>>> I took a look at this a little bit.  It looks as if you are still
>>> treating the Security Label as a special attribute of the tuple, which
>>> seems completely unnecessary given that this patch set is not
>>> attempting to implement row-level security.  It seems to me that all
>>> you should need is regular old columns pg_class.relseclabel,
>>> pg_attribute.attseclabel, etc; it also seems to me that would simplify
>>> the code.
>> Are you saying that whole of the pg_security mechanism also should be
>> postponed to the second commit fest, not only system column's definitions?
> 
> I'm not sure what you mean by "the whole of the pg_security
> mechanism".  But I believe there was negative feedback previously
> about the idea of sandwhiching the security label into the tuple
> header instead of making it an ordinary (non-system) column in a
> system catalog table.

It means the mechanism to translate a security identifier and its
text representation each other.
The reason why I separated the pg_security mechanism was to reduce
scale of the patch, because we agreed to postpone row-level stuffs
in the v8.4 development cycle. Please note that only four relations
needed to have a capability to assign security labels (pg_database,
pg_class, pg_attribute and pg_proc).
However, when row-level security is available, consumption of the
storage by security labels in raw-text cannot be ignored, although
massive number of database objects shares limited number of security
labels.

>>> As a general comment, I don't have much confidence that your original
>>> design for row-level security is a good one.  I think that needs to be
>>> thought about very carefully before anything is implemented.  I note
>>> that your original design called for a system catalog lookup on each
>>> row returned, which is basically saying that all security-label
>>> filtering will be implemented as a nested loop with inner index scan.
>>> It seems to me that if we ever implement row-level security (which is
>>> far from a sure thing) we're certainly going to want to allow the
>>> planner to make other decisions, such as sorting the tuples by
>>> security label and performing a merge join against the pg_security
>>> catalog, or hashing the pg_security catalog and performing a hash
>>> join, or using an index on the security label column to perform a
>>> bitmap index scan, or any other technique that the planner already
>>> knows how to consider.  I say all this not to encourage you to spend
>>> more time on row-level security now (because I think that would be
>>> premature) but to encourage you to abandon all the design decisions in
>>> this patch that are presuming a particular design for row-level
>>> security and focus on making the features that are actually in this
>>> patch set as lean and robust as possible.
>> I'm confusing a bit for the comments.
>> The row-level access control stuff (which is managed in my repository
>> now) does not need to look up the pg_security system catalog every time.
>> I guess you believe SE-PgSQL looks up the system catalog to fetch security
>> label in text form, and calls in-kernel SELinux to make its decision for
>> every tuples. However, it is incorrect.
>>
>> The userspace avc (security/sepgsql/avc.c) routines enable to cache access
>> control decisions recently used, and return its result for the required
>> pair of security identifier (not a text form) and type of actions (like
>> db_table:{select}), if it hits a cache entries.
>> It means SE-PgSQL can return its decisions using only security identifier
>> in most cases.
> 
> Perhaps you're not making a kernel call for each row (good!), but I
> think you are still assuming that you're going to fetch the rows
> first, without any sepgsql-specific decision making, and then perform
> an operation of some kind on each row to see whether to filter it.  If
> so, what I'm saying is that's bad.
> 
> Suppose we have two security contexts A and B, and two users Alice and
> Bob.  Alice can see only tuples in security context A, and Bob can see
> only tuples in security context B.  If I create an index on the
> security ID of a table with row-level security, (a) then will it work?
> and (b) if one of the users issues a command like "select * for
> table", will the planner consider a bitmap-index-scan using that index
> rather than a sequential scan of the entire table?

In the current implementation, the index should be used, if user refers
the "security_label" system column as a part of query, such as:
 SELECT * FROM t1 WHERE security_label = 'A';

In this case, tuples are fetched with index scan based on text comparison
at first, then SE-PgSQL checks the security label of the fetched tuple
to determine whether it should be filtered, or not.
It is hooked at the ExecScan(), and performs as if a volatile function
is appended at WHERE condition.

However, the (current) planner does not apply indexes for the (b) case.
If the SeqScan is selected, SE-PgSQL also applies its controls on the
fetched tuples.

>> I think what you suggested is useful, if SE-PgSQL needs security label
>> in text form on making its decision. However, it seems to me your comments
>> bases on incorrect assumption. Could you point to me, if I incorrectly
>> understood the intentions of your comments.
>>
>>> Another problem that I have with this patch set is that it STILL
>>> doesn't have parity with the DAC permissions scheme (despite previous
>>> requests to make it have parity).  For example, you're checking
>>> privileges like db_column:{drop}, which have no analogue in our
>>> current permissions scheme.  I think this has been discussed several
>>> times before, and it seems that you still haven't chosen to fully take
>>> that advice, which I suspect is going to be an absolute bar to getting
>>> this committed.  (I am not a committer, of course, so take whatever I
>>> say with a grain of salt, but that's my opinion for what it's worth.)
>>> It seems to me that if you have REAL parity with the existing
>>> permissions scheme, it shouldn't be necessary to add additional,
>>> separate sepgsql checks in every place currently has a standard
>>> permissions check.  Instead, you should be able to put all of the
>>> logic into functions like pg_class_aclmask().  This will be:
>> I moved several security hooks to the pg_xxx_aclmask() because these
>> permissions to be checked in same places.
>> However, both of security models also have different definitions.
>> For example, when we create a new table, dac checks ACL_CREATE on
>> the namespace (it may be equivalent to db_schema:{add_object}),
>> but MAC checks db_table:{create} permission on the new table itself
>> labeled with default or user given one.
> 
> So why do it differently?  I don't see why you have to invent a whole
> new paradigm here.

The paradigm is not my inventment. It just follows the security model
in SELinux. It assigns its security label on the managed object (such
as files, sockets, processes, ipc objects and so on) on their creation
time, and checks user's privileges to create it labeled.

From viewpoint of the system-wide consistency in access controls,
we should not ignore SELinux's manner which provides security policy.

The filesystem DAC and MAC mechanism is a good analogy.
When we create a file under the directoly, the kernel checks "w" permission
of the directoly. In addition, SELinux checks dir:{add_name} permission on
the directoly and file:{create} permission on the newly created file with
a certain security label.
A part of security hooks are called from the DAC check routines, but rest
of them are called from other strategic points, due to the differences
in security models.

I never oppose to move security hooks into pg_xxx_aclcheck() routines
as far as we can define one-to-oen mapping between DAC and MAC.
However, in some cases, we cannot implement the security model correctly.

Thanks,

>> For the security hooks we can move to pg_xxx_aclmask(), I can agree
>> to move them as possible as we can, such as sepgsqlCheclProcedureExecute()
>> invoked from pg_proc_aclmask(). But, I concluded rest of security hooks
>> are hard to integrate with pg_xxxx_aclmask() mechanism.
> 
> ...Robert

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


Re: [PATCH] SE-PgSQL/lite rev.2163

From
KaiGai Kohei
Date:
Robert,

The sepgsql/avc.c provides a facility to cache access control decisions
in userspace, and enables to reduce time of kernel invocations.
However, its size is the largest one in the SE-PgSQL patch.
[kaigai@saba gram]$ wc -l src/backend/security/sepgsql/avc.c829 src/backend/security/sepgsql/avc.c

I think separation of the avc facility contributes to keep code
size smaller, rather than pg_security mechanism.
[kaigai@saba gram]$ wc -l src/backend/catalog/pg_security.c381 src/backend/catalog/pg_security.c

I guess it enables to reduce 20% of patch scale. (800L/4KL)

I can also add more source code comments, such as when the security
hooks to be invoked, what is the purpose and so on, instead of ruduced
lines. It will make clear where the hooks to be placed.

Thanks,

KaiGai Kohei wrote:
> Robert Haas wrote:
>> 2009/7/12 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>>> Robert, thanks for your comments.
>>>
>>> Robert Haas wrote:
>>>> 2009/7/10 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>>>>> The SE-PostgreSQL patches are updated as follows:
>>>>>
>>>>> [1/5] http://sepgsql.googlecode.com/files/sepgsql-01-sysatt-8.5devel-r2163.patch
>>>>> [2/5] http://sepgsql.googlecode.com/files/sepgsql-02-core-8.5devel-r2163.patch
>>>>> [3/5] http://sepgsql.googlecode.com/files/sepgsql-03-gram-8.5devel-r2163.patch
>>>>> [4/5] http://sepgsql.googlecode.com/files/sepgsql-04-tests-8.5devel-r2163.patch
>>>>> [5/5] http://sepgsql.googlecode.com/files/sepgsql-05-docs-8.5devel-r2163.patch
>>>>>
>>>>> List of updates:
>>>>> * Patch set was organized to a few ones which provides only core features.
>>>>> * Code base was upgraded to the latest CVS HEAD.
>>>>> * Some of features in the fullset edition were separated, to focus on
>>>>>  the core feature of SE-PostgreSQL at the first commit fest.
>>>> I took a look at this a little bit.  It looks as if you are still
>>>> treating the Security Label as a special attribute of the tuple, which
>>>> seems completely unnecessary given that this patch set is not
>>>> attempting to implement row-level security.  It seems to me that all
>>>> you should need is regular old columns pg_class.relseclabel,
>>>> pg_attribute.attseclabel, etc; it also seems to me that would simplify
>>>> the code.
>>> Are you saying that whole of the pg_security mechanism also should be
>>> postponed to the second commit fest, not only system column's definitions?
>> I'm not sure what you mean by "the whole of the pg_security
>> mechanism".  But I believe there was negative feedback previously
>> about the idea of sandwhiching the security label into the tuple
>> header instead of making it an ordinary (non-system) column in a
>> system catalog table.
> 
> It means the mechanism to translate a security identifier and its
> text representation each other.
> The reason why I separated the pg_security mechanism was to reduce
> scale of the patch, because we agreed to postpone row-level stuffs
> in the v8.4 development cycle. Please note that only four relations
> needed to have a capability to assign security labels (pg_database,
> pg_class, pg_attribute and pg_proc).
> However, when row-level security is available, consumption of the
> storage by security labels in raw-text cannot be ignored, although
> massive number of database objects shares limited number of security
> labels.
> 
>>>> As a general comment, I don't have much confidence that your original
>>>> design for row-level security is a good one.  I think that needs to be
>>>> thought about very carefully before anything is implemented.  I note
>>>> that your original design called for a system catalog lookup on each
>>>> row returned, which is basically saying that all security-label
>>>> filtering will be implemented as a nested loop with inner index scan.
>>>> It seems to me that if we ever implement row-level security (which is
>>>> far from a sure thing) we're certainly going to want to allow the
>>>> planner to make other decisions, such as sorting the tuples by
>>>> security label and performing a merge join against the pg_security
>>>> catalog, or hashing the pg_security catalog and performing a hash
>>>> join, or using an index on the security label column to perform a
>>>> bitmap index scan, or any other technique that the planner already
>>>> knows how to consider.  I say all this not to encourage you to spend
>>>> more time on row-level security now (because I think that would be
>>>> premature) but to encourage you to abandon all the design decisions in
>>>> this patch that are presuming a particular design for row-level
>>>> security and focus on making the features that are actually in this
>>>> patch set as lean and robust as possible.
>>> I'm confusing a bit for the comments.
>>> The row-level access control stuff (which is managed in my repository
>>> now) does not need to look up the pg_security system catalog every time.
>>> I guess you believe SE-PgSQL looks up the system catalog to fetch security
>>> label in text form, and calls in-kernel SELinux to make its decision for
>>> every tuples. However, it is incorrect.
>>>
>>> The userspace avc (security/sepgsql/avc.c) routines enable to cache access
>>> control decisions recently used, and return its result for the required
>>> pair of security identifier (not a text form) and type of actions (like
>>> db_table:{select}), if it hits a cache entries.
>>> It means SE-PgSQL can return its decisions using only security identifier
>>> in most cases.
>> Perhaps you're not making a kernel call for each row (good!), but I
>> think you are still assuming that you're going to fetch the rows
>> first, without any sepgsql-specific decision making, and then perform
>> an operation of some kind on each row to see whether to filter it.  If
>> so, what I'm saying is that's bad.
>>
>> Suppose we have two security contexts A and B, and two users Alice and
>> Bob.  Alice can see only tuples in security context A, and Bob can see
>> only tuples in security context B.  If I create an index on the
>> security ID of a table with row-level security, (a) then will it work?
>> and (b) if one of the users issues a command like "select * for
>> table", will the planner consider a bitmap-index-scan using that index
>> rather than a sequential scan of the entire table?
> 
> In the current implementation, the index should be used, if user refers
> the "security_label" system column as a part of query, such as:
> 
>   SELECT * FROM t1 WHERE security_label = 'A';
> 
> In this case, tuples are fetched with index scan based on text comparison
> at first, then SE-PgSQL checks the security label of the fetched tuple
> to determine whether it should be filtered, or not.
> It is hooked at the ExecScan(), and performs as if a volatile function
> is appended at WHERE condition.
> 
> However, the (current) planner does not apply indexes for the (b) case.
> If the SeqScan is selected, SE-PgSQL also applies its controls on the
> fetched tuples.
> 
>>> I think what you suggested is useful, if SE-PgSQL needs security label
>>> in text form on making its decision. However, it seems to me your comments
>>> bases on incorrect assumption. Could you point to me, if I incorrectly
>>> understood the intentions of your comments.
>>>
>>>> Another problem that I have with this patch set is that it STILL
>>>> doesn't have parity with the DAC permissions scheme (despite previous
>>>> requests to make it have parity).  For example, you're checking
>>>> privileges like db_column:{drop}, which have no analogue in our
>>>> current permissions scheme.  I think this has been discussed several
>>>> times before, and it seems that you still haven't chosen to fully take
>>>> that advice, which I suspect is going to be an absolute bar to getting
>>>> this committed.  (I am not a committer, of course, so take whatever I
>>>> say with a grain of salt, but that's my opinion for what it's worth.)
>>>> It seems to me that if you have REAL parity with the existing
>>>> permissions scheme, it shouldn't be necessary to add additional,
>>>> separate sepgsql checks in every place currently has a standard
>>>> permissions check.  Instead, you should be able to put all of the
>>>> logic into functions like pg_class_aclmask().  This will be:
>>> I moved several security hooks to the pg_xxx_aclmask() because these
>>> permissions to be checked in same places.
>>> However, both of security models also have different definitions.
>>> For example, when we create a new table, dac checks ACL_CREATE on
>>> the namespace (it may be equivalent to db_schema:{add_object}),
>>> but MAC checks db_table:{create} permission on the new table itself
>>> labeled with default or user given one.
>> So why do it differently?  I don't see why you have to invent a whole
>> new paradigm here.
> 
> The paradigm is not my inventment. It just follows the security model
> in SELinux. It assigns its security label on the managed object (such
> as files, sockets, processes, ipc objects and so on) on their creation
> time, and checks user's privileges to create it labeled.
> 
>>From viewpoint of the system-wide consistency in access controls,
> we should not ignore SELinux's manner which provides security policy.
> 
> The filesystem DAC and MAC mechanism is a good analogy.
> When we create a file under the directoly, the kernel checks "w" permission
> of the directoly. In addition, SELinux checks dir:{add_name} permission on
> the directoly and file:{create} permission on the newly created file with
> a certain security label.
> A part of security hooks are called from the DAC check routines, but rest
> of them are called from other strategic points, due to the differences
> in security models.
> 
> I never oppose to move security hooks into pg_xxx_aclcheck() routines
> as far as we can define one-to-oen mapping between DAC and MAC.
> However, in some cases, we cannot implement the security model correctly.
> 
> Thanks,
> 
>>> For the security hooks we can move to pg_xxx_aclmask(), I can agree
>>> to move them as possible as we can, such as sepgsqlCheclProcedureExecute()
>>> invoked from pg_proc_aclmask(). But, I concluded rest of security hooks
>>> are hard to integrate with pg_xxxx_aclmask() mechanism.
>> ...Robert
> 


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


Re: [PATCH] SE-PgSQL/lite rev.2163

From
Robert Haas
Date:
2009/7/13 KaiGai Kohei <kaigai@ak.jp.nec.com>:
> The sepgsql/avc.c provides a facility to cache access control decisions
> in userspace, and enables to reduce time of kernel invocations.
> However, its size is the largest one in the SE-PgSQL patch.

I think that caching access control decisions in userspace is a good
thing and I don't think you should remove it.  What I'm concerned
about is the fact that you're making the security ID part of the heap
tuple header, like OID, rather than a normal column.  These are two
separate issues.

>  [kaigai@saba gram]$ wc -l src/backend/security/sepgsql/avc.c
>  829 src/backend/security/sepgsql/avc.c
>
> I think separation of the avc facility contributes to keep code
> size smaller, rather than pg_security mechanism.
>
>  [kaigai@saba gram]$ wc -l src/backend/catalog/pg_security.c
>  381 src/backend/catalog/pg_security.c
>
> I guess it enables to reduce 20% of patch scale. (800L/4KL)
>
> I can also add more source code comments, such as when the security
> hooks to be invoked, what is the purpose and so on, instead of ruduced
> lines. It will make clear where the hooks to be placed.

Comments are always good, but I think you're missing the point.  The
point here is that everything I'm pointing out now has been pointed
out in previous review rounds, and you've decide not to do it.  For
example, consider this:

http://archives.postgresql.org/message-id/498030E8.9030905@gmx.net

I call your attention to the following paragraph in particular:

"If I had to do this, I would first write a patch for #1: A patch that
additionally executes existing privilege checks against an SELinux
policy. Existing privilege checks are a well-defined set: they mostly
happen through pg_xxx_aclcheck() functions. Hook your checks in there.This patch would already provide useful
functionality,but it would 
be much easier to review and verify, because we know how permission
checking works in the existing system, so we can compare them. And it
would build confidence among developers and users about the whole
idea, about SELinux integration etc."

Now, here we are five-and-a-half months later, and guess what?  I'm
telling you that you need to put your privilege checks in
pg_xxx_aclcheck() rather than having them scattered all over the code.I didn't even know when I wrote that that Peter
hadmade EXACTLY THE 
SAME SUGGESTION back in January, and judging by his comments,
apparently not for the first time.

You need to decide whether you want to maintain this as a private
patch set forever (in which case you can do whatever you want) or
whether you want to try to create something that is acceptable to the
committers (in which case you need to be responsive to their
feedback).  You've said multiple times that your goal is the latter,
but you've had five months to rework this patch since the last round
of reviewing, and I am now spending my limited reviewing time
rediscovering the same problems that other people have discovered
previously and told you about before.

...Robert


Re: [PATCH] SE-PgSQL/lite rev.2163

From
KaiGai Kohei
Date:
Robert Haas wrote:
> 2009/7/13 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>> The sepgsql/avc.c provides a facility to cache access control decisions
>> in userspace, and enables to reduce time of kernel invocations.
>> However, its size is the largest one in the SE-PgSQL patch.
> 
> I think that caching access control decisions in userspace is a good
> thing and I don't think you should remove it.  What I'm concerned
> about is the fact that you're making the security ID part of the heap
> tuple header, like OID, rather than a normal column.  These are two
> separate issues.

What's your opinion to separate it _at the first stage_?
It can reduce about 800L.

I can implement without security identifiers while we don't have
row level security. I'll update it.
(Please don't count changeset in catalog/pg_proc.h, about 8KL...)

>>  [kaigai@saba gram]$ wc -l src/backend/security/sepgsql/avc.c
>>  829 src/backend/security/sepgsql/avc.c
>>
>> I think separation of the avc facility contributes to keep code
>> size smaller, rather than pg_security mechanism.
>>
>>  [kaigai@saba gram]$ wc -l src/backend/catalog/pg_security.c
>>  381 src/backend/catalog/pg_security.c
>>
>> I guess it enables to reduce 20% of patch scale. (800L/4KL)
>>
>> I can also add more source code comments, such as when the security
>> hooks to be invoked, what is the purpose and so on, instead of ruduced
>> lines. It will make clear where the hooks to be placed.
> 
> Comments are always good, but I think you're missing the point.  The
> point here is that everything I'm pointing out now has been pointed
> out in previous review rounds, and you've decide not to do it.  For
> example, consider this:
> 
> http://archives.postgresql.org/message-id/498030E8.9030905@gmx.net
> 
> I call your attention to the following paragraph in particular:
> 
> "If I had to do this, I would first write a patch for #1: A patch that
> additionally executes existing privilege checks against an SELinux
> policy. Existing privilege checks are a well-defined set: they mostly
> happen through pg_xxx_aclcheck() functions. Hook your checks in there.
>  This patch would already provide useful functionality, but it would
> be much easier to review and verify, because we know how permission
> checking works in the existing system, so we can compare them. And it
> would build confidence among developers and users about the whole
> idea, about SELinux integration etc."

Are you suggesting me to move the hooks into pg_xxx_aclcheck()
_at the first stage_, aren't you?

If so, I can postpone some of permission checks outside of the
pg_xxx_aclcheck(). However, SELinux's security model often
requires different criteria to make its decision.
(Please note that I never say either of them is better or worse.)
Thus, we will need to add security hooks outside of the pg_xxx_aclcheck()
in the future, although it can be done step-by-step.

I guess the following permissions can be checked at the first version.- db_database:{access}   ... equivalent to
ACL_CONNECTon pg_database- db_procedure:{execute} ... equivalent to ACL_EXECUTE on pg_proc- db_schema:{search}     ...
equivalentto ACL_USAGE on pg_namespace
 
- db_database:{superuser} to be called from superuser_arg().  Should it be postponed to the next phase?

BTW, SELinux needs objects to be labeled correctly on its creation time.
At least, we have to put hooks to provide security labels to be assigned
on the DefineRelation() and so on, although it does not check permission
to create the objects.

Thanks,

> Now, here we are five-and-a-half months later, and guess what?  I'm
> telling you that you need to put your privilege checks in
> pg_xxx_aclcheck() rather than having them scattered all over the code.
>  I didn't even know when I wrote that that Peter had made EXACTLY THE
> SAME SUGGESTION back in January, and judging by his comments,
> apparently not for the first time.
> 
> You need to decide whether you want to maintain this as a private
> patch set forever (in which case you can do whatever you want) or
> whether you want to try to create something that is acceptable to the
> committers (in which case you need to be responsive to their
> feedback).  You've said multiple times that your goal is the latter,
> but you've had five months to rework this patch since the last round
> of reviewing, and I am now spending my limited reviewing time
> rediscovering the same problems that other people have discovered
> previously and told you about before.

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


Re: [PATCH] SE-PgSQL/lite rev.2163

From
Robert Haas
Date:
2009/7/14 KaiGai Kohei <kaigai@ak.jp.nec.com>:
> Robert Haas wrote:
>> 2009/7/13 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>>> The sepgsql/avc.c provides a facility to cache access control decisions
>>> in userspace, and enables to reduce time of kernel invocations.
>>> However, its size is the largest one in the SE-PgSQL patch.
>>
>> I think that caching access control decisions in userspace is a good
>> thing and I don't think you should remove it.  What I'm concerned
>> about is the fact that you're making the security ID part of the heap
>> tuple header, like OID, rather than a normal column.  These are two
>> separate issues.
>
> What's your opinion to separate it _at the first stage_?
> It can reduce about 800L.
>
> I can implement without security identifiers while we don't have
> row level security. I'll update it.
> (Please don't count changeset in catalog/pg_proc.h, about 8KL...)

I don't have a strong feeling on it.  As I said before, what I mostly
care about is eliminating the heap tuple header stuff.  I haven't
looked at the patch in enough detail yet to figure out what should
replace it.  If you think that's the best way to go, give it a shot.

>>>  [kaigai@saba gram]$ wc -l src/backend/security/sepgsql/avc.c
>>>  829 src/backend/security/sepgsql/avc.c
>>>
>>> I think separation of the avc facility contributes to keep code
>>> size smaller, rather than pg_security mechanism.
>>>
>>>  [kaigai@saba gram]$ wc -l src/backend/catalog/pg_security.c
>>>  381 src/backend/catalog/pg_security.c
>>>
>>> I guess it enables to reduce 20% of patch scale. (800L/4KL)
>>>
>>> I can also add more source code comments, such as when the security
>>> hooks to be invoked, what is the purpose and so on, instead of ruduced
>>> lines. It will make clear where the hooks to be placed.
>>
>> Comments are always good, but I think you're missing the point.  The
>> point here is that everything I'm pointing out now has been pointed
>> out in previous review rounds, and you've decide not to do it.  For
>> example, consider this:
>>
>> http://archives.postgresql.org/message-id/498030E8.9030905@gmx.net
>>
>> I call your attention to the following paragraph in particular:
>>
>> "If I had to do this, I would first write a patch for #1: A patch that
>> additionally executes existing privilege checks against an SELinux
>> policy. Existing privilege checks are a well-defined set: they mostly
>> happen through pg_xxx_aclcheck() functions. Hook your checks in there.
>>  This patch would already provide useful functionality, but it would
>> be much easier to review and verify, because we know how permission
>> checking works in the existing system, so we can compare them. And it
>> would build confidence among developers and users about the whole
>> idea, about SELinux integration etc."
>
> Are you suggesting me to move the hooks into pg_xxx_aclcheck()
> _at the first stage_, aren't you?
>
> If so, I can postpone some of permission checks outside of the
> pg_xxx_aclcheck(). However, SELinux's security model often
> requires different criteria to make its decision.
> (Please note that I never say either of them is better or worse.)
> Thus, we will need to add security hooks outside of the pg_xxx_aclcheck()
> in the future, although it can be done step-by-step.

Yes: to repeat what has been said multiple times previously, you
should postpone everything that isn't a mirror of the current security
model: there should only be permission checks in places where there
are permissions checks now, and they should be mirror images of the
current DAC checks.

As for the rest, I think you've likely got it backwards: we shouldn't
start by not cluttering the entire code-base with sepgsql permission
checks, and then go and do it later after the basic functionality is
in.  We shouldn't EVER clutter the whole code-base with sepgsql
permission checks.  If we want to expose other sepgsql functionality,
we should do that by putting in more kinds of pg_xxx_aclcheck()-type
calls in other places, and the sepgsql can leverage those call sites
too.  But never mind that, because it's irrelevant for right now: what
you need to do is strip this down to the minimal feature set without
worrying about what will or won't happen later.  Otherwise, nothing is
going to happen at all.

> I guess the following permissions can be checked at the first version.
>  - db_database:{access}   ... equivalent to ACL_CONNECT on pg_database
>  - db_procedure:{execute} ... equivalent to ACL_EXECUTE on pg_proc
>  - db_schema:{search}     ... equivalent to ACL_USAGE on pg_namespace

Why not db_database:{connect} and db_schema:{usage}?  It seems to me
that there is zero value in inventing new names for the same things.

>  - db_database:{superuser} to be called from superuser_arg().
>   Should it be postponed to the next phase?

No, I don't see any reason to postpone that.  That seems analagous to
what we do now, and should be included in the first version, I would
think.

> BTW, SELinux needs objects to be labeled correctly on its creation time.
> At least, we have to put hooks to provide security labels to be assigned
> on the DefineRelation() and so on, although it does not check permission
> to create the objects.

That seems very reasonable to me.

...Robert


Re: [PATCH] SE-PgSQL/lite rev.2163

From
KaiGai Kohei
Date:
Robert Haas wrote:
>> If so, I can postpone some of permission checks outside of the
>> pg_xxx_aclcheck(). However, SELinux's security model often
>> requires different criteria to make its decision.
>> (Please note that I never say either of them is better or worse.)
>> Thus, we will need to add security hooks outside of the pg_xxx_aclcheck()
>> in the future, although it can be done step-by-step.
> 
> Yes: to repeat what has been said multiple times previously, you
> should postpone everything that isn't a mirror of the current security
> model: there should only be permission checks in places where there
> are permissions checks now, and they should be mirror images of the
> current DAC checks.

OK, it seems to me reasonable.

> As for the rest, I think you've likely got it backwards: we shouldn't
> start by not cluttering the entire code-base with sepgsql permission
> checks, and then go and do it later after the basic functionality is
> in.  We shouldn't EVER clutter the whole code-base with sepgsql
> permission checks.  If we want to expose other sepgsql functionality,
> we should do that by putting in more kinds of pg_xxx_aclcheck()-type
> calls in other places, and the sepgsql can leverage those call sites
> too.  But never mind that, because it's irrelevant for right now: what
> you need to do is strip this down to the minimal feature set without
> worrying about what will or won't happen later.  Otherwise, nothing is
> going to happen at all.

I might call them pgace_xxx....
Anyway, I can agree to begin and focus on the minimal features which
can be implemented on the existing pg_xxx_aclcheck().

>> I guess the following permissions can be checked at the first version.
>>  - db_database:{access}   ... equivalent to ACL_CONNECT on pg_database
>>  - db_procedure:{execute} ... equivalent to ACL_EXECUTE on pg_proc
>>  - db_schema:{search}     ... equivalent to ACL_USAGE on pg_namespace
> 
> Why not db_database:{connect} and db_schema:{usage}?  It seems to me
> that there is zero value in inventing new names for the same things.

We don't have any specific reason for db_database:{connect}.
In the early phase, SELinux's upstream security policy merged definitions
of object classes (such as db_table) and corresponding permissions,
so I uses its naming scheme.

However, security policy maintainer also said it is acceptable
if PostgreSQL folks merge SE-PgSQL with new permissions.
I don't mind to change db_database:{access} to {connect}.
(In actually, we need to add {connect} newly and mark {access} as
obsolete due to the compatibility issue.)

On the other hand, db_schema class was designed as an analogy to
directoty in filesystems. SELinux defines several permissions on
"dir" object class, such as "add_name", "remove_name" and "search".

dir:{search} is checked when we lookup filesystem object under
a certain directory, it seems like what schema object performs.
So, I would not like to change this naming scheme, if possible.

FYI, I summarized the list of SE-PgSQL permissions in the fullset
functionality here: http://wiki.postgresql.org/wiki/SEPostgreSQL_References#Object_classes_and_access_vector

>>  - db_database:{superuser} to be called from superuser_arg().
>>   Should it be postponed to the next phase?
> 
> No, I don't see any reason to postpone that.  That seems analagous to
> what we do now, and should be included in the first version, I would
> think.

OK,

>> BTW, SELinux needs objects to be labeled correctly on its creation time.
>> At least, we have to put hooks to provide security labels to be assigned
>> on the DefineRelation() and so on, although it does not check permission
>> to create the objects.
> 
> That seems very reasonable to me.

OK,

I'll update my patch set within this week.
Please wait for SE-PgSQL/tiny.                        ^^^^
Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>


Re: [PATCH] SE-PgSQL/lite rev.2163

From
Robert Haas
Date:
2009/7/14 KaiGai Kohei <kaigai@ak.jp.nec.com>:
> On the other hand, db_schema class was designed as an analogy to
> directoty in filesystems. SELinux defines several permissions on
> "dir" object class, such as "add_name", "remove_name" and "search".

I think that's a bad analogy and you need to make the permission names
match the way PostgreSQL handles schema permissions generally.
There's only so many times and ways to says this...

...Robert


Re: [PATCH] SE-PgSQL/lite rev.2163

From
KaiGai Kohei
Date:
Robert Haas wrote:
> 2009/7/14 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>> On the other hand, db_schema class was designed as an analogy to
>> directoty in filesystems. SELinux defines several permissions on
>> "dir" object class, such as "add_name", "remove_name" and "search".
> 
> I think that's a bad analogy and you need to make the permission names
> match the way PostgreSQL handles schema permissions generally.
> There's only so many times and ways to says this...

OK...
I can replace "search" by "usage".

Do you have any alternative ideas for "add_name" and "remove_name"?
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>


[PATCH] SE-PgSQL/tiny rev.2193

From
KaiGai Kohei
Date:
The following patch is the tiny version of SE-PostgreSQL:
 http://sepgsql.googlecode.com/files/sepgsql-01-tiny-8.5devel-r2193.patch.gz

In this version, all the security hooks (to make decision) invoked from
outside of the pg_xxx_aclcheck() and superuser_arg() were separated.
So, SE-PgSQL/tiny only checks the following only four permissions:- db_database:{connect}   ... equivalent to
ACL_CONNECTon the database- db_database:{superuser} ... equivalent to the superuser privilege- db_schema:{usage}
...equivalent to ACL_USAGE on the namespace- db_procedure:{execute}  ... equivalent to ACL_EXECUTE on the procedure
 

All the database objects to be labeled are databases, namespaces and
procedures, so I modified system column definitions.- db_database.datseclabel (text)- db_namespace.nspseclabel (text)-
db_procedure.proseclabel(text)
 
When we create a new one, a default security label shall be assigned
as far as we don't give any explicit security label.
(In the current version, it only checks sanity check of security label,
no any permission checks.)

The following features were separated.- Facility to cache access control decisions- Table/column level access controls-
Trustedprocedures
 

FYI, It is the scale of patch.

It may seem you the "tiny" is larger than the "lite".
But, 50% of changeset is at include/catalog/pg_proc.h, because we separate
the pg_security facility, so it was necessary to add a new regular attribute
into pg_proc system catalog. The pg_proc.h has 2000 of definitions for built
in functions, using DATA(...) macro. I updates them by sed secript.
It is the reason for the big changeset.

This patch also contains 570L of documentation changes, and 442L of testcases.
So, actual code changeset is about 2700L.

[kaigai@saba]$ diffstat /home/kaigai/RPMS/SOURCES/sepgsql-01-tiny-8.5devel-r2193.patch.gzconfigure
                                   |  112configure.in                                                       |
13doc/src/sgml/catalogs.sgml                                        |   21doc/src/sgml/config.sgml
                    |   42doc/src/sgml/errcodes.sgml                                         |
21doc/src/sgml/filelist.sgml                                        |    1doc/src/sgml/postgres.sgml
                    |    1doc/src/sgml/ref/alter_database.sgml                               |
12doc/src/sgml/ref/alter_function.sgml                              |   13doc/src/sgml/ref/alter_schema.sgml
                    |   11doc/src/sgml/ref/create_database.sgml                              |
14doc/src/sgml/ref/create_function.sgml                             |   12doc/src/sgml/ref/create_schema.sgml
                    |   16doc/src/sgml/ref/initdb.sgml                                       |
11doc/src/sgml/sepgsql.sgml                                         |  395src/Makefile.global.in
                    |    1src/backend/Makefile                                               |
7src/backend/bootstrap/bootstrap.c                                 |    4src/backend/catalog/aclchk.c
                   |   22src/backend/catalog/namespace.c                                    |
17src/backend/catalog/pg_aggregate.c                                |    3src/backend/catalog/pg_namespace.c
                    |    6src/backend/catalog/pg_proc.c                                      |
29src/backend/commands/alter.c                                      |   31src/backend/commands/dbcommands.c
                    |   86src/backend/commands/functioncmds.c                                |
82src/backend/commands/proclang.c                                   |    6src/backend/commands/schemacmds.c
                    |   69src/backend/nodes/copyfuncs.c                                      |
19src/backend/nodes/equalfuncs.c                                    |   17src/backend/parser/gram.y
                    |   68src/backend/security/Makefile                                      |
11src/backend/security/sepgsql/Makefile                             |   16src/backend/security/sepgsql/avc.c
                    |  331src/backend/security/sepgsql/dummy.c                               |
31src/backend/security/sepgsql/hooks.c                              |  167src/backend/security/sepgsql/label.c
                    |  523 +src/backend/security/sepgsql/misc.c                                |
152src/backend/security/sepgsql/perms.c                              |  353src/backend/security/sepgsql/policy/Makefile
                     |   28src/backend/security/sepgsql/policy/sepostgresql-devel.fc.template |
12src/backend/security/sepgsql/policy/sepostgresql-devel.te         |  119src/backend/tcop/utility.c
                    |   27src/backend/utils/init/postinit.c                                  |
11src/backend/utils/misc/guc.c                                      |   19src/backend/utils/misc/postgresql.conf.sample
                    |    4src/backend/utils/misc/superuser.c                                 |
16src/bin/initdb/initdb.c                                           |   13src/include/catalog/pg_attribute.h
                    |    4src/include/catalog/pg_class.h                                     |
2src/include/catalog/pg_database.h                                 |    6src/include/catalog/pg_namespace.h
                   |   12src/include/catalog/pg_proc.h                                      | 4242
!!!!!!!!!!src/include/catalog/pg_proc_fn.h                                  |    3src/include/commands/alter.h
                            |    1src/include/commands/dbcommands.h                                  |
1src/include/commands/defrem.h                                     |    1src/include/commands/schemacmds.h
                   |    1src/include/nodes/nodes.h                                          |
1src/include/nodes/parsenodes.h                                    |   15src/include/parser/kwlist.h
                   |    1src/include/pg_config.h.in                                         |
3src/include/security/sepgsql.h                                    |  295src/include/utils/errcodes.h
                   |    5src/test/sepgsql/Makefile                                          |
74src/test/sepgsql/expected/functions.out                           |   54src/test/sepgsql/expected/seclabel.out
                    |  129src/test/sepgsql/launch_psql.c                                     |
86src/test/sepgsql/sql/functions.sql                                |   27src/test/sepgsql/sql/seclabel.sql
                    |   7270 files changed, 3695 insertions(+), 4335 modifications(!)
 

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


Re: [PATCH] SE-PgSQL/lite rev.2163

From
Robert Haas
Date:
2009/7/15 KaiGai Kohei <kaigai@ak.jp.nec.com>:
> Robert Haas wrote:
>> 2009/7/14 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>>> On the other hand, db_schema class was designed as an analogy to
>>> directoty in filesystems. SELinux defines several permissions on
>>> "dir" object class, such as "add_name", "remove_name" and "search".
>>
>> I think that's a bad analogy and you need to make the permission names
>> match the way PostgreSQL handles schema permissions generally.
>> There's only so many times and ways to says this...
>
> OK...
> I can replace "search" by "usage".
>
> Do you have any alternative ideas for "add_name" and "remove_name"?

Aack!  Come on!  Use whatever names those permissions already have!
If there are no corresponding names, then rip them out!!!

...Robert


Re: [PATCH] SE-PgSQL/lite rev.2163

From
KaiGai Kohei
Date:
Robert Haas wrote:
> 2009/7/15 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>> Robert Haas wrote:
>>> 2009/7/14 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>>>> On the other hand, db_schema class was designed as an analogy to
>>>> directoty in filesystems. SELinux defines several permissions on
>>>> "dir" object class, such as "add_name", "remove_name" and "search".
>>> I think that's a bad analogy and you need to make the permission names
>>> match the way PostgreSQL handles schema permissions generally.
>>> There's only so many times and ways to says this...
>> OK...
>> I can replace "search" by "usage".
>>
>> Do you have any alternative ideas for "add_name" and "remove_name"?
> 
> Aack!  Come on!  Use whatever names those permissions already have!
> If there are no corresponding names, then rip them out!!!

OK, I'll rip definitions of unused SELinux's permissions from
the permission table of SE-PgSQL.

Is it correct for what you say?
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
KaiGai Kohei
Date:
Updated SE-PgSQL patch is here:
 http://sepgsql.googlecode.com/files/sepgsql-01-tiny-8.5devel-r2196.patch.gz

Unused definitions of SELinux's permissions are ripped out from
the permission table.

KaiGai Kohei wrote:
> The following patch is the tiny version of SE-PostgreSQL:
> 
>   http://sepgsql.googlecode.com/files/sepgsql-01-tiny-8.5devel-r2193.patch.gz
> 
> In this version, all the security hooks (to make decision) invoked from
> outside of the pg_xxx_aclcheck() and superuser_arg() were separated.
> So, SE-PgSQL/tiny only checks the following only four permissions:
>  - db_database:{connect}   ... equivalent to ACL_CONNECT on the database
>  - db_database:{superuser} ... equivalent to the superuser privilege
>  - db_schema:{usage}       ... equivalent to ACL_USAGE on the namespace
>  - db_procedure:{execute}  ... equivalent to ACL_EXECUTE on the procedure
> 
> All the database objects to be labeled are databases, namespaces and
> procedures, so I modified system column definitions.
>  - db_database.datseclabel (text)
>  - db_namespace.nspseclabel (text)
>  - db_procedure.proseclabel (text)
> When we create a new one, a default security label shall be assigned
> as far as we don't give any explicit security label.
> (In the current version, it only checks sanity check of security label,
> no any permission checks.)
> 
> The following features were separated.
>  - Facility to cache access control decisions
>  - Table/column level access controls
>  - Trusted procedures
> 
> FYI, It is the scale of patch.
> 
> It may seem you the "tiny" is larger than the "lite".
> But, 50% of changeset is at include/catalog/pg_proc.h, because we separate
> the pg_security facility, so it was necessary to add a new regular attribute
> into pg_proc system catalog. The pg_proc.h has 2000 of definitions for built
> in functions, using DATA(...) macro. I updates them by sed secript.
> It is the reason for the big changeset.
> 
> This patch also contains 570L of documentation changes, and 442L of testcases.
> So, actual code changeset is about 2700L.
> 
> [kaigai@saba]$ diffstat /home/kaigai/RPMS/SOURCES/sepgsql-01-tiny-8.5devel-r2193.patch.gz
>  configure                                                          |  112
>  configure.in                                                       |   13
>  doc/src/sgml/catalogs.sgml                                         |   21
>  doc/src/sgml/config.sgml                                           |   42
>  doc/src/sgml/errcodes.sgml                                         |   21
>  doc/src/sgml/filelist.sgml                                         |    1
>  doc/src/sgml/postgres.sgml                                         |    1
>  doc/src/sgml/ref/alter_database.sgml                               |   12
>  doc/src/sgml/ref/alter_function.sgml                               |   13
>  doc/src/sgml/ref/alter_schema.sgml                                 |   11
>  doc/src/sgml/ref/create_database.sgml                              |   14
>  doc/src/sgml/ref/create_function.sgml                              |   12
>  doc/src/sgml/ref/create_schema.sgml                                |   16
>  doc/src/sgml/ref/initdb.sgml                                       |   11
>  doc/src/sgml/sepgsql.sgml                                          |  395
>  src/Makefile.global.in                                             |    1
>  src/backend/Makefile                                               |    7
>  src/backend/bootstrap/bootstrap.c                                  |    4
>  src/backend/catalog/aclchk.c                                       |   22
>  src/backend/catalog/namespace.c                                    |   17
>  src/backend/catalog/pg_aggregate.c                                 |    3
>  src/backend/catalog/pg_namespace.c                                 |    6
>  src/backend/catalog/pg_proc.c                                      |   29
>  src/backend/commands/alter.c                                       |   31
>  src/backend/commands/dbcommands.c                                  |   86
>  src/backend/commands/functioncmds.c                                |   82
>  src/backend/commands/proclang.c                                    |    6
>  src/backend/commands/schemacmds.c                                  |   69
>  src/backend/nodes/copyfuncs.c                                      |   19
>  src/backend/nodes/equalfuncs.c                                     |   17
>  src/backend/parser/gram.y                                          |   68
>  src/backend/security/Makefile                                      |   11
>  src/backend/security/sepgsql/Makefile                              |   16
>  src/backend/security/sepgsql/avc.c                                 |  331
>  src/backend/security/sepgsql/dummy.c                               |   31
>  src/backend/security/sepgsql/hooks.c                               |  167
>  src/backend/security/sepgsql/label.c                               |  523 +
>  src/backend/security/sepgsql/misc.c                                |  152
>  src/backend/security/sepgsql/perms.c                               |  353
>  src/backend/security/sepgsql/policy/Makefile                       |   28
>  src/backend/security/sepgsql/policy/sepostgresql-devel.fc.template |   12
>  src/backend/security/sepgsql/policy/sepostgresql-devel.te          |  119
>  src/backend/tcop/utility.c                                         |   27
>  src/backend/utils/init/postinit.c                                  |   11
>  src/backend/utils/misc/guc.c                                       |   19
>  src/backend/utils/misc/postgresql.conf.sample                      |    4
>  src/backend/utils/misc/superuser.c                                 |   16
>  src/bin/initdb/initdb.c                                            |   13
>  src/include/catalog/pg_attribute.h                                 |    4
>  src/include/catalog/pg_class.h                                     |    2
>  src/include/catalog/pg_database.h                                  |    6
>  src/include/catalog/pg_namespace.h                                 |   12
>  src/include/catalog/pg_proc.h                                      | 4242 !!!!!!!!!!
>  src/include/catalog/pg_proc_fn.h                                   |    3
>  src/include/commands/alter.h                                       |    1
>  src/include/commands/dbcommands.h                                  |    1
>  src/include/commands/defrem.h                                      |    1
>  src/include/commands/schemacmds.h                                  |    1
>  src/include/nodes/nodes.h                                          |    1
>  src/include/nodes/parsenodes.h                                     |   15
>  src/include/parser/kwlist.h                                        |    1
>  src/include/pg_config.h.in                                         |    3
>  src/include/security/sepgsql.h                                     |  295
>  src/include/utils/errcodes.h                                       |    5
>  src/test/sepgsql/Makefile                                          |   74
>  src/test/sepgsql/expected/functions.out                            |   54
>  src/test/sepgsql/expected/seclabel.out                             |  129
>  src/test/sepgsql/launch_psql.c                                     |   86
>  src/test/sepgsql/sql/functions.sql                                 |   27
>  src/test/sepgsql/sql/seclabel.sql                                  |   72
>  70 files changed, 3695 insertions(+), 4335 modifications(!)
> 


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


Re: [PATCH] SE-PgSQL/lite rev.2163

From
Robert Haas
Date:
On Jul 15, 2009, at 11:41 PM, KaiGai Kohei <kaigai@ak.jp.nec.com> wrote:

> Robert Haas wrote:
>> 2009/7/15 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>>> Robert Haas wrote:
>>>> 2009/7/14 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>>>>> On the other hand, db_schema class was designed as an analogy to
>>>>> directoty in filesystems. SELinux defines several permissions on
>>>>> "dir" object class, such as "add_name", "remove_name" and  
>>>>> "search".
>>>> I think that's a bad analogy and you need to make the permission  
>>>> names
>>>> match the way PostgreSQL handles schema permissions generally.
>>>> There's only so many times and ways to says this...
>>> OK...
>>> I can replace "search" by "usage".
>>>
>>> Do you have any alternative ideas for "add_name" and "remove_name"?
>>
>> Aack!  Come on!  Use whatever names those permissions already have!
>> If there are no corresponding names, then rip them out!!!
>
> OK, I'll rip definitions of unused SELinux's permissions from
> the permission table of SE-PgSQL.
>
> Is it correct for what you say?

So the point we keep repeating here is that SEPostgreSQL should be  
doing the same kinds of permissions checks as regular PostgreSQL using  
the same names, code paths, etc. I don't know how to say it any more  
clearly than that.

I will read through your latest version soon.

...Robert


Re: [PATCH] SE-PgSQL/lite rev.2163

From
KaiGai Kohei
Date:
Robert Haas wrote:
> On Jul 15, 2009, at 11:41 PM, KaiGai Kohei <kaigai@ak.jp.nec.com> wrote:
> 
>> Robert Haas wrote:
>>> 2009/7/15 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>>>> Robert Haas wrote:
>>>>> 2009/7/14 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>>>>>> On the other hand, db_schema class was designed as an analogy to
>>>>>> directoty in filesystems. SELinux defines several permissions on
>>>>>> "dir" object class, such as "add_name", "remove_name" and "search".
>>>>> I think that's a bad analogy and you need to make the permission names
>>>>> match the way PostgreSQL handles schema permissions generally.
>>>>> There's only so many times and ways to says this...
>>>> OK...
>>>> I can replace "search" by "usage".
>>>>
>>>> Do you have any alternative ideas for "add_name" and "remove_name"?
>>>
>>> Aack!  Come on!  Use whatever names those permissions already have!
>>> If there are no corresponding names, then rip them out!!!
>>
>> OK, I'll rip definitions of unused SELinux's permissions from
>> the permission table of SE-PgSQL.
>>
>> Is it correct for what you say?
> 
> So the point we keep repeating here is that SEPostgreSQL should be doing 
> the same kinds of permissions checks as regular PostgreSQL using the 
> same names, code paths, etc. I don't know how to say it any more clearly 
> than that.

Thanks, it makes clear for me.

> I will read through your latest version soon.
> 
> ...Robert
> 

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


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Robert Haas
Date:
2009/7/16 KaiGai Kohei <kaigai@ak.jp.nec.com>:
> Updated SE-PgSQL patch is here:
>
>  http://sepgsql.googlecode.com/files/sepgsql-01-tiny-8.5devel-r2196.patch.gz
>
> Unused definitions of SELinux's permissions are ripped out from
> the permission table.

OK, I'm looking at this version of the patch, and my first reaction is
that it appears to be completely useless.  Standard PostgreSQL has
grantable privileges on 10 classes of objects: TABLE, COLUMN,
SEQUENCE, DATABASE, FOREIGN DATA WRAPPER, FOREIGN SERVER, FUNCTION,
LANGUAGE, SCHEMA, and TABLESPACE.  This patch, on the other hand,
implements SE-pgsql privileges for 3 classes of objects: DATABASE,
NAMESPACE, and FUNCTION.  That doesn't seem to jive in any conceivable
way with the advice that has been given repeatedly and in every single
review of this patch, which is to make the scope of SE-pgsql line up
exactly with the standard permissions that PostgreSQL already
implements.  In earlier versions, there were checks on all sorts of
extra things (like rows, and various DDL operations) that invented
whole new classes of permission checks.  Now you've gone to the
opposite extreme of checking almost nothing.

The goal here is not to pare this patch down to nothing: it's to
implement a coherent feature set that matches what PostgreSQL already
does.  Here is what I wrote 4 days ago:

"Another problem that I have with this patch set is that it STILL
doesn't have parity with the DAC permissions scheme (despite previous
requests to make it have parity)"

Here is what I wrote 3 days ago:

"Yes: to repeat what has been said multiple times previously, you
should postpone everything that isn't a mirror of the current security
model: there should only be permission checks in places where there
are permissions checks now, and they should be mirror images of the
current DAC checks."

Here is what I wrote yesterday:

"So the point we keep repeating here is that SEPostgreSQL should be
doing the same kinds of permissions checks as regular PostgreSQL using
the same names, code paths, etc. I don't know how to say it any more
clearly than that."

And just for reference, here is what Peter wrote 5 months ago, which
is basically saying the same thing:

"If I had to do this, I would first write a patch for #1: A patch that
additionally executes existing privilege checks against an SELinux
policy. Existing privilege checks are a well-defined set: they mostly
happen through pg_xxx_aclcheck() functions. Hook your checks in
there."

I really don't understand why this is so difficult.  But I don't think
you should bother resubmitting this patch until you've taken this
advice.  I am kind of running out patience here.  I've reviewed this
patch 3 times and found the exact same issue each time.

...Robert


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
KaiGai Kohei
Date:
Robert Haas wrote:
> 2009/7/16 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>> Updated SE-PgSQL patch is here:
>>
>>  http://sepgsql.googlecode.com/files/sepgsql-01-tiny-8.5devel-r2196.patch.gz
>>
>> Unused definitions of SELinux's permissions are ripped out from
>> the permission table.
> 
> OK, I'm looking at this version of the patch, and my first reaction is
> that it appears to be completely useless.  Standard PostgreSQL has
> grantable privileges on 10 classes of objects: TABLE, COLUMN,
> SEQUENCE, DATABASE, FOREIGN DATA WRAPPER, FOREIGN SERVER, FUNCTION,
> LANGUAGE, SCHEMA, and TABLESPACE.  This patch, on the other hand,
> implements SE-pgsql privileges for 3 classes of objects: DATABASE,
> NAMESPACE, and FUNCTION.  That doesn't seem to jive in any conceivable
> way with the advice that has been given repeatedly and in every single
> review of this patch, which is to make the scope of SE-pgsql line up
> exactly with the standard permissions that PostgreSQL already
> implements.  In earlier versions, there were checks on all sorts of
> extra things (like rows, and various DDL operations) that invented
> whole new classes of permission checks.  Now you've gone to the
> opposite extreme of checking almost nothing.

Yes, the tiny version will not give any advantages in security without
future enhancements.
It is not difficult to add object classes and permissions.
If necessary, I'll add checks them with corresponding permissions.

One anxiety is PostgreSQL specific object class, such as LANGUAGE.
It's not clear for me whether the maintainer of the SELinux security
policy accept these kind of object classes, or not.
I would like to implement them except for PostgreSQL specific object
class in this phase.

> The goal here is not to pare this patch down to nothing: it's to
> implement a coherent feature set that matches what PostgreSQL already
> does.  Here is what I wrote 4 days ago:
> 
> "Another problem that I have with this patch set is that it STILL
> doesn't have parity with the DAC permissions scheme (despite previous
> requests to make it have parity)"
> 
> Here is what I wrote 3 days ago:
> 
> "Yes: to repeat what has been said multiple times previously, you
> should postpone everything that isn't a mirror of the current security
> model: there should only be permission checks in places where there
> are permissions checks now, and they should be mirror images of the
> current DAC checks."
> 
> Here is what I wrote yesterday:
> 
> "So the point we keep repeating here is that SEPostgreSQL should be
> doing the same kinds of permissions checks as regular PostgreSQL using
> the same names, code paths, etc. I don't know how to say it any more
> clearly than that."
> 
> And just for reference, here is what Peter wrote 5 months ago, which
> is basically saying the same thing:
> 
> "If I had to do this, I would first write a patch for #1: A patch that
> additionally executes existing privilege checks against an SELinux
> policy. Existing privilege checks are a well-defined set: they mostly
> happen through pg_xxx_aclcheck() functions. Hook your checks in
> there."
> 
> I really don't understand why this is so difficult.  But I don't think
> you should bother resubmitting this patch until you've taken this
> advice.  I am kind of running out patience here.  I've reviewed this
> patch 3 times and found the exact same issue each time.

Here is a few differences in access control model between PostgreSQL and
SELinux, so I could not map all the SELinux permissions on the pg_xxx_aclcheck()
mechanism.

For example, ExecCheckRTEPerms() checks permissions on the tables and
columns appeared in the user given query. When the user have SELECT
permission on the required table, it bypasses to check permissions on
the columns.
SELinux's security model needs to check permissions on all the required
objects. For example, "SELECT A,B FROM T" requires the client to have
db_table:{select} on T and db_column:{select} on A and B.

For other example, some of pg_xxx_aclcheck() is bypassed when the client
has superuser privilege. In this case, SELinux requires the client to
have both of db_database:{superuser} and a certain permission.

If we deploy security hooks at the pg_xxx_aclcheck() or related stuff
at the first version, it is necessary to move them more appropriate
points later.
I can agree to deploy security hooks on pg_xxx_aclcheck(), if we can
move them to other points which can implement security model correctly
in the later version.

Sorry, I could not read it from the previous suggestions.
If you have been suggesting it repeatedly, I'm sorry so much.

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


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Robert Haas
Date:
2009/7/16 KaiGai Kohei <kaigai@ak.jp.nec.com>:
> Yes, the tiny version will not give any advantages in security without
> future enhancements.
> It is not difficult to add object classes and permissions.
> If necessary, I'll add checks them with corresponding permissions.
>
> One anxiety is PostgreSQL specific object class, such as LANGUAGE.
> It's not clear for me whether the maintainer of the SELinux security
> policy accept these kind of object classes, or not.
> I would like to implement them except for PostgreSQL specific object
> class in this phase.

I'm starting to think that there's just no hope of this matching up
well enough with the way PostgreSQL already works to have a chance of
being accepted.

> Here is a few differences in access control model between PostgreSQL and
> SELinux, so I could not map all the SELinux permissions on the pg_xxx_aclcheck()
> mechanism.
>
> For example, ExecCheckRTEPerms() checks permissions on the tables and
> columns appeared in the user given query. When the user have SELECT
> permission on the required table, it bypasses to check permissions on
> the columns.
> SELinux's security model needs to check permissions on all the required
> objects. For example, "SELECT A,B FROM T" requires the client to have
> db_table:{select} on T and db_column:{select} on A and B.

Isn't this a purely arbitrary decision on your part to implement
incompatible semantics?  I don't see why it can't check for
db_table:{select} and if that fails then check for db_column:[select}
on each column?  Maybe that's not legit, I don't understand SE-Linux
well enough to know.  But I think we need to get someone from the
SE-Linux community involved to help review and consider these kinds of
issues, because it is obvious that we don't have the expertise in the
PostgreSQL community.

> For other example, some of pg_xxx_aclcheck() is bypassed when the client
> has superuser privilege. In this case, SELinux requires the client to
> have both of db_database:{superuser} and a certain permission.

Surely you can't just transform (A OR B) into (A AND B) and pretend
that's the same thing...

> Sorry, I could not read it from the previous suggestions.
> If you have been suggesting it repeatedly, I'm sorry so much.

I think the language barrier is part of what is making this a very
difficult process.  Your English is surely better than my Japanese,
but we are definitely going around in circles.

...Robert


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
KaiGai Kohei
Date:
Robert Haas wrote:
> 2009/7/16 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>> Yes, the tiny version will not give any advantages in security without
>> future enhancements.
>> It is not difficult to add object classes and permissions.
>> If necessary, I'll add checks them with corresponding permissions.
>>
>> One anxiety is PostgreSQL specific object class, such as LANGUAGE.
>> It's not clear for me whether the maintainer of the SELinux security
>> policy accept these kind of object classes, or not.
>> I would like to implement them except for PostgreSQL specific object
>> class in this phase.
> 
> I'm starting to think that there's just no hope of this matching up
> well enough with the way PostgreSQL already works to have a chance of
> being accepted.

I believe different security mechanisms can have different viewpoints,
security models, criteria to make its access control decision and so on.
Some of SELinux part can match with PostgreSQL's permission with one-to-one,
such as db_database:{connect} and ACL_CONNECT.
But, we don't need to mind different security mechanism has different stuffs.

For example, Linux applies its DAC permission checks at inode_permission()
which also calls selinux_inode_permission(). It is the one-to-one mapping
part. But SELinux (LSM in actually) also put its security hooks in other
points, such as selinux_inode_rename() to check file renaming permissions,
selinux_socket_accept() to check accept connections permissions although
vanilla OS does not have corresponding permissions.

What I would like to say is that it is quite natural different security
mechanism has not-identical security models and so on.

>> Here is a few differences in access control model between PostgreSQL and
>> SELinux, so I could not map all the SELinux permissions on the pg_xxx_aclcheck()
>> mechanism.
>>
>> For example, ExecCheckRTEPerms() checks permissions on the tables and
>> columns appeared in the user given query. When the user have SELECT
>> permission on the required table, it bypasses to check permissions on
>> the columns.
>> SELinux's security model needs to check permissions on all the required
>> objects. For example, "SELECT A,B FROM T" requires the client to have
>> db_table:{select} on T and db_column:{select} on A and B.
> 
> Isn't this a purely arbitrary decision on your part to implement
> incompatible semantics?  I don't see why it can't check for
> db_table:{select} and if that fails then check for db_column:[select}
> on each column?  Maybe that's not legit, I don't understand SE-Linux
> well enough to know.  But I think we need to get someone from the
> SE-Linux community involved to help review and consider these kinds of
> issues, because it is obvious that we don't have the expertise in the
> PostgreSQL community.

Indeed, operating system does not have well analogy, such as a relationship
between tables and columns. However, SELinux's rule is simple.

Please imagine a situation when we write a security policy module.
When we would like to control accesses a certain object (such as column),
all we need to focus on is users' privileges on the column only.
If checks on columns are bypassed when he is allowed to access to the
table, we need to focus on both of the table and the column.
It seems to me it goes against to the principle in SELinux.
(Note that I never say database ACL model is bad. Both models have
their philosophies, and I believe both are worth each other.)

If you need any more comments from other persons in SELinux community,
I try to call Joshua Brindle who had been invoked in pgsql-hackers
several months ago.

>> For other example, some of pg_xxx_aclcheck() is bypassed when the client
>> has superuser privilege. In this case, SELinux requires the client to
>> have both of db_database:{superuser} and a certain permission.
> 
> Surely you can't just transform (A OR B) into (A AND B) and pretend
> that's the same thing...

It needs to change code path but it is not reasonable when user
disables SE-PgSQL. So, I thought separated security hooks are
reasonable, because it is replaced by empty macro if disabled.

>> Sorry, I could not read it from the previous suggestions.
>> If you have been suggesting it repeatedly, I'm sorry so much.
> 
> I think the language barrier is part of what is making this a very
> difficult process.  Your English is surely better than my Japanese,
> but we are definitely going around in circles.
> 
> ...Robert
> 

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


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Peter Eisentraut
Date:
On Friday 17 July 2009 06:10:12 Robert Haas wrote:
> 2009/7/16 KaiGai Kohei <kaigai@ak.jp.nec.com>:
> > Yes, the tiny version will not give any advantages in security without
> > future enhancements.
> > It is not difficult to add object classes and permissions.
> > If necessary, I'll add checks them with corresponding permissions.
> >
> > One anxiety is PostgreSQL specific object class, such as LANGUAGE.
> > It's not clear for me whether the maintainer of the SELinux security
> > policy accept these kind of object classes, or not.
> > I would like to implement them except for PostgreSQL specific object
> > class in this phase.
>
> I'm starting to think that there's just no hope of this matching up
> well enough with the way PostgreSQL already works to have a chance of
> being accepted.

What I'm understanding here is the apparent requirement that the SEPostgreSQL 
implementation be done in a way that a generic SELinux policy that has been 
written for an operating system and file system can be applied to PostgreSQL 
without change and do something useful.  I can see merits for or against that. 
But in any case, this needs to be clarified, if I understand this requirement 
correctly anyway.


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Martijn van Oosterhout
Date:
On Fri, Jul 17, 2009 at 03:59:29PM +0300, Peter Eisentraut wrote:
> > I'm starting to think that there's just no hope of this matching up
> > well enough with the way PostgreSQL already works to have a chance of
> > being accepted.
>
> What I'm understanding here is the apparent requirement that the SEPostgreSQL
> implementation be done in a way that a generic SELinux policy that has been
> written for an operating system and file system can be applied to PostgreSQL
> without change and do something useful.  I can see merits for or against that.
> But in any case, this needs to be clarified, if I understand this requirement
> correctly anyway.

Indeed. My impression was also that there are existing SELinux rules
and permission concepts already in use and people would like to apply
them to PostgreSQL, which puts the translation layer inside postgres.
However, from the postgres side there appears to be a push to make
unique postgresql SELinux rules and requiring every user of SELinux to
do the mapping of rights to postgresql specific terms.

Specifically, creating SELinux permissions for CREATE LANGUAGE seems
particularly useless since that's not a data protection issue. The same
with aggregates, operator classes, etc. ISTM the goal of SELinux is not
primarily to restrict DDL but mostly to protect the data.

Personally I find the idea that SELinux permissions need to meet parity
with the existing permission structure crazy, since it's not going to
replace the existing permissions, just supplement them.

Merely my opinion ofcourse, which doesn't count for much :)

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Please line up in a tree and maintain the heap invariant while
> boarding. Thank you for flying nlogn airlines.

Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Robert Haas
Date:
On Sat, Jul 18, 2009 at 7:10 AM, Martijn van
Oosterhout<kleptog@svana.org> wrote:
> On Fri, Jul 17, 2009 at 03:59:29PM +0300, Peter Eisentraut wrote:
>> > I'm starting to think that there's just no hope of this matching up
>> > well enough with the way PostgreSQL already works to have a chance of
>> > being accepted.
>>
>> What I'm understanding here is the apparent requirement that the SEPostgreSQL
>> implementation be done in a way that a generic SELinux policy that has been
>> written for an operating system and file system can be applied to PostgreSQL
>> without change and do something useful.  I can see merits for or against that.
>> But in any case, this needs to be clarified, if I understand this requirement
>> correctly anyway.
>
> Indeed. My impression was also that there are existing SELinux rules
> and permission concepts already in use and people would like to apply
> them to PostgreSQL, which puts the translation layer inside postgres.
> However, from the postgres side there appears to be a push to make
> unique postgresql SELinux rules and requiring every user of SELinux to
> do the mapping of rights to postgresql specific terms.

I think this is only semi-accurate.  My impression is that a
supposedly generic policy has been written for database objects and
merged into model SE-Linux policy, but I think that was done largely
in the hops of implementing SE-PostgreSQL.  It would be one think if
KaiGai showed up and said, see, there are three other databases that
do this, now we want you to do it too, that would be one thing.  But I
don't think that's the case: I believe that we are the first, which
makes the argument that we have to conform to the "standard" ways of
doing this a lot weaker.

> Specifically, creating SELinux permissions for CREATE LANGUAGE seems
> particularly useless since that's not a data protection issue. The same
> with aggregates, operator classes, etc. ISTM the goal of SELinux is not
> primarily to restrict DDL but mostly to protect the data.

I'd actually be willing to buy that.  If KaiGai wants to take the list
of objects for which SE-PostgreSQL supports grantable permissions and
the list of objects for which $COMPETITOR supports permissions and
implement only the intersection of the two, I think that would be
reasonable.  What I don't think is reasonable is trying to implement,
in the first version of the patch, 50 types of permissions that we
never had before, or on the other hand such a trivial percentage of
what we already do that it's totally useless.

> Personally I find the idea that SELinux permissions need to meet parity with the existing permission structure
> crazy, since it's not going to replace the existing permissions, just supplement them.

Maybe it is crazy, but here are my concerns...

1. If the permissions work in a way that is very different than the
existing permissions, then they need lots and lots of hooks all over
the code.  And nobody except KaiGai understands why those hooks are
where they are instead of somewhere else.  That's going to make it
difficult to maintain.

2. If we add a lot of new kinds of permission checks that PostgreSQL
has never done before, using a framework that none of the committers
understand, how do we verify that they are well-designed (correct,
secure, etc)?  I am fairly well convinced that the design for
row-level security was a really bad design.  Whether I'm right or not,
I think something like that is far too large a feature to add "by the
wayside" in the context of another patch.

3. As far as I can tell, there is a lot of resistance from the
committers who have looked at this patch (Tom, Peter, and maybe Bruce,
though I think his review was not quite so unfavorable) to the idea of
committing it at all.  I don't think they're going to be willing to
work extra-hard to implement security features that are only going to
be useful to people using SE-Linux.

For what it's worth, this problem is not confined to the committers:
SE-PostgreSQL is the only patch that I had people specifically tell me
they didn't want to review because they just didn't care about it.
Frankly, I don't care about it much either: ordinarily, the first and
last thing I do with SE-Linux is shut it off.  What is making me care
even less is the fact that after many revisions we still don't have
anything that can be reviewed with any seriousness.  The initial
versions had so many extra bells and whistles (row-level security,
complex DDL permissions) that they went way beyond basic SE-Linux
support, and now we have a version that is stripped down to the point
where it does barely anything.  I feel like I'm spinning my wheels on
a patch that nobody in the PostgreSQL community really wants anyway.

...Robert


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
KaiGai Kohei
Date:
Robert Haas wrote:
> On Sat, Jul 18, 2009 at 7:10 AM, Martijn van
> Oosterhout<kleptog@svana.org> wrote:
>> On Fri, Jul 17, 2009 at 03:59:29PM +0300, Peter Eisentraut wrote:
>>>> I'm starting to think that there's just no hope of this matching up
>>>> well enough with the way PostgreSQL already works to have a chance of
>>>> being accepted.
>>> What I'm understanding here is the apparent requirement that the SEPostgreSQL
>>> implementation be done in a way that a generic SELinux policy that has been
>>> written for an operating system and file system can be applied to PostgreSQL
>>> without change and do something useful.  I can see merits for or against that.
>>> But in any case, this needs to be clarified, if I understand this requirement
>>> correctly anyway.
>> Indeed. My impression was also that there are existing SELinux rules
>> and permission concepts already in use and people would like to apply
>> them to PostgreSQL, which puts the translation layer inside postgres.
>> However, from the postgres side there appears to be a push to make
>> unique postgresql SELinux rules and requiring every user of SELinux to
>> do the mapping of rights to postgresql specific terms.
> 
> I think this is only semi-accurate.  My impression is that a
> supposedly generic policy has been written for database objects and
> merged into model SE-Linux policy, but I think that was done largely
> in the hops of implementing SE-PostgreSQL.  It would be one think if
> KaiGai showed up and said, see, there are three other databases that
> do this, now we want you to do it too, that would be one thing.  But I
> don't think that's the case: I believe that we are the first, which
> makes the argument that we have to conform to the "standard" ways of
> doing this a lot weaker.

My representation might be confusable. What I would like to say was that
it is "unclear" whether the security policy maintainer can accept
definitions of object classes and permissions "as-is" we discussed here,
so I would like to implement them at the second or later phase.
It never means SELinux does not accept PostgreSQL specific permissions
due to the its characteristic features. In fact, SELinux already provides
db_blob object class and corresponding permissions to describe access
control policies on largeobjects.

As you said bellow, SELinux have several db_xxx object classes (it neans
object classes for various kind of database objects) and allows DBMSs to
choose a part of them.

In addition, the security policy developer understand each RDBMS's have
its characteristics, such as largeobjects and loadable C-libraries, so
their security policy also should be developed for each object manager,
although definitions of object classes and permissions can/cannot be shared.

>> Specifically, creating SELinux permissions for CREATE LANGUAGE seems
>> particularly useless since that's not a data protection issue. The same
>> with aggregates, operator classes, etc. ISTM the goal of SELinux is not
>> primarily to restrict DDL but mostly to protect the data.
> 
> I'd actually be willing to buy that.  If KaiGai wants to take the list
> of objects for which SE-PostgreSQL supports grantable permissions and
> the list of objects for which $COMPETITOR supports permissions and
> implement only the intersection of the two, I think that would be
> reasonable.  What I don't think is reasonable is trying to implement,
> in the first version of the patch, 50 types of permissions that we
> never had before, or on the other hand such a trivial percentage of
> what we already do that it's totally useless.

I also think the SE-PostgreSQL should support its object classes and
permissions on its characteristic features and the intersection of
the two RDBMSs.

The reason why I dropped most of permission checks was that SE-PgSQL
has been pointed out its patch scale several times in the v8.4 development
cycle, and suggested to drop all the permission check which does not
have 1:1 relationship with the existing pg_xxx_aclcheck().

If we can make clear what should be/should not be implement at the first
patch, I'll modify my patch set again.

>> Personally I find the idea that SELinux permissions need to meet parity with the existing permission structure
>> crazy, since it's not going to replace the existing permissions, just supplement them.
> 
> Maybe it is crazy, but here are my concerns...
> 
> 1. If the permissions work in a way that is very different than the
> existing permissions, then they need lots and lots of hooks all over
> the code.  And nobody except KaiGai understands why those hooks are
> where they are instead of somewhere else.  That's going to make it
> difficult to maintain.

Linux is a former case example in success. It puts various kind of security
hooks (called LSM; Linux Security Module) rather than existing permissions.
All the interfaces are designed not to give any side effects when security
module (such as SELinux) is disabled, and source code comments explicitly
introduce where/when the hooks should be invoked and what arguments should
be given and what results should be returned.
Needless to say, most of Linux kernel developers are not also security
experts, but its development cycles works well.

In addition, smaller number of security hooks are generally considered
better security design, so these are deployed on primordial routines
these are seldom modified.

In addition, I (and NEC) can provide our capability to the PostgreSQL
community to keep these security features work correctly.

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


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Joshua Brindle
Date:
Robert Haas wrote:
> On Sat, Jul 18, 2009 at 7:10 AM, Martijn van
> Oosterhout<kleptog@svana.org>  wrote:
>> On Fri, Jul 17, 2009 at 03:59:29PM +0300, Peter Eisentraut wrote:
>>>> I'm starting to think that there's just no hope of this matching up
>>>> well enough with the way PostgreSQL already works to have a chance of
>>>> being accepted.
>>> What I'm understanding here is the apparent requirement that the SEPostgreSQL
>>> implementation be done in a way that a generic SELinux policy that has been
>>> written for an operating system and file system can be applied to PostgreSQL
>>> without change and do something useful.  I can see merits for or against that.
>>> But in any case, this needs to be clarified, if I understand this requirement
>>> correctly anyway.
>> Indeed. My impression was also that there are existing SELinux rules
>> and permission concepts already in use and people would like to apply
>> them to PostgreSQL, which puts the translation layer inside postgres.
>> However, from the postgres side there appears to be a push to make
>> unique postgresql SELinux rules and requiring every user of SELinux to
>> do the mapping of rights to postgresql specific terms.
>
> I think this is only semi-accurate.  My impression is that a
> supposedly generic policy has been written for database objects and
> merged into model SE-Linux policy, but I think that was done largely
> in the hops of implementing SE-PostgreSQL.  It would be one think if
> KaiGai showed up and said, see, there are three other databases that
> do this, now we want you to do it too, that would be one thing.  But I
> don't think that's the case: I believe that we are the first, which
> makes the argument that we have to conform to the "standard" ways of
> doing this a lot weaker.
>

FYI Trusted RUBIX already has SELinux integration in their DBMS:
http://www.nsa.gov/research/selinux/list-archive/0807/26900.shtml

They are using the same SELinux object classes that KaiGai developed for 
SE-Postgres.

>> Specifically, creating SELinux permissions for CREATE LANGUAGE seems
>> particularly useless since that's not a data protection issue. The same
>> with aggregates, operator classes, etc. ISTM the goal of SELinux is not
>> primarily to restrict DDL but mostly to protect the data.
>
> I'd actually be willing to buy that.  If KaiGai wants to take the list
> of objects for which SE-PostgreSQL supports grantable permissions and
> the list of objects for which $COMPETITOR supports permissions and
> implement only the intersection of the two, I think that would be
> reasonable.  What I don't think is reasonable is trying to implement,
> in the first version of the patch, 50 types of permissions that we
> never had before, or on the other hand such a trivial percentage of
> what we already do that it's totally useless.
>

The reason for comprehensively protecting objects isn't necessarily about 
protecting the data in the database but for limiting information flow between 
clients of differing security levels. Eg., if someone top secret can create 
language and use that to pass information down to someone unclassified then 
postgres could be used as an information downgrader illegitimately.

>> Personally I find the idea that SELinux permissions need to meet parity with the existing permission structure
>> crazy, since it's not going to replace the existing permissions, just supplement them.
>
> Maybe it is crazy, but here are my concerns...
>
> 1. If the permissions work in a way that is very different than the
> existing permissions, then they need lots and lots of hooks all over
> the code.  And nobody except KaiGai understands why those hooks are
> where they are instead of somewhere else.  That's going to make it
> difficult to maintain.
>
> 2. If we add a lot of new kinds of permission checks that PostgreSQL
> has never done before, using a framework that none of the committers
> understand, how do we verify that they are well-designed (correct,
> secure, etc)?  I am fairly well convinced that the design for
> row-level security was a really bad design.  Whether I'm right or not,
> I think something like that is far too large a feature to add "by the
> wayside" in the context of another patch.
>
> 3. As far as I can tell, there is a lot of resistance from the
> committers who have looked at this patch (Tom, Peter, and maybe Bruce,
> though I think his review was not quite so unfavorable) to the idea of
> committing it at all.  I don't think they're going to be willing to
> work extra-hard to implement security features that are only going to
> be useful to people using SE-Linux.
>

I've said it before but I have firsthand knowledge of people wanting/waiting for 
SELinux integration into Postgres (or any open source DBMS)

> For what it's worth, this problem is not confined to the committers:
> SE-PostgreSQL is the only patch that I had people specifically tell me
> they didn't want to review because they just didn't care about it.
> Frankly, I don't care about it much either: ordinarily, the first and
> last thing I do with SE-Linux is shut it off.  What is making me care
> even less is the fact that after many revisions we still don't have
> anything that can be reviewed with any seriousness.  The initial
> versions had so many extra bells and whistles (row-level security,
> complex DDL permissions) that they went way beyond basic SE-Linux
> support, and now we have a version that is stripped down to the point
> where it does barely anything.  I feel like I'm spinning my wheels on
> a patch that nobody in the PostgreSQL community really wants anyway.
>

That is your (and the communities) prerogative. Linus wasn't very supportive of 
SELinux in the kernel either but it is the only way Linux got an EAL4+ LSPP 
evaluation for use in certain government systems. I personally would love to see 
an open source DBMS evaluated for systems like this because the current state of 
the art is fairly sad.


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Martijn van Oosterhout
Date:
On Mon, Jul 20, 2009 at 10:52:44AM -0400, Joshua Brindle wrote:
>>> Specifically, creating SELinux permissions for CREATE LANGUAGE seems
>>> particularly useless since that's not a data protection issue. The same
>>> with aggregates, operator classes, etc. ISTM the goal of SELinux is not
>>> primarily to restrict DDL but mostly to protect the data.
>
> The reason for comprehensively protecting objects isn't necessarily about
> protecting the data in the database but for limiting information flow
> between clients of differing security levels. Eg., if someone top secret
> can create language and use that to pass information down to someone
> unclassified then postgres could be used as an information downgrader
> illegitimately.

Consider the pl/pgsql language. The creation of the language must be
protected, because it involves loading shared libraries and thus could
be used to bypass the system. However, once loaded the language only
uses the internal SQL interface and thus is subject to the restrictions
imposed by the caller (except for setuid functions ofcourse).

Would you agree if the language is transparent with respect to
permissions that *usage* of the laguage doesn't need to be restricted.

I'm asking because from my position it looks like KaiGai is being
simultaneously told "you patch is too big, make it smaller" and "your
patch is not complete (with respect to some metric), make it bigger"
and we need to define a middle ground if we want to avoid the
appearence of moving goalposts.

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Please line up in a tree and maintain the heap invariant while
> boarding. Thank you for flying nlogn airlines.

Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Joshua Brindle
Date:
Martijn van Oosterhout wrote:
> On Mon, Jul 20, 2009 at 10:52:44AM -0400, Joshua Brindle wrote:
>>>> Specifically, creating SELinux permissions for CREATE LANGUAGE seems
>>>> particularly useless since that's not a data protection issue. The same
>>>> with aggregates, operator classes, etc. ISTM the goal of SELinux is not
>>>> primarily to restrict DDL but mostly to protect the data.
>> The reason for comprehensively protecting objects isn't necessarily about
>> protecting the data in the database but for limiting information flow
>> between clients of differing security levels. Eg., if someone top secret
>> can create language and use that to pass information down to someone
>> unclassified then postgres could be used as an information downgrader
>> illegitimately.
>
> Consider the pl/pgsql language. The creation of the language must be
> protected, because it involves loading shared libraries and thus could
> be used to bypass the system. However, once loaded the language only
> uses the internal SQL interface and thus is subject to the restrictions
> imposed by the caller (except for setuid functions ofcourse).
>
> Would you agree if the language is transparent with respect to
> permissions that *usage* of the laguage doesn't need to be restricted.
>

Using something is typically controlled because of information you may get from 
using it (for example, stat() on a file may not get you the data in the file but 
it gets you plenty of other information). I guess the question is, can the 
person creating the language leak information to people using the language and 
it sounds like they can.

However, because language creation is controlled via superuser privilege (which 
is never ideal, we like to be able to break superusers up and give them only 
permission to do what they need to do, principle of least privilege) then it is 
probably a lot less important than many of the other things KaiGai is trying to 
get in.

> I'm asking because from my position it looks like KaiGai is being
> simultaneously told "you patch is too big, make it smaller" and "your
> patch is not complete (with respect to some metric), make it bigger"
> and we need to define a middle ground if we want to avoid the
> appearence of moving goalposts.
>

Agreed, there are lots of mixed signals in this thread. For my uses the 'basic' 
support being pushed won't be enough to use this in secure applications, nothing 
less than full row-level access control would be.

I'm all for the community taking smaller patches over time but if there is no 
intention of going all the way at the end then I'm not sure what you/we are gaining.


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Tom Lane
Date:
Martijn van Oosterhout <kleptog@svana.org> writes:
> I'm asking because from my position it looks like KaiGai is being
> simultaneously told "you patch is too big, make it smaller" and "your
> patch is not complete (with respect to some metric), make it bigger"
> and we need to define a middle ground if we want to avoid the
> appearence of moving goalposts.

Well, the assumption behind the "make it smaller" advice was that
followon patches might re-add (some of) the advanced functionality
once we had a basic connection to SELinux up and running.  So yes,
there's an expectation of the goals changing over time, and I don't
see a reason to apologize for that.

But on the whole the problem is that the community just isn't interested
in this topic.  Every so often somebody pops up and tells us "if you
build it they will come", but there is a complete lack of hard evidence
that there's significant interest out there.  Without that, I don't think
we'll ever get to the point of wanting to put in the work to make it
happen and then support it over time.  This is a huge project, and
taking it seriously will mean draining resources away from other
high-priority issues.  We need way more evidence of interest than has
been presented.
        regards, tom lane


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Joshua Brindle
Date:
Tom Lane wrote:
> Martijn van Oosterhout<kleptog@svana.org>  writes:
>> I'm asking because from my position it looks like KaiGai is being
>> simultaneously told "you patch is too big, make it smaller" and "your
>> patch is not complete (with respect to some metric), make it bigger"
>> and we need to define a middle ground if we want to avoid the
>> appearence of moving goalposts.
>
> Well, the assumption behind the "make it smaller" advice was that
> followon patches might re-add (some of) the advanced functionality
> once we had a basic connection to SELinux up and running.  So yes,
> there's an expectation of the goals changing over time, and I don't
> see a reason to apologize for that.
>
> But on the whole the problem is that the community just isn't interested
> in this topic.  Every so often somebody pops up and tells us "if you
> build it they will come", but there is a complete lack of hard evidence
> that there's significant interest out there.  Without that, I don't think
> we'll ever get to the point of wanting to put in the work to make it
> happen and then support it over time.  This is a huge project, and
> taking it seriously will mean draining resources away from other
> high-priority issues.  We need way more evidence of interest than has
> been presented.
>

How many people are you looking for? Is there a number or are you waiting for a 
good feeling?

The unfortunate part is that many of the people that would use it are unable to 
publicly say so.


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Alvaro Herrera
Date:
Joshua Brindle escribió:

> The unfortunate part is that many of the people that would use it are 
> unable to publicly say so.

So they will be similarly unable to help with it.  Such a black hole is
not of much use, is it?  Or are they getting a contract with some PG
support company to which they'll shell out the bucks under NDA to ensure
that the thing is delivered and maintained?  I think that if this looked
less like a one-man project it could have better support around here.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Peter Eisentraut
Date:
On Monday 20 July 2009 21:05:38 Joshua Brindle wrote:
> How many people are you looking for? Is there a number or are you waiting
> for a good feeling?

In my mind, the number of interested people is relatively uninteresting, as 
long as it is greater than, say, three.

What is lacking here is a written specification.

When it comes to larger features, this development group has a great deal of 
experience in implementing existing specifications, even relatively terrible 
ones like SQL or ODBC or Oracle compatibility.  But the expected behavior has 
to be written down somewhere, endorsed by someone with authority.  It can't 
just be someone's idea.  Especially for features that are so complex, 
esoteric, invasive, and critical for security and performance.

So I think if you want to get anywhere with this, scrap the code, and start 
writing a specification.  One with references.  And then let's consider that 
one.


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Joshua Brindle
Date:
Peter Eisentraut wrote:
> On Monday 20 July 2009 21:05:38 Joshua Brindle wrote:
>> How many people are you looking for? Is there a number or are you waiting
>> for a good feeling?
>
> In my mind, the number of interested people is relatively uninteresting, as
> long as it is greater than, say, three.
>
> What is lacking here is a written specification.
>
> When it comes to larger features, this development group has a great deal of
> experience in implementing existing specifications, even relatively terrible
> ones like SQL or ODBC or Oracle compatibility.  But the expected behavior has
> to be written down somewhere, endorsed by someone with authority.  It can't
> just be someone's idea.  Especially for features that are so complex,
> esoteric, invasive, and critical for security and performance.
>

Who do you consider has authority? The security community has as many opinions 
as any other. There are papers written on mandatory access controls in rdbms's 
but they are mostly about multi-level security, which SELinux has but primarily 
uses type enforcement. The SELinux community are all on board with KaiGai's 
object model (the object classes and permissions and how they are enforced), 
there has been quite a bit of discussion about them over the years. Trusted 
RUBIX integrated SELinux using the object classes that KaiGai made for SEPostgres.

> So I think if you want to get anywhere with this, scrap the code, and start
> writing a specification.  One with references.  And then let's consider that
> one.
>

Harsh.


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Ron Mayer
Date:
Joshua Brindle wrote:
> How many people are you looking for? Is there a number or are you
> waiting for a good feeling?

Is it individuals or organizations people are looking for?

I see KaiGai wrote "In addition, I (and NEC) can provide our
capability to the PostgreSQL community to keep these security
features work correctly. "

Does that imply that a larger part of NEC is interested?


> The unfortunate part is that many of the people that would use it are
> unable to publicly say so.

Could they publicly say something softer.

I see SELinux had a number of large organizations (NSA) and
publicly traded companies (Secure Computing Corp,  Network
Associates, etc) pushing it and contributing to it.

If people who could speak for those organizations were here
saying "ooh, and such features in a F/OSS database would be
interesting too", that would probably convince a lot of people.

Joshua - if you're still associated with Tresys - could someone
who could speak for that company say what they think about this
project?   The seem quite in-the-loop on what SELinux customers
want.



Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Joshua Brindle
Date:
Ron Mayer wrote:
> Joshua Brindle wrote:
>> How many people are you looking for? Is there a number or are you
>> waiting for a good feeling?
> <snip>

> Joshua - if you're still associated with Tresys - could someone
> who could speak for that company say what they think about this
> project?   The seem quite in-the-loop on what SELinux customers
> want.
>

I am capable of speaking for Tresys in this matter. We are very interested in 
this work and our US DoD customers need the capabilities that this project adds 
(assuming row level access controls are a possibility).


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
KaiGai Kohei
Date:
Peter Eisentraut wrote:
> On Monday 20 July 2009 21:05:38 Joshua Brindle wrote:
>> How many people are you looking for? Is there a number or are you waiting
>> for a good feeling?
> 
> In my mind, the number of interested people is relatively uninteresting, as 
> long as it is greater than, say, three.
> 
> What is lacking here is a written specification.
> 
> When it comes to larger features, this development group has a great deal of 
> experience in implementing existing specifications, even relatively terrible 
> ones like SQL or ODBC or Oracle compatibility.  But the expected behavior has 
> to be written down somewhere, endorsed by someone with authority.  It can't 
> just be someone's idea.  Especially for features that are so complex, 
> esoteric, invasive, and critical for security and performance.
> 
> So I think if you want to get anywhere with this, scrap the code, and start 
> writing a specification.  One with references.  And then let's consider that 
> one.

At least, what is important is that SE-PgSQL performs with its security model
correctly, not how it is implemented. In fast, I have modified its implementation
and separated some of non-primary features several times.
As I said before, its implementation is flexible as far as it can implement
SELinux's security model correctly.

If PostgreSQL community requires its design specifications from the viewpoints
of developers, I don't have any reason not to provide it.

One question is what items should be described in the specifications?
I already provide a reference including a list of object classes and permissions.
http://wiki.postgresql.org/wiki/SEPostgreSQL_References

I guess you would like to see when/where/how SE-PgSQL checks what permissions,
what criteria to make its decision should be used, and so on.

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


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Andrew Dunstan
Date:

Joshua Brindle wrote:
> Peter Eisentraut wrote:
>>
>> When it comes to larger features, this development group has a great 
>> deal of
>> experience in implementing existing specifications, even relatively 
>> terrible
>> ones like SQL or ODBC or Oracle compatibility.  But the expected 
>> behavior has
>> to be written down somewhere, endorsed by someone with authority.  It 
>> can't
>> just be someone's idea.  Especially for features that are so complex,
>> esoteric, invasive, and critical for security and performance.
>>
>
> Who do you consider has authority? The security community has as many 
> opinions as any other. There are papers written on mandatory access 
> controls in rdbms's but they are mostly about multi-level security, 
> which SELinux has but primarily uses type enforcement. The SELinux 
> community are all on board with KaiGai's object model (the object 
> classes and permissions and how they are enforced), there has been 
> quite a bit of discussion about them over the years. Trusted RUBIX 
> integrated SELinux using the object classes that KaiGai made for 
> SEPostgres.

Then document those in a reasonably formal sense. I don't think you can 
just say that the implementation is the spec. I should have thought that 
such a spec would actually appeal to the security community.

>
>> So I think if you want to get anywhere with this, scrap the code, and 
>> start
>> writing a specification.  One with references.  And then let's 
>> consider that
>> one.
>>
>
> Harsh.
>

Yeah, it is a bit. But we're being asked to swallow a fairly large lump, 
so don't be surprised if we gag a bit.

I haven't followed the entire history of this patch set closely, but we 
have over and over again emphasized the importance of getting community 
buyin before you start coding a large feature, and this is a *very* 
large feature. Reviewing the history briefly, it appears that KaiGai 
prepared an initial set of patches before ever approaching the Postgres 
community with it about 2 years ago. That is to some extent the source 
of the friction, I suspect.

I'm also slightly surprised that some of the government and commercial 
players in this space aren't speaking up much. I should have thought 
this would generate some interest from players as disparate as RedHat 
and the NSA.

cheers

andrew


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Robert Haas
Date:
On Mon, Jul 20, 2009 at 3:44 PM, Joshua Brindle<method@manicmethod.com> wrote:
> Ron Mayer wrote:
>>
>> Joshua Brindle wrote:
>>>
>>> How many people are you looking for? Is there a number or are you
>>> waiting for a good feeling?
>>
>> <snip>
>
>> Joshua - if you're still associated with Tresys - could someone
>> who could speak for that company say what they think about this
>> project?   The seem quite in-the-loop on what SELinux customers
>> want.
>
> I am capable of speaking for Tresys in this matter. We are very interested
> in this work and our US DoD customers need the capabilities that this
> project adds (assuming row level access controls are a possibility).

This is very interesting, if by "interested" you mean that you are
willing to commit time, money, and development resources to make it
happen.  If by "interested" you mean that you will use it if someone
else does the work of implementing it, that's nice, but fundamentally
doesn't help us very much.

At least in my view, the problem with this patch set is that it's not
committable, and it's not measurably closer to being committable than
it was a year ago. I am not saying (and I don't necessarily think
ANYONE is saying) "this is a great patch, but I don't want to commit
it because I hate SE-Linux".  What I at least am saying is that this
patch has serious problems that need to be fixed in order for it to be
committed, and because KaiGai has not been able to fix those problems
after over a year of back and forth, a new approach is needed here or
we are dead in the water.

I have attempted, on the relevant threads, to enumerate those problems
as I see them.  Mainly they have to do with hooks all over the code in
strange and unmaintainable places, documentation that is written in
poor English and is not easily understandable by people who are not
already experts in SE-Linux, and a complete inability to get a patch
that implements a useful subset of the total realm of SE-Linux
desiderata that more or less matches up with what the existing
PostgreSQL permissions structure already does.  What we've established
so far is that the following things should NOT be in the list of
permissions that we attempt in the initial patch:

- row-level security
- complex DDL permissions

But I think the following things probably should be:

- tables
- columns
- sequences
- functions
- schemas
- databases
- tablespaces

I'd be willing to negotiate on all but the first.

I also agree with Peter's contention that a spec would be useful.  If
you could read a clear description of what the patch was going to do,
then you could separate the problem of figuring out whether that was
the right thing from the question of whether the patch actually did
it.

To restate my basic point: The problem with this patch is that there
are plenty of people who are interested in *having* it, but the only
person who seems to be interested in *writing* it is KaiGai, and that
isn't working.

...Robert


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Greg Stark
Date:
On Mon, Jul 20, 2009 at 8:44 PM, Joshua Brindle<method@manicmethod.com> wrote:
> I am capable of speaking for Tresys in this matter. We are very interested
> in this work and our US DoD customers need the capabilities that this
> project adds (assuming row level access controls are a possibility).


I'm kind of curious about how these features get used. What specific
problems do they solve?

-- 
greg
http://mit.edu/~gsstark/resume.pdf


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
KaiGai Kohei
Date:
Greg Stark wrote:
> On Mon, Jul 20, 2009 at 8:44 PM, Joshua Brindle<method@manicmethod.com> wrote:
>> I am capable of speaking for Tresys in this matter. We are very interested
>> in this work and our US DoD customers need the capabilities that this
>> project adds (assuming row level access controls are a possibility).
> 
> 
> I'm kind of curious about how these features get used. What specific
> problems do they solve?

I would like to introduce a key word: data flow control (DFC).

Most of mandatory access control system focuses on the direction
of data, and tries to control it when user/client gives a request
to object manager (such as OS-kernel, RDBMS, ...).

MAC system assigns a security label on all the objects managed to
identify its sensitivity level. It typically has hierarchical
relationship, such as "secret" is more sensitive than "classified",
and "classified" is more than "unclassified", for example. secret > classified > unclassified

When user requires the object manager to read a certain object
with a security label being equal or lower than user's security
label, MAC system within the object manager allows it.
In this case, the direction of data is from object to subject. (Object) ---(read)---> (Subject)

When user requires the object manager to write a certain object
with a security label being equal to user's security label, MAC
system allows it.
In this case, the direction of data is from subject to object. (Subject) ---(write)---> (Object)

This constraint enables to prevent to leak a sensitive data to
others with lower sensitive level.
Note that subject never has data with higher than himself, and
he cannot write his data to objects lower than himself (to prevent
information leaks, by malicious internals) and higher than himself
(to prevent manipulation).

The security certification (ISO/IEC15408) also contains DFC as a
part of functional requirements. (Please note that it does not
requires DFC all the producets; it depends on the environment to
be used.)

Oracle Label Security is a product which provides DFC mechanism
using row-level access controls based on security labels, and
its security certification report mentions its DFC features and
access control rules in the FDP_IFF section.

SE-PostgreSQL also tries to apply such kind of DFC policies.
In addition, its security policy is integrated with operating system.
It enables to handle multiple object manager seamlessly.

For example, we cannot prevent a user with classified security label
to insert a sensitive information into database and unclassified user
to see them later, without SE-PgSQL.

BTW, Oracle Label Security is priced at about $13,000/CPU in Japan.
I believe security sensitive customers feel it fair for their purpose.

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


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Josh Berkus
Date:
> How many people are you looking for? Is there a number or are you
> waiting for a good feeling?

The problem is not the number of people who like the patch, but the 
number of people who are willing to refactor and  maintain it.  Right 
now, if NEC decided to abandon Postgres, or if they decided that they 
don't like the changes we make in order to merge it, we'd have nobody to 
maintain it.

Given the amount of money which the security community represents, it 
seems like at least a few of these people could become, or sponsor, code 
maintainers.

I was hoping to support some of this effort through Sun when SEPostgres 
was introduced, but that didn't happen.

-- 
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
KaiGai Kohei
Date:
Robert Haas wrote:
> I have attempted, on the relevant threads, to enumerate those problems
> as I see them.  Mainly they have to do with hooks all over the code in
> strange and unmaintainable places, documentation that is written in
> poor English and is not easily understandable by people who are not
> already experts in SE-Linux, and a complete inability to get a patch
> that implements a useful subset of the total realm of SE-Linux
> desiderata that more or less matches up with what the existing
> PostgreSQL permissions structure already does.  What we've established
> so far is that the following things should NOT be in the list of
> permissions that we attempt in the initial patch:
> 
> - row-level security
> - complex DDL permissions

Is the complex DDL permissions mean something like db_xxx:{create},
db_xxx:{relabelfrom relabelto} and others?
If so, I can agree to implement these checks at the later patch.

However, please note that the initial patch cannot achieve actual
security in this case, because it means anyone can change security
label of objects.

> But I think the following things probably should be:
> 
> - tables
> - columns
> - sequences
> - functions
> - schemas
> - databases
> - tablespaces

I also think it is reasonable to apply access controls on the types
of object classes at the initial pach.

> I'd be willing to negotiate on all but the first.
> 
> I also agree with Peter's contention that a spec would be useful.  If
> you could read a clear description of what the patch was going to do,
> then you could separate the problem of figuring out whether that was
> the right thing from the question of whether the patch actually did
> it.

I can also agree with the suggestion.
The specifications (from viewpoint of the developer) will introduces
the fundamental principles to be implemented, and it will figure out
what implementation is better.

As I noted before, I've been flexible about how SE-PgSQL is implemented
as far as it can perform SELinux's security model correctly.

Please for a several days. I'll describe it.
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Robert Haas
Date:
2009/7/20 KaiGai Kohei <kaigai@ak.jp.nec.com>:
> Robert Haas wrote:
>> - row-level security
>> - complex DDL permissions
>
> Is the complex DDL permissions mean something like db_xxx:{create},
> db_xxx:{relabelfrom relabelto} and others?
> If so, I can agree to implement these checks at the later patch.
>
> However, please note that the initial patch cannot achieve actual
> security in this case, because it means anyone can change security
> label of objects.

I'm not qualified to answer this question, and that's exactly why we
need more documentation of what this patch tries to do and why (i.e. a
spec).  What I was specifically referring to is things like
db_column:{drop}, which are much more specific than anything
PostgreSQL currently offers to control DDL.  I think we should make an
attempt to get objects labelled in a reasonable way (I did not like
the approach your latest patch took of just assigning everything a
default label), which might include relabelling also, but certainly
doesn't include things like distinguishing between different kinds of
DDL operations.  I'm not really up on SELinux terminology (this is
another thing that needs to be covered in the documentation, and
isn't) but maybe something like db_table:{owner}.

>> I also agree with Peter's contention that a spec would be useful.  If
>> you could read a clear description of what the patch was going to do,
>> then you could separate the problem of figuring out whether that was
>> the right thing from the question of whether the patch actually did
>> it.
>
> I can also agree with the suggestion.
> The specifications (from viewpoint of the developer) will introduces
> the fundamental principles to be implemented, and it will figure out
> what implementation is better.
>
> As I noted before, I've been flexible about how SE-PgSQL is implemented
> as far as it can perform SELinux's security model correctly.
>
> Please for a several days. I'll describe it.

I really, really think you need to find someone to help you with the
documentation.  As I've said before, your English is a lot better than
my Japanese, but the current documentation is just hard to read.  More
than that, writing good documentation is HARD, and there are not a ton
of people who can do it well.  It seems to me that the documentation
for this project could require as much work as the actual code.

And this is an excellent time to bring up the whole issue of community
involvement again.  Many advocacy-type folks and end-users and
security folks have written in over the last year to say how much they
want this feature, but as far as anyone can tell KaiGai is the only
one doing any actual development to make it happen.  Considering how
the alternative is, or so we're told, to spend $13,000/year (per CPU?)
for a similar Oracle feature, one might suppose that there would be
developers lining up to volunteer to help make this happen, and
companies making sponsorship dollars available to fund work by major
PostgreSQL contributors to get this whipped into shape.  No?  Well, at
the very least, one would hope that some of the people who say what
they want this feature to do could help document it, since they
presumably understand what it's supposed to do (if not, why are they
so sure they want it?).  But so far the only person who has done
anything like this, as far as I'm aware, is me.  And that's pretty
funny when you consider that I've learned enough about SE-Linux to do
exactly one thing: shut it off.

What I want for documentation of this feature is something like the
"Database Roles and Privileges" chapter of the current documentation.
You can read this chapter from beginning to end and have a pretty good
idea how to manage permissions in PostgreSQL.  SE-Linux is complex
enough that you might have to refer to other sources of information
for certain topics (e.g. policy writing), because a full overview of
those topics might exceed the scope of our documentation.  But it
should still be possible to start knowing nothing, read the chapter,
and have a pretty good idea how all the pieces fit together, at least
as far as PostgreSQL is concerned.

I would go so far as to suggest that we use the willingness of someone
from the community to volunteer to help KaiGai get this put together
as a litmus test for support for this patch.  If someone sent in a
patch for Hot Standby or Streaming Rep tomorrow that was done except
for the documentation, how long do you think it would take them to get
some volunteers to help finish the docs?  I'd bet less than a day.  On
the other hand, it appears to me that we have had more people put more
time into __builtin_clz() over the last three months than
SE-PostgreSQL.  __builtin_clz() has had multiple people benchmarking
it and testing it, giving feedback on the patch, etc.  More often than
not, nobody even responds to KaiGai's patch set; it looks to me like
the last response that he got prior to when I started reviewing this
for CommitFest 2009-07 was a note from David Wheeler admiring his
persistence; prior to that, the last response was back in April, when
Bruce asked him about including something in the 8.4 release notes.
All I can say is: ouch.

...Robert


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
KaiGai Kohei
Date:
Robert Haas wrote:
> 2009/7/20 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>> Robert Haas wrote:
>>> - row-level security
>>> - complex DDL permissions
>> Is the complex DDL permissions mean something like db_xxx:{create},
>> db_xxx:{relabelfrom relabelto} and others?
>> If so, I can agree to implement these checks at the later patch.
>>
>> However, please note that the initial patch cannot achieve actual
>> security in this case, because it means anyone can change security
>> label of objects.
> 
> I'm not qualified to answer this question, and that's exactly why we
> need more documentation of what this patch tries to do and why (i.e. a
> spec).

Agreed,

> What I was specifically referring to is things like
> db_column:{drop}, which are much more specific than anything
> PostgreSQL currently offers to control DDL.  I think we should make an
> attempt to get objects labelled in a reasonable way (I did not like
> the approach your latest patch took of just assigning everything a
> default label), which might include relabelling also, but certainly
> doesn't include things like distinguishing between different kinds of
> DDL operations.  I'm not really up on SELinux terminology (this is
> another thing that needs to be covered in the documentation, and
> isn't) but maybe something like db_table:{owner}.

It also should be described in the documentation again...

The current design reflects a fundamental principle in SELinux.
It requires how objects are labeled and what permission should
be checked on the creation/deletion time.

It seems to me you misunderstand the default security context is
uniform for all the clients, but it's incorrect.
The default security context depends on client's security context
and parent object's security context (if exists).
For example, when a user with "classified" security context creates
a table under the regular schema, the new table will be also labeled
as "classified" to prevent accesses from "unclassified" users.

All the object classes that we can create or delete (such as files,
sockets, ipc objects, x-windows, ...) have their permissions to be
checked on creation or deletion time. These permissions are defined
on the relationship between user's security context and object's
security context. If only database objects ignore the design, it
breaks consistency in the security policy.

Again, the fact I needed to introduce such a fundamental design in
SELinux shows the current documents are poor for native English
users and database experts but not for security.

>>> I also agree with Peter's contention that a spec would be useful.  If
>>> you could read a clear description of what the patch was going to do,
>>> then you could separate the problem of figuring out whether that was
>>> the right thing from the question of whether the patch actually did
>>> it.
>> I can also agree with the suggestion.
>> The specifications (from viewpoint of the developer) will introduces
>> the fundamental principles to be implemented, and it will figure out
>> what implementation is better.
>>
>> As I noted before, I've been flexible about how SE-PgSQL is implemented
>> as far as it can perform SELinux's security model correctly.
>>
>> Please for a several days. I'll describe it.
> 
> I really, really think you need to find someone to help you with the
> documentation.  As I've said before, your English is a lot better than
> my Japanese, but the current documentation is just hard to read.  More
> than that, writing good documentation is HARD, and there are not a ton
> of people who can do it well.  It seems to me that the documentation
> for this project could require as much work as the actual code.

Agreed,
If possible, I would like to know what is the easy-understandable
documentation from the viewpoint of non-security-experts also.

> What I want for documentation of this feature is something like the
> "Database Roles and Privileges" chapter of the current documentation.
> You can read this chapter from beginning to end and have a pretty good
> idea how to manage permissions in PostgreSQL.  SE-Linux is complex
> enough that you might have to refer to other sources of information
> for certain topics (e.g. policy writing), because a full overview of
> those topics might exceed the scope of our documentation.  But it
> should still be possible to start knowing nothing, read the chapter,
> and have a pretty good idea how all the pieces fit together, at least
> as far as PostgreSQL is concerned.

Indeed, I've learned before it is a good composition of documentations
to compare similar two different features (one is well-known, the other
is new) to introduce a new concept.

Here is one idea. I'll upload the draft of the documentation on the
wikipage shorter than the current one.
Is somebody available to check it from the viewpoint of native English
user or database users?

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


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Greg Williamson
Date:
KaiGai Kohei asked:


<...>

> 
> Here is one idea. I'll upload the draft of the documentation on the
> wikipage shorter than the current one.
> Is somebody available to check it from the viewpoint of native English
> user or database users?

I'll give a shot ... native english speaker, some experience with documentation,
and I'm fond of postgres, but not really deserving of being on this alias (not a
hacker in the useful sense of the word).

Greg Williamson

     


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
KaiGai Kohei
Date:
Greg Williamson wrote:
> KaiGai Kohei asked:
> 
> 
> <...>
> 
>> Here is one idea. I'll upload the draft of the documentation on the
>> wikipage shorter than the current one.
>> Is somebody available to check it from the viewpoint of native English
>> user or database users?
> 
> I'll give a shot ... native english speaker, some experience with documentation,
> and I'm fond of postgres, but not really deserving of being on this alias (not a
> hacker in the useful sense of the word).
> 
> Greg Williamson

Thanks for your help.
I'm now under writing the initial draft, seeing the "Database Roles and
Privileges" chapter. Please wait for a few days.

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


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Greg Stark
Date:
On Tue, Jul 21, 2009 at 5:51 AM, Robert Haas<robertmhaas@gmail.com> wrote:
>
> I really, really think you need to find someone to help you with the
> documentation.  As I've said before, your English is a lot better than
> my Japanese, but the current documentation is just hard to read.


In general we're very generous with English writing quality. I
wouldn't worry too much about the language barrier as far as writing
documentation.

However the snippets Robert posted of the documentation had a more
fundamental problem unrelated to language. The problem is that it
explained what an option did in terms of what that option did -- ie,
the documentation only made sense if you already knew what it was
trying to say. That's not an unusual trap to get into when writing
documentation regardless of what language you're writing in.

--
greg
http://mit.edu/~gsstark/resume.pdf


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Joshua Brindle
Date:
Greg Stark wrote:
> On Mon, Jul 20, 2009 at 8:44 PM, Joshua Brindle<method@manicmethod.com>  wrote:
>> I am capable of speaking for Tresys in this matter. We are very interested
>> in this work and our US DoD customers need the capabilities that this
>> project adds (assuming row level access controls are a possibility).
>
>
> I'm kind of curious about how these features get used. What specific
> problems do they solve?
>

Backing up from KaiGai's description a bit, basically what this is needed for is 
storing multilevel data in a single db instance.

For example, you have people logging in from different classifications (unclass, 
secret, top secret, etc) and the data they put in is marked (labeled) with their 
classification label.

Then for queries the policy is used to determine who can see what. An unclass 
user will only be able to see unclassified data. A top secret user, however, can 
see all the data from top secret, secret and unclassified. This is why the view 
model doesn't work, we need 'read down' support. This also implies that key 
constraints are held between data of differing levels. This has some information 
leak side effects (eg., if an unclass user inserts data then waits 5 minutes and 
inserts again and sees the key incremented greatly he knows lots of classified 
data is going into the database) but this is a small information leak compared 
to the gain in functionality of a read-down system.

The alternative to having this built in to the system is having many instances 
of postgres (or something else) running and having to query the right one to get 
the data (or all of them) and manually collating and associating rows. Very 
non-ideal. I have seen this method used en luau of having a multilevel database.

Another scenerio is that all the data being put in is of some classification 
(eg., radar data coming from a piece of equipment is all marked secret) but some 
of the data is top secret. For example a column that has precise coordinates of 
enemy soldiers is higher classification than the rest of the data. A secret user 
may be allowed to query the data with some 'fuzz', through a trusted stored 
procedure. So the secret user wouldn't have direct access to the coordinates but 
could call a stored procedure to get the information with precision removed.

This is why column level, row level and stored procedure access controls are all 
really required for the applications we are looking at.


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Greg Stark
Date:
On Tue, Jul 21, 2009 at 3:20 PM, Joshua Brindle<method@manicmethod.com> wrote:
>
> Backing up from KaiGai's description a bit, basically what this is needed
> for is storing multilevel data in a single db instance.
>
> For example, you have people logging in from different classifications
> (unclass, secret, top secret, etc) and the data they put in is marked
> (labeled) with their classification label.
>


I'm beginning to wonder if we haven't gone about this all wrong. Every
time someone asks my question about use cases the only answers that
come back are about row-level security. Perhaps that's the only case
that really matters here.

If we provide a way to control access to database objects through
SELinux policies -- ie, one which is functionally equivalent to what
we have today but just allows administrators to control it in the same
place they control other SELinux system privileges, is that useful? Is
that something SE administrators want? Or are they happy to use
Postgres roles and grants and just want the finer row-level data
access controls?

-- 
greg
http://mit.edu/~gsstark/resume.pdf


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Joshua Brindle
Date:
Greg Stark wrote:
> On Tue, Jul 21, 2009 at 3:20 PM, Joshua Brindle<method@manicmethod.com>  wrote:
>> Backing up from KaiGai's description a bit, basically what this is needed
>> for is storing multilevel data in a single db instance.
>>
>> For example, you have people logging in from different classifications
>> (unclass, secret, top secret, etc) and the data they put in is marked
>> (labeled) with their classification label.
>>
>
>
> I'm beginning to wonder if we haven't gone about this all wrong. Every
> time someone asks my question about use cases the only answers that
> come back are about row-level security. Perhaps that's the only case
> that really matters here.
>
> If we provide a way to control access to database objects through
> SELinux policies -- ie, one which is functionally equivalent to what
> we have today but just allows administrators to control it in the same
> place they control other SELinux system privileges, is that useful? Is
> that something SE administrators want? Or are they happy to use
> Postgres roles and grants and just want the finer row-level data
> access controls?
>

No, for multiple reasons. First a single person (role) could be logging in at 
different levels (eg., running the same application as the same linux user with 
the same credentials) and would need to see different things from the database. 
The SELinux contexts would provide the differentiation in this case and the 
SELinux policy would enforce the multilevel policy.

I also don't think your roles and grants could enforce a multilevel policy but 
that is something I'd have to look into deeper to know for sure. Also the 
objects need to be labeled based on how they were inserted, and 're-grading' 
applications need to be able to relabel them. You still need the 'read-down' 
behavior I talked about above.

You also snipped the other scenario I had where row based access control isn't 
required but column level and stored procedure level are. I understand you 
already have column level access controls but it still goes back to how the user 
is accessing the data, as a top secret user who can read the column with full 
precision or as a secret user with precision removed via a trusted stored 
procedure. The SELinux policy would have to give the stored procedure the 
ability to read the column and trust it to remove the necessary amount of precision.


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Greg Stark
Date:
On Tue, Jul 21, 2009 at 4:24 PM, Joshua Brindle<method@manicmethod.com> wrote:
> You also snipped the other scenario I had where row based access control
> isn't required but column level and stored procedure level are.

Well we already have column level and stored procedure privileges.

> I understand
> you already have column level access controls but it still goes back to how
> the user is accessing the data, as a top secret user who can read the column
> with full precision or as a secret user with precision removed via a trusted
> stored procedure.

Sure, and people do this every day already with postgres roles and privileges.

> The SELinux policy would have to give the stored procedure
> the ability to read the column and trust it to remove the necessary amount
> of precision.

Well the question is: Is the important feature of SEPostgres the
unification of the privilege model with every other piece of the
system in the SELinux policy? Or is that not the main thing and only
the row-level access security is interesting. None of the use cases
seem to put any emphasis on the unification of the security policy.

-- 
greg
http://mit.edu/~gsstark/resume.pdf


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
KaiGai Kohei
Date:
Greg Stark wrote:
> On Tue, Jul 21, 2009 at 4:24 PM, Joshua Brindle<method@manicmethod.com> wrote:
>> You also snipped the other scenario I had where row based access control
>> isn't required but column level and stored procedure level are.
> 
> Well we already have column level and stored procedure privileges.
> 
>> I understand
>> you already have column level access controls but it still goes back to how
>> the user is accessing the data, as a top secret user who can read the column
>> with full precision or as a secret user with precision removed via a trusted
>> stored procedure.
> 
> Sure, and people do this every day already with postgres roles and privileges.
> 
>> The SELinux policy would have to give the stored procedure
>> the ability to read the column and trust it to remove the necessary amount
>> of precision.
> 
> Well the question is: Is the important feature of SEPostgres the
> unification of the privilege model with every other piece of the
> system in the SELinux policy? Or is that not the main thing and only
> the row-level access security is interesting. None of the use cases
> seem to put any emphasis on the unification of the security policy.

It is the most significant feature of SE-PostgreSQL to enforce the
centralized security policy, as if SELinux enforces its policy to
the accesses on objects manged by operating system.

Indeed, SELinux can control accesses on user's data stored within
filesystems (operating system feature) based on its security policy.
However, nowadays, filesystem is not only piece to store user's data
with variable credentials (ie; secret, unclassified, ...).

Needless to say, database is a significant pieces to store variable
user's data, but it is not invisible from the operating system how
does it controled, because all the database objects are managed in
userspace. It means SELinux cannot control data flows via databases,
although it occupies an important place in our information system.

Granularity in access controls is an independent issue.

Integration of the access control policy is an issue how to decide
whether the required accesses to be allowed, or not.

On the other hand, table/column/row level access control is an issue
what objects to be controled.

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


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Joshua Brindle
Date:
<br /><br /> Greg Stark wrote: <blockquote cite="mid:407d949e0907211538wb1e5cc5gbf65b8d085c9684@mail.gmail.com"
type="cite"><prewrap="">On Tue, Jul 21, 2009 at 4:24 PM, Joshua Brindle<a class="moz-txt-link-rfc2396E"
href="mailto:method@manicmethod.com"><method@manicmethod.com></a>wrote: </pre><blockquote type="cite"><pre
wrap="">Youalso snipped the other scenario I had where row based access control
 
isn't required but column level and stored procedure level are.   </pre></blockquote><pre wrap="">
Well we already have column level and stored procedure privileges.
 </pre><blockquote type="cite"><pre wrap="">I understand
you already have column level access controls but it still goes back to how
the user is accessing the data, as a top secret user who can read the column
with full precision or as a secret user with precision removed via a trusted
stored procedure.   </pre></blockquote><pre wrap="">
Sure, and people do this every day already with postgres roles and privileges.
 </pre><blockquote type="cite"><pre wrap="">The SELinux policy would have to give the stored procedure
the ability to read the column and trust it to remove the necessary amount
of precision.   </pre></blockquote><pre wrap="">
Well the question is: Is the important feature of SEPostgres the
unification of the privilege model with every other piece of the
system in the SELinux policy? Or is that not the main thing and only
the row-level access security is interesting. None of the use cases
seem to put any emphasis on the unification of the security policy.
 </pre></blockquote><br /> I don't understand how you came to this conclusion. Quoting from my prior email:<br /><br />
""No,for multiple reasons. First a single person (role) could be logging in at different levels (eg., running the same
applicationas the same linux user with the same credentials) and would need to see different things from the database.
TheSELinux contexts would provide the differentiation in this case and the SELinux policy would enforce the multilevel
policy.""<br /><br /> In this case the selinux context (which specifies their MLS level) says the user is running at
unclassand there should be no possible credentials or role or anything that gives him access to data above unclass in
thedatabase. This _is_ a unification of the policy because whether the unclass user is accessing files or rows in a
mixed-leveldatabase or entire databases that are classified as such the SELinux policy is governing that. <br /><br />
Itsthe same with selinux aware X. If a user is running 2 database clients, one at unclass and one at secret then each
applicationwould how the appropriate results. The policy also disallows that user from copying from the secret database
clientinto the unclass client (in this case there are SELinux rules enforcing behavior of X apps, of the database
clientsand of the filesystem), unified indeed.<br /> 

Re: [PATCH] SE-PgSQL/tiny rev.2193

From
KaiGai Kohei
Date:
Greg Stark wrote:
> On Tue, Jul 21, 2009 at 5:51 AM, Robert Haas<robertmhaas@gmail.com> wrote:
>> I really, really think you need to find someone to help you with the
>> documentation.  As I've said before, your English is a lot better than
>> my Japanese, but the current documentation is just hard to read.
> 
> 
> In general we're very generous with English writing quality. I
> wouldn't worry too much about the language barrier as far as writing
> documentation.
> 
> However the snippets Robert posted of the documentation had a more
> fundamental problem unrelated to language. The problem is that it
> explained what an option did in terms of what that option did -- ie,
> the documentation only made sense if you already knew what it was
> trying to say. That's not an unusual trap to get into when writing
> documentation regardless of what language you're writing in.
> 

Indeed, language quality is a causality, but not all.

The matter is we've not been able to share what does it try to
achieve correctly, what is identicals or differences between
database privileges and SELinux's security model, and so on.

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


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
Peter Eisentraut
Date:
On Monday 20 July 2009 17:52:44 Joshua Brindle wrote:
> That is your (and the communities) prerogative. Linus wasn't very
> supportive of SELinux in the kernel either but it is the only way Linux got
> an EAL4+ LSPP evaluation for use in certain government systems. I
> personally would love to see an open source DBMS evaluated for systems like
> this because the current state of the art is fairly sad.

This would actually be a reasonable baseline to work against, if we define a 
project goal to be satisfying this standard.

This is presumably the web site that describes this standard: http://www.niap-
ccevs.org/cc-scheme/pp/pp_os_ls_v1.b/  There I see

Succeeded By:  pp_os_ml_mr2.0_v1.91  
Sunset Date: 16 September 2007

And the successor document is vastly more comprehensive than implementing a 
MAC scheme.

So how do we realistically get from here to there (and where is "there")?


Re: [PATCH] SE-PgSQL/tiny rev.2193

From
KaiGai Kohei
Date:
Peter Eisentraut wrote:
> On Monday 20 July 2009 17:52:44 Joshua Brindle wrote:
>> That is your (and the communities) prerogative. Linus wasn't very
>> supportive of SELinux in the kernel either but it is the only way Linux got
>> an EAL4+ LSPP evaluation for use in certain government systems. I
>> personally would love to see an open source DBMS evaluated for systems like
>> this because the current state of the art is fairly sad.
> 
> This would actually be a reasonable baseline to work against, if we define a 
> project goal to be satisfying this standard.
> 
> This is presumably the web site that describes this standard: http://www.niap-
> ccevs.org/cc-scheme/pp/pp_os_ls_v1.b/  There I see
> 
> Succeeded By:  pp_os_ml_mr2.0_v1.91  
> Sunset Date: 16 September 2007
> 
> And the successor document is vastly more comprehensive than implementing a 
> MAC scheme.

The target of this protection profile is operating system,
so it is a bit mismatch.

Referring to the prior case, Oracle Label Security (it also provides
label based access controls, but no collaboration with OS) is cerfified
with the BR-DBMSPP (U.S.Government Protection Profile Database Management
Systems For Basic Robustness Environments) and additional functional
components (such as FDP_IFC.1: Subset Information Flow Control,
FDP_IFF.2: Hierarchical Security Attributes, and so on).

http://www.commoncriteriaportal.org/products_DB.html#DB

* ST for Oracle Label Security for Oracle Database 10g Release 2
http://www.commoncriteriaportal.org/files/epfiles/20080306_0402b.pdf

> So how do we realistically get from here to there (and where is "there")?

Security functionality is a factor to get ISO/IEC15408 certification,
but not all. (For example, who can defray the cost for evaluation?)
However, it is important to pull up the baseline functionality to
satisfy these security functional components.

If certification is only valid on the "patched" version, rest of people
will not be happy.

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