nm-l2tp / NetworkManager-l2tp

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

Add ipsec-left vpn-data option #214

Closed araujorm closed 9 months ago

araujorm commented 10 months ago

By default a value of %defaultroute is used for the "left" parameter of the IPSec daemon configuration. However that is not suitable on a machine that needs to establish the IPSec connection via an interface that does not correspond to the default route.

Add a "ipsec-left" option for vpn-data that allows a custom value to be passed as the ipsec.conf "left" option for these more advanced setup scenarios.

dkosovic commented 9 months ago

I was thinking of incorporating the following left=%defaultroute replacment :

Would that work in your case?

araujorm commented 9 months ago

I took a look at that fork and ported only the relevant part, removing changes that rely only on strongswan and would break compatibility with libreswan.

Seems to work for me, without the need to tweak manually the connection like my ipsec-left alternative, which is a plus.

Here's the result: https://github.com/araujorm/NetworkManager-l2tp/commit/90a64d66fa47a22d50c922a1093508d42382266a

I made a specific branch for it: https://github.com/araujorm/NetworkManager-l2tp/tree/ipsec-find-localaddr

I'm going to close this pull request and submit another with that, but feel free to decide which you prefer.

araujorm commented 9 months ago

Closing, suggesting to use https://github.com/nm-l2tp/NetworkManager-l2tp/pull/215 instead.

dkosovic commented 9 months ago

Thanks for the new pull request, it looks good to me and can confirm the original author is okay with the credits (I will mention that in the comments of the new PR when I do the merge).

I agree with the removal of the strongswan specific code, plus all strongswong ipsec.conf options are now deprecated (in favor of swanctl.conf), there is no benefit in replacing one deprecated option with another.

I've yet to do obligatory compiling and testing on a number of VMs for the new PR, but don't anticipate issues.