Re: [PATCH] SE-PgSQL/tiny rev.2193

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>
Cc: Joshua Brindle <method(at)manicmethod(dot)com>, Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Martijn van Oosterhout <kleptog(at)svana(dot)org>, Peter Eisentraut <peter_e(at)gmx(dot)net>, pgsql-hackers(at)postgresql(dot)org, KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>
Subject: Re: [PATCH] SE-PgSQL/tiny rev.2193
Date: 2009-07-21 04:51:46
Message-ID: 603c8f070907202151s79532836p4f6875b69945d5e@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2009/7/20 KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>:
> Robert Haas wrote:
>> - row-level security
>> - complex DDL permissions
>
> Is the complex DDL permissions mean something like db_xxx:{create},
> db_xxx:{relabelfrom relabelto} and others?
> If so, I can agree to implement these checks at the later patch.
>
> However, please note that the initial patch cannot achieve actual
> security in this case, because it means anyone can change security
> label of objects.

I'm not qualified to answer this question, and that's exactly why we
need more documentation of what this patch tries to do and why (i.e. a
spec). What I was specifically referring to is things like
db_column:{drop}, which are much more specific than anything
PostgreSQL currently offers to control DDL. I think we should make an
attempt to get objects labelled in a reasonable way (I did not like
the approach your latest patch took of just assigning everything a
default label), which might include relabelling also, but certainly
doesn't include things like distinguishing between different kinds of
DDL operations. I'm not really up on SELinux terminology (this is
another thing that needs to be covered in the documentation, and
isn't) but maybe something like db_table:{owner}.

>> I also agree with Peter's contention that a spec would be useful.  If
>> you could read a clear description of what the patch was going to do,
>> then you could separate the problem of figuring out whether that was
>> the right thing from the question of whether the patch actually did
>> it.
>
> I can also agree with the suggestion.
> The specifications (from viewpoint of the developer) will introduces
> the fundamental principles to be implemented, and it will figure out
> what implementation is better.
>
> As I noted before, I've been flexible about how SE-PgSQL is implemented
> as far as it can perform SELinux's security model correctly.
>
> Please for a several days. I'll describe it.

I really, really think you need to find someone to help you with the
documentation. As I've said before, your English is a lot better than
my Japanese, but the current documentation is just hard to read. More
than that, writing good documentation is HARD, and there are not a ton
of people who can do it well. It seems to me that the documentation
for this project could require as much work as the actual code.

And this is an excellent time to bring up the whole issue of community
involvement again. Many advocacy-type folks and end-users and
security folks have written in over the last year to say how much they
want this feature, but as far as anyone can tell KaiGai is the only
one doing any actual development to make it happen. Considering how
the alternative is, or so we're told, to spend $13,000/year (per CPU?)
for a similar Oracle feature, one might suppose that there would be
developers lining up to volunteer to help make this happen, and
companies making sponsorship dollars available to fund work by major
PostgreSQL contributors to get this whipped into shape. No? Well, at
the very least, one would hope that some of the people who say what
they want this feature to do could help document it, since they
presumably understand what it's supposed to do (if not, why are they
so sure they want it?). But so far the only person who has done
anything like this, as far as I'm aware, is me. And that's pretty
funny when you consider that I've learned enough about SE-Linux to do
exactly one thing: shut it off.

What I want for documentation of this feature is something like the
"Database Roles and Privileges" chapter of the current documentation.
You can read this chapter from beginning to end and have a pretty good
idea how to manage permissions in PostgreSQL. SE-Linux is complex
enough that you might have to refer to other sources of information
for certain topics (e.g. policy writing), because a full overview of
those topics might exceed the scope of our documentation. But it
should still be possible to start knowing nothing, read the chapter,
and have a pretty good idea how all the pieces fit together, at least
as far as PostgreSQL is concerned.

I would go so far as to suggest that we use the willingness of someone
from the community to volunteer to help KaiGai get this put together
as a litmus test for support for this patch. If someone sent in a
patch for Hot Standby or Streaming Rep tomorrow that was done except
for the documentation, how long do you think it would take them to get
some volunteers to help finish the docs? I'd bet less than a day. On
the other hand, it appears to me that we have had more people put more
time into __builtin_clz() over the last three months than
SE-PostgreSQL. __builtin_clz() has had multiple people benchmarking
it and testing it, giving feedback on the patch, etc. More often than
not, nobody even responds to KaiGai's patch set; it looks to me like
the last response that he got prior to when I started reviewing this
for CommitFest 2009-07 was a note from David Wheeler admiring his
persistence; prior to that, the last response was back in April, when
Bruce asked him about including something in the 8.4 release notes.
All I can say is: ouch.

...Robert

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Itagaki Takahiro 2009-07-21 05:07:47 Re: Duplicate key value error
Previous Message Jeremy Kerr 2009-07-21 03:08:11 Re: [PATCH v4] Avoid manual shift-and-test logic in AllocSetFreeIndex