mirror of
https://github.com/netbirdio/netbird.git
synced 2026-07-16 19:49:56 +00:00
[client] clear orphaned v6 exit selection when v4 pair is toggled
Root cause of the leaking ::/0 route, confirmed from client logs: the synthesized "-v6" exit route could stay explicitly selected in the persisted route-selector state while its v4 base was deselected (selected=[...-v6], deselected=[...v4base]). Because the v6 entry then has its own explicit state, effectiveNetID stops mirroring the v4 base, so FilterSelectedExitNodes keeps ::/0 and it is installed on the tunnel even though the user disabled the exit node. This happened because the iOS SDK's deselect only pairs the "-v6" sibling via ExpandV6ExitPairs when the v6 route is present in the current routesMap; a deselect at a moment it wasn't expanded left the v6 selection orphaned. Fix at the selector write path so it is independent of routesMap timing: when a v4 exit NetID is selected or deselected, clear any orphaned explicit state on its "-v6" sibling (clearPairedV6Locked), unless the sibling is part of the same batch (the deliberate ExpandV6ExitPairs case). The v6 then falls back to inheriting the v4 base via effectiveNetID, so a v4 deselect also drops ::/0 and a v4 select brings both back. Adds regression tests: a stale explicit v6 selection is cleared by a later v4 deselect, and an explicit v6 select made in the same batch is preserved.
This commit is contained in:
@@ -56,6 +56,11 @@ func (rs *RouteSelector) SelectRoutes(routes []route.NetID, appendRoute bool, al
|
||||
}
|
||||
delete(rs.deselectedRoutes, route)
|
||||
rs.selectedRoutes[route] = struct{}{}
|
||||
// Keep the v4/v6 exit pair consistent: clear any orphaned explicit state on
|
||||
// the "-v6" sibling so it inherits the v4 base via effectiveNetID. Skip when
|
||||
// the pair is itself part of this batch (callers expand it deliberately when
|
||||
// it should diverge).
|
||||
rs.clearPairedV6Locked(route, routes)
|
||||
}
|
||||
|
||||
rs.deselectAll = false
|
||||
@@ -96,11 +101,32 @@ func (rs *RouteSelector) DeselectRoutes(routes []route.NetID, allRoutes []route.
|
||||
}
|
||||
rs.deselectedRoutes[route] = struct{}{}
|
||||
delete(rs.selectedRoutes, route)
|
||||
// Keep the v4/v6 exit pair consistent: clear any orphaned explicit selection
|
||||
// on the "-v6" sibling so it falls back to inheriting the v4 base's state
|
||||
// (via effectiveNetID) instead of staying stuck as explicitly selected.
|
||||
rs.clearPairedV6Locked(route, routes)
|
||||
}
|
||||
|
||||
return errors.FormatErrorOrNil(err)
|
||||
}
|
||||
|
||||
// clearPairedV6Locked removes any explicit selected/deselected state on the "-v6"
|
||||
// sibling of a v4 exit-node NetID, so the synthesized v6 entry resolves through
|
||||
// effectiveNetID to its v4 base. No-op for IDs that already carry the "-v6" suffix,
|
||||
// or when the sibling is itself part of the current batch (the caller is setting it
|
||||
// deliberately, e.g. via ExpandV6ExitPairs). Must be called with rs.mu held.
|
||||
func (rs *RouteSelector) clearPairedV6Locked(id route.NetID, batch []route.NetID) {
|
||||
if strings.HasSuffix(string(id), route.V6ExitSuffix) {
|
||||
return
|
||||
}
|
||||
v6ID := route.NetID(string(id) + route.V6ExitSuffix)
|
||||
if slices.Contains(batch, v6ID) {
|
||||
return
|
||||
}
|
||||
delete(rs.selectedRoutes, v6ID)
|
||||
delete(rs.deselectedRoutes, v6ID)
|
||||
}
|
||||
|
||||
// DeselectAllRoutes deselects all routes, effectively disabling route selection.
|
||||
func (rs *RouteSelector) DeselectAllRoutes() {
|
||||
rs.mu.Lock()
|
||||
|
||||
@@ -423,6 +423,45 @@ func TestRouteSelector_V6ExitPairInherits(t *testing.T) {
|
||||
assert.Empty(t, filtered, "deselecting v4 base must also drop the v6 pair")
|
||||
})
|
||||
|
||||
// Regression for the observed bug: a stale explicit selection on the "-v6"
|
||||
// sibling (e.g. persisted from an earlier select where the pair was expanded)
|
||||
// must not survive a later deselect of the v4 base. Without clearing the orphan,
|
||||
// effectiveNetID sees the v6's own explicit "selected" state and the ::/0 route
|
||||
// leaks into the tunnel despite the user disabling the exit node.
|
||||
t.Run("deselect v4 base clears orphaned explicit v6 selection", func(t *testing.T) {
|
||||
rs := routeselector.NewRouteSelector()
|
||||
|
||||
// Prior state: both v4 and v6 explicitly selected (pair was expanded once).
|
||||
require.NoError(t, rs.SelectRoutes([]route.NetID{"exit1", "exit1-v6"}, true, all))
|
||||
require.True(t, rs.IsSelected("exit1-v6"))
|
||||
|
||||
// User later deselects only the v4 base (v6 not expanded into this batch).
|
||||
require.NoError(t, rs.DeselectRoutes([]route.NetID{"exit1"}, all))
|
||||
|
||||
// The orphaned explicit v6 selection must be gone, so the v6 inherits the
|
||||
// v4 deselect via effectiveNetID instead of staying selected.
|
||||
assert.False(t, rs.IsSelectedForExitNode("exit1-v6"), "v6 must inherit v4 deselect after orphan cleared")
|
||||
|
||||
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 drop the v6 pair even if it was explicitly selected before")
|
||||
})
|
||||
|
||||
// The inverse: an explicit v6 selection made in the SAME batch as the v4 (the
|
||||
// deliberate ExpandV6ExitPairs case) must be preserved, not wiped by the pair sync.
|
||||
t.Run("explicit v6 select in same batch is preserved", func(t *testing.T) {
|
||||
rs := routeselector.NewRouteSelector()
|
||||
require.NoError(t, rs.SelectRoutes([]route.NetID{"exit1", "exit1-v6"}, true, all))
|
||||
|
||||
assert.True(t, rs.IsSelectedForExitNode("exit1"))
|
||||
assert.True(t, rs.IsSelectedForExitNode("exit1-v6"), "v6 selected in the same batch must survive")
|
||||
})
|
||||
|
||||
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))
|
||||
|
||||
Reference in New Issue
Block a user