Revert "hacky all-operating-systems solution"

This reverts commit 75fac258e743f95f9f2187d1cf7670be18012ec7.

client/internal/routemanager/systemops_nonandroid.go

@@ -6,9 +6,26 @@ import (
 	"fmt"
 	"net"
 	"net/netip"
+	"syscall"
 
 	"github.com/libp2p/go-netroute"
 	log "github.com/sirupsen/logrus"
+	"golang.org/x/net/route"
+)
+
+// selected BSD Route flags.
+const (
+	RTF_UP        = 0x1
+	RTF_GATEWAY   = 0x2
+	RTF_HOST      = 0x4
+	RTF_REJECT    = 0x8
+	RTF_DYNAMIC   = 0x10
+	RTF_MODIFIED  = 0x20
+	RTF_STATIC    = 0x800
+	RTF_BLACKHOLE = 0x1000
+	RTF_LOCAL     = 0x200000
+	RTF_BROADCAST = 0x400000
+	RTF_MULTICAST = 0x800000
 )
 
 var errRouteNotFound = fmt.Errorf("route not found")
@@ -25,16 +42,7 @@ func addToRouteTableIfNoExists(prefix netip.Prefix, addr string) error {
 		return nil
 	}
 
-	ok, err := routeExistsWithDifferentGateway(prefix, addr)
+	ok, err := existsInRouteTable(prefix)
-	if err != nil {
-		return err
-	}
-	if ok {
-		log.Warnf("skipping adding a new route for network %s because it already exists", prefix)
-		return nil
-	}
-
-	ok, err = routeExistsWithSameGateway(prefix, addr)
 	if err != nil {
 		return err
 	}
@@ -64,53 +72,65 @@ func getExistingRIBRouteGateway(prefix netip.Prefix) (net.IP, error) {
 	if err != nil {
 		return nil, err
 	}
-	_, gateway, preferredSrc, err := r.Route(prefix.Addr().AsSlice())
+	_, gateway, _, err := r.Route(prefix.Addr().AsSlice())
 	if err != nil {
 		log.Errorf("getting routes returned an error: %v", err)
 		return nil, errRouteNotFound
 	}
 
-	if gateway == nil {
-		return preferredSrc, nil
-	}
-
 	return gateway, nil
 }
 
