From fa932fefe7ba1c826231171418bf1b6de9884649 Mon Sep 17 00:00:00 2001 From: Mihai Parparita Date: Fri, 10 Feb 2023 15:02:12 -0800 Subject: [PATCH] net/interfaces: redo how we get the default interface on macOS and iOS With #6566 we added an external mechanism for getting the default interface, and used it on macOS and iOS (see tailscale/corp#8201). The goal was to be able to get the default physical interface even when using an exit node (in which case the routing table would say that the Tailscale utun* interface is the default). However, the external mechanism turns out to be unreliable in some cases, e.g. when multiple cellular interfaces are present/toggled (I have occasionally gotten my phone into a state where it reports the pdp_ip1 interface as the default, even though it can't actually route traffic). It was observed that `ifconfig -v` on macOS reports an "effective interface" for the Tailscale utn* interface, which seems promising. By examining the ifconfig source code, it turns out that this is done via a SIOCGIFDELEGATE ioctl syscall. Though this is a private API, it appears to have been around for a long time (e.g. it's in the 10.13 xnu release at https://opensource.apple.com/source/xnu/xnu-4570.41.2/bsd/net/if_types.h.auto.html) and thus is unlikely to go away. We can thus use this ioctl if the routing table says that a utun* interface is the default, and go back to the simpler mechanism that we had before #6566. Updates #7184 Updates #7188 Signed-off-by: Mihai Parparita --- cmd/derper/depaware.txt | 2 +- ipn/ipnlocal/local.go | 1 - net/interfaces/interfaces.go | 12 ----- net/interfaces/interfaces_bsd.go | 30 ++--------- net/interfaces/interfaces_darwin.go | 76 ++++++++++++++++++++++++++++ net/interfaces/interfaces_freebsd.go | 4 ++ 6 files changed, 85 insertions(+), 40 deletions(-) diff --git a/cmd/derper/depaware.txt b/cmd/derper/depaware.txt index 529494f56..f8f2ce63e 100644 --- a/cmd/derper/depaware.txt +++ b/cmd/derper/depaware.txt @@ -81,7 +81,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa tailscale.com/util/dnsname from tailscale.com/hostinfo+ tailscale.com/util/httpm from tailscale.com/client/tailscale tailscale.com/util/lineread from tailscale.com/hostinfo+ - tailscale.com/util/mak from tailscale.com/syncs + tailscale.com/util/mak from tailscale.com/syncs+ tailscale.com/util/multierr from tailscale.com/health tailscale.com/util/set from tailscale.com/health tailscale.com/util/singleflight from tailscale.com/net/dnscache diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index cc56040ba..64b9999cb 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -3840,7 +3840,6 @@ func (b *LocalBackend) setNetMapLocked(nm *netmap.NetworkMap) { // See the netns package for documentation on what this capability does. netns.SetBindToInterfaceByRoute(hasCapability(nm, tailcfg.CapabilityBindToInterfaceByRoute)) - interfaces.SetDisableAlternateDefaultRouteInterface(hasCapability(nm, tailcfg.CapabilityDebugDisableAlternateDefaultRouteInterface)) netns.SetDisableBindConnToInterface(hasCapability(nm, tailcfg.CapabilityDebugDisableBindConnToInterface)) b.setTCPPortsInterceptedFromNetmapAndPrefsLocked(b.pm.CurrentPrefs()) diff --git a/net/interfaces/interfaces.go b/net/interfaces/interfaces.go index 40cb1e122..c9d92dfcb 100644 --- a/net/interfaces/interfaces.go +++ b/net/interfaces/interfaces.go @@ -13,7 +13,6 @@ "runtime" "sort" "strings" - "sync/atomic" "tailscale.com/hostinfo" "tailscale.com/net/netaddr" @@ -757,14 +756,3 @@ func HasCGNATInterface() (bool, error) { } return hasCGNATInterface, nil } - -var disableAlternateDefaultRouteInterface atomic.Bool - -// SetDisableAlternateDefaultRouteInterface disables the optional external/ -// alternate mechanism for getting the default route network interface. -// -// Currently, this only changes the behaviour on BSD-like sytems (FreeBSD and -// Darwin). -func SetDisableAlternateDefaultRouteInterface(v bool) { - disableAlternateDefaultRouteInterface.Store(v) -} diff --git a/net/interfaces/interfaces_bsd.go b/net/interfaces/interfaces_bsd.go index 610871d7c..99c82e166 100644 --- a/net/interfaces/interfaces_bsd.go +++ b/net/interfaces/interfaces_bsd.go @@ -19,7 +19,6 @@ "golang.org/x/net/route" "golang.org/x/sys/unix" "tailscale.com/net/netaddr" - "tailscale.com/syncs" ) func defaultRoute() (d DefaultRouteDetails, err error) { @@ -40,19 +39,6 @@ func defaultRoute() (d DefaultRouteDetails, err error) { // owns the default route. It returns the first IPv4 or IPv6 default route it // finds (it does not prefer one or the other). func DefaultRouteInterfaceIndex() (int, error) { - disabledAlternateDefaultRouteInterface := false - if f := defaultRouteInterfaceIndexFunc.Load(); f != nil { - if ifIndex := f(); ifIndex != 0 { - if !disableAlternateDefaultRouteInterface.Load() { - return ifIndex, nil - } else { - disabledAlternateDefaultRouteInterface = true - log.Printf("interfaces_bsd: alternate default route interface function disabled, would have returned interface %d", ifIndex) - } - } - // Fallthrough if we can't use the alternate implementation. - } - // $ netstat -nr // Routing tables // Internet: @@ -81,8 +67,10 @@ func DefaultRouteInterfaceIndex() (int, error) { continue } if isDefaultGateway(rm) { - if disabledAlternateDefaultRouteInterface { - log.Printf("interfaces_bsd: alternate default route interface function disabled, default implementation returned %d", rm.Index) + if delegatedIndex, err := getDelegatedInterface(rm.Index); err == nil && delegatedIndex != 0 { + return delegatedIndex, nil + } else if err != nil { + log.Printf("interfaces_bsd: could not get delegated interface: %v", err) } return rm.Index, nil } @@ -90,16 +78,6 @@ func DefaultRouteInterfaceIndex() (int, error) { return 0, errors.New("no gateway index found") } -var defaultRouteInterfaceIndexFunc syncs.AtomicValue[func() int] - -// SetDefaultRouteInterfaceIndexFunc allows an alternate implementation of -// DefaultRouteInterfaceIndex to be provided. If none is set, or if f() returns a 0 -// (indicating an unknown interface index), then the default implementation (that parses -// the routing table) will be used. -func SetDefaultRouteInterfaceIndexFunc(f func() int) { - defaultRouteInterfaceIndexFunc.Store(f) -} - func init() { likelyHomeRouterIP = likelyHomeRouterIPBSDFetchRIB } diff --git a/net/interfaces/interfaces_darwin.go b/net/interfaces/interfaces_darwin.go index cc9df865d..d16f1a5e7 100644 --- a/net/interfaces/interfaces_darwin.go +++ b/net/interfaces/interfaces_darwin.go @@ -4,9 +4,15 @@ package interfaces import ( + "net" + "strings" + "sync" "syscall" + "unsafe" "golang.org/x/net/route" + "golang.org/x/sys/unix" + "tailscale.com/util/mak" ) // fetchRoutingTable calls route.FetchRIB, fetching NET_RT_DUMP2. @@ -17,3 +23,73 @@ func fetchRoutingTable() (rib []byte, err error) { func parseRoutingTable(rib []byte) ([]route.Message, error) { return route.ParseRIB(syscall.NET_RT_IFLIST2, rib) } + +var ifNames struct { + sync.Mutex + m map[int]string // ifindex => name +} + +// getDelegatedInterface returns the interface index of the underlying interface +// for the given interface index. 0 is returned if the interface does not +// delegate. +func getDelegatedInterface(ifIndex int) (int, error) { + ifNames.Lock() + defer ifNames.Unlock() + + // To get the delegated interface, we do what ifconfig does and use the + // SIOCGIFDELEGATE ioctl. It operates in term of a ifreq struct, which + // has to be populated with a interface name. To avoid having to do a + // interface index -> name lookup every time, we cache interface names + // (since indexes and names are stable after boot). + ifName, ok := ifNames.m[ifIndex] + if !ok { + iface, err := net.InterfaceByIndex(ifIndex) + if err != nil { + return 0, err + } + ifName = iface.Name + mak.Set(&ifNames.m, ifIndex, ifName) + } + + // Only tunnels (like Tailscale itself) have a delegated interface, avoid + // the ioctl if we can. + if !strings.HasPrefix(ifName, "utun") { + return 0, nil + } + + // We don't cache the result of the ioctl, since the delegated interface can + // change, e.g. if the user changes the preferred service order in the + // network preference pane. + fd, err := unix.Socket(unix.AF_INET, unix.SOCK_DGRAM, 0) + if err != nil { + return 0, err + } + defer unix.Close(fd) + + // Match the ifreq struct/union from the bsd/net/if.h header in the Darwin + // open source release. + var ifr struct { + ifr_name [unix.IFNAMSIZ]byte + ifr_delegated uint32 + } + copy(ifr.ifr_name[:], ifName) + + // SIOCGIFDELEGATE is not in the Go x/sys package or in the public macOS + // headers. However, it is in the Darwin/xnu open source + // release (and is used by ifconfig, see + // https://github.com/apple-oss-distributions/network_cmds/blob/6ccdc225ad5aa0d23ea5e7d374956245d2462427/ifconfig.tproj/ifconfig.c#L2183-L2187). + // We generate its value by evaluating the `_IOWR('i', 157, struct ifreq)` + // macro, which is how it's defined in + // https://github.com/apple/darwin-xnu/blob/2ff845c2e033bd0ff64b5b6aa6063a1f8f65aa32/bsd/sys/sockio.h#L264 + const SIOCGIFDELEGATE = 0xc020699d + + _, _, errno := syscall.Syscall( + syscall.SYS_IOCTL, + uintptr(fd), + uintptr(SIOCGIFDELEGATE), + uintptr(unsafe.Pointer(&ifr))) + if errno != 0 { + return 0, errno + } + return int(ifr.ifr_delegated), nil +} diff --git a/net/interfaces/interfaces_freebsd.go b/net/interfaces/interfaces_freebsd.go index ae2262c9d..085db0d96 100644 --- a/net/interfaces/interfaces_freebsd.go +++ b/net/interfaces/interfaces_freebsd.go @@ -22,3 +22,7 @@ func fetchRoutingTable() (rib []byte, err error) { func parseRoutingTable(rib []byte) ([]route.Message, error) { return route.ParseRIB(syscall.NET_RT_IFLIST, rib) } + +func getDelegatedInterface(ifIndex int) (int, error) { + return 0, nil +}