From 75fac258e743f95f9f2187d1cf7670be18012ec7 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 9 Jun 2023 17:40:10 +0200 Subject: [PATCH] hacky all-operating-systems solution --- .../routemanager/systemops_nonandroid.go | 102 +++++++----------- .../routemanager/systemops_nonandroid_test.go | 62 +++++++---- 2 files changed, 80 insertions(+), 84 deletions(-) diff --git a/client/internal/routemanager/systemops_nonandroid.go b/client/internal/routemanager/systemops_nonandroid.go index c9efd7882..dd2d18ad8 100644 --- a/client/internal/routemanager/systemops_nonandroid.go +++ b/client/internal/routemanager/systemops_nonandroid.go @@ -6,26 +6,9 @@ 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") @@ -42,7 +25,16 @@ func addToRouteTableIfNoExists(prefix netip.Prefix, addr string) error { return nil } - ok, err := existsInRouteTable(prefix) + ok, err := routeExistsWithDifferentGateway(prefix, addr) + 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 } @@ -72,65 +64,53 @@ func getExistingRIBRouteGateway(prefix netip.Prefix) (net.IP, error) { if err != nil { return nil, err } - _, gateway, _, err := r.Route(prefix.Addr().AsSlice()) + _, gateway, preferredSrc, 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 existsInRouteTable(prefix netip.Prefix) (bool, error) { - tab, err := route.FetchRIB(syscall.AF_UNSPEC, route.RIBTypeRoute, 0) +func routeExistsWithSameGateway(prefix netip.Prefix, addr string) (bool, error) { + r, err := netroute.New() if err != nil { - return false, err - } - msgs, err := route.ParseRIB(route.RIBTypeRoute, tab) - if err != nil { - return false, err + return true, err } - for _, msg := range msgs { - m := msg.(*route.RouteMessage) + _, gateway, preferredSrc, err := r.Route(prefix.Addr().AsSlice()) + if err != nil { + log.Errorf("getting routes returned an error: %v", err) + return true, errRouteNotFound + } - if m.Version < 3 || m.Version > 5 { - 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 - } + if gateway.String() == addr || preferredSrc.String() == addr { + return true, nil } return false, nil } -func toIPAddr(a route.Addr) (net.IP, error) { - switch t := a.(type) { - case *route.Inet4Addr: - 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) +func routeExistsWithDifferentGateway(prefix netip.Prefix, addr string) (bool, error) { + r, err := netroute.New() + if err != nil { + return true, err } + + _, 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 } diff --git a/client/internal/routemanager/systemops_nonandroid_test.go b/client/internal/routemanager/systemops_nonandroid_test.go index 2020aa8d4..1f086fe62 100644 --- a/client/internal/routemanager/systemops_nonandroid_test.go +++ b/client/internal/routemanager/systemops_nonandroid_test.go @@ -86,43 +86,59 @@ func TestAddExistAndRemoveRoute(t *testing.T) { t.Fatal("shouldn't return error when fetching the gateway: ", err) } testCases := []struct { - name string - prefix netip.Prefix - preExistingPrefix netip.Prefix - shouldAddRoute bool + name string + prefix netip.Prefix + gateway string + preExistingPrefix netip.Prefix + 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", - prefix: netip.MustParsePrefix("100.100.100.0/24"), - preExistingPrefix: netip.MustParsePrefix("100.100.0.0/16"), - shouldAddRoute: true, + name: "Should Add Route if bigger network exists", + prefix: netip.MustParsePrefix("100.100.100.0/24"), + gateway: "127.0.0.1", + preExistingPrefix: netip.MustParsePrefix("100.100.0.0/16"), + preExistingGateway: "127.0.0.1", + shouldAddRoute: true, }, { - name: "Should Add Route if smaller network exists", - prefix: netip.MustParsePrefix("100.100.0.0/16"), - preExistingPrefix: netip.MustParsePrefix("100.100.100.0/24"), - shouldAddRoute: true, + name: "Should Add Route if smaller network exists", + prefix: netip.MustParsePrefix("100.100.0.0/16"), + gateway: "127.0.0.1", + preExistingPrefix: netip.MustParsePrefix("100.100.100.0/24"), + preExistingGateway: "127.0.0.1", + shouldAddRoute: true, }, { - name: "Should Not Add Route if same network exists", - prefix: netip.MustParsePrefix("100.100.0.0/16"), - preExistingPrefix: netip.MustParsePrefix("100.100.0.0/16"), - shouldAddRoute: false, + name: "Should Not Add Route if same network with same gateway exists", + prefix: netip.MustParsePrefix("100.100.0.0/16"), + gateway: "127.0.0.1", + preExistingPrefix: netip.MustParsePrefix("100.100.0.0/16"), + 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) @@ -132,28 +148,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, MOCK_ADDR) + err := addToRouteTableIfNoExists(testCase.preExistingPrefix, testCase.preExistingGateway) require.NoError(t, err, "should not return err when adding pre-existing route") } // Add the route - err = addToRouteTableIfNoExists(testCase.prefix, MOCK_ADDR) + err = addToRouteTableIfNoExists(testCase.prefix, testCase.gateway) require.NoError(t, err, "should not return err when adding pre-existing route") if testCase.shouldAddRoute { // test if route exists after adding - ok, err := existsInRouteTable(testCase.prefix) + ok, err := routeExistsWithSameGateway(testCase.prefix, testCase.gateway) require.NoError(t, err, "should not return err") require.True(t, ok, "route should exist") // remove route again if added - err = removeFromRouteTableIfNonSystem(testCase.prefix, MOCK_ADDR) + err = removeFromRouteTableIfNonSystem(testCase.prefix, testCase.gateway) 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 := existsInRouteTable(testCase.prefix) + ok, err := routeExistsWithSameGateway(testCase.prefix, testCase.gateway) fmt.Println("Buffer string: ", buf.String()) require.NoError(t, err, "should not return err") if !strings.Contains(buf.String(), "because it already exists") {