-func routeExistsWithSameGateway(prefix netip.Prefix, addr string) (bool, error) {
+func existsInRouteTable(prefix netip.Prefix) (bool, error) {
-	r, err := netroute.New()
+	tab, err := route.FetchRIB(syscall.AF_UNSPEC, route.RIBTypeRoute, 0)
 	if err != nil {
-		return true, err
+		return false, err
+	}
+	msgs, err := route.ParseRIB(route.RIBTypeRoute, tab)
+	if err != nil {
+		return false, err
 	}
 
-	_, gateway, preferredSrc, err := r.Route(prefix.Addr().AsSlice())
+	for _, msg := range msgs {
-	if err != nil {
+		m := msg.(*route.RouteMessage)
-		log.Errorf("getting routes returned an error: %v", err)
-		return true, errRouteNotFound
-	}
 
-	if gateway.String() == addr || preferredSrc.String() == addr {
+		if m.Version < 3 || m.Version > 5 {
-		return true, nil
+			return false, fmt.Errorf("unexpected RIB message version: %d", m.Version)
+		}
+		if m.Type != 4 /* RTM_GET */ {
+			return true, fmt.Errorf("unexpected RIB message type: %d", m.Type)
+		}
+
+		if m.Flags&RTF_UP == 0 ||
+			m.Flags&(RTF_REJECT|RTF_BLACKHOLE) != 0 {
+			continue
+		}
+
+		dst, err := toIPAddr(m.Addrs[0])
+		if err != nil {
+			return true, fmt.Errorf("unexpected RIB destination: %v", err)
+		}
+
+		mask, _ := toIPAddr(m.Addrs[2])
+		cidr, _ := net.IPMask(mask.To4()).Size()
+		if dst.String() == prefix.Addr().String() && cidr == prefix.Bits() {
+			return true, nil
+		}
 	}
 
 	return false, nil
 }
 
-func routeExistsWithDifferentGateway(prefix netip.Prefix, addr string) (bool, error) {
+func toIPAddr(a route.Addr) (net.IP, error) {
-	r, err := netroute.New()
+	switch t := a.(type) {
-	if err != nil {
+	case *route.Inet4Addr:
-		return true, err
+		ip := net.IPv4(t.IP[0], t.IP[1], t.IP[2], t.IP[3])
+		return ip, nil
+	case *route.Inet6Addr:
+		ip := make(net.IP, net.IPv6len)
+		copy(ip, t.IP[:])
+		return ip, nil
+	default:
+		return net.IP{}, fmt.Errorf("unknown family: %v", t)
 	}
-
-	_, gateway, preferredSrc, err := r.Route(prefix.Addr().AsSlice())
-	if err != nil {
-		log.Errorf("getting routes returned an error: %v", err)
-		return true, errRouteNotFound
-	}
-
-	if gateway.String() != addr && preferredSrc.String() != addr {
-		return false, nil
-	}
-
-	return true, nil
 }

client/internal/routemanager/systemops_nonandroid_test.go

@@ -86,59 +86,43 @@ func TestAddExistAndRemoveRoute(t *testing.T) {
 		t.Fatal("shouldn't return error when fetching the gateway: ", err)
 	}
 	testCases := []struct {
-		name               string
+		name              string
-		prefix             netip.Prefix
+		prefix            netip.Prefix
-		gateway            string
+		preExistingPrefix netip.Prefix
-		preExistingPrefix  netip.Prefix
+		shouldAddRoute    bool
-		preExistingGateway string
-		shouldAddRoute     bool
 	}{
 		{
 			name:           "Should Add And Remove random Route",
 			prefix:         netip.MustParsePrefix("99.99.99.99/32"),
-			gateway:        "127.0.0.1",
 			shouldAddRoute: true,
 		},
 		{
 			name:           "Should Not Add Route if overlaps with default gateway",
 			prefix:         netip.MustParsePrefix(defaultGateway.String() + "/31"),
-			gateway:        "127.0.0.1",
 			shouldAddRoute: false,
 		},
 		{
-			name:               "Should Add Route if bigger network exists",
+			name:              "Should Add Route if bigger network exists",
-			prefix:             netip.MustParsePrefix("100.100.100.0/24"),
+			prefix:            netip.MustParsePrefix("100.100.100.0/24"),
-			gateway:            "127.0.0.1",
+			preExistingPrefix: netip.MustParsePrefix("100.100.0.0/16"),
-			preExistingPrefix:  netip.MustParsePrefix("100.100.0.0/16"),
+			shouldAddRoute:    true,
-			preExistingGateway: "127.0.0.1",
-			shouldAddRoute:     true,
 		},
 		{
-			name:               "Should Add Route if smaller network exists",
+			name:              "Should Add Route if smaller network exists",
-			prefix:             netip.MustParsePrefix("100.100.0.0/16"),
+			prefix:            netip.MustParsePrefix("100.100.0.0/16"),
-			gateway:            "127.0.0.1",
+			preExistingPrefix: netip.MustParsePrefix("100.100.100.0/24"),
-			preExistingPrefix:  netip.MustParsePrefix("100.100.100.0/24"),
+			shouldAddRoute:    true,
-			preExistingGateway: "127.0.0.1",
-			shouldAddRoute:     true,
 		},
 		{
-			name:               "Should Not Add Route if same network with same gateway exists",
+			name:              "Should Not Add Route if same network exists",
-			prefix:             netip.MustParsePrefix("100.100.0.0/16"),
+			prefix:            netip.MustParsePrefix("100.100.0.0/16"),
-			gateway:            "127.0.0.1",
+			preExistingPrefix: netip.MustParsePrefix("100.100.0.0/16"),
-			preExistingPrefix:  netip.MustParsePrefix("100.100.0.0/16"),
+			shouldAddRoute:    false,
-			preExistingGateway: "127.0.0.1",
-			shouldAddRoute:     false,
-		},
-		{
-			name:               "Should Not Add Route if same network with different gateway exists",
-			prefix:             netip.MustParsePrefix("100.100.0.0/16"),
-			gateway:            "127.0.0.1",
-			preExistingPrefix:  netip.MustParsePrefix("100.100.0.0/16"),
-			preExistingGateway: "123.0.0.1",
-			shouldAddRoute:     false,
 		},
 	}
 
+	MOCK_ADDR := "127.0.0.1"
+
 	for _, testCase := range testCases {
 		var buf bytes.Buffer
 		log.SetOutput(&buf)
@@ -148,28 +132,28 @@ func TestAddExistAndRemoveRoute(t *testing.T) {
 		t.Run(testCase.name, func(t *testing.T) {
 			// Prepare the environment
 			if testCase.preExistingPrefix.IsValid() {
-				err := addToRouteTableIfNoExists(testCase.preExistingPrefix, testCase.preExistingGateway)
+				err := addToRouteTableIfNoExists(testCase.preExistingPrefix, MOCK_ADDR)
 				require.NoError(t, err, "should not return err when adding pre-existing route")
 			}
 
 			// Add the route
-			err = addToRouteTableIfNoExists(testCase.prefix, testCase.gateway)
+			err = addToRouteTableIfNoExists(testCase.prefix, MOCK_ADDR)
 			require.NoError(t, err, "should not return err when adding pre-existing route")
 
 			if testCase.shouldAddRoute {
 				// test if route exists after adding
-				ok, err := routeExistsWithSameGateway(testCase.prefix, testCase.gateway)
+				ok, err := existsInRouteTable(testCase.prefix)
 				require.NoError(t, err, "should not return err")
 				require.True(t, ok, "route should exist")
 
 				// remove route again if added
-				err = removeFromRouteTableIfNonSystem(testCase.prefix, testCase.gateway)
+				err = removeFromRouteTableIfNonSystem(testCase.prefix, MOCK_ADDR)
 				require.NoError(t, err, "should not return err")
 			}
 
 			// route should either not have been added or should have been removed
 			// In case of already existing route, it should not have been added (but still exist)
-			ok, err := routeExistsWithSameGateway(testCase.prefix, testCase.gateway)
+			ok, err := existsInRouteTable(testCase.prefix)
 			fmt.Println("Buffer string: ", buf.String())
 			require.NoError(t, err, "should not return err")
 			if !strings.Contains(buf.String(), "because it already exists") {