Re: Adding support for SE-Linux security - Mailing list pgsql-hackers

On Fri, Dec 11, 2009 at 3:28 PM, Stephen Frost <sfrost@snowman.net> wrote:
> I sincerely hope that even if you suggest an approach down the road
> unrelated to this on some other patch you're reviewing, and then you see
> the results and say "whoah, that's horrible, and should never be
> committed", that you understand none of us would want you to commit it.

I have to thank you for saying this - unfortunately, I don't think
everyone takes this approach.  As you can probably figure out, my
alleged rock management upthread was actually a poorly executed
attempt to avoid being accused of bait-and-switch.  If I don't tell
you how to write the patch, you can't accuse me of moving the
goalposts (of course I've now discovered the pitfalls of that approach
as well...).

> Sharing your ideas or putting out suggestions isn't a commitment on your
> part that you'll commit the results when someone else rights it.  Heck,
> I bet you've been down that road on your own projects and come to the
> realization at the end of "err, bad idea" and not committed it.

Well, I haven't been a committer long enough to have gone through that
precise process, but sure, I've tossed out ideas when they don't turn
out to be good.

>> So with that said, the idea I had was to try to pass around
>> pre-existing data structures related to the objects on which access
>> control decisions are being made, rather than Oids.
>
> That thought had crossed my mind as well, but I wasn't convinced that
> would actually be seen as a signifigantly different API to just having
> the arguments passed inline...

If you'll forgive me for saying so, this is exactly the sort of
thinking that I think is killing us.  Who cares how it will be seen?
Seen by whom?  We shouldn't be writing this code to be seen - we
should be writing it to be good.  If doing this makes a clean, tight
abstraction layer, then it's a good design.  If it doesn't, then it
sucks.  I realize that opinions enter into this at some level, but
let's try to proceed as though there's a technically right answer out
there and bend our best efforts to finding it.

> Then again, using structures does
> allow you to add to them without having to modify the function
> definitions,

Personal opinion time, but I think that "without having to modify the
function definitions" is a CRITICAL design component.  Furthermore,
"adding to them [the structures]" shouldn't be something that only
happens when we decide to support a new security model, or we're
little better off than if we just passed a kajillion arguments.
Again, I don't want to overstate my confidence in this point, but it
feels to me like we need to pass PRE-EXISTING data structures that are
already being used for other things and happen to be the right stuff
to enable access control decisions, and to which fields that are
likely to be needed are likely to get added anyway.  But it's
difficult to know whether this approach can be made to work without
trying it, and there are bound to be problem cases that need to be
thought about, and that thinking will be more likely to lead to a good
result if it happens in the community, rather than by KaiGai or any
other single person in isolation.

> and would allow David's suggestion of using function
> pointers to work, which we do in some other specific cases.  I guess I'm
> curious if we (PG) have any particular feeling one way or the other
> about function pointers; I just don't recall seeing them used terribly
> often and would worry about that they might be passively discouraged?

I'm going to vote fairly strongly against inserting function pointers
at the outset.  I think that we should look at the first phase of this
project as an attempt to restructure the code so that the access
control decisions are isolated from the rest of the code.  *If* we can
do that successfully, I think it will represent good progress all on
its own.  Inserting function pointers is something that can be done
later if it turns out to be useful with a very small, self-contained
patch.

>> It does have a bit of a rock management feel to it and I
>> really want to see if we can find a way to break that cycle.
>
> Agreed.  It's been a point of frustration for me, but I've been trying
> to work with it so long as we at least get some constructive critisim
> back (Tom's review of the patch I reviewed fell into the "questionable"
> category for me on that call, which is what really frustrated me the
> most about it).  A cyclic approach is typical in all software
> development, it's when information stops flowing about why something
> doesn't meet expectations or requirments that progress breaks down.

Part of my frustration here is that I don't see a lot of evidence that
anyone is willing to put really tangible resources into this other
than KaiGai and his employer.  I don't want to keep reviewing the same
patches over again just because they keep getting resubmitted with
various modifications.  I am willing to try to be helpful with this
project if that is something that you and others who are involved feel
is helpful and if it can be done with a small time commitment or if,
uh, I get paid.  But I don't want to see a lot of community resources
go into a project that is basically doomed from the get-go because
KaiGai is the only one caring about it.  This has already gone on for
a really long time with less progress than might be hoped for, and if
it's not going to live, I want it to die.  Having it stick around as a
zombie is the worst possible outcome.

...Robert