wgengine: randomize client port if netmap says to

For testing out #2187

Signed-off-by: David Crawshaw <[email protected]>

ipn/ipnlocal/local.go

@@ -1838,7 +1838,7 @@ func (b *LocalBackend) authReconfig() {
 		}
 	}
 
-	err = b.e.Reconfig(cfg, rcfg, &dcfg)
+	err = b.e.Reconfig(cfg, rcfg, &dcfg, nm.Debug)
 	if err == wgengine.ErrNoChanges {
 		return
 	}
@@ -2199,7 +2199,7 @@ func (b *LocalBackend) enterState(newState ipn.State) {
 		b.blockEngineUpdates(true)
 		fallthrough
 	case ipn.Stopped:
-		err := b.e.Reconfig(&wgcfg.Config{}, &router.Config{}, &dns.Config{})
+		err := b.e.Reconfig(&wgcfg.Config{}, &router.Config{}, &dns.Config{}, nil)
 		if err != nil {
 			b.logf("Reconfig(down): %v", err)
 		}
@@ -2319,7 +2319,7 @@ func (b *LocalBackend) stateMachine() {
 // a status update that predates the "I've shut down" update.
 func (b *LocalBackend) stopEngineAndWait() {
 	b.logf("stopEngineAndWait...")
-	b.e.Reconfig(&wgcfg.Config{}, &router.Config{}, &dns.Config{})
+	b.e.Reconfig(&wgcfg.Config{}, &router.Config{}, &dns.Config{}, nil)
 	b.requestEngineStatusAndWait()
 	b.logf("stopEngineAndWait: done.")
 }

wgengine/bench/wg.go

@@ -127,7 +127,7 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netadd
 			Endpoints:  endpoint,
 		}
 		c2.Peers = []wgcfg.Peer{p}
-		e2.Reconfig(&c2, &router.Config{}, new(dns.Config))
+		e2.Reconfig(&c2, &router.Config{}, new(dns.Config), nil)
 		e1waitDoneOnce.Do(wait.Done)
 	})
 
@@ -171,7 +171,7 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netadd
 			Endpoints:  endpoint,
 		}
 		c1.Peers = []wgcfg.Peer{p}
-		e1.Reconfig(&c1, &router.Config{}, new(dns.Config))
+		e1.Reconfig(&c1, &router.Config{}, new(dns.Config), nil)
 		e2waitDoneOnce.Do(wait.Done)
 	})
 

wgengine/userspace.go

@@ -87,6 +87,7 @@ type userspaceEngine struct {
 	tundev            *tstun.Wrapper
 	wgdev             *device.Device
 	router            router.Router
+	confListenPort    uint16 // original conf.ListenPort
 	dns               *dns.Manager
 	magicConn         *magicsock.Conn
 	linkMon           *monitor.Mon
@@ -232,12 +233,13 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error)
 	closePool.add(tsTUNDev)
 
 	e := &userspaceEngine{
-		timeNow: time.Now,
-		logf:    logf,
-		reqCh:   make(chan struct{}, 1),
-		waitCh:  make(chan struct{}),
-		tundev:  tsTUNDev,
-		router:  conf.Router,
+		timeNow:        time.Now,
+		logf:           logf,
+		reqCh:          make(chan struct{}, 1),
+		waitCh:         make(chan struct{}),
+		tundev:         tsTUNDev,
+		router:         conf.Router,
+		confListenPort: conf.ListenPort,
 	}
 	e.isLocalAddr.Store(tsaddr.NewContainsIPFunc(nil))
 
@@ -732,7 +734,7 @@ func (e *userspaceEngine) updateActivityMapsLocked(trackDisco []tailcfg.DiscoKey
 	e.tundev.SetDestIPActivityFuncs(e.destIPActivityFuncs)
 }
 
-func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, dnsCfg *dns.Config) error {
+func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, dnsCfg *dns.Config, debug *tailcfg.Debug) error {
 	if routerCfg == nil {
 		panic("routerCfg must not be nil")
 	}
@@ -754,9 +756,14 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config,
 	}
 	e.mu.Unlock()
 
+	listenPort := e.confListenPort
+	if debug != nil && debug.RandomizeClientPort {
+		listenPort = 0
+	}
+
 	engineChanged := deephash.UpdateHash(&e.lastEngineSigFull, cfg)
 	routerChanged := deephash.UpdateHash(&e.lastRouterSig, routerCfg, dnsCfg)
-	if !engineChanged && !routerChanged {
+	if !engineChanged && !routerChanged && listenPort == e.magicConn.LocalPort() {
 		return ErrNoChanges
 	}
 
@@ -795,6 +802,7 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config,
 		e.logf("wgengine: Reconfig: SetPrivateKey: %v", err)
 	}
 	e.magicConn.UpdatePeers(peerSet)
