Re: [NM-openconnect PATCH] Allow 'lasthost' and 'autoconnect' settings.

[Dan Williams <dcbw redhat com>]

On Mon, 2009-05-11 at 17:02 +0100, David Woodhouse wrote:
> On Mon, 2009-05-11 at 11:46 -0400, Dan Williams wrote:
> > Ok, then maybe /apps is a better place for it. 
> 
> I was thinking /system/network/connections/$N/auth-dialog-cache or
> something like that. It does belong _with_ the connection, surely?

Sure, but I'd rather not start a precedent of stuffing other random
stuff into the connection config that's not actually part of the
connection specification.  Isn't this basically the difference
between /etc and /tmp?

dan
> >  Is the cookie the sole
> > secret, or is there other auth required?  If it's just the cookie, that
> > cookie should go in the keyring...
> 
> It's the cookie, as well as the name/address of the host we ended up at
> after all the HTTP redirects (load-balancing). One is more of a 'secret'
> than the other, of course, but both are currently passed back from the
> auth-dialog to the nm-openconnect-service as "secrets".
> 
> The cookie has a lifetime equal to the maximum session time -- typically
> 24h, 48h or so. I've just been looking at making the auth-dialog store
> that in the keyring.
> 
> One thing that makes it fun is that if you re-use a cookie after some of
> its lifetime is elapsed, your session lifetime is still counted from the
> time you originally obtained a cookie -- so it doesn't make a lot of
> sense to re-use an existing cookie if it's only got a few minutes to
> live.
> 
> And we have to track the issue time and the life expectancy of the
> cookies.... but we don't actually get told the lifetime until we've
> _connected_, so the auth-dialog doesn't see it.
> 
> I knocked up some code to _store_ it, but got distracted by real work
> shortly after noticing that it was storing duplicate items in my keyring
> (because the issue time for each one was different). I haven't yet
> looked at whether I can deal with that, or whether I have to encode
> _all_ the issue time, host, cookie, etc. into a single string as the
> 'contents' of the key, rather than as attributes of the key.
> 
> It would also be good to throw away the stored cookie if openconnect
> returns '2', meaning authentication failed. But again, the auth-dialog
> doesn't see that.
> 
> diff --git a/Makefile b/Makefile
> index a100dfc..8101d6f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -41,6 +41,12 @@ ifeq ($(GCONF_LDFLAGS),)
>  NMAUTHDIALOG := $(warning "Not building NetworkManager UI due to lack of GConf supprt.");
>  endif
>  
> +GKR_CFLAGS += $(shell pkg-config --cflags gnome-keyring-1)
> +GKR_LDFLAGS += $(shell pkg-config --libs gnome-keyring-1)
> +ifeq ($(GKR_LDFLAGS),)
> +NMAUTHDIALOG := $(warning "Not building NetworkManager UI due to lack of gnome-keyring supprt.");
> +endif
> +
>  CFLAGS := $(OPT_FLAGS) $(SSL_CFLAGS) $(XML2_CFLAGS) $(EXTRA_CFLAGS)
>  LDFLAGS := $(SSL_LDFLAGS) $(XML2_LDFLAGS) $(EXTRA_LDFLAGS)
>  
> @@ -48,7 +54,7 @@ ifdef SSL_UI
>  CFLAGS += -DSSL_UI
>  endif
>  
> -CFLAGS_nm-auth-dialog.o += $(GTK_CFLAGS) $(GCONF_CFLAGS) $(XML2_CFLAGS)
> +CFLAGS_nm-auth-dialog.o += $(GTK_CFLAGS) $(GCONF_CFLAGS) $(XML2_CFLAGS) $(GKR_CFLAGS)
>  
>  OPENCONNECT_OBJS := main.o $(SSL_UI) xml.o
>  CONNECTION_OBJS := dtls.o cstp.o mainloop.o tun.o 
> @@ -72,7 +78,7 @@ openconnect: $(OPENCONNECT_OBJS) $(CONNECTION_OBJS) libopenconnect.a
>  	$(CC) -o $@ $^ $(LDFLAGS)
>  
>  nm-openconnect-auth-dialog: nm-auth-dialog.o libopenconnect.a 
> -	$(CC) -o $@ $^ $(LDFLAGS) $(GTK_LDFLAGS) $(GCONF_LDFLAGS) $(XML2_LDFLAGS)
> +	$(CC) -o $@ $^ $(LDFLAGS) $(GTK_LDFLAGS) $(GCONF_LDFLAGS) $(XML2_LDFLAGS) $(GKR_LDFLAGS)
>  
>  %.o: %.c
>  	$(CC) -c -o $@ $(CFLAGS) $(CFLAGS_$@) $< -MD -MF  $  dep
> diff --git a/nm-auth-dialog.c b/nm-auth-dialog.c
> index 0199470..099c0bf 100644
> --- a/nm-auth-dialog.c
> +++ b/nm-auth-dialog.c
> @@ -36,6 +36,8 @@
>  
>  #include <gtk/gtk.h>
>  
> +#include <gnome-keyring.h>
> +
>  #include "auth-dlg-settings.h"
>  #include "openconnect.h"
>  
> @@ -1055,6 +1057,24 @@ void write_progress(struct openconnect_info *info, int level, const char *fmt, .
>  	g_free(msg);
>  }
>  
> +char *vpn_name = NULL, *vpn_uuid = NULL, *vpn_service = NULL;
> +
> +static void save_keyring_secret(struct openconnect_info *vpninfo)
> +{
> +	GnomeKeyringAttributeList *attrs;
> +	char *name;
> +	guint32 id;
> +
> +	name = g_strdup_printf("VPN cookie for %s", vpninfo->vpn_name);
> +	
> +	attrs = gnome_keyring_attribute_list_new();
> +	gnome_keyring_attribute_list_append_string(attrs, "uuid", vpn_uuid);
> +	gnome_keyring_attribute_list_append_string(attrs, "host", vpninfo->hostname);
> +	gnome_keyring_attribute_list_append_uint32(attrs, "timestamp", time(NULL));
> +	gnome_keyring_item_create_sync(NULL, GNOME_KEYRING_ITEM_GENERIC_SECRET,
> +				       name, attrs, vpninfo->cookie, TRUE, &id);
> +	printf("Saved cookie %s\n", vpninfo->cookie);
> +}
>  static gboolean cookie_obtained(auth_ui_data *ui_data)
>  {
>  	ui_data->getting_cookie = FALSE;
> @@ -1084,6 +1104,14 @@ static gboolean cookie_obtained(auth_ui_data *ui_data)
>  		ui_data->retval = 1;
>  	} else if (!ui_data->cookie_retval) {
>  		/* got cookie */
> +		printf("%s\n%s\n", NM_OPENCONNECT_KEY_GATEWAY, ui_data->vpninfo->hostname);
> +		printf("%s\n%s\n", NM_OPENCONNECT_KEY_COOKIE, ui_data->vpninfo->cookie);
> +		printf("\n\n");
> +		fflush(stdout);
> +		ui_data->retval = 0;
> +
> +		save_keyring_secret(ui_data->vpninfo);
> +
>  		while (ui_data->success_keys) {
>  			struct gconf_key *k = ui_data->success_keys;
>  			char *key = g_strdup_printf("%s/vpn/%s", config_path, k->key);
> @@ -1097,13 +1125,7 @@ static gboolean cookie_obtained(auth_ui_data *ui_data)
>  			g_free(k);
>  		}
>  
> -		printf("%s\n%s\n", NM_OPENCONNECT_KEY_GATEWAY, ui_data->vpninfo->hostname);
> -		printf("%s\n%s\n", NM_OPENCONNECT_KEY_COOKIE, ui_data->vpninfo->cookie);
> -		memset((void *)ui_data->vpninfo->cookie, 0, strlen(ui_data->vpninfo->cookie));
> -		printf("\n\n");
> -		fflush(stdout);
> -		ui_data->retval = 0;
> -
> +		
>  		gtk_main_quit();
>  	} else {
>  		/* no cookie; user cancellation */
> @@ -1380,9 +1402,9 @@ static struct option long_options[] = {
>  	{NULL, 0, 0, 0},
>  };
>  
> +
>  int main (int argc, char **argv)
>  {
> -	char *vpn_name = NULL, *vpn_uuid = NULL, *vpn_service = NULL;
>  	int reprompt;
>  	int opt;
>  
>