Re: 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(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)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(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)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(at)agliodbs(dot)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(at)kaigai(dot)gr(dot)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=DBMS
it 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(at)fetter(dot)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(at)ak(dot)jp(dot)nec(dot)com>