Re: How to get SE-PostgreSQL acceptable

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

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.

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>

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

I've been thinking about this issue as well. I think a framework of
some kind could be acceptable, but only if we're convinced that the
framework is really general enough to handle all of the cases that we
care about. For example, can we back-port our existing DAC
infrastructure to use PGACE? Is it really true that SE-PostgreSQL
interacts through the rest of the system ONLY through PGACE? If the
answers to both questions are YES, then it's a framework. If the
answer to either question is NO, then it's just a bunch of places
where you needed to stick code to make SE-PostgreSQL work.

I haven't read the code, but from reading the docs, I have a feeling
that right now the answer to both questions are NO, which means it
doesn't really have a lot of value. One example of this is the
pg_security system catalog. The catalog representation you're
proposing is probably just fine for associating OIDs to SELinux
security labels. But trying to present it as a general thing that
some other security implementation could reuse just doesn't seem
realistic. Who is to say that the next person who writes an enhanced
security feature will want to use text as the representation for their
security domains? It could just as easily be two integers, or an
array of booleans. This is after all a database product, so the
chances that someone would want to do something with structured data
seem non-negligible.

In the end, you're going to have to be the one who makes the decision
on which way to go. In some ways, I think that a plugin architecture
would be better for everyone: we worry about the things on our side of
the abstraction boundary, and you worry about the things on your side.
Potentially you or someone else can release enhanced security plugins
without needing any changes to core, and potentially on a different
release cycle. On the other hand, a plugin architecture is probably
going to be a lot of work. It seems that to install the plugin you
would need to make system catalog changes as well as stuff additional
system attributes into every table, which are relatively invasive
changes. And you'll need to convince everyone that it's really a
plug-in architecture and not just a special case for SE-PostgreSQL, so
you'll need to prove that there are multiple viable clients, perhaps
by backporting our existing DAC.

...Robert

* KaiGai Kohei (kaigai(at)kaigai(dot)gr(dot)jp) wrote:
> 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?

Well, just to be clear, ACLs are checked on tables under views, but
they're checked using the privileges of the view owner rather than
the privileges of the current user. I've run into that empirically
because I've gotten 'permission denied' errors when using a view that
I've clearly got full rights on but was owned by someone else (who
didn't have rights on the table underneath).

That being said, I'd think that if we do need different semantics from
that for SE-PostgreSQL, we could implement it using a GUC or similar to
keep the current behavior as well allow the SE-PostgreSQL behavior.

> However, we have to make clear whether the PGACE architecture
> is incorrect, or not, at first.

It really bothers me that it seems like these kinds of reviews of the
larger patches don't happen until it's time to decide about the next
release. Perhaps these issues were all brought up seperately in prior
threads, or they weren't articulated as requirements or show-stoppers,
and if so then I apologize for not following those more closely.

If the approach Peter outlined is what core wants to see and is willing
to go along with to get SE-PostgreSQL included then let's please decide
that now and agree that unless some serious problem comes up we'll stick
to it and not require the whole thing be rewritten again later.

I'm not sure about KaiGai's feelings on this, but it strikes me that
adding SELinux support for the existing levels of access control in PG
might be straight-forward and small enough to include for 8.4 and would
show some commitment to this approach of "do it for PG, add SELinux
checks for it". Alternatively, maybe a progression-towards-SE-PostgreSQL
wiki/webpage that outlines the plan, current work, what's been
committed, etc, that everyone reviews and agrees to?

As a side-note, I've gotten some extremely positive feedback about
SE-PostgreSQL from folks in my organization who run systems where it
would be used. I'm going to be having a more detailed discussion later
today.

Thanks,

Stephen

> That being said, I'd think that if we do need different semantics from
> that for SE-PostgreSQL, we could implement it using a GUC or similar to
> keep the current behavior as well allow the SE-PostgreSQL behavior.

I don't think a GUC is what you need because you need both behaviors
simultaneously, one for MAC and the other for DAC. But I am sure
there must be some way to code around the problem.

...Robert

Robert Haas wrote:
> I haven't read the code, but from reading the docs, I have a feeling
> that right now the answer to both questions are NO, which means it
> doesn't really have a lot of value. One example of this is the
> pg_security system catalog. The catalog representation you're
> proposing is probably just fine for associating OIDs to SELinux
> security labels. But trying to present it as a general thing that
> some other security implementation could reuse just doesn't seem
> realistic. Who is to say that the next person who writes an enhanced
> security feature will want to use text as the representation for their
> security domains? It could just as easily be two integers, or an
> array of booleans. This is after all a database product, so the
> chances that someone would want to do something with structured data
> seem non-negligible.

Text represented security attribute is the most common format
for any other security mechanism also.
(For example, T-SOL also have its text representation.)
In addition, TEXT is the most flexible format. If any other
feature want to handle it as an array, it can be stored as
a text representation.

> In the end, you're going to have to be the one who makes the decision
> on which way to go. In some ways, I think that a plugin architecture
> would be better for everyone: we worry about the things on our side of
> the abstraction boundary, and you worry about the things on your side.
> Potentially you or someone else can release enhanced security plugins
> without needing any changes to core, and potentially on a different
> release cycle. On the other hand, a plugin architecture is probably
> going to be a lot of work. It seems that to install the plugin you
> would need to make system catalog changes as well as stuff additional
> system attributes into every table, which are relatively invasive
> changes. And you'll need to convince everyone that it's really a
> plug-in architecture and not just a special case for SE-PostgreSQL, so
> you'll need to prove that there are multiple viable clients, perhaps
> by backporting our existing DAC.

Please make clear the meaning of terms.
The 'plugin' means a loadable module which provides its own security
policy, doesn't it?

There is fundamental difference between built-in and plug-in.
The most important factor is where the hooks are deployed, and
what facility enables to manage security attribute.

Even if I implement SE-PostgreSQL as a loadable module, core
PostgreSQL has to provide proper hooks in strategic points and
facilities to manage security attribute (pg_security system catalog
and security_label system column).
If you require to implement it without these facilities, I think
it is impossible and prefer scraping PGACE and integrate SE- code
into core.

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

> Text represented security attribute is the most common format
> for any other security mechanism also.
> (For example, T-SOL also have its text representation.)
> In addition, TEXT is the most flexible format. If any other
> feature want to handle it as an array, it can be stored as
> a text representation.

Right, but that sort of misses the point of using a DATABASE. I could
store everything as text if I wanted to and skip using a database
altogether, but I don't want to. That's why I use a database.

> Please make clear the meaning of terms.
> The 'plugin' means a loadable module which provides its own security
> policy, doesn't it?

That is what I meant, yes.

> Even if I implement SE-PostgreSQL as a loadable module, core
> PostgreSQL has to provide proper hooks in strategic points and
> facilities to manage security attribute (pg_security system catalog
> and security_label system column).
> If you require to implement it without these facilities, I think
> it is impossible and prefer scraping PGACE and integrate SE- code
> into core.

I am not in a position to require anything since I am not a committer,
but I would think that you would need to convince people that the
facilities which your plugin requires were pretty much the same as the
facilities that any other future plugin might require - that the
plugin framework was client-agnostic.

...Robert

Stephen Frost wrote:
> * KaiGai Kohei (kaigai(at)kaigai(dot)gr(dot)jp) wrote:
>> 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?
>
> Well, just to be clear, ACLs are checked on tables under views, but
> they're checked using the privileges of the view owner rather than
> the privileges of the current user. I've run into that empirically
> because I've gotten 'permission denied' errors when using a view that
> I've clearly got full rights on but was owned by someone else (who
> didn't have rights on the table underneath).
>
> That being said, I'd think that if we do need different semantics from
> that for SE-PostgreSQL, we could implement it using a GUC or similar to
> keep the current behavior as well allow the SE-PostgreSQL behavior.

I think it is not reasonable.
If there are different philosophies, "one for one" seems to me
straight forward approach, for security especially.

>> However, we have to make clear whether the PGACE architecture
>> is incorrect, or not, at first.
>
> It really bothers me that it seems like these kinds of reviews of the
> larger patches don't happen until it's time to decide about the next
> release. Perhaps these issues were all brought up seperately in prior
> threads, or they weren't articulated as requirements or show-stoppers,
> and if so then I apologize for not following those more closely.
>
> If the approach Peter outlined is what core wants to see and is willing
> to go along with to get SE-PostgreSQL included then let's please decide
> that now and agree that unless some serious problem comes up we'll stick
> to it and not require the whole thing be rewritten again later.

As I noted, PGACE is not my goal.
I don't tremble to integrate SELinux related code into the core.

> I'm not sure about KaiGai's feelings on this, but it strikes me that
> adding SELinux support for the existing levels of access control in PG
> might be straight-forward and small enough to include for 8.4 and would
> show some commitment to this approach of "do it for PG, add SELinux
> checks for it". Alternatively, maybe a progression-towards-SE-PostgreSQL
> wiki/webpage that outlines the plan, current work, what's been
> committed, etc, that everyone reviews and agrees to?

Are you saying enlargement step-by-step, aren't you?
At least, it is far preferable to a death punishment.

I would like to here Joshua's opinion also.

> adding SELinux support for the existing levels of access control in PG

is

- table/column level access controls
- permission checks on database login
- permission checks on function invocation
- they need a facility to manage security label
- I want permission checks on loading a library,
though existing PG checks superuser() only.

and
- removing PGACE, integrate SEPG code into core
- permission checks on largeobjects is postponed
- row level security is postponed (NOT REJECTED!)
- so, writable system column is also postponed

If summary is necessary, I'll post it tommorow JST.

Because it is not a zero-based implementation, so I believe it can
be minimized within acceptable timescale.

> As a side-note, I've gotten some extremely positive feedback about
> SE-PostgreSQL from folks in my organization who run systems where it
> would be used. I'm going to be having a more detailed discussion later
> today.

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

KaiGai Kohei wrote:
>> I'm not sure about KaiGai's feelings on this, but it strikes me that
>> adding SELinux support for the existing levels of access control in PG
>> might be straight-forward and small enough to include for 8.4 and would
>> show some commitment to this approach of "do it for PG, add SELinux
>> checks for it". Alternatively, maybe a progression-towards-SE-PostgreSQL
>> wiki/webpage that outlines the plan, current work, what's been
>> committed, etc, that everyone reviews and agrees to?
>
> Are you saying enlargement step-by-step, aren't you?
> At least, it is far preferable to a death punishment.
>
> I would like to here Joshua's opinion also.

s/here/hear/g

Sorry,

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

>> Even if I implement SE-PostgreSQL as a loadable module, core
>> PostgreSQL has to provide proper hooks in strategic points and
>> facilities to manage security attribute (pg_security system catalog
>> and security_label system column).
>> If you require to implement it without these facilities, I think
>> it is impossible and prefer scraping PGACE and integrate SE- code
>> into core.
>
> I am not in a position to require anything since I am not a committer,
> but I would think that you would need to convince people that the
> facilities which your plugin requires were pretty much the same as the
> facilities that any other future plugin might require - that the
> plugin framework was client-agnostic.

We (as a security folks) know any MAC facility have similar
architecture called as reference monitor, so I believe it is
quite possible to implement them as same basis.
But it is a hard request to take an evidence immediately.
IMO, the framework is purely implementation matter, so it is
not late when the second one appeared.

As I noted to another message, I can accept to integrate limited
functional SE-PostgreSQL without any PGACE.

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

KaiGai Kohei wrote:
> Stephen Frost wrote:
>> * KaiGai Kohei (kaigai(at)kaigai(dot)gr(dot)jp) wrote:
>>> 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?
>> Well, just to be clear, ACLs are checked on tables under views, but
>> they're checked using the privileges of the view owner rather than
>> the privileges of the current user. I've run into that empirically
>> because I've gotten 'permission denied' errors when using a view that
>> I've clearly got full rights on but was owned by someone else (who
>> didn't have rights on the table underneath).
>>
>> That being said, I'd think that if we do need different semantics from
>> that for SE-PostgreSQL, we could implement it using a GUC or similar to
>> keep the current behavior as well allow the SE-PostgreSQL behavior.
>
> I think it is not reasonable.
> If there are different philosophies, "one for one" seems to me
> straight forward approach, for security especially.
>

When we talk about mandatory access control we are generally very clear that for
it to be mandatory and for the policy to be analyzable you must identify objects
unambiguously. For SE-Postgres this is an absolute necessity.

At the same time, if view-based ACL's are there, and people want them and use
them then that doesn't affect us (the people that want to use sepostgres). They
must be orthogonal and sepostgres must have a way of unambiguously labeling objects.

>>> However, we have to make clear whether the PGACE architecture
>>> is incorrect, or not, at first.
>> It really bothers me that it seems like these kinds of reviews of the
>> larger patches don't happen until it's time to decide about the next
>> release. Perhaps these issues were all brought up seperately in prior
>> threads, or they weren't articulated as requirements or show-stoppers,
>> and if so then I apologize for not following those more closely.
>>

As a new-comer to this discussion (and indeed this list/community) I find it
baffling that a patchset that has been around this long is just now having basic
architectural questions asked about it.

>> If the approach Peter outlined is what core wants to see and is willing
>> to go along with to get SE-PostgreSQL included then let's please decide
>> that now and agree that unless some serious problem comes up we'll stick
>> to it and not require the whole thing be rewritten again later.
>
> As I noted, PGACE is not my goal.
> I don't tremble to integrate SELinux related code into the core.
>

Flask (from which SELinux is derived) has the basic architecture of separating
enforcement from policy. This generally means that a security server (or
backend) knows what the policy is and how to calculate access vectors and it
passes those answers to the enforcement points. Flask itself is a framework. In
Linux and Xorg a more general framework was built that interfaces to flask,
because the flask security server wasn't seen as something that could implement
any kind of security policy (whether that is correct or not is a point of
contention, as the selinux security server already implements a kind of RBAC,
type enforcement and multilevel security).

PCACE is, architecturally, similar to LSM in the linux kernel and XACE in in
Xorg but is not an absolute necessity for SELinux integration.

>> I'm not sure about KaiGai's feelings on this, but it strikes me that
>> adding SELinux support for the existing levels of access control in PG
>> might be straight-forward and small enough to include for 8.4 and would
>> show some commitment to this approach of "do it for PG, add SELinux
>> checks for it". Alternatively, maybe a progression-towards-SE-PostgreSQL
>> wiki/webpage that outlines the plan, current work, what's been
>> committed, etc, that everyone reviews and agrees to?
>

If you look at the patches you'll see that the bulk of them are adding the
SELinux infrastructure. An access vector cache, the label translation functions,
the libselinux interface and so on. Over 2/3's of the patch is in
src/backend/security.

I'm not sure how much it would cut to remove row level access controls, but I do
have some points here. To me, row level access controls are the most important
part, this is the feature that lets us put secret and top secret data in the
same table and use the clients selinux context to decide what they can see,
transparently and without modification to the application. Without it you have
to use seperate postgres instances, modify the application to select from
different sets of tables depending on their label and so on.

That said, SELinux didn't start out as fine grained as it is today. A fair
number of object classes were present back in '99 when it started but more have
been added over time, the policy infrastructure allows that to happen, in fact
the policy allows object class/permissions to be added, removed and modified if
the access controls were inadequate or unnecessary.

> Are you saying enlargement step-by-step, aren't you?
> At least, it is far preferable to a death punishment.
>
> I would like to here Joshua's opinion also.
>

namespace collision...

>> adding SELinux support for the existing levels of access control in PG
>
> is
>
> - table/column level access controls
> - permission checks on database login
> - permission checks on function invocation
> - they need a facility to manage security label
> - I want permission checks on loading a library,
> though existing PG checks superuser() only.
>
> and
> - removing PGACE, integrate SEPG code into core
> - permission checks on largeobjects is postponed
> - row level security is postponed (NOT REJECTED!)
> - so, writable system column is also postponed
>
> If summary is necessary, I'll post it tommorow JST.
>
> Because it is not a zero-based implementation, so I believe it can
> be minimized within acceptable timescale.
>
>> As a side-note, I've gotten some extremely positive feedback about
>> SE-PostgreSQL from folks in my organization who run systems where it
>> would be used. I'm going to be having a more detailed discussion later
>> today.
>

It's good to hear that, I hope you'll ask them how important row level access
controls are.

Joshua Brindle <method(at)manicmethod(dot)com> writes:
> I'm not sure how much it would cut to remove row level access
> controls, but I do have some points here. To me, row level access
> controls are the most important part, this is the feature that lets us
> put secret and top secret data in the same table and use the clients
> selinux context to decide what they can see,

For me, the row-level access controls are really the sticking point.
There is absolutely nothing you can say that will convince me that they
don't break SQL in fundamental ways, and I also don't believe that it's
going to be possible to implement them without a constant stream of bugs
of omission and commission. (Those two points are not unrelated.)

Now that you've admitted that you aren't trying to accomplish the
equivalent sort of data hiding within the filesystem, the argument that
you have to have them seems fairly weak, certainly not strong enough to
justify the costs. We have already touched on some ways that you can
accomplish similar goals with just table- and column-level access
permissions, which are well understood and don't violate anyone's
expectations. There's probably a need to twiddle our permissions rules
for inheritance/partitioning cases, but that was already on the table
anyway for other reasons.

I could be persuaded to get behind a patch that does Peter's step #1
(ie, use SELinux permissions as an additional filter for existing SQL
permission checks). I don't believe I will ever think that row-level
checks are a good idea; as long as those are in the patch I will vote
against applying it.

regards, tom lane

Tom Lane wrote:
> Joshua Brindle <method(at)manicmethod(dot)com> writes:
>> I'm not sure how much it would cut to remove row level access
>> controls, but I do have some points here. To me, row level access
>> controls are the most important part, this is the feature that lets us
>> put secret and top secret data in the same table and use the clients
>> selinux context to decide what they can see,
>
> For me, the row-level access controls are really the sticking point.
> There is absolutely nothing you can say that will convince me that they
> don't break SQL in fundamental ways, and I also don't believe that it's
> going to be possible to implement them without a constant stream of bugs
> of omission and commission. (Those two points are not unrelated.)
>

