Re: [0/4] Proposal of SE-PostgreSQL patches

The series of patches are the proposal of Security-Enhanced PostgreSQL
(SE-PostgreSQL) for the upstreamed PostgreSQL 8.4 development cycle.

[1/4] sepostgresql-pgace-8.4devel-3.patch
provides PGACE (PostgreSQL Access Control Extension) framework
[2/4] sepostgresql-sepgsql-8.4devel-3.patch
provides SE-PostgreSQL feature, based on PGACE framework.
[3/4] sepostgresql-pg_dump-8.4devel-3.patch
enables pg_dump to dump database with security attribute.
[4/4] sepostgresql-policy-8.4devel-3.patch
provides the default security policy for SE-PostgreSQL.

We can provide a quick overview of SE-PostgreSQL at:
http://code.google.com/p/sepgsql/wiki/WhatIsSEPostgreSQL

Any comment and suggestion are welcome.
Thanks,

ENVIRONMENT
-----------
Please confirm your environment.
The followings are requriements of SE-PostgreSQL.
* Fedora 8 or later system
* SELinux is enabled and working
* kernel-2.6.24 or later
* selinux-policy and selinux-policy-devel v3.0.8 or later
* libselinux, policycoreutils

INSTALLATION
------------
$ tar jxvf postgresql-snapshot.tar.bz2
$ cd postgresql-snapshot
$ patch -p1 < ../sepostgresql-pgace-8.4devel-3.patch
$ patch -p1 < ../sepostgresql-sepgsql-8.4devel-3.patch
$ patch -p1 < ../sepostgresql-pg_dump-8.4devel-3.patch
$ patch -p1 < ../sepostgresql-policy-8.4devel-3.patch

$ ./configure --enable-selinux
$ make
$ make -C contrib/sepgsql-policy
$ su
# make install

# /usr/sbin/semodule -i contrib/sepgsql-policy/sepostgresql.pp
(NOTE: semodule is a utility to load/unload security policy modules.)

# /sbin/restorecon -R /usr/local/pgsql
(NOTE: restorecon is a utilicy to initialize security context of files.)

SETUP
-----
# mkdir -p /opt/sepgsql
# chown foo_user:var_group /opt/sepgsql
# chcon -t postgresql_db_t /opt/sepgsql
(NOTE: chcon is a utility to set up security context of files.)
# exit

$ /usr/sbin/run_init /usr/local/pgsql/bin/initdb -D /opt/sepgsql
(NOTE: run_init is a utility to start a program, as if it is branched from init script.)
$ /usr/local/pgsql/bin/pg_ctl -D /opt/sepgsql start

--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

[3/4] - sepostgresql-pg_dump-8.4devel-3.patch

This patch gives us a feature to dump database with security attribute.
It is turned on with '--enable-selinux' option at pg_dump/pg_dumpall,
when the server works as SE- version.
No need to say, users need to have enough capabilities to dump whole of
database. It it same when they tries to restore the database.

--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

[4/4] - sepostgresql-policy-8.4devel-3.patch

This patch gives us the default security policy for SE-PostgreSQL.
You can build it as a security policy module. It can be linked with
the existing distributor's policy, and reloaded.

--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

It seems to me some of SE-PostgreSQL patches are not delivered yet,
although [3/4] and [4/4] were already done.

Does anti-spam system caught my previous three messages?
If necessary, I will send them again.

Thanks,

Kohei KaiGai wrote:
> The series of patches are the proposal of Security-Enhanced PostgreSQL
> (SE-PostgreSQL) for the upstreamed PostgreSQL 8.4 development cycle.
>
> [1/4] sepostgresql-pgace-8.4devel-3.patch
> provides PGACE (PostgreSQL Access Control Extension) framework
> [2/4] sepostgresql-sepgsql-8.4devel-3.patch
> provides SE-PostgreSQL feature, based on PGACE framework.
> [3/4] sepostgresql-pg_dump-8.4devel-3.patch
> enables pg_dump to dump database with security attribute.
> [4/4] sepostgresql-policy-8.4devel-3.patch
> provides the default security policy for SE-PostgreSQL.
- snip -

--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

Kohei KaiGai napsal(a):
> It seems to me some of SE-PostgreSQL patches are not delivered yet,
> although [3/4] and [4/4] were already done.
>
> Does anti-spam system caught my previous three messages?
> If necessary, I will send them again.

There is a file size limitation. If your patch is too big (I guess over
40kB), please gzip it or send only url for download.

Zdenek

Zdenek Kotala wrote:
> Kohei KaiGai napsal(a):
>> It seems to me some of SE-PostgreSQL patches are not delivered yet,
>> although [3/4] and [4/4] were already done.
>>
>> Does anti-spam system caught my previous three messages?
>> If necessary, I will send them again.
>
> There is a file size limitation. If your patch is too big (I guess over
> 40kB), please gzip it or send only url for download.
>
> Zdenek

Thanks for your information,

Your estimation is correct. Two of them are over the limitaion.
So, I'll send it again with gzip'ed attachment.

[kaigai(at)saba a]$ ls -lh *-8.4devel-*.patch
-rw-r--r-- 1 kaigai users 17K 2008-03-17 13:01 sepostgresql-pg_dump-8.4devel-3.patch
-rw-r--r-- 1 kaigai users 134K 2008-03-17 13:01 sepostgresql-pgace-8.4devel-3.patch
-rw-r--r-- 1 kaigai users 17K 2008-03-17 13:01 sepostgresql-policy-8.4devel-3.patch
-rw-r--r-- 1 kaigai users 138K 2008-03-17 13:01 sepostgresql-sepgsql-8.4devel-3.patch

--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

[1/4] - sepostgresql-pgace-8.4devel-3.patch.gz

This patch provides PGACE (PostgreSQL Access Control Extension) framework.

It has a similar idea of LSM (Linu Security Module).
It can provide a guest module several hooks at strategic points.
The guest module can make its decision whether required actions should be
allowed, or not.
In addition, PGACE also provides falicilites to manage security attribute
of database objects. Any tuple can have a its security attribute, and the
guest module can refer it to control accesses.

A more conprehensive memo at:
http://code.google.com/p/sepgsql/wiki/WhatIsPGACE

