HA Network Routes: prevent routing directly-accessible networks throu…

…gh VPN interface (#612) Prevent routing peer to add routes from the same HA group as client routes
netbirdio · Dec 8, 2022 · 1204bbd · 1204bbd
1 parent 0be46c0
commit 1204bbd
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 11 deletions.
diff --git a/client/internal/routemanager/client.go b/client/internal/routemanager/client.go
@@ -3,12 +3,13 @@ package routemanager
 import (
 	"context"
 	"fmt"
+	"net/netip"
+
 	"github.com/netbirdio/netbird/client/internal/peer"
 	"github.com/netbirdio/netbird/client/status"
 	"github.com/netbirdio/netbird/iface"
 	"github.com/netbirdio/netbird/route"
 	log "github.com/sirupsen/logrus"
-	"net/netip"
 )
 
 type routerPeerStatus struct {
@@ -52,7 +53,7 @@ func newClientNetworkWatcher(ctx context.Context, wgInterface *iface.WGIface, st
 	return client
 }
 
-func getClientNetworkID(input *route.Route) string {
+func getHANetworkID(input *route.Route) string {
 	return input.NetID + "-" + input.Network.String()
 }
 

diff --git a/client/internal/routemanager/manager.go b/client/internal/routemanager/manager.go
@@ -3,13 +3,14 @@ package routemanager
 import (
 	"context"
 	"fmt"
+	"runtime"
+	"sync"
+
 	"github.com/netbirdio/netbird/client/status"
 	"github.com/netbirdio/netbird/client/system"
 	"github.com/netbirdio/netbird/iface"
 	"github.com/netbirdio/netbird/route"
 	log "github.com/sirupsen/logrus"
-	"runtime"
-	"sync"
 )
 
 // Manager is a route manager interface
@@ -147,25 +148,32 @@ func (m *DefaultManager) UpdateRoutes(updateSerial uint64, newRoutes []*route.Ro
 
 		newClientRoutesIDMap := make(map[string][]*route.Route)
 		newServerRoutesMap := make(map[string]*route.Route)
+		ownNetworkIDs := make(map[string]bool)
 
 		for _, newRoute := range newRoutes {
-			// only linux is supported for now
+			networkID := getHANetworkID(newRoute)
 			if newRoute.Peer == m.pubKey {
+				ownNetworkIDs[networkID] = true
+				// only linux is supported for now
 				if runtime.GOOS != "linux" {
 					log.Warnf("received a route to manage, but agent doesn't support router mode on %s OS", runtime.GOOS)
 					continue
 				}
 				newServerRoutesMap[newRoute.ID] = newRoute
-			} else {
+			}
+		}
+
+		for _, newRoute := range newRoutes {
+			networkID := getHANetworkID(newRoute)
+			if !ownNetworkIDs[networkID] {
 				// if prefix is too small, lets assume is a possible default route which is not yet supported
 				// we skip this route management
 				if newRoute.Network.Bits() < 7 {
 					log.Errorf("this agent version: %s, doesn't support default routes, received %s, skiping this route",
 						system.NetbirdVersion(), newRoute.Network)
 					continue
 				}
-				clientNetworkID := getClientNetworkID(newRoute)
-				newClientRoutesIDMap[clientNetworkID] = append(newClientRoutesIDMap[clientNetworkID], newRoute)
+				newClientRoutesIDMap[networkID] = append(newClientRoutesIDMap[networkID], newRoute)
 			}
 		}
 

diff --git a/client/internal/routemanager/manager_test.go b/client/internal/routemanager/manager_test.go
@@ -3,13 +3,14 @@ package routemanager
 import (
 	"context"
 	"fmt"
+	"net/netip"
+	"runtime"
+	"testing"
+
 	"github.com/netbirdio/netbird/client/status"
 	"github.com/netbirdio/netbird/iface"
 	"github.com/netbirdio/netbird/route"
 	"github.com/stretchr/testify/require"
-	"net/netip"
-	"runtime"
-	"testing"
 )
 
 // send 5 routes, one for server and 4 for clients, one normal and 2 HA and one small
@@ -336,6 +337,55 @@ func TestManagerUpdateRoutes(t *testing.T) {
 			serverRoutesExpected:          0,
 			clientNetworkWatchersExpected: 0,
 		},
+		{
+			name: "HA server should not register routes from the same HA group",
+			inputRoutes: []*route.Route{
+				{
+					ID:          "l1",
+					NetID:       "routeA",
+					Peer:        localPeerKey,
+					Network:     netip.MustParsePrefix("100.64.251.250/30"),
+					NetworkType: route.IPv4Network,
+					Metric:      9999,
+					Masquerade:  false,
+					Enabled:     true,
+				},
+				{
+					ID:          "l2",
+					NetID:       "routeA",
+					Peer:        localPeerKey,
+					Network:     netip.MustParsePrefix("8.8.9.8/32"),
+					NetworkType: route.IPv4Network,
+					Metric:      9999,
+					Masquerade:  false,
+					Enabled:     true,
+				},
+				{
+					ID:          "r1",
+					NetID:       "routeA",
+					Peer:        remotePeerKey1,
+					Network:     netip.MustParsePrefix("100.64.251.250/30"),
+					NetworkType: route.IPv4Network,
+					Metric:      9999,
+					Masquerade:  false,
+					Enabled:     true,
+				},
+				{
+					ID:          "r2",
+					NetID:       "routeC",
+					Peer:        remotePeerKey1,
+					Network:     netip.MustParsePrefix("8.8.9.9/32"),
+					NetworkType: route.IPv4Network,
+					Metric:      9999,
+					Masquerade:  false,
+					Enabled:     true,
+				},
+			},
+			inputSerial:                   1,
+			shouldCheckServerRoutes:       runtime.GOOS == "linux",
+			serverRoutesExpected:          2,
+			clientNetworkWatchersExpected: 1,
+		},
 	}
 
 	for n, testCase := range testCases {