SE-PostgreSQL Updated Revision (r1460)

I also think it is a good idea to summarize current status of
SE-PostgreSQL, as Simon Riggs doing on his works.

The current revision of SE-PostgreSQL is 1425, available here:

[1/5] http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1425.patch
[2/5] http://sepgsql.googlecode.com/files/sepostgresql-utils-8.4devel-3-r1425.patch
[3/5] http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1425.patch
[4/5] http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1425.patch
[5/5] http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1425.patch

We had various kind of comments, feature requests and discussions during
previous/current commit fest, then whole of them are already included.

Currently, we have no open issues here.

As I summarized as follows, we had many discussions about its design
issues mainly, so my patch set has been updated to support them.
I believe we should move to detailed-reviews to merge the feature any
time now, since we should aware of v8.4 schedule.

I really would like folks to help/volunteer reviewing the patches, please!

* CommitFest:Nov
- Simon Riggs requires a new GUC option to turn on/off row-level security
labeling to reduce storage comsumption, then updated as follows:
http://archives.postgresql.org/message-id/492691A8.8030103@ak.jp.nec.com
- Bruce Momjian suggested Row-level database ACLs to be compiled in default.
- Discussions for default compile options: PostgreSQL doesn't prefer compile
time option to turn on/off features, except for platform specific one.
SE-PostgreSQL is indeed platform specific feature. But, it makes other
issue that need mutually-exclusive enhanced security feature.
We concluded it as follows:
- All configurable features should be compiled within a single binary.
- Both of DAC and MAC should be available simultaneously in row-level also.
- DAC is hardwired, and we allow users to choose an enhanced security feature.
- I updated the patch set to support both of Row-level database ACLs and
an enhanced security feature (SELinux) simultaneously. ('08/12/17)
http://archives.postgresql.org/message-id/4948B6BD.1050402@ak.jp.nec.com
- Robert Haas concerned about Stephen Frost's column-level privileges has
a trouble, so it's unclear whether it can get merged into v8.4.
- I also worked for his patch, then it got being ready for commit:
http://archives.postgresql.org/message-id/20090116045825.GY4656@tamriel.snowman.net
- Alvaro Herrera suggested "static inline" is not preferable.

* CommitFest:Sep
- Peter Eisentraut commented about its design specifications:
http://archives.postgresql.org/message-id/48D03953.6000308@gmx.net
- The hot issues were lack of fine-grained access controls in SQL-level,
and covert channels with row-level controls.
- We finally made agreement to provide platform independent row-level controls,
and explicit documentation about covert channels in PK/FK constraints.
No one didn't want to apply polyinstantiation idea.
- Simon Riggs requires wiki article to introduce SE-PostgreSQL.
http://wiki.postgresql.org/wiki/SEPostgreSQL
- Patch set was updated to support Row-level database ACLs
http://archives.postgresql.org/message-id/48F46606.4080207@ak.jp.nec.com

* CommitFest:Jul
- The patch set got documentation/testcases.
- Peter Eisentraut commented about some of items:
http://archives.postgresql.org/message-id/200807071739.58428.peter_e@gmx.net
- Then, these items are updated:
http://archives.postgresql.org/message-id/48773188.6000809@ak.jp.nec.com

* CommitFest:May
- First patch set for v8.4 were proposed.
- Tom Lane gave us various items to be improved.
http://archives.postgresql.org/message-id/3275.1210019965@sss.pgh.pa.us
- I had a presentation at PGcon2008 ottawa.
http://sepgsql.googlecode.com/files/PGCON20080523.pdf

* Prior phase
- First proposal of PGACE security framework, but I didn't know it was
just after the date of feature freeze in v8.3. So, it was suggested
to wait for v8.4 development cycle. ('07/04/17)
- 8.2.x based SE-PostgreSQL announced. ('07/09/04)
- SE-PostgreSQL package got merged into Fedora Project. ('07/11/08)
- 8.3.x based SE-PostgreSQL announced. ('08/03/08)

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

The patch set of SE-PostgreSQL and related stuff were updated (r1460).

[1/5] http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1460.patch
[2/5] http://sepgsql.googlecode.com/files/sepostgresql-utils-8.4devel-3-r1460.patch
[3/5] http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1460.patch
[4/5] http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1460.patch
[5/5] http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1460.patch

I reviewed the patch set by myself, and updated the following items.
However, I would like other hackers to review the code in honesty.

SE-PostgreSQL need any volunteers to review and comment the patch set.
Please give us your support!

List of updates:
- Rebased to the latest CVS HEAD, which includes the column-level
privileges based on the SQL-standard.
(The previous r1425 conflicts in some points.)

- Security policy (sepostgresql-devel.pp) was updated to fit both of
Fedora 10 and rawhide. Test cases are also modified to care the new
security policy.

- Cleanup: NUM_SELINUX_CATALOG was replaced by lengthof() macro to
avoid code duplications.

- Cleanup: sepgsqlCheckEmbeddedProcedure() is renamed to
sepgsqlCheckProcedureInstall() due to its confusable naming.

- Add a new permission: db_procedure:{install}
It enables to prevent malicious user-defined functions are installed
as a part of operators, conversions, types and so on.
The default policy allows to install functions labeled as "sepgsql_proc_t"
only, as an implementation of these facilities.
Meanwhile, functions defined by unprivileged users are labeled as
"user_sepgsql_proc_t" in default, and it is not allowed to install as
an operator and so on.
If DBA want to install user-defined functions for the purpose, he has to
confirm its harmless and relabel it to "sepgsql_proc_t" at first.
In the previous revision, it checked "db_procedure:{execute}" here,
but it is not enough actually, because unprivilged user is allowed to
execute self defined function.

- Code revising: The previous revision always denied required permissions,
when the kernel does not define them within its security policy.
But it can make unexpected behavior when we work SE-PostgreSQL on
a system with legacy security policy which lacks a part of newly
added permissions.
The revised one simply allows actions when these are undefined.

- Fixbug: It required superfluous permissions when we try to update
"security_label" system column but it does not change anything actually.
For example:
UPDATE t SET security_label = security_label;
This query does not change security_label, so we don't need to check
"db_tuple:{relabelfrom}" permission here.
It is obvious we cannot know what tuples are actually relabeled on
sepgsqlExecScan(), so any permission checks for write-operations are
moved to sepgsqlHeapTuple(Insert|Update|Delete) hooks.

- Fixbug: when we update pg_largeobject system catalog by hand, it has
a possibility to create/drop specific largeobject, so we add a check
on "db_blob:{create drop}" when pg_largeobject.loid is modified by
UPDATE statement.
For example:
UPDATE pg_largeobject SET loid = loid::int + 10 WHERE loid = 1234;
It is theoretically same as dropping a largeobject with loid:1234 and
creating a largeobject with loid:1244.

- Fixbug: Tome Lane pointed out a matter when a whole-row-reference on
the relation with RTE_JOIN makes crash at the "Column-Level Privileges"
thread. This revision added a special care for the situation.
It recursively walks on refered JoinExpr and picks up its sources to
check permission to them.

- Code revising: T_SEvalItemRelation and T_SEvalItemAttribute nodes are
integrated into T_SelinuxEvalItem node. In the previous revision,
it simply chains all appeared tables and columns as a list of obsoleted
node on Query->pgaceItem. But it has a trend the length of list grows long.
T_SelinuxEvalItem contains required permissions on a table and an array of
permissions for columns. It enables to keep the length of the list minimum.
Related stuffs in sepgsql/proxy.c is also revised.
- addEvalRelation() / addEvalAttribute() enhanced to handle T_SelinuxEvalItem.
- Functions to handle inheritance tables and whole-row-reference are clearly
sorted out. expandEvalItemInheritance() handles inheritance tables, and
expandEvalItemWholeRowRefs() handles whole-row-reference.

- Add a hook: pgaceExecuteTruncate()
The previous revision checks permissions on truncated tables and tuples
on pgaceProcessUtility(), but this approach need to extract all the target
including cascaded ones, so it made code duplication.
The new hook is deployed on ExecuteTruncate() and delivers a list of already
opened relations with AccessExclusiveLock.
A new sepgsqlExecuteTruncate() checks needed permission on the hook.

- Cleanup: sepgsqlTupleName() always copied an identifier of tuple for audit
record into its internal buffer, and returns its pointer to caller.
But it is not necessary for most cases. It is revised to return a pointer
within given tuple to avoid useless strcpy(), if possible. So, its valid
duration is limited to the duration of tuple, but there is no real matter.
In addition, callers of sepgsqlTupleName() are cleaned up, because its
length of line tend to grow a bit long.

- Add a check: CREATE/ALTER FOREIGN DATA WRAPPER has a capability to load
a discretional shared library module, so it is necessary to check
db_database:{install_module} permission.

- Add source code comments: src/backend/security/sepgsql/avc.c

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

On Fri, Jan 23, 2009 at 12:30 AM, KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
> The patch set of SE-PostgreSQL and related stuff were updated (r1460).
>
> [1/5] http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1460.patch
> [2/5] http://sepgsql.googlecode.com/files/sepostgresql-utils-8.4devel-3-r1460.patch
> [3/5] http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1460.patch
> [4/5] http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1460.patch
> [5/5] http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1460.patch

KaiGai -

I read through your docs patch tonight and did some copy editing.
Please see the attached patches, which I hope you will find helpful.
I have attached my suggested changes both as a patch against v1460
(sepostgresql-docs-rmh-vs-1460.gz) and also as patch against CVS HEAD
(sepostgresql-docs-rmh-vs-cvs-head), since I am not sure which is
easier for you. I have a couple of general comments about the
documentation:

1. The docs as written are very Red Hat-centric, even to the point of
making reference to specific versions of Red Hat RPMs. I think that
the community will find this unacceptable, as Red Hat is certainly not
the only SELinux-enabled distribution and I presume that we want to
support all of them to an equal degree.

2. Some of the information that is documented here properly belongs in
other sections of the documentation. For example, the information
about GUCs clearly belongs somewhere in the section on server
configuration where all of the other GUCs are documented, not in a
separate sections about SE-PostgreSQL. I suspect that all of the
information about row-level ACLs should be ripped out of security.sgml
and inserted into an appropriate portion of the "Database Roles and
Privileges" chapter, leaving this file to talk just about
SE-PostgreSQL.

3. It seems to me that the analogy between SQL DAC and Unix user/group
DAC is mentioned far too many times, and there are other cases where
information is repeated as well. I think it might help to reorganize
the document a bit so that you introduce concepts in the right order.
For example, the section that defines MAC and DAC is a ways down in
the document, but you use those terms a whole bunch of times before
defining them. I'm not 100% sure that we even want to be defining MAC
and DAC in our documentation, since those are general industry terms
that are not PostgreSQL-specific. But if we are going to define them
then we should try to do so in the clearest way possible.

Overall, I would say there is a fair amount of work left to be done to
get this documentation up to par, but it's a good start and I hope
that the attached patches and suggestions will be helpful.

...Robert

Robert Haas wrote:
> On Fri, Jan 23, 2009 at 12:30 AM, KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
>> The patch set of SE-PostgreSQL and related stuff were updated (r1460).
>>
>> [1/5] http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1460.patch
>> [2/5] http://sepgsql.googlecode.com/files/sepostgresql-utils-8.4devel-3-r1460.patch
>> [3/5] http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1460.patch
>> [4/5] http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1460.patch
>> [5/5] http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1460.patch
>
> KaiGai -
>
> I read through your docs patch tonight and did some copy editing.
> Please see the attached patches, which I hope you will find helpful.
> I have attached my suggested changes both as a patch against v1460
> (sepostgresql-docs-rmh-vs-1460.gz) and also as patch against CVS HEAD
> (sepostgresql-docs-rmh-vs-cvs-head), since I am not sure which is
> easier for you. I have a couple of general comments about the
> documentation:

Thanks your feedbacks!

I basically applied your fixes as is, expect for the following items:
- You replaced
! Its providing access controls are not _bypassable_ for any clients ...
by
! The access controls implemented by SE-PostgrSQL may not be _biased_ even ...

I wanted to express it is "unavoidable" here, so I changed as:
! The access controls implemented by SE-PostgrSQL may not be _bypassed_ even ...

- I found a typo: "MAC" is described as "MAc".

And, I have a question about documentation manner.
- You represented "getpeercon()" function as a system call.
But, it is actually a wrapper function of getsockopt(2) system call,
so the "getpeercon(3)" is not a system call strictly.
Is it necessary to represent these stuffs strictly correct?
(Thus, I wrote it as "API" in the r1460.)

> 1. The docs as written are very Red Hat-centric, even to the point of
> making reference to specific versions of Red Hat RPMs. I think that
> the community will find this unacceptable, as Red Hat is certainly not
> the only SELinux-enabled distribution and I presume that we want to
> support all of them to an equal degree.

I guess you pointed out about:
1. The "Requirement" section in "Build and Installation" assumes
RedHat/Fedora's RPM package and its version number.
2. The security context and security policy used to explanation
assumes specific security policy.
3. "Labeled IPsec" seciton points to "RedHatEL4 Security Guide",
and it assumes the racoon's configuration files are deployed
as RPM package doing.

About 1, is it necessary to rip the RPM specific version number
and replace it as:
selinux-policy which includes SE-PostgreSQL related stuffs.

About 2, SELinux community provides its default security policy,
and distributor's policy (including RedHat's one) is a derivative
of the default policy.
It is developed independent from distributor's cycle.
http://oss.tresys.com/projects/refpolicy
http://oss.tresys.com/repos/refpolicy/trunk/policy/modules/services/postgresql.te

You can find some of sepgsql_xxxx identifiers in postgresql.te.
All the appeared identifiers are upstreamed, so these are not Red Hat
specific.

About 3, If it rips the link to Red Hat and does not assume specific
path of "racoon.conf", the explnation become neutral.

> 2. Some of the information that is documented here properly belongs in
> other sections of the documentation. For example, the information
> about GUCs clearly belongs somewhere in the section on server
> configuration where all of the other GUCs are documented, not in a
> separate sections about SE-PostgreSQL.

These explanations are moved to "Security and Authentication" section
in "Chapter 18. Server Configuration".

> I suspect that all of the
> information about row-level ACLs should be ripped out of security.sgml
> and inserted into an appropriate portion of the "Database Roles and
> Privileges" chapter, leaving this file to talk just about
> SE-PostgreSQL.

It is indeed an aspect of row-level ACLs.
However, it is also a feature on PGACE framework, same as SE-PostgreSQL.
An idea is to put a reference to indicate the row-level ACLs section
on "Database Roles and Privileges" chapter, like:

PostgreSQL has an enhancement of database roles and privileges mechanism
which allows to database ACLs in row-level granuality. See, <xref ...>
for more details.

What do you think?

> 3. It seems to me that the analogy between SQL DAC and Unix user/group
> DAC is mentioned far too many times, and there are other cases where
> information is repeated as well. I think it might help to reorganize
> the document a bit so that you introduce concepts in the right order.

Indeed, it was redundant explanation. Thanks for youe edit.

> For example, the section that defines MAC and DAC is a ways down in
> the document, but you use those terms a whole bunch of times before
> defining them. I'm not 100% sure that we even want to be defining MAC
> and DAC in our documentation, since those are general industry terms
> that are not PostgreSQL-specific. But if we are going to define them
> then we should try to do so in the clearest way possible.

I can add the definitions of terms.
However, it is unclear whether PostgreSQL documentation should include
them, or not. For example, wikipedia has enough explanation for their
generam meanings.
http://en.wikipedia.org/wiki/Discretionary_Access_Control
http://en.wikipedia.org/wiki/Mandatory_Access_Control

It seems to me "Discretionary Access Control (DAC)" is an enough key
to search its meaning.

> Overall, I would say there is a fair amount of work left to be done to
> get this documentation up to par, but it's a good start and I hope
> that the attached patches and suggestions will be helpful.

I'm glad to see your help.
I'll pay my efforts for documentations also. But English is not my mother
language, so any suggestions are helpful for me.

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

The patch set of SE-PostgreSQL and related stuff were updated (r1467).

[1/5] http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1467.patch
[2/5] http://sepgsql.googlecode.com/files/sepostgresql-utils-8.4devel-3-r1467.patch
[3/5] http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1467.patch
[4/5] http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1467.patch
[5/5] http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1467.patch

List of updates:
- Documentation is updated based on Robert Haas's suggestions.
See, http://archives.postgresql.org/message-id/497C6808.2060109@kaigai.gr.jp
- Bugfix: SE-PostgreSQL related functions didn't raise an error
when "pgace_feature" option is not "selinux".
- Add a launcher program to run testcases with various kind of security context.

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

Robert,

The attached patch is a draft to replace RedHat/Fedora RPM centric
expressions, to add a reference at "Database Roles and Privileges"
chapter and a bit cleanups for the latest revision (r1467).
In the previous revision, it noted users to check the version of
RPM package, but the revised one notes actually required features.
The version number is rewritten as a hint.

What is your opinion?

Thanks,

KaiGai Kohei wrote:
> Robert Haas wrote:
>> On Fri, Jan 23, 2009 at 12:30 AM, KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>
>> wrote:
>>> The patch set of SE-PostgreSQL and related stuff were updated (r1460).
>>>
>>> [1/5]
>>> http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1460.patch
>>>
>>> [2/5]
>>> http://sepgsql.googlecode.com/files/sepostgresql-utils-8.4devel-3-r1460.patch
>>>
>>> [3/5]
>>> http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1460.patch
>>>
>>> [4/5]
>>> http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1460.patch
>>>
>>> [5/5]
>>> http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1460.patch
>>>
>>
>> KaiGai -
>>
>> I read through your docs patch tonight and did some copy editing.
>> Please see the attached patches, which I hope you will find helpful.
>> I have attached my suggested changes both as a patch against v1460
>> (sepostgresql-docs-rmh-vs-1460.gz) and also as patch against CVS HEAD
>> (sepostgresql-docs-rmh-vs-cvs-head), since I am not sure which is
>> easier for you. I have a couple of general comments about the
>> documentation:
>
> Thanks your feedbacks!
>
> I basically applied your fixes as is, expect for the following items:
> - You replaced
> ! Its providing access controls are not _bypassable_ for any clients ...
> by
> ! The access controls implemented by SE-PostgrSQL may not be _biased_
> even ...
>
> I wanted to express it is "unavoidable" here, so I changed as:
> ! The access controls implemented by SE-PostgrSQL may not be
> _bypassed_ even ...
>
> - I found a typo: "MAC" is described as "MAc".
>
> And, I have a question about documentation manner.
> - You represented "getpeercon()" function as a system call.
> But, it is actually a wrapper function of getsockopt(2) system call,
> so the "getpeercon(3)" is not a system call strictly.
> Is it necessary to represent these stuffs strictly correct?
> (Thus, I wrote it as "API" in the r1460.)
>
>
>> 1. The docs as written are very Red Hat-centric, even to the point of
>> making reference to specific versions of Red Hat RPMs. I think that
>> the community will find this unacceptable, as Red Hat is certainly not
>> the only SELinux-enabled distribution and I presume that we want to
>> support all of them to an equal degree.
>
> I guess you pointed out about:
> 1. The "Requirement" section in "Build and Installation" assumes
> RedHat/Fedora's RPM package and its version number.
> 2. The security context and security policy used to explanation
> assumes specific security policy.
> 3. "Labeled IPsec" seciton points to "RedHatEL4 Security Guide",
> and it assumes the racoon's configuration files are deployed
> as RPM package doing.
>
> About 1, is it necessary to rip the RPM specific version number
> and replace it as:
> selinux-policy which includes SE-PostgreSQL related stuffs.
>
> About 2, SELinux community provides its default security policy,
> and distributor's policy (including RedHat's one) is a derivative
> of the default policy.
> It is developed independent from distributor's cycle.
> http://oss.tresys.com/projects/refpolicy
>
> http://oss.tresys.com/repos/refpolicy/trunk/policy/modules/services/postgresql.te
>
>
> You can find some of sepgsql_xxxx identifiers in postgresql.te.
> All the appeared identifiers are upstreamed, so these are not Red Hat
> specific.
>
> About 3, If it rips the link to Red Hat and does not assume specific
> path of "racoon.conf", the explnation become neutral.
>
>
>> 2. Some of the information that is documented here properly belongs in
>> other sections of the documentation. For example, the information
>> about GUCs clearly belongs somewhere in the section on server
>> configuration where all of the other GUCs are documented, not in a
>> separate sections about SE-PostgreSQL.
>
> These explanations are moved to "Security and Authentication" section
> in "Chapter 18. Server Configuration".
>
>> I suspect that all of the
>> information about row-level ACLs should be ripped out of security.sgml
>> and inserted into an appropriate portion of the "Database Roles and
>> Privileges" chapter, leaving this file to talk just about
>> SE-PostgreSQL.
>
> It is indeed an aspect of row-level ACLs.
> However, it is also a feature on PGACE framework, same as SE-PostgreSQL.
> An idea is to put a reference to indicate the row-level ACLs section
> on "Database Roles and Privileges" chapter, like:
>
> PostgreSQL has an enhancement of database roles and privileges mechanism
> which allows to database ACLs in row-level granuality. See, <xref ...>
> for more details.
>
> What do you think?
>
>> 3. It seems to me that the analogy between SQL DAC and Unix user/group
>> DAC is mentioned far too many times, and there are other cases where
>> information is repeated as well. I think it might help to reorganize
>> the document a bit so that you introduce concepts in the right order.
>
> Indeed, it was redundant explanation. Thanks for youe edit.
>
>> For example, the section that defines MAC and DAC is a ways down in
>> the document, but you use those terms a whole bunch of times before
>> defining them. I'm not 100% sure that we even want to be defining MAC
>> and DAC in our documentation, since those are general industry terms
>> that are not PostgreSQL-specific. But if we are going to define them
>> then we should try to do so in the clearest way possible.
>
> I can add the definitions of terms.
> However, it is unclear whether PostgreSQL documentation should include
> them, or not. For example, wikipedia has enough explanation for their
> generam meanings.
> http://en.wikipedia.org/wiki/Discretionary_Access_Control
> http://en.wikipedia.org/wiki/Mandatory_Access_Control
>
> It seems to me "Discretionary Access Control (DAC)" is an enough key
> to search its meaning.
>
>> Overall, I would say there is a fair amount of work left to be done to
>> get this documentation up to par, but it's a good start and I hope
>> that the attached patches and suggestions will be helpful.
>
> I'm glad to see your help.
> I'll pay my efforts for documentations also. But English is not my mother
> language, so any suggestions are helpful for me.
>
> Thanks,

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

Sorry, I attached incorrect patch file.
It is the correct one.

KaiGai Kohei wrote:
> Robert,
>
> The attached patch is a draft to replace RedHat/Fedora RPM centric
> expressions, to add a reference at "Database Roles and Privileges"
> chapter and a bit cleanups for the latest revision (r1467).
> In the previous revision, it noted users to check the version of
> RPM package, but the revised one notes actually required features.
> The version number is rewritten as a hint.
>
> What is your opinion?
>
> Thanks,
>
> KaiGai Kohei wrote:
>> Robert Haas wrote:
>>> On Fri, Jan 23, 2009 at 12:30 AM, KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>
>>> wrote:
>>>> The patch set of SE-PostgreSQL and related stuff were updated (r1460).
>>>>
>>>> [1/5]
>>>> http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1460.patch
>>>>
>>>> [2/5]
>>>> http://sepgsql.googlecode.com/files/sepostgresql-utils-8.4devel-3-r1460.patch
>>>>
>>>> [3/5]
>>>> http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1460.patch
>>>>
>>>> [4/5]
>>>> http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1460.patch
>>>>
>>>> [5/5]
>>>> http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1460.patch
>>>>
>>>
>>> KaiGai -
>>>
>>> I read through your docs patch tonight and did some copy editing.
>>> Please see the attached patches, which I hope you will find helpful.
>>> I have attached my suggested changes both as a patch against v1460
>>> (sepostgresql-docs-rmh-vs-1460.gz) and also as patch against CVS HEAD
>>> (sepostgresql-docs-rmh-vs-cvs-head), since I am not sure which is
>>> easier for you. I have a couple of general comments about the
>>> documentation:
>>
>> Thanks your feedbacks!
>>
>> I basically applied your fixes as is, expect for the following items:
>> - You replaced
>> ! Its providing access controls are not _bypassable_ for any clients
>> ...
>> by
>> ! The access controls implemented by SE-PostgrSQL may not be
>> _biased_ even ...
>>
>> I wanted to express it is "unavoidable" here, so I changed as:
>> ! The access controls implemented by SE-PostgrSQL may not be
>> _bypassed_ even ...
>>
>> - I found a typo: "MAC" is described as "MAc".
>>
>> And, I have a question about documentation manner.
>> - You represented "getpeercon()" function as a system call.
>> But, it is actually a wrapper function of getsockopt(2) system call,
>> so the "getpeercon(3)" is not a system call strictly.
>> Is it necessary to represent these stuffs strictly correct?
>> (Thus, I wrote it as "API" in the r1460.)
>>
>>
>>> 1. The docs as written are very Red Hat-centric, even to the point of
>>> making reference to specific versions of Red Hat RPMs. I think that
>>> the community will find this unacceptable, as Red Hat is certainly not
>>> the only SELinux-enabled distribution and I presume that we want to
>>> support all of them to an equal degree.
>>
>> I guess you pointed out about:
>> 1. The "Requirement" section in "Build and Installation" assumes
>> RedHat/Fedora's RPM package and its version number.
>> 2. The security context and security policy used to explanation
>> assumes specific security policy.
>> 3. "Labeled IPsec" seciton points to "RedHatEL4 Security Guide",
>> and it assumes the racoon's configuration files are deployed
>> as RPM package doing.
>>
>> About 1, is it necessary to rip the RPM specific version number
>> and replace it as:
>> selinux-policy which includes SE-PostgreSQL related stuffs.
>>
>> About 2, SELinux community provides its default security policy,
>> and distributor's policy (including RedHat's one) is a derivative
>> of the default policy.
>> It is developed independent from distributor's cycle.
>> http://oss.tresys.com/projects/refpolicy
>>
>> http://oss.tresys.com/repos/refpolicy/trunk/policy/modules/services/postgresql.te
>>
>>
>> You can find some of sepgsql_xxxx identifiers in postgresql.te.
>> All the appeared identifiers are upstreamed, so these are not Red Hat
>> specific.
>>
>> About 3, If it rips the link to Red Hat and does not assume specific
>> path of "racoon.conf", the explnation become neutral.
>>
>>
>>> 2. Some of the information that is documented here properly belongs in
>>> other sections of the documentation. For example, the information
>>> about GUCs clearly belongs somewhere in the section on server
>>> configuration where all of the other GUCs are documented, not in a
>>> separate sections about SE-PostgreSQL.
>>
>> These explanations are moved to "Security and Authentication" section
>> in "Chapter 18. Server Configuration".
>>
>>> I suspect that all of the
>>> information about row-level ACLs should be ripped out of security.sgml
>>> and inserted into an appropriate portion of the "Database Roles and
>>> Privileges" chapter, leaving this file to talk just about
>>> SE-PostgreSQL.
>>
>> It is indeed an aspect of row-level ACLs.
>> However, it is also a feature on PGACE framework, same as SE-PostgreSQL.
>> An idea is to put a reference to indicate the row-level ACLs section
>> on "Database Roles and Privileges" chapter, like:
>>
>> PostgreSQL has an enhancement of database roles and privileges
>> mechanism
>> which allows to database ACLs in row-level granuality. See, <xref ...>
>> for more details.
>>
>> What do you think?
>>
>>> 3. It seems to me that the analogy between SQL DAC and Unix user/group
>>> DAC is mentioned far too many times, and there are other cases where
>>> information is repeated as well. I think it might help to reorganize
>>> the document a bit so that you introduce concepts in the right order.
>>
>> Indeed, it was redundant explanation. Thanks for youe edit.
>>
>>> For example, the section that defines MAC and DAC is a ways down in
>>> the document, but you use those terms a whole bunch of times before
>>> defining them. I'm not 100% sure that we even want to be defining MAC
>>> and DAC in our documentation, since those are general industry terms
>>> that are not PostgreSQL-specific. But if we are going to define them
>>> then we should try to do so in the clearest way possible.
>>
>> I can add the definitions of terms.
>> However, it is unclear whether PostgreSQL documentation should include
>> them, or not. For example, wikipedia has enough explanation for their
>> generam meanings.
>> http://en.wikipedia.org/wiki/Discretionary_Access_Control
>> http://en.wikipedia.org/wiki/Mandatory_Access_Control
>>
>> It seems to me "Discretionary Access Control (DAC)" is an enough key
>> to search its meaning.
>>
>>> Overall, I would say there is a fair amount of work left to be done to
>>> get this documentation up to par, but it's a good start and I hope
>>> that the attached patches and suggestions will be helpful.
>>
>> I'm glad to see your help.
>> I'll pay my efforts for documentations also. But English is not my mother
>> language, so any suggestions are helpful for me.
>>
>> Thanks,
>
>

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

> I basically applied your fixes as is, expect for the following items:
> - You replaced
> ! Its providing access controls are not _bypassable_ for any clients ...
> by
> ! The access controls implemented by SE-PostgrSQL may not be _biased_ even
> I wanted to express it is "unavoidable" here, so I changed as:
> ! The access controls implemented by SE-PostgrSQL may not be _bypassed_
> even ...

Good catch, my mistake.

> - I found a typo: "MAC" is described as "MAc".

Also my mistake.

> And, I have a question about documentation manner.
> - You represented "getpeercon()" function as a system call.
> But, it is actually a wrapper function of getsockopt(2) system call,
> so the "getpeercon(3)" is not a system call strictly.
> Is it necessary to represent these stuffs strictly correct?
> (Thus, I wrote it as "API" in the r1460.)

Oh, OK. It sounds a little awkward to me to refer to it as an API.
Perhaps we could just refer to it as getpeercon(3) and not call it
either an API or a system call.

> About 2, SELinux community provides its default security policy,
> and distributor's policy (including RedHat's one) is a derivative
> of the default policy.
> It is developed independent from distributor's cycle.
> http://oss.tresys.com/projects/refpolicy
> http://oss.tresys.com/repos/refpolicy/trunk/policy/modules/services/postgresql.te

OK, I wasn't aware of that. I think perhaps you could spell this out
a little more in the docs so people understand that there is an
upstream version which includes SE-PostgreSQL support from version
<whatever>.

>> I suspect that all of the
>> information about row-level ACLs should be ripped out of security.sgml
>> and inserted into an appropriate portion of the "Database Roles and
>> Privileges" chapter, leaving this file to talk just about
>> SE-PostgreSQL.
>
> It is indeed an aspect of row-level ACLs.
> However, it is also a feature on PGACE framework, same as SE-PostgreSQL.
> An idea is to put a reference to indicate the row-level ACLs section
> on "Database Roles and Privileges" chapter, like:

Actually, I think this should probably be broken up into three
sections. All of the stuff about how PGACE is not very interesting to
anyone who isn't a developer, so it should be moved to someplace under
"Internals". I would suggest just adding a new chapter to the end of
that section, after "How the Planner Uses Statistics".

The database ACL stuff properly belongs in the "Database Roles and
Privileges" section, and needs to be moved there, not just a
cross-reference.

The discussion of enhanced security and SE-PostgreSQL is another new
chapter, probably immediately following "Database Roles and
Privileges". I would suggest calling it "Enhanced Security and
SE-PostgreSQL".

>> For example, the section that defines MAC and DAC is a ways down in
>> the document, but you use those terms a whole bunch of times before
>> defining them. I'm not 100% sure that we even want to be defining MAC
>> and DAC in our documentation, since those are general industry terms
>> that are not PostgreSQL-specific. But if we are going to define them
>> then we should try to do so in the clearest way possible.
>
> I can add the definitions of terms.
> However, it is unclear whether PostgreSQL documentation should include
> them, or not. For example, wikipedia has enough explanation for their
> generam meanings.
> http://en.wikipedia.org/wiki/Discretionary_Access_Control
> http://en.wikipedia.org/wiki/Mandatory_Access_Control
>
> It seems to me "Discretionary Access Control (DAC)" is an enough key
> to search its meaning.

I agree. I think you should go through and rip out all of the
definitions and explanations of what these terms mean, and just use
them in the appropriate context. I think in general that the current
documentation spends far too much time explaining what SE-PostgreSQL
is and not enough time discussing the issues that are likely to come
up when you're actually using it. For example, it seems to me that
anyone who has any interest in using SE-PostgreSQL to control access
to functions will need a much more complicated policy than what you
are proposing here, and there doesn't seem to be much discussion of
that issue. I'm not really looking for specific examples of how to
build a policy so much as general considerations that you should keep
in mind when trying to prevent information leakage via functions.

> I'm glad to see your help.
> I'll pay my efforts for documentations also. But English is not my mother
> language, so any suggestions are helpful for me.

Well, your English is certainly better than my Japanese...

...Robert

Robert Haas wrote:
>> And, I have a question about documentation manner.
>> - You represented "getpeercon()" function as a system call.
>> But, it is actually a wrapper function of getsockopt(2) system call,
>> so the "getpeercon(3)" is not a system call strictly.
>> Is it necessary to represent these stuffs strictly correct?
>> (Thus, I wrote it as "API" in the r1460.)
>
> Oh, OK. It sounds a little awkward to me to refer to it as an API.
> Perhaps we could just refer to it as getpeercon(3) and not call it
> either an API or a system call.

I replaced all the "getpeercon()" by "getpeercon(3)", and removed
expression both "system call" and "API".

>> About 2, SELinux community provides its default security policy,
>> and distributor's policy (including RedHat's one) is a derivative
>> of the default policy.
>> It is developed independent from distributor's cycle.
>> http://oss.tresys.com/projects/refpolicy
>> http://oss.tresys.com/repos/refpolicy/trunk/policy/modules/services/postgresql.te
>
> OK, I wasn't aware of that. I think perhaps you could spell this out
> a little more in the docs so people understand that there is an
> upstream version which includes SE-PostgreSQL support from version
> <whatever>.

I noted it as:
| The upstreamed security policy (<literal>20080702</literal>
| or later) already has a set of rules for SE-PostgreSQL,
| as a part of PostgreSQL policy.

The "<whatever>" is not a tag, is it?

> Actually, I think this should probably be broken up into three
> sections. All of the stuff about how PGACE is not very interesting to
> anyone who isn't a developer, so it should be moved to someplace under
> "Internals". I would suggest just adding a new chapter to the end of
> that section, after "How the Planner Uses Statistics".
>
> The database ACL stuff properly belongs in the "Database Roles and
> Privileges" section, and needs to be moved there, not just a
> cross-reference.
>
> The discussion of enhanced security and SE-PostgreSQL is another new
> chapter, probably immediately following "Database Roles and
> Privileges". I would suggest calling it "Enhanced Security and
> SE-PostgreSQL".

OK, I deployed these section as you suggested.
- The "Row-level Database ACLs" section is moved to the tail of
Chapter 20. Database Roles and Privileges.
- The new "Enhanced Security and SE-PostgreSQL" is moved to Chapter.21.
- The new "Chapter 57. PGACE Security Framework" is moved to Chapter.57,
next to the "56. How the Planner Uses Statistics".

> I think in general that the current
> documentation spends far too much time explaining what SE-PostgreSQL
> is and not enough time discussing the issues that are likely to come
> up when you're actually using it. For example, it seems to me that
> anyone who has any interest in using SE-PostgreSQL to control access
> to functions will need a much more complicated policy than what you
> are proposing here, and there doesn't seem to be much discussion of
> that issue. I'm not really looking for specific examples of how to
> build a policy so much as general considerations that you should keep
> in mind when trying to prevent information leakage via functions.

I added a new section "21.3. Making a Security Policy", but it is
still empty. I think it is not necessary to document comprehensive
information about security policy, since it is not a SELinux document.

My plan is to introduce a simple copy & pastable example and steps to
build it (with standard toolchain) and to install it as an security
policy module.

Please wait for filling up the section...

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