Thread: SE-PostgreSQL?
Folks, At this point, SE-PostgreSQL has taken up a *lot* of community resources, not to mention an enormous and doubtless frustrating amount of Kohei-san's time and effort, thus far without a single committed patch, or even a consensus as to what it should (or could) do. Rather than continuing to blunder into the future, I think we need to do a reality check in the form of a couple of questions: 1. Among the committers who could maintain the features, whatever they turn out to be, who is actually volunteering to do so? 2. Apart from Kohei-san and Stephen Frost, is anybody actually interested in having this feature at all? I would submit that if we get fewer than three enthusiastic, "me!"s on the first, or fewer people than five on the second, we just need to bounce this feature and move on. As I see it, those numbers are a bare minimum, although one could fairly argue that I've underestimated the minimum for the second. What say? Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
On Saturday 18 July 2009 18:06:00 David Fetter wrote: > Folks, > > At this point, SE-PostgreSQL has taken up a *lot* of community > resources, not to mention an enormous and doubtless frustrating amount > of Kohei-san's time and effort, thus far without a single committed > patch, or even a consensus as to what it should (or could) do. > > Rather than continuing to blunder into the future, I think we need to > do a reality check in the form of a couple of questions: > > 1. Among the committers who could maintain the features, whatever > they turn out to be, who is actually volunteering to do so? > 2. Apart from Kohei-san and Stephen Frost, is anybody actually > interested in having this feature at all? I would definitely be interested. Andres
On Saturday 18 July 2009 12:31:34 Andres Freund wrote: > > 2. Apart from Kohei-san and Stephen Frost, is anybody actually > > interested in having this feature at all? > > I would definitely be interested. > +1 Best regards,Jesper
David, > 2. Apart from Kohei-san and Stephen Frost, is anybody actually > interested in having this feature at all? I'm interested in a version of the feature. That is, I'm specifically interested in an SEPostgres which delivers: a) SELinux-label control (pluggable with TrustedSolaris and other frameworks) of the existing PostgreSQL privileges. b) Efficient constraint-based row-level security (as opposed to individual row labelling)[1] I also believe that an SEPostgres which delivered row masking and data substitution would be of interest to a significant number of new users, but that these features are complex and unintuitive enough that they should always be an optional module. Secondarily, I believe that having integrated SEPostgres support woudl bring us new users from the government security sector and the health care sector who do not currently use PostgreSQL. Whether any of these users would contribute substantially to maintaining it is an open question; they certainly have funding, though, and the NSA has contributed a substantial amount of resources to Linux, and the Japanese Security Agency has contributed to PostgreSQL before. [1] For an explanation of the two ways to do row-level security, see here: http://it.toolbox.com/blogs/database-soup/thinking-about-row-level-security-part-1-30732 http://it.toolbox.com/blogs/database-soup/thinking-about-row-level-security-part-2-30757 -- Josh Berkus PostgreSQL Experts Inc. www.pgexperts.com
On Sat, Jul 18, 2009 at 10:19:16AM -0700, Josh Berkus wrote: > David, > >> 2. Apart from Kohei-san and Stephen Frost, is anybody actually >> interested in having this feature at all? > > I'm interested in a version of the feature. Not, so far, one congruent to anything Kohei-san has actually sent, though. I'd say this one's a bust. Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
On Sat, Jul 18, 2009 at 1:19 PM, Josh Berkus<josh@agliodbs.com> wrote: > b) Efficient constraint-based row-level security (as opposed to individual > row labelling)[1] [...] > [1] For an explanation of the two ways to do row-level security, see here: > http://it.toolbox.com/blogs/database-soup/thinking-about-row-level-security-part-1-30732 > http://it.toolbox.com/blogs/database-soup/thinking-about-row-level-security-part-2-30757 Yeah. That would be teh awesome (though admittedly I'm a sucker for a cool feature), but it's quite beside the point of the original email here, which is whether there's any point in continuing with the current SE-Linux patches. I think there isn't. Quite beyond the fact that we never seem to be able to get a patch that implements a reasonable first set of features, the amount of work that's going to be required to get these patches committable is going to be enormous. Just to cite a few examples, here is the documentation for the "sepostgresql_mctrans" documentation. % Enables to turn on/off Mcstrans feature on SE-PostgreSQL. It is on by default. Every users can set this % parameter on their sessision without any limitations. SELinux provides a feature to translate a part of security % context between raw format and human-readable format, called Mcstrans. If the parameter is turned on, % SE-PostgreSQL translate the security context when it is exported to or imported from users. So, one problem is that this is written in poor English. While I'm willing to do a certain amount of copy-editing as part of the review process, SE-PostgreSQL is a massive feature and it's unfair to put the entire burden of making the documentation readable on the shoulders of the reviewers. I already proofread and corrected these docs once, back in December, but now they need more reworking because they've been extensively revised since then, and lots of non-idiomatic constructs have crept back in. It's worse than that, though: the above is not only bad English, it's also not a very good description of the feature. I certainly can't tell from reading it what that GUC actually does, and there are no references to anyplace where I can turn for followup reading. And then there's this, which is unduly RedHat-centric: % Please note that SELinux requires installed files, directories and others should be labeled properly. RPM % installation do it implicitly. And then look at this patch hunk: pg_database_aclcheck(Oid db_oid, Oid roleid, AclMode mode) { if (pg_database_aclmask(db_oid, roleid, mode, ACLMASK_ANY)!= 0) + { + /* SELinux checks db_database:{connect} permission */ + if ((mode & ACL_CONNECT) && + !sepgsqlCheckDatabaseConnect(db_oid)) + return ACLCHECK_NO_PRIV; + return ACLCHECK_OK; + } else return ACLCHECK_NO_PRIV; } I don't think I'm stepping too far out on a limb to say that this looks like a very poor interface design. Surely we don't want a separate sepgsqlCheck<object-type><privilege> function for every combination of an object type and a privilege, and shouldn't the check be inside pg_database_aclmask anyway? These are just a few examples from reading through the first bit of the patch. I have no doubt that Kaigai is good at what he does, but I don't think he understands what the community is looking for from this patch in terms of features and coding style. I think the question is not so much whether anyone is interested in the features (I know some are) but whether anyone who understands what will be acceptable for PostgreSQL is willling to do the necessary amount of work to get a committable patch that implements them. I would be willing to work with someone to fine-tune such a patch set, but the ratio of reviewer effort to patch quality improvement on this patch set is well above what I am prepared to put in. ...Robert
David Fetter asked: > > 1. Among the committers who could maintain the features, whatever > they turn out to be, who is actually volunteering to do so? I am not a committer, but I saw a message about issues with documentation -- I would be willing to help on making documentation and comments more robust; I don't speak Japanese but have a couple of friends who do that I can call upon for some help. > > 2. Apart from Kohei-san and Stephen Frost, is anybody actually > interested in having this feature at all? This feature would be of interest in some corporate circles as well as government and health care. That said it seems like two issues are biting those working on it: (a) it's a lot of work even to get a simple version and (b) the model differs from the more tradition ACL and trying to wrap collective heads around the implications is an exercise in and of itself. My gut feeling is this is so much work that it needs a paying sponsor to see it through. Trying to do it without more hands is likely to distract from other items on the to-do list. Greg W.
Robert Haas wrote: > Quite beyond the fact that we never seem to be able to get a patch > that implements a reasonable first set of features, the amount of work > that's going to be required to get these patches committable is going > to be enormous. Just to cite a few examples, here is the > documentation for the "sepostgresql_mctrans" documentation. > > % Enables to turn on/off Mcstrans feature on SE-PostgreSQL. It is on > by default. Every users can set this > % parameter on their sessision without any limitations. SELinux > provides a feature to translate a part of security > % context between raw format and human-readable format, called > Mcstrans. If the parameter is turned on, > % SE-PostgreSQL translate the security context when it is exported to > or imported from users. > > So, one problem is that this is written in poor English. While I'm > willing to do a certain amount of copy-editing as part of the review > process, SE-PostgreSQL is a massive feature and it's unfair to put the > entire burden of making the documentation readable on the shoulders of > the reviewers. I already proofread and corrected these docs once, > back in December, but now they need more reworking because they've > been extensively revised since then, and lots of non-idiomatic > constructs have crept back in. It's worse than that, though: the > above is not only bad English, it's also not a very good description > of the feature. I certainly can't tell from reading it what that GUC > actually does, and there are no references to anyplace where I can > turn for followup reading. As you know, I'm not a native English user, so I need to admit it is not avoidable the documentations are not idiomatic more or less. If you consider it is unclear what the documentation actually said, could you please tell me. I'll revise it soon. I believe PostgreSQL community is internationally open. > And then there's this, which is unduly RedHat-centric: > > % Please note that SELinux requires installed files, directories and > others should be labeled properly. RPM > % installation do it implicitly. IIRC, I introduced the way to label files and directories installed on the later section? > And then look at this patch hunk: > > pg_database_aclcheck(Oid db_oid, Oid roleid, AclMode mode) > { > if (pg_database_aclmask(db_oid, roleid, mode, ACLMASK_ANY) != 0) > + { > + /* SELinux checks db_database:{connect} permission */ > + if ((mode & ACL_CONNECT) && > + !sepgsqlCheckDatabaseConnect(db_oid)) > + return ACLCHECK_NO_PRIV; > + > return ACLCHECK_OK; > + } > else > return ACLCHECK_NO_PRIV; > } > > I don't think I'm stepping too far out on a limb to say that this > looks like a very poor interface design. Surely we don't want a > separate sepgsqlCheck<object-type><privilege> function for every > combination of an object type and a privilege, and shouldn't the check > be inside pg_database_aclmask anyway? If we can have more graceful interface design, I can agree to replace the current design. However, several kind of permission checks requires extra informations rather than OID of database. For example, when we modify the security label of the database, SELinux shall requires to check db_database:{relabelfrom} on the database with older label and db_database:{relabelto} on the database with newer label. In this case, the security hook need the user given security label rather than OID of the database. When I designed the interfaces, a part of security hooks requires only OID of the database object, but rest of them are not. So, I considered that it is confusable for developers some of hooks are common for various kind of permissions but others are not so. > These are just a few examples from reading through the first bit of > the patch. I have no doubt that Kaigai is good at what he does, but I > don't think he understands what the community is looking for from this > patch in terms of features and coding style. I think the question is > not so much whether anyone is interested in the features (I know some > are) but whether anyone who understands what will be acceptable for > PostgreSQL is willling to do the necessary amount of work to get a > committable patch that implements them. I would be willing to work > with someone to fine-tune such a patch set, but the ratio of reviewer > effort to patch quality improvement on this patch set is well above > what I am prepared to put in. IIRC, at the v8.4 development cycle, Peter Eisentraut asked to me what feature is the fundamental principle of the SE-PostgreSQL and what is not separatable. I replied the system-wide consistency in access controls and mandatory access controls are the essence in SE-PostgreSQL. It is the security model in other word. Rest of features are not as significant as the security model, such as granularity of access controls, extra permissions (largeobejcts, ...). So, I could agree to postpone a part of features in the later version to reduce the scale of patches. Needless to say, it is not something fundamental things how to implement them and what coding style is acceptable. I'm always flexible for them. Please remind that older SE-PgSQL wrapped all the security hooks with LSM like interfaces called PGACE. But, it was pointed out we don't need any in-code framework if the feature tries to be mainlined, then I removed all the pgace_xxxx() hooks from the patch. Thanks, -- KaiGai Kohei <kaigai@kaigai.gr.jp>
David Fetter wrote: > > 2. Apart from Kohei-san and Stephen Frost, is anybody actually > interested in having this feature at all? The features (both MAC, and row-level security), are interesting. * I've worked with organizations where MAC was a big deal. * I've had use cases where row-level security would be useful. * If this feature's a right step of getting PG into getting onto lists of EAL-certified databases like these: http://www.niap-ccevs.org/cc-scheme/vpl/?tech_name=DBMSit could make selling PG-backed solutions to some companies easier. I guess that'd count as a sales/PR feature? What I don't know is if this particular patch is the best step to getting any of those features.
On Sat, Jul 18, 2009 at 12:06 PM, David Fetter<david@fetter.org> wrote: > At this point, SE-PostgreSQL has taken up a *lot* of community > resources, not to mention an enormous and doubtless frustrating amount > of Kohei-san's time and effort, thus far without a single committed > patch, or even a consensus as to what it should (or could) do. > > Rather than continuing to blunder into the future, I think we need to > do a reality check in the form of a couple of questions: > > 1. Among the committers who could maintain the features, whatever > they turn out to be, who is actually volunteering to do so? > > 2. Apart from Kohei-san and Stephen Frost, is anybody actually > interested in having this feature at all? > > I would submit that if we get fewer than three enthusiastic, "me!"s on > the first, or fewer people than five on the second, we just need to > bounce this feature and move on. As I see it, those numbers are a > bare minimum, although one could fairly argue that I've underestimated > the minimum for the second. I count zero for the first question and five for the second, although two of those five (Josh Berkus and Ron Mayer) expressed doubt about this patch set as an implementation of this feature, and only one person (Greg Williamson) volunteered to help. I think, though, that we have on the other thread gotten closer to a solution to some of the problems that have been plaguing this feature, including, in particular, the need for a clear spec and very complete docs. I think the best thing for this patch right now is to move it to "Returned with Feedback". I can't see any way that this patch is going to be made committable for this CommitFest, and I think that pretending otherwise is only encouraging KaiGai to do another of his lighting rework-and-resubmits. While those are very impressive, they're not getting us where we need to be. I think that what KaiGai needs to do here is get the spec written (with the help of Greg Williamson and anyone else who is willing to pitch in), and submit it for comments. I don't think there will be a problem getting that reviewed outside of a CommitFest, and it's not a patch anyway, so the time that it gets submitted is not crucial. What is crucial is that it is a good spec that everyone can read, and hopefully understand and discuss. There is no point writing any more code, or submitting any more patches, until we have agreement on what those patches are supposed to do. I am going to go ahead and mark this as "Returned with Feedback". ...Robert
Robert Haas wrote: > I think the best thing for this patch right now is to move it to > "Returned with Feedback". I can't see any way that this patch is > going to be made committable for this CommitFest, and I think that > pretending otherwise is only encouraging KaiGai to do another of his > lighting rework-and-resubmits. While those are very impressive, > they're not getting us where we need to be. I think that what KaiGai > needs to do here is get the spec written (with the help of Greg > Williamson and anyone else who is willing to pitch in), and submit it > for comments. I don't think there will be a problem getting that > reviewed outside of a CommitFest, and it's not a patch anyway, so the > time that it gets submitted is not crucial. What is crucial is that > it is a good spec that everyone can read, and hopefully understand and > discuss. There is no point writing any more code, or submitting any > more patches, until we have agreement on what those patches are > supposed to do. I also agree that the easy understandable specification what SE-PostgreSQL tries to achieve is more important than implementation. I described it from the scratch again. Here is an initial draft: http://wiki.postgresql.org/wiki/SEPostgreSQL_Draft I would like to improve documentation quality and fix its specification during the discussion. > I am going to go ahead and mark this as "Returned with Feedback". Agreed. -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai@ak.jp.nec.com>