wgengine/wgcfg,wgengine,ipn/ipnlocal: remove Peers from wgcfg.Config

The wireguard-go device now learns its peer set solely from the live
per-peer config source that LocalBackend installs with
Engine.SetPeerConfigFunc, backed by the route manager. Peers are
created lazily on first packet and converged per peer with
Engine.SyncDevicePeer, so the full-peer-list snapshot in wgcfg.Config
and the diff-and-reconfigure machinery around it (wgcfg.Peer,
ReconfigDevice, and the engine's full device sync in
maybeReconfigWireguardLocked) are dead weight: they duplicated state
that the route manager already owns and forced every netmap change to
rebuild and rehash the entire peer list.

Delete the Peers field and the Peer type from wgcfg, along with
ReconfigDevice and maybeReconfigWireguardLocked. Engine.Reconfig no
longer does any device peer work; it only manages the private key,
addresses, and the non-peer subsystems. Full-netmap application converges the device by
syncing exactly the peers whose routes the route manager reports as
changed or removed.

Updates #12542

Change-Id: Ic776e42cfaa5be6b9329b3d381d5cbde17d7078b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

ipn/ipnlocal/local.go

@@ -6144,13 +6144,14 @@ func (b *LocalBackend) authReconfigLocked() {
 		return
 	}
 
-	oneCGNATRoute := shouldUseOneCGNATRoute(b.logf, b.sys.NetMon.Get(), b.sys.ControlKnobs(), version.OS())
+	// Note: b.goos (set only by tests) speaks runtime.GOOS while
+	// version.OS is Tailscale-style ("macOS", "iOS"); they agree for
+	// the values tests pin ("linux", "windows"), which is all the
+	// override needs.
+	oneCGNATRoute := shouldUseOneCGNATRoute(b.logf, b.sys.NetMon.Get(), b.sys.ControlKnobs(), cmp.Or(b.goos, version.OS()))
 	// Sync the WireGuard device for any peers whose allowed source
 	// prefixes changed with the new prefs, such as the old and new
