diff --git a/client/android/client.go b/client/android/client.go index 2266ff53d..2ce627f10 100644 --- a/client/android/client.go +++ b/client/android/client.go @@ -439,10 +439,6 @@ func (c *Client) RemoveConnectionListener() { c.recorder.RemoveConnectionListener() } -func (c *Client) toggleRoute(command routeCommand) error { - return command.toggleRoute() -} - func (c *Client) getRouteManager() (routemanager.Manager, error) { client := c.getConnectClient() if client == nil { @@ -462,22 +458,22 @@ func (c *Client) getRouteManager() (routemanager.Manager, error) { return manager, nil } -func (c *Client) SelectRoute(route string) error { +func (c *Client) SelectRoute(id string) error { manager, err := c.getRouteManager() if err != nil { return err } - return c.toggleRoute(selectRouteCommand{route: route, manager: manager}) + return manager.SelectRoutes([]route.NetID{route.NetID(id)}, true) } -func (c *Client) DeselectRoute(route string) error { +func (c *Client) DeselectRoute(id string) error { manager, err := c.getRouteManager() if err != nil { return err } - return c.toggleRoute(deselectRouteCommand{route: route, manager: manager}) + return manager.DeselectRoutes([]route.NetID{route.NetID(id)}) } // getNetworkDomainsFromRoute extracts domains from a route and enriches each domain diff --git a/client/android/route_command.go b/client/android/route_command.go deleted file mode 100644 index 5e7357335..000000000 --- a/client/android/route_command.go +++ /dev/null @@ -1,70 +0,0 @@ -//go:build android - -package android - -import ( - "fmt" - - log "github.com/sirupsen/logrus" - "golang.org/x/exp/maps" - - "github.com/netbirdio/netbird/client/internal/routemanager" - "github.com/netbirdio/netbird/route" -) - -func executeRouteToggle(id string, manager routemanager.Manager, - operationName string, - routeOperation func(routes []route.NetID, allRoutes []route.NetID) error) error { - netID := route.NetID(id) - routes := []route.NetID{netID} - - routesMap := manager.GetClientRoutesWithNetID() - routes = route.ExpandV6ExitPairs(routes, routesMap) - - log.Debugf("%s with ids: %v", operationName, routes) - - if err := routeOperation(routes, maps.Keys(routesMap)); err != nil { - log.Debugf("error when %s: %s", operationName, err) - return fmt.Errorf("error %s: %w", operationName, err) - } - - manager.TriggerSelection(manager.GetClientRoutes()) - - return nil -} - -type routeCommand interface { - toggleRoute() error -} - -type selectRouteCommand struct { - route string - manager routemanager.Manager -} - -func (s selectRouteCommand) toggleRoute() error { - routeSelector := s.manager.GetRouteSelector() - if routeSelector == nil { - return fmt.Errorf("no route selector available") - } - - routeOperation := func(routes []route.NetID, allRoutes []route.NetID) error { - return routeSelector.SelectRoutes(routes, true, allRoutes) - } - - return executeRouteToggle(s.route, s.manager, "selecting route", routeOperation) -} - -type deselectRouteCommand struct { - route string - manager routemanager.Manager -} - -func (d deselectRouteCommand) toggleRoute() error { - routeSelector := d.manager.GetRouteSelector() - if routeSelector == nil { - return fmt.Errorf("no route selector available") - } - - return executeRouteToggle(d.route, d.manager, "deselecting route", routeSelector.DeselectRoutes) -} diff --git a/client/internal/routemanager/manager.go b/client/internal/routemanager/manager.go index ef69b81a4..615a91e94 100644 --- a/client/internal/routemanager/manager.go +++ b/client/internal/routemanager/manager.go @@ -52,6 +52,10 @@ type Manager interface { UpdateRoutes(updateSerial uint64, serverRoutes map[route.ID]*route.Route, clientRoutes route.HAMap, useNewDNSRoute bool) error ClassifyRoutes(newRoutes []*route.Route) (map[route.ID]*route.Route, route.HAMap) TriggerSelection(route.HAMap) + SelectRoutes(ids []route.NetID, appendRoute bool) error + DeselectRoutes(ids []route.NetID) error + SelectAllRoutes() + DeselectAllRoutes() GetRouteSelector() *routeselector.RouteSelector GetClientRoutes() route.HAMap GetSelectedClientRoutes() route.HAMap @@ -800,7 +804,7 @@ func (m *DefaultManager) collectExitNodeInfo(clientRoutes route.HAMap) exitNodeI var info exitNodeInfo for haID, routes := range clientRoutes { - if !m.isExitNodeRoute(routes) { + if !isExitNodeRoutes(routes) { continue } @@ -820,13 +824,6 @@ func (m *DefaultManager) collectExitNodeInfo(clientRoutes route.HAMap) exitNodeI return info } -func (m *DefaultManager) isExitNodeRoute(routes []*route.Route) bool { - 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) { if m.routeSelector.IsSelected(netID) { info.userSelected = append(info.userSelected, netID) diff --git a/client/internal/routemanager/mock.go b/client/internal/routemanager/mock.go index c1620b24c..cf761091d 100644 --- a/client/internal/routemanager/mock.go +++ b/client/internal/routemanager/mock.go @@ -16,6 +16,8 @@ type MockManager struct { ClassifyRoutesFunc func(routes []*route.Route) (map[route.ID]*route.Route, route.HAMap) UpdateRoutesFunc func(updateSerial uint64, serverRoutes map[route.ID]*route.Route, clientRoutes route.HAMap, useNewDNSRoute bool) error TriggerSelectionFunc func(haMap route.HAMap) + SelectRoutesFunc func(ids []route.NetID, appendRoute bool) error + DeselectRoutesFunc func(ids []route.NetID) error GetRouteSelectorFunc func() *routeselector.RouteSelector GetClientRoutesFunc func() route.HAMap GetSelectedClientRoutesFunc func() route.HAMap @@ -55,6 +57,30 @@ func (m *MockManager) TriggerSelection(networks route.HAMap) { } } +// SelectRoutes mock implementation of SelectRoutes from Manager interface +func (m *MockManager) SelectRoutes(ids []route.NetID, appendRoute bool) error { + if m.SelectRoutesFunc != nil { + return m.SelectRoutesFunc(ids, appendRoute) + } + return nil +} + +// DeselectRoutes mock implementation of DeselectRoutes from Manager interface +func (m *MockManager) DeselectRoutes(ids []route.NetID) error { + if m.DeselectRoutesFunc != nil { + return m.DeselectRoutesFunc(ids) + } + return nil +} + +// SelectAllRoutes mock implementation of SelectAllRoutes from Manager interface +func (m *MockManager) SelectAllRoutes() { +} + +// DeselectAllRoutes mock implementation of DeselectAllRoutes from Manager interface +func (m *MockManager) DeselectAllRoutes() { +} + // GetRouteSelector mock implementation of GetRouteSelector from Manager interface func (m *MockManager) GetRouteSelector() *routeselector.RouteSelector { if m.GetRouteSelectorFunc != nil { diff --git a/client/internal/routemanager/selection.go b/client/internal/routemanager/selection.go new file mode 100644 index 000000000..8320c2ac5 --- /dev/null +++ b/client/internal/routemanager/selection.go @@ -0,0 +1,120 @@ +package routemanager + +import ( + "fmt" + "slices" + + log "github.com/sirupsen/logrus" + "golang.org/x/exp/maps" + + "github.com/netbirdio/netbird/route" +) + +// SelectRoutes selects the routes with the given network IDs and applies the +// new selection. V4/v6 exit-node pairs are expanded automatically. Exit nodes +// are mutually exclusive: if the selection activates an exit node, every other +// available exit node is deselected so two can't be active at once. With +// appendRoute=false the previous selection is replaced instead of extended. +func (m *DefaultManager) SelectRoutes(ids []route.NetID, appendRoute bool) error { + if err := m.selectRoutes(ids, appendRoute); err != nil { + return err + } + m.TriggerSelection(m.GetClientRoutes()) + return nil +} + +// DeselectRoutes removes the routes with the given network IDs from the +// selection and applies the change. V4/v6 exit-node pairs are expanded +// automatically. +func (m *DefaultManager) DeselectRoutes(ids []route.NetID) error { + if err := m.deselectRoutes(ids); err != nil { + return err + } + m.TriggerSelection(m.GetClientRoutes()) + return nil +} + +func (m *DefaultManager) deselectRoutes(ids []route.NetID) error { + routesMap := m.GetClientRoutesWithNetID() + routes := route.ExpandV6ExitPairs(slices.Clone(ids), routesMap) + + log.Debugf("deselecting routes with ids: %v", routes) + + if err := m.routeSelector.DeselectRoutes(routes, maps.Keys(routesMap)); err != nil { + return fmt.Errorf("deselect routes: %w", err) + } + + return nil +} + +// SelectAllRoutes selects every available route and applies the selection. +func (m *DefaultManager) SelectAllRoutes() { + m.routeSelector.SelectAllRoutes() + m.TriggerSelection(m.GetClientRoutes()) +} + +// DeselectAllRoutes deselects every route and applies the change. +func (m *DefaultManager) DeselectAllRoutes() { + m.routeSelector.DeselectAllRoutes() + m.TriggerSelection(m.GetClientRoutes()) +} + +func (m *DefaultManager) selectRoutes(ids []route.NetID, appendRoute bool) error { + routesMap := m.GetClientRoutesWithNetID() + routes := route.ExpandV6ExitPairs(slices.Clone(ids), routesMap) + allIDs := maps.Keys(routesMap) + + log.Debugf("selecting routes with ids: %v", routes) + + if err := m.routeSelector.SelectRoutes(routes, appendRoute, allIDs); err != nil { + return fmt.Errorf("select routes: %w", err) + } + + // Exit nodes are mutually exclusive: if this selection activates an + // exit node, deselect every other available exit node so two can't be + // selected at once. Non-exit route selections are left untouched. + if requestActivatesExitNode(routes, routesMap) { + if others := otherExitNodeIDs(routesMap, routes); len(others) > 0 { + if err := m.routeSelector.DeselectRoutes(others, allIDs); err != nil { + return fmt.Errorf("deselect sibling exit nodes: %w", err) + } + } + } + + return nil +} + +func isExitNodeRoutes(routes []*route.Route) bool { + return len(routes) > 0 && (route.IsV4DefaultRoute(routes[0].Network) || route.IsV6DefaultRoute(routes[0].Network)) +} + +// requestActivatesExitNode reports whether any requested NetID maps to an exit +// node (default route) in the current route table. +func requestActivatesExitNode(requested []route.NetID, routesMap map[route.NetID][]*route.Route) bool { + for _, id := range requested { + if isExitNodeRoutes(routesMap[id]) { + return true + } + } + return false +} + +// otherExitNodeIDs returns every available exit-node NetID that is not in the +// requested set — the siblings to deselect so a single exit node stays active. +func otherExitNodeIDs(routesMap map[route.NetID][]*route.Route, requested []route.NetID) []route.NetID { + keep := make(map[route.NetID]struct{}, len(requested)) + for _, id := range requested { + keep[id] = struct{}{} + } + var others []route.NetID + for id, routes := range routesMap { + if !isExitNodeRoutes(routes) { + continue + } + if _, ok := keep[id]; ok { + continue + } + others = append(others, id) + } + return others +} diff --git a/client/internal/routemanager/selection_test.go b/client/internal/routemanager/selection_test.go new file mode 100644 index 000000000..f76ecce80 --- /dev/null +++ b/client/internal/routemanager/selection_test.go @@ -0,0 +1,82 @@ +package routemanager + +import ( + "net/netip" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/client/internal/routeselector" + "github.com/netbirdio/netbird/route" +) + +func v6ExitRoute(netID, peer string) *route.Route { + return &route.Route{ + NetID: route.NetID(netID), + Network: netip.MustParsePrefix("::/0"), + Peer: peer, + } +} + +func newSelectionTestManager() *DefaultManager { + return &DefaultManager{ + routeSelector: routeselector.NewRouteSelector(), + clientRoutes: route.HAMap{ + "exitA|0.0.0.0/0": {exitRoute("exitA", "p1", true)}, + "exitA-v6|::/0": {v6ExitRoute("exitA-v6", "p1")}, + "exitB|0.0.0.0/0": {exitRoute("exitB", "p2", true)}, + "lan|192.168.1.0/24": {{NetID: "lan", Network: netip.MustParsePrefix("192.168.1.0/24"), Peer: "p3"}}, + }, + } +} + +func TestSelectRoutes_ExitNodeExclusivity(t *testing.T) { + m := newSelectionTestManager() + + // Selecting an exit node selects its v6 pair and deselects the sibling. + require.NoError(t, m.selectRoutes([]route.NetID{"exitA"}, true)) + assert.True(t, m.routeSelector.IsSelected("exitA"), "exitA should be selected") + assert.True(t, m.routeSelector.IsSelected("exitA-v6"), "the v6 pair follows its v4 base") + assert.False(t, m.routeSelector.IsSelected("exitB"), "the sibling exit node must be deselected") + + // Switching to the sibling deselects the previous exit node and its v6 pair. + require.NoError(t, m.selectRoutes([]route.NetID{"exitB"}, true)) + assert.True(t, m.routeSelector.IsSelected("exitB"), "exitB should now be selected") + assert.False(t, m.routeSelector.IsSelected("exitA"), "the previous exit node must be deselected") + assert.False(t, m.routeSelector.IsSelected("exitA-v6"), "the previous exit node's v6 pair must be deselected") + assert.True(t, m.routeSelector.IsSelected("lan"), "non-exit route selection is untouched") + + // Selecting a non-exit route leaves the active exit node alone. + require.NoError(t, m.selectRoutes([]route.NetID{"lan"}, true)) + assert.True(t, m.routeSelector.IsSelected("exitB"), "selecting a non-exit route keeps the exit node") + + // Deselecting the active exit node turns every exit node off. + require.NoError(t, m.deselectRoutes([]route.NetID{"exitB"})) + assert.False(t, m.routeSelector.IsSelected("exitB"), "exitB should be deselected") + assert.False(t, m.routeSelector.IsSelected("exitA"), "exitA stays deselected") + assert.True(t, m.routeSelector.IsSelected("lan"), "non-exit route selection is untouched") +} + +func TestSelectRoutes_UnknownRoute(t *testing.T) { + m := newSelectionTestManager() + + assert.Error(t, m.selectRoutes([]route.NetID{"missing"}, true), "selecting an unavailable route must fail") + assert.Error(t, m.deselectRoutes([]route.NetID{"missing"}), "deselecting an unavailable route must fail") +} + +func TestExitNodeSelectionHelpers(t *testing.T) { + routesMap := map[route.NetID][]*route.Route{ + "exitA": {{Network: netip.MustParsePrefix("0.0.0.0/0")}}, + "exitB": {{Network: netip.MustParsePrefix("::/0")}}, + "lan": {{Network: netip.MustParsePrefix("192.168.0.0/16")}}, + } + + assert.True(t, requestActivatesExitNode([]route.NetID{"exitA"}, routesMap), "v4 default route is an exit node") + assert.True(t, requestActivatesExitNode([]route.NetID{"exitB"}, routesMap), "v6 default route is an exit node") + assert.False(t, requestActivatesExitNode([]route.NetID{"lan"}, routesMap), "lan route is not an exit node") + assert.False(t, requestActivatesExitNode([]route.NetID{"missing"}, routesMap), "unknown id is not an exit node") + + others := otherExitNodeIDs(routesMap, []route.NetID{"exitB"}) + assert.ElementsMatch(t, []route.NetID{"exitA"}, others, "only the other exit node is a sibling; the lan route is ignored") +} diff --git a/client/ios/NetBirdSDK/client.go b/client/ios/NetBirdSDK/client.go index a2f123900..9289a3910 100644 --- a/client/ios/NetBirdSDK/client.go +++ b/client/ios/NetBirdSDK/client.go @@ -13,7 +13,6 @@ import ( "time" log "github.com/sirupsen/logrus" - "golang.org/x/exp/maps" "github.com/netbirdio/netbird/client/internal" "github.com/netbirdio/netbird/client/internal/auth" @@ -637,23 +636,18 @@ func (c *Client) SelectRoute(id string) error { } routeManager := engine.GetRouteManager() - routeSelector := routeManager.GetRouteSelector() if id == "All" { log.Debugf("select all routes") - routeSelector.SelectAllRoutes() - } else { - log.Debugf("select route with id: %s", id) - routes := toNetIDs([]string{id}) - routesMap := routeManager.GetClientRoutesWithNetID() - routes = route.ExpandV6ExitPairs(routes, routesMap) - if err := routeSelector.SelectRoutes(routes, true, maps.Keys(routesMap)); err != nil { - log.Debugf("error when selecting routes: %s", err) - return fmt.Errorf("select routes: %w", err) - } + routeManager.SelectAllRoutes() + return nil } - routeManager.TriggerSelection(routeManager.GetClientRoutes()) - return nil + log.Debugf("select route with id: %s", id) + if err := routeManager.SelectRoutes(toNetIDs([]string{id}), true); err != nil { + log.Debugf("error when selecting routes: %s", err) + return err + } + return nil } func (c *Client) DeselectRoute(id string) error { @@ -667,21 +661,17 @@ func (c *Client) DeselectRoute(id string) error { } routeManager := engine.GetRouteManager() - routeSelector := routeManager.GetRouteSelector() if id == "All" { log.Debugf("deselect all routes") - routeSelector.DeselectAllRoutes() - } else { - log.Debugf("deselect route with id: %s", id) - routes := toNetIDs([]string{id}) - routesMap := routeManager.GetClientRoutesWithNetID() - routes = route.ExpandV6ExitPairs(routes, routesMap) - if err := routeSelector.DeselectRoutes(routes, maps.Keys(routesMap)); err != nil { - log.Debugf("error when deselecting routes: %s", err) - return fmt.Errorf("deselect routes: %w", err) - } + routeManager.DeselectAllRoutes() + return nil + } + + log.Debugf("deselect route with id: %s", id) + if err := routeManager.DeselectRoutes(toNetIDs([]string{id})); err != nil { + log.Debugf("error when deselecting routes: %s", err) + return err } - routeManager.TriggerSelection(routeManager.GetClientRoutes()) return nil } diff --git a/client/server/network.go b/client/server/network.go index c38715256..c390b8180 100644 --- a/client/server/network.go +++ b/client/server/network.go @@ -8,7 +8,6 @@ import ( "sort" "strings" - "golang.org/x/exp/maps" "google.golang.org/grpc/codes" gstatus "google.golang.org/grpc/status" @@ -161,30 +160,11 @@ func (s *Server) SelectNetworks(_ context.Context, req *proto.SelectNetworksRequ return nil, fmt.Errorf("no route manager") } - routeSelector := routeManager.GetRouteSelector() if req.GetAll() { - routeSelector.SelectAllRoutes() - } else { - routes := toNetIDs(req.GetNetworkIDs()) - routesMap := routeManager.GetClientRoutesWithNetID() - routes = route.ExpandV6ExitPairs(routes, routesMap) - netIdRoutes := maps.Keys(routesMap) - if err := routeSelector.SelectRoutes(routes, req.GetAppend(), netIdRoutes); err != nil { - return nil, fmt.Errorf("select routes: %w", err) - } - - // Exit nodes are mutually exclusive: if this selection activates an - // exit node, deselect every other available exit node so two can't be - // selected at once. Non-exit route selections are left untouched. - if requestActivatesExitNode(routes, routesMap) { - if others := otherExitNodeIDs(routesMap, routes); len(others) > 0 { - if err := routeSelector.DeselectRoutes(others, netIdRoutes); err != nil { - return nil, fmt.Errorf("deselect sibling exit nodes: %w", err) - } - } - } + routeManager.SelectAllRoutes() + } else if err := routeManager.SelectRoutes(toNetIDs(req.GetNetworkIDs()), req.GetAppend()); err != nil { + return nil, err } - routeManager.TriggerSelection(routeManager.GetClientRoutes()) s.statusRecorder.PublishEvent( proto.SystemEvent_INFO, @@ -224,19 +204,11 @@ func (s *Server) DeselectNetworks(_ context.Context, req *proto.SelectNetworksRe return nil, fmt.Errorf("no route manager") } - routeSelector := routeManager.GetRouteSelector() if req.GetAll() { - routeSelector.DeselectAllRoutes() - } else { - routes := toNetIDs(req.GetNetworkIDs()) - routesMap := routeManager.GetClientRoutesWithNetID() - routes = route.ExpandV6ExitPairs(routes, routesMap) - netIdRoutes := maps.Keys(routesMap) - if err := routeSelector.DeselectRoutes(routes, netIdRoutes); err != nil { - return nil, fmt.Errorf("deselect routes: %w", err) - } + routeManager.DeselectAllRoutes() + } else if err := routeManager.DeselectRoutes(toNetIDs(req.GetNetworkIDs())); err != nil { + return nil, err } - routeManager.TriggerSelection(routeManager.GetClientRoutes()) s.statusRecorder.PublishEvent( proto.SystemEvent_INFO, @@ -261,37 +233,3 @@ func toNetIDs(routes []string) []route.NetID { return netIDs } -func isExitNodeRoutes(routes []*route.Route) bool { - return len(routes) > 0 && (route.IsV4DefaultRoute(routes[0].Network) || route.IsV6DefaultRoute(routes[0].Network)) -} - -// requestActivatesExitNode reports whether any requested NetID maps to an exit -// node (default route) in the current route table. -func requestActivatesExitNode(requested []route.NetID, routesMap map[route.NetID][]*route.Route) bool { - for _, id := range requested { - if isExitNodeRoutes(routesMap[id]) { - return true - } - } - return false -} - -// otherExitNodeIDs returns every available exit-node NetID that is not in the -// requested set — the siblings to deselect so a single exit node stays active. -func otherExitNodeIDs(routesMap map[route.NetID][]*route.Route, requested []route.NetID) []route.NetID { - keep := make(map[route.NetID]struct{}, len(requested)) - for _, id := range requested { - keep[id] = struct{}{} - } - var others []route.NetID - for id, routes := range routesMap { - if !isExitNodeRoutes(routes) { - continue - } - if _, ok := keep[id]; ok { - continue - } - others = append(others, id) - } - return others -} diff --git a/client/server/network_exitnode_test.go b/client/server/network_exitnode_test.go deleted file mode 100644 index 1c0ba0ecb..000000000 --- a/client/server/network_exitnode_test.go +++ /dev/null @@ -1,26 +0,0 @@ -package server - -import ( - "net/netip" - "testing" - - "github.com/stretchr/testify/assert" - - "github.com/netbirdio/netbird/route" -) - -func TestExitNodeSelectionHelpers(t *testing.T) { - routesMap := map[route.NetID][]*route.Route{ - "exitA": {{Network: netip.MustParsePrefix("0.0.0.0/0")}}, - "exitB": {{Network: netip.MustParsePrefix("::/0")}}, - "lan": {{Network: netip.MustParsePrefix("192.168.0.0/16")}}, - } - - assert.True(t, requestActivatesExitNode([]route.NetID{"exitA"}, routesMap), "v4 default route is an exit node") - assert.True(t, requestActivatesExitNode([]route.NetID{"exitB"}, routesMap), "v6 default route is an exit node") - assert.False(t, requestActivatesExitNode([]route.NetID{"lan"}, routesMap), "lan route is not an exit node") - assert.False(t, requestActivatesExitNode([]route.NetID{"missing"}, routesMap), "unknown id is not an exit node") - - others := otherExitNodeIDs(routesMap, []route.NetID{"exitB"}) - assert.ElementsMatch(t, []route.NetID{"exitA"}, others, "only the other exit node is a sibling; the lan route is ignored") -}