diff --git a/overlay/tun_darwin.go b/overlay/tun_darwin.go index 3a7b85cd..0824d8bf 100644 --- a/overlay/tun_darwin.go +++ b/overlay/tun_darwin.go @@ -545,7 +545,8 @@ func (t *tun) removeRoutes(routes []Route) error { } for _, r := range routes { - if !r.Install { + if len(r.Via) == 0 || !r.Install { + // addRoutes never installs these continue } diff --git a/overlay/tun_darwin_route_test.go b/overlay/tun_darwin_route_test.go index 9df527bd..72fe008b 100644 --- a/overlay/tun_darwin_route_test.go +++ b/overlay/tun_darwin_route_test.go @@ -5,10 +5,12 @@ package overlay import ( "net" "net/netip" + "os" "os/exec" "strings" "testing" + "github.com/slackhq/nebula/config" "github.com/slackhq/nebula/routing" "github.com/slackhq/nebula/test" "github.com/stretchr/testify/assert" @@ -73,3 +75,29 @@ func TestTunRoutesBeforeActivate(t *testing.T) { assert.NoError(t, tn.addRoutes(true)) assert.NoError(t, tn.removeRoutes(routes)) } + +// A tun.routes entry with no via is never installed, so removing it must not delete a route for the same cidr that +// belongs to someone else +func TestTunReloadLeavesRoutesItNeverInstalled(t *testing.T) { + if os.Geteuid() != 0 { + t.Skip("creating a utun and adding routes needs root") + } + lo0, err := net.InterfaceByName("lo0") + require.NoError(t, err) + cidr := "10.250.0.128/25" + require.NoError(t, exec.Command("route", "-q", "-n", "add", "-net", cidr, "-interface", "lo0").Run()) + t.Cleanup(func() { _ = exec.Command("route", "-q", "-n", "delete", "-net", cidr).Run() }) + + l := test.NewLogger() + c := config.NewC(l) + require.NoError(t, c.LoadString("tun:\n routes:\n - route: "+cidr+"\n mtu: 1300\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()) + + require.NoError(t, c.ReloadConfigString("tun: {}\n")) + msgs, err := fetchRoutes() + require.NoError(t, err) + assert.Equal(t, lo0.Index, routeInterfaceIn(msgs, netip.MustParsePrefix(cidr))) +} diff --git a/overlay/tun_freebsd.go b/overlay/tun_freebsd.go index cebf959d..8ed90daf 100644 --- a/overlay/tun_freebsd.go +++ b/overlay/tun_freebsd.go @@ -589,7 +589,8 @@ func (t *tun) addRoutes(logErrors bool) error { func (t *tun) removeRoutes(routes []Route) error { for _, r := range routes { - if !r.Install { + if len(r.Via) == 0 || !r.Install { + // addRoutes never installs these continue } diff --git a/overlay/tun_netbsd.go b/overlay/tun_netbsd.go index c6f17848..d9852ba5 100644 --- a/overlay/tun_netbsd.go +++ b/overlay/tun_netbsd.go @@ -420,7 +420,8 @@ func (t *tun) addRoutes(logErrors bool) error { func (t *tun) removeRoutes(routes []Route) error { for _, r := range routes { - if !r.Install { + if len(r.Via) == 0 || !r.Install { + // addRoutes never installs these continue } diff --git a/overlay/tun_openbsd.go b/overlay/tun_openbsd.go index aa90b5fe..b310a0d1 100644 --- a/overlay/tun_openbsd.go +++ b/overlay/tun_openbsd.go @@ -399,7 +399,8 @@ func (t *tun) addRoutes(logErrors bool) error { func (t *tun) removeRoutes(routes []Route) error { for _, r := range routes { - if !r.Install { + if len(r.Via) == 0 || !r.Install { + // addRoutes never installs these continue }