core: check valid uid for D-Bus load_connection(s)/set_logging calls

Commit 4c7fa8dfdcbf ("core: drop root requirement for
load_connection(s)/set_logging D-Bus calls") removed the enforcing of
permission in the daemon for such methods since the D-Bus daemon
configuration already does that. That change also allows clients to
send a request and not wait for a response, since we don't have to
check the caller credentials in the daemon.

In the future we might switch to polkit for these methods, breaking
clients that don't wait for a reponse, so it seems better to prevent
from beginning such behavior.

Fixes: 4c7fa8dfdcbf13f3633b565af53896ac79366912

4 files changed, 61 insertions, 9 deletions

diff --git a/src/nm-bus-manager.c b/src/nm-bus-manager.c
index 5894d7570d..449de4e68a 100644
--- a/src/nm-bus-manager.c
+++ b/src/nm-bus-manager.c
@@ -533,11 +533,28 @@ nm_bus_manager_get_caller_info_from_message (NMBusManager *self,
 	return _get_caller_info (self, NULL, connection, message, out_sender, out_uid, out_pid);
 }
 
+/**
+ * nm_bus_manager_ensure_uid:
+ *
+ * @self: bus manager instance
+ * @context: D-Bus method invocation
+ * @uid: a user-id
+ * @error_domain: error domain to return on failure
+ * @error_code: error code to return on failure
+ *
+ * Retrieves the uid of the D-Bus method caller and
+ * checks that it matches @uid, unless @uid is G_MAXULONG.
+ * In case of failure the function returns FALSE and finishes
+ * handling the D-Bus method with an error.
+ *
+ * Returns: %TRUE if the check succeeded, %FALSE otherwise
+ */
 gboolean
-nm_bus_manager_ensure_root (NMBusManager          *self,
-                            GDBusMethodInvocation *context,
-                            GQuark error_domain,
-                            int error_code)
+nm_bus_manager_ensure_uid (NMBusManager          *self,
+                           GDBusMethodInvocation *context,
+                           gulong uid,
+                           GQuark error_domain,
+                           int error_code)
 {
 	gulong caller_uid;
 	GError *error = NULL;
@@ -552,7 +569,8 @@ nm_bus_manager_ensure_root (NMBusManager          *self,
 		g_dbus_method_invocation_take_error (context, error);
 		return FALSE;
 	}
-	if (caller_uid != 0) {
+
+	if (uid != G_MAXULONG && caller_uid != uid) {
 		error = g_error_new_literal (error_domain,
 		                             error_code,
 		                             "Permission denied");
diff --git a/src/nm-bus-manager.h b/src/nm-bus-manager.h
index f49901db2c..bc23d2ce38 100644
--- a/src/nm-bus-manager.h
+++ b/src/nm-bus-manager.h
@@ -66,10 +66,11 @@ gboolean nm_bus_manager_get_caller_info (NMBusManager *self,
                                          gulong *out_uid,
                                          gulong *out_pid);
 
-gboolean nm_bus_manager_ensure_root (NMBusManager          *self,
-                                     GDBusMethodInvocation *context,
-                                     GQuark error_domain,
-                                     int error_code);
+gboolean nm_bus_manager_ensure_uid (NMBusManager          *self,
+                                    GDBusMethodInvocation *context,
+                                    gulong uid,
+                                    GQuark error_domain,
+                                    int error_code);
 
 const char *nm_bus_manager_connection_get_private_name (NMBusManager *self,
                                                         GDBusConnection *connection);
diff --git a/src/nm-manager.c b/src/nm-manager.c
index 237a45537d..667c08c2c2 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -4476,6 +4476,17 @@ impl_manager_set_logging (NMManager *self,
 {
 	GError *error = NULL;
 
+	/* The permission is already enforced by the D-Bus daemon, but we ensure
+	 * that the caller is still alive so that clients are forced to wait and
+	 * we'll be able to switch to polkit without breaking behavior.
+	 */
+	if (!nm_bus_manager_ensure_uid (nm_bus_manager_get (),
+	                                context,
+	                                G_MAXULONG,
+	                                NM_MANAGER_ERROR,
+	                                NM_MANAGER_ERROR_PERMISSION_DENIED))
+		return;
+
 	if (nm_logging_setup (level, domains, NULL, &error)) {
 		_LOGI (LOGD_CORE, "logging: level '%s' domains '%s'",
 		       nm_logging_level_to_string (), nm_logging_domains_to_string ());
diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c
index 59a4aa07e6..1f691c0bd5 100644
--- a/src/settings/nm-settings.c
+++ b/src/settings/nm-settings.c
@@ -1522,6 +1522,17 @@ impl_settings_load_connections (NMSettings *self,
 	GSList *iter;
 	int i;
 
+	/* The permission is already enforced by the D-Bus daemon, but we ensure
+	 * that the caller is still alive so that clients are forced to wait and
+	 * we'll be able to switch to polkit without breaking behavior.
+	 */
+	if (!nm_bus_manager_ensure_uid (nm_bus_manager_get (),
+	                                context,
+	                                G_MAXULONG,
+	                                NM_SETTINGS_ERROR,
+	                                NM_SETTINGS_ERROR_PERMISSION_DENIED))
+		return;
+
 	failures = g_ptr_array_new ();
 
 	for (i = 0; filenames[i]; i++) {
@@ -1555,6 +1566,17 @@ impl_settings_reload_connections (NMSettings *self,
 	NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self);
 	GSList *iter;
 
+	/* The permission is already enforced by the D-Bus daemon, but we ensure
+	 * that the caller is still alive so that clients are forced to wait and
+	 * we'll be able to switch to polkit without breaking behavior.
+	 */
+	if (!nm_bus_manager_ensure_uid (nm_bus_manager_get (),
+	                                context,
+	                                G_MAXULONG,
+	                                NM_SETTINGS_ERROR,
+	                                NM_SETTINGS_ERROR_PERMISSION_DENIED))
+		return;
+
 	for (iter = priv->plugins; iter; iter = g_slist_next (iter)) {
 		NMSettingsPlugin *plugin = NM_SETTINGS_PLUGIN (iter->data);