From 8a6f9ba70eae87e66aef1b42b5edd206beaf789c Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 29 Nov 2024 14:57:31 +0100 Subject: [PATCH] kernel-netlink: Don't fallback to peer address as gateway/nexthop This doesn't really seem useful (perhaps it was before we started to configure the outbound interface on our routes). And it can actually cause the route installation to fail e.g. for routes over point-to-point interfaces where we'd get "Error: Nexthop has invalid gateway" errors. Note that we can't return NULL if we find an interface as e.g. the updown plugin uses this method to determine the outbound interface (it ignores the nexthop), which it passes to the script. If we returned NULL, it would pass "unknown" instead, which would cause the firewall rules to mismatch. While it seems that 0.0.0.0/:: is ignored as nexthop by the kernel on the installed route, I still explicitly ignore such addresses to avoid any unintended side-effects. The automatic route installation in the ikev2/shunt-manual-prio scenario had to be disabled on the clients. The reason is that the route in table 220 won't have a nexthop set (the peers are directly connected), so when trying to reach alice or venus via SSH, which matches the port-specific bypass policies for which we don't install throw routes, the hosts will do ARP requests for the target IPs instead of routing the packets via moon. Closes strongswan/strongswan#2548 --- src/libcharon/kernel/kernel_interface.h | 2 ++ src/libcharon/kernel/kernel_net.h | 2 ++ .../plugins/kernel_netlink/kernel_netlink_ipsec.c | 2 +- .../plugins/kernel_netlink/kernel_netlink_net.c | 9 +++++---- .../shunt-manual-prio/hosts/carol/etc/strongswan.conf | 2 ++ .../shunt-manual-prio/hosts/dave/etc/strongswan.conf | 2 ++ 6 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/libcharon/kernel/kernel_interface.h b/src/libcharon/kernel/kernel_interface.h index 21cabe7f0..a8c9d6f41 100644 --- a/src/libcharon/kernel/kernel_interface.h +++ b/src/libcharon/kernel/kernel_interface.h @@ -312,6 +312,8 @@ struct kernel_interface_t { * The returned host is allocated and must be destroyed. * An optional src address can be used to check if a route is available * for the given source to dest. + * 0.0.0.0/:: may get returend if an interface but no next hop can be + * determined. * * @param dest target destination address * @param prefix prefix length if dest is a subnet, -1 for auto diff --git a/src/libcharon/kernel/kernel_net.h b/src/libcharon/kernel/kernel_net.h index 2dc247e90..ece59ad4d 100644 --- a/src/libcharon/kernel/kernel_net.h +++ b/src/libcharon/kernel/kernel_net.h @@ -85,6 +85,8 @@ struct kernel_net_t { * The returned host is allocated and must be destroyed. * An optional src address can be used to check if a route is available * for the given source to dest. + * 0.0.0.0/:: may get returend if an interface but no next hop can be + * determined. * * @param dest target destination address * @param prefix prefix length if dest is a subnet, -1 for auto diff --git a/src/libcharon/plugins/kernel_netlink/kernel_netlink_ipsec.c b/src/libcharon/plugins/kernel_netlink/kernel_netlink_ipsec.c index dc1486b0f..810b07e61 100644 --- a/src/libcharon/plugins/kernel_netlink/kernel_netlink_ipsec.c +++ b/src/libcharon/plugins/kernel_netlink/kernel_netlink_ipsec.c @@ -3008,7 +3008,7 @@ static void install_route(private_kernel_netlink_ipsec_t *this, policy->route = NULL; } - DBG2(DBG_KNL, "installing route: %R via %H src %H dev %s", out->dst_ts, + DBG2(DBG_KNL, "installing route: %R via %+H src %H dev %s", out->dst_ts, route->gateway, route->src_ip, route->if_name); switch (charon->kernel->add_route(charon->kernel, route->dst_net, route->prefixlen, route->gateway, diff --git a/src/libcharon/plugins/kernel_netlink/kernel_netlink_net.c b/src/libcharon/plugins/kernel_netlink/kernel_netlink_net.c index 416ae7811..6742b618d 100644 --- a/src/libcharon/plugins/kernel_netlink/kernel_netlink_net.c +++ b/src/libcharon/plugins/kernel_netlink/kernel_netlink_net.c @@ -2075,9 +2075,9 @@ static host_t *get_route(private_kernel_netlink_net_t *this, host_t *dest, *iface = get_interface_name_by_index(this, best->oif); } } - if (!addr && !match_net) - { /* fallback to destination address */ - addr = dest->clone(dest); + if (!addr && !match_net && iface && *iface) + { + addr = host_create_any(dest->get_family(dest)); } } else @@ -2600,7 +2600,8 @@ static status_t manage_srcroute(private_kernel_netlink_net_t *this, { chunk = src_ip->get_address(src_ip); netlink_add_attribute(hdr, RTA_PREFSRC, chunk, sizeof(request)); - if (gateway && gateway->get_family(gateway) == src_ip->get_family(src_ip)) + if (gateway && !gateway->is_anyaddr(gateway) && + gateway->get_family(gateway) == src_ip->get_family(src_ip)) { chunk = gateway->get_address(gateway); netlink_add_attribute(hdr, RTA_GATEWAY, chunk, sizeof(request)); diff --git a/testing/tests/ikev2/shunt-manual-prio/hosts/carol/etc/strongswan.conf b/testing/tests/ikev2/shunt-manual-prio/hosts/carol/etc/strongswan.conf index 203f1a7b9..0fe180ced 100755 --- a/testing/tests/ikev2/shunt-manual-prio/hosts/carol/etc/strongswan.conf +++ b/testing/tests/ikev2/shunt-manual-prio/hosts/carol/etc/strongswan.conf @@ -6,4 +6,6 @@ charon-systemd { load = random nonce openssl pem pkcs1 revocation curl kernel-netlink socket-default updown vici + # conflicts with the route via moon we have installed to reach hosts in its subnet directly + install_routes = no } diff --git a/testing/tests/ikev2/shunt-manual-prio/hosts/dave/etc/strongswan.conf b/testing/tests/ikev2/shunt-manual-prio/hosts/dave/etc/strongswan.conf index 203f1a7b9..0fe180ced 100755 --- a/testing/tests/ikev2/shunt-manual-prio/hosts/dave/etc/strongswan.conf +++ b/testing/tests/ikev2/shunt-manual-prio/hosts/dave/etc/strongswan.conf @@ -6,4 +6,6 @@ charon-systemd { load = random nonce openssl pem pkcs1 revocation curl kernel-netlink socket-default updown vici + # conflicts with the route via moon we have installed to reach hosts in its subnet directly + install_routes = no }