From 9b3bd4585b69f133dad70b11120d51f846e5d6d2 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 26 Jan 2015 19:12:01 +0000 Subject: [PATCH 6/6] bus driver: factor out bus_driver_check_caller_is_privileged, and allow root Unlike the initial mitigation for CVE-2014-8148, we now allow uid 0 to call UpdateActivationEnvironment. There's no point in root doing that, but there's also no reason why it's particularly bad - if an attacker is uid 0 we've already lost - and it simplifies use of this function for future things that do want to be callable by root, like BecomeMonitor for #46787. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=88810 --- bus/driver.c | 121 +++++++++++++++++++++++++++++++++++++------------ test/uid-permissions.c | 6 +-- 2 files changed, 95 insertions(+), 32 deletions(-) diff --git a/bus/driver.c b/bus/driver.c index 952061c..c3dd32c 100644 --- a/bus/driver.c +++ b/bus/driver.c @@ -82,6 +82,92 @@ bus_driver_get_conn_helper (DBusConnection *connection, return conn; } +static dbus_bool_t +bus_driver_check_caller_is_privileged (DBusConnection *connection, + BusTransaction *transaction, + DBusMessage *message, + DBusError *error) +{ +#ifdef DBUS_UNIX + unsigned long uid; + + if (!dbus_connection_get_unix_user (connection, &uid)) + { + const char *method = dbus_message_get_member (message); + + /* Yes this repetition is pretty horrible, but there's no + * bus_context_log_valist() or dbus_set_error_valist() or + * bus_context_log_literal() or dbus_set_error_literal(). + */ + bus_context_log (bus_transaction_get_context (transaction), + DBUS_SYSTEM_LOG_SECURITY, + "rejected attempt to call %s by unknown uid", method); + dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED, + "rejected attempt to call %s by unknown uid", method); + return FALSE; + } + + /* I'm writing it in this slightly strange form so that it's more + * obvious that this security-sensitive code is correct. + */ + if (_dbus_unix_user_is_process_owner (uid)) + { + /* OK */ + } + else if (uid == 0) + { + /* OK */ + } + else + { + const char *method = dbus_message_get_member (message); + + bus_context_log (bus_transaction_get_context (transaction), + DBUS_SYSTEM_LOG_SECURITY, + "rejected attempt to call %s by uid %lu", method, uid); + dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED, + "rejected attempt to call %s by uid %lu", method, uid); + return FALSE; + } + + return TRUE; +#elif DBUS_WIN + char *windows_sid; + + if (!dbus_connection_get_windows_user (connection, &windows_sid)) + { + const char *method = dbus_message_get_member (message); + + bus_context_log (bus_transaction_get_context (transaction), + DBUS_SYSTEM_LOG_SECURITY, + "rejected attempt to call %s by unknown uid", method); + dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED, + "rejected attempt to call %s by unknown uid", method); + return FALSE; + } + + if (!_dbus_windows_user_is_process_owner (windows_sid)) + { + const char *method = dbus_message_get_member (message); + + bus_context_log (bus_transaction_get_context (transaction), + DBUS_SYSTEM_LOG_SECURITY, + "rejected attempt to call %s by uid %s", method, windows_sid); + dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED, + "rejected attempt to call %s by uid %s", method, windows_sid); + return FALSE; + } + + return TRUE; +#else + /* make sure we fail closed in the hypothetical case that we are neither + * Unix nor Windows */ + dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED, + "please teach bus/driver.c how uids work on this platform"); + return FALSE; +#endif +} + static dbus_bool_t bus_driver_send_welcome_message (DBusConnection *connection, DBusMessage *hello_message, BusTransaction *transaction, @@ -884,35 +970,12 @@ bus_driver_handle_update_activation_environment (DBusConnection *connection, #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; - } + * escalation so let's be extra-careful: do not allow the sysadmin + * to shoot themselves in the foot. + */ + if (!bus_driver_check_caller_is_privileged (connection, transaction, + message, error)) + return FALSE; } #endif diff --git a/test/uid-permissions.c b/test/uid-permissions.c index c200528..6fee31a 100644 --- a/test/uid-permissions.c +++ b/test/uid-permissions.c @@ -150,10 +150,10 @@ teardown (Fixture *f, test_main_context_unref (f->ctx); } -static Config root_fail_config = { +static Config root_ok_config = { "valid-config-files/multi-user.conf", TEST_USER_ROOT, - FALSE + TRUE }; static Config messagebus_ok_config = { @@ -175,7 +175,7 @@ main (int argc, g_test_init (&argc, &argv, NULL); g_test_bug_base ("https://bugs.freedesktop.org/show_bug.cgi?id="); - g_test_add ("/uid-permissions/uae/root", Fixture, &root_fail_config, + g_test_add ("/uid-permissions/uae/root", Fixture, &root_ok_config, setup, test_uae, teardown); g_test_add ("/uid-permissions/uae/messagebus", Fixture, &messagebus_ok_config, setup, test_uae, teardown); -- 2.1.4