How to get SE-PostgreSQL acceptable - Mailing list pgsql-hackers
From | Peter Eisentraut |
---|---|
Subject | How to get SE-PostgreSQL acceptable |
Date | |
Msg-id | 498030E8.9030905@gmx.net Whole thread Raw |
Responses |
Re: How to get SE-PostgreSQL acceptable
|
List | pgsql-hackers |
I have re-reviewed the SE-PostgreSQL patch set. I don't want to talk about here whether the security model is appropriate, how foreign keys are to be handled etc. I want to discuss that I really don't like the architecture of this patch. I have said this before in previous review rounds, but let me make it a little clearer here. Steps to get your patch accepted: One feature at a time --------------------- By my count, your patch set implements at least three or four major features: 1a. System-wide consistency in access control 1b. Mandatory access control 2. Row-level security 3. Additional privileges (permission to alter tables, modify large objects, etc.) You may object and say, these morally belong together in a proper/professional/adequate implementation of this feature you have planned. But realistically, they can be separated. And if a feature is controversial, difficult, or complicated, it would be in your interest to deal with one feature at a time. "Deal" means the whole round: discuss design, write patch, review, test, commit, relax. If I had to do this, I would first write a patch for #1: A patch that additionally executes existing privilege checks against an SELinux policy. Existing privilege checks are a well-defined set: they mostly happen through pg_xxx_aclcheck() functions. Hook your checks in there. This patch would already provide useful functionality,but it would be much easier to review and verify, because we know how permission checking works in the existing system, so we can compare them. And it would build confidence among developers and users about the whole idea, about SELinux integration etc. Then you can tackle #3: Place permission checks in more places. This patch would be simple to review and verify, because at this point we already know how SELinux integration works, and making more use of it is not such a big step. At this point we could also discuss whether some of these additional checks are useful enough to also expose via the traditional SQL ACLs, which would further simplify the patch. Row-level security should also be developed as a completely separate feature, without any SELinux tie-in initially. This is not only important to make review and verification simpler, but also because we really need a wider test community for such a tricky feature. And the set of SELinux users is quite limited, and the intersection with PostgreSQL developers is almost empty. This was already previously discussed at length. No in-code frameworks --------------------- Write your code so that it is fully integrated with the existing code. Or write a plugin interface and then write a plugin. But don't invent a "framework" because you are afraid to integrate the new code with the old code. As mentioned above, permission checks are done through pg_xxx_aclcheck() functions, which is enough of a framework. I wouldn't want yet another framework that does more permission checking at other times and places. If the existing interfaces are not adequate foryour purpose, by all means, extend, refactor, or rewrite them. But don't just avoid it because you don't want to interfere with the existing code. So scrap the whole "PGACE" thing. If you need to refactor the aclcheck interfaces, that's another separate patch, which can easily be reviewed and verified, simplifying the following patches even further. These things are not going to get done within two weeks. But if you start producing small, self-contained patches along the above lines, you are much more likely to make progress over the coming development cycle.
pgsql-hackers by date: