Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AppArmor support #169

Open
bitstreamout opened this issue Oct 10, 2018 · 10 comments
Open

AppArmor support #169

bitstreamout opened this issue Oct 10, 2018 · 10 comments

Comments

@bitstreamout
Copy link

I see this here in the journal

 dbus-broker-launch[1035]: AppArmor enabled, but not supported. Ignoring.
@dvdhrm
Copy link
Member

dvdhrm commented Oct 10, 2018

The AppArmor code is, to a large extent, a downstream patchset carried by Ubuntu in their respective packages. The upstream linux kernel does not have the required AppArmor features to implement AppArmor-support in dbus-broker. In particular, SO_PEERSEC is not sufficiently implemented.

dbus-daemon(1) carries patches against the downstream AppArmor code of Ubuntu. We currently are a bit reluctant to add code that relies on downstream kernel features. However, we did adjust the dbus-broker infrastructure to easily allow downstream patches that add AppArmor support. Meaning, we already parse all the AppArmor related configuration and can easily insert the required AppArmor hooks.

@bitstreamout
Copy link
Author

Hmm ... the apparmor patch set is part of our kernel here. Otherwise I would not see this messages I guess. I found https://lkml.org/lkml/2018/5/4/476 ... now let's see if your patchset will reach the kernel

@dvdhrm
Copy link
Member

dvdhrm commented Oct 11, 2018

Basic Apparmor support is upstream in the linux kernel. If you now enable apparmor support in your dbus configuration, you will see the message you mentioned. The are two modes to run dbus-AppArmor in: enforcing or informative. Your configuration apparently uses the latter, hence AppArmor is ignored by dbus-broker, as it is informative only. If you selected the enforcing mode, dbus-broker would refuse to start (since it lacks apparmor support).

If anyone wants enforcing behavior of AppArmor in dbus-broker, they have to patch it into dbus-broker manually, or upstream the AppArmor patches so we can work on it.

The patchset you linked to is not related to this issue. It was already merged upstream and is about SO_PEERSEC support on socketpairs.

@dvdhrm
Copy link
Member

dvdhrm commented Mar 9, 2019

For documentational purposes:

The downstream Ubuntu patch that currently provides the required kernel functionality is:

https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/cosmic/commit/security/apparmor?id=2d33c63704a49b005dfa4597882be265ce0bdb06

I saved it on github as well:

https://gist.github.com/dvdhrm/0b8c556960957970d646c9bba814692a

In particular, we need upstream kernel sources to include:

+	LSM_HOOK_INIT(unix_stream_connect, apparmor_unix_stream_connect),
+	LSM_HOOK_INIT(unix_may_send, apparmor_unix_may_send),

Or something along those lines. Also, ctx->peer is currently a read-only dummy. This field needs to be written to somewhere to update AF_UNIX connection state.

I will keep this issue open for now, so we can track upstream AppArmor progress.

@DemiMarie
Copy link

This is a case where I wish D-Bus clients sent a /proc/self FD at connection. We already know the client’s PID, and can check that it has not been recycled by opening /proc/$PID and checking that the device and inode numbers match. If the launcher passes an FD pointing to /proc, we don’t even need access to /proc ourselves.

@dvdhrm
Copy link
Member

dvdhrm commented Dec 21, 2020

This is a case where I wish D-Bus clients sent a /proc/self FD at connection. We already know the client’s PID, and can check that it has not been recycled by opening /proc/$PID and checking that the device and inode numbers match. If the launcher passes an FD pointing to /proc, we don’t even need access to /proc ourselves.

I am not sure how this is related to AppArmor support? This is a tracking issue waiting for AppArmor-patches to be upstreamed to the linux sources.

Nevertheless, I agree that an FD to /proc is a quite elegant solution. But so far the linux ABI kinda expects /proc to be mounted, so in real-world scenarios it makes little difference. This gets more interesting, when we consider open FDs on /proc/<pid> as process handles. It does definitely get rid of pid-recycle-races. However, due to it being O_PATH FDs, it would not solve permission issuey, since it would still not be a proper capability-based model.

The recent pidfd_xyz() syscalls went into the right direction, but then they decided to still perform permission checks for all operations on those FDs, thus completely subverting the idea of using process handles to "own" access to a process.

Is there a particular improvement you have in mind?

@DemiMarie
Copy link

This is a case where I wish D-Bus clients sent a /proc/self FD at connection. We already know the client’s PID, and can check that it has not been recycled by opening /proc/$PID and checking that the device and inode numbers match. If the launcher passes an FD pointing to /proc, we don’t even need access to /proc ourselves.

I am not sure how this is related to AppArmor support? This is a tracking issue waiting for AppArmor-patches to be upstreamed to the linux sources.

Nevertheless, I agree that an FD to /proc is a quite elegant solution. But so far the linux ABI kinda expects /proc to be mounted, so in real-world scenarios it makes little difference. This gets more interesting, when we consider open FDs on /proc/<pid> as process handles. It does definitely get rid of pid-recycle-races. However, due to it being O_PATH FDs, it would not solve permission issuey, since it would still not be a proper capability-based model.

The recent pidfd_xyz() syscalls went into the right direction, but then they decided to still perform permission checks for all operations on those FDs, thus completely subverting the idea of using process handles to "own" access to a process.

Is there a particular improvement you have in mind?

Once we have a race-free way to get the client’s /proc/$PID FD, we can obtain their AppArmor context from /proc, without using SO_PEERSEC. For this to work, a client must send a /proc/self FD, so that we can check that their PID hasn’t been reused.

@dvdhrm
Copy link
Member

dvdhrm commented Dec 21, 2020

Once we have a race-free way to get the client’s /proc/$PID FD, we can obtain their AppArmor context from /proc, without using SO_PEERSEC. For this to work, a client must send a /proc/self FD, so that we can check that their PID hasn’t been reused.

But is the reduced AppArmor kernel module enough to provide the required level of support for the actual enforcement of the runtime policies? I was under the impression that those patches are needed to implement a meaningful policy.

Is there actual interest in AppArmor support for dbus-broker? So far, there was little interest in the Ubuntu development community to adopt dbus-broker, so we never pushed for AppArmor support.

@DemiMarie
Copy link

Once we have a race-free way to get the client’s /proc/$PID FD, we can obtain their AppArmor context from /proc, without using SO_PEERSEC. For this to work, a client must send a /proc/self FD, so that we can check that their PID hasn’t been reused.

But is the reduced AppArmor kernel module enough to provide the required level of support for the actual enforcement of the runtime policies? I was under the impression that those patches are needed to implement a meaningful policy.

Is there actual interest in AppArmor support for dbus-broker? So far, there was little interest in the Ubuntu development community to adopt dbus-broker, so we never pushed for AppArmor support.

I have no idea. I just know that this can be used whenever we need information from a client’s /proc.

@dvdhrm
Copy link
Member

dvdhrm commented Dec 21, 2020

Ok!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants