From 01286af82b37e8edc787efcd681d95b61d3085d7 Mon Sep 17 00:00:00 2001 From: Andrew Dunham Date: Tue, 19 Dec 2023 16:27:52 -0500 Subject: [PATCH] net/interfaces: better handle multiple interfaces in LikelyHomeRouterIP Currently, we get the "likely home router" gateway IP and then iterate through all IPs for all interfaces trying to match IPs to determine the source IP. However, on many platforms we know what interface the gateway is through, and thus we don't need to iterate through all interfaces checking IPs. Instead, use the IP address of the associated interface. This better handles the case where we have multiple interfaces on a system all connected to the same gateway, and where the first interface that we visit (as iterated by ForeachInterfaceAddress) isn't also the default internet route. Updates #8992 Signed-off-by: Andrew Dunham Change-Id: I8632f577f1136930f4ec60c76376527a19a47d1f --- net/interfaces/interfaces.go | 42 ++++++++++++++++++++---- net/interfaces/interfaces_bsd.go | 18 +++++++--- net/interfaces/interfaces_darwin_test.go | 2 +- net/interfaces/interfaces_linux.go | 32 ++++++++++++++---- net/interfaces/interfaces_test.go | 8 ++--- net/interfaces/interfaces_windows.go | 6 ++-- 6 files changed, 83 insertions(+), 25 deletions(-) diff --git a/net/interfaces/interfaces.go b/net/interfaces/interfaces.go index d1fc3be56..216521bfa 100644 --- a/net/interfaces/interfaces.go +++ b/net/interfaces/interfaces.go @@ -15,6 +15,7 @@ import ( "sort" "strings" + "tailscale.com/envknob" "tailscale.com/hostinfo" "tailscale.com/net/netaddr" "tailscale.com/net/tsaddr" @@ -525,7 +526,21 @@ func HTTPOfListener(ln net.Listener) string { } -var likelyHomeRouterIP func() (netip.Addr, bool) +// likelyHomeRouterIP, if present, is a platform-specific function that is used +// to determine the likely home router IP of the current system. The signature +// of this function is: +// +// func() (homeRouter, localAddr netip.Addr, ok bool) +// +// It should return a homeRouter IP and ok=true, or no homeRouter IP and +// ok=false. Optionally, an implementation can return the "self" IP address as +// well, which will be used instead of attempting to determine it by reading +// the system's interfaces. +var likelyHomeRouterIP func() (netip.Addr, netip.Addr, bool) + +// For debugging the new behaviour where likelyHomeRouterIP can return the +// "self" IP; should remove after we're confidant this won't cause issues. +var disableLikelyHomeRouterIPSelf = envknob.RegisterBool("TS_DEBUG_DISABLE_LIKELY_HOME_ROUTER_IP_SELF") // LikelyHomeRouterIP returns the likely IP of the residential router, // which will always be an IPv4 private address, if found. @@ -533,15 +548,30 @@ var likelyHomeRouterIP func() (netip.Addr, bool) // the LAN using that gateway. // This is used as the destination for UPnP, NAT-PMP, PCP, etc queries. func LikelyHomeRouterIP() (gateway, myIP netip.Addr, ok bool) { - if likelyHomeRouterIP != nil { - gateway, ok = likelyHomeRouterIP() - if !ok { - return - } + // If we don't have a way to get the home router IP, then we can't do + // anything; just return. + if likelyHomeRouterIP == nil { + return } + + // Get the gateway next; if that fails, we can't continue. + gateway, myIP, ok = likelyHomeRouterIP() if !ok { return } + + // If the platform-specific implementation returned a valid myIP, then + // we can return it as-is without needing to iterate through all + // interface addresses. + if disableLikelyHomeRouterIPSelf() { + myIP = netip.Addr{} + } + if myIP.IsValid() { + return + } + + // The platform-specific implementation didn't return a valid myIP; + // iterate through all interfaces and try to find the correct one. ForeachInterfaceAddress(func(i Interface, pfx netip.Prefix) { if !i.IsUp() { // Skip interfaces that aren't up. diff --git a/net/interfaces/interfaces_bsd.go b/net/interfaces/interfaces_bsd.go index 2b63908d9..3f793f464 100644 --- a/net/interfaces/interfaces_bsd.go +++ b/net/interfaces/interfaces_bsd.go @@ -86,16 +86,16 @@ func init() { likelyHomeRouterIP = likelyHomeRouterIPBSDFetchRIB } -func likelyHomeRouterIPBSDFetchRIB() (ret netip.Addr, ok bool) { +func likelyHomeRouterIPBSDFetchRIB() (ret, myIP netip.Addr, ok bool) { rib, err := fetchRoutingTable() if err != nil { log.Printf("routerIP/FetchRIB: %v", err) - return ret, false + return ret, myIP, false } msgs, err := parseRoutingTable(rib) if err != nil { log.Printf("routerIP/ParseRIB: %v", err) - return ret, false + return ret, myIP, false } for _, m := range msgs { rm, ok := m.(*route.RouteMessage) @@ -110,10 +110,18 @@ func likelyHomeRouterIPBSDFetchRIB() (ret netip.Addr, ok bool) { if !ok { continue } - return netaddr.IPv4(gw.IP[0], gw.IP[1], gw.IP[2], gw.IP[3]), true + // If the route entry has an interface address associated with + // it, then parse and return that. This is optional. + if len(rm.Addrs) >= unix.RTAX_IFA { + if addr, ok := rm.Addrs[unix.RTAX_IFA].(*route.Inet4Addr); ok { + myIP = netaddr.IPv4(addr.IP[0], addr.IP[1], addr.IP[2], addr.IP[3]) + } + } + + return netaddr.IPv4(gw.IP[0], gw.IP[1], gw.IP[2], gw.IP[3]), myIP, true } - return ret, false + return ret, myIP, false } var v4default = [4]byte{0, 0, 0, 0} diff --git a/net/interfaces/interfaces_darwin_test.go b/net/interfaces/interfaces_darwin_test.go index 9c56df9bb..ec53a0833 100644 --- a/net/interfaces/interfaces_darwin_test.go +++ b/net/interfaces/interfaces_darwin_test.go @@ -15,7 +15,7 @@ import ( ) func TestLikelyHomeRouterIPSyscallExec(t *testing.T) { - syscallIP, syscallOK := likelyHomeRouterIPBSDFetchRIB() + syscallIP, _, syscallOK := likelyHomeRouterIPBSDFetchRIB() netstatIP, netstatIf, netstatOK := likelyHomeRouterIPDarwinExec() if syscallOK != netstatOK || syscallIP != netstatIP { diff --git a/net/interfaces/interfaces_linux.go b/net/interfaces/interfaces_linux.go index cad5d4a4a..440b74f89 100644 --- a/net/interfaces/interfaces_linux.go +++ b/net/interfaces/interfaces_linux.go @@ -45,14 +45,14 @@ Iface Destination Gateway Flags RefCnt Use Metric Mask ens18 00000000 0100000A 0003 0 0 0 00000000 0 0 0 ens18 0000000A 00000000 0001 0 0 0 0000FFFF 0 0 0 */ -func likelyHomeRouterIPLinux() (ret netip.Addr, ok bool) { +func likelyHomeRouterIPLinux() (ret netip.Addr, myIP netip.Addr, ok bool) { if procNetRouteErr.Load() { // If we failed to read /proc/net/route previously, don't keep trying. // But if we're on Android, go into the Android path. if runtime.GOOS == "android" { return likelyHomeRouterIPAndroid() } - return ret, false + return ret, myIP, false } lineNum := 0 var f []mem.RO @@ -99,7 +99,27 @@ func likelyHomeRouterIPLinux() (ret netip.Addr, ok bool) { log.Printf("interfaces: failed to read /proc/net/route: %v", err) } if ret.IsValid() { - return ret, true + // Try to get the local IP of the interface associated with + // this route to short-circuit finding the IP associated with + // this gateway. This isn't fatal if it fails. + if len(f) > 0 && !disableLikelyHomeRouterIPSelf() { + ForeachInterface(func(ni Interface, pfxs []netip.Prefix) { + // Ensure this is the same interface + if !f[0].EqualString(ni.Name) { + return + } + + // Find the first IPv4 address and use it. + for _, pfx := range pfxs { + if addr := pfx.Addr(); addr.Is4() { + myIP = addr + break + } + } + }) + } + + return ret, myIP, true } if lineNum >= maxProcNetRouteRead { // If we went over our line limit without finding an answer, assume @@ -113,12 +133,12 @@ func likelyHomeRouterIPLinux() (ret netip.Addr, ok bool) { // find one in the future. procNetRouteErr.Store(true) } - return netip.Addr{}, false + return netip.Addr{}, netip.Addr{}, false } // Android apps don't have permission to read /proc/net/route, at // least on Google devices and the Android emulator. -func likelyHomeRouterIPAndroid() (ret netip.Addr, ok bool) { +func likelyHomeRouterIPAndroid() (ret netip.Addr, _ netip.Addr, ok bool) { cmd := exec.Command("/system/bin/ip", "route", "show", "table", "0") out, err := cmd.StdoutPipe() if err != nil { @@ -148,7 +168,7 @@ func likelyHomeRouterIPAndroid() (ret netip.Addr, ok bool) { }) cmd.Process.Kill() cmd.Wait() - return ret, ret.IsValid() + return ret, netip.Addr{}, ret.IsValid() } func defaultRoute() (d DefaultRouteDetails, err error) { diff --git a/net/interfaces/interfaces_test.go b/net/interfaces/interfaces_test.go index 40e9d42d2..02014309b 100644 --- a/net/interfaces/interfaces_test.go +++ b/net/interfaces/interfaces_test.go @@ -83,8 +83,8 @@ func TestLikelyHomeRouterIP(t *testing.T) { }) // Mock out the likelyHomeRouterIP to return a known gateway. - tstest.Replace(t, &likelyHomeRouterIP, func() (netip.Addr, bool) { - return netip.MustParseAddr("192.168.7.1"), true + tstest.Replace(t, &likelyHomeRouterIP, func() (netip.Addr, netip.Addr, bool) { + return netip.MustParseAddr("192.168.7.1"), netip.Addr{}, true }) gw, my, ok := LikelyHomeRouterIP() @@ -177,8 +177,8 @@ func TestLikelyHomeRouterIP_Prefix(t *testing.T) { }) // Mock out the likelyHomeRouterIP to return a known gateway. - tstest.Replace(t, &likelyHomeRouterIP, func() (netip.Addr, bool) { - return netip.MustParseAddr("192.168.7.1"), true + tstest.Replace(t, &likelyHomeRouterIP, func() (netip.Addr, netip.Addr, bool) { + return netip.MustParseAddr("192.168.7.1"), netip.Addr{}, true }) gw, my, ok := LikelyHomeRouterIP() diff --git a/net/interfaces/interfaces_windows.go b/net/interfaces/interfaces_windows.go index 30b766130..eaf759234 100644 --- a/net/interfaces/interfaces_windows.go +++ b/net/interfaces/interfaces_windows.go @@ -25,7 +25,7 @@ func init() { getPAC = getPACWindows } -func likelyHomeRouterIPWindows() (ret netip.Addr, ok bool) { +func likelyHomeRouterIPWindows() (ret netip.Addr, _ netip.Addr, ok bool) { rs, err := winipcfg.GetIPForwardTable2(windows.AF_INET) if err != nil { log.Printf("routerIP/GetIPForwardTable2 error: %v", err) @@ -92,10 +92,10 @@ func likelyHomeRouterIPWindows() (ret netip.Addr, ok bool) { if ret.IsValid() && !ret.IsPrivate() { // Default route has a non-private gateway - return netip.Addr{}, false + return netip.Addr{}, netip.Addr{}, false } - return ret, ret.IsValid() + return ret, netip.Addr{}, ret.IsValid() } // NonTailscaleMTUs returns a map of interface LUID to interface MTU,