-	// exit node when the selection changes. The Reconfig below still
-	// converges every peer via its full device sync, but this
-	// incremental sync is what will remain once the full reconfig is
-	// gated on actual router/DNS changes.
+	// exit node when the selection changes.
 	changedAllowedIPs := cn.updateRouteManagerPrefs(routePrefs{
 		ExitNodeID:       prefs.ExitNodeID(),
 		ExitNodeSelected: prefs.ExitNodeID() != "" || prefs.ExitNodeIP().IsValid(),
@@ -6168,21 +6169,12 @@ func (b *LocalBackend) authReconfigLocked() {
 	// allowed source prefixes (including for lazily created peers)
 	// while keeping them out of the OS route set, because the
 	// expected extension (features/conn25) does not want these routes
-	// installed on the OS. This runs after routerConfigLocked above
-	// for the same reason: rcfg is derived from cfg.Peers, which must
-	// not yet include the extras.
+	// installed on the OS.
 	// See also [Hooks.ExtraWireGuardAllowedIPs].
 	if extraAllowedIPsFn, ok := b.extHost.hooks.ExtraWireGuardAllowedIPs.GetOk(); ok {
 		for k := range cn.updateRouteManagerExtras(extraAllowedIPsFn) {
 			b.e.SyncDevicePeer(k)
 		}
-		// Also append the extras to cfg.Peers so the full SyncPeers
-		// in Reconfig below doesn't strip them from active peers.
-		// This loop goes away when cfg.Peers does.
-		for i := range cfg.Peers {
-			extras := extraAllowedIPsFn(cfg.Peers[i].PublicKey)
-			cfg.Peers[i].AllowedIPs = extras.AppendTo(cfg.Peers[i].AllowedIPs)
-		}
 	}
 	// The prefs and extras commits above can both change the outbound
 	// table (such as installing the selected exit node's /0 routes),
@@ -7342,7 +7334,7 @@ func (b *LocalBackend) setNetMapLocked(nm *netmap.NetworkMap) {
 	if nm != nil {
 		login = cmp.Or(profileFromView(nm.UserProfiles[nm.User()]).LoginName, "<missing-profile>")
 	}
-	discoChanged := b.currentNode().SetNetMap(nm)
+	discoChanged, routeChanged := b.currentNode().SetNetMap(nm)
 	b.setDataPlanePeerRoutes()
 	if ms, ok := b.sys.MagicSock.GetOK(); ok {
 		if nm != nil {
@@ -7361,6 +7353,12 @@ func (b *LocalBackend) setNetMapLocked(nm *netmap.NetworkMap) {
 	for _, k := range discoChanged {
 		b.e.ResetDevicePeer(k)
 	}
+	// Converge the wireguard-go device for peers whose routes changed
+	// (or that were removed) in the full-netmap resync above; peers not
+	// in routeChanged are already up to date.
+	for k := range routeChanged {
+		b.e.SyncDevicePeer(k)
+	}
 	if login != b.activeLogin {
 		b.logf("active login: %v", login)
 		b.activeLogin = login

ipn/ipnlocal/local_test.go

@@ -5367,6 +5367,12 @@ func withAddresses(addresses ...netip.Prefix) peerOptFunc {
 	}
 }
 
+func withAllowedIPs(prefixes ...netip.Prefix) peerOptFunc {
+	return func(n *tailcfg.Node) {
+		n.AllowedIPs = append(n.AllowedIPs, prefixes...)
+	}
+}
+
 func deterministicRegionForTest(t testing.TB, want views.Slice[int], use int) selectRegionFunc {
 	t.Helper()
 

ipn/ipnlocal/node_backend.go

@@ -594,7 +594,7 @@ func (nb *nodeBackend) netMapWithPeers() *netmap.NetworkMap {
 	return nm
 }
 
-func (nb *nodeBackend) SetNetMap(nm *netmap.NetworkMap) (discoChanged []key.NodePublic) {
+func (nb *nodeBackend) SetNetMap(nm *netmap.NetworkMap) (discoChanged []key.NodePublic, routeChanged routemanager.PeersWithRouteChanges) {
 	nb.mu.Lock()
 	defer nb.mu.Unlock()
 	nb.netMap = nm
@@ -602,7 +602,7 @@ func (nb *nodeBackend) SetNetMap(nm *netmap.NetworkMap) (discoChanged []key.Node
 	nb.updateNodeByKeyLocked()
 	nb.updateNodeByStableIDLocked()
 	nb.updateNodeByNameLocked()
-	discoChanged = nb.updatePeersLocked()
+	discoChanged, routeChanged = nb.updatePeersLocked()
 	nb.signalKeyWaitersForTestLocked()
 	if nm != nil {
 		nb.userProfiles = maps.Clone(nm.UserProfiles)
@@ -615,7 +615,7 @@ func (nb *nodeBackend) SetNetMap(nm *netmap.NetworkMap) (discoChanged []key.Node
 		nb.packetFilter = nil
 		nb.derpMapViewPub.Publish(tailcfg.DERPMapView{})
 	}
-	return discoChanged
+	return discoChanged, routeChanged
 }
 
 // AwaitNodeKeyForTest returns a channel that is closed once a peer with the
@@ -796,7 +796,7 @@ func (nb *nodeBackend) ExtraDNSByName(hostname string) (_ netip.Addr, ok bool) {
 	return ip, ok
 }
 
-func (nb *nodeBackend) updatePeersLocked() (discoChanged []key.NodePublic) {
+func (nb *nodeBackend) updatePeersLocked() (discoChanged []key.NodePublic, routeChanged routemanager.PeersWithRouteChanges) {
 	nm := nb.netMap
 	oldIDs := slices.Collect(maps.Keys(nb.peers))
 
@@ -843,8 +843,8 @@ func (nb *nodeBackend) updatePeersLocked() (discoChanged []key.NodePublic) {
 	for _, p := range nb.peers {
 		rt.UpsertPeer(p)
 	}
-	rt.Commit()
-	return discoChanged
+	res := rt.Commit()
+	return discoChanged, res.AllowedIPs
 }
 
 // recordTSMPLearnedDisco notes that a peer's new disco key was learned via

ipn/ipnlocal/node_backend_test.go

@@ -390,31 +390,31 @@ func TestNodeBackendDiscoChanged(t *testing.T) {
 
 	// A brand-new peer is not a disco change.
 	d1 := newDisco()
-	if got := nb.SetNetMap(mkNetMap(d1)); len(got) != 0 {
+	if got, _ := nb.SetNetMap(mkNetMap(d1)); len(got) != 0 {
 		t.Errorf("SetNetMap(new peer) discoChanged = %v; want none", got)
 	}
 
 	// A changed disco key requires a session reset.
 	d2 := newDisco()
-	if got := nb.SetNetMap(mkNetMap(d2)); !slices.Contains(got, nk) {
+	if got, _ := nb.SetNetMap(mkNetMap(d2)); !slices.Contains(got, nk) {
 		t.Errorf("SetNetMap(changed disco) discoChanged = %v; want %v", got, nk)
 	}
 
 	// An unchanged disco key does not.
-	if got := nb.SetNetMap(mkNetMap(d2)); len(got) != 0 {
+	if got, _ := nb.SetNetMap(mkNetMap(d2)); len(got) != 0 {
 		t.Errorf("SetNetMap(same disco) discoChanged = %v; want none", got)
 	}
 
 	// A change already learned via TSMP is suppressed...
 	d3 := newDisco()
 	nb.recordTSMPLearnedDisco(nk, d3)
-	if got := nb.SetNetMap(mkNetMap(d3)); len(got) != 0 {
+	if got, _ := nb.SetNetMap(mkNetMap(d3)); len(got) != 0 {
 		t.Errorf("SetNetMap(TSMP-learned disco) discoChanged = %v; want none", got)
 	}
 
 	// ...but the TSMP entry is consumed, so the next change resets again.
 	d4 := newDisco()
-	if got := nb.SetNetMap(mkNetMap(d4)); !slices.Contains(got, nk) {
+	if got, _ := nb.SetNetMap(mkNetMap(d4)); !slices.Contains(got, nk) {
 		t.Errorf("SetNetMap(after TSMP entry consumed) discoChanged = %v; want %v", got, nk)
 	}
 
@@ -423,7 +423,7 @@ func TestNodeBackendDiscoChanged(t *testing.T) {
 	before := metricTSMPLearnedKeyMismatch.Value()
 	nb.recordTSMPLearnedDisco(nk, newDisco())
 	d5 := newDisco()
-	if got := nb.SetNetMap(mkNetMap(d5)); !slices.Contains(got, nk) {
+	if got, _ := nb.SetNetMap(mkNetMap(d5)); !slices.Contains(got, nk) {
 		t.Errorf("SetNetMap(TSMP mismatch) discoChanged = %v; want %v", got, nk)
 	}
 	if delta := metricTSMPLearnedKeyMismatch.Value() - before; delta != 1 {
@@ -436,15 +436,15 @@ func TestNodeBackendDiscoChanged(t *testing.T) {
 	nb.recordTSMPLearnedDisco(nk, d6)
 	nb.SetNetMap(&netmap.NetworkMap{})
 	nb.SetNetMap(mkNetMap(d5))
-	if got := nb.SetNetMap(mkNetMap(d6)); !slices.Contains(got, nk) {
+	if got, _ := nb.SetNetMap(mkNetMap(d6)); !slices.Contains(got, nk) {
 		t.Errorf("SetNetMap(after TSMP entry GC) discoChanged = %v; want %v", got, nk)
 	}
 
 	// Transitions to or from a zero disco key never reset.
-	if got := nb.SetNetMap(mkNetMap(key.DiscoPublic{})); len(got) != 0 {
+	if got, _ := nb.SetNetMap(mkNetMap(key.DiscoPublic{})); len(got) != 0 {
 		t.Errorf("SetNetMap(to zero disco) discoChanged = %v; want none", got)
 	}
-	if got := nb.SetNetMap(mkNetMap(d1)); len(got) != 0 {
+	if got, _ := nb.SetNetMap(mkNetMap(d1)); len(got) != 0 {
 		t.Errorf("SetNetMap(from zero disco) discoChanged = %v; want none", got)
 	}
 }

ipn/ipnlocal/state_test.go

@@ -1238,10 +1238,10 @@ func TestEngineReconfigOnStateChange(t *testing.T) {
 	connect := &ipn.MaskedPrefs{Prefs: ipn.Prefs{WantRunning: true}, WantRunningSet: true}
 	disconnect := &ipn.MaskedPrefs{Prefs: ipn.Prefs{WantRunning: false}, WantRunningSet: true}
 	node1 := buildNetmapWithPeers(
-		makePeer(1, withName("node-1"), withAddresses(netip.MustParsePrefix("100.64.1.1/32"))),
+		makePeer(1, withName("node-1"), withAddresses(netip.MustParsePrefix("100.64.1.1/32")), withAllowedIPs(netip.MustParsePrefix("100.64.1.1/32"))),
 	)
 	node2 := buildNetmapWithPeers(
-		makePeer(2, withName("node-2"), withAddresses(netip.MustParsePrefix("100.64.1.2/32"))),
+		makePeer(2, withName("node-2"), withAddresses(netip.MustParsePrefix("100.64.1.2/32")), withAllowedIPs(netip.MustParsePrefix("100.64.1.2/32"))),
 	)
 	node3 := buildNetmapWithPeers(
 		makePeer(3, withName("node-3"), withAddresses(netip.MustParsePrefix("100.64.1.3/32"))),
@@ -1257,6 +1257,7 @@ func TestEngineReconfigOnStateChange(t *testing.T) {
 		steps         func(*testing.T, *LocalBackend, func() *mockControl)
 		wantState     ipn.State
 		wantCfg       *wgcfg.Config
+		wantPeers     []key.NodePublic
 		wantRouterCfg *router.Config
 		wantDNSCfg    *dns.Config
 	}{
@@ -1301,7 +1302,6 @@ func TestEngineReconfigOnStateChange(t *testing.T) {
 			// After the auth is completed, the configs must be updated to reflect the node's netmap.
 			wantState: ipn.Starting,
 			wantCfg: &wgcfg.Config{
-				Peers:     []wgcfg.Peer{},
 				Addresses: node1.SelfNode.Addresses().AsSlice(),
 			},
 			wantRouterCfg: &router.Config{
@@ -1359,7 +1359,6 @@ func TestEngineReconfigOnStateChange(t *testing.T) {
 			// Once the auth is completed, the configs must be updated to reflect the node's netmap.
 			wantState: ipn.Starting,
 			wantCfg: &wgcfg.Config{
-				Peers:     []wgcfg.Peer{},
 				Addresses: node2.SelfNode.Addresses().AsSlice(),
 			},
 			wantRouterCfg: &router.Config{
@@ -1409,7 +1408,6 @@ func TestEngineReconfigOnStateChange(t *testing.T) {
 			// must be updated to reflect the node's netmap.
 			wantState: ipn.Starting,
 			wantCfg: &wgcfg.Config{
-				Peers:     []wgcfg.Peer{},
 				Addresses: node1.SelfNode.Addresses().AsSlice(),
 			},
 			wantRouterCfg: &router.Config{
@@ -1434,23 +1432,17 @@ func TestEngineReconfigOnStateChange(t *testing.T) {
 			},
 			wantState: ipn.Starting,
 			wantCfg: &wgcfg.Config{
-				Peers: []wgcfg.Peer{
-					{
-						PublicKey: node1.SelfNode.Key(),
-						DiscoKey:  node1.SelfNode.DiscoKey(),
-					},
-					{
-						PublicKey: node2.SelfNode.Key(),
-						DiscoKey:  node2.SelfNode.DiscoKey(),
-					},
-				},
 				Addresses: node3.SelfNode.Addresses().AsSlice(),
 			},
+			wantPeers: []key.NodePublic{
+				node1.SelfNode.Key(),
+				node2.SelfNode.Key(),
+			},
 			wantRouterCfg: &router.Config{
 				SNATSubnetRoutes: true,
 				NetfilterMode:    preftype.NetfilterOn,
 				LocalAddrs:       node3.SelfNode.Addresses().AsSlice(),
-				Routes:           routesWithQuad100(),
+				Routes:           routesWithQuad100(netip.MustParsePrefix("100.64.1.1/32"), netip.MustParsePrefix("100.64.1.2/32")),
 			},
 			wantDNSCfg: &dns.Config{
 				AcceptDNS:             true,
@@ -1490,7 +1482,6 @@ func TestEngineReconfigOnStateChange(t *testing.T) {
 			// Starting a reauth should leave everything up:
 			wantState: ipn.Starting,
 			wantCfg: &wgcfg.Config{
-				Peers:     []wgcfg.Peer{},
 				Addresses: node1.SelfNode.Addresses().AsSlice(),
 			},
 			wantRouterCfg: &router.Config{
@@ -1522,7 +1513,6 @@ func TestEngineReconfigOnStateChange(t *testing.T) {
 			},
 			wantState: ipn.Starting,
 			wantCfg: &wgcfg.Config{
-				Peers:     []wgcfg.Peer{},
 				Addresses: node1.SelfNode.Addresses().AsSlice(),
 			},
 			wantRouterCfg: &router.Config{
@@ -1571,13 +1561,12 @@ func TestEngineReconfigOnStateChange(t *testing.T) {
 				t.Errorf("State: got %v; want %v", gotState, tt.wantState)
 			}
 
-			if engine.Config() != nil {
-				for _, p := range engine.Config().Peers {
-					pKey := p.PublicKey.UntypedHexString()
-					_, err := lb.MagicConn().ParseEndpoint(pKey)
-					if err != nil {
-						t.Errorf("ParseEndpoint(%q) failed: %v", pKey, err)
-					}
+			// Peers are not part of wgcfg.Config; the engine learns
+			// them from the config source installed by LocalBackend
+			// via SetPeerConfigFunc.
+			for _, k := range tt.wantPeers {
+				if _, ok := engine.PeerAllowedIPs(k); !ok {
+					t.Errorf("PeerAllowedIPs(%v) = false; want peer known", k.ShortString())
 				}
 			}
 
@@ -1597,7 +1586,10 @@ func TestEngineReconfigOnStateChange(t *testing.T) {
 	}
 }
 
-func TestEngineReconfigOnPeerRouteDelta(t *testing.T) {
+// TestPeerConfigUpdatedOnPeerRouteDelta tests that a netmap delta that
+// changes a peer's allowed IPs is visible through the live per-peer
+// config source that LocalBackend installs on the engine.
+func TestPeerConfigUpdatedOnPeerRouteDelta(t *testing.T) {
 	connect := &ipn.MaskedPrefs{Prefs: ipn.Prefs{WantRunning: true}, WantRunningSet: true}
 	peerAddr := netip.MustParsePrefix("100.64.1.1/32")
 	vipAddr := netip.MustParsePrefix("100.99.99.99/32")
@@ -1623,20 +1615,13 @@ func TestEngineReconfigOnPeerRouteDelta(t *testing.T) {
 		t.Fatal("UpdateNetmapDelta = false, want true")
 	}
 
-	cfg := engine.Config()
-	if cfg == nil {
-		t.Fatal("engine config is nil")
+	ips, ok := engine.PeerAllowedIPs(replacement.Key)
+	if !ok {
+		t.Fatalf("peer config source missing peer %v", replacement.Key.ShortString())
 	}
-	for _, peer := range cfg.Peers {
-		if peer.PublicKey != replacement.Key {
-			continue
-		}
-		if !slices.Contains(peer.AllowedIPs, vipAddr) {
-			t.Fatalf("peer AllowedIPs = %v; want %v", peer.AllowedIPs, vipAddr)
-		}
-		return
+	if !slices.Contains(ips, vipAddr) {
+		t.Fatalf("peer AllowedIPs = %v; want %v", ips, vipAddr)
 	}
-	t.Fatalf("engine config missing peer %v", replacement.Key.ShortString())
 }
 
 // TestSendPreservesAuthURL tests that wgengine updates arriving in the middle of
@@ -1900,6 +1885,8 @@ type mockEngine struct {
 
 	filter, jailedFilter *filter.Filter
 
+	peerConfigFn func(key.NodePublic) (allowedIPs []netip.Prefix, ok bool)
+
 	statusCb wgengine.StatusCallback
 }
 
@@ -2005,8 +1992,24 @@ func (e *mockEngine) InstallCaptureHook(packet.CaptureCallback) {}
 
 func (e *mockEngine) SetPeerByIPPacketFunc(func(netip.Addr) (_ key.NodePublic, ok bool)) {}
 func (e *mockEngine) SetPeerForIPFunc(func(netip.Addr) (_ wgengine.PeerForIP, ok bool))  {}
-func (e *mockEngine) SetPeerConfigFunc(func(key.NodePublic) (allowedIPs []netip.Prefix, ok bool)) {
+func (e *mockEngine) SetPeerConfigFunc(fn func(key.NodePublic) (allowedIPs []netip.Prefix, ok bool)) {
+	e.mu.Lock()
+	defer e.mu.Unlock()
+	e.peerConfigFn = fn
 }
+
+// PeerAllowedIPs looks up a peer's allowed IPs via the live per-peer
+// config source installed by LocalBackend with SetPeerConfigFunc.
+func (e *mockEngine) PeerAllowedIPs(k key.NodePublic) (_ []netip.Prefix, ok bool) {
+	e.mu.Lock()
+	fn := e.peerConfigFn
+	e.mu.Unlock()
+	if fn == nil {
+		return nil, false
+	}
+	return fn(k)
+}
+
 func (e *mockEngine) SyncDevicePeer(key.NodePublic)  {}
 func (e *mockEngine) ResetDevicePeer(key.NodePublic) {}
 func (e *mockEngine) SetPeerSessionStateFunc(func(key.NodePublic, wgengine.PeerWireGuardState)) {