This isn't going to be something that the vast majority of your users use. The
people that need them understand the ramifications of row filtering and the
absence of inaccessible rows won't be surprising.

> Now that you've admitted that you aren't trying to accomplish the
> equivalent sort of data hiding within the filesystem, the argument that

We apply access controls on reading and writing files, this is equivalent (in my
mind) to applying access controls on tuples, as they are the structured object
holding data (just like files).

> you have to have them seems fairly weak, certainly not strong enough to
> justify the costs. We have already touched on some ways that you can

The costs are nil for people who don't want this feature.

> accomplish similar goals with just table- and column-level access
> permissions, which are well understood and don't violate anyone's

I've already pointed out how table and column level access control don't provide
enough. Holding data of multiple classifications in a single table is something
of great concern. Unmodified applications need to be able to query out of a
table and get the data they are cleared to see. Column level access control
doesn't help because those same applications, of course, will need access to the
data in those columns.

Splitting data into different tables isn't an option in many cases because
applications aren't always configurable wrt to the database or tables they use.
Further, clients of multiple clearances should be able to use the same
application, and we can not trust the application to make the decision as to
which database or table is correct.

And even further, maintaining multiple sets of databases or tables that hold the
same kinds of information (and therefore have the same schema) is a maintenance,
backup and restoration issue.

> expectations. There's probably a need to twiddle our permissions rules
> for inheritance/partitioning cases, but that was already on the table
> anyway for other reasons.
>

partitions don't help because, once again, the application would be making the
determination about which partition to query. For mandatory access control that
we need in the kind of environments to work the application doesn't get to make
security relevant decisions, the database holds the data and needs to say who
can access it.

Further, partitioning isn't fine grained. I can't say user X can read secret
rows and read/write top secret rows and get that data out in a transparent way
(the applications would have to be aware of the partitions). Relabeling of data
also looks like a challenge with partitions (if I correctly understand how they
work).

> I could be persuaded to get behind a patch that does Peter's step #1
> (ie, use SELinux permissions as an additional filter for existing SQL
> permission checks). I don't believe I will ever think that row-level
> checks are a good idea; as long as those are in the patch I will vote
> against applying it.
>

We've already used postgresql in sensitive environments and had to make
compromises because of the lack of fine grained access control. We've been
telling customers for years that we hope postgresql will have said access
controls in the future and that it will help them solve many of the problems
they have.

We've been enthusiastic about the work KaiGai has been doing with respect to the
environments we operate in and it would be a shame for all that to be discarded.

Joshua Brindle <method(at)manicmethod(dot)com> writes:

> partitions don't help because, once again, the application would be making the
> determination about which partition to query.

Not necessarily since the database can be programmed to automatically put the
records into the right partition. Right now it's a pain but we're definitely
headed in that direction.

> Further, partitioning isn't fine grained. I can't say user X can read secret
> rows and read/write top secret rows and get that data out in a transparent way
> (the applications would have to be aware of the partitions). Relabeling of data
> also looks like a challenge with partitions (if I correctly understand how they
> work).

I think the "transparent" is the source of the problem. The application should
issue a query for the data it wants. It shouldn't "transparently" get magic
extra clauses attached to the query. That's where the SQL semantics are being
violated.

Row-level security isn't inherently a problem. It's just that the security is
affecting the data returned that's causing a problem.

I don't think partitioning is really the same thing as row-level security. But
I wonder if some of the same infrastructure could be used for both -- once we
have it.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training!

Gregory Stark <stark(at)enterprisedb(dot)com> writes:
> I don't think partitioning is really the same thing as row-level
> security.

Of course not, but it seems to me that it can be used to accomplish most
of the same practical use-cases. The main gripe about doing it via
partitioning is that the user's nose gets rubbed in the fact that there
can't be an enormous number of different security classifications in the
same table (since he has to explicitly make a partition for each one).
But the proposed implementation of row-level security would poop out
pretty darn quick for such a case, too, and frankly I'm not seeing an
application that would demand it.

regards, tom lane

On Wed, Jan 28, 2009 at 01:49:21PM -0500, Joshua Brindle wrote:
> use. The people that need them understand the ramifications of row
> filtering and the absence of inaccessible rows won't be surprising.

I wish there was even a little bit of evidence that users of a
security system may be relied upon to understand its implications and
effects. In my experience, however, they often don't.

>> you have to have them seems fairly weak, certainly not strong enough to
>> justify the costs. We have already touched on some ways that you can
>
> The costs are nil for people who don't want this feature.

That's also false, because developers who don't care about the feature
have to continue to maintain it as part of the system. If maintenance
were free, I suspect nobody would be objecting to the feature. But
this feature will in fact constrain future development and will impose
maintenance requirements on the programmers of the system. Those
maintenance requirements in turn amount to a cost that every user has
to pay, because time spent addressing issues that result from this
feature (or accommodating it in new development) is time that is not
spent on other problems users might face.

If I imagined I were project manager of the PostgreSQL project (a
preposterous supposition, let me be clear), then I'd be very worried
that this feature, which is apparently poorly understood by at least
one big contributor to the project, would amount to a significant drag
on future development work. In that case, I'd have to ask why having
this feature as part of the main line of PostgreSQL is a good
trade-off. Happily, I'm not someone who has to make that
determination, so I can't say whether it _is_ a good trade-off. But I
think that's what the resistance to the feature is all about, so
you'll need to make the case that the trade-off is a good one.

A

--
Andrew Sullivan
ajs(at)crankycanuck(dot)ca

Andrew Sullivan <ajs(at)crankycanuck(dot)ca> writes:
> On Wed, Jan 28, 2009 at 01:49:21PM -0500, Joshua Brindle wrote:
>> The costs are nil for people who don't want this feature.

> That's also false, because developers who don't care about the feature
> have to continue to maintain it as part of the system. If maintenance
> were free, I suspect nobody would be objecting to the feature. But
> this feature will in fact constrain future development and will impose
> maintenance requirements on the programmers of the system.

Right. The major implementation problem I have with row-level security
is that it will require sticking its hands into every part of the
backend; at least if you want it to be actually *secure* with no holes,
and if not I guess I'm failing to grasp the point. Every future patch
will have to be vetted to ensure that it's not accidentally breaking
that security. This stems directly from the fact that you're trying to
impose behavior that's fundamentally at odds with SQL, and therefore
there isn't any well-defined choke point at which you could apply the
checks and be done with it. The system simply isn't modularized that
way. (Of course we could throw it all away and start over...)

BTW, in regard to the upthread question about how much of the patch
could be discarded if we removed row-level security: having now taken
another look through it, I'd put the fraction at well north of 90%.
(That's exclusive of the security policy file, which I don't understand
at all and so can't tell how much might be specific to row security.)
What's worse, the current patch footprint is conservative because the
placement of hooks is simply wrong. You can't usefully apply checks in
simple_heap_insert, for example, since it has no idea who called it or
why. It's got to be done at a higher level and therefore in a lot more
places. And I don't see any attempt at all to restrict system-driven
fetches, yet surely there has to be some control over that (otherwise
why are we worrying about system-driven updates?)

regards, tom lane

Sorry for top-posting and avoiding to paste online doc URL.

Joshua, you sound like you're missing an important point. Sorry for
misunderstanding if that's not true.

Partitioning is supported in a way that a query does not need to know
about it, be it a SELECT, INSERT, UPDATE or DELETE. And 8.4 onwards,
some more.

What Tom is talking about is adding support for discarding partitions
based on GRANTed rights rather than WHERE clauses.

No application rewrite.

Columns obfuscating remains to be cared of: what about allowing views
as partitions?

HTH, regards,
--
dim

Le 28 janv. 09 à 19:49, Joshua Brindle <method(at)manicmethod(dot)com> a
écrit :

> Tom Lane wrote:
>> Joshua Brindle <method(at)manicmethod(dot)com> writes:
>>> I'm not sure how much it would cut to remove row level access
>>> controls, but I do have some points here. To me, row level access
>>> controls are the most important part, this is the feature that
>>> lets us
>>> put secret and top secret data in the same table and use the clients
>>> selinux context to decide what they can see,
>> For me, the row-level access controls are really the sticking point.
>> There is absolutely nothing you can say that will convince me that
>> they
>> don't break SQL in fundamental ways, and I also don't believe that
>> it's
>> going to be possible to implement them without a constant stream of
>> bugs
>> of omission and commission. (Those two points are not unrelated.)
>>
>
> This isn't going to be something that the vast majority of your
> users use. The people that need them understand the ramifications of
> row filtering and the absence of inaccessible rows won't be
> surprising.
>
>
>> Now that you've admitted that you aren't trying to accomplish the
>> equivalent sort of data hiding within the filesystem, the argument
>> that
>
>
> We apply access controls on reading and writing files, this is
> equivalent (in my mind) to applying access controls on tuples, as
> they are the structured object holding data (just like files).
>
>> you have to have them seems fairly weak, certainly not strong
>> enough to
>> justify the costs. We have already touched on some ways that you can
>
> The costs are nil for people who don't want this feature.
>
>> accomplish similar goals with just table- and column-level access
>> permissions, which are well understood and don't violate anyone's
>
> I've already pointed out how table and column level access control
> don't provide enough. Holding data of multiple classifications in a
> single table is something of great concern. Unmodified applications
> need to be able to query out of a table and get the data they are
> cleared to see. Column level access control doesn't help because
> those same applications, of course, will need access to the data in
> those columns.
>
> Splitting data into different tables isn't an option in many cases
> because applications aren't always configurable wrt to the database
> or tables they use. Further, clients of multiple clearances should
> be able to use the same application, and we can not trust the
> application to make the decision as to which database or table is
> correct.
>
> And even further, maintaining multiple sets of databases or tables
> that hold the same kinds of information (and therefore have the same
> schema) is a maintenance, backup and restoration issue.
>
>> expectations. There's probably a need to twiddle our permissions
>> rules
>> for inheritance/partitioning cases, but that was already on the table
>> anyway for other reasons.
>
> partitions don't help because, once again, the application would be
> making the determination about which partition to query. For
> mandatory access control that we need in the kind of environments to
> work the application doesn't get to make security relevant
> decisions, the database holds the data and needs to say who can
> access it.
>
> Further, partitioning isn't fine grained. I can't say user X can
> read secret rows and read/write top secret rows and get that data
> out in a transparent way (the applications would have to be aware of
> the partitions). Relabeling of data also looks like a challenge with
> partitions (if I correctly understand how they work).
>
>> I could be persuaded to get behind a patch that does Peter's step #1
>> (ie, use SELinux permissions as an additional filter for existing SQL
>> permission checks). I don't believe I will ever think that row-level
>> checks are a good idea; as long as those are in the patch I will vote
>> against applying it.
>
> We've already used postgresql in sensitive environments and had to
> make compromises because of the lack of fine grained access control.
> We've been telling customers for years that we hope postgresql will
> have said access controls in the future and that it will help them
> solve many of the problems they have.
>
> We've been enthusiastic about the work KaiGai has been doing with
> respect to the environments we operate in and it would be a shame
> for all that to be discarded.
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Jan 28, 2009 at 3:58 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Andrew Sullivan <ajs(at)crankycanuck(dot)ca> writes:
>> On Wed, Jan 28, 2009 at 01:49:21PM -0500, Joshua Brindle wrote:
>>> The costs are nil for people who don't want this feature.
>
>> That's also false, because developers who don't care about the feature
>> have to continue to maintain it as part of the system. If maintenance
>> were free, I suspect nobody would be objecting to the feature. But
>> this feature will in fact constrain future development and will impose
>> maintenance requirements on the programmers of the system.
>
> Right. The major implementation problem I have with row-level security
> is that it will require sticking its hands into every part of the
> backend; at least if you want it to be actually *secure* with no holes,
> and if not I guess I'm failing to grasp the point. Every future patch
> will have to be vetted to ensure that it's not accidentally breaking
> that security. This stems directly from the fact that you're trying to
> impose behavior that's fundamentally at odds with SQL, and therefore
> there isn't any well-defined choke point at which you could apply the
> checks and be done with it. The system simply isn't modularized that
> way. (Of course we could throw it all away and start over...)
>
> BTW, in regard to the upthread question about how much of the patch
> could be discarded if we removed row-level security: having now taken
> another look through it, I'd put the fraction at well north of 90%.
> (That's exclusive of the security policy file, which I don't understand
> at all and so can't tell how much might be specific to row security.)
> What's worse, the current patch footprint is conservative because the
> placement of hooks is simply wrong. You can't usefully apply checks in
> simple_heap_insert, for example, since it has no idea who called it or
> why. It's got to be done at a higher level and therefore in a lot more
> places. And I don't see any attempt at all to restrict system-driven
> fetches, yet surely there has to be some control over that (otherwise
> why are we worrying about system-driven updates?)

I'm not clear that I understand why it would be necessary for
row-level security to touch every part of the code. If the current
implementation requires that, then maybe that's a defect in the
implementation rather than an inherent problem with row-level
security. It seems to me that the crucial decision is "Where are we
trying to erect the security wall?". If we're trying to put it
between the client and the postgres backend, then maybe the right
thing to do is apply some sort of processing step to each query that
is submitted, essentially rewriting "SELECT * FROM a, b" as "SELECT *
FROM a, b WHERE you_can_see(a.securityid) AND
you_can_see(b.securityid)". Maybe you (Tom) still won't like this
because it breaks SQL semantics, but it seems like it would at least
centralize the code. Unless I'm totally off base here?

...Robert

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> For me, the row-level access controls are really the sticking point.
> There is absolutely nothing you can say that will convince me that they
> don't break SQL in fundamental ways, and I also don't believe that it's
> going to be possible to implement them without a constant stream of bugs
> of omission and commission. (Those two points are not unrelated.)

And, just to go full circle, row-level access controls are exactly what
the other enterprise RDBMSs have and is what is used in these security
circles today. One of the major issues, as I understand it, is to be
able to use stock applications with multiple security levels where the
application doesn't know (or care about) the security level. Doing that
through views and partitions and triggers and whatnot for each and every
application that is run on these systems will be a big hurdle to those
users, if it ends up being workable at all.

Thanks,

Stephen

Joshua Brindle wrote:
> Tom Lane wrote:
>> Joshua Brindle <method(at)manicmethod(dot)com> writes:
>>> I'm not sure how much it would cut to remove row level access
>>> controls, but I do have some points here. To me, row level access
>>> controls are the most important part, this is the feature that lets us
>>> put secret and top secret data in the same table and use the clients
>>> selinux context to decide what they can see,

Help me understand this.

It seems to me exactly as easy/hard to make sure that the secret and
top-secret rows are put into their appropriate partitions, as it is
to make sure that the secret and top-secret rows are tagged with the
right row-level-access-info.

If the idea is that the top-secret-row-inserter magically forces the
row-level tag "top-secret" it seems just as easy if the top-secret
row-inserter only has write permission to the "top-secret" partition
and not the others.

If the idea is that the less-secret-reader can't read rows
tagged "top-secret" it seems just as easy if the less-secret-reader
has no read access on the top-secret partition.

At first glance, partition level seems quite a bit easier to
manage in all the cases I can think of immediately.

> partitions don't help because, once again, the application would be
> making the determination about which partition to query. For mandatory

I think that's not true. I would hope that the application
queries the master table, and the SEPostgres ACLs prevent
any data coming from the inappropriate partitions.

> access control that we need in the kind of environments to work the
> application doesn't get to make security relevant decisions, the
> database holds the data and needs to say who can access it.
>
> Further, partitioning isn't fine grained. I can't say user X can read
> secret rows and read/write top secret rows and get that data out in a

Why not? It seems one can define the user with read access on the
partition with secret rows and read/write on top-secret rows.
Queries done on the master partition should get the data out in
a transparent way.

> transparent way (the applications would have to be aware of the
> partitions). Relabeling of data also looks like a challenge with
> partitions (if I correctly understand how they work).

ISTM we need to have a discussion of how partitioned tables
work - and what the postgres roadmap is.

If they can't yet, I think they should.

>> I could be persuaded to get behind a patch that does Peter's step #1
>> (ie, use SELinux permissions as an additional filter for existing SQL
>> permission checks). I don't believe I will ever think that row-level
>> checks are a good idea; as long as those are in the patch I will vote
>> against applying it.
>>
>
> We've already used postgresql in sensitive environments and had to make
> compromises because of the lack of fine grained access control. We've
> been telling customers for years that we hope postgresql will have said
> access controls in the future and that it will help them solve many of
> the problems they have.
>
> We've been enthusiastic about the work KaiGai has been doing with
> respect to the environments we operate in and it would be a shame for
> all that to be discarded.

AFACT there's nowhere near a consensus that it should be discarded (or
accepted); but rather that if the project can be split into two phases
parts of it could go in sooner.

There seems to be much less debate about the column/table/partition
level MAC parts.

Once those get in, I think the next valuable discussion would be
whether the fine-grained access control is best achieved through
improving the partition system or by adding the row-level acls as proposed.

Stephen Frost wrote:
> And, just to go full circle, row-level access controls are exactly what
> the other enterprise RDBMSs have and is what is used in these security
> circles today. One of the major issues, as I understand it, is to be
> able to use stock applications with multiple security levels where the
> application doesn't know (or care about) the security level. Doing that
> through views and partitions and triggers and whatnot for each and every
> application that is run on these systems will be a big hurdle to those
> users, if it ends up being workable at all.

That seems to me to be a shortcoming of the partition system and a good
TODO for the future partitioning improvements.

Why shouldn't be just as easy to make sure a row ends up in the
right partition as opposed to making sure it's tagged with right
row-level ACLs.

Ron Mayer wrote:
> Stephen Frost wrote:
>> And, just to go full circle, row-level access controls are exactly what
>> the other enterprise RDBMSs have and is what is used in these security
>> circles today. One of the major issues, as I understand it, is to be
>> able to use stock applications with multiple security levels where the
>> application doesn't know (or care about) the security level. Doing that
>> through views and partitions and triggers and whatnot for each and every
>> application that is run on these systems will be a big hurdle to those
>> users, if it ends up being workable at all.
>
> That seems to me to be a shortcoming of the partition system and a good
> TODO for the future partitioning improvements.
>
> Why shouldn't be just as easy to make sure a row ends up in the
> right partition as opposed to making sure it's tagged with right
> row-level ACLs.
>

In reality it isn't, unless postgres won't mind thousands of partitions being
made. In the military/gov implementations BLP lets you have hierarchical levels
and non-hierarchical categories. Since I linked to an article about it upthread
I assumed everyone participating was familiar but perhaps not. Typically there
are 3 levels, unclass, secret, top secret. In addition to those 3 levels there
may be a few, hundreds or even thousands of categories. Those categories apply
to each of the levels so if you are using 1000 categories you have 3*1000
possible BLP labels. On top of that SELinux has users, roles and types, which
are all also multiplied.

There is a reason we don't do things like have 3 filesystems, one for unclass,
one for secret and one for top secret.

Partitions/views may be a good way to implement row-level access control when
there are a few different options but they won't necessarily be known ahead of
time. We don't want to encode policy logic in to the database itself, that is
why we have a separate security server that handles the policy. The system
should scale to any number of contexts that are available on the system.

Relabeling with partitions (that is, moving things from one partition to
another) seems to be handled with triggers, which is inconvenient to the
application developers but also devastating to the security system (unless you
can map triggers back to who did the operation that caused them), and managing
partitions on dozens or hundreds of tables and moving data between them in those
tables seems at the very least challenging.

I also have questions concerning partitions. Perhaps someone wants to use
partitions for a functional purpose (eg., to split log entries by month/year).
What would be the affect of concurrently using partitions to split security
contexts out?

Nonetheless, this conversation seems moot now that Tom has walled off and won't
even discuss row-level access controls.

Cheers.

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> I don't believe I will ever think that row-level checks are a good idea; as
> long as those are in the patch I will vote against applying it.

I think we're conflating two behaviours here.

The data hiding behaviour clearly changes the semantics of queries in ways
that make a lot of deductions about the data incorrect. That's a pretty severe
problem which would cause massive consequences down the road.

The imposing of security restrictions based on the data in the row isn't
really in the same league. I'm not sure I see any semantic problems with it at
all.

It's still true that it would be quite invasive to Postgres to implement. The
current security restrictions are all checked and determined statically based
on the query. Once a query is planned we can execute it without checking any
security constraints at run-time.

I don't think raising partitioning makes a useful contribution to this
discussion. Firstly, it's not like partitioning doesn't change SQL semantics
in its own way anyways. Even aside from that, partitioning is largely about
the location that data is stored. Forcing data to be stored in different
physical places just because our security model is based on the place the data
is stored is kind of silly.

Unless perhaps we implement partitioning which supports having many partitions
share the same underlying, er, physical partition. But then I don't see how
that's any different from row-level permissions.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's RemoteDBA services!

Joshua Brindle wrote:
> Nonetheless, this conversation seems moot now that Tom has walled off
> and won't even discuss row-level access controls.

I think that's a bit of an overstatement.

He says he's against them[1] and he says that they are the sticking
point on this patch[2], and that they break SQL[2] and that he believes
that implementations of row level acls he can imagine would be buggy[2].

Elsewhere other people on the core team are suggesting that
others want to see SQL-level row permissions[3].

My reading of the discussion is that row-level access controls aren't
vetoed permanently, but rather that (a) it's still clear what SQL
semantics they'll break, (b) the implementations discussed so far
seem at high risk of bugs to some people, and (c) some people haven't
been sold on the need for them. None of those necessarily state
that the feature will never get into postgres; but it makes it sound
like a really high bar to jump over for a release that was originally
scheduled to be done a while ago.

[1] http://archives.postgresql.org/pgsql-hackers/2009-01/msg02389.php
[2] http://archives.postgresql.org/pgsql-hackers/2009-01/msg02339.php
[3] http://archives.postgresql.org/pgsql-hackers/2009-01/msg02391.php

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> I'm not clear that I understand why it would be necessary for
> row-level security to touch every part of the code.

OK, I admit it's not "every" part, just every place that fetches,
inserts, updates, or deletes tuples. There are quite a few ;-)

As an example, the present patch imagines that it will have adequate
control over inserts by putting hooks into simple_heap_insert and the
(rather small number of) places that call heap_insert directly. But
there are dozens of calls of simple_heap_insert and no way for the
function itself to know why it is being called or on whose behalf.
The patch's hook function tries to work around the fact that it hasn't
got that information by means of heuristics. Aside from the question of
whether there are bugs in those heuristics today (I'm certain there
are), every time we accept a patch that adds another call of
simple_heap_insert, we're going to have to revisit the hook to see
if it needs to be twiddled.

Another problem is that since the hook only knows the parameters to
simple_heap_insert plus global state (such as current_user), it can't
cope very well with security-related context changes. We have already
heard that situations involving views are simply broken in the patch as
it stands --- row-level permissions are checked against current_user
and not the view owner, and there's no good way to fix that.

> It seems to me that the crucial decision is "Where are we
> trying to erect the security wall?". If we're trying to put it
> between the client and the postgres backend, then maybe the right
> thing to do is apply some sort of processing step to each query that
> is submitted, essentially rewriting "SELECT * FROM a, b" as "SELECT *
> FROM a, b WHERE you_can_see(a.securityid) AND
> you_can_see(b.securityid)". Maybe you (Tom) still won't like this
> because it breaks SQL semantics, but it seems like it would at least
> centralize the code.

Well, it wouldn't break SQL semantics from the point of view of the
planner, so that's one demerit taken off the books. However, I seem
to recall that exactly that approach was taken in a older version of
the patch (many moons ago) and we found fatal problems in it too.

The "where's the wall" point is a good one. I think part of the issue
is that the patch is to some extent trying to erect a security wall
that's between the data and large parts of the backend C code. Which is
an interesting idea and maybe could have been made to work if it'd been
designed in from the beginning, but to retrofit it today seems pretty
impractical.

regards, tom lane

Tom Lane wrote:
> As an example, the present patch imagines that it will have adequate
> control over inserts by putting hooks into simple_heap_insert and the
> (rather small number of) places that call heap_insert directly. But
> there are dozens of calls of simple_heap_insert and no way for the
> function itself to know why it is being called or on whose behalf.
> The patch's hook function tries to work around the fact that it hasn't
> got that information by means of heuristics. Aside from the question of
> whether there are bugs in those heuristics today (I'm certain there
> are), every time we accept a patch that adds another call of
> simple_heap_insert, we're going to have to revisit the hook to see
> if it needs to be twiddled.
>
> Another problem is that since the hook only knows the parameters to
> simple_heap_insert plus global state (such as current_user), it can't
> cope very well with security-related context changes. We have already
> heard that situations involving views are simply broken in the patch as
> it stands --- row-level permissions are checked against current_user
> and not the view owner, and there's no good way to fix that.
>

Leaving aside any other issues, it seems to me that the chance of
remedying these defects reasonably in a couple of weeks is just about nil.

That leaves the following questions: can the row-level part of the patch
be separated out, and if so how easily, and is what would be left worth
doing?

cheers

andrew

Joshua Brindle <method(at)manicmethod(dot)com> writes:
> In reality it isn't, unless postgres won't mind thousands of
> partitions being made. In the military/gov implementations BLP lets
> you have hierarchical levels and non-hierarchical categories. Since I
> linked to an article about it upthread I assumed everyone
> participating was familiar but perhaps not. Typically there are 3
> levels, unclass, secret, top secret. In addition to those 3 levels
> there may be a few, hundreds or even thousands of categories. Those
> categories apply to each of the levels so if you are using 1000
> categories you have 3*1000 possible BLP labels. On top of that SELinux
> has users, roles and types, which are all also multiplied.

Hmm. If that's the expected application environment then the patch as
proposed has fatal performance problems anyway, for lack of a way to
get rid of no-longer-referenced pg_security rows. We had been led to
understand that there wouldn't be all that many distinct labels in use,
but this seems to imply that there are going to be $bignum of them.
That changes pg_security leakage from a can-live-with-for-first-cut
issue to a must-fix-to-be-credible issue.

(Just for context, thousands of partitions isn't practical with our
current implementation, but might be in the future.)

> Nonetheless, this conversation seems moot now that Tom has walled off
> and won't even discuss row-level access controls.

You realize, I trust, that I don't have the only vote around here.
But I am convinced that the row-level-security aspect of this proposal
is far less than fully baked, and isn't going to become so in the time
frame available for 8.4.

regards, tom lane

Good morning, I started to follow the discussion.
(Time difference is unconfortable for me!)

>> adding SELinux support for the existing levels of access control in PG
>
> is
>
> - table/column level access controls
> - permission checks on database login
> - permission checks on function invocation
> - they need a facility to manage security label
> - I want permission checks on loading a library,
> though existing PG checks superuser() only.
>
> and
> - removing PGACE, integrate SEPG code into core
> - permission checks on largeobjects is postponed
> - row level security is postponed (NOT REJECTED!)
> - so, writable system column is also postponed

If I postponed a part of functionalities as Stephen suggested,
how many lines can be reduced? It is a quick estimation.

Currently, the main patch has:
110 files changed, 9813 insertions(+), 16 deletions(-), 924 modifications(!)

* src/backend/commands/copy.c | 293 +++!
Most of them are to support writable system column,
so about -300 lines are expected.
* src/backend/executor/execMain.c | 209 +++
Most of them are to support writable system column,
so about -200 lines are expected
* src/backend/security/pgaceCommon.c | 729 ++++++++++++
It will get scraped, but management of security attribute
has to SELinux specific code, so -250 lines are expected
* src/backend/security/pgaceHooks.c | 1547 ++++++++++++++++++++++++++
It will be gone, so -1550 lines are expected
* src/backend/security/rowacl/rowacl.c | 721 ++++++++++++
It will be postponed, -700 lines are expected
* src/backend/security/sepgsql/hooks.c | 1019 +++++++++++++++++
A part of permission checks (aka row,blob) is postponed,
so -300 lines are expected.
* src/include/security/pgace.h | 181 +++
* src/include/security/rowacl.h | 41
It will be gone, so -200 lines are expected

At the total, -3,200 lines are expected.
In addition, any other small-sized stuffs can be postponed.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

Tom Lane wrote:
> Joshua Brindle <method(at)manicmethod(dot)com> writes:
>> In reality it isn't, unless postgres won't mind thousands of
>> partitions being made. In the military/gov implementations BLP lets
>> you have hierarchical levels and non-hierarchical categories. Since I
>> linked to an article about it upthread I assumed everyone
>> participating was familiar but perhaps not. Typically there are 3
>> levels, unclass, secret, top secret. In addition to those 3 levels
>> there may be a few, hundreds or even thousands of categories. Those
>> categories apply to each of the levels so if you are using 1000
>> categories you have 3*1000 possible BLP labels. On top of that SELinux
>> has users, roles and types, which are all also multiplied.
>
> Hmm. If that's the expected application environment then the patch as

It's one application environment, maybe not a common one but one I've seen.

> proposed has fatal performance problems anyway, for lack of a way to
> get rid of no-longer-referenced pg_security rows. We had been led to
> understand that there wouldn't be all that many distinct labels in use,
> but this seems to imply that there are going to be $bignum of them.
> That changes pg_security leakage from a can-live-with-for-first-cut
> issue to a must-fix-to-be-credible issue.

I see. SELinux handles this by dynamically filling a sidtab (context to security
identifier table) at runtime. This can be done because the contexts are stored
as strings in the xattr's. I understand KaiGai wanted to save space in the
tables so he stored sids, which would require a persistent sid mapping.

I'm not sure this is a deal breaker though, I still know people that would
rather have it now that may have performance issues than wait for something that
can prune the sid mappings.

>
> (Just for context, thousands of partitions isn't practical with our
> current implementation, but might be in the future.)
>

There are still other unresolved issues with partitions that I pointed out upthread.

>> Nonetheless, this conversation seems moot now that Tom has walled off
>> and won't even discuss row-level access controls.
>
> You realize, I trust, that I don't have the only vote around here.
> But I am convinced that the row-level-security aspect of this proposal
> is far less than fully baked, and isn't going to become so in the time
> frame available for 8.4.
>

I know, but I also know how opensource communities work :) I haven't seen any
enthusiastic support from other commiters which means, at least for the time
being, it's being tabled.

I mean no disrespect, I also agree with your assessment about the maintenance
burden. As an opensource developer myself I would be nervous about intrusive
patches that I don't understand, and what that means for me when I write new
patches.

This brings up my next point. The purpose of pgace is to mitigate this issue.
The core team only needs to know when/where/why data can be selected, updated,
deleted, etc and maintain the pgace hooks. As long as your side of the boundary
stays correct the security model shouldn't break. Granted if you add new things
(a new loadable module system, new kinds of large objects, etc) those would need
to have the hooks added from the get go.

This is similar to how the Linux kernel works, and the Xorg's XACE works. We do
watch for updates and sometimes people mess things up but it isn't hard to spot.
Especially with a small core team like postgres I'd think you guys could
successfully spot places access controls need to be added.

Andrew Dunstan wrote:
> Tom Lane wrote:
>> As an example, the present patch imagines that it will have adequate
>> control over inserts by putting hooks into simple_heap_insert and the
>> (rather small number of) places that call heap_insert directly. But
>> there are dozens of calls of simple_heap_insert and no way for the
>> function itself to know why it is being called or on whose behalf.
>> The patch's hook function tries to work around the fact that it hasn't
>> got that information by means of heuristics. Aside from the question of
>> whether there are bugs in those heuristics today (I'm certain there
>> are), every time we accept a patch that adds another call of
>> simple_heap_insert, we're going to have to revisit the hook to see
>> if it needs to be twiddled.
>>
>> Another problem is that since the hook only knows the parameters to
>> simple_heap_insert plus global state (such as current_user), it can't
>> cope very well with security-related context changes. We have already
>> heard that situations involving views are simply broken in the patch as
>> it stands --- row-level permissions are checked against current_user
>> and not the view owner, and there's no good way to fix that.
>>
>
> Leaving aside any other issues, it seems to me that the chance of
> remedying these defects reasonably in a couple of weeks is just about nil.
>
> That leaves the following questions: can the row-level part of the patch
> be separated out, and if so how easily, and is what would be left worth
> doing?

At least, I believe the row-level access control make PostgreSQL more
attractive in security aspect like a few major commercial dbms, and
its tradeoff (PK/FK covert channel, optimization issue) should be
a decision by end-users.

However, I can accept an opinion the row-level facilities can be
separated and postponed, then, step-by-step enlargement approach,
as Peter & Stephen suggested.

Even if we don't have row-level at v8.4, column-level MAC has its
(may be limited) worth, for example, a column to store credit-card
number never visible from web application context.

I found a proverbial phrase in my dictionaly:
Between two stools you fall to the ground.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

Obviously, we cannot make clear state of the row-level access
controls by the date of v8.4 freeze.

I agree the row-level access controls can be separated and
postponed to v8.5 development cycle.

So, I'll cut off a few part of previous patches for v8.4.
Stephen Frost gave me a guideline about what should be remained
or postponed *now*. It is SE-PG feature covers granularity
existing PG facility enables to provide.
(Typically, table/column level checks without row level)

* The following features are still included for v8.4 (stage #01)
- Centralized SELinux policy and getpeercon(3).
- Table/Column level access controls.
Keep the current behavior.
It checks client's privileges and raises error, if violated.

- Functions, Databases
The places to make decision are controversial.
Is pg_xxx_aclcheck() possible to add SELinx check?
I'll check it later.

- Permission checks on shared library file
The vanilla PG trusts superuser, but MAC don't trust him.
So, we have to check the library's attribute. It should not
be postponed because a malicious library once loaded can do
anything internally.

* The following features are postponed for v8.5
- Row-level access controls
- Binary-large-objects access controls
- Writable system column
Because Row-level one is postponed, we don't need to export/import
security_label of tuples to/from users now.

* The following features are scraped.
- PGACE security framework

As I estimated in the previous message, scale of the main patch
will be 6000-7000 lines. I'll pay my highest efforts to show the
stripped patches within 5 days, due to Feb 04 JST.

Thanks,

| ---- Road Map (My plan) ----
|
| * The stage#1 patches.
V
---- PostgreSQL v8.4 ----
|
| * Platform independent row-level access controls
| * Writable system column (security_label, security_acl).
| Maybe, we can also discuss writable "oid" in same time.
V
---- First CommitFest ----
|
| * SE-PostgreSQL row-level access controls.
V
---- Second CommitFest ----
|
| * Rest of features like binary large objects.
V
---- Third CommitFest ----
|
:
V
---- PostgreSQL v8.5 ----

Ron Mayer wrote:
> Joshua Brindle wrote:
>> Nonetheless, this conversation seems moot now that Tom has walled off
>> and won't even discuss row-level access controls.
>
> I think that's a bit of an overstatement.
>
> He says he's against them[1] and he says that they are the sticking
> point on this patch[2], and that they break SQL[2] and that he believes
> that implementations of row level acls he can imagine would be buggy[2].
>
> Elsewhere other people on the core team are suggesting that
> others want to see SQL-level row permissions[3].
>
>
> My reading of the discussion is that row-level access controls aren't
> vetoed permanently, but rather that (a) it's still clear what SQL
> semantics they'll break, (b) the implementations discussed so far
> seem at high risk of bugs to some people, and (c) some people haven't
> been sold on the need for them. None of those necessarily state
> that the feature will never get into postgres; but it makes it sound
> like a really high bar to jump over for a release that was originally
> scheduled to be done a while ago.
>
> [1] http://archives.postgresql.org/pgsql-hackers/2009-01/msg02389.php
> [2] http://archives.postgresql.org/pgsql-hackers/2009-01/msg02339.php
> [3] http://archives.postgresql.org/pgsql-hackers/2009-01/msg02391.php

--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

On Wed, Jan 28, 2009 at 6:34 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> As an example, the present patch imagines that it will have adequate
> control over inserts by putting hooks into simple_heap_insert and the
> (rather small number of) places that call heap_insert directly. But
> there are dozens of calls of simple_heap_insert and no way for the
> function itself to know why it is being called or on whose behalf.
> The patch's hook function tries to work around the fact that it hasn't
> got that information by means of heuristics. Aside from the question of
> whether there are bugs in those heuristics today (I'm certain there
> are), every time we accept a patch that adds another call of
> simple_heap_insert, we're going to have to revisit the hook to see
> if it needs to be twiddled.

I agree that that's no good.

> Another problem is that since the hook only knows the parameters to
> simple_heap_insert plus global state (such as current_user), it can't
> cope very well with security-related context changes. We have already
> heard that situations involving views are simply broken in the patch as
> it stands --- row-level permissions are checked against current_user
> and not the view owner, and there's no good way to fix that.

I thought that was intentional, and I sort of think that it's the
right decision.

>> It seems to me that the crucial decision is "Where are we
>> trying to erect the security wall?". If we're trying to put it
>> between the client and the postgres backend, then maybe the right
>> thing to do is apply some sort of processing step to each query that
>> is submitted, essentially rewriting "SELECT * FROM a, b" as "SELECT *
>> FROM a, b WHERE you_can_see(a.securityid) AND
>> you_can_see(b.securityid)". Maybe you (Tom) still won't like this
>> because it breaks SQL semantics, but it seems like it would at least
>> centralize the code.
>
> Well, it wouldn't break SQL semantics from the point of view of the
> planner, so that's one demerit taken off the books. However, I seem
> to recall that exactly that approach was taken in a older version of
> the patch (many moons ago) and we found fatal problems in it too.

Can you (or someone) provide a pointer to the archives? I can't
immediately see any reason why that problem wouldn't be fixable.

> The "where's the wall" point is a good one. I think part of the issue
> is that the patch is to some extent trying to erect a security wall
> that's between the data and large parts of the backend C code. Which is
> an interesting idea and maybe could have been made to work if it'd been
> designed in from the beginning, but to retrofit it today seems pretty
> impractical.

Agreed. With all respect to Kaigai-san, I am suspicious that some of
this may be unintentional. It may be that cleaning this up and
putting the wall in one well-defined spot will allay a number of your
concerns.

...Robert

On Wed, Jan 28, 2009 at 6:57 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Hmm. If that's the expected application environment then the patch as
> proposed has fatal performance problems anyway, for lack of a way to
> get rid of no-longer-referenced pg_security rows. We had been led to
> understand that there wouldn't be all that many distinct labels in use,
> but this seems to imply that there are going to be $bignum of them.
> That changes pg_security leakage from a can-live-with-for-first-cut
> issue to a must-fix-to-be-credible issue.

It's worth noting that this is yet another thing that is mostly a
problem in the context of row-level security. It seems to me that if
security labels are only applied to tables and columns, then it will
be possible to scan the whole database relatively quickly and find all
the labels that are still in use, probably without breaking a sweat.
On the other hand, when you have row-level security, it gets a lot
harder.

I'm wondering if this problem could be solved with a sort of
mark-and-sweep garbage collection: add a boolean column `used' to
pg_security (which I really think out to be renamed to
pg_selinux_context or something, and make a new table if we someday
support Trusted Solaris or whatever). Whenever you do an OID lookup
and find a row that says "false", do a non-transactional update and
change it to say "true".

Then you can write something which goes through and sets all the rows
to false and then visits every row of every table in the database and
forces OID lookups on the security ID of each. When you get done, any
rows that still say false are unreferenced and can be killed.

Also... even if there are thousands of contexts, it only matters to
the extent that there is a lot of churn, and I'm not sure whether
that's something that is expected. Josh Brindle, any thoughts?

> (Just for context, thousands of partitions isn't practical with our
> current implementation, but might be in the future.)
>
>> Nonetheless, this conversation seems moot now that Tom has walled off
>> and won't even discuss row-level access controls.
>
> You realize, I trust, that I don't have the only vote around here.
> But I am convinced that the row-level-security aspect of this proposal
> is far less than fully baked, and isn't going to become so in the time
> frame available for 8.4.

You have a pretty big vote, though. But, I see signs though that you
might relent at some future point when and if the proposal is
well-baked. In any case, whether or not we do row-level security at
some point in the future, you've certainly convinced me that the
issues have not been thought through in enough detail to consider
including any of this in 8.4.

...Robert

> I found a proverbial phrase in my dictionaly:
> Between two stools you fall to the ground.

LOL. I like that one - and it's very apt.

...Robert

KaiGai,

* KaiGai Kohei (kaigai(at)ak(dot)jp(dot)nec(dot)com) wrote:
> Obviously, we cannot make clear state of the row-level access
> controls by the date of v8.4 freeze.

Indeed, I have to agree that's where things are headed, which is
certainly unfortunate but I think it's good that we were able to provide
some additional feedback on what the PG community is looking for.

> I agree the row-level access controls can be separated and
> postponed to v8.5 development cycle.

Having row-level security would be very nice in 8.5.

> So, I'll cut off a few part of previous patches for v8.4.
> Stephen Frost gave me a guideline about what should be remained
> or postponed *now*. It is SE-PG feature covers granularity
> existing PG facility enables to provide.
> (Typically, table/column level checks without row level)

This feels like a pretty reasonable compromise to me, and was based on
Peter's suggestion of an overall plan, but I'm not a committer and I
havn't seen any response to my request that they comment on that plan.
I'm sure your enthusiasm is appreciated though, and regardless of its
inclusion in 8.4, one of Peter's points was to develop the patches as
smaller changes that build on each other anyway.

Thanks,

Stephen

Robert,

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> pg_security (which I really think out to be renamed to
> pg_selinux_context or something, and make a new table if we someday
> support Trusted Solaris or whatever).

Err, this doesn't really make sense if we're doing row-level security,
that's not something which is tied to SELinux or Trusted Solaris. Of
course, it's likely we'll need such a pg_selinux_context table or
something too.. Or maybe pg_security can be pg_rls instead. Just
wanted to avoid confusion over this point.. Assuming Peter's approach
is the path that is generally agreed upon by core..

Thanks,

Stephen

Robert Haas wrote:
> On Wed, Jan 28, 2009 at 6:57 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Then you can write something which goes through and sets all the rows
> to false and then visits every row of every table in the database and
> forces OID lookups on the security ID of each. When you get done, any
> rows that still say false are unreferenced and can be killed.
>
> Also... even if there are thousands of contexts, it only matters to
> the extent that there is a lot of churn, and I'm not sure whether
> that's something that is expected. Josh Brindle, any thoughts?
>

I wouldn't expect a whole lot of churn. Maybe when a project is archived you'd
grab everything with a particular security context, save it off and remove it
from the table. Constant relabels or removals based on context don't seem too
likely though.

Robert Haas wrote:
> On Wed, Jan 28, 2009 at 6:34 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> As an example, the present patch imagines that it will have adequate
>> control over inserts by putting hooks into simple_heap_insert and the
>> (rather small number of) places that call heap_insert directly. But
>> there are dozens of calls of simple_heap_insert and no way for the
>> function itself to know why it is being called or on whose behalf.
>> The patch's hook function tries to work around the fact that it hasn't
>> got that information by means of heuristics. Aside from the question of
>> whether there are bugs in those heuristics today (I'm certain there
>> are), every time we accept a patch that adds another call of
>> simple_heap_insert, we're going to have to revisit the hook to see
>> if it needs to be twiddled.
>
> I agree that that's no good.

My concern is that superuser is allowed to modify system catalog
by hand, like:

UPDATE pg_proc SET probin = '/tmp/malicious_library.so'
WHERE oid = ...;

It is logically same as ALTER FUNCTION.

Even if I remove a hook from simple_heap_xxxx(), it is necessary
to check queries from clients.

>> Another problem is that since the hook only knows the parameters to
>> simple_heap_insert plus global state (such as current_user), it can't
>> cope very well with security-related context changes. We have already
>> heard that situations involving views are simply broken in the patch as
>> it stands --- row-level permissions are checked against current_user
>> and not the view owner, and there's no good way to fix that.
>
> I thought that was intentional, and I sort of think that it's the
> right decision.

From the viewpoint of SELinux, it is not a matter because core
PostgreSQL does not change client's security context.
But, it is indeed controversial for Row-level ACLs.
(We have a time about this issue.)

>>> It seems to me that the crucial decision is "Where are we
>>> trying to erect the security wall?". If we're trying to put it
>>> between the client and the postgres backend, then maybe the right
>>> thing to do is apply some sort of processing step to each query that
>>> is submitted, essentially rewriting "SELECT * FROM a, b" as "SELECT *
>>> FROM a, b WHERE you_can_see(a.securityid) AND
>>> you_can_see(b.securityid)". Maybe you (Tom) still won't like this
>>> because it breaks SQL semantics, but it seems like it would at least
>>> centralize the code.
>> Well, it wouldn't break SQL semantics from the point of view of the
>> planner, so that's one demerit taken off the books. However, I seem
>> to recall that exactly that approach was taken in a older version of
>> the patch (many moons ago) and we found fatal problems in it too.
>
> Can you (or someone) provide a pointer to the archives? I can't
> immediately see any reason why that problem wouldn't be fixable.

IIRC, 0racle or M$ has a patent to rewrite WHERE clause for security
purpose, so Tom suggested it should be implemented using a hook
deployed within executor.
At least, it also enables code more simple.

>> The "where's the wall" point is a good one. I think part of the issue
>> is that the patch is to some extent trying to erect a security wall
>> that's between the data and large parts of the backend C code. Which is
>> an interesting idea and maybe could have been made to work if it'd been
>> designed in from the beginning, but to retrofit it today seems pretty
>> impractical.
>
> Agreed. With all respect to Kaigai-san, I am suspicious that some of
> this may be unintentional. It may be that cleaning this up and
> putting the wall in one well-defined spot will allay a number of your
> concerns.

A wall between the data and _backend C code_ is not my intention.

Its purpose is a wall between the data and clients including superuser
via SQL queries. It is a basic assumption we cannot acquire any accesses
from system internal entities, as SELinux do nothing for kernel loadable
modules.

However, please note that making a decision at more hot point is more
good design, because it enables to reduce potential bypasses.
At the begining, I choosed simple_heap_xxxx() as a most hot point.
However, it can be replacable, if we can find any other place without
omission and consistent.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

* KaiGai Kohei (kaigai(at)ak(dot)jp(dot)nec(dot)com) wrote:
>> I agree that that's no good.

As do I.

> My concern is that superuser is allowed to modify system catalog
> by hand, like:
>
> UPDATE pg_proc SET probin = '/tmp/malicious_library.so'
> WHERE oid = ...;

That UPDATE still goes through permissions checking, and that checking
even includes an explicit check when system catalogs are involved.
Appropriate hooks in that permission checking could prevent this from
ever being allowed.

> It is logically same as ALTER FUNCTION.

Sure, but I think it's straight-forward to make a case for "don't update
the system catalogs when you're running SE-PostgreSQL, use the
appropriate ALTER commands", and then remove the ability to do so when
SE-PostgreSQL is enabled.

>> Can you (or someone) provide a pointer to the archives? I can't
>> immediately see any reason why that problem wouldn't be fixable.
>
> IIRC, 0racle or M$ has a patent to rewrite WHERE clause for security
> purpose, so Tom suggested it should be implemented using a hook
> deployed within executor.
> At least, it also enables code more simple.

It'd probably be Oracle.. I'm not a big fan of that approach anyway
though, although I don't have any particular reason beyond 'it feels
kludgy' at the moment.

Thanks,

Stephen

Robert Haas wrote:
> On Wed, Jan 28, 2009 at 6:57 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Hmm. If that's the expected application environment then the patch as
>> proposed has fatal performance problems anyway, for lack of a way to
>> get rid of no-longer-referenced pg_security rows. We had been led to
>> understand that there wouldn't be all that many distinct labels in use,
>> but this seems to imply that there are going to be $bignum of them.
>> That changes pg_security leakage from a can-live-with-for-first-cut
>> issue to a must-fix-to-be-credible issue.
>
> It's worth noting that this is yet another thing that is mostly a
> problem in the context of row-level security. It seems to me that if
> security labels are only applied to tables and columns, then it will
> be possible to scan the whole database relatively quickly and find all
> the labels that are still in use, probably without breaking a sweat.
> On the other hand, when you have row-level security, it gets a lot
> harder.

Yes, I think we shoule not ignore upcoming feature, even if it is
not on time of the next release.

> I'm wondering if this problem could be solved with a sort of
> mark-and-sweep garbage collection: add a boolean column `used' to
> pg_security (which I really think out to be renamed to
> pg_selinux_context or something, and make a new table if we someday
> support Trusted Solaris or whatever). Whenever you do an OID lookup
> and find a row that says "false", do a non-transactional update and
> change it to say "true".

It seems to me reference-counter is more preferable than boolean,
at least. But it makes performance pain on writer access when it
is expanded to row-level security.

> Then you can write something which goes through and sets all the rows
> to false and then visits every row of every table in the database and
> forces OID lookups on the security ID of each. When you get done, any
> rows that still say false are unreferenced and can be killed.
>
> Also... even if there are thousands of contexts, it only matters to
> the extent that there is a lot of churn, and I'm not sure whether
> that's something that is expected. Josh Brindle, any thoughts?

Unless the security policy writer got crazy, it would be unrealistic.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

On Wed, Jan 28, 2009 at 9:27 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> Robert,
>
> * Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
>> pg_security (which I really think out to be renamed to
>> pg_selinux_context or something, and make a new table if we someday
>> support Trusted Solaris or whatever).
>
> Err, this doesn't really make sense if we're doing row-level security,
> that's not something which is tied to SELinux or Trusted Solaris. Of
> course, it's likely we'll need such a pg_selinux_context table or
> something too.. Or maybe pg_security can be pg_rls instead. Just
> wanted to avoid confusion over this point.. Assuming Peter's approach
> is the path that is generally agreed upon by core..

I don't think there's anything about pg_security that is specific to
row-level security.

...Robert

> My concern is that superuser is allowed to modify system catalog
> by hand, like:
>
> UPDATE pg_proc SET probin = '/tmp/malicious_library.so'
> WHERE oid = ...;
>
> It is logically same as ALTER FUNCTION.
>
> Even if I remove a hook from simple_heap_xxxx(), it is necessary
> to check queries from clients.

That's a valid concern, I think all we're saying here is that you need
to find a better place to block that, maybe by assigning pg_proc an
security label that prevents modification by the superuser.

> IIRC, 0racle or M$ has a patent to rewrite WHERE clause for security
> purpose, so Tom suggested it should be implemented using a hook
> deployed within executor.
> At least, it also enables code more simple.

Patents suck. I guess we need to understand this issue better before
we can make any decisions.

> A wall between the data and _backend C code_ is not my intention.
>
> Its purpose is a wall between the data and clients including superuser
> via SQL queries. It is a basic assumption we cannot acquire any accesses
> from system internal entities, as SELinux do nothing for kernel loadable
> modules.
>
> However, please note that making a decision at more hot point is more
> good design, because it enables to reduce potential bypasses.
> At the begining, I choosed simple_heap_xxxx() as a most hot point.
> However, it can be replacable, if we can find any other place without
> omission and consistent.

That's why I was thinking about the planner...

...Robert

Robert Haas wrote:
> On Wed, Jan 28, 2009 at 9:27 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>> Robert,
>>
>> * Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
>>> pg_security (which I really think out to be renamed to
>>> pg_selinux_context or something, and make a new table if we someday
>>> support Trusted Solaris or whatever).
>> Err, this doesn't really make sense if we're doing row-level security,
>> that's not something which is tied to SELinux or Trusted Solaris. Of
>> course, it's likely we'll need such a pg_selinux_context table or
>> something too.. Or maybe pg_security can be pg_rls instead. Just
>> wanted to avoid confusion over this point.. Assuming Peter's approach
>> is the path that is generally agreed upon by core..
>
> I don't think there's anything about pg_security that is specific to
> row-level security.

Yes, SELinux requires any objects (not only tuples) to be labeled.
The pg_security is also necessary for tables/columns/...

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

On Wed, Jan 28, 2009 at 10:15 PM, KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
> It seems to me reference-counter is more preferable than boolean,
> at least. But it makes performance pain on writer access when it
> is expanded to row-level security.

A reference counter will never work. You could easily end up
serializing all access to the database around the row-level lock on
one popular security context. That is a performance problem two or
three orders of magnitude worse than anything that has been suggested
so far in connection with this feature, and probably six orders of
magnitude worse than the problem you're trying to solve (pg_security,
or whatever we want to call it, getting too big).

For the record, I think Tom's concern in this area is largely
off-base, especially in light of the fact that Joshua Brindle and
Kaigai both agree that churn is not likely to be large. I think we
need some kind of plausible way to clean out the table, but I don't
think it needs to be fully automated or super-efficient, just
something that someone could do if they felt the need.

...Robert

Robert Haas wrote:
> On Wed, Jan 28, 2009 at 10:15 PM, KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
>> It seems to me reference-counter is more preferable than boolean,
>> at least. But it makes performance pain on writer access when it
>> is expanded to row-level security.
>
> A reference counter will never work. You could easily end up
> serializing all access to the database around the row-level lock on
> one popular security context. That is a performance problem two or
> three orders of magnitude worse than anything that has been suggested
> so far in connection with this feature, and probably six orders of
> magnitude worse than the problem you're trying to solve (pg_security,
> or whatever we want to call it, getting too big).

Yes, I don't think it is a good approach also.
Just I though it was relatively preferable than boolean.

> For the record, I think Tom's concern in this area is largely
> off-base, especially in light of the fact that Joshua Brindle and
> Kaigai both agree that churn is not likely to be large. I think we
> need some kind of plausible way to clean out the table, but I don't
> think it needs to be fully automated or super-efficient, just
> something that someone could do if they felt the need.

If pg_security is actually overflowed or makes compress storages
by unused ones, I recommend to stop the system and run a utility
to reclaim them. But I don't think it actually happen.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

Robert Haas wrote:
>> My concern is that superuser is allowed to modify system catalog
>> by hand, like:
>>
>> UPDATE pg_proc SET probin = '/tmp/malicious_library.so'
>> WHERE oid = ...;
>>
>> It is logically same as ALTER FUNCTION.
>>
>> Even if I remove a hook from simple_heap_xxxx(), it is necessary
>> to check queries from clients.
>
> That's a valid concern, I think all we're saying here is that you need
> to find a better place to block that, maybe by assigning pg_proc an
> security label that prevents modification by the superuser.

On SE-PostgreSQL, we have two kind of superuser:
1. A superuser with privileged domain for ALTER FUNCTION.
2. A superuser with unprivileged domain for ALTER FUNCTION.

SE-PostgreSQL also allows (1) to modify pg_proc by hand,
becuase security policy allows it.
(But, OS feature can block someone untrusted (like web app) to
translate into privileged domain.)

Stephen's suggestion (deny to update all the system catalog) seems
to me a bit rough. I don't make sure there is no application which
depends on superuser is writable to system catalog.

In addition, this limitation is not based on security policy.

So, I think we have to deploy a hook on ExecUpdate() at least,
simple_heap_update() aside.
If we cannot obtain enough information from context, we can
apply possible maximum permissions here.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

Tom Lane wrote:
> Gregory Stark <stark(at)enterprisedb(dot)com> writes:
> > I don't think partitioning is really the same thing as row-level
> > security.
>
> Of course not, but it seems to me that it can be used to accomplish most
> of the same practical use-cases. The main gripe about doing it via
> partitioning is that the user's nose gets rubbed in the fact that there
> can't be an enormous number of different security classifications in the
> same table (since he has to explicitly make a partition for each one).
> But the proposed implementation of row-level security would poop out
> pretty darn quick for such a case, too, and frankly I'm not seeing an
> application that would demand it.

OK, putting on my crazy idea hat, if we split the primary and foreign
keys by partition, it would give us polyinstantiation:

http://en.wikipedia.org/wiki/Polyinstantiation

because our unique indexes do not apply across partitions.
Polyinstantiation is a desirable security feature and one that would be
tough to implement without partitions.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Robert Haas wrote:
> On Wed, Jan 28, 2009 at 6:57 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Hmm. If that's the expected application environment then the patch as
> > proposed has fatal performance problems anyway, for lack of a way to
> > get rid of no-longer-referenced pg_security rows. We had been led to
> > understand that there wouldn't be all that many distinct labels in use,
> > but this seems to imply that there are going to be $bignum of them.
> > That changes pg_security leakage from a can-live-with-for-first-cut
> > issue to a must-fix-to-be-credible issue.
>
> It's worth noting that this is yet another thing that is mostly a
> problem in the context of row-level security. It seems to me that if
> security labels are only applied to tables and columns, then it will
> be possible to scan the whole database relatively quickly and find all
> the labels that are still in use, probably without breaking a sweat.
> On the other hand, when you have row-level security, it gets a lot
> harder.

If we are not labeling every row, why not just use a TEXT column without
using an OID to reference pg_security; there aren't that places,
pg_class, pg_attribute, etc, i.e. they are not on every data row.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> Tom Lane wrote:
>> Gregory Stark <stark(at)enterprisedb(dot)com> writes:
>>> I don't think partitioning is really the same thing as row-level
>>> security.
>> Of course not, but it seems to me that it can be used to accomplish most
>> of the same practical use-cases. The main gripe about doing it via
>> partitioning is that the user's nose gets rubbed in the fact that there
>> can't be an enormous number of different security classifications in the
>> same table (since he has to explicitly make a partition for each one).
>> But the proposed implementation of row-level security would poop out
>> pretty darn quick for such a case, too, and frankly I'm not seeing an
>> application that would demand it.
>
> OK, putting on my crazy idea hat, if we split the primary and foreign
> keys by partition, it would give us polyinstantiation:
>
> http://en.wikipedia.org/wiki/Polyinstantiation
>
> because our unique indexes do not apply across partitions.
> Polyinstantiation is a desirable security feature and one that would be
> tough to implement without partitions.

The issue is not such a simple one.

SELinux allows to change its security context, if user has
appropriate privileges (relabelfrom and relabelto).
When a tuple (label=Secret, PK=123) is inserted then it is
relabeled to (label=TopSecret, PK=123), it has to return an
error due to the PK confliction, even if he does not have
permission to refer it.

I guess you already noticed that the previous security research
report about polyinstantiation implicitly assumes a fixed form
security policy, like "higher can read lower", "higher cannot
write lower" and so on.
But, SELinux's security policy does not assume anything.
It always returns its decision based on flexible security policy.

So, polyinstantiation is very difficult technology. :-(
(And will be overspec for enterprise class purpose.)

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

FYI, I have created a wiki page that summerizes the SE-PostgreSQL
information we have gathered so far:

http://wiki.postgresql.org/wiki/SEPostgreSQL-patch

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> Robert Haas wrote:
>> On Wed, Jan 28, 2009 at 6:57 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Hmm. If that's the expected application environment then the patch as
>>> proposed has fatal performance problems anyway, for lack of a way to
>>> get rid of no-longer-referenced pg_security rows. We had been led to
>>> understand that there wouldn't be all that many distinct labels in use,
>>> but this seems to imply that there are going to be $bignum of them.
>>> That changes pg_security leakage from a can-live-with-for-first-cut
>>> issue to a must-fix-to-be-credible issue.
>> It's worth noting that this is yet another thing that is mostly a
>> problem in the context of row-level security. It seems to me that if
>> security labels are only applied to tables and columns, then it will
>> be possible to scan the whole database relatively quickly and find all
>> the labels that are still in use, probably without breaking a sweat.
>> On the other hand, when you have row-level security, it gets a lot
>> harder.
>
> If we are not labeling every row, why not just use a TEXT column without
> using an OID to reference pg_security; there aren't that places,
> pg_class, pg_attribute, etc, i.e. they are not on every data row.

We should not assume every row are not labeled forever, at least.
It will lose expandability soon.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

On Wed, Jan 28, 2009 at 9:49 PM, KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:

> IIRC, 0racle or M$ has a patent to rewrite WHERE clause for security
> purpose, so Tom suggested it should be implemented using a hook
> deployed within executor.

Yes, it was Oracle. There are a couple newer revisions, but they're all
based primarily on Patent #6487552, Database Fine-grained Access Control,
Filed Oct 5, 1998/Issued Nov 26, 2002. The patent covers defining a
security context, retrieving-defined policies from that context, and
applying those policies by directly calling a security-context-related
stored procedure in the WHERE clause as well as dynamically adding
security-related predicates to the WHERE-clause.

--
Jonah H. Harris, Senior DBA
myYearbook.com

Robert Haas <robertmhaas(at)gmail(dot)com> writes:

> I'm wondering if this problem could be solved with a sort of
> mark-and-sweep garbage collection:
>...
> Then you can write something which goes through and sets all the rows
> to false and then visits every row of every table in the database and
> forces OID lookups on the security ID of each. When you get done, any
> rows that still say false are unreferenced and can be killed.

This sounds awfully similar to the bitmap index vacuum problem. I wonder if
security labels could be implemented as some kind of funky special index.

Just thinking out loud. I don't have a well-formed idea based on this.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's 24x7 Postgres support!

Bruce Momjian wrote:
> Tom Lane wrote:
>> Gregory Stark <stark(at)enterprisedb(dot)com> writes:
>>> I don't think partitioning is really the same thing as row-level
>>> security.
>> Of course not, but it seems to me that it can be used to accomplish most
>> of the same practical use-cases. The main gripe about doing it via
>> partitioning is that the user's nose gets rubbed in the fact that there
>> can't be an enormous number of different security classifications in the
>> same table (since he has to explicitly make a partition for each one).
>> But the proposed implementation of row-level security would poop out
>> pretty darn quick for such a case, too, and frankly I'm not seeing an
>> application that would demand it.
>
> OK, putting on my crazy idea hat, if we split the primary and foreign
> keys by partition, it would give us polyinstantiation:
>
> http://en.wikipedia.org/wiki/Polyinstantiation
>
> because our unique indexes do not apply across partitions.
> Polyinstantiation is a desirable security feature and one that would be
> tough to implement without partitions.
>

Polyinstantiation in this manner won't do it I don't think (if I'm understanding
you correctly). As KaiGai already said, SELinux policy is flexible so we'll have
more than just BLP policy to worry about.

Also a top secret user will need to see all rows when he selects, and they
should still have unique keys. He won't be able to write to secret or unclass
rows but he'll be able to see them.

Further, a secret user may be able to see top secret rows, but not some of the
columns, going back to my coordinates example. He'd have to use a trusted stored
procedure that can do the necessary operations to fuzz the data.

A flexible policy is really the crux of this. We aren't trying to reproduce what
other DBMS's have done, we are trying to integrate with a completely flexible
policy from the ground up.

> > I don't think partitioning is really the same thing as row-level security.
>
> Of course not, but it seems to me that it can be used to accomplish most
> of the same practical use-cases. The main gripe about doing it via
> partitioning is that the user's nose gets rubbed in the fact that there
> can't be an enormous number of different security classifications in the
> same table (since he has to explicitly make a partition for each one).

Imho a useful partitioning feature that is worth extra syntax additions
will have to include the ability to automatically create partitions on demand
(and maybe remove empty ones during vacuum).
(I have refrained from discussing partitioning until now, because I thought
this is not the time. But the certainty with which manual creation
is implied here makes me nervous.)

I short it (imho) requires a partitioning clause (much like a group by clause in sql)
and optionally an expression to produce a partition name (+ maybe for the nostalgic
a tablespace name mapping expression).

If partitioning for row level sec includes a sec column as proposed,
I think the two could be combined as a means for performance optimization.
But I am not sure partitioning alone can efficiently replace the sec column approach.
(especially in the admittedly unlikely >100 sec label scenario).
(When a constraint says the partition only contains visible security labels,
the sec check can be done at the partition level (including CE for denied labels))

Andreas

Joshua Brindle wrote:
> Bruce Momjian wrote:
> > Tom Lane wrote:
> >> Gregory Stark <stark(at)enterprisedb(dot)com> writes:
> >>> I don't think partitioning is really the same thing as row-level
> >>> security.
> >> Of course not, but it seems to me that it can be used to accomplish most
> >> of the same practical use-cases. The main gripe about doing it via
> >> partitioning is that the user's nose gets rubbed in the fact that there
> >> can't be an enormous number of different security classifications in the
> >> same table (since he has to explicitly make a partition for each one).
> >> But the proposed implementation of row-level security would poop out
> >> pretty darn quick for such a case, too, and frankly I'm not seeing an
> >> application that would demand it.
> >
> > OK, putting on my crazy idea hat, if we split the primary and foreign
> > keys by partition, it would give us polyinstantiation:
> >
> > http://en.wikipedia.org/wiki/Polyinstantiation
> >
> > because our unique indexes do not apply across partitions.
> > Polyinstantiation is a desirable security feature and one that would be
> > tough to implement without partitions.
> >
>
> Polyinstantiation in this manner won't do it I don't think (if I'm understanding
> you correctly). As KaiGai already said, SELinux policy is flexible so we'll have
> more than just BLP policy to worry about.
>
> Also a top secret user will need to see all rows when he selects, and they
> should still have unique keys. He won't be able to write to secret or unclass
> rows but he'll be able to see them.

Yea, it would take some work but it is an idea.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

> Yea, it would take some work but it is an idea.

It's *an* idea,yes. But it introduces as many (or more) problems than
it solves.

--Josh

Josh Berkus wrote:
>
> > Yea, it would take some work but it is an idea.
>
> It's *an* idea,yes. But it introduces as many (or more) problems than
> it solves.

Ah, but my problems might be easier solved than the row-level permission
problems. ;-)

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> Josh Berkus wrote:
>>> Yea, it would take some work but it is an idea.
>> It's *an* idea,yes. But it introduces as many (or more) problems than
>> it solves.
>
> Ah, but my problems might be easier solved than the row-level permission
> problems. ;-)
>

Or might not. Multi-partition indexes? Multi-partition uniqueness?
Automated moving of rows between partitions?

--Josh

Josh Berkus wrote:
> Bruce Momjian wrote:
> > Josh Berkus wrote:
> >>> Yea, it would take some work but it is an idea.
> >> It's *an* idea,yes. But it introduces as many (or more) problems than
> >> it solves.
> >
> > Ah, but my problems might be easier solved than the row-level permission
> > problems. ;-)
> >
>
> Or might not. Multi-partition indexes? Multi-partition uniqueness?
> Automated moving of rows between partitions?

Are you trying to make some kind of point?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> Josh Berkus wrote:
>> Bruce Momjian wrote:
>>> Josh Berkus wrote:
>>>>> Yea, it would take some work but it is an idea.
>>>> It's *an* idea,yes. But it introduces as many (or more) problems than
>>>> it solves.
>>> Ah, but my problems might be easier solved than the row-level permission
>>> problems. ;-)
>>>
>> Or might not. Multi-partition indexes? Multi-partition uniqueness?
>> Automated moving of rows between partitions?
>
> Are you trying to make some kind of point?
>

IMVHO Josh was describing a nice-to-have TODO list for a partitions feature
in general. :-) Maybe he was saying that when they partitioning feature
is designed that they try to think of polyinstantiation as they design it :-)

Bruce,

> Are you trying to make some kind of point?
>

Yeah, that we're certainly not doing any of this for 8.4.

If we're going for radical new approaches for row-level, why not also
look at the VIEWS approach? If we worked out the same problems we need
to fix for Bernd's patch, using automated manatory views to enforce
row-level access is also plausible.

--Josh

Josh Berkus wrote:
> Bruce,
>
> > Are you trying to make some kind of point?
> >
>
> Yeah, that we're certainly not doing any of this for 8.4.
>
> If we're going for radical new approaches for row-level, why not also
> look at the VIEWS approach? If we worked out the same problems we need
> to fix for Bernd's patch, using automated manatory views to enforce
> row-level access is also plausible.

Sure, we can explore that too.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Joshua, Kohei-san,

So, for 8.4: *if* we included in 8.4 a version of SEPostgres with all
features *except* row-level security, would it still be useful to the
SELinux community?

I think we're just not going to work out the headache-inducing issues
around row-level security in time for 8.4, and it seems to me that
integrated system-level security labels at the table-and-column level
are still very useful, even without row-level security.

--Josh

On Fri, Jan 30, 2009 at 5:37 PM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> Bruce,
>> Are you trying to make some kind of point?
>>
>
> Yeah, that we're certainly not doing any of this for 8.4.
>
> If we're going for radical new approaches for row-level, why not also look
> at the VIEWS approach? If we worked out the same problems we need to fix
> for Bernd's patch, using automated manatory views to enforce row-level
> access is also plausible.

I'm rather enchanted with the idea of using table partitioning to
implement row-level security, but the obstacles seem rather
formidable. Right now, a partitioned relation behaves nothing like a
regular relation, and to use it for this purpose you'd need to make it
transparent. IOW, you'd need to be able to define indices that
spanned multiple partitions (including enforcement of unique
constraints), you'd need to be able to make foreign keys that could
point to a row in arbitrary subset of the partitions, you'd need
automatic creation and deletion of partitions, you'd need better
planner support for partitions, and you'd need to somehow deal with
the issue of pg_class bloat. Plus, to make it truly transparent,
you'd need multiple layers of partitioning, in case someone wanted to
do row-level security and range partitioning simultaneously.

Now, the plus side is that if we could do all of that, we'd have the
infrastructure to support some truly awesome partitioning stuff, and
not just row-level security. But it seems awfully hard.

...Robert

Josh Berkus wrote:
> Joshua, Kohei-san,
>
> So, for 8.4: *if* we included in 8.4 a version of SEPostgres with all
> features *except* row-level security, would it still be useful to the
> SELinux community?
>
> I think we're just not going to work out the headache-inducing issues
> around row-level security in time for 8.4, and it seems to me that
> integrated system-level security labels at the table-and-column level
> are still very useful, even without row-level security.
>
>

Hasn't a plan for this already been posted? See
http://archives.postgresql.org/pgsql-hackers/2009-01/msg02407.php

cheers

andrew

* Andrew Dunstan (andrew(at)dunslane(dot)net) wrote:
> Josh Berkus wrote:
>> So, for 8.4: *if* we included in 8.4 a version of SEPostgres with all
>> features *except* row-level security, would it still be useful to the
>> SELinux community?
>>
>> I think we're just not going to work out the headache-inducing issues
>> around row-level security in time for 8.4, and it seems to me that
>> integrated system-level security labels at the table-and-column level
>> are still very useful, even without row-level security.

I tend to agree that they will be very useful. I'm not sure there will
be much adoption without row-level in the security community though, to
be honest. I'd like to see it as part of an overall plan to eventually
do row-level support. Given the size of this overall work and feature
set, I think it's appropriate to do it in a staged manner regardless.

> Hasn't a plan for this already been posted? See
> http://archives.postgresql.org/pgsql-hackers/2009-01/msg02407.php

Sure, that's a plan, but Josh's question is certainly appropriate.

Thanks,

Stephen

Josh Berkus wrote:
> Joshua, Kohei-san,
>
> So, for 8.4: *if* we included in 8.4 a version of SEPostgres with all
> features *except* row-level security, would it still be useful to the
> SELinux community?

Yes, obviously.

I think the granularity of access controls is an aspect of security.

