Review Request: Support for importing/exporting OpenVPN connections

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102059/
-----------------------------------------------------------
(Updated July 23, 2011, 4:41 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.cpp 780d50f
vpnplugins/novellvpn/novellvpn.h a95926b
vpnplugins/novellvpn/novellvpn.cpp 61d5519
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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110723/fb0b0585/attachment.htm

Rajeesh K Nambiar

2011-07-23 17:43:44 UTC

vpnplugins/openvpn/openvpn.cpp, line 204
<http://git.reviewboard.kde.org/r/102059/diff/1/?file=29553#file29553line204>
You're using these a lot here, without actually checking the return value, so regardless of what the user clicked, the import will continue. I think you'd need to introduce a new error type, like UserCanceled, and return an empty QVariantList after setting mError to this if Cancel was clicked (also, ManageConnectionWidget should recognize this error type and not display any additional pop-ups).

These are actually non-fatal messages, could be safely ignored. Is there a better way than warningContinueCancel?

settings/config/manageconnectionwidget.cpp, line 405
<http://git.reviewboard.kde.org/r/102059/diff/1/?file=29549#file29549line405>
At this point, it would probably be best to let the plugins specify the file extensions. The method could return a QStringList, which ManageConnectionWidget would collect from all installed plugins, then join them to a QString and call KFileDialog.

Possibly another method into VpnUiPlugin?

- Rajeesh

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102059/#review5005
-----------------------------------------------------------

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110723/c9ed28e0/attachment-0001.htm

Ilia Kats

2011-07-23 18:10:12 UTC

Possibly another method into VpnUiPlugin?

That's what I thought.

These are actually non-fatal messages, could be safely ignored. Is there a better way than warningContinueCancel?

How about KMessageBox::information?

- Ilia

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102059/#review5005
-----------------------------------------------------------

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110723/bbecee83/attachment.htm

Lamarque Vieira Souza

2011-07-23 18:53:41 UTC

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110723/0c134af3/attachment.htm

Ilia Kats

2011-07-23 21:57:15 UTC

vpnplugins/openvpn/openvpn.cpp, line 157
<http://git.reviewboard.kde.org/r/102059/diff/1/?file=29553#file29553line157>
You should not try to import a file whose extension is not supported (.ovpn and *.conf this this case). Windows does not support file extensions with more than three characters, do you know what is the extension they use?

Actually, that's not quite true. Yes, on old DOS-like systems you had the 8+3 rule, but any modern Windows system can handle arbitrary file names files with an arbitrary file extension (or none at all). Just look at for example XAMPP for Win, they're using the standard *.conf files for apache configuration. And according to http://openvpn.net/index.php/open-source/documentation/howto.html#install they are in fact using *.ovpn.

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.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110723/8d548424/attachment-0001.htm

Lamarque Vieira Souza

2011-07-23 22:20:25 UTC

Ok then.

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.

- Lamarque Vieira

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102059/#review5004
-----------------------------------------------------------

Post by Ilia Kats
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102059/
-----------------------------------------------------------
(Updated July 23, 2011, 4:41 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.cpp 780d50f
vpnplugins/novellvpn/novellvpn.h a95926b
vpnplugins/novellvpn/novellvpn.cpp 61d5519
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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110723/44158886/attachment.htm

Ilia Kats

2011-07-23 22:48:59 UTC

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?

- Ilia

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102059/#review5004
-----------------------------------------------------------

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110723/30a5f774/attachment-0001.htm

Lamarque Vieira Souza

2011-07-23 23:29:42 UTC

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.

- Lamarque Vieira

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102059/#review5004
-----------------------------------------------------------

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110723/7e4a90bd/attachment.htm

Rajeesh K Nambiar

2011-07-24 14:42:26 UTC

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102059/
-----------------------------------------------------------

(Updated July 24, 2011, 2:42 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 (updated)
-----

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110724/fe9062bf/attachment.htm

Lamarque Vieira Souza

2011-07-24 15:56:07 UTC

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102059/
-----------------------------------------------------------
(Updated July 24, 2011, 2:42 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110724/fbca12d9/attachment-0001.htm

Rajeesh K Nambiar

2011-07-24 16:56:07 UTC

-----------------------------------------------------------
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.

Changes
-------

Updated patch adding comments.

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 (updated)
-----

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110724/4398db88/attachment.htm

Ilia Kats

2011-07-24 17:48:33 UTC

Post by Rajeesh K Nambiar
-----------------------------------------------------------
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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110724/234c5c46/attachment.htm

Rajeesh K Nambiar

2011-07-25 06:00:15 UTC

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102059/
-----------------------------------------------------------

(Updated July 25, 2011, 6 a.m.)

Review request for Network Management.

Changes
-------

Fixed KMessageBox::warningContinueCancel.

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 (updated)
-----

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110725/f96e9290/attachment.htm

Ilia Kats

2011-07-25 14:37:19 UTC

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102059/
-----------------------------------------------------------
(Updated July 25, 2011, 6 a.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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110725/a90b4a04/attachment.htm

Lamarque Vieira Souza

2011-07-25 14:56:50 UTC

Post by Ilia Kats
I think we are good to go. Lamarque, what do you say?

Agreed. I have just commited the patch since Rajeesh does not own a developer account.

- Lamarque Vieira

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102059/#review5087
-----------------------------------------------------------

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102059/
-----------------------------------------------------------
(Updated July 25, 2011, 6 a.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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110725/54c99fd0/attachment.htm

Commit Hook

2011-07-25 14:58:21 UTC

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102059/#review5089
-----------------------------------------------------------

This review has been submitted with commit 655b70e4fac222feba74fbab6b6fb3a67be97258 by Lamarque V. Souza to branch nm09.

- Commit

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102059/
-----------------------------------------------------------
(Updated July 25, 2011, 6 a.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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110725/f4e076c4/attachment.htm

Lamarque Vieira Souza

2011-07-26 19:12:52 UTC

Hi,

Today I have noticed that the import/export code only exports VPN
specific configurations, but some configurations (such as static routes,
aditional DNS name servers, etc) are also very important to make the
connection work. I thought we could add those configurations as comments in
the exported files. What do you think? Do you know how nm-applet handles those
configurations?

--
Lamarque V. Souza
http://www.geographicguide.com/brazil.htm
Linux User #57137 - http://counter.li.org/
http://planetkde.org/pt-br
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110726/ac7a5d85/attachment.htm

Ilia Kats

2011-07-26 23:02:48 UTC

Hi,

I just tested it with vpnc, nm-applet then adds X-NM-Routes=route1
route2 to the file. Note however that the routes consist only of
destination/prefix, no gateway or metric are saved (looking at the
exporter source, this is by design). In total the vpnc plugin knows the
following additinal entries: X-NM-Use-NAT-T, X-NM-Force-NAT-T,
X-NM-SaveGroupPassword, X-NM-Routes. So no static IPs or DNS servers so
far, although additional X-NM-entries couldn't do much harm if we were
to add them ourselves. The OpenVPN plugin does not appear to be able to
handle anything apart from the VPN specific settings. However, as
comments in OpenVPN files definitely exist and are prefixed with # or ;,
it shoud be possible to add routing, DNS and IP export/import ourselves.

Ilia

-------- Original Message --------
Subject: Re: Review Request: Support for importing/exporting OpenVPN
connections
Date: 26.07.2011 16:12:52 -0300
From: Lamarque Vieira Souza <lamarque at gmail.com>
To: Rajeesh K Nambiar
<rajeeshknambiar at gmail.com>,kde-networkmanager at kde.org

Post by Lamarque Vieira Souza
Hi,
Today I have noticed that the import/export code only exports VPN
specific configurations, but some configurations (such as static
routes, aditional DNS name servers, etc) are also very important to
make the connection work. I thought we could add those configurations
as comments in the exported files. What do you think? Do you know how
nm-applet handles those configurations?
--
Lamarque V. Souza
http://www.geographicguide.com/brazil.htm
Linux User #57137 - http://counter.li.org/
http://planetkde.org/pt-br
_______________________________________________
kde-networkmanager mailing list
kde-networkmanager at kde.org
https://mail.kde.org/mailman/listinfo/kde-networkmanager

Rajeesh K Nambiar

2011-07-27 05:19:50 UTC

On Wed, Jul 27, 2011 at 12:42 AM, Lamarque Vieira Souza

Hi,
Today I have noticed that the import/export code only exports VPN specific
configurations, but some configurations (such as static routes, aditional
DNS name servers, etc) are also very important to make the connection work.
I thought we could add those configurations as comments in the exported
files. What do you think? Do you know how nm-applet handles those
configurations?

For VPNC, there's a TODO comment at line 352 of
vpnplugins/vpnc/vpnc.cpp; to export X-NM-Routes :-)

--
Lamarque V. Souza
http://www.geographicguide.com/brazil.htm
Linux User #57137 - http://counter.li.org/
http://planetkde.org/pt-br

--
Cheers,
Rajeesh
http://rajeeshknambiar.wordpress.com

Rajeesh K Nambiar

2011-07-24 14:51:04 UTC

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.
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.
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?
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.

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.

- Rajeesh

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102059/#review5004
-----------------------------------------------------------

Post by Ilia Kats
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102059/
-----------------------------------------------------------
(Updated July 24, 2011, 2:42 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110724/b9c068dc/attachment.htm

Lamarque Vieira Souza

2011-07-24 18:15:29 UTC

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
-----------------------------------------------------------

Post by Ilia Kats
-----------------------------------------------------------
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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110724/484d6010/attachment-0001.htm

Lamarque Vieira Souza

2011-07-23 18:32:26 UTC