summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlban Crequy <alban.crequy@collabora.co.uk>2014-05-20 14:37:37 +0100
committerSimon McVittie <simon.mcvittie@collabora.co.uk>2014-06-05 14:43:10 +0100
commit72a8759c224ec37b75a7631c7d90ce266a4bf3bd (patch)
tree3bc9934f82d67388a3263e32c7d35cd5ee2ab3ee
parenta804410760b18e54eb2dc0796410da8a283b9f72 (diff)
CVE-2014-3477: deliver activation errors correctly, fixing Denial of Servicedbus-1.2
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" If the sender and the service are malicious confederates and agree on a protocol to insert information in the member name, the sender can leak information to the service, 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 following changes fix the bug: - bus_activation_send_pending_auto_activation_messages() only returns an error in case of OOM. The prototype is changed to return TRUE, or FALSE on OOM (and its only caller sets the OOM error). - When a client is not allowed to talk to the service, a D-Bus error message is pre-allocated to be delivered to the client as part of the transaction. The error is not propagated to the caller so RequestName will not fail (except on OOM). [fixed a misleading comment -smcv] Bug: https://bugs.freedesktop.org/show_bug.cgi?id=78979 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Reviewed-by: Colin Walters <walters@verbum.org> Backported: to dbus-1.2, whitespace conflicts in bus/activation.c
-rw-r--r--bus/activation.c29
-rw-r--r--bus/activation.h3
-rw-r--r--bus/services.c5
3 files changed, 25 insertions, 12 deletions
diff --git a/bus/activation.c b/bus/activation.c
index 2fcd85d2..e5bc1fb5 100644
--- a/bus/activation.c
+++ b/bus/activation.c
@@ -1106,14 +1106,11 @@ bus_activation_service_created (BusActivation *activation,
dbus_bool_t
bus_activation_send_pending_auto_activation_messages (BusActivation *activation,
BusService *service,
- BusTransaction *transaction,
- DBusError *error)
+ BusTransaction *transaction)
{
BusPendingActivation *pending_activation;
DBusList *link;
- _DBUS_ASSERT_ERROR_IS_CLEAR (error);
-
/* Check if it's a pending activation */
pending_activation = _dbus_hash_table_lookup_string (activation->pending_activations,
bus_service_get_name (service));
@@ -1130,15 +1127,32 @@ bus_activation_send_pending_auto_activation_messages (BusActivation *activation
if (entry->auto_activation && dbus_connection_get_is_connected (entry->connection))
{
DBusConnection *addressed_recipient;
-
+ DBusError error;
+
+ dbus_error_init (&error);
+
addressed_recipient = bus_service_get_primary_owners_connection (service);
/* Resume dispatching where we left off in bus_dispatch() */
if (!bus_dispatch_matches (transaction,
entry->connection,
addressed_recipient,
- entry->activation_message, error))
- goto error;
+ entry->activation_message, &error))
+ {
+ /* 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). */
+ if (!bus_transaction_send_error_reply (transaction, entry->connection,
+ &error, entry->activation_message))
+ {
+ bus_connection_send_oom_error (entry->connection,
+ entry->activation_message);
+ }
+
+ link = next;
+ continue;
+ }
}
link = next;
@@ -1147,7 +1161,6 @@ bus_activation_send_pending_auto_activation_messages (BusActivation *activation
if (!add_restore_pending_to_transaction (transaction, pending_activation))
{
_dbus_verbose ("Could not add cancel hook to transaction to revert removing pending activation\n");
- BUS_SET_OOM (error);
goto error;
}
diff --git a/bus/activation.h b/bus/activation.h
index 03bfed28..6516cfc3 100644
--- a/bus/activation.h
+++ b/bus/activation.h
@@ -60,8 +60,7 @@ dbus_bool_t bus_activation_list_services (BusActivation *registry,
dbus_bool_t bus_activation_send_pending_auto_activation_messages (BusActivation *activation,
BusService *service,
- BusTransaction *transaction,
- DBusError *error);
+ BusTransaction *transaction);
diff --git a/bus/services.c b/bus/services.c
index b260c633..00d3828b 100644
--- a/bus/services.c
+++ b/bus/services.c
@@ -590,8 +590,9 @@ bus_registry_acquire_service (BusRegistry *registry,
activation = bus_context_get_activation (registry->context);
retval = bus_activation_send_pending_auto_activation_messages (activation,
service,
- transaction,
- error);
+ transaction);
+ if (!retval)
+ BUS_SET_OOM (error);
out:
return retval;