> I think we're just not going to work out the headache-inducing issues
> around row-level security in time for 8.4, and it seems to me that
> integrated system-level security labels at the table-and-column level
> are still very useful, even without row-level security.

For example, table-and-column level access control can provide such a
worth which enables to store customer's credit-card-number within
unaccessable column from all the web application (children of Apache)
but accessable from settlement system (child of crond).
It enables to prevent SQL injection to steal very sensitive info.

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

Andrew Dunstan wrote:
>
>
> Josh Berkus wrote:
>> Joshua, Kohei-san,
>>
>> So, for 8.4: *if* we included in 8.4 a version of SEPostgres with all
>> features *except* row-level security, would it still be useful to the
>> SELinux community?
>>
>> I think we're just not going to work out the headache-inducing issues
>> around row-level security in time for 8.4, and it seems to me that
>> integrated system-level security labels at the table-and-column level
>> are still very useful, even without row-level security.
>
> Hasn't a plan for this already been posted? See
> http://archives.postgresql.org/pgsql-hackers/2009-01/msg02407.php

FYI:

* previous full-functional SE-PostgreSQL/Row-ACLs

[kaigai(at)fedora10 security]$ wc -l *.c */*.c
729 pgaceCommon.c
1547 pgaceHooks.c
721 rowacl/rowacl.c
1200 sepgsql/avc.c
623 sepgsql/core.c
1019 sepgsql/hooks.c
785 sepgsql/permissions.c
1097 sepgsql/proxy.c
7721 total

* A lite SE-PostgreSQL without row-level security,
large object support, writable system column

[kaigai(at)fedora10 sepgsql]$ wc -l *.c
904 checker.c
1181 avc.c
360 core.c
55 dummy.c
683 hooks.c
478 label.c
553 perms.c
4214 total

Today, I'll debug the modified code...
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

KaiGai Kohei wrote:
> > Hasn't a plan for this already been posted? See
> > http://archives.postgresql.org/pgsql-hackers/2009-01/msg02407.php
>
> FYI:
>
> * previous full-functional SE-PostgreSQL/Row-ACLs
>
> [kaigai(at)fedora10 security]$ wc -l *.c */*.c
> 729 pgaceCommon.c
> 1547 pgaceHooks.c
> 721 rowacl/rowacl.c
> 1200 sepgsql/avc.c
> 623 sepgsql/core.c
> 1019 sepgsql/hooks.c
> 785 sepgsql/permissions.c
> 1097 sepgsql/proxy.c
> 7721 total
>
> * A lite SE-PostgreSQL without row-level security,
> large object support, writable system column
>
> [kaigai(at)fedora10 sepgsql]$ wc -l *.c
> 904 checker.c
> 1181 avc.c
> 360 core.c
> 55 dummy.c
> 683 hooks.c
> 478 label.c
> 553 perms.c
> 4214 total
>
> Today, I'll debug the modified code...

Wow, that was fast. Where are you storing the security information for
tables and columns? Did you add a special column to pg_class, etc?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> KaiGai Kohei wrote:
>>> Hasn't a plan for this already been posted? See
>>> http://archives.postgresql.org/pgsql-hackers/2009-01/msg02407.php
>> FYI:
>>
>> * previous full-functional SE-PostgreSQL/Row-ACLs
>>
>> [kaigai(at)fedora10 security]$ wc -l *.c */*.c
>> 729 pgaceCommon.c
>> 1547 pgaceHooks.c
>> 721 rowacl/rowacl.c
>> 1200 sepgsql/avc.c
>> 623 sepgsql/core.c
>> 1019 sepgsql/hooks.c
>> 785 sepgsql/permissions.c
>> 1097 sepgsql/proxy.c
>> 7721 total
>>
>> * A lite SE-PostgreSQL without row-level security,
>> large object support, writable system column
>>
>> [kaigai(at)fedora10 sepgsql]$ wc -l *.c
>> 904 checker.c
>> 1181 avc.c
>> 360 core.c
>> 55 dummy.c
>> 683 hooks.c
>> 478 label.c
>> 553 perms.c
>> 4214 total
>>
>> Today, I'll debug the modified code...
>
> Wow, that was fast. Where are you storing the security information for
> tables and columns? Did you add a special column to pg_class, etc?

Security information is stored within padding field of HeapTupleHeader
as we did. It can be fetched via sepgsql_(table|column|...)_getcon()
functions, and can be set via SECURITY_LABEL = 'xxx'.

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

KaiGai Kohei wrote:
> >> Today, I'll debug the modified code...
> >
> > Wow, that was fast. Where are you storing the security information for
> > tables and columns? Did you add a special column to pg_class, etc?
>
> Security information is stored within padding field of HeapTupleHeader
> as we did. It can be fetched via sepgsql_(table|column|...)_getcon()
> functions, and can be set via SECURITY_LABEL = 'xxx'.

Well, we are not using row-level security values so why not store it in
its own column regular or as part of the existing ACL structure. I
think it will be very odd for system tables to have this special column
but not user rows.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> KaiGai Kohei wrote:
>>>> Today, I'll debug the modified code...
>>> Wow, that was fast. Where are you storing the security information for
>>> tables and columns? Did you add a special column to pg_class, etc?
>> Security information is stored within padding field of HeapTupleHeader
>> as we did. It can be fetched via sepgsql_(table|column|...)_getcon()
>> functions, and can be set via SECURITY_LABEL = 'xxx'.
>
> Well, we are not using row-level security values so why not store it in
> its own column regular or as part of the existing ACL structure. I
> think it will be very odd for system tables to have this special column
> but not user rows.

Sorry, my description might easily make confusion.
I read it again myself, indeed, it makes confusion. :(

SECURITY_LABEL = 'xxx' means following sytle:

CREATE TABLE t (
a int,
b text SECURITY_LABEL = '...'
) SECURITY_LABEL = '...';

I don't provide both of "security_label" and "security_acl"
system columns for system/user tables.
I didn't write it explicitly, it might make you confusing.

User cannot see what security label is assigned to them
due to lack of system column, so new sepgsql_xxx_getcon()
functions are provided an interface to see security label.

In this patch, I don't touch new system columns.

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

KaiGai,

* KaiGai Kohei (kaigai(at)kaigai(dot)gr(dot)jp) wrote:
> I don't provide both of "security_label" and "security_acl"
> system columns for system/user tables.
> I didn't write it explicitly, it might make you confusing.
>
> User cannot see what security label is assigned to them
> due to lack of system column, so new sepgsql_xxx_getcon()
> functions are provided an interface to see security label.
>
> In this patch, I don't touch new system columns.

I think Bruce's question was where you stored the security_acl and
security_label columns. Based on your response (and a bit of purusal
through the code.google site), it looks like you still have security_acl
and security_label defined as internal columns and being included
for at least system tables (or is it everywhere?). I think what people
are looking for, instead, is either additional columns to just the
existing system tables that need them (eg: pg_class, pg_attribute) or
included in the existing ACL structure (the aclitem structure). Another
option might be a seperate set of tables.

This would further reduce the patch pretty significantly, I suspect,
though you will need to rework some things. In terms of minimally
invasive, I would guess modifying the existing ACL structure for the ACL
info, and a seperate table to track the labels for different
objects/sub-objects (similar to pg_depend) would be your best approach.
That would require no changes to existing system tables, but a few
changes in places where the ACL is handled, and then the hooks in the
right places to do the permission checking.

Just my 2c.

Thanks,

Stephen

Stephen Frost wrote:
> KaiGai,
>
> * KaiGai Kohei (kaigai(at)kaigai(dot)gr(dot)jp) wrote:
>> I don't provide both of "security_label" and "security_acl"
>> system columns for system/user tables.
>> I didn't write it explicitly, it might make you confusing.
>>
>> User cannot see what security label is assigned to them
>> due to lack of system column, so new sepgsql_xxx_getcon()
>> functions are provided an interface to see security label.
>>
>> In this patch, I don't touch new system columns.
>
> I think Bruce's question was where you stored the security_acl and
> security_label columns. Based on your response (and a bit of purusal
> through the code.google site), it looks like you still have security_acl
> and security_label defined as internal columns and being included
> for at least system tables (or is it everywhere?).

In the current working tree, it (security id) is stored within
padding field of HeapTupleHeader, so internal facility can read
it via HeapTupleGetSecLabel(), but I already removed "security_acl"
and "security_label" definition.
Its basic structure is unchanged, the text form of security label
is stored within pg_security system catalog.

> I think what people
> are looking for, instead, is either additional columns to just the
> existing system tables that need them (eg: pg_class, pg_attribute) or
> included in the existing ACL structure (the aclitem structure). Another
> option might be a seperate set of tables.

It should not be held in text form within each tuples.
Please remind why I rework for the feature again.
Its facilities are a bit large to get included at a time,
so its development should be done step-by-step approach and
separatable ones (row-level, largeobjects) are postponed.
This change is extreamly large, almost same as implementing
from zero. I think it is far from step-by-step.

> This would further reduce the patch pretty significantly, I suspect,
> though you will need to rework some things.

No, it need to rework *any* things. :(

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

KaiGai,

* KaiGai Kohei (kaigai(at)kaigai(dot)gr(dot)jp) wrote:
> Stephen Frost wrote:
> > I think Bruce's question was where you stored the security_acl and
> > security_label columns. Based on your response (and a bit of purusal
> > through the code.google site), it looks like you still have security_acl
> > and security_label defined as internal columns and being included
> > for at least system tables (or is it everywhere?).
>
> In the current working tree, it (security id) is stored within
> padding field of HeapTupleHeader, so internal facility can read
> it via HeapTupleGetSecLabel(), but I already removed "security_acl"
> and "security_label" definition.
> Its basic structure is unchanged, the text form of security label
> is stored within pg_security system catalog.

I'm pretty sure that structure is part of what people were unhappy about
though, and what makes it a much more invasive change that violates
certain levels in the system by requiring information at much lower
levels than it had before.

> > I think what people
> > are looking for, instead, is either additional columns to just the
> > existing system tables that need them (eg: pg_class, pg_attribute) or
> > included in the existing ACL structure (the aclitem structure). Another
> > option might be a seperate set of tables.
>
> It should not be held in text form within each tuples.
> Please remind why I rework for the feature again.
> Its facilities are a bit large to get included at a time,
> so its development should be done step-by-step approach and
> separatable ones (row-level, largeobjects) are postponed.
> This change is extreamly large, almost same as implementing
> from zero. I think it is far from step-by-step.

Yes, the development should be step-by-step, with minimally invasive
changes each time. The first step is just hooking into SELinux for
access levels that PostgreSQL already supports (database, schema,
table, column). Requiring that the HeapTupleHeader be changed and that
additional internal system columns be added, along with SELinux checks
at levels that aren't appropriate, is alot more than this first patch
should really need. Those kinds of changes would be with the patch to
add non-SELinux row-level security. Following that, SELinux hooks would
be added to the RLS checks.

I think you need to look at it from the point of view of the committers
and consider the whole patch, not just the quantity of code under
security/. While that is important, the changes to the code *outside*
of security/ is what the committers are going to be most concerned about
because that gets into the overall structure and design and are areas
which are much more likely to affect users who don't use this feature,
performance, etc.

Or at least, that's what I'd expect. Getting some commentary from core
would be nice..

Thanks,

Stephen

On Sat, Jan 31, 2009 at 8:32 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * KaiGai Kohei (kaigai(at)kaigai(dot)gr(dot)jp) wrote:
>> Stephen Frost wrote:
>> > I think Bruce's question was where you stored the security_acl and
>> > security_label columns. Based on your response (and a bit of purusal
>> > through the code.google site), it looks like you still have security_acl
>> > and security_label defined as internal columns and being included
>> > for at least system tables (or is it everywhere?).
>>
>> In the current working tree, it (security id) is stored within
>> padding field of HeapTupleHeader, so internal facility can read
>> it via HeapTupleGetSecLabel(), but I already removed "security_acl"
>> and "security_label" definition.
>> Its basic structure is unchanged, the text form of security label
>> is stored within pg_security system catalog.
>
> I'm pretty sure that structure is part of what people were unhappy about
> though, and what makes it a much more invasive change that violates
> certain levels in the system by requiring information at much lower
> levels than it had before.

IANAC, but that's my impression too. The simplified patch shouldn't
assume that row-level security in its current form is going to end up
getting put back in. AFAICS, there's no reason why the security ID
for tables can't be a regular attribute in pg_class, or why the
security attribute for columns can't be a regular attribute in
pg_attribute.

...Robert

>>> I think what people
>>> are looking for, instead, is either additional columns to just the
>>> existing system tables that need them (eg: pg_class, pg_attribute) or
>>> included in the existing ACL structure (the aclitem structure). Another
>>> option might be a seperate set of tables.
>> It should not be held in text form within each tuples.
>> Please remind why I rework for the feature again.
>> Its facilities are a bit large to get included at a time,
>> so its development should be done step-by-step approach and
>> separatable ones (row-level, largeobjects) are postponed.
>> This change is extreamly large, almost same as implementing
>> from zero. I think it is far from step-by-step.
>
> Yes, the development should be step-by-step, with minimally invasive
> changes each time. The first step is just hooking into SELinux for
> access levels that PostgreSQL already supports (database, schema,
> table, column). Requiring that the HeapTupleHeader be changed and that
> additional internal system columns be added, along with SELinux checks
> at levels that aren't appropriate, is alot more than this first patch
> should really need. Those kinds of changes would be with the patch to
> add non-SELinux row-level security. Following that, SELinux hooks would
> be added to the RLS checks.

A concetn is current implementation assumes objects to be checked
(tables, columns, ..., now) have its security identifier. Whole of
the access control facility makes its decision based on security
identifier.
When the text form is used? It is used only when SE-PostgreSQL
communicate with in-kernel SELinux because it is protocol.
The result of decision making is cached in userspace to reduce
kernel invocation, and all the entries are tagged by security
identifier. So, any other facility does not need to handle
text form. Removing security id also means replacement of whole
of the current SE-PostgreSQL facilities!

BTW, I had tried an approach which have security label by text,
but its performance pain (due to string comparison) is unacceptable.
(IIRC, about 50% of transaction per sec by pg_bench.)
Indeed, we don't apply MAC on row-level in v8.4, so it does not
make serious matter on table/column level, but is surely has to
be scrapped once and implementaed from zero again.

> I think you need to look at it from the point of view of the committers
> and consider the whole patch, not just the quantity of code under
> security/. While that is important, the changes to the code *outside*
> of security/ is what the committers are going to be most concerned about
> because that gets into the overall structure and design and are areas
> which are much more likely to affect users who don't use this feature,
> performance, etc.

IIRC, we recently discussed that we should not afraid to apply
core code if it tries to get merged, so, I scraped all the
"PGACE" facilities.
Keeping independency under "security/*" is a concept of PGACE.
It enabled to implement a new security feature with minimum
impact to the core code, but, we concluded it is not necessary
a few days ago, then it is just before to finish most of works...

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

Robert Haas wrote:
> On Sat, Jan 31, 2009 at 8:32 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>> * KaiGai Kohei (kaigai(at)kaigai(dot)gr(dot)jp) wrote:
>>> Stephen Frost wrote:
>>>> I think Bruce's question was where you stored the security_acl and
>>>> security_label columns. Based on your response (and a bit of purusal
>>>> through the code.google site), it looks like you still have security_acl
>>>> and security_label defined as internal columns and being included
>>>> for at least system tables (or is it everywhere?).
>>> In the current working tree, it (security id) is stored within
>>> padding field of HeapTupleHeader, so internal facility can read
>>> it via HeapTupleGetSecLabel(), but I already removed "security_acl"
>>> and "security_label" definition.
>>> Its basic structure is unchanged, the text form of security label
>>> is stored within pg_security system catalog.
>> I'm pretty sure that structure is part of what people were unhappy about
>> though, and what makes it a much more invasive change that violates
>> certain levels in the system by requiring information at much lower
>> levels than it had before.
>
> IANAC, but that's my impression too. The simplified patch shouldn't
> assume that row-level security in its current form is going to end up
> getting put back in. AFAICS, there's no reason why the security ID
> for tables can't be a regular attribute in pg_class, or why the
> security attribute for columns can't be a regular attribute in
> pg_attribute.

If it is "identifier", it can be compoundable.

I dislike it is held as "text". It fundamentaly breaks SE-PostgreSQL's
architecture, and requires to scrap near future.
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

>> IANAC, but that's my impression too. The simplified patch shouldn't
>> assume that row-level security in its current form is going to end up
>> getting put back in. AFAICS, there's no reason why the security ID
>> for tables can't be a regular attribute in pg_class, or why the
>> security attribute for columns can't be a regular attribute in
>> pg_attribute.
>
> If it is "identifier", it can be compoundable.
>
> I dislike it is held as "text". It fundamentaly breaks SE-PostgreSQL's
> architecture, and requires to scrap near future.

I think the column in pg_attribute and pg_class can and should be an
OID. The issue is whether it's a regular OID column or a new system
column. Stephen and I are saying it should be a regular column.
pg_security can stick around to map OIDs to text labels.

...Robert

Robert Haas wrote:
>>> IANAC, but that's my impression too. The simplified patch shouldn't
>>> assume that row-level security in its current form is going to end up
>>> getting put back in. AFAICS, there's no reason why the security ID
>>> for tables can't be a regular attribute in pg_class, or why the
>>> security attribute for columns can't be a regular attribute in
>>> pg_attribute.
>> If it is "identifier", it can be compoundable.
>>
>> I dislike it is held as "text". It fundamentaly breaks SE-PostgreSQL's
>> architecture, and requires to scrap near future.
>
> I think the column in pg_attribute and pg_class can and should be an
> OID. The issue is whether it's a regular OID column or a new system
> column. Stephen and I are saying it should be a regular column.
> pg_security can stick around to map OIDs to text labels.

OK, I accept to omit a facility to save security id on padding field
of HeapTupleHeader *in this step*, if is has no other matter unexpected.

One melancholic thing is adding a member into pg_proc.
It defines more than 2000 of entries which I have to modify correctly. :(
Is there any script to help it?

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

* KaiGai Kohei (kaigai(at)kaigai(dot)gr(dot)jp) wrote:
> One melancholic thing is adding a member into pg_proc.
> It defines more than 2000 of entries which I have to modify correctly. :(
> Is there any script to help it?

Not that I'm aware of.. You might be able to create an appropriate
regex though, esp. if you're adding it at the end..

Stephen

KaiGai Kohei wrote:
>
> One melancholic thing is adding a member into pg_proc.
> It defines more than 2000 of entries which I have to modify correctly. :(
> Is there any script to help it?
>
>

Last time I added a column to a large catalog, I used a perl script to
help me, IIRC, but I haven't kept it. Could be a useful tool if somebody
could generalize such a thing.

cheers

andrew

Andrew Dunstan wrote:
>
>
> KaiGai Kohei wrote:
>>
>> One melancholic thing is adding a member into pg_proc.
>> It defines more than 2000 of entries which I have to modify correctly. :(
>> Is there any script to help it?
>>
>>
>
> Last time I added a column to a large catalog, I used a perl script to
> help me, IIRC, but I haven't kept it. Could be a useful tool if somebody
> could generalize such a thing.

My preference is sed & awk. :)

OK, I'll also do it by myself.
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

KaiGai Kohei wrote:
> Andrew Dunstan wrote:
>>
>>
>> KaiGai Kohei wrote:
>>>
>>> One melancholic thing is adding a member into pg_proc.
>>> It defines more than 2000 of entries which I have to modify
>>> correctly. :(
>>> Is there any script to help it?
>>>
>>>
>>
>> Last time I added a column to a large catalog, I used a perl script to
>> help me, IIRC, but I haven't kept it. Could be a useful tool if
>> somebody could generalize such a thing.
>
> My preference is sed & awk. :)
>
> OK, I'll also do it by myself.

FYI:

* revision 1468
$ diffstat sepostgresql-sepgsql-8.4devel-3-r1467.patch
:
110 files changed, 9813 insertions(+), 16 deletions(-), 924 modifications(!)

* current revision
$ diffstat sepostgresql-sepgsql-8.4devel-3-r1492.patch
:
src/include/catalog/pg_attribute.h | 507 !!!
src/include/catalog/pg_class.h | 28
src/include/catalog/pg_database.h | 10
src/include/catalog/pg_proc.h | 4234 !!!!!!!!!!!!!!!!!!!!!!!!!!
:
68 files changed, 5302 insertions(+), 2 deletions(-), 4910 modifications(!)

Don't say patch too large. :D

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

* KaiGai Kohei (kaigai(at)kaigai(dot)gr(dot)jp) wrote:
> Don't say patch too large. :D

Those are include files that are changing.. People do understand that
adding a column to a catalog can introduce alot of changed lines.

Stephen

Josh Berkus wrote:
> Joshua, Kohei-san,
>
> So, for 8.4: *if* we included in 8.4 a version of SEPostgres with all
> features *except* row-level security, would it still be useful to the
> SELinux community?
>
> I think we're just not going to work out the headache-inducing issues
> around row-level security in time for 8.4, and it seems to me that
> integrated system-level security labels at the table-and-column level
> are still very useful, even without row-level security.
>

Sorry for the delay in answering, I'm currently on vacation (I haven't been able
to catch up on this thread yet either, I'll try to a little later).

The answer is yes, at least to get people started using it and make sure there
are no practical issues with the security model sans row access control.

But as I said earlier row based access control is going to be the most
compelling part so hopefully the issues everyone is having can get worked out
and the community will agree on the path forward, sooner rather than later.

On 1/30/09 5:43 PM, "Josh Berkus" <josh(at)agliodbs(dot)com> wrote:

> Joshua, Kohei-san,
>
> So, for 8.4: *if* we included in 8.4 a version of SEPostgres with all
> features *except* row-level security, would it still be useful to the
> SELinux community?
>
Yes, it's definitely still useful. While many of the use cases we've wanted
this for require row-level access control, there have been several that did
not. It is definitely still useful, especially if it is a path toward
row-level access control in a later release.

Chad

Robert Haas wrote:
> >> IANAC, but that's my impression too. The simplified patch shouldn't
> >> assume that row-level security in its current form is going to end up
> >> getting put back in. AFAICS, there's no reason why the security ID
> >> for tables can't be a regular attribute in pg_class, or why the
> >> security attribute for columns can't be a regular attribute in
> >> pg_attribute.
> >
> > If it is "identifier", it can be compoundable.
> >
> > I dislike it is held as "text". It fundamentaly breaks SE-PostgreSQL's
> > architecture, and requires to scrap near future.
>
> I think the column in pg_attribute and pg_class can and should be an
> OID. The issue is whether it's a regular OID column or a new system
> column. Stephen and I are saying it should be a regular column.
> pg_security can stick around to map OIDs to text labels.

Why an OID? We store acl items now without a lookup table; I think
there will be at most the same number of SE-Linux entries. Also, by
using text we avoid the problem of cleaning out unreferenced pg_security
rows, improve performance (no lookups), and simplify the code.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> Robert Haas wrote:
>>>> IANAC, but that's my impression too. The simplified patch shouldn't
>>>> assume that row-level security in its current form is going to end up
>>>> getting put back in. AFAICS, there's no reason why the security ID
>>>> for tables can't be a regular attribute in pg_class, or why the
>>>> security attribute for columns can't be a regular attribute in
>>>> pg_attribute.
>>> If it is "identifier", it can be compoundable.
>>>
>>> I dislike it is held as "text". It fundamentaly breaks SE-PostgreSQL's
>>> architecture, and requires to scrap near future.
>> I think the column in pg_attribute and pg_class can and should be an
>> OID. The issue is whether it's a regular OID column or a new system
>> column. Stephen and I are saying it should be a regular column.
>> pg_security can stick around to map OIDs to text labels.
>
> Why an OID? We store acl items now without a lookup table; I think
> there will be at most the same number of SE-Linux entries. Also, by
> using text we avoid the problem of cleaning out unreferenced pg_security
> rows, improve performance (no lookups), and simplify the code.

The reason why I concern about text formed security context is
it has variable length, so it requires to deform/form a HeapTuple
again when SE-PostgreSQL assigns a default security context.
If a user inserts a new tuple into pg_xxxx without explicit security
context, it has to be labeled based on security context. We cannot
estimate what string will be given prior to ExecInsert(), it needs
to put a security label on the given HeapTuple.
If is is a fixed length variable (like "oid"), it is not necessary
to deform/form them. So, I prefer the security identifier.

In addition, it also has performance gain.
The current architecture does not need to look up pg_security in most
cases. SE-PostgreSQL caches results of access controls in userspace
to reduce the number of kernel invocation.
(In generally, context switch is a heavy one.)
All cached entries are tagged by its security identifier, so we can
lookup the entry without string comparing. The text form is used
only when it could not find the entry on the cache. In this case,
SE-PostgreSQL translate security identifier into text form and
ask for in-kernel SELinux. It requires a text form due to the
protocol.

At least, we cannot apply this scheme on the next phase (row-level)
due to the storage consumption and others. So, I don't think it is
a preferable way to design the first step without ignoring upcoming
expandability.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

KaiGai Kohei wrote:
> > Why an OID? We store acl items now without a lookup table; I think
> > there will be at most the same number of SE-Linux entries. Also, by
> > using text we avoid the problem of cleaning out unreferenced pg_security
> > rows, improve performance (no lookups), and simplify the code.
>
> The reason why I concern about text formed security context is
> it has variable length, so it requires to deform/form a HeapTuple
> again when SE-PostgreSQL assigns a default security context.
> If a user inserts a new tuple into pg_xxxx without explicit security
> context, it has to be labeled based on security context. We cannot
> estimate what string will be given prior to ExecInsert(), it needs
> to put a security label on the given HeapTuple.
> If is is a fixed length variable (like "oid"), it is not necessary
> to deform/form them. So, I prefer the security identifier.
>
> In addition, it also has performance gain.
> The current architecture does not need to look up pg_security in most
> cases. SE-PostgreSQL caches results of access controls in userspace
> to reduce the number of kernel invocation.
> (In generally, context switch is a heavy one.)
> All cached entries are tagged by its security identifier, so we can
> lookup the entry without string comparing. The text form is used
> only when it could not find the entry on the cache. In this case,
> SE-PostgreSQL translate security identifier into text form and
> ask for in-kernel SELinux. It requires a text form due to the
> protocol.

That is an interesting optimization I had not thought of.

> At least, we cannot apply this scheme on the next phase (row-level)
> due to the storage consumption and others. So, I don't think it is
> a preferable way to design the first step without ignoring upcoming
> expandability.

The big problem is that the security value on system tables controls the
_object_ represented by the row, while on user tables the security value
represents access to the row. That is just an odd design, and why a
regular system table security value makes sense, independent of the
row-level security feature.

FYI, it is possible we might implement row-level security a different
way in 8.5.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

>> Why an OID? We store acl items now without a lookup table; I think
>> there will be at most the same number of SE-Linux entries. Also, by
>> using text we avoid the problem of cleaning out unreferenced pg_security
>> rows, improve performance (no lookups), and simplify the code.
>
> In addition, it also has performance gain.
> The current architecture does not need to look up pg_security in most
> cases. SE-PostgreSQL caches results of access controls in userspace

I think this is a very compelling point.

...Robert

Bruce Momjian wrote:
>> At least, we cannot apply this scheme on the next phase (row-level)
>> due to the storage consumption and others. So, I don't think it is
>> a preferable way to design the first step without ignoring upcoming
>> expandability.
>
> The big problem is that the security value on system tables controls the
> _object_ represented by the row, while on user tables the security value
> represents access to the row. That is just an odd design, and why a
> regular system table security value makes sense, independent of the
> row-level security feature.

I don't think there is a fundamental differences between "ALTER FUNCTION"
and "UPDATE pg_proc SET ...", for example. It is necessary to apply
same privileges in this case.
(In this case, db_procedure:{setattr} is checked on the object.)

The security label of system catalogs (like pg_class, pg_proc, ...) are
also used when the objects are used as target of user's request, like
a target of SELECT statement, a target of function invocation.
Please note that different permissions are checked in this case.
(db_table:{select} and db_procedure:{execute})

Sorry, it is a bit unclear what is a problem you pointed out.
I guessed you concerned about a tuple (within system catalogs) is handled
as an object when user tries to modify the system catalogs by hand.
However, I cannot understand why it is an odd design.
If we keep free to update system catalogs, it makes a bypassable
route to create/alter/drop objects.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

Bruce Momjian wrote:
> KaiGai Kohei wrote:
>>> Why an OID? We store acl items now without a lookup table; I think
>>> there will be at most the same number of SE-Linux entries. Also, by
>>> using text we avoid the problem of cleaning out unreferenced pg_security
>>> rows, improve performance (no lookups), and simplify the code.
>> The reason why I concern about text formed security context is
>> it has variable length, so it requires to deform/form a HeapTuple
>> again when SE-PostgreSQL assigns a default security context.
>> If a user inserts a new tuple into pg_xxxx without explicit security
>> context, it has to be labeled based on security context. We cannot
>> estimate what string will be given prior to ExecInsert(), it needs
>> to put a security label on the given HeapTuple.
>> If is is a fixed length variable (like "oid"), it is not necessary
>> to deform/form them. So, I prefer the security identifier.
>>
>> In addition, it also has performance gain.
>> The current architecture does not need to look up pg_security in most
>> cases. SE-PostgreSQL caches results of access controls in userspace
>> to reduce the number of kernel invocation.
>> (In generally, context switch is a heavy one.)
>> All cached entries are tagged by its security identifier, so we can
>> lookup the entry without string comparing. The text form is used
>> only when it could not find the entry on the cache. In this case,
>> SE-PostgreSQL translate security identifier into text form and
>> ask for in-kernel SELinux. It requires a text form due to the
>> protocol.
>
> That is an interesting optimization I had not thought of.
>

Just as an FYI, SELinux does this in general. There is an access vector cache in
the kernel that caches the access computations, and there is also a userspace
implementation in libselinux that most apps use. KaiGai reimplemented the AVC
because he wanted it to work in a shm and be shared by multiple postgres processes.

There is also a sidtab which is just a hashtable that maps string contexts to
sids. The sidtab is filled at runtime and not persistent across boots, which
means the contexts are generally stored as text on the persistent medium (like
the xattr's on the filesystem).

It doesn't matter from a security perspective whether the contexts are stored as
strings or sids, its just an optimization you guys need to work out.

>> At least, we cannot apply this scheme on the next phase (row-level)
>> due to the storage consumption and others. So, I don't think it is
>> a preferable way to design the first step without ignoring upcoming
>> expandability.
>
> The big problem is that the security value on system tables controls the
> _object_ represented by the row, while on user tables the security value
> represents access to the row. That is just an odd design, and why a
> regular system table security value makes sense, independent of the
> row-level security feature.
>

I may not be understanding this but I don't see why. In SELinux everything is an
object, and all objects have contexts. No access is specified on the object or
in the context, that is all done in the policy currently loaded in the security
server. system tables and user tables shouldn't be treated differently
implementation wise, they should just have a context and defer the decision
making to the policy.

In practice the system tables (and rows within the tables) would have a context
that restricts access tightly, but this is up to the policy, not the implementation.

> FYI, it is possible we might implement row-level security a different
> way in 8.5.

Joshua Brindle wrote:
> > The big problem is that the security value on system tables controls the
> > _object_ represented by the row, while on user tables the security value
> > represents access to the row. That is just an odd design, and why a
> > regular system table security value makes sense, independent of the
> > row-level security feature.
> >
>
> I may not be understanding this but I don't see why. In SELinux everything is an
> object, and all objects have contexts. No access is specified on the object or
> in the context, that is all done in the policy currently loaded in the security
> server. system tables and user tables shouldn't be treated differently
> implementation wise, they should just have a context and defer the decision
> making to the policy.
>
> In practice the system tables (and rows within the tables) would have a context
> that restricts access tightly, but this is up to the policy, not the implementation.
>
> > FYI, it is possible we might implement row-level security a different
> > way in 8.5.

Seeing a pg_attribute row and seeing the column referenced by the row
are not the same thing.

Also, we are discussing system catalog values, (table, column,
function), etc, so I don't see an performance issue. I haven't heard of
anyone complaining about our ACL parsing overhead recently. A cache
could still be used, but on the text string, not the oid.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> Joshua Brindle wrote:
>>> The big problem is that the security value on system tables controls the
>>> _object_ represented by the row, while on user tables the security value
>>> represents access to the row. That is just an odd design, and why a
>>> regular system table security value makes sense, independent of the
>>> row-level security feature.
>>>
>> I may not be understanding this but I don't see why. In SELinux everything is an
>> object, and all objects have contexts. No access is specified on the object or
>> in the context, that is all done in the policy currently loaded in the security
>> server. system tables and user tables shouldn't be treated differently
>> implementation wise, they should just have a context and defer the decision
>> making to the policy.
>>
>> In practice the system tables (and rows within the tables) would have a context
>> that restricts access tightly, but this is up to the policy, not the implementation.
>>
>>> FYI, it is possible we might implement row-level security a different
>>> way in 8.5.
>
> Seeing a pg_attribute row and seeing the column referenced by the row
> are not the same thing.

Yes, it is quite different. It seems to me we are now confusing.

Are you saying:
a) SELECT attname FROM pg_attribute where attrelid='t'::regclass and attname='a';
and
b) SELECT a FROM t;
are different, aren't you?

Yes, it is not same thing.

For the query a), SE-PostgreSQL should check db_column:{getattr} permission
on the selected tuples, when row-level security is available.
In this case, it also checks db_column:{select} permission on the
"attname" column and db_table:{select} on the "pg_attribute" table.

For the query b), SE-PostgreSQL checks db_column:{select} permission on
the column "a", and it also checks db_table:{select} on the table "t".
And db_tuple:{select} permission when row-level security is available.

Please note that it checks db_column:* class permission on tuples within
pg_attribute system catalog, although db_tuple:* class ones are applied
on user defined tables.

When it checks permission of column, for example, it requires a label
assigned to the target object. In this case, an object is a row within
pg_attribute system catalog. It needs to be labeled as a column.
Thus, we have to add a field to hold its security label within pg_attribute
system catalog.

My concern is INSERT/UPDATE/DELETE these system catalogs by hand.
When user tries to insert a tuple without explicit security context,
it is necessary to be labeled as default one.
But, if it has variable length form, we have to deform the given
HeapTuple once then form HeapTuple again with text variable.
If it has fixed length oid, we can put it directly, as "oid" doing
at heap_insert() or heap_update().

> Also, we are discussing system catalog values, (table, column,
> function), etc, so I don't see an performance issue. I haven't heard of
> anyone complaining about our ACL parsing overhead recently. A cache
> could still be used, but on the text string, not the oid.

Yes, performance is not the first issue here.
The variable length type makes hard to assign a newly inserted tuple
(into pg_class, etc...) a default security context.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

If we add a field on pg_xxxx to store security label in text form,
it is necessary to attach a default one at the following points.

* pg_class
- InsertPgClassTuple() at heap.c

* pg_attribute
- InsertPgAttributeTuple() at heap.c

* pg_proc
- ProcedureCreate() at pg_proc.c

* pg_database
- createdb() at dbcommands.c

* for whole of them
- InsertOneTuple() at bootstrap.c
- ExecInsert() at execMain.c

The reason why I prefer security identifier ("oid") is that we can put
a hook to assign a default security context inside the simple_heap_insert().
But, if above functions are the all to insert a new tuple into these
issued relations, it may be a reasonable approach for those four system
catalogs. Please point out if I overlooks somewhere.

At the previous message, I noted I'll submit revised patches on thie
Wednesday, but it become impossible due to the change. (T-T)
Please wait for a while.

KaiGai Kohei wrote:
> Bruce Momjian wrote:
>> Joshua Brindle wrote:
>>>> The big problem is that the security value on system tables controls
>>>> the
>>>> _object_ represented by the row, while on user tables the security
>>>> value
>>>> represents access to the row. That is just an odd design, and why a
>>>> regular system table security value makes sense, independent of the
>>>> row-level security feature.
>>>>
>>> I may not be understanding this but I don't see why. In SELinux
>>> everything is an object, and all objects have contexts. No access is
>>> specified on the object or in the context, that is all done in the
>>> policy currently loaded in the security server. system tables and
>>> user tables shouldn't be treated differently implementation wise,
>>> they should just have a context and defer the decision making to the
>>> policy.
>>>
>>> In practice the system tables (and rows within the tables) would have
>>> a context that restricts access tightly, but this is up to the
>>> policy, not the implementation.
>>>
>>>> FYI, it is possible we might implement row-level security a different
>>>> way in 8.5.
>>
>> Seeing a pg_attribute row and seeing the column referenced by the row
>> are not the same thing.
>
> Yes, it is quite different. It seems to me we are now confusing.
>
> Are you saying:
> a) SELECT attname FROM pg_attribute where attrelid='t'::regclass and
> attname='a';
> and
> b) SELECT a FROM t;
> are different, aren't you?
>
> Yes, it is not same thing.
>
> For the query a), SE-PostgreSQL should check db_column:{getattr} permission
> on the selected tuples, when row-level security is available.
> In this case, it also checks db_column:{select} permission on the
> "attname" column and db_table:{select} on the "pg_attribute" table.
>
> For the query b), SE-PostgreSQL checks db_column:{select} permission on
> the column "a", and it also checks db_table:{select} on the table "t".
> And db_tuple:{select} permission when row-level security is available.
>
> Please note that it checks db_column:* class permission on tuples within
> pg_attribute system catalog, although db_tuple:* class ones are applied
> on user defined tables.
>
> When it checks permission of column, for example, it requires a label
> assigned to the target object. In this case, an object is a row within
> pg_attribute system catalog. It needs to be labeled as a column.
> Thus, we have to add a field to hold its security label within pg_attribute
> system catalog.
>
> My concern is INSERT/UPDATE/DELETE these system catalogs by hand.
> When user tries to insert a tuple without explicit security context,
> it is necessary to be labeled as default one.
> But, if it has variable length form, we have to deform the given
> HeapTuple once then form HeapTuple again with text variable.
> If it has fixed length oid, we can put it directly, as "oid" doing
> at heap_insert() or heap_update().
>
>> Also, we are discussing system catalog values, (table, column,
>> function), etc, so I don't see an performance issue. I haven't heard of
>> anyone complaining about our ACL parsing overhead recently. A cache
>> could still be used, but on the text string, not the oid.
>
> Yes, performance is not the first issue here.
> The variable length type makes hard to assign a newly inserted tuple
> (into pg_class, etc...) a default security context.
>
> Thanks,

--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

The following patches are updated ones:

[1/5] http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1522.patch
[2/5] http://sepgsql.googlecode.com/files/sepostgresql-utils-8.4devel-3-r1522.patch
[3/5] http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1522.patch
[4/5] http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1522.patch
[5/5] http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1522.patch

- List of updates:
* The facilities of PGACE are removed.
* The facilities of row-level access controls are separated.
* The facilities of security attribute management are separated.
- The pg_security system catalog, the idea of security identifier
and the "security_label" system column are included.
- AVC become to accept text form security context.
- pg_class, pg_attribute, pg_database and pg_proc got a new field
to store text form security context.
* A few of security hooks are integrated into pg_xxx_aclcheck()
- sepgsqlCheckProcedureExecute() from pg_proc_aclmask()
- sepgsqlCheckDatabaseAccess() from pg_database_aclmask()
* Access controls on large objects are separated.
* The baseline security policy module is omitted, so the 3rd patch
provides only developer's policy.
* Descriptions about PGACE and row-level access controls are separated.
* Testcases are reworked.
* Anyway, most of patches are reworked!

- Scale of patches
It may seem you the updated version is not smaller than previous
version, but more than half of affected lines are come from changes
in system catalog.

* The previous full-functional version (r1467)
$ diffstat sepostgresql-sepgsql-8.4devel-3-r1467.patch
:
110 files changed, 9813 insertions(+), 16 deletions(-), 924 modifications(!)

* Current version (r1522)
$ diffstat sepostgresql-sepgsql-8.4devel-3-r1522.patch
:
src/include/catalog/pg_attribute.h | 500 !!!
src/include/catalog/pg_class.h | 12
src/include/catalog/pg_database.h | 6
src/include/catalog/pg_proc.h | 4207 !!!!!!!!!!!!!!!!!!!!!!!!!!
:
65 files changed, 4737 insertions(+), 11 deletions(-), 4908 modifications(!)

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

KaiGai Kohei escribió:

> One melancholic thing is adding a member into pg_proc.
> It defines more than 2000 of entries which I have to modify correctly. :(
> Is there any script to help it?

Try a search for "coccinelle", "sdiff", or was it "spatch"? It got
featured on http://LWN.net/ not many weeks ago.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> KaiGai Kohei escribi:
>> One melancholic thing is adding a member into pg_proc.
>> It defines more than 2000 of entries which I have to modify correctly. :(
>> Is there any script to help it?

> Try a search for "coccinelle", "sdiff", or was it "spatch"? It got
> featured on http://LWN.net/ not many weeks ago.

FWIW, every single time I've had to add a column to pg_proc (and I've
done it several times now), an Emacs macro got the job done with a few
minutes' thought. I'm sure sed would work as well. The contents of
those DATA lines are really pretty stylized.

regards, tom lane

Tom Lane escribió:
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> > KaiGai Kohei escribi:
> >> One melancholic thing is adding a member into pg_proc.
> >> It defines more than 2000 of entries which I have to modify correctly. :(
> >> Is there any script to help it?
>
> > Try a search for "coccinelle", "sdiff", or was it "spatch"? It got
> > featured on http://LWN.net/ not many weeks ago.

Here it is:
http://lwn.net/Articles/315686/

> FWIW, every single time I've had to add a column to pg_proc (and I've
> done it several times now), an Emacs macro got the job done with a few
> minutes' thought. I'm sure sed would work as well. The contents of
> those DATA lines are really pretty stylized.

I've had to do it only once, but yes, I did it with a simple s// command
in Vim.

The series of SE-PostgreSQL patches are updated:

[1/5] http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1530.patch
[2/5] http://sepgsql.googlecode.com/files/sepostgresql-utils-8.4devel-3-r1530.patch
[3/5] http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1530.patch
[4/5] http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1530.patch
[5/5] http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1530.patch

- List of updates:
* These are rebased to the latest CVS HEAD because of conflictions.
- The src/include/catalog/pg_proc.h got a confliction due to the
newly added SQL functions.
- The src/bin/pg_dump/pg_dump.c got a confliction due to the stuff
to dump "toast_reloptions".
* bugfix: An incorrect procedure entry for sepgsql_server_getcon().
* cleanup: A strange error message in testcases.

Rest of parts are unchanged.

Please comment anything.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

KaiGai Kohei wrote:
> The series of SE-PostgreSQL patches are updated:
> [1/5] http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1530.patch
> [2/5] http://sepgsql.googlecode.com/files/sepostgresql-utils-8.4devel-3-r1530.patch
> [3/5] http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1530.patch
> [4/5] http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1530.patch
> [5/5] http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1530.patch

BTW, what is the current status of revewing the patches?
Is it necessary to wait for a few days more?

If you have anything unclear, please feel free to ask me anything.

Thanks,

> - List of updates:
> * These are rebased to the latest CVS HEAD because of conflictions.
> - The src/include/catalog/pg_proc.h got a confliction due to the
> newly added SQL functions.
> - The src/bin/pg_dump/pg_dump.c got a confliction due to the stuff
> to dump "toast_reloptions".
> * bugfix: An incorrect procedure entry for sepgsql_server_getcon().
> * cleanup: A strange error message in testcases.
>
> Rest of parts are unchanged.
>
> Please comment anything.
>
> Thanks,

--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

KaiGai Kohei wrote:
> KaiGai Kohei wrote:
>> The series of SE-PostgreSQL patches are updated:
>> [1/5]
>> http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1530.patch
>>
>> [2/5]
>> http://sepgsql.googlecode.com/files/sepostgresql-utils-8.4devel-3-r1530.patch
>>
>> [3/5]
>> http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1530.patch
>>
>> [4/5]
>> http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1530.patch
>>
>> [5/5]
>> http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1530.patch
>>
>
> BTW, what is the current status of revewing the patches?
> Is it necessary to wait for a few days more?
>
> If you have anything unclear, please feel free to ask me anything.
>

Yes, what was the decision about 8.4? Is this going to make it in?

> Thanks,
>
>> - List of updates:
>> * These are rebased to the latest CVS HEAD because of conflictions.
>> - The src/include/catalog/pg_proc.h got a confliction due to the
>> newly added SQL functions.
>> - The src/bin/pg_dump/pg_dump.c got a confliction due to the stuff
>> to dump "toast_reloptions".
>> * bugfix: An incorrect procedure entry for sepgsql_server_getcon().
>> * cleanup: A strange error message in testcases.
>>
>> Rest of parts are unchanged.
>>
>> Please comment anything.
>>
>> Thanks,
>

On Fri, Feb 13, 2009 at 9:07 AM, Joshua Brindle <method(at)manicmethod(dot)com> wrote:
> KaiGai Kohei wrote:
>>
>> KaiGai Kohei wrote:
>>>
>>> The series of SE-PostgreSQL patches are updated:
>>> [1/5]
>>> http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1530.patch
>>> [2/5]
>>> http://sepgsql.googlecode.com/files/sepostgresql-utils-8.4devel-3-r1530.patch
>>> [3/5]
>>> http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1530.patch
>>> [4/5]
>>> http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1530.patch
>>> [5/5]
>>> http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1530.patch
>>
>> BTW, what is the current status of revewing the patches?
>> Is it necessary to wait for a few days more?
>>
>> If you have anything unclear, please feel free to ask me anything.
>>
>
> Yes, what was the decision about 8.4? Is this going to make it in?
>

can you try the functional parts of it? ie: compile with the patch
with --enable-selinux and test if the patch does wath you expect?

i will try it but i have to install a VM to install selinux on it...
then i will try some cases... can you give me an example of a typical
scenario to make those tests?

i will do some code review too but i'm not a hacker so you will need
to wait for them to review it anyway...

--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

Jaime Casanova wrote:
> On Fri, Feb 13, 2009 at 9:07 AM, Joshua Brindle <method(at)manicmethod(dot)com> wrote:
>> KaiGai Kohei wrote:
>>> KaiGai Kohei wrote:
>>>> The series of SE-PostgreSQL patches are updated:
>>>> [1/5]
>>>> http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1530.patch
>>>> [2/5]
>>>> http://sepgsql.googlecode.com/files/sepostgresql-utils-8.4devel-3-r1530.patch
>>>> [3/5]
>>>> http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1530.patch
>>>> [4/5]
>>>> http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1530.patch
>>>> [5/5]
>>>> http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1530.patch
>>> BTW, what is the current status of revewing the patches?
>>> Is it necessary to wait for a few days more?
>>>
>>> If you have anything unclear, please feel free to ask me anything.
>>>
>> Yes, what was the decision about 8.4? Is this going to make it in?
>>
>
> can you try the functional parts of it? ie: compile with the patch
> with --enable-selinux and test if the patch does wath you expect?
>
> i will try it but i have to install a VM to install selinux on it...
> then i will try some cases... can you give me an example of a typical
> scenario to make those tests?

If you can help to test the patches, I recommend you to install Fedora 10
on your VM images, because it includes SELinux in the default and its
default security policy (selinux-policy-targeted) also supports SE-PostgreSQL.

Then, could you try the following steps?

1) installation
$ ./configure --enable-selinux
$ make
$ make -C src/backend/security/sepgsql/policy
(NOTE: We provide a policy module for development purpose)
$ su
# make install
# /usr/sbin/semodule -i src/backend/security/sepgsql/policy/sepostgresql-devel.pp
(NOTE: It installs the development policy)
# /sbin/restorecon -R /usr/local/pgsql
(NOTE: It assigns correct security context for installed binaries)
$ export PGDATA=/path/to/database
$ chcon -t postgresql_db_t -R $PGDATA
(NOTE: It assigns correct security context for database files)
$ initdb --enable-selinux
(NOTE: --enable-selinux turns on SE-PostgreSQL feature)
$ pg_ctl start

2) check installation
2-1) Please confirm SE-PostgreSQL works
$ psql postgres
psql (8.4devel)
Type "help" for help.

postgres=# SHOW sepostgresql;
sepostgresql
--------------
on
(1 row)

2-2) Please confirm client's privileges
$ id -Z
unconfined_u:unconfined_r:unconfined_t
$ psql postgres
psql (8.4devel)
Type "help" for help.

postgres=# SELECT sepgsql_getcon();
sepgsql_getcon
----------------------------------------
unconfined_u:unconfined_r:unconfined_t
(1 row)

NOTE: It has to be matched with privileges on OS.

2-3) Please confirm server's privileges

postgres=# SELECT sepgsql_server_getcon();
sepgsql_server_getcon
------------------------------------
unconfined_u:system_r:postgresql_t
(1 row)

NOTE: It is necessary restricted domain (like PHP scripts) to connect
PostgreSQL server process.

2-4) Please confirm to connect from restricted domain

$ runcon -t sepgsql_test_t -- psql postgres
psql (8.4devel)
Type "help" for help.

postgres=# SELECT sepgsql_getcon();
sepgsql_getcon
------------------------------------------
unconfined_u:unconfined_r:sepgsql_test_t
(1 row)

NOTE: The "sepgsql_test_t" has restricted privileges same as PHP scripts
invoked from Apache web server.
NOTE: If SELinux denied to connect, please try the following command (in root):
# setsebool -P allow_user_postgresql_connect 1

3) Example of a typical scenario
3-1) Setup of column level access controls
postgres=# CREATE TABLE customer (
cid int primary key,
cname text,
credit varchar(32)
SECURITY_LABEL = 'system_u:object_r:sepgsql_secret_table_t:s0'
);
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "customer_pkey" for table "customer"
CREATE TABLE
postgres=# INSERT INTO customer VALUES (1, 'kaigai', '1111-2222-3333-4444'),
(2, 'yamada', '5555-6666-7777-8888'),
(3, 'kimura', '9999-0000-1234-5678');
INSERT 0 3
postgres=# SELECT * FROM customer;
cid | cname | credit
-----+--------+---------------------
1 | kaigai | 1111-2222-3333-4444
2 | yamada | 5555-6666-7777-8888
3 | kimura | 9999-0000-1234-5678
(3 rows)

postgres=# CREATE OR REPLACE FUNCTION show_credit (int)
RETURNS text LANGUAGE 'sql'
SECURITY_LABEL = 'system_u:object_r:sepgsql_trusted_proc_exec_t:s0'
AS 'SELECT regexp_replace(credit, ''-[0-9]+'', ''-xxxx'', ''g'') FROM customer WHERE cid = $1';
CREATE FUNCTION

3-2) Example of column level access controls
$ runcon -t sepgsql_test_t -- psql postgres
psql (8.4devel)
Type "help" for help.

postgres=# SELECT * FROM customer;
ERROR: SELinux: denied { select } scontext=unconfined_u:unconfined_r:sepgsql_test_t tcontext=system_u:object_r:sepgsql_secret_table_t tclass=db_column name=customer.credit
(NOTE: SE-PostgreSQL prevent restricted domain to select a column labeled as 'sepgsql_secret_table_t')
postgres=# SELECT cid, cname FROM customer;
cid | cname
-----+--------
1 | kaigai
2 | yamada
3 | kimura
(3 rows)

postgres=# SELECT cid, cname, show_credit(cid) FROM customer;
cid | cname | show_credit
-----+--------+---------------------
1 | kaigai | 1111-xxxx-xxxx-xxxx
2 | yamada | 5555-xxxx-xxxx-xxxx
3 | kimura | 9999-xxxx-xxxx-xxxx
(3 rows)
(NOTE: The show_credit() is labeled as 'sepgsql_trusted_proc_exec_t', it enables to
switch client privilege during the function running.)
(NOTE: Please note that sepgsql_test_t has same privileges with PHP script invoked
from web servers, so it means PHP script cannot show "customer.credit" directly.)

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

On Fri, Feb 13, 2009 at 8:32 PM, KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>>
> If you can help to test the patches, I recommend you to install Fedora 10
> on your VM images, because it includes SELinux in the default and its
> default security policy (selinux-policy-targeted) also supports
> SE-PostgreSQL.
>

i only have ubuntu at hand, and i can install selinux from
repositories... do you see any problem with that?
i'm creating the VM now... will test on weekend...

--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

Jaime Casanova wrote:
> On Fri, Feb 13, 2009 at 8:32 PM, KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>> If you can help to test the patches, I recommend you to install Fedora 10
>> on your VM images, because it includes SELinux in the default and its
>> default security policy (selinux-policy-targeted) also supports
>> SE-PostgreSQL.
>>
>
> i only have ubuntu at hand, and i can install selinux from
> repositories... do you see any problem with that?
> i'm creating the VM now... will test on weekend...

Now I checked the repository of Ubuntu, however, I can find
several matters to build/run SE-PostgreSQL.
- The default security policy is quite old.
Is does not contain SE-PostgreSQL support which is merged
at the refpolicy-20080702.
- The libselinux is a bit old. It does not contains several
symbols required by SE-PostgreSQL, so it is not possible
to compile.

If you set up a VM from scratch, Fedora enables to install
via remote repository using minimum bootable image.
http://download.fedora.redhat.com/pub/fedora/linux/releases/10/Fedora/i386/iso/

I don't think it is not a essential for you to resolve troubles
related to SELinux on Ubuntu.

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