Bug 88811

Summary: CVE-2015-0245: systemd activation in dbus >= 1.4 is subject to local denial of service
Product: dbus Reporter: Simon McVittie <smcv>
Component: coreAssignee: Simon McVittie <smcv>
Status: RESOLVED FIXED QA Contact: D-Bus Maintainers <dbus>
Severity: normal    
Priority: high CC: alban.crequy, amigadave, bugzilla, lennart, sjoerd, thiago, walters
Version: 1.4.xKeywords: patch, security
Hardware: Other   
OS: All   
Whiteboard: review+
i915 platform: i915 features:
Attachments: [PATCH 1/4] bus_driver_get_owner_of_name: factor out from bus_driver_get_conn_helper
[PATCH 2/4] Prevent a local attacker from making systemd activation appear to fail
[master only] Add a regression test for making systemd activation appear to fail
[PATCH 4/4] Prevent forged ActivationFailure messages from non-root processes
[1.8] CVE-2015-0245: prevent forged ActivationFailure from non-root processes
[1.8] bus_driver_get_owner_of_name: factor out from bus_driver_get_conn_helper
[1.8] CVE-2015-0245: discard forged ActivationFailure messages

Description Simon McVittie 2015-01-26 20:50:19 UTC
We haven't had a regression test for systemd activation in the 4.5 years since it was implemented (dbus 1.4.0). I finally wrote one recently (while trying to get good test coverage for Bug #46787), and perhaps predictably, while implementing the test I noticed that it was vulnerable to a local denial of service.
Comment 1 Simon McVittie 2015-01-26 20:51:57 UTC
Created attachment 112849 [details] [review]
[PATCH 1/4] bus_driver_get_owner_of_name: factor out from  bus_driver_get_conn_helper

---

Part of a "nice" code fix for master. Opinions welcome on whether I should backport this to 1.8; I'm certainly not going back as far as 1.6.
Comment 2 Simon McVittie 2015-01-26 20:52:26 UTC
Created attachment 112850 [details] [review]
[PATCH 2/4] Prevent a local attacker from making systemd activation  appear to fail

---

This is the actual code fix. Requires patch 1.
Comment 3 Simon McVittie 2015-01-26 20:52:58 UTC
Created attachment 112851 [details] [review]
[master only] Add a regression test for making systemd activation  appear to fail

---

Requires Bug #57952 fixed. Certainly not going to be backported.
Comment 4 Simon McVittie 2015-01-26 20:53:15 UTC
Created attachment 112852 [details] [review]
[PATCH 4/4] Prevent forged ActivationFailure messages from non-root  processes

This is redundant with the code fix but is likely to be easier to
backport.
Comment 5 Simon McVittie 2015-01-26 20:56:54 UTC
Proof of concept (it's easier to demonstrate with a service whose activation succeeds or fails slowly):

% cat /usr/share/dbus-1/system-services/com.example.Slow.service
[D-BUS Service]
Name=com.example.Slow
Exec=/bin/sleep 60
User=root
SystemdService=dbus-com.example.Slow.service
% cat /etc/systemd/system/dbus-com.example.Slow.service 
[Unit]
Description=Slow service

[Service]
Type=dbus
BusName=com.example.Slow
ExecStart=/bin/sleep 60

% dbus-send --system --dest=com.example.Slow / org.freedesktop.DBus.Peer.Ping &
% dbus-send --system --type=signal --dest=org.freedesktop.DBus /org/freedesktop/systemd1 org.freedesktop.systemd1.Activator.ActivationFailure string:dbus-com.example.Slow.service string:com.example.Badness string:"This is bad"

"Good" result: Forged ActivationFailure is rejected with a syslog message. Activation of com.example.Slow times out after 60 seconds.

"Bad" result: Forged ActivationFailure is received and processed. The syslog message from dbus-daemon says "This is bad".
Comment 6 Simon McVittie 2015-01-26 20:58:11 UTC
I'll ask the distros@vs list for a CVE ID when someone has ack'd these patches.

Not screamingly urgent because it's only local DoS, but should be fixed.
Comment 7 Simon McVittie 2015-01-27 16:05:47 UTC
(In reply to Simon McVittie from comment #6)
> I'll ask the distros@vs list for a CVE ID when someone has ack'd these
> patches.

now done anyway
Comment 8 David King 2015-01-27 16:08:11 UTC
Sorry for the slow response. I am at the GNOME developer experience/docs hackfest this week, so I will not have much to look at these until next week. I had a (very) cursory look and they seem OK.
Comment 9 Simon McVittie 2015-01-28 11:03:37 UTC
This is CVE-2015-0245
Comment 10 Simon McVittie 2015-01-28 14:47:08 UTC
Created attachment 112911 [details] [review]
[1.8] CVE-2015-0245: prevent forged ActivationFailure from  non-root processes

Without either this rule or better checking in dbus-daemon, non-systemd
processes can make dbus-daemon think systemd failed to activate a system
service, resulting in an error reply back to the requester.

This is redundant with the fix in the C code (which I consider to be
the real solution), but is likely to be easier to backport.

---

Better commit message, now applicable to 1.8 without patch conflicts, no functional changes.

This is the only one I am considering backporting to 1.6.
Comment 11 Simon McVittie 2015-01-28 14:48:02 UTC
Created attachment 112912 [details] [review]
[1.8] bus_driver_get_owner_of_name: factor out from  bus_driver_get_conn_helper

We need this, or something equivalent, to address CVE-2015-0245 via
code changes.

---

Clearer commit message, diff is relative to 1.8, no functional changes.
Comment 12 Simon McVittie 2015-01-28 14:48:48 UTC
Created attachment 112913 [details] [review]
[1.8] CVE-2015-0245: discard forged ActivationFailure messages

Without this code change, non-systemd processes can make dbus-daemon
think systemd failed to activate a system service, resulting in an
error reply back to the requester. In practice we can address this in
system.conf by only allowing root to forge these messages, but this
check is the real solution, particularly on systems where root is
not all-powerful.

---

Better commit message, diff is relative to 1.8, no functional changes.
Comment 13 Simon McVittie 2015-01-28 14:49:54 UTC
Comment on attachment 112851 [details] [review]
[master only] Add a regression test for making systemd activation  appear to fail

Renaming patch to indicate it is for master only (additionally, it requires part of Bug #88810, and Bug #59752).
Comment 14 Simon McVittie 2015-01-28 14:54:14 UTC
(In reply to Simon McVittie from comment #10)
> [1.8] CVE-2015-0245: prevent forged ActivationFailure from  non-root
> processes

This is going to conflict when applied to master, because master added the Stats interface... but fixing the conflicts is not difficult, and anyone who really cares about security should be running (1.6 or) 1.8 anyway, so the 1.8 patches are better ones to release to distros.

I'll do the merge into master and release a new 1.9 version on unembargo day (which is currently undecided; if people review things somewhat soon, then this time next week seems reasonable).
Comment 15 Philip Withnall 2015-02-02 14:46:53 UTC
Comment on attachment 112912 [details] [review]
[1.8] bus_driver_get_owner_of_name: factor out from  bus_driver_get_conn_helper

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

++
Comment 16 Philip Withnall 2015-02-02 14:49:23 UTC
Comment on attachment 112913 [details] [review]
[1.8] CVE-2015-0245: discard forged ActivationFailure messages

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

The code looks good to me, though I’m not familiar enough with D-Bus internals to do a security review on this (or other patches on this bug).
Comment 17 Simon McVittie 2015-02-02 15:14:34 UTC
(In reply to Simon McVittie from comment #1)
> Part of a "nice" code fix for master. Opinions welcome on whether I should
> backport this to 1.8; I'm certainly not going back as far as 1.6.

I'm currently leaning towards using Attachment #112911 [details] as the only fix for both 1.6.30 and 1.8.16, and applying the C-code fixes that Philip just reviewed (which are the proper fix for this, IMO) to only master for now. I can always backport them for 1.8.18 if desired, but it would be easier to do that in public once the embargo has lifted.

I would really appreciate review on these from another upstream D-Bus maintainer, but I don't want to sit on this vuln forever; so I intend to contact distros again this time tomorrow, setting 2015-02-05 15:00 UTC (3 days from now) as unembargo day, unless someone tells me not to.
Comment 18 Alban Crequy 2015-02-02 15:42:01 UTC
I have not tested the patches but they all look good to me.
Comment 19 David King 2015-02-03 13:14:21 UTC
Comment on attachment 112911 [details] [review]
[1.8] CVE-2015-0245: prevent forged ActivationFailure from  non-root processes

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

Looks like a safe and reasonable hardening of policy.
Comment 20 David King 2015-02-03 13:14:31 UTC
Comment on attachment 112912 [details] [review]
[1.8] bus_driver_get_owner_of_name: factor out from  bus_driver_get_conn_helper

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

Seems like a reasonable bit of refactoring, especially getting rid of the assert.
Comment 21 David King 2015-02-03 13:15:55 UTC
Comment on attachment 112913 [details] [review]
[1.8] CVE-2015-0245: discard forged ActivationFailure messages

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

The discarding and logging seems fine to me, although I have a similar level of familiarity with the D-Bus internals as Philip.
Comment 22 Simon McVittie 2015-02-04 20:04:18 UTC
(In reply to Simon McVittie from comment #17)
> I intend to
> contact distros again this time tomorrow, setting 2015-02-05 15:00 UTC (3
> days from now) as unembargo day, unless someone tells me not to.

Well this clearly didn't happen. Now intending to sort out the 1.8.16 and possibly 1.9.10 releases tomorrow, for unembargo on Monday.
Comment 23 Simon McVittie 2015-02-06 15:46:58 UTC
(In reply to Simon McVittie from comment #22)
> Well this clearly didn't happen. Now intending to sort out the 1.8.16 and
> possibly 1.9.10 releases tomorrow, for unembargo on Monday.

Unembargo o'clock is 15:00 UTC on Monday. I have 1.8.16 and 1.6.30 ready for upload; 1.9.10 will follow afterwards (so it isn't too out of sync with the current state of master at the time).

Attachment #112911 [details] is the only thing applied to the stable branches right now; the others will initially only go in master, although I might do a 1.8.18 with Attachment #112912 [details] and Attachment #112913 [details] when the dust has settled.
Comment 24 Simon McVittie 2015-02-09 15:34:19 UTC
Fixed in 1.8.16, 1.6.30, 1.9.10. Unembargoing.

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.