Attachment 100282 Details for Bug 78979 – [PATCH v2] security and activation: deliver error messages correctly and fix Denial of Service

[patch] [PATCH v2] security and activation: deliver error messages correctly and fix Denial of Service

0001-security-and-activation-deliver-error-messages-corre.patch (text/plain), 6.15 KB, created by Alban Crequy on 2014-06-02 10:48:57 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Alban Crequy

Created: 2014-06-02 10:48:57 UTC

Size: 6.15 KB

patch

obsolete

>From 0819ea751bca135aade3591b98d5ed17421c41f5 Mon Sep 17 00:00:00 2001
>From: Alban Crequy <alban.crequy@collabora.co.uk>
>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"
>
>   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).
>
>Bug: https://bugs.freedesktop.org/show_bug.cgi?id=78979
>Signed-off-by: Alban Crequy <alban.crequy@collabora.co.uk>
>Reviewed-by:
>---
> activation.c |   26 +++++++++++++++++++-------
> activation.h |    3 +--
> services.c   |    5 +++--
> 3 files changed, 23 insertions(+), 11 deletions(-)
>
>Index: dbus-1.8.0/bus/activation.c
>===================================================================
>--- dbus-1.8.0.orig/bus/activation.c	2013-11-12 10:55:43.000000000 +0000
>+++ dbus-1.8.0/bus/activation.c	2014-06-02 10:08:36.493639412 +0000
>@@ -1162,14 +1162,11 @@
> 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));
>@@ -1186,6 +1183,9 @@
>       if (entry->auto_activation && (entry->connection == NULL || 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);
> 
>@@ -1193,8 +1193,21 @@
>           if (!bus_dispatch_matches (transaction,
>                                      entry->connection,
>                                      addressed_recipient,
>-                                     entry->activation_message, error))
>-            goto error;
>+                                     entry->activation_message, &error))
>+            {
>+              /* 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. */
>+              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;
>@@ -1203,7 +1216,6 @@
>   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;
>     }
> 
>Index: dbus-1.8.0/bus/activation.h
>===================================================================
>--- dbus-1.8.0.orig/bus/activation.h	2012-09-29 13:03:59.000000000 +0000
>+++ dbus-1.8.0/bus/activation.h	2014-06-02 10:08:36.497638602 +0000
>@@ -62,8 +62,7 @@
> 
> dbus_bool_t    bus_activation_send_pending_auto_activation_messages (BusActivation     *activation,
> 								     BusService        *service,
>-								     BusTransaction    *transaction,
>-								     DBusError         *error);
>+								     BusTransaction    *transaction);
> 
> 
> #endif /* BUS_ACTIVATION_H */
>Index: dbus-1.8.0/bus/services.c
>===================================================================
>--- dbus-1.8.0.orig/bus/services.c	2014-06-02 10:06:04.000000000 +0000
>+++ dbus-1.8.0/bus/services.c	2014-06-02 10:08:42.197517503 +0000
>@@ -595,8 +595,9 @@
>   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;

Actions: View | Splinter Review

Attachments on bug 78979: 99433 | 100282 | 100463

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.