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

Bug#776144: unblock: dbus/1.8.14-1



Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock

Please consider unblocking dbus/1.8.14-1, a minimal upstream release
adding some security hardening to mitigate incorrect third-party <allow/>
rules in /etc/dbus-1/system.d.

I know I didn't get pre-approval before the 1.8.14-1 upload to unstable,
and I apologise for that, but it was a coordinated/embargoed
upload to mitigate CVE-2014-8148 (<https://bugs.debian.org/774630>,
<http://www.openwall.com/lists/oss-security/2015/01/05/4>) and a similar
but less serious vulnerability in a different third-party package (for
which I'm unfortunately still doing the CVE/embargo dance). The attack
being mitigated is just a denial of service, but it's in an area that could
potentially have privilege escalation flaws, which is why I'm being
a bit paranoid.

The reason that I've waited this long to ask for the unblock is that I
did get one regression report from Mageia, for KDE Plasma Desktop 5,
which calls UpdateActivationEnvironment on the session bus on a
non-canonical path and is blocked by the hardening. (Newer KDE 5 versions
have fixed this.) However, Debian does not have KDE 5 yet,
codesearch.debian.net indicates that all callers of
UpdateActivationEnvironment in Debian (e.g. gnome-session)
are OK with the hardening, and it has now been in unstable for 18 days
without problems.

I'm considering altering the check for the wrong object path upstream
in 1.8.16 so that the AccessDenied error is logged but otherwise ignored
for buses with <type>session</type>, which would have prevented KDE from
regressing. 1.9.x (for experimental and eventually stretch)
will continue to do the current check, because that's simpler.

Possible ways forward from here are:

* Let 1.8.14-1 through, then follow up with an upload reverting or
  relaxing whatever checks you want reverted or relaxed (possibly none)

* Upload a new version reverting or relaxing whatever checks you want
  and let that through

* Do not let the unstable version migrate until there is some other
  reason why it needs to, e.g. RC bugs (but if you go this route,
  be aware that the reason to need an update might be a security
  vulnerability - it has happened in the past and will probably
  happen again)

I would really like to have dbus in jessie track the upstream 1.8
branch, rather than being a fork like wheezy vs. 1.6, and I'm prepared
to revert any changes that you don't like (either upstream or as
Debian patches) to make that possible.

Thanks,
    S
diffstat for dbus-1.8.12 dbus-1.8.14

 NEWS                    |   31 +++++++++++++++++
 bus/driver.c            |   70 ++++++++++++++++++++++++++++++++++++++
 bus/driver.h            |    4 +-
 bus/stats.c             |    7 +++
 configure               |   26 +++++++-------
 configure.ac            |    4 +-
 dbus/dbus-sysdeps-win.c |    2 -
 debian/changelog        |    7 +++
 doc/Makefile.in         |    2 -
 test/dbus-daemon.c      |   87 ++++++++++++++++++++++++++++++++++++++++++++++++
 10 files changed, 221 insertions(+), 19 deletions(-)

diff -Nru dbus-1.8.12/bus/driver.c dbus-1.8.14/bus/driver.c
--- dbus-1.8.12/bus/driver.c	2014-11-22 10:49:21.000000000 +0000
+++ dbus-1.8.14/bus/driver.c	2015-01-01 23:32:22.000000000 +0000
@@ -878,6 +878,44 @@
 
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
 
+  if (!bus_driver_check_message_is_for_us (message, error))
+    return FALSE;
+
+#ifdef DBUS_UNIX
+    {
+      /* UpdateActivationEnvironment is basically a recipe for privilege
+      * escalation so let's be extra-careful: do not allow the sysadmin
+      * to shoot themselves in the foot. */
+      unsigned long uid;
+
+      if (!dbus_connection_get_unix_user (connection, &uid))
+        {
+          bus_context_log (bus_transaction_get_context (transaction),
+              DBUS_SYSTEM_LOG_SECURITY,
+              "rejected attempt to call UpdateActivationEnvironment by "
+              "unknown uid");
+          dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED,
+              "rejected attempt to call UpdateActivationEnvironment by "
+              "unknown uid");
+          return FALSE;
+        }
+
+      /* On the system bus, we could in principle allow uid 0 to call
+       * UpdateActivationEnvironment; but they should know better anyway,
+       * and our default system.conf has always forbidden it */
+      if (!_dbus_unix_user_is_process_owner (uid))
+        {
+          bus_context_log (bus_transaction_get_context (transaction),
+              DBUS_SYSTEM_LOG_SECURITY,
+              "rejected attempt to call UpdateActivationEnvironment by uid %lu",
+              uid);
+          dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED,
+              "rejected attempt to call UpdateActivationEnvironment");
+          return FALSE;
+        }
+    }
+#endif
+
   activation = bus_connection_get_activation (connection);
 
   dbus_message_iter_init (message, &iter);
