Let the darwin tun adopt an fd the host already configured

e2e/recovery_timing_test.go

@@ -0,0 +1,136 @@
+//go:build e2e_testing
+// +build e2e_testing
+
+package e2e
+
+import (
+	"testing"
+	"time"
+
+	"github.com/slackhq/nebula"
+	"github.com/slackhq/nebula/cert"
+	"github.com/slackhq/nebula/cert_test"
+	"github.com/slackhq/nebula/e2e/router"
+	"github.com/slackhq/nebula/udp"
+)
+
+// TestRecoveryTiming measures how long a tunnel takes to come back after the peer stops accepting our traffic,
+// which is what a laptop waking on a new network looks like from the peer's side: its NAT has no state for where
+// we are now, so everything we send disappears.
+//
+// It is a measurement, not a pass/fail assertion. Recovery is timed to the moment the peer punches back at us,
+// since that is when its NAT opens and the tunnel is usable again.
+//
+//	go test -tags e2e_testing -v -run TestRecoveryTiming ./e2e/
+func TestRecoveryTiming(t *testing.T) {
+	for _, tc := range []struct {
+		name   string
+		rebind bool
+	}{
+		{"no trigger", false},
+		{"rebind counter", true},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			d, lost := measureRecovery(t, tc.rebind)
+			t.Logf("RESULT %-16s recovered in %-9v (%d packets lost)", tc.name, d.Round(time.Millisecond), lost)
+		})
+	}
+}
+
+// measureRecovery returns how long until the peer punched back, and how many of our packets died meanwhile. When
+// rebind is true we call RebindUDPServer once the tunnel goes dark, which is what the darwin network change
+// monitor does and what iOS has always done. When false, nothing tells nebula anything is wrong.
+func measureRecovery(t *testing.T, rebind bool) (time.Duration, int) {
+	t.Helper()
+	ca, _, caKey, _ := cert_test.NewTestCaCert(cert.Version2, cert.Curve_CURVE25519, time.Now(), time.Now().Add(10*time.Minute), nil, nil, []string{})
+
+	lhControl, lhVpnIpNet, lhUdpAddr, _ := newSimpleServer(cert.Version2, ca, caKey, "lh", "10.128.0.1/24", m{
+		"lighthouse": m{"am_lighthouse": true},
+	})
+
+	peerCfg := m{
+		"lighthouse": m{
+			"hosts":    []any{lhVpnIpNet[0].Addr().String()},
+			"interval": 600,
+			"local_allow_list": m{
+				"10.0.0.0/24": true,
+				"::/0":        false,
+			},
+		},
+		"static_host_map": m{
+			lhVpnIpNet[0].Addr().String(): []any{lhUdpAddr.String()},
+		},
+	}
+
+	myControl, myVpnIpNet, myUdpAddr, _ := newSimpleServer(cert.Version2, ca, caKey, "me", "10.128.0.2/24", peerCfg)
+	theirControl, theirVpnIpNet, theirUdpAddr, _ := newSimpleServer(cert.Version2, ca, caKey, "them", "10.128.0.3/24", peerCfg)
+
+	r := router.NewR(t, lhControl, myControl, theirControl)
+	defer r.RenderFlow()
+	defer func() {
+		lhControl.Stop()
+		myControl.Stop()
+		theirControl.Stop()
+	}()
+
+	lhControl.Start()
+	myControl.Start()
+	theirControl.Start()
+	r.RouteFor(time.Millisecond * 500)
+
+	myControl.InjectLightHouseAddr(theirVpnIpNet[0].Addr(), theirUdpAddr)
+	theirControl.InjectLightHouseAddr(myVpnIpNet[0].Addr(), myUdpAddr)
+
+	myControl.InjectTunPacket(BuildTunUDPPacket(theirVpnIpNet[0].Addr(), 80, myVpnIpNet[0].Addr(), 80, []byte("establish")))
+	r.RouteFor(time.Second)
+	if myControl.GetHostInfoByVpnAddr(theirVpnIpNet[0].Addr(), false) == nil {
+		t.Fatal("failed to establish the tunnel we are measuring")
+	}
+	r.RouteFor(time.Millisecond * 500)
+
+	// From here the peer's NAT has no state for us, everything we send it disappears
+	start := time.Now()
+	blackholed := 0
+	var recovered time.Duration
+
+	if rebind {
+		myControl.RebindUDPServer()
+	}
+
+	// Keep the tun busy the way someone retrying a stalled connection would
+	stop := make(chan struct{})
+	defer close(stop)
+	go func() {
+		tick := time.NewTicker(time.Millisecond * 200)
+		defer tick.Stop()
+		for {
+			select {
+			case <-stop:
+				return
+			case <-tick.C:
+				myControl.InjectTunPacket(BuildTunUDPPacket(
+					theirVpnIpNet[0].Addr(), 80, myVpnIpNet[0].Addr(), 80, []byte("retry")))
+			}
+		}
+	}()
+
+	r.RouteForAllExitFuncOrTimeout(time.Second*30, func(p *udp.Packet, c *nebula.Control) router.ExitType {
+		if c == theirControl && p.From == myControl.GetUDPAddr() {
+			blackholed++
+			return router.Drop
+		}
+
+		// The peer reaching us directly is the moment its NAT opened, whether that is a punch or a handshake
+		if c == myControl && p.From == theirUdpAddr {
+			recovered = time.Since(start)
+			return router.RouteAndExit
+		}
+
+		return router.KeepRouting
+	})
+
+	if recovered == 0 {
+		t.Fatalf("no recovery within 30s (%d packets blackholed)", blackholed)
+	}
+	return recovered, blackholed
+}

e2e/router/router.go

@@ -153,6 +153,9 @@ const (
 	ExitNow ExitType = 1
 	// RouteAndExit routes this packet and exits immediately afterwards
 	RouteAndExit ExitType = 2
+	// Drop discards this packet without delivering it and keeps routing. Use it to simulate a blackhole, such as
+	// a restrictive NAT refusing traffic from an address it has not seen.
+	Drop ExitType = 3
 )
 
 type ExitFunc func(packet *udp.Packet, receiver *nebula.Control) ExitType
@@ -163,7 +166,9 @@ type ExitFunc func(packet *udp.Packet, receiver *nebula.Control) ExitType
 func NewR(t testing.TB, controls ...*nebula.Control) *R {
 	ctx, cancel := context.WithCancel(context.Background())
 
-	if err := os.MkdirAll("mermaid", 0755); err != nil {
+	// t.Name() contains a slash for subtests, so the flow log can land in a nested directory
+	fn := filepath.Join("mermaid", fmt.Sprintf("%s.md", t.Name()))
+	if err := os.MkdirAll(filepath.Dir(fn), 0755); err != nil {
 		panic(err)
 	}
 
@@ -174,7 +179,7 @@ func NewR(t testing.TB, controls ...*nebula.Control) *R {
 		outNat:       make(map[outNatKey]netip.AddrPort),
 		flow:         []flowEntry{},
 		ignoreFlows:  []ignoreFlow{},
-		fn:           filepath.Join("mermaid", fmt.Sprintf("%s.md", t.Name())),
+		fn:           fn,
 		t:            t,
 		cancelRender: cancel,
 	}
@@ -687,6 +692,10 @@ func (r *R) RouteExitFunc(sender *nebula.Control, whatDo ExitFunc) {
 			p.Release()
 			return
 
+		case Drop:
+			// Record it so the flow log shows the attempt, but never hand it to the receiver
+			r.unlockedInjectFlow(sender, receiver, p, false)
+
 		case KeepRouting:
 			fp := r.unlockedInjectFlow(sender, receiver, p, false)
 			receiver.InjectUDPPacket(p)
@@ -779,6 +788,10 @@ func (r *R) RouteForAllExitFuncOrTimeout(timeout time.Duration, whatDo ExitFunc)
 			p.Release()
 			return true
 
+		case Drop:
+			// Record it so the flow log shows the attempt, but never hand it to the receiver
+			r.unlockedInjectFlow(cm[x], receiver, p, false)
+
 		case KeepRouting:
 			fp := r.unlockedInjectFlow(cm[x], receiver, p, false)
 			receiver.InjectUDPPacket(p)
@@ -884,6 +897,10 @@ func (r *R) RouteForAllExitFunc(whatDo ExitFunc) {
 			p.Release()
 			return
 
+		case Drop:
+			// Record it so the flow log shows the attempt, but never hand it to the receiver
+			r.unlockedInjectFlow(cm[x], receiver, p, false)
+
 		case KeepRouting:
 			fp := r.unlockedInjectFlow(cm[x], receiver, p, false)
 			receiver.InjectUDPPacket(p)

main.go

@@ -22,9 +22,12 @@ type m = map[string]any
 
 func Main(c *config.C, configTest bool, buildVersion string, l *slog.Logger, deviceFactory overlay.DeviceFactory) (retcon *Control, reterr error) {
 	ctx, cancel := context.WithCancel(context.Background())
-	// Automatically cancel the context if Main returns an error, to signal all created goroutines to quit.
+	// The goroutines started below stop only when this context does, and only a caller holding the
+	// Control can arrange that. Cancel whenever we are not handing one back, which covers an error
+	// and a config test alike: a config test used to leave the lighthouse query worker, and a
+	// hostname resolver per dns named static host, running for the life of the process.
 	defer func() {
-		if reterr != nil {
+		if retcon == nil {
 			cancel()
 		}
 	}()

main_test.go

@@ -0,0 +1,82 @@
+package nebula
+
+import (
+	"fmt"
+	"net/netip"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/slackhq/nebula/cert"
+	cert_test "github.com/slackhq/nebula/cert_test"
+	"github.com/slackhq/nebula/config"
+	"github.com/slackhq/nebula/test"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/goleak"
+)
+
+// TestMain_ConfigTestReleasesItsGoroutines pins the rule that Main only leaves goroutines running
+// when it hands back a Control to stop them with.
+//
+// A config test gets no Control, so anything it started had nothing to stop it: the lighthouse
+// query worker, and a hostname resolver per dns named static host, ran for the life of the
+// process. That matters to every embedder that validates a config in process rather than by
+// exec'ing, dnclient and the apple clients included, because they do it on each config load and
+// the leak accumulates.
+func TestMain_ConfigTestReleasesItsGoroutines(t *testing.T) {
+	defer goleak.VerifyNone(t, goleak.IgnoreCurrent())
+
+	l := test.NewLogger()
+	dir := t.TempDir()
+
+	before := time.Now().Add(-time.Hour)
+	after := time.Now().Add(time.Hour)
+	ca, _, caKey, caPEM := cert_test.NewTestCaCert(cert.Version2, cert.Curve_CURVE25519, before, after, nil, nil, nil)
+	networks := []netip.Prefix{netip.MustParsePrefix("10.0.0.1/24")}
+	_, _, keyPEM, certPEM := cert_test.NewTestCert(
+		cert.Version2, cert.Curve_CURVE25519, ca, caKey, "config-test", before, after, networks, nil, nil)
+
+	caPath := filepath.Join(dir, "ca.pem")
+	certPath := filepath.Join(dir, "cert.pem")
+	keyPath := filepath.Join(dir, "key.pem")
+	require.NoError(t, os.WriteFile(caPath, caPEM, 0o600))
+	require.NoError(t, os.WriteFile(certPath, certPEM, 0o600))
+	require.NoError(t, os.WriteFile(keyPath, keyPEM, 0o600))
+
+	// A static host by address, not by name: the query worker is the goroutine under test and a
+	// hostname would drag a real dns lookup into a unit test.
+	configBody := fmt.Sprintf(`
+pki:
+  ca: %s
+  cert: %s
+  key: %s
+static_host_map:
+  "10.0.0.2": ["192.0.2.1:4242"]
+lighthouse:
+  hosts:
+    - "10.0.0.2"
+listen:
+  host: 127.0.0.1
+  port: 0
+tun:
+  disabled: true
+firewall:
+  outbound:
+    - port: any
+      proto: any
+      host: any
+  inbound:
+    - port: any
+      proto: any
+      host: any
+`, caPath, certPath, keyPath)
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "config.yml"), []byte(configBody), 0o600))
+
+	c := config.NewC(l)
+	require.NoError(t, c.Load(dir))
+
+	ctrl, err := Main(c, true, "config-test", l, nil)
+	require.NoError(t, err)
+	require.Nil(t, ctrl, "a config test hands back nothing to stop, so it must stop itself")
+}

overlay/tun_darwin.go

@@ -30,6 +30,9 @@ type tun struct {
 	Routes      atomic.Pointer[[]Route]
 	routeTree   atomic.Pointer[bart.Table[routing.Gateways]]
 	linkAddr    *netroute.LinkAddr
+	// hostOwned means the fd arrived from the OS, which has already configured addressing, mtu
+	// and routes for it. NEPacketTunnelProvider on darwin does this.
+	hostOwned bool
 	l         *slog.Logger
 }
 
@@ -150,8 +153,48 @@ func (t *tun) deviceBytes() (o [16]byte) {
 	return
 }
 
-func newTunFromFd(_ *config.C, _ *slog.Logger, _ int, _ []netip.Prefix) (*tun, error) {
-	return nil, fmt.Errorf("newTunFromFd not supported in Darwin")
+// newTunFromFd adopts a utun the host already created and configured, which is how a darwin
+// network extension is handed its device. Everything about moving packets is shared with newTun,
+// only the setup differs: the host owns addressing and routing here.
+func newTunFromFd(c *config.C, l *slog.Logger, deviceFd int, vpnNetworks []netip.Prefix) (*tun, error) {
+	if err := unix.SetNonblock(deviceFd, true); err != nil {
+		// We own the fd from the moment it is handed to us
+		_ = unix.Close(deviceFd)
+		return nil, fmt.Errorf("failed to set the tun fd to non-blocking mode: %w", err)
+	}
+
+	file := os.NewFile(uintptr(deviceFd), "/dev/tun")
+	t := &tun{
+		f:           file,
+		Device:      utunNameFromFd(deviceFd),
+		vpnNetworks: vpnNetworks,
+		DefaultMTU:  c.GetInt("tun.mtu", DefaultMTU),
+		hostOwned:   true,
+		l:           l,
+	}
+
+	if err := t.reload(c, true); err != nil {
+		_ = file.Close()
+		return nil, err
+	}
+
+	c.RegisterReloadCallback(func(c *config.C) {
+		if err := t.reload(c, false); err != nil {
+			util.LogWithContextIfNeeded("failed to reload tun device", err, t.l)
+		}
+	})
+
+	return t, nil
+}
+
+// utunNameFromFd asks the socket what interface it is, for logs. A blank name is not worth
+// failing a tunnel over, so an error just leaves it empty.
+func utunNameFromFd(fd int) string {
+	name, err := unix.GetsockoptString(fd, unix.AF_SYS_CONTROL, _UTUN_OPT_IFNAME)
+	if err != nil {
+		return ""
+	}
+	return name
 }
 
 func (t *tun) Close() error {
@@ -162,6 +205,12 @@ func (t *tun) Close() error {
 }
 
 func (t *tun) Activate() error {
+	// The host handed us a configured device. Its addresses, mtu and routes come from the network
+	// settings it applied, and a sandboxed extension cannot change them anyway.
+	if t.hostOwned {
+		return nil
+	}
+
 	devName := t.deviceBytes()
 
 	s, err := unix.Socket(
@@ -375,6 +424,11 @@ func getLinkAddr(name string) (*netroute.LinkAddr, error) {
 }
 
 func (t *tun) addRoutes(logErrors bool) error {
+	// The route tree is still ours, the system routing table is not
+	if t.hostOwned {
+		return nil
+	}
+
 	routes := *t.Routes.Load()
 
 	for _, r := range routes {
@@ -404,6 +458,10 @@ func (t *tun) addRoutes(logErrors bool) error {
 }
 
 func (t *tun) removeRoutes(routes []Route) error {
+	if t.hostOwned {
+		return nil
+	}
+
 	for _, r := range routes {
 		if !r.Install {
 			continue