diff --git a/network/route.go b/network/route.go index 175f7b0..844e3c3 100644 --- a/network/route.go +++ b/network/route.go @@ -119,7 +119,7 @@ func LinuxAddRoute(destination string, gateway string, interfaceName string) err return nil } -func LinuxRemoveRoute(destination string) error { +func LinuxRemoveRoute(destination string, interfaceName string) error { if runtime.GOOS != "linux" { return nil } @@ -130,12 +130,24 @@ func LinuxRemoveRoute(destination string) error { return fmt.Errorf("invalid destination address: %v", err) } - // Create route to delete + // Create route to delete. LinkIndex and Priority are set to match the + // route we added exactly, so this only ever deletes the route we own - + // a local/native route to the same destination on a different + // interface (or with a different metric) must never be touched. route := &netlink.Route{ - Dst: ipNet, + Dst: ipNet, + Priority: VPNRouteMetric, } - logger.Info("Removing route to %s", destination) + if interfaceName != "" { + link, err := netlink.LinkByName(interfaceName) + if err != nil { + return fmt.Errorf("failed to get interface %s: %v", interfaceName, err) + } + route.LinkIndex = link.Attrs().Index + } + + logger.Info("Removing route to %s via interface %s", destination, interfaceName) // Delete the route if err := netlink.RouteDel(route); err != nil { @@ -178,9 +190,9 @@ func RemoveRouteForServerIP(serverIP string, interfaceName string) error { return DarwinRemoveRoute(serverIP) } // else if runtime.GOOS == "windows" { - // return WindowsRemoveRoute(serverIP) + // return WindowsRemoveRoute(serverIP, interfaceName) // } else if runtime.GOOS == "linux" { - // return LinuxRemoveRoute(serverIP) + // return LinuxRemoveRoute(serverIP, interfaceName) // } return nil } @@ -263,8 +275,11 @@ func AddRoutes(remoteSubnets []string, interfaceName string) error { return nil } -// removeRoutesForRemoteSubnets removes routes for each subnet in RemoteSubnets -func RemoveRoutes(remoteSubnets []string) error { +// removeRoutesForRemoteSubnets removes routes for each subnet in RemoteSubnets. +// interfaceName must match the interface the routes were added on (see +// AddRoutes) so that only the routes we own are deleted, never an unrelated +// local/native route to the same destination on another interface. +func RemoveRoutes(remoteSubnets []string, interfaceName string) error { if len(remoteSubnets) == 0 { return nil } @@ -288,11 +303,11 @@ func RemoveRoutes(remoteSubnets []string) error { logger.Error("Failed to remove Darwin route for subnet %s: %v", subnet, err) } case "windows": - if err := WindowsRemoveRoute(subnet); err != nil { + if err := WindowsRemoveRoute(subnet, interfaceName); err != nil { logger.Error("Failed to remove Windows route for subnet %s: %v", subnet, err) } case "linux": - if err := LinuxRemoveRoute(subnet); err != nil { + if err := LinuxRemoveRoute(subnet, interfaceName); err != nil { logger.Error("Failed to remove Linux route for subnet %s: %v", subnet, err) } case "android", "ios": diff --git a/network/route_notwindows.go b/network/route_notwindows.go index 6984c71..1214a7f 100644 --- a/network/route_notwindows.go +++ b/network/route_notwindows.go @@ -6,6 +6,6 @@ func WindowsAddRoute(destination string, gateway string, interfaceName string) e return nil } -func WindowsRemoveRoute(destination string) error { +func WindowsRemoveRoute(destination string, interfaceName string) error { return nil } diff --git a/network/route_windows.go b/network/route_windows.go index 3b62964..5bca2d1 100644 --- a/network/route_windows.go +++ b/network/route_windows.go @@ -96,7 +96,7 @@ func WindowsAddRoute(destination string, gateway string, interfaceName string) e return nil } -func WindowsRemoveRoute(destination string) error { +func WindowsRemoveRoute(destination string, interfaceName string) error { // Parse destination CIDR _, ipNet, err := net.ParseCIDR(destination) if err != nil { @@ -120,8 +120,25 @@ func WindowsRemoveRoute(destination string) error { } prefix := netip.PrefixFrom(addr, maskBits) + // Resolve the LUID of the interface we added the route on, so we only + // ever delete the route we own rather than any route matching the + // destination - a local/native route to the same destination on a + // different interface must never be touched. + var luid winipcfg.LUID + var haveLuid bool + if interfaceName != "" { + iface, err := net.InterfaceByName(interfaceName) + if err != nil { + return fmt.Errorf("failed to get interface %s: %v", interfaceName, err) + } + luid, err = winipcfg.LUIDFromIndex(uint32(iface.Index)) + if err != nil { + return fmt.Errorf("failed to get LUID for interface %s: %v", interfaceName, err) + } + haveLuid = true + } + // Get all routes and find the one to delete - // We need to get the LUID from the existing route var family winipcfg.AddressFamily if addr.Is4() { family = 2 // AF_INET @@ -134,17 +151,23 @@ func WindowsRemoveRoute(destination string) error { return fmt.Errorf("failed to get route table: %v", err) } - // Find and delete matching route + // Find and delete matching route. When we know which interface we added + // the route on, only delete the entry on that interface with our + // VPNRouteMetric so we never remove an unrelated local/native route to + // the same destination. for _, route := range routes { routePrefix := route.DestinationPrefix.Prefix() - if routePrefix == prefix { - logger.Info("Removing route to %s", destination) - err = route.Delete() - if err != nil { - return fmt.Errorf("failed to delete route: %v", err) - } - return nil + if routePrefix != prefix { + continue } + if haveLuid && (route.InterfaceLUID != luid || route.Metric != VPNRouteMetric) { + continue + } + logger.Info("Removing route to %s on interface %s", destination, interfaceName) + if err := route.Delete(); err != nil { + return fmt.Errorf("failed to delete route: %v", err) + } + return nil } return fmt.Errorf("route to %s not found", destination) diff --git a/newt/handlers.go b/newt/handlers.go index 0a448f1..55c7f4e 100644 --- a/newt/handlers.go +++ b/newt/handlers.go @@ -430,7 +430,7 @@ func (n *Newt) registerHandlers(ctx context.Context) { } if n.config.UseNativeMainInterface { - if err := network.RemoveRoutes(data.Subnets); err != nil { + if err := network.RemoveRoutes(data.Subnets, n.config.NativeMainInterfaceName); err != nil { logger.Warn("Failed to remove routes for subnets: %v", err) } } diff --git a/newt/tunnel.go b/newt/tunnel.go index b396cce..fdacbf7 100644 --- a/newt/tunnel.go +++ b/newt/tunnel.go @@ -24,7 +24,7 @@ func (n *Newt) updateRemoteExitNodeSubnets(subnets []string) { } } if len(toRemove) > 0 { - if err := network.RemoveRoutes(toRemove); err != nil { + if err := network.RemoveRoutes(toRemove, n.config.NativeMainInterfaceName); err != nil { logger.Warn("Failed to remove old subnet routes: %v", err) } } @@ -88,7 +88,7 @@ func (n *Newt) closeWgTunnel() { } toRemove = append(toRemove, n.activeRemoteSubnets...) if len(toRemove) > 0 { - if err := network.RemoveRoutes(toRemove); err != nil { + if err := network.RemoveRoutes(toRemove, n.config.NativeMainInterfaceName); err != nil { logger.Warn("Failed to remove native main tunnel routes: %v", err) } }