system-settings: add helper for updating plugin connections

nm_connection_replace_settings() replaces the connection's settings
but doesn't allow interception of the new settings.  Plugins would then
send out the update signal, but secrets are scrubbed out of them to
ensure secrets aren't leaked out into D-Bus signals.

With NM 0.8 the system settings service was integrated into NM and
thus nm_connection_clear_secrets() acts directly on the system
settings plugins' NMConnection objects.  So when NM cleared secrets
(for example after determining that they might be bad in a device's
stage2 handler), we completely lost the secrets forever.

Adding this function allows the system settings service to hook into
the connection updates when the plugin connection's backing storage
(like config files or whatever) changes and cache the secrets for
use in NMSettingsConnectionInterface get_secrets() requestes.

6 files changed, 67 insertions, 60 deletions

diff --git a/src/system-settings/nm-sysconfig-connection.c b/src/system-settings/nm-sysconfig-connection.c
index 7401da5571..167c86d26f 100644
--- a/src/system-settings/nm-sysconfig-connection.c
+++ b/src/system-settings/nm-sysconfig-connection.c
@@ -47,6 +47,41 @@ typedef struct {
 
 /**************************************************************/
 
+static void
+ignore_cb (NMSettingsConnectionInterface *connection,
+           GError *error,
+           gpointer user_data)
+{
+}
+
+gboolean
+nm_sysconfig_connection_update (NMSysconfigConnection *self,
+                                NMConnection *new,
+                                GError **error)
+{
+	GHashTable *new_settings;
+	gboolean success = FALSE;
+
+	/* Do nothing if there's nothing to update */
+	if (nm_connection_compare (NM_CONNECTION (self),
+	                           NM_CONNECTION (new),
+	                           NM_SETTING_COMPARE_FLAG_EXACT))
+		return TRUE;
+
+	new_settings = nm_connection_to_hash (new);
+	g_assert (new_settings);
+	if (nm_connection_replace_settings (NM_CONNECTION (self), new_settings, error)) {
+		nm_settings_connection_interface_update (NM_SETTINGS_CONNECTION_INTERFACE (self),
+		                                         ignore_cb,
+		                                         NULL);
+		success = TRUE;
+	}
+	g_hash_table_destroy (new_settings);
+	return success;
+}
+
+/**************************************************************/
+
 static GValue *
 string_to_gvalue (const char *str)
 {
diff --git a/src/system-settings/nm-sysconfig-connection.h b/src/system-settings/nm-sysconfig-connection.h
index 3e67f52473..3eeb1e5519 100644
--- a/src/system-settings/nm-sysconfig-connection.h
+++ b/src/system-settings/nm-sysconfig-connection.h
@@ -43,6 +43,13 @@ typedef struct {
 
 GType nm_sysconfig_connection_get_type (void);
 
+/* Called by a system-settings plugin to update a connection when the
+ * connection's backing storage has changed.
+ */
+gboolean nm_sysconfig_connection_update (NMSysconfigConnection *self,
+                                         NMConnection *new_settings,
+                                         GError **error);
+
 G_END_DECLS
 
 #endif /* NM_SYSCONFIG_CONNECTION_H */
diff --git a/src/system-settings/nm-sysconfig-settings.c b/src/system-settings/nm-sysconfig-settings.c
index ad95463c9c..104eb008ab 100644
--- a/src/system-settings/nm-sysconfig-settings.c
+++ b/src/system-settings/nm-sysconfig-settings.c
@@ -60,6 +60,7 @@ EXPORT(nm_inotify_helper_add_watch)
 EXPORT(nm_inotify_helper_remove_watch)
 
 EXPORT(nm_sysconfig_connection_get_type)
+EXPORT(nm_sysconfig_connection_update)
 /* END LINKER CRACKROCK */
 
 static void claim_connection (NMSysconfigSettings *self,
diff --git a/system-settings/plugins/ifcfg-rh/plugin.c b/system-settings/plugins/ifcfg-rh/plugin.c
index deaffff038..a5543ee908 100644
--- a/system-settings/plugins/ifcfg-rh/plugin.c
+++ b/system-settings/plugins/ifcfg-rh/plugin.c
@@ -222,13 +222,6 @@ read_connections (SCPluginIfcfg *plugin)
 /* Monitoring */
 
 static void
-ignore_cb (NMSettingsConnectionInterface *connection,
-           GError *error,
-           gpointer user_data)
-{
-}
-
-static void
 connection_changed_handler (SCPluginIfcfg *plugin,
                             const char *path,
                             NMIfcfgConnection *connection,
@@ -237,7 +230,6 @@ connection_changed_handler (SCPluginIfcfg *plugin,
 {
 	NMIfcfgConnection *new;
 	GError *error = NULL;
-	GHashTable *settings;
 	gboolean ignore_error = FALSE;
 	const char *new_unmanaged = NULL, *old_unmanaged = NULL;
 
@@ -289,20 +281,12 @@ connection_changed_handler (SCPluginIfcfg *plugin,
 			g_signal_emit_by_name (plugin, NM_SYSTEM_CONFIG_INTERFACE_CONNECTION_ADDED, connection);
 		}
 
-		/* Only update if different */
-		if (!nm_connection_compare (NM_CONNECTION (new),
-		                            NM_CONNECTION (connection),
-		                            NM_SETTING_COMPARE_FLAG_EXACT)) {
-			settings = nm_connection_to_hash (NM_CONNECTION (new));
-			if (!nm_connection_replace_settings (NM_CONNECTION (connection), settings, &error)) {
-				PLUGIN_WARN (IFCFG_PLUGIN_NAME, "    error updating: %s",
-				             (error && error->message) ? error->message : "(unknown)");
-				g_clear_error (&error);
-			}
-			g_hash_table_destroy (settings);
-			nm_settings_connection_interface_update (NM_SETTINGS_CONNECTION_INTERFACE (connection),
-			                                         ignore_cb,
-			                                         NULL);
+		if (!nm_sysconfig_connection_update (NM_SYSCONFIG_CONNECTION (connection),
+		                                     NM_CONNECTION (new),
+		                                     &error)) {
+			PLUGIN_WARN (IFCFG_PLUGIN_NAME, "    error updating: %s",
+			             (error && error->message) ? error->message : "(unknown)");
+			g_clear_error (&error);
 		}
 
 		/* Update unmanaged status */
diff --git a/system-settings/plugins/ifcfg-suse/nm-suse-connection.c b/system-settings/plugins/ifcfg-suse/nm-suse-connection.c
index b114b74ed5..72f5d683af 100644
--- a/system-settings/plugins/ifcfg-suse/nm-suse-connection.c
+++ b/system-settings/plugins/ifcfg-suse/nm-suse-connection.c
@@ -42,13 +42,6 @@ typedef struct {
 } NMSuseConnectionPrivate;
 
 static void
-ignore_cb (NMSettingsConnectionInterface *connection,
-           GError *error,
-           gpointer user_data)
-{
-}
-
-static void
 file_changed (GFileMonitor *monitor,
               GFile *file,
               GFile *other_file,
@@ -58,30 +51,21 @@ file_changed (GFileMonitor *monitor,
 	NMSuseConnection *self = NM_SUSE_CONNECTION (user_data);
 	NMSuseConnectionPrivate *priv = NM_SUSE_CONNECTION_GET_PRIVATE (self);
 	NMConnection *new;
+	GError *error = NULL;
 
 	switch (event_type) {
 	case G_FILE_MONITOR_EVENT_CHANGES_DONE_HINT:
 		new = parse_ifcfg (priv->iface, priv->dev_type);
 		if (new) {
-			GError *error = NULL;
-			GHashTable *settings;
-
-			if (!nm_connection_compare (new,
-			                            NM_CONNECTION (self),
-			                            NM_SETTING_COMPARE_FLAG_EXACT)) {
-				settings = nm_connection_to_hash (new);
-				if (!nm_connection_replace_settings (NM_CONNECTION (self), settings, &error)) {
-					g_warning ("%s: '%s' / '%s' invalid: %d",
-					           __func__,
-					           error ? g_type_name (nm_connection_lookup_setting_type_by_quark (error->domain)) : "(none)",
-					           (error && error->message) ? error->message : "(none)",
-					           error ? error->code : -1);
-					g_clear_error (&error);
-				}
-				g_hash_table_destroy (settings);
-				nm_settings_connection_interface_update (NM_SETTINGS_CONNECTION_INTERFACE (self),
-				                                         ignore_cb,
-				                                         NULL);
+			if (!nm_sysconfig_connection_update (NM_SYSCONFIG_CONNECTION (self),
+			                                     NM_CONNECTION (new),
+			                                     &error)) {
+				g_warning ("%s: '%s' / '%s' invalid: %d",
+				           __func__,
+				           error ? g_type_name (nm_connection_lookup_setting_type_by_quark (error->domain)) : "(none)",
+				           (error && error->message) ? error->message : "(none)",
+				           error ? error->code : -1);
+				g_clear_error (&error);
 			}
 			g_object_unref (new);
 		} else
diff --git a/system-settings/plugins/keyfile/plugin.c b/system-settings/plugins/keyfile/plugin.c
index bc2c447379..9b82a8b05a 100644
--- a/system-settings/plugins/keyfile/plugin.c
+++ b/system-settings/plugins/keyfile/plugin.c
@@ -123,26 +123,23 @@ find_by_uuid (gpointer key, gpointer data, gpointer user_data)
 }
 
 static void
-update_connection_settings (NMConnection *orig,
-                            NMConnection *new)
+update_connection_settings (NMKeyfileConnection *orig,
+                            NMKeyfileConnection *new)
 {
-	GHashTable *new_settings;
 	GError *error = NULL;
 
-	new_settings = nm_connection_to_hash (new);
-	if (nm_connection_replace_settings (orig, new_settings, &error))
-		nm_settings_connection_interface_emit_updated (NM_SETTINGS_CONNECTION_INTERFACE (orig));
-	else {
+	if (!nm_sysconfig_connection_update (NM_SYSCONFIG_CONNECTION (orig),
+	                                     NM_CONNECTION (new),
+	                                     &error)) {
 		g_warning ("%s: '%s' / '%s' invalid: %d",
 		           __func__,
 		           error ? g_type_name (nm_connection_lookup_setting_type_by_quark (error->domain)) : "(none)",
 		           (error && error->message) ? error->message : "(none)",
 		           error ? error->code : -1);
 		g_clear_error (&error);
+
 		g_signal_emit_by_name (orig, "removed");
 	}
-
-	g_hash_table_destroy (new_settings);
 }
 
 /* Monitoring */
@@ -180,7 +177,7 @@ dir_changed (GFileMonitor *monitor,
 
 			tmp = (NMKeyfileConnection *) nm_keyfile_connection_new (name);
 			if (tmp) {
-				update_connection_settings (NM_CONNECTION (connection), NM_CONNECTION (tmp));
+				update_connection_settings (connection, tmp);
 				g_object_unref (tmp);
 			}
 		} else {
@@ -219,8 +216,7 @@ dir_changed (GFileMonitor *monitor,
 					/* Updating settings should update the NMKeyfileConnection's
 					 * filename property too.
 					 */
-					update_connection_settings (NM_CONNECTION (found),
-					                            NM_CONNECTION (connection));
+					update_connection_settings (found, connection);
 
 					/* Re-insert the connection back into the hash with the new filename */
 					g_hash_table_insert (priv->hash,