From 2ccae7ec479c106efb6d7a7edff4bb55affb2aa4 Mon Sep 17 00:00:00 2001 From: Viktor Liu <17948409+lixmal@users.noreply.github.com> Date: Fri, 15 May 2026 23:58:47 +0900 Subject: [PATCH] [client] Mirror v4 exit selection onto v6 pair and honour SkipAutoApply per route (#6150) --- client/internal/routemanager/manager.go | 5 +- .../internal/routeselector/routeselector.go | 84 ++++++----- .../routeselector/routeselector_test.go | 131 ++++++++++++++++++ client/ui/network.go | 10 +- 4 files changed, 197 insertions(+), 33 deletions(-) diff --git a/client/internal/routemanager/manager.go b/client/internal/routemanager/manager.go index e5d9363ca..907f1f592 100644 --- a/client/internal/routemanager/manager.go +++ b/client/internal/routemanager/manager.go @@ -704,7 +704,10 @@ func (m *DefaultManager) collectExitNodeInfo(clientRoutes route.HAMap) exitNodeI } func (m *DefaultManager) isExitNodeRoute(routes []*route.Route) bool { - return len(routes) > 0 && routes[0].Network.String() == vars.ExitNodeCIDR + if len(routes) == 0 { + return false + } + return route.IsV4DefaultRoute(routes[0].Network) || route.IsV6DefaultRoute(routes[0].Network) } func (m *DefaultManager) categorizeUserSelection(netID route.NetID, info *exitNodeInfo) { diff --git a/client/internal/routeselector/routeselector.go b/client/internal/routeselector/routeselector.go index 30afc013b..2ddc24bf2 100644 --- a/client/internal/routeselector/routeselector.go +++ b/client/internal/routeselector/routeselector.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "slices" + "strings" "sync" "github.com/hashicorp/go-multierror" @@ -12,10 +13,6 @@ import ( "github.com/netbirdio/netbird/route" ) -const ( - exitNodeCIDR = "0.0.0.0/0" -) - type RouteSelector struct { mu sync.RWMutex deselectedRoutes map[route.NetID]struct{} @@ -124,13 +121,7 @@ func (rs *RouteSelector) IsSelected(routeID route.NetID) bool { rs.mu.RLock() defer rs.mu.RUnlock() - if rs.deselectAll { - return false - } - - _, deselected := rs.deselectedRoutes[routeID] - isSelected := !deselected - return isSelected + return rs.isSelectedLocked(routeID) } // FilterSelected removes unselected routes from the provided map. @@ -144,23 +135,22 @@ func (rs *RouteSelector) FilterSelected(routes route.HAMap) route.HAMap { filtered := route.HAMap{} for id, rt := range routes { - netID := id.NetID() - _, deselected := rs.deselectedRoutes[netID] - if !deselected { + if !rs.isDeselectedLocked(id.NetID()) { filtered[id] = rt } } return filtered } -// HasUserSelectionForRoute returns true if the user has explicitly selected or deselected this specific route +// HasUserSelectionForRoute returns true if the user has explicitly selected or deselected this route. +// Intended for exit-node code paths: a v6 exit-node pair (e.g. "MyExit-v6") with no explicit state of +// its own inherits its v4 base's state, so legacy persisted selections that predate v6 pairing +// transparently apply to the synthesized v6 entry. func (rs *RouteSelector) HasUserSelectionForRoute(routeID route.NetID) bool { rs.mu.RLock() defer rs.mu.RUnlock() - _, selected := rs.selectedRoutes[routeID] - _, deselected := rs.deselectedRoutes[routeID] - return selected || deselected + return rs.hasUserSelectionForRouteLocked(rs.effectiveNetID(routeID)) } func (rs *RouteSelector) FilterSelectedExitNodes(routes route.HAMap) route.HAMap { @@ -174,7 +164,7 @@ func (rs *RouteSelector) FilterSelectedExitNodes(routes route.HAMap) route.HAMap filtered := make(route.HAMap, len(routes)) for id, rt := range routes { netID := id.NetID() - if rs.isDeselected(netID) { + if rs.isDeselectedLocked(netID) { continue } @@ -189,13 +179,48 @@ func (rs *RouteSelector) FilterSelectedExitNodes(routes route.HAMap) route.HAMap return filtered } -func (rs *RouteSelector) isDeselected(netID route.NetID) bool { +// effectiveNetID returns the v4 base for a "-v6" exit pair entry that has no explicit +// state of its own, so selections made on the v4 entry govern the v6 entry automatically. +// Only call this from exit-node-specific code paths: applying it to a non-exit "-v6" route +// would make it inherit unrelated v4 state. Must be called with rs.mu held. +func (rs *RouteSelector) effectiveNetID(id route.NetID) route.NetID { + name := string(id) + if !strings.HasSuffix(name, route.V6ExitSuffix) { + return id + } + if _, ok := rs.selectedRoutes[id]; ok { + return id + } + if _, ok := rs.deselectedRoutes[id]; ok { + return id + } + return route.NetID(strings.TrimSuffix(name, route.V6ExitSuffix)) +} + +func (rs *RouteSelector) isSelectedLocked(routeID route.NetID) bool { + if rs.deselectAll { + return false + } + _, deselected := rs.deselectedRoutes[routeID] + return !deselected +} + +func (rs *RouteSelector) isDeselectedLocked(netID route.NetID) bool { + if rs.deselectAll { + return true + } _, deselected := rs.deselectedRoutes[netID] - return deselected || rs.deselectAll + return deselected +} + +func (rs *RouteSelector) hasUserSelectionForRouteLocked(routeID route.NetID) bool { + _, selected := rs.selectedRoutes[routeID] + _, deselected := rs.deselectedRoutes[routeID] + return selected || deselected } func isExitNode(rt []*route.Route) bool { - return len(rt) > 0 && rt[0].Network.String() == exitNodeCIDR + return len(rt) > 0 && (route.IsV4DefaultRoute(rt[0].Network) || route.IsV6DefaultRoute(rt[0].Network)) } func (rs *RouteSelector) applyExitNodeFilter( @@ -204,26 +229,23 @@ func (rs *RouteSelector) applyExitNodeFilter( rt []*route.Route, out route.HAMap, ) { - - if rs.hasUserSelections() { - // user made explicit selects/deselects - if rs.IsSelected(netID) { + // Exit-node path: apply the v4/v6 pair mirror so a deselect on the v4 base also + // drops the synthesized v6 entry that lacks its own explicit state. + effective := rs.effectiveNetID(netID) + if rs.hasUserSelectionForRouteLocked(effective) { + if rs.isSelectedLocked(effective) { out[id] = rt } return } - // no explicit selections: only include routes marked !SkipAutoApply (=AutoApply) + // no explicit selection for this route: defer to management's SkipAutoApply flag sel := collectSelected(rt) if len(sel) > 0 { out[id] = sel } } -func (rs *RouteSelector) hasUserSelections() bool { - return len(rs.selectedRoutes) > 0 || len(rs.deselectedRoutes) > 0 -} - func collectSelected(rt []*route.Route) []*route.Route { var sel []*route.Route for _, r := range rt { diff --git a/client/internal/routeselector/routeselector_test.go b/client/internal/routeselector/routeselector_test.go index 5faea2456..3f0d9f120 100644 --- a/client/internal/routeselector/routeselector_test.go +++ b/client/internal/routeselector/routeselector_test.go @@ -330,6 +330,137 @@ func TestRouteSelector_FilterSelectedExitNodes(t *testing.T) { assert.Len(t, filtered, 0) // No routes should be selected } +// TestRouteSelector_V6ExitPairInherits covers the v4/v6 exit-node pair selection +// mirror. The mirror is scoped to exit-node code paths: HasUserSelectionForRoute +// and FilterSelectedExitNodes resolve a "-v6" entry without explicit state to its +// v4 base, so legacy persisted selections that predate v6 pairing transparently +// apply to the synthesized v6 entry. General lookups (IsSelected, FilterSelected) +// stay literal so unrelated routes named "*-v6" don't inherit unrelated state. +func TestRouteSelector_V6ExitPairInherits(t *testing.T) { + all := []route.NetID{"exit1", "exit1-v6", "exit2", "exit2-v6", "corp", "corp-v6"} + + t.Run("HasUserSelectionForRoute mirrors deselected v4 base", func(t *testing.T) { + rs := routeselector.NewRouteSelector() + require.NoError(t, rs.DeselectRoutes([]route.NetID{"exit1"}, all)) + + assert.True(t, rs.HasUserSelectionForRoute("exit1-v6"), "v6 pair sees v4 base's user selection") + + // unrelated v6 with no v4 base touched is unaffected + assert.False(t, rs.HasUserSelectionForRoute("exit2-v6")) + }) + + t.Run("IsSelected stays literal for non-exit lookups", func(t *testing.T) { + rs := routeselector.NewRouteSelector() + require.NoError(t, rs.DeselectRoutes([]route.NetID{"corp"}, all)) + + // A non-exit route literally named "corp-v6" must not inherit "corp"'s state + // via the mirror; the mirror only applies in exit-node code paths. + assert.False(t, rs.IsSelected("corp")) + assert.True(t, rs.IsSelected("corp-v6"), "non-exit *-v6 routes must not inherit unrelated v4 state") + }) + + t.Run("explicit v6 state overrides v4 base in filter", func(t *testing.T) { + rs := routeselector.NewRouteSelector() + require.NoError(t, rs.DeselectRoutes([]route.NetID{"exit1"}, all)) + require.NoError(t, rs.SelectRoutes([]route.NetID{"exit1-v6"}, true, all)) + + v4Route := &route.Route{NetID: "exit1", Network: netip.MustParsePrefix("0.0.0.0/0")} + v6Route := &route.Route{NetID: "exit1-v6", Network: netip.MustParsePrefix("::/0")} + routes := route.HAMap{ + "exit1|0.0.0.0/0": {v4Route}, + "exit1-v6|::/0": {v6Route}, + } + + filtered := rs.FilterSelectedExitNodes(routes) + assert.NotContains(t, filtered, route.HAUniqueID("exit1|0.0.0.0/0")) + assert.Contains(t, filtered, route.HAUniqueID("exit1-v6|::/0"), "explicit v6 select wins over v4 base") + }) + + t.Run("non-v6-suffix routes unaffected", func(t *testing.T) { + rs := routeselector.NewRouteSelector() + require.NoError(t, rs.DeselectRoutes([]route.NetID{"exit1"}, all)) + + // A route literally named "exit1-something" must not pair-resolve. + assert.False(t, rs.HasUserSelectionForRoute("exit1-something")) + }) + + t.Run("filter v6 paired with deselected v4 base", func(t *testing.T) { + rs := routeselector.NewRouteSelector() + require.NoError(t, rs.DeselectRoutes([]route.NetID{"exit1"}, all)) + + v4Route := &route.Route{NetID: "exit1", Network: netip.MustParsePrefix("0.0.0.0/0")} + v6Route := &route.Route{NetID: "exit1-v6", Network: netip.MustParsePrefix("::/0")} + routes := route.HAMap{ + "exit1|0.0.0.0/0": {v4Route}, + "exit1-v6|::/0": {v6Route}, + } + + filtered := rs.FilterSelectedExitNodes(routes) + assert.Empty(t, filtered, "deselecting v4 base must also drop the v6 pair") + }) + + t.Run("non-exit *-v6 routes pass through FilterSelectedExitNodes", func(t *testing.T) { + rs := routeselector.NewRouteSelector() + require.NoError(t, rs.DeselectRoutes([]route.NetID{"corp"}, all)) + + // A non-default-route entry named "corp-v6" is not an exit node and + // must not be skipped because its v4 base "corp" is deselected. + corpV6 := &route.Route{NetID: "corp-v6", Network: netip.MustParsePrefix("10.0.0.0/8")} + routes := route.HAMap{ + "corp-v6|10.0.0.0/8": {corpV6}, + } + + filtered := rs.FilterSelectedExitNodes(routes) + assert.Contains(t, filtered, route.HAUniqueID("corp-v6|10.0.0.0/8"), + "non-exit *-v6 routes must not inherit unrelated v4 state in FilterSelectedExitNodes") + }) +} + +// TestRouteSelector_SkipAutoApplyPerRoute verifies that management's +// SkipAutoApply flag governs each untouched route independently, even when +// the user has explicit selections on other routes. +func TestRouteSelector_SkipAutoApplyPerRoute(t *testing.T) { + autoApplied := &route.Route{ + NetID: "Auto", + Network: netip.MustParsePrefix("0.0.0.0/0"), + SkipAutoApply: false, + } + skipApply := &route.Route{ + NetID: "Skip", + Network: netip.MustParsePrefix("0.0.0.0/0"), + SkipAutoApply: true, + } + routes := route.HAMap{ + "Auto|0.0.0.0/0": {autoApplied}, + "Skip|0.0.0.0/0": {skipApply}, + } + + rs := routeselector.NewRouteSelector() + // User makes an unrelated explicit selection elsewhere. + require.NoError(t, rs.DeselectRoutes([]route.NetID{"Unrelated"}, []route.NetID{"Auto", "Skip", "Unrelated"})) + + filtered := rs.FilterSelectedExitNodes(routes) + assert.Contains(t, filtered, route.HAUniqueID("Auto|0.0.0.0/0"), "AutoApply route should be included") + assert.NotContains(t, filtered, route.HAUniqueID("Skip|0.0.0.0/0"), "SkipAutoApply route should be excluded without explicit user selection") +} + +// TestRouteSelector_V6ExitIsExitNode verifies that ::/0 routes are recognized +// as exit nodes by the selector's filter path. +func TestRouteSelector_V6ExitIsExitNode(t *testing.T) { + v6Exit := &route.Route{ + NetID: "V6Only", + Network: netip.MustParsePrefix("::/0"), + SkipAutoApply: true, + } + routes := route.HAMap{ + "V6Only|::/0": {v6Exit}, + } + + rs := routeselector.NewRouteSelector() + filtered := rs.FilterSelectedExitNodes(routes) + assert.Empty(t, filtered, "::/0 should be treated as an exit node and respect SkipAutoApply") +} + func TestRouteSelector_NewRoutesBehavior(t *testing.T) { initialRoutes := []route.NetID{"route1", "route2", "route3"} newRoutes := []route.NetID{"route1", "route2", "route3", "route4", "route5"} diff --git a/client/ui/network.go b/client/ui/network.go index 1619f78a2..cd5d23558 100644 --- a/client/ui/network.go +++ b/client/ui/network.go @@ -193,7 +193,15 @@ func getOverlappingNetworks(routes []*proto.Network) []*proto.Network { } func isDefaultRoute(routeRange string) bool { - return routeRange == "0.0.0.0/0" || routeRange == "::/0" + // routeRange is the merged display string from the daemon, e.g. "0.0.0.0/0", + // "::/0", or "0.0.0.0/0, ::/0" when a v4 exit node has a paired v6 entry. + for _, part := range strings.Split(routeRange, ",") { + switch strings.TrimSpace(part) { + case "0.0.0.0/0", "::/0": + return true + } + } + return false } func getExitNodeNetworks(routes []*proto.Network) []*proto.Network {