After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 596762 - nm-applet GUI allows invalid PPTP configuration CancelOk
nm-applet GUI allows invalid PPTP configuration CancelOk
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: nm-applet
git master
Other Linux
: Normal normal
: ---
Assigned To: Dan Williams
Dan Williams
Depends on:
Blocks:
 
 
Reported: 2009-09-29 17:09 UTC by Matt
Modified: 2009-10-21 22:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
mppe is for mschap and mschapV2 only (6.72 KB, patch)
2009-10-15 00:06 UTC, Mathieu Trudel-Lapierre
none Details | Review
updated patch (mppe for mschap*) (3.85 KB, patch)
2009-10-16 00:24 UTC, Mathieu Trudel-Lapierre
none Details | Review
updated patch (mppe for mschap*, disable pap/chap on selection) (4.66 KB, patch)
2009-10-16 01:21 UTC, Mathieu Trudel-Lapierre
none Details | Review
updated patch (5.11 KB, patch)
2009-10-20 23:43 UTC, Mathieu Trudel-Lapierre
none Details | Review

Description Matt 2009-09-29 17:09:05 UTC
OS: Ubuntu Karmic Alpha 64-bit with updates to 2009/09/28
Network Manager version: 0.8~a~git.20090923t064445.b20cef2-0ubuntu1

According to http://technet.microsoft.com/en-us/library/cc732393%28WS.10%29.aspx MPPE is not supported with PAP or CHAP authentication. However, when creating or editing a PPTP VPN connection in nm-applet, the Advanced Options window allows 'Use Point-to-Point Encryption (MPPE)' to be ticked even when only PAP and/or CHAP are selected as authentication methods.

The VPN connection then fails, with the following /var/log/syslog message:

MPPE required, but MS-CHAP[v2] auth not performed.

(which makes sense)
Comment 1 Alexander Sack 2009-09-29 23:15:39 UTC
ubuntu bug: https://bugs.launchpad.net/bugs/438834
Comment 2 Mathieu Trudel-Lapierre 2009-10-15 00:05:27 UTC
Here is a patch that aims to fix the issue: when both MSCHAP and MSCHAPv2 are disabled, the MPPE checkbox is made non-sensitive. Otherwise, it's clickable.
Comment 3 Mathieu Trudel-Lapierre 2009-10-15 00:06:11 UTC
Created attachment 145462 [details] [review]
mppe is for mschap and mschapV2 only
Comment 4 Dan Williams 2009-10-15 21:44:21 UTC
(02:40:31 PM) dcbw: style issue:
(02:40:34 PM) dcbw: + }
(02:40:36 PM) dcbw: + else {
(02:40:41 PM) dcbw: should be "} else {"
(02:41:05 PM) dcbw: need space before the ( too there on the widget_set_sensitive calls
(02:43:09 PM) dcbw: and lets do the "Note: MPPE is only supported with MSCHAP and MSCHAPv2" as a tooltip on the "Use MPPE" checkbox
(02:43:12 PM) dcbw: instead of a label

other than that looks pretty good, thanks!
Comment 5 Dan Williams 2009-10-15 21:45:49 UTC
also checking MPPE should probably disable auth methods other than MSCHAP/MSCHAPv2.  Otherwise you get into a situation where they are all checked, you check Use MPPE, and PAP/CHAP are still enabled, no?
Comment 6 Mathieu Trudel-Lapierre 2009-10-16 00:24:54 UTC
Created attachment 145564 [details] [review]
updated patch (mppe for mschap*)

Yes, you would. I'm concerned about the complexity of it though, and how much MPPE is really used. If it's not a frequent thing, then maybe it would be best to put it somewhere else?
Comment 7 Mathieu Trudel-Lapierre 2009-10-16 00:25:45 UTC
The above updated patch doesn't contain that additional requirement btw. :)
Comment 8 Mathieu Trudel-Lapierre 2009-10-16 01:21:13 UTC
Created attachment 145566 [details] [review]
updated patch (mppe for mschap*, disable pap/chap on selection)

Here is an updated patch.
Comment 9 Dan Williams 2009-10-20 23:08:16 UTC
(04:06:00 PM) dcbw: cyphermox: gtk_tooltips_set_tip() is deprecated since gtk+ 2.12
(04:06:14 PM) dcbw: cyphermox: we could just set the tip in the glade file too
(04:06:35 PM) dcbw: and there's two instances of 
(04:06:48 PM) dcbw: +	}
(04:06:48 PM) dcbw: +	else {
(04:06:48 PM) dcbw: +		gtk_widget_set_sensitive (widget, TRUE);
(04:06:48 PM) dcbw: +	}
(04:06:53 PM) dcbw: which should really be
(04:07:04 PM) dcbw: } else
(04:07:04 PM) dcbw:     gtk_widget_set_sensitive (widget, TRUE); 
(04:07:14 PM) dcbw: or at least
(04:07:16 PM) dcbw: } else {
(04:07:18 PM) dcbw:     gtk_widget_set_sensitive (widget, TRUE); 
(04:07:18 PM) dcbw: }
(04:07:27 PM) dcbw: depending on your preference

Other than that, I think we're good to go.  Thanks!
Comment 10 Mathieu Trudel-Lapierre 2009-10-20 23:43:00 UTC
Created attachment 145906 [details] [review]
updated patch

As per our discussion on IRC... another update to the patch :)
Comment 11 Dan Williams 2009-10-21 22:48:14 UTC
Thanks!  I also discovered we can desensitize the checkboxes in the treeview, so I fixed up the patch to do that.

418b3c8f953e516df049c4c764e3467c0a19512a (master)
5661093ca9a58f373a2e432871a7322352a2bdeb (0.7.x)