From 76ceb459dae9c8d504fb3d6d68cd6ea097a1ed74 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 26 Jan 2015 21:10:18 +0000 Subject: [PATCH 1/2] Fix regression in 1.8.14 with KDE Plasma Workspace 5 codesearch.debian.net said everything in Debian that called UpdateActivationEnvironment (e.g. gnome-shell) used the canonical object path to do so. Unfortunately, the heuristic "all open source that matters is in Debian" is not completely true, and in particular, newer versions of KDE Plasma Workspace call this method on "/". Partially revert the change, logging a warning but not rejecting the method call if the bus has session. --- bus/driver.c | 36 +++++++++++++- configure.ac | 1 + .../valid-config-files/type-unspecified.conf.in | 14 ++++++ test/dbus-daemon.c | 56 +++++++++++++++++++++- 4 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 test/data/valid-config-files/type-unspecified.conf.in diff --git a/bus/driver.c b/bus/driver.c index f5d3ebe..6048d24 100644 --- a/bus/driver.c +++ b/bus/driver.c @@ -40,6 +40,13 @@ #include #include +static inline const char * +nonnull (const char *maybe_null, + const char *if_null) +{ + return (maybe_null ? maybe_null : if_null); +} + static DBusConnection * bus_driver_get_conn_helper (DBusConnection *connection, DBusMessage *message, @@ -879,7 +886,34 @@ 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; + { + const char *type; + + /* Unfortunately some versions of KDE Plasma Desktop 5 call this + * method on the session bus on a non-canonical path, so this check + * makes KDE regress. This is temporary regression-avoidance + * and will be removed in dbus 1.10 to simplify this + * security-sensitive code + */ + type = bus_context_get_type (bus_transaction_get_context (transaction)); + + if (type != NULL && strcmp (type, "session") == 0) + { + bus_context_log (bus_transaction_get_context (transaction), + DBUS_SYSTEM_LOG_INFO, + "connection %s (%s) called UpdateActivationEnvironment " + "at a path that is not /org/freedesktop/DBus. This will " + "not be allowed in dbus 1.10; please fix the application", + nonnull (bus_connection_get_name (connection), "(inactive)"), + bus_connection_get_loginfo (connection)); + if (error != NULL) + dbus_error_free (error); + } + else + { + return FALSE; + } + } #ifdef DBUS_UNIX { diff --git a/configure.ac b/configure.ac index e4d0f0f..ed9bafc 100644 --- a/configure.ac +++ b/configure.ac @@ -1772,6 +1772,7 @@ dbus-1-uninstalled.pc test/data/valid-config-files/debug-allow-all.conf test/data/valid-config-files/debug-allow-all-sha1.conf test/data/valid-config-files/incoming-limit.conf +test/data/valid-config-files/type-unspecified.conf test/data/valid-config-files-system/debug-allow-all-pass.conf test/data/valid-config-files-system/debug-allow-all-fail.conf test/data/valid-service-files/org.freedesktop.DBus.TestSuite.PrivServer.service diff --git a/test/data/valid-config-files/type-unspecified.conf.in b/test/data/valid-config-files/type-unspecified.conf.in new file mode 100644 index 0000000..52bd292 --- /dev/null +++ b/test/data/valid-config-files/type-unspecified.conf.in @@ -0,0 +1,14 @@ + + + @TEST_LISTEN@ + + + + + + + + + + diff --git a/test/dbus-daemon.c b/test/dbus-daemon.c index dc0f131..bff836d 100644 --- a/test/dbus-daemon.c +++ b/test/dbus-daemon.c @@ -542,6 +542,54 @@ test_canonical_path_uae (Fixture *f, dbus_message_unref (m); } +/* This imitates KDE Plasma Workspace 5, versions < 5.2.0, + * which worked under dbus-daemon 1.8.12 but not 1.8.14. */ +static void +test_canonical_path_uae_session (Fixture *f, + gconstpointer context) +{ + DBusMessage *m = dbus_message_new_method_call (DBUS_SERVICE_DBUS, + "/", DBUS_INTERFACE_DBUS, "UpdateActivationEnvironment"); + /* ^^^ should be DBUS_PATH_DBUS but until 5.2.0 it wasn't */ + 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); +} + static void teardown (Fixture *f, gconstpointer context G_GNUC_UNUSED) @@ -588,6 +636,10 @@ static Config limited_config = { "34393", 10000, "valid-config-files/incoming-limit.conf" }; +static Config non_session_config = { + NULL, 0, "valid-config-files/type-unspecified.conf" +}; + int main (int argc, char **argv) @@ -599,7 +651,9 @@ main (int argc, 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, + g_test_add ("/canonical-path/uae/session", Fixture, NULL, + setup, test_canonical_path_uae_session, teardown); + g_test_add ("/canonical-path/uae", Fixture, &non_session_config, setup, test_canonical_path_uae, teardown); return g_test_run (); -- 2.1.4