From 318807bdb9b96c59f2499e39028723c38df30850 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 9 Jul 2026 02:25:08 +0000 Subject: [PATCH] ipn/ipnlocal: derive the OS routes from the route manager routerConfigLocked previously computed router.Config.Routes with peerRoutes, a from-scratch pass over cfg.Peers on every reconfig. The route manager already maintains the same set incrementally (ULA and CGNAT coarsening included) and updateRouteManagerPrefs runs earlier in authReconfig, so its OS route set is current by the time the router config is built. Read it from there instead, and delete peerRoutes and its tests; the routemanager package tests cover the same behavior. This removes a consumer of cfg.Peers, which is on its way out. Updates #12542 Change-Id: I4a5b7a63d530e3fe1b70f0faf3f49def6a10be2e Signed-off-by: Brad Fitzpatrick --- ipn/ipnlocal/local.go | 60 +------ ipn/ipnlocal/local_test.go | 299 ++++++++--------------------------- ipn/ipnlocal/node_backend.go | 11 ++ 3 files changed, 76 insertions(+), 294 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 020bb0e84..7b6358830 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -6150,7 +6150,7 @@ func (b *LocalBackend) authReconfigLocked() { for k := range changedAllowedIPs { b.e.SyncDevicePeer(k) } - rcfg := b.routerConfigLocked(cfg, prefs, nm, oneCGNATRoute) + 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. @@ -6487,64 +6487,10 @@ func magicDNSRootDomains(nm *netmap.NetworkMap) []dnsname.FQDN { return nil } -// peerRoutes returns the routerConfig.Routes to access peers. -// If there are over cgnatThreshold CGNAT routes, one big CGNAT route -// is used instead. -func peerRoutes(logf logger.Logf, peers []wgcfg.Peer, cgnatThreshold int, routeAll bool) (routes []netip.Prefix) { - tsULA := tsaddr.TailscaleULARange() - cgNAT := tsaddr.CGNATRange() - var didULA bool - var cgNATIPs []netip.Prefix - for _, peer := range peers { - for _, aip := range peer.AllowedIPs { - aip = unmapIPPrefix(aip) - - // Ensure that we're only accepting properly-masked - // prefixes; the control server should be masking - // these, so if we get them, skip. - if mm := aip.Masked(); aip != mm { - // To avoid a DoS where a peer could cause all - // reconfigs to fail by sending a bad prefix, we just - // skip, but don't error, on an unmasked route. - logf("advertised route %s from %s has non-address bits set; expected %s", aip, peer.PublicKey.ShortString(), mm) - continue - } - - // Only add the Tailscale IPv6 ULA once, if we see anybody using part of it. - if aip.Addr().Is6() && aip.IsSingleIP() && tsULA.Contains(aip.Addr()) { - if !didULA { - didULA = true - routes = append(routes, tsULA) - } - continue - } - if aip.IsSingleIP() && cgNAT.Contains(aip.Addr()) { - cgNATIPs = append(cgNATIPs, aip) - } else if routeAll { - routes = append(routes, aip) - } - } - } - if len(cgNATIPs) > cgnatThreshold { - // Probably the hello server. Just append one big route. - routes = append(routes, cgNAT) - } else { - routes = append(routes, cgNATIPs...) - } - - tsaddr.SortPrefixes(routes) - return routes -} - // routerConfig produces a router.Config from a wireguard config and IPN prefs. // // b.mu must be held. -func (b *LocalBackend) routerConfigLocked(cfg *wgcfg.Config, prefs ipn.PrefsView, nm *netmap.NetworkMap, oneCGNATRoute bool) *router.Config { - singleRouteThreshold := 10_000 - if oneCGNATRoute { - singleRouteThreshold = 1 - } - +func (b *LocalBackend) routerConfigLocked(cfg *wgcfg.Config, prefs ipn.PrefsView, nm *netmap.NetworkMap) *router.Config { netfilterKind := b.capForcedNetfilter // protected by b.mu (hence the Locked suffix) if prefs.NetfilterKind() != "" { @@ -6568,7 +6514,7 @@ func (b *LocalBackend) routerConfigLocked(cfg *wgcfg.Config, prefs ipn.PrefsView SNATSubnetRoutes: !prefs.NoSNAT(), StatefulFiltering: doStatefulFiltering, NetfilterMode: prefs.NetfilterMode(), - Routes: peerRoutes(b.logf, cfg.Peers, singleRouteThreshold, prefs.RouteAll()), + Routes: b.currentNode().osRoutes(), NetfilterKind: netfilterKind, RemoveCGNATDropRule: nm.HasCap(tailcfg.NodeAttrDisableLinuxCGNATDropRule), } diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index df2d71f11..7644e178f 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -189,135 +189,6 @@ func TestShrinkDefaultRoute(t *testing.T) { } } -func TestPeerRoutes(t *testing.T) { - pp := netip.MustParsePrefix - tests := []struct { - name string - peers []wgcfg.Peer - want []netip.Prefix - }{ - { - name: "small_v4", - peers: []wgcfg.Peer{ - { - AllowedIPs: []netip.Prefix{ - pp("100.101.102.103/32"), - }, - }, - }, - want: []netip.Prefix{ - pp("100.101.102.103/32"), - }, - }, - { - name: "big_v4", - peers: []wgcfg.Peer{ - { - AllowedIPs: []netip.Prefix{ - pp("100.101.102.103/32"), - pp("100.101.102.104/32"), - pp("100.101.102.105/32"), - }, - }, - }, - want: []netip.Prefix{ - pp("100.64.0.0/10"), - }, - }, - { - name: "has_1_v6", - peers: []wgcfg.Peer{ - { - AllowedIPs: []netip.Prefix{ - pp("fd7a:115c:a1e0:ab12:4843:cd96:6258:b240/128"), - }, - }, - }, - want: []netip.Prefix{ - pp("fd7a:115c:a1e0::/48"), - }, - }, - { - name: "has_2_v6", - peers: []wgcfg.Peer{ - { - AllowedIPs: []netip.Prefix{ - pp("fd7a:115c:a1e0:ab12:4843:cd96:6258:b240/128"), - pp("fd7a:115c:a1e0:ab12:4843:cd96:6258:b241/128"), - }, - }, - }, - want: []netip.Prefix{ - pp("fd7a:115c:a1e0::/48"), - }, - }, - { - name: "big_v4_big_v6", - peers: []wgcfg.Peer{ - { - AllowedIPs: []netip.Prefix{ - pp("100.101.102.103/32"), - pp("100.101.102.104/32"), - pp("100.101.102.105/32"), - pp("fd7a:115c:a1e0:ab12:4843:cd96:6258:b240/128"), - pp("fd7a:115c:a1e0:ab12:4843:cd96:6258:b241/128"), - }, - }, - }, - want: []netip.Prefix{ - pp("100.64.0.0/10"), - pp("fd7a:115c:a1e0::/48"), - }, - }, - { - name: "output-should-be-sorted", - peers: []wgcfg.Peer{ - { - AllowedIPs: []netip.Prefix{ - pp("100.64.0.2/32"), - pp("10.0.0.0/16"), - }, - }, - { - AllowedIPs: []netip.Prefix{ - pp("100.64.0.1/32"), - pp("10.0.0.0/8"), - }, - }, - }, - want: []netip.Prefix{ - pp("10.0.0.0/8"), - pp("10.0.0.0/16"), - pp("100.64.0.1/32"), - pp("100.64.0.2/32"), - }, - }, - { - name: "skip-unmasked-prefixes", - peers: []wgcfg.Peer{ - { - PublicKey: key.NewNode().Public(), - AllowedIPs: []netip.Prefix{ - pp("100.64.0.2/32"), - pp("10.0.0.100/16"), - }, - }, - }, - want: []netip.Prefix{ - pp("100.64.0.2/32"), - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := peerRoutes(t.Logf, tt.peers, 2, true) - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("got = %v; want %v", got, tt.want) - } - }) - } -} - func TestPeerAPIBase(t *testing.T) { tests := []struct { name string @@ -8773,72 +8644,65 @@ func TestStripKeysFromPrefs(t *testing.T) { func TestRouteAllDisabled(t *testing.T) { pp := netip.MustParsePrefix + peer := &tailcfg.Node{ + ID: 1, + Key: key.NewNode().Public(), + HomeDERP: 1, + Addresses: []netip.Prefix{ + pp("100.80.207.38/32"), + }, + AllowedIPs: []netip.Prefix{ + pp("100.80.207.38/32"), + + // If one IP in the Tailscale ULA range is added, the + // entire range is added to the router config. + pp("fd7a:115c:a1e0::2501:9b83/128"), + + // Other single CGNAT IPs (such as VIP service addresses) + // are added individually regardless of RouteAll. + pp("100.80.207.56/32"), + pp("100.80.207.40/32"), + pp("100.94.122.93/32"), + pp("100.79.141.115/32"), + + // A /28 is a subnet route, added only with RouteAll. + pp("100.64.0.0/28"), + + // Single IPs outside the Tailscale CGNAT/ULA ranges are + // subnet routes too. + pp("192.168.0.45/32"), + pp("fd7a:115c:b1e0::2501:9b83/128"), + pp("fdf8:f966:e27c:0:5:0:0:10/128"), + }, + } + tests := []struct { - name string - peers []wgcfg.Peer - wantEndpoints []netip.Prefix - routeAll bool + name string + routeAll bool + want []netip.Prefix }{ { name: "route_all_disabled", routeAll: false, - peers: []wgcfg.Peer{ - { - AllowedIPs: []netip.Prefix{ - // if one ip in the Tailscale ULA range is added, the entire range is added to the router config - pp("fd7a:115c:a1e0::2501:9b83/128"), - pp("100.80.207.38/32"), - pp("100.80.207.56/32"), - pp("100.80.207.40/32"), - pp("100.94.122.93/32"), - pp("100.79.141.115/32"), - - // a /28 range will not be added, since this is not a Service IP range (which is always /32, a single IP) - pp("100.64.0.0/28"), - - // ips outside the tailscale cgnat/ula range are not added to the router config - pp("192.168.0.45/32"), - pp("fd7a:115c:b1e0::2501:9b83/128"), - pp("fdf8:f966:e27c:0:5:0:0:10/128"), - }, - }, - }, - wantEndpoints: []netip.Prefix{ - pp("100.80.207.38/32"), - pp("100.80.207.56/32"), - pp("100.80.207.40/32"), - pp("100.94.122.93/32"), + want: []netip.Prefix{ pp("100.79.141.115/32"), + pp("100.80.207.38/32"), + pp("100.80.207.40/32"), + pp("100.80.207.56/32"), + pp("100.94.122.93/32"), pp("fd7a:115c:a1e0::/48"), }, }, { name: "route_all_enabled", routeAll: true, - peers: []wgcfg.Peer{ - { - AllowedIPs: []netip.Prefix{ - // if one ip in the Tailscale ULA range is added, the entire range is added to the router config - pp("fd7a:115c:a1e0::2501:9b83/128"), - pp("100.80.207.38/32"), - pp("100.80.207.56/32"), - pp("100.80.207.40/32"), - pp("100.94.122.93/32"), - pp("100.79.141.115/32"), - - // ips outside the tailscale cgnat/ula range are not added to the router config - pp("192.168.0.45/32"), - pp("fd7a:115c:b1e0::2501:9b83/128"), - pp("fdf8:f966:e27c:0:5:0:0:10/128"), - }, - }, - }, - wantEndpoints: []netip.Prefix{ - pp("100.80.207.38/32"), - pp("100.80.207.56/32"), - pp("100.80.207.40/32"), - pp("100.94.122.93/32"), + want: []netip.Prefix{ + pp("100.64.0.0/28"), pp("100.79.141.115/32"), + pp("100.80.207.38/32"), + pp("100.80.207.40/32"), + pp("100.80.207.56/32"), + pp("100.94.122.93/32"), pp("192.168.0.45/32"), pp("fd7a:115c:a1e0::/48"), pp("fd7a:115c:b1e0::2501:9b83/128"), @@ -8851,9 +8715,6 @@ func TestRouteAllDisabled(t *testing.T) { t.Run(tt.name, func(t *testing.T) { prefs := ipn.Prefs{RouteAll: tt.routeAll} lb := newTestLocalBackend(t) - cfg := &wgcfg.Config{ - Peers: tt.peers, - } ServiceIPMappings := tailcfg.ServiceIPMappings{ "svc:test-service": []netip.Addr{ netip.MustParseAddr("100.64.1.2"), @@ -8876,20 +8737,15 @@ func TestRouteAllDisabled(t *testing.T) { }, }, }).View(), + Peers: []tailcfg.NodeView{peer.View()}, } + cn := lb.currentNode() + cn.SetNetMap(nm) + cn.updateRouteManagerPrefs(routePrefs{RouteAll: tt.routeAll}) - rcfg := lb.routerConfigLocked(cfg, prefs.View(), nm, false) - for _, p := range rcfg.Routes { - found := false - for _, r := range tt.wantEndpoints { - if p.Addr() == r.Addr() { - found = true - break - } - } - if !found { - t.Errorf("unexpected prefix %q in router config", p.String()) - } + rcfg := lb.routerConfigLocked(&wgcfg.Config{}, prefs.View(), nm) + if !slices.Equal(rcfg.Routes, tt.want) { + t.Errorf("Routes = %v; want %v", rcfg.Routes, tt.want) } }) } @@ -9323,44 +9179,6 @@ func TestShouldUseOneCGNATRoute(t *testing.T) { } -func TestPeerRoutesCGNATCollapse(t *testing.T) { - pp := netip.MustParsePrefix - - // With cgnatThreshold=1 (oneCGNATRoute), adding a peer should not - // change the route list. Both collapse to a single 100.64.0.0/10. - twoPeers := []wgcfg.Peer{ - {AllowedIPs: []netip.Prefix{pp("100.64.0.1/32")}}, - {AllowedIPs: []netip.Prefix{pp("100.64.0.2/32")}}, - } - threePeers := []wgcfg.Peer{ - {AllowedIPs: []netip.Prefix{pp("100.64.0.1/32")}}, - {AllowedIPs: []netip.Prefix{pp("100.64.0.2/32")}}, - {AllowedIPs: []netip.Prefix{pp("100.64.0.3/32")}}, - } - - routesTwo := peerRoutes(t.Logf, twoPeers, 1, true) - routesThree := peerRoutes(t.Logf, threePeers, 1, true) - - wantCGNAT := []netip.Prefix{pp("100.64.0.0/10")} - if !reflect.DeepEqual(routesTwo, wantCGNAT) { - t.Errorf("two peers: got %v; want %v", routesTwo, wantCGNAT) - } - if !reflect.DeepEqual(routesThree, wantCGNAT) { - t.Errorf("three peers: got %v; want %v", routesThree, wantCGNAT) - } - - // Subnet routes must still appear alongside the collapsed CGNAT route. - peersWithSubnet := []wgcfg.Peer{ - {AllowedIPs: []netip.Prefix{pp("100.64.0.1/32")}}, - {AllowedIPs: []netip.Prefix{pp("100.64.0.2/32"), pp("10.0.0.0/24")}}, - } - got := peerRoutes(t.Logf, peersWithSubnet, 1, true) - want := []netip.Prefix{pp("100.64.0.0/10"), pp("10.0.0.0/24")} - if !reflect.DeepEqual(got, want) { - t.Errorf("with subnet: got %v; want %v", got, want) - } -} - func TestResetAuthClearsMachineKey(t *testing.T) { store := new(mem.Store) @@ -9689,16 +9507,23 @@ func TestRouterConfigExitNodeBlackhole(t *testing.T) { hasDefaults := func(routes []netip.Prefix) bool { return slices.Contains(routes, tsaddr.AllIPv4()) && slices.Contains(routes, tsaddr.AllIPv6()) } + // Push the netmap and prefs into the route manager first, as + // authReconfigLocked does before calling routerConfigLocked, now + // that the OS routes are derived from the route manager. + cn := lb.currentNode() + cn.SetNetMap(nm) for _, exitID := range []tailcfg.StableNodeID{"auto:any", "no-such-node"} { prefs := ipn.Prefs{ExitNodeID: exitID} - rcfg := lb.routerConfigLocked(cfg, prefs.View(), nm, false) + cn.updateRouteManagerPrefs(routePrefs{ExitNodeID: exitID, ExitNodeSelected: true}) + rcfg := lb.routerConfigLocked(cfg, prefs.View(), nm) if !hasDefaults(rcfg.Routes) { t.Errorf("ExitNodeID=%q: Routes = %v; want blackhole default routes", exitID, rcfg.Routes) } } // With no exit node selected, there must be no default routes. - rcfg := lb.routerConfigLocked(cfg, new(ipn.Prefs).View(), nm, false) + cn.updateRouteManagerPrefs(routePrefs{}) + rcfg := lb.routerConfigLocked(cfg, new(ipn.Prefs).View(), nm) if hasDefaults(rcfg.Routes) { t.Errorf("no exit node: Routes = %v; want no default routes", rcfg.Routes) } diff --git a/ipn/ipnlocal/node_backend.go b/ipn/ipnlocal/node_backend.go index 3aab1a402..bf16f25ad 100644 --- a/ipn/ipnlocal/node_backend.go +++ b/ipn/ipnlocal/node_backend.go @@ -861,6 +861,17 @@ func (nb *nodeBackend) updateRouteManagerPrefs(p routePrefs) (changedAllowedIPs 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 { + var routes []netip.Prefix + for pfx := range nb.routeMgr.OSRoutes().All() { + routes = append(routes, pfx) + } + tsaddr.SortPrefixes(routes) + return routes +} + // setPacketFilter stores the live packet filter rules and parsed // matches. It does not touch the frozen netMap. nb.mu is acquired by // this method.