> On July 23, 2011, 6:31 p.m., Lamarque Vieira Souza wrote: > > vpnplugins/openvpn/openvpn.cpp, line 110 > > <http://git.reviewboard.kde.org/r/102059/diff/1/?file=29553#file29553line110> > > > > There is no problem in adding nss-devel as dependency. Nss is also used > > by NetworkManager, so it is already installed. We just need to change the > > main CMakeLists.txt to look for it and disable openvpn importing/exporting > > if it is not installed and emmit a warning when that happen. > > Ilia Kats wrote: > Do we need to change the main CMakeLists.txt, or the one of the OpenVPN > plugin? Another question is if that's actually needed. The isEncrypted method > is only used to set the flags for the cerficate password, but since we're > calling the connection editor afterwards anyway, I think we can skip that > part here, the user will then simply change the combo box as needed and the > flags will be set when saving the connection. And from what I can tell > (./configure --help from NM's sources) NM can be compiled with either Nss or > GnuTLS, so Nss is not always installed. > > Lamarque Vieira Souza wrote: > Actually it is vpnplugins/CMakeLists.txt, that is the one that selects > which plugins are going to be compiled. The importing code must do everything > automatically without user intervention, we cannot rely on user changing the > combo box value before saving the configuration. If nss is not always used > then we have a problem, we need to detect which one is installed and > implement isEncrypted method with the one we find. > > Ilia Kats wrote: > I'd still prefer keeping plugin-specific stuff in the > OpenVPN-CMakeLists.txt. > The fact is, we already require user interaction by omitting any password > flags, which defaults to None (saved by NM), so the combo box will be set to > Store, and the user will have to change it to Not Required. If no user > password is required or the certificate is unencrypted, the flags should be > set to NotRequired. We could, of course, default to NotRequired for all > certificates, that way not encrypted certificates would not require any > interaction, while encrypted certificates, which require interaction anyway > (either by typing in the password or by changing the combo box to Always > ask), would require two mouse clicks more. > Oh, and Rajeesh, I noticed you changed the return type of > exportConnectionSettings from void to bool, could you do that for the > OpenConnect plugin, too? > > Lamarque Vieira Souza wrote: > Since this is needed only for a small part of the import code it can be > done in vpnplugins/openvpn/CMakeLists.txt, but we will have to detect which > implementation we will use, preferably the one used by NM. > We must use sane defaults when possible. This is an importing code, it is > supposed to minimize user interaction as much as possible. Set the combo box > value accordingly to the information we have: if the certificate is not > encrypted than set the value to NotRequired. If it is encrypted set it to > AlwaysAsk. If the user wants to save the password than he/she can change the > combo box. In all cases if user starts to type a password and the combo box > is different from Store then change the combo box to AlwaysAsk. Always Ask > should be default unless user has configured it to Store. NotRequired is to > be used only when there is no other option. > > Rajeesh K Nambiar wrote: > I've changed Knm::Setting::AgentOwned to Knm::Setting::NotSaved by > default. > Not sure how to go about is_pkcs12 check; I'm in favour of Ilia to drop > the check altogether; considering if the complexity is really worth it.
It seems NM is smart enough to not ask for certificate's password when the certificate in not encrypted even when we set the password flag to NotSaved. So everything seems Ok here. - Lamarque Vieira ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102059/#review5004 ----------------------------------------------------------- On July 24, 2011, 4:56 p.m., Rajeesh K Nambiar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/102059/ > ----------------------------------------------------------- > > (Updated July 24, 2011, 4:56 p.m.) > > > Review request for Network Management. > > > Summary > ------- > > Caveat emptor: There is one unresolved issue with isEncrypted() function - we > need to check of key file is PKCS12 format or not. nm-applet achieves this > through a call to nm_setting_802_1x_set_private_key(). I've googled a lot, > and looks like there is no way to check PKCS12 file format in either Qt or > KDE. > If we are to add ad-hoc support via SEC_PKCS12DecoderStart and related > functions from nss (which is what NetworkManager does), it wil introduce a > new dependency on nss-devel. > > > This addresses bug 194099. > http://bugs.kde.org/show_bug.cgi?id=194099 > > > Diffs > ----- > > libs/ui/vpnuiplugin.h 932d86f > settings/config/manageconnectionwidget.h 8f7a205 > settings/config/manageconnectionwidget.cpp 780d50f > vpnplugins/novellvpn/novellvpn.h a95926b > vpnplugins/novellvpn/novellvpn.cpp 61d5519 > vpnplugins/openconnect/openconnectui.h 8098aec > vpnplugins/openconnect/openconnectui.cpp 4d43afd > vpnplugins/openvpn/openvpn.h 4607cd5 > vpnplugins/openvpn/openvpn.cpp 6f126b8 > vpnplugins/pptp/pptp.h e513d3c > vpnplugins/pptp/pptp.cpp e4efbd7 > vpnplugins/strongswan/strongswan.h d648217 > vpnplugins/strongswan/strongswan.cpp 9d4a8be > vpnplugins/vpnc/vpnc.h 0b3f6db > vpnplugins/vpnc/vpnc.cpp ea24cf1 > > Diff: http://git.reviewboard.kde.org/r/102059/diff > > > Testing > ------- > > Only lightly tested, seems to import/export OK for sample configuration file. > I don't have an OpenVPN connection, so it would be great if someone could > test. > > > Thanks, > > Rajeesh > >
_______________________________________________ kde-networkmanager mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-networkmanager
