Skip to content

Commit

Permalink
Fix routing issues with MacOS (#1815)
Browse files Browse the repository at this point in the history
* Handle zones properly

* Use host routes for single IPs 

* Add GOOS and GOARCH to startup log

* Log powershell command
  • Loading branch information
lixmal authored Apr 9, 2024
1 parent c286577 commit ac0fe60
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 29 deletions.
3 changes: 2 additions & 1 deletion client/internal/connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"runtime"
"strings"
"time"

Expand Down Expand Up @@ -93,7 +94,7 @@ func runClient(
relayProbe *Probe,
wgProbe *Probe,
) 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.
// Otherwise, we might not be able to connect to the management server to retrieve new config.
Expand Down
35 changes: 28 additions & 7 deletions client/internal/routemanager/systemops.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"fmt"
"net"
"net/netip"
"runtime"
"strconv"

"github.com/hashicorp/go-multierror"
"github.com/libp2p/go-netroute"
Expand Down Expand Up @@ -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)
if gateway == 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)

addr, ok := netip.AddrFromSlice(preferredSrc)
if !ok {
return netip.Addr{}, nil, fmt.Errorf("failed to parse IP address: %s", preferredSrc)
addr, err := ipToAddr(preferredSrc, intf)
if err != nil {
return netip.Addr{}, nil, fmt.Errorf("convert preferred source to address: %w", err)
}
return addr.Unmap(), intf, nil
}

addr, ok := netip.AddrFromSlice(gateway)
addr, err := ipToAddr(gateway, intf)
if err != nil {
return netip.Addr{}, nil, fmt.Errorf("convert gateway to address: %w", err)
}

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{}, nil, fmt.Errorf("failed to parse IP address: %s", gateway)
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(), intf, nil
return addr.Unmap(), nil
}

func existsInRouteTable(prefix netip.Prefix) (bool, error) {
Expand Down
28 changes: 18 additions & 10 deletions client/internal/routemanager/systemops_bsd.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net/netip"
"syscall"

log "github.com/sirupsen/logrus"
"golang.org/x/net/route"
)

Expand Down Expand Up @@ -51,16 +52,24 @@ func getRoutesFromTable() ([]netip.Prefix, error) {
continue
}

addr, ok := toNetIPAddr(m.Addrs[0])
if !ok {
if len(m.Addrs) < 3 {
log.Warnf("Unexpected RIB message Addrs: %v", m.Addrs)
continue
}

mask, ok := toNetIPMASK(m.Addrs[2])
addr, ok := toNetIPAddr(m.Addrs[0])
if !ok {
continue
}
cidr, _ := mask.Size()

cidr := 32
if mask := m.Addrs[2]; mask != nil {
cidr, ok = toCIDR(mask)
if !ok {
log.Debugf("Unexpected RIB message Addrs[2]: %v", mask)
continue
}
}

routePrefix := netip.PrefixFrom(addr, cidr)
if routePrefix.IsValid() {
Expand All @@ -73,20 +82,19 @@ func getRoutesFromTable() ([]netip.Prefix, error) {
func toNetIPAddr(a route.Addr) (netip.Addr, bool) {
switch t := a.(type) {
case *route.Inet4Addr:
ip := net.IPv4(t.IP[0], t.IP[1], t.IP[2], t.IP[3])
addr := netip.MustParseAddr(ip.String())
return addr, true
return netip.AddrFrom4(t.IP), true
default:
return netip.Addr{}, false
}
}

func toNetIPMASK(a route.Addr) (net.IPMask, bool) {
func toCIDR(a route.Addr) (int, bool) {
switch t := a.(type) {
case *route.Inet4Addr:
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:
return nil, false
return 0, false
}
}
6 changes: 5 additions & 1 deletion client/internal/routemanager/systemops_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
inet := "-inet"
network := prefix.String()
if prefix.IsSingleIP() {
network = prefix.Addr().String()
}
if prefix.Addr().Is6() {
inet = "-inet6"
// Special case for IPv6 split default route, pointing to the wg interface fails
Expand All @@ -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() {
args = append(args, nexthop.Unmap().String())
} else if intf != "" {
Expand Down
3 changes: 3 additions & 0 deletions client/internal/routemanager/systemops_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,9 @@ func removeAllRules(params ruleParams) error {
func addNextHop(addr netip.Addr, intf string, route *netlink.Route) error {
if addr.IsValid() {
route.Gw = addr.AsSlice()
if intf == "" {
intf = addr.Zone()
}
}

if intf != "" {
Expand Down
33 changes: 25 additions & 8 deletions client/internal/routemanager/systemops_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func getRoutesFromTable() ([]netip.Prefix, error) {
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()
psCmd := "New-NetRoute"

Expand All @@ -73,18 +73,28 @@ func addRoutePowershell(prefix netip.Prefix, nexthop netip.Addr, intf string) er
}

script := fmt.Sprintf(
`%s -AddressFamily "%s" -DestinationPrefix "%s" -InterfaceAlias "%s" -Confirm:$False -ErrorAction Stop`,
psCmd, addressFamily, destinationPrefix, intf,
`%s -AddressFamily "%s" -DestinationPrefix "%s" -Confirm:$False -ErrorAction Stop`,
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() {
script = fmt.Sprintf(
`%s -NextHop "%s"`, script, nexthop,
)
}

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 {
return fmt.Errorf("PowerShell add route: %w", err)
Expand All @@ -98,7 +108,7 @@ func addRouteCmd(prefix netip.Prefix, nexthop netip.Addr, _ string) error {

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 {
return fmt.Errorf("route add: %w", err)
}
Expand All @@ -107,21 +117,28 @@ func addRouteCmd(prefix netip.Prefix, nexthop netip.Addr, _ 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
if intf != "" {
return addRoutePowershell(prefix, nexthop, intf)
if intf != "" || intfIdx != "" {
return addRoutePowershell(prefix, nexthop, intf, intfIdx)
}
return addRouteCmd(prefix, nexthop, intf)
}

func removeFromRouteTable(prefix netip.Prefix, nexthop netip.Addr, _ string) error {
args := []string{"delete", prefix.String()}
if nexthop.IsValid() {
nexthop.WithZone("")
args = append(args, nexthop.Unmap().String())
}

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 {
return fmt.Errorf("remove route: %w", err)
Expand Down
4 changes: 2 additions & 2 deletions util/net/dialer_generic.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (d *Dialer) DialContext(ctx context.Context, network, address string) (net.

connID := GenerateConnID()
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)
}
}
Expand Down Expand Up @@ -97,7 +97,7 @@ func (c *Conn) Close() error {
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)
if err != nil {
return fmt.Errorf("split host and port: %w", err)
Expand Down

0 comments on commit ac0fe60

Please sign in to comment.