net/interfaces: ensure we return valid 'self' IP in LikelyHomeRouterIP

Before this fix, LikelyHomeRouterIP could return a 'self' IP that
doesn't correspond to the gateway address, since it picks the first
private address when iterating over the set interfaces as the 'self' IP,
without checking that the address corresponds with the
previously-detected gateway.

This behaviour was introduced by accident in aaf2df7, where we deleted
the following code:

    for _, prefix := range privatev4s {
        if prefix.Contains(gateway) && prefix.Contains(ip) {
            myIP = ip
            ok = true
            return
        }
    }

Other than checking that 'gateway' and 'ip' were private IP addresses
(which were correctly replaced with a call to the netip.Addr.IsPrivate
method), it also implicitly checked that both 'gateway' and 'ip' were a
part of the *same* prefix, and thus likely to be the same interface.

Restore that behaviour by explicitly checking pfx.Contains(gateway),
which, given that the 'ip' variable is derived from our prefix 'pfx',
ensures that the 'self' IP will correspond to the returned 'gateway'.

Fixes #10466

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Iddd2ee70cefb9fb40071986fefeace9ca2441ee6

net/interfaces/interfaces.go

@@ -557,6 +557,15 @@ func LikelyHomeRouterIP() (gateway, myIP netip.Addr, ok bool) {
 			// always return an IPv4 address.
 			return
 		}
+
+		// If this prefix ("interface") doesn't contain the gateway,
+		// then we skip it; this can happen if we have multiple valid
+		// interfaces and the interface with the route to the internet
+		// is ordered after another valid+running interface.
+		if !pfx.Contains(gateway) {
+			return
+		}
+
 		if gateway.IsPrivate() && ip.IsPrivate() {
 			myIP = ip
 			ok = true

net/interfaces/interfaces_test.go

@@ -125,6 +125,76 @@ func TestLikelyHomeRouterIP(t *testing.T) {
 	})
 }
 
+// https://github.com/tailscale/tailscale/issues/10466
+func TestLikelyHomeRouterIP_Prefix(t *testing.T) {
+	ipnet := func(s string) net.Addr {
+		ip, ipnet, err := net.ParseCIDR(s)
+		ipnet.IP = ip
+		if err != nil {
+			t.Fatal(err)
+		}
+		return ipnet
+	}
+
+	mockInterfaces := []Interface{
+		// Valid and running interface that doesn't have a route to the
+		// internet, and comes before the interface that does.
+		{
+			Interface: &net.Interface{
+				Index: 1,
+				MTU:   1500,
+				Name:  "docker0",
+				Flags: net.FlagUp |
+					net.FlagBroadcast |
+					net.FlagMulticast |
+					net.FlagRunning,
+			},
+			AltAddrs: []net.Addr{
+				ipnet("172.17.0.0/16"),
+			},
+		},
+
+		// Fake interface with a gateway to the internet.
+		{
+			Interface: &net.Interface{
+				Index: 2,
+				MTU:   1500,
+				Name:  "fake0",
+				Flags: net.FlagUp |
+					net.FlagBroadcast |
+					net.FlagMulticast |
+					net.FlagRunning,
+			},
+			AltAddrs: []net.Addr{
+				ipnet("192.168.7.100/24"),
+			},
+		},
+	}
+
+	// Mock out the responses from netInterfaces()
+	tstest.Replace(t, &altNetInterfaces, func() ([]Interface, error) {
+		return mockInterfaces, nil
+	})
+
+	// 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
+	})
+
+	gw, my, ok := LikelyHomeRouterIP()
+	if !ok {
+		t.Fatal("expected success")
+	}
+	t.Logf("myIP = %v; gw = %v", my, gw)
+
+	if want := netip.MustParseAddr("192.168.7.1"); gw != want {
+		t.Errorf("got gateway %v; want %v", gw, want)
+	}
+	if want := netip.MustParseAddr("192.168.7.100"); my != want {
+		t.Errorf("got self IP %v; want %v", my, want)
+	}
+}
+
 func TestLikelyHomeRouterIP_NoMocks(t *testing.T) {
 	// Verify that this works properly when called on a real live system,
 	// without any mocks.