libnm-util: add 'flags' argument to nm_connection_to_hash() and nm_setting_to_hash()

Simplifies code internally, and makes it easier for clients as well in
some cases where they want to control what ends up in the resulting
hash and what does not.

examples/C/add-connection-glib.c

@@ -74,7 +74,7 @@ add_connection (DBusGProxy *proxy, const char *con_name)
 	              NULL);
 	nm_connection_add_setting (connection, NM_SETTING (s_ip4));
 
-	hash = nm_connection_to_hash (connection);
+	hash = nm_connection_to_hash (connection, NM_SETTING_HASH_FLAG_ALL);
 
 	/* Call AddConnection with the hash as argument */
 	dbus_g_proxy_call (proxy, "AddConnection", &error,

libnm-glib/nm-client.c

@@ -1200,7 +1200,7 @@ nm_client_add_and_activate_connection (NMClient *client,
 	info->client = client;
 
 	if (partial)
-		hash = nm_connection_to_hash (partial);
+		hash = nm_connection_to_hash (partial, NM_SETTING_HASH_FLAG_ALL);
 	else
 		hash = g_hash_table_new (g_str_hash, g_str_equal);
 	org_freedesktop_NetworkManager_add_and_activate_connection_async (NM_CLIENT_GET_PRIVATE (client)->client_proxy,

libnm-glib/nm-remote-connection.c

@@ -125,7 +125,7 @@ nm_remote_connection_commit_changes (NMRemoteConnection *self,
 	call->callback = (GFunc) callback;
 	call->user_data = user_data;
 
-	settings = nm_connection_to_hash (NM_CONNECTION (self));
+	settings = nm_connection_to_hash (NM_CONNECTION (self), NM_SETTING_HASH_FLAG_ALL);
 
 	call->call = org_freedesktop_NetworkManager_Settings_Connection_update_async (priv->proxy,
 	                                                                              settings,

libnm-glib/nm-remote-settings.c

@@ -444,7 +444,7 @@ nm_remote_settings_add_connection (NMRemoteSettings *settings,
 	info->callback = callback;
 	info->callback_data = user_data;
 
-	new_settings = nm_connection_to_hash (connection);
+	new_settings = nm_connection_to_hash (connection, NM_SETTING_HASH_FLAG_ALL);
 	org_freedesktop_NetworkManager_Settings_add_connection_async (priv->proxy,
 	                                                              new_settings,
 	                                                              add_connection_done,

libnm-util/nm-connection.c

@@ -798,26 +798,10 @@ nm_connection_clear_secrets (NMConnection *connection)
 	g_hash_table_foreach (priv->settings, clear_setting_secrets, NULL);
 }
 
-static void
-add_one_setting_to_hash (gpointer key, gpointer data, gpointer user_data)
-{
-	NMSetting *setting = (NMSetting *) data;
-	GHashTable *connection_hash = (GHashTable *) user_data;
-	GHashTable *setting_hash;
-
-	g_return_if_fail (setting != NULL);
-	g_return_if_fail (connection_hash != NULL);
-
-	setting_hash = nm_setting_to_hash (setting);
-	if (setting_hash)
-		g_hash_table_insert (connection_hash,
-							 g_strdup (nm_setting_get_name (setting)),
-							 setting_hash);
-}
-
 /**
  * nm_connection_to_hash:
  * @connection: the #NMConnection
+ * @flags: hash flags, e.g. %NM_SETTING_HASH_FLAG_ALL
  *
  * Converts the #NMConnection into a #GHashTable describing the connection,
  * suitable for marshalling over D-Bus or serializing.  The hash table mapping
@@ -832,26 +816,39 @@ add_one_setting_to_hash (gpointer key, gpointer data, gpointer user_data)
  * with g_hash_table_unref() when it is no longer needed.
  **/
 GHashTable *
-nm_connection_to_hash (NMConnection *connection)
+nm_connection_to_hash (NMConnection *connection, NMSettingHashFlags flags)
 {
 	NMConnectionPrivate *priv;
-	GHashTable *connection_hash;
+	GHashTableIter iter;
+	gpointer key, data;
+	GHashTable *ret, *setting_hash;
 
+	g_return_val_if_fail (connection != NULL, NULL);
 	g_return_val_if_fail (NM_IS_CONNECTION (connection), NULL);
 
-	connection_hash = g_hash_table_new_full (g_str_hash, g_str_equal,
+	ret = g_hash_table_new_full (g_str_hash, g_str_equal,
-									 g_free, (GDestroyNotify) g_hash_table_destroy);
+	                             g_free, (GDestroyNotify) g_hash_table_destroy);
 
 	priv = NM_CONNECTION_GET_PRIVATE (connection);
-	g_hash_table_foreach (priv->settings, add_one_setting_to_hash, connection_hash);
 
-	/* Don't send empty hashes */
+	/* Add each setting's hash to the main hash */
-	if (g_hash_table_size (connection_hash) < 1) {
+	g_hash_table_iter_init (&iter, priv->settings);
-		g_hash_table_destroy (connection_hash);
+	while (g_hash_table_iter_next (&iter, &key, &data)) {
-		connection_hash = NULL;
+		const char *setting_name = key;
+		NMSetting *setting = NM_SETTING (data);
+
+		setting_hash = nm_setting_to_hash (setting, flags);
+		if (setting_hash)
+			g_hash_table_insert (ret, g_strdup (setting_name), setting_hash);
 	}
 
-	return connection_hash;
+	/* Don't send empty hashes */
+	if (g_hash_table_size (ret) < 1) {
+		g_hash_table_destroy (ret);
+		ret = NULL;
+	}
+
+	return ret;
 }
 
 typedef struct ForEachValueInfo {

libnm-util/nm-connection.h

@@ -130,7 +130,9 @@ void          nm_connection_for_each_setting_value (NMConnection *connection,
 										  NMSettingValueIterFn func,
 										  gpointer user_data);
 
-GHashTable   *nm_connection_to_hash       (NMConnection *connection);
+GHashTable   *nm_connection_to_hash       (NMConnection *connection,
+                                           NMSettingHashFlags flags);
+
 void          nm_connection_dump          (NMConnection *connection);
 
 NMSetting    *nm_connection_create_setting (const char *name);

libnm-util/nm-setting.c

@@ -107,6 +107,7 @@ destroy_gvalue (gpointer data)
 /**
  * nm_setting_to_hash:
  * @setting: the #NMSetting
+ * @flags: hash flags, e.g. %NM_SETTING_HASH_FLAG_ALL
  *
  * Converts the #NMSetting into a #GHashTable mapping each setting property
  * name to a GValue describing that property, suitable for marshalling over
@@ -115,13 +116,14 @@ destroy_gvalue (gpointer data)
  * Returns: (transfer full) (element-type utf8 GObject.Value): a new #GHashTable describing the setting's properties
  **/
 GHashTable *
-nm_setting_to_hash (NMSetting *setting)
+nm_setting_to_hash (NMSetting *setting, NMSettingHashFlags flags)
 {
 	GHashTable *hash;
 	GParamSpec **property_specs;
 	guint n_property_specs;
 	guint i;
 
+	g_return_val_if_fail (setting != NULL, NULL);
 	g_return_val_if_fail (NM_IS_SETTING (setting), NULL);
 
 	property_specs = g_object_class_list_properties (G_OBJECT_GET_CLASS (setting), &n_property_specs);
@@ -132,29 +134,35 @@ nm_setting_to_hash (NMSetting *setting)
 	}
 
 	hash = g_hash_table_new_full (g_str_hash, g_str_equal,
-							(GDestroyNotify) g_free,
+	                              (GDestroyNotify) g_free, destroy_gvalue);
-							destroy_gvalue);
 
 	for (i = 0; i < n_property_specs; i++) {
 		GParamSpec *prop_spec = property_specs[i];
+		GValue *value;
 
-		if (prop_spec->flags & NM_SETTING_PARAM_SERIALIZE) {
+		if (!(prop_spec->flags & NM_SETTING_PARAM_SERIALIZE))
-			GValue *value;
+			continue;
 
-			value = g_slice_new0 (GValue);
+		if (   (flags & NM_SETTING_HASH_FLAG_NO_SECRETS)
-			g_value_init (value, prop_spec->value_type);
+		    && (prop_spec->flags & NM_SETTING_PARAM_SECRET))
-			g_object_get_property (G_OBJECT (setting), prop_spec->name, value);
+			continue;
 
-			/* Don't serialize values with default values */
+		if (   (flags & NM_SETTING_HASH_FLAG_ONLY_SECRETS)
-			if (!g_param_value_defaults (prop_spec, value))
+		    && !(prop_spec->flags & NM_SETTING_PARAM_SECRET))
-				g_hash_table_insert (hash, g_strdup (prop_spec->name), value);
+			continue;
-			else
+
-				destroy_gvalue (value);
+		value = g_slice_new0 (GValue);
-		}
+		g_value_init (value, prop_spec->value_type);
+		g_object_get_property (G_OBJECT (setting), prop_spec->name, value);
+
+		/* Don't serialize values with default values */
+		if (!g_param_value_defaults (prop_spec, value))
+			g_hash_table_insert (hash, g_strdup (prop_spec->name), value);
+		else
+			destroy_gvalue (value);
 	}
 
 	g_free (property_specs);
-
 	return hash;
 }
 

libnm-util/nm-setting.h

@@ -124,7 +124,25 @@ typedef void (*NMSettingValueIterFn) (NMSetting *setting,
 
 GType nm_setting_get_type (void);
 
-GHashTable *nm_setting_to_hash       (NMSetting *setting);
+/**
+ * NMSettingHashFlags:
+ * @NM_SETTING_HASH_FLAG_ALL: hash all properties (including secrets)
+ * @NM_SETTING_HASH_FLAG_NO_SECRETS: do not include secrets
+ * @NM_SETTING_HASH_FLAG_ONLY_SECRETS: only hash secrets
+ *
+ * These flags determine which properties are added to the resulting hash
+ * when calling nm_setting_to_hash().
+ *
+ **/
+typedef enum {
+	NM_SETTING_HASH_FLAG_ALL = 0x00000000,
+	NM_SETTING_HASH_FLAG_NO_SECRETS = 0x00000001,
+	NM_SETTING_HASH_FLAG_ONLY_SECRETS = 0x00000002,
+} NMSettingHashFlags;
+
+GHashTable *nm_setting_to_hash       (NMSetting *setting,
+                                      NMSettingHashFlags flags);
+
 NMSetting  *nm_setting_new_from_hash (GType setting_type,
                                       GHashTable *hash);
 

libnm-util/tests/test-general.c

@@ -29,6 +29,7 @@
 #include "nm-setting-connection.h"
 #include "nm-setting-vpn.h"
 #include "nm-setting-gsm.h"
+#include "nm-setting-wireless-security.h"
 #include "nm-setting-ip6-config.h"
 #include "nm-dbus-glib-types.h"
 
@@ -289,6 +290,100 @@ test_setting_gsm_apn_bad_chars (void)
 	        "gsm-apn-bad-chars", "unexpectedly valid GSM setting");
 }
 
+static NMSettingWirelessSecurity *
+make_test_wsec_setting (const char *detail)
+{
+	NMSettingWirelessSecurity *s_wsec;
+
+	s_wsec = (NMSettingWirelessSecurity *) nm_setting_wireless_security_new ();
+	ASSERT (s_wsec != NULL, detail, "error creating setting");
+
+	g_object_set (s_wsec,
+	              NM_SETTING_WIRELESS_SECURITY_KEY_MGMT, "wpa-psk",
+	              NM_SETTING_WIRELESS_SECURITY_LEAP_USERNAME, "foobarbaz",
+	              NM_SETTING_WIRELESS_SECURITY_PSK, "random psk",
+	              NM_SETTING_WIRELESS_SECURITY_WEP_KEY0, "aaaaaaaaaa",
+	              NULL);
+
+	return s_wsec;
+}
+
+static void
+test_setting_to_hash_all (void)
+{
+	NMSettingWirelessSecurity *s_wsec;
+	GHashTable *hash;
+
+	s_wsec = make_test_wsec_setting ("setting-to-hash-all");
+
+	hash = nm_setting_to_hash (NM_SETTING (s_wsec), NM_SETTING_HASH_FLAG_ALL);
+
+	/* Make sure all keys are there */
+	ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT),
+	        "setting-to-hash-all", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_KEY_MGMT);
+	ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_LEAP_USERNAME),
+	        "setting-to-hash-all", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_LEAP_USERNAME);
+	ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_PSK),
+	        "setting-to-hash-all", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_PSK);
+	ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_WEP_KEY0),
+	        "setting-to-hash-all", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_WEP_KEY0);
+
+	g_hash_table_destroy (hash);
+	g_object_unref (s_wsec);
+}
+
+static void
+test_setting_to_hash_no_secrets (void)
+{
+	NMSettingWirelessSecurity *s_wsec;
+	GHashTable *hash;
+
+	s_wsec = make_test_wsec_setting ("setting-to-hash-no-secrets");
+
+	hash = nm_setting_to_hash (NM_SETTING (s_wsec), NM_SETTING_HASH_FLAG_NO_SECRETS);
+
+	/* Make sure non-secret keys are there */
+	ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT),
+	        "setting-to-hash-no-secrets", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_KEY_MGMT);
+	ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_LEAP_USERNAME),
+	        "setting-to-hash-no-secrets", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_LEAP_USERNAME);
+
+	/* Make sure secrets are not there */
+	ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_PSK) == NULL,
+	        "setting-to-hash-no-secrets", "unexpectedly present " NM_SETTING_WIRELESS_SECURITY_PSK);
+	ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_WEP_KEY0) == NULL,
+	        "setting-to-hash-no-secrets", "unexpectedly present " NM_SETTING_WIRELESS_SECURITY_WEP_KEY0);
+
+	g_hash_table_destroy (hash);
+	g_object_unref (s_wsec);
+}
+
+static void
+test_setting_to_hash_only_secrets (void)
+{
+	NMSettingWirelessSecurity *s_wsec;
+	GHashTable *hash;
+
+	s_wsec = make_test_wsec_setting ("setting-to-hash-only-secrets");
+
+	hash = nm_setting_to_hash (NM_SETTING (s_wsec), NM_SETTING_HASH_FLAG_ONLY_SECRETS);
+
+	/* Make sure non-secret keys are there */
+	ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT) == NULL,
+	        "setting-to-hash-only-secrets", "unexpectedly present " NM_SETTING_WIRELESS_SECURITY_KEY_MGMT);
+	ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_LEAP_USERNAME) == NULL,
+	        "setting-to-hash-only-secrets", "unexpectedly present " NM_SETTING_WIRELESS_SECURITY_LEAP_USERNAME);
+
+	/* Make sure secrets are not there */
+	ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_PSK),
+	        "setting-to-hash-only-secrets", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_PSK);
+	ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_WEP_KEY0),
+	        "setting-to-hash-only-secrets", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_WEP_KEY0);
+
+	g_hash_table_destroy (hash);
+	g_object_unref (s_wsec);
+}
+
 int main (int argc, char **argv)
 {
 	GError *error = NULL;
@@ -306,6 +401,9 @@ int main (int argc, char **argv)
 	test_setting_ip6_config_old_address_array ();
 	test_setting_gsm_apn_spaces ();
 	test_setting_gsm_apn_bad_chars ();
+	test_setting_to_hash_all ();
+	test_setting_to_hash_no_secrets ();
+	test_setting_to_hash_only_secrets ();
 
 	base = g_path_get_basename (argv[0]);
 	fprintf (stdout, "%s: SUCCESS\n", base);

src/NetworkManagerUtils.c

@@ -405,7 +405,7 @@ nm_utils_call_dispatcher (const char *action,
 	}
 
 	if (connection) {
-		connection_hash = nm_connection_to_hash (connection);
+		connection_hash = nm_connection_to_hash (connection, NM_SETTING_HASH_FLAG_NO_SECRETS);
 
 		connection_props = value_hash_create ();
 

src/nm-secret-agent.c

@@ -187,7 +187,8 @@ nm_secret_agent_get_secrets (NMSecretAgent *self,
 
 	priv = NM_SECRET_AGENT_GET_PRIVATE (self);
 
-	hash = nm_connection_to_hash (connection);
+	/* FIXME: allow system secrets to be sent to the agent? */
+	hash = nm_connection_to_hash (connection, NM_SETTING_HASH_FLAG_ALL);
 
 	r = request_new (self, nm_connection_get_path (connection), setting_name, callback, callback_data);
 	r->call = dbus_g_proxy_begin_call_with_timeout (priv->proxy,

src/settings/nm-settings-connection.c

@@ -196,7 +196,7 @@ nm_settings_connection_replace_settings (NMSettingsConnection *self,
 
 	priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self);
 
-	new_settings = nm_connection_to_hash (new);
+	new_settings = nm_connection_to_hash (new, NM_SETTING_HASH_FLAG_ALL);
 	g_assert (new_settings);
 	if (nm_connection_replace_settings (NM_CONNECTION (self), new_settings, error)) {
 		/* Copy the connection to keep its secrets around even if NM
@@ -605,28 +605,20 @@ get_settings_auth_cb (NMSettingsConnection *self,
 	                  GError *error,
 	                  gpointer data)
 {
-	NMConnection *no_secrets;
+	if (error)
-	GHashTable *settings;
-
-	if (error) {
 		dbus_g_method_return_error (context, error);
-		return;
+	else {
+		GHashTable *settings;
+
+		/* Secrets should *never* be returned by the GetSettings method, they
+		 * get returned by the GetSecrets method which can be better
+		 * protected against leakage of secrets to unprivileged callers.
+		 */
+		settings = nm_connection_to_hash (NM_CONNECTION (self), NM_SETTING_HASH_FLAG_NO_SECRETS);
+		g_assert (settings);
+		dbus_g_method_return (context, settings);
+		g_hash_table_destroy (settings);
 	}
-
-	/* Secrets should *never* be returned by the GetSettings method, they
-	 * get returned by the GetSecrets method which can be better
-	 * protected against leakage of secrets to unprivileged callers.
-	 */
-	no_secrets = nm_connection_duplicate (NM_CONNECTION (self));
-	g_assert (no_secrets);
-	nm_connection_clear_secrets (no_secrets);
-	settings = nm_connection_to_hash (no_secrets);
-	g_assert (settings);
-
-	dbus_g_method_return (context, settings);
-	
-	g_object_unref (no_secrets);
-	g_hash_table_destroy (settings);
 }
 
 static void

src/vpn-manager/nm-vpn-connection.c

@@ -591,6 +591,7 @@ static void
 really_activate (NMVPNConnection *connection)
 {
 	NMVPNConnectionPrivate *priv;
+	GHashTable *hash;
 
 	g_return_if_fail (NM_IS_VPN_CONNECTION (connection));
 	g_return_if_fail (nm_vpn_connection_get_vpn_state (connection) == NM_VPN_CONNECTION_STATE_NEED_AUTH);
@@ -607,10 +608,12 @@ really_activate (NMVPNConnection *connection)
 						    G_CALLBACK (nm_vpn_connection_ip4_config_get),
 						    connection, NULL);
 
+	hash = nm_connection_to_hash (priv->connection, NM_SETTING_HASH_FLAG_ALL);
 	org_freedesktop_NetworkManager_VPN_Plugin_connect_async (priv->proxy,
-												  nm_connection_to_hash (priv->connection),
+	                                                         hash,
-												  nm_vpn_connection_connect_cb,
+	                                                         nm_vpn_connection_connect_cb,
-												  connection);
+	                                                         connection);
+	g_hash_table_destroy (hash);
 
 	nm_vpn_connection_set_vpn_state (connection,
 	                                 NM_VPN_CONNECTION_STATE_CONNECT,
@@ -826,7 +829,7 @@ call_need_secrets (NMVPNConnection *vpn_connection)
 	GHashTable *settings;
 
 	priv = NM_VPN_CONNECTION_GET_PRIVATE (vpn_connection);
-	settings = nm_connection_to_hash (priv->connection);
+	settings = nm_connection_to_hash (priv->connection, NM_SETTING_HASH_FLAG_ALL);
 	org_freedesktop_NetworkManager_VPN_Plugin_need_secrets_async (priv->proxy,
 	                         settings,
 	                         connection_need_secrets_cb,