nm-l2tp / NetworkManager-l2tp

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

1.20.14 fails with libreswan version 4.15 #226

Closed gregorburger closed 1 month ago

gregorburger commented 1 month ago

It seems that the introduced compatibility with 1.20.14 is regressing for libreswan 4.15 with:

nm-l2tp[25138] <info> starting ipsec Redirecting to: systemctl restart ipsec.service /nix/store/38v98zpbjp55avbbaq5rf35kk1hq7f42-libreswan-4.15/bin/ipsec: unknown IPsec command "add" ("ipsec --help" for list) I don't know for sure but it seems here: https://github.com/nm-l2tp/NetworkManager-l2tp/blob/0b0a416a14c07912c51b89971d8d6587267461fb/src/nm-l2tp-service.c#L1521 instead off add there should be an --add

dkosovic commented 1 month ago

I only tested with libreswan 4.14 (and some earlier versions) along with a release candidate of libreswan 5.0, so looks like the logic I used fails with 4.15.

The code is supposed to be doing the following:

As auto comes up with a deprecated message with libreswan 5.0, --config has to be after add and that version doesn't like it if the add command is --add.

In the code I use ipsec --help to determine if the auto command is listed, if it is, the code should use ipsec auto ... , otherwise use ipsec add ...

Could you provide the output of ipsec --help | grep auto ? This is what I see with Libreswan 4.14 and 5.0 :

$ ipsec --version
Libreswan 4.14

$ ipsec --help | grep auto
    algparse        auto
$ ipsec --version
Libreswan 5.0rc2

$ ipsec --help | grep auto
gregorburger commented 1 month ago

auto is listed in libreswan 4.15. see the outputs below

$ ipsec --version
Libreswan 4.15

$ ipsec --help | grep auto
    auto            barf
gregorburger commented 1 month ago

It seems you are too strict on your match here: https://github.com/nm-l2tp/NetworkManager-l2tp/blob/0b0a416a14c07912c51b89971d8d6587267461fb/shared/utils.c#L53 "\tauto\n" won't match the output from libreswan 4.15 because auto is not at the end of the line. would it hurt to just match "auto"?

dkosovic commented 1 month ago

Ahh.

https://github.com/nm-l2tp/NetworkManager-l2tp/blob/0b0a416a14c07912c51b89971d8d6587267461fb/shared/utils.c#L53

The code is looking for "\tauto\n" in the output and failing, the code probably should be doing:

    if (strstr(output, "\tauto\n") != NULL || strstr(output, "\tauto\t") != NULL)
dkosovic commented 1 month ago

Figured it would be safer to test for either "\tauto\t" or "\tauto\n" in case auto is used as a prefix.

libreswan 5.0 was released 2 weeks ago and libreswan 4.15 was 3 weeks ago. Fedora and Debian are still on 4.14 and suspect they will jump straight to 5.0. Hopefully there aren't too many Linux distros with libreswan 4.15. Regardless, I'll probably release a new version of NetworkManager-l2tp in the next few days.

Thanks for the bug report!