New upstream version 1.19.90 upstream/1.19.90

1 files changed, 257 insertions, 133 deletions

diff --git a/src/nm-auth-utils.c b/src/nm-auth-utils.c
index a0ad84c1..7235cba1 100644
--- a/src/nm-auth-utils.c
+++ b/src/nm-auth-utils.c
@@ -1,4 +1,3 @@
-/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
 /* NetworkManager -- Network link manager
  *
  * This program is free software; you can redistribute it and/or modify
@@ -31,7 +30,10 @@
 /*****************************************************************************/
 
 struct NMAuthChain {
-	GHashTable *data_hash;
+
+	CList parent_lst;
+
+	CList data_lst_head;
 
 	CList auth_call_lst_head;
 
@@ -41,26 +43,49 @@ struct NMAuthChain {
 	NMAuthChainResultFunc done_func;
 	gpointer user_data;
 
-	guint32 refcount;
+	guint num_pending_auth_calls;
 
-	bool done:1;
+	bool is_destroyed:1;
+	bool is_finishing:1;
 };
 
+G_STATIC_ASSERT (G_STRUCT_OFFSET (NMAuthChain, parent_lst) == 0);
+
 typedef struct {
 	CList auth_call_lst;
 	NMAuthChain *chain;
 	NMAuthManagerCallId *call_id;
-	char *permission;
+	const char *permission;
+	NMAuthCallResult result;
 } AuthCall;
 
 /*****************************************************************************/
 
+static void _auth_chain_destroy (NMAuthChain *self);
+
+/*****************************************************************************/
+
 static void
 _ASSERT_call (AuthCall *call)
 {
 	nm_assert (call);
 	nm_assert (call->chain);
+	nm_assert (call->permission && strlen (call->permission) > 0);
 	nm_assert (nm_c_list_contains_entry (&call->chain->auth_call_lst_head, call, auth_call_lst));
+#if NM_MORE_ASSERTS > 5
+	{
+		AuthCall *auth_call;
+		guint n = 0;
+
+		c_list_for_each_entry (auth_call, &call->chain->auth_call_lst_head, auth_call_lst) {
+			nm_assert (   auth_call->result == NM_AUTH_CALL_RESULT_UNKNOWN
+			           || !auth_call->call_id);
+			if (auth_call->call_id)
+				n++;
+		}
+		nm_assert (n == call->chain->num_pending_auth_calls);
+	}
+#endif
 }
 
 /*****************************************************************************/
@@ -68,67 +93,68 @@ _ASSERT_call (AuthCall *call)
 static void
 auth_call_free (AuthCall *call)
 {
-	if (call->call_id)
-		nm_auth_manager_check_authorization_cancel (call->call_id);
+	_ASSERT_call (call);
+
 	c_list_unlink_stale (&call->auth_call_lst);
-	g_free (call->permission);
+	if (call->call_id) {
+		call->chain->num_pending_auth_calls--;
+		nm_auth_manager_check_authorization_cancel (call->call_id);
+	}
 	g_slice_free (AuthCall, call);
 }
 
+static AuthCall *
+_find_auth_call (NMAuthChain *self, const char *permission)
+{
+	AuthCall *auth_call;
+
+	c_list_for_each_entry (auth_call, &self->auth_call_lst_head, auth_call_lst) {
+		if (nm_streq (auth_call->permission, permission))
+			return auth_call;
+	}
+	return NULL;
+}
+
 /*****************************************************************************/
 
 typedef struct {
-
-	/* must be the first field. */
+	CList data_lst;
 	const char *tag;
-
 	gpointer data;
 	GDestroyNotify destroy;
-	char tag_data[];
 } ChainData;
 
-static ChainData *
-chain_data_new (const char *tag, gpointer data, GDestroyNotify destroy)
-{
-	ChainData *tmp;
-	gsize l = strlen (tag);
-
-	tmp = g_malloc (sizeof (ChainData) + l + 1);
-	tmp->tag = &tmp->tag_data[0];
-	tmp->data = data;
-	tmp->destroy = destroy;
-	memcpy (&tmp->tag_data[0], tag, l + 1);
-	return tmp;
-}
-
 static void
-chain_data_free (gpointer data)
+chain_data_free (ChainData *chain_data)
 {
-	ChainData *tmp = data;
-
-	if (tmp->destroy)
-		tmp->destroy (tmp->data);
-	g_free (tmp);
+	c_list_unlink_stale (&chain_data->data_lst);
+	if (chain_data->destroy)
+		chain_data->destroy (chain_data->data);
+	g_slice_free (ChainData, chain_data);
 }
 
-static gpointer
+static ChainData *
 _get_data (NMAuthChain *self, const char *tag)
 {
-	ChainData *tmp;
+	ChainData *chain_data;
 
-	if (!self->data_hash)
-		return NULL;
-	tmp = g_hash_table_lookup (self->data_hash, &tag);
-	return tmp ? tmp->data : NULL;
+	c_list_for_each_entry (chain_data, &self->data_lst_head, data_lst) {
+		if (nm_streq (chain_data->tag, tag))
+			return chain_data;
+	}
+	return NULL;
 }
 
 gpointer
 nm_auth_chain_get_data (NMAuthChain *self, const char *tag)
 {
+	ChainData *chain_data;
+
 	g_return_val_if_fail (self, NULL);
 	g_return_val_if_fail (tag, NULL);
 
-	return _get_data (self, tag);
+	chain_data = _get_data (self, tag);
+	return chain_data ? chain_data->data : NULL;
 }
 
 /**
@@ -145,47 +171,77 @@ nm_auth_chain_get_data (NMAuthChain *self, const char *tag)
 gpointer
 nm_auth_chain_steal_data (NMAuthChain *self, const char *tag)
 {
-	ChainData *tmp;
-	gpointer value = NULL;
+	ChainData *chain_data;
+	gpointer value;
 
 	g_return_val_if_fail (self, NULL);
 	g_return_val_if_fail (tag, NULL);
 
-	if (!self->data_hash)
-		return NULL;
-
-	tmp = g_hash_table_lookup (self->data_hash, &tag);
-	if (!tmp)
+	chain_data = _get_data (self, tag);
+	if (!chain_data)
 		return NULL;
 
-	value = tmp->data;
+	value = chain_data->data;
 
 	/* Make sure the destroy handler isn't called when freeing */
-	tmp->destroy = NULL;
-	g_hash_table_remove (self->data_hash, tmp);
+	chain_data->destroy = NULL;
+	chain_data_free (chain_data);
 	return value;
 }
 
+/**
+ * nm_auth_chain_set_data_unsafe:
+ * @self: the #NMAuthChain
+ * @tag: the tag for referencing the attached data.
+ * @data: the data to attach. If %NULL, this call has no effect
+ *   and nothing is attached.
+ * @data_destroy: (allow-none): the destroy function for the data pointer.
+ *
+ * @tag string is not cloned and must outlife @self. That is why
+ * the function is "unsafe". Use nm_auth_chain_set_data() with a C literal
+ * instead.
+ *
+ * It is a bug to add the same tag more than once.
+ */
 void
-nm_auth_chain_set_data (NMAuthChain *self,
-                        const char *tag,
-                        gpointer data,
-                        GDestroyNotify data_destroy)
+nm_auth_chain_set_data_unsafe (NMAuthChain *self,
+                               const char *tag,
+                               gpointer data,
+                               GDestroyNotify data_destroy)
 {
+	ChainData *chain_data;
+
 	g_return_if_fail (self);
 	g_return_if_fail (tag);
 
-	if (data == NULL) {
-		if (self->data_hash)
-			g_hash_table_remove (self->data_hash, &tag);
-	} else {
-		if (!self->data_hash) {
-			self->data_hash = g_hash_table_new_full (nm_pstr_hash, nm_pstr_equal,
-			                                         NULL, chain_data_free);
-		}
-		g_hash_table_add (self->data_hash,
-		                  chain_data_new (tag, data, data_destroy));
+	/* we should not track a large number of elements via a linked list. If this becomes
+	 * necessary, revert the code to use GHashTable again. */
+	nm_assert (c_list_length (&self->data_lst_head) < 25);
+
+	/* The tag must not yet exist. Otherwise we'd have to first search the linked
+	 * list for an existing entry. */
+	nm_assert (!_get_data (self, tag));
+
+	if (!data) {
+		/* we don't track user data of %NULL.
+		 *
+		 * In the past this had also the meaning of removing a user-data. But since
+		 * nm_auth_chain_set_data() does not allow being called more than once
+		 * for the same tag, we don't need to remove anything. */
+		return;
 	}
+
+	chain_data = g_slice_new (ChainData);
+	*chain_data = (ChainData) {
+		.tag     = tag,
+		.data    = data,
+		.destroy = data_destroy,
+	};
+
+	/* we assert that no duplicate tags are added. But still, add the new
+	 * element to the front, so that it would shadow the duplicate element
+	 * in the list. */
+	c_list_link_front (&self->data_lst_head, &chain_data->data_lst);
 }
 
 /*****************************************************************************/
@@ -193,13 +249,26 @@ nm_auth_chain_set_data (NMAuthChain *self,
 NMAuthCallResult
 nm_auth_chain_get_result (NMAuthChain *self, const char *permission)
 {
-	gpointer data;
+	AuthCall *auth_call;
 
 	g_return_val_if_fail (self, NM_AUTH_CALL_RESULT_UNKNOWN);
 	g_return_val_if_fail (permission, NM_AUTH_CALL_RESULT_UNKNOWN);
 
-	data = _get_data (self, permission);
-	return data ? GPOINTER_TO_UINT (data) : NM_AUTH_CALL_RESULT_UNKNOWN;
+	/* it is a bug to request the result other than from the done_func()
+	 * callback. You are not supposed to poll for the result but request
+	 * it upon notification. */
+	nm_assert (self->is_finishing);
+
+	auth_call = _find_auth_call (self, permission);
+
+	/* it is a bug to request a permission result that was not
+	 * previously requested or which did not complete yet. */
+	if (!auth_call)
+		g_return_val_if_reached (NM_AUTH_CALL_RESULT_UNKNOWN);
+
+	nm_assert (!auth_call->call_id);
+
+	return auth_call->result;
 }
 
 NMAuthSubject *
@@ -212,39 +281,6 @@ nm_auth_chain_get_subject (NMAuthChain *self)
 
 /*****************************************************************************/
 
-static gboolean
-auth_chain_finish (NMAuthChain *self)
-{
-	self->done = TRUE;
-
-	/* Ensure we stay alive across the callback */
-	nm_assert (self->refcount == 1);
-	self->refcount++;
-	self->done_func (self, NULL, self->context, self->user_data);
-	nm_assert (NM_IN_SET (self->refcount, 1, 2));
-	nm_auth_chain_destroy (self);
-	return FALSE;
-}
-
-static void
-auth_call_complete (AuthCall *call)
-{
-	NMAuthChain *self;
-
-	_ASSERT_call (call);
-
-	self = call->chain;
-
-	nm_assert (!self->done);
-
-	auth_call_free (call);
-
-	if (c_list_is_empty (&self->auth_call_lst_head)) {
-		/* we are on an idle-handler or a clean call-stack (non-reentrant). */
-		auth_chain_finish (self);
-	}
-}
-
 static void
 pk_call_cb (NMAuthManager *auth_manager,
             NMAuthManagerCallId *call_id,
@@ -253,49 +289,110 @@ pk_call_cb (NMAuthManager *auth_manager,
             GError *error,
             gpointer user_data)
 {
+	NMAuthChain *self;
 	AuthCall *call;
-	NMAuthCallResult call_result;
+
+	nm_assert (call_id);
 
 	if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
 		return;
 
 	call = user_data;
 
+	_ASSERT_call (call);
 	nm_assert (call->call_id == call_id);
+	nm_assert (call->result == NM_AUTH_CALL_RESULT_UNKNOWN);
+
+	self = call->chain;
+
+	nm_assert (!self->is_destroyed);
+	nm_assert (!self->is_finishing);
 
 	call->call_id = NULL;
 
-	call_result = nm_auth_call_result_eval (is_authorized, is_challenge, error);
+	call->result = nm_auth_call_result_eval (is_authorized, is_challenge, error);
+
+	call->chain->num_pending_auth_calls--;
 
-	nm_auth_chain_set_data (call->chain, call->permission, GUINT_TO_POINTER (call_result), NULL);
+	_ASSERT_call (call);
 
-	auth_call_complete (call);
+	if (call->chain->num_pending_auth_calls == 0) {
+		/* we are on an idle-handler or a clean call-stack (non-reentrant) so it's safe
+		 * to invoke the callback right away. */
+		self->is_finishing = TRUE;
+		self->done_func (self, self->context, self->user_data);
+		nm_assert (self->is_finishing);
+		_auth_chain_destroy (self);
+	}
 }
 
+/**
+ * nm_auth_chain_add_call_unsafe:
+ * @self: the #NMAuthChain
+ * @permission: the permission string. This string is kept by reference
+ *   and you must make sure that it's lifetime lasts until the NMAuthChain
+ *   gets destroyed. That's why the function is "unsafe". Use
+ *   nm_auth_chain_add_call() instead.
+ * @allow_interaction: flag
+ *
+ * It's "unsafe" because @permission is not copied. It's the callers responsibility
+ * that the permission string stays valid as long as NMAuthChain.
+ *
+ * If you can, use nm_auth_chain_add_call() instead!
+ *
+ * If you have a non-static string, you may attach the permission string as
+ * user-data via nm_auth_chain_set_data().
+ */
 void
-nm_auth_chain_add_call (NMAuthChain *self,
-                        const char *permission,
-                        gboolean allow_interaction)
+nm_auth_chain_add_call_unsafe (NMAuthChain *self,
+                               const char *permission,
+                               gboolean allow_interaction)
 {
 	AuthCall *call;
-	NMAuthManager *auth_manager = nm_auth_manager_get ();
 
 	g_return_if_fail (self);
 	g_return_if_fail (self->subject);
-	g_return_if_fail (!self->done);
+	g_return_if_fail (!self->is_finishing);
+	g_return_if_fail (!self->is_destroyed);
 	g_return_if_fail (permission && *permission);
-	g_return_if_fail (nm_auth_subject_is_unix_process (self->subject) || nm_auth_subject_is_internal (self->subject));
+	nm_assert (   nm_auth_subject_is_unix_process (self->subject)
+	           || nm_auth_subject_is_internal (self->subject));
+
+	/* duplicate permissions are not supported, also because nm_auth_chain_get_result()
+	 * can only return one-permission. */
+	nm_assert (!_find_auth_call (self, permission));
+
+	call = g_slice_new (AuthCall);
 
-	call = g_slice_new0 (AuthCall);
-	call->chain = self;
-	call->permission = g_strdup (permission);
-	c_list_link_tail (&self->auth_call_lst_head, &call->auth_call_lst);
-	call->call_id = nm_auth_manager_check_authorization (auth_manager,
+	*call = (AuthCall) {
+		.chain      = self,
+		.call_id    = NULL,
+		.result     = NM_AUTH_CALL_RESULT_UNKNOWN,
+
+		/* we don't clone the permission string. It's the callers responsiblity. */
+		.permission = permission,
+	};
+
+	/* above we assert that no duplicate permissions are added. Still, track the
+	 * new request to the front of the list so that it would shadow an earlier
+	 * call. */
+	c_list_link_front (&self->auth_call_lst_head, &call->auth_call_lst);
+
+	call->call_id = nm_auth_manager_check_authorization (nm_auth_manager_get (),
 	                                                     self->subject,
 	                                                     permission,
 	                                                     allow_interaction,
 	                                                     pk_call_cb,
 	                                                     call);
+
+	self->num_pending_auth_calls++;
+
+	_ASSERT_call (call);
+
+	/* we track auth-calls in a linked list. If we end up requesting too many permissions this
+	 * becomes inefficient. If that ever happens, consider a more efficient data structure for
+	 * a large number of requests. */
+	nm_assert (self->num_pending_auth_calls < 25);
 }
 
 /*****************************************************************************/
@@ -310,6 +407,7 @@ nm_auth_chain_new_context (GDBusMethodInvocation *context,
 	NMAuthChain *chain;
 
 	g_return_val_if_fail (context, NULL);
+	nm_assert (done_func);
 
 	subject = nm_auth_subject_new_unix_process_from_context (context);
 	if (!subject)
@@ -323,7 +421,6 @@ nm_auth_chain_new_context (GDBusMethodInvocation *context,
 	return chain;
 }
 
-/* Requires an NMAuthSubject */
 NMAuthChain *
 nm_auth_chain_new_subject (NMAuthSubject *subject,
                            GDBusMethodInvocation *context,
@@ -333,15 +430,20 @@ nm_auth_chain_new_subject (NMAuthSubject *subject,
 	NMAuthChain *self;
 
 	g_return_val_if_fail (NM_IS_AUTH_SUBJECT (subject), NULL);
-	nm_assert (nm_auth_subject_is_unix_process (subject) || nm_auth_subject_is_internal (subject));
-
-	self = g_slice_new0 (NMAuthChain);
-	c_list_init (&self->auth_call_lst_head);
-	self->refcount = 1;
-	self->done_func = done_func;
-	self->user_data = user_data;
-	self->context = context ? g_object_ref (context) : NULL;
-	self->subject = g_object_ref (subject);
+	nm_assert (   nm_auth_subject_is_unix_process (subject)
+	           || nm_auth_subject_is_internal (subject));
+	nm_assert (done_func);
+
+	self = g_slice_new (NMAuthChain);
+	*self = (NMAuthChain) {
+		.done_func          = done_func,
+		.user_data          = user_data,
+		.context            = nm_g_object_ref (context),
+		.subject            = g_object_ref (subject),
+		.parent_lst         = C_LIST_INIT (self->parent_lst),
+		.data_lst_head      = C_LIST_INIT (self->data_lst_head),
+		.auth_call_lst_head = C_LIST_INIT (self->auth_call_lst_head),
+	};
 	return self;
 }
 
@@ -352,30 +454,51 @@ nm_auth_chain_new_subject (NMAuthSubject *subject,
  * Destroys the auth-chain. By destroying the auth-chain, you also cancel
  * the receipt of the done-callback. IOW, the callback will not be invoked.
  *
- * The only exception is, if may call nm_auth_chain_destroy() from inside
- * the callback. In this case, @self stays alive until the callback returns.
+ * The only exception is, you may call nm_auth_chain_destroy() from inside
+ * the callback. In this case the call has no effect and @self stays alive
+ * until the callback returns.
  *
  * Note that you might only destroy an auth-chain exactly once, and never
- * after the callback was handled.
+ * after the callback was handled. After the callback returns, the auth chain
+ * always gets automatically destroyed. So you only need to explicitly destroy
+ * it, if you want to abort it before the callback complets.
  */
 void
 nm_auth_chain_destroy (NMAuthChain *self)
 {
-	AuthCall *call;
-
 	g_return_if_fail (self);
-	g_return_if_fail (NM_IN_SET (self->refcount, 1, 2));
+	g_return_if_fail (!self->is_destroyed);
 
-	if (--self->refcount > 0)
+	self->is_destroyed = TRUE;
+
+	if (self->is_finishing) {
+		/* we are called from inside the callback. Keep the instance alive for the moment. */
 		return;
+	}
+
+	_auth_chain_destroy (self);
+}
+
+static void
+_auth_chain_destroy (NMAuthChain *self)
+{
+	AuthCall *call;
+	ChainData *chain_data;
+
+	c_list_unlink (&self->parent_lst);
 
 	nm_clear_g_object (&self->subject);
 	nm_clear_g_object (&self->context);
 
+	/* we must first destry all AuthCall instances before ChainData. The reason is
+	 * that AuthData.permission is not cloned and the lifetime of the string must
+	 * be ensured by the caller. A sensible thing to do for the caller is attach the
+	 * permission string via nm_auth_chain_set_data(). Hence, first free the AuthCall. */
 	while ((call = c_list_first_entry (&self->auth_call_lst_head, AuthCall, auth_call_lst)))
 		auth_call_free (call);
 
-	nm_clear_pointer (&self->data_hash, g_hash_table_destroy);
+	while ((chain_data = c_list_first_entry (&self->data_lst_head, ChainData, data_lst)))
+		chain_data_free (chain_data);
 
 	g_slice_free (NMAuthChain, self);
 }
@@ -395,7 +518,8 @@ nm_auth_is_subject_in_acl (NMConnection *connection,
 
 	g_return_val_if_fail (connection, FALSE);
 	g_return_val_if_fail (NM_IS_AUTH_SUBJECT (subject), FALSE);
-	g_return_val_if_fail (nm_auth_subject_is_internal (subject) || nm_auth_subject_is_unix_process (subject), FALSE);
+	nm_assert (   nm_auth_subject_is_internal (subject)
+	           || nm_auth_subject_is_unix_process (subject));
 
 	if (nm_auth_subject_is_internal (subject))
 		return TRUE;