Merge remote-tracking branch 'upstream/unsafe-route-reload'

overlay/route.go

@@ -22,6 +22,8 @@ type Route struct {
 	Install bool
 }
 
+// Equal determines if a route that could be installed in the system route table is equal to another
+// Via is ignored since that is only consumed within nebula itself
 func (r Route) Equal(t Route) bool {
 	if !r.Cidr.IP.Equal(t.Cidr.IP) {
 		return false
@@ -35,6 +37,9 @@ func (r Route) Equal(t Route) bool {
 	if r.MTU != t.MTU {
 		return false
 	}
+	if r.Install != t.Install {
+		return false
+	}
 	return true
 }
 

overlay/tun.go

@@ -49,6 +49,8 @@ func getAllRoutesFromConfig(c *config.C, cidr *net.IPNet, initial bool) (bool, [
 	return true, routes, nil
 }
 
+// findRemovedRoutes will return all routes that are not present in the newRoutes list and would affect the system route table.
+// Via is not used to evaluate since it does not affect the system route table.
 func findRemovedRoutes(newRoutes, oldRoutes []Route) []Route {
 	var removed []Route
 	has := func(entry Route) bool {

overlay/tun_android.go

@@ -8,52 +8,85 @@ import (
 	"io"
 	"net"
 	"os"
+	"sync/atomic"
 
 	"github.com/sirupsen/logrus"
 	"github.com/slackhq/nebula/cidr"
+	"github.com/slackhq/nebula/config"
 	"github.com/slackhq/nebula/iputil"
+	"github.com/slackhq/nebula/util"
 )
 
 type tun struct {
 	io.ReadWriteCloser
 	fd        int
 	cidr      *net.IPNet
-	routeTree *cidr.Tree4[iputil.VpnIp]
+	Routes    atomic.Pointer[[]Route]
+	routeTree atomic.Pointer[cidr.Tree4[iputil.VpnIp]]
 	l         *logrus.Logger
 }
 
-func newTunFromFd(l *logrus.Logger, deviceFd int, cidr *net.IPNet, _ int, routes []Route, _ int, _ bool) (*tun, error) {
-	routeTree, err := makeRouteTree(l, routes, false)
-	if err != nil {
-		return nil, err
-	}
-
+func newTunFromFd(c *config.C, l *logrus.Logger, deviceFd int, cidr *net.IPNet) (*tun, error) {
 	// XXX Android returns an fd in non-blocking mode which is necessary for shutdown to work properly.
 	// Be sure not to call file.Fd() as it will set the fd to blocking mode.
 	file := os.NewFile(uintptr(deviceFd), "/dev/net/tun")
 
-	return &tun{
+	t := &tun{
 		ReadWriteCloser: file,
 		fd:              deviceFd,
 		cidr:            cidr,
 		l:               l,
-		routeTree:       routeTree,
-	}, nil
+	}
+
+	err := t.reload(c, true)
+	if err != nil {
+		return nil, err
+	}
+
+	c.RegisterReloadCallback(func(c *config.C) {
+		err := t.reload(c, false)
+		if err != nil {
+			util.LogWithContextIfNeeded("failed to reload tun device", err, t.l)
+		}
+	})
+
+	return t, nil
 }
 
-func newTun(_ *logrus.Logger, _ string, _ *net.IPNet, _ int, _ []Route, _ int, _ bool, _ bool) (*tun, error) {
+func newTun(_ *config.C, _ *logrus.Logger, _ *net.IPNet, _ bool) (*tun, error) {
 	return nil, fmt.Errorf("newTun not supported in Android")
 }
 
 func (t *tun) RouteFor(ip iputil.VpnIp) iputil.VpnIp {
-	_, r := t.routeTree.MostSpecificContains(ip)
+	_, r := t.routeTree.Load().MostSpecificContains(ip)
 	return r
 }
 
 func (t tun) Activate() error {
 	return nil
 }
 
+func (t *tun) reload(c *config.C, initial bool) error {
+	change, routes, err := getAllRoutesFromConfig(c, t.cidr, initial)
+	if err != nil {
+		return err
+	}
+
+	if !initial && !change {
+		return nil
+	}
+
+	routeTree, err := makeRouteTree(t.l, routes, false)
+	if err != nil {
+		return err
+	}
+
+	// Teach nebula how to handle the routes
+	t.Routes.Store(&routes)
+	t.routeTree.Store(routeTree)
+	return nil
+}
+
 func (t *tun) Cidr() *net.IPNet {
 	return t.cidr
 }

overlay/tun_freebsd.go

@@ -13,12 +13,15 @@ import (
 	"os"
 	"os/exec"
 	"strconv"
+	"sync/atomic"
 	"syscall"
 	"unsafe"
 
 	"github.com/sirupsen/logrus"
 	"github.com/slackhq/nebula/cidr"
+	"github.com/slackhq/nebula/config"
 	"github.com/slackhq/nebula/iputil"
+	"github.com/slackhq/nebula/util"
 )
 
 const (
@@ -47,8 +50,8 @@ type tun struct {
 	Device    string
 	cidr      *net.IPNet
 	MTU       int
-	Routes    []Route
-	routeTree *cidr.Tree4[iputil.VpnIp]
+	Routes    atomic.Pointer[[]Route]
+	routeTree atomic.Pointer[cidr.Tree4[iputil.VpnIp]]
 	l         *logrus.Logger
 
 	io.ReadWriteCloser
@@ -76,14 +79,15 @@ func (t *tun) Close() error {
 	return nil
 }
 
-func newTunFromFd(_ *logrus.Logger, _ int, _ *net.IPNet, _ int, _ []Route, _ int, _ bool) (*tun, error) {
+func newTunFromFd(_ *config.C, _ *logrus.Logger, _ int, _ *net.IPNet) (*tun, error) {
 	return nil, fmt.Errorf("newTunFromFd not supported in FreeBSD")
 }
 
-func newTun(l *logrus.Logger, deviceName string, cidr *net.IPNet, defaultMTU int, routes []Route, _ int, _ bool, _ bool) (*tun, error) {
+func newTun(c *config.C, l *logrus.Logger, cidr *net.IPNet, _ bool) (*tun, error) {
 	// Try to open existing tun device
 	var file *os.File
 	var err error
+	deviceName := c.GetString("tun.dev", "")
 	if deviceName != "" {
 		file, err = os.OpenFile("/dev/"+deviceName, os.O_RDWR, 0)
 	}
@@ -144,20 +148,27 @@ func newTun(l *logrus.Logger, deviceName string, cidr *net.IPNet, defaultMTU int
 		ioctl(fd, syscall.SIOCSIFNAME, uintptr(unsafe.Pointer(&ifrr)))
 	}
 
-	routeTree, err := makeRouteTree(l, routes, false)
-	if err != nil {
-		return nil, err
-	}
-
-	return &tun{
+	t := &tun{
 		ReadWriteCloser: file,
 		Device:          deviceName,
 		cidr:            cidr,
-		MTU:             defaultMTU,
-		Routes:          routes,
-		routeTree:       routeTree,
+		MTU:             c.GetInt("tun.mtu", DefaultMTU),
 		l:               l,
-	}, nil
+	}
+
+	err = t.reload(c, true)
+	if err != nil {
+		return nil, err
+	}
+
+	c.RegisterReloadCallback(func(c *config.C) {
+		err := t.reload(c, false)
+		if err != nil {
+			util.LogWithContextIfNeeded("failed to reload tun device", err, t.l)
+		}
+	})
+
+	return t, nil
 }
 
 func (t *tun) Activate() error {
@@ -175,24 +186,50 @@ func (t *tun) Activate() error {
 	if err = exec.Command("/sbin/ifconfig", t.Device, "mtu", strconv.Itoa(t.MTU)).Run(); err != nil {
 		return fmt.Errorf("failed to run 'ifconfig': %s", err)
 	}
+
 	// Unsafe path routes
-	for _, r := range t.Routes {
-		if r.Via == nil || !r.Install {
-			// We don't allow route MTUs so only install routes with a via
-			continue
+	return t.addRoutes(false)
+}
+
+func (t *tun) reload(c *config.C, initial bool) error {
+	change, routes, err := getAllRoutesFromConfig(c, t.cidr, initial)
+	if err != nil {
+		return err
+	}
+
+	if !initial && !change {
+		return nil
+	}
+
+	routeTree, err := makeRouteTree(t.l, routes, false)
+	if err != nil {
+		return err
+	}
+
+	// Teach nebula how to handle the routes before establishing them in the system table
+	oldRoutes := t.Routes.Swap(&routes)
+	t.routeTree.Store(routeTree)
+
+	if !initial {
+		// Remove first, if the system removes a wanted route hopefully it will be re-added next
+		err := t.removeRoutes(findRemovedRoutes(routes, *oldRoutes))
+		if err != nil {
+			util.LogWithContextIfNeeded("Failed to remove routes", err, t.l)
 		}
 
-		t.l.Debug("command: route", "-n", "add", "-net", r.Cidr.String(), "-interface", t.Device)
-		if err = exec.Command("/sbin/route", "-n", "add", "-net", r.Cidr.String(), "-interface", t.Device).Run(); err != nil {
-			return fmt.Errorf("failed to run 'route add' for unsafe_route %s: %s", r.Cidr.String(), err)
+		// Ensure any routes we actually want are installed
+		err = t.addRoutes(true)
+		if err != nil {
+			// Catch any stray logs
+			util.LogWithContextIfNeeded("Failed to add routes", err, t.l)
 		}
 	}
 
 	return nil
 }
 
 func (t *tun) RouteFor(ip iputil.VpnIp) iputil.VpnIp {
-	_, r := t.routeTree.MostSpecificContains(ip)
+	_, r := t.routeTree.Load().MostSpecificContains(ip)
 	return r
 }
 
@@ -208,6 +245,46 @@ func (t *tun) NewMultiQueueReader() (io.ReadWriteCloser, error) {
 	return nil, fmt.Errorf("TODO: multiqueue not implemented for freebsd")
 }
 
+func (t *tun) addRoutes(logErrors bool) error {
+	routes := *t.Routes.Load()
+	for _, r := range routes {
+		if r.Via == nil || !r.Install {
+			// We don't allow route MTUs so only install routes with a via
+			continue
+		}
+
+		cmd := exec.Command("/sbin/route", "-n", "add", "-net", r.Cidr.String(), "-interface", t.Device)
+		t.l.Debug("command: ", cmd.String())
+		if err := cmd.Run(); err != nil {
+			retErr := util.NewContextualError("failed to run 'route add' for unsafe_route", map[string]interface{}{"route": r}, err)
+			if logErrors {
+				retErr.Log(t.l)
+			} else {
+				return retErr
+			}
+		}
+	}
+
+	return nil
+}
+
+func (t *tun) removeRoutes(routes []Route) error {
+	for _, r := range routes {
+		if !r.Install {
+			continue
+		}
+
+		cmd := exec.Command("/sbin/route", "-n", "delete", "-net", r.Cidr.String(), "-interface", t.Device)
+		t.l.Debug("command: ", cmd.String())
+		if err := cmd.Run(); err != nil {
+			t.l.WithError(err).WithField("route", r).Error("Failed to remove route")
+		} else {
+			t.l.WithField("route", r).Info("Removed route")
+		}
+	}
+	return nil
+}
+
 func (t *tun) deviceBytes() (o [16]byte) {
 	for i, c := range t.Device {
 		o[i] = byte(c)