Re: FWD: [PATCH] (Fixed) Support for openvpn --auth option

[Robert Vogelgesang <vogel users sourceforge net>]

Hello,

as promised, I'm back after the holidays, having done my "self-assigned
homework". ;-)

On Mon, Dec 22, 2008 at 12:27:59PM +0100, Robert Vogelgesang wrote:
> On Fri, Dec 19, 2008 at 06:15:24PM -0500, Dan Williams wrote:
> [...]
> > > > For minimal impact, I choose to implement the --auth option in the
> > > > same way as the --cipher option.  Both the "new" --auth and the "old"
> > > > --cipher options share the following issues:
> > > > 
> > > > o	When a non-default value was saved and you want to switch back
> > > > 	to "Default" later on, then this change does not get saved and
> > > > 	the non-default value remains in the config.
> > > > 
> > > > 	As far as I understand the plugin code, this issue seems to be
> > > > 	caused by NetworkManager or gconfd, not by the openvpn plugin
> > > > 	(the hash returned by advanced_dialog_new_hash_from_dialog() does
> > > > 	not contain the --auth/--cipher value when "Default" was chosen).
> > > > 
> > > > 	Is this a known issue?  (bugzilla.gnome.org didn't show anything
> > > > 	similar for NetworkManager)
> > 
> > That should be handled in nm_gconf_set_stringhash_helper() in
> > src/gconf-helpers/gconf-helpers.c, where keys not in the hash table get
> > deleted from GConf.  If the parameter is the default value, it shouldn't
> > show up in GConf at all, as you see by
> > advanced_dialog_new_hash_from_dialog() returning a hash table without
> > that key in the table.  Could you check to see if the non-default value
> > key is correctly getting removed from GConf by the code in
> > nm_gconf_set_stringhash_helper()?
> 
> I don't know what nm_gconf_set_stringhash_helper() does, but I checked
> the xml file written by gconfd in a subdirectory of the user's home.
> When setting "auth" or "cipher" to "Default" in the GUI, the previous
> value was not removed from that file.
> 
> I will re-check after having upgraded my patch to the freshly
> released NetworkManager-openvpn-0.7.0-16.svn4326.fc9, which is first
> priority for me now, because after updating all the other
> NetworkManager rpms, my VPN connection does no longer work.

With NetworkManager-openvpn-0.7.0-16.svn4326.fc9 this issue is gone.


> > > > o	Openvpn supports these options for both static and TLS modes.
> > > > 	The openvpn plugin for NetworkManager carries the --cipher option
> > > > 	(and with my patch, the --auth option, too) on the "Certificates
> > > > 	(TLS)" tab of the "advanced" popup, which is only available when
> > > > 	using TLS modes and not when using static keys.
> > > > 
> > > > 	The easiest fix would be to move the popup-menue(s) (GtkComboBox)
> > > > 	for --cipher (and --auth) to the "General" tab.  A little bit more
> > > > 	work, but maybe better for future extensions:  Introduce a new
> > > > 	tab "Encryption" for these options.  What do you think/prefer?
> > 
> > How about we name it "Security" instead?  I'd take a good look at a
> > patch that did that.
> 
> OK, fine.

I've created a patch that does just this, and I'll post it in a few
minutes.  In a third email I'll send a new version of my patch that
implements support for the openvpn --auth option.

	Robert