net/interfaces: improve default route detection

Instead of treating any interface with a non-ifscope route as a
potential default gateway, now verify that a given route is
actually a default route (0.0.0.0/0 or ::/0).

Fixes #5879

Signed-off-by: Anton Tolchanov <anton@tailscale.com>

net/interfaces/interfaces_bsd.go

@@ -2,7 +2,8 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// Common code for FreeBSD and Darwin.
+// Common code for FreeBSD and Darwin. This might also work on other
+// BSD systems (e.g. OpenBSD) but has not been tested.
 
 //go:build darwin || freebsd
 // +build darwin freebsd
@@ -15,6 +16,7 @@
 	"log"
 	"net"
 	"net/netip"
+	"syscall"
 
 	"golang.org/x/net/route"
 	"golang.org/x/sys/unix"
@@ -58,29 +60,16 @@ func DefaultRouteInterfaceIndex() (int, error) {
 	if err != nil {
 		return 0, fmt.Errorf("route.ParseRIB: %w", err)
 	}
-	indexSeen := map[int]int{} // index => count
 	for _, m := range msgs {
 		rm, ok := m.(*route.RouteMessage)
 		if !ok {
 			continue
 		}
-		if rm.Flags&unix.RTF_GATEWAY == 0 {
+		if isDefaultGateway(rm) {
-			continue
+			return rm.Index, nil
 		}
-		if rm.Flags&unix.RTF_IFSCOPE != 0 {
-			continue
 	}
-		indexSeen[rm.Index]++
-	}
-	if len(indexSeen) == 0 {
 	return 0, errors.New("no gateway index found")
-	}
-	if len(indexSeen) == 1 {
-		for idx := range indexSeen {
-			return idx, nil
-		}
-	}
-	return 0, fmt.Errorf("ambiguous gateway interfaces found: %v", indexSeen)
 }
 
 func init() {
@@ -103,25 +92,54 @@ func likelyHomeRouterIPBSDFetchRIB() (ret netip.Addr, ok bool) {
 		if !ok {
 			continue
 		}
-		if rm.Flags&unix.RTF_GATEWAY == 0 {
+		if !isDefaultGateway(rm) {
-			continue
-		}
-		if rm.Flags&unix.RTF_IFSCOPE != 0 {
-			continue
-		}
-		if len(rm.Addrs) > unix.RTAX_GATEWAY {
-			dst4, ok := rm.Addrs[unix.RTAX_DST].(*route.Inet4Addr)
-			if !ok || dst4.IP != ([4]byte{0, 0, 0, 0}) {
-				// Expect 0.0.0.0 as DST field.
 			continue
 		}
+
 		gw, ok := rm.Addrs[unix.RTAX_GATEWAY].(*route.Inet4Addr)
 		if !ok {
 			continue
 		}
 		return netaddr.IPv4(gw.IP[0], gw.IP[1], gw.IP[2], gw.IP[3]), true
 	}
-	}
 
 	return ret, false
 }
+
+var v4default = [4]byte{0, 0, 0, 0}
+var v6default = [16]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}
+
+func isDefaultGateway(rm *route.RouteMessage) bool {
+	if rm.Flags&unix.RTF_GATEWAY == 0 {
+		return false
+	}
+	// Defined locally because FreeBSD does not have unix.RTF_IFSCOPE.
+	const RTF_IFSCOPE = 0x1000000
+	if rm.Flags&RTF_IFSCOPE != 0 {
+		return false
+	}
+
+	// Addrs is [RTAX_DST, RTAX_GATEWAY, RTAX_NETMASK, ...]
+	if len(rm.Addrs) <= unix.RTAX_NETMASK {
+		return false
+	}
+
+	dst := rm.Addrs[unix.RTAX_DST]
+	netmask := rm.Addrs[unix.RTAX_NETMASK]
+
+	if dst.Family() == syscall.AF_INET &&
+		netmask.Family() == syscall.AF_INET &&
+		dst.(*route.Inet4Addr).IP == v4default &&
+		netmask.(*route.Inet4Addr).IP == v4default {
+		return true
+	}
+
+	if dst.Family() == syscall.AF_INET6 &&
+		netmask.Family() == syscall.AF_INET6 &&
+		dst.(*route.Inet6Addr).IP == v6default &&
+		netmask.(*route.Inet6Addr).IP == v6default {
+		return true
+	}
+
+	return false
+}

net/interfaces/interfaces_darwin_test.go

@@ -17,17 +17,31 @@
 
 func TestLikelyHomeRouterIPSyscallExec(t *testing.T) {
 	syscallIP, syscallOK := likelyHomeRouterIPBSDFetchRIB()
-	netstatIP, netstatOK := likelyHomeRouterIPDarwinExec()
+	netstatIP, netstatIf, netstatOK := likelyHomeRouterIPDarwinExec()
+
 	if syscallOK != netstatOK || syscallIP != netstatIP {
 		t.Errorf("syscall() = %v, %v, netstat = %v, %v",
 			syscallIP, syscallOK,
 			netstatIP, netstatOK,
 		)
 	}
+
+	if !syscallOK {
+		return
+	}
+
+	def, err := defaultRoute()
+	if err != nil {
+		t.Errorf("defaultRoute() error: %v", err)
+	}
+
+	if def.InterfaceName != netstatIf {
+		t.Errorf("syscall default route interface %s differs from netstat %s", def.InterfaceName, netstatIf)
+	}
 }
 
 /*
-Parse out 10.0.0.1 from:
+Parse out 10.0.0.1 and en0 from:
 
 $ netstat -r -n -f inet
 Routing tables
@@ -40,12 +54,12 @@ func TestLikelyHomeRouterIPSyscallExec(t *testing.T) {
 10.0.0.1/32        link#4             UCS            en0      !
 ...
 */
-func likelyHomeRouterIPDarwinExec() (ret netip.Addr, ok bool) {
+func likelyHomeRouterIPDarwinExec() (ret netip.Addr, netif string, ok bool) {
 	if version.IsMobile() {
 		// Don't try to do subprocesses on iOS. Ends up with log spam like:
 		// kernel: "Sandbox: IPNExtension(86580) deny(1) process-fork"
 		// This is why we have likelyHomeRouterIPDarwinSyscall.
-		return ret, false
+		return ret, "", false
 	}
 	cmd := exec.Command("/usr/sbin/netstat", "-r", "-n", "-f", "inet")
 	stdout, err := cmd.StdoutPipe()
@@ -64,22 +78,26 @@ func likelyHomeRouterIPDarwinExec() (ret netip.Addr, ok bool) {
 			return nil
 		}
 		f = mem.AppendFields(f[:0], line)
-		if len(f) < 3 || !f[0].EqualString("default") {
+		if len(f) < 4 || !f[0].EqualString("default") {
 			return nil
 		}
-		ipm, flagsm := f[1], f[2]
+		ipm, flagsm, netifm := f[1], f[2], f[3]
 		if !mem.Contains(flagsm, mem.S("G")) {
 			return nil
 		}
+		if mem.Contains(flagsm, mem.S("I")) {
+			return nil
+		}
 		ip, err := netip.ParseAddr(string(mem.Append(nil, ipm)))
 		if err == nil && ip.IsPrivate() {
 			ret = ip
+			netif = netifm.StringCopy()
 			// We've found what we're looking for.
 			return errStopReadingNetstatTable
 		}
 		return nil
 	})
-	return ret, ret.IsValid()
+	return ret, netif, ret.IsValid()
 }
 
 func TestFetchRoutingTable(t *testing.T) {