diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 4b60ed2f7..3e84e9268 100644 --- a/ipn/ipnlocal/local.go +++ b/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, "") } - 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 diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 7644e178f..1751792e1 100644 --- a/ipn/ipnlocal/local_test.go +++ b/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() diff --git a/ipn/ipnlocal/node_backend.go b/ipn/ipnlocal/node_backend.go index 811d0802f..2098ebadd 100644 --- a/ipn/ipnlocal/node_backend.go +++ b/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 diff --git a/ipn/ipnlocal/node_backend_test.go b/ipn/ipnlocal/node_backend_test.go index b8eb98479..adad0c32b 100644 --- a/ipn/ipnlocal/node_backend_test.go +++ b/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) } } diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go index d8b045c00..3ce554fac 100644 --- a/ipn/ipnlocal/state_test.go +++ b/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)) { diff --git a/tsnet/tsnet_test.go b/tsnet/tsnet_test.go index 906008b06..ca3ed035e 100644 --- a/tsnet/tsnet_test.go +++ b/tsnet/tsnet_test.go @@ -1746,7 +1746,7 @@ func TestFallbackTCPHandler(t *testing.T) { // Before aa5da2e5f22a, every peer change rebuilt a full netmap on // the engine, so [wgengine.Engine.SetNetworkMap] kept the engine's // cached netmap fresh and [wgengine.Engine.Reconfig] kept wgdev and -// e.lastCfgFull fresh. After that refactor, peer adds and removes +// e.lastCfg fresh. After that refactor, peer adds and removes // ride the delta path and only mutate [nodeBackend]; the engine's // cached netmap and wireguard config stayed stale, and // [wgengine.Engine.PeerForIP] / [wgengine.Engine.Ping] / wgdev's @@ -1925,7 +1925,7 @@ func TestPingSubnetRouteOfDeltaPeer(t *testing.T) { // outbound wgdev encryption, magicsock transport. The receiving // side (s2's tstun.Wrapper) intercepts TSMP regardless of dst // IP and replies with a pong. So this catches both halves of - // the bug: a stale BART / lastCfgFull (PeerForIP / lookupPeerByIP + // the bug: a stale BART / lastCfg (PeerForIP / lookupPeerByIP // miss) AND a stale wgdev PeerLookupFunc closure (peer's noise // key not yet registered for outbound encryption). pingCtx, cancelPing := pingTimeout(ctx) diff --git a/util/deephash/tailscale_types_test.go b/util/deephash/tailscale_types_test.go index 7e803c841..7cff5335c 100644 --- a/util/deephash/tailscale_types_test.go +++ b/util/deephash/tailscale_types_test.go @@ -86,11 +86,6 @@ func getVal() *tailscaleTypes { return &tailscaleTypes{ &wgcfg.Config{ Addresses: []netip.Prefix{netip.PrefixFrom(netip.AddrFrom16([16]byte{3: 3}).Unmap(), 5)}, - Peers: []wgcfg.Peer{ - { - PublicKey: key.NodePublic{}, - }, - }, }, &router.Config{ Routes: []netip.Prefix{ diff --git a/wgengine/bench/wg.go b/wgengine/bench/wg.go index 1e28707e2..0e96a7b61 100644 --- a/wgengine/bench/wg.go +++ b/wgengine/bench/wg.go @@ -83,8 +83,8 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netip. e2.SetFilter(filter.NewAllowAllForTest(l2)) // There is no LocalBackend in this benchmark, so install trivial - // outbound peer lookups; without one, outbound packets can't - // lazily create their WireGuard peer. + // outbound peer lookups and per-peer config sources; without them, + // outbound packets can't lazily create their WireGuard peer. k1pub, k2pub := k1.Public(), k2.Public() e1.SetPeerByIPPacketFunc(func(dst netip.Addr) (_ key.NodePublic, ok bool) { return k2pub, a2.Contains(dst) @@ -92,6 +92,18 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netip. e2.SetPeerByIPPacketFunc(func(dst netip.Addr) (_ key.NodePublic, ok bool) { return k1pub, a1.Contains(dst) }) + e1.SetPeerConfigFunc(func(pubk key.NodePublic) (_ []netip.Prefix, ok bool) { + if pubk == k2pub { + return []netip.Prefix{a2}, true + } + return nil, false + }) + e2.SetPeerConfigFunc(func(pubk key.NodePublic) (_ []netip.Prefix, ok bool) { + if pubk == k1pub { + return []netip.Prefix{a1}, true + } + return nil, false + }) var wait sync.WaitGroup wait.Add(2) @@ -107,12 +119,6 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netip. logf("e1 status: %v", *st) e2.SetSelfNode(tailcfg.NodeView{}) - - p := wgcfg.Peer{ - PublicKey: c1.PrivateKey.Public(), - AllowedIPs: []netip.Prefix{a1}, - } - c2.Peers = []wgcfg.Peer{p} e2.Reconfig(&c2, &router.Config{}, new(dns.Config)) e1waitDoneOnce.Do(wait.Done) }) @@ -128,12 +134,6 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netip. logf("e2 status: %v", *st) e1.SetSelfNode(tailcfg.NodeView{}) - - p := wgcfg.Peer{ - PublicKey: c2.PrivateKey.Public(), - AllowedIPs: []netip.Prefix{a2}, - } - c1.Peers = []wgcfg.Peer{p} e1.Reconfig(&c1, &router.Config{}, new(dns.Config)) e2waitDoneOnce.Do(wait.Done) }) diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index 4286b6028..63876bc51 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/wgengine/magicsock/magicsock_test.go @@ -246,50 +246,71 @@ func newMagicStackWithKey(t testing.TB, logf logger.Logf, ln nettype.PacketListe } } -// Reconfig applies cfg to the stack's device and tun layer. peers, -// if non-nil, are the tailcfg nodes that cfg was derived from; the -// tun layer's per-peer data-plane attributes (masquerade addresses, -// jailed classification) are derived from them with a real -// [routemanager.RouteManager], exactly as LocalBackend does in -// production, so this helper cannot drift from the production -// derivation. Tests whose hand-built configs carry no such -// attributes may pass nil. +// Reconfig applies cfg and peers to the stack's WireGuard device and +// tun-layer data plane. In production these flow from LocalBackend +// (via [tailscale.com/wgengine.Engine.Reconfig] and the live per-peer +// config source installed with Engine.SetPeerConfigFunc); tests that +// bypass LocalBackend replicate that wiring here, deriving everything +// from a real [routemanager.RouteManager] fed the given peers, +// exactly as LocalBackend does in production, so this helper cannot +// drift from the production derivation. func (s *magicStack) Reconfig(cfg *wgcfg.Config, peers []tailcfg.NodeView) error { s.tsTun.SetWGConfig(cfg) - if peers != nil { - rm := routemanager.New(nil) - mut := rm.Begin() - // Mirror the netmap.AllowSubnetRoutes flag the tests pass to - // nmcfg.WGCfg. - mut.SetPrefs(routemanager.Prefs{RouteAll: true}) - for _, n := range peers { - mut.UpsertPeer(n) + rm := routemanager.New(nil) + mut := rm.Begin() + // Mirror the netmap.AllowSubnetRoutes flag the tests pass to + // nmcfg.WGCfg. + mut.SetPrefs(routemanager.Prefs{RouteAll: true}) + // idByKey stands in for nodeBackend's public-key-to-node-ID + // index, which LocalBackend uses to serve the engine's per-peer + // config source. + idByKey := make(map[key.NodePublic]tailcfg.NodeID, len(peers)) + for _, n := range peers { + idByKey[n.Key()] = n.ID() + mut.UpsertPeer(n) + } + mut.Commit() + peerAllowedIPs := func(k key.NodePublic) ([]netip.Prefix, bool) { + id, ok := idByKey[k] + if !ok { + return nil, false } - mut.Commit() - native4, native6 := tsaddr.FirstTailscaleAddrs(slices.All(cfg.Addresses)) - s.tsTun.SetPeerRoutes(native4, native6, rm.Outbound()) + return rm.PeerAllowedIPs(id) } - // In production, LocalBackend installs a PeerByIPPacketFunc via - // Engine.SetPeerByIPPacketFunc. Tests that bypass LocalBackend need - // to install one here for outbound packet routing. - ipToPeer := make(map[netip.Addr]device.NoisePublicKey, len(cfg.Peers)) - for _, p := range cfg.Peers { - pk := p.PublicKey.Raw32() - for _, pfx := range p.AllowedIPs { - if pfx.IsSingleIP() { - ipToPeer[pfx.Addr()] = pk + // The tun-layer per-peer route attributes (masquerade, jailed). + native4, native6 := tsaddr.FirstTailscaleAddrs(slices.All(cfg.Addresses)) + s.tsTun.SetPeerRoutes(native4, native6, rm.Outbound()) + + // Outbound packet routing, as LocalBackend's lookupPeerByIP does + // via the outbound table. + s.dev.SetPeerByIPPacketFunc(func(_, dst netip.Addr, _ []byte) (device.NoisePublicKey, bool) { + if pr, ok := rm.Outbound().Lookup(dst); ok { + return pr.Key.Raw32(), true + } + return device.NoisePublicKey{}, false + }) + + // The live per-peer config source backing lazy peer creation, as + // LocalBackend's peerAllowedIPs does via PeerAllowedIPs, and the + // per-peer device convergence that Engine.SyncDevicePeer does. + s.dev.SetPeerLookupFunc(wgcfg.NewPeerLookupFunc(s.conn.Bind(), s.conn.logf, func(pubk device.NoisePublicKey) ([]netip.Prefix, bool) { + return peerAllowedIPs(key.NodePublicFromRaw32(mem.B(pubk[:]))) + })) + s.dev.SetPrivateKey(key.NodePrivateAs[device.NoisePrivateKey](cfg.PrivateKey)) + s.dev.RemoveMatchingPeers(func(pk device.NoisePublicKey) bool { + _, ok := peerAllowedIPs(key.NodePublicFromRaw32(mem.B(pk[:]))) + return !ok + }) + for _, n := range peers { + if peer, ok := s.dev.LookupActivePeer(n.Key().Raw32()); ok { + if ips, ok := peerAllowedIPs(n.Key()); ok { + peer.SetAllowedIPs(ips) } } } - s.dev.SetPeerByIPPacketFunc(func(_, dst netip.Addr, _ []byte) (device.NoisePublicKey, bool) { - pk, ok := ipToPeer[dst] - return pk, ok - }) - - s.dev.SetPrivateKey(key.NodePrivateAs[device.NoisePrivateKey](cfg.PrivateKey)) - return wgcfg.ReconfigDevice(s.dev, cfg, s.conn.logf) + return nil } func (s *magicStack) String() string { @@ -1203,30 +1224,32 @@ func testTwoDevicePing(t *testing.T, d *devices) { m1cfg := &wgcfg.Config{ PrivateKey: m1.privateKey, Addresses: []netip.Prefix{netip.MustParsePrefix("1.0.0.1/32")}, - Peers: []wgcfg.Peer{ - { - PublicKey: m2.privateKey.Public(), - DiscoKey: m2.conn.DiscoPublicKey(), - AllowedIPs: []netip.Prefix{netip.MustParsePrefix("1.0.0.2/32")}, - }, - }, } + m1peers := nodeViews([]*tailcfg.Node{{ + ID: 2, + Key: m2.privateKey.Public(), + DiscoKey: m2.conn.DiscoPublicKey(), + HomeDERP: 1, + Addresses: []netip.Prefix{netip.MustParsePrefix("1.0.0.2/32")}, + AllowedIPs: []netip.Prefix{netip.MustParsePrefix("1.0.0.2/32")}, + }}) m2cfg := &wgcfg.Config{ PrivateKey: m2.privateKey, Addresses: []netip.Prefix{netip.MustParsePrefix("1.0.0.2/32")}, - Peers: []wgcfg.Peer{ - { - PublicKey: m1.privateKey.Public(), - DiscoKey: m1.conn.DiscoPublicKey(), - AllowedIPs: []netip.Prefix{netip.MustParsePrefix("1.0.0.1/32")}, - }, - }, } + m2peers := nodeViews([]*tailcfg.Node{{ + ID: 1, + Key: m1.privateKey.Public(), + DiscoKey: m1.conn.DiscoPublicKey(), + HomeDERP: 1, + Addresses: []netip.Prefix{netip.MustParsePrefix("1.0.0.1/32")}, + AllowedIPs: []netip.Prefix{netip.MustParsePrefix("1.0.0.1/32")}, + }}) - if err := m1.Reconfig(m1cfg, nil); err != nil { + if err := m1.Reconfig(m1cfg, m1peers); err != nil { t.Fatal(err) } - if err := m2.Reconfig(m2cfg, nil); err != nil { + if err := m2.Reconfig(m2cfg, m2peers); err != nil { t.Fatal(err) } @@ -1350,7 +1373,7 @@ func testTwoDevicePing(t *testing.T, d *devices) { t.Run("no-op-dev1-reconfig", func(t *testing.T) { setT(t) defer setT(outerT) - if err := m1.Reconfig(m1cfg, nil); err != nil { + if err := m1.Reconfig(m1cfg, m1peers); err != nil { t.Fatal(err) } ping1(t) diff --git a/wgengine/userspace.go b/wgengine/userspace.go index f36d16620..e2b1b85c0 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -94,8 +94,6 @@ type userspaceEngine struct { // feature/bird package is not linked into the binary. bird Bird - testMaybeReconfigHook func() // for tests; if non-nil, fires if maybeReconfigWireguardLocked called - // isLocalAddr reports the whether an IP is assigned to the local // tunnel interface. It's used to reflect local packets // incorrectly sent to us. @@ -118,7 +116,7 @@ type userspaceEngine struct { // no longer install per-config lookup closures. peerConfigFn atomic.Pointer[func(key.NodePublic) (allowedIPs []netip.Prefix, ok bool)] - lastCfgFull wgcfg.Config + lastCfg wgcfg.Config lastRouter *router.Config lastDNSConfig dns.ConfigView // or invalid if none reconfigureVPN func() error // or nil @@ -678,59 +676,6 @@ func (e *userspaceEngine) handleLocalPackets(p *packet.Parsed, t *tstun.Wrapper) return filter.Accept } -// maybeReconfigWireguardLocked reconfigures wireguard-go with the current -// full config, installing a PeerLookupFunc for on-demand peer creation. -// -// e.wgLock must be held. -func (e *userspaceEngine) maybeReconfigWireguardLocked() error { - if hook := e.testMaybeReconfigHook; hook != nil { - hook() - return nil - } - - full := e.lastCfgFull - // The wireguard-go peer set may have changed; drop the cached - // peer-string rewrites so the next log line re-resolves them - // against the current lookup. - e.wgLogger.Invalidate() - - e.logf("wgengine: Reconfig: configuring userspace WireGuard config (with %d peers)", len(full.Peers)) - if e.peerConfigFn.Load() != nil { - // The device has a long-lived PeerLookupFunc backed by the - // live config source, so only the peer set needs syncing; - // there is no per-config lookup closure to (re)install, and - // no removed peer can be resurrected with stale state. - // - // TODO(bradfitz): remove this O(n peers) sync. It's redundant - // with the incremental SyncDevicePeer calls that LocalBackend - // makes for exactly the peers whose allowed IPs changed. It - // only remains because peer changes still force a full - // Reconfig; once that's gated on actual router/DNS changes, - // this sync (and full-config peer syncing generally) can go. - peers := make(map[device.NoisePublicKey][]netip.Prefix, len(full.Peers)) - for _, p := range full.Peers { - peers[p.PublicKey.Raw32()] = p.AllowedIPs - } - e.wgdev.RemoveMatchingPeers(func(pk device.NoisePublicKey) bool { - _, exists := peers[pk] - return !exists - }) - // Update AllowedIPs on any already-active peers whose config - // may have changed. Peers that don't exist yet will get the - // correct AllowedIPs from the device's PeerLookupFunc when - // they are lazily created. - for pk, allowedIPs := range peers { - if peer, ok := e.wgdev.LookupActivePeer(pk); ok { - peer.SetAllowedIPs(allowedIPs) - } - } - } else if err := wgcfg.ReconfigDevice(e.wgdev, &full, e.logf); err != nil { - e.logf("wgdev.Reconfig: %v", err) - return err - } - return nil -} - // SetPeerConfigFunc implements [Engine.SetPeerConfigFunc]. It stores // fn and installs a single wgdev PeerLookupFunc wrapping it, so // lazily-created peers always get current allowed IPs and the lookup @@ -753,6 +698,9 @@ func (e *userspaceEngine) SyncDevicePeer(k key.NodePublic) { } e.wgLock.Lock() defer e.wgLock.Unlock() + // The peer set may be about to change; drop the wgLogger's cached + // peer-string rewrites so the next log line re-resolves them. + e.wgLogger.Invalidate() allowedIPs, ok := (*fn)(k) if !ok { e.wgdev.RemovePeer(k.Raw32()) @@ -767,6 +715,7 @@ func (e *userspaceEngine) SyncDevicePeer(k key.NodePublic) { func (e *userspaceEngine) ResetDevicePeer(k key.NodePublic) { e.wgLock.Lock() defer e.wgLock.Unlock() + e.wgLogger.Invalidate() e.wgdev.RemovePeer(k.Raw32()) } @@ -878,7 +827,7 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, birdChanged = e.bird.Reconfig(self) } - engineChanged := !e.lastCfgFull.Equal(cfg) + engineChanged := !e.lastCfg.Equal(cfg) routerChanged := checkchange.Update(&e.lastRouter, routerCfg) dnsChanged := buildfeatures.HasDNS && !e.lastDNSConfig.Equal(dnsCfg.View()) if dnsChanged { @@ -910,7 +859,7 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, e.isDNSIPOverTailscale.Store(ipset.NewContainsIPFunc(views.SliceOf(dnsIPsOverTailscale(dnsCfg, routerCfg)))) } - if !e.lastCfgFull.PrivateKey.Equal(cfg.PrivateKey) { + if !e.lastCfg.PrivateKey.Equal(cfg.PrivateKey) { // Tell magicsock about the new (or initial) private key // (which is needed by DERP) before wgdev gets it, as wgdev // will start trying to handshake, which we want to be able to @@ -924,16 +873,16 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, } } - e.lastCfgFull = *cfg.Clone() + e.lastCfg = *cfg.Clone() e.magicConn.SetPreferredPort(listenPort) e.magicConn.UpdatePMTUD() - if engineChanged { - if err := e.maybeReconfigWireguardLocked(); err != nil { - return err - } - } + // Note: no wireguard-go device reconfig happens here. The device + // learns its peer set from the live config source installed via + // [Engine.SetPeerConfigFunc] (peers are lazily created and synced + // per peer by [Engine.SyncDevicePeer]), and its private key is set + // above when it changes. if routerChanged { e.logf("wgengine: Reconfig: configuring router") diff --git a/wgengine/userspace_test.go b/wgengine/userspace_test.go index 890a9cb0b..fc1c89403 100644 --- a/wgengine/userspace_test.go +++ b/wgengine/userspace_test.go @@ -90,7 +90,7 @@ func TestUserspaceEngineReconfig(t *testing.T) { routerCfg := &router.Config{} - for _, nodeHex := range []string{ + for i, nodeHex := range []string{ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", } { @@ -102,18 +102,9 @@ func TestUserspaceEngineReconfig(t *testing.T) { }, }), } - nk, err := key.ParseNodePublicUntyped(mem.S(nodeHex)) - if err != nil { - t.Fatal(err) - } cfg := &wgcfg.Config{ - Peers: []wgcfg.Peer{ - { - PublicKey: nk, - AllowedIPs: []netip.Prefix{ - netip.PrefixFrom(netaddr.IPv4(100, 100, 99, 1), 32), - }, - }, + Addresses: []netip.Prefix{ + netip.PrefixFrom(netaddr.IPv4(100, 100, 99, byte(1+i)), 32), }, } @@ -156,18 +147,9 @@ func TestUserspaceEnginePortReconfig(t *testing.T) { t.Cleanup(ue.Close) startingPort := ue.magicConn.LocalPort() - nodeKey, err := key.ParseNodePublicUntyped(mem.S("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")) - if err != nil { - t.Fatal(err) - } cfg := &wgcfg.Config{ - Peers: []wgcfg.Peer{ - { - PublicKey: nodeKey, - AllowedIPs: []netip.Prefix{ - netip.PrefixFrom(netaddr.IPv4(100, 100, 99, 1), 32), - }, - }, + Addresses: []netip.Prefix{ + netip.PrefixFrom(netaddr.IPv4(100, 100, 99, 1), 32), }, } routerCfg := &router.Config{} @@ -238,18 +220,9 @@ func TestUserspaceEnginePeerMTUReconfig(t *testing.T) { t.Logf("Info: OS default don't fragment bit(s) setting: %v", osDefaultDF) // Build a set of configs to use as we change the peer MTU settings. - nodeKey, err := key.ParseNodePublicUntyped(mem.S("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")) - if err != nil { - t.Fatal(err) - } cfg := &wgcfg.Config{ - Peers: []wgcfg.Peer{ - { - PublicKey: nodeKey, - AllowedIPs: []netip.Prefix{ - netip.PrefixFrom(netaddr.IPv4(100, 100, 99, 1), 32), - }, - }, + Addresses: []netip.Prefix{ + netip.PrefixFrom(netaddr.IPv4(100, 100, 99, 1), 32), }, } routerCfg := &router.Config{} @@ -315,14 +288,7 @@ func TestTSMPKeyAdvertisement(t *testing.T) { }).View(), } cfg := &wgcfg.Config{ - Peers: []wgcfg.Peer{ - { - PublicKey: nodeKey, - AllowedIPs: []netip.Prefix{ - netip.PrefixFrom(netaddr.IPv4(100, 100, 99, 1), 32), - }, - }, - }, + Addresses: nm.SelfNode.Addresses().AsSlice(), } ue.SetSelfNode(nm.SelfNode) diff --git a/wgengine/wgcfg/config.go b/wgengine/wgcfg/config.go index bd37c19f3..d613f0fc5 100644 --- a/wgengine/wgcfg/config.go +++ b/wgengine/wgcfg/config.go @@ -11,14 +11,17 @@ "tailscale.com/types/key" ) -//go:generate go run tailscale.com/cmd/cloner -type=Config,Peer +//go:generate go run tailscale.com/cmd/cloner -type=Config // Config is a WireGuard configuration. // It only supports the set of things Tailscale uses. +// +// Peers are not part of the config: wireguard-go learns the peer set +// and each peer's allowed IPs from the live per-peer config source +// installed via [tailscale.com/wgengine.Engine.SetPeerConfigFunc]. type Config struct { PrivateKey key.NodePrivate Addresses []netip.Prefix - Peers []Peer } func (c *Config) Equal(o *Config) bool { @@ -26,43 +29,5 @@ func (c *Config) Equal(o *Config) bool { return c == o } return c.PrivateKey.Equal(o.PrivateKey) && - slices.Equal(c.Addresses, o.Addresses) && - slices.EqualFunc(c.Peers, o.Peers, Peer.Equal) -} - -type Peer struct { - PublicKey key.NodePublic - DiscoKey key.DiscoPublic // present only so we can handle restarts within wgengine, not passed to WireGuard - AllowedIPs []netip.Prefix - V4MasqAddr *netip.Addr // if non-nil, masquerade IPv4 traffic to this peer using this address - V6MasqAddr *netip.Addr // if non-nil, masquerade IPv6 traffic to this peer using this address - IsJailed bool // if true, this peer is jailed and cannot initiate connections - PersistentKeepalive uint16 // in seconds between keep-alives; 0 to disable -} - -func addrPtrEq(a, b *netip.Addr) bool { - if a == nil || b == nil { - return a == b - } - return *a == *b -} - -func (p Peer) Equal(o Peer) bool { - return p.PublicKey == o.PublicKey && - p.DiscoKey == o.DiscoKey && - slices.Equal(p.AllowedIPs, o.AllowedIPs) && - p.IsJailed == o.IsJailed && - p.PersistentKeepalive == o.PersistentKeepalive && - addrPtrEq(p.V4MasqAddr, o.V4MasqAddr) && - addrPtrEq(p.V6MasqAddr, o.V6MasqAddr) -} - -// PeerWithKey returns the Peer with key k and reports whether it was found. -func (config Config) PeerWithKey(k key.NodePublic) (Peer, bool) { - for _, p := range config.Peers { - if p.PublicKey == k { - return p, true - } - } - return Peer{}, false + slices.Equal(c.Addresses, o.Addresses) } diff --git a/wgengine/wgcfg/config_test.go b/wgengine/wgcfg/config_test.go index fc8fdcbbb..64ffc8041 100644 --- a/wgengine/wgcfg/config_test.go +++ b/wgengine/wgcfg/config_test.go @@ -14,25 +14,10 @@ func TestConfigEqual(t *testing.T) { rt := reflect.TypeFor[Config]() for sf := range rt.Fields() { switch sf.Name { - case "Name", "NodeID", "PrivateKey", "Addresses", "Peers": + case "Name", "NodeID", "PrivateKey", "Addresses": // These are compared in [Config.Equal]. default: t.Errorf("Have you added field %q to Config.Equal? Do so if not, and then update TestConfigEqual", sf.Name) } } } - -// Tests that [Peer.Equal] tests all fields of [Peer], even ones -// that might get added in the future. -func TestPeerEqual(t *testing.T) { - rt := reflect.TypeFor[Peer]() - for sf := range rt.Fields() { - switch sf.Name { - case "PublicKey", "DiscoKey", "AllowedIPs", "IsJailed", - "PersistentKeepalive", "V4MasqAddr", "V6MasqAddr": - // These are compared in [Peer.Equal]. - default: - t.Errorf("Have you added field %q to Peer.Equal? Do so if not, and then update TestPeerEqual", sf.Name) - } - } -} diff --git a/wgengine/wgcfg/device.go b/wgengine/wgcfg/device.go index 554cc9c1e..c5d39ea3d 100644 --- a/wgengine/wgcfg/device.go +++ b/wgengine/wgcfg/device.go @@ -38,69 +38,3 @@ func NewPeerLookupFunc(bind conn.Bind, logf logger.Logf, allowedIPs func(device. }, true } } - -// ReconfigDevice replaces the existing device configuration with cfg. -// -// Instead of using the UAPI text protocol, it uses the wireguard-go direct API -// to install a [device.PeerLookupFunc] callback that creates peers on demand. -// -// The caller is responsible for: -// - calling [device.Device.SetPrivateKey] when the key changes -// - installing a [device.PeerByIPPacketFunc] on the device for outbound -// packet routing (e.g. via [tailscale.com/wgengine.Engine.SetPeerByIPPacketFunc]) -func ReconfigDevice(d *device.Device, cfg *Config, logf logger.Logf) (err error) { - defer func() { - if err != nil { - logf("wgcfg.Reconfig failed: %v", err) - } - }() - - // Build peer map: public key → allowed IPs. - peers := make(map[device.NoisePublicKey][]netip.Prefix, len(cfg.Peers)) - for _, p := range cfg.Peers { - peers[p.PublicKey.Raw32()] = p.AllowedIPs - } - - // Remove peers not in the new config. - d.RemoveMatchingPeers(func(pk device.NoisePublicKey) bool { - _, exists := peers[pk] - return !exists - }) - - // Update AllowedIPs on any already-active peers whose config may have - // changed. Peers that don't exist yet will get the correct AllowedIPs - // from PeerLookupFunc when they are lazily created. - for pk, allowedIPs := range peers { - if peer, ok := d.LookupActivePeer(pk); ok { - peer.SetAllowedIPs(allowedIPs) - } - } - - // Install callback for lazy peer creation (incoming packets). - bind := d.Bind() - d.SetPeerLookupFunc(func(pubk device.NoisePublicKey) (_ *device.NewPeerConfig, ok bool) { - allowedIPs, ok := peers[pubk] - if !ok { - return nil, false - } - ep, err := bind.ParseEndpoint(fmt.Sprintf("%x", pubk[:])) - if err != nil { - logf("wgcfg: failed to parse endpoint for peer %x: %v", pubk[:8], err) - return nil, false - } - return &device.NewPeerConfig{ - AllowedIPs: allowedIPs, - Endpoint: ep, - }, true - }) - - // RemoveMatchingPeers _again_, now that SetPeerLookupFunc is installed, - // lest any removed peers got re-created before the new SetPeerLookupFunc - // func was installed. - d.RemoveMatchingPeers(func(pk device.NoisePublicKey) bool { - _, exists := peers[pk] - return !exists - }) - - return nil -} diff --git a/wgengine/wgcfg/device_test.go b/wgengine/wgcfg/device_test.go index 07eb41adb..c07e14702 100644 --- a/wgengine/wgcfg/device_test.go +++ b/wgengine/wgcfg/device_test.go @@ -15,76 +15,47 @@ "tailscale.com/types/key" ) -func TestReconfigDevice(t *testing.T) { - k1, pk1 := newK() - ip1 := netip.MustParsePrefix("10.0.0.1/32") +func TestNewPeerLookupFunc(t *testing.T) { + k1, _ := newK() k2, _ := newK() ip2 := netip.MustParsePrefix("10.0.0.2/32") k3, _ := newK() - ip3 := netip.MustParsePrefix("10.0.0.3/32") - - cfg1 := &Config{ - PrivateKey: pk1, - Peers: []Peer{ - {PublicKey: k2, AllowedIPs: []netip.Prefix{ip2}}, - }, - } dev := NewDevice(newNilTun(), new(noopBind), device.NewLogger(device.LogLevelError, "test")) defer dev.Close() - t.Run("initial-config", func(t *testing.T) { - if err := ReconfigDevice(dev, cfg1, t.Logf); err != nil { - t.Fatal(err) - } - // Peer should be creatable on demand via LookupPeer. - peer := dev.LookupPeer(k2.Raw32()) - if peer == nil { + // peers is the live per-peer config source, standing in for what + // LocalBackend provides via wgengine.Engine.SetPeerConfigFunc. + peers := map[device.NoisePublicKey][]netip.Prefix{ + k2.Raw32(): {ip2}, + } + dev.SetPeerLookupFunc(NewPeerLookupFunc(dev.Bind(), t.Logf, func(pubk device.NoisePublicKey) ([]netip.Prefix, bool) { + ips, ok := peers[pubk] + return ips, ok + })) + + t.Run("lazy-creation", func(t *testing.T) { + // A peer known to the config source should be creatable on + // demand via LookupPeer. + if p := dev.LookupPeer(k2.Raw32()); p == nil { t.Fatal("expected peer k2 to exist via LookupPeer") } - // Unknown peer should not be found. - peer = dev.LookupPeer(k3.Raw32()) - if peer != nil { + // An unknown peer should not be found. + if p := dev.LookupPeer(k3.Raw32()); p != nil { t.Fatal("expected unknown peer k3 to not exist") } }) - t.Run("add-peer", func(t *testing.T) { - cfg1.Peers = append(cfg1.Peers, Peer{ - PublicKey: k3, - AllowedIPs: []netip.Prefix{ip3}, - }) - if err := ReconfigDevice(dev, cfg1, t.Logf); err != nil { - t.Fatal(err) - } - // Both peers should now be discoverable. - if p := dev.LookupPeer(k2.Raw32()); p == nil { - t.Fatal("expected peer k2 to exist") - } - if p := dev.LookupPeer(k3.Raw32()); p == nil { - t.Fatal("expected peer k3 to exist") - } - }) - t.Run("remove-peer", func(t *testing.T) { - cfg2 := &Config{ - PrivateKey: pk1, - Peers: []Peer{ - {PublicKey: k2, AllowedIPs: []netip.Prefix{ip2}}, - }, - } - if err := ReconfigDevice(dev, cfg2, t.Logf); err != nil { - t.Fatal(err) - } - // k2 should still be discoverable. - if p := dev.LookupPeer(k2.Raw32()); p == nil { - t.Fatal("expected peer k2 to exist") - } - // k3 should no longer be discoverable. - if p := dev.LookupPeer(k3.Raw32()); p != nil { - t.Fatal("expected peer k3 to not exist after removal") + delete(peers, k2.Raw32()) + dev.RemoveMatchingPeers(func(pk device.NoisePublicKey) bool { + _, ok := peers[pk] + return !ok + }) + if p := dev.LookupPeer(k2.Raw32()); p != nil { + t.Fatal("expected peer k2 to not exist after removal") } }) @@ -94,8 +65,6 @@ func TestReconfigDevice(t *testing.T) { t.Fatal("expected own key to not be a peer") } }) - - _ = ip1 // suppress unused } func newK() (key.NodePublic, key.NodePrivate) { diff --git a/wgengine/wgcfg/nmcfg/nmcfg.go b/wgengine/wgcfg/nmcfg/nmcfg.go index 7f633e4b5..da4d038db 100644 --- a/wgengine/wgcfg/nmcfg/nmcfg.go +++ b/wgengine/wgcfg/nmcfg/nmcfg.go @@ -45,11 +45,16 @@ func cidrIsSubnet(node tailcfg.NodeView, cidr netip.Prefix) bool { } // WGCfg returns the NetworkMaps's WireGuard configuration. +// +// The config does not include peers; wireguard-go gets those from the +// live per-peer config source installed via +// [tailscale.com/wgengine.Engine.SetPeerConfigFunc], fed by the route +// manager. WGCfg still walks the peers to log which ones are not +// routable and why, mirroring the route manager's filtering. func WGCfg(pk key.NodePrivate, nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags, exitNode tailcfg.StableNodeID) (*wgcfg.Config, error) { cfg := &wgcfg.Config{ PrivateKey: pk, Addresses: nm.GetAddresses().AsSlice(), - Peers: make([]wgcfg.Peer, 0, len(nm.Peers)), } var skippedExitNode, skippedSubnetRouter, skippedExpired []tailcfg.NodeView @@ -69,16 +74,7 @@ func WGCfg(pk key.NodePrivate, nm *netmap.NetworkMap, logf logger.Logf, flags ne continue } - cfg.Peers = append(cfg.Peers, wgcfg.Peer{ - PublicKey: peer.Key(), - DiscoKey: peer.DiscoKey(), - }) - cpeer := &cfg.Peers[len(cfg.Peers)-1] - didExitNodeLog := false - cpeer.V4MasqAddr = peer.SelfNodeV4MasqAddrForThisPeer().Clone() - cpeer.V6MasqAddr = peer.SelfNodeV6MasqAddrForThisPeer().Clone() - cpeer.IsJailed = peer.IsJailed() for _, allowedIP := range peer.AllowedIPs().All() { if allowedIP.Bits() == 0 && peer.StableID() != exitNode { if didExitNodeLog { @@ -87,14 +83,11 @@ func WGCfg(pk key.NodePrivate, nm *netmap.NetworkMap, logf logger.Logf, flags ne } didExitNodeLog = true skippedExitNode = append(skippedExitNode, peer) - continue } else if cidrIsSubnet(peer, allowedIP) { if (flags & netmap.AllowSubnetRoutes) == 0 { skippedSubnetRouter = append(skippedSubnetRouter, peer) - continue } } - cpeer.AllowedIPs = append(cpeer.AllowedIPs, allowedIP) } } diff --git a/wgengine/wgcfg/wgcfg_clone.go b/wgengine/wgcfg/wgcfg_clone.go index 32e7e31e5..7b56729c4 100644 --- a/wgengine/wgcfg/wgcfg_clone.go +++ b/wgengine/wgcfg/wgcfg_clone.go @@ -20,12 +20,6 @@ func (src *Config) Clone() *Config { dst := new(Config) *dst = *src dst.Addresses = append(src.Addresses[:0:0], src.Addresses...) - if src.Peers != nil { - dst.Peers = make([]Peer, len(src.Peers)) - for i := range dst.Peers { - dst.Peers[i] = *src.Peers[i].Clone() - } - } return dst } @@ -33,34 +27,4 @@ func (src *Config) Clone() *Config { var _ConfigCloneNeedsRegeneration = Config(struct { PrivateKey key.NodePrivate Addresses []netip.Prefix - Peers []Peer -}{}) - -// Clone makes a deep copy of Peer. -// The result aliases no memory with the original. -func (src *Peer) Clone() *Peer { - if src == nil { - return nil - } - dst := new(Peer) - *dst = *src - dst.AllowedIPs = append(src.AllowedIPs[:0:0], src.AllowedIPs...) - if dst.V4MasqAddr != nil { - dst.V4MasqAddr = new(*src.V4MasqAddr) - } - if dst.V6MasqAddr != nil { - dst.V6MasqAddr = new(*src.V6MasqAddr) - } - return dst -} - -// A compilation failure here means this code must be regenerated, with the command at the top of this file. -var _PeerCloneNeedsRegeneration = Peer(struct { - PublicKey key.NodePublic - DiscoKey key.DiscoPublic - AllowedIPs []netip.Prefix - V4MasqAddr *netip.Addr - V6MasqAddr *netip.Addr - IsJailed bool - PersistentKeepalive uint16 }{})