linux: Don't replace another interface's route with the same destination and metric

overlay/tun_linux.go

@@ -552,13 +552,17 @@ func (t *tun) setDefaultRoute(cidr netip.Prefix) error {
 	if cidr.Addr().Is6() {
 		nr.Priority = 256
 	}
-	err := netlink.RouteReplace(&nr)
+	held, err := t.otherLinkRoutes()
+	if err != nil {
+		return err
+	}
+	err = t.replaceRoute(&nr, held)
 	if err != nil {
 		t.l.Warn("Failed to set default route MTU, retrying", "error", err, "cidr", cidr)
 		//retry twice more -- on some systems there appears to be a race condition where if we set routes too soon, netlink says `invalid argument`
 		for range 2 {
 			time.Sleep(100 * time.Millisecond)
-			err = netlink.RouteReplace(&nr)
+			err = t.replaceRoute(&nr, held)
 			if err == nil {
 				break
 			} else {
@@ -578,6 +582,15 @@ func (t *tun) setDefaultRoute(cidr netip.Prefix) error {
 }
 
 func (t *tun) addRoutes(logErrors bool) error {
+	held, err := t.otherLinkRoutes()
+	if err != nil {
+		if logErrors {
+			t.l.Error("Failed to add routes", "error", err)
+			return nil
+		}
+		return err
+	}
+
 	// Path routes
 	routes := *t.Routes.Load()
 	for _, r := range routes {
@@ -602,7 +615,7 @@ func (t *tun) addRoutes(logErrors bool) error {
 			nr.Priority = r.Metric
 		}
 
-		err := netlink.RouteReplace(&nr)
+		err := t.replaceRoute(&nr, held)
 		if err != nil {
 			retErr := util.NewContextualError("Failed to add route", map[string]any{"route": r}, err)
 			if logErrors {
@@ -618,6 +631,86 @@ func (t *tun) addRoutes(logErrors bool) error {
 	return nil
 }
 
+// ip6RoutePrioUser is the metric the kernel gives an IPv6 route added without one
+const ip6RoutePrioUser = 1024
+
+// routeKey is where the main table files a route. Replacing a route replaces the first one with the same key, whatever
+// device it is on
+type routeKey struct {
+	dst      netip.Prefix
+	priority int
+}
+
+func routeKeyOf(dst *net.IPNet, family, priority int) (routeKey, bool) {
+	var p netip.Prefix
+	if dst == nil {
+		// A default route
+		if family == unix.AF_INET6 {
+			p = netip.PrefixFrom(netip.IPv6Unspecified(), 0)
+		} else {
+			p = netip.PrefixFrom(netip.IPv4Unspecified(), 0)
+		}
+	} else {
+		addr, ok := netip.AddrFromSlice(dst.IP)
+		if !ok {
+			return routeKey{}, false
+		}
+		ones, _ := dst.Mask.Size()
+		p = netip.PrefixFrom(addr.Unmap(), ones).Masked()
+	}
+	if priority == 0 && p.Addr().Is6() {
+		priority = ip6RoutePrioUser
+	}
+	return routeKey{dst: p, priority: priority}, true
+}
+
+// otherLinkRoutes returns the keys of the main table routes that are not on our device
+func (t *tun) otherLinkRoutes() (map[routeKey]struct{}, error) {
+	routes, err := netlink.RouteListFiltered(netlink.FAMILY_ALL, &netlink.Route{Table: unix.RT_TABLE_MAIN}, netlink.RT_FILTER_TABLE)
+	// An interrupted dump may miss a route, which then gets replaced like it always used to be
+	if err != nil && !errors.Is(err, netlink.ErrDumpInterrupted) {
+		return nil, fmt.Errorf("failed to list the system routes: %w", err)
+	}
+	held := make(map[routeKey]struct{}, len(routes))
+	for _, r := range routes {
+		if r.LinkIndex == t.deviceIndex {
+			continue
+		}
+		if k, ok := routeKeyOf(r.Dst, r.Family, r.Priority); ok {
+			held[k] = struct{}{}
+		}
+	}
+	return held, nil
+}
+
+// replaceRoute puts nr in the main table in place of our own route with its key. Replacing would take the route of
+// another device holding that key, so ours goes in behind it instead. Theirs keeps routing and the kernel falls back
+// to ours once theirs is removed, like when leaving the lan that had it. A different metric decides which one wins.
+func (t *tun) replaceRoute(nr *netlink.Route, held map[routeKey]struct{}) error {
+	family := unix.AF_INET6
+	if nr.Dst.IP.To4() != nil {
+		family = unix.AF_INET
+	}
+	k, ok := routeKeyOf(nr.Dst, family, nr.Priority)
+	if _, taken := held[k]; !ok || !taken {
+		return netlink.RouteReplace(nr)
+	}
+
+	// Appending doesn't update a route that is already ours, it adds a second one
+	ours := netlink.Route{
+		LinkIndex: nr.LinkIndex,
+		Dst:       nr.Dst,
+		Scope:     nr.Scope,
+		Table:     nr.Table,
+		Priority:  k.priority,
+	}
+	if err := netlink.RouteDel(&ours); err != nil && !errors.Is(err, unix.ESRCH) {
+		return err
+	}
+	t.l.Warn("Another interface already routes this destination at the same metric, our route is used once theirs is removed", "destination", k.dst, "metric", k.priority)
+	return netlink.RouteAppend(nr)
+}
+
 func (t *tun) removeRoutes(routes []Route) {
 	for _, r := range routes {
 		if !r.Install {

overlay/tun_linux_route_test.go

@@ -0,0 +1,133 @@
+//go:build linux && !android && !e2e_testing
+
+package overlay
+
+import (
+	"net"
+	"net/netip"
+	"os"
+	"testing"
+
+	"github.com/DefinedNet/netlink"
+	"github.com/slackhq/nebula/config"
+	"github.com/slackhq/nebula/test"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/unix"
+)
+
+// An unsafe_route with the same destination and metric as a lan's route must not replace it. Ours waits behind theirs
+// and takes over when the lan goes away. Run as root, or unprivileged with `unshare -rn`.
+func TestTunRouteLeavesAnotherDevicesRoute(t *testing.T) {
+	if os.Geteuid() != 0 {
+		t.Skip("creating links and adding routes needs root")
+	}
+
+	lan := &netlink.Veth{LinkAttrs: netlink.LinkAttrs{Name: "nebrt0"}, PeerName: "nebrt1"}
+	require.NoError(t, netlink.LinkAdd(lan))
+	t.Cleanup(func() { _ = netlink.LinkDel(lan) })
+	peer, err := netlink.LinkByName("nebrt1")
+	require.NoError(t, err)
+	require.NoError(t, netlink.LinkSetUp(lan))
+	require.NoError(t, netlink.LinkSetUp(peer))
+
+	lanAddrs := []*netlink.Addr{
+		{IPNet: &net.IPNet{IP: net.ParseIP("198.51.100.5"), Mask: net.CIDRMask(24, 32)}},
+		{IPNet: &net.IPNet{IP: net.ParseIP("2001:db8:1::5"), Mask: net.CIDRMask(64, 128)}, Flags: unix.IFA_F_NODAD},
+	}
+	for _, a := range lanAddrs {
+		require.NoError(t, netlink.AddrAdd(lan, a))
+	}
+
+	l := test.NewLogger()
+	c := config.NewC(l)
+	// The kernel puts a v4 lan route at metric 0, our default, and a v6 one at 256
+	require.NoError(t, c.LoadString(`
+tun:
+  dev: nebrt2
+  unsafe_routes:
+    - route: 198.51.100.0/24
+      via: 10.250.0.2
+    - route: 2001:db8:1::/64
+      via: fd00:250::2
+      metric: 256
+`))
+	tn, err := newTun(c, l, []netip.Prefix{netip.MustParsePrefix("10.250.0.1/24"), netip.MustParsePrefix("fd00:250::1/64")}, false)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = tn.Close() })
+	require.NoError(t, tn.Activate())
+
+	via := func(dst string) int {
+		routes, err := netlink.RouteGet(net.ParseIP(dst))
+		require.NoError(t, err)
+		require.NotEmpty(t, routes)
+		return routes[0].LinkIndex
+	}
+	devices := func(dst string) []int {
+		_, n, err := net.ParseCIDR(dst)
+		require.NoError(t, err)
+		routes, err := netlink.RouteListFiltered(netlink.FAMILY_ALL, &netlink.Route{Dst: n}, netlink.RT_FILTER_DST)
+		require.NoError(t, err)
+		var out []int
+		for _, r := range routes {
+			out = append(out, r.LinkIndex)
+		}
+		return out
+	}
+
+	for _, dst := range []string{"198.51.100.0/24", "2001:db8:1::/64"} {
+		assert.ElementsMatch(t, []int{lan.Index, tn.deviceIndex}, devices(dst), dst)
+	}
+	assert.Equal(t, lan.Index, via("198.51.100.9"))
+	assert.Equal(t, lan.Index, via("2001:db8:1::9"))
+
+	// Leaving the lan
+	for _, a := range lanAddrs {
+		require.NoError(t, netlink.AddrDel(lan, a))
+	}
+	assert.Equal(t, tn.deviceIndex, via("198.51.100.9"))
+	assert.Equal(t, tn.deviceIndex, via("2001:db8:1::9"))
+
+	// Dropping the unsafe_routes removes only ours
+	for _, a := range lanAddrs {
+		require.NoError(t, netlink.AddrAdd(lan, a))
+	}
+	require.NoError(t, c.ReloadConfigString("tun:\n  dev: nebrt2\n"))
+	for _, dst := range []string{"198.51.100.0/24", "2001:db8:1::/64"} {
+		assert.Equal(t, []int{lan.Index}, devices(dst), dst)
+	}
+}
+
+// The route for our own vpn network is replaced to set its mtu, which must not take a lan's route for the same network
+func TestTunVpnNetworkLeavesAnotherDevicesRoute(t *testing.T) {
+	if os.Geteuid() != 0 {
+		t.Skip("creating links and adding routes needs root")
+	}
+
+	lan := &netlink.Veth{LinkAttrs: netlink.LinkAttrs{Name: "nebrt0"}, PeerName: "nebrt1"}
+	require.NoError(t, netlink.LinkAdd(lan))
+	t.Cleanup(func() { _ = netlink.LinkDel(lan) })
+	require.NoError(t, netlink.LinkSetUp(lan))
+	require.NoError(t, netlink.AddrAdd(lan, &netlink.Addr{IPNet: &net.IPNet{IP: net.ParseIP("10.250.0.5"), Mask: net.CIDRMask(24, 32)}}))
+
+	l := test.NewLogger()
+	c := config.NewC(l)
+	require.NoError(t, c.LoadString("tun:\n  dev: nebrt2\n"))
+	tn, err := newTun(c, l, []netip.Prefix{netip.MustParsePrefix("10.250.0.1/24")}, false)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = tn.Close() })
+	require.NoError(t, tn.Activate())
+
+	_, n, err := net.ParseCIDR("10.250.0.0/24")
+	require.NoError(t, err)
+	routes, err := netlink.RouteListFiltered(netlink.FAMILY_V4, &netlink.Route{Dst: n}, netlink.RT_FILTER_DST)
+	require.NoError(t, err)
+	var devices []int
+	for _, r := range routes {
+		devices = append(devices, r.LinkIndex)
+		if r.LinkIndex == tn.deviceIndex {
+			assert.Equal(t, tn.DefaultMTU, r.MTU, "ours still carries the mtu")
+		}
+	}
+	assert.ElementsMatch(t, []int{lan.Index, tn.deviceIndex}, devices)
+}