From 549b126a5c964f9e3b3e9e12a4b66c139cb46044 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 24 Sep 2020 18:34:21 +0200 Subject: [PATCH] device: allow non-privileged users to call device.GetAppliedConnection() Compare to the connection's GetSettings() call, which is not protected by policykit permissions. It only checks that the requesting user is allowed according to "connection.permission". Previously, device's GetAppliedConnection() requires "network-control" permissions. This although it only reads a profile, without modifying anything. That seems unnecessary, also because in the common case the applied connection is identical to the current settings connection, and the latter can be read without special permissions. Don't require a special policykit permission to read the applied connection. https://bugzilla.redhat.com/show_bug.cgi?id=1882380 --- NEWS | 2 + src/devices/nm-device.c | 92 +++++++++++------------------------------ 2 files changed, 27 insertions(+), 67 deletions(-) diff --git a/NEWS b/NEWS index 4b00eeba53..e0eca35c4d 100644 --- a/NEWS +++ b/NEWS @@ -17,6 +17,8 @@ USE AT YOUR OWN RISK. NOT RECOMMENDED FOR PRODUCTION USE! was built with the respective options enabled. * The long deprecated D-Bus property "Ip4Address" on "org.freedesktop.NetworkManager.Device" interface is not defunct and always returns zero. +* dbus: don't require policykit permission for GetAppliedConnection of + a device. ============================================= NetworkManager-1.26 diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 04331bd3a3..883dee3b8d 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -12876,62 +12876,6 @@ impl_device_reapply(NMDBusObject * obj, /*****************************************************************************/ -static void -get_applied_connection_cb(NMDevice * self, - GDBusMethodInvocation *context, - NMAuthSubject * subject, - GError * error, - gpointer user_data /* possibly dangling pointer */) -{ - NMDevicePrivate *priv; - NMConnection * applied_connection; - GVariant * settings; - - g_return_if_fail(NM_IS_DEVICE(self)); - - if (error) { - g_dbus_method_invocation_return_gerror(context, error); - return; - } - - priv = NM_DEVICE_GET_PRIVATE(self); - - applied_connection = nm_device_get_applied_connection(self); - - if (!applied_connection) { - error = g_error_new_literal(NM_DEVICE_ERROR, - NM_DEVICE_ERROR_NOT_ACTIVE, - "Device is not activated"); - g_dbus_method_invocation_take_error(context, error); - return; - } - - if (applied_connection != user_data) { - /* The applied connection changed due to a race. Reauthenticate. */ - nm_device_auth_request( - self, - context, - applied_connection, - NM_AUTH_PERMISSION_NETWORK_CONTROL, - TRUE, - NULL, - get_applied_connection_cb, - applied_connection /* no need take a ref. We will not dereference this pointer. */); - return; - } - - settings = nm_connection_to_dbus(applied_connection, NM_CONNECTION_SERIALIZE_NO_SECRETS); - if (!settings) - settings = g_variant_new_array(G_VARIANT_TYPE("{sa{sv}}"), NULL, 0); - - g_dbus_method_invocation_return_value( - context, - g_variant_new( - "(@a{sa{sv}}t)", - settings, - nm_active_connection_version_id_get((NMActiveConnection *) priv->act_request.obj))); -} - static void impl_device_get_applied_connection(NMDBusObject * obj, const NMDBusInterfaceInfoExtended *interface_info, @@ -12941,9 +12885,12 @@ impl_device_get_applied_connection(NMDBusObject * obj, GDBusMethodInvocation * invocation, GVariant * parameters) { - NMDevice * self = NM_DEVICE(obj); - NMConnection *applied_connection; - guint32 flags; + NMDevice * self = NM_DEVICE(obj); + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); + gs_free_error GError *error = NULL; + NMConnection * applied_connection; + guint32 flags; + GVariant * var_settings; g_variant_get(parameters, "(u)", &flags); @@ -12965,15 +12912,26 @@ impl_device_get_applied_connection(NMDBusObject * obj, return; } - nm_device_auth_request( - self, + if (!nm_auth_is_invocation_in_acl_set_error(applied_connection, + invocation, + NM_MANAGER_ERROR, + NM_MANAGER_ERROR_PERMISSION_DENIED, + NULL, + &error)) { + g_dbus_method_invocation_take_error(invocation, g_steal_pointer(&error)); + return; + } + + var_settings = nm_connection_to_dbus(applied_connection, NM_CONNECTION_SERIALIZE_NO_SECRETS); + if (!var_settings) + var_settings = g_variant_new_array(G_VARIANT_TYPE("{sa{sv}}"), NULL, 0); + + g_dbus_method_invocation_return_value( invocation, - applied_connection, - NM_AUTH_PERMISSION_NETWORK_CONTROL, - TRUE, - NULL, - get_applied_connection_cb, - applied_connection /* no need take a ref. We will not dereference this pointer. */); + g_variant_new( + "(@a{sa{sv}}t)", + var_settings, + nm_active_connection_version_id_get((NMActiveConnection *) priv->act_request.obj))); } /*****************************************************************************/