Re: Adding support for SE-Linux security

On Tue, Dec 8, 2009 at 2:50 PM, David P. Quigley <dpquigl(at)tycho(dot)nsa(dot)gov> wrote:
> On Tue, 2009-12-08 at 14:22 -0500, Robert Haas wrote:
>> On Tue, Dec 8, 2009 at 1:50 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> > Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> >> One of the major and fundamental stumbling blocks we've run into is
>> >> that every solution we've looked at so far seems to involve adding
>> >> SE-Linux-specific checks in many places in the code.  It would be nice
>> >> if it were possible to use the exist permissions-checking functions
>> >> and have them check a few more things while they're at it, but it's
>> >> looking like that won't be feasible, or at least no one's come up with
>> >> a plausible design yet.
>> >
>> > I don't think that it's about SELinux.  The real issue here is that
>> > KaiGai-san is about a mile out in front of the PG hackers community
>> > in terms of his ambitions for the scope of what can be controlled by
>> > security policy.  If the patch were only doing what the community has
>> > actually agreed to, there would be little need for it to touch anything
>> > but the aclcheck functions.
>> >
>> > Now I recognize that a large part of the potential attraction in this
>> > for the security community is exactly the idea of having fine-grain
>> > security control.  But if you ever want anything significantly different
>> > from SQL-standard permission mechanisms, there's going to have to be a
>> > whole lot more work done.  Basically, nobody in the PG community has got
>> > any confidence either in the overall design or the implementation
>> > details for locking things down that aren't already controlled by SQL
>> > permission mechanisms.
>>
>> I think that's basically right.  Further, I think this is basically a
>> resource issue.  If you were inclined to spend a large amount of your
>> time on this problem, you could either gain confidence in the present
>> design and implementation or come up with a new one in which you did
>> have confidence.  But it doesn't seem important enough to you (or your
>> employer) for the amount of time it would take, so you're not.  I
>> think there are other committers and community members in a similar
>> situation - basically all of them.
>
> I have to agree with Chad (downthread) that I don't see much in KaiGai's
> hook patch that prevents its use by other security models. I will say
> though one thing that might have been done wrong was with how it was
> presented. In actuality his patch set is two projects (at least). The
> first is the framework. So I think the goal should have been to get the
> framework integrated first and then work on the SELinux module after
> that. The framework patch alone consists of at least 4 sets of logical
> changes that could have been separated to make review easier. Once the
> framework was in, work could be done to get the SELinux module in while
> reducing overhead from the case where no module is loaded.

I can say from experience that this project is very skeptical of
frameworks that aren't accompanied by at least one, and preferably
multiple, working implementations. So there is a bit of a chicken and
egg problem here. What can sometimes work is to say, look, here's a
place where I can put a hook and later I will do something complex
with it but here are a couple of relatively simple but useful examples
to get started. The examples form the justification for the commit
because they are independently useful, but they are much simpler than
what the framework may eventually end up being used for.

I don't believe that this approach is feasible for SE-PostgreSQL. A
simple version of label security is still going to be very
complicated, and there are probably even fewer customers for such a
thing than there are for SE-PostgreSQL.

> So with regard to confidence in the design I think that part of the
> reason for the skepticism in the fact that it was such a large code
> drop. Even the separated parts were very large.

Definitely true.

> I think if the framework
> patches are broken up more logically and in a way that is easier to
> discuss then that might help the community get a grasp on what it is
> trying to accomplish.

Maybe, maybe not. Nobody's going to commit anything unless it's a
complete feature. It can be a cut-down feature, but it has to be of
independent use. If there are parts that can be peeled off of
SE-PostgreSQL and committed independently to some good benefit, then I
think that's a great idea. But if it's just a separation of the patch
for clarity, I don't think there's much value in that.

> In terms of documentation I was reading through the wiki at
> sepgsql.googlecode.com and aside from some wordsmithing/grammar things
> it is pretty solid with describing what it is trying to accomplish. One
> problem that I see is that at first glance it does appear to be very
> SELinux centric. It describes access based on types and SELinux contexts
> which is understandable based on the fact that it describes the
> framework and the module. Something to note is that the documentation
> describes an object model for the program. I think that would be a good
> place to focus the discussion with respect to a framework if we decide
> to pursue it.

I have proofread earlier versions of the docs and found them
inadequate. There were language issues, but the bigger problem was
that they were both too specific and yet not sufficiently detailed.
For example, they'd say "install X package on Red Hat". Well, what if
I'm not on Red Hat? The on the other hand they'd say "this GUC
enables mcstrans" which means nothing to me. I think this has been
improved somewhat in more recent version, but clearly we need (1) good
developer documentation, so someone other than KaiGai has a chance of
maintaining the code and keeping it correct as other things are
changed; (2) user documentation, so that someone can read the "feature
story" for this capability; and (3) reference documentation,
cross-referenced with the user documentation.

Having said that, fixing the documentation won't get this patch
committed, though it's certainly a step in the right direction. For
that we need a committer-owner.

...Robert