nm-l2tp / NetworkManager-l2tp

L2TP and L2TP/IPsec support for NetworkManager
GNU General Public License v2.0
488 stars 84 forks source link

IPSec options hard coded #34

Closed corro closed 7 years ago

corro commented 7 years ago

Most configuration options of the IPSec connection are hard coded in nm-l2tp-service.c. There seems to be no possibility to adjust these options to match the requirements of a VPN server other than patching the source code.

I understand that not every option needs to be configurable, but some of them depend on the configuration of the remote VPN server. The ones I encountered so far are:

Please see the strongSwan documentation for an explanation of these options.

Would it be possible to allow users to tune these options to their needs?

dkosovic commented 7 years ago

There was a voluteer that was going to write code to add the more obscure options in a config file, but I never heard anything back from him.

Although I don't disagree with you that many parameters should be user configurable, I'm not sure if the ones you listed need to be:

pfs

Since strongSwan version 5.0.0, this parameter has no effect anymore, please see the strongSwan docs for more details. strongSwan version 5.0.0 was released in 2012 and most Linux distributions were still using Openswan back then.

With Libreswan, even if "pfs=no" is set, PFS will still be used if the VPN server wants to use it.

esp and ike

With Libreswan these hardcoded parameters were removed as it has an extensive set of default initial proposal ciphers to negotiate with the VPN server. There were lots of requests to remove these hardcoded parameters in Fedora Bugzilla and I haven't seen any new cipher related bugs since removing them.

With newer versions of strongSwan, it has been removing the weaker ciphers from the default initial proposals. So I've been adding the older ciphers for backwards compatiblity using these parameters. See commit https://github.com/nm-l2tp/network-manager-l2tp/commit/45815d102a8871d3cf24f8f05145a2a31b3d3932 . The ciphers added using these parameters are in addition to the default initial proposal ciphers.

Are there some missing ciphers that users would need to add for strongSwan? Perhaps I should modify the code to add all the default ciphers Libreswan uses.

keyexchange

If a user wants to use IKEv2, they should use network-manager-strongswan instead.

For a L2TP NetworkManager plugin with optional IPsec support, IKEv2 doesn't make sense (as L2TP isn't needed), so "keyexchange=ikev1" is hard coded in this case.

corro commented 7 years ago

@dkosovic, thank you for your extensive answer. I'm no IPSec expert, but your explanations make sense to me. There seems to be no reason for the options pfs and keyexchange to be configurable. But I still think it would be beneficial to allow users to customize the options esp and ike.

The reason why I'd like to change those options is, that I'm having problems connecting to a proprietary Zyxel firewall using the default settings. In order for the connection to work, I have to apply the following patch:

diff --git a/src/nm-l2tp-service.c b/src/nm-l2tp-service.c
index 60d59ff..8ec43b2 100644
--- a/src/nm-l2tp-service.c
+++ b/src/nm-l2tp-service.c
@@ -713,7 +713,7 @@ nm_l2tp_config_write (NML2tpPlugin *plugin,
        write_config_option (ipsec_fd, "  pfs=no\n");
    } else {
        write_config_option (ipsec_fd, "  esp=aes128-sha1,3des-sha1\n");
-       write_config_option (ipsec_fd, "  ike=aes128-sha1-modp2048,3des-sha1-modp1536,3des-sha1-modp1024\n");
+       write_config_option (ipsec_fd, "  ike=3des-sha1-modp1024!\n");
        write_config_option (ipsec_fd, "  keyexchange=ikev1\n");
    }
    value = nm_setting_vpn_get_data_item (s_vpn, NM_L2TP_KEY_IPSEC_FORCEENCAPS);

The exclamation mark is important here. If I understand it correctly, this prevents the default ciphers from being appended to the list. I don't know why it does not work otherwise, since 3des-sha1-modp1024 is already in the list, but this is the only configuration that works so far. All the other combinations I tried result in a NO_PROPOSAL_CHOSEN error.

I'm not sure if anybody else has problems like this, but the possibility to configure these settings would certainly help me in this situation.

dkosovic commented 7 years ago

I'll look into it after I get back from holiday leave after the new year.

fstronin commented 7 years ago

Hey guys! Are you think it would be good idea to make an ability to configure/override all strongswan options, on IPSec configuration window maybe? I think that it is not too hard to implement and I can do it if you think my proposal it is a right way.

dkosovic commented 7 years ago

As a general rule of thumb, the L2TP/IPsec VPN GUI shouldn't need to expose anything more than GUI clients on Windows, macOS, iOS or Android. The biggest thing missing that the other platforms have is the support of certificates for authentication (L2TP and IPsec). To add certificates support will involve using Gnome libsecret with the GUI and src/nm-l2tp-service.c (libsecret is currently used with the passwords). There are other NetworkManger VPN clients that support certificates and would be a good place to see how it is done.

I don't think the list of ciphers discussed above should be added as a GUI option as it will only confuse users. In this case the need to configure esp and ike is due to a bug on the VPN server and I have seen a similar bug with a strongswan VPN server which was fixed with a minor version update (as far as I know, only Arch Linux ever shipped that buggy minor version number).

I think esp and ike and possibly several other options probably ought be able to be specified in a text based config file. If specified in the file, they'll override the corresponding value hard coded in src/nm-l2tp-service.c.

I don't know if any of the other NetworkManger VPN clients have any config files, although I suspect NetworkManager-strongswan might.

jlundan commented 7 years ago

+1 For the config file for ike and esp. My VPN provider's Cisco Unity seems to want

ike=aes128-sha1-modp1024,3des-sha1-modp1024! esp=3des-md5

for the connection to work (that is if I control strongswan and xl2tpd manually). Weirdly enough network-manager-l2tp made ipsec connection establishes with esp=aes128-sha1,3des-sha1 (as it is now in the network-manager-l2tp generated config), but xl2tpd just cannot send anything through the connection. So, I do not know if just adding 3des-md5 to esp list would work since it seems already accept aes128-sha1 or 3des-sha1 and then fail to send anything through. Can be of course some other setting, but at least if I change esp=aes128-sha1,3des-sha1 in my "manual" config, it does not work. So, I think it would be nice to have option to override ike and esp completely in my setup.

dkosovic commented 7 years ago

I've been wanting to make a new release soon of this VPN plug-in and started thinking about this issue more in the last few days. I noticed NetworkManager-libreswan :

has an Advanced expander that hides Phase 1 Algorithms (i.e. ike) and Phase 2 Algorithms (i.e. esp) text boxes. So I'll probably do the same.

I'm thinking of not explicitly setting ike or esp unless the corresponding text boxes have something in them. When ike or esp are not explicitly set, the default cipher algorithms should be okay for the majority of users.

Hopefully have something out soon after I've fixed up a few other issues.

mbakhtin commented 7 years ago

This commit produces some problems

  1. There is a misprint in nm-l2tp-dialog.ui#L255 "i[sec_phase2" should be "ipsec_phase2"

Because of that it is impossible to change phase2 cipher. Unfortunately it accompanied with additional issue:

  1. Defaults have been changed with this change.

It looks like in my personal case my provider (Cisco Meraki is used) denies sha256 so it doesn't allow to use current defaults (there is no sha1 in current defaults). Previous settings "3des-sha1-modp1024" and "3des-sha1" works just fine. I think Cisco Meraki is actually not so rare case, so probably this change will affect many users.

dkosovic commented 7 years ago

Thanks, the ipsec_phase2 typo has now been corrected in the repo.

Before I explicitly added the weaker ciphers in commit https://github.com/nm-l2tp/network-manager-l2tp/commit/45815d102a8871d3cf24f8f05145a2a31b3d3932 that were dropped in the initial proposal defaults with newer versions of strongSwan, there weren't that many people with issues. But things might have changed by now with more people using newer versions of strongSwan.

Apparently SHA-1 was removed from the strongSwan initial proposal defaults as they claim it is broken and insecure, they list the broken algorithms on the following page :

In a new troubleshooting section on the wiki, I was going to document how to add the weak algorithms that earlier versions of strongSwan used.

I have mixed feelings about not using explicit esp and ike fallback values if the phase 1 & 2 text boxes aren't filled in, in regards to security vs backwards compatibility. but perhaps my main concern is that KDE users won't be able to set esp and ike as their GUI client (not maintained by me) doesn't have the new text boxes. I'm open to suggestion.

corro commented 7 years ago

@dkosovic, I can confirm that the recent changes allow us to override the ike settings as needed and we can now successfully connect to our VPN server without having to patch the source code. Thank you for your work!

I don't quite understand your concerns regarding the new settings, though. Couldn't you just provide the same values as before as default? If strongSwan changed their preferences, this sounds like a different issue to me.

dkosovic commented 7 years ago

The updated README.md file has links to the strongswan documentation regarding the broken ciphers and which ciphers they recommend not to use, It also includes an example of how to add the ciphers back that earlier versions of NetworkManager-l2tp and strongswan used.

Although I was slow to admit the need to override esp and ike options, now they can be overridden, I figured it's better for the end-user to decide if they want to risk using a broken cipher.

dkosovic commented 7 years ago

The KDE users can use nmcli if they need to edit a VPN connection to add a broken cipher. I'll probably document this in the wiki.

dkosovic commented 7 years ago

@corro in a sense I am going back to the same values of ike and esp prior to NetworkManager-l2tp 1.2.4 which was not explicitly setting them. I only started setting ike and esp because of a newer strongSwan which dropped a few ciphers that I added back. At the time I figured I might as well add 3des-sha1-modp1024 as the earlier versions of NetworkManager were using them. So I think I agree with you.

NetworkManager-l2tp 0.9.8.8 and earlier had the following supplementary cipher suites:

NetworkManager-l2tp 1.2.2, 1.2.0, 1.0.2 and 1.0.0 didn't set ike or esp.

NetworkManager-l2tp 1.2.4 had the following supplementary cipher suites: