add a MAC check for TRUNCATE

Started by Yuli Khodorkovskiyover 6 years ago39 messageshackers
Jump to latest
#1Yuli Khodorkovskiy
yuli.khodorkovskiy@crunchydata.com

Hackers,

Since all DAC checks should have corresponding MAC, this patch adds a
hook to allow extensions to implement a MAC check on TRUNCATE. I have
also implemented this access check in the sepgsql extension.

One important thing to note is that refpolicy [1]https://github.com/SELinuxProject/refpolicy/blob/master/policy/flask/access_vectors#L794 and Redhat based
distributions do not have the SELinux permission for db_table {truncate}
implemented. This patch is the first step to add this permission to the
upstream SELinux policy. If this permission does not exist in the
policy, sepgsql is being used, and `deny_unknown` is set to 1, the
TRUNCATE will be denied.

As a workaround for this behavior, the SELinux aware system would need
to have `/sys/fs/selinux/deny_unknown` set to 0 until the permission has
been added to refpolicy/Redhat SELinux policy.

The deny_unknown behavior can be set using CIL [2]https://github.com/SELinuxProject/selinux/blob/master/secilc/docs/cil_policy_config_statements.md#handleunknown 0001-Use-MAC-in-addition-to-DAC-for-TRUNCATE.patch by extracting the
base SELinux module, and setting how the kernel handles unknown
permissions. The dependencies for overriding handle_unknown are
policycoreutils, selinux-policy-targeted, and a libsemanage version that
supports CIL (CentOS 7+).

$ sudo semodule -cE base
$ sed -Ei 's/(handleunknown )deny/\1allow/g' base.cil
$ sudo semodule -i base.cil

Thanks,

Yuli

[1]: https://github.com/SELinuxProject/refpolicy/blob/master/policy/flask/access_vectors#L794
[2]: https://github.com/SELinuxProject/selinux/blob/master/secilc/docs/cil_policy_config_statements.md#handleunknown 0001-Use-MAC-in-addition-to-DAC-for-TRUNCATE.patch
0001-Use-MAC-in-addition-to-DAC-for-TRUNCATE.patch

Attachments:

0001-Use-MAC-in-addition-to-DAC-for-TRUNCATE.patchapplication/octet-stream; name=0001-Use-MAC-in-addition-to-DAC-for-TRUNCATE.patchDownload+87-2
#2KaiGai Kohei
kaigai@ak.jp.nec.com
In reply to: Yuli Khodorkovskiy (#1)
Re: add a MAC check for TRUNCATE

Hello Yuli,

2019年7月25日(木) 3:52 Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com>:

Since all DAC checks should have corresponding MAC, this patch adds a
hook to allow extensions to implement a MAC check on TRUNCATE. I have
also implemented this access check in the sepgsql extension.

One important thing to note is that refpolicy [1] and Redhat based
distributions do not have the SELinux permission for db_table {truncate}
implemented.

How db_table:{delete} permission is different from truncate?
From the standpoint of data access, TRUNCATE is equivalent to DELETE
without WHERE, isn't it?
Of course, there are some differences between them. TRUNCATE takes
exclusive locks and eliminates underlying data blocks, on the other hands,
DELETE removes rows under MVCC manner. However, both of them
eventually removes data from the target table.

I like to recommend to reuse "db_table:{delete}" permission for TRUNCATE.
How about your opinions?

Best regards,
--
HeteroDB, Inc / The PG-Strom Project
KaiGai Kohei <kaigai@heterodb.com>

#3Stephen Frost
sfrost@snowman.net
In reply to: KaiGai Kohei (#2)
Re: add a MAC check for TRUNCATE

Greetings,

* Kohei KaiGai (kaigai@heterodb.com) wrote:

2019年7月25日(木) 3:52 Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com>:

Since all DAC checks should have corresponding MAC, this patch adds a
hook to allow extensions to implement a MAC check on TRUNCATE. I have
also implemented this access check in the sepgsql extension.

One important thing to note is that refpolicy [1] and Redhat based
distributions do not have the SELinux permission for db_table {truncate}
implemented.

How db_table:{delete} permission is different from truncate?

From the standpoint of data access, TRUNCATE is equivalent to DELETE

without WHERE, isn't it?
Of course, there are some differences between them. TRUNCATE takes
exclusive locks and eliminates underlying data blocks, on the other hands,
DELETE removes rows under MVCC manner. However, both of them
eventually removes data from the target table.

I like to recommend to reuse "db_table:{delete}" permission for TRUNCATE.
How about your opinions?

There's been much discussion and justifcation for adding an independent
TRUNCATE privilege to GRANT (which actually took many years to be
allowed). I don't see why we wouldn't represent that as a different
privilege to external MAC systems. If the external MAC system wishes to
use db_table:{delete} to decide if the privilege is allowed or not, they
can, but I don't think core should force that when we have them as
independent permissions.

So, perhaps we can argue about what the sepgsql extension should do, but
it's clear that we should have an independent hook for this in core.

Isn't there a way to allow an admin to control if db_table:{truncate} is
allowed for users with db_table:{delete}, or not? Ideally, this could
be managed at the SELinux level instead of having to have something
different in sepgsql or core, but if it needs to be configurable there
too then hopefully we can come up with a good solution.

Thanks,

Stephen

#4Yuli Khodorkovskiy
yuli.khodorkovskiy@crunchydata.com
In reply to: KaiGai Kohei (#2)
Re: add a MAC check for TRUNCATE

On Mon, Sep 2, 2019 at 10:58 AM Kohei KaiGai <kaigai@heterodb.com> wrote:

Hello Yuli,

Hello KaiGai,

2019年7月25日(木) 3:52 Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com>:

Since all DAC checks should have corresponding MAC, this patch adds a
hook to allow extensions to implement a MAC check on TRUNCATE. I have
also implemented this access check in the sepgsql extension.

One important thing to note is that refpolicy [1] and Redhat based
distributions do not have the SELinux permission for db_table {truncate}
implemented.

How db_table:{delete} permission is different from truncate?
From the standpoint of data access, TRUNCATE is equivalent to DELETE
without WHERE, isn't it?

To echo Stephen's reply, since TRUNCATE has a dedicated privilege in
the GRANT system, there should be a MAC based permission as well.
Increased granularity for an integrator to add least privileged policy
is a good idea in my view.

Of course, there are some differences between them. TRUNCATE takes
exclusive locks and eliminates underlying data blocks, on the other hands,
DELETE removes rows under MVCC manner. However, both of them
eventually removes data from the target table.

I like to recommend to reuse "db_table:{delete}" permission for TRUNCATE.
How about your opinions?

Now that I think about it, using "db_table { delete }" would be fine,
and that would remove the CIL requirement that I stated earlier. Thank
you for the suggestion. I'll send a v2 patch using the delete
permission.

Thank you,

Yuli

Show quoted text

Best regards,
--
HeteroDB, Inc / The PG-Strom Project
KaiGai Kohei <kaigai@heterodb.com>

#5Yuli Khodorkovskiy
yuli.khodorkovskiy@crunchydata.com
In reply to: Stephen Frost (#3)
Re: add a MAC check for TRUNCATE

On Tue, Sep 3, 2019 at 3:25 PM Stephen Frost <sfrost@snowman.net> wrote:

Greetings,

* Kohei KaiGai (kaigai@heterodb.com) wrote:

2019年7月25日(木) 3:52 Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com>:

Since all DAC checks should have corresponding MAC, this patch adds a
hook to allow extensions to implement a MAC check on TRUNCATE. I have
also implemented this access check in the sepgsql extension.

One important thing to note is that refpolicy [1] and Redhat based
distributions do not have the SELinux permission for db_table {truncate}
implemented.

How db_table:{delete} permission is different from truncate?

From the standpoint of data access, TRUNCATE is equivalent to DELETE

without WHERE, isn't it?
Of course, there are some differences between them. TRUNCATE takes
exclusive locks and eliminates underlying data blocks, on the other hands,
DELETE removes rows under MVCC manner. However, both of them
eventually removes data from the target table.

I like to recommend to reuse "db_table:{delete}" permission for TRUNCATE.
How about your opinions?

There's been much discussion and justifcation for adding an independent
TRUNCATE privilege to GRANT (which actually took many years to be
allowed). I don't see why we wouldn't represent that as a different
privilege to external MAC systems. If the external MAC system wishes to
use db_table:{delete} to decide if the privilege is allowed or not, they
can, but I don't think core should force that when we have them as
independent permissions.

So, perhaps we can argue about what the sepgsql extension should do, but
it's clear that we should have an independent hook for this in core.

Isn't there a way to allow an admin to control if db_table:{truncate} is
allowed for users with db_table:{delete}, or not? Ideally, this could
be managed at the SELinux level instead of having to have something
different in sepgsql or core, but if it needs to be configurable there
too then hopefully we can come up with a good solution.

If I understand you correctly, you are asking if an SELinux domain can
have the db_table:{truncate} permission but not db_table:{delete}
using SELinux policy? This would only work if the userspace object
manager, sepgsql in this case, reaches out to the policy server to
check if db_table:{truncate} is allowed for a subject accessing an
object.

I think it should be okay to use db_table:{delete} as the permission
to check for TRUNCATE in the object manager. I have attached a second
version of the hook and sepgsql changes to demonstrate this.

Thank you.

Show quoted text

Thanks,

Stephen

Attachments:

Truncate-Hook.patchapplication/octet-stream; name=Truncate-Hook.patchDownload+27-2
Sepgsql-Truncate.patchapplication/octet-stream; name=Sepgsql-Truncate.patchDownload+61-5
#6Stephen Frost
sfrost@snowman.net
In reply to: Yuli Khodorkovskiy (#5)
Re: add a MAC check for TRUNCATE

Greetings,

* Yuli Khodorkovskiy (yuli.khodorkovskiy@crunchydata.com) wrote:

On Tue, Sep 3, 2019 at 3:25 PM Stephen Frost <sfrost@snowman.net> wrote:

* Kohei KaiGai (kaigai@heterodb.com) wrote:

2019年7月25日(木) 3:52 Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com>:

Since all DAC checks should have corresponding MAC, this patch adds a
hook to allow extensions to implement a MAC check on TRUNCATE. I have
also implemented this access check in the sepgsql extension.

One important thing to note is that refpolicy [1] and Redhat based
distributions do not have the SELinux permission for db_table {truncate}
implemented.

How db_table:{delete} permission is different from truncate?

From the standpoint of data access, TRUNCATE is equivalent to DELETE

without WHERE, isn't it?
Of course, there are some differences between them. TRUNCATE takes
exclusive locks and eliminates underlying data blocks, on the other hands,
DELETE removes rows under MVCC manner. However, both of them
eventually removes data from the target table.

I like to recommend to reuse "db_table:{delete}" permission for TRUNCATE.
How about your opinions?

There's been much discussion and justifcation for adding an independent
TRUNCATE privilege to GRANT (which actually took many years to be
allowed). I don't see why we wouldn't represent that as a different
privilege to external MAC systems. If the external MAC system wishes to
use db_table:{delete} to decide if the privilege is allowed or not, they
can, but I don't think core should force that when we have them as
independent permissions.

So, perhaps we can argue about what the sepgsql extension should do, but
it's clear that we should have an independent hook for this in core.

Isn't there a way to allow an admin to control if db_table:{truncate} is
allowed for users with db_table:{delete}, or not? Ideally, this could
be managed at the SELinux level instead of having to have something
different in sepgsql or core, but if it needs to be configurable there
too then hopefully we can come up with a good solution.

If I understand you correctly, you are asking if an SELinux domain can
have the db_table:{truncate} permission but not db_table:{delete}
using SELinux policy? This would only work if the userspace object
manager, sepgsql in this case, reaches out to the policy server to
check if db_table:{truncate} is allowed for a subject accessing an
object.

I was saying that, I believe, it would be pretty straight-forward for an
SELinux admin to add db_table:{truncate} to whatever set of individuals
are allowed to use db_table:{delete}.

I think it should be okay to use db_table:{delete} as the permission
to check for TRUNCATE in the object manager. I have attached a second
version of the hook and sepgsql changes to demonstrate this.

There are actual reasons why the 'DELETE' privilege is *not* the same as
'TRUNCATE' in PostgreSQL and I'm really not convinced that we should
just be tossing that distinction out the window for users of SELinux. A
pretty obvious one is that DELETE triggers don't get fired for a
TRUNCATE command, but TRUNCATE also doesn't follow the same MVCC rules
that the rest of the system does.

If TRUNCATE and DELETE were the same, we'd only have DELETE and we would
just make it super-fast by implementing it the way we implement
TRUNCATE.

Thanks,

Stephen

#7Robert Haas
robertmhaas@gmail.com
In reply to: Stephen Frost (#6)
Re: add a MAC check for TRUNCATE

On Fri, Sep 6, 2019 at 10:40 AM Stephen Frost <sfrost@snowman.net> wrote:

There are actual reasons why the 'DELETE' privilege is *not* the same as
'TRUNCATE' in PostgreSQL and I'm really not convinced that we should
just be tossing that distinction out the window for users of SELinux.

+1.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#8Yuli Khodorkovskiy
yuli.khodorkovskiy@crunchydata.com
In reply to: Stephen Frost (#6)
Re: add a MAC check for TRUNCATE

On Fri, Sep 6, 2019 at 10:40 AM Stephen Frost <sfrost@snowman.net> wrote:

Greetings,

Hello Stephen,

* Yuli Khodorkovskiy (yuli.khodorkovskiy@crunchydata.com) wrote:

On Tue, Sep 3, 2019 at 3:25 PM Stephen Frost <sfrost@snowman.net> wrote:

* Kohei KaiGai (kaigai@heterodb.com) wrote:

2019年7月25日(木) 3:52 Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com>:

Since all DAC checks should have corresponding MAC, this patch adds a
hook to allow extensions to implement a MAC check on TRUNCATE. I have
also implemented this access check in the sepgsql extension.

One important thing to note is that refpolicy [1] and Redhat based
distributions do not have the SELinux permission for db_table {truncate}
implemented.

How db_table:{delete} permission is different from truncate?

From the standpoint of data access, TRUNCATE is equivalent to DELETE

without WHERE, isn't it?
Of course, there are some differences between them. TRUNCATE takes
exclusive locks and eliminates underlying data blocks, on the other hands,
DELETE removes rows under MVCC manner. However, both of them
eventually removes data from the target table.

I like to recommend to reuse "db_table:{delete}" permission for TRUNCATE.
How about your opinions?

There's been much discussion and justifcation for adding an independent
TRUNCATE privilege to GRANT (which actually took many years to be
allowed). I don't see why we wouldn't represent that as a different
privilege to external MAC systems. If the external MAC system wishes to
use db_table:{delete} to decide if the privilege is allowed or not, they
can, but I don't think core should force that when we have them as
independent permissions.

So, perhaps we can argue about what the sepgsql extension should do, but
it's clear that we should have an independent hook for this in core.

Isn't there a way to allow an admin to control if db_table:{truncate} is
allowed for users with db_table:{delete}, or not? Ideally, this could
be managed at the SELinux level instead of having to have something
different in sepgsql or core, but if it needs to be configurable there
too then hopefully we can come up with a good solution.

If I understand you correctly, you are asking if an SELinux domain can
have the db_table:{truncate} permission but not db_table:{delete}
using SELinux policy? This would only work if the userspace object
manager, sepgsql in this case, reaches out to the policy server to
check if db_table:{truncate} is allowed for a subject accessing an
object.

I was saying that, I believe, it would be pretty straight-forward for an
SELinux admin to add db_table:{truncate} to whatever set of individuals
are allowed to use db_table:{delete}.

Okay that makes sense. Yes that can definitely be done, and the
original sepgsql patch accomplished what you are describing. I did not
add tests or SELinux policy granting `db_table: { truncate }` in the
regressions of the original patch. If the community decides a new
SELinux permission in sepgsql for TRUNCATE is the correct path, I will
gladly update the original patch.

I think it should be okay to use db_table:{delete} as the permission
to check for TRUNCATE in the object manager. I have attached a second
version of the hook and sepgsql changes to demonstrate this.

There are actual reasons why the 'DELETE' privilege is *not* the same as
'TRUNCATE' in PostgreSQL and I'm really not convinced that we should
just be tossing that distinction out the window for users of SELinux. A
pretty obvious one is that DELETE triggers don't get fired for a
TRUNCATE command, but TRUNCATE also doesn't follow the same MVCC rules
that the rest of the system does.

I do agree with you there should be a distinction between TRUNCATE and
DELETE in the SELinux perms. I'll wait a few days for more discussion
and send an updated patch.

Thank you.

Show quoted text

If TRUNCATE and DELETE were the same, we'd only have DELETE and we would
just make it super-fast by implementing it the way we implement
TRUNCATE.

Thanks,

Stephen

#9Joe Conway
mail@joeconway.com
In reply to: Yuli Khodorkovskiy (#8)
Re: add a MAC check for TRUNCATE

On 9/6/19 11:26 AM, Yuli Khodorkovskiy wrote:

On Fri, Sep 6, 2019 at 10:40 AM Stephen Frost <sfrost@snowman.net> wrote:

There are actual reasons why the 'DELETE' privilege is *not* the same as
'TRUNCATE' in PostgreSQL and I'm really not convinced that we should
just be tossing that distinction out the window for users of SELinux. A
pretty obvious one is that DELETE triggers don't get fired for a
TRUNCATE command, but TRUNCATE also doesn't follow the same MVCC rules
that the rest of the system does.

I do agree with you there should be a distinction between TRUNCATE and
DELETE in the SELinux perms. I'll wait a few days for more discussion
and send an updated patch.

+1 - I don't think there is any question about it.

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

#10Yuli Khodorkovskiy
yuli.khodorkovskiy@crunchydata.com
In reply to: Yuli Khodorkovskiy (#8)
Re: add a MAC check for TRUNCATE

On Fri, Sep 6, 2019 at 11:26 AM Yuli Khodorkovskiy
<yuli.khodorkovskiy@crunchydata.com> wrote:

On Fri, Sep 6, 2019 at 10:40 AM Stephen Frost <sfrost@snowman.net> wrote:

Greetings,

Hello Stephen,

* Yuli Khodorkovskiy (yuli.khodorkovskiy@crunchydata.com) wrote:

On Tue, Sep 3, 2019 at 3:25 PM Stephen Frost <sfrost@snowman.net> wrote:

* Kohei KaiGai (kaigai@heterodb.com) wrote:

2019年7月25日(木) 3:52 Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com>:

Since all DAC checks should have corresponding MAC, this patch adds a
hook to allow extensions to implement a MAC check on TRUNCATE. I have
also implemented this access check in the sepgsql extension.

One important thing to note is that refpolicy [1] and Redhat based
distributions do not have the SELinux permission for db_table {truncate}
implemented.

How db_table:{delete} permission is different from truncate?

From the standpoint of data access, TRUNCATE is equivalent to DELETE

without WHERE, isn't it?
Of course, there are some differences between them. TRUNCATE takes
exclusive locks and eliminates underlying data blocks, on the other hands,
DELETE removes rows under MVCC manner. However, both of them
eventually removes data from the target table.

I like to recommend to reuse "db_table:{delete}" permission for TRUNCATE.
How about your opinions?

There's been much discussion and justifcation for adding an independent
TRUNCATE privilege to GRANT (which actually took many years to be
allowed). I don't see why we wouldn't represent that as a different
privilege to external MAC systems. If the external MAC system wishes to
use db_table:{delete} to decide if the privilege is allowed or not, they
can, but I don't think core should force that when we have them as
independent permissions.

So, perhaps we can argue about what the sepgsql extension should do, but
it's clear that we should have an independent hook for this in core.

Isn't there a way to allow an admin to control if db_table:{truncate} is
allowed for users with db_table:{delete}, or not? Ideally, this could
be managed at the SELinux level instead of having to have something
different in sepgsql or core, but if it needs to be configurable there
too then hopefully we can come up with a good solution.

If I understand you correctly, you are asking if an SELinux domain can
have the db_table:{truncate} permission but not db_table:{delete}
using SELinux policy? This would only work if the userspace object
manager, sepgsql in this case, reaches out to the policy server to
check if db_table:{truncate} is allowed for a subject accessing an
object.

I was saying that, I believe, it would be pretty straight-forward for an
SELinux admin to add db_table:{truncate} to whatever set of individuals
are allowed to use db_table:{delete}.

Okay that makes sense. Yes that can definitely be done, and the
original sepgsql patch accomplished what you are describing. I did not
add tests or SELinux policy granting `db_table: { truncate }` in the
regressions of the original patch. If the community decides a new
SELinux permission in sepgsql for TRUNCATE is the correct path, I will
gladly update the original patch.

Ah, now I remember why I didn't add regressions to the original patch.
As stated at the top of the thread, the "db_table: { truncate }"
permission does not currently exist in refpolicy. A workaround would
be to add the policy with CIL, but that adds unneeded complexity to
the regressions. I think the correct path forward is:

1) Get the sepgsql changes in without policy/regressions
2) Send a patch to refpolicy for the new permission
3) Once Redhat updates the selinux-policy-targeted RPM to include the
new permissions, I will send an update to the sepgsql regressions and
policy.

Thank you.

Show quoted text

I think it should be okay to use db_table:{delete} as the permission
to check for TRUNCATE in the object manager. I have attached a second
version of the hook and sepgsql changes to demonstrate this.

There are actual reasons why the 'DELETE' privilege is *not* the same as
'TRUNCATE' in PostgreSQL and I'm really not convinced that we should
just be tossing that distinction out the window for users of SELinux. A
pretty obvious one is that DELETE triggers don't get fired for a
TRUNCATE command, but TRUNCATE also doesn't follow the same MVCC rules
that the rest of the system does.

I do agree with you there should be a distinction between TRUNCATE and
DELETE in the SELinux perms. I'll wait a few days for more discussion
and send an updated patch.

Thank you.

If TRUNCATE and DELETE were the same, we'd only have DELETE and we would
just make it super-fast by implementing it the way we implement
TRUNCATE.

Thanks,

Stephen

#11Tom Lane
tgl@sss.pgh.pa.us
In reply to: Yuli Khodorkovskiy (#10)
Re: add a MAC check for TRUNCATE

Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com> writes:

Ah, now I remember why I didn't add regressions to the original patch.
As stated at the top of the thread, the "db_table: { truncate }"
permission does not currently exist in refpolicy. A workaround would
be to add the policy with CIL, but that adds unneeded complexity to
the regressions. I think the correct path forward is:

1) Get the sepgsql changes in without policy/regressions
2) Send a patch to refpolicy for the new permission
3) Once Redhat updates the selinux-policy-targeted RPM to include the
new permissions, I will send an update to the sepgsql regressions and
policy.

That's going to be a problem. I do not think it will be acceptable
to commit tests that fail on less-than-bleeding-edge SELinux.

regards, tom lane

#12Stephen Frost
sfrost@snowman.net
In reply to: Tom Lane (#11)
Re: add a MAC check for TRUNCATE

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com> writes:

Ah, now I remember why I didn't add regressions to the original patch.
As stated at the top of the thread, the "db_table: { truncate }"
permission does not currently exist in refpolicy. A workaround would
be to add the policy with CIL, but that adds unneeded complexity to
the regressions. I think the correct path forward is:

1) Get the sepgsql changes in without policy/regressions
2) Send a patch to refpolicy for the new permission
3) Once Redhat updates the selinux-policy-targeted RPM to include the
new permissions, I will send an update to the sepgsql regressions and
policy.

That's going to be a problem. I do not think it will be acceptable
to commit tests that fail on less-than-bleeding-edge SELinux.

This is why I was suggesting up-thread that it'd be neat if we made this
somehow optional, though I don't quite see a way to do that sensibly.

We could though, of course, make running the regression test optional
and then have a buildfarm member that's got the bleeding-edge SELinux
(or is just configured with the additional control) and then have it
enabled there.

Thanks,

Stephen

#13Yuli Khodorkovskiy
yuli.khodorkovskiy@crunchydata.com
In reply to: Tom Lane (#11)
Re: add a MAC check for TRUNCATE

On Fri, Sep 6, 2019 at 11:47 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com> writes:

Ah, now I remember why I didn't add regressions to the original patch.
As stated at the top of the thread, the "db_table: { truncate }"
permission does not currently exist in refpolicy. A workaround would
be to add the policy with CIL, but that adds unneeded complexity to
the regressions. I think the correct path forward is:

1) Get the sepgsql changes in without policy/regressions
2) Send a patch to refpolicy for the new permission
3) Once Redhat updates the selinux-policy-targeted RPM to include the
new permissions, I will send an update to the sepgsql regressions and
policy.

That's going to be a problem. I do not think it will be acceptable
to commit tests that fail on less-than-bleeding-edge SELinux.

regards, tom lane

The tests pass as long as deny_unknown is set to 0, which is the
default on fedora 30.

#14Tom Lane
tgl@sss.pgh.pa.us
In reply to: Stephen Frost (#12)
Re: add a MAC check for TRUNCATE

Stephen Frost <sfrost@snowman.net> writes:

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com> writes:

1) Get the sepgsql changes in without policy/regressions
2) Send a patch to refpolicy for the new permission
3) Once Redhat updates the selinux-policy-targeted RPM to include the
new permissions, I will send an update to the sepgsql regressions and
policy.

That's going to be a problem. I do not think it will be acceptable
to commit tests that fail on less-than-bleeding-edge SELinux.

This is why I was suggesting up-thread that it'd be neat if we made this
somehow optional, though I don't quite see a way to do that sensibly.
We could though, of course, make running the regression test optional
and then have a buildfarm member that's got the bleeding-edge SELinux
(or is just configured with the additional control) and then have it
enabled there.

Well, the larger question, independent of the regression tests, is
will the new policy work at all on older SELinux? If not, that
doesn't seem very acceptable. Worse, it implies we're going to
have another flag day anytime we want to add any new element
to sepgsql's view of the universe. I think we need some hard
thought about upgrade paths here --- at least, if we want to
believe that sepgsql is anything but a toy for demonstration
purposes.

regards, tom lane

#15Yuli Khodorkovskiy
yuli.khodorkovskiy@crunchydata.com
In reply to: Tom Lane (#14)
Re: add a MAC check for TRUNCATE

On Fri, Sep 6, 2019 at 11:57 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Stephen Frost <sfrost@snowman.net> writes:

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com> writes:

1) Get the sepgsql changes in without policy/regressions
2) Send a patch to refpolicy for the new permission
3) Once Redhat updates the selinux-policy-targeted RPM to include the
new permissions, I will send an update to the sepgsql regressions and
policy.

That's going to be a problem. I do not think it will be acceptable
to commit tests that fail on less-than-bleeding-edge SELinux.

This is why I was suggesting up-thread that it'd be neat if we made this
somehow optional, though I don't quite see a way to do that sensibly.
We could though, of course, make running the regression test optional
and then have a buildfarm member that's got the bleeding-edge SELinux
(or is just configured with the additional control) and then have it
enabled there.

Well, the larger question, independent of the regression tests, is
will the new policy work at all on older SELinux? If not, that
doesn't seem very acceptable. Worse, it implies we're going to
have another flag day anytime we want to add any new element
to sepgsql's view of the universe. I think we need some hard
thought about upgrade paths here --- at least, if we want to
believe that sepgsql is anything but a toy for demonstration
purposes.

