From 9ea3dafe7d61cd0751d0a8800c7cf2d9b9104370 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 21 Nov 2008 18:11:15 +0000 Subject: [PATCH] 2008-11-21 Dan Williams * src/nm-dbus-manager.c src/nm-dbus-manager.h - (nm_dbus_manager_get_name_owner): return error * src/nm-manager.c - (impl_manager_activate_connection): perform additional validation on ActivateConnection calls of user connections - (is_user_request_authorized): ensure that the requestor is the same UID as the UID that owns the user settings service; users shouldn't be able to control another user's connections git-svn-id: http://svn-archive.gnome.org/svn/NetworkManager/trunk@4325 4912f4e0-d625-0410-9fb7-b9a5a253dbdc --- ChangeLog | 13 +++++ src/nm-dbus-manager.c | 21 ++++---- src/nm-dbus-manager.h | 3 +- src/nm-manager.c | 115 +++++++++++++++++++++++++++++++++++++++--- 4 files changed, 134 insertions(+), 18 deletions(-) diff --git a/ChangeLog b/ChangeLog index 8dd6a42ba0..b869dfe590 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +2008-11-21 Dan Williams + + * src/nm-dbus-manager.c + src/nm-dbus-manager.h + - (nm_dbus_manager_get_name_owner): return error + + * src/nm-manager.c + - (impl_manager_activate_connection): perform additional validation on + ActivateConnection calls of user connections + - (is_user_request_authorized): ensure that the requestor is the same + UID as the UID that owns the user settings service; users shouldn't + be able to control another user's connections + 2008-11-21 Dan Williams * gfilemonitor/inotify-sub.c diff --git a/src/nm-dbus-manager.c b/src/nm-dbus-manager.c index a138447d84..f8d48f5875 100644 --- a/src/nm-dbus-manager.c +++ b/src/nm-dbus-manager.c @@ -163,22 +163,23 @@ start_reconnection_timeout (NMDBusManager *self) char * nm_dbus_manager_get_name_owner (NMDBusManager *self, - const char *name) + const char *name, + GError **error) { char *owner = NULL; - GError *err = NULL; g_return_val_if_fail (NM_IS_DBUS_MANAGER (self), NULL); g_return_val_if_fail (name != NULL, NULL); + if (error) + g_return_val_if_fail (*error == NULL, NULL); - if (!dbus_g_proxy_call (NM_DBUS_MANAGER_GET_PRIVATE (self)->proxy, - "GetNameOwner", &err, - G_TYPE_STRING, name, - G_TYPE_INVALID, - G_TYPE_STRING, &owner, - G_TYPE_INVALID)) { - nm_warning ("Error on GetNameOwner DBUS call: %s", err->message); - g_error_free (err); + if (!dbus_g_proxy_call_with_timeout (NM_DBUS_MANAGER_GET_PRIVATE (self)->proxy, + "GetNameOwner", 2000, error, + G_TYPE_STRING, name, + G_TYPE_INVALID, + G_TYPE_STRING, &owner, + G_TYPE_INVALID)) { + return NULL; } return owner; diff --git a/src/nm-dbus-manager.h b/src/nm-dbus-manager.h index 953ecb5a8e..275ae4e092 100644 --- a/src/nm-dbus-manager.h +++ b/src/nm-dbus-manager.h @@ -62,7 +62,8 @@ GType nm_dbus_manager_get_type (void); NMDBusManager * nm_dbus_manager_get (void); char * nm_dbus_manager_get_name_owner (NMDBusManager *self, - const char *name); + const char *name, + GError **error); gboolean nm_dbus_manager_start_service (NMDBusManager *self); diff --git a/src/nm-manager.c b/src/nm-manager.c index 9133368918..e5ebdb2829 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -21,6 +21,8 @@ #include #include +#include +#include #include "nm-manager.h" #include "nm-utils.h" @@ -1973,13 +1975,109 @@ connection_added_default_handler (NMManager *manager, pending_connection_info_destroy (info); } +static gboolean +is_user_request_authorized (NMManager *manager, + DBusGMethodInvocation *context, + GError **error) +{ + NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager); + DBusConnection *connection; + char *sender = NULL; + gulong sender_uid = G_MAXULONG; + DBusError dbus_error; + char *service_owner = NULL; + const char *service_name; + gulong service_uid = G_MAXULONG; + gboolean success = FALSE; + + /* Ensure the request to activate the user connection came from the + * same session as the user settings service. FIXME: use ConsoleKit + * too. + */ + if (!priv->user_proxy) { + g_set_error (error, NM_MANAGER_ERROR, + NM_MANAGER_ERROR_INVALID_SERVICE, + "%s", "No user settings service available"); + goto out; + } + + sender = dbus_g_method_get_sender (context); + if (!sender) { + g_set_error (error, NM_MANAGER_ERROR, + NM_MANAGER_ERROR_PERMISSION_DENIED, + "%s", "Could not determine D-Bus requestor"); + goto out; + } + + connection = nm_dbus_manager_get_dbus_connection (priv->dbus_mgr); + if (!connection) { + g_set_error (error, NM_MANAGER_ERROR, + NM_MANAGER_ERROR_PERMISSION_DENIED, + "%s", "Could not get the D-Bus system bus"); + goto out; + } + + dbus_error_init (&dbus_error); + /* FIXME: do this async */ + sender_uid = dbus_bus_get_unix_user (connection, sender, &dbus_error); + if (dbus_error_is_set (&dbus_error)) { + dbus_error_free (&dbus_error); + g_set_error (error, NM_MANAGER_ERROR, + NM_MANAGER_ERROR_PERMISSION_DENIED, + "%s", "Could not determine the Unix user ID of the requestor"); + goto out; + } + + service_name = dbus_g_proxy_get_bus_name (priv->user_proxy); + if (!service_name) { + g_set_error (error, NM_MANAGER_ERROR, + NM_MANAGER_ERROR_PERMISSION_DENIED, + "%s", "Could not determine user settings service name"); + goto out; + } + + service_owner = nm_dbus_manager_get_name_owner (priv->dbus_mgr, service_name, NULL); + if (!service_owner) { + g_set_error (error, NM_MANAGER_ERROR, + NM_MANAGER_ERROR_PERMISSION_DENIED, + "%s", "Could not determine D-Bus owner of the user settings service"); + goto out; + } + + dbus_error_init (&dbus_error); + /* FIXME: do this async */ + service_uid = dbus_bus_get_unix_user (connection, service_owner, &dbus_error); + if (dbus_error_is_set (&dbus_error)) { + dbus_error_free (&dbus_error); + g_set_error (error, NM_MANAGER_ERROR, + NM_MANAGER_ERROR_PERMISSION_DENIED, + "%s", "Could not determine the Unix UID of the sender of the request"); + goto out; + } + + /* And finally, the actual UID check */ + if (sender_uid != service_uid) { + g_set_error (error, NM_MANAGER_ERROR, + NM_MANAGER_ERROR_PERMISSION_DENIED, + "%s", "Requestor UID does not match the UID of the user settings service"); + goto out; + } + + success = TRUE; + +out: + g_free (sender); + g_free (service_owner); + return success; +} + static void impl_manager_activate_connection (NMManager *manager, - const char *service_name, - const char *connection_path, - const char *device_path, - const char *specific_object_path, - DBusGMethodInvocation *context) + const char *service_name, + const char *connection_path, + const char *device_path, + const char *specific_object_path, + DBusGMethodInvocation *context) { NMConnectionScope scope = NM_CONNECTION_SCOPE_UNKNOWN; NMConnection *connection; @@ -1987,9 +2085,12 @@ impl_manager_activate_connection (NMManager *manager, char *real_sop = NULL; char *path = NULL; - if (!strcmp (service_name, NM_DBUS_SERVICE_USER_SETTINGS)) + if (!strcmp (service_name, NM_DBUS_SERVICE_USER_SETTINGS)) { + if (!is_user_request_authorized (manager, context, &error)) + goto err; + scope = NM_CONNECTION_SCOPE_USER; - else if (!strcmp (service_name, NM_DBUS_SERVICE_SYSTEM_SETTINGS)) + } else if (!strcmp (service_name, NM_DBUS_SERVICE_SYSTEM_SETTINGS)) scope = NM_CONNECTION_SCOPE_SYSTEM; else { g_set_error (&error,