dbus-broker
dbus-broker copied to clipboard
bus1
Apparmor support
Hi,
This adds support for AppArmor using libapparmor and the Ubuntu kernel patch requested in #169. For the implementation dbus-daemon has been used as reference. The patchset handles restriction of all common DBus operations (sending, receiving, bus owning and monitoring) - just like dbus-daemon.
Note, that the downstream kernel patch is no longer limited to Ubuntu. It has been applied to some kernels used in the embedded sector. Missing support for AppArmor is the limiting factor to switch from dbus-daemon to dbus-broker on these embedded systems and the reason I wrote this code.
Regarding the status of the kernel patch required for this support: The main reason, that the kernel is still missing support for kernel based af_unix/dbus meditation is a pending code restructuring that got postponed for multiple years. Current expectation is, that this restructuring finally happens in the 5.19 cycle (fingers crossed).
-- Sebastian
Thanks for having a look at it. I will check the things you pointed out next week and also try to figure out a way to split the second patch into smaller chunks.
@sre Is there a link to the patch review on lore for the revised patch slated for 5.19 yet?
@sre Is there a link to the patch review on lore for the revised patch slated for 5.19 yet?
No, not that I'm aware of.
So I pulled in most of the preparatory patches, and I adjusted some bits. The major apparmor.c code is still missing. I am working on it.
Thanks, the changes look fine to me.
Let me know if something is unclear, or if I unintentionally broke something.
I just rebased the pull-request to current HEAD dropping the merged patches (no other changes) and the branch is still working. I will look into the CI issues and the bus_type problem.
I am quite confused now where you got the transmission checks from. dbus-daemon simply checks with the destination-string and source-unique-id, but your code iterates the set of names on the destination. This will very easily break existing AppArmor rules. I also don't understand why that change was done. Is there any reason not to imitate what dbus-daemon does?
That is, I would make apparmor check whether the sender is allowed to send to the destination used in the message. Then check whether the receiver is allowed to receive from the unique-id of the sender. If either is false, refuse the transmission.
I can implement this, but I want to make sure you agree on that, and that this is indeed the intended behavior?
From an AppArmor perspective, this LGTM. We updated our regression tests upstream and they are passing with these patches.
As far as I can tell the CI is currently unaware of AppArmor in the sense that -Dapparmor=true isn't passed to meson anywhere. At least CodeQL analyzes the apparmor fallback only as far as I can see because libapparmor-dev isn't installed there and the "autobuild" job can't figure out what to do. The other CI workflow passes -Daudit=true only. There it would probably make sense to build dbus-broker with both -Dapparmor=true and -Dapparmor=false to make sure it compiles fine with -Werror with and without libapparmor.
(I'm not sure it should be fixed here but I think it should be fixed one way or another).
looks like arch is adopting dbus-broker as default. any chance this can be merged?
Arch kernels don't have apparmor dbus mediation patches so included so this is irrelevant.
Arch kernels don't have apparmor dbus mediation patches so included so this is irrelevant.
I don't see how that can be, but I'm not an expert. Per https://wiki.archlinux.org/title/AppArmor I have it enabled and running, a service loaded, a kernel option enabled, aa-enabled returns true, I see "DENIED" in my logs sometimes, etc.
The required AppArmor features to get D-Bus message security policies working are not part of upstream linux kernels. Basic AppArmor support has been included in upstream kernels, though.
So to get AppArmor working with dbus-broker, you would definitely need downstream patches. Ubuntu applies them to all their kernel builds.