Commit 1f716452 authored by Simon McVittie's avatar Simon McVittie

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
Reviewed-by: Philip Withnall
parent 98ae1149
......@@ -82,6 +82,107 @@ bus_driver_get_conn_helper (DBusConnection *connection,
return conn;
}
/*
* Log a security warning and set error unless the uid of the connection
* is either the uid of this process, or on Unix, uid 0 (root).
*
* This is intended to be a second line of defence after <deny> rules,
* to mitigate incorrect system bus security policy configuration files
* like the ones in CVE-2014-8148 and CVE-2014-8156, and (if present)
* LSM rules; so it doesn't need to be perfect, but as long as we have
* potentially dangerous functionality in the system bus, it does need
* to exist.
*/
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 = NULL;
dbus_bool_t ret = FALSE;
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);
goto out;
}
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);
goto out;
}
ret = TRUE;
out:
dbus_free (windows_sid);
return ret;
#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 +985,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
......
......@@ -164,10 +164,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 = {
......@@ -189,7 +189,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);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment