settings: fix updating agent-owned VPN secretsbg/agent-secrets-issue230

property_to_dbus() returns NULL when called with
NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED and the property is
not an agent-owned secrets. The function doesn't handle VPN secrets
correctly, since they are all stored as a hash in the vpn.secrets
property and the flag for each of them is a matching '*-flags' key in
the vpn.data property. VPN secrets must be handled differently; do it
in the VPN setting to_dbus_fcn() function.

Fixes: 71928a3e5cab ('settings: avoid cloning the connection to maintain agent-owned secrets')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/230

2 files changed, 62 insertions, 6 deletions

diff --git a/libnm-core/nm-setting-vpn.c b/libnm-core/nm-setting-vpn.c
index 4554111cc1..a337d857cf 100644
--- a/libnm-core/nm-setting-vpn.c
+++ b/libnm-core/nm-setting-vpn.c
@@ -891,6 +891,54 @@ clear_secrets (const NMSettInfoSetting *sett_info,
 	return changed;
 }
 
+static gboolean
+vpn_secrets_from_dbus (NMSetting *setting,
+                       GVariant *connection_dict,
+                       const char *property,
+                       GVariant *value,
+                       NMSettingParseFlags parse_flags,
+                       GError **error)
+{
+	nm_auto_unset_gvalue GValue object_value = G_VALUE_INIT;
+
+	g_value_init (&object_value, G_TYPE_HASH_TABLE);
+	_nm_utils_strdict_from_dbus (value, &object_value);
+	return nm_g_object_set_property (G_OBJECT (setting), property, &object_value, error);
+}
+
+static GVariant *
+vpn_secrets_to_dbus (const NMSettInfoSetting *sett_info,
+                     guint property_idx,
+                     NMConnection *connection,
+                     NMSetting *setting,
+                     NMConnectionSerializationFlags flags,
+                     const NMConnectionSerializationOptions *options)
+{
+	gs_unref_hashtable GHashTable *secrets = NULL;
+	const char *property_name = sett_info->property_infos[property_idx].name;
+	GVariantBuilder builder;
+	GHashTableIter iter;
+	const char *key, *value;
+	NMSettingSecretFlags secret_flags;
+
+	g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{ss}"));
+	g_object_get (setting, property_name, &secrets, NULL);
+
+	if (secrets) {
+		g_hash_table_iter_init (&iter, secrets);
+		while (g_hash_table_iter_next (&iter, (gpointer *) &key, (gpointer *) &value)) {
+			if (NM_FLAGS_HAS (flags, NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED)) {
+				if (   !nm_setting_get_secret_flags (setting, key, &secret_flags, NULL)
+				    || !NM_FLAGS_HAS (secret_flags, NM_SETTING_SECRET_FLAG_AGENT_OWNED))
+					continue;
+			}
+			g_variant_builder_add (&builder, "{ss}", key, value);
+		}
+	}
+
+	return g_variant_builder_end (&builder);
+}
+
 /*****************************************************************************/
 
 static void
@@ -1109,11 +1157,12 @@ nm_setting_vpn_class_init (NMSettingVpnClass *klass)
 	                        NM_SETTING_PARAM_SECRET |
 	                        G_PARAM_STATIC_STRINGS);
 
-	_properties_override_add_transform (properties_override,
-	                                    obj_properties[PROP_SECRETS],
-	                                    G_VARIANT_TYPE ("a{ss}"),
-	                                    _nm_utils_strdict_to_dbus,
-	                                    _nm_utils_strdict_from_dbus);
+	_properties_override_add_override (properties_override,
+	                                   obj_properties[PROP_SECRETS],
+	                                   G_VARIANT_TYPE ("a{ss}"),
+	                                   vpn_secrets_to_dbus,
+	                                   vpn_secrets_from_dbus,
+	                                   NULL);
 
 	/**
 	 * NMSettingVpn:timeout:
diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c
index 1e853f8575..cc7131feed 100644
--- a/libnm-core/nm-setting.c
+++ b/libnm-core/nm-setting.c
@@ -698,7 +698,14 @@ property_to_dbus (const NMSettInfoSetting *sett_info,
 		if (NM_FLAGS_HAS (property->param_spec->flags, NM_SETTING_PARAM_SECRET)) {
 			if (NM_FLAGS_HAS (flags, NM_CONNECTION_SERIALIZE_NO_SECRETS))
 				return NULL;
-			if (NM_FLAGS_HAS (flags, NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED)) {
+
+			/* Check agent secrets. Secrets in the vpn.secrets property are special as
+			 * the flag for each of them is specified as a separate key in the
+			 * vpn.data property. They are handled separately in the to_dbus_fcn()
+			 * of VPN setting. */
+			if (   NM_FLAGS_HAS (flags, NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED)
+			    && !nm_streq (nm_setting_get_name (setting), NM_SETTING_VPN_SETTING_NAME)
+			    && !nm_streq (property->name, NM_SETTING_VPN_SECRETS)) {
 				NMSettingSecretFlags f;
 
 				/* see also _nm_connection_serialize_secrets() */