Re: How to get SE-PostgreSQL acceptable

From: KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>
To: Peter Eisentraut <peter_e(at)gmx(dot)net>
Cc: KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>, Joshua Brindle <method(at)manicmethod(dot)com>
Subject: Re: How to get SE-PostgreSQL acceptable
Date: 2009-01-28 13:28:11
Message-ID: 49805D6B.20909@kaigai.gr.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Sorry for long description again.

Peter Eisentraut wrote:
> 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.

I can't afford not to make clear these issues.

In this case, (1a) and (1b) are indivisible, because I want to apply
SELinux as a security server of (1a), SELinux has to be MAC feature.
However, I don't deny a (1b) without (1a) feature like "Oracle Label
Security", which is not a facility I want to make.
I guess (1b) also contains a feature to manage security label.
Please note that I don't really want to (1b) only feature.
The system-wide consistency in access control is the soul.

We can consider (2) as a separated issue. In fact, I already provide
a GUC: sepostgresql_row_level=on/off. It also means whether users
accept a set of benefit(row-level granularity) and demerit(cover
channel of PK/FK), or not.
OK, I don't discuss about covert channel here.

The (3) is involved to (1b). As a basic assumption, MAC system
need to check *any actions* come from clients, even if they have
superuser privileges.
We already have SQL-level privileges, like ACL_SELECT and so on.
However, some of operations implicitly assume request come from
superuser is safe. For example, superuser is allowed to load
a discretionary shared library file, but it also means he is
trusted.
If security design (which is defined by (1a) and (1b) primarily)
does not allow unconditionally trusted user, it is quite natural
to apply additional privileges, even if vanilla one unconditionally
trust superuser.
Unfortunatelly, we don't have any access controls on large object,
but SELinux does not want to provide an information store without
mandatory access controls. So, SE-PostgreSQL applies its access
controls on large object. Thus, (3) is also indivisible from (1a).

> 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.

I had a concern about pg_xxx_aclcheck().
When we accesses a table via a view, pg_xxx_aclcheck() checkes view's
ACLs and table's ACLs are left for unchecked, even if it accesses
same object.
SELinux always makes its decision based on the attribute of object
itself. In other word, the route to access taget object does not
make any affect on its decision making.
For example, a filesystem object (inode) can have multiple pathname
using hard link on operating system. SELinux always makes ite decision
based on inode's attribute (called as security context), independent
from its pathname. Do you know a previous hot discussion between
SELinux and AppArmor in Linux developers?
Please note that I don't deny the benefit of view. It has various
effective usages so we cannot ignore them.
However, it mismatched with the security design, so I needed to put
security hooks on different strategic points.

> 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.

It would be possible.
However, note that we should not implement the first step ignoring
upcoming facility.
Eventually I would implement "SE-PostgreSQL 1st edition" with
paying attention for the upcoming row level security.

> 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.

At least, PGACE is not a purpose for me.
If fully integrated archtencture is absolutely necessary,
I'll scrap PGACE framework.
Trusted Solaris folks may concern about it, but I cannot give
them higher priority than SE-PostgreSQL getting merged.
(Might sound insensitive, I don't have leeway now.)

The very earlier SE-PostgreSQL put its code directly, because
I didn't pay mention for T-Sol folks at that time.
However...

> 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 for your 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.

However, it is a different issue whether pg_xxx_aclcheck() can work as
an alternative of PGACE, or not.
SE-PostgreSQL want to make its decision more than pg_xxx_aclcheck(),
so, in finally, we need to put a code to check on different strategic
points where currently pgaceXXXX() are deployed.

> 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.

I believe a basic premise is vanilla PostgreSQL keeps correct behavior
based on SQL standard. An enhanced security should apply its access
controls orthogonally.
So, I cannot believe refactoring pg_xxx_aclcheck() is not acceptable.
If vanilla PostgreSQL become to check ACLs on tables, independent
from views, do you think it is acceptable?

> These things are not going to get done within two weeks.

No wonder!

However, we have to make clear whether the PGACE architecture
is incorrect, or not, at first.
I think the name of PGACE is not important, but it is necessary
to make SELinux's decision in similar strategic point in finally.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jonah H. Harris 2009-01-28 13:41:35 Re: 8.4 release planning
Previous Message Magnus Hagander 2009-01-28 13:22:39 Re: [COMMITTERS] pgsql: Silence compiler warning on win32.