Thread: [PATCH] SE-PostgreSQL for v8.5 development (r1769)
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>
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>
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. +
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
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>
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. +
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. +
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>
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>
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
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>
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
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>
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
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>
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
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>
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
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>
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
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>
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>
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
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>
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
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>
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
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>
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>
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
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>
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>
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
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>
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
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>
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
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>
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.
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.
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
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>
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.
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.
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.
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
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.
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.
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.
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.
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.
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).
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>
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
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
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
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>
> 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
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>
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
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>
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
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>
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
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.
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
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.
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
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>
<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 />
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>
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")?
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>