Bug 49062 - [PATCH] allow D-Bus starting up without CAP_AUDIT_WRITE available
Summary: [PATCH] allow D-Bus starting up without CAP_AUDIT_WRITE available
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Colin Walters
QA Contact: John (J5) Palmieri
URL:
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2012-04-22 15:59 UTC by Lennart Poettering
Modified: 2013-11-01 20:18 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


Attachments
the patch (1.26 KB, patch)
2012-04-22 15:59 UTC, Lennart Poettering
Details | Splinter Review
0001-bus-selinux-Fix-previous-commit-for-CAP_AUDIT_WRITE-.patch (1.33 KB, patch)
2013-10-27 20:23 UTC, Colin Walters
Details | Splinter Review

Description Lennart Poettering 2012-04-22 15:59:33 UTC
Created attachment 60468 [details] [review]
the patch

When D-Bus is used in containers it will usually be executed without the CAP_AUDIT_WRITE capability. Currently when it tries to drop capabilities it tries to keep CAP_AUDIT_WRITE which will fail with EPERM if it didn't have i in the first place. The attached patch changes D-Bus to keep CAP_AUDIT_WRITE only if it actually has the capability.
Comment 1 Thiago Macieira 2012-04-23 01:33:49 UTC
Comment on attachment 60468 [details] [review]
the patch

Review of attachment 60468 [details] [review]:
-----------------------------------------------------------------

Patch looks good, but I don't know the API to be completely sure.
Comment 2 Simon McVittie 2012-07-19 10:46:16 UTC
Comment on attachment 60468 [details] [review]
the patch

Review of attachment 60468 [details] [review]:
-----------------------------------------------------------------

Please ask a SELinux expert to check this before applying (I assume Red Hat can find such a person), but it looks reasonable.
Comment 3 Simon McVittie 2012-11-09 10:50:17 UTC
(In reply to comment #2)
> Please ask a SELinux expert to check this before applying (I assume Red Hat
> can find such a person)

Ping?
Comment 4 Simon McVittie 2013-01-23 16:06:34 UTC
Re-ping. Lennart, if you want this patch to land, you will have to find someone with SELinux expertise (not necessarily a D-Bus expert) to check it - perhaps Red Hat's SELinux maintainer? Otherwise, it'll remain unmerged.
Comment 5 Lennart Poettering 2013-01-23 19:12:48 UTC
Well, let's just say that discussing audit with the audit guy seldom yields any results.

I did this patch initially so that containers could turn off auditing by dropping caps, as the kernel audit layer is too broken to deal with containers.

In nspawn (systemd's container tool) I recently decided to just grant the audit caps by default. This will fuck up the audit logs on the host, but then again, who uses audit anyway, so nobody will notice... 

Ultimately for my specific problem the best solution if somebody would just teach the kernel audit layer to handle containers properly.

So my interest in getting this fixed is not that big anymore, but it still should be fixed, as what D-Bus is currently doing is simply wrong. That all said, I don't think we'll get a review from any audit folks for this anytime soon...
Comment 6 Simon McVittie 2013-09-05 12:58:56 UTC
Colin, do you know enough about SELinux/audit to review this?
Comment 7 Colin Walters 2013-09-05 15:08:22 UTC
(In reply to comment #6)
> Colin, do you know enough about SELinux/audit to review this?

It won't affect the normal case where we have all caps; what the semantics should be between audit and containers is still...up in the air, let's say.

So there's no harm from the patch, but on the other hand, if systemd doesn't actually need it anymore, then...tougher call. 

I'd say we should take it.
Comment 8 Simon McVittie 2013-09-13 13:24:42 UTC
Applied for 1.7.6
Comment 9 Laurent Bigonville 2013-10-27 15:01:34 UTC
Hi,

I might be wrong here, but with this patch applied, and dbus running outside of a container, I have no capabilities at all associated with the system bus.

If I'm reverting this patch, getpcaps shows me the process has the following capability: "cap_audit_write+ep"

Am I missing something here?
Comment 10 Colin Walters 2013-10-27 20:22:54 UTC
(In reply to comment #9)
> Hi,
> 
> I might be wrong here, but with this patch applied, and dbus running outside
> of a container, I have no capabilities at all associated with the system bus.
> 
> If I'm reverting this patch, getpcaps shows me the process has the following
> capability: "cap_audit_write+ep"

Blah, ok from a quick read of the libcap-ng source I bet we need this patch (not tested locally yet):
Comment 11 Colin Walters 2013-10-27 20:23:18 UTC
Created attachment 88198 [details] [review]
0001-bus-selinux-Fix-previous-commit-for-CAP_AUDIT_WRITE-.patch
Comment 12 Colin Walters 2013-10-27 20:24:08 UTC
Laurent (or anyone else), if you have a chance to test that'd be great, otherwise I should be able to do it sometime next week after my office is reconstructed and I have a testing environment again.
Comment 13 Laurent Bigonville 2013-10-27 20:47:16 UTC
OK, just tested and now the system bus has the "cap_audit_write+ep" capability

Thanks for the patch
Comment 14 Simon McVittie 2013-10-28 11:05:14 UTC
Comment on attachment 88198 [details] [review]
0001-bus-selinux-Fix-previous-commit-for-CAP_AUDIT_WRITE-.patch

Review of attachment 88198 [details] [review]:
-----------------------------------------------------------------

Seems sane to me, so with Reviewed-by and Tested-by Laurent, it seems OK.

If you can get a SELinux expert to have a look at it (I hear Red Hat/Fedora like SELinux, so maybe you can find one there?) so much the better.
Comment 15 Colin Walters 2013-10-28 16:26:41 UTC
(In reply to comment #14)
> Comment on attachment 88198 [details] [review] [review]
> 0001-bus-selinux-Fix-previous-commit-for-CAP_AUDIT_WRITE-.patch
> 
> Review of attachment 88198 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> Seems sane to me, so with Reviewed-by and Tested-by Laurent, it seems OK.
> 
> If you can get a SELinux expert to have a look at it (I hear Red Hat/Fedora
> like SELinux, so maybe you can find one there?) so much the better.

I currently maintain DBus in Red Hat Enterprise Linux, and in the past did extensive work with SELinux: http://www.nsa.gov/research/selinux/contrib.shtml
(Including porting to Debian)

What I *didn't* know well before was the libcap API.  If you want another reviewer though, Lennart should be OK.

Lennart?
Comment 16 Lennart Poettering 2013-10-28 18:42:53 UTC
Patch looks good afaics, but I didn't test it. Thanks!
Comment 17 Simon McVittie 2013-10-29 13:13:47 UTC
Good to merge whenever convenient then, thanks!

I'll try this out in Debian experimental, since Laurent wanted audit support there.
Comment 18 Simon McVittie 2013-11-01 20:18:58 UTC
Fixed in git for 1.7.8


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.