diff options
author | Simon McVittie <simon.mcvittie@collabora.co.uk> | 2015-01-01 23:48:13 +0000 |
---|---|---|
committer | Simon McVittie <simon.mcvittie@collabora.co.uk> | 2015-01-01 23:48:13 +0000 |
commit | ae9d7149aa9a9f8f276c35b2343e78aaa7c9054c (patch) | |
tree | 8697326f4c120119e825b82f96bd239002de3712 | |
parent | fda9d8a44aa1bde4f2777fb9ad8650f45820fb6b (diff) | |
parent | abbbf449f17e0a74a5d9a50fb5b074e96e9b7030 (diff) |
Merge branch 'dbus-1.8' and prepare 1.9.6dbus-1.9.6
Conflicts:
NEWS
configure.ac
test/dbus-daemon.c
-rw-r--r-- | NEWS | 30 | ||||
-rw-r--r-- | bus/driver.c | 70 | ||||
-rw-r--r-- | bus/driver.h | 4 | ||||
-rw-r--r-- | bus/stats.c | 7 | ||||
-rw-r--r-- | configure.ac | 4 | ||||
-rw-r--r-- | dbus/dbus-sysdeps-win.c | 2 | ||||
-rw-r--r-- | test/dbus-daemon.c | 87 |
7 files changed, 197 insertions, 7 deletions
@@ -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 777b2f89..952061c6 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 713b2764..201709c4 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 859c6a52..dace0e29 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 ce6f9d6c..6dcb2818 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 2c18f40e..208e8522 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 bb1d1374..1cd51d20 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 (); } |