auth-chain: don't clone the permission string for AuthChain

Out of the 33 callers of nm_auth_chain_add_call(), the permission
argument is:

 - 29 times a C string literal like NM_AUTH_PERMISSION_NETWORK_CONTROL.

 - 3 times assign a string that is in fact a static string (it's just
   not a string literal)

 - only NMManager's device_auth_request_cb() passes a permission of
   (possibly) non static origin. But it already duplicates the string
   for it's own purposes and attaches it as user-data to the
   NMAuthChain.

There really is no need to duplicate the string.

Replace nm_auth_chain_add_call() by a macro that ensures that the
permission string is a C literal.

Rename nm_auth_chain_add_call() to nm_auth_chain_add_call_unsafe() to
indicate that the lifetime of the permission argument is now the
responsibility of the caller.

src/nm-auth-utils.c

@@ -51,8 +51,8 @@ typedef struct {
 	CList auth_call_lst;
 	NMAuthChain *chain;
 	NMAuthManagerCallId *call_id;
+	const char *permission;
 	NMAuthCallResult result;
-	char permission[];
 } AuthCall;
 
 /*****************************************************************************/
@@ -66,6 +66,7 @@ _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
 	{
@@ -321,14 +322,30 @@ pk_call_cb (NMAuthManager *auth_manager,
 	}
 }
 
+/**
+ * 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 ();
-	gsize l_p_1;
 
 	g_return_if_fail (self);
 	g_return_if_fail (self->subject);
@@ -342,12 +359,16 @@ nm_auth_chain_add_call (NMAuthChain *self,
 	 * can only return one-permission. */
 	nm_assert (!_find_auth_call (self, permission));
 
-	l_p_1 = strlen (permission) + 1;
-	call = g_malloc (sizeof (AuthCall) + l_p_1);
-	call->chain = self;
-	call->call_id = NULL;
-	call->result = NM_AUTH_CALL_RESULT_UNKNOWN;
-	memcpy (call->permission, permission, l_p_1);
+	call = g_slice_new (AuthCall);
+
+	*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
@@ -463,6 +484,10 @@ _auth_chain_destroy (NMAuthChain *self)
 	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);
 

src/nm-auth-utils.h

@@ -52,9 +52,12 @@ void nm_auth_chain_set_data (NMAuthChain *chain,
 NMAuthCallResult nm_auth_chain_get_result (NMAuthChain *chain,
                                            const char *permission);
 
-void nm_auth_chain_add_call (NMAuthChain *chain,
-                             const char *permission,
-                             gboolean allow_interaction);
+void nm_auth_chain_add_call_unsafe (NMAuthChain *chain,
+                                    const char *permission,
+                                    gboolean allow_interaction);
+
+#define nm_auth_chain_add_call(chain, permission, allow_interaction) \
+	nm_auth_chain_add_call_unsafe ((chain), ""permission"", (allow_interaction))
 
 void nm_auth_chain_destroy (NMAuthChain *chain);
 

src/nm-manager.c

@@ -2385,6 +2385,7 @@ device_auth_request_cb (NMDevice *device,
 	GError *error = NULL;
 	NMAuthSubject *subject = NULL;
 	NMAuthChain *chain;
+	char *permission_dup;
 
 	/* Validate the caller */
 	subject = nm_auth_subject_new_unix_process_from_context (context);
@@ -2413,12 +2414,14 @@ device_auth_request_cb (NMDevice *device,
 		goto done;
 	}
 
+	permission_dup = g_strdup (permission);
+
 	priv->auth_chains = g_slist_append (priv->auth_chains, chain);
 	nm_auth_chain_set_data (chain, "device", g_object_ref (device), g_object_unref);
-	nm_auth_chain_set_data (chain, "requested-permission", g_strdup (permission), g_free);
 	nm_auth_chain_set_data (chain, "callback", callback, NULL);
 	nm_auth_chain_set_data (chain, "user-data", user_data, NULL);
-	nm_auth_chain_add_call (chain, permission, allow_interaction);
+	nm_auth_chain_set_data (chain, "requested-permission", permission_dup /* transfer ownership */, g_free);
+	nm_auth_chain_add_call_unsafe (chain, permission_dup, allow_interaction);
 
 done:
 	if (error)
@@ -6844,7 +6847,7 @@ nm_manager_dbus_set_property_handle (NMDBusObject *obj,
 
 	chain = nm_auth_chain_new_subject (subject, invocation, _dbus_set_property_auth_cb, handle_data);
 	priv->auth_chains = g_slist_append (priv->auth_chains, chain);
-	nm_auth_chain_add_call (chain, property_info->writable.permission, TRUE);
+	nm_auth_chain_add_call_unsafe (chain, property_info->writable.permission, TRUE);
 	return;
 
 err:

src/settings/nm-agent-manager.c

@@ -1073,7 +1073,7 @@ _con_get_request_start (Request *req)
 			perm = NM_AUTH_PERMISSION_SETTINGS_MODIFY_SYSTEM;
 		nm_auth_chain_set_data (req->con.chain, "perm", (gpointer) perm, NULL);
 
-		nm_auth_chain_add_call (req->con.chain, perm, TRUE);
+		nm_auth_chain_add_call_unsafe (req->con.chain, perm, TRUE);
 	} else {
 		_LOGD (NULL, "("LOG_REQ_FMT") requesting user-owned secrets from agent %s",
 		       LOG_REQ_ARG (req), agent_dbus_owner);

src/settings/nm-settings.c

@@ -1274,7 +1274,7 @@ nm_settings_add_connection_dbus (NMSettings *self,
 	 * request affects more than just the caller, require 'modify.system'.
 	 */
 	s_con = nm_connection_get_setting_connection (connection);
-	g_assert (s_con);
+	nm_assert (s_con);
 	if (nm_setting_connection_get_num_permissions (s_con) == 1)
 		perm = NM_AUTH_PERMISSION_SETTINGS_MODIFY_OWN;
 	else
@@ -1296,7 +1296,7 @@ nm_settings_add_connection_dbus (NMSettings *self,
 	nm_auth_chain_set_data (chain, "callback-data", user_data, NULL);
 	nm_auth_chain_set_data (chain, "subject", g_object_ref (subject), g_object_unref);
 	nm_auth_chain_set_data (chain, "save-to-disk", GUINT_TO_POINTER (save_to_disk), NULL);
-	nm_auth_chain_add_call (chain, perm, TRUE);
+	nm_auth_chain_add_call_unsafe (chain, perm, TRUE);
 	return;
 
 done: