device: apply a loose IPv4 rp_filter when it would interfere with multihominglr/rp-filter

The IPv4 Strict Reverse Path Forwarding filter (RFC 3704) drops legitimate
traffic when the same route is present on multiple interfaces, which is a
pretty common scenario for IPv4 hosts. In particular, if the traffic is
routable via multiple interfaces it drops traffic incoming via the device that
has lower metric on the route to the originating network.

Among other things, this disrupts existing connection when the user connected
to the Internet via Wi-Fi activates a Wired Ethernet connection that also has a
default route. Also, the Strict filter (and Reverse Path filters in general)
provide practically no value to hosts that have a default route.

The solution this patch uses is to detect scenarios where Strict filter is
known to interfere and switch to a saner RP filter on the affected links.
Routes to the same network on multiple interfaces is a good indication the RP
filter would drop the legitimate traffice from the link with a lower metric.
This includes the default routes.

In such cases, we switch to the Loose Reverse Path Forwarding. This addresses
the problems the multihomed hosts face, at the cost of disabling filtering
altogether when a default route is present. A Feasible Path Reverse Path
Forwarding would address the main problems with the Strict filter, but it's
not implemented by the Linux kernel.

1 files changed, 49 insertions, 0 deletions

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 1ed2800341..017d55a3a5 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -339,6 +339,8 @@ typedef struct _NMDevicePrivate {
 		NMPlatformIP4Route v4;
 		NMPlatformIP6Route v6;
 	} default_route;
+	bool v4_has_shadowed_routes;
+	const char *ip4_rp_filter;
 
 	/* DHCPv4 tracking */
 	struct {
@@ -2394,6 +2396,45 @@ link_changed_cb (NMPlatform *platform,
 }
 
 static void
+ip4_rp_filter_update (NMDevice *self)
+{
+	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
+	const char *ip4_rp_filter;
+
+	if (   priv->v4_has_shadowed_routes
+	    || priv->default_route.v4_has) {
+		if (nm_device_ipv4_sysctl_get_uint32 (self, "rp_filter", 0) != 1) {
+			/* Don't touch the rp_filter if it's not strict. */
+			return;
+		}
+		/* Loose rp_filter */
+		ip4_rp_filter = "2";
+	} else {
+		/* Default rp_filter */
+		ip4_rp_filter = NULL;
+	}
+
+	if (ip4_rp_filter != priv->ip4_rp_filter) {
+		nm_device_ipv4_sysctl_set (self, "rp_filter", ip4_rp_filter);
+		priv->ip4_rp_filter = ip4_rp_filter;
+	}
+}
+
+static void
+ip4_routes_changed_changed_cb (NMRouteManager *route_manager, NMDevice *self)
+{
+	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
+	int ifindex = nm_device_get_ip_ifindex (self);
+
+	if (nm_device_sys_iface_state_is_external_or_assume (self))
+		return;
+
+	priv->v4_has_shadowed_routes = nm_route_manager_ip4_routes_shadowed (route_manager,
+	                                                                     ifindex);
+	ip4_rp_filter_update (self);
+}
+
+static void
 link_changed (NMDevice *self, const NMPlatformLink *pllink)
 {
 	/* stub implementation of virtual function to allow subclasses to chain up. */
@@ -9442,6 +9483,8 @@ nm_device_set_ip4_config (NMDevice *self,
 	}
 
 	nm_default_route_manager_ip4_update_default_route (nm_default_route_manager_get (), self);
+	if (!nm_device_sys_iface_state_is_external_or_assume (self))
+		ip4_rp_filter_update (self);
 
 	if (has_changes) {
 		NMSettingsConnection *settings_connection;
@@ -13375,6 +13418,9 @@ constructed (GObject *object)
 	g_signal_connect (platform, NM_PLATFORM_SIGNAL_IP6_ROUTE_CHANGED, G_CALLBACK (device_ipx_changed), self);
 	g_signal_connect (platform, NM_PLATFORM_SIGNAL_LINK_CHANGED, G_CALLBACK (link_changed_cb), self);
 
+	g_signal_connect (nm_route_manager_get (), NM_ROUTE_MANAGER_IP4_ROUTES_CHANGED,
+	                  G_CALLBACK (ip4_routes_changed_changed_cb), self);
+
 	priv->settings = g_object_ref (NM_SETTINGS_GET);
 	g_assert (priv->settings);
 
@@ -13413,6 +13459,9 @@ dispose (GObject *object)
 	g_signal_handlers_disconnect_by_func (platform, G_CALLBACK (device_ipx_changed), self);
 	g_signal_handlers_disconnect_by_func (platform, G_CALLBACK (link_changed_cb), self);
 
+	g_signal_handlers_disconnect_by_func (nm_route_manager_get (),
+	                                      G_CALLBACK (ip4_routes_changed_changed_cb), self);
+
 	g_slist_free_full (priv->arping.dad_list, (GDestroyNotify) nm_arping_manager_destroy);
 	priv->arping.dad_list = NULL;