Control: tags -1 patch upstream
Hi,
On Sat, 11 May 2024 00:58:03 +0200 Maurizio Avogadro <mav...@gmail.com>
wrote:
> Some more info gathered this afternoon.
>
> It seems that network-manager-fortisslvpn also makes a mess with the routing
> table after the connection has been established.
>
> I could easily get a working VPN by adding
>
> ipcp-accept-remote
>
> to /etc/ppp/options and manually launching openfortivpn; such setting also
> allowed NetworkManager to accept the IP address supplied by the counterpart
> (it
> was the default until ppp v2.4.9), but in this case the connection didn't
> work
> until I deleted a rule in the routing table which routed the IP address of
> the
> remote gateway through the ppp0 interface (!).
>
> A duplicate default route through the main physical network interface was
> created too and the spawned pppd process didn't exit after the connection has
> been terminated and had to be killed manually: none of these issues happened
> when openfortivpn was launched manually.
I also did a little investigation yesterday on this as well as #1070343.
There are many upstream issues in both the openfortivpn GitHub and
network-manager Gitlab instances related to this.
To sum up what I think is happening (gathered from various issues):
* During PPP IPCP negotiation the Fortinet server requests it's link IP
address to be the same as it's public IP address. This doesn't make any
sense to me - the link address inside the PPP session should be a
private 10.x address. Maybe there's some reason it does this that I
don't understand...
* pppd configures this address on the ppp device using the
SIOCSIFDSTADDR ioctl. The kernel internally adds a route to the routing
table because it knows it can use this link to get to that IP (lol).
* Pre ppp-2.4 this didn't matter because pppd forcefully used the
address supplied on the command line which is a 169.x address. In this
case the kernel adds a route to a dummy IP which didn't affect anything.
* ppp-2.5 started enforcing the remote IP on the command line matched
the IP sent over IPCP. This initially broke negotiation until
"ipcp-accept-remote" was added, but this option breaks things in another
way by allowing pppd to configure the server supplied IP as the peer
address in the kernel and create a bad route.
* openfortivpn has a hack to workaround this - after pppd is up it
manually removes the route from the routing table. The network-manager
plugin doesn't do this.
I ended up writing a patch which does something similar in the
network-manager plugin - it manually invokes the SIOCSIFDSTADDR ioctl on
the ppp device to switch the peer address to a new fake address. This
seems to work to remove the bad route. Doing this feels really hacky
though and I'm kind of hesitant to upload it unless approved by upstream!
Note if you're testing this patch, you also need the fix for #1070343 in
openfortivpn as well to make it work (or manually set ipcp-accept-remote).
Having done this I'm very tempted to just switch to OpenConnect which
seems to work ok. It has its own ppp implementation which doesn't seem
to set the peer address in the kernel at all (and may just ignore the
server's IPCP requests - I have not checked) and so avoids these issues.
James
From 416b65c3f23d795707ed122f06c54eebd515bc3b Mon Sep 17 00:00:00 2001
From: James Cowgill <jcowg...@debian.org>
Date: Sat, 11 May 2024 13:59:17 +0100
Subject: [PATCH] Modify PPP peer address to avoid wrong route
---
src/nm-fortisslvpn-service.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/src/nm-fortisslvpn-service.c b/src/nm-fortisslvpn-service.c
index e4433d5..cdfada4 100644
--- a/src/nm-fortisslvpn-service.c
+++ b/src/nm-fortisslvpn-service.c
@@ -37,6 +37,11 @@
#include <locale.h>
#include <errno.h>
+#include <arpa/inet.h>
+#include <net/if.h>
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+
#include <glib/gstdio.h>
#include "nm-fortissl-properties.h"
@@ -313,14 +318,39 @@ handle_set_state (NMDBusFortisslvpnPpp *object,
return TRUE;
}
+static void
+hack_ppp_peer(const char *tundev)
+{
+ struct ifreq req;
+ int fd, ret;
+
+ _LOGI ("FORTISSLVPN Modifying PPP peer address to avoid incorrect host route");
+
+ memset(&req, 0, sizeof(req));
+ strlcpy(req.ifr_name, tundev, sizeof(req.ifr_name));
+ req.ifr_dstaddr.sa_family = AF_INET;
+ inet_aton("169.254.0.1", &((struct sockaddr_in*)(&req.ifr_dstaddr))->sin_addr);
+
+ fd = socket(AF_INET, SOCK_STREAM, 0);
+ ret = ioctl(fd, SIOCSIFDSTADDR, &req);
+ if (ret)
+ _LOGW ("FORTISSLVPN Failed to set PPP peer address, errno = %d", errno);
+ close(fd);
+}
+
static gboolean
handle_set_ip4_config (NMDBusFortisslvpnPpp *object,
GDBusMethodInvocation *invocation,
GVariant *arg_config,
gpointer user_data)
{
+ const char *tundev;
+
_LOGI ("FORTISSLVPN service (IP Config Get) reply received.");
+ if (g_variant_lookup(arg_config, NM_VPN_PLUGIN_CONFIG_TUNDEV, "&s", &tundev))
+ hack_ppp_peer(tundev);
+
nm_vpn_service_plugin_set_ip4_config (NM_VPN_SERVICE_PLUGIN (user_data), arg_config);
nmdbus_fortisslvpn_ppp_complete_set_ip4_config (object, invocation); /// remove me
--
2.43.0
