From c4ccdd1bd10a134471c46042f779e8576c3a4ed5 Mon Sep 17 00:00:00 2001
From: Andrew Dunham <andrew@du.nham.ca>
Date: Tue, 5 Dec 2023 09:44:32 -0500
Subject: [PATCH] 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      |  9 ++++
 net/interfaces/interfaces_test.go | 70 +++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)

diff --git a/net/interfaces/interfaces.go b/net/interfaces/interfaces.go
index 6fe24c651..d1fc3be56 100644
--- a/net/interfaces/interfaces.go
+++ b/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
diff --git a/net/interfaces/interfaces_test.go b/net/interfaces/interfaces_test.go
index 98448dbb2..40e9d42d2 100644
--- a/net/interfaces/interfaces_test.go
+++ b/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.