agent-manager: merge branch 'th/agent-manager-cleanup'

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

8 files changed, 246 insertions, 221 deletions

diff --git a/shared/nm-glib-aux/nm-glib.h b/shared/nm-glib-aux/nm-glib.h
index e7e69dc5fe..4043ec2240 100644
--- a/shared/nm-glib-aux/nm-glib.h
+++ b/shared/nm-glib-aux/nm-glib.h
@@ -608,21 +608,30 @@ g_hash_table_steal_extended (GHashTable    *hash_table,
                              gpointer      *stolen_key,
                              gpointer      *stolen_value)
 {
+	g_assert (stolen_key);
+	g_assert (stolen_value);
+
 	if (g_hash_table_lookup_extended (hash_table, lookup_key, stolen_key, stolen_value)) {
 		g_hash_table_steal (hash_table, lookup_key);
 		return TRUE;
 	}
-	if (stolen_key)
-		*stolen_key = NULL;
-	if (stolen_value)
-		*stolen_value = NULL;
+	*stolen_key = NULL;
+	*stolen_value = NULL;
 	return FALSE;
 }
 #else
 #define g_hash_table_steal_extended(hash_table, lookup_key, stolen_key, stolen_value) \
 	({ \
+		gpointer *_stolen_key = (stolen_key); \
+		gpointer *_stolen_value = (stolen_value); \
+		\
+		/* we cannot allow NULL arguments, because then we would leak the values in
+		 * the compat implementation. */ \
+		g_assert (_stolen_key); \
+		g_assert (_stolen_value); \
+		\
 		G_GNUC_BEGIN_IGNORE_DEPRECATIONS \
-		g_hash_table_steal_extended (hash_table, lookup_key, stolen_key, stolen_value); \
+		g_hash_table_steal_extended (hash_table, lookup_key, _stolen_key, _stolen_value); \
 		G_GNUC_END_IGNORE_DEPRECATIONS \
 	})
 #endif
diff --git a/src/nm-auth-utils.c b/src/nm-auth-utils.c
index 099fcfbfc3..ca2870db72 100644
--- a/src/nm-auth-utils.c
+++ b/src/nm-auth-utils.c
@@ -16,7 +16,7 @@
 
 /*****************************************************************************/
 