regards, tom lane

The default SELinux policy on Fedora ships with deny_unknown set to 0.
Deny_unknown was added to the kernel in 2.6.24, so unless someone is
using RHEL 5.x, which is in ELS, they will have the ability to
override the default behavior on CentOS/RHEL.

CIL was added to RHEL starting with RHEL 7. As stated before, an
integrator can export the base module and override the deny_unknown
behavior.

On RHEL 6, which goes into ELS in 2020, it's a bit more complicated
and requires rebuilding the base SELinux module from source.

Hope this helps,

Yuli

#16Yuli Khodorkovskiy
yuli.khodorkovskiy@crunchydata.com
In reply to: Yuli Khodorkovskiy (#15)
Re: add a MAC check for TRUNCATE

As Joe Conway pointed out to me out of band, the build animal for RHEL
7 has handle_unknown set to `0`. Are there any other concerns with
this approach?

On Fri, Sep 6, 2019 at 1:00 PM Yuli Khodorkovskiy
<yuli.khodorkovskiy@crunchydata.com> wrote:

Show quoted text

On Fri, Sep 6, 2019 at 11:57 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Stephen Frost <sfrost@snowman.net> writes:

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com> writes:

1) Get the sepgsql changes in without policy/regressions
2) Send a patch to refpolicy for the new permission
3) Once Redhat updates the selinux-policy-targeted RPM to include the
new permissions, I will send an update to the sepgsql regressions and
policy.

That's going to be a problem. I do not think it will be acceptable
to commit tests that fail on less-than-bleeding-edge SELinux.

This is why I was suggesting up-thread that it'd be neat if we made this
somehow optional, though I don't quite see a way to do that sensibly.
We could though, of course, make running the regression test optional
and then have a buildfarm member that's got the bleeding-edge SELinux
(or is just configured with the additional control) and then have it
enabled there.

Well, the larger question, independent of the regression tests, is
will the new policy work at all on older SELinux? If not, that
doesn't seem very acceptable. Worse, it implies we're going to
have another flag day anytime we want to add any new element
to sepgsql's view of the universe. I think we need some hard
thought about upgrade paths here --- at least, if we want to
believe that sepgsql is anything but a toy for demonstration
purposes.

regards, tom lane

The default SELinux policy on Fedora ships with deny_unknown set to 0.
Deny_unknown was added to the kernel in 2.6.24, so unless someone is
using RHEL 5.x, which is in ELS, they will have the ability to
override the default behavior on CentOS/RHEL.

CIL was added to RHEL starting with RHEL 7. As stated before, an
integrator can export the base module and override the deny_unknown
behavior.

On RHEL 6, which goes into ELS in 2020, it's a bit more complicated
and requires rebuilding the base SELinux module from source.

Hope this helps,

Yuli

#17Tom Lane
tgl@sss.pgh.pa.us
In reply to: Yuli Khodorkovskiy (#15)
Re: add a MAC check for TRUNCATE

Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com> writes:

On Fri, Sep 6, 2019 at 11:57 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Well, the larger question, independent of the regression tests, is
will the new policy work at all on older SELinux? If not, that
doesn't seem very acceptable.

The default SELinux policy on Fedora ships with deny_unknown set to 0.
Deny_unknown was added to the kernel in 2.6.24, so unless someone is
using RHEL 5.x, which is in ELS, they will have the ability to
override the default behavior on CentOS/RHEL.

OK, that sounds like it will work.

On RHEL 6, which goes into ELS in 2020, it's a bit more complicated
and requires rebuilding the base SELinux module from source.

sepgsql hasn't worked on RHEL6 in a long time, if ever; it requires
a newer version of libselinux than what ships in RHEL6. So I'm not
concerned about that. We do need to worry about RHEL7, and whatever
is the oldest version of Fedora that is running the sepgsql tests
in the buildfarm.

regards, tom lane

#18Joe Conway
mail@joeconway.com
In reply to: Tom Lane (#17)
Re: add a MAC check for TRUNCATE

On 9/6/19 2:18 PM, Tom Lane wrote:

Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com> writes:

On Fri, Sep 6, 2019 at 11:57 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Well, the larger question, independent of the regression tests, is
will the new policy work at all on older SELinux? If not, that
doesn't seem very acceptable.

The default SELinux policy on Fedora ships with deny_unknown set to 0.
Deny_unknown was added to the kernel in 2.6.24, so unless someone is
using RHEL 5.x, which is in ELS, they will have the ability to
override the default behavior on CentOS/RHEL.

OK, that sounds like it will work.

On RHEL 6, which goes into ELS in 2020, it's a bit more complicated
and requires rebuilding the base SELinux module from source.

sepgsql hasn't worked on RHEL6 in a long time, if ever; it requires
a newer version of libselinux than what ships in RHEL6. So I'm not
concerned about that. We do need to worry about RHEL7, and whatever
is the oldest version of Fedora that is running the sepgsql tests
in the buildfarm.

I could be wrong, but as far as I know rhinoceros is the only buildfarm
animal running sepgsql tests.

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

#19Joe Conway
mail@joeconway.com
In reply to: Yuli Khodorkovskiy (#16)
Re: add a MAC check for TRUNCATE

On 9/6/19 2:13 PM, Yuli Khodorkovskiy wrote:

As Joe Conway pointed out to me out of band, the build animal for RHEL
7 has handle_unknown set to `0`. Are there any other concerns with
this approach?

You mean deny_unknown I believe.

"Allow unknown object class / permissions. This will set the returned AV
with all 1's."

As I understand it, this would make the sepgsql behavior unchanged from
before if the policy does not support the new permission.

Joe

On Fri, Sep 6, 2019 at 1:00 PM Yuli Khodorkovskiy wrote:

The default SELinux policy on Fedora ships with deny_unknown set to 0.
Deny_unknown was added to the kernel in 2.6.24, so unless someone is
using RHEL 5.x, which is in ELS, they will have the ability to
override the default behavior on CentOS/RHEL.

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

#20Yuli Khodorkovskiy
yuli.khodorkovskiy@crunchydata.com
In reply to: Joe Conway (#19)
Re: add a MAC check for TRUNCATE

On Fri, Sep 6, 2019 at 4:31 PM Joe Conway <mail@joeconway.com> wrote:

On 9/6/19 2:13 PM, Yuli Khodorkovskiy wrote:

As Joe Conway pointed out to me out of band, the build animal for RHEL
7 has handle_unknown set to `0`. Are there any other concerns with
this approach?

You mean deny_unknown I believe.

I do, thanks. Not sure where I pulled handle_unknown from.

Show quoted text

"Allow unknown object class / permissions. This will set the returned AV
with all 1's."

As I understand it, this would make the sepgsql behavior unchanged from
before if the policy does not support the new permission.

Joe

On Fri, Sep 6, 2019 at 1:00 PM Yuli Khodorkovskiy wrote:

The default SELinux policy on Fedora ships with deny_unknown set to 0.
Deny_unknown was added to the kernel in 2.6.24, so unless someone is
using RHEL 5.x, which is in ELS, they will have the ability to
override the default behavior on CentOS/RHEL.

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

#21Tom Lane
tgl@sss.pgh.pa.us
In reply to: Joe Conway (#18)
#22Joe Conway
mail@joeconway.com
In reply to: Tom Lane (#21)
#23Yuli Khodorkovskiy
yuli.khodorkovskiy@crunchydata.com
In reply to: Joe Conway (#22)
#24Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Yuli Khodorkovskiy (#1)
#25Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Yuli Khodorkovskiy (#23)
#26Joe Conway
mail@joeconway.com
In reply to: Alvaro Herrera (#25)
#27Yuli Khodorkovskiy
yuli.khodorkovskiy@crunchydata.com
In reply to: Alvaro Herrera (#25)
#28Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Yuli Khodorkovskiy (#27)
#29Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#28)
#30Yuli Khodorkovskiy
yuli.khodorkovskiy@crunchydata.com
In reply to: Tom Lane (#29)
#31Joe Conway
mail@joeconway.com
In reply to: Joe Conway (#26)
#32Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Joe Conway (#31)
#33Michael Paquier
michael@paquier.xyz
In reply to: Alvaro Herrera (#32)
#34Yuli Khodorkovskiy
yuli.khodorkovskiy@crunchydata.com
In reply to: Michael Paquier (#33)
#35Joe Conway
mail@joeconway.com
In reply to: Yuli Khodorkovskiy (#34)
#36Joe Conway
mail@joeconway.com
In reply to: Joe Conway (#35)
#37Joe Conway
mail@joeconway.com
In reply to: Joe Conway (#36)
#38Michael Paquier
michael@paquier.xyz
In reply to: Joe Conway (#36)
#39Joe Conway
mail@joeconway.com
In reply to: Michael Paquier (#38)