mirror of
https://github.com/netbirdio/netbird.git
synced 2024-11-22 08:03:30 +01:00
Don't add exclusion routes for IPs that are part of connected networks (#2258)
This prevents arp/ndp issues on macOS leading to unreachability of that IP.
This commit is contained in:
parent
95d725f2c1
commit
e78ec2e985
@ -50,7 +50,7 @@ func (r *SysOps) setupRefCounter(initAddresses []net.IP) (nbnet.AddHookFunc, nbn
|
|||||||
nexthop, err := r.addRouteToNonVPNIntf(prefix, r.wgInterface, initialNexthop)
|
nexthop, err := r.addRouteToNonVPNIntf(prefix, r.wgInterface, initialNexthop)
|
||||||
if errors.Is(err, vars.ErrRouteNotAllowed) || errors.Is(err, vars.ErrRouteNotFound) {
|
if errors.Is(err, vars.ErrRouteNotAllowed) || errors.Is(err, vars.ErrRouteNotFound) {
|
||||||
log.Tracef("Adding for prefix %s: %v", prefix, err)
|
log.Tracef("Adding for prefix %s: %v", prefix, err)
|
||||||
// These errors are not critical but also we should not track and try to remove the routes either.
|
// These errors are not critical, but also we should not track and try to remove the routes either.
|
||||||
return nexthop, refcounter.ErrIgnore
|
return nexthop, refcounter.ErrIgnore
|
||||||
}
|
}
|
||||||
return nexthop, err
|
return nexthop, err
|
||||||
@ -135,6 +135,11 @@ func (r *SysOps) addRouteToNonVPNIntf(prefix netip.Prefix, vpnIntf *iface.WGIfac
|
|||||||
return Nexthop{}, vars.ErrRouteNotAllowed
|
return Nexthop{}, vars.ErrRouteNotAllowed
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Check if the prefix is part of any local subnets
|
||||||
|
if isLocal, subnet := r.isPrefixInLocalSubnets(prefix); isLocal {
|
||||||
|
return Nexthop{}, fmt.Errorf("prefix %s is part of local subnet %s: %w", prefix, subnet, vars.ErrRouteNotAllowed)
|
||||||
|
}
|
||||||
|
|
||||||
// Determine the exit interface and next hop for the prefix, so we can add a specific route
|
// Determine the exit interface and next hop for the prefix, so we can add a specific route
|
||||||
nexthop, err := GetNextHop(addr)
|
nexthop, err := GetNextHop(addr)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@ -167,6 +172,36 @@ func (r *SysOps) addRouteToNonVPNIntf(prefix netip.Prefix, vpnIntf *iface.WGIfac
|
|||||||
return exitNextHop, nil
|
return exitNextHop, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (r *SysOps) isPrefixInLocalSubnets(prefix netip.Prefix) (bool, *net.IPNet) {
|
||||||
|
localInterfaces, err := net.Interfaces()
|
||||||
|
if err != nil {
|
||||||
|
log.Errorf("Failed to get local interfaces: %v", err)
|
||||||
|
return false, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, intf := range localInterfaces {
|
||||||
|
addrs, err := intf.Addrs()
|
||||||
|
if err != nil {
|
||||||
|
log.Errorf("Failed to get addresses for interface %s: %v", intf.Name, err)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, addr := range addrs {
|
||||||
|
ipnet, ok := addr.(*net.IPNet)
|
||||||
|
if !ok {
|
||||||
|
log.Errorf("Failed to convert address to IPNet: %v", addr)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
if ipnet.Contains(prefix.Addr().AsSlice()) {
|
||||||
|
return true, ipnet
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return false, nil
|
||||||
|
}
|
||||||
|
|
||||||
// genericAddVPNRoute adds a new route to the vpn interface, it splits the default prefix
|
// genericAddVPNRoute adds a new route to the vpn interface, it splits the default prefix
|
||||||
// in two /1 prefixes to avoid replacing the existing default route
|
// in two /1 prefixes to avoid replacing the existing default route
|
||||||
func (r *SysOps) genericAddVPNRoute(prefix netip.Prefix, intf *net.Interface) error {
|
func (r *SysOps) genericAddVPNRoute(prefix netip.Prefix, intf *net.Interface) error {
|
||||||
|
@ -73,7 +73,7 @@ var testCases = []testCase{
|
|||||||
{
|
{
|
||||||
name: "To duplicate internal route without custom dialer via physical interface", // local route takes precedence
|
name: "To duplicate internal route without custom dialer via physical interface", // local route takes precedence
|
||||||
destination: "10.0.0.2:53",
|
destination: "10.0.0.2:53",
|
||||||
expectedSourceIP: "10.0.0.1",
|
expectedSourceIP: "127.0.0.1",
|
||||||
expectedDestPrefix: "10.0.0.0/8",
|
expectedDestPrefix: "10.0.0.0/8",
|
||||||
expectedNextHop: "0.0.0.0",
|
expectedNextHop: "0.0.0.0",
|
||||||
expectedInterface: "Loopback Pseudo-Interface 1",
|
expectedInterface: "Loopback Pseudo-Interface 1",
|
||||||
@ -110,7 +110,7 @@ var testCases = []testCase{
|
|||||||
{
|
{
|
||||||
name: "To more specific route (local) without custom dialer via physical interface",
|
name: "To more specific route (local) without custom dialer via physical interface",
|
||||||
destination: "127.0.10.2:53",
|
destination: "127.0.10.2:53",
|
||||||
expectedSourceIP: "10.0.0.1",
|
expectedSourceIP: "127.0.0.1",
|
||||||
expectedDestPrefix: "127.0.0.0/8",
|
expectedDestPrefix: "127.0.0.0/8",
|
||||||
expectedNextHop: "0.0.0.0",
|
expectedNextHop: "0.0.0.0",
|
||||||
expectedInterface: "Loopback Pseudo-Interface 1",
|
expectedInterface: "Loopback Pseudo-Interface 1",
|
||||||
@ -181,31 +181,6 @@ func testRoute(t *testing.T, destination string, dialer dialer) *FindNetRouteOut
|
|||||||
return combinedOutput
|
return combinedOutput
|
||||||
}
|
}
|
||||||
|
|
||||||
func createAndSetupDummyInterface(t *testing.T, interfaceName, ipAddressCIDR string) string {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
ip, ipNet, err := net.ParseCIDR(ipAddressCIDR)
|
|
||||||
require.NoError(t, err)
|
|
||||||
subnetMaskSize, _ := ipNet.Mask.Size()
|
|
||||||
script := fmt.Sprintf(`New-NetIPAddress -InterfaceAlias "%s" -IPAddress "%s" -PrefixLength %d -PolicyStore ActiveStore -Confirm:$False`, interfaceName, ip.String(), subnetMaskSize)
|
|
||||||
_, err = exec.Command("powershell", "-Command", script).CombinedOutput()
|
|
||||||
require.NoError(t, err, "Failed to assign IP address to loopback adapter")
|
|
||||||
|
|
||||||
// Wait for the IP address to be applied
|
|
||||||
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
|
|
||||||
defer cancel()
|
|
||||||
err = waitForIPAddress(ctx, interfaceName, ip.String())
|
|
||||||
require.NoError(t, err, "IP address not applied within timeout")
|
|
||||||
|
|
||||||
t.Cleanup(func() {
|
|
||||||
script = fmt.Sprintf(`Remove-NetIPAddress -InterfaceAlias "%s" -IPAddress "%s" -Confirm:$False`, interfaceName, ip.String())
|
|
||||||
_, err = exec.Command("powershell", "-Command", script).CombinedOutput()
|
|
||||||
require.NoError(t, err, "Failed to remove IP address from loopback adapter")
|
|
||||||
})
|
|
||||||
|
|
||||||
return interfaceName
|
|
||||||
}
|
|
||||||
|
|
||||||
func fetchOriginalGateway() (*RouteInfo, error) {
|
func fetchOriginalGateway() (*RouteInfo, error) {
|
||||||
cmd := exec.Command("powershell", "-Command", "Get-NetRoute -DestinationPrefix 0.0.0.0/0 | Select-Object Nexthop, RouteMetric, InterfaceAlias | ConvertTo-Json")
|
cmd := exec.Command("powershell", "-Command", "Get-NetRoute -DestinationPrefix 0.0.0.0/0 | Select-Object Nexthop, RouteMetric, InterfaceAlias | ConvertTo-Json")
|
||||||
output, err := cmd.CombinedOutput()
|
output, err := cmd.CombinedOutput()
|
||||||
@ -231,30 +206,6 @@ func verifyOutput(t *testing.T, output *FindNetRouteOutput, sourceIP, destPrefix
|
|||||||
assert.Equal(t, intf, output.InterfaceAlias, "Interface mismatch")
|
assert.Equal(t, intf, output.InterfaceAlias, "Interface mismatch")
|
||||||
}
|
}
|
||||||
|
|
||||||
func waitForIPAddress(ctx context.Context, interfaceAlias, expectedIPAddress string) error {
|
|
||||||
ticker := time.NewTicker(1 * time.Second)
|
|
||||||
defer ticker.Stop()
|
|
||||||
|
|
||||||
for {
|
|
||||||
select {
|
|
||||||
case <-ctx.Done():
|
|
||||||
return ctx.Err()
|
|
||||||
case <-ticker.C:
|
|
||||||
out, err := exec.Command("powershell", "-Command", fmt.Sprintf(`Get-NetIPAddress -InterfaceAlias "%s" | Select-Object -ExpandProperty IPAddress`, interfaceAlias)).CombinedOutput()
|
|
||||||
if err != nil {
|
|
||||||
return err
|
|
||||||
}
|
|
||||||
|
|
||||||
ipAddresses := strings.Split(strings.TrimSpace(string(out)), "\n")
|
|
||||||
for _, ip := range ipAddresses {
|
|
||||||
if strings.TrimSpace(ip) == expectedIPAddress {
|
|
||||||
return nil
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
func combineOutputs(outputs []FindNetRouteOutput) *FindNetRouteOutput {
|
func combineOutputs(outputs []FindNetRouteOutput) *FindNetRouteOutput {
|
||||||
var combined FindNetRouteOutput
|
var combined FindNetRouteOutput
|
||||||
|
|
||||||
@ -285,5 +236,25 @@ func combineOutputs(outputs []FindNetRouteOutput) *FindNetRouteOutput {
|
|||||||
func setupDummyInterfacesAndRoutes(t *testing.T) {
|
func setupDummyInterfacesAndRoutes(t *testing.T) {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
|
|
||||||
createAndSetupDummyInterface(t, "Loopback Pseudo-Interface 1", "10.0.0.1/8")
|
addDummyRoute(t, "10.0.0.0/8")
|
||||||
|
}
|
||||||
|
|
||||||
|
func addDummyRoute(t *testing.T, dstCIDR string) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
script := fmt.Sprintf(`New-NetRoute -DestinationPrefix "%s" -InterfaceIndex 1 -PolicyStore ActiveStore`, dstCIDR)
|
||||||
|
|
||||||
|
output, err := exec.Command("powershell", "-Command", script).CombinedOutput()
|
||||||
|
if err != nil {
|
||||||
|
t.Logf("Failed to add dummy route: %v\nOutput: %s", err, output)
|
||||||
|
t.FailNow()
|
||||||
|
}
|
||||||
|
|
||||||
|
t.Cleanup(func() {
|
||||||
|
script = fmt.Sprintf(`Remove-NetRoute -DestinationPrefix "%s" -InterfaceIndex 1 -Confirm:$false`, dstCIDR)
|
||||||
|
output, err := exec.Command("powershell", "-Command", script).CombinedOutput()
|
||||||
|
if err != nil {
|
||||||
|
t.Logf("Failed to remove dummy route: %v\nOutput: %s", err, output)
|
||||||
|
}
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user