-struct NMAuthChain {
+struct _NMAuthChain {
 
 	CList parent_lst;
 
@@ -266,6 +266,14 @@ nm_auth_chain_get_subject (NMAuthChain *self)
 	return self->subject;
 }
 
+GDBusMethodInvocation *
+nm_auth_chain_get_context (NMAuthChain *self)
+{
+	g_return_val_if_fail (self, NULL);
+
+	return self->context;
+}
+
 /*****************************************************************************/
 
 static void
diff --git a/src/nm-auth-utils.h b/src/nm-auth-utils.h
index 808d20fdb6..978d607aed 100644
--- a/src/nm-auth-utils.h
+++ b/src/nm-auth-utils.h
@@ -12,7 +12,7 @@
 
 /*****************************************************************************/
 
-typedef struct NMAuthChain NMAuthChain;
+typedef struct _NMAuthChain NMAuthChain;
 
 typedef void (*NMAuthChainResultFunc) (NMAuthChain *chain,
                                        GDBusMethodInvocation *context,
@@ -53,6 +53,8 @@ void nm_auth_chain_destroy (NMAuthChain *chain);
 
 NMAuthSubject *nm_auth_chain_get_subject (NMAuthChain *self);
 
+GDBusMethodInvocation *nm_auth_chain_get_context (NMAuthChain *self);
+
 /*****************************************************************************/
 
 struct CList;
diff --git a/src/settings/nm-agent-manager.c b/src/settings/nm-agent-manager.c
index af6358a360..1519361314 100644
--- a/src/settings/nm-agent-manager.c
+++ b/src/settings/nm-agent-manager.c
@@ -35,15 +35,9 @@ typedef struct {
 	NMAuthManager *auth_mgr;
 	NMSessionMonitor *session_monitor;
 
-	/* Auth chains for checking agent permissions */
-	GSList *chains;
+	CList agent_lst_head;
 
-	/* Hashed by owner name, not identifier, since two agents in different
-	 * sessions can use the same identifier.
-	 */
-	GHashTable *agents;
-
-	CList requests;
+	CList request_lst_head;
 
 	guint64 agent_version_id;
 } NMAgentManagerPrivate;
@@ -119,6 +113,8 @@ static void _con_del_request_start (Request *req);
 
 static gboolean _con_get_try_complete_early (Request *req);
 
+static void agent_disconnected_cb (NMSecretAgent *agent, gpointer user_data);
+
 /*****************************************************************************/
 
 guint64
@@ -152,7 +148,7 @@ _request_type_to_string (RequestType request_type, gboolean verbose)
 /*****************************************************************************/
 
 struct _NMAgentManagerCallId {
-	CList lst_request;
+	CList request_lst;
 
 	NMAgentManager *self;
 
@@ -201,40 +197,73 @@ struct _NMAgentManagerCallId {
 
 /*****************************************************************************/
 
-static gboolean
-remove_agent (NMAgentManager *self, const char *owner)
+static NMSecretAgent *
+_agent_find_by_owner (NMAgentManagerPrivate *priv,
+                      const char *owner)
 {
-	NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self);
 	NMSecretAgent *agent;
-	CList *iter, *safe;
 
-	g_return_val_if_fail (owner != NULL, FALSE);
+	c_list_for_each_entry (agent, &priv->agent_lst_head, agent_lst) {
+		if (nm_streq (nm_secret_agent_get_dbus_owner (agent), owner))
+			return agent;
+	}
+	return NULL;
+}
 
-	/* Make sure this agent has already registered */
-	agent = g_hash_table_lookup (priv->agents, owner);
-	if (!agent)
-		return FALSE;
+static NMSecretAgent *
+_agent_find_by_identifier_and_uid (NMAgentManagerPrivate *priv,
+                                   const char *identifier,
+                                   gulong sender_uid)
+{
+	NMSecretAgent *agent;
+
+	c_list_for_each_entry (agent, &priv->agent_lst_head, agent_lst) {
+		if (    nm_streq0 (nm_secret_agent_get_identifier (agent), identifier)
+		    && sender_uid == nm_secret_agent_get_owner_uid (agent))
+			return agent;
+	}
+	return NULL;
+}
+
+/*****************************************************************************/
+
+static void
+_agent_remove (NMAgentManager *self, NMSecretAgent *agent)
+{
+	NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self);
+	CList *iter, *safe;
+
+	nm_assert (NM_IS_SECRET_AGENT (agent));
+	nm_assert (c_list_contains (&priv->agent_lst_head, &agent->agent_lst));
 
 	_LOGD (agent, "agent unregistered or disappeared");
 
+	nm_clear_pointer (&agent->auth_chain, nm_auth_chain_destroy);
+
+	c_list_unlink (&agent->agent_lst);
+
+	g_signal_handlers_disconnect_by_func (agent, G_CALLBACK (agent_disconnected_cb), self);
+
 	/* Remove this agent from any in-progress secrets requests */
-	c_list_for_each_safe (iter, safe, &priv->requests)
-		request_remove_agent (c_list_entry (iter, Request, lst_request), agent);
+	c_list_for_each_safe (iter, safe, &priv->request_lst_head)
+		request_remove_agent (c_list_entry (iter, Request, request_lst), agent);
 
-	/* And dispose of the agent */
-	g_hash_table_remove (priv->agents, owner);
-	return TRUE;
+	g_object_unref (agent);
 }
 
 /* Call this *after* calling request_next_agent() */
 static void
-maybe_remove_agent_on_error (NMSecretAgent *agent,
+maybe_remove_agent_on_error (NMAgentManager *self,
+                             NMSecretAgent *agent,
                              GError *error)
 {
-	if (   g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CLOSED)
-	    || g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_DISCONNECTED)
-	    || g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_NAME_HAS_NO_OWNER))
-		remove_agent (nm_agent_manager_get (), nm_secret_agent_get_dbus_owner (agent));
+	if (   !g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CLOSED)
+	    && !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_DISCONNECTED)
+	    && !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_NAME_HAS_NO_OWNER))
+		return;
+
+	if (!c_list_is_empty (&agent->agent_lst))
+		_agent_remove (self, agent);
 }
 
 /*****************************************************************************/
@@ -295,70 +324,92 @@ validate_identifier (const char *identifier, GError **error)
 }
 
 static void
-agent_register_permissions_done (NMAuthChain *chain,
-                                 GDBusMethodInvocation *context,
-                                 gpointer user_data)
+_agent_permissions_check_done (NMAuthChain *chain,
+                               GDBusMethodInvocation *context,
+                               gpointer user_data)
 {
 	NMAgentManager *self = NM_AGENT_MANAGER (user_data);
 	NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self);
 	NMSecretAgent *agent;
-	const char *sender;
-	NMAuthCallResult result;
-	CList *iter;
+	Request *request;
 
-	nm_assert (G_IS_DBUS_METHOD_INVOCATION (context));
-
-	priv->chains = g_slist_remove (priv->chains, chain);
+	nm_assert (!context || G_IS_DBUS_METHOD_INVOCATION (context));
 
 	agent = nm_auth_chain_steal_data (chain, "agent");
-	nm_assert (agent);
 
-	result = nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_WIFI_SHARE_PROTECTED);
-	if (result == NM_AUTH_CALL_RESULT_YES)
-		nm_secret_agent_add_permission (agent, NM_AUTH_PERMISSION_WIFI_SHARE_PROTECTED, TRUE);
+	nm_assert (NM_IS_SECRET_AGENT (agent));
+	nm_assert (agent->auth_chain == chain);
+	nm_assert (agent->fully_registered == (!context));
+	nm_assert (c_list_contains (&priv->agent_lst_head, &agent->agent_lst));
 
-	result = nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_WIFI_SHARE_OPEN);
-	if (result == NM_AUTH_CALL_RESULT_YES)
-		nm_secret_agent_add_permission (agent, NM_AUTH_PERMISSION_WIFI_SHARE_OPEN, TRUE);
+	agent->auth_chain = NULL;
+
+	nm_secret_agent_add_permission (agent,
+	                                NM_AUTH_PERMISSION_WIFI_SHARE_PROTECTED,
+	                                (nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_WIFI_SHARE_PROTECTED) == NM_AUTH_CALL_RESULT_YES));
+	nm_secret_agent_add_permission (agent,
+	                                NM_AUTH_PERMISSION_WIFI_SHARE_OPEN,
+	                                (nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_WIFI_SHARE_OPEN) == NM_AUTH_CALL_RESULT_YES));
+
+	if (agent->fully_registered) {
+		_LOGD (agent, "updated agent permissions");
+		return;
+	}
 
-	priv->agent_version_id += 1;
-	sender = nm_secret_agent_get_dbus_owner (agent);
-	g_hash_table_insert (priv->agents, g_strdup (sender), agent);
 	_LOGI (agent, "agent registered");
+
+	agent->fully_registered = TRUE;
+
+	priv->agent_version_id += 1;
+
 	g_dbus_method_invocation_return_value (context, NULL);
 
-	/* Signal an agent was registered */
-	g_signal_emit (self, signals[AGENT_REGISTERED], 0, agent);
+	c_list_for_each_entry (request, &priv->request_lst_head, request_lst)
+		request_add_agent (request, agent);
 
-	/* Add this agent to any in-progress secrets requests */
-	c_list_for_each (iter, &priv->requests)
-		request_add_agent (c_list_entry (iter, Request, lst_request), agent);
+	g_signal_emit (self, signals[AGENT_REGISTERED], 0, agent);
 }
 
-static NMSecretAgent *
-find_agent_by_identifier_and_uid (NMAgentManager *self,
-                                  const char *identifier,
-                                  gulong sender_uid)
+static NMAuthChain *
+_agent_create_auth_chain (NMAgentManager *self,
+                          NMSecretAgent *agent,
+                          GDBusMethodInvocation *context)
 {
-	NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self);
-	GHashTableIter iter;
-	NMSecretAgent *agent;
+	NMAuthChain *chain;
 
-	g_hash_table_iter_init (&iter, priv->agents);
-	while (g_hash_table_iter_next (&iter, NULL, (gpointer) &agent)) {
-		if (   g_strcmp0 (nm_secret_agent_get_identifier (agent), identifier) == 0
-		    && nm_secret_agent_get_owner_uid (agent) == sender_uid)
-			return agent;
+	_LOGD (agent, "requesting permissions");
+
+	nm_assert (   !agent->auth_chain
+	           || (agent->fully_registered == (!nm_auth_chain_get_context (agent->auth_chain))));
+
+	if (   agent->auth_chain
+	    && !context
+	    && !agent->fully_registered) {
+		/* we restart the authorization check (without a @context), but the currently
+		 * pending auth-chain carries a context. We need to pass it on as we replace
+		 * the auth-chain. */
+		context = nm_auth_chain_get_context (agent->auth_chain);
+		nm_assert (context);
 	}
-	return NULL;
+
+	chain = nm_auth_chain_new_subject (nm_secret_agent_get_subject (agent),
+	                                   context,
+	                                   _agent_permissions_check_done,
+	                                   self);
+
+	nm_auth_chain_set_data (chain, "agent", agent, NULL);
+	nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_WIFI_SHARE_PROTECTED, FALSE);
+	nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_WIFI_SHARE_OPEN, FALSE);
+
+	nm_clear_pointer (&agent->auth_chain, nm_auth_chain_destroy);
+	agent->auth_chain = chain;
+	return chain;
 }
 
 static void
 agent_disconnected_cb (NMSecretAgent *agent, gpointer user_data)
 {
-	/* The agent quit, so remove it and let interested clients know */
-	remove_agent (NM_AGENT_MANAGER (user_data),
-	              nm_secret_agent_get_dbus_owner (agent));
+	_agent_remove (NM_AGENT_MANAGER (user_data), agent);
 }
 
 static void
@@ -368,65 +419,44 @@ agent_manager_register_with_capabilities (NMAgentManager *self,
                                           guint32 capabilities)
 {
 	NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self);
-	NMAuthSubject *subject;
+	gs_unref_object NMAuthSubject *subject = NULL;
 	gulong sender_uid = G_MAXULONG;
 	GError *error = NULL;
 	NMSecretAgent *agent;
-	NMAuthChain *chain;
 
 	subject = nm_dbus_manager_new_auth_subject_from_context (context);
 	if (!subject) {
 		error = g_error_new_literal (NM_AGENT_MANAGER_ERROR,
 		                             NM_AGENT_MANAGER_ERROR_PERMISSION_DENIED,
 		                             NM_UTILS_ERROR_MSG_REQ_UID_UKNOWN);
-		goto done;
+		g_dbus_method_invocation_take_error (context, error);
+		return;
 	}
 	sender_uid = nm_auth_subject_get_unix_process_uid (subject);
 
 	/* Validate the identifier */
-	if (!validate_identifier (identifier, &error))
-		goto done;
+	if (!validate_identifier (identifier, &error)) {
+		g_dbus_method_invocation_take_error (context, error);
+		return;
+	}
 
 	/* Only one agent for each identifier is allowed per user */
-	if (find_agent_by_identifier_and_uid (self, identifier, sender_uid)) {
+	if (_agent_find_by_identifier_and_uid (priv, identifier, sender_uid)) {
 		error = g_error_new_literal (NM_AGENT_MANAGER_ERROR,
 		                             NM_AGENT_MANAGER_ERROR_PERMISSION_DENIED,
 		                             "An agent with this ID is already registered for this user.");
-		goto done;
+		g_dbus_method_invocation_take_error (context, error);
+		return;
 	}
 
-	/* Success, add the new agent */
 	agent = nm_secret_agent_new (context, subject, identifier, capabilities);
-	if (!agent) {
-		error = g_error_new_literal (NM_AGENT_MANAGER_ERROR,
-		                             NM_AGENT_MANAGER_ERROR_FAILED,
-		                             "Failed to initialize the agent");
-		goto done;
-	}
+
 	g_signal_connect (agent, NM_SECRET_AGENT_DISCONNECTED,
 	                  G_CALLBACK (agent_disconnected_cb), self);
 
-	_LOGD (agent, "requesting permissions");
-
-	/* Kick off permissions requests for this agent */
-	chain = nm_auth_chain_new_subject (subject, context, agent_register_permissions_done, self);
-	if (chain) {
-		nm_auth_chain_set_data (chain, "agent", agent, g_object_unref);
-		nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_WIFI_SHARE_PROTECTED, FALSE);
-		nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_WIFI_SHARE_OPEN, FALSE);
+	c_list_link_tail (&priv->agent_lst_head, &agent->agent_lst);
 
-		priv->chains = g_slist_append (priv->chains, chain);
-	} else {
-		g_object_unref (agent);
-		error = g_error_new_literal (NM_AGENT_MANAGER_ERROR,
-		                             NM_AGENT_MANAGER_ERROR_FAILED,
-		                             "Unable to start agent authentication.");
-	}
-
-done:
-	if (error)
-		g_dbus_method_invocation_take_error (context, error);
-	g_clear_object (&subject);
+	_agent_create_auth_chain (self, agent, context);
 }
 
 static void
@@ -470,8 +500,11 @@ impl_agent_manager_unregister (NMDBusObject *obj,
                                GVariant *parameters)
 {
 	NMAgentManager *self = NM_AGENT_MANAGER (obj);
+	NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self);
+	NMSecretAgent *agent;
 
-	if (!remove_agent (self, sender)) {
+	agent = _agent_find_by_owner (priv, sender);
+	if (!agent) {
 		g_dbus_method_invocation_return_error_literal (invocation,
 		                                               NM_AGENT_MANAGER_ERROR,
 		                                               NM_AGENT_MANAGER_ERROR_NOT_REGISTERED,
@@ -479,6 +512,8 @@ impl_agent_manager_unregister (NMDBusObject *obj,
 		return;
 	}
 
+	_agent_remove (self, agent);
+
 	g_dbus_method_invocation_return_value (invocation, NULL);
 }
 
@@ -497,7 +532,7 @@ request_new (NMAgentManager *self,
 	req->request_type = request_type;
 	req->detail = g_strdup (detail);
 	req->subject = g_object_ref (subject);
-	c_list_link_tail (&NM_AGENT_MANAGER_GET_PRIVATE (self)->requests, &req->lst_request);
+	c_list_link_tail (&NM_AGENT_MANAGER_GET_PRIVATE (self)->request_lst_head, &req->request_lst);
 	return req;
 }
 
@@ -583,7 +618,7 @@ req_complete_cancel (Request *req, gboolean is_disposing)
 	gs_free_error GError *error = NULL;
 
 	nm_assert (req && req->self);
-	nm_assert (!c_list_contains (&NM_AGENT_MANAGER_GET_PRIVATE (req->self)->requests, &req->lst_request));
+	nm_assert (!c_list_contains (&NM_AGENT_MANAGER_GET_PRIVATE (req->self)->request_lst_head, &req->request_lst));
 
 	nm_utils_error_set_cancelled (&error, is_disposing, "NMAgentManager");
 	req_complete_release (req, NULL, NULL, NULL, error);
@@ -598,9 +633,9 @@ req_complete (Request *req,
 {
 	NMAgentManager *self = req->self;
 
-	nm_assert (c_list_contains (&NM_AGENT_MANAGER_GET_PRIVATE (self)->requests, &req->lst_request));
+	nm_assert (c_list_contains (&NM_AGENT_MANAGER_GET_PRIVATE (self)->request_lst_head, &req->request_lst));
 
-	c_list_unlink (&req->lst_request);
+	c_list_unlink (&req->request_lst);
 
 	req_complete_release (req, secrets, agent_dbus_owner, agent_username, error);
 }
@@ -710,12 +745,12 @@ static void
 request_add_agents (NMAgentManager *self, Request *req)
 {
 	NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self);
-	GHashTableIter iter;
-	gpointer data;
+	NMSecretAgent *agent;
 
-	g_hash_table_iter_init (&iter, priv->agents);
-	while (g_hash_table_iter_next (&iter, NULL, &data))
-		request_add_agent (req, NM_SECRET_AGENT (data));
+	c_list_for_each_entry (agent, &priv->agent_lst_head, agent_lst) {
+		if (agent->fully_registered)
+			request_add_agent (req, agent);
+	}
 }
 
 static void
@@ -868,7 +903,7 @@ _con_get_request_done (NMSecretAgent *agent,
 
 			/* Try the next agent */
 			request_next_agent (req);
-			maybe_remove_agent_on_error (agent, error);
+			maybe_remove_agent_on_error (self, agent, error);
 		}
 		return;
 	}
@@ -1037,7 +1072,7 @@ _con_get_request_start (Request *req)
 		                                            NULL,
 		                                            _con_get_request_start_validated,
 		                                            req);
-		g_assert (req->con.chain);
+		nm_assert (req->con.chain);
 
 		/* If the caller is the only user in the connection's permissions, then
 		 * we use the 'modify.own' permission instead of 'modify.system'.  If the
@@ -1188,7 +1223,6 @@ nm_agent_manager_get_secrets (NMAgentManager *self,
 	req->con.get.callback = callback;
 	req->con.get.callback_data = callback_data;
 
-	/* Kick off the request */
 	if (!(req->con.get.flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_ONLY_SYSTEM))
 		request_add_agents (self, req);
 	req->idle_id = g_idle_add (request_start, req);
@@ -1203,9 +1237,9 @@ nm_agent_manager_cancel_secrets (NMAgentManager *self,
 	g_return_if_fail (request_id);
 	g_return_if_fail (request_id->request_type == REQUEST_TYPE_CON_GET);
 
-	nm_assert (c_list_contains (&NM_AGENT_MANAGER_GET_PRIVATE (self)->requests, &request_id->lst_request));
+	nm_assert (c_list_contains (&NM_AGENT_MANAGER_GET_PRIVATE (self)->request_lst_head, &request_id->request_lst));
 
-	c_list_unlink (&request_id->lst_request);
+	c_list_unlink (&request_id->request_lst);
 
 	req_complete_cancel (request_id, FALSE);
 }
@@ -1242,7 +1276,7 @@ _con_save_request_done (NMSecretAgent *agent,
 		       LOG_REQ_ARG (req), error->message);
 		/* Try the next agent */
 		request_next_agent (req);
-		maybe_remove_agent_on_error (agent, error);
+		maybe_remove_agent_on_error (self, agent, error);
 		return;
 	}
 
@@ -1291,7 +1325,6 @@ nm_agent_manager_save_secrets (NMAgentManager *self,
 	req->con.path = g_strdup (path);
 	req->con.connection = g_object_ref (connection);
 
-	/* Kick off the request */
 	request_add_agents (self, req);
 	req->idle_id = g_idle_add (request_start, req);
 }
@@ -1333,7 +1366,7 @@ _con_del_request_done (NMSecretAgent *agent,
 	/* Tell the next agent to delete secrets */
 	request_next_agent (req);
 	if (error)
-		maybe_remove_agent_on_error (agent, error);
+		maybe_remove_agent_on_error (self, agent, error);
 }
 
 static void
@@ -1376,27 +1409,36 @@ nm_agent_manager_delete_secrets (NMAgentManager *self,
 	req->con.connection = g_object_ref (connection);
 	g_object_unref (subject);
 
-	/* Kick off the request */
 	request_add_agents (self, req);
 	req->idle_id = g_idle_add (request_start, req);
 }
 
 /*****************************************************************************/
 
-NMSecretAgent *
-nm_agent_manager_get_agent_by_user (NMAgentManager *self, const char *username)
+gboolean
+nm_agent_manager_has_agent_with_permission (NMAgentManager *self,
+                                            const char *username,
+                                            const char *permission)
 {
-	NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self);
-	GHashTableIter iter;
+	NMAgentManagerPrivate *priv;
 	NMSecretAgent *agent;
 
-	g_hash_table_iter_init (&iter, priv->agents);
-	while (g_hash_table_iter_next (&iter, NULL, (gpointer) &agent)) {
-		if (g_strcmp0 (nm_secret_agent_get_owner_username (agent), username) == 0)
-			return agent;
+	g_return_val_if_fail (NM_IS_AGENT_MANAGER (self), FALSE);
+	g_return_val_if_fail (username, FALSE);
+	g_return_val_if_fail (permission, FALSE);
+
+	priv = NM_AGENT_MANAGER_GET_PRIVATE (self);
+
+	c_list_for_each_entry (agent, &priv->agent_lst_head, agent_lst) {
+		if (!agent->fully_registered)
+			continue;
+		if (!nm_streq0 (nm_secret_agent_get_owner_username (agent), username))
+			continue;
+		if (nm_secret_agent_has_permission (agent, permission))
+			return TRUE;
 	}
 
-	return NULL;
+	return FALSE;
 }
 
 /*****************************************************************************/
@@ -1407,17 +1449,16 @@ nm_agent_manager_all_agents_have_capability (NMAgentManager *manager,
                                              NMSecretAgentCapabilities capability)
 {
 	NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (manager);
-	GHashTableIter iter;
 	NMSecretAgent *agent;
 	gboolean subject_is_unix_process = (nm_auth_subject_get_subject_type (subject) == NM_AUTH_SUBJECT_TYPE_UNIX_PROCESS);
 	gulong subject_uid = subject_is_unix_process ? nm_auth_subject_get_unix_process_uid (subject) : 0u;
 
-	g_hash_table_iter_init (&iter, priv->agents);
-	while (g_hash_table_iter_next (&iter, NULL, (gpointer) &agent)) {
+	c_list_for_each_entry (agent, &priv->agent_lst_head, agent_lst) {
+		if (!agent->fully_registered)
+			continue;
 		if (   subject_is_unix_process
 		    && nm_secret_agent_get_owner_uid (agent) != subject_uid)
 			continue;
-
 		if (!(nm_secret_agent_get_capabilities (agent) & capability))
 			return FALSE;
 	}
@@ -1428,58 +1469,13 @@ nm_agent_manager_all_agents_have_capability (NMAgentManager *manager,
 /*****************************************************************************/
 
 static void
-agent_permissions_changed_done (NMAuthChain *chain,
-                                GDBusMethodInvocation *context,
-                                gpointer user_data)
-{
-	NMAgentManager *self = NM_AGENT_MANAGER (user_data);
-	NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self);
-	NMSecretAgent *agent;
-	gboolean share_protected = FALSE, share_open = FALSE;
-
-	priv->chains = g_slist_remove (priv->chains, chain);
-
-	agent = nm_auth_chain_get_data (chain, "agent");
-	g_assert (agent);
-
-	_LOGD (agent, "updated agent permissions");
-
-	if (nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_WIFI_SHARE_PROTECTED) == NM_AUTH_CALL_RESULT_YES)
-		share_protected = TRUE;
-	if (nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_WIFI_SHARE_OPEN) == NM_AUTH_CALL_RESULT_YES)
-		share_open = TRUE;
-
-	nm_secret_agent_add_permission (agent, NM_AUTH_PERMISSION_WIFI_SHARE_PROTECTED, share_protected);
-	nm_secret_agent_add_permission (agent, NM_AUTH_PERMISSION_WIFI_SHARE_OPEN, share_open);
-}
-
-static void
 authority_changed_cb (NMAuthManager *auth_manager, NMAgentManager *self)
 {
 	NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self);
-	GHashTableIter iter;
 	NMSecretAgent *agent;
 
-	/* Recheck the permissions of all secret agents */
-	g_hash_table_iter_init (&iter, priv->agents);
-	while (g_hash_table_iter_next (&iter, NULL, (gpointer) &agent)) {
-		NMAuthChain *chain;
-
-		/* Kick off permissions requests for this agent */
-		chain = nm_auth_chain_new_subject (nm_secret_agent_get_subject (agent),
-		                                   NULL,
-		                                   agent_permissions_changed_done,
-		                                   self);
-		g_assert (chain);
-		priv->chains = g_slist_append (priv->chains, chain);
-
-		/* Make sure if the agent quits while the permissions call is in progress
-		 * that the object sticks around until our callback.
-		 */
-		nm_auth_chain_set_data (chain, "agent", g_object_ref (agent), g_object_unref);
-		nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_WIFI_SHARE_PROTECTED, FALSE);
-		nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_WIFI_SHARE_OPEN, FALSE);
-	}
+	c_list_for_each_entry (agent, &priv->agent_lst_head, agent_lst)
+		_agent_create_auth_chain (self, agent, NULL);
 }
 
 /*****************************************************************************/
@@ -1490,8 +1486,8 @@ nm_agent_manager_init (NMAgentManager *self)
 	NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self);
 
 	priv->agent_version_id = 1;
-	c_list_init (&priv->requests);
-	priv->agents = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, g_object_unref);
+	c_list_init (&priv->agent_lst_head);
+	c_list_init (&priv->request_lst_head);
 }
 
 static void
@@ -1515,23 +1511,18 @@ constructed (GObject *object)
 static void
 dispose (GObject *object)
 {
-	NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE ((NMAgentManager *) object);
-	CList *iter;
+	NMAgentManager *self = NM_AGENT_MANAGER (object);
+	NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self);
+	Request *request;
+	NMSecretAgent *agent;
 
-cancel_more:
-	c_list_for_each (iter, &priv->requests) {
-		c_list_unlink (iter);
-		req_complete_cancel (c_list_entry (iter, Request, lst_request), TRUE);
-		goto cancel_more;
+	while ((request = c_list_first_entry (&priv->request_lst_head, Request, request_lst))) {
+		c_list_unlink (&request->request_lst);
+		req_complete_cancel (request, TRUE);
 	}
 
-	g_slist_free_full (priv->chains, (GDestroyNotify) nm_auth_chain_destroy);
-	priv->chains = NULL;
-
-	if (priv->agents) {
-		g_hash_table_destroy (priv->agents);
-		priv->agents = NULL;
-	}
+	while ((agent = c_list_first_entry (&priv->agent_lst_head, NMSecretAgent, agent_lst)))
+		_agent_remove (self, agent);
 
 	if (priv->auth_mgr) {
 		g_signal_handlers_disconnect_by_func (priv->auth_mgr,
diff --git a/src/settings/nm-agent-manager.h b/src/settings/nm-agent-manager.h
index 5200d241ca..bf2dcb6ccf 100644
--- a/src/settings/nm-agent-manager.h
+++ b/src/settings/nm-agent-manager.h
@@ -65,8 +65,9 @@ void nm_agent_manager_delete_secrets (NMAgentManager *manager,
                                       const char *path,
                                       NMConnection *connection);
 
-NMSecretAgent *nm_agent_manager_get_agent_by_user (NMAgentManager *manager,
-                                                   const char *username);
+gboolean nm_agent_manager_has_agent_with_permission (NMAgentManager *self,
+                                                     const char *username,
+                                                     const char *permission);
 
 gboolean nm_agent_manager_all_agents_have_capability (NMAgentManager *manager,
                                                       NMAuthSubject *subject,
diff --git a/src/settings/nm-secret-agent.c b/src/settings/nm-secret-agent.c
index 82a8c904a5..e5c727b49f 100644
--- a/src/settings/nm-secret-agent.c
+++ b/src/settings/nm-secret-agent.c
@@ -34,33 +34,28 @@ enum {
 
 static guint signals[LAST_SIGNAL] = { 0 };
 
-typedef struct {
+typedef struct _NMSecretAgentPrivate {
 	CList permissions;
+	CList requests;
+	GDBusConnection *dbus_connection;
 	char *description;
 	NMAuthSubject *subject;
 	char *identifier;
 	char *owner_username;
 	char *dbus_owner;
-	GDBusConnection *dbus_connection;
 	GCancellable *name_owner_cancellable;
-	CList requests;
-	NMSecretAgentCapabilities capabilities;
 	guint name_owner_changed_id;
+	NMSecretAgentCapabilities capabilities;
 	bool shutdown_wait_obj_registered:1;
 } NMSecretAgentPrivate;
 
-struct _NMSecretAgent {
-	GObject parent;
-	NMSecretAgentPrivate _priv;
-};
-
 struct _NMSecretAgentClass {
 	GObjectClass parent;
 };
 
 G_DEFINE_TYPE (NMSecretAgent, nm_secret_agent, G_TYPE_OBJECT)
 
-#define NM_SECRET_AGENT_GET_PRIVATE(self) _NM_GET_PRIVATE (self, NMSecretAgent, NM_IS_SECRET_AGENT)
+#define NM_SECRET_AGENT_GET_PRIVATE(self) _NM_GET_PRIVATE_PTR (self, NMSecretAgent, NM_IS_SECRET_AGENT)
 
 /*****************************************************************************/
 
@@ -765,8 +760,13 @@ nm_secret_agent_new (GDBusMethodInvocation *context,
 static void
 nm_secret_agent_init (NMSecretAgent *self)
 {
-	NMSecretAgentPrivate *priv = NM_SECRET_AGENT_GET_PRIVATE (self);
+	NMSecretAgentPrivate *priv;
 
+	priv = G_TYPE_INSTANCE_GET_PRIVATE (self, NM_TYPE_SECRET_AGENT, NMSecretAgentPrivate);
+
+	self->_priv = priv;
+
+	c_list_init (&self->agent_lst);
 	c_list_init (&priv->permissions);
 	c_list_init (&priv->requests);
 }
@@ -777,6 +777,8 @@ dispose (GObject *object)
 	NMSecretAgent *self = NM_SECRET_AGENT (object);
 	NMSecretAgentPrivate *priv = NM_SECRET_AGENT_GET_PRIVATE (self);
 
+	nm_assert (c_list_is_empty (&self->agent_lst));
+	nm_assert (!self->auth_chain);
 	nm_assert (c_list_is_empty (&priv->requests));
 
 	nm_clear_g_dbus_connection_signal (priv->dbus_connection,
@@ -814,6 +816,8 @@ nm_secret_agent_class_init (NMSecretAgentClass *config_class)
 {
 	GObjectClass *object_class = G_OBJECT_CLASS (config_class);
 
+	g_type_class_add_private (object_class, sizeof (NMSecretAgentPrivate));
+
 	object_class->dispose = dispose;
 	object_class->finalize = finalize;
 
diff --git a/src/settings/nm-secret-agent.h b/src/settings/nm-secret-agent.h
index ea86e432ad..37a93d1720 100644
--- a/src/settings/nm-secret-agent.h
+++ b/src/settings/nm-secret-agent.h
@@ -8,6 +8,8 @@
 
 #include "nm-connection.h"
 
+#include "c-list/src/c-list.h"
+
 #define NM_TYPE_SECRET_AGENT            (nm_secret_agent_get_type ())
 #define NM_SECRET_AGENT(obj)            (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_SECRET_AGENT, NMSecretAgent))
 #define NM_SECRET_AGENT_CLASS(klass)    (G_TYPE_CHECK_CLASS_CAST ((klass), NM_TYPE_SECRET_AGENT, NMSecretAgentClass))
@@ -20,6 +22,17 @@
 typedef struct _NMSecretAgentClass NMSecretAgentClass;
 typedef struct _NMSecretAgentCallId NMSecretAgentCallId;
 
+struct _NMAuthChain;
+struct _NMSecretAgentPrivate;
+
+struct _NMSecretAgent {
+	GObject parent;
+	CList agent_lst;
+	struct _NMAuthChain *auth_chain;
+	struct _NMSecretAgentPrivate *_priv;
+	bool fully_registered:1;
+};
+
 GType nm_secret_agent_get_type (void);
 
 NMSecretAgent *nm_secret_agent_new (GDBusMethodInvocation *context,
diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c
index 3368359980..fd701ecde4 100644
--- a/src/settings/nm-settings-connection.c
+++ b/src/settings/nm-settings-connection.c
@@ -409,10 +409,7 @@ nm_settings_connection_check_permission (NMSettingsConnection *self,
 		 * either.
 		 */
 		if (nm_setting_connection_get_permission (s_con, i, NULL, &puser, NULL)) {
-			NMSecretAgent *agent = nm_agent_manager_get_agent_by_user (priv->agent_mgr, puser);
-
-			if (   agent
-			    && nm_secret_agent_has_permission (agent, permission))
+			if (nm_agent_manager_has_agent_with_permission (priv->agent_mgr, puser, permission))
 				return TRUE;
 		}
 	}