settings: merge branch 'th/various-settings-cleanup-2'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/184

13 files changed, 346 insertions, 239 deletions

diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c
index f986f824e3..a2bd72b0d9 100644
--- a/libnm-core/nm-connection.c
+++ b/libnm-core/nm-connection.c
@@ -1585,63 +1585,29 @@ nm_connection_verify_secrets (NMConnection *connection, GError **error)
 	return TRUE;
 }
 
-/**
- * nm_connection_normalize:
- * @connection: the #NMConnection to normalize
- * @parameters: (allow-none) (element-type utf8 gpointer): a #GHashTable with
- * normalization parameters to allow customization of the normalization by providing
- * specific arguments. Unknown arguments will be ignored and the default will be
- * used. The keys must be strings compared with g_str_equal() function.
- * The values are opaque and depend on the parameter name.
- * @modified: (out) (allow-none): outputs whether any settings were modified.
- * @error: location to store error, or %NULL. Contains the reason,
- * why the connection is invalid, if the function returns an error.
- *
- * Does some basic normalization and fixup of well known inconsistencies
- * and deprecated fields. If the connection was modified in any way,
- * the output parameter @modified is set %TRUE.
- *
- * Finally the connection will be verified and %TRUE returns if the connection
- * is valid. As this function only performs some specific normalization steps
- * it cannot repair all connections. If the connection has errors that
- * cannot be normalized, the connection will not be modified.
- *
- * Returns: %TRUE if the connection is valid, %FALSE if it is not
- **/
-gboolean
-nm_connection_normalize (NMConnection *connection,
-                         GHashTable *parameters,
-                         gboolean *modified,
-                         GError **error)
+static gboolean
+_connection_normalize (NMConnection *connection,
+                       GHashTable *parameters,
+                       gboolean *modified,
+                       GError **error)
 {
 	NMSettingVerifyResult success;
-	gboolean was_modified = FALSE;
-	GError *normalizable_error = NULL;
-
-	success = _nm_connection_verify (connection, &normalizable_error);
-
-	if (success == NM_SETTING_VERIFY_ERROR ||
-	    success == NM_SETTING_VERIFY_SUCCESS) {
-		if (normalizable_error)
-			g_propagate_error (error, normalizable_error);
-		if (modified)
-			*modified = FALSE;
-		if (success == NM_SETTING_VERIFY_ERROR && error && !*error) {
-			g_set_error_literal (error,
-			                     NM_CONNECTION_ERROR,
-			                     NM_CONNECTION_ERROR_FAILED,
-			                     _("Unexpected failure to verify the connection"));
-			g_return_val_if_reached (FALSE);
-		}
-		return success == NM_SETTING_VERIFY_SUCCESS;
-	}
-	g_assert (success == NM_SETTING_VERIFY_NORMALIZABLE || success == NM_SETTING_VERIFY_NORMALIZABLE_ERROR);
-	g_clear_error (&normalizable_error);
+	gboolean was_modified;
+
+#if NM_MORE_ASSERTS > 10
+	/* only call this _nm_connection_verify() confirms that the connection
+	 * requires normalization and is normalizable. */
+	nm_assert (NM_IN_SET (_nm_connection_verify (connection, NULL),
+	                      NM_SETTING_VERIFY_NORMALIZABLE,
+	                      NM_SETTING_VERIFY_NORMALIZABLE_ERROR));
+#endif
 
 	/* Try to perform all kind of normalizations on the settings to fix it.
 	 * We only do this, after verifying that the connection contains no un-normalizable
 	 * errors, because in that case we rather fail without touching the settings. */
 
+	was_modified = FALSE;
+
 	was_modified |= _normalize_connection_uuid (connection);
 	was_modified |= _normalize_connection_type (connection);
 	was_modified |= _normalize_connection_slave_type (connection);
@@ -1663,11 +1629,12 @@ nm_connection_normalize (NMConnection *connection,
 	was_modified |= _normalize_bridge_vlan_order (connection, parameters);
 	was_modified |= _normalize_bridge_port_vlan_order (connection, parameters);
 
-	/* Verify anew. */
+	was_modified = !!was_modified;
+
+	/* Verify anew */
 	success = _nm_connection_verify (connection, error);
 
-	if (modified)
-		*modified = was_modified;
+	NM_SET_OUT (modified, was_modified);
 
 	if (success != NM_SETTING_VERIFY_SUCCESS) {
 		/* we would expect, that after normalization, the connection can be verified.
@@ -1689,10 +1656,76 @@ nm_connection_normalize (NMConnection *connection,
 	return TRUE;
 }
 
+/**
+ * nm_connection_normalize:
+ * @connection: the #NMConnection to normalize
+ * @parameters: (allow-none) (element-type utf8 gpointer): a #GHashTable with
+ * normalization parameters to allow customization of the normalization by providing
+ * specific arguments. Unknown arguments will be ignored and the default will be
+ * used. The keys must be strings compared with g_str_equal() function.
+ * The values are opaque and depend on the parameter name.
+ * @modified: (out) (allow-none): outputs whether any settings were modified.
+ * @error: location to store error, or %NULL. Contains the reason,
+ * why the connection is invalid, if the function returns an error.
+ *
+ * Does some basic normalization and fixup of well known inconsistencies
+ * and deprecated fields. If the connection was modified in any way,
+ * the output parameter @modified is set %TRUE.
+ *
+ * Finally the connection will be verified and %TRUE returns if the connection
+ * is valid. As this function only performs some specific normalization steps
+ * it cannot repair all connections. If the connection has errors that
+ * cannot be normalized, the connection will not be modified.
+ *
+ * Returns: %TRUE if the connection is valid, %FALSE if it is not
+ **/
+gboolean
+nm_connection_normalize (NMConnection *connection,
+                         GHashTable *parameters,
+                         gboolean *modified,
+                         GError **error)
+{
+	NMSettingVerifyResult success;
+	gs_free_error GError *normalizable_error = NULL;
+
+	success = _nm_connection_verify (connection, &normalizable_error);
+
+	if (!NM_IN_SET (success,
+	                NM_SETTING_VERIFY_NORMALIZABLE,
+	                NM_SETTING_VERIFY_NORMALIZABLE_ERROR)) {
+		if (normalizable_error) {
+			nm_assert (success == NM_SETTING_VERIFY_ERROR);
+			g_propagate_error (error, g_steal_pointer (&normalizable_error));
+		} else
+			nm_assert (success == NM_SETTING_VERIFY_SUCCESS);
+
+		NM_SET_OUT (modified, FALSE);
+
+		if (success != NM_SETTING_VERIFY_SUCCESS) {
+			if (   error
+			    && !*error) {
+				g_set_error_literal (error,
+				                     NM_CONNECTION_ERROR,
+				                     NM_CONNECTION_ERROR_FAILED,
+				                     _("Unexpected failure to verify the connection"));
+				return FALSE;
+			}
+			return FALSE;
+		}
+
+		if (error && *error)
+			return FALSE;
+		return TRUE;
+	}
+
+	return _connection_normalize (connection, parameters, modified, error);
+}
+
 gboolean
 _nm_connection_ensure_normalized (NMConnection *connection,
                                   gboolean allow_modify,
-                                  const char *enforce_uuid,
+                                  const char *expected_uuid,
+                                  gboolean coerce_uuid,
                                   NMConnection **out_connection_clone,
                                   GError **error)
 {
@@ -1701,8 +1734,21 @@ _nm_connection_ensure_normalized (NMConnection *connection,
 	NMSettingVerifyResult vresult;
 
 	nm_assert (NM_IS_CONNECTION (connection));
-	nm_assert (out_connection_clone && !*out_connection_clone);
-	nm_assert (!enforce_uuid || nm_utils_is_uuid (enforce_uuid));
+	nm_assert (!out_connection_clone || !*out_connection_clone);
+	nm_assert (!expected_uuid || nm_utils_is_uuid (expected_uuid));
+
+	if (expected_uuid) {
+		if (nm_streq0 (expected_uuid, nm_connection_get_uuid (connection)))
+			expected_uuid = NULL;
+		else if (   !coerce_uuid
+		         || (!allow_modify && !out_connection_clone)) {
+			g_set_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY,
+			             _("unexpected uuid %s instead of %s"),
+			             nm_connection_get_uuid (connection),
+			             expected_uuid);
+			return FALSE;
+		}
+	}
 
 	vresult = _nm_connection_verify (connection, &local);
 	if (vresult != NM_SETTING_VERIFY_SUCCESS) {
@@ -1712,34 +1758,36 @@ _nm_connection_ensure_normalized (NMConnection *connection,
 			return FALSE;
 		}
 		if (!allow_modify) {
+			if (!out_connection_clone) {
+				/* even NM_SETTING_VERIFY_NORMALIZABLE is treated as an error. We could normalize,
+				 * but are not allowed to (and no out argument is provided for cloning).  */
+				g_propagate_error (error, g_steal_pointer (&local));
+				return FALSE;
+			}
 			connection_clone = nm_simple_connection_new_clone (connection);
 			connection = connection_clone;
 		}
-		if (!nm_connection_normalize (connection, NULL, NULL, error)) {
-			nm_assert_not_reached ();
-			return FALSE;
-		}
+		if (!_connection_normalize (connection, NULL, NULL, error))
+			g_return_val_if_reached (FALSE);
 	}
 
-	if (enforce_uuid) {
-		if (!nm_streq (enforce_uuid, nm_connection_get_uuid (connection))) {
-			NMSettingConnection *s_con;
+	if (expected_uuid) {
+		NMSettingConnection *s_con;
 
-			if (   !allow_modify
-			    && !connection_clone) {
-				connection_clone = nm_simple_connection_new_clone (connection);
-				connection = connection_clone;
-			}
-			s_con = nm_connection_get_setting_connection (connection);
-			g_object_set (s_con,
-			              NM_SETTING_CONNECTION_UUID,
-			              enforce_uuid,
-			              NULL);
+		if (   !allow_modify
+		    && !connection_clone) {
+			nm_assert (out_connection_clone);
+			connection_clone = nm_simple_connection_new_clone (connection);
+			connection = connection_clone;
 		}
+		s_con = nm_connection_get_setting_connection (connection);
+		g_object_set (s_con,
+		              NM_SETTING_CONNECTION_UUID,
+		              expected_uuid,
+		              NULL);
 	}
 
-	if (connection_clone)
-		*out_connection_clone = g_steal_pointer (&connection_clone);
+	NM_SET_OUT (out_connection_clone, g_steal_pointer (&connection_clone));
 	return TRUE;
 }
 
@@ -2002,6 +2050,52 @@ nm_connection_clear_secrets_with_flags (NMConnection *connection,
 	g_signal_emit (connection, signals[SECRETS_CLEARED], 0);
 }
 
+static gboolean
+_clear_secrets_by_secret_flags_cb (NMSetting *setting,
+                                   const char *secret,
+                                   NMSettingSecretFlags flags,
+                                   gpointer user_data)
+{
+	NMSettingSecretFlags filter_flags = GPOINTER_TO_UINT (user_data);
+	gboolean remove_secret;
+
+	if (filter_flags == NM_SETTING_SECRET_FLAG_NONE) {
+		/* Can't use bitops with SECRET_FLAG_NONE so handle that specifically */
+		remove_secret = (flags != NM_SETTING_SECRET_FLAG_NONE);
+	} else {
+		/* Otherwise if the secret has at least one of the desired flags keep it */
+		remove_secret = !NM_FLAGS_ANY (flags, filter_flags);
+	}
+
+	return remove_secret;
+}
+
+/**
+ * _nm_connection_clear_secrets_by_secret_flags:
+ * @self: the #NMConnection to filter (will be modified)
+ * @filter_flags: the secret flags to control whether to drop/remove
+ *   a secret or to keep it. The meaning of the filter flags is to
+ *   preseve the secrets. The secrets that have matching (see below)
+ *   flags are kept, the others are dropped.
+ *
+ * Removes/drops secrets from @self according to @filter_flags.
+ * If @filter_flags is %NM_SETTING_SECRET_NONE, then only secrets that
+ * have %NM_SETTING_SECRET_NONE flags are kept.
+ * Otherwise, only secrets with secret flags are kept that have at least
+ * one of the filter flags.
+ */
+void
+_nm_connection_clear_secrets_by_secret_flags (NMConnection *self,
+                                              NMSettingSecretFlags filter_flags)
+{
+	nm_connection_clear_secrets_with_flags (self,
+	                                        _clear_secrets_by_secret_flags_cb,
+	                                        GUINT_TO_POINTER (filter_flags));
+}
+
+/*****************************************************************************/
+
+
 /*****************************************************************************/
 
 /* Returns always a non-NULL, floating variant that must
diff --git a/libnm-core/nm-connection.h b/libnm-core/nm-connection.h
index bddec74bae..4399ad67da 100644
--- a/libnm-core/nm-connection.h
+++ b/libnm-core/nm-connection.h
@@ -111,14 +111,17 @@ NMSetting    *nm_connection_get_setting_by_name (NMConnection *connection,
  * @NM_CONNECTION_SERIALIZE_ALL: serialize all properties (including secrets)
  * @NM_CONNECTION_SERIALIZE_NO_SECRETS: do not include secrets
  * @NM_CONNECTION_SERIALIZE_ONLY_SECRETS: only serialize secrets
+ * @NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED: if set, only secrets that
+ *   are agent owned will be serialized.
  *
  * These flags determine which properties are serialized when calling when
  * calling nm_connection_to_dbus().
  **/
 typedef enum { /*< flags >*/
-	NM_CONNECTION_SERIALIZE_ALL = 0x00000000,
-	NM_CONNECTION_SERIALIZE_NO_SECRETS = 0x00000001,
-	NM_CONNECTION_SERIALIZE_ONLY_SECRETS = 0x00000002,
+	NM_CONNECTION_SERIALIZE_ALL                      = 0x00000000,
+	NM_CONNECTION_SERIALIZE_NO_SECRETS               = 0x00000001,
+	NM_CONNECTION_SERIALIZE_ONLY_SECRETS             = 0x00000002,
+	NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED = 0x00000004,
 } NMConnectionSerializationFlags;
 
 GVariant     *nm_connection_to_dbus       (NMConnection *connection,
diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h
index 685b2cc0d9..eaf29849cf 100644
--- a/libnm-core/nm-core-internal.h
+++ b/libnm-core/nm-core-internal.h
@@ -204,7 +204,8 @@ NMSettingVerifyResult _nm_connection_verify (NMConnection *connection, GError **
 
 gboolean _nm_connection_ensure_normalized (NMConnection *connection,
                                            gboolean allow_modify,
-                                           const char *enforce_uuid,
+                                           const char *expected_uuid,
+                                           gboolean coerce_uuid,
                                            NMConnection **out_connection_clone,
                                            GError **error);
 
@@ -805,6 +806,21 @@ GBytes *_nm_setting_802_1x_cert_value_to_bytes (NMSetting8021xCKScheme scheme,
 
 /*****************************************************************************/
 
+static inline gboolean
+_nm_connection_serialize_secrets (NMConnectionSerializationFlags flags,
+                                  NMSettingSecretFlags secret_flags)
+{
+	if (NM_FLAGS_HAS (flags, NM_CONNECTION_SERIALIZE_NO_SECRETS))
+		return FALSE;
+	if (   NM_FLAGS_HAS (flags, NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED)
+	    && !NM_FLAGS_HAS (secret_flags, NM_SETTING_SECRET_FLAG_AGENT_OWNED))
+		return FALSE;
+	return TRUE;
+}
+
+void _nm_connection_clear_secrets_by_secret_flags (NMConnection *self,
+                                                   NMSettingSecretFlags filter_flags);
+
 GVariant *_nm_connection_for_each_secret (NMConnection *self,
                                           GVariant *secrets,
                                           gboolean remove_non_secrets,
diff --git a/libnm-core/nm-setting-wireguard.c b/libnm-core/nm-setting-wireguard.c
index dd26a1ea48..64a9b1a228 100644
--- a/libnm-core/nm-setting-wireguard.c
+++ b/libnm-core/nm-setting-wireguard.c
@@ -1490,7 +1490,7 @@ _peers_dbus_only_synth (const NMSettInfoSetting *sett_info,
 		    && peer->endpoint)
 			g_variant_builder_add (&builder, "{sv}", NM_WIREGUARD_PEER_ATTR_ENDPOINT, g_variant_new_string (nm_sock_addr_endpoint_get_endpoint (peer->endpoint)));
 
-		if (   !NM_FLAGS_HAS (flags, NM_CONNECTION_SERIALIZE_NO_SECRETS)
+		if (   _nm_connection_serialize_secrets (flags, peer->preshared_key_flags)
 		    && peer->preshared_key)
 			g_variant_builder_add (&builder, "{sv}", NM_WIREGUARD_PEER_ATTR_PRESHARED_KEY, g_variant_new_string (peer->preshared_key));
 
diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c
index 297804b5ce..dfb2393ca3 100644
--- a/libnm-core/nm-setting.c
+++ b/libnm-core/nm-setting.c
@@ -708,13 +708,22 @@ property_to_dbus (const NMSettInfoSetting *sett_info,
 		    && !_nm_utils_is_manager_process)
 			return NULL;
 
-		if (   NM_FLAGS_HAS (flags, NM_CONNECTION_SERIALIZE_NO_SECRETS)
-		    && NM_FLAGS_HAS (property->param_spec->flags, NM_SETTING_PARAM_SECRET))
-			return NULL;
-
-		if (   NM_FLAGS_HAS (flags, NM_CONNECTION_SERIALIZE_ONLY_SECRETS)
-		    && !NM_FLAGS_HAS (property->param_spec->flags, NM_SETTING_PARAM_SECRET))
-			return NULL;
+		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)) {
+				NMSettingSecretFlags f;
+
+				/* see also _nm_connection_serialize_secrets() */
+				if (!nm_setting_get_secret_flags (setting, property->param_spec->name, &f, NULL))
+					return NULL;
+				if (!NM_FLAGS_HAS (f, NM_SETTING_SECRET_FLAG_AGENT_OWNED))
+					return NULL;
+			}
+		} else {
+			if (NM_FLAGS_HAS (flags, NM_CONNECTION_SERIALIZE_ONLY_SECRETS))
+				return NULL;
+		}
 	}
 
 	if (property->to_dbus_fcn) {
diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c
index 6ed86f5916..c8b0eef84e 100644
--- a/shared/nm-glib-aux/nm-shared-utils.c
+++ b/shared/nm-glib-aux/nm-shared-utils.c
@@ -2709,6 +2709,25 @@ nm_utils_g_slist_find_str (const GSList *list,
 	return NULL;
 }
 
+/**
+ * nm_utils_g_slist_strlist_cmp:
+ * @a: the left #GSList of strings
+ * @b: the right #GSList of strings to compare.
+ *
+ * Compares two string lists. The data elements are compared with
+ * strcmp(), alloing %NULL elements.
+ *
+ * Returns: 0, 1, or -1, depending on how the lists compare.
+ */
+int
+nm_utils_g_slist_strlist_cmp (const GSList *a, const GSList *b)
+{
+	for (; a && b; a = a->next, b = b->next)
+		NM_CMP_DIRECT_STRCMP0 (a->data, b->data);
+	NM_CMP_SELF (a, b);
+	return 0;
+}
+
 /*****************************************************************************/
 
 gpointer
diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h
index 8f11a9b0c3..a8f2966b38 100644
--- a/shared/nm-glib-aux/nm-shared-utils.h
+++ b/shared/nm-glib-aux/nm-shared-utils.h
@@ -981,6 +981,8 @@ nm_utils_strv_make_deep_copied_nonnull (const char **strv)
 GSList *nm_utils_g_slist_find_str (const GSList *list,
                                    const char *needle);
 
+int nm_utils_g_slist_strlist_cmp (const GSList *a, const GSList *b);
+
 /*****************************************************************************/
 
 gssize nm_utils_ptrarray_find_binary_search (gconstpointer *list,
diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c
index e684aab6ee..ab9df65429 100644
--- a/src/settings/nm-settings-connection.c
+++ b/src/settings/nm-settings-connection.c
@@ -312,25 +312,6 @@ nm_settings_connection_check_permission (NMSettingsConnection *self,
 
 /*****************************************************************************/
 
-static gboolean
-secrets_filter_cb (NMSetting *setting,
-                   const char *secret,
-                   NMSettingSecretFlags flags,
-                   gpointer user_data)
-{
-	NMSettingSecretFlags filter_flags = GPOINTER_TO_UINT (user_data);
-
-	/* Returns TRUE to remove the secret */
-
-	/* Can't use bitops with SECRET_FLAG_NONE so handle that specifically */
-	if (   (flags == NM_SETTING_SECRET_FLAG_NONE)
-	    && (filter_flags == NM_SETTING_SECRET_FLAG_NONE))
-		return FALSE;
-
-	/* Otherwise if the secret has at least one of the desired flags keep it */
-	return (flags & filter_flags) ? FALSE : TRUE;
-}
-
 static void
 update_system_secrets_cache (NMSettingsConnection *self)
 {
@@ -341,16 +322,14 @@ update_system_secrets_cache (NMSettingsConnection *self)
 	priv->system_secrets = nm_simple_connection_new_clone (nm_settings_connection_get_connection (self));
 
 	/* Clear out non-system-owned and not-saved secrets */
-	nm_connection_clear_secrets_with_flags (priv->system_secrets,
-	                                        secrets_filter_cb,
-	                                        GUINT_TO_POINTER (NM_SETTING_SECRET_FLAG_NONE));
+	_nm_connection_clear_secrets_by_secret_flags (priv->system_secrets,
+	                                              NM_SETTING_SECRET_FLAG_NONE);
 }
 
 static void
 update_agent_secrets_cache (NMSettingsConnection *self, NMConnection *new)
 {
 	NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self);
-	NMSettingSecretFlags filter_flags = NM_SETTING_SECRET_FLAG_NOT_SAVED | NM_SETTING_SECRET_FLAG_AGENT_OWNED;
 
 	if (priv->agent_secrets)
 		g_object_unref (priv->agent_secrets);
@@ -358,9 +337,9 @@ update_agent_secrets_cache (NMSettingsConnection *self, NMConnection *new)
 	                                                      ?: nm_settings_connection_get_connection (self));
 
 	/* Clear out non-system-owned secrets */
-	nm_connection_clear_secrets_with_flags (priv->agent_secrets,
-	                                        secrets_filter_cb,
-	                                        GUINT_TO_POINTER (filter_flags));
+	_nm_connection_clear_secrets_by_secret_flags (priv->agent_secrets,
+	                                                NM_SETTING_SECRET_FLAG_NOT_SAVED
+	                                              | NM_SETTING_SECRET_FLAG_AGENT_OWNED);
 }
 
 static void
@@ -503,7 +482,6 @@ nm_settings_connection_update (NMSettingsConnection *self,
 	gboolean replaced = FALSE;
 	gs_free char *logmsg_change = NULL;
 	GError *local = NULL;
-	gs_unref_object NMConnection *simple = NULL;
 	gs_unref_variant GVariant *con_agent_secrets = NULL;
 	gs_unref_variant GVariant *new_agent_secrets = NULL;
 
@@ -548,12 +526,8 @@ nm_settings_connection_update (NMSettingsConnection *self,
 
 	/* Save agent-owned secrets from the new connection for later use */
 	if (new_connection) {
-		simple = nm_simple_connection_new_clone (new_connection);
-		nm_connection_clear_secrets_with_flags (simple,
-		                                        secrets_filter_cb,
-		                                        GUINT_TO_POINTER (NM_SETTING_SECRET_FLAG_AGENT_OWNED));
-		new_agent_secrets = nm_connection_to_dbus (simple, NM_CONNECTION_SERIALIZE_ONLY_SECRETS);
-		g_clear_object (&simple);
+		new_agent_secrets = nm_connection_to_dbus (new_connection,   NM_CONNECTION_SERIALIZE_ONLY_SECRETS
+		                                                           | NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED);
 	}
 
 	/* Disconnect the changed signal to ensure we don't set Unsaved when
@@ -575,11 +549,9 @@ nm_settings_connection_update (NMSettingsConnection *self,
 		/* Make a copy of agent-owned secrets because they won't be present in
 		 * the connection returned by plugins, as plugins return only what was
 		 * reread from the file. */
-		simple = nm_simple_connection_new_clone (nm_settings_connection_get_connection (self));
-		nm_connection_clear_secrets_with_flags (simple,
-		                                        secrets_filter_cb,
-		                                        GUINT_TO_POINTER (NM_SETTING_SECRET_FLAG_AGENT_OWNED));
-		con_agent_secrets = nm_connection_to_dbus (simple, NM_CONNECTION_SERIALIZE_ONLY_SECRETS);
+		con_agent_secrets = nm_connection_to_dbus (nm_settings_connection_get_connection (self),
+		                                             NM_CONNECTION_SERIALIZE_ONLY_SECRETS
+		                                           | NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED);
 
 		nm_connection_replace_settings_from_connection (nm_settings_connection_get_connection (self), replace_connection);
 
@@ -1637,9 +1609,8 @@ update_auth_cb (NMSettingsConnection *self,
 		 * Only send secrets to agents of the same UID that called update too.
 		 */
 		for_agent = nm_simple_connection_new_clone (nm_settings_connection_get_connection (self));
-		nm_connection_clear_secrets_with_flags (for_agent,
-		                                        secrets_filter_cb,
-		                                        GUINT_TO_POINTER (NM_SETTING_SECRET_FLAG_AGENT_OWNED));
+		_nm_connection_clear_secrets_by_secret_flags (for_agent,
+		                                              NM_SETTING_SECRET_FLAG_AGENT_OWNED);
 		nm_agent_manager_save_secrets (info->agent_mgr,
 		                               nm_dbus_object_get_path (NM_DBUS_OBJECT (self)),
 		                               for_agent,
diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c
index d42d439ff3..1c259be98a 100644
--- a/src/settings/nm-settings.c
+++ b/src/settings/nm-settings.c
@@ -114,8 +114,12 @@ typedef struct {
 
 	NMConfig *config;
 
+	NMHostnameManager *hostname_manager;
+
 	CList auth_lst_head;
 
+	NMSKeyfilePlugin *keyfile_plugin;
+
 	GSList *plugins;
 
 	NMKeyFileDB *kf_db_timestamps;
@@ -124,11 +128,10 @@ typedef struct {
 	CList connections_lst_head;
 
 	NMSettingsConnection **connections_cached_list;
+
 	GSList *unmanaged_specs;
 	GSList *unrecognized_specs;
 
-	NMHostnameManager *hostname_manager;
-
 	NMSettingsConnection *startup_complete_blocked_by;
 
 	guint connections_len;
@@ -242,15 +245,14 @@ nm_settings_get_unmanaged_specs (NMSettings *self)
 	return priv->unmanaged_specs;
 }
 
-static void
+static gboolean
 update_specs (NMSettings *self, GSList **specs_ptr,
               GSList * (*get_specs_func) (NMSettingsPlugin *))
 {
 	NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self);
+	GSList *new = NULL;
 	GSList *iter;
 
-	g_slist_free_full (g_steal_pointer (specs_ptr), g_free);
-
 	for (iter = priv->plugins; iter; iter = g_slist_next (iter)) {
 		GSList *specs;
 
@@ -259,15 +261,25 @@ update_specs (NMSettings *self, GSList **specs_ptr,
 			GSList *s = specs;
 
 			specs = g_slist_remove_link (specs, s);
-			if (nm_utils_g_slist_find_str (*specs_ptr, s->data)) {
+			if (nm_utils_g_slist_find_str (new, s->data)) {
 				g_free (s->data);
 				g_slist_free_1 (s);
 				continue;
 			}
-			s->next = *specs_ptr;
-			*specs_ptr = s;
+			s->next = new;
+			new = s;
 		}
 	}
+
+	if (nm_utils_g_slist_strlist_cmp (new, *specs_ptr) == 0) {
+		g_slist_free_full (new, g_free);
+		return FALSE;
+	}
+
+	g_slist_free_full (*specs_ptr, g_free);
+	*specs_ptr = new;
+	return TRUE;
+
 }
 
 static void
@@ -277,9 +289,9 @@ unmanaged_specs_changed (NMSettingsPlugin *config,
 	NMSettings *self = NM_SETTINGS (user_data);
 	NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self);
 
-	update_specs (self, &priv->unmanaged_specs,
-	              nm_settings_plugin_get_unmanaged_specs);
-	_notify (self, PROP_UNMANAGED_SPECS);
+	if (update_specs (self, &priv->unmanaged_specs,
+	                  nm_settings_plugin_get_unmanaged_specs))
+		_notify (self, PROP_UNMANAGED_SPECS);
 }
 
 static void
@@ -527,25 +539,6 @@ claim_connection (NMSettings *self, NMSettingsConnection *sett_conn)
 
 /*****************************************************************************/
 
-static gboolean
-secrets_filter_cb (NMSetting *setting,
-                   const char *secret,
-                   NMSettingSecretFlags flags,
-                   gpointer user_data)
-{
-	NMSettingSecretFlags filter_flags = GPOINTER_TO_UINT (user_data);
-
-	/* Returns TRUE to remove the secret */
-
-	/* Can't use bitops with SECRET_FLAG_NONE so handle that specifically */
-	if (   (flags == NM_SETTING_SECRET_FLAG_NONE)
-	    && (filter_flags == NM_SETTING_SECRET_FLAG_NONE))
-		return FALSE;
-
-	/* Otherwise if the secret has at least one of the desired flags keep it */
-	return (flags & filter_flags) ? FALSE : TRUE;
-}
-
 /**
  * nm_settings_add_connection:
  * @self: the #NMSettings object
@@ -596,17 +589,14 @@ nm_settings_add_connection (NMSettings *self,
 	for (iter = priv->plugins; iter; iter = g_slist_next (iter)) {
 		NMSettingsPlugin *plugin = NM_SETTINGS_PLUGIN (iter->data);
 		GError *add_error = NULL;
-		gs_unref_object NMConnection *simple = NULL;
 		gs_unref_variant GVariant *secrets = NULL;
 
 		/* Make a copy of agent-owned secrets because they won't be present in
 		 * the connection returned by plugins, as plugins return only what was
 		 * reread from the file. */
-		simple = nm_simple_connection_new_clone (connection);
-		nm_connection_clear_secrets_with_flags (simple,
-		                                        secrets_filter_cb,
-		                                        GUINT_TO_POINTER (NM_SETTING_SECRET_FLAG_AGENT_OWNED));
-		secrets = nm_connection_to_dbus (simple, NM_CONNECTION_SERIALIZE_ONLY_SECRETS);
+		secrets = nm_connection_to_dbus (connection,
+		                                   NM_CONNECTION_SERIALIZE_ONLY_SECRETS
+		                                 | NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED);
 
 		added = nm_settings_plugin_add_connection (plugin, connection, save_to_disk, &add_error);
 		if (added) {
@@ -645,9 +635,8 @@ send_agent_owned_secrets (NMSettings *self,
 	 * Only send secrets to agents of the same UID that called update too.
 	 */
 	for_agent = nm_simple_connection_new_clone (nm_settings_connection_get_connection (sett_conn));
-	nm_connection_clear_secrets_with_flags (for_agent,
-	                                        secrets_filter_cb,
-	                                        GUINT_TO_POINTER (NM_SETTING_SECRET_FLAG_AGENT_OWNED));
+	_nm_connection_clear_secrets_by_secret_flags (for_agent,
+	                                              NM_SETTING_SECRET_FLAG_AGENT_OWNED);
 	nm_agent_manager_save_secrets (priv->agent_mgr,
 	                               nm_dbus_object_get_path (NM_DBUS_OBJECT (sett_conn)),
 	                               for_agent,
@@ -1339,17 +1328,18 @@ add_plugin_load_file (NMSettings *self, const char *pname, GError **error)
 static void
 add_plugin_keyfile (NMSettings *self)
 {
-	gs_unref_object NMSKeyfilePlugin *keyfile_plugin = NULL;
+	NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self);
 
-	keyfile_plugin = nms_keyfile_plugin_new ();
-	add_plugin (self, NM_SETTINGS_PLUGIN (keyfile_plugin), "keyfile", NULL);
+	if (priv->keyfile_plugin)
+		return;
+	priv->keyfile_plugin = nms_keyfile_plugin_new ();
+	add_plugin (self, NM_SETTINGS_PLUGIN (priv->keyfile_plugin), "keyfile", NULL);
 }
 
 static gboolean
-load_plugins (NMSettings *self, const char **plugins, GError **error)
+load_plugins (NMSettings *self, const char *const*plugins, GError **error)
 {
-	const char **iter;
-	gboolean keyfile_added = FALSE;
+	const char *const*iter;
 	gboolean success = TRUE;
 	gboolean add_ibft = FALSE;
 	gboolean has_no_ibft;
@@ -1382,10 +1372,7 @@ load_plugins (NMSettings *self, const char **plugins, GError **error)
 
 		/* keyfile plugin is built-in now */
 		if (nm_streq (pname, "keyfile")) {
-			if (!keyfile_added) {
-				add_plugin_keyfile (self);
-				keyfile_added = TRUE;
-			}
+			add_plugin_keyfile (self);
 			continue;
 		}
 
@@ -1401,12 +1388,11 @@ load_plugins (NMSettings *self, const char **plugins, GError **error)
 		if (!success)
 			break;
 
-		if (add_ibft && nm_streq (pname, "ifcfg-rh")) {
+		if (   add_ibft
+		    && nm_streq (pname, "ifcfg-rh")) {
 			/* The plugin ibft is not explicitly mentioned but we just enabled "ifcfg-rh".
 			 * Enable "ibft" by default after "ifcfg-rh". */
-			pname = "ibft";
 			add_ibft = FALSE;
-
 			success = add_plugin_load_file (self, "ibft", error);
 			if (!success)
 				break;
@@ -1414,7 +1400,7 @@ load_plugins (NMSettings *self, const char **plugins, GError **error)
 	}
 
 	/* If keyfile plugin was not among configured plugins, add it as the last one */
-	if (!keyfile_added && success)
+	if (success)
 		add_plugin_keyfile (self);
 
 	return success;
@@ -1839,7 +1825,7 @@ nm_settings_start (NMSettings *self, GError **error)
 	/* Load the plugins; fail if a plugin is not found. */
 	plugins = nm_config_data_get_plugins (nm_config_get_data_orig (priv->config), TRUE);
 
-	if (!load_plugins (self, (const char **) plugins, error))
+	if (!load_plugins (self, (const char *const*) plugins, error))
 		return FALSE;
 
 	load_connections (self);
@@ -1964,6 +1950,8 @@ finalize (GObject *object)
 		g_signal_handlers_disconnect_by_data (plugin, self);
 	}
 
+	g_clear_object (&priv->keyfile_plugin);
+
 	g_clear_object (&priv->agent_mgr);
 
 	g_clear_object (&priv->config);
diff --git a/src/settings/plugins/keyfile/nms-keyfile-connection.c b/src/settings/plugins/keyfile/nms-keyfile-connection.c
index 6003612a3a..faf39abbfd 100644
--- a/src/settings/plugins/keyfile/nms-keyfile-connection.c
+++ b/src/settings/plugins/keyfile/nms-keyfile-connection.c
@@ -68,6 +68,8 @@ commit_changes (NMSettingsConnection *connection,
 	                                    nm_settings_connection_get_filename (connection),
 	                                    NM_FLAGS_ALL (commit_reason,   NM_SETTINGS_CONNECTION_COMMIT_REASON_USER_ACTION
 	                                                                 | NM_SETTINGS_CONNECTION_COMMIT_REASON_ID_CHANGED),
+	                                    NULL,
+	                                    NULL,
 	                                    &path,
 	                                    &reread,
 	                                    &reread_same,
diff --git a/src/settings/plugins/keyfile/nms-keyfile-plugin.c b/src/settings/plugins/keyfile/nms-keyfile-plugin.c
index 59b246d6cd..46432fd257 100644
--- a/src/settings/plugins/keyfile/nms-keyfile-plugin.c
+++ b/src/settings/plugins/keyfile/nms-keyfile-plugin.c
@@ -485,6 +485,8 @@ add_connection (NMSettingsPlugin *config,
 	                                    save_to_disk,
 	                                    NULL,
 	                                    FALSE,
+	                                    NULL,
+	                                    NULL,
 	                                    &path,
 	                                    &reread,
 	                                    NULL,
diff --git a/src/settings/plugins/keyfile/nms-keyfile-writer.c b/src/settings/plugins/keyfile/nms-keyfile-writer.c
index 837bc1115f..a0c3c17b98 100644
--- a/src/settings/plugins/keyfile/nms-keyfile-writer.c
+++ b/src/settings/plugins/keyfile/nms-keyfile-writer.c
@@ -176,6 +176,8 @@ _internal_write_connection (NMConnection *connection,
                             const char *existing_path,
                             gboolean existing_path_read_only,
                             gboolean force_rename,
+                            NMSKeyfileWriterAllowFilenameCb allow_filename_cb,
+                            gpointer allow_filename_user_data,
                             char **out_path,
                             NMConnection **out_reread,
                             gboolean *out_reread_same,
@@ -190,27 +192,20 @@ _internal_write_connection (NMConnection *connection,
 	GError *local_err = NULL;
 	int errsv;
 	gboolean rename;
+	int i_path;
 
 	g_return_val_if_fail (!out_path || !*out_path, FALSE);
 	g_return_val_if_fail (keyfile_dir && keyfile_dir[0] == '/', FALSE);
 
+	nm_assert (_nm_connection_verify (connection, NULL) == NM_SETTING_VERIFY_SUCCESS);
+
 	rename =    force_rename
 	         || existing_path_read_only
 	         || (   existing_path
 	             && !nm_utils_file_is_in_path (existing_path, keyfile_dir));
 
-	switch (_nm_connection_verify (connection, error)) {
-	case NM_SETTING_VERIFY_NORMALIZABLE:
-		nm_assert_not_reached ();
-		/* fall-through */
-	case NM_SETTING_VERIFY_SUCCESS:
-		break;
-	default:
-		g_return_val_if_reached (FALSE);
-	}
-
 	id = nm_connection_get_id (connection);
-	g_assert (id && *id);
+	nm_assert (id && *id);
 
 	info.keyfile_dir = keyfile_dir;
 
@@ -224,64 +219,59 @@ _internal_write_connection (NMConnection *connection,
 	if (!g_file_test (keyfile_dir, G_FILE_TEST_IS_DIR))
 		(void) g_mkdir_with_parents (keyfile_dir, 0755);
 
-	/* If we have existing file path, use it. Else generate one from
-	 * connection's ID.
-	 */
-	if (   existing_path
-	    && !rename)
-		path = g_strdup (existing_path);
-	else {
-		gs_free char *filename_escaped = NULL;
+	for (i_path = -2; i_path < 10000; i_path++) {
+		gs_free char *path_candidate = NULL;
+		gboolean is_existing_path;
 
-		filename_escaped = nm_keyfile_utils_create_filename (id, with_extension);
-		path = g_build_filename (keyfile_dir, filename_escaped, NULL);
-	}
-
-	/* If a file with this path already exists (but isn't the existing path
-	 * of the connection) then we need another name.  Multiple connections
-	 * can have the same ID (ie if two connections with the same ID are visible
-	 * to different users) but of course can't have the same path.  Yeah,
-	 * there's a race here, but there's not a lot we can do about it, and
-	 * we shouldn't get more than one connection with the same UUID either.
-	 */
-	if (   !nm_streq0 (path, existing_path)
-	    && g_file_test (path, G_FILE_TEST_EXISTS)) {
-		guint i;
-		gboolean name_found = FALSE;
+		if (i_path == -2) {
+			if (   !existing_path
+			    || rename)
+				continue;
+			path_candidate = g_strdup (existing_path);
+		} else if (i_path == -1) {
+			gs_free char *filename_escaped = NULL;
 
-		/* A keyfile with this connection's ID already exists. Pick another name. */
-		for (i = 0; i < 100; i++) {
+			filename_escaped = nm_keyfile_utils_create_filename (id, with_extension);
+			path_candidate = g_build_filename (keyfile_dir, filename_escaped, NULL);
+		} else {
 			gs_free char *filename_escaped = NULL;
 			gs_free char *filename = NULL;
 
-			if (i == 0)
+			if (i_path == 0)
 				filename = g_strdup_printf ("%s-%s", id, nm_connection_get_uuid (connection));
 			else
-				filename = g_strdup_printf ("%s-%s-%u", id, nm_connection_get_uuid (connection), i);
+				filename = g_strdup_printf ("%s-%s-%d", id, nm_connection_get_uuid (connection), i_path);
 
 			filename_escaped = nm_keyfile_utils_create_filename (filename, with_extension);
 
-			g_free (path);
-			path = g_strdup_printf ("%s/%s", keyfile_dir, filename_escaped);
-
-			if (   nm_streq0 (path, existing_path)
-			    || !g_file_test (path, G_FILE_TEST_EXISTS)) {
-				name_found = TRUE;
-				break;
-			}
+			path_candidate = g_strdup_printf ("%s/%s", keyfile_dir, filename_escaped);
 		}
-		if (!name_found) {
-			if (existing_path_read_only || !existing_path) {
-				/* this really should not happen, we tried hard to find an unused name... bail out. */
-				g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
-				                    "could not find suitable keyfile file name (%s already used)", path);
-				return FALSE;
-			}
-			/* Both our preferred path based on connection id and id-uuid are taken.
-			 * Fallback to @existing_path */
-			g_free (path);
-			path = g_strdup (existing_path);
+
+		is_existing_path =    existing_path
+		                   && nm_streq (existing_path, path_candidate);
+
+		if (   is_existing_path
+		    && rename)
+			continue;
+
+		if (   allow_filename_cb
+		    && !allow_filename_cb (path_candidate, allow_filename_user_data))
+			continue;
+
+		if (!is_existing_path) {
+			if (g_file_test (path_candidate, G_FILE_TEST_EXISTS))
+				continue;
 		}
+
+		path = g_steal_pointer (&path_candidate);
+		break;
+	}
+
+	if (!path) {
+		/* this really should not happen, we tried hard to find an unused name... bail out. */
+		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
+		                    "could not find suitable keyfile file name (%s already used)", path);
+		return FALSE;
 	}
 
 	nm_utils_file_set_contents (path, kf_content_buf, kf_content_len, 0600, &local_err);
@@ -350,6 +340,8 @@ nms_keyfile_writer_connection (NMConnection *connection,
                                gboolean save_to_disk,
                                const char *existing_path,
                                gboolean force_rename,
+                               NMSKeyfileWriterAllowFilenameCb allow_filename_cb,
+                               gpointer allow_filename_user_data,
                                char **out_path,
                                NMConnection **out_reread,
                                gboolean *out_reread_same,
@@ -371,6 +363,8 @@ nms_keyfile_writer_connection (NMConnection *connection,
 	                                   existing_path,
 	                                   FALSE,
 	                                   force_rename,
+	                                   allow_filename_cb,
+	                                   allow_filename_user_data,
 	                                   out_path,
 	                                   out_reread,
 	                                   out_reread_same,
@@ -396,6 +390,8 @@ nms_keyfile_writer_test_connection (NMConnection *connection,
 	                                   NULL,
 	                                   FALSE,
 	                                   FALSE,
+	                                   NULL,
+	                                   NULL,
 	                                   out_path,
 	                                   out_reread,
 	                                   out_reread_same,
diff --git a/src/settings/plugins/keyfile/nms-keyfile-writer.h b/src/settings/plugins/keyfile/nms-keyfile-writer.h
index 0b51280c3c..db41b81c50 100644
--- a/src/settings/plugins/keyfile/nms-keyfile-writer.h
+++ b/src/settings/plugins/keyfile/nms-keyfile-writer.h
@@ -23,10 +23,15 @@
 
 #include "nm-connection.h"
 
+typedef gboolean (*NMSKeyfileWriterAllowFilenameCb) (const char *check_filename,
+                                                     gpointer allow_filename_user_data);
+
 gboolean nms_keyfile_writer_connection (NMConnection *connection,
                                         gboolean save_to_disk,
                                         const char *existing_path,
                                         gboolean force_rename,
+                                        NMSKeyfileWriterAllowFilenameCb allow_filename_cb,
+                                        gpointer allow_filename_user_data,
                                         char **out_path,
                                         NMConnection **out_reread,
                                         gboolean *out_reread_same,