Re: Remaining items for 8.4 (was Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710))
Koichi Suzuki wrote: I believe all the issues pointed out in http://archives.postgresql.org//pgsql-hackers/2008-10/msg01387.php as been covered in the current patch, as discussed with Simon. I also understand that we're running out of time. I pointed out a few more issues here: http://archives.postgresql.org/message-id/49b51791.5080...@enterprisedb.com I'd like to push this to pgFoundry first and then work again together with Sync.Rep and Hot Standby for 8.5. Great! -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Remaining items for 8.4 (was Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710))
Sorry I see the comment. I'll continue the work to fulfill the comment. 2009/3/17 Heikki Linnakangas heikki.linnakan...@enterprisedb.com: Koichi Suzuki wrote: I believe all the issues pointed out in http://archives.postgresql.org//pgsql-hackers/2008-10/msg01387.php as been covered in the current patch, as discussed with Simon. I also understand that we're running out of time. I pointed out a few more issues here: http://archives.postgresql.org/message-id/49b51791.5080...@enterprisedb.com I'd like to push this to pgFoundry first and then work again together with Sync.Rep and Hot Standby for 8.5. Great! -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- -- Koichi Suzuki - -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Remaining items for 8.4 (was Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710))
Bruce Momjian wrote: Well, we have been trying to go simplify the SE-PostgreSQL patch since September, and while we have made progress, we still have work to do, and at this point I think we have run out of time. I think we have given it a fair shot, but I don't think it is going to make 8.4. Agreed. At some point we just have to wrap up and cut the release. Tweaking indefinitely is not fair to all those patches that have already been pushed back, nor to those that have already been committed and are waiting to be released as part of 8.4. Apart from SE-PostgreSQL, we have four remaining items on the commitfest page: GIN fast insert I agree with Tom that we should just disable regular index scans for GIN. If someone misses it in 8.4, we can try to find a way to do it safely in 8.5. Removing existing capability is a bit worrisome, but I'm even less happy with the out of memory condition in this patch and in the partial match patch that has been committed already. That really needs to be cleaned up. B-Tree emulation for GIN I think this is in pretty good shape. Improve Performance of Multi-Batch Hash Join for Skewed Data Sets I believe everyone's happy with the performance testing that's been done. Some of the logic ought to be moved to the planner, and maybe there's some other cleanup to do. Proposal of PITR performance improvement Hmm. The first version of this was submitted back in October, as an external tool. There's still some outstanding issues: http://archives.postgresql.org//pgsql-hackers/2008-10/msg01387.php. I think we should push this to 8.5, for the same reasons as SE-PostgreSQL. On the positive side, the external tool is on pgFoundry for use with 8.4 (and earlier releases too?). -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Remaining items for 8.4 (was Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710))
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: Improve Performance of Multi-Batch Hash Join for Skewed Data Sets I believe everyone's happy with the performance testing that's been done. Some of the logic ought to be moved to the planner, and maybe there's some other cleanup to do. I'll take this up next. AFAIR refactoring to put that which should be in the planner into the planner was the only significant issue. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Remaining items for 8.4 (was Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710))
Hi, I believe all the issues pointed out in http://archives.postgresql.org//pgsql-hackers/2008-10/msg01387.php as been covered in the current patch, as discussed with Simon. I also understand that we're running out of time. I'd like to push this to pgFoundry first and then work again together with Sync.Rep and Hot Standby for 8.5. 2009/3/16 Heikki Linnakangas heikki.linnakan...@enterprisedb.com: Bruce Momjian wrote: Well, we have been trying to go simplify the SE-PostgreSQL patch since September, and while we have made progress, we still have work to do, and at this point I think we have run out of time. I think we have given it a fair shot, but I don't think it is going to make 8.4. Agreed. At some point we just have to wrap up and cut the release. Tweaking indefinitely is not fair to all those patches that have already been pushed back, nor to those that have already been committed and are waiting to be released as part of 8.4. Apart from SE-PostgreSQL, we have four remaining items on the commitfest page: GIN fast insert I agree with Tom that we should just disable regular index scans for GIN. If someone misses it in 8.4, we can try to find a way to do it safely in 8.5. Removing existing capability is a bit worrisome, but I'm even less happy with the out of memory condition in this patch and in the partial match patch that has been committed already. That really needs to be cleaned up. B-Tree emulation for GIN I think this is in pretty good shape. Improve Performance of Multi-Batch Hash Join for Skewed Data Sets I believe everyone's happy with the performance testing that's been done. Some of the logic ought to be moved to the planner, and maybe there's some other cleanup to do. Proposal of PITR performance improvement Hmm. The first version of this was submitted back in October, as an external tool. There's still some outstanding issues: http://archives.postgresql.org//pgsql-hackers/2008-10/msg01387.php. I think we should push this to 8.5, for the same reasons as SE-PostgreSQL. On the positive side, the external tool is on pgFoundry for use with 8.4 (and earlier releases too?). -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- -- Koichi Suzuki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710)
Alvaro Herrera wrote: Would it make sense to instead of removing and deferring pieces bit by bit to instead work the other way around? Extract just the part of the patch that maps SELinux capabilities to Postgres privileges as a first patch? Then discuss any other parts individually at a later date? I think that makes sense. Implement just a very basic core in a first patch, and start adding checks slowly, one patch each. We have talked about incremental patches in the past. We wouldn't get unbreakable PostgreSQL in a single commit, but we would at least start moving. The good thing about having started in the opposite direction is that by now we know that the foundation APIs are good enough to build the complete feature. Well, we have been trying to go simplify the SE-PostgreSQL patch since September, and while we have made progress, we still have work to do, and at this point I think we have run out of time. I think we have given it a fair shot, but I don't think it is going to make 8.4. KaiGai-san, the only option I can offer is perhaps to list a URL for your SE-PostgreSQL patch to be applied by people who want to use SE-PG. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710)
Bruce Momjian wrote:
Alvaro Herrera wrote:
Would it make sense to instead of removing and deferring pieces bit by bit to
instead work the other way around? Extract just the part of the patch that
maps SELinux capabilities to Postgres privileges as a first patch? Then
discuss any other parts individually at a later date?
I think that makes sense. Implement just a very basic core in a first
patch, and start adding checks slowly, one patch each. We have talked
about incremental patches in the past.
We wouldn't get unbreakable PostgreSQL in a single commit, but we
would at least start moving.
The good thing about having started in the opposite direction is that by
now we know that the foundation APIs are good enough to build the
complete feature.
Well, we have been trying to go simplify the SE-PostgreSQL patch since
September, and while we have made progress, we still have work to do,
and at this point I think we have run out of time. I think we have
given it a fair shot, but I don't think it is going to make 8.4.
KaiGai-san, the only option I can offer is perhaps to list a URL for
your SE-PostgreSQL patch to be applied by people who want to use SE-PG.
Needless to say, I'm dissatisfied with this offer. But I can understand
we're paying effort to release the v8.4 on schedule as far as possible,
and we don't have infinite time for development all the items.
Yes, it is possible to accept the offer for me.
Meanwhile, I would not like to repeat same thing in the v8.5 development
cycle again. At the head of this year, we have rest of three big new
features (Sync-replication, Hot-standby and SE-PostgreSQL) but all of
them have been bursted for the v8.4.
In the v8.5 development cycle, I'll pay effort to propose this feature
with smaller part, to build it up step-by-step, as suggested in this
commit fest. So, I would like folks to review and commit it in the
earlier phase.
What is your opinion?
Currently, we have the following action items for the SE-PostgreSQL with
full-functionalities. I'll consider what components can be implemented as
a separated patch again, and submit them at:
http://wiki.postgresql.org/wiki/CommitFest_2009-First
0. Changes in security policy.
- I got a few requirements for the SELinux security policy.
It is necessary to discuss in the SELinux community also.
- *:{select} and *:{use} permission should be integrated
- db_database:{get_param set_param} to be removed
- A new db_database:{superuser} permission
1. Security system columns support
It contains a few sub-facilities, and they can be submitted
in the different patches.
1-1. security system columns and pg_security system catalog
1-2. writable system columns support(security_label and security_acl)
1-3. reclaim unused entries in pg_security system catalog
2. Basic tables/columns-level access controls (dependency: 1-1)
It means the facility proposed in the v8.4 development.
We also need to discuss an issue of ACL_UPDATE/ACL_SELECT_FOR_UPDATE.
3. Row-level access control facilities (dependency: 1-2, 2)
It also provides permission checks in row-level granularity
both of DAC and MAC.
4. Advanced permission support (dependency: 2)
4-1. db_procedure:{install} permission.
Heikki also required similar stuff in the vanilla PostgreSQL.
4-2. db_database:{load_module install_module} permission.
4-3. file:{read write} permission for COPY TO/FROM and others.
4-4. permission checks on the large objects.
(I guess vanilla PostgreSQL also requires it.)
5. Audit support (dependency: 2)
It is not a facility proposed in the v8.4.
SE-PostgreSQL enables to generate audit records for violated accesses.
This facility write them out to system audit daemon.
Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei kai...@ak.jp.nec.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1714)
Heikki, I updated the SE-PostgreSQL patches: http://sepgsql.googlecode.com/files/sepgsql-core-8.4devel-r1714.patch http://sepgsql.googlecode.com/files/sepgsql-utils-8.4devel-r1714.patch http://sepgsql.googlecode.com/files/sepgsql-policy-8.4devel-r1714.patch http://sepgsql.googlecode.com/files/sepgsql-docs-8.4devel-r1714.patch http://sepgsql.googlecode.com/files/sepgsql-tests-8.4devel-r1714.patch - List of updates: * Removed the fixupSelectedColsByTrigger() which added ACL_SELECT on requiredPerms, and set a bit of attno=0 on selectedCols. * Removed the invocatin of sepgsqlCheckProcedureExecute() from ExecCallTriggerFunc() These two changes make clear the attitude for trigger functions in SE-PostgreSQL. So, it now considers them as a part of system internal operations. - Scale of patches: * r1714 (the latest revision) 59 files changed, 3622 insertions(+), 10 deletions(-), 4957 modifications(!) * r1710 (previous revision) 60 files changed, 3686 insertions(+), 10 deletions(-), 4952 modifications(!) ... about 70 lines were downsized. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710)
Alvaro Herrera wrote:
Gregory Stark escribió:
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
KaiGai Kohei wrote:
* ACL_SELECT_FOR_UPDATE has same value with ACL_UPDATE, so SE-PostgreSQL
checks db_table:{update} permission on SELECT ... FOR SHARE OF,
instead of db_table:{lock} permission.
This again falls into the category of trying to have more fine-grained
permissions than vanilla PostgreSQL has. Just give up on the lock permission,
and let it check update permission instead. Yes, it can be annoying that you
need update-permission to do SELECT FOR SHARE, but that's an existing problem
and not in scope for this patch.
Would it make sense to instead of removing and deferring pieces bit by bit to
instead work the other way around? Extract just the part of the patch that
maps SELinux capabilities to Postgres privileges as a first patch? Then
discuss any other parts individually at a later date?
I think that makes sense. Implement just a very basic core in a first
patch, and start adding checks slowly, one patch each. We have talked
about incremental patches in the past.
We wouldn't get unbreakable PostgreSQL in a single commit, but we
would at least start moving.
The good thing about having started in the opposite direction is that by
now we know that the foundation APIs are good enough to build the
complete feature.
What should I do for this matter?
At least, it is necessary to decide when we should fix it. v8.4? v8.5?
If we fix it soon, what strategy is desirable?
1) Add a new GRANT privilege something like LOCK.
It is straight forward approach, but contains user visible change.
In MySQL, it has an individual privilege for explicit table locks.
2) Shrink ACL_SELECT_FOR_UPDATE to ACL_UPDATE in runtime.
It is invisible from users, but GRANT UPDATE still contains
a meaning of explicit table locks.
3) GRANT UPDATE ... also allows users ACL_SELECT_FOR_UPDATE, not only
ACL_UPDATE.
It is similar to 2) option, but it also modifies ACL side, not the
requiredPerms side.
4) Other strategy?
Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei kai...@ak.jp.nec.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710)
KaiGai Kohei wrote:
* ACL_SELECT_FOR_UPDATE has same value with ACL_UPDATE, so SE-PostgreSQL
checks db_table:{update} permission on SELECT ... FOR SHARE OF,
instead of db_table:{lock} permission.
This again falls into the category of trying to have more fine-grained
permissions than vanilla PostgreSQL has. Just give up on the lock
permission, and let it check update permission instead. Yes, it can be
annoying that you need update-permission to do SELECT FOR SHARE, but
that's an existing problem and not in scope for this patch.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710)
Heikki Linnakangas wrote:
KaiGai Kohei wrote:
* ACL_SELECT_FOR_UPDATE has same value with ACL_UPDATE, so SE-PostgreSQL
checks db_table:{update} permission on SELECT ... FOR SHARE OF,
instead of db_table:{lock} permission.
This again falls into the category of trying to have more fine-grained
permissions than vanilla PostgreSQL has. Just give up on the lock
permission, and let it check update permission instead. Yes, it can be
annoying that you need update-permission to do SELECT FOR SHARE, but
that's an existing problem and not in scope for this patch.
Can I consider the term of problem means it can be resolved
in the future (v8.5, if possible) version?
--
KaiGai Kohei kai...@kaigai.gr.jp
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710)
KaiGai Kohei wrote:
Heikki Linnakangas wrote:
KaiGai Kohei wrote:
* ACL_SELECT_FOR_UPDATE has same value with ACL_UPDATE, so
SE-PostgreSQL
checks db_table:{update} permission on SELECT ... FOR SHARE OF,
instead of db_table:{lock} permission.
This again falls into the category of trying to have more fine-grained
permissions than vanilla PostgreSQL has. Just give up on the lock
permission, and let it check update permission instead. Yes, it can be
annoying that you need update-permission to do SELECT FOR SHARE, but
that's an existing problem and not in scope for this patch.
Can I consider the term of problem means it can be resolved
in the future (v8.5, if possible) version?
Sure, a patch to address that in 8.5 would be welcome.
I don't know why it's like that. Maybe no-one has just bothered. Or
maybe it's because of backwards-compatibility or SQL standard
compliance. In any case, it would seem useful to separate them in the
future.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710)
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
KaiGai Kohei wrote:
* ACL_SELECT_FOR_UPDATE has same value with ACL_UPDATE, so SE-PostgreSQL
checks db_table:{update} permission on SELECT ... FOR SHARE OF,
instead of db_table:{lock} permission.
This again falls into the category of trying to have more fine-grained
permissions than vanilla PostgreSQL has. Just give up on the lock permission,
and let it check update permission instead. Yes, it can be annoying that you
need update-permission to do SELECT FOR SHARE, but that's an existing problem
and not in scope for this patch.
Would it make sense to instead of removing and deferring pieces bit by bit to
instead work the other way around? Extract just the part of the patch that
maps SELinux capabilities to Postgres privileges as a first patch? Then
discuss any other parts individually at a later date?
That might relieve critics of the sneaking suspicion that there may be some
semantic change that hasn't been identified and discussed and snuck through?
Some of them are probably good ideas but if they are they're probably good
ideas even for non-SE semantics too.
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's PostGIS support!
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710)
Gregory Stark escribió:
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
KaiGai Kohei wrote:
* ACL_SELECT_FOR_UPDATE has same value with ACL_UPDATE, so SE-PostgreSQL
checks db_table:{update} permission on SELECT ... FOR SHARE OF,
instead of db_table:{lock} permission.
This again falls into the category of trying to have more fine-grained
permissions than vanilla PostgreSQL has. Just give up on the lock
permission,
and let it check update permission instead. Yes, it can be annoying that you
need update-permission to do SELECT FOR SHARE, but that's an existing
problem
and not in scope for this patch.
Would it make sense to instead of removing and deferring pieces bit by bit to
instead work the other way around? Extract just the part of the patch that
maps SELinux capabilities to Postgres privileges as a first patch? Then
discuss any other parts individually at a later date?
I think that makes sense. Implement just a very basic core in a first
patch, and start adding checks slowly, one patch each. We have talked
about incremental patches in the past.
We wouldn't get unbreakable PostgreSQL in a single commit, but we
would at least start moving.
The good thing about having started in the opposite direction is that by
now we know that the foundation APIs are good enough to build the
complete feature.
--
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710)
Alvaro Herrera wrote: Gregory Stark escribió: Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: KaiGai Kohei wrote: * [..feature description..] This again falls into the category of trying to have more fine-grained permissions than vanilla PostgreSQL has Would it make sense to instead of removing and deferring pieces bit by bit to instead work the other way around? Extract just the part of the patch that maps SELinux capabilities to Postgres privileges as a first patch? Then discuss any other parts individually at a later date? I think that makes sense. Implement just a very basic core in a first patch, and start adding checks slowly, one patch each. We have talked about incremental patches in the past. +1 from an end-user's point of view too. I'm quite aware of the postgres privileges, and if there were a MAC system of enforcing those I'd be reasonably likely to enable them right away. On the other hand, if SEPostgres initially comes with a different set of privileges that don't map to what I'm already using, I'm much less likely to spend the time to figure out the two different systems. And I do see row-level restrictions (and the other access restrictions mentioned in this thread) as quite orthogonal to MAC vs DAC. Would it be cool to have row-level permissions in postgres? Sure, as an abstract concept. Do I have any use for them? Seeing that I'm getting by without them, the answer must be not now. We wouldn't get unbreakable PostgreSQL in a single commit, but we would at least start moving. The good thing about having started in the opposite direction is that by now we know that the foundation APIs are good enough to build the complete feature. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710)
Alvaro Herrera alvhe...@commandprompt.com writes: Gregory Stark escribió: Would it make sense to instead of removing and deferring pieces bit by bit to instead work the other way around? Extract just the part of the patch that maps SELinux capabilities to Postgres privileges as a first patch? Then discuss any other parts individually at a later date? I think that makes sense. That's pretty much the advice we gave KaiGai-san in January ... which I gather he hasn't taken. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710)
Ron Mayer wrote:
Alvaro Herrera wrote:
Gregory Stark escribió:
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
KaiGai Kohei wrote:
* [..feature description..]
This again falls into the category of trying to have more fine-grained
permissions than vanilla PostgreSQL has
Would it make sense to instead of removing and deferring pieces bit by bit to
instead work the other way around? Extract just the part of the patch that
maps SELinux capabilities to Postgres privileges as a first patch? Then
discuss any other parts individually at a later date?
I think that makes sense. Implement just a very basic core in a first
patch, and start adding checks slowly, one patch each. We have talked
about incremental patches in the past.
+1 from an end-user's point of view too.
I'm quite aware of the postgres privileges, and if there were a MAC
system of enforcing those I'd be reasonably likely to enable them
right away.
On the other hand, if SEPostgres initially comes with a different set
of privileges that don't map to what I'm already using, I'm much less
likely to spend the time to figure out the two different systems.
I cannot update whole of the wikipage yet, but updated some of descriptions
in object classes and permission.
http://wiki.postgresql.org/wiki/SEPostgreSQL#Object_classes_and_permission
Some of permissions are mapped to the vanilla PostgreSQL privileges,
and some of them are not so.
* ACL_INSERT
The db_table:{insert} and db_column:{insert} for all the target
columns are checked. The table-level permission does not override
column-level permission, so the client need to have privileges
for both of objects. It is same as other permissions.
* ACL_SELECT
The db_table:{select} and db_column:{select} for all the target
columns are checked.
But it applies db_table:{lock} on LockTableCommand().
* ACL_UPDATE
The db_table:{update} and db_column:{update} for all the target
columns are checked.
* ACL_DELETE
The db_table:{delete} is also checked. No column-level checks here.
* ACL_TRUNCATE
The db_table:{delete} is also checked.
SE-PostgreSQL does not discriminate between TRUNCATE and DELETE.
* ACL_REFERENCES
* ACL_TRIGGER
SE-PostgreSQL does not care about these privileges.
But, it checks db_procedure:{execute} on trigger invocation time,
and it also checks db_table:{select} on checks of FK constraint
within its secondary SQL execution.
* ACL_EXECUTE
The db_procedure:{execute} is also checked.
This check is embedded within pg_proc_ackcheck().
* ACL_USAGE
* ACL_CREATE
* ACL_CREATE_TEMP
SE-PostgreSQL does not care about there privileges.
* ACL_CONNECT
The db_database:{access} is also checked.
This check is embedded within pg_database_aclcheck().
* ACL_SELECT_FOR_UPDATE
The db_table:{lock} should be also checked, but ...
* database superuser privilege
The db_database:{superuser} newly added should be also checked.
In addition, SE-PostgreSQL defines and users some of new privileges.
* db_xxx:{relabelfrom} and db_xxx:{relabelto}
It is checked when the security context of database objects are
changed.
* db_xxx:{create}
It is typically checked when CREATE TABLE and others.
SE-PostgreSQL assigns a default security context on the table and
columns newly created, if user does not give any security context
explicitly.
Then, it checks whether the user have db_xxx:{create} privileges
on the tables/columns/etc labeled as the security context, or not.
* db_xxx:{setattr}
* db_xxx:{drop}
It is typically cheched when ALTER/DROP TABLE and others.
The vanilla PostgreSQL checks user's privileges based on the ownership,
but SE-PostgreSQL does not consider the concept of owner due to its
MAC policy. These permission are checked based on the security context
assigned to the target objects.
* db_procedure:{entrypoint}
SE-PostgreSQL allows client to change its privilege during execution of
certain procedures (called as trusted procedure). It checks this
permission when user tries to invoke trusted procedure.
The vanilla PostgreSQL does not have similar ACL, but it concept it
similar to security definer or setuid on operating system.
And I do see row-level restrictions (and the other access restrictions
mentioned in this thread) as quite orthogonal to MAC vs DAC. Would it
be cool to have row-level permissions in postgres? Sure, as an abstract
concept. Do I have any use for them? Seeing that I'm getting by
without them, the answer must be not now.
We defined six permissions for row-level, but not used (included) now.
* db_tuple:{relabelfrom}
* db_tuple:{relabelto}
* db_tuple:{select}
* db_tuple:{update}
* db_tuple:{insert}
* db_tuple:{delete}
As SE-PostgreSQL doing on any other database object, it (can) assigns
a default security context on the tuple newly inserted, if user does
not given any security context explicitly.
Then, it checks db_tuple:{insert} permission on them.
Do you need explanation for any other permissions on db_tuple
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710)
* ACL_INSERT
The db_table:{insert} and db_column:{insert} for all the target
columns are checked. The table-level permission does not override
column-level permission, so the client need to have privileges
for both of objects. It is same as other permissions.
* ACL_SELECT
The db_table:{select} and db_column:{select} for all the target
columns are checked.
But it applies db_table:{lock} on LockTableCommand().
* ACL_UPDATE
The db_table:{update} and db_column:{update} for all the target
columns are checked.
* ACL_DELETE
The db_table:{delete} is also checked. No column-level checks here.
I'm more or less with you up to this point.
* ACL_TRUNCATE
The db_table:{delete} is also checked.
SE-PostgreSQL does not discriminate between TRUNCATE and DELETE.
But this seems wrong.
* ACL_REFERENCES
* ACL_TRIGGER
SE-PostgreSQL does not care about these privileges.
But, it checks db_procedure:{execute} on trigger invocation time,
and it also checks db_table:{select} on checks of FK constraint
within its secondary SQL execution.
And so do these. Why should there be any asymmetry with regular
PostgreSQL here?
* ACL_EXECUTE
The db_procedure:{execute} is also checked.
This check is embedded within pg_proc_ackcheck().
Good...
* ACL_USAGE
* ACL_CREATE
* ACL_CREATE_TEMP
SE-PostgreSQL does not care about there privileges.
Again, there doesn't seem to be any reason for this asymmetry. I
think you should change it.
...Robert
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710)
Robert Haas wrote:
* ACL_INSERT
The db_table:{insert} and db_column:{insert} for all the target
columns are checked. The table-level permission does not override
column-level permission, so the client need to have privileges
for both of objects. It is same as other permissions.
* ACL_SELECT
The db_table:{select} and db_column:{select} for all the target
columns are checked.
But it applies db_table:{lock} on LockTableCommand().
* ACL_UPDATE
The db_table:{update} and db_column:{update} for all the target
columns are checked.
* ACL_DELETE
The db_table:{delete} is also checked. No column-level checks here.
I'm more or less with you up to this point.
* ACL_TRUNCATE
The db_table:{delete} is also checked.
SE-PostgreSQL does not discriminate between TRUNCATE and DELETE.
But this seems wrong.
We consider these differences are just only the way to remove
all the tuples, not the target of the tables and its result.
* ACL_REFERENCES
* ACL_TRIGGER
SE-PostgreSQL does not care about these privileges.
But, it checks db_procedure:{execute} on trigger invocation time,
and it also checks db_table:{select} on checks of FK constraint
within its secondary SQL execution.
And so do these. Why should there be any asymmetry with regular
PostgreSQL here?
The ACL_REFERENCES is checked when we set up FK constraint, then
ri_PerformCheck() invokes another query to check constraint with
privileges of the table owner. It assumes the owner can access
on the table owned.
However, SE-PostgreSQL works orthogonally to the ownership.
We don't assume the client can access on the FK constrainted
table, and it apply appropriate checks on the secondary query
also, so it does not need to check on FK creation time.
The ACL_TRIGGER is also checked when CREATE TRIGGER.
If someone set a trigger function which is not allowed to execute
from other persons, the other person can invoke this function via
trigger mechanism.
I wonder why the vanilla PostgreSQL does not put pg_proc_aclcheck()
on the ExecCallTriggerFunc().
* ACL_EXECUTE
The db_procedure:{execute} is also checked.
This check is embedded within pg_proc_ackcheck().
Good...
* ACL_USAGE
* ACL_CREATE
* ACL_CREATE_TEMP
SE-PostgreSQL does not care about there privileges.
Again, there doesn't seem to be any reason for this asymmetry. I
think you should change it.
The ACL_CREATE and ACL_CREATE_TEMP are checked on namespace
in which the object newly created belongs. And the ACL_USAGE
is checked on various kind of database objects, but they don't
have its security context.
Funfamentally, SELinux requires to check user's privileges
on the object newly created. The object is labeled as a default
security context (if user does not specify anything) on its
creation.
So, if we implement it now, a facility is necessary to store
a security context of namespace and others.
Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei kai...@ak.jp.nec.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710)
KaiGai Kohei wrote:
I wonder why the vanilla PostgreSQL does not put pg_proc_aclcheck()
on the ExecCallTriggerFunc().
I don't think we can assume any trigger functions are trusted,
because normal users with ACL_TRIGGER privilege can set their
procedures on the allowed tables.
It also means someone without ACL_EXECUTE to invoke the functions,
but I cannot believe ACL_TRIGGER implicitly contains such a meaning.
Indeed, I put a hook to check db_procedure:{execute} permission
in SELinux, but putting pg_proc_aclcheck() here is meaningful
not only SE-PostgreSQL users.
I found another matter related to triggers.
I'll report it on another messages.
Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei kai...@ak.jp.nec.com
*** src/backend/commands/trigger.c (revision 1704)
--- src/backend/commands/trigger.c (working copy)
***
*** 1560,1566
--- 1560,1576
* call.
*/
if (finfo-fn_oid == InvalidOid)
+ {
+ AclResult aclresult;
+
+ aclresult = pg_proc_aclcheck(trigdata-tg_trigger-tgfoid,
+ GetUserId(), ACL_EXECUTE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_PROC,
+ get_func_name(trigdata-tg_trigger-tgfoid));
+
fmgr_info(trigdata-tg_trigger-tgfoid, finfo);
+ }
Assert(finfo-fn_oid == trigdata-tg_trigger-tgfoid);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
On Tue, 2009-03-10 at 09:56 +0900, KaiGai Kohei wrote: Joshua D. Drake wrote: ... Is there any possibility of having it be enabled at compile time? The default would be know but those distributions that would like to make use of it could? It was the design a half year ago, but Bruce suggested me a certain feature should not be enabled/disabled by compile time options, except for library/platform dependency. In addition, he also suggested a feature should be turned on/off by configuration option, because of it enables to distribute a single binary for more wider users. SE-PostgreSQL need the libselinux to communicate the in-kernel SELinux. So, --enable-selinux is necessary on compile time, it is fair enough. If we omit it, all the sepgsql() invocations are replaced by empty macros. seems ok. Another option to disable it would be something similar to how we currently handle DTrace ? If we compile it with --enable-selinux, it has two working modes controled by a guc option: sepostgresql (bool). If it is disabled, all the sepgsql() invocations returns at the head of themself without doing anything. I believe this behavior follows the previous suggestion. Have you been able to measure any speed difference between --enable-selinux on and off ? -- Hannu Krosing http://www.2ndQuadrant.com PostgreSQL Scalability and Availability Services, Consulting and Training -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Hannu Krosing wrote:
On Tue, 2009-03-10 at 09:56 +0900, KaiGai Kohei wrote:
Joshua D. Drake wrote:
...
Is there any possibility of having it be enabled at compile time? The
default would be know but those distributions that would like to make
use of it could?
It was the design a half year ago, but Bruce suggested me a certain
feature should not be enabled/disabled by compile time options,
except for library/platform dependency.
In addition, he also suggested
a feature should be turned on/off by configuration option, because of
it enables to distribute a single binary for more wider users.
SE-PostgreSQL need the libselinux to communicate the in-kernel SELinux.
So, --enable-selinux is necessary on compile time, it is fair enough.
If we omit it, all the sepgsql() invocations are replaced by empty
macros.
seems ok.
Another option to disable it would be something similar to how we
currently handle DTrace ?
DTrace uses Makefile to hack it.
I don't think it is a good example for me.
[src/backend/utils/Makefile]
probes.h: probes.d
ifeq ($(enable_dtrace), yes)
$(DTRACE) -C -h -s $ -o $...@.tmp
sed -e 's/POSTGRESQL_/TRACE_POSTGRESQL_/g' $...@.tmp $@
rm $...@.tmp
else
sed -f $(srcdir)/Gen_dummy_probes.sed $ $@
endif
Another example:
* POSIX fadvise
It puts #ifdef ... #endif block inside the functions, so it means
caller invokes an empty functions when POSIX fadvise is disabled.
int
FilePrefetch(File file, off_t offset, int amount)
{
#if defined(USE_POSIX_FADVISE) defined(POSIX_FADV_WILLNEED)
int returnCode;
Assert(FileIsValid(file));
[snip]
return returnCode;
#else
Assert(FileIsValid(file));
return 0;
#endif
}
* LDAP
It put #ifdef .. #endif block both of implementations and caller
side, but the number of blocks are quite small.
Basically, I think many of #ifdef ... #endif blocks are noisy, so
the current manner (using empty macro) can keep the code clean.
But, I'll follows the manner if we have anything in this situation.
If we compile it with --enable-selinux, it has two working modes
controled by a guc option: sepostgresql (bool).
If it is disabled, all the sepgsql() invocations returns at
the head of themself without doing anything.
I believe this behavior follows the previous suggestion.
Have you been able to measure any speed difference between
--enable-selinux on and off ?
I don't have measurement on the current revision, so please wait for
a while to get it measured.
Previous measurement includes effects in row-level access controls:
http://kaigai.sblo.jp/article/20303941.html (01 Oct 2008)
Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei kai...@ak.jp.nec.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Stephen Frost wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: KaiGai Kohei wrote: As I promised last week, SE-PostgreSQL patches are revised here: The patch adds permission checks to SET/SHOW. If that's useful functionality, it would be nice to see that as a separate patch, not requiring SE-Linux. My goodness. This patch seems to be going FAR beyond what I thought its charter was. I agree. I thought the idea was that the first round of SE-PostgreSQL additions would be to add SE hooks for permissions that PG already implements. Other permissions would then be implemented in a PG-way first, and SE hooks then added to those later. This seems to be a recurring theme with this patch. We stripped row-level permissions, now we have SET/SHOW and the function installation permissions. And the read/write file permissions. To make progress, we need to consider each new feature like that separately, as separate patches. KaiGei: Let's drop SET/SHOW permissions from the patch, I presume that's not critical for the overall goal. Dropping the function installation permissions would simplify the patch a lot. It would make the patch as whole a lot easier to swallow. Let's ask the same question as with the row-level permissions: If we drop the function installation stuff, would the rest of the patch still be useful? We can revisit that part in the future. I have the same concern as Tom about trying to curtail what superusers can do. We have not been concerned about what a superuser can and can't do before, so there could be small loopholes all over the codebase that we haven't thought about. Just as an example, you added checks to COPY to prevent reads from/writes to files. That's restricted to superusers. However, you left pg_read_file() in src/backend/utils/adt/genfile.c wide open. If we drop the goal of trying to restrict what a superuser can do, is the patch still useful? One idea is to add a single is superuser permission to sepgsql. That can be checked in a single place: superuser_arg(). If SE-Linux says that you don't have superuser permissions, then superuser() will return false even if the current user is marked as a superuser in pg_role. It would give the same level of protection as sprinkling those fine-grained checks all over the code, just in a more coarse-grain fashion. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Heikki Linnakangas wrote: This seems to be a recurring theme with this patch. We stripped row-level permissions, now we have SET/SHOW and the function installation permissions. And the read/write file permissions. To make progress, we need to consider each new feature like that separately, as separate patches. KaiGei: Let's drop SET/SHOW permissions from the patch, I presume that's not critical for the overall goal. OK, its significance is relatively low. Dropping the function installation permissions would simplify the patch a lot. It would make the patch as whole a lot easier to swallow. Let's ask the same question as with the row-level permissions: If we drop the function installation stuff, would the rest of the patch still be useful? We can revisit that part in the future. As far as we have the idea of superuser permission on SELinux also, we can do it later. I have the same concern as Tom about trying to curtail what superusers can do. We have not been concerned about what a superuser can and can't do before, so there could be small loopholes all over the codebase that we haven't thought about. Just as an example, you added checks to COPY to prevent reads from/writes to files. That's restricted to superusers. However, you left pg_read_file() in src/backend/utils/adt/genfile.c wide open. Oops, it was my overlooks. If we drop the goal of trying to restrict what a superuser can do, is the patch still useful? I want to keep permission checks on files specified by users, because the superuser permission affects very wide scope, and all or nothing policy in other word. However, the combination of clients and files is not so simple, and I think it is necessary to apply permission checks individually. I can agree to postpone the checks for procedure installation, C-libraries installation/loading. One idea is to add a single is superuser permission to sepgsql. That can be checked in a single place: superuser_arg(). If SE-Linux says that you don't have superuser permissions, then superuser() will return false even if the current user is marked as a superuser in pg_role. It would give the same level of protection as sprinkling those fine-grained checks all over the code, just in a more coarse-grain fashion. At least, I think it is not a strange design. In-kernel SELinux also has similar permission (capability class) to restrict root privileges, in additon to the invididual checks. (NOTE: In Linux, root privilges is a set of capability.) Maybe, I can submit the revised patch within 1.5 days. Please wait for a while. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
KaiGai Kohei kai...@kaigai.gr.jp writes: Heikki Linnakangas wrote: If we drop the goal of trying to restrict what a superuser can do, is the patch still useful? I want to keep permission checks on files specified by users, because the superuser permission affects very wide scope, and all or nothing policy in other word. However, the combination of clients and files is not so simple, and I think it is necessary to apply permission checks individually. I would think the big advantage of something like SELinux is precisely in cases like this. So for example a client that has a capability that allows him to read a file can pass that capability to the server and be able to use COPY to read it directly on the server. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's PostGIS support! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: If we drop the goal of trying to restrict what a superuser can do, is the patch still useful? One idea is to add a single is superuser permission to sepgsql. The agreement back in January was that what we'd consider for 8.4 is a patch that adds SELinux-driven enforcement of permissions checks that already exist in Postgres. Allowing the above seems to me to fit within that charter, but this other stuff definitely doesn't. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
On Tue, Mar 10, 2009 at 08:02:05PM +0900, KaiGai Kohei wrote: Please wait for a while. With all due respect to your hard work, waiting for this patch, even one more hour, is exactly what we shouldn't do for 8.4. Sad as it is, even if this patch were causing no controversy in its design, it would still be too late on grounds of its size and invasiveness. Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
On Mon, 2009-03-09 at 20:55 -0400, Tom Lane wrote: Joshua D. Drake j...@commandprompt.com writes: I know we are a little uncomfortable here but KaiGai-San (forgive me if I type that wrong) has proven to be a contributor in his own right, Perhaps it would help you calibrate the problem if I stated that I wouldn't trust a patch for this purpose written by myself, let alone somebody who hasn't been hacking the backend for ten years. (Where this purpose means the type of control KaiGai-san seems to hope to enforce, as opposed to just plugging some additional constraints into the existing ACL-check routines.) I think you misunderstand me. I have watched this thread very closely because it has specific strategic interest. For the record: * This patch does scare me * With great risk comes great reward So my question is, if the default is that sepostgres is disabled and can only be enabled via a compile time option, are your concerns just as weighty? What about marking the feature experimental. ./configure --help --enable-seEnables SE version of PostgreSQL for linux platforms (experimental) Yes it would be a break from what we do but it wouldn't hurt us to be just a little bit less stodgy as long as it is done in a responsible manner. Sincerely, Joshua D. Drake regards, tom lane -- PostgreSQL - XMPP: jdr...@jabber.postgresql.org Consulting, Development, Support, Training 503-667-4564 - http://www.commandprompt.com/ The PostgreSQL Company, serving since 1997 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Joshua D. Drake j...@commandprompt.com writes: I think you misunderstand me. I have watched this thread very closely because it has specific strategic interest. For the record: * This patch does scare me * With great risk comes great reward ... or great failure. My key concern is that we are setting ourselves up for failure by accepting a patch that hasn't attracted sufficient community interest. This patch needs way more eyeballs on it than it has gotten; which is not only bad in terms of the level of trust we should have in the patch right now, but it is a very negative signal about how much maintenance manpower it can expect in the future. Now the entire effort on KaiGai-san's part has been founded on the assumption that if you build it, they will come; and that is exactly the same argument I hear you making for continued investment in the project. But the track record so far, in terms of attracting hackers to work on the patch, says otherwise. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
On Tue, 2009-03-10 at 13:08 -0400, Tom Lane wrote: Joshua D. Drake j...@commandprompt.com writes: I think you misunderstand me. I have watched this thread very closely because it has specific strategic interest. For the record: * This patch does scare me * With great risk comes great reward ... or great failure. Sure, which all humans and projects must do at some point. It is how one learns after all. Sometimes the only thing you can do is fail. On the other hand if we succeed it will be a great reward. My key concern is that we are setting ourselves up for failure by accepting a patch that hasn't attracted sufficient community interest. This patch needs way more eyeballs on it than it has gotten; which is not only bad in terms of the level of trust we should have in the patch right now, but it is a very negative signal about how much maintenance manpower it can expect in the future. Now the entire effort on KaiGai-san's part has been founded on the assumption that if you build it, they will come; and that is exactly the same argument I hear you making for continued investment in the project. Yes but I am also offering an opportunity for others to show up. Which denying the patch does not do. If we provide SE support (even with marking it experimental), I would wager that some Linux distributions would begin to test it themselves which would allow us in turn to benefit by taking it out of experimental. Since RH, SuSE etc... are not going to Patch postgresql outside of some general compatibility issues. But all of this is moot. I see this as coming down to a simple result. * We don't enable it by default. * We mark it as experimental (or beta or whatever) Is there a serious regression in this line of thinking? It isn't unheard of in other projects. It allows the user to make a determination if they want to test/use the feature. It also continues the positive process of removing the fork which is pulling community away from us (at least to some degree) because those who are using SEpostgres are doing so out of his tree and not ours. Sincerely, Joshua D. Drake regards, tom lane -- PostgreSQL - XMPP: jdr...@jabber.postgresql.org Consulting, Development, Support, Training 503-667-4564 - http://www.commandprompt.com/ The PostgreSQL Company, serving since 1997 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Joshua D. Drake escribió: Yes but I am also offering an opportunity for others to show up. Which denying the patch does not do. If we provide SE support (even with marking it experimental), I would wager that some Linux distributions would begin to test it themselves which would allow us in turn to benefit by taking it out of experimental. Since RH, SuSE etc... are not going to Patch postgresql outside of some general compatibility issues. It was said upthread that SEPostgres is already packaged for Fedora. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
On Tue, 2009-03-10 at 14:47 -0300, Alvaro Herrera wrote: Joshua D. Drake escribió: Yes but I am also offering an opportunity for others to show up. Which denying the patch does not do. If we provide SE support (even with marking it experimental), I would wager that some Linux distributions would begin to test it themselves which would allow us in turn to benefit by taking it out of experimental. Since RH, SuSE etc... are not going to Patch postgresql outside of some general compatibility issues. It was said upthread that SEPostgres is already packaged for Fedora. Yes for but not by, AFAIK it is not actually included with Fedora. Essentially it is packaged like the PGSQLRPMS are packaged, available but outside of the project itself. Joshua D. Drake -- PostgreSQL - XMPP: jdr...@jabber.postgresql.org Consulting, Development, Support, Training 503-667-4564 - http://www.commandprompt.com/ The PostgreSQL Company, serving since 1997 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Joshua D. Drake j...@commandprompt.com writes: On Tue, 2009-03-10 at 14:47 -0300, Alvaro Herrera wrote: It was said upthread that SEPostgres is already packaged for Fedora. Yes for but not by, AFAIK it is not actually included with Fedora. Included with Fedora is an extremely loose concept. You can get it via yum install from the standard Fedora download servers. I don't believe it's counted as part of the PostgreSQL package group, nor included in the core distro CD set, but the CD-set approach to distribution seems to be dying anyway. There's too much stuff out there. However, if we did accept the patch, then the question would immediately become whether Devrim and I and other packagers for SELinux-capable distros ought to enable the feature in our builds. It does not work to deny responsibility for something by making it a configure option. You're just putting the hard decision onto packagers, who have no more knowledge than you do about what their users want, and (probably) considerably less understanding of the benefits/risks of some new configure option they happen to notice. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Tom Lane wrote: Joshua D. Drake j...@commandprompt.com writes: I know we are a little uncomfortable here but KaiGai-San (forgive me if I type that wrong) has proven to be a contributor in his own right, Not to put too fine a point on it, but: no, he hasn't. Show me one significant patch he's contributed before/beside this one. The only I thought Joshua was talking about his contribtions to F/OSS in general. He's credited on the NSA site for SELinux kernel scalability and locking issues: http://www.nsa.gov/research/selinux/contrib.shtml Kaigai Kohei of NEC replaced the original Access Vector Cache (AVC) locking scheme with a RCU-based approach, which solved the major SELinux kernel scalability problem, and fixed other locking issues in the SELinux kernel code. He later optimized the SELinux ebitmap implementation to improve performance on AVC misses. He also developed SE PostgreSQL, and is one of the developers for the SE busybox project. At first glance it seems it'd be valuable to have him as an active member of this community. Frankly, what we have here is a large patch, with insanely difficult correctness requirements, written by a Postgres newbie. I'm kinda hoping the discussion could turn to what parts (no matter how small) seem both useful safe enough for 8.4 - even if the main use of the small parts ar just as hooks to make it easier for SEPostgres to live as a parallel side project. As far as I can tell, the community feels interested in the feature set; but relatively unable to contribute since none of the people have that much of a security background. It seems the best way to fix that would be to get more people with a security background more involved. Not push them away. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
On Tue, 2009-03-10 at 10:49 -0700, Joshua D. Drake wrote: It was said upthread that SEPostgres is already packaged for Fedora. Yes for but not by, AFAIK it is not actually included with Fedora. It is, with the names sepostgresql*. Essentially it is packaged like the PGSQLRPMS are packaged, available but outside of the project itself. It is included in Fedora standard repositories: https://bugzilla.redhat.com/show_bug.cgi?id=249522 (It also includes my objections.) Regards, -- Devrim GÜNDÜZ, RHCE devrim~gunduz.org, devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr http://www.gunduz.org signature.asc Description: This is a digitally signed message part
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
On Tue, 2009-03-10 at 14:14 -0400, Tom Lane wrote: Joshua D. Drake j...@commandprompt.com writes: On Tue, 2009-03-10 at 14:47 -0300, Alvaro Herrera wrote: It was said upthread that SEPostgres is already packaged for Fedora. You're just putting the hard decision onto packagers, who have no more knowledge than you do about what their users want, and (probably) considerably less understanding of the benefits/risks of some new configure option they happen to notice. Which is exactly why we have two types of RPMS, --integer-datetimes and not. We would do the same thing with SE-Postgres. Joshua D. Drake -- PostgreSQL - XMPP: jdr...@jabber.postgresql.org Consulting, Development, Support, Training 503-667-4564 - http://www.commandprompt.com/ The PostgreSQL Company, serving since 1997 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Joshua D. Drake j...@commandprompt.com writes: On Tue, 2009-03-10 at 14:14 -0400, Tom Lane wrote: You're just putting the hard decision onto packagers, who have no more knowledge than you do about what their users want, and (probably) considerably less understanding of the benefits/risks of some new configure option they happen to notice. Which is exactly why we have two types of RPMS, --integer-datetimes and not. Maybe Devrim is doing that, but nobody else is. Debian went for --integer-datetimes years ago, Red Hat stuck with floats. Nobody is going to go to the trouble of maintaining two sets of RPMs, even assuming they notice there's a choice. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
On Tue, 2009-03-10 at 14:59 -0400, Tom Lane wrote: Joshua D. Drake j...@commandprompt.com writes: On Tue, 2009-03-10 at 14:14 -0400, Tom Lane wrote: You're just putting the hard decision onto packagers, who have no more knowledge than you do about what their users want, and (probably) considerably less understanding of the benefits/risks of some new configure option they happen to notice. At this point I don't know that any of this is going anywhere. I have presented what I think is a reasonable compromise to accept the feature. A compile-time option which was as designed all along with a flag called experimental. Which will be vastly easier to get people to test over time versus having to run a fork. I am for including this patch. I believe it is worth the risk. Sincerely, Joshua D. Drake -- PostgreSQL - XMPP: jdr...@jabber.postgresql.org Consulting, Development, Support, Training 503-667-4564 - http://www.commandprompt.com/ The PostgreSQL Company, serving since 1997 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
On Tue, 2009-03-10 at 14:59 -0400, Tom Lane wrote: Which is exactly why we have two types of RPMS, --integer-datetimes and not. Maybe Devrim is doing that, but nobody else is. It is only available *if* yum repo conf file is specially configured and if the distro is Fedora 10 and RHEL 5. Those packages are not distributed in regular channels. I already used this switch in 8.4 packages to follow upstream. -- Devrim GÜNDÜZ, RHCE devrim~gunduz.org, devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr http://www.gunduz.org signature.asc Description: This is a digitally signed message part
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Ron Mayer rm...@cheapcomplexdevices.com writes: As far as I can tell, the community feels interested in the feature set; but relatively unable to contribute since none of the people have that much of a security background. It seems the best way to fix that would be to get more people with a security background more involved. It's experience with the Postgres code base that I'm worried about. I don't question KaiGai-san's security background; I do doubt that he knows where all the skeletons are buried in the PG backend. A couple of very recent examples of that: his patch to fix a problem with inheritance of column privileges was approximately the right thing, but inefficiently duplicated the functionality of nearby code: http://archives.postgresql.org/pgsql-hackers/2009-03/msg00196.php and it didn't take Heikki long at all to note an oversight in the part of the latest sepostgres patch that attempted to confine superusers' file read/write abilities: http://archives.postgresql.org/pgsql-hackers/2009-03/msg00446.php More generally, there's been no discussion or community buy-in on design questions such as whether the patch should even try to confine superusers on such a fine-grained basis. (I agree with Heikki's thought that this may be a lost cause given our historical design assumption that superusers can do anything.) So I remain strongly of the opinion that what this patch lacks is review from longtime PG hackers. It's not the security community that is missing from the equation. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
On Tue, 2009-03-10 at 11:44 -0700, Joshua D. Drake wrote: We would do the same thing with SE-Postgres. No, no. I already experienced this with --integer-datetimes sets, and I don't ever want to maintain another set. It is horrible. -- Devrim GÜNDÜZ, RHCE devrim~gunduz.org, devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr http://www.gunduz.org signature.asc Description: This is a digitally signed message part
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Hannu Krosing wrote: If we compile it with --enable-selinux, it has two working modes controled by a guc option: sepostgresql (bool). If it is disabled, all the sepgsql() invocations returns at the head of themself without doing anything. I believe this behavior follows the previous suggestion. Have you been able to measure any speed difference between --enable-selinux on and off ? At the last night, I measured TPS by pgbench in three cases: 1) A binary compiled without --enable-selinux 2) A binary compiled with --enable-selinux, runtime disabled 3) A binary compiled with --enable-selinux, runtime enabled Then, I cannot observe statically meaningful differences here. * Environment CPU: Core2Duo E6400 (2.13GHz) Mem: 2048MB kernel: 2.6.28-3.fc11.i686 * Parameters - shared_buffers = 512MB - rest of parameters are in the default * Benchmarch % pgbench -i -s 10 postgres % pgbench -c 2 -t 10 postgres --- 6 times * Results (1) compiled without --enable-selinux 1st: 478.565569 2nd: 478.223391 3rd: 442.365636 4th: 468.988499 5th: 482.173836 6th: 448.208615 - AVG: 466.420924 (STD: 17.0404) (2) compiled with --enable-selinux, runtime disabled 1st: 469.005777 2nd: 485.602091 3rd: 449.096123 4th: 460.657368 5th: 476.791923 6th: 444.027405 - AVG: 464.196781 (STD: 16.0456) (3) compiled with --enable-selinux, runtime enabled 1st: 462.702242 2nd: 473.312013 3rd: 442.214347 4th: 468.465614 5th: 473.498682 6th: 468.973759 - AVG: 464.861109 (STD: 11.7768) -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1710)
Heikki, it is the list of updated patches:
http://sepgsql.googlecode.com/files/sepgsql-core-8.4devel-r1710.patch
http://sepgsql.googlecode.com/files/sepgsql-utils-8.4devel-r1710.patch
http://sepgsql.googlecode.com/files/sepgsql-policy-8.4devel-r1710.patch
http://sepgsql.googlecode.com/files/sepgsql-docs-8.4devel-r1710.patch
http://sepgsql.googlecode.com/files/sepgsql-tests-8.4devel-r1710.patch
- List of updates:
* Permission checks on SET/SHOW were removed.
* Add a new permission: db_database:{superuser}
sepgsqlCheckDatabaseSuperuser() is invoked from superuser_arg()
to check whether the clietn can perform as a superuser in this
database, or not.
* Permission checks on procedure installation is separated.
* Permission checks on install/load C-libraries are separated.
* Read file checks on pg_read_file() is added.
- Scale of patches:
* r1710 (the latest revision)
60 files changed, 3686 insertions(+), 10 deletions(-), 4952 modifications(!)
* r1704 (previous revision)
60 files changed, 4048 insertions(+), 11 deletions(-), 4944 modifications(!)
... about 300 lines were downsized.
- Remaining issue:
* ACL_SELECT_FOR_UPDATE has same value with ACL_UPDATE, so SE-PostgreSQL
checks db_table:{update} permission on SELECT ... FOR SHARE OF,
instead of db_table:{lock} permission.
Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei kai...@ak.jp.nec.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Tom Lane wrote: Ron Mayer rm...@cheapcomplexdevices.com writes: As far as I can tell, the community feels interested in the feature set; but relatively unable to contribute since none of the people have that much of a security background. It seems the best way to fix that would be to get more people with a security background more involved. It's experience with the Postgres code base that I'm worried about. I don't question KaiGai-san's security background; I do doubt that he knows where all the skeletons are buried in the PG backend. A couple of very recent examples of that: his patch to fix a problem with inheritance of column privileges was approximately the right thing, but inefficiently duplicated the functionality of nearby code: http://archives.postgresql.org/pgsql-hackers/2009-03/msg00196.php and it didn't take Heikki long at all to note an oversight in the part of the latest sepostgres patch that attempted to confine superusers' file read/write abilities: http://archives.postgresql.org/pgsql-hackers/2009-03/msg00446.php Indeed, I have less than three years experience of development in PostgreSQL backend. However, I don't believe it is a productive discussion to point out such kind of failures. At least, I think it is worthwhile to report bugs/submit patches much more than keeping silent with being afraid of failures. If submitted patches are not still enough elegant, we can fix and improve them via discussions. More generally, there's been no discussion or community buy-in on design questions such as whether the patch should even try to confine superusers on such a fine-grained basis. (I agree with Heikki's thought that this may be a lost cause given our historical design assumption that superusers can do anything.) So I remain strongly of the opinion that what this patch lacks is review from longtime PG hackers. It's not the security community that is missing from the equation. Two months ago, I agreed to postpone some of features especially hot in discussion, to reduce the scale of patches and burden of reviewers on the v8.4 development phase. In addition, I also reduced more than 1,000 lines as Heikki suggested. Its purpose is to focus the points to be discussed. I would like to have a productive discssion. -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
As I promised last week, SE-PostgreSQL patches are revised here:
[1/5] http://sepgsql.googlecode.com/files/sepgsql-core-8.4devel-r1704.patch
[2/5] http://sepgsql.googlecode.com/files/sepgsql-utils-8.4devel-r1704.patch
[3/5] http://sepgsql.googlecode.com/files/sepgsql-policy-8.4devel-r1704.patch
[4/5] http://sepgsql.googlecode.com/files/sepgsql-docs-8.4devel-r1704.patch
[5/5] http://sepgsql.googlecode.com/files/sepgsql-tests-8.4devel-r1704.patch
* List of updates:
- It is rebased to the latest CVS HEAD.
- The two reader permission (select and use) are integrated into
a single one (select) as the original design did two year's ago.
(It also enables to pick up read columns from rte-selectedCols.)
- The 'walker' code in sepgsql/checker.c is removed.
In the previous revision, SE-PostgreSQL walked on the given query
tree to pick up all the appeared tables/columns. The reason why
we needed separated walker phase is SE-PostgreSQL wanted to apply
two kind of reader permissions, but these are integrated into one.
(In addition, column-level privileges are not available when I
started to develop SE-PostgreSQL. :-))
In the currect version, SE-PostgreSQL knows what tables/columns
are appeared in the given query, from relid, selectedCols and
modifiedCols in RangeTblEntry. Then, it makes access controls
decision communicating with in-kernel SELinux.
After the existing DAC are checked, SE-PostgreSQL also checks
client's privileges on the appeared tables/columns as DAC doing.
Required privilges are follows these rules:
* If ACL_SELECT is set on rte-requiredPerms, client need to have
db_table:{select} and db_column:{select} for the tables/columns.
* If ACL_INSERT is set on rte-requiredPerms, client need to have
db_table:{insert} and db_column:{insert} for the tables/columns.
* If ACL_UPDATE is set on rte-requiredPerms, client need to have
db_table:{update} and db_column:{update} for the tables/columns.
* If ACL_DELETE is set on rte-requiredPerms, client need to have
db_table:{delete} for the tables.
This design change gives us a great benefit in code maintenance.
A trade-off is hardwired rules to modify pg_rewrite system catalog.
The correctness access controls depends on this catalog is protected
from unexpected manipulation by hand, because it stores a parsed
query tree of views.
- T_SelinuxItem is removed from include/node/nodes.h, and the codes
related to the node type is eliminated from copyfuncs.c ant others.
It was used to store all appeared tables/columns in the walker phase,
but now it is unnecessary.
- Several functions are moved to appropriate files:
- The codes related to permission bits are consolidated to
'sepgsql/perms.c' as its filename means.
(It was placed at 'avc.c' due to historical reason.)
- A few hooks (such as sepgsqlHeapTupleInsert) are moved to 'checker.c',
and rest of simple hooks are kept in 'hooks.c'.
- The scale of patches become a bit slim more.
rev.1668 rev.1704
security/Makefile| 11 - | 11
security/sepgsql/Makefile| 16 - | 16
security/sepgsql/avc.c | 1157 +++ - | 837 +
security/sepgsql/checker.c | 902 +- | 538 +++
security/sepgsql/core.c | 235 +- | 247 +
security/sepgsql/dummy.c | 37 - | 43
security/sepgsql/hooks.c | 748 - | 621 +++
security/sepgsql/label.c | 360 ++ - | 360 ++
security/sepgsql/perms.c | 295 +- | 400 ++
[kai...@saba ~]$ diffstat sepgsql-core-8.4devel-r1668.patch
:
64 files changed, 4770 insertions(+), 11 deletions(-), 4947 modifications(!)
[kai...@saba ~]$ diffstat sepgsql-core-8.4devel-r1704.patch
:
60 files changed, 4048 insertions(+), 11 deletions(-), 4944 modifications(!)
About 700 lines can be reduced in total!
I believe this revision can reduce the burden of reviewers.
Please any comments!
* An issue:
I found a new issue in this revision.
- ACL_SELECT_FOR_UPDATE and ACL_UPDATE have identical value.
In this version, SE-PostgreSQL knows what permission should be checked
via RangeTblEntry::requiredPerms, and it applies its access control
policy with translating them into SELinux's permissions.
But we have a trouble in the following query.
[kai...@saba ~]$ psql postgres
psql (8.4devel)
Type help for help.
postgres=# CREATE TABLE t1 (a int, b text)
postgres-# SECURITY_LABEL = 'system_u:object_r:sepgsql_ro_table_t:s0';
CREATE TABLE
-- NOTE: sepgsql_ro_table_t means read-only table from unpriv clients.
postgres=# INSERT INTO t1 VALUES (1, 'aaa'), (2, 'bbb');
INSERT 0 2
postgres=# \q
[kai...@saba ~]$ runcon -t sepgsql_test_t -- psql postgres
psql (8.4devel)
Type help for help.
-- NOTE: sepgsql_test_t means unpriv client.
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
KaiGai Kohei wrote: As I promised last week, SE-PostgreSQL patches are revised here: There's checks for reading/writing files with COPY, in sepgsqlCheckFileRead sepgsqlCheckFileWrite). Doesn't the OS do similar checks when the process tries to invoke the read()/write()? Is that not enough? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
KaiGai Kohei wrote:
As I promised last week, SE-PostgreSQL patches are revised here:
I think I now understand what sepgsqlCheckProcedureInstall is trying to
achieve. It's trying to stop attacks where you trick another user to run
your malicious code. We had a serious vulnerability of that kind a while
ago (http://archives.postgresql.org//pgsql-hackers/2008-01/msg00268.php)
when ANALYZE and VACUUM FULL ran expression and partial index predicates
with (typically) superuser privileges.
It seems that sepgsqlCheckProcedureInstall is trying to provide a more
thorough solution to the trojan horse problem than what we did back
then. It stops you from installing an untrusted function as a type
input/output function, operator implementing function etc. Now that
could be useful on its own, quite apart from the rest of the
SE-PostgreSQL patch, in which case I'd like to see that implemented as a
separate patch, so that you can use the facility even if you're not
using SE-PostgreSQL.
Some details of that:
+ void
+ sepgsqlCheckProcedureInstall(Relation rel, HeapTuple newtup, HeapTuple oldtup)
+ {
+ /*
+* db_procedure:{install} check prevent a malicious functions
+* to be installed, as a part of system catalogs.
+* It is necessary to prevent other person implicitly to invoke
+* malicious functions.
+*/
+ switch (RelationGetRelid(rel))
+ {
+ case AggregateRelationId:
+ /*
+* db_procedure:{execute} is checked on invocations of:
+* pg_aggregate.aggfnoid
+* pg_aggregate.aggtransfn
+* pg_aggregate.aggfinalfn
+*/
+ break;
+
+ case AccessMethodRelationId:
+ CHECK_PROC_INSTALL_PERM(pg_am, aminsert, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, ambeginscan, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, amgettuple, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, amgetbitmap, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, amrescan, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, amendscan, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, ammarkpos, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, amrestrpos, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, ambuild, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, ambulkdelete, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, amvacuumcleanup, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, amcostestimate, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, amoptions, newtup, oldtup);
+ break;
ISTM that you should just forbid any changes to pg_am in the default
policy. That's very low level stuff. If you can modify that, you can
wreck a lot of havoc, quite possibly turning it into a vulnerability
even if you can't directly install a malicious function there.
+ case AccessMethodProcedureRelationId:
+ CHECK_PROC_INSTALL_PERM(pg_amproc, amproc, newtup, oldtup);
+ break;
+
+ case CastRelationId:
+ CHECK_PROC_INSTALL_PERM(pg_cast, castfunc, newtup, oldtup);
+ break;
We check execute permission on the cast function at runtime.
+ case ConversionRelationId:
+ CHECK_PROC_INSTALL_PERM(pg_conversion, conproc, newtup, oldtup);
+ break;
This ought to be unnecessary now. Only C-functions can be installed as
conversion procs, and a C function can do anything, so there's little
point in checking this anymore.
+ case ForeignDataWrapperRelationId:
+ CHECK_PROC_INSTALL_PERM(pg_foreign_data_wrapper, fdwvalidator,
newtup, oldtup);
+ break;
Hmm, calls to fdwvalidator are not at all performance critical, so maybe
we should just check execute permission when it's called.
+ case LanguageRelationId:
+ CHECK_PROC_INSTALL_PERM(pg_language, lanplcallfoid, newtup,
oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_language, lanvalidator, newtup,
oldtup);
+ break;
I think these need to be C-functions.
+ case OperatorRelationId:
+ CHECK_PROC_INSTALL_PERM(pg_operator, oprcode, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_operator, oprrest, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_operator, oprjoin, newtup, oldtup);
+ break;
oprcode is checked for execute permission when the operator is used. For
oprrest and oprjoin, we could add checks into the planner where they're
called. (pg_operator.oprcom and pg_operator.oprnegate are missing?)
+ case TSParserRelationId:
+ CHECK_PROC_INSTALL_PERM(pg_ts_parser, prsstart, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_ts_parser, prstoken, newtup, oldtup);
+
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Heikki Linnakangas wrote: KaiGai Kohei wrote: As I promised last week, SE-PostgreSQL patches are revised here: There's checks for reading/writing files with COPY, in sepgsqlCheckFileRead sepgsqlCheckFileWrite). Doesn't the OS do similar checks when the process tries to invoke the read()/write()? Is that not enough? Please note that who invokes read()/write() system calls. In this case, PostgreSQL server process invokes these system calls instead of the client process. So, operating system need to allow the PostgreSQL server process to invoke these system calls on the target files of COPY TO/FROM. In addition, SE-PostgreSQL also checks read/write permission of client process for these files. Why it is possible is client's privileges are represented in same form of operating system. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
KaiGai Kohei wrote: As I promised last week, SE-PostgreSQL patches are revised here: The patch adds permission checks to SET/SHOW. If that's useful functionality, it would be nice to see that as a separate patch, not requiring SE-Linux. I think it's leaking because current_setting(), set_config() and pg_show_all_settings() functions don't perform the same checks. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: KaiGai Kohei wrote: As I promised last week, SE-PostgreSQL patches are revised here: The patch adds permission checks to SET/SHOW. If that's useful functionality, it would be nice to see that as a separate patch, not requiring SE-Linux. My goodness. This patch seems to be going FAR beyond what I thought its charter was. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
* Tom Lane (t...@sss.pgh.pa.us) wrote: Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: KaiGai Kohei wrote: As I promised last week, SE-PostgreSQL patches are revised here: The patch adds permission checks to SET/SHOW. If that's useful functionality, it would be nice to see that as a separate patch, not requiring SE-Linux. My goodness. This patch seems to be going FAR beyond what I thought its charter was. I agree. I thought the idea was that the first round of SE-PostgreSQL additions would be to add SE hooks for permissions that PG already implements. Other permissions would then be implemented in a PG-way first, and SE hooks then added to those later. Stephen signature.asc Description: Digital signature
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Heikki Linnakangas wrote: KaiGai Kohei wrote: As I promised last week, SE-PostgreSQL patches are revised here: I think I now understand what sepgsqlCheckProcedureInstall is trying to achieve. It's trying to stop attacks where you trick another user to run your malicious code. We had a serious vulnerability of that kind a while ago (http://archives.postgresql.org//pgsql-hackers/2008-01/msg00268.php) when ANALYZE and VACUUM FULL ran expression and partial index predicates with (typically) superuser privileges. It seems that sepgsqlCheckProcedureInstall is trying to provide a more thorough solution to the trojan horse problem than what we did back then. It stops you from installing an untrusted function as a type input/output function, operator implementing function etc. Now that could be useful on its own, quite apart from the rest of the SE-PostgreSQL patch, in which case I'd like to see that implemented as a separate patch, so that you can use the facility even if you're not using SE-PostgreSQL. Yes, the purpose of sepgsqlCheckProcedureInstall() is to prevent users to invoke functions installed by other malicious/untrusted one, typically known as trojan-horse. Basically, I can agree the vanilla PostgreSQL also provide similar stuff to prevent to install untrusted functions as a part of system internal codes. If we have such a facility as a basis, we can implement sepgsqlCheckProcedureInstall() hook more simple and easier to maintenance. [snip] + case ConversionRelationId: + CHECK_PROC_INSTALL_PERM(pg_conversion, conproc, newtup, oldtup); + break; This ought to be unnecessary now. Only C-functions can be installed as conversion procs, and a C function can do anything, so there's little point in checking this anymore. We should not assume only C-functions can be installed on pg_conversion (and other internal stuff), because a superuser can update system catalog by hand. Example) postgres=# CREATE OR REPLACE FUNCTION testfn() postgres-# RETURNS int LANGUAGE sql AS 'SELECT 1'; CREATE FUNCTION postgres=# UPDATE pg_conversion SET conproc = 'testfn'::regproc where oid=11276; UPDATE 1 postgres=# set client_encoding = 'sjis'; server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. The connection to the server was lost. Attempting reset: WARNING: terminating connection because of crash of another server process DETAIL: The postmaster has commanded this server process to roll back the current transaction and exit, because another server process exited abnormally and possibly corrupted shared memory. HINT: In a moment you should be able to reconnect to the database and repeat your command. Failed. ! SE-PostgreSQL intends to acquire them and apply access control policy in this case also. Aside note: sepgsqlCheckDatabaseInstallModule() check permissions on a newly installed C-library file, to prevent to load a file deployed by untrusted client. + case ForeignDataWrapperRelationId: + CHECK_PROC_INSTALL_PERM(pg_foreign_data_wrapper, fdwvalidator, newtup, oldtup); + break; Hmm, calls to fdwvalidator are not at all performance critical, so maybe we should just check execute permission when it's called. If pg_proc_aclcheck() on its invocation, it is not necessary to check on the installation time. [snip] + case OperatorRelationId: + CHECK_PROC_INSTALL_PERM(pg_operator, oprcode, newtup, oldtup); + CHECK_PROC_INSTALL_PERM(pg_operator, oprrest, newtup, oldtup); + CHECK_PROC_INSTALL_PERM(pg_operator, oprjoin, newtup, oldtup); + break; oprcode is checked for execute permission when the operator is used. For oprrest and oprjoin, we could add checks into the planner where they're called. (pg_operator.oprcom and pg_operator.oprnegate are missing?) If runtime checks are added, it is more desirable. + case TSParserRelationId: + CHECK_PROC_INSTALL_PERM(pg_ts_parser, prsstart, newtup, oldtup); + CHECK_PROC_INSTALL_PERM(pg_ts_parser, prstoken, newtup, oldtup); + CHECK_PROC_INSTALL_PERM(pg_ts_parser, prsend, newtup, oldtup); + CHECK_PROC_INSTALL_PERM(pg_ts_parser, prsheadline, newtup, oldtup); + CHECK_PROC_INSTALL_PERM(pg_ts_parser, prslextype, newtup, oldtup); + break; + + case TSTemplateRelationId: + CHECK_PROC_INSTALL_PERM(pg_ts_template, tmplinit, newtup, oldtup); + CHECK_PROC_INSTALL_PERM(pg_ts_template, tmpllexize, newtup, oldtup); + break; Not sure about these. Maybe we should add checks to where these are called. I'll check the behavior of them tomorrow. + case TypeRelationId: + CHECK_PROC_INSTALL_PERM(pg_type, typinput, newtup, oldtup); + CHECK_PROC_INSTALL_PERM(pg_type, typoutput, newtup, oldtup); + CHECK_PROC_INSTALL_PERM(pg_type, typreceive,
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
KaiGai Kohei kai...@kaigai.gr.jp writes: Yes, the purpose of sepgsqlCheckProcedureInstall() is to prevent users to invoke functions installed by other malicious/untrusted one, typically known as trojan-horse. ... We should not assume only C-functions can be installed on pg_conversion (and other internal stuff), because a superuser can update system catalog by hand. ... SE-PostgreSQL intends to acquire them and apply access control policy in this case also. I don't think that anyone except KaiGai-san has bought into the concept that sepostgres should get to override superuser capabilities, much less that it should be trying to control semantics at this kind of level of detail. I've been convinced for awhile that the sepostgres project is going off the rails, and these last couple of exchanges just confirm the fear. This is absolutely *not* the kind of thing that we should be designing four months after feature freeze. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
On Mon, Mar 9, 2009 at 1:25 PM, Tom Lane t...@sss.pgh.pa.us wrote: KaiGai Kohei kai...@kaigai.gr.jp writes: Yes, the purpose of sepgsqlCheckProcedureInstall() is to prevent users to invoke functions installed by other malicious/untrusted one, typically known as trojan-horse. ... We should not assume only C-functions can be installed on pg_conversion (and other internal stuff), because a superuser can update system catalog by hand. ... SE-PostgreSQL intends to acquire them and apply access control policy in this case also. I don't think that anyone except KaiGai-san has bought into the concept that sepostgres should get to override superuser capabilities, much less that it should be trying to control semantics at this kind of level of detail. I'd find that VERY surprising. One of the major features of MAC systems is that the system policy trumps decisions by individual users, so root or the database superuser is confined by that policy just like everyone else. They may or may not have the ability to change the policy, but that's a separate issue. I've been convinced for awhile that the sepostgres project is going off the rails, and these last couple of exchanges just confirm the fear. I'm not sure what you mean by going off the rails. I think we are still beating our way through what Peter Eisentraut said in one of his first reviews of this patch: SE-PostgreSQL shouldn't implement MAC that isn't a mirror of existing DAC capabilities. If more capabilities are needed, the DAC side of things should be designed and implemented first. Interestingly, Heikki's latest review comments are coming back to exactly this point. So I think we have unanimity that everything that doesn't meet this criterion should be ripped out for now. But I don't see anyone arguing that those capabilities are intrinsically worthless, except possibly you, just that we won't be ready to support them in SE-PostgreSQL until we support them in some more general sense. This is absolutely *not* the kind of thing that we should be designing four months after feature freeze. On this point I am in agreement. We need very much to bring this November CommitFest to an end. Unfortunately, the pace of reviewing slowed dramatically after Thanksgiving and has since dropped to a crawl. However, since the decision to bump Hot Standby was made, things have picked up again, mostly due to a bunch of reviewing by Heikki. The thing we need to do now is make that reviewing reach some conclusion about exactly what needs to be fixed and what of it will be fixed by the author vs. by the committer. It would be easier to make the decision to bump SE-PostgreSQL if it were the only thing holding up beta, but we're not there yet. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Robert Haas robertmh...@gmail.com writes: On Mon, Mar 9, 2009 at 1:25 PM, Tom Lane t...@sss.pgh.pa.us wrote: I've been convinced for awhile that the sepostgres project is going off the rails, and these last couple of exchanges just confirm the fear. I'm not sure what you mean by going off the rails. I think we are still beating our way through what Peter Eisentraut said in one of his first reviews of this patch: SE-PostgreSQL shouldn't implement MAC that isn't a mirror of existing DAC capabilities. If more capabilities are needed, the DAC side of things should be designed and implemented first. Interestingly, Heikki's latest review comments are coming back to exactly this point. So I think we have unanimity that everything that doesn't meet this criterion should be ripped out for now. But I don't see anyone arguing that those capabilities are intrinsically worthless, except possibly you, just that we won't be ready to support them in SE-PostgreSQL until we support them in some more general sense. I'm not saying that I think the capability is intrinsically worthless. What I *am* saying is that I have zero confidence in the current development process, ie one guy producing patches without any previous design discussion. What's missing is 1. Community buy-in on the objectives and user-visible semantics. 2. High-level review of the proposed implementation method. 3. Review of the coding details. We seem to be starting at #3. Now it's not really KaiGai-san's fault; the fundamental problem IMHO is that no one else is taking very much interest in the patch. But that in itself speaks volumes about whether we actually want this patch or should accept it. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
On Mon, Mar 9, 2009 at 4:04 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Mon, Mar 9, 2009 at 1:25 PM, Tom Lane t...@sss.pgh.pa.us wrote: I've been convinced for awhile that the sepostgres project is going off the rails, and these last couple of exchanges just confirm the fear. I'm not sure what you mean by going off the rails. I think we are still beating our way through what Peter Eisentraut said in one of his first reviews of this patch: SE-PostgreSQL shouldn't implement MAC that isn't a mirror of existing DAC capabilities. If more capabilities are needed, the DAC side of things should be designed and implemented first. Interestingly, Heikki's latest review comments are coming back to exactly this point. So I think we have unanimity that everything that doesn't meet this criterion should be ripped out for now. But I don't see anyone arguing that those capabilities are intrinsically worthless, except possibly you, just that we won't be ready to support them in SE-PostgreSQL until we support them in some more general sense. I'm not saying that I think the capability is intrinsically worthless. What I *am* saying is that I have zero confidence in the current development process, ie one guy producing patches without any previous design discussion. What's missing is 1. Community buy-in on the objectives and user-visible semantics. 2. High-level review of the proposed implementation method. 3. Review of the coding details. We seem to be starting at #3. OK, I agree. Now it's not really KaiGai-san's fault; the fundamental problem IMHO is that no one else is taking very much interest in the patch. But that in itself speaks volumes about whether we actually want this patch or should accept it. Are you sure that this isn't just because the original patch was so enormous? If you're referring to reviewing, it's certainly easier to find someone willing to review a 100-line patch than it is to find someone willing to review a 10,000-line patch. But in terms of potential user feedback, there have been a number of people writing in about how much they would like to use this feature, and some security folks have written in with positive comments, too. It also seems to me that with Heikki's feedback this is rapidly shrinking down to a project of managable size and scope. I don't think it's there yet, and maybe it won't get there soon enough to include in 8.4, but it certainly seems to be moving in the right direction. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Robert Haas robertmh...@gmail.com writes: On Mon, Mar 9, 2009 at 4:04 PM, Tom Lane t...@sss.pgh.pa.us wrote: Now it's not really KaiGai-san's fault; the fundamental problem IMHO is that no one else is taking very much interest in the patch. But that in itself speaks volumes about whether we actually want this patch or should accept it. Are you sure that this isn't just because the original patch was so enormous? If you're referring to reviewing, it's certainly easier to find someone willing to review a 100-line patch than it is to find someone willing to review a 10,000-line patch. Well, the huge size of the original patch didn't help any, for sure. But the nature of this type of problem --- particularly given the not-designed-for-it architecture we have --- is that there are going to be a lot of subtle issues and very probably a lot of places to touch. It gets even worse if you want to put performance constraints on the result. I will not have any confidence in SEPostgres until both the design and the code details have been reviewed by a fair number of experienced PG hackers; and what I see happening is that there simply aren't enough of them who care. If it were a small localized patch I might not particularly care ... but what I'm afraid of is that we'll have a monstrous patch that does severe damage to readability and modifiability of the backend, and has a bunch of bugs to boot (every one of which will qualify as a security issue when it's discovered). And on top of that, I'm still not sold that there is enough of a user base for it to justify the effort we'll have to put into it. If there were, we'd be seeing more interest in reviewing it. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
On Mon, 2009-03-09 at 16:39 -0400, Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: On Mon, Mar 9, 2009 at 4:04 PM, Tom Lane t...@sss.pgh.pa.us wrote: Now it's not really KaiGai-san's fault; the fundamental problem IMHO is that no one else is taking very much interest in the patch. But that in itself speaks volumes about whether we actually want this patch or should accept it. Are you sure that this isn't just because the original patch was so enormous? If you're referring to reviewing, it's certainly easier to find someone willing to review a 100-line patch than it is to find someone willing to review a 10,000-line patch. Well, the huge size of the original patch didn't help any, for sure. But the nature of this type of problem --- particularly given the not-designed-for-it architecture we have --- is that there are going to be a lot of subtle issues and very probably a lot of places to touch. It gets even worse if you want to put performance constraints on the result. I will not have any confidence in SEPostgres until both the design and the code details have been reviewed by a fair number of experienced PG hackers; and what I see happening is that there simply aren't enough of them who care. If it were a small localized patch I might not particularly care ... but what I'm afraid of is that we'll have a monstrous patch that does severe damage to readability and modifiability of the backend, and has a bunch of bugs to boot (every one of which will qualify as a security issue when it's discovered). And on top of that, I'm still not sold that there is enough of a user base for it to justify the effort we'll have to put into it. If there were, we'd be seeing more interest in reviewing it. Can't it be kept separately maintained release for a version or two, so that we will have both PostgreSQL and SE-PostgreSQL and thus have an easy way to compare both correctness and performance ? Anyone remember how did Linux implement/introduce SE Linux ? -- Hannu Krosing http://www.2ndQuadrant.com PostgreSQL Scalability and Availability Services, Consulting and Training -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Hannu Krosing ha...@2ndquadrant.com writes: Can't it be kept separately maintained release for a version or two, so that we will have both PostgreSQL and SE-PostgreSQL and thus have an easy way to compare both correctness and performance ? Actually, KaiGai-san has been distributing a patched SEPostgres on Fedora for awhile already (and it's been rather a pain in the neck I fear to keep it in sync with the regular distribution). One thing I would love to know is how many people are actually using that, but AFAIK there's no good way to find out. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
On Mon, 2009-03-09 at 19:45 -0400, Tom Lane wrote: Hannu Krosing ha...@2ndquadrant.com writes: Can't it be kept separately maintained release for a version or two, so that we will have both PostgreSQL and SE-PostgreSQL and thus have an easy way to compare both correctness and performance ? Actually, KaiGai-san has been distributing a patched SEPostgres on Fedora for awhile already (and it's been rather a pain in the neck I fear to keep it in sync with the regular distribution). One thing I would love to know is how many people are actually using that, but AFAIK there's no good way to find out. To point out the obvious, we are in a quandary here. Nobody argues the feature would be interesting and although I don't have use for it I could see where some people would. I also see where it would open doors for us. Is there any possibility of having it be enabled at compile time? The default would be know but those distributions that would like to make use of it could? I am actually surprised we are not seeing traction on this from SuSE and Redhat. My understanding is that they are both SE Linux supporters. Joshua D. Drake regards, tom lane -- PostgreSQL - XMPP: jdr...@jabber.postgresql.org Consulting, Development, Support, Training 503-667-4564 - http://www.commandprompt.com/ The PostgreSQL Company, serving since 1997 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Joshua D. Drake j...@commandprompt.com writes: Is there any possibility of having it be enabled at compile time? That's been assumed right along (unless you think it's okay for Postgres to stop working on every non-SELinux platform). The problem here is mostly about whether we have enough confidence in the code to put our project name on it. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
On Mon, 2009-03-09 at 20:05 -0400, Tom Lane wrote: Joshua D. Drake j...@commandprompt.com writes: Is there any possibility of having it be enabled at compile time? That's been assumed right along (unless you think it's okay for Postgres to stop working on every non-SELinux platform). Good point. The problem here is mostly about whether we have enough confidence in the code to put our project name on it. This patch has been bandied about for what, two years? There is a known fork of our project that runs with it. It has a live googlecode site: http://code.google.com/p/sepgsql/ Which has lots of documentation. I also appears to be active within the SE community: http://www.nsa.gov/applications/search/index.cfm?q=se-postgresql It is also part of the Secure OS project out of Japan (as far as I can tell). I know we are a little uncomfortable here but KaiGai-San (forgive me if I type that wrong) has proven to be a contributor in his own right, jumping over every hurdle we have presented him. He is obviously sticking around for a while. If we accept this code, we lose a fork of our project (good) and we pull those people into our project (better) and hopefully they will help us mature the project over time (best). Sincerely, Joshua D. Drake regards, tom lane -- PostgreSQL - XMPP: jdr...@jabber.postgresql.org Consulting, Development, Support, Training 503-667-4564 - http://www.commandprompt.com/ The PostgreSQL Company, serving since 1997 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Joshua D. Drake wrote: http://www.nsa.gov/applications/search/index.cfm?q=se-postgresql It is also part of the Secure OS project out of Japan (as far as I can tell). I know we are a little uncomfortable here but KaiGai-San (forgive me if I type that wrong) has proven to be a contributor in his own right, jumping over every hurdle we have presented him. He is obviously sticking around for a while. KaiGai-San also submitted a patch for an unrelated bug today. :-) -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Hannu Krosing wrote: Can't it be kept separately maintained release for a version or two, so that we will have both PostgreSQL and SE-PostgreSQL and thus have an easy way to compare both correctness and performance ? Anyone remember how did Linux implement/introduce SE Linux ? SELinux has been distributed as a part of mainlined Linux 2.6.x series for the recent five years. It can be enabled/disabled on both of compile time and system bootup time, by user's preference. SELinux is implemented as a security module on the LSM framework which provides a set of neutral hooks for any other stuffs. SE-PostgreSQL also had a similar stuff named as PGACE, but I agreed an opinion that we (pgsql-hackers) don't want in-code framework two months ago, so the PGACE has gone now. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
On Mon, 2009-03-09 at 20:31 -0400, Bruce Momjian wrote: Joshua D. Drake wrote: http://www.nsa.gov/applications/search/index.cfm?q=se-postgresql It is also part of the Secure OS project out of Japan (as far as I can tell). I know we are a little uncomfortable here but KaiGai-San (forgive me if I type that wrong) has proven to be a contributor in his own right, jumping over every hurdle we have presented him. He is obviously sticking around for a while. KaiGai-San also submitted a patch for an unrelated bug today. :-) I also found some completely external references to it: http://lwn.net/Articles/242087/ http://itknowledgeexchange.techtarget.com/enterprise-linux/se-postgres-tightens-sql-security/ Sincerely, Joshua D. Drake -- PostgreSQL - XMPP: jdr...@jabber.postgresql.org Consulting, Development, Support, Training 503-667-4564 - http://www.commandprompt.com/ The PostgreSQL Company, serving since 1997 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Joshua D. Drake j...@commandprompt.com writes: I know we are a little uncomfortable here but KaiGai-San (forgive me if I type that wrong) has proven to be a contributor in his own right, Not to put too fine a point on it, but: no, he hasn't. Show me one significant patch he's contributed before/beside this one. The only thing I see in the CVS logs is that he helped Stephen Frost with column privileges; I don't recall who did how much, but in any case that patch still needed nontrivial fixes when it got to me. Frankly, what we have here is a large patch, with insanely difficult correctness requirements, written by a Postgres newbie. If it doesn't scare you, you haven't been paying attention. We have a long track record of problems with patches written by people who thought they were ready to do major backend hacking without having bitten off some smaller chunks first. Perhaps it would help you calibrate the problem if I stated that I wouldn't trust a patch for this purpose written by myself, let alone somebody who hasn't been hacking the backend for ten years. (Where this purpose means the type of control KaiGai-san seems to hope to enforce, as opposed to just plugging some additional constraints into the existing ACL-check routines.) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Joshua D. Drake wrote: On Mon, 2009-03-09 at 19:45 -0400, Tom Lane wrote: Hannu Krosing ha...@2ndquadrant.com writes: Can't it be kept separately maintained release for a version or two, so that we will have both PostgreSQL and SE-PostgreSQL and thus have an easy way to compare both correctness and performance ? Actually, KaiGai-san has been distributing a patched SEPostgres on Fedora for awhile already (and it's been rather a pain in the neck I fear to keep it in sync with the regular distribution). One thing I would love to know is how many people are actually using that, but AFAIK there's no good way to find out. To point out the obvious, we are in a quandary here. Nobody argues the feature would be interesting and although I don't have use for it I could see where some people would. I also see where it would open doors for us. Is there any possibility of having it be enabled at compile time? The default would be know but those distributions that would like to make use of it could? It was the design a half year ago, but Bruce suggested me a certain feature should not be enabled/disabled by compile time options, except for library/platform dependency. In addition, he also suggested a feature should be turned on/off by configuration option, because of it enables to distribute a single binary for more wider users. SE-PostgreSQL need the libselinux to communicate the in-kernel SELinux. So, --enable-selinux is necessary on compile time, it is fair enough. If we omit it, all the sepgsql() invocations are replaced by empty macros. If we compile it with --enable-selinux, it has two working modes controled by a guc option: sepostgresql (bool). If it is disabled, all the sepgsql() invocations returns at the head of themself without doing anything. I believe this behavior follows the previous suggestion. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Perhaps it would help you calibrate the problem if I stated that I wouldn't trust a patch for this purpose written by myself, let alone somebody who hasn't been hacking the backend for ten years. (Where this purpose means the type of control KaiGai-san seems to hope to enforce, as opposed to just plugging some additional constraints into the existing ACL-check routines.) It seems to me this comment is a bit emotional... :( If we need ten more years of experimence before proposing a new security feature, all the new developers (outcome from other community) need to wait for the v8.14 (not 8.1.4) development cycle opened at 2019(?). I would like folks to review what the patch tries to do, not who submitted the patch. (And, I also would like experts to help/suggest this development.) Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
On Mon, Mar 9, 2009 at 1:52 AM, KaiGai Kohei kai...@ak.jp.nec.com wrote: As I promised last week, SE-PostgreSQL patches are revised here: [1/5] http://sepgsql.googlecode.com/files/sepgsql-core-8.4devel-r1704.patch [2/5] http://sepgsql.googlecode.com/files/sepgsql-utils-8.4devel-r1704.patch [3/5] http://sepgsql.googlecode.com/files/sepgsql-policy-8.4devel-r1704.patch [4/5] http://sepgsql.googlecode.com/files/sepgsql-docs-8.4devel-r1704.patch [5/5] http://sepgsql.googlecode.com/files/sepgsql-tests-8.4devel-r1704.patch has anyone noted that the links are malformed? in my browser they include the [x/5 part of the next line anyway, i have given up trying to test the functional parts of the patch (my knowledge of selinux is almost zero and is a lot of info just to understand the basics... i'm still on that but don't think will get anything for 8.4... if someone can provide some simple info on that will be great) but now i'm trying the performance impacts of it... what seems interesting is that on some queries are some little gain with the patch applied... that seems interesting 'cause i thought it will be the opposite... i want to try to isolate where is the difference... can someone explain me how can i trace that? (sorry for my ignorance but if i don't ask that ignorance will stay) -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Jaime Casanova wrote:
On Mon, Mar 9, 2009 at 1:52 AM, KaiGai Kohei kai...@ak.jp.nec.com wrote:
As I promised last week, SE-PostgreSQL patches are revised here:
[1/5] http://sepgsql.googlecode.com/files/sepgsql-core-8.4devel-r1704.patch
[2/5] http://sepgsql.googlecode.com/files/sepgsql-utils-8.4devel-r1704.patch
[3/5] http://sepgsql.googlecode.com/files/sepgsql-policy-8.4devel-r1704.patch
[4/5] http://sepgsql.googlecode.com/files/sepgsql-docs-8.4devel-r1704.patch
[5/5] http://sepgsql.googlecode.com/files/sepgsql-tests-8.4devel-r1704.patch
has anyone noted that the links are malformed? in my browser they
include the [x/5 part of the next line
Above URLs might be a bit long.
I'll omit the [x/5] part on the next submission.
i want to try to isolate where is the difference... can someone
explain me how can i trace that? (sorry for my ignorance but if i
don't ask that ignorance will stay)
The sepgsql_enable_auditallow system boolean will help you to
understand what permissions are checked on the given query.
-
% make -C src/backend/security/sepgsql/policy
# su
# semodule -i src/backend/security/sepgsql/policy/sepostgresql-devel.pp
(installation of development purpose policy)
# setsebool sepgsql_enable_auditallow 1
% psql postgres
NOTICE: SELinux: granted { access }
scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c63
tcontext=unconfined_u:object_r:sepgsql_db_t:s0 tclass=db_database name=postgres
psql (8.4devel)
Type help for help.
postgres=# SELECT * FROM t1;
NOTICE: SELinux: granted { select }
scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c63
tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_table name=t1
NOTICE: SELinux: granted { select }
scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c63
tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_column name=t1.a
NOTICE: SELinux: granted { select }
scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c63
tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_column name=t1.b
NOTICE: SELinux: granted { select }
scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c63
tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_column name=t1.c
a | b | c
---+---+---
(0 rows)
postgres=# INSERT INTO t1 (a,c) VALUES (1,2);
NOTICE: SELinux: granted { insert }
scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c63
tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_table name=t1
NOTICE: SELinux: granted { insert }
scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c63
tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_column name=t1.a
NOTICE: SELinux: granted { insert }
scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c63
tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_column name=t1.c
INSERT 0 1
postgres=#
-
The meanings of each fields:
- The scontext is the client's privileges
- The tcontext is the security context of tables, columns and so on.
- The tclass shows the kind of target object.
- The name is the name of object.
I recommend you to turn off it in normal case due to noisy and disk
consumption with logs.
Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei kai...@ak.jp.nec.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Tom, Frankly, what we have here is a large patch, with insanely difficult correctness requirements, written by a Postgres newbie. If it doesn't scare you, you haven't been paying attention. We have a long track record of problems with patches written by people who thought they were ready to do major backend hacking without having bitten off some smaller chunks first. If that was the case, why didn't you say it 4 months ago? It seems rather unfair to Kaigai and everyone else who worked on it to be getting cold feet about the entire concept after several months of debugging. --Josh Berkus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Josh Berkus j...@agliodbs.com writes: Frankly, what we have here is a large patch, with insanely difficult correctness requirements, written by a Postgres newbie. If it doesn't scare you, you haven't been paying attention. We have a long track record of problems with patches written by people who thought they were ready to do major backend hacking without having bitten off some smaller chunks first. If that was the case, why didn't you say it 4 months ago? It seems rather unfair to Kaigai and everyone else who worked on it to be getting cold feet about the entire concept after several months of debugging. Josh, I've had cold feet about this patch from day one, and have not been very shy about expressing it, for instance here http://archives.postgresql.org/pgsql-hackers/2008-05/msg00122.php here http://archives.postgresql.org//pgsql-hackers/2008-09/msg01662.php and here http://archives.postgresql.org//pgsql-hackers/2009-01/msg01928.php (those are just samples from long threads in each case). Despite all that arm-waving, no one besides KaiGai-san has really stepped up to work on it. That leaves me not only with worries about the patch itself, but with an extremely low estimate of the community's interest in it. And it is too big, complicated, and risky to go in if there's not strong community support for it. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Tom Lane wrote: Despite all that arm-waving, no one besides KaiGai-san has really stepped up to work on it. That leaves me not only with worries about the patch itself, but with an extremely low estimate of the community's interest in it. And it is too big, complicated, and risky to go in if there's not strong community support for it. The only reason why I separated a few major facilities (writable system columns, row-level controls, largeobject permission and so on) and reduces the scale of patches as someone required is to help SE-PostgreSQL getting merged into the v8.4. In addition, as I said before, I can provide my efforts to maintenance SE-PostgreSQL feature to the future version, once it getting merged, no need to say. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1704)
Heikki Linnakangas wrote:
+ void
+ sepgsqlCheckProcedureInstall(Relation rel, HeapTuple newtup,
HeapTuple oldtup)
+ {
[snip]
+ + case AccessMethodRelationId:
+ CHECK_PROC_INSTALL_PERM(pg_am, aminsert, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, ambeginscan, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, amgettuple, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, amgetbitmap, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, amrescan, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, amendscan, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, ammarkpos, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, amrestrpos, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, ambuild, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, ambulkdelete, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, amvacuumcleanup, newtup,
oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, amcostestimate, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_am, amoptions, newtup, oldtup);
+ break;
ISTM that you should just forbid any changes to pg_am in the default
policy. That's very low level stuff. If you can modify that, you can
wreck a lot of havoc, quite possibly turning it into a vulnerability
even if you can't directly install a malicious function there.
Heikki,
My opinion is still we should check db_procedure:{install} privilege
for functions expected to be implemented by C-language.
In the basis of security, it requires security facilities should
improve confidentiality, integrity and availability in the assumption
and environment required by the facility.
For example, existing database ACL improves confidentiality and
integrity with applying DAC policy, and improves availability to
prevent to load C-library by users except for superuser.
(Here, the assumption is that database superuser is trusted.)
If we write a oid of SQL-function onto pg_am.aminsert, it will
not work correctly independent from existence of maliciou code,
but it also enables to crash the backend immediately.
It can be a damage to the availability of the backend, so I still
think we need to check this permission for functions expected to
be implemented by C-language.
NOTE: when we create a new C-function or replace its definition,
sepgsqlCheckDatabaseInstallModule() checks whether the given
loadable library file has appropriate security context, or not.
In the default security policy, only files labeled as lib_t
are allowed to load.
+ case AccessMethodProcedureRelationId:
+ CHECK_PROC_INSTALL_PERM(pg_amproc, amproc, newtup, oldtup);
+ break;
+ + case CastRelationId:
+ CHECK_PROC_INSTALL_PERM(pg_cast, castfunc, newtup, oldtup);
+ break;
We check execute permission on the cast function at runtime.
We have a corner case.
The ri_HashCompareOp() in ri_triggers.c can invokes castfunc without
runtime checks, if I can understand the implementation correctly.
Indeed, most cases invokes pg_proc_aclcheck() and SE-PostgreSQL also
checks db_procedure:{execute} permission in runtime.
This design requires either of install or execute should be checked
at least, so double checks are not matter.
[snip]
+ case ForeignDataWrapperRelationId:
+ CHECK_PROC_INSTALL_PERM(pg_foreign_data_wrapper,
fdwvalidator, newtup, oldtup);
+ break;
Hmm, calls to fdwvalidator are not at all performance critical, so maybe
we should just check execute permission when it's called.
If pg_proc_aclcheck() is added on the invocation of fdwvalidator,
I'll remove install checks on it from here.
[snip]
+ case OperatorRelationId:
+ CHECK_PROC_INSTALL_PERM(pg_operator, oprcode, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_operator, oprrest, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_operator, oprjoin, newtup, oldtup);
+ break;
oprcode is checked for execute permission when the operator is used. For
oprrest and oprjoin, we could add checks into the planner where they're
called. (pg_operator.oprcom and pg_operator.oprnegate are missing?)
For example, ExecInitAgg() set up opcode function as follows:
fmgr_info(get_opcode(eq_opr), (peraggstate-equalfn));
and it can be invoked later without checks.
I think it is a case to be checked. Indeed, pg_operator.oprcom and
pg_operator.oprnegate were missed. Thanks for your pointed out.
+ case TSParserRelationId:
+ CHECK_PROC_INSTALL_PERM(pg_ts_parser, prsstart, newtup,
oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_ts_parser, prstoken, newtup,
oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_ts_parser, prsend, newtup, oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_ts_parser, prsheadline, newtup,
oldtup);
+ CHECK_PROC_INSTALL_PERM(pg_ts_parser, prslextype, newtup,
oldtup);
+ break;
+ + case TSTemplateRelationId:
+ CHECK_PROC_INSTALL_PERM(pg_ts_template, tmplinit,
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1668)
KaiGai Kohei wrote: One matter was use permission, but I can agree to integrate it into select permission as the original design did. Ok, great. The other is view. When we use a view in the query, it is extracted as a subquery and its query tree is fetched from pg_rewrite.ev_action which is already parsed. It means we need to ensure the parsed representation is not manipulated. The simplest solution is to prevent updating the pg_rewrite.ev_action by hand when SE-PostgreSQL is enabled. Agreed. If SE-PostgreSQL is enabled, you need to forbid manual updates to a lot of catalog tables. This is just another case of the same. I think smaller hard-wired rules are better, but it is a very corner-case and its benefit cannot be ignorable. - It enables to reduce the walker code from sepgsql/checker.c. (I guess it makes reduce a few hundreds lines.) - It helps to maintain code to pick up what tables/columns are accessed. If nobody disagree it, I'll integrate use permission into select and remove the walker code from sepgsql/checker.c due to the next Monday. It affects on sepgsql/checker.c, but I expect little changes on others. I'm happy, if you don't stop reviewing patches except for checker.c. Sounds good, though I'm not 100% sure I understood what you're going to replace the walker with. Seeing the patch will surely enlighten that :-). -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1668)
Heikki Linnakangas wrote:
KaiGai Kohei wrote:
The other one is it has two kind of reader permissions (select and
use).
The select permission is applied when user tries to read tables/columns
and its contents are returned to the user.
The use permission is applied when user tries to read table/columns,
but its contents are consumed internally (not returned to user directly).
For example:
SELECT a, b FROM t WHERE b 10 and c = 'aaa';
In this case,
db_column:{select} permission is applied on t.a.
db_column:{select use} permission is applied on t.b.
db_column:{use} permission is applied on t.c.
db_table:{select use} permission is applied on t
However, I don't absolutely oppose to integrate them into a single
reader select permission, because it was originally a single
permission, then use is added.
If you have use permisson on c, you can easily use it to find out the
exact value. Just do queries like SELECT 'foo' FROM t WHERE b 10 AND
c = 'aaa' AND c BETWEEN 1 AND 1000 repeatedly with different ranges to
zoom into the exact value. So I think separating those two permissions
is a mistake,
Hmm.
At least, I can agree to integrate these two permissions into a single
select. It is originally upper compatible to use.
Please note that user's privileges are not limited to create/alter/drop
them. One sensitive permission is db_procedure:{install}.
It is checked when user defined functions are set up as a function
internally invoked.
For example, pg_conversion.conproc is internally invoked to translate
a text, but it does not check pg_proc_aclcheck() in runtime.
We consider all user defined functions should be checked either of:
- db_procedure:{execute} permission for the client in runtime
or
- db_procedure:{install} permission for the DBA on installation time
Needless to say, {install} is more sensitive permission because it
means anyones to invoke it implicitly. So, the default policy only
allows it on functions defined by DBA, but the execute permission
is allowed normal users to invoke functions defined by himself.
Hmm. We normally rely on the fact that a conversion function needs to be
a C-function, and because only superusers can create C-functions we have
assumed that they're safe to call. Which was actually not true until
recently, when we added checks into all the conversion functions to
check that the source and target encoding of the strings passed as
arguments match the ones specified in the CREATE CONVERSION command.
There has been talks of making CREATE CONVERSION superuser-only, so we
could easily just do that. Can you give some other examples of where the
install permission is used?
Because SE-PostgreSQL works orthogonally to the existing access controls,
so we need to consider two cases.
a) A superuser connected from unprivileged domain (like: user_t, httpd_t)
b) A superuser connected from privileged domain (like: sysadm_t, unconfined_t)
The superuser is a concept of PostgreSQL, and the domain is a concept
of SELinux. It seems to me you assumes the b) case.
The a) case should be focused on here.
In the a) case, SE-PostgreSQL does not prevent to create C-function
(as far as shared library has an appropriate label: lib_t), and newly
created functions are labeled as unpriv-user defined functions which
depends on the domain.
It allows unpriv-domains to invoke functions defined by himself,
but does not allow to install as a conversion and others, because
it can be implicitly used by other domains.
NOTE: unprivileged domain cannot create files labeled as lib_t on the
operating system, so all they can do is to load libraries already
set up.
Our assumption is a client connected from user_t or httpd_t is suspicious,
even if they logged in as a superuser, so SE-PostgreSQL applies additional
checks.
But if I've understood correctly, one goal is to restrict the actions of
superusers as well. Is there something to disallow superusers from
creating C-functions? If yes, isn't that enough protection from things
like the conversion functions?
Please note that SE-PostgreSQL focuses on the domain a client connected
from, not attributes of database user.
(Needless to say, vanilla PostgreSQL concurrently prevents C-functions
by normal database users.)
The conversion is an example of install permissions.
The list of functions to be checked are here.
http://code.google.com/p/sepgsql/source/browse/trunk/sepgsql/src/backend/security/sepgsql/hooks.c#446
Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei kai...@ak.jp.nec.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1668)
KaiGai Kohei wrote: Heikki, Thanks for your comments. Heikki Linnakangas wrote: Ok, I've taken a quick look at this too. My first impression is that this is actually not a very big patch. Much much smaller than I was afraid of. It seems that dropping the row-level security and the other change you've already done have helped a great deal. My first question is, why does the patch need the walker implementation to gather all the accessed tables and columns? Can't you hook into the usual pg_xxx_aclcheck() functions? In fact, Peter asked that same question here: http://archives.postgresql.org/pgsql-hackers/2009-01/msg02295.php (among other things). Many things have changed since, but I don't think that question has been adequately answered. Different handling of permissions on views was mentioned, but I think that could be handled with just a few extra checks in the rewriter or executor. Yes, one major reason is to handle views. SE-PostgreSQL need to check permissions on after it is extracted. : I'll check some of corner cases, such as inherited tables, COPY statement, trigger invocations and others, to consider whether your suggestion is possible, or not. Please wait for a while to fix my attitude. Heikki, I now feel tempted by an idea to utilize the facilities of table/column-level privileges. One matter was use permission, but I can agree to integrate it into select permission as the original design did. The other is view. When we use a view in the query, it is extracted as a subquery and its query tree is fetched from pg_rewrite.ev_action which is already parsed. It means we need to ensure the parsed representation is not manipulated. The simplest solution is to prevent updating the pg_rewrite.ev_action by hand when SE-PostgreSQL is enabled. I think smaller hard-wired rules are better, but it is a very corner-case and its benefit cannot be ignorable. - It enables to reduce the walker code from sepgsql/checker.c. (I guess it makes reduce a few hundreds lines.) - It helps to maintain code to pick up what tables/columns are accessed. If nobody disagree it, I'll integrate use permission into select and remove the walker code from sepgsql/checker.c due to the next Monday. It affects on sepgsql/checker.c, but I expect little changes on others. I'm happy, if you don't stop reviewing patches except for checker.c. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1668)
Ok, I've taken a quick look at this too. My first impression is that this is actually not a very big patch. Much much smaller than I was afraid of. It seems that dropping the row-level security and the other change you've already done have helped a great deal. My first question is, why does the patch need the walker implementation to gather all the accessed tables and columns? Can't you hook into the usual pg_xxx_aclcheck() functions? In fact, Peter asked that same question here: http://archives.postgresql.org/pgsql-hackers/2009-01/msg02295.php (among other things). Many things have changed since, but I don't think that question has been adequately answered. Different handling of permissions on views was mentioned, but I think that could be handled with just a few extra checks in the rewriter or executor. The hooks in simple_heap_insert also seem a bit weird. Perhaps an artifact of the row-level security stuff that's no longer there. ISTM that setting the defaults should be done in the same places where the defaults for acl columns are filled, e.g in ProcedureCreate. PS. s/proselabal/proselabel -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1668)
Heikki, Thanks for your comments.
Heikki Linnakangas wrote:
Ok, I've taken a quick look at this too. My first impression is that
this is actually not a very big patch. Much much smaller than I was
afraid of. It seems that dropping the row-level security and the other
change you've already done have helped a great deal.
My first question is, why does the patch need the walker implementation
to gather all the accessed tables and columns? Can't you hook into the
usual pg_xxx_aclcheck() functions? In fact, Peter asked that same
question here:
http://archives.postgresql.org/pgsql-hackers/2009-01/msg02295.php (among
other things). Many things have changed since, but I don't think that
question has been adequately answered. Different handling of permissions
on views was mentioned, but I think that could be handled with just a
few extra checks in the rewriter or executor.
Yes, one major reason is to handle views. SE-PostgreSQL need to check
permissions on after it is extracted.
The other one is it has two kind of reader permissions (select and use).
The select permission is applied when user tries to read tables/columns
and its contents are returned to the user.
The use permission is applied when user tries to read table/columns,
but its contents are consumed internally (not returned to user directly).
For example:
SELECT a, b FROM t WHERE b 10 and c = 'aaa';
In this case,
db_column:{select} permission is applied on t.a.
db_column:{select use} permission is applied on t.b.
db_column:{use} permission is applied on t.c.
db_table:{select use} permission is applied on t
However, I don't absolutely oppose to integrate them into a single
reader select permission, because it was originally a single
permission, then use is added.
The purpose of use permission is to set up a writable table, but
not readable. When we use UPDATE or DELETE statement, it need to
add WHERE clause to make clear its target, but it always requires
reader permission. So, I separated it into two cases.
Thus, it is not a reason as strong as one for views.
I'll check some of corner cases, such as inherited tables, COPY
statement, trigger invocations and others, to consider whether
your suggestion is possible, or not.
Please wait for a while to fix my attitude.
The hooks in simple_heap_insert also seem a bit weird. Perhaps an
artifact of the row-level security stuff that's no longer there. ISTM
that setting the defaults should be done in the same places where the
defaults for acl columns are filled, e.g in ProcedureCreate.
Its purpose is not set a default security label in the current version.
(It is set in ProcedureCreate and others.)
Its purpose is to check user's privileges on tables, columns, procedures
and databases, and raises an error if violated.
Please note that user's privileges are not limited to create/alter/drop
them. One sensitive permission is db_procedure:{install}.
It is checked when user defined functions are set up as a function
internally invoked.
For example, pg_conversion.conproc is internally invoked to translate
a text, but it does not check pg_proc_aclcheck() in runtime.
We consider all user defined functions should be checked either of:
- db_procedure:{execute} permission for the client in runtime
or
- db_procedure:{install} permission for the DBA on installation time
Needless to say, {install} is more sensitive permission because it
means anyones to invoke it implicitly. So, the default policy only
allows it on functions defined by DBA, but the execute permission
is allowed normal users to invoke functions defined by himself.
sepgsqlCheckProcedureInstall() checks this permission called from
the hooks of simple_heap_insert()/_update().
From the viewpoint of security, it is good design to put hooks on
more frequently used point, than checking it many points.
It is same reason why SELinux checks system-calls from applications.
It is the only path to access resources managed by operating system,
so necessary and sufficient.
PS. s/proselabal/proselabel
Oops,
Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei kai...@ak.jp.nec.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1668)
KaiGai Kohei wrote:
The other one is it has two kind of reader permissions (select and
use).
The select permission is applied when user tries to read tables/columns
and its contents are returned to the user.
The use permission is applied when user tries to read table/columns,
but its contents are consumed internally (not returned to user directly).
For example:
SELECT a, b FROM t WHERE b 10 and c = 'aaa';
In this case,
db_column:{select} permission is applied on t.a.
db_column:{select use} permission is applied on t.b.
db_column:{use} permission is applied on t.c.
db_table:{select use} permission is applied on t
However, I don't absolutely oppose to integrate them into a single
reader select permission, because it was originally a single
permission, then use is added.
If you have use permisson on c, you can easily use it to find out the
exact value. Just do queries like SELECT 'foo' FROM t WHERE b 10 AND
c = 'aaa' AND c BETWEEN 1 AND 1000 repeatedly with different ranges to
zoom into the exact value. So I think separating those two permissions
is a mistake,
Please note that user's privileges are not limited to create/alter/drop
them. One sensitive permission is db_procedure:{install}.
It is checked when user defined functions are set up as a function
internally invoked.
For example, pg_conversion.conproc is internally invoked to translate
a text, but it does not check pg_proc_aclcheck() in runtime.
We consider all user defined functions should be checked either of:
- db_procedure:{execute} permission for the client in runtime
or
- db_procedure:{install} permission for the DBA on installation time
Needless to say, {install} is more sensitive permission because it
means anyones to invoke it implicitly. So, the default policy only
allows it on functions defined by DBA, but the execute permission
is allowed normal users to invoke functions defined by himself.
Hmm. We normally rely on the fact that a conversion function needs to be
a C-function, and because only superusers can create C-functions we have
assumed that they're safe to call. Which was actually not true until
recently, when we added checks into all the conversion functions to
check that the source and target encoding of the strings passed as
arguments match the ones specified in the CREATE CONVERSION command.
There has been talks of making CREATE CONVERSION superuser-only, so we
could easily just do that. Can you give some other examples of where the
install permission is used?
But if I've understood correctly, one goal is to restrict the actions of
superusers as well. Is there something to disallow superusers from
creating C-functions? If yes, isn't that enough protection from things
like the conversion functions?
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1668)
The series of SE-PostgreSQL patches for v8.4 were updated: [1/5] http://sepgsql.googlecode.com/files/sepgsql-core-8.4devel-r1668.patch [2/5] http://sepgsql.googlecode.com/files/sepgsql-utils-8.4devel-r1668.patch [3/5] http://sepgsql.googlecode.com/files/sepgsql-policy-8.4devel-r1668.patch [4/5] http://sepgsql.googlecode.com/files/sepgsql-docs-8.4devel-r1668.patch [5/5] http://sepgsql.googlecode.com/files/sepgsql-tests-8.4devel-r1668.patch - List of updates: * It is rebased to the latest CVS HEAD. * sepgsqlCheckProcedureInstall() is moved to sepgsql/hooks.c from sepgsql/perms.c, like as other sepgsqlCheck() is delopyed on. * sepgsqlCheckDatabaseAccess() is moved to pg_database_aclcheck() from pg_database_aclmask(), because pg_aclmask() can be invoked on ExecGrant_, but SE-PostgreSQL should not intervene existing DAC policy. * sepgsqlCheckProcedureExecute() is moved to pg_proc_aclcheck() in same reason. These changes are obvious and minor, and rest of implementations keep unchanged, so don't consider this updates needs to review whole of patches again, please. I would like to know the current status of reviewing the patches. It is welcome, if it is not still 100% completed and partial ones. And, please tell me, if I missed anything. Anyway, it is obvious we don't have enough time! Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1530)
On Sat, Feb 14, 2009 at 12:46 AM, KaiGai Kohei kai...@kaigai.gr.jp wrote: Jaime Casanova wrote: i only have ubuntu at hand, and i can install selinux from repositories... do you see any problem with that? i'm creating the VM now... will test on weekend... Now I checked the repository of Ubuntu, however, I can find several matters to build/run SE-PostgreSQL. - The default security policy is quite old. Is does not contain SE-PostgreSQL support which is merged at the refpolicy-20080702. - The libselinux is a bit old. It does not contains several symbols required by SE-PostgreSQL, so it is not possible to compile. If you set up a VM from scratch, Fedora enables to install via remote repository using minimum bootable image. http://download.fedora.redhat.com/pub/fedora/linux/releases/10/Fedora/i386/iso/ at last i managed to install fedora 10 and it seems like it have selinux installed (both locate selinux and locate libselinux gives results). executing: yum install libselinux gives Package libselinux-2.0.73-1.fc10.i386 already installed and latest version nevertheless, when i run: ./configure --prefix=/usr/local/pgsql/8.4.se --enable-cassert --enable-debug --enable-depend --enable-selinux i get this message: checking for getpeercon in -lselinux... no configure: error: --enable-selinux requires libselinux. attached config.log -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 config.log.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1530)
Jaime Casanova jcasa...@systemguards.com.ec writes: executing: yum install libselinux gives Package libselinux-2.0.73-1.fc10.i386 already installed and latest version Yeah, but have you got libselinux-devel? A general rule on Red Hat based systems is that compiling a program that depends on library package foo will require foo-devel to be installed too. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1530)
On Fri, Feb 27, 2009 at 5:06 PM, Tom Lane t...@sss.pgh.pa.us wrote: Jaime Casanova jcasa...@systemguards.com.ec writes: executing: yum install libselinux gives Package libselinux-2.0.73-1.fc10.i386 already installed and latest version Yeah, but have you got libselinux-devel? A general rule on Red Hat based systems is that compiling a program that depends on library package foo will require foo-devel to be installed too. ah! ok, i tried libselinux-dev and nothing was found (i'm more familiar with the debian naming convention ;) -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1608)
KaiGai Kohei wrote: The series of SE-PostgreSQL patches for v8.4 were updated: [1/5] http://sepgsql.googlecode.com/files/sepgsql-core-8.4devel-r1608.patch [2/5] http://sepgsql.googlecode.com/files/sepgsql-utils-8.4devel-r1608.patch [3/5] http://sepgsql.googlecode.com/files/sepgsql-policy-8.4devel-r1608.patch [4/5] http://sepgsql.googlecode.com/files/sepgsql-docs-8.4devel-r1608.patch [5/5] http://sepgsql.googlecode.com/files/sepgsql-tests-8.4devel-r1608.patch - List of updates: * bugfix: sepgsqlCheckProcedureEntrypoint() was invoked twice when security invoker functions are invoked. Rest of parts are unchanged. Don't mind contracted filename. Please comment anything. It will help to improve our code. I did an analysis of the core file: http://sepgsql.googlecode.com/files/sepgsql-core-8.4devel-r1608.patch changed lines 3226 new files 4075 syscatalog 9977 total 17278 The good news is that 3226 is the affect on the non-system-catalog main core code, and is a context diff size, not total changed lines. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1608)
Bruce Momjian wrote: KaiGai Kohei wrote: The series of SE-PostgreSQL patches for v8.4 were updated: [1/5] http://sepgsql.googlecode.com/files/sepgsql-core-8.4devel-r1608.patch [2/5] http://sepgsql.googlecode.com/files/sepgsql-utils-8.4devel-r1608.patch [3/5] http://sepgsql.googlecode.com/files/sepgsql-policy-8.4devel-r1608.patch [4/5] http://sepgsql.googlecode.com/files/sepgsql-docs-8.4devel-r1608.patch [5/5] http://sepgsql.googlecode.com/files/sepgsql-tests-8.4devel-r1608.patch - List of updates: * bugfix: sepgsqlCheckProcedureEntrypoint() was invoked twice when security invoker functions are invoked. Rest of parts are unchanged. Don't mind contracted filename. Please comment anything. It will help to improve our code. I did an analysis of the core file: http://sepgsql.googlecode.com/files/sepgsql-core-8.4devel-r1608.patch changed lines 3226 new files 4075 syscatalog 9977 total 17278 The good news is that 3226 is the affect on the non-system-catalog main core code, and is a context diff size, not total changed lines. Hum...? What utility did you use to compute the lines? It seems to me the changed lines except for system catalogs are larger than actual one. The diffstat says: 65 files changed, 4769 insertions(+), 11 deletions(-), 4945 modifications(!) The (4244 + 500) of 4945 modifications come from pg_proc.h and pg_attribute.h due to a new field to store security label of procedures and columns. The new files adds 4014 in total, so rest of (755 + 11 + 201 = 967) lines are estimated changes in the main core code. Anyway, I believe the burden of reviewer became smaller than the prior full-set version. Thanks, - [kai...@masu ~]$ diffstat ~/sepgsql-core-8.4devel-r1608.patch configure | 113 configure.in | 13 src/Makefile.global.in|1 src/backend/Makefile |7 src/backend/access/heap/heapam.c | 12 src/backend/bootstrap/bootparse.y |4 src/backend/bootstrap/bootstrap.c |3 src/backend/catalog/aclchk.c | 11 src/backend/catalog/heap.c| 94 src/backend/catalog/index.c |8 src/backend/catalog/pg_aggregate.c|3 src/backend/catalog/pg_proc.c |9 src/backend/catalog/toasting.c|3 src/backend/commands/cluster.c|4 src/backend/commands/copy.c |9 src/backend/commands/dbcommands.c | 33 src/backend/commands/foreigncmds.c|7 src/backend/commands/functioncmds.c | 77 src/backend/commands/lockcmds.c |4 src/backend/commands/proclang.c |6 src/backend/commands/tablecmds.c | 99 src/backend/commands/trigger.c|6 src/backend/executor/execMain.c | 22 src/backend/nodes/copyfuncs.c | 25 src/backend/nodes/equalfuncs.c| 21 src/backend/nodes/outfuncs.c | 28 src/backend/nodes/readfuncs.c | 41 src/backend/optimizer/plan/planner.c |1 src/backend/parser/gram.y | 63 src/backend/postmaster/postmaster.c | 43 src/backend/rewrite/rewriteHandler.c |6 src/backend/security/Makefile | 11 src/backend/security/sepgsql/Makefile | 16 src/backend/security/sepgsql/avc.c| 1157 +++ src/backend/security/sepgsql/checker.c| 902 + src/backend/security/sepgsql/core.c | 235 + src/backend/security/sepgsql/dummy.c | 37 src/backend/security/sepgsql/hooks.c | 576 +++ src/backend/security/sepgsql/label.c | 360 ++ src/backend/security/sepgsql/perms.c | 463 ++ src/backend/storage/ipc/ipci.c|2 src/backend/tcop/utility.c|5 src/backend/utils/cache/catcache.c| 32 src/backend/utils/cache/syscache.c| 15 src/backend/utils/fmgr/dfmgr.c| 10 src/backend/utils/fmgr/fmgr.c |8 src/backend/utils/init/postinit.c | 11 src/backend/utils/misc/guc.c | 18 src/backend/utils/misc/postgresql.conf.sample |3 src/include/catalog/heap.h|9 src/include/catalog/pg_attribute.h| 500 !!! src/include/catalog/pg_class.h| 12 src/include/catalog/pg_database.h |6 src/include/catalog/pg_proc.h | 4244 !! src/include/catalog/pg_proc_fn.h |3 src/include/fmgr.h| 10 src/include/nodes/nodes.h
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1608)
Bruce Momjian wrote: KaiGai Kohei wrote: Bruce Momjian wrote: KaiGai Kohei wrote: The series of SE-PostgreSQL patches for v8.4 were updated: [1/5] http://sepgsql.googlecode.com/files/sepgsql-core-8.4devel-r1608.patch [2/5] http://sepgsql.googlecode.com/files/sepgsql-utils-8.4devel-r1608.patch [3/5] http://sepgsql.googlecode.com/files/sepgsql-policy-8.4devel-r1608.patch [4/5] http://sepgsql.googlecode.com/files/sepgsql-docs-8.4devel-r1608.patch [5/5] http://sepgsql.googlecode.com/files/sepgsql-tests-8.4devel-r1608.patch - List of updates: * bugfix: sepgsqlCheckProcedureEntrypoint() was invoked twice when security invoker functions are invoked. Rest of parts are unchanged. Don't mind contracted filename. Please comment anything. It will help to improve our code. I did an analysis of the core file: http://sepgsql.googlecode.com/files/sepgsql-core-8.4devel-r1608.patch changed lines 3226 new files 4075 syscatalog 9977 total 17278 The good news is that 3226 is the affect on the non-system-catalog main core code, and is a context diff size, not total changed lines. Hum...? What utility did you use to compute the lines? It seems to me the changed lines except for system catalogs are larger than actual one. The diffstat says: 65 files changed, 4769 insertions(+), 11 deletions(-), 4945 modifications(!) The (4244 + 500) of 4945 modifications come from pg_proc.h and pg_attribute.h due to a new field to store security label of procedures and columns. The new files adds 4014 in total, so rest of (755 + 11 + 201 = 967) lines are estimated changes in the main core code. Anyway, I believe the burden of reviewer became smaller than the prior full-set version. I manually created the three files and then got totals; I have attached the files. OK, I can understand. It also counts unchanged lines, like: | *** base/src/backend/Makefile Sat Jan 3 13:01:35 2009 | --- sepgsql/src/backend/MakefileFri Feb 6 13:13:08 2009 | *** include $(top_builddir)/src/Makefile.glo | *** 16,22 | |SUBDIRS = access bootstrap catalog parser commands executor foreign lib libpq \ | main nodes optimizer port postmaster regex rewrite \ | ! storage tcop tsearch utils $(top_builddir)/src/timezone | |include $(srcdir)/common.mk | | --- 16,22 | |SUBDIRS = access bootstrap catalog parser commands executor foreign lib libpq \ | main nodes optimizer port postmaster regex rewrite \ | ! security storage tcop tsearch utils $(top_builddir)/src/timezone | |include $(srcdir)/common.mk But it is not a point to be discussed here. The significant point is the scale of patches are indeed reduced as I promised before, by separating a few facilities. :) I've waited for reviewer's comment. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1627)
The series of SE-PostgreSQL patches for v8.4 were updated: [1/5] http://sepgsql.googlecode.com/files/sepgsql-core-8.4devel-r1627.patch [2/5] http://sepgsql.googlecode.com/files/sepgsql-utils-8.4devel-r1627.patch [3/5] http://sepgsql.googlecode.com/files/sepgsql-policy-8.4devel-r1627.patch [4/5] http://sepgsql.googlecode.com/files/sepgsql-docs-8.4devel-r1627.patch [5/5] http://sepgsql.googlecode.com/files/sepgsql-tests-8.4devel-r1627.patch - List of updates: * It is rebased to the latest CVS HEAD. - Adding a new funcction into pg_proc.h made a patch confliction. - Changing usage messages at initdb made a patch confliction. * sepgsqlCheckDatabaseInstallModule() is removed from commands/foreigncmds.c because CreateFdwStmt-library has gone in the recent changes. Rest of parts are unchanged, so don't consider this updates needs to review whole of patches again, please. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1627)
On Wed, Feb 25, 2009 at 11:04 PM, KaiGai Kohei kai...@ak.jp.nec.com wrote: The series of SE-PostgreSQL patches for v8.4 were updated: [1/5] http://sepgsql.googlecode.com/files/sepgsql-core-8.4devel-r1627.patch [2/5] http://sepgsql.googlecode.com/files/sepgsql-utils-8.4devel-r1627.patch [3/5] http://sepgsql.googlecode.com/files/sepgsql-policy-8.4devel-r1627.patch [4/5] http://sepgsql.googlecode.com/files/sepgsql-docs-8.4devel-r1627.patch [5/5] http://sepgsql.googlecode.com/files/sepgsql-tests-8.4devel-r1627.patch - List of updates: * It is rebased to the latest CVS HEAD. actually i see fails when trying to apply sepgsql-core-8.4devel-r1627.patch to head (in pg_proc.h)... Hunk #4 FAILED at 113. 1 out of 4 hunks FAILED -- saving rejects to file src/include/catalog/pg_proc.h.rej -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1627)
Jaime Casanova wrote: On Wed, Feb 25, 2009 at 11:04 PM, KaiGai Kohei kai...@ak.jp.nec.com wrote: The series of SE-PostgreSQL patches for v8.4 were updated: [1/5] http://sepgsql.googlecode.com/files/sepgsql-core-8.4devel-r1627.patch [2/5] http://sepgsql.googlecode.com/files/sepgsql-utils-8.4devel-r1627.patch [3/5] http://sepgsql.googlecode.com/files/sepgsql-policy-8.4devel-r1627.patch [4/5] http://sepgsql.googlecode.com/files/sepgsql-docs-8.4devel-r1627.patch [5/5] http://sepgsql.googlecode.com/files/sepgsql-tests-8.4devel-r1627.patch - List of updates: * It is rebased to the latest CVS HEAD. actually i see fails when trying to apply sepgsql-core-8.4devel-r1627.patch to head (in pg_proc.h)... Hunk #4 FAILED at 113. 1 out of 4 hunks FAILED -- saving rejects to file src/include/catalog/pg_proc.h.rej I'll check it soon, please wait for a while. Thanks for your interesting! -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1627)
KaiGai Kohei wrote: Jaime Casanova wrote: - List of updates: * It is rebased to the latest CVS HEAD. actually i see fails when trying to apply sepgsql-core-8.4devel-r1627.patch to head (in pg_proc.h)... Hunk #4 FAILED at 113. 1 out of 4 hunks FAILED -- saving rejects to file src/include/catalog/pg_proc.h.rej I'll check it soon, please wait for a while. I could not reproduce this patch confliction you reported. Could you confirm whether the base tree is updated to the latest one, or not? The previous revision (r1608) conflicts to this commit, so it is necessary the base tree to be updated in this two days. http://git.postgresql.org/?p=postgresql.git;a=commitdiff;h=a3c594d20e6e713c2bed8e0ba416ab34cdec8ecf#patch24 [kai...@masu tmp]$ cvs -z3 -d :pserver:anon...@anoncvs.postgresql.org:/projects/cvsroot \ export -r HEAD -d pgsql.cvs pgsql [kai...@masu tmp]$ wget http://sepgsql.googlecode.com/files/sepgsql-core-8.4devel-r1627.patch [kai...@masu tmp]$ cd pgsql.cvs/ [kai...@masu pgsql.cvs]$ cat ../sepgsql-core-8.4devel-r1627.patch | patch -p1 patching file configure : patching file src/include/catalog/pg_database.h patching file src/include/catalog/pg_proc.h (*) patching file src/include/catalog/pg_proc_fn.h : patching file src/include/utils/syscache.h [kai...@masu pgsql.cvs]$ Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1627)
On Thu, Feb 26, 2009 at 1:46 AM, KaiGai Kohei kai...@ak.jp.nec.com wrote: KaiGai Kohei wrote: Jaime Casanova wrote: - List of updates: * It is rebased to the latest CVS HEAD. actually i see fails when trying to apply sepgsql-core-8.4devel-r1627.patch to head (in pg_proc.h)... Hunk #4 FAILED at 113. 1 out of 4 hunks FAILED -- saving rejects to file src/include/catalog/pg_proc.h.rej I'll check it soon, please wait for a while. I could not reproduce this patch confliction you reported. Could you confirm whether the base tree is updated to the latest one, or not? The previous revision (r1608) conflicts to this commit, so it is necessary the base tree to be updated in this two days. ah! please forget it... it's to late here... i updated my local repo but doesn't execute the cvs update -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1608)
The series of SE-PostgreSQL patches for v8.4 were updated: [1/5] http://sepgsql.googlecode.com/files/sepgsql-core-8.4devel-r1608.patch [2/5] http://sepgsql.googlecode.com/files/sepgsql-utils-8.4devel-r1608.patch [3/5] http://sepgsql.googlecode.com/files/sepgsql-policy-8.4devel-r1608.patch [4/5] http://sepgsql.googlecode.com/files/sepgsql-docs-8.4devel-r1608.patch [5/5] http://sepgsql.googlecode.com/files/sepgsql-tests-8.4devel-r1608.patch - List of updates: * bugfix: sepgsqlCheckProcedureEntrypoint() was invoked twice when security invoker functions are invoked. Rest of parts are unchanged. Don't mind contracted filename. Please comment anything. It will help to improve our code. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1590)
The series of SE-PostgreSQL patches for v8.4 are updated: [1/5] http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1590.patch [2/5] http://sepgsql.googlecode.com/files/sepostgresql-utils-8.4devel-3-r1590.patch [3/5] http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1590.patch [4/5] http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1590.patch [5/5] http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1590.patch - List of updates: * These are rebased to the latest CVS HEAD due to conflictions. * bugfix: incorrect columns were checked on trigger invocations as FK checks. Rest of parts are unchanged. I guess the reviewer (Bruce?) is in mid-flow of reviewing the patches. If it is not comprehensive yet, any comments will help to improve the code. Could you tell me what is the current status? Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1530)
On 2/14/09, Jaime Casanova jcasa...@systemguards.com.ec wrote: On Sat, Feb 14, 2009 at 12:46 AM, KaiGai Kohei kai...@kaigai.gr.jp wrote: If you set up a VM from scratch, Fedora enables to install via remote repository using minimum bootable image. http://download.fedora.redhat.com/pub/fedora/linux/releases/10/Fedora/i386/iso/ ok, i'm going to download the image 5 days later... i'm having troubles with my home internet conection (that's a usual problem when living in a third-world country ;) i think the problem was solved so i will try to start the download again... or will have to make some other tricks for getting it... but i will make these tests starting saturday (we are in holydays until tuesday and don't think i will go to beach) -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1530)
KaiGai Kohei wrote: If you set up a VM from scratch, Fedora enables to install via remote repository using minimum bootable image. http://download.fedora.redhat.com/pub/fedora/linux/releases/10/Fedora/i386/iso/ I don't think it is not a essential for you to resolve troubles related to SELinux on Ubuntu. Oops, s/I don't think/I think/g Sorry, I often misuse negative sentences. :( -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1530)
On Sat, Feb 14, 2009 at 12:46 AM, KaiGai Kohei kai...@kaigai.gr.jp wrote: Now I checked the repository of Ubuntu, however, I can find several matters to build/run SE-PostgreSQL. - The default security policy is quite old. Is does not contain SE-PostgreSQL support which is merged at the refpolicy-20080702. - The libselinux is a bit old. It does not contains several symbols required by SE-PostgreSQL, so it is not possible to compile. so we need to state pre-requisites in docs, maybe make configure to check the version of libselinux, haven't looked is the patch already does that... If you set up a VM from scratch, Fedora enables to install via remote repository using minimum bootable image. http://download.fedora.redhat.com/pub/fedora/linux/releases/10/Fedora/i386/iso/ ok, i'm going to download the image -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1530)
KaiGai Kohei wrote: KaiGai Kohei wrote: The series of SE-PostgreSQL patches are updated: [1/5] http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1530.patch [2/5] http://sepgsql.googlecode.com/files/sepostgresql-utils-8.4devel-3-r1530.patch [3/5] http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1530.patch [4/5] http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1530.patch [5/5] http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1530.patch BTW, what is the current status of revewing the patches? Is it necessary to wait for a few days more? If you have anything unclear, please feel free to ask me anything. Yes, what was the decision about 8.4? Is this going to make it in? Thanks, - List of updates: * These are rebased to the latest CVS HEAD because of conflictions. - The src/include/catalog/pg_proc.h got a confliction due to the newly added SQL functions. - The src/bin/pg_dump/pg_dump.c got a confliction due to the stuff to dump toast_reloptions. * bugfix: An incorrect procedure entry for sepgsql_server_getcon(). * cleanup: A strange error message in testcases. Rest of parts are unchanged. Please comment anything. Thanks, -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1530)
On Fri, Feb 13, 2009 at 9:07 AM, Joshua Brindle met...@manicmethod.com wrote: KaiGai Kohei wrote: KaiGai Kohei wrote: The series of SE-PostgreSQL patches are updated: [1/5] http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1530.patch [2/5] http://sepgsql.googlecode.com/files/sepostgresql-utils-8.4devel-3-r1530.patch [3/5] http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1530.patch [4/5] http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1530.patch [5/5] http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1530.patch BTW, what is the current status of revewing the patches? Is it necessary to wait for a few days more? If you have anything unclear, please feel free to ask me anything. Yes, what was the decision about 8.4? Is this going to make it in? can you try the functional parts of it? ie: compile with the patch with --enable-selinux and test if the patch does wath you expect? i will try it but i have to install a VM to install selinux on it... then i will try some cases... can you give me an example of a typical scenario to make those tests? i will do some code review too but i'm not a hacker so you will need to wait for them to review it anyway... -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1530)
Jaime Casanova wrote:
On Fri, Feb 13, 2009 at 9:07 AM, Joshua Brindle met...@manicmethod.com wrote:
KaiGai Kohei wrote:
KaiGai Kohei wrote:
The series of SE-PostgreSQL patches are updated:
[1/5]
http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1530.patch
[2/5]
http://sepgsql.googlecode.com/files/sepostgresql-utils-8.4devel-3-r1530.patch
[3/5]
http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1530.patch
[4/5]
http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1530.patch
[5/5]
http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1530.patch
BTW, what is the current status of revewing the patches?
Is it necessary to wait for a few days more?
If you have anything unclear, please feel free to ask me anything.
Yes, what was the decision about 8.4? Is this going to make it in?
can you try the functional parts of it? ie: compile with the patch
with --enable-selinux and test if the patch does wath you expect?
i will try it but i have to install a VM to install selinux on it...
then i will try some cases... can you give me an example of a typical
scenario to make those tests?
If you can help to test the patches, I recommend you to install Fedora 10
on your VM images, because it includes SELinux in the default and its
default security policy (selinux-policy-targeted) also supports SE-PostgreSQL.
Then, could you try the following steps?
1) installation
$ ./configure --enable-selinux
$ make
$ make -C src/backend/security/sepgsql/policy
(NOTE: We provide a policy module for development purpose)
$ su
# make install
# /usr/sbin/semodule -i
src/backend/security/sepgsql/policy/sepostgresql-devel.pp
(NOTE: It installs the development policy)
# /sbin/restorecon -R /usr/local/pgsql
(NOTE: It assigns correct security context for installed binaries)
$ export PGDATA=/path/to/database
$ chcon -t postgresql_db_t -R $PGDATA
(NOTE: It assigns correct security context for database files)
$ initdb --enable-selinux
(NOTE: --enable-selinux turns on SE-PostgreSQL feature)
$ pg_ctl start
2) check installation
2-1) Please confirm SE-PostgreSQL works
$ psql postgres
psql (8.4devel)
Type help for help.
postgres=# SHOW sepostgresql;
sepostgresql
--
on
(1 row)
2-2) Please confirm client's privileges
$ id -Z
unconfined_u:unconfined_r:unconfined_t
$ psql postgres
psql (8.4devel)
Type help for help.
postgres=# SELECT sepgsql_getcon();
sepgsql_getcon
unconfined_u:unconfined_r:unconfined_t
(1 row)
NOTE: It has to be matched with privileges on OS.
2-3) Please confirm server's privileges
postgres=# SELECT sepgsql_server_getcon();
sepgsql_server_getcon
unconfined_u:system_r:postgresql_t
(1 row)
NOTE: It is necessary restricted domain (like PHP scripts) to connect
PostgreSQL server process.
2-4) Please confirm to connect from restricted domain
$ runcon -t sepgsql_test_t -- psql postgres
psql (8.4devel)
Type help for help.
postgres=# SELECT sepgsql_getcon();
sepgsql_getcon
--
unconfined_u:unconfined_r:sepgsql_test_t
(1 row)
NOTE: The sepgsql_test_t has restricted privileges same as PHP scripts
invoked from Apache web server.
NOTE: If SELinux denied to connect, please try the following command (in
root):
# setsebool -P allow_user_postgresql_connect 1
3) Example of a typical scenario
3-1) Setup of column level access controls
postgres=# CREATE TABLE customer (
cid int primary key,
cname text,
credit varchar(32)
SECURITY_LABEL = 'system_u:object_r:sepgsql_secret_table_t:s0'
);
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index customer_pkey for
table customer
CREATE TABLE
postgres=# INSERT INTO customer VALUES (1, 'kaigai', '---'),
(2, 'yamada', '---'),
(3, 'kimura', '--1234-5678');
INSERT 0 3
postgres=# SELECT * FROM customer;
cid | cname | credit
-++-
1 | kaigai | ---
2 | yamada | ---
3 | kimura | --1234-5678
(3 rows)
postgres=# CREATE OR REPLACE FUNCTION show_credit (int)
RETURNS text LANGUAGE 'sql'
SECURITY_LABEL = 'system_u:object_r:sepgsql_trusted_proc_exec_t:s0'
AS 'SELECT regexp_replace(credit, ''-[0-9]+'', ''-'', ''g'') FROM
customer WHERE cid = $1';
CREATE FUNCTION
3-2) Example of column level access controls
$ runcon -t sepgsql_test_t -- psql postgres
psql (8.4devel)
Type help for help.
postgres=# SELECT * FROM customer;
ERROR: SELinux: denied { select }
scontext=unconfined_u:unconfined_r:sepgsql_test_t
