Fix routing issues with MacOS (#1815)

* Handle zones properly

* Use host routes for single IPs 

* Add GOOS and GOARCH to startup log

* Log powershell command
This commit is contained in:
Viktor Liu 2024-04-09 13:25:14 +02:00 committed by GitHub
parent c28657710a
commit ac0fe6025b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 84 additions and 30 deletions

View File

@ -4,6 +4,7 @@ import (
"context" "context"
"errors" "errors"
"fmt" "fmt"
"runtime"
"strings" "strings"
"time" "time"
@ -93,7 +94,7 @@ func runClient(
relayProbe *Probe, relayProbe *Probe,
wgProbe *Probe, wgProbe *Probe,
) error { ) error {
log.Infof("starting NetBird client version %s", version.NetbirdVersion()) log.Infof("starting NetBird client version %s on %s/%s", version.NetbirdVersion(), runtime.GOOS, runtime.GOARCH)
// Check if client was not shut down in a clean way and restore DNS config if required. // Check if client was not shut down in a clean way and restore DNS config if required.
// Otherwise, we might not be able to connect to the management server to retrieve new config. // Otherwise, we might not be able to connect to the management server to retrieve new config.

View File

@ -8,6 +8,8 @@ import (
"fmt" "fmt"
"net" "net"
"net/netip" "net/netip"
"runtime"
"strconv"
"github.com/hashicorp/go-multierror" "github.com/hashicorp/go-multierror"
"github.com/libp2p/go-netroute" "github.com/libp2p/go-netroute"
@ -85,23 +87,42 @@ func getNextHop(ip netip.Addr) (netip.Addr, *net.Interface, error) {
log.Debugf("Route for %s: interface %v, nexthop %v, preferred source %v", ip, intf, gateway, preferredSrc) log.Debugf("Route for %s: interface %v, nexthop %v, preferred source %v", ip, intf, gateway, preferredSrc)
if gateway == nil { if gateway == nil {
if preferredSrc == nil { if preferredSrc == nil {
return netip.Addr{}, nil, ErrRouteNotFound return netip.Addr{}, nil, ErrRouteNotFound
} }
log.Debugf("No next hop found for ip %s, using preferred source %s", ip, preferredSrc) log.Debugf("No next hop found for ip %s, using preferred source %s", ip, preferredSrc)
addr, ok := netip.AddrFromSlice(preferredSrc) addr, err := ipToAddr(preferredSrc, intf)
if !ok { if err != nil {
return netip.Addr{}, nil, fmt.Errorf("failed to parse IP address: %s", preferredSrc) return netip.Addr{}, nil, fmt.Errorf("convert preferred source to address: %w", err)
} }
return addr.Unmap(), intf, nil return addr.Unmap(), intf, nil
} }
addr, ok := netip.AddrFromSlice(gateway) addr, err := ipToAddr(gateway, intf)
if !ok { if err != nil {
return netip.Addr{}, nil, fmt.Errorf("failed to parse IP address: %s", gateway) return netip.Addr{}, nil, fmt.Errorf("convert gateway to address: %w", err)
} }
return addr.Unmap(), intf, nil return addr, intf, nil
}
// converts a net.IP to a netip.Addr including the zone based on the passed interface
func ipToAddr(ip net.IP, intf *net.Interface) (netip.Addr, error) {
addr, ok := netip.AddrFromSlice(ip)
if !ok {
return netip.Addr{}, fmt.Errorf("failed to convert IP address to netip.Addr: %s", ip)
}
if intf != nil && (addr.IsLinkLocalMulticast() || addr.IsLinkLocalUnicast()) {
log.Tracef("Adding zone %s to address %s", intf.Name, addr)
if runtime.GOOS == "windows" {
addr = addr.WithZone(strconv.Itoa(intf.Index))
} else {
addr = addr.WithZone(intf.Name)
}
}
return addr.Unmap(), nil
} }
func existsInRouteTable(prefix netip.Prefix) (bool, error) { func existsInRouteTable(prefix netip.Prefix) (bool, error) {

View File

@ -8,6 +8,7 @@ import (
"net/netip" "net/netip"
"syscall" "syscall"
log "github.com/sirupsen/logrus"
"golang.org/x/net/route" "golang.org/x/net/route"
) )
@ -51,16 +52,24 @@ func getRoutesFromTable() ([]netip.Prefix, error) {
continue continue
} }
if len(m.Addrs) < 3 {
log.Warnf("Unexpected RIB message Addrs: %v", m.Addrs)
continue
}
addr, ok := toNetIPAddr(m.Addrs[0]) addr, ok := toNetIPAddr(m.Addrs[0])
if !ok { if !ok {
continue continue
} }
mask, ok := toNetIPMASK(m.Addrs[2]) cidr := 32
if !ok { if mask := m.Addrs[2]; mask != nil {
continue cidr, ok = toCIDR(mask)
if !ok {
log.Debugf("Unexpected RIB message Addrs[2]: %v", mask)
continue
}
} }
cidr, _ := mask.Size()
routePrefix := netip.PrefixFrom(addr, cidr) routePrefix := netip.PrefixFrom(addr, cidr)
if routePrefix.IsValid() { if routePrefix.IsValid() {
@ -73,20 +82,19 @@ func getRoutesFromTable() ([]netip.Prefix, error) {
func toNetIPAddr(a route.Addr) (netip.Addr, bool) { func toNetIPAddr(a route.Addr) (netip.Addr, bool) {
switch t := a.(type) { switch t := a.(type) {
case *route.Inet4Addr: case *route.Inet4Addr:
ip := net.IPv4(t.IP[0], t.IP[1], t.IP[2], t.IP[3]) return netip.AddrFrom4(t.IP), true
addr := netip.MustParseAddr(ip.String())
return addr, true
default: default:
return netip.Addr{}, false return netip.Addr{}, false
} }
} }
func toNetIPMASK(a route.Addr) (net.IPMask, bool) { func toCIDR(a route.Addr) (int, bool) {
switch t := a.(type) { switch t := a.(type) {
case *route.Inet4Addr: case *route.Inet4Addr:
mask := net.IPv4Mask(t.IP[0], t.IP[1], t.IP[2], t.IP[3]) mask := net.IPv4Mask(t.IP[0], t.IP[1], t.IP[2], t.IP[3])
return mask, true cidr, _ := mask.Size()
return cidr, true
default: default:
return nil, false return 0, false
} }
} }

View File

@ -35,6 +35,10 @@ func removeFromRouteTable(prefix netip.Prefix, nexthop netip.Addr, intf string)
func routeCmd(action string, prefix netip.Prefix, nexthop netip.Addr, intf string) error { func routeCmd(action string, prefix netip.Prefix, nexthop netip.Addr, intf string) error {
inet := "-inet" inet := "-inet"
network := prefix.String()
if prefix.IsSingleIP() {
network = prefix.Addr().String()
}
if prefix.Addr().Is6() { if prefix.Addr().Is6() {
inet = "-inet6" inet = "-inet6"
// Special case for IPv6 split default route, pointing to the wg interface fails // Special case for IPv6 split default route, pointing to the wg interface fails
@ -44,7 +48,7 @@ func routeCmd(action string, prefix netip.Prefix, nexthop netip.Addr, intf strin
} }
} }
args := []string{"-n", action, inet, prefix.String()} args := []string{"-n", action, inet, network}
if nexthop.IsValid() { if nexthop.IsValid() {
args = append(args, nexthop.Unmap().String()) args = append(args, nexthop.Unmap().String())
} else if intf != "" { } else if intf != "" {

View File

@ -487,6 +487,9 @@ func removeAllRules(params ruleParams) error {
func addNextHop(addr netip.Addr, intf string, route *netlink.Route) error { func addNextHop(addr netip.Addr, intf string, route *netlink.Route) error {
if addr.IsValid() { if addr.IsValid() {
route.Gw = addr.AsSlice() route.Gw = addr.AsSlice()
if intf == "" {
intf = addr.Zone()
}
} }
if intf != "" { if intf != "" {

View File

@ -63,7 +63,7 @@ func getRoutesFromTable() ([]netip.Prefix, error) {
return prefixList, nil return prefixList, nil
} }
func addRoutePowershell(prefix netip.Prefix, nexthop netip.Addr, intf string) error { func addRoutePowershell(prefix netip.Prefix, nexthop netip.Addr, intf, intfIdx string) error {
destinationPrefix := prefix.String() destinationPrefix := prefix.String()
psCmd := "New-NetRoute" psCmd := "New-NetRoute"
@ -73,10 +73,20 @@ func addRoutePowershell(prefix netip.Prefix, nexthop netip.Addr, intf string) er
} }
script := fmt.Sprintf( script := fmt.Sprintf(
`%s -AddressFamily "%s" -DestinationPrefix "%s" -InterfaceAlias "%s" -Confirm:$False -ErrorAction Stop`, `%s -AddressFamily "%s" -DestinationPrefix "%s" -Confirm:$False -ErrorAction Stop`,
psCmd, addressFamily, destinationPrefix, intf, psCmd, addressFamily, destinationPrefix,
) )
if intfIdx != "" {
script = fmt.Sprintf(
`%s -InterfaceIndex %s`, script, intfIdx,
)
} else {
script = fmt.Sprintf(
`%s -InterfaceAlias "%s"`, script, intf,
)
}
if nexthop.IsValid() { if nexthop.IsValid() {
script = fmt.Sprintf( script = fmt.Sprintf(
`%s -NextHop "%s"`, script, nexthop, `%s -NextHop "%s"`, script, nexthop,
@ -84,7 +94,7 @@ func addRoutePowershell(prefix netip.Prefix, nexthop netip.Addr, intf string) er
} }
out, err := exec.Command("powershell", "-Command", script).CombinedOutput() out, err := exec.Command("powershell", "-Command", script).CombinedOutput()
log.Tracef("PowerShell add route: %s", string(out)) log.Tracef("PowerShell %s: %s", script, string(out))
if err != nil { if err != nil {
return fmt.Errorf("PowerShell add route: %w", err) return fmt.Errorf("PowerShell add route: %w", err)
@ -98,7 +108,7 @@ func addRouteCmd(prefix netip.Prefix, nexthop netip.Addr, _ string) error {
out, err := exec.Command("route", args...).CombinedOutput() out, err := exec.Command("route", args...).CombinedOutput()
log.Tracef("route %s output: %s", strings.Join(args, " "), out) log.Tracef("route %s: %s", strings.Join(args, " "), out)
if err != nil { if err != nil {
return fmt.Errorf("route add: %w", err) return fmt.Errorf("route add: %w", err)
} }
@ -107,9 +117,15 @@ func addRouteCmd(prefix netip.Prefix, nexthop netip.Addr, _ string) error {
} }
func addToRouteTable(prefix netip.Prefix, nexthop netip.Addr, intf string) error { func addToRouteTable(prefix netip.Prefix, nexthop netip.Addr, intf string) error {
var intfIdx string
if nexthop.Zone() != "" {
intfIdx = nexthop.Zone()
nexthop.WithZone("")
}
// Powershell doesn't support adding routes without an interface but allows to add interface by name // Powershell doesn't support adding routes without an interface but allows to add interface by name
if intf != "" { if intf != "" || intfIdx != "" {
return addRoutePowershell(prefix, nexthop, intf) return addRoutePowershell(prefix, nexthop, intf, intfIdx)
} }
return addRouteCmd(prefix, nexthop, intf) return addRouteCmd(prefix, nexthop, intf)
} }
@ -117,11 +133,12 @@ func addToRouteTable(prefix netip.Prefix, nexthop netip.Addr, intf string) error
func removeFromRouteTable(prefix netip.Prefix, nexthop netip.Addr, _ string) error { func removeFromRouteTable(prefix netip.Prefix, nexthop netip.Addr, _ string) error {
args := []string{"delete", prefix.String()} args := []string{"delete", prefix.String()}
if nexthop.IsValid() { if nexthop.IsValid() {
nexthop.WithZone("")
args = append(args, nexthop.Unmap().String()) args = append(args, nexthop.Unmap().String())
} }
out, err := exec.Command("route", args...).CombinedOutput() out, err := exec.Command("route", args...).CombinedOutput()
log.Tracef("route %s output: %s", strings.Join(args, " "), out) log.Tracef("route %s: %s", strings.Join(args, " "), out)
if err != nil { if err != nil {
return fmt.Errorf("remove route: %w", err) return fmt.Errorf("remove route: %w", err)

View File

@ -56,7 +56,7 @@ func (d *Dialer) DialContext(ctx context.Context, network, address string) (net.
connID := GenerateConnID() connID := GenerateConnID()
if dialerDialHooks != nil { if dialerDialHooks != nil {
if err := calliDialerHooks(ctx, connID, address, resolver); err != nil { if err := callDialerHooks(ctx, connID, address, resolver); err != nil {
log.Errorf("Failed to call dialer hooks: %v", err) log.Errorf("Failed to call dialer hooks: %v", err)
} }
} }
@ -97,7 +97,7 @@ func (c *Conn) Close() error {
return err return err
} }
func calliDialerHooks(ctx context.Context, connID ConnectionID, address string, resolver *net.Resolver) error { func callDialerHooks(ctx context.Context, connID ConnectionID, address string, resolver *net.Resolver) error {
host, _, err := net.SplitHostPort(address) host, _, err := net.SplitHostPort(address)
if err != nil { if err != nil {
return fmt.Errorf("split host and port: %w", err) return fmt.Errorf("split host and port: %w", err)