From 6a635c4e55ac026a9bf7d2d1a25e1870fb18173d Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 9 Jul 2026 02:01:56 +0000 Subject: [PATCH] ipn/ipnlocal: route extra WireGuard AllowedIPs through the route manager The conn25 extension's ExtraWireGuardAllowedIPs hook (Transit IPs) was only appended to wgcfg.Config.Peers in authReconfig. Now that outbound peer selection comes from the route manager's outbound table via the engine's PeerByIPPacketFunc (which, when installed, replaces wireguard-go's AllowedIPs trie lookup entirely) and lazily created peers get their allowed IPs from the route manager via the engine's peer config func, those extras never reached either path: outbound packets to Transit IPs matched no peer, and lazily created peers didn't accept inbound Transit IP sources. Teach the route manager a per-peer set of extra allowed IPs, staged by Mutation.SetExtraAllowedIPs. They appear in the outbound table and in PeerAllowedIPs (so both outbound routing and per-peer allowed source prefixes see them) but are excluded from the OS route set, preserving the hook's contract that the extras reach WireGuard but not the OS routing table. authReconfig now feeds the hook's results into the route manager and incrementally syncs any changed peers to the WireGuard device; the append to cfg.Peers remains only so Reconfig's full per-peer device sync doesn't strip the extras from active peers, and goes away with Config.Peers. Updates #12542 Change-Id: I06c8fa30929fbf8fe171a2d34c47c6fcc3abfa16 Signed-off-by: Brad Fitzpatrick --- ipn/ipnlocal/local.go | 17 ++++++++-- ipn/ipnlocal/node_backend.go | 22 +++++++++++++ ipn/ipnlocal/node_backend_test.go | 54 +++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 2 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index c34dfc30a..134444ed8 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -6155,10 +6155,23 @@ func (b *LocalBackend) authReconfigLocked() { } rcfg := b.routerConfigLocked(cfg, prefs, nm) - // Add these extra Allowed IPs after router configuration, because the expected - // extension (features/conn25), does not want these routes installed on the OS. + // Push each peer's extra WireGuard-only allowed IPs (the conn25 + // extension's Transit IPs) into the route manager, which feeds + // them to WireGuard via the outbound peer lookup and the per-peer + // 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. // 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) diff --git a/ipn/ipnlocal/node_backend.go b/ipn/ipnlocal/node_backend.go index d7ef60f59..6c13d5211 100644 --- a/ipn/ipnlocal/node_backend.go +++ b/ipn/ipnlocal/node_backend.go @@ -938,6 +938,28 @@ func (nb *nodeBackend) updateRouteManagerPrefs(p routePrefs) (changedAllowedIPs return res.AllowedIPs } +// updateRouteManagerExtras pushes each peer's extra WireGuard-only +// allowed IPs into the route manager, obtained by calling fn (the +// [ipnext.Hooks.ExtraWireGuardAllowedIPs] hook) with each peer's +// public key. +// +// It returns the peers whose allowed source prefixes changed as a +// result, as described by [routemanager.Result.AllowedIPs]. +func (nb *nodeBackend) updateRouteManagerExtras(fn func(key.NodePublic) views.Slice[netip.Prefix]) (changedAllowedIPs map[key.NodePublic][]netip.Prefix) { + nb.mu.Lock() + defer nb.mu.Unlock() + var extras map[tailcfg.NodeID][]netip.Prefix + for id, p := range nb.peers { + if pfxs := fn(p.Key()); pfxs.Len() > 0 { + mak.Set(&extras, id, pfxs.AsSlice()) + } + } + rt := nb.routeMgr.Begin() + rt.SetExtraAllowedIPs(extras) + res := rt.Commit() + return res.AllowedIPs +} + // osRoutes returns the sorted set of prefixes that the route manager // wants programmed into the OS routing table. func (nb *nodeBackend) osRoutes() []netip.Prefix { diff --git a/ipn/ipnlocal/node_backend_test.go b/ipn/ipnlocal/node_backend_test.go index 04a7b8969..9b09ed930 100644 --- a/ipn/ipnlocal/node_backend_test.go +++ b/ipn/ipnlocal/node_backend_test.go @@ -17,6 +17,7 @@ "tailscale.com/tstest" "tailscale.com/types/key" "tailscale.com/types/netmap" + "tailscale.com/types/views" "tailscale.com/util/eventbus" "tailscale.com/util/mak" "tailscale.com/util/set" @@ -505,3 +506,56 @@ func TestNodeBackendDiscoChangedDelta(t *testing.T) { t.Errorf("upsert(after TSMP entry GC) discoChanged = %v; want %v", got, nk2) } } + +func TestNodeBackendRouteManagerExtras(t *testing.T) { + nb := newNodeBackend(t.Context(), tstest.WhileTestRunningLogger(t), eventbus.New()) + + n := &tailcfg.Node{ + ID: 1, + Key: key.NewNode().Public(), + HomeDERP: 1, + Addresses: []netip.Prefix{ + netip.MustParsePrefix("100.64.0.1/32"), + }, + } + n.AllowedIPs = n.Addresses + p1 := n.View() + nb.SetNetMap(&netmap.NetworkMap{Peers: []tailcfg.NodeView{p1}}) + + transit := netip.MustParsePrefix("fe80::1234/128") + extrasFor := func(k key.NodePublic) views.Slice[netip.Prefix] { + if k == p1.Key() { + return views.SliceOf([]netip.Prefix{transit}) + } + return views.Slice[netip.Prefix]{} + } + + // Installing extras reports the peer's allowed prefixes as + // changed and adds the transit IP to the outbound table. + changed := nb.updateRouteManagerExtras(extrasFor) + if len(changed) != 1 || !slices.Contains(changed[p1.Key()], transit) { + t.Errorf("updateRouteManagerExtras changed = %v; want %v including %v", changed, p1.Key(), transit) + } + if pr, ok := nb.routeMgr.Outbound().Lookup(transit.Addr()); !ok || pr.Key != p1.Key() { + t.Errorf("Outbound lookup %v = %v, %v; want %v", transit.Addr(), pr, ok, p1.Key()) + } + if nb.routeMgr.OSRoutes().Get(transit) { + t.Errorf("OSRoutes contains %v; extras must not reach the OS route set", transit) + } + + // An unchanged hook result is a no-op. + if changed := nb.updateRouteManagerExtras(extrasFor); changed != nil { + t.Errorf("unchanged extras reported changes: %v", changed) + } + + // A hook that no longer returns extras removes them. + changed = nb.updateRouteManagerExtras(func(key.NodePublic) views.Slice[netip.Prefix] { + return views.Slice[netip.Prefix]{} + }) + if len(changed) != 1 { + t.Errorf("clearing extras changed = %v; want just %v", changed, p1.Key()) + } + if _, ok := nb.routeMgr.Outbound().Lookup(transit.Addr()); ok { + t.Errorf("Outbound still routes %v after extras cleared", transit.Addr()) + } +}