Re: [PATCH v2] Saving agent-owned secrets for newly created connections

[Jirka Klimes <jklimes redhat com>]

On Wednesday 09 of November 2011 19:12:34 Dan Williams wrote:
> On Wed, 2011-11-09 at 17:13 +0100, Jirka Klimes wrote:
> > Hello,
> > 
> > attached is a patch for sending secrets to agents when a new connection
> > is created (AddConnection, AddAndActivateConnection).
> > 
> > At present, when a new connection is created, the secrets are not sent to
> > agents and thus not saved. One has to edit the connection again. It is
> > easily reproducible by adding a VPN connection. When it is opened again
> > in the editor, the secrets are not there.
> 
> Hmm, could we do the save from pk_add_cb() and make the patch smaller?
> That way nm_settings_connection_save_agent_secrets() could be made
> private to nm-settings.c.  And we wouldn't have to pass the caller_uid
> to the nm_settings_add_connection() callback, which saves some churn.
> 

You are right. Done. 
I wanted to save some (copy-paste) code, but due to passing caller_uid 
didn't save much. I also didn't realize that having the stuff in pk_add_cb() 
allows more contained code and not to pollute nm-manager.c.

> Ordering might also be interesting here; since this could trigger a
> SaveSecrets before the agent has been able to process the new
> connection, meaning that the agent may not actually have grabbed the
> connection yet (since that's another dbus call) when SaveSecrets comes
> in.  I don't think that's a problem in practice for the nm-applet agent
> but it might be for others.  Not sure if there's anything we can do
> about it though since the ordering would be correct.
> 
> One thing we should probably do (later though) is not call SaveSecrets
> at all if there aren't any secrets left to send after filtering for
> AGENT_OWNED secrets.  Shouldn't have any real effect, but would be more
> "correct".
> 
> Dan

From a737f50f5a304660c118ea0db68655f765785251 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ji=C5=99=C3=AD=20Klime=C5=A1?= <jklimes redhat com>
Date: Wed, 9 Nov 2011 16:48:35 +0100
Subject: [PATCH] settings: send agent-owned secrets also for newly created
 connections
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We have to send agent-owned secrets to agents via SaveSecrets() D-Bus call for
newly created connections, the same way we do for connection updates.
Without the change secrets aren't saved for new created VPN connections,
only after a connection update.

Signed-off-by: Jiří Klimeš <jklimes redhat com>
---
 src/settings/nm-settings.c |   46 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c
index 7cf930a..09433cc 100644
--- a/src/settings/nm-settings.c
+++ b/src/settings/nm-settings.c
@@ -902,6 +902,45 @@ add_new_connection (NMSettings *self,
 	return NULL;
 }
 
+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
+send_agent_owned_secrets (NMSettings *self,
+                          NMSettingsConnection *connection,
+                          gulong caller_uid)
+{
+	NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self);
+	NMConnection *for_agent;
+
+	/* Dupe the connection so we can clear out non-agent-owned secrets,
+	 * as agent-owned secrets are the only ones we send back to be saved.
+	 * Only send secrets to agents of the same UID that called update too.
+	 */
+	for_agent = nm_connection_duplicate (NM_CONNECTION (connection));
+	nm_connection_clear_secrets_with_flags (for_agent,
+	                                        secrets_filter_cb,
+	                                        GUINT_TO_POINTER (NM_SETTING_SECRET_FLAG_AGENT_OWNED));
+	nm_agent_manager_save_secrets (priv->agent_mgr, for_agent, TRUE, caller_uid);
+	g_object_unref (for_agent);
+}
+
 static void
 pk_add_cb (NMAuthChain *chain,
            GError *chain_error,
@@ -916,6 +955,7 @@ pk_add_cb (NMAuthChain *chain,
 	NMSettingsConnection *added = NULL;
 	NMSettingsAddCallback callback;
 	gpointer callback_data;
+	gulong caller_uid;
 	const char *perm;
 
 	priv->auths = g_slist_remove (priv->auths, chain);
@@ -955,9 +995,14 @@ pk_add_cb (NMAuthChain *chain,
 done:
 	callback = nm_auth_chain_get_data (chain, "callback");
 	callback_data = nm_auth_chain_get_data (chain, "callback-data");
+	caller_uid = nm_auth_chain_get_data_ulong (chain, "caller-uid");
 
 	callback (self, added, error, context, callback_data);
 
+	/* Send agent-owned secrets to the agents */
+	if (!error && added)
+		send_agent_owned_secrets (self, added, caller_uid);
+
 	g_clear_error (&error);
 	nm_auth_chain_unref (chain);
 }
@@ -1061,6 +1106,7 @@ nm_settings_add_connection (NMSettings *self,
 	nm_auth_chain_set_data (chain, "connection", g_object_ref (connection), g_object_unref);
 	nm_auth_chain_set_data (chain, "callback", callback, NULL);
 	nm_auth_chain_set_data (chain, "callback-data", user_data, NULL);
+	nm_auth_chain_set_data_ulong (chain, "caller-uid", caller_uid);
 }
 
 static void
-- 
1.7.6.4