(This patch is gzip'ed, bacause it overed the limitation of filesize.)

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

[2/4] - sepostgresql-sepgsql-8.4devel-3.patch.gz

This patch provides SE-PostgreSQL facilities based on PGACE.

Security-Enhanced PostgreSQL (SE-PostgreSQL) is a security extension
built in PostgreSQL, to provide system-wide consistency in access
controls. It enables to apply a single unigied security policy of
SELinux for both operating system and database management system.
In addition, it also provides fine-grained mandatory access which
includes column-/row- level non-bypassable access control even if
privileged database users.

Quick overview at:
http://code.google.com/p/sepgsql/wiki/WhatIsSEPostgreSQL

(This patch is gzip'ed, bacause it overed the limitation of filesize.)

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

Kohei KaiGai wrote:
> The series of patches are the proposal of Security-Enhanced PostgreSQL
> (SE-PostgreSQL) for the upstreamed PostgreSQL 8.4 development cycle.

Before we go any further, is this work derived from SELinux? If so, is
it covered under the GPL? If so, can it be licensed under BSD terms?

Obviously, if it's not BSD, we cannot include it in Postgres.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera wrote:
> Kohei KaiGai wrote:
>> The series of patches are the proposal of Security-Enhanced PostgreSQL
>> (SE-PostgreSQL) for the upstreamed PostgreSQL 8.4 development cycle.
>
> Before we go any further, is this work derived from SELinux? If so, is
> it covered under the GPL? If so, can it be licensed under BSD terms?
>
> Obviously, if it's not BSD, we cannot include it in Postgres.

All of SE-PostgreSQL works are licensed unser BSD terms.
We are considering to push SE-PostgreSQL into upstreamed PostgreSQL from
the beginning, and we understand to choose GPL makes it impossible.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

I'll submit the proposal of SE-PostgreSQL patches again, because some of previous
messages are filtered due to attachment and I cannot provide whole of patches yet.
Please refer the pointed URL, as follows.

------
The series of patches are the proposal of Security-Enhanced PostgreSQL (SE-PostgreSQL)
for the upstreamed PostgreSQL 8.4 development cycle.

[1/4] sepostgresql-pgace-8.4devel-3.patch
provides PGACE (PostgreSQL Access Control Extension) framework
http://sepgsql.googlecode.com/files/sepostgresql-pgace-8.4devel-3-r704.patch

[2/4] sepostgresql-sepgsql-8.4devel-3.patch
provides SE-PostgreSQL feature, based on PGACE framework.
http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r704.patch

[3/4] sepostgresql-pg_dump-8.4devel-3.patch
enables pg_dump to dump database with security attribute.
http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r704.patch

[4/4] sepostgresql-policy-8.4devel-3.patch
provides the default security policy for SE-PostgreSQL.
http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r704.patch

We can provide a quick overview for SE-PostgreSQL at:
http://code.google.com/p/sepgsql/wiki/WhatIsSEPostgreSQL

ENVIRONMENT
-----------
Please confirm your environment.
The followings are requriements of SE-PostgreSQL.
* Fedora 8 or later system
* SELinux is enabled and working
* kernel-2.6.24 or later
* selinux-policy and selinux-policy-devel v3.0.8 or later
* libselinux, policycoreutils

INSTALLATION
------------
$ tar jxvf postgresql-snapshot.tar.bz2
$ cd postgresql-snapshot
$ patch -p1 < ../sepostgresql-pgace-8.4devel-3.patch
$ patch -p1 < ../sepostgresql-sepgsql-8.4devel-3.patch
$ patch -p1 < ../sepostgresql-pg_dump-8.4devel-3.patch
$ patch -p1 < ../sepostgresql-policy-8.4devel-3.patch

$ ./configure --enable-selinux
$ make
$ make -C contrib/sepgsql-policy
$ su
# make install

# /usr/sbin/semodule -i contrib/sepgsql-policy/sepostgresql.pp
(NOTE: semodule is a utility to load/unload security policy modules.)

# /sbin/restorecon -R /usr/local/pgsql
(NOTE: restorecon is a utilicy to initialize security context of files.)

SETUP
-----
# mkdir -p /opt/sepgsql
# chown foo_user:var_group /opt/sepgsql
# chcon -t postgresql_db_t /opt/sepgsql
(NOTE: chcon is a utility to set up security context of files.)
# exit

$ /usr/sbin/run_init /usr/local/pgsql/bin/initdb -D /opt/sepgsql
(NOTE: run_init is a utility to start a program, as if it is branched from init script.)
$ /usr/local/pgsql/bin/pg_ctl -D /opt/sepgsql start

SUMMARYS FOR EVERY PATCHES
--------------------------
[1/4] - sepostgresql-pgace-8.4devel-3.patch

This patch provides PGACE (PostgreSQL Access Control Extension) framework.

It has a similar idea of LSM (Linu Security Module).
It can provide a guest module several hooks at strategic points.
The guest module can make its decision whether required actions should be
allowed, or not.
In addition, PGACE also provides falicilites to manage security attribute
of database objects. Any tuple can have a its security attribute, and the
guest module can refer it to control accesses.

A more conprehensive memo at:
http://code.google.com/p/sepgsql/wiki/WhatIsPGACE

[2/4] - sepostgresql-sepgsql-8.4devel-3.patch

This patch provides SE-PostgreSQL facilities based on PGACE.

Security-Enhanced PostgreSQL (SE-PostgreSQL) is a security extension
built in PostgreSQL, to provide system-wide consistency in access
controls. It enables to apply a single unigied security policy of
SELinux for both operating system and database management system.
In addition, it also provides fine-grained mandatory access which
includes column-/row- level non-bypassable access control even if
privileged database users.

Quick overview at:
http://code.google.com/p/sepgsql/wiki/WhatIsSEPostgreSQL

[3/4] - sepostgresql-pg_dump-8.4devel-3.patch

This patch gives us a feature to dump database with security attribute.
It is turned on with '--enable-selinux' option at pg_dump/pg_dumpall,
when the server works as SE- version.
No need to say, users need to have enough capabilities to dump whole of
database. It it same when they tries to restore the database.

[4/4] - sepostgresql-policy-8.4devel-3.patch

This patch gives us the default security policy for SE-PostgreSQL.
You can build it as a security policy module. It can be linked with
the existing distributor's policy, and reloaded.

--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

KaiGai Kohei wrote:
> Alvaro Herrera wrote:

>> Before we go any further, is this work derived from SELinux? If so, is
>> it covered under the GPL? If so, can it be licensed under BSD terms?
>
> All of SE-PostgreSQL works are licensed unser BSD terms.
> We are considering to push SE-PostgreSQL into upstreamed PostgreSQL from
> the beginning, and we understand to choose GPL makes it impossible.

Right. The question is: since this is derived from SE-Linux, is it
affected by SE-Linux license?

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

Alvaro Herrera wrote:
> KaiGai Kohei wrote:
>> Alvaro Herrera wrote:
>
>>> Before we go any further, is this work derived from SELinux? If so, is
>>> it covered under the GPL? If so, can it be licensed under BSD terms?
>> All of SE-PostgreSQL works are licensed unser BSD terms.
>> We are considering to push SE-PostgreSQL into upstreamed PostgreSQL from
>> the beginning, and we understand to choose GPL makes it impossible.
>
> Right. The question is: since this is derived from SE-Linux, is it
> affected by SE-Linux license?

No, SE-PostgreSQL does not derivered from SELinux.

I guess you worry about SE-PostgreSQL contains a part of SELinux licensed
as GPL, but it is incorrect.
SE-PostgreSQL communicate with SELinux to make its decision in access control,
via an official interface provided by libselinux, because it does not have
information to make its decision.
The libselinux is linked with SE-PostgreSQL, but it is licensed as public
domain software by NSA.

Therefore, we have no issues related to imcompatible licenses.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

On Mon, 17 Mar 2008, KaiGai Kohei wrote:

> I'll submit the proposal of SE-PostgreSQL patches again, because some of previous
> messages are filtered due to attachment and I cannot provide whole of patches yet.

This is actually what you should have done from the beginning. And it
only should have gone to the pgsql-hackers list, which is the only one I'm
replying to. Your patches are at this point a proposal, as you say in the
subject, and those go to the pgsql-hackers list with the minimum of files
necessary to support them. pgsql-patches is generally aimed at patches
that have already been discussed on the hackers list, ones that are
basically ready to apply to the source code.

> The libselinux is linked with SE-PostgreSQL, but it is licensed as
> public domain software by NSA.

As for the licensing issues here, what everyone is looking for is a clear
statement of the SELinux license from the source of that code. The
official NSA statment at http://www.nsa.gov/selinux/info/license.cfm says:

"All source code found on this site is released under the same terms and
conditions as the original sources. For example, the patches to the Linux
kernel, patches to many existing utilities, and some of the new programs
available here are released under the terms and conditions of the GNU
General Public License (GPL). Please refer to the source code for specific
license information."

GPL is a perfectly good license, but it's far from clear whether code
derived from it can be incorporated into PostgreSQL even if you wrote all
of it yourself. I just checked libselinux, and as you say it includes a
LICENSE file that states "This library (libselinux) is public domain
software, i.e. not copyrighted.". That's good, but a similar independant
review will need to happen for every component you interact with here, on
top of a technical review. Luckily this is something a lot of people
would like and that should all get taken care of.

--
* Greg Smith gsmith(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD

KaiGai,

> The series of patches are the proposal of Security-Enhanced PostgreSQL
> (SE-PostgreSQL) for the upstreamed PostgreSQL 8.4 development cycle.

Since I'm (Finally!) expecting the TrustedSolaris folks to put some work into
PostgreSQL as well this year, I'm going to ask them to look over PGACE to see
if this implementation is (still) generic enough to support TS as well. If
it is, then it's probably generic enough to be a general building block.

--
Josh Berkus
PostgreSQL @ Sun
San Francisco

"Greg Smith" <gsmith(at)gregsmith(dot)com> writes:

> On Mon, 17 Mar 2008, KaiGai Kohei wrote:
>
>> I'll submit the proposal of SE-PostgreSQL patches again, because some of previous
>> messages are filtered due to attachment and I cannot provide whole of patches yet.
>
> This is actually what you should have done from the beginning. And it only
> should have gone to the pgsql-hackers list, which is the only one I'm replying
> to. Your patches are at this point a proposal, as you say in the subject, and
> those go to the pgsql-hackers list with the minimum of files necessary to
> support them. pgsql-patches is generally aimed at patches that have already
> been discussed on the hackers list, ones that are basically ready to apply to
> the source code.

Some people shout any time you send patches to -hackers. For the -patches is
there mainly to catch large attachments regardless of their maturity.

But it's true that it's best to post a plan and have discussion prior to
developing big patches.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's PostGIS support!

On Mon, 17 Mar 2008, Gregory Stark wrote:

> Some people shout any time you send patches to -hackers.

Right, but if you note the improved version I give the thumbs-up to didn't
include any patches--just links to where you could get them. There's
little reason to include any code as an attachment for a proposal if you
can direct people to the web for them. That's why I suggested sending
"the minimum of files necessary", which in this case was zero.

--
* Greg Smith gsmith(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD

Josh Berkus wrote:
> KaiGai,
>
>> The series of patches are the proposal of Security-Enhanced PostgreSQL
>> (SE-PostgreSQL) for the upstreamed PostgreSQL 8.4 development cycle.
>
> Since I'm (Finally!) expecting the TrustedSolaris folks to put some work into
> PostgreSQL as well this year, I'm going to ask them to look over PGACE to see
> if this implementation is (still) generic enough to support TS as well. If
> it is, then it's probably generic enough to be a general building block.

We can extend PGACE framework to mount TrustedSolaris features.
If they need new hooks which is not used in SE-PostgreSQL, it can
remain the default behavior.
The default PGACE behavior gives us no effects in access controls.

A flexible framework is worthwhile for both operating systems.
Please confirm it to the TS folks.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

Greg Smith wrote:
> On Mon, 17 Mar 2008, KaiGai Kohei wrote:
>
>> I'll submit the proposal of SE-PostgreSQL patches again, because some
>> of previous
>> messages are filtered due to attachment and I cannot provide whole of
>> patches yet.
>
> This is actually what you should have done from the beginning. And it
> only should have gone to the pgsql-hackers list, which is the only one
> I'm replying to. Your patches are at this point a proposal, as you say
> in the subject, and those go to the pgsql-hackers list with the minimum
> of files necessary to support them. pgsql-patches is generally aimed at
> patches that have already been discussed on the hackers list, ones that
> are basically ready to apply to the source code.

OK, I can understand the purpose of pgsql-hackers and pgsql-patches list.
At first, I'll have a discussion here.

>> The libselinux is linked with SE-PostgreSQL, but it is licensed as
>> public domain software by NSA.
>
> As for the licensing issues here, what everyone is looking for is a
> clear statement of the SELinux license from the source of that code.
> The official NSA statment at http://www.nsa.gov/selinux/info/license.cfm
> says:
>
> "All source code found on this site is released under the same terms and
> conditions as the original sources. For example, the patches to the
> Linux kernel, patches to many existing utilities, and some of the new
> programs available here are released under the terms and conditions of
> the GNU General Public License (GPL). Please refer to the source code
> for specific license information."
>
> GPL is a perfectly good license, but it's far from clear whether code
> derived from it can be incorporated into PostgreSQL even if you wrote
> all of it yourself. I just checked libselinux, and as you say it
> includes a LICENSE file that states "This library (libselinux) is public
> domain software, i.e. not copyrighted.". That's good, but a similar
> independant review will need to happen for every component you interact
> with here, on top of a technical review. Luckily this is something a
> lot of people would like and that should all get taken care of.

SE-PostgreSQL internally uses libselinux, glibc and PostgreSQL internal
APIs like SearchSysCache().
I'm not a lawyer, but I believe they cannot enforce us to apply a specific
lisence. So, I clearly say SE-PostgreSQL feature is licensed with the same
one of PostgreSQL.
No need to say, more conprehensive checks and reviews are welcome.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Tue, 18 Mar 2008 10:41:42 +0900
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:

> > GPL is a perfectly good license, but it's far from clear whether
> > code derived from it can be incorporated into PostgreSQL even if
> > you wrote all of it yourself. I just checked libselinux, and as
> > you say it includes a LICENSE file that states "This library
> > (libselinux) is public domain software, i.e. not copyrighted.".
> > That's good, but a similar independant review will need to happen
> > for every component you interact with here, on top of a technical
> > review. Luckily this is something a lot of people would like and
> > that should all get taken care of.
>
> SE-PostgreSQL internally uses libselinux, glibc and PostgreSQL
> internal APIs like SearchSysCache().
> I'm not a lawyer, but I believe they cannot enforce us to apply a
> specific lisence. So, I clearly say SE-PostgreSQL feature is licensed
> with the same one of PostgreSQL.
> No need to say, more conprehensive checks and reviews are welcome.

Hmmm,

Everything that I read says that libselinux is GPL. That could present
a problem for anyone that wants to use the BSD features of
PostgreSQL :).

I can check with SFLC if people are really curious.

Sincerely,

Joshua D. Drake

- --
The PostgreSQL Company since 1997: http://www.commandprompt.com/
PostgreSQL Community Conference: http://www.postgresqlconference.org/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL political pundit | Mocker of Dolphins

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFH3x/wATb/zqfZUUQRAq6WAJ9h2ecrYrsZ5bJUTJGhyS2LZSOqkACfeGoB
EHwcHtq7Ow5k3AlKNPwOVzs=
=yamT
-----END PGP SIGNATURE-----

Joshua D. Drake wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On Tue, 18 Mar 2008 10:41:42 +0900
> KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
>
>>> GPL is a perfectly good license, but it's far from clear whether
>>> code derived from it can be incorporated into PostgreSQL even if
>>> you wrote all of it yourself. I just checked libselinux, and as
>>> you say it includes a LICENSE file that states "This library
>>> (libselinux) is public domain software, i.e. not copyrighted.".
>>> That's good, but a similar independant review will need to happen
>>> for every component you interact with here, on top of a technical
>>> review. Luckily this is something a lot of people would like and
>>> that should all get taken care of.
>> SE-PostgreSQL internally uses libselinux, glibc and PostgreSQL
>> internal APIs like SearchSysCache().
>> I'm not a lawyer, but I believe they cannot enforce us to apply a
>> specific lisence. So, I clearly say SE-PostgreSQL feature is licensed
>> with the same one of PostgreSQL.
>> No need to say, more conprehensive checks and reviews are welcome.
>
> Hmmm,
>
> Everything that I read says that libselinux is GPL. That could present
> a problem for anyone that wants to use the BSD features of
> PostgreSQL :).

It is incorrect.
SELinux is indeed GPL because it is a part of kernel feature.
But libselinux is a public domain software, as follows:
https://selinux.svn.sourceforge.net/svnroot/selinux/trunk/libselinux/LICENSE

SE-PostgreSQL is linked with *ONLY* libselinux. It communicate to SELinux
via system call. As you know, GPL does not consider invokation of system calls
as a "link". Thus, we can release SE-PostgreSQL as a BSD licensed software.

Thanks,

> I can check with SFLC if people are really curious.
>
> Sincerely,
>
> Joshua D. Drake
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

"Joshua D. Drake" <jd(at)commandprompt(dot)com> writes:
> Everything that I read says that libselinux is GPL.

What exactly did you read? The LICENSE file in the source tarball
says

: This library (libselinux) is public domain software, i.e. not copyrighted.
:
: Warranty Exclusion
: ------------------
: You agree that this software is a
: non-commercially developed program that may contain "bugs" (as that
: term is used in the industry) and that it may not function as intended.
: The software is licensed "as is". NSA makes no, and hereby expressly
: disclaims all, warranties, express, implied, statutory, or otherwise
: with respect to the software, including noninfringement and the implied
: warranties of merchantability and fitness for a particular purpose.
:
: Limitation of Liability
: -----------------------
: In no event will NSA be liable for any damages, including loss of data,
: lost profits, cost of cover, or other special, incidental,
: consequential, direct or indirect damages arising from the software or
: the use thereof, however caused and on any theory of liability. This
: limitation will apply even if NSA has been advised of the possibility
: of such damage. You acknowledge that this is a reasonable allocation of
: risk.

I have a feeling that NSA did not bother to run this by any actual
lawyers, because the second and third paragraphs are only meaningful as
part of a license (ie something recipients agree to) and by definition
there is no license on public-domain software. They've given up
*everything*, including the right to make you agree to any terms of
distribution.

Not that anyone would likely be dumb enough to try to sue the NSA,
but it's still pretty funny.

regards, tom lane

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Mon, 17 Mar 2008 22:45:14 -0400
Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> "Joshua D. Drake" <jd(at)commandprompt(dot)com> writes:
> > Everything that I read says that libselinux is GPL.
>
> What exactly did you read? The LICENSE file in the source tarball
> says

I was reading a directory listing. I found the actual license file and
you are correct.

>
> Not that anyone would likely be dumb enough to try to sue the NSA,
> but it's still pretty funny.
>

One of those few entities on the planet that it just really doesn't
matter how much money you have. You don't sue them. In fact, NSA who?

Joshua D. Drake

- --
The PostgreSQL Company since 1997: http://www.commandprompt.com/
PostgreSQL Community Conference: http://www.postgresqlconference.org/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL political pundit | Mocker of Dolphins

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFH3y31ATb/zqfZUUQRAgs/AKCJORQO+N4xfTeG6MXvhXFMn06DKwCfT33V
j8xTb5clqdPK3zWnA3XYuMQ=
=mVVu
-----END PGP SIGNATURE-----

KaiGai,

> A flexible framework is worthwhile for both operating systems.
> Please confirm it to the TS folks.

Yep, that's the idea. Glenn was fine with your stuff last year, I expect
it'll still be fine.

Other than SELinux and TrustedSolaris, does anyone know of other role-based or
multilevel security frameworks we should check against? I'd like to have any
security framework we approve be plug-in adaptable to everything out there.

--
Josh Berkus
PostgreSQL @ Sun
San Francisco

Gregory Stark wrote:
> "Greg Smith" <gsmith(at)gregsmith(dot)com> writes:
>
>> On Mon, 17 Mar 2008, KaiGai Kohei wrote:
>>
>>> I'll submit the proposal of SE-PostgreSQL patches again, because some of previous
>>> messages are filtered due to attachment and I cannot provide whole of patches yet.
>> This is actually what you should have done from the beginning. And it only
>> should have gone to the pgsql-hackers list, which is the only one I'm replying
>> to. Your patches are at this point a proposal, as you say in the subject, and
>> those go to the pgsql-hackers list with the minimum of files necessary to
>> support them. pgsql-patches is generally aimed at patches that have already
>> been discussed on the hackers list, ones that are basically ready to apply to
>> the source code.
>
> Some people shout any time you send patches to -hackers. For the -patches is
> there mainly to catch large attachments regardless of their maturity.
>
> But it's true that it's best to post a plan and have discussion prior to
> developing big patches.

Yes, it might be a better way to develop this feature on reflection.

I'm sorry that I could not submit a proposal by the feature freeze
date of v8.3 unfortunately, so the development of SE-PostgreSQL
(based on v8.2) is overlapped with development cycle of v8.3.

I don't want to repeat same thing twice, so these series of patches
are submitted fot v8.4 development cycle.
The first two of them ([1/4] and [2/4]) are significant part of
SE-PostgreSQL. We can discuss rest of them later. They contains
utility extension and security policy.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

Josh Berkus wrote:
> KaiGai,
>
>> A flexible framework is worthwhile for both operating systems.
>> Please confirm it to the TS folks.
>
> Yep, that's the idea. Glenn was fine with your stuff last year, I expect
> it'll still be fine.
>
> Other than SELinux and TrustedSolaris, does anyone know of other role-based or
> multilevel security frameworks we should check against? I'd like to have any
> security framework we approve be plug-in adaptable to everything out there.

In the recent news,
http://www.sun.com/aboutsun/pr/2008-03/sunflash.20080313.1.xml

NSA and SUN announces a new project launched which tries to port FLASK
security architecture, it is named as FMAC.
It can be a good candidate to host multilevel database security.
(SELinux is also based on FLASK security architecture.)

SEBSD project is also known. It is a poring of SELinux into BSD platform.
SEBSD: http://www.trustedbsd.org/sebsd.html

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

KaiGai,

> NSA and SUN announces a new project launched which tries to port FLASK
> security architecture, it is named as FMAC.
> It can be a good candidate to host multilevel database security.
> (SELinux is also based on FLASK security architecture.)
>
> SEBSD project is also known. It is a poring of SELinux into BSD platform.
> SEBSD: http://www.trustedbsd.org/sebsd.html

Keen. I'm sure Kathy's team understands all this. It's over my head. ;-)

--
Josh Berkus
PostgreSQL @ Sun
San Francisco

For easier reviewing, the most fundamental patch to implement PGACE (PostgreSQL
Access Control Extension) was separated into thress parts.

I want to start discussion about PGACE security framework at first.
Any comments are welcome.

[1/3] PGACE core features
http://sepgsql.googlecode.com/files/sepostgresql-8.4devel-pgace-1-core.r713.patch

It adds the following new files:
- src/include/security/pgace.h
declares all of PGACE related hooks and functions.
- src/backend/security/pgaceHooks.c
provides dummy functions to be invoked when no security modules are enabled.
They don't affect anything in access control.
- src/backend/security/pgaceCommon.c
provides common feature for every security modules, including security-attribute
system column support, SQL extention, functions to manage security-attribute of
large object.

[2/3] Security attribute system column
http://sepgsql.googlecode.com/files/sepostgresql-8.4devel-pgace-2-security-attr.r713.patch

Guest modules of PGACE can associate a security attribute with a tuple.
The guest module can utilize this to make its decision in access control.
(Unclassified users cannot access 'Secret' tuples, for example)
This attribute is stored in the padding field of HeapTupleHeaderData, as
oid doing. It requires additional "sizeof(Oid)" bytes to store it.

Users can refer this attribute via system column.
The name of new system column is defined as SECURITY_SYSATTR_NAME
at include/pg_config.h.in, and the guest module decide its name.
In SE-PostgreSQL, it is named as "security_context".

EXAMPLE of security attribute)
postgres=# SELECT security_context, * FROM drink;
security_context | id | name | price | alcohol
------------------------------------------+----+-------+-------+---------
unconfined_u:object_r:sepgsql_table_t:s0 | 1 | water | 100 | f
unconfined_u:object_r:sepgsql_table_t:s0 | 2 | coke | 120 | f
unconfined_u:object_r:sepgsql_table_t:s0 | 3 | juice | 130 | f
system_u:object_r:sepgsql_table_t:s0:c0 | 4 | cofee | 180 | f
system_u:object_r:sepgsql_table_t:s0:c0 | 5 | beer | 240 | t
system_u:object_r:sepgsql_table_t:s0:c0 | 6 | sake | 320 | t
(6 rows)

We can use this security attribute as a target of UPDATE or INSERT statement.
It enables DBA to manage security attribute with normal SQL operation.

[3/3] PGACE security hooks
http://sepgsql.googlecode.com/files/sepostgresql-8.4devel-pgace-3-security-hooks.r713.patch

This patch deploys several PGACE hooks on strategic points in PostgreSQL.
These hooks invoke a security module mounted on PGACE, and it can make its
decision whether this action should be allowed, or not.

The list of PGACE hooks at:
http://code.google.com/p/sepgsql/wiki/WhatIsPGACE
It shouws us more comprehensive specification about what kind of hooks are provided,
what informations are given and what value should be returned.

NOTE: I categorized patched files into three parts. However, some of them
contains security attribute system column facilities and PGACE hooks
facilities.
In this case, I categorized these files into part 2.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

I updated the series of SE-PostgreSQL patches for the latest pgsql-8.4devel tree.

[1/4] sepostgresql-pgace-8.4devel-3-r739.patch
provides PGACE (PostgreSQL Access Control Extension) framework.
http://sepgsql.googlecode.com/files/sepostgresql-pgace-8.4devel-3-r739.patch

[2/4] sepostgresql-sepgsql-8.4devel-3-r739.patch
provides SE-PostgreSQL feature, based on PGACE framework.
http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r739.patch

[3/4] sepostgresql-pg_dump-8.4devel-3-r739.patch
enables to dump databases with security attribute.
http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r739.patch

[4/4] sepostgresql-policy-8.4devel-3-r739.patch
provides the default security policy of SE-PostgreSQL.
http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r739.patch

We provide a quick overview for SE-PostgreSQL at:
http://code.google.com/p/sepgsql/wiki/WhatIsSEPostgreSQL

Thanks,

KaiGai Kohei wrote:
> The series of patches are the proposal of Security-Enhanced PostgreSQL (SE-PostgreSQL)
> for the upstreamed PostgreSQL 8.4 development cycle.
>
> [1/4] sepostgresql-pgace-8.4devel-3.patch
> provides PGACE (PostgreSQL Access Control Extension) framework
> http://sepgsql.googlecode.com/files/sepostgresql-pgace-8.4devel-3-r704.patch
>
> [2/4] sepostgresql-sepgsql-8.4devel-3.patch
> provides SE-PostgreSQL feature, based on PGACE framework.
> http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r704.patch
>
> [3/4] sepostgresql-pg_dump-8.4devel-3.patch
> enables pg_dump to dump database with security attribute.
> http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r704.patch
>
> [4/4] sepostgresql-policy-8.4devel-3.patch
> provides the default security policy for SE-PostgreSQL.
> http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r704.patch
>
> We can provide a quick overview for SE-PostgreSQL at:
> http://code.google.com/p/sepgsql/wiki/WhatIsSEPostgreSQL
>
> ENVIRONMENT
> -----------
> Please confirm your environment.
> The followings are requriements of SE-PostgreSQL.
> * Fedora 8 or later system
> * SELinux is enabled and working
> * kernel-2.6.24 or later
> * selinux-policy and selinux-policy-devel v3.0.8 or later
> * libselinux, policycoreutils
>
> INSTALLATION
> ------------
> $ tar jxvf postgresql-snapshot.tar.bz2
> $ cd postgresql-snapshot
> $ patch -p1 < ../sepostgresql-pgace-8.4devel-3.patch
> $ patch -p1 < ../sepostgresql-sepgsql-8.4devel-3.patch
> $ patch -p1 < ../sepostgresql-pg_dump-8.4devel-3.patch
> $ patch -p1 < ../sepostgresql-policy-8.4devel-3.patch
>
> $ ./configure --enable-selinux
> $ make
> $ make -C contrib/sepgsql-policy
> $ su
> # make install
>
> # /usr/sbin/semodule -i contrib/sepgsql-policy/sepostgresql.pp
> (NOTE: semodule is a utility to load/unload security policy modules.)
>
> # /sbin/restorecon -R /usr/local/pgsql
> (NOTE: restorecon is a utilicy to initialize security context of files.)
>
> SETUP
> -----
> # mkdir -p /opt/sepgsql
> # chown foo_user:var_group /opt/sepgsql
> # chcon -t postgresql_db_t /opt/sepgsql
> (NOTE: chcon is a utility to set up security context of files.)
> # exit
>
> $ /usr/sbin/run_init /usr/local/pgsql/bin/initdb -D /opt/sepgsql
> (NOTE: run_init is a utility to start a program, as if it is branched from init script.)
> $ /usr/local/pgsql/bin/pg_ctl -D /opt/sepgsql start
>
>
> SUMMARYS FOR EVERY PATCHES
> --------------------------
> [1/4] - sepostgresql-pgace-8.4devel-3.patch
>
> This patch provides PGACE (PostgreSQL Access Control Extension) framework.
>
> It has a similar idea of LSM (Linu Security Module).
> It can provide a guest module several hooks at strategic points.
> The guest module can make its decision whether required actions should be
> allowed, or not.
> In addition, PGACE also provides falicilites to manage security attribute
> of database objects. Any tuple can have a its security attribute, and the
> guest module can refer it to control accesses.
>
> A more conprehensive memo at:
> http://code.google.com/p/sepgsql/wiki/WhatIsPGACE
>
> [2/4] - sepostgresql-sepgsql-8.4devel-3.patch
>
> This patch provides SE-PostgreSQL facilities based on PGACE.
>
> Security-Enhanced PostgreSQL (SE-PostgreSQL) is a security extension
> built in PostgreSQL, to provide system-wide consistency in access
> controls. It enables to apply a single unigied security policy of
> SELinux for both operating system and database management system.
> In addition, it also provides fine-grained mandatory access which
> includes column-/row- level non-bypassable access control even if
> privileged database users.
>
> Quick overview at:
> http://code.google.com/p/sepgsql/wiki/WhatIsSEPostgreSQL
>
> [3/4] - sepostgresql-pg_dump-8.4devel-3.patch
>
> This patch gives us a feature to dump database with security attribute.
> It is turned on with '--enable-selinux' option at pg_dump/pg_dumpall,
> when the server works as SE- version.
> No need to say, users need to have enough capabilities to dump whole of
> database. It it same when they tries to restore the database.
>
> [4/4] - sepostgresql-policy-8.4devel-3.patch
>
> This patch gives us the default security policy for SE-PostgreSQL.
> You can build it as a security policy module. It can be linked with
> the existing distributor's policy, and reloaded.
>

--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

Folks,

For hackers who don't understand security frameworks, I'm going to make a
strong case for KaiGai's patch. Because of my current presentation
series, I've been talking to PostgreSQL users about security features
around the world for the last several months, and there's a *lot* of
interest in security framework support. Not only are existing users
interested in it, but some potential users (security agencies, banks) who
didn't use PostgreSQL before have come to talk to me becuase of
SE-Postgres.

Further, I've asked the TrustedSolaris folks to take a look at KaiGai's
implementation to see if it was "generic" enough for them to build on as a
test of whether SE-Postgres was too specific to SE-Linux; the answer has
been a tentative "yes, it's generic".

So it would be much better to have this functionality be "mainstream"
rather than a fork. If it does get bounced, please do it becuase of code
quality and not because "nobody is asking for this".

Thanks!

--
--Josh

Josh Berkus
PostgreSQL @ Sun
San Francisco

Josh Berkus wrote:
> Folks,

> So it would be much better to have this functionality be "mainstream"
> rather than a fork. If it does get bounced, please do it becuase of code
> quality and not because "nobody is asking for this".

+1

Joshua D. Drake

On 01/05/2008, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> Further, I've asked the TrustedSolaris folks to take a look at KaiGai's
> implementation to see if it was "generic" enough for them to build on as a
> test of whether SE-Postgres was too specific to SE-Linux; the answer has
> been a tentative "yes, it's generic".

> So it would be much better to have this functionality be "mainstream"
> rather than a fork. If it does get bounced, please do it becuase of code
> quality and not because "nobody is asking for this".

Not a hacker, just a curious reader ... are there equivalent frameworks
for the other supported platforms? E.g. MacOS, *BSD, Windows? Are
the similarities between those (if they exist) close enough not to introduce
a maintenance nightmare?

Cheers,
Andrej

--
Please don't top post, and don't use HTML e-Mail :} Make your quotes concise.

http://www.american.edu/econ/notes/htmlmail.htm

On Thu, 1 May 2008, Andrej Ricnik-Bay wrote:

> Not a hacker, just a curious reader ... are there equivalent frameworks
> for the other supported platforms? E.g. MacOS, *BSD, Windows?

SELinux is a Linux implementation of ideas from an earlier NSA project
named Flask. There is port of another variant of that, Flask/TE, that is
making its way into the BSD variants via a project called SEBSD.
TrustedBSD, Darwin (OS X), and OpenSolaris all have projects in this area
already (the Solaris one just launched last month). A good starter page
is http://www.trustedbsd.org/sebsd.html

Particularly given the common heritage, I suspect that the PostgreSQL side
of all these projects will be similar, and that once those hooks are in
place it will just be a matter of tying them into the higher levels of the
other framework. It would be too ambitious to target all of them all at
once for a first pass, but it may be worth a look at the fundamentals of
SEBSD to make sure the right hooks look like they're in place.

Windows has this thing called "Group Policy" that's supposedly leaped
forward for Windows Server 2008. They are now advertising it as like
SELinux, but better. The presentation PDF I just read on that subject
sounds like something written by the crazy guy at Broadway & 57th street I
used to walk by, as he talked on fruit as if they were his cell phone.
It's such a deluded and wildly misguided bit of sales fluff that you can't
take it seriously, and the whole thing just leaves me feeling sorry for
them instead.

