From 02b9375f350390380efd2f127c2be3c7412ed14f Mon Sep 17 00:00:00 2001 From: Alban Crequy Date: Tue, 20 May 2014 14:37:37 +0100 Subject: [PATCH] security and activation: deliver error messages correctly and fix Denial of Service 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. This bug is caused by wrongly using the same transaction for replying to "RequestName" from the service and "MaliciousMethod" from the sender. Additionally, bus_activation_send_pending_auto_activation_messages() should not return an error when it fails to deliver some activation messages. Bug: https://bugs.freedesktop.org/show_bug.cgi?id= Signed-off-by: Alban Crequy Reviewed-by: nobody yet --- bus/activation.c | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/bus/activation.c b/bus/activation.c index fa6c156..847ce7a 100644 --- a/bus/activation.c +++ b/bus/activation.c @@ -1186,15 +1186,40 @@ bus_activation_send_pending_auto_activation_messages (BusActivation *activation if (entry->auto_activation && (entry->connection == NULL || dbus_connection_get_is_connected (entry->connection))) { DBusConnection *addressed_recipient; + BusTransaction *sub_transaction; + DBusError tmp_error; + + dbus_error_init (&tmp_error); addressed_recipient = bus_service_get_primary_owners_connection (service); + sub_transaction = bus_transaction_new (activation->context); + if (sub_transaction == NULL) + goto error; + /* Resume dispatching where we left off in bus_dispatch() */ - if (!bus_dispatch_matches (transaction, + if (!bus_dispatch_matches (sub_transaction, entry->connection, addressed_recipient, - entry->activation_message, error)) - goto error; + entry->activation_message, &tmp_error)) + { + if (!bus_transaction_send_error_reply (sub_transaction, entry->connection, + &tmp_error, entry->activation_message)) + { + bus_connection_send_oom_error (entry->connection, + entry->activation_message); + bus_transaction_cancel_and_free (sub_transaction); + } + else + { + bus_transaction_execute_and_free (sub_transaction); + } + + link = next; + continue; + } + + bus_transaction_execute_and_free (sub_transaction); } link = next; -- 1.8.5.3