Re: NetworManager and openconnect: using cookies
On Thu, 2010-12-02 at 17:53 -0200, Murilo Opsfelder wrote: > By optional, you mean a "save password" checkbox in the GUI or a > compile-time flag (e.g.: --with-gnome-keyring)? The former. Or *some* way of exposing that choice to the user, at least. Web browsers do it differently, with a pop-up after a successful login. > > 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. > > If I understood it correctly, in remember_keyring_key() I should only > store form_id, name and value in auth_ui_data and actually save them in > gnome-keyring inside cookie_obtained() function. Is that correct? Yes. So you're not storing the password when it was *wrong* :) -- dwmw2 ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: NetworManager and openconnect: using cookies
On 12/02/2010 04:16 PM, David Woodhouse wrote: On Thu, 2010-12-02 at 17:53 +, 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. Thanks David. I appreciated your attention. 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. By optional, you mean a "save password" checkbox in the GUI or a compile-time flag (e.g.: --with-gnome-keyring)? 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. If I understood it correctly, in remember_keyring_key() I should only store form_id, name and value in auth_ui_data and actually save them in gnome-keyring inside cookie_obtained() function. Is that correct? (Third problem was that your patch lacked a Signed-off-by) Thanks for making me aware of this. I'll add it in the next patch. -- Murilo ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: NetworManager and openconnect: using cookies
On Thu, 2010-12-02 at 17:53 +, 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
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: NetworManager and openconnect: using cookies
On Mon, 2010-10-11 at 15:09 -0300, muri...@br.ibm.com wrote:
> @@ -590,6 +641,11 @@ int nm_process_auth_form (struct openconnect_info
> *vpninfo,
> keyname = g_strdup_printf("form:%s:%s", form->auth_id,
> data->opt->name);
> remember_gconf_key(ui_data, keyname, strdup(data->entry_text));
> }
> + /* save user password in gnome-keyring */
> + if (data->opt->type == OC_FORM_OPT_PASSWORD) {
> + remember_keyring_key(data->opt->name, strdup(data->entry_text));
> + }
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.
Other than that, it looks good. Will be nice to extend it to save the
*cookie* too, so that it doesn't have to log in again at all.
--
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com Intel Corporation
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: NetworManager and openconnect: using cookies
Hi guys, have you had any chance to look at this patch? http://mail.gnome.org/archives/networkmanager-list/2010-October/msg00048.html Thanks in advance, -- Murilo ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: NetworManager and openconnect: using cookies
David Woodhouse wrote on 10/11/2010 12:11:20 PM: > Your message was rejected by the openconnect-devel list. No HTML please. I'm sorry for this. > Not convinced that 'username' should be part of the equation. There > isn't necessarily a username at all -- if you're using a certificate to > authenticate, there's no need for one. > > The lookup key probably should include the UUID of the VPN connection > though. Agreed. I've used 'vpn_uuid' and 'name' to save every field of type ' OC_FORM_OPT_PASSWORD' in the keyring. Please see the attachment openconnect-add-gnome-keyring-support.patch Murilo openconnect-add-gnome-keyring-support.patch Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: NetworManager and openconnect: using cookies
On Mon, 2010-10-11 at 09:36 -0300, muri...@br.ibm.com wrote: > > Hi David, > > your comments make sense. I really appreciated. > > See my comments below and help me find the best way to follow. Your message was rejected by the openconnect-devel list. No HTML please. > I'm wondering on creating a function 'remember_keyring_key' that will store > 'form->auth_id', 'data->opt->name' and 'username'. > > Add other attributes 'auth_id' and 'auth_value' in keyring_password_schema. > > Create a function 'find_form_password' that will return the password saved in > gnome-keyring according to 'form->auth_id', 'data->opt->name' and 'username'. Not convinced that 'username' should be part of the equation. There isn't necessarily a username at all -- if you're using a certificate to authenticate, there's no need for one. The lookup key probably should include the UUID of the VPN connection though. > These functions will be called inside the loops instead of directly calling > gnome_keyring_* functions. Not entirely sure what you mean here. Just be careful of threading -- certain parts are already buggered because gconf isn't thread-safe and they get called in callbacks from the openconnect auth code, instead of from the main thread. > I think that way it will save all password entries in the keyring. > > Thanks in advance, > > -- dwmw2 ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: NetworManager and openconnect: using cookies
David Woodhouse wrote on 10/09/2010 08:25:37 AM: > David Woodhouse > 10/09/2010 08:25 AM > > To > > muri...@br.ibm.com > > cc > > "ebar...@us.ibm.com" , "networkmanager- > l...@gnome.org" , openconnect- > de...@lists.infradead.org > > Subject > > Re: NetworManager and openconnect: using cookies > > On Fri, 2010-10-08 at 11:03 -0300, muri...@br.ibm.com wrote: > > > > Thanks for your reply David. > > > > I think we could implement keyring support for password first and after > > implement a function to test if cookie is still valid and save cookie in > > gnome-keyring either. > > Yeah, that definitely sounds like the sanest approach. > > > > For now, I've drafted a patch to add gnome-keyring support for user's > > password. Please refer to the attachment openconnect-add-gnome- > keyring-support.patch > > Hm, that doesn't seem generic enough. Remember, the server can present > you with *arbitrary* forms to fill in. Some text input boxes are marked > as 'text' and some are marked as 'password' but there can be any number > of each. Just because it's *common* to have a single username and a > single password entry, that doesn't mean it's all we have to cope with. > > See how the non-password entries are saved in gconf with under a key > named 'form:$AUTH_ID:$OPT_NAME'? We want something similar for password > entries, I think -- so it can save *any* of the text boxes, not just > one. > > Or am I misreading your patch? > > -- > dwmw2 > Hi David, your comments make sense. I really appreciated. See my comments below and help me find the best way to follow. I'm wondering on creating a function 'remember_keyring_key' that will store 'form->auth_id', 'data->opt->name' and 'username'. Add other attributes 'auth_id' and 'auth_value' in keyring_password_schema. Create a function 'find_form_password' that will return the password saved in gnome-keyring according to 'form->auth_id', 'data->opt->name' and 'username'. These functions will be called inside the loops instead of directly calling gnome_keyring_* functions. I think that way it will save all password entries in the keyring. Thanks in advance, Murilo ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: NetworManager and openconnect: using cookies
On Fri, 2010-10-08 at 11:03 -0300, muri...@br.ibm.com wrote: > > Thanks for your reply David. > > I think we could implement keyring support for password first and after > implement a function to test if cookie is still valid and save cookie in > gnome-keyring either. Yeah, that definitely sounds like the sanest approach. > For now, I've drafted a patch to add gnome-keyring support for user's > password. Please refer to the attachment > openconnect-add-gnome-keyring-support.patch Hm, that doesn't seem generic enough. Remember, the server can present you with *arbitrary* forms to fill in. Some text input boxes are marked as 'text' and some are marked as 'password' but there can be any number of each. Just because it's *common* to have a single username and a single password entry, that doesn't mean it's all we have to cope with. See how the non-password entries are saved in gconf with under a key named 'form:$AUTH_ID:$OPT_NAME'? We want something similar for password entries, I think -- so it can save *any* of the text boxes, not just one. Or am I misreading your patch? -- dwmw2 ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: NetworManager and openconnect: using cookies
On Fri, 2010-10-08 at 11:03 -0300, muri...@br.ibm.com wrote: > David Woodhouse wrote on 10/01/2010 11:42:31 PM: > > > David Woodhouse > > 10/01/2010 11:42 PM > > > > To > > > > "muri...@br.ibm.com" > > > > cc > > > > "networkmanager-list@gnome.org" , > > "ebar...@us.ibm.com" , openconnect- > > de...@lists.infradead.org > > > > Subject > > > > Re: NetworManager and openconnect: using cookies > > > > Tbanks; this looks good. > > > > But we should really be using gnome-keyring for storing the cookie, > not > > gconf. That way it's much less likely that it'll 'leak'. I think we > can > > get away with enabling this behaviour by default then. > > > > We should probably make some attempt to remember the lifetime of the > > cookie too, so we don't try to use it when we *know* it's already > timed > > out. > > > > > I'm stuck on this step: if it fails on cookie, jump to ask > > > username/password inputs from user. It always tries to use cookie. > > > > Yeah, I suspect it's best to try to validate the cookie directly, > rather > > than passing it to openconnect and praying. We can implement a > > 'test-cookie' option in (lib)openconnect, which can either try a > CONNECT > > request, or hopefully there's a way to use the cookie with an HTTP > GET > > request that'll tell us if it's working too. > > > > Not sure about sending SIGKILL immediately -- that may upset the > people > > who had the issues which made me implement the BYE packet in the > first > > place. Perhaps we need an option to avoid the BYE on disconnect > (which > > would be nice in other situations too). > > > > -- > > dwmw2 > > > > Hi guys, > > Thanks for your reply David. > > I think we could implement keyring support for password first and > after > implement a function to test if cookie is still valid and save cookie > in > gnome-keyring either. > > For now, I've drafted a patch to add gnome-keyring support for user's > password. Please refer to the attachment > openconnect-add-gnome-keyring-support.patch > > Feel free to make any comments about it. I'd be glad to improve it. Whenever you feel its good enough David, feel free to push to git if you have access. If not let me know. Dan ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: NetworManager and openconnect: using cookies
David Woodhouse wrote on 10/01/2010 11:42:31 PM: > David Woodhouse > 10/01/2010 11:42 PM > > To > > "muri...@br.ibm.com" > > cc > > "networkmanager-list@gnome.org" , > "ebar...@us.ibm.com" , openconnect- > de...@lists.infradead.org > > Subject > > Re: NetworManager and openconnect: using cookies > > Tbanks; this looks good. > > But we should really be using gnome-keyring for storing the cookie, not > gconf. That way it's much less likely that it'll 'leak'. I think we can > get away with enabling this behaviour by default then. > > We should probably make some attempt to remember the lifetime of the > cookie too, so we don't try to use it when we *know* it's already timed > out. > > > I'm stuck on this step: if it fails on cookie, jump to ask > > username/password inputs from user. It always tries to use cookie. > > Yeah, I suspect it's best to try to validate the cookie directly, rather > than passing it to openconnect and praying. We can implement a > 'test-cookie' option in (lib)openconnect, which can either try a CONNECT > request, or hopefully there's a way to use the cookie with an HTTP GET > request that'll tell us if it's working too. > > Not sure about sending SIGKILL immediately -- that may upset the people > who had the issues which made me implement the BYE packet in the first > place. Perhaps we need an option to avoid the BYE on disconnect (which > would be nice in other situations too). > > -- > dwmw2 > Hi guys, Thanks for your reply David. I think we could implement keyring support for password first and after implement a function to test if cookie is still valid and save cookie in gnome-keyring either. For now, I've drafted a patch to add gnome-keyring support for user's password. Please refer to the attachment openconnect-add-gnome-keyring-support.patch Feel free to make any comments about it. I'd be glad to improve it. Murilo openconnect-add-gnome-keyring-support.patch Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: NetworManager and openconnect: using cookies
Tbanks; this looks good. But we should really be using gnome-keyring for storing the cookie, not gconf. That way it's much less likely that it'll 'leak'. I think we can get away with enabling this behaviour by default then. We should probably make some attempt to remember the lifetime of the cookie too, so we don't try to use it when we *know* it's already timed out. > I'm stuck on this step: if it fails on cookie, jump to ask > username/password inputs from user. It always tries to use cookie. Yeah, I suspect it's best to try to validate the cookie directly, rather than passing it to openconnect and praying. We can implement a 'test-cookie' option in (lib)openconnect, which can either try a CONNECT request, or hopefully there's a way to use the cookie with an HTTP GET request that'll tell us if it's working too. Not sure about sending SIGKILL immediately -- that may upset the people who had the issues which made me implement the BYE packet in the first place. Perhaps we need an option to avoid the BYE on disconnect (which would be nice in other situations too). -- dwmw2 ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
NetworManager and openconnect: using cookies
Hi guys, we are trying to improve openconnect in some points that we believe it will improve its behaviour. Let me explain what would be the perfect scenario: - start a vpn connection using NetworkManager plugin (NM-openconnect) - nm-auth-dialog.c is launched - try to connect only once using cookie stored in gconf key '/system/networking/connections/%d/vpn/cookie' - on failure, ask user for username and password When NetworkManager suspends, it should send a SIGKILL to openconnect child pid (not a SIGTERM). That way, when a Linux box resumed from a suspend, it would try to connect once using the cookie previously saved and if it failed with cookie, it would prompt user for username and password. In order to test, I forced NM to always send a SIGKILL to openconnect pid and added 'cookie' as a valid property. Please refer to the attachment patch-nm-openconnect-cookie-sigkill.patch On openconnect front, I drafted a patch that tries to connect firstly using cookie from gconf. The idea is that if it fails using cookie, it should jump to user and ask for username and password. Please refer to the attachment patch-openconnect-cookie-first.patch Using these patches, I'm able to connect via NM-openconnect using my username/password. After it connects, it saves cookie in user's gconf settings. I disconnect from vpn (it now sends a SIGKILL to openconnect) via NM-openconnect so my openconnect cookie will be still valid on the next connection because openconnect didn't send a BYE packet to the gateway. I'm stuck on this step: if it fails on cookie, jump to ask username/password inputs from user. It always tries to use cookie. Please feel free to tell me which is the best approach to do this and point me the right directions. I'd be happy to improve my patch based on your recommendations. Thanks in advance, Murilo patch-nm-openconnect-cookie-sigkill.patch Description: Binary data patch-openconnect-cookie-first.patch Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
