Re: NetworManager and openconnect: using cookies

[David Woodhouse <dwmw2 infradead org>]

On Thu, 2010-12-02 at 17:53 +0000, David Woodhouse wrote:
> 
> Hrm, why not using the *same* 'keyname' string as we're using for the
> TEXT and SELECT cases? There was a reason we included the form->auth_id
> in that key.

Patch below should do that. But I notice two problems now I look closer.

Firstly, it's not optional. I think it needs to be; we don't want to
*unconditionally* save the password. Not only for security reasons, but
also because it might be a one-time password.

Secondly, it's saving the password even if the authentication fails.
You'll note that 'remember_gconf_key' doesn't actually set it
immediately; it just *stores* it, and the entry later gets set when the
cookie_obtained() function walks through the ui_data->success_keys list.

(Third problem was that your patch lacked a Signed-off-by)

diff -u b/nm-auth-dialog.c b/nm-auth-dialog.c
--- b/nm-auth-dialog.c
+++ b/nm-auth-dialog.c
@@ -117,6 +117,7 @@
 	GNOME_KEYRING_ITEM_GENERIC_SECRET,
 	{
 		{ "vpn_uuid", GNOME_KEYRING_ATTRIBUTE_TYPE_STRING },
+		{ "form_id",  GNOME_KEYRING_ATTRIBUTE_TYPE_STRING },
 		{ "name",     GNOME_KEYRING_ATTRIBUTE_TYPE_STRING },
 		{ NULL, 0 }
 	}
@@ -498,7 +499,8 @@
 	}
 }
 
-void remember_keyring_key(const char *name, const char *value)
+void remember_keyring_key(const char *form_id, const char *name,
+			  const char *value)
 {
 	char *description;
 	description = g_strdup_printf("openconnect %s", name);
@@ -507,6 +509,7 @@
 		description,
 		value,
 		"vpn_uuid", vpn_uuid,
+		"form_id", form_id,
 		"name", name,
 		NULL);
 }
@@ -521,7 +524,7 @@
 	return result;
 }
 
-char *find_form_password(const char *name)
+char *find_form_password(const char *form_id, const char *name)
 {
 	char *ret = NULL;
 	char *password;
@@ -530,6 +533,7 @@
 	res = gnome_keyring_find_password_sync(&keyring_password_schema,
 		&password,
 		"vpn_uuid", vpn_uuid,
+		"form_id", form_id,
 		"name", name,
 		NULL);
 
@@ -583,7 +587,7 @@
 
 			if (opt->type == OC_FORM_OPT_PASSWORD) {
 				/* obtain password from gnome-keyring */
-				data->entry_text = find_form_password(opt->name);
+				data->entry_text = find_form_password(form->auth_id, opt->name);
 			} else {
 				data->entry_text = find_form_answer(form, opt);
 			}
@@ -644,7 +648,8 @@
 
 				/* save user password in gnome-keyring */
 				if (data->opt->type == OC_FORM_OPT_PASSWORD) {
-					remember_keyring_key(data->opt->name, strdup(data->entry_text));
+					remember_keyring_key(form->auth_id, data->opt->name, 
+							     strdup(data->entry_text));
 				}
 			}
 			g_slice_free (ui_fragment_data, data);

-- 
dwmw2