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  (KaiGai Kohei <kaigai@kaigai.gr.jp>)
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:

Previous
From: Heikki Linnakangas
Date:
Subject: Hot standby, recovery infra
Next
From: Peter Eisentraut
Date:
Subject: Column privileges for system catalogs