summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSimon McVittie <simon.mcvittie@collabora.co.uk>2015-01-01 23:48:13 +0000
committerSimon McVittie <simon.mcvittie@collabora.co.uk>2015-01-01 23:48:13 +0000
commitae9d7149aa9a9f8f276c35b2343e78aaa7c9054c (patch)
tree8697326f4c120119e825b82f96bd239002de3712
parentfda9d8a44aa1bde4f2777fb9ad8650f45820fb6b (diff)
parentabbbf449f17e0a74a5d9a50fb5b074e96e9b7030 (diff)
Merge branch 'dbus-1.8' and prepare 1.9.6
Conflicts: NEWS configure.ac test/dbus-daemon.c
-rw-r--r--NEWS30
-rw-r--r--bus/driver.c70
-rw-r--r--bus/driver.h4
-rw-r--r--bus/stats.c7
-rw-r--r--configure.ac4
-rw-r--r--dbus/dbus-sysdeps-win.c2
-rw-r--r--test/dbus-daemon.c87
7 files changed, 197 insertions, 7 deletions
diff --git a/NEWS b/NEWS
index 65f4e3a24..3ed896674 100644
--- a/NEWS
+++ b/NEWS
@@ -1,7 +1,33 @@
-D-Bus 1.9.6 (UNRELEASED)
+D-Bus 1.9.6 (2015-01-05)
==
-...
+The “I do have a bread knife” 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.9.4 (2014-11-24)
==
diff --git a/bus/driver.c b/bus/driver.c
index 777b2f897..952061c63 100644
--- a/bus/driver.c
+++ b/bus/driver.c
@@ -878,6 +878,44 @@ bus_driver_handle_update_activation_environment (DBusConnection *connection,
_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);
@@ -1966,6 +2004,38 @@ bus_driver_handle_introspect (DBusConnection *connection,
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 --git a/bus/driver.h b/bus/driver.h
index 713b27640..201709c4c 100644
--- a/bus/driver.h
+++ b/bus/driver.h
@@ -46,7 +46,7 @@ dbus_bool_t bus_driver_send_service_owner_changed (const char *service_name
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 --git a/bus/stats.c b/bus/stats.c
index 859c6a520..dace0e29d 100644
--- a/bus/stats.c
+++ b/bus/stats.c
@@ -29,6 +29,7 @@
#include <dbus/dbus-connection-internal.h>
#include "connection.h"
+#include "driver.h"
#include "services.h"
#include "signals.h"
#include "utils.h"
@@ -50,6 +51,9 @@ bus_stats_handle_get_stats (DBusConnection *connection,
_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);
@@ -132,6 +136,9 @@ bus_stats_handle_get_connection_stats (DBusConnection *caller_connection,
_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 --git a/configure.ac b/configure.ac
index ce6f9d6cf..6dcb2818c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3,7 +3,7 @@ AC_PREREQ([2.63])
m4_define([dbus_major_version], [1])
m4_define([dbus_minor_version], [9])
-m4_define([dbus_micro_version], [5])
+m4_define([dbus_micro_version], [6])
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 @@ LT_CURRENT=13
## increment any time the source changes; set to
## 0 if you increment CURRENT
-LT_REVISION=1
+LT_REVISION=2
## increment if any interfaces have been added; set to 0
## if any interfaces have been changed or removed. removal has
diff --git a/dbus/dbus-sysdeps-win.c b/dbus/dbus-sysdeps-win.c
index 2c18f40e7..208e8522e 100644
--- a/dbus/dbus-sysdeps-win.c
+++ b/dbus/dbus-sysdeps-win.c
@@ -146,7 +146,7 @@ static dbus_pid_t
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 --git a/test/dbus-daemon.c b/test/dbus-daemon.c
index bb1d13740..1cd51d20a 100644
--- a/test/dbus-daemon.c
+++ b/test/dbus-daemon.c
@@ -635,6 +635,91 @@ test_processid (Fixture *f,
}
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)
{
@@ -700,6 +785,8 @@ main (int argc,
setup, test_no_reply, teardown);
g_test_add ("/creds", Fixture, NULL, setup, test_creds, teardown);
g_test_add ("/processid", Fixture, NULL, setup, test_processid, teardown);
+ g_test_add ("/canonical-path/uae", Fixture, NULL,
+ setup, test_canonical_path_uae, teardown);
return g_test_run ();
}