Bug 78979

Summary: [CVE-2014-3477] security and activation: error and denial of service
Product: dbus Reporter: Daniel Stone <daniel>
Component: coreAssignee: Simon McVittie <smcv>
Status: RESOLVED FIXED QA Contact: Sjoerd Simons <sjoerd>
Severity: normal    
Priority: medium CC: alban.crequy, fridrich.strba, lennart, thiago, walters
Version: unspecifiedKeywords: security
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: resolve the error
[PATCH v2] security and activation: deliver error messages correctly and fix Denial of Service
CVE-2014-3477: deliver activation errors correctly, fixing Denial of Service

Description Daniel Stone 2014-05-20 17:12:58 UTC
Created attachment 99433 [details] [review]
resolve the error

How it should work:

When a D-Bus message activates a service, LSMs (SELinux or AppArmor) check
whether the message can be delivered after the service has been activated. The
service is considered activated when its well-known name is requested with
org.freedesktop.DBus.RequestName. When the message delivery is denied, the
service stays activated but should not receive the activating message (the
message which triggered the activation). dbus-daemon is supposed to drop the
activating message and reply to the sender with a D-Bus error message.

However, it does not work as expected:

1. The error message is delivered to the service instead of being delivered to
   the sender. As an example, the error message could be something like:

     An SELinux policy prevents this sender from sending this
     message to this recipient, [...] member="MaliciousMethod"

   So the sender could maliciously leak information to the service by inserting
   the information in the member name, even though the LSM attempted to block
   the communication between the sender and the service.

2. The error message is delivered as a reply to the RequestName call from
   service. It means the activated service will believe it cannot request the
   name and might exit. The sender could activate the service frequently and
   systemd will give up activating it. Thus the denial of service.

The attached patch fixes the bug in my test.
Comment 1 Daniel Stone 2014-05-20 17:13:39 UTC
(Filed on behalf of Alban as he couldn't restrict it to security-only. I don't know the first thing about it, and didn't even read what I copy and pasted.)
Comment 2 Alban Crequy 2014-05-23 17:25:48 UTC
This bug is not specific to LSMs (SELinux or AppArmor): I manage to trigger it with a D-Bus security policy alone.

My test is the following:

/etc/dbus-1/system.d/org.freedesktop.RealtimeKit1.conf:

<!DOCTYPE busconfig PUBLIC
 "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN"
 "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
<busconfig>
  <policy user="rtkit">
    <allow own="org.freedesktop.RealtimeKit1"/>
  </policy>
  <policy user="root">
    <allow own="org.freedesktop.RealtimeKit1"/>
  </policy>
  <policy context="default">
    <!-- XXX: I replaced "allow" by "deny" here for my test! -->
    <deny send_destination="org.freedesktop.RealtimeKit1"/>
  </policy>
  <policy user="root">
    <allow send_destination="org.freedesktop.RealtimeKit1"/>
    <allow receive_sender="org.freedesktop.RealtimeKit1"/>
  </policy>
</busconfig>

For the purpose of the test, I forbid non-root processes (such as pulseaudio) to deliver a message to rtkit. Then, when starting pulseaudio, it attempts to send a message to rtkit: the message is not delivered (that's ok, that's what I wanted) but rtkit couldn't request its well-known name (that's not ok, that's a denial of service):

rtkit-daemon[1167]: Failed to register name on bus: Rejected send message, 2 matched rules; type="method_call", sender=":1.59" (uid=1000 pid=1093 comm="/usr/bin/pulseaudio --start --log-target=syslog ") interface="org.freedesktop.RealtimeKit1" member="MakeThreadRealtime" error name="(unset)" requested_reply="0" destination="org.freedesktop.RealtimeKit1" (uid=0 pid=1167 comm="/usr/lib/rtkit/rtkit-daemon ")

When rtkit-daemon calls RequestName(), it receives the signal "NameAcquired" successfully but RequestName() returns the D-Bus error message above that should have been delivered to Pulseaudio, and it does not receive the real D-Bus reply. So rtkit believes it failed to claim its well-known name and exits.

The patch works by sending the D-Bus error message to pulseaudio immediately in a different context (using local variables "sub_transaction" and "tmp_error") instead of hijacking the context of RequestName (parameters "transaction" and "error").
Comment 3 Simon McVittie 2014-05-26 09:19:37 UTC
Adding de facto D-Bus security contacts to Cc
Comment 4 Simon McVittie 2014-05-26 09:25:48 UTC
(In reply to comment #0)
> 1. The error message is delivered to the service instead of being delivered
> to
>    the sender. As an example, the error message could be something like:
> 
>      An SELinux policy prevents this sender from sending this
>      message to this recipient, [...] member="MaliciousMethod"
> 
>    So the sender could maliciously leak information to the service by
> inserting
>    the information in the member name, even though the LSM attempted to block
>    the communication between the sender and the service.

I do not consider this sort of compartmentalization to be something that D-Bus should claim to support - it isn't something we test, and isn't necessary for mainstream Linux. In my view, if you have a mixture of "normal" and "top secret" processes and you want it to be impossible for the "top secret" processes to leak information to the "normal" processes, you should be using virtual machines or containers. However, if fans of SELinux do want it, they are welcome to help support it.

So I don't consider (1) to be a vulnerability, as such...

> 2. The error message is delivered as a reply to the RequestName call from
>    service. It means the activated service will believe it cannot request the
>    name and might exit. The sender could activate the service frequently and
>    systemd will give up activating it. Thus the denial of service.

... but (2) is certainly a denial-of-service vulnerability, so this is worth addressing. I'll try to do a detailed review on this soon.
Comment 5 Thiago Macieira 2014-05-26 16:33:57 UTC
(In reply to comment #4)
> (In reply to comment #0)
> > 1. The error message is delivered to the service instead of being delivered
> > to
> >    the sender. As an example, the error message could be something like:
> > 
> >      An SELinux policy prevents this sender from sending this
> >      message to this recipient, [...] member="MaliciousMethod"
> > 
> >    So the sender could maliciously leak information to the service by
> > inserting
> >    the information in the member name, even though the LSM attempted to block
> >    the communication between the sender and the service.
> 
> I do not consider this sort of compartmentalization to be something that
> D-Bus should claim to support - it isn't something we test, and isn't
> necessary for mainstream Linux.

I don't see how this is leaking any information. You get an error message saying the message you sent was rejected, with some of the details of the message you sent. That is, the malicious sender already knew about "MaliciousMethod" in order to send a call to that method.

Once we fix the destination of the error message, the problem is gone.
Comment 6 Alban Crequy 2014-05-26 17:20:36 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #0)
> > > 1. The error message is delivered to the service instead of being delivered
> > > to
> > >    the sender. As an example, the error message could be something like:
> > > 
> > >      An SELinux policy prevents this sender from sending this
> > >      message to this recipient, [...] member="MaliciousMethod"
> > > 
> > >    So the sender could maliciously leak information to the service by
> > > inserting
> > >    the information in the member name, even though the LSM attempted to block
> > >    the communication between the sender and the service.
> > 
> > I do not consider this sort of compartmentalization to be something that
> > D-Bus should claim to support - it isn't something we test, and isn't
> > necessary for mainstream Linux.
> 
> I don't see how this is leaking any information. You get an error message
> saying the message you sent was rejected, with some of the details of the
> message you sent. That is, the malicious sender already knew about
> "MaliciousMethod" in order to send a call to that method.

It can only leak information if the sender and the service are malicious confederates and agree on a protocol to leak the information, e.g. base64 the secret information and insert the result in the "member" field of the D-Bus message.

> Once we fix the destination of the error message, the problem is gone.

I agree.
Comment 7 Simon McVittie 2014-05-30 15:37:13 UTC
Comment on attachment 99433 [details] [review]
resolve the error

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

Code review: looks correct, but could be simpler.

::: bus/activation.c
@@ +1203,5 @@
>                                       addressed_recipient,
> +                                     entry->activation_message, &tmp_error))
> +            {
> +              if (!bus_transaction_send_error_reply (sub_transaction, entry->connection,
> +                                                     &tmp_error, entry->activation_message))

(As a side note, that function is confusingly-named: it does not actually send anything. Instead, it reserves enough memory that sending the message later cannot fail.)

@@ +1207,5 @@
> +                                                     &tmp_error, entry->activation_message))
> +                {
> +                  bus_connection_send_oom_error (entry->connection,
> +                                                 entry->activation_message);
> +                  bus_transaction_cancel_and_free (sub_transaction);

The bug here is really: if bus_activation_send_pending_auto_activation_messages() raises a DBusError, it is sent back to the service requesting the name. That's correct if the error is OOM, but not correct if the intention was to send the error back to the process that originally requested activation instead.

So the function at least deserves a comment

/* Any DBusError raised here will make the RequestName call fail with that error,
 * so do not return a DBusError unless the whole transaction has failed. */

I'm tempted to say it should not have a DBusError parameter at all, so it's more clearly an example of the "return TRUE, or FALSE on OOM" pattern (and its only caller would have to set the OOM error instead).

In addition to fixing that, you're introducing slightly more complex transactions. Your code essentially reads: if there is not enough memory to send the error message(s) to each client that was not allowed to talk to the service, send OOM to the client(s) that hit OOM instead, and ignore those OOMs for the purposes of deciding whether RequestName() was successful or needs to be rolled back.

Not using the sub-transaction, and just using @transaction, would instead result in: if there is not enough memory to send the error message(s) to each client that was not allowed to talk to the service, RequestName() fails with OOM; the service may retry, or just exit (which is theoretically DoS I suppose, but if you're hitting OOM in dbus-daemon then your system is already being denied service).

This is all pretty theoretical - dbus-daemon shouldn't OOM under any normal circumstance - so I'm tempted to ask for the simpler version. Alban, would you mind implementing and testing that simpler version, and we can decide which is better when we can compare both patches?
Comment 8 Simon McVittie 2014-05-30 15:39:57 UTC
Proposed advisory text:

----8<----
D-Bus <http://www.freedesktop.org/wiki/Software/dbus/> is an
asynchronous inter-process communication system, commonly used
for system services or within a desktop session on Linux and other
operating systems.

Alban Crequy discovered a denial-of-service flaw in the reference
implementation of dbus-daemon, the D-Bus message bus daemon. Additionally,
in highly unusual environments the same flaw could lead to a side channel
between processes that should not be able to communicate.

Summary:

If a client C1 is prohibited from sending a message to a service S1,
and S1 is not currently running, then C1 can attempt to send a message
to S1's well-known bus name, causing dbus-daemon to start S1 [1].
When S1 has started and obtained its well-known bus name, the
dbus-daemon evaluates its security policy, decides that it will not
deliver the message to S1, and constructs an AccessDenied error.
However, instead of sending that AccessDenied error reply to C1
as a reply to the denied message, dbus-daemon incorrectly sends it to S1
as a reply to the request to obtain its well-known bus name.

Impact 1: denial of service. S1 will fail to initialize, and exit,
denying service to legitimate clients of S1.

Impact 2: in environments where C1 and S1 are untrusted and are
administratively prohibited from communicating, S1 could also use these
incorrectly-directed error messages as a side channel to receive information
from C1.

Mitigations:

Impact 1: if a legitimate client was actively using S1, S1 would already
have been started, so C1 can only deny service to a legitimate client
that only recently became active.

Impact 2: in practice processes sharing a system bus can typically
communicate in other ways (non-D-Bus IPC mechanisms, files in /tmp, etc.),
so impact 2 is not relevant on normal systems. It might be relevant
on systems when an LSM such as SELinux is used in a highly restrictive
configuration.

Footnotes:

[1] This is perhaps unexpected, but the dbus-daemon is behaving as designed:
it cannot necessarily evaluate which security policies it should apply
to S1 until S1 has actually connected back to dbus-daemon, because S1
might change its uid, SELinux context, etc. during startup.
The conceptual model is that activatable services are always running,
and that the dbus-daemon delaying their startup until they are actually
needed is a form of lazy evaluation. As such, the D-Bus maintainers do not
consider this to be a bug or vulnerability.
----8<----
Comment 9 Alban Crequy 2014-06-02 10:48:57 UTC
Created attachment 100282 [details] [review]
[PATCH v2] security and activation: deliver error messages correctly and fix Denial of Service

patch updated after smcv's review:
- prototype changed to remove the error parameter and let the caller set the OOM error if it retuns FALSE.
- do not use subtransaction but the same transaction.
- do not cancel, execute or free the transaction: let the caller do it.

I tested the patch in the same way as the previous one and this one works as well for me.

The proposed advisory text looks good to me.
Comment 10 Simon McVittie 2014-06-02 12:15:02 UTC
Comment on attachment 100282 [details] [review]
[PATCH v2] security and activation: deliver error messages correctly and fix Denial of Service

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

This looks good to me, but reviews from other D-Bus maintainers would be extremely welcome.

I'll contact distros@ and see whether I can get a CVE ID before releasing.
Comment 11 Simon McVittie 2014-06-04 20:20:53 UTC
Colin pointed out that the comment is confusingly wrong now, and suggested this:

               /* Any DBusError raised here will make the RequestName call fail
-               * with that error, so do not return a DBusError unless the
-               * whole transaction has failed. */
+               * with that error; we want to return the error to the original
+               * method invoker. */

I don't think that's really right either: maybe this?

/* If permission is denied, we just want to return the
 * error to the original method invoker; in particular,
 * we don't want to make the RequestName call fail
 * with that error (see fd.o #78979, CVE-2014-3477). */

I'll commit --amend if people are happy with that wording. I need to do that to add the CVE reference anyway.
Comment 12 Colin Walters 2014-06-04 21:54:33 UTC
(In reply to comment #11)

> I don't think that's really right either: maybe this?
> 
> /* If permission is denied, we just want to return the
>  * error to the original method invoker; in particular,
>  * we don't want to make the RequestName call fail
>  * with that error (see fd.o #78979, CVE-2014-3477). */

Yep, this looks good to me.
Comment 13 Alban Crequy 2014-06-05 08:23:26 UTC
(In reply to comment #11)

> I'll commit --amend if people are happy with that wording. I need to do that
> to add the CVE reference anyway.

This looks good to me too.
Comment 14 Simon McVittie 2014-06-05 14:31:44 UTC
Created attachment 100463 [details] [review]
CVE-2014-3477: deliver activation errors correctly, fixing  Denial of Service

Here is a revised version of Alban's patch with the adjusted comment. This is what I'm going to recommend to vendors, assuming my tests are successful. I'm preparing 1.6.20 and 1.8.4 releases ahead of time so that they're ready for upload on $(date -d '2014-06-10 16:00+0000').

The same patch seems to cherry-pick nicely to 1.6 and 1.4, and to 1.2 with a bit of conflict-fixing (where trailing whitespace was deleted post-1.2).

I do not consider 1.4 or 1.2 to be security-supported upstream any more, but I will push suggested patches to those branches.
Comment 15 Alban Crequy 2014-06-06 16:41:01 UTC
(In reply to comment #14)
> Created attachment 100463 [details] [review] [review]
> CVE-2014-3477: deliver activation errors correctly, fixing  Denial of Service
> 
> Here is a revised version of Alban's patch with the adjusted comment. This
> is what I'm going to recommend to vendors, assuming my tests are successful.
> I'm preparing 1.6.20 and 1.8.4 releases ahead of time so that they're ready
> for upload on $(date -d '2014-06-10 16:00+0000').
> 
> The same patch seems to cherry-pick nicely to 1.6 and 1.4, and to 1.2 with a
> bit of conflict-fixing (where trailing whitespace was deleted post-1.2).
> 
> I do not consider 1.4 or 1.2 to be security-supported upstream any more, but
> I will push suggested patches to those branches.

It looks good to me.

I gave the patch another try in the environment I was using to trigger the bug (basically, dbus 1.8.0) and it worked fine for me.
Comment 16 Simon McVittie 2014-06-10 17:38:29 UTC
Unembargoing.

Fixed in 1.8.4, 1.6.20. Fixed in git for 1.4 and 1.2 branches.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct.