From 46669e364f69f6ac314d0e2838d76fee4c34cccd Mon Sep 17 00:00:00 2001 From: Fernando Serboncini Date: Fri, 12 Jun 2026 18:25:54 -0400 Subject: [PATCH] wgengine, ipn/ipnlocal: apply netmap peer deltas to the engine's netmap e.netMap is only written by SetNetworkMap, which doesn't run when a MapResponse is handled incrementally. Therefore, when a peer re auths as a delta and magicsock learns the new key, e.netMap still has the old one, so PeerForIP feeds magicsock a key that will not work and "tailscale ping" fails until the next full netmap. Add UpdateNetmapDelta to the Engine interface, dispatched from lb UpdateNetmapDelta. Only upserts and removals are applied. Fixes tailscale/corp#43394 Change-Id: Id87637d8003c7dbb61ada5297e05d58e8e2f53b3 Signed-off-by: Fernando Serboncini --- ipn/ipnlocal/local.go | 2 + ipn/ipnlocal/local_test.go | 38 ++++++++++++++++++ ipn/ipnlocal/state_test.go | 2 + wgengine/userspace.go | 46 ++++++++++++++++++++++ wgengine/userspace_test.go | 79 ++++++++++++++++++++++++++++++++++++++ wgengine/wgengine.go | 6 +++ 6 files changed, 173 insertions(+) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 7bd444d2b..3f3270168 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -2452,6 +2452,8 @@ func (b *LocalBackend) UpdateNetmapDelta(muts []netmap.NodeMutation) (handled bo } } ms.UpdateNetmapDelta(muts) + b.e.UpdateNetmapDelta(muts) + if needsAuthReconfig { b.authReconfigLocked() } diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 2c5a39dd3..64720f152 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -2621,6 +2621,44 @@ func TestUpdateNetmapDelta(t *testing.T) { } } +func TestEnginePeerForIPAfterDeltaUpsert(t *testing.T) { + b := newTestLocalBackend(t) + + peerAddr := netip.MustParsePrefix("100.64.0.2/32") + peer := makePeer(2, withAddresses(peerAddr)) + + nm := &netmap.NetworkMap{ + SelfNode: (&tailcfg.Node{ + ID: 1, + Addresses: []netip.Prefix{netip.MustParsePrefix("100.64.0.1/32")}, + }).View(), + Peers: []tailcfg.NodeView{peer}, + } + + b.e.SetNetworkMap(nm) + b.currentNode().SetNetMap(nm) + + reauthed := peer.AsStruct() + reauthed.Key = makeNodeKeyFromID(102) + muts, ok := netmap.MutationsFromMapResponse(&tailcfg.MapResponse{ + PeersChanged: []*tailcfg.Node{reauthed}, + }, time.Unix(123, 0)) + if !ok { + t.Fatal("netmap.MutationsFromMapResponse failed") + } + if !b.UpdateNetmapDelta(muts) { + t.Fatal("UpdateNetmapDelta = false, want true") + } + + pip, ok := b.e.PeerForIP(peerAddr.Addr()) + if !ok { + t.Fatalf("PeerForIP(%v) = !ok, want peer", peerAddr.Addr()) + } + if got, want := pip.Node.Key(), reauthed.Key; got != want { + t.Errorf("PeerForIP(%v) returned node key %v, want post-re-auth key %v", peerAddr.Addr(), got, want) + } +} + type whoIsTestParams struct { testName string q string diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go index 4eb8ac136..0d27bdef1 100644 --- a/ipn/ipnlocal/state_test.go +++ b/ipn/ipnlocal/state_test.go @@ -1973,6 +1973,8 @@ func (e *mockEngine) PeerByKey(key.NodePublic) (_ wgint.Peer, ok bool) { func (e *mockEngine) SetNetworkMap(*netmap.NetworkMap) {} +func (e *mockEngine) UpdateNetmapDelta([]netmap.NodeMutation) {} + func (e *mockEngine) UpdateStatus(*ipnstate.StatusBuilder) {} func (e *mockEngine) Ping(ip netip.Addr, pingType tailcfg.PingType, size int, cb func(*ipnstate.PingResult)) { diff --git a/wgengine/userspace.go b/wgengine/userspace.go index 58a09a50f..d95872f1a 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -4,6 +4,7 @@ package wgengine import ( + "cmp" "context" crand "crypto/rand" "crypto/x509" @@ -1290,6 +1291,51 @@ func (e *userspaceEngine) SetNetworkMap(nm *netmap.NetworkMap) { } } +// UpdateNetmapDelta implements [Engine]. It only applies peer upserts and +// removals; per-field mutations are handled by magicsock and don't affect +// the IP-to-node mapping served by PeerForIP. +func (e *userspaceEngine) UpdateNetmapDelta(muts []netmap.NodeMutation) { + changesPeerSet := false + for _, m := range muts { + switch m.(type) { + case netmap.NodeMutationUpsert, netmap.NodeMutationRemove: + changesPeerSet = true + } + } + if !changesPeerSet { + return + } + + e.mu.Lock() + defer e.mu.Unlock() + if e.netMap == nil { + return + } + // e.netMap is shared and read-only; mutate a copy. + nm := new(*e.netMap) + nm.Peers = slices.Clone(nm.Peers) + peerIndex := func(id tailcfg.NodeID) (int, bool) { + return slices.BinarySearchFunc(nm.Peers, id, func(p tailcfg.NodeView, id tailcfg.NodeID) int { + return cmp.Compare(p.ID(), id) + }) + } + for _, m := range muts { + switch m := m.(type) { + case netmap.NodeMutationUpsert: + if i, ok := peerIndex(m.Node.ID()); ok { + nm.Peers[i] = m.Node + } else { + nm.Peers = slices.Insert(nm.Peers, i, m.Node) + } + case netmap.NodeMutationRemove: + if i, ok := peerIndex(m.NodeIDBeingMutated()); ok { + nm.Peers = slices.Delete(nm.Peers, i, i+1) + } + } + } + e.netMap = nm +} + func (e *userspaceEngine) UpdateStatus(sb *ipnstate.StatusBuilder) { st, err := e.getStatus() if err != nil { diff --git a/wgengine/userspace_test.go b/wgengine/userspace_test.go index b2f40fada..56ea66adc 100644 --- a/wgengine/userspace_test.go +++ b/wgengine/userspace_test.go @@ -12,6 +12,7 @@ "slices" "sync" "testing" + "time" "go4.org/mem" "tailscale.com/cmd/testwrapper/flakytest" @@ -91,6 +92,84 @@ func TestUserspaceEngineReconfig(t *testing.T) { } } +func TestUserspaceEngineUpdateNetmapDelta(t *testing.T) { + bus := eventbustest.NewBus(t) + + ht := health.NewTracker(bus) + reg := new(usermetric.Registry) + e, err := NewFakeUserspaceEngine(t.Logf, 0, ht, reg, bus) + if err != nil { + t.Fatal(err) + } + t.Cleanup(e.Close) + + node := func(id tailcfg.NodeID, key key.NodePublic) *tailcfg.Node { + return &tailcfg.Node{ + ID: id, + Key: key, + Addresses: []netip.Prefix{netip.PrefixFrom(netaddr.IPv4(100, 64, 0, byte(id)), 32)}, + } + } + k1 := nkFromHex("1111111111111111111111111111111111111111111111111111111111111111") + k3 := nkFromHex("3333333333333333333333333333333333333333333333333333333333333333") + k5 := nkFromHex("5555555555555555555555555555555555555555555555555555555555555555") + k5new := nkFromHex("5555555555555555555555555555555555555555555555555555555555555aaa") + k4 := nkFromHex("4444444444444444444444444444444444444444444444444444444444444444") + + nm := &netmap.NetworkMap{ + Peers: nodeViews([]*tailcfg.Node{node(1, k1), node(3, k3), node(5, k5)}), + } + e.SetNetworkMap(nm) + + // Replace an existing peer, insert a new one between existing IDs, + // and remove another. + muts, ok := netmap.MutationsFromMapResponse(&tailcfg.MapResponse{ + PeersChanged: []*tailcfg.Node{node(5, k5new), node(4, k4)}, + PeersRemoved: []tailcfg.NodeID{1}, + }, time.Unix(123, 0)) + if !ok { + t.Fatal("netmap.MutationsFromMapResponse failed") + } + e.UpdateNetmapDelta(muts) + + wantPeers := map[netip.Addr]struct { + key key.NodePublic + ok bool + }{ + netaddr.IPv4(100, 64, 0, 1): {ok: false}, // removed + netaddr.IPv4(100, 64, 0, 3): {ok: true, key: k3}, // untouched + netaddr.IPv4(100, 64, 0, 4): {ok: true, key: k4}, // inserted + netaddr.IPv4(100, 64, 0, 5): {ok: true, key: k5new}, // replaced + } + for ip, want := range wantPeers { + pip, ok := e.PeerForIP(ip) + if ok != want.ok { + t.Errorf("PeerForIP(%v) ok = %v, want %v", ip, ok, want.ok) + continue + } + if ok && pip.Node.Key() != want.key { + t.Errorf("PeerForIP(%v) key = %v, want %v", ip, pip.Node.Key(), want.key) + } + } + + // The netmap passed to SetNetworkMap is read-only; deltas must not + // have mutated it. + if len(nm.Peers) != 3 || nm.Peers[2].Key() != k5 { + t.Errorf("UpdateNetmapDelta mutated the caller's netmap: %v", nm.Peers) + } + + ue := e.(*userspaceEngine) + gotIDs := make([]tailcfg.NodeID, 0, 3) + ue.mu.Lock() + for _, p := range ue.netMap.Peers { + gotIDs = append(gotIDs, p.ID()) + } + ue.mu.Unlock() + if want := []tailcfg.NodeID{3, 4, 5}; !slices.Equal(gotIDs, want) { + t.Errorf("peer IDs after delta = %v, want %v (sorted)", gotIDs, want) + } +} + func TestUserspaceEngineTSMPLearned(t *testing.T) { bus := eventbustest.NewBus(t) diff --git a/wgengine/wgengine.go b/wgengine/wgengine.go index 5ca4b75cf..6d833dc05 100644 --- a/wgengine/wgengine.go +++ b/wgengine/wgengine.go @@ -123,6 +123,12 @@ type Engine interface { // The network map should only be read from. SetNetworkMap(*netmap.NetworkMap) + // UpdateNetmapDelta applies the given peer mutations to the network + // map previously provided via SetNetworkMap. It is called instead of + // SetNetworkMap when the control plane sends an incremental (delta) + // update. + UpdateNetmapDelta([]netmap.NodeMutation) + // UpdateStatus populates the network state using the provided // status builder. UpdateStatus(*ipnstate.StatusBuilder)