@@ -1965,6 +2003,38 @@
   return FALSE;
 }
 
+/*
+ * Set @error and return FALSE if the message is not directed to the
+ * dbus-daemon by its canonical object path. This is hardening against
+ * system services with poorly-written security policy files, which
+ * might allow sending dangerously broad equivalence classes of messages
+ * such as "anything with this assumed-to-be-safe object path".
+ *
+ * dbus-daemon is unusual in that it normally ignores the object path
+ * of incoming messages; we need to keep that behaviour for the "read"
+ * read-only method calls like GetConnectionUnixUser for backwards
+ * compatibility, but it seems safer to be more restrictive for things
+ * intended to be root-only or privileged-developers-only.
+ *
+ * It is possible that there are other system services with the same
+ * quirk as dbus-daemon.
+ */
+dbus_bool_t
+bus_driver_check_message_is_for_us (DBusMessage *message,
+                                    DBusError   *error)
+{
+  if (!dbus_message_has_path (message, DBUS_PATH_DBUS))
+    {
+      dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED,
+          "Method '%s' is only available at the canonical object path '%s'",
+          dbus_message_get_member (message), DBUS_PATH_DBUS);
+
+      return FALSE;
+    }
+
+  return TRUE;
+}
+
 dbus_bool_t
 bus_driver_handle_message (DBusConnection *connection,
                            BusTransaction *transaction,
diff -Nru dbus-1.8.12/bus/driver.h dbus-1.8.14/bus/driver.h
--- dbus-1.8.12/bus/driver.h	2014-11-04 14:51:05.000000000 +0000
+++ dbus-1.8.14/bus/driver.h	2015-01-01 23:32:16.000000000 +0000
@@ -46,7 +46,7 @@
 						    BusTransaction *transaction,
 						    DBusError      *error);
 dbus_bool_t bus_driver_generate_introspect_string  (DBusString *xml);
-
-
+dbus_bool_t bus_driver_check_message_is_for_us     (DBusMessage *message,
+                                                    DBusError   *error);
 
 #endif /* BUS_DRIVER_H */
diff -Nru dbus-1.8.12/bus/stats.c dbus-1.8.14/bus/stats.c
--- dbus-1.8.12/bus/stats.c	2014-11-04 14:51:05.000000000 +0000
+++ dbus-1.8.14/bus/stats.c	2015-01-01 23:33:10.000000000 +0000
@@ -29,6 +29,7 @@
 #include <dbus/dbus-connection-internal.h>
 
 #include "connection.h"
+#include "driver.h"
 #include "services.h"
 #include "utils.h"
 
@@ -49,6 +50,9 @@
 
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
 
+  if (!bus_driver_check_message_is_for_us (message, error))
+    return FALSE;
+
   context = bus_transaction_get_context (transaction);
   connections = bus_context_get_connections (context);
 
@@ -131,6 +135,9 @@
 
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
 
+  if (!bus_driver_check_message_is_for_us (message, error))
+    return FALSE;
+
   registry = bus_connection_get_registry (caller_connection);
 
   if (! dbus_message_get_args (message, error,
diff -Nru dbus-1.8.12/configure dbus-1.8.14/configure
--- dbus-1.8.12/configure	2014-11-24 13:02:03.000000000 +0000
+++ dbus-1.8.14/configure	2015-01-01 23:33:31.000000000 +0000
@@ -1,6 +1,6 @@
 #! /bin/sh
 # Guess values for system-dependent variables and create Makefiles.
-# Generated by GNU Autoconf 2.69 for dbus 1.8.12.
+# Generated by GNU Autoconf 2.69 for dbus 1.8.14.
 #
 # Report bugs to <https://bugs.freedesktop.org/enter_bug.cgi?product=dbus>.
 #
@@ -591,8 +591,8 @@
 # Identity of this package.
 PACKAGE_NAME='dbus'
 PACKAGE_TARNAME='dbus'
-PACKAGE_VERSION='1.8.12'
-PACKAGE_STRING='dbus 1.8.12'
+PACKAGE_VERSION='1.8.14'
+PACKAGE_STRING='dbus 1.8.14'
 PACKAGE_BUGREPORT='https://bugs.freedesktop.org/enter_bug.cgi?product=dbus'
 PACKAGE_URL=''
 
@@ -1513,7 +1513,7 @@
   # Omit some internal or obsolete options to make the list less imposing.
   # This message is too long to be a string in the A/UX 3.1 sh.
   cat <<_ACEOF
-\`configure' configures dbus 1.8.12 to adapt to many kinds of systems.
+\`configure' configures dbus 1.8.14 to adapt to many kinds of systems.
 
 Usage: $0 [OPTION]... [VAR=VALUE]...
 
@@ -1587,7 +1587,7 @@
 
 if test -n "$ac_init_help"; then
   case $ac_init_help in
-     short | recursive ) echo "Configuration of dbus 1.8.12:";;
+     short | recursive ) echo "Configuration of dbus 1.8.14:";;
    esac
   cat <<\_ACEOF
 
@@ -1784,7 +1784,7 @@
 test -n "$ac_init_help" && exit $ac_status
 if $ac_init_version; then
   cat <<\_ACEOF
-dbus configure 1.8.12
+dbus configure 1.8.14
 generated by GNU Autoconf 2.69
 
 Copyright (C) 2012 Free Software Foundation, Inc.
@@ -2503,7 +2503,7 @@
 This file contains any messages produced by compilers while
 running configure, to aid debugging if configure makes a mistake.
 
-It was created by dbus $as_me 1.8.12, which was
+It was created by dbus $as_me 1.8.14, which was
 generated by GNU Autoconf 2.69.  Invocation command line was
 
   $ $0 $@
@@ -3446,7 +3446,7 @@
 
 # Define the identity of the package.
  PACKAGE='dbus'
- VERSION='1.8.12'
+ VERSION='1.8.14'
 
 
 cat >>confdefs.h <<_ACEOF
@@ -3746,7 +3746,7 @@
 
 ## increment any time the source changes; set to
 ##  0 if you increment CURRENT
-LT_REVISION=9
+LT_REVISION=10
 
 ## increment if any interfaces have been added; set to 0
 ## if any interfaces have been changed or removed. removal has
@@ -3759,8 +3759,8 @@
 
 DBUS_MAJOR_VERSION=1
 DBUS_MINOR_VERSION=8
-DBUS_MICRO_VERSION=12
-DBUS_VERSION=1.8.12
+DBUS_MICRO_VERSION=14
+DBUS_VERSION=1.8.14
 
 
 
@@ -23428,7 +23428,7 @@
 # report actual input values of CONFIG_FILES etc. instead of their
 # values after options handling.
 ac_log="
-This file was extended by dbus $as_me 1.8.12, which was
+This file was extended by dbus $as_me 1.8.14, which was
 generated by GNU Autoconf 2.69.  Invocation command line was
 
   CONFIG_FILES    = $CONFIG_FILES
@@ -23494,7 +23494,7 @@
 cat >>$CONFIG_STATUS <<_ACEOF || ac_write_fail=1
 ac_cs_config="`$as_echo "$ac_configure_args" | sed 's/^ //; s/[\\""\`\$]/\\\\&/g'`"
 ac_cs_version="\\
-dbus config.status 1.8.12
+dbus config.status 1.8.14
 configured by $0, generated by GNU Autoconf 2.69,
   with options \\"\$ac_cs_config\\"
 
diff -Nru dbus-1.8.12/configure.ac dbus-1.8.14/configure.ac
--- dbus-1.8.12/configure.ac	2014-11-24 13:01:26.000000000 +0000
+++ dbus-1.8.14/configure.ac	2015-01-01 23:33:20.000000000 +0000
@@ -3,7 +3,7 @@
 
 m4_define([dbus_major_version], [1])
 m4_define([dbus_minor_version], [8])
-m4_define([dbus_micro_version], [12])
+m4_define([dbus_micro_version], [14])
 m4_define([dbus_version],
           [dbus_major_version.dbus_minor_version.dbus_micro_version])
 AC_INIT([dbus],[dbus_version],[https://bugs.freedesktop.org/enter_bug.cgi?product=dbus],[dbus])
@@ -37,7 +37,7 @@
 
 ## increment any time the source changes; set to
 ##  0 if you increment CURRENT
-LT_REVISION=9
+LT_REVISION=10
 
 ## increment if any interfaces have been added; set to 0
 ## if any interfaces have been changed or removed. removal has
diff -Nru dbus-1.8.12/dbus/dbus-sysdeps-win.c dbus-1.8.14/dbus/dbus-sysdeps-win.c
--- dbus-1.8.12/dbus/dbus-sysdeps-win.c	2014-11-04 14:51:06.000000000 +0000
+++ dbus-1.8.14/dbus/dbus-sysdeps-win.c	2014-12-31 15:41:32.000000000 +0000
@@ -146,7 +146,7 @@
 get_pid_from_extended_tcp_table(int peer_port)
 {
   dbus_pid_t result;
-  DWORD errorCode, size, i;
+  DWORD errorCode, size = 0, i;
   MIB_TCPTABLE_OWNER_PID *tcp_table;
 
   if ((errorCode =
diff -Nru dbus-1.8.12/debian/changelog dbus-1.8.14/debian/changelog
--- dbus-1.8.12/debian/changelog	2014-12-23 21:24:31.000000000 +0000
+++ dbus-1.8.14/debian/changelog	2015-01-02 11:07:15.000000000 +0000
@@ -1,3 +1,10 @@
+dbus (1.8.14-1) unstable; urgency=medium
+
+  * New upstream release to harden dbus-daemon against packages that install
+    unsafe security policy configurations.
+
+ -- Simon McVittie <smcv@debian.org>  Thu, 01 Jan 2015 13:07:23 +0000
+
 dbus (1.8.12-3) unstable; urgency=medium
 
   * preinst: partially revert change from 1.8.12-2. It seems that the
diff -Nru dbus-1.8.12/doc/Makefile.in dbus-1.8.14/doc/Makefile.in
--- dbus-1.8.12/doc/Makefile.in	2014-11-24 13:02:05.000000000 +0000
+++ dbus-1.8.14/doc/Makefile.in	2015-01-01 23:33:31.000000000 +0000
@@ -666,8 +666,8 @@
 maintainer-clean-generic:
 	@echo "This command is intended for maintainers to use"
 	@echo "it deletes files that may require special tools to rebuild."
-@DBUS_DOXYGEN_DOCS_ENABLED_FALSE@uninstall-local:
 @DBUS_DOXYGEN_DOCS_ENABLED_FALSE@install-data-local:
+@DBUS_DOXYGEN_DOCS_ENABLED_FALSE@uninstall-local:
 clean: clean-am
 
 clean-am: clean-generic clean-libtool clean-local mostlyclean-am
diff -Nru dbus-1.8.12/NEWS dbus-1.8.14/NEWS
--- dbus-1.8.12/NEWS	2014-11-24 13:01:19.000000000 +0000
+++ dbus-1.8.14/NEWS	2015-01-01 23:42:32.000000000 +0000
@@ -1,3 +1,34 @@
+D-Bus 1.8.14 (2015-01-05)
+==
+
+The “40lb of roofing nails” release.
+
+Security hardening:
+
+• Do not allow calls to UpdateActivationEnvironment from uids other than
+  the uid of the dbus-daemon. If a system service installs unsafe
+  security policy rules that allow arbitrary method calls
+  (such as CVE-2014-8148) then this prevents memory consumption and
+  possible privilege escalation via UpdateActivationEnvironment.
+
+  We believe that in practice, privilege escalation here is avoided
+  by dbus-daemon-launch-helper sanitizing its environment; but
+  it seems better to be safe.
+
+• Do not allow calls to UpdateActivationEnvironment or the Stats interface
+  on object paths other than /org/freedesktop/DBus. Some system services
+  install unsafe security policy rules that allow arbitrary method calls
+  to any destination, method and interface with a specified object path;
+  while less bad than allowing arbitrary method calls, these security
+  policies are still harmful, since dbus-daemon normally offers the
+  same API on all object paths and other system services might behave
+  similarly.
+
+Other fixes:
+
+• Add missing initialization so GetExtendedTcpTable doesn't crash on
+  Windows Vista SP0 (fd.o #77008, Илья А. Ткаченко)
+
 D-Bus 1.8.12 (2014-11-24)
 ==
 
diff -Nru dbus-1.8.12/test/dbus-daemon.c dbus-1.8.14/test/dbus-daemon.c
--- dbus-1.8.12/test/dbus-daemon.c	2014-11-04 14:51:06.000000000 +0000
+++ dbus-1.8.14/test/dbus-daemon.c	2015-01-01 23:32:33.000000000 +0000
@@ -458,6 +458,91 @@
 }
 
 static void
+test_canonical_path_uae (Fixture *f,
+    gconstpointer context)
+{
+  DBusMessage *m = dbus_message_new_method_call (DBUS_SERVICE_DBUS,
+      DBUS_PATH_DBUS, DBUS_INTERFACE_DBUS, "UpdateActivationEnvironment");
+  DBusPendingCall *pc;
+  DBusMessageIter args_iter;
+  DBusMessageIter arr_iter;
+
+  if (m == NULL)
+    g_error ("OOM");
+
+  dbus_message_iter_init_append (m, &args_iter);
+
+  /* Append an empty a{ss} (string => string dictionary). */
+  if (!dbus_message_iter_open_container (&args_iter, DBUS_TYPE_ARRAY,
+        "{ss}", &arr_iter) ||
+      !dbus_message_iter_close_container (&args_iter, &arr_iter))
+    g_error ("OOM");
+
+  if (!dbus_connection_send_with_reply (f->left_conn, m, &pc,
+                                        DBUS_TIMEOUT_USE_DEFAULT) ||
+      pc == NULL)
+    g_error ("OOM");
+
+  dbus_message_unref (m);
+  m = NULL;
+
+  if (dbus_pending_call_get_completed (pc))
+    pending_call_store_reply (pc, &m);
+  else if (!dbus_pending_call_set_notify (pc, pending_call_store_reply,
+                                          &m, NULL))
+    g_error ("OOM");
+
+  while (m == NULL)
+    test_main_context_iterate (f->ctx, TRUE);
+
+  /* it succeeds */
+  g_assert_cmpint (dbus_message_get_type (m), ==,
+      DBUS_MESSAGE_TYPE_METHOD_RETURN);
+
+  dbus_message_unref (m);
+
+  /* Now try with the wrong object path */
+  m = dbus_message_new_method_call (DBUS_SERVICE_DBUS,
+      "/com/example/Wrong", DBUS_INTERFACE_DBUS, "UpdateActivationEnvironment");
+
+  if (m == NULL)
+    g_error ("OOM");
+
+  dbus_message_iter_init_append (m, &args_iter);
+
+  /* Append an empty a{ss} (string => string dictionary). */
+  if (!dbus_message_iter_open_container (&args_iter, DBUS_TYPE_ARRAY,
+        "{ss}", &arr_iter) ||
+      !dbus_message_iter_close_container (&args_iter, &arr_iter))
+    g_error ("OOM");
+
+  if (!dbus_connection_send_with_reply (f->left_conn, m, &pc,
+                                        DBUS_TIMEOUT_USE_DEFAULT) ||
+      pc == NULL)
+    g_error ("OOM");
+
+  dbus_message_unref (m);
+  m = NULL;
+
+  if (dbus_pending_call_get_completed (pc))
+    pending_call_store_reply (pc, &m);
+  else if (!dbus_pending_call_set_notify (pc, pending_call_store_reply,
+                                          &m, NULL))
+    g_error ("OOM");
+
+  while (m == NULL)
+    test_main_context_iterate (f->ctx, TRUE);
+
+  /* it fails, yielding an error message with one string argument */
+  g_assert_cmpint (dbus_message_get_type (m), ==, DBUS_MESSAGE_TYPE_ERROR);
+  g_assert_cmpstr (dbus_message_get_error_name (m), ==,
+      DBUS_ERROR_ACCESS_DENIED);
+  g_assert_cmpstr (dbus_message_get_signature (m), ==, "s");
+
+  dbus_message_unref (m);
+}
+
+static void
 teardown (Fixture *f,
     gconstpointer context G_GNUC_UNUSED)
 {
@@ -514,6 +599,8 @@
   g_test_add ("/echo/limited", Fixture, &limited_config,
       setup, test_echo, teardown);
   g_test_add ("/creds", Fixture, NULL, setup, test_creds, teardown);
+  g_test_add ("/canonical-path/uae", Fixture, NULL,
+      setup, test_canonical_path_uae, teardown);
 
   return g_test_run ();
 }

Reply to: