[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Please test and review dbus 1.2.24-4+squeeze3



Hello,

I have a prepared a new version of dbus for squeeze-lts that fixes 3 CVE:

 dbus (1.2.24-4+squeeze3) squeeze-lts; urgency=medium
 .
   * Security upload by the Debian LTS team.
   * CVE-2014-3477: Backport patch from upstream to fix a denial of service
     (failure to obtain bus name) in newly-activated system services that not
     all users are allowed to access.
   * CVE-2014-3638: Backport patch from upstream to reduce maximum number of
     pending replies per connection to avoid algorithmic complexity DoS.
   * CVE-2014-3639: Backport patch from upstream to not accept() new
     connections when all unauthenticated connection slots are in use,
     so that malicious processes cannot prevent new connections to the
     system bus. Note that the patch that reduced the authentication delay
     to 5s has not been applied due to known regressions:
     https://bugs.freedesktop.org/show_bug.cgi?id=86431

I grabbed the patches in the wheezy package. Two patches required some
small adjustments to apply on the dbus version in squeeze. The debdiff is
attached if you want to review it.

You can grab some .debs to test here:
dget https://people.debian.org/~hertzog/packages/dbus_1.2.24-4+squeeze3_amd64.changes
dget https://people.debian.org/~hertzog/packages/dbus_1.2.24-4+squeeze3_i386.changes

Please install the packages and let me know whether everything still works
fine (trying a reboot is a good idea).

Thank you!
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: http://www.freexian.com/services/debian-lts.html
Learn to master Debian: http://debian-handbook.info/get/
diff -Nru dbus-1.2.24/debian/changelog dbus-1.2.24/debian/changelog
--- dbus-1.2.24/debian/changelog	2012-10-04 09:47:17.000000000 +0200
+++ dbus-1.2.24/debian/changelog	2014-11-19 17:51:47.000000000 +0100
@@ -1,3 +1,20 @@
+dbus (1.2.24-4+squeeze3) squeeze-lts; urgency=medium
+
+  * Security upload by the Debian LTS team.
+  * CVE-2014-3477: Backport patch from upstream to fix a denial of service
+    (failure to obtain bus name) in newly-activated system services that not
+    all users are allowed to access.
+  * CVE-2014-3638: Backport patch from upstream to reduce maximum number of
+    pending replies per connection to avoid algorithmic complexity DoS.
+  * CVE-2014-3639: Backport patch from upstream to not accept() new
+    connections when all unauthenticated connection slots are in use,
+    so that malicious processes cannot prevent new connections to the
+    system bus. Note that the patch that reduced the authentication delay
+    to 5s has not been applied due to know regressions:
+    https://bugs.freedesktop.org/show_bug.cgi?id=86431
+
+ -- Raphaël Hertzog <hertzog@debian.org>  Wed, 19 Nov 2014 15:12:37 +0100
+
 dbus (1.2.24-4+squeeze2) stable; urgency=low
 
   * CVE-2012-3524: apply patches from upstream 1.6.6 to avoid arbitrary
diff -Nru dbus-1.2.24/debian/patches/CVE-2014-3477.patch dbus-1.2.24/debian/patches/CVE-2014-3477.patch
--- dbus-1.2.24/debian/patches/CVE-2014-3477.patch	1970-01-01 01:00:00.000000000 +0100
+++ dbus-1.2.24/debian/patches/CVE-2014-3477.patch	2014-11-19 17:14:32.000000000 +0100
@@ -0,0 +1,143 @@
+From 8205e515f99bdc635bdb51af158dd11d9582dc48 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] CVE-2014-3477: deliver activation errors correctly, fixing
+ 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).
+
+[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>
+
+Patch backported to dbus 1.2.24 by Raphaël Hertzog <hertzog@debian.org>
+---
+ bus/activation.c | 27 ++++++++++++++++++++-------
+ bus/activation.h |  3 +--
+ bus/services.c   |  5 +++--
+ 3 files changed, 24 insertions(+), 11 deletions(-)
+
+--- a/bus/activation.c
++++ b/bus/activation.c
+@@ -1106,14 +1106,11 @@ bus_activation_service_created (BusActiv
+ 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,6 +1127,9 @@ bus_activation_send_pending_auto_activat
+       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);
+ 
+@@ -1137,8 +1137,22 @@ bus_activation_send_pending_auto_activat
+           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_activat
+   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;
+     }
+   
+--- a/bus/activation.h
++++ b/bus/activation.h
+@@ -60,8 +60,7 @@ dbus_bool_t    bus_activation_list_servi
+ 
+ dbus_bool_t    bus_activation_send_pending_auto_activation_messages (BusActivation     *activation,
+ 								     BusService        *service,
+-								     BusTransaction    *transaction,
+-								     DBusError         *error);
++								     BusTransaction    *transaction);
+ 
+ 
+ 
+--- a/bus/services.c
++++ b/bus/services.c
+@@ -590,8 +590,9 @@ bus_registry_acquire_service (BusRegistr
+   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;
diff -Nru dbus-1.2.24/debian/patches/CVE-2014-3638.patch dbus-1.2.24/debian/patches/CVE-2014-3638.patch
--- dbus-1.2.24/debian/patches/CVE-2014-3638.patch	1970-01-01 01:00:00.000000000 +0100
+++ dbus-1.2.24/debian/patches/CVE-2014-3638.patch	2014-11-19 15:18:55.000000000 +0100
@@ -0,0 +1,26 @@
+From 6060aaa0ea1e9bbe1dd7a1864c8df52e333a45ee Mon Sep 17 00:00:00 2001
+From: Alban Crequy <alban.crequy@collabora.co.uk>
+Date: Thu, 10 Jul 2014 15:08:06 +0100
+Subject: [PATCH 01/10] system bus limit: use max_replies_per_connection=128 by
+ default
+
+This addresses CVE-2014-3638.
+
+Bug: https://bugs.freedesktop.org/show_bug.cgi?id=81053
+Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
+(cherry picked from commit 5bc7f9519ebc6117ba300c704794b36b87c2194b)
+---
+ bus/config-parser.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+--- a/bus/config-parser.c
++++ b/bus/config-parser.c
+@@ -442,7 +442,7 @@ bus_config_parser_new (const DBusString
+       /* this is effectively a limit on message queue size for messages
+        * that require a reply
+        */
+-      parser->limits.max_replies_per_connection = 1024*8;
++      parser->limits.max_replies_per_connection = 128;
+     }
+       
+   parser->refcount = 1;
diff -Nru dbus-1.2.24/debian/patches/CVE-2014-3639.patch dbus-1.2.24/debian/patches/CVE-2014-3639.patch
--- dbus-1.2.24/debian/patches/CVE-2014-3639.patch	1970-01-01 01:00:00.000000000 +0100
+++ dbus-1.2.24/debian/patches/CVE-2014-3639.patch	2014-11-19 16:59:34.000000000 +0100
@@ -0,0 +1,271 @@
+From 89219baab0bf6ff05142518110f45c8159be8092 Mon Sep 17 00:00:00 2001
+From: Alban Crequy <alban.crequy@collabora.co.uk>
+Date: Fri, 4 Jul 2014 15:05:51 +0100
+Subject: [PATCH 04/10] Stop listening on DBusServer sockets when reaching
+ max_incomplete_connections
+
+This addresses the parts of CVE-2014-3639 not already addressed by
+reducing the default authentication timeout.
+
+Bug: https://bugs.freedesktop.org/show_bug.cgi?id=80851
+Bug: https://bugs.freedesktop.org/show_bug.cgi?id=80919
+Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
+(cherry picked from commit 8ad179a8dad789fc6a5402780044bc0ec3d41115)
+
+Patch backported to dbus 1.2.24 by Raphaël Hertzog <hertzog@debian.org>.
+---
+ bus/bus.c                    | 37 +++++++++++++++++++++++++++++++++++++
+ bus/bus.h                    |  1 +
+ bus/connection.c             | 42 ++++++++++++++++++------------------------
+ bus/connection.h             |  3 ++-
+ dbus/dbus-server-protected.h |  5 ++---
+ dbus/dbus-server.c           | 19 +++++--------------
+ dbus/dbus-watch.c            | 21 +++++++++++++++++++++
+ dbus/dbus-watch.h            |  2 ++
+ 8 files changed, 88 insertions(+), 42 deletions(-)
+
+--- a/bus/bus.c
++++ b/bus/bus.c
+@@ -35,6 +35,7 @@
+ #include <dbus/dbus-hash.h>
+ #include <dbus/dbus-credentials.h>
+ #include <dbus/dbus-internals.h>
++#include <dbus/dbus-server-protected.h>
+ 
+ struct BusContext
+ {
+@@ -58,6 +59,7 @@ struct BusContext
+   unsigned int fork : 1;
+   unsigned int syslog : 1;
+   unsigned int keep_umask : 1;
++  dbus_bool_t watches_enabled;
+ };
+ 
+ static dbus_int32_t server_data_slot = -1;
+@@ -656,6 +658,8 @@ bus_context_new (const DBusString *confi
+       goto failed;
+     }
+ 
++  context->watches_enabled = TRUE;
++
+   context->registry = bus_registry_new (context);
+   if (context->registry == NULL)
+     {
+@@ -1567,3 +1571,36 @@ bus_context_check_security_policy (BusCo
+   _dbus_verbose ("security policy allowing message\n");
+   return TRUE;
+ }
++
++void
++bus_context_check_all_watches (BusContext *context)
++{
++  DBusList *link;
++  dbus_bool_t enabled = TRUE;
++
++  if (bus_connections_get_n_incomplete (context->connections) >=
++      bus_context_get_max_incomplete_connections (context))
++    {
++      enabled = FALSE;
++    }
++
++  if (context->watches_enabled == enabled)
++    return;
++
++  context->watches_enabled = enabled;
++
++  for (link = _dbus_list_get_first_link (&context->servers);
++       link != NULL;
++       link = _dbus_list_get_next_link (&context->servers, link))
++    {
++      /* A BusContext might contains several DBusServer (if there are
++       * several <listen> configuration items) and a DBusServer might
++       * contain several DBusWatch in its DBusWatchList (if getaddrinfo
++       * returns several addresses on a dual IPv4-IPv6 stack or if
++       * systemd passes several fds).
++       * We want to enable/disable them all.
++       */
++      DBusServer *server = link->data;
++      _dbus_server_toggle_all_watches (server, enabled);
++    }
++}
+--- a/bus/bus.h
++++ b/bus/bus.h
+@@ -118,5 +118,6 @@ dbus_bool_t       bus_context_check_secu
+                                                                   DBusConnection   *proposed_recipient,
+                                                                   DBusMessage      *message,
+                                                                   DBusError        *error);
++void              bus_context_check_all_watches                  (BusContext       *context);
+ 
+ #endif /* BUS_BUS_H */
+--- a/bus/connection.c
++++ b/bus/connection.c
+@@ -276,6 +276,10 @@ bus_connection_disconnected (DBusConnect
+           _dbus_list_remove_link (&d->connections->incomplete, d->link_in_connection_list);
+           d->link_in_connection_list = NULL;
+           d->connections->n_incomplete -= 1;
++
++          /* If we have dropped below the max. number of incomplete
++           * connections, start accept()ing again */
++          bus_context_check_all_watches (d->connections->context);
+         }
+       
+       _dbus_assert (d->connections->n_incomplete >= 0);
+@@ -700,31 +704,17 @@ bus_connections_setup_connection (BusCon
+   
+   dbus_connection_ref (connection);
+ 
+-  /* Note that we might disconnect ourselves here, but it only takes
+-   * effect on return to the main loop. We call this to free up
+-   * expired connections if possible, and to queue the timeout for our
+-   * own expiration.
+-   */
+   bus_connections_expire_incomplete (connections);
+   
+-  /* And we might also disconnect ourselves here, but again it
+-   * only takes effect on return to main loop.
+-   */
+-  if (connections->n_incomplete >
+-      bus_context_get_max_incomplete_connections (connections->context))
+-    {
+-      _dbus_verbose ("Number of incomplete connections exceeds max, dropping oldest one\n");
+-      
+-      _dbus_assert (connections->incomplete != NULL);
+-      /* Disconnect the oldest unauthenticated connection.  FIXME
+-       * would it be more secure to drop a *random* connection?  This
+-       * algorithm seems to mean that if someone can create new
+-       * connections quickly enough, they can keep anyone else from
+-       * completing authentication. But random may or may not really
+-       * help with that, a more elaborate solution might be required.
+-       */
+-      dbus_connection_close (connections->incomplete->data);
+-    }
++  /* The listening socket is removed from the main loop,
++   * i.e. does not accept(), while n_incomplete is at its
++   * maximum value; so we shouldn't get here in that case */
++  _dbus_assert (connections->n_incomplete <=
++      bus_context_get_max_incomplete_connections (connections->context));
++
++  /* If we have the maximum number of incomplete connections,
++   * stop accept()ing any more, to avert a DoS. See fd.o #80919 */
++  bus_context_check_all_watches (d->connections->context);
+   
+   retval = TRUE;
+ 
+@@ -1399,6 +1389,10 @@ bus_connection_complete (DBusConnection
+   _dbus_assert (d->connections->n_incomplete >= 0);
+   _dbus_assert (d->connections->n_completed > 0);
+ 
++  /* If we have dropped below the max. number of incomplete
++   * connections, start accept()ing again */
++  bus_context_check_all_watches (d->connections->context);
++
+   /* See if we can remove the timeout */
+   bus_connections_expire_incomplete (d->connections);
+ 
+@@ -2301,3 +2295,9 @@ bus_transaction_add_cancel_hook (BusTran
+ 
+   return TRUE;
+ }
++
++int
++bus_connections_get_n_incomplete (BusConnections *connections)
++{
++  return connections->n_incomplete;
++}
+--- a/bus/connection.h
++++ b/bus/connection.h
+@@ -138,4 +138,5 @@ dbus_bool_t     bus_transaction_add_canc
+                                                   void                         *data,
+                                                   DBusFreeFunction              free_data_function);
+ 
++int bus_connections_get_n_incomplete              (BusConnections *connections);
+ #endif /* BUS_CONNECTION_H */
+--- a/dbus/dbus-server-protected.h
++++ b/dbus/dbus-server-protected.h
+@@ -98,9 +98,8 @@ dbus_bool_t _dbus_server_add_watch
+                                          DBusWatch              *watch);
+ void        _dbus_server_remove_watch   (DBusServer             *server,
+                                          DBusWatch              *watch);
+-void        _dbus_server_toggle_watch   (DBusServer             *server,
+-                                         DBusWatch              *watch,
+-                                         dbus_bool_t             enabled);
++void        _dbus_server_toggle_all_watches (DBusServer         *server,
++                                             dbus_bool_t         enabled);
+ dbus_bool_t _dbus_server_add_timeout    (DBusServer             *server,
+                                          DBusTimeout            *timeout);
+ void        _dbus_server_remove_timeout (DBusServer             *server,
+--- a/dbus/dbus-server.c
++++ b/dbus/dbus-server.c
+@@ -287,26 +287,17 @@ _dbus_server_remove_watch  (DBusServer *
+ }
+ 
+ /**
+- * Toggles a watch and notifies app via server's
+- * DBusWatchToggledFunction if available. It's an error to call this
+- * function on a watch that was not previously added.
++ * Toggles all watch and notifies app via server's
++ * DBusWatchToggledFunction if available.
+  *
+  * @param server the server.
+- * @param watch the watch to toggle.
+  * @param enabled whether to enable or disable
+  */
+ void
+-_dbus_server_toggle_watch (DBusServer  *server,
+-                           DBusWatch   *watch,
+-                           dbus_bool_t  enabled)
++_dbus_server_toggle_all_watches (DBusServer  *server,
++                                dbus_bool_t  enabled)
+ {
+-  _dbus_assert (watch != NULL);
+-
+-  HAVE_LOCK_CHECK (server);
+-  protected_change_watch (server, watch,
+-                          NULL, NULL,
+-                          _dbus_watch_list_toggle_watch,
+-                          enabled);
++  _dbus_watch_list_toggle_all_watches (server->watches, enabled);
+ }
+ 
+ /** Function to be called in protected_change_timeout() with refcount held */
+--- a/dbus/dbus-watch.c
++++ b/dbus/dbus-watch.c
+@@ -436,6 +436,27 @@ _dbus_watch_list_toggle_watch (DBusWatch
+ }
+ 
+ /**
++ * Sets all watches to the given enabled state, invoking the
++ * application's DBusWatchToggledFunction if appropriate.
++ *
++ * @param watch_list the watch list.
++ * @param enabled #TRUE to enable
++ */
++void
++_dbus_watch_list_toggle_all_watches (DBusWatchList           *watch_list,
++                                     dbus_bool_t              enabled)
++{
++  DBusList *link;
++
++  for (link = _dbus_list_get_first_link (&watch_list->watches);
++       link != NULL;
++       link = _dbus_list_get_next_link (&watch_list->watches, link))
++    {
++      _dbus_watch_list_toggle_watch (watch_list, link->data, enabled);
++    }
++}
++
++/**
+  * Sets the handler for the watch.
+  *
+  * @todo this function only exists because of the weird
+--- a/dbus/dbus-watch.h
++++ b/dbus/dbus-watch.h
+@@ -74,6 +74,8 @@ void           _dbus_watch_list_remove_w
+ void           _dbus_watch_list_toggle_watch  (DBusWatchList           *watch_list,
+                                                DBusWatch               *watch,
+                                                dbus_bool_t              enabled);
++void           _dbus_watch_list_toggle_all_watches (DBusWatchList      *watch_list,
++                                               dbus_bool_t              enabled);
+ dbus_bool_t    _dbus_watch_get_enabled        (DBusWatch              *watch);
+ 
+ /** @} */
diff -Nru dbus-1.2.24/debian/patches/series dbus-1.2.24/debian/patches/series
--- dbus-1.2.24/debian/patches/series	2012-10-04 09:47:17.000000000 +0200
+++ dbus-1.2.24/debian/patches/series	2014-11-19 17:04:44.000000000 +0100
@@ -8,3 +8,6 @@
 0002-hardening-Ensure-_dbus_check_setuid-is-initialized-t.patch
 0003-hardening-Remove-activation-helper-handling-for-DBUS.patch
 0004-activation-helper-Ensure-DBUS_STARTER_ADDRESS-is-set.patch
+CVE-2014-3638.patch
+CVE-2014-3639.patch
+CVE-2014-3477.patch

Reply to: