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
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,
