ipn/ipnlocal: use netaddr.IPSetBuilder when constructing list of interface IPPrefixes.

Signed-off-by: Maisem Ali <maisem@tailscale.com>

ipn/ipnlocal/local.go

@@ -1022,7 +1022,18 @@ var removeFromDefaultRoute = []netaddr.IPPrefix{
 // Given that "internal" routes don't leave the device, we choose to
 // trust them more, allowing access to them when an Exit Node is enabled.
 func internalAndExternalInterfaces() (internal, external []netaddr.IPPrefix, err error) {
-	if err := interfaces.ForeachInterfaceAddress(func(iface interfaces.Interface, pfx netaddr.IPPrefix) {
+	il, err := interfaces.GetList()
+	if err != nil {
+		return nil, nil, err
+	}
+	return internalAndExternalInterfacesFrom(il, runtime.GOOS)
+}
+
+func internalAndExternalInterfacesFrom(il interfaces.List, goos string) (internal, external []netaddr.IPPrefix, err error) {
+	// We use an IPSetBuilder here to canonicalize the prefixes
+	// and to remove any duplicate entries.
+	var internalBuilder, externalBuilder netaddr.IPSetBuilder
+	if err := il.ForeachInterfaceAddress(func(iface interfaces.Interface, pfx netaddr.IPPrefix) {
 		if tsaddr.IsTailscaleIP(pfx.IP()) {
 			return
 		}
@@ -1030,10 +1041,10 @@ func internalAndExternalInterfaces() (internal, external []netaddr.IPPrefix, err
 			return
 		}
 		if iface.IsLoopback() {
-			internal = append(internal, pfx)
+			internalBuilder.AddPrefix(pfx)
 			return
 		}
-		if runtime.GOOS == "windows" {
+		if goos == "windows" {
 			// Windows Hyper-V prefixes all MAC addresses with 00:15:5d.
 			// https://docs.microsoft.com/en-us/troubleshoot/windows-server/virtualization/default-limit-256-dynamic-mac-addresses
 			//
@@ -1044,16 +1055,24 @@ func internalAndExternalInterfaces() (internal, external []netaddr.IPPrefix, err
 			// configuration breaks WSL2 DNS without this.
 			mac := iface.Interface.HardwareAddr
 			if len(mac) == 6 && mac[0] == 0x00 && mac[1] == 0x15 && mac[2] == 0x5d {
-				internal = append(internal, pfx)
+				internalBuilder.AddPrefix(pfx)
 				return
 			}
 		}
-		external = append(external, pfx)
+		externalBuilder.AddPrefix(pfx)
 	}); err != nil {
 		return nil, nil, err
 	}
+	iSet, err := internalBuilder.IPSet()
+	if err != nil {
+		return nil, nil, err
+	}
+	eSet, err := externalBuilder.IPSet()
+	if err != nil {
+		return nil, nil, err
+	}
 
-	return internal, external, nil
+	return iSet.Prefixes(), eSet.Prefixes(), nil
 }
 
 func interfaceRoutes() (ips *netaddr.IPSet, hostIPs []netaddr.IP, err error) {

ipn/ipnlocal/local_test.go

@@ -6,6 +6,7 @@ package ipnlocal
 
 import (
 	"fmt"
+	"net"
 	"net/http"
 	"reflect"
 	"testing"
@@ -494,3 +495,103 @@ func TestFileTargets(t *testing.T) {
 	}
 	// (other cases handled by TestPeerAPIBase above)
 }
+
+func TestInternalAndExternalInterfaces(t *testing.T) {
+	type interfacePrefix struct {
+		i   interfaces.Interface
+		pfx netaddr.IPPrefix
+	}
+
+	masked := func(ips ...interfacePrefix) (pfxs []netaddr.IPPrefix) {
+		for _, ip := range ips {
+			pfxs = append(pfxs, ip.pfx.Masked())
+		}
+		return pfxs
+	}
+	iList := func(ips ...interfacePrefix) (il interfaces.List) {
+		for _, ip := range ips {
+			il = append(il, ip.i)
+		}
+		return il
+	}
+	newInterface := func(name, pfx string, wsl2, loopback bool) interfacePrefix {
+		ippfx := netaddr.MustParseIPPrefix(pfx)
+		ip := interfaces.Interface{
+			Interface: &net.Interface{},
+			AltAddrs: []net.Addr{
+				ippfx.IPNet(),
+			},
+		}
+		if loopback {
+			ip.Flags = net.FlagLoopback
+		}
+		if wsl2 {
+			ip.HardwareAddr = []byte{0x00, 0x15, 0x5d, 0x00, 0x00, 0x00}
+		}
+		return interfacePrefix{i: ip, pfx: ippfx}
+	}
+	var (
+		en0      = newInterface("en0", "10.20.2.5/16", false, false)
+		en1      = newInterface("en1", "192.168.1.237/24", false, false)
+		wsl      = newInterface("wsl", "192.168.5.34/24", true, false)
+		loopback = newInterface("lo0", "127.0.0.1/8", false, true)
+	)
+
+	tests := []struct {
+		name    string
+		goos    string
+		il      interfaces.List
+		wantInt []netaddr.IPPrefix
+		wantExt []netaddr.IPPrefix
+	}{
+		{
+			name: "single-interface",
+			goos: "linux",
+			il: iList(
+				en0,
+				loopback,
+			),
+			wantInt: masked(loopback),
+			wantExt: masked(en0),
+		},
+		{
+			name: "multiple-interfaces",
+			goos: "linux",
+			il: iList(
+				en0,
+				en1,
+				wsl,
+				loopback,
+			),
+			wantInt: masked(loopback),
+			wantExt: masked(en0, en1, wsl),
+		},
+		{
+			name: "wsl2",
+			goos: "windows",
+			il: iList(
+				en0,
+				en1,
+				wsl,
+				loopback,
+			),
+			wantInt: masked(loopback, wsl),
+			wantExt: masked(en0, en1),
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			gotInt, gotExt, err := internalAndExternalInterfacesFrom(tc.il, tc.goos)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if !reflect.DeepEqual(gotInt, tc.wantInt) {
+				t.Errorf("unexpected internal prefixes\ngot %v\nwant %v", gotInt, tc.wantInt)
+			}
+			if !reflect.DeepEqual(gotExt, tc.wantExt) {
+				t.Errorf("unexpected external prefixes\ngot %v\nwant %v", gotExt, tc.wantExt)
+			}
+		})
+	}
+}