+	e.magicConn.SetPreferredPort(listenPort)
 
 	if err := e.maybeReconfigWireguardLocked(discoChanged); err != nil {
 		return err

wgengine/userspace_test.go

@@ -109,7 +109,7 @@ func TestUserspaceEngineReconfig(t *testing.T) {
 			},
 		}
 
-		err = e.Reconfig(cfg, routerCfg, &dns.Config{})
+		err = e.Reconfig(cfg, routerCfg, &dns.Config{}, nil)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -130,6 +130,71 @@ func TestUserspaceEngineReconfig(t *testing.T) {
 	}
 }
 
+func TestUserspaceEnginePortReconfig(t *testing.T) {
+	const defaultPort = 49983
+	// Keep making a wgengine until we find an unused port
+	var ue *userspaceEngine
+	for i := 0; i < 100; i++ {
+		attempt := uint16(defaultPort + i)
+		e, err := NewFakeUserspaceEngine(t.Logf, attempt)
+		if err != nil {
+			t.Fatal(err)
+		}
+		ue = e.(*userspaceEngine)
+		if ue.magicConn.LocalPort() == attempt {
+			break
+		}
+		ue.Close()
+		ue = nil
+	}
+	if ue == nil {
+		t.Fatal("could not create a wgengine with a specific port")
+	}
+	defer ue.Close()
+
+	startingPort := ue.magicConn.LocalPort()
+	discoKey := dkFromHex("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")
+	cfg := &wgcfg.Config{
+		Peers: []wgcfg.Peer{
+			{
+				AllowedIPs: []netaddr.IPPrefix{
+					netaddr.IPPrefixFrom(netaddr.IPv4(100, 100, 99, 1), 32),
+				},
+				Endpoints: wgcfg.Endpoints{DiscoKey: discoKey},
+			},
+		},
+	}
+	routerCfg := &router.Config{}
+	if err := ue.Reconfig(cfg, routerCfg, &dns.Config{}, nil); err != nil {
+		t.Fatal(err)
+	}
+	if got := ue.magicConn.LocalPort(); got != startingPort {
+		t.Errorf("no debug setting changed local port to %d from %d", got, startingPort)
+	}
+	if err := ue.Reconfig(cfg, routerCfg, &dns.Config{}, &tailcfg.Debug{RandomizeClientPort: true}); err != nil {
+		t.Fatal(err)
+	}
+	if got := ue.magicConn.LocalPort(); got == startingPort {
+		t.Errorf("debug setting did not change local port from %d", startingPort)
+	}
+
+	lastPort := ue.magicConn.LocalPort()
+	if err := ue.Reconfig(cfg, routerCfg, &dns.Config{}, nil); err != nil {
+		t.Fatal(err)
+	}
+	if startingPort == defaultPort {
+		// Only try this if we managed to bind defaultPort the first time.
+		// Otherwise, assume someone else on the computer is using defaultPort
+		// and so Reconfig would have caused magicSockt to bind some other port.
+		if got := ue.magicConn.LocalPort(); got != defaultPort {
+			t.Errorf("debug setting did not change local port from %d to %d", startingPort, defaultPort)
+		}
+	}
+	if got := ue.magicConn.LocalPort(); got == lastPort {
+		t.Errorf("Reconfig did not change local port from %d", lastPort)
+	}
+}
+
 func dkFromHex(hex string) tailcfg.DiscoKey {
 	if len(hex) != 64 {
 		panic(fmt.Sprintf("%q is len %d; want 64", hex, len(hex)))

wgengine/watchdog.go

@@ -74,8 +74,8 @@ func (e *watchdogEngine) watchdog(name string, fn func()) {
 	})
 }
 
-func (e *watchdogEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, dnsCfg *dns.Config) error {
-	return e.watchdogErr("Reconfig", func() error { return e.wrap.Reconfig(cfg, routerCfg, dnsCfg) })
+func (e *watchdogEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, dnsCfg *dns.Config, debug *tailcfg.Debug) error {
+	return e.watchdogErr("Reconfig", func() error { return e.wrap.Reconfig(cfg, routerCfg, dnsCfg, debug) })
 }
 func (e *watchdogEngine) GetLinkMonitor() *monitor.Mon {
 	return e.wrap.GetLinkMonitor()

wgengine/wgengine.go

@@ -56,8 +56,10 @@ type Engine interface {
 	// This is called whenever tailcontrol (the control plane)
 	// sends an updated network map.
 	//
+	// The *tailcfg.Debug parameter can be nil.
+	//
 	// The returned error is ErrNoChanges if no changes were made.
-	Reconfig(*wgcfg.Config, *router.Config, *dns.Config) error
+	Reconfig(*wgcfg.Config, *router.Config, *dns.Config, *tailcfg.Debug) error
 
 	// GetFilter returns the current packet filter, if any.
 	GetFilter() *filter.Filter