--
* Greg Smith gsmith(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD

On Wed, 30 Apr 2008, KaiGai Kohei wrote:

> [1/4] sepostgresql-pgace-8.4devel-3-r739.patch
> provides PGACE (PostgreSQL Access Control Extension) framework.
> http://sepgsql.googlecode.com/files/sepostgresql-pgace-8.4devel-3-r739.patch

For those overwhelmed by sheer volume here, this is the patch to start
with, because it's got all the core changes to the server. I'm also in
the camp that would like to see this feature added, but rather than just
giving it a +1 I started looking at it.

The overall code is nice: easy to understand, structured modularly. I
have some concerns though. The first two things that jump out at me on an
initial review appear right from the beginning for those who want to take
a look:

-I'm a bit unnerved by both the performance and reliability implications
from how the security check calls are done in every case, even if there is
no SELinux support included. Those checks are sitting in some pretty low
level tuple and heap calls.

The approach taken here is to put all the "#ifdef" logic into the
underlying ACE interface (see patch [2/4]), so that the caller doesn't
have to care. If SELinux support is off then the calls turns into

void x(y) {} or
bool a(b) { return true; }

This is a very clean design, but it's putting extra (possibly optimized
away) calls into a lot of places. While it would be uglier, it might make
sense to put that on/off logic in all the places where the calls are made,
so that when you turn SELinux support off most of the code really does go
completely away rather than just turning into stubs.

-The only error reporting and handling method used is "elog(ERROR,...".
That seems a bit heavy handed for something that can be expected to happen
all the time.

If I understand this correctly, when you're scanning a table with 1000
rows where you're only allowed to see 50% of them, that's going to be 500
call to elog(), one for each tuple you can't see. Having a tuple get
screened out isn't really an error per se, and while I can see how
sensitive installs would want those all reported there are others where
this volume of log activity would be too much. Just because someone with
classified clearance is looking at a big table that also has a lot of
secret info in it, not all installs will want a million errors reported
just because there's data that person can't see available.

At a minimum, this needs some finer log control, and maybe a rethinking
altogether of how to handle error cases.

--
* Greg Smith gsmith(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD

"Greg Smith" <gsmith(at)gregsmith(dot)com> writes:

> The approach taken here is to put all the "#ifdef" logic into the underlying
> ACE interface (see patch [2/4]), so that the caller doesn't have to care. If
> SELinux support is off then the calls turns into
>
> void x(y) {} or
> bool a(b) { return true; }
>
> This is a very clean design, but it's putting extra (possibly optimized away)
> calls into a lot of places. While it would be uglier, it might make sense to
> put that on/off logic in all the places where the calls are made, so that when
> you turn SELinux support off most of the code really does go completely away
> rather than just turning into stubs.

It's nicer to do it the way they have but we don't generally trust compilers
to inline functions. Is it hard to make those functions into macros?

> -The only error reporting and handling method used is "elog(ERROR,...". That
> seems a bit heavy handed for something that can be expected to happen all the
> time.
>
> If I understand this correctly, when you're scanning a table with 1000 rows
> where you're only allowed to see 50% of them, that's going to be 500 call to
> elog(), one for each tuple you can't see. Having a tuple get screened out
> isn't really an error per se, and while I can see how sensitive installs would
> want those all reported there are others where this volume of log activity
> would be too much. Just because someone with classified clearance is looking
> at a big table that also has a lot of secret info in it, not all installs will
> want a million errors reported just because there's data that person can't see
> available.

I don't understand, if it's ERROR it would throw an error and stop the current
query. Or is this all within a PG_TRY() ?

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's RemoteDBA services!

Greg Smith wrote:
> On Wed, 30 Apr 2008, KaiGai Kohei wrote:
>
>> [1/4] sepostgresql-pgace-8.4devel-3-r739.patch
>> provides PGACE (PostgreSQL Access Control Extension) framework.
>>
>> http://sepgsql.googlecode.com/files/sepostgresql-pgace-8.4devel-3-r739.patch
>>
>
> For those overwhelmed by sheer volume here, this is the patch to start
> with, because it's got all the core changes to the server. I'm also in
> the camp that would like to see this feature added, but rather than just
> giving it a +1 I started looking at it.
>
> The overall code is nice: easy to understand, structured modularly. I
> have some concerns though. The first two things that jump out at me on
> an initial review appear right from the beginning for those who want to
> take a look:

Thanks for your attention & reviewing.

> -I'm a bit unnerved by both the performance and reliability implications
> from how the security check calls are done in every case, even if there
> is no SELinux support included. Those checks are sitting in some pretty
> low level tuple and heap calls.
>
> The approach taken here is to put all the "#ifdef" logic into the
> underlying ACE interface (see patch [2/4]), so that the caller doesn't
> have to care. If SELinux support is off then the calls turns into
>
> void x(y) {} or
> bool a(b) { return true; }
>
> This is a very clean design, but it's putting extra (possibly optimized
> away) calls into a lot of places. While it would be uglier, it might
> make sense to put that on/off logic in all the places where the calls
> are made, so that when you turn SELinux support off most of the code
> really does go completely away rather than just turning into stubs.

Your concern is fair enough. I indeed used inline function to avoid unnecessary
invokation in the previous version, as follows:
http://code.google.com/p/sepgsql/source/browse/branches/pgsql-8.2.x/sepgsql/src/include/security/pgace.h

However, I changed this design, because it requires any security modules
have to provides its implementation for any hooks, including just a stub.
In the current design, author of another security modules can leave unused
hooks unchanged. It improves code maintenancability.

But an inspiration hit me just now.
If we declare any hooks as static inline functions which includes #ifdef
block, it can resolve these matters as follows:
---- EXAMPLE ----
static inline bool pgaceHeapTupleInsert(Relation rel, HeapTuple tuple,
bool is_internal, bool with_returning) {
#ifdef HAVE_SELINUX
if (sepgsqlIsEnabled())
return sepgsqlHeapTupleInsert(rel, tuple, is_internal, with_returning);
#endif
return true;
}
----------------
Is it reasonable idea?

> -The only error reporting and handling method used is "elog(ERROR,...".
> That seems a bit heavy handed for something that can be expected to
> happen all the time.

SE-PostgreSQL invokes "elog(ERROR,...", when a given query tries to access
violated columns, tables and so on, but not for each tuple.

In row-level access controls, it filters violated tuples without aborting
query execution. However, "elog(NOTICE,..." can be invoked, if the security
policy requires to generate access denied logs on row-level.

> If I understand this correctly, when you're scanning a table with 1000
> rows where you're only allowed to see 50% of them, that's going to be
> 500 call to elog(), one for each tuple you can't see.

The security policy can control whether access denied logs should be
printed, or not.
In the default security policy provided by the patch [4/4], row-level
access denied logs are disabled to avoid flood of logs as you noticed.

If you want to turn on/off row-level logs, use the following command:
# setsebool -P sepgsql_enable_audittuple=(1|0)
It changes the internal state of security policy, and SE-PostgreSQL
works according to this one.

> Having a tuple
> get screened out isn't really an error per se, and while I can see how
> sensitive installs would want those all reported there are others where
> this volume of log activity would be too much. Just because someone
> with classified clearance is looking at a big table that also has a lot
> of secret info in it, not all installs will want a million errors
> reported just because there's data that person can't see available.
>
> At a minimum, this needs some finer log control, and maybe a rethinking
> altogether of how to handle error cases.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> writes:
> I updated the series of SE-PostgreSQL patches for the latest pgsql-8.4devel tree.

I tried to do a bit of testing of this, but it does not work on current
Fedora 8, because the policy module doesn't build:

[tgl(at)rh2 sepgsql-policy]$ make
make -f /usr/share/selinux/devel/Makefile NAME=targeted
make[1]: Entering directory `/home/tgl/sepgsql/contrib/sepgsql-policy'
Compiling targeted sepostgresql module
/usr/bin/checkmodule: loading policy configuration from tmp/sepostgresql.tmp
sepostgresql.te:349:ERROR 'syntax error' at token 'corenet_tcp_recvfrom_labeled' on line 5675:
# NOTE: These changes are to be merged in the later releases.
corenet_tcp_recvfrom_labeled(sepgsql_server_type, sepgsql_client_type)
/usr/bin/checkmodule: error(s) encountered while parsing configuration
make[1]: *** [tmp/sepostgresql.mod] Error 1
make[1]: Leaving directory `/home/tgl/sepgsql/contrib/sepgsql-policy'
make: *** [sepostgresql.pp] Error 2
[tgl(at)rh2 sepgsql-policy]$

In the meantime, here are some random comments after my failed test
build and a very fast scan through the patch:

The patch tries to re-add ipcclean to the source tree --- probably a
merge error?

autoconf complains about the description-free AC_DEFINEs

Doesn't compile warning-free if selinux not enabled ... for that matter,
it doesn't compile warning-free if selinux IS enabled.

No documentation updates whatsoever :-(

About half of the patch seems to be conditional on
#ifdef SECURITY_SYSATTR_NAME
and the other half on
#ifdef HAVE_SELINUX
This seems bizarre: is there really any chance that there are two
independently usable chunks of code here? And why is it conditional
on a macro that is a field name, rather than conditional on a feature
macro? That is, I'd expect to see something like
#ifdef ENABLE_SEPOSTGRES
throughout.

For that matter, what is the point of treating SECURITY_SYSATTR_NAME
as a configurable thing in the first place? I can hardly imagine a
worse idea than a security-critical column having different names in
different installations.

The patch hasn't got a mode in which SELinux support is compiled in but
not active. This is a good way to ensure that no one will ever ship
standard RPMs with the feature compiled in, because they will be entirely
nonfunctional for people who aren't interested in setting up SELinux.
I think you need an "enable_sepostgres" GUC, or something like that.
(Of course, the overhead of the per-row security column would probably
discourage anyone from wanting to use such a configuration anyway,
so maybe the point is moot.)

sepgsql-policy has got usability problems:
* It should pay attention to the configured installation PREFIX instead of
hardwiring a couple of random possible installation locations
* It can only support the build machine's SELINUXTYPE --- how am I supposed
to produce RPMs that support all available types?

The contents and use of sepgsqlGetTupleName() make it look like the
entire security scheme is based on object name alone. That doesn't
even account for schemas, let alone overloaded function names. Please
tell me this is not as broken-by-design as it looks.

I occasionally tell people "try to make the patch look like it's always
been there". This is pretty far from meeting that goal. Random bits
of code that are commented "PGACE:" are obviously not intended to just
fit in. You've generally ignored the Postgres code layout conventions
(pgindent will help to some extent but it's far from a panacea) and our
commenting conventions --- eg, hardly any of the functions have header
comments, and the ones that do follow conventions seen noplace in the
Postgres code, like using "@" on parameter names. In general the number
and quality of the comments is far below the standard for Postgres code,
and the lack of any implementation documentation isn't helping.

Another big problem, which I understand your motivation for but that
doesn't make the code any less ugly, is that you've got trivial bits
of code that're separated by two(!) levels of hook calls from where
they're actually being used. Not only does that make it unreadable
but the files that actually do the work combine bits of code that
should be scattered across a lot of modules, causing those files to
be just horrid from a modularity standpoint --- they've got their
fingers stuck in everyplace. If you want this to get applied you need
to start thinking of it as an integral part of the code, not an add-on.

Some other bits of add-on-itis:
* If you need a dedicated LWLock, declare it in lwlock.h.
* If you need a node type, declare it in nodes.h (T_SEvalItem is utterly
broken)

Why in the world would you have security restrictions associated with
TOAST tuples? Seems like all the interesting restrictions should be
on the parent table.

Don't randomly invent your own style of management of a postmaster
child process. For one thing, this code doesn't cope with either
unexpected death of the postmaster or unexpected death of the child.
If you need another child, manage it in postmaster.c the same way
the other children are managed.

The code in hooks.c looks suspiciously not-HOT-aware, eg use of
ItemIdIsUsed() for what probably needs to be ItemIdIsNormal().
(Not that this code ought to be fetching the tuple for itself
in the first place --- probable big performance loss there...)

pgaceHooks.c seems to be a useless layer of indirection
--- lose it all, and inline into callers instead.

Is the hard-wired shmem cache size really adequate? Why are you
using such a cache in shared memory at all, rather than backend-local?
The locking implications likely take away more performance than you
save by not having each session need to load up its cache.
(We don't use shared catalog caches, in general.)

If we're going to support assignment to system columns, we probably
want a general solution that will work for OID not only the security
column. Also, I'm unconvinced that setting resjunk = true for such
targetlist entries is a good idea.

The whole "early security" business looks like a mess :-(. I suspect
you should rip all that out of the backend and add a step to initdb
that fills in those tables.

The idea of input functions that alter system tables scares me.

elog() should not be used for user-facing errors. I couldn't easily
tell just which of the messages are likely to be seen by users and
which ones should be "can't happen" cases, but certainly there are
a whole lot of these that need to be ereport()s. Likely there need
to be some new ERRCODEs too.

__lookupRelationForm() strikes me as a complete crock. It probably
means that you've selected the wrong places to call its callers from.
This gets back to the point above that adding additional fetches of
tuples isn't good for performance anyway.

Don't use identifiers with a leading double-underscore. These are
reserved for system-private identifiers according to the C standard.

Use of a function in genbki.sh is very likely not portable.

use of flock() is probably not portable and even less probably necessary.

Declaring a function foo() rather than foo(void) is poor style ---
at least some compilers will complain about that.

I see a lot of "Copyright 2007 KaiGai Kohei" notices. Are you
willing to assign those copyrights to the Postgres Global Development
Group? If not, we'll at least need statements along the line of
"Distributed under the PostgreSQL license".

regards, tom lane

On Mon, 5 May 2008, Tom Lane wrote:

> elog() should not be used for user-facing errors. I couldn't easily
> tell just which of the messages are likely to be seen by users and
> which ones should be "can't happen" cases, but certainly there are
> a whole lot of these that need to be ereport()s. Likely there need
> to be some new ERRCODEs too.

And it would be a nice step toward the scenarios I was asking about if
there was a GUC variable for what level to log security violations at. I
realize now the tuple-level warnings are going into the SELinux logs
rather than the PostgreSQL ones, but it should be easier to change policy
violations that impact the server to something other than just ERROR.

--
* Greg Smith gsmith(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD

Josh Berkus <josh(at)agliodbs(dot)com> writes:
> For hackers who don't understand security frameworks, I'm going to make a
> strong case for KaiGai's patch.
> ...
> So it would be much better to have this functionality be "mainstream"
> rather than a fork. If it does get bounced, please do it becuase of code
> quality and not because "nobody is asking for this".

Well, let's be perfectly blunt here: this is an extremely sizable patch
that will be of interest to less than 1% of our users (and I think I'm
being generous in that estimate). If it's going to get in, it's going
to have to not cost us anything much on reliability, maintainability,
or performance --- rank those however you please, there is someone out
there who will be most interested in any one of them.

My look at the code today convinced me that it's not going to get
applied without a whole lot of further work. I think what we must
figure out in this commit-fest is whether to encourage KaiGai to start
doing that work, or to decide that it probably is a dead issue and he
shouldn't bother.

As to reliability: the issue that worries me the most was already raised
by Greg Smith --- this patch puts security checks and consequent catalog
lookups into some mighty low-level places that IMHO have no business
triggering catalog accesses. If I'd been able to make the patch work
today, I'd have tested whether it could get through the regression tests
with CLOBBER_CACHE_ALWAYS defined. I'd be happier if it were refactored
to put the security checks only into executor code paths, and not try to
enforce security restrictions against the system itself. (Thought
experiment: what happens if SELinux denies access to a row of
pg_security to the code that is supposed to be checking security?
Or try this one: it looks to me like you can set up the system to
pretend to some user that the pg_class rows for certain indexes don't
exist, even though he has update rights on their parent table. Instant
index corruption.)

As to maintainability: I already posted a lot of unhappy remarks on code
style and readability. I don't think there's anything unfixable there,
but there's a lot of work to do to convert what was evidently written as
an arms-length add-on into a sensible part of core Postgres.

As to performance: frankly, I'm afraid the performance is abysmal.
This was what I was mainly hoping to check out if I could have gotten
it to build today. We need to at least get some rough pgbench readings
before deciding whether it's worth pushing forward.

regards, tom lane

I wrote:
> I tried to do a bit of testing of this, but it does not work on current
> Fedora 8, because the policy module doesn't build:

I got past that by commenting out the last few lines of sepostgresql.te,
which apparently are intended for some SELinux feature that's not
shipped yet in F8.

After that, and a whole bunch of foolery with manually putting the right
security labels on files (because the contrib module is unhelpful for
test installations with nonstandard PREFIXes), I managed to get a
version that worked well enough to test. What I found out:

* It does not come close to passing the regression tests. I saw a lot of
! ERROR: unrecognized node type: 903
which suggests that something's been screwed up about parse analysis
(903 = T_A_Const, which shouldn't get further than parse analysis),
and it dumps core in the TRUNCATE test.

* pgbench shows a relatively marginal speed difference, which is not
too surprising since it pushes only a few tuples around per query.
The worst case for sepostgres is likely to be bulk operations.
I tried "select count(*) from accounts" (at scale factor 20, so this
is 2000000 tuples) and got about 550 msec from CVS HEAD, about
1340 msec from HEAD plus patch. That's with no actual useful policy
loaded, mind you.

So the overhead is indeed pretty bad, but it seems reasonable to think
that with some optimization effort it could be brought down to where
it'd be acceptable for people who really need the feature.

AFAICS the only thing left that really needs to be discussed more during
this commit-fest is the business about whether it's sane to be trying to
apply selinux restrictions in simple_heap_update and friends. The more
I think about that the more I think it's simply a bad idea. Those
functions are used primarily (if not solely) for system-initiated
catalog updates, and I fail to see any plausible reason to let
user-written policy be applied at that level. For instance, it would
be a completely bad move to allow such policy to prevent autovacuum
from updating relfrozenxid values. Furthermore, those functions are
much too low-level to have any idea of the reason for the change they
are being asked to make, so they can't apply any intelligence to the
allow-or-not decision. Lastly, I don't see any use-case for policy there
that wouldn't be served as well or better by the standard SQL privilege
mechanism. So I think that really all needs to be ripped out, and
do enforcement in the executor instead.

regards, tom lane

On Tue, May 06, 2008 at 02:56:41PM -0400, Tom Lane wrote:

> AFAICS the only thing left that really needs to be discussed more during
> this commit-fest is the business about whether it's sane to be trying to
> apply selinux restrictions in simple_heap_update and friends.

I don't have any opinion about the patches, obviously, but I'm
wondering whether there is, somewhere, an outline of what the _goals_
of this system are. If we have requirements that we can measure the
implementation against, we can know whether to throw away a particular
goal as being basically incompatible with the design of Postgres
(i.e. not an "enhancement" but a "reconception") or else as being
implementable with another approach.

A

--
Andrew Sullivan
ajs(at)commandprompt(dot)com
+1 503 667 4564 x104
http://www.commandprompt.com/

Andrew Sullivan <ajs(at)commandprompt(dot)com> writes:
> I don't have any opinion about the patches, obviously, but I'm
> wondering whether there is, somewhere, an outline of what the _goals_
> of this system are.

The only documentation I've seen is

http://code.google.com/p/sepgsql/wiki/WhatIsSEPostgreSQL

which contains only examples of enforcing restrictions on *user*
queries and tables.

regards, tom lane

On Tue, May 06, 2008 at 03:28:25PM -0400, Tom Lane wrote:
>
> The only documentation I've seen is
>
> http://code.google.com/p/sepgsql/wiki/WhatIsSEPostgreSQL
>
> which contains only examples of enforcing restrictions on *user*
> queries and tables.

I agree that, having just read that, anything that involves itself
with the system catalogues and such is way overstepping the stated
design goal.

There is an issue in most high-security systems having to do with
side-channel leakage of supposedly sensitive data. So, the mere
exsistence of certain tables, columns, or users might be regarded as
security-sensitive data. I'm not sure I see how to get around that
without mucking in the areas that are causing some of the trouble.

But I think before we get into that discussion, a fairly clear
statement of exactly which problems are going to be in scope is
needed.

FWIW, I support and think important the row- and column- level access
controls this seems to be proposing, at least in principle. Whether
that's a support that will extend to 2x overhead on everything is
rather a different matter. Also, I am more than prepared to trade
away some cases in order to get a broadly useful functionality (so if
you can't hide the existence of a table, but all efforts to learn its
contents don't work, I might be willing to support that trade-off).

A

--
Andrew Sullivan
ajs(at)commandprompt(dot)com
+1 503 667 4564 x104
http://www.commandprompt.com/

Andrew Sullivan <ajs(at)commandprompt(dot)com> writes:
> There is an issue in most high-security systems having to do with
> side-channel leakage of supposedly sensitive data. So, the mere
> exsistence of certain tables, columns, or users might be regarded as
> security-sensitive data. I'm not sure I see how to get around that
> without mucking in the areas that are causing some of the trouble.

Well, the setup that I suggested would still allow people to set
restrictions that would prevent *user* queries from seeing rows in the
system catalogs. I'm not sure that's a good idea either --- pg_dump,
meet foot gun --- but at least it can't corrupt the functioning of the
server itself.

> But I think before we get into that discussion, a fairly clear
> statement of exactly which problems are going to be in scope is
> needed.

Agreed, that would help.

> FWIW, I support and think important the row- and column- level access
> controls this seems to be proposing, at least in principle. Whether
> that's a support that will extend to 2x overhead on everything is
> rather a different matter. Also, I am more than prepared to trade
> away some cases in order to get a broadly useful functionality (so if
> you can't hide the existence of a table, but all efforts to learn its
> contents don't work, I might be willing to support that trade-off).

Yeah, that's something that we need to think about too. ISTM that this
patch's proposed restrictions on SQL object access are largely
duplicative of SQL-standard access permissions, and they'll become more
so if the pending patch to implement SQL column permissions gets in.
I think a good case could be made for throwing out all of that and using
the SELinux code only for row-level filtering.

(And of course the next question after that is why we should want to
depend on SELinux at all, rather than implementing row filtering
in the framework of SQL permissions...)

regards, tom lane

On Tue, 6 May 2008, Tom Lane wrote:

> And of course the next question after that is why we should want to
> depend on SELinux at all, rather than implementing row filtering in the
> framework of SQL permissions...

It may be the case that clean row and column filtering at the SQL layer
are pre-requisites for a clean SELinux implementation, where the only
difference is that the permission checks are handled by asking SELinux
instead of looking in the catalog.

As for why SQL restrictions alone aren't enough...the simple answer is
because it's not SELinux, which I say in all seriousness because it is
turning into a requirement in some places.

SELinux lets you control what a user login is capable of no matter what
application they run, and managing those capabilities can happen in one
place--the SELinux tools. There's lots of ways to address OS login
problems. Let's say the logins have a PAM plug-in that restrict what you
can login to based on what machine you're on, and also require one of
those randomly generating key cards so that you can't steal someone else's
username/password. If you've got a scheme like that, and the database
enforces SELinux restrictions, it doesn't matter whether your DBA followed
all the PostgreSQL security rules correctly, as long as they got the
SELinux mapping part right. And you don't have to make sure whatever
custom security mechanism you've integrated into the login or post-login
process is recognized by the database proper at all, as long as the
restrictions can be mapped to the SELinux+database space.

Simple example of something hard to replicate without this framework:
you discover someone is a rat. You update your list of active users and
push that to all your servers. Now even if said rat is already logged
into the database server and busy doing 'psql -o /disk/usbkey -c "select *
from secretdata"' you just cut them off in the middle of the
query--without needing to find all the database servers and execute "alter
table secretdata set ...", just by doing simple user account maintenance
the way people are already comfortable with and have procedures for.

That's the basic idea here--put the authorization into one layer where
it's easy (for some definitions of "easy") to manage and extensible as
needed, without having to touch the individual applications directly, just
by adjusting what permissions you publish when data is requested. I'm
sure someone can raise issues or suggest alternate implemenations for my
specific examples, but much like other privledge escalation defense
mechanisms these environments look for redundant layers of security. In
reality users of this would aim for a completely locked down base
PostgreSQL *and* a completely locked down SELinux implementation
integrated into that, reinforcing one another, rather than just relying on
one level of security.

--
* Greg Smith gsmith(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD

Tom, Thanks for your reviewing.

Tom Lane wrote:
> KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> writes:
>> I updated the series of SE-PostgreSQL patches for the latest pgsql-8.4devel tree.
>
> I tried to do a bit of testing of this, but it does not work on current
> Fedora 8, because the policy module doesn't build:
>
> [tgl(at)rh2 sepgsql-policy]$ make
> make -f /usr/share/selinux/devel/Makefile NAME=targeted
> make[1]: Entering directory `/home/tgl/sepgsql/contrib/sepgsql-policy'
> Compiling targeted sepostgresql module
> /usr/bin/checkmodule: loading policy configuration from tmp/sepostgresql.tmp
> sepostgresql.te:349:ERROR 'syntax error' at token 'corenet_tcp_recvfrom_labeled' on line 5675:
> # NOTE: These changes are to be merged in the later releases.
> corenet_tcp_recvfrom_labeled(sepgsql_server_type, sepgsql_client_type)
> /usr/bin/checkmodule: error(s) encountered while parsing configuration
> make[1]: *** [tmp/sepostgresql.mod] Error 1
> make[1]: Leaving directory `/home/tgl/sepgsql/contrib/sepgsql-policy'
> make: *** [sepostgresql.pp] Error 2
> [tgl(at)rh2 sepgsql-policy]$

This policy can be built on Fedora 9 or rawhide now.
The latest selinux-policy on Fedora 8 does not contain labeled networking
interfaces, so it makes errors.

FYI,
I also have a discussion to merge the SE-PostgreSQL security policy module
into the default upstreamed policy (called as reference policy).
http://marc.info/?t=120574668300001&r=1&w=2

I guess it will be merged former than the release date of PostgreSQL v8.4,
so it may not be necessary to include it within contrib/ .

In the next patch set, I like to provide it with different form.

> In the meantime, here are some random comments after my failed test
> build and a very fast scan through the patch:

> The patch tries to re-add ipcclean to the source tree --- probably a
> merge error?

I don't intend to modify ipcclean.
It is a bug in patch generation scripts, to be fixed soon.

> autoconf complains about the description-free AC_DEFINEs

OK, I add the following descriptions for my AC_DEFINEs.

AC_DEFINE(SECURITY_SYSATTR_NAME, "security_context",
[Enables system column for security attribute support])
AC_DEFINE_UNQUOTED(HAVE_SELINUX, 1,
[Enables SE-PostgreSQL feature])

> Doesn't compile warning-free if selinux not enabled ... for that matter,
> it doesn't compile warning-free if selinux IS enabled.

Does it mean you run configure script without --enable-selinux?
I also confirmed some warnings to be fixed in the next patch set.
Or, is selinux disabled on your workstation?

> No documentation updates whatsoever :-(

OK, I'll add a new documentation at doc/src/sgml .
Is it your intention, isn't it?

> About half of the patch seems to be conditional on
> #ifdef SECURITY_SYSATTR_NAME
> and the other half on
> #ifdef HAVE_SELINUX
> This seems bizarre: is there really any chance that there are two
> independently usable chunks of code here? And why is it conditional
> on a macro that is a field name, rather than conditional on a feature
> macro? That is, I'd expect to see something like
> #ifdef ENABLE_SEPOSTGRES
> throughout.

SECURITY_SYSATTR_NAME is a name of security attribute system column,
and used to enable/disable this feature provided by PGACE.
I think PGACE has possible users more than SELinux, like Solaris TX.
Applying separated symbols are worthwhile when a new security module
requires security attribute but it is not SELinux.

> For that matter, what is the point of treating SECURITY_SYSATTR_NAME
> as a configurable thing in the first place? I can hardly imagine a
> worse idea than a security-critical column having different names in
> different installations.

SECURITY_SYSATTR_NAME is defined as "security_context" in SE-PostgreSQL.
However, it is a special term of SELinux.
If we can agree this name, we don't have to abstract the name of security
attribute system column.

> The patch hasn't got a mode in which SELinux support is compiled in but
> not active. This is a good way to ensure that no one will ever ship
> standard RPMs with the feature compiled in, because they will be entirely
> nonfunctional for people who aren't interested in setting up SELinux.
> I think you need an "enable_sepostgres" GUC, or something like that.
> (Of course, the overhead of the per-row security column would probably
> discourage anyone from wanting to use such a configuration anyway,
> so maybe the point is moot.)

We can turn on/off SELinux globally, not bounded to SE-PostgreSQL.
The reason why I didn't provide a mode bit like "enable_sepostgresql"
is to keep consistency in system configuration.
It is undesirable situation to disable SELinux on a partial inter-process
communication channels.

BTW, what does it mean the overhead of the per-row security column?
When SELinux is disabled, it is not labeled and does not consume any
additional spaces.

> sepgsql-policy has got usability problems:
> * It should pay attention to the configured installation PREFIX instead of
> hardwiring a couple of random possible installation locations
> * It can only support the build machine's SELINUXTYPE --- how am I supposed
> to produce RPMs that support all available types?

As noted above, I'll provide its security policy in a different way.
The hardcoded parameters are also changed to configurable ones.

> The contents and use of sepgsqlGetTupleName() make it look like the
> entire security scheme is based on object name alone. That doesn't
> even account for schemas, let alone overloaded function names. Please
> tell me this is not as broken-by-design as it looks.

The result of sepgsqlGetTupleName() is used to generate access denied
logs as a name of target object, not used to access controls.

> I occasionally tell people "try to make the patch look like it's always
> been there". This is pretty far from meeting that goal. Random bits
> of code that are commented "PGACE:" are obviously not intended to just
> fit in. You've generally ignored the Postgres code layout conventions
> (pgindent will help to some extent but it's far from a panacea) and our
> commenting conventions --- eg, hardly any of the functions have header
> comments, and the ones that do follow conventions seen noplace in the
> Postgres code, like using "@" on parameter names. In general the number
> and quality of the comments is far below the standard for Postgres code,
> and the lack of any implementation documentation isn't helping.

OK, I'll improvement coding style, comments and so on.

> Another big problem, which I understand your motivation for but that
> doesn't make the code any less ugly, is that you've got trivial bits
> of code that're separated by two(!) levels of hook calls from where
> they're actually being used. Not only does that make it unreadable
> but the files that actually do the work combine bits of code that
> should be scattered across a lot of modules, causing those files to
> be just horrid from a modularity standpoint --- they've got their
> fingers stuck in everyplace. If you want this to get applied you need
> to start thinking of it as an integral part of the code, not an add-on.

I thought small patches help reviewer check it comfortably.
However, it is unwelcome style...
I'll update the next patch without separating both features.

> Some other bits of add-on-itis:
> * If you need a dedicated LWLock, declare it in lwlock.h.
> * If you need a node type, declare it in nodes.h (T_SEvalItem is utterly
> broken)

OK, I'll move them.

> Why in the world would you have security restrictions associated with
> TOAST tuples? Seems like all the interesting restrictions should be
> on the parent table.

sepgsqlHeapTuple*() hooks are also invoked for tuples within TOAST tables,
but it does not apply any restriction. When given tuple is belong to
TOAST tables, SE-PostgreSQL skips its checks.

> Don't randomly invent your own style of management of a postmaster
> child process. For one thing, this code doesn't cope with either
> unexpected death of the postmaster or unexpected death of the child.
> If you need another child, manage it in postmaster.c the same way
> the other children are managed.

OK,

> The code in hooks.c looks suspiciously not-HOT-aware, eg use of
> ItemIdIsUsed() for what probably needs to be ItemIdIsNormal().
> (Not that this code ought to be fetching the tuple for itself
> in the first place --- probable big performance loss there...)

Does GetTupleForTrigger() help me as a sample to obtain the older
version of tuple on the hooks of simaple_heap_update()?

> pgaceHooks.c seems to be a useless layer of indirection
> --- lose it all, and inline into callers instead.

OK, I'll push them into inline function and remove pgaceHooks.c.

> Is the hard-wired shmem cache size really adequate? Why are you
> using such a cache in shared memory at all, rather than backend-local?
> The locking implications likely take away more performance than you
> save by not having each session need to load up its cache.
> (We don't use shared catalog caches, in general.)

I think caches of security policy should be placed on shared memory.
If backend held it locally, it requires some system call invokations
per connection at least, although the security policy is seldom reloaded.
However, we have to invalidate the cache when it is reloaded.
If every backend have its cache, cache invalidation will be a uneasy task.

> If we're going to support assignment to system columns, we probably
> want a general solution that will work for OID not only the security
> column. Also, I'm unconvinced that setting resjunk = true for such
> targetlist entries is a good idea.

OK, it is possible to expand.
However, why are you unconvinced for setting resjunk = true, when users
try to update system column? I think it is a reasonable way to compute
right hand of terms, like:
UPDATE foo SET security_context = security_context::text || ':Classified' ...

> The whole "early security" business looks like a mess :-(. I suspect
> you should rip all that out of the backend and add a step to initdb
> that fills in those tables.

I also think "early security" codes are ad-hoc. :-(
Pushing it into initdb seems me a good idea.
I'll try to consider whether it is possible, or not.

> The idea of input functions that alter system tables scares me.

Are you saying about pg_security system catalog?

> elog() should not be used for user-facing errors. I couldn't easily
> tell just which of the messages are likely to be seen by users and
> which ones should be "can't happen" cases, but certainly there are
> a whole lot of these that need to be ereport()s. Likely there need
> to be some new ERRCODEs too.

OK, I'll replace elog() by ereport().
Is adding a new error code (ERRCODE_SECURITY_VIOLATION) in Class XX appropriate?

> __lookupRelationForm() strikes me as a complete crock. It probably
> means that you've selected the wrong places to call its callers from.
> This gets back to the point above that adding additional fetches of
> tuples isn't good for performance anyway.

The purpose of __lookupRelationForm() is to obtain the HeapTuple of
parent relation when new attributes are inserted into pg_attribute
system catalog.
But we cannot refer the tuple of parent table because CommandCounterIncrement()
are not invoked between inserting a tuple into pg_class and pg_attribute.

Is there any good idea to refer a tuple of parent table just before
tuples are inserted into pg_attribute?

> Don't use identifiers with a leading double-underscore. These are
> reserved for system-private identifiers according to the C standard.

OK, I'll fix rest of them.

> Use of a function in genbki.sh is very likely not portable.

OK, I'll consider something

> use of flock() is probably not portable and even less probably necessary.

OK, I'll fix it.

> Declaring a function foo() rather than foo(void) is poor style ---
> at least some compilers will complain about that.

OK, I'll fix these kind of function declarations.

> I see a lot of "Copyright 2007 KaiGai Kohei" notices. Are you
> willing to assign those copyrights to the Postgres Global Development
> Group? If not, we'll at least need statements along the line of
> "Distributed under the PostgreSQL license".

No need to say, I'm willing to assign these copytights to the PostgreSQL
Global Development Group.
Can I replace them on the next patch set?

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

Tom Lane wrote:
> After that, and a whole bunch of foolery with manually putting the right
> security labels on files (because the contrib module is unhelpful for
> test installations with nonstandard PREFIXes), I managed to get a
> version that worked well enough to test. What I found out:

I'll improve the way to provide security policy.
Please wait for a while.

> * It does not come close to passing the regression tests. I saw a lot of
> ! ERROR: unrecognized node type: 903
> which suggests that something's been screwed up about parse analysis
> (903 = T_A_Const, which shouldn't get further than parse analysis),

Could you tell me what queries hit these errors?
I guess this message was generated in expression_tree_walker() called within
sepgsqlExprWalker(), but it is invoked after query rewriter.

> and it dumps core in the TRUNCATE test.

This bug came from misoperations of List.

> * pgbench shows a relatively marginal speed difference, which is not
> too surprising since it pushes only a few tuples around per query.
> The worst case for sepostgres is likely to be bulk operations.
> I tried "select count(*) from accounts" (at scale factor 20, so this
> is 2000000 tuples) and got about 550 msec from CVS HEAD, about
> 1340 msec from HEAD plus patch. That's with no actual useful policy
> loaded, mind you.
>
> So the overhead is indeed pretty bad, but it seems reasonable to think
> that with some optimization effort it could be brought down to where
> it'd be acceptable for people who really need the feature.

Thanks for your information.
It indeed has performance degrading, but, I expect users who want to apply
SELinux policy don't put their first priority on edge performance.

> AFAICS the only thing left that really needs to be discussed more during
> this commit-fest is the business about whether it's sane to be trying to
> apply selinux restrictions in simple_heap_update and friends. The more
> I think about that the more I think it's simply a bad idea. Those
> functions are used primarily (if not solely) for system-initiated
> catalog updates, and I fail to see any plausible reason to let
> user-written policy be applied at that level. For instance, it would
> be a completely bad move to allow such policy to prevent autovacuum
> from updating relfrozenxid values.

In generally, fewer and more fundamental security checks are more
comprehensive. simple_heap_update() can trap any updating, called
from more than 80 places.

The purpose of SE-PostgreSQL is to apply security policy on user's
queries, not an internal one. Therefore, we don't apply any policy
on some of *internal* operations, like updating TOAST table.

Currently I don't apply any special care for autovacuum, however,
it is a solution to handle *internal* updating as an exception for
checks. (Because it is NOT came from external users.)

> Furthermore, those functions are
> much too low-level to have any idea of the reason for the change they
> are being asked to make, so they can't apply any intelligence to the
> allow-or-not decision.

We can abstract finer-grained access control unit later, however,
its reverse is difficult.
For example, we have to have the following permission to create
a new table.
- Insert a new tuple into pg_class
- Insert a new tuple into pg_type
- Insert new tuples into pg_attribute

These are abstracted in the default security policy.
If you want non-administrative users to create/drop table,
set sepgsql_enable_users_ddl boolean to turn on/off the permission set.

# setsebool sepgsql_enable_users_ddl=1

> Lastly, I don't see any use-case for policy there
> that wouldn't be served as well or better by the standard SQL privilege
> mechanism. So I think that really all needs to be ripped out, and
> do enforcement in the executor instead.

The primary purpose of SE-PostgreSQL is to apply common access control
policy on database objects, as if clients access filesystem objects.

See the following diagram:
http://sepgsql.googlecode.com/files/sepgsql_dfc.png

It shows two users within different domain tries to communicate each
other, and any message have to go through inter-processes communication
method like files, networks, and so on.
(Security policy enforces single directed data flow, from lower to higher.)

SELinux can prevent data leaking via shared files because it is managed
by operating system. However, it is impossible for database objects.

SE-PostgreSQL apply (system-wide) common security policy on database objects.
This concept enables to maintain consistency of access control policy.

The standard SQL privilegs machanism is suitable to control accesses
on database objects, however, it is not easy to maintain its consistency
with operating systems's policy.

Is it appropriate answer for your question?

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

On Wed, May 07, 2008 at 12:01:21AM -0400, Greg Smith wrote:

> It may be the case that clean row and column filtering at the SQL layer are
> pre-requisites for a clean SELinux implementation, where the only
> difference is that the permission checks are handled by asking SELinux
> instead of looking in the catalog.

That strikes me as an approach more likely to be fruitful.

I get the point about imposing all the restrictions at the SELinux
layer. But the way to do that, I think, is to make the individual
policies possible to implement in PostgreSQL simpliciter, and then
have some interface to the SELinux permissions system so that it
becomes possible to set those definitions outside Postgres. (I know
that the latter raises all sorts of nasty DoS scenarios. That's
clearly one of the problems that will need addressing.)

Again, I support the effort in principle; I'm just not sure that the
current proof-of-concept work is what will be needed to address the
design goals. I do think that somewhat clearer scope definitions
would be a big help in deciding which modifications are really needed,
and where.

A

--
Andrew Sullivan
ajs(at)commandprompt(dot)com
+1 503 667 4564 x104
http://www.commandprompt.com/

KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> writes:
> Tom Lane wrote:
>> * It does not come close to passing the regression tests. I saw a lot of
>> ! ERROR: unrecognized node type: 903
>> which suggests that something's been screwed up about parse analysis
>> (903 = T_A_Const, which shouldn't get further than parse analysis),

> Could you tell me what queries hit these errors?

I remember seeing it on some EXECUTEs, but you really ought to run the
tests for yourself. A *minimum* requirement on any submitted patch
is that it should pass the regression tests.

regards, tom lane

Andrew, Marc,

> FWIW, I support and think important the row- and column- level access
> controls this seems to be proposing, at least in principle. Whether
> that's a support that will extend to 2x overhead on everything is
> rather a different matter. Also, I am more than prepared to trade
> away some cases in order to get a broadly useful functionality (so if
> you can't hide the existence of a table, but all efforts to learn its
> contents don't work, I might be willing to support that trade-off).

Well, there are two different goals we can satisfy. One is to help
support the kind of VPS functionality that Veil is designed for, and the
majority of users want. The second goal is upholding the kind of
security systems demanded by highly secure environments which have
statutory requirements about how security should work.

That is, while Veil-like funcitonality is what most developers want,
it's not what NSA/Banks/military want, who have their own ideas about
security. I think we can conceivably capture both.

I do think that SE functionality which goes beyond reasonable SQL
requirements should be a build-time options because I don't feasably see
ways to implement them that won't cause a big performance hit.

Also, I think you should be aware that for serious multilevel security
hackers (one of whom will be working on Postgres soon) SEPostgres is the
beginning and not the end. One of the requirements of many militaries,
for example, is not merely data hiding by data substitution, where the
row contents you see depend on your security clearance.

Also, re: pg_dump: it's actually a desired feature that, for example,
some users only be able to dump a subset of the database. Including
some DBAs. One of the issues which SE/Mulitlevel tries to address is
"what happens if you don't trust your DBA 100%?" So if we can retain
that, it's actually a feature and not a bug.

--Josh

>> The whole "early security" business looks like a mess :-(. I suspect
>> you should rip all that out of the backend and add a step to initdb
>> that fills in those tables.
>
> I also think "early security" codes are ad-hoc. :-(
> Pushing it into initdb seems me a good idea.
> I'll try to consider whether it is possible, or not.

The purpose of "early security" code is to manage relationship security
identifier and text representation before pg_security creation.

Therefore, we can make it redundant if initializing security attributes
can be done later. In the next patch set, I'll inject a hook to initialize
them at the last of bootstraping phase, and remove "early security" code.

Any tuples inserted during bootstraping mode are unlabeled, and this
operation is always allowed. Then, this hook is invoked after setting
up all system catalog, and it labels whole of database, as if "restorecon"
command initialize security context of filesystem objects.

How do you think about this design?
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

Tom Lane wrote:
> KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> writes:
>> Tom Lane wrote:
>>> * It does not come close to passing the regression tests. I saw a lot of
>>> ! ERROR: unrecognized node type: 903
>>> which suggests that something's been screwed up about parse analysis
>>> (903 = T_A_Const, which shouldn't get further than parse analysis),
>
>> Could you tell me what queries hit these errors?
>
> I remember seeing it on some EXECUTEs, but you really ought to run the
> tests for yourself. A *minimum* requirement on any submitted patch
> is that it should pass the regression tests.

Some of the test fails contains minor differences from expected results, like:

| SELECT '' AS "xxx", *
| FROM J1_TBL t1 (a, b, c) NATURAL JOIN J2_TBL t2 (d, a);
| xxx | a | b | c | d
| -----+---+---+------+---
| - | 0 | | zero |
| | 2 | 3 | two | 2
| | 4 | 1 | four | 2
| + | 0 | | zero |
| (3 rows)

and, some of them are trivial ones, like:

| SELECT p1.oid, p1.typname
| FROM pg_type as p1
| WHERE p1.typtype in ('b','e') AND p1.typname NOT LIKE E'\\_%' AND NOT EXISTS
| (SELECT 1 FROM pg_type as p2
| WHERE p2.typname = ('_' || p1.typname)::name AND
| p2.typelem = p1.oid and p1.typarray = p2.oid);
| - oid | typname
| ------+---------
| - 210 | smgr
| - 705 | unknown
| -(2 rows)
| + oid | typname
| +------+----------------
| + 210 | smgr
| + 705 | unknown
| + 3403 | security_label
| +(3 rows)

Isn't it necessary to consider them as regressions?

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> writes:
> Some of the test fails contains minor differences from expected results, like:

> | SELECT '' AS "xxx", *
> | FROM J1_TBL t1 (a, b, c) NATURAL JOIN J2_TBL t2 (d, a);
> | xxx | a | b | c | d
> | -----+---+---+------+---
> | - | 0 | | zero |
> | | 2 | 3 | two | 2
> | | 4 | 1 | four | 2
> | + | 0 | | zero |
> | (3 rows)

Yeah, I remember those. What needs to be looked at here is *why* the
output is changing. For a patch that allegedly does not touch the
planner, it's fairly disturbing that you don't get the same results.

> and, some of them are trivial ones, like:

> | SELECT p1.oid, p1.typname
> | FROM pg_type as p1
> | WHERE p1.typtype in ('b','e') AND p1.typname NOT LIKE E'\\_%' AND NOT EXISTS
> | (SELECT 1 FROM pg_type as p2
> | WHERE p2.typname = ('_' || p1.typname)::name AND
> | p2.typelem = p1.oid and p1.typarray = p2.oid);
> | - oid | typname
> | ------+---------
> | - 210 | smgr
> | - 705 | unknown
> | -(2 rows)
> | + oid | typname
> | +------+----------------
> | + 210 | smgr
> | + 705 | unknown
> | + 3403 | security_label
> | +(3 rows)

Are you sure that the security_label type should not have an array type?
I do not offhand see a good argument for that. If it really shouldn't,
we can change the expected output here, but you've got to make that
case first.

regards, tom lane

Tom Lane wrote:
> KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> writes:
>> Some of the test fails contains minor differences from expected results, like:
>
>> | SELECT '' AS "xxx", *
>> | FROM J1_TBL t1 (a, b, c) NATURAL JOIN J2_TBL t2 (d, a);
>> | xxx | a | b | c | d
>> | -----+---+---+------+---
>> | - | 0 | | zero |
>> | | 2 | 3 | two | 2
>> | | 4 | 1 | four | 2
>> | + | 0 | | zero |
>> | (3 rows)
>
> Yeah, I remember those. What needs to be looked at here is *why* the
> output is changing. For a patch that allegedly does not touch the
> planner, it's fairly disturbing that you don't get the same results.

SE-PostgreSQL does not touch the planner, but it modifies given query
to filter violated tuples for the current user.
Thus, the above query is dealt as if the following query is inputed.

SELECT '' AS "xxx", *
FROM J1_TBL t1 (a, b, c) NATURAL JOIN J2_TBL t2 (d, a)
ON sepgsql_tuple_perms(t1.security_context, SEPGSQL_PERMS_SELECT, ...)
and sepgsql_tuple_perms(t2.security_context, SEPGSQL_PERMS_SELECT, ...);

sepgsql_tuple_perms() returns true, if the security policy allows user
to access a given tuple. It returns false, if not so.

The result of EXPLAIN shows it more clearly:

| kaigai=# EXPLAIN SELECT '' AS "xxx", * FROM J1_TBL t1 (a, b, c) NATURAL JOIN J2_TBL t2 (d, a);
| QUERY PLAN
| -----------------------------------------------------------------------------------------------
| Hash Join (cost=29023.54..82599.93 rows=1380 width=44)
| Hash Cond: (t2.a = t1.a)
| -> Seq Scan on j2_tbl t2 (cost=0.00..53526.05 rows=713 width=8)
| Filter: pg_catalog.sepgsql_tuple_perms(tableoid, security_context, 12288, t2.*)
| -> Hash (cost=29018.70..29018.70 rows=387 width=40)
| -> Seq Scan on j1_tbl t1 (cost=0.00..29018.70 rows=387 width=40)
| Filter: pg_catalog.sepgsql_tuple_perms(tableoid, security_context, 12288, t1.*)
| (7 rows)

>> and, some of them are trivial ones, like:
>
>> | SELECT p1.oid, p1.typname
>> | FROM pg_type as p1
>> | WHERE p1.typtype in ('b','e') AND p1.typname NOT LIKE E'\\_%' AND NOT EXISTS
>> | (SELECT 1 FROM pg_type as p2
>> | WHERE p2.typname = ('_' || p1.typname)::name AND
>> | p2.typelem = p1.oid and p1.typarray = p2.oid);
>> | - oid | typname
>> | ------+---------
>> | - 210 | smgr
>> | - 705 | unknown
>> | -(2 rows)
>> | + oid | typname
>> | +------+----------------
>> | + 210 | smgr
>> | + 705 | unknown
>> | + 3403 | security_label
>> | +(3 rows)
>
> Are you sure that the security_label type should not have an array type?
> I do not offhand see a good argument for that. If it really shouldn't,
> we can change the expected output here, but you've got to make that
> case first.

Yes, security_label type should not have an array type.
It is defined with typelem=0 and typarray=0.
The purpose of this type is to represent the new system column of
security attribute ("security_context" column).

So, I think it is a case that a new expented output should be modified.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> writes:
> Tom Lane wrote:
>> Yeah, I remember those. What needs to be looked at here is *why* the
>> output is changing. For a patch that allegedly does not touch the
>> planner, it's fairly disturbing that you don't get the same results.

> SE-PostgreSQL does not touch the planner, but it modifies given query
> to filter violated tuples for the current user.

Hmm. Is that really a good idea, compared to hard-wiring the checks
into nodeSeqscan and friends? I didn't look at the query-rewriting
portion of the patch in any detail, but I'd tend not to trust such
a technique very far: getting it right is going to be quite complex
and probably bug prone.

>> Are you sure that the security_label type should not have an array type?

> Yes, security_label type should not have an array type.

You didn't provide one ounce of justification for making it not obey the
expected behavior, so I'm not accepting this position. It doesn't seem
to me to be all that unlikely that users would want to compute with
arrays of security labels. As an example:
select ... where security_label in ('foo', 'bar')
which will become an = ANY(ARRAY[]) construct under the hood.

regards, tom lane

Tom Lane wrote:
> KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> writes:
>
>> Tom Lane wrote:
>>
>>> Yeah, I remember those. What needs to be looked at here is *why* the
>>> output is changing. For a patch that allegedly does not touch the
>>> planner, it's fairly disturbing that you don't get the same results.
>>>
>
>
>> SE-PostgreSQL does not touch the planner, but it modifies given query
>> to filter violated tuples for the current user.
>>
>
> Hmm. Is that really a good idea, compared to hard-wiring the checks
> into nodeSeqscan and friends? I didn't look at the query-rewriting
> portion of the patch in any detail, but I'd tend not to trust such
> a technique very far: getting it right is going to be quite complex
> and probably bug prone.
>
>

My eyebrows went up when I read this too. Presumably, if it's hardwired
like you suggest then the planner can't take any account of the filter,
though. Do we want it to?

OTOH, I'm not happy about silently rewriting queries, either - it would
make optimising queries a lot harder, I suspect.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Tom Lane wrote:
>> Hmm. Is that really a good idea, compared to hard-wiring the checks
>> into nodeSeqscan and friends?

> My eyebrows went up when I read this too. Presumably, if it's hardwired
> like you suggest then the planner can't take any account of the filter,
> though. Do we want it to?

Well, the planner could have hardwired knowledge about the existence of
the hardwired filters --- if anything, that'd probably be easier than
hacking it to have a similar level of knowledge about generic-looking
function calls. But in reality, since we don't know at plan time which
userid will execute the constructed plan, I'm not sure we could come up
with useful estimates anyway.

regards, tom lane

Tom Lane wrote:
> KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> writes:
>> Tom Lane wrote:
>>> Yeah, I remember those. What needs to be looked at here is *why* the
>>> output is changing. For a patch that allegedly does not touch the
>>> planner, it's fairly disturbing that you don't get the same results.
>
>> SE-PostgreSQL does not touch the planner, but it modifies given query
>> to filter violated tuples for the current user.
>
> Hmm. Is that really a good idea, compared to hard-wiring the checks
> into nodeSeqscan and friends? I didn't look at the query-rewriting
> portion of the patch in any detail, but I'd tend not to trust suchte
> a technique very far: getting it right is going to be quite complex
> and probably bug prone.

In the prior base version (8.2.x and 8.3.x), I tended to implement
these stuffs in the modular part as far as possible, because massive
patched hanks makes more difficult to follow the mainstreamed PostgreSQL. :-(

However, the hard-wides checks look like a good idea for me.
I tried to implement a prototype of the disign, and currently it works fine.
If we can replace the implementation of tuple-level access controls by this
design, it makes the implementation simpler.

Now, I add a hook into ExecScan(). It can return true or false, to decide
whether a given tuple is filtered or not.
Permissions to be evaluated are delivered via Scan structure. A variable
named as pgaceTuplePerms (uint32) is added to keep permission set for
tuple level access controls into Scan structure.
If the security module put a proper bitmask on pgaceTuplePerms of RangeTblEntry,
it is copied to related Scan structure later.

>>> Are you sure that the security_label type should not have an array type?
>
>> Yes, security_label type should not have an array type.
>
> You didn't provide one ounce of justification for making it not obey the
> expected behavior, so I'm not accepting this position. It doesn't seem
> to me to be all that unlikely that users would want to compute with
> arrays of security labels. As an example:
> select ... where security_label in ('foo', 'bar')
> which will become an = ANY(ARRAY[]) construct under the hood.

Ah.., I didn't intend such kind of usage, so security_label type does not
have operators to use it directly, not only array support.

I'll add it in the next patch set.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

Tom Lane wrote:
> The idea of input functions that alter system tables scares me.

An example:
SELECT 'system_u:object_r:sepgsql_table_t:SystemHigh'::security_label;

can insert a new tuple into pg_security, but it is not a desirable behavior.

To fix this, I'll remove security_label type and define "security_context"
system column as a text type column, and translate the contents into
an internal identifical representation just before update or insert a tuple.

It enables to avoid to insert a temporary used (unnecessary) security context,
and enables to use various kind of text functions.

Please comment it, if you have anything.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

On Wed, May 7, 2008 at 7:52 AM, KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
> Tom, Thanks for your reviewing.
>> The patch hasn't got a mode in which SELinux support is compiled in but
>> not active. This is a good way to ensure that no one will ever ship
>> standard RPMs with the feature compiled in, because they will be entirely
>> nonfunctional for people who aren't interested in setting up SELinux.
>> I think you need an "enable_sepostgres" GUC, or something like that.
>> (Of course, the overhead of the per-row security column would probably
>> discourage anyone from wanting to use such a configuration anyway,
>> so maybe the point is moot.)
> We can turn on/off SELinux globally, not bounded to SE-PostgreSQL.
> The reason why I didn't provide a mode bit like "enable_sepostgresql"
> is to keep consistency in system configuration.

Hmm, I think ACE should be a CREATE DATABASE parameter.

If I were to create a SE-database I would wish that disabling it was
more difficult than changing a GUC in database. And being able to
set it on per-database basis would help get SE/ACE enabled by
packagers.

Regards,
Dawid
--
Solving [site load issues] with [more database replication] is a lot
like solving your own personal problems with heroin - at first it
sorta works, but after a while things just get out of hand.

Added to TODO:

* Improve server security options

http://archives.postgresql.org/pgsql-hackers/2008-04/msg01875.php
http://archives.postgresql.org/pgsql-hackers/2008-05/msg00000.php

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

KaiGai Kohei wrote:
> I updated the series of SE-PostgreSQL patches for the latest pgsql-8.4devel tree.
>
> [1/4] sepostgresql-pgace-8.4devel-3-r739.patch
> provides PGACE (PostgreSQL Access Control Extension) framework.
> http://sepgsql.googlecode.com/files/sepostgresql-pgace-8.4devel-3-r739.patch
>
> [2/4] sepostgresql-sepgsql-8.4devel-3-r739.patch
> provides SE-PostgreSQL feature, based on PGACE framework.
> http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r739.patch
>
> [3/4] sepostgresql-pg_dump-8.4devel-3-r739.patch
> enables to dump databases with security attribute.
> http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r739.patch
>
> [4/4] sepostgresql-policy-8.4devel-3-r739.patch
> provides the default security policy of SE-PostgreSQL.
> http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r739.patch
>
> We provide a quick overview for SE-PostgreSQL at:
> http://code.google.com/p/sepgsql/wiki/WhatIsSEPostgreSQL
>
> Thanks,
>
> KaiGai Kohei wrote:
> > The series of patches are the proposal of Security-Enhanced PostgreSQL (SE-PostgreSQL)
> > for the upstreamed PostgreSQL 8.4 development cycle.
> >
> > [1/4] sepostgresql-pgace-8.4devel-3.patch
> > provides PGACE (PostgreSQL Access Control Extension) framework
> > http://sepgsql.googlecode.com/files/sepostgresql-pgace-8.4devel-3-r704.patch
> >
> > [2/4] sepostgresql-sepgsql-8.4devel-3.patch
> > provides SE-PostgreSQL feature, based on PGACE framework.
> > http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r704.patch
> >
> > [3/4] sepostgresql-pg_dump-8.4devel-3.patch
> > enables pg_dump to dump database with security attribute.
> > http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r704.patch
> >
> > [4/4] sepostgresql-policy-8.4devel-3.patch
> > provides the default security policy for SE-PostgreSQL.
> > http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r704.patch
> >
> > We can provide a quick overview for SE-PostgreSQL at:
> > http://code.google.com/p/sepgsql/wiki/WhatIsSEPostgreSQL
> >
> > ENVIRONMENT
> > -----------
> > Please confirm your environment.
> > The followings are requriements of SE-PostgreSQL.
> > * Fedora 8 or later system
> > * SELinux is enabled and working
> > * kernel-2.6.24 or later
> > * selinux-policy and selinux-policy-devel v3.0.8 or later
> > * libselinux, policycoreutils
> >
> > INSTALLATION
> > ------------
> > $ tar jxvf postgresql-snapshot.tar.bz2
> > $ cd postgresql-snapshot
> > $ patch -p1 < ../sepostgresql-pgace-8.4devel-3.patch
> > $ patch -p1 < ../sepostgresql-sepgsql-8.4devel-3.patch
> > $ patch -p1 < ../sepostgresql-pg_dump-8.4devel-3.patch
> > $ patch -p1 < ../sepostgresql-policy-8.4devel-3.patch
> >
> > $ ./configure --enable-selinux
> > $ make
> > $ make -C contrib/sepgsql-policy
> > $ su
> > # make install
> >
> > # /usr/sbin/semodule -i contrib/sepgsql-policy/sepostgresql.pp
> > (NOTE: semodule is a utility to load/unload security policy modules.)
> >
> > # /sbin/restorecon -R /usr/local/pgsql
> > (NOTE: restorecon is a utilicy to initialize security context of files.)
> >
> > SETUP
> > -----
> > # mkdir -p /opt/sepgsql
> > # chown foo_user:var_group /opt/sepgsql
> > # chcon -t postgresql_db_t /opt/sepgsql
> > (NOTE: chcon is a utility to set up security context of files.)
> > # exit
> >
> > $ /usr/sbin/run_init /usr/local/pgsql/bin/initdb -D /opt/sepgsql
> > (NOTE: run_init is a utility to start a program, as if it is branched from init script.)
> > $ /usr/local/pgsql/bin/pg_ctl -D /opt/sepgsql start
> >
> >
> > SUMMARYS FOR EVERY PATCHES
> > --------------------------
> > [1/4] - sepostgresql-pgace-8.4devel-3.patch
> >
> > This patch provides PGACE (PostgreSQL Access Control Extension) framework.
> >
> > It has a similar idea of LSM (Linu Security Module).
> > It can provide a guest module several hooks at strategic points.
> > The guest module can make its decision whether required actions should be
> > allowed, or not.
> > In addition, PGACE also provides falicilites to manage security attribute
> > of database objects. Any tuple can have a its security attribute, and the
> > guest module can refer it to control accesses.
> >
> > A more conprehensive memo at:
> > http://code.google.com/p/sepgsql/wiki/WhatIsPGACE
> >
> > [2/4] - sepostgresql-sepgsql-8.4devel-3.patch
> >
> > This patch provides SE-PostgreSQL facilities based on PGACE.
> >
> > Security-Enhanced PostgreSQL (SE-PostgreSQL) is a security extension
> > built in PostgreSQL, to provide system-wide consistency in access
> > controls. It enables to apply a single unigied security policy of
> > SELinux for both operating system and database management system.
> > In addition, it also provides fine-grained mandatory access which
> > includes column-/row- level non-bypassable access control even if
> > privileged database users.
> >
> > Quick overview at:
> > http://code.google.com/p/sepgsql/wiki/WhatIsSEPostgreSQL
> >
> > [3/4] - sepostgresql-pg_dump-8.4devel-3.patch
> >
> > This patch gives us a feature to dump database with security attribute.
> > It is turned on with '--enable-selinux' option at pg_dump/pg_dumpall,
> > when the server works as SE- version.
> > No need to say, users need to have enough capabilities to dump whole of
> > database. It it same when they tries to restore the database.
> >
> > [4/4] - sepostgresql-policy-8.4devel-3.patch
> >
> > This patch gives us the default security policy for SE-PostgreSQL.
> > You can build it as a security policy module. It can be linked with
> > the existing distributor's policy, and reloaded.
> >
>
>
> --
> OSS Platform Development Division, NEC
> KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce, KaiGai,

Is PGACE ready for CommitFest, now?

--Josh

Josh Berkus wrote:
> Bruce, KaiGai,
>
> Is PGACE ready for CommitFest, now?
>
> --Josh

Yes, it's ready now.

See the following message:
http://archives.postgresql.org/pgsql-hackers/2008-06/msg00980.php
http://archives.postgresql.org/pgsql-hackers/2008-06/msg00840.php

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

KaiGai Kohei wrote:
> Josh Berkus wrote:
>> Bruce, KaiGai,
>>
>> Is PGACE ready for CommitFest, now?
>>
>> --Josh
>
> Yes, it's ready now.

OK, added to CommitFest.

--Josh