From eb8bc5396e0f41b9ebcba5e45916de8b523168c3 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 17 Jan 2014 11:18:23 -0600 Subject: [PATCH] core: respect connection user permissions for activation/deactivation This appears to be a bug since the original 0.9.0 release when connection permissions were implemented. If all the following are true: - the user is local (as determined by systemd or ConsoleKit) - the user has been given the NETWORK_CONTROL PolicyKit permission - the user is not listed in the connection's permissions - the user knows the D-Bus object path of a connection which they have no permissions for then that user may activate/deactivate connections that are not visible to that user as determined by the connection permissions. Fix that by ensuring that these operations check whether the user has permission. These operations are *not* affected, and have always checked user permissions before allowing the operation: - modifying any connection details - requesting any secrets or passwords for the connection - deleting the connection --- src/devices/nm-device.c | 54 ++++++++++++++++------------- src/nm-manager.c | 76 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 101 insertions(+), 29 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index bb6d177024..4748c2c404 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -4601,37 +4601,39 @@ disconnect_cb (NMDevice *device, NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device); GError *local = NULL; - if (error) + if (error) { dbus_g_method_return_error (context, error); - else { - /* Authorized */ - if (priv->state <= NM_DEVICE_STATE_DISCONNECTED) { - local = g_error_new_literal (NM_DEVICE_ERROR, - NM_DEVICE_ERROR_NOT_ACTIVE, - "Device is not active"); - dbus_g_method_return_error (context, local); - g_error_free (local); - } else { - priv->autoconnect = FALSE; + return; + } - /* Software devices are removed when manually disconnected and thus - * we need to track the autoconnect flag outside the device. - */ - nm_manager_prevent_device_auto_connect (nm_manager_get (), - nm_device_get_ip_iface (device), - TRUE); + /* Authorized */ + if (priv->state <= NM_DEVICE_STATE_DISCONNECTED) { + local = g_error_new_literal (NM_DEVICE_ERROR, + NM_DEVICE_ERROR_NOT_ACTIVE, + "Device is not active"); + dbus_g_method_return_error (context, local); + g_error_free (local); + } else { + priv->autoconnect = FALSE; - nm_device_state_changed (device, - NM_DEVICE_STATE_DISCONNECTED, - NM_DEVICE_STATE_REASON_USER_REQUESTED); - dbus_g_method_return (context); - } + /* Software devices are removed when manually disconnected and thus + * we need to track the autoconnect flag outside the device. + */ + nm_manager_prevent_device_auto_connect (nm_manager_get (), + nm_device_get_ip_iface (device), + TRUE); + + nm_device_state_changed (device, + NM_DEVICE_STATE_DISCONNECTED, + NM_DEVICE_STATE_REASON_USER_REQUESTED); + dbus_g_method_return (context); } } static void impl_device_disconnect (NMDevice *device, DBusGMethodInvocation *context) { + NMConnection *connection; GError *error = NULL; if (NM_DEVICE_GET_PRIVATE (device)->act_request == NULL) { @@ -4643,9 +4645,13 @@ impl_device_disconnect (NMDevice *device, DBusGMethodInvocation *context) return; } + connection = nm_device_get_connection (device); + g_assert (connection); + /* Ask the manager to authenticate this request for us */ g_signal_emit (device, signals[AUTH_REQUEST], 0, context, + connection, NM_AUTH_PERMISSION_NETWORK_CONTROL, TRUE, disconnect_cb, @@ -5952,8 +5958,8 @@ nm_device_class_init (NMDeviceClass *klass) G_OBJECT_CLASS_TYPE (object_class), G_SIGNAL_RUN_FIRST, 0, NULL, NULL, NULL, - /* dbus-glib context, permission, allow_interaction, callback, user_data */ - G_TYPE_NONE, 5, G_TYPE_POINTER, G_TYPE_STRING, G_TYPE_BOOLEAN, G_TYPE_POINTER, G_TYPE_POINTER); + /* dbus-glib context, connection, permission, allow_interaction, callback, user_data */ + G_TYPE_NONE, 6, G_TYPE_POINTER, G_TYPE_POINTER, G_TYPE_STRING, G_TYPE_BOOLEAN, G_TYPE_POINTER, G_TYPE_POINTER); signals[IP4_CONFIG_CHANGED] = g_signal_new (NM_DEVICE_IP4_CONFIG_CHANGED, diff --git a/src/nm-manager.c b/src/nm-manager.c index c4d0d953d0..96c42fe6a4 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -1672,6 +1672,7 @@ device_auth_done_cb (NMAuthChain *chain, static void device_auth_request_cb (NMDevice *device, DBusGMethodInvocation *context, + NMConnection *connection, const char *permission, gboolean allow_interaction, NMDeviceAuthRequestFunc callback, @@ -1680,17 +1681,38 @@ device_auth_request_cb (NMDevice *device, { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); GError *error = NULL; + NMAuthSubject *subject = NULL; + char *error_desc = NULL; NMAuthChain *chain; + /* Validate the caller */ + subject = nm_auth_subject_new_from_context (context); + if (!subject) { + error = g_error_new_literal (NM_MANAGER_ERROR, + NM_MANAGER_ERROR_PERMISSION_DENIED, + "Failed to get request UID."); + goto done; + } + + /* Ensure the subject has permissions for this connection */ + if (!nm_auth_uid_in_acl (connection, + nm_session_monitor_get (), + nm_auth_subject_get_uid (subject), + &error_desc)) { + error = g_error_new_literal (NM_MANAGER_ERROR, + NM_MANAGER_ERROR_PERMISSION_DENIED, + error_desc); + g_free (error_desc); + goto done; + } + /* Validate the request */ - chain = nm_auth_chain_new_context (context, device_auth_done_cb, self); + chain = nm_auth_chain_new_subject (subject, context, device_auth_done_cb, self); if (!chain) { error = g_error_new_literal (NM_MANAGER_ERROR, NM_MANAGER_ERROR_PERMISSION_DENIED, "Unable to authenticate request."); - callback (device, context, error, user_data); - g_clear_error (&error); - return; + goto done; } priv->auth_chains = g_slist_append (priv->auth_chains, chain); @@ -1699,6 +1721,12 @@ device_auth_request_cb (NMDevice *device, nm_auth_chain_set_data (chain, "callback", callback, NULL); nm_auth_chain_set_data (chain, "user-data", user_data, NULL); nm_auth_chain_add_call (chain, permission, allow_interaction); + +done: + g_clear_object (&subject); + if (error) + callback (device, context, error, user_data); + g_clear_error (&error); } /* This should really be moved to gsystem. */ @@ -3013,6 +3041,7 @@ validate_activation_request (NMManager *self, NMDevice *device = NULL; gboolean vpn = FALSE; NMAuthSubject *subject = NULL; + char *error_desc = NULL; g_assert (connection); g_assert (out_device); @@ -3028,6 +3057,19 @@ validate_activation_request (NMManager *self, return NULL; } + /* Ensure the subject has permissions for this connection */ + if (!nm_auth_uid_in_acl (connection, + nm_session_monitor_get (), + nm_auth_subject_get_uid (subject), + &error_desc)) { + g_set_error_literal (error, + NM_MANAGER_ERROR, + NM_MANAGER_ERROR_PERMISSION_DENIED, + error_desc); + g_free (error_desc); + goto error; + } + /* Check whether it's a VPN or not */ if ( nm_connection_get_setting_vpn (connection) || nm_connection_is_type (connection, NM_SETTING_VPN_SETTING_NAME)) @@ -3499,8 +3541,10 @@ impl_manager_deactivate_connection (NMManager *self, NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); NMConnection *connection = NULL; GError *error = NULL; + NMAuthSubject *subject = NULL; GSList *iter; NMAuthChain *chain; + char *error_desc = NULL; /* Find the connection by its object path */ for (iter = priv->active_connections; iter; iter = g_slist_next (iter)) { @@ -3519,8 +3563,29 @@ impl_manager_deactivate_connection (NMManager *self, goto done; } + /* Validate the caller */ + subject = nm_auth_subject_new_from_context (context); + if (!subject) { + error = g_error_new_literal (NM_MANAGER_ERROR, + NM_MANAGER_ERROR_PERMISSION_DENIED, + "Failed to get request UID."); + goto done; + } + + /* Ensure the subject has permissions for this connection */ + if (!nm_auth_uid_in_acl (connection, + nm_session_monitor_get (), + nm_auth_subject_get_uid (subject), + &error_desc)) { + error = g_error_new_literal (NM_MANAGER_ERROR, + NM_MANAGER_ERROR_PERMISSION_DENIED, + error_desc); + g_free (error_desc); + goto done; + } + /* Validate the user request */ - chain = nm_auth_chain_new_context (context, deactivate_net_auth_done_cb, self); + chain = nm_auth_chain_new_subject (subject, context, deactivate_net_auth_done_cb, self); if (!chain) { error = g_error_new_literal (NM_MANAGER_ERROR, NM_MANAGER_ERROR_PERMISSION_DENIED, @@ -3533,6 +3598,7 @@ impl_manager_deactivate_connection (NMManager *self, nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_NETWORK_CONTROL, TRUE); done: + g_clear_object (&subject); if (error) dbus_g_method_return_error (context, error); g_clear_error (&error);