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

Started by KaiGai Koheiabout 17 years ago73 messageshackers
Jump to latest
#1KaiGai Kohei
kaigai@ak.jp.nec.com

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>

#2KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: KaiGai Kohei (#1)
[PATCH] SE-PostgreSQL for v8.5 development (r1819)

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>

#3Bruce Momjian
bruce@momjian.us
In reply to: KaiGai Kohei (#2)
Re: [PATCH] SE-PostgreSQL for v8.5 development (r1819)

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. +

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#3)
Re: [PATCH] SE-PostgreSQL for v8.5 development (r1819)

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

#5KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Tom Lane (#4)
Re: [PATCH] SE-PostgreSQL for v8.5 development (r1819)

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>

#6Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#4)
Re: [PATCH] SE-PostgreSQL for v8.5 development (r1819)

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. +

#7Bruce Momjian
bruce@momjian.us
In reply to: KaiGai Kohei (#5)
Re: [PATCH] SE-PostgreSQL for v8.5 development (r1819)

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. +

#8KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: KaiGai Kohei (#2)
[PATCH] SE-PostgreSQL for v8.5 development (r1891)

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>

#9KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: KaiGai Kohei (#8)
[PATCH][v8.5] SE-PostgreSQL Patch Updates (r2016)

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 | aaa
system_u:object_r:sepgsql_table_t:s0:c2 | 2 | bbb
system_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 reclaimed
security_reclaim_label
------------------------
4
(1 row)

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

#10David E. Wheeler
david@kineticode.com
In reply to: KaiGai Kohei (#9)
Re: [PATCH][v8.5] SE-PostgreSQL Patch Updates (r2016)

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

#11KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: KaiGai Kohei (#9)
[PATCH] SE-PostgreSQL Updates rev.2096

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 extensions in 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>

#12Robert Haas
robertmhaas@gmail.com
In reply to: KaiGai Kohei (#11)
Re: [PATCH] SE-PostgreSQL Updates rev.2096

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 -1
219 files changed, 11819 insertions(+), 29 deletions(-), 857 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.

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

#13KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Robert Haas (#12)
Re: [PATCH] SE-PostgreSQL Updates rev.2096

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 stuff including 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>

#14Robert Haas
robertmhaas@gmail.com
In reply to: KaiGai Kohei (#13)
Re: [PATCH] SE-PostgreSQL Updates rev.2096

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

#15KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Robert Haas (#14)
Re: [PATCH] SE-PostgreSQL Updates rev.2096

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>

#16Robert Haas
robertmhaas@gmail.com
In reply to: KaiGai Kohei (#15)
Re: [PATCH] SE-PostgreSQL Updates rev.2096

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

#17KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: KaiGai Kohei (#15)
[PATCH] SE-PgSQL/lite rev.2163

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.

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.
It enables to assign a certain security label on several kinds of database
objects. The security label in text form is stored within the new system
catalog (pg_security), and the catalog/pg_security.c provides a facility
to translate it and the security identifier.

* Core facility to communicate with in-kernel SELinux and to make its
decision on various kind of database objcets. (2nd patch)
The second patch provides the core functionality to perform with SELinux.
It deploys security hooks on the strategic points of PostgreSQL.
The hooks invoke SE-PostgreSQL routines, when it is enabled. The routines
makes its decision based on the system's security policy.
The userspace access vector cache (src/backend/security/sepgsql/avc.c)
minimizes the number of kernel space invocations, and enables to make
a decision (previously asked) without context switching.
The routines of security hooks (hooks.c and checker.c) pulls the security
label of given database objects like a table, and asks the userspace AVC
whether 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 given
based on the security policy. But we can give an explicit security label
for a new object, as far as user is allowed to create it with the given
security label.
This patch provide SECURITY_LABEL = '...' option for several kinds of
CREATE or ALTER statement. It allows 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-level
access controls 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 are newlines 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>

#18Robert Haas
robertmhaas@gmail.com
In reply to: KaiGai Kohei (#17)
Re: [PATCH] SE-PgSQL/lite rev.2163

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

#19KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Robert Haas (#18)
Re: [PATCH] SE-PgSQL/lite rev.2163

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>

#20Robert Haas
robertmhaas@gmail.com
In reply to: KaiGai Kohei (#19)
Re: [PATCH] SE-PgSQL/lite rev.2163

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

#21KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Robert Haas (#20)
#22KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: KaiGai Kohei (#21)
#23Robert Haas
robertmhaas@gmail.com
In reply to: KaiGai Kohei (#22)
#24KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Robert Haas (#23)
#25Robert Haas
robertmhaas@gmail.com
In reply to: KaiGai Kohei (#24)
#26KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Robert Haas (#25)
#27Robert Haas
robertmhaas@gmail.com
In reply to: KaiGai Kohei (#26)
#28KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Robert Haas (#27)
#29KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: KaiGai Kohei (#26)
#30Robert Haas
robertmhaas@gmail.com
In reply to: KaiGai Kohei (#28)
#31KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Robert Haas (#30)
#32KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: KaiGai Kohei (#29)
#33Robert Haas
robertmhaas@gmail.com
In reply to: KaiGai Kohei (#31)
#34KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Robert Haas (#33)
#35Robert Haas
robertmhaas@gmail.com
In reply to: KaiGai Kohei (#32)
#36KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Robert Haas (#35)
#37Robert Haas
robertmhaas@gmail.com
In reply to: KaiGai Kohei (#36)
#38KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Robert Haas (#37)
#39Peter Eisentraut
peter_e@gmx.net
In reply to: Robert Haas (#37)
#40Martijn van Oosterhout
kleptog@svana.org
In reply to: Peter Eisentraut (#39)
#41Robert Haas
robertmhaas@gmail.com
In reply to: Martijn van Oosterhout (#40)
#42KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Robert Haas (#41)
#43Joshua Brindle
method@manicmethod.com
In reply to: Robert Haas (#41)
#44Martijn van Oosterhout
kleptog@svana.org
In reply to: Joshua Brindle (#43)
#45Joshua Brindle
method@manicmethod.com
In reply to: Martijn van Oosterhout (#44)
#46Tom Lane
tgl@sss.pgh.pa.us
In reply to: Martijn van Oosterhout (#44)
#47Joshua Brindle
method@manicmethod.com
In reply to: Tom Lane (#46)
#48Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Joshua Brindle (#47)
#49Peter Eisentraut
peter_e@gmx.net
In reply to: Joshua Brindle (#47)
#50Joshua Brindle
method@manicmethod.com
In reply to: Peter Eisentraut (#49)
#51Ron Mayer
rm_pg@cheapcomplexdevices.com
In reply to: Joshua Brindle (#47)
#52Joshua Brindle
method@manicmethod.com
In reply to: Ron Mayer (#51)
#53KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Peter Eisentraut (#49)
#54Andrew Dunstan
andrew@dunslane.net
In reply to: Joshua Brindle (#50)
#55Robert Haas
robertmhaas@gmail.com
In reply to: Joshua Brindle (#52)
#56Bruce Momjian
bruce@momjian.us
In reply to: Joshua Brindle (#52)
#57KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Bruce Momjian (#56)
#58Josh Berkus
josh@agliodbs.com
In reply to: Joshua Brindle (#47)
#59KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Robert Haas (#55)
#60Robert Haas
robertmhaas@gmail.com
In reply to: KaiGai Kohei (#59)
#61KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Robert Haas (#60)
#62Greg Williamson
gwilliamson39@yahoo.com
In reply to: KaiGai Kohei (#61)
#63KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Greg Williamson (#62)
#64Bruce Momjian
bruce@momjian.us
In reply to: Robert Haas (#60)
#65Joshua Brindle
method@manicmethod.com
In reply to: Bruce Momjian (#56)
#66Bruce Momjian
bruce@momjian.us
In reply to: Joshua Brindle (#65)
#67Joshua Brindle
method@manicmethod.com
In reply to: Bruce Momjian (#66)
#68Bruce Momjian
bruce@momjian.us
In reply to: Joshua Brindle (#67)
#69KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Bruce Momjian (#68)
#70Joshua Brindle
method@manicmethod.com
In reply to: Bruce Momjian (#68)
#71KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Bruce Momjian (#64)
#72Peter Eisentraut
peter_e@gmx.net
In reply to: Joshua Brindle (#43)
#73KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Peter Eisentraut (#72)