prevent unnecessary Interface.SyncAddresses work; normalize IPNets in…

… deltaNets

Due to different representations of IPv4 addresses (4 byte vs 16 byte)
from different places, deltaNets was computing the wrong result and
SyncAddresses kept adding & removing the same Tailscale IP from the interface
each time, breaking all connections.

Updates tailscale/tailscale#565 (among many support emails)

interface.go

@@ -294,14 +294,17 @@ func (ifc *Interface) SetAddresses(addresses []*net.IPNet) error {
 	return nil
 }
 
-// Incrementally sets interface's unicase IP addresses.
-// This avoids the full FlushAddresses().
+// SyncAddresses incrementally sets the interface's unicast IP addresses,
+// doing the minimum number of AddAddresses & DeleteAddress calls.
+// This avoids the full FlushAddresses.
+//
+// Any IPv6 link-local addresses are not deleted.
 func (ifc *Interface) SyncAddresses(want []*net.IPNet) error {
 	var erracc error
 
 	got := ifc.UnicastIPNets
 	add, del := deltaNets(got, want)
-
+	del = excludeIPv6LinkLocal(del)
 	for _, a := range del {
 		err := ifc.DeleteAddress(&a.IP)
 		if err != nil {
@@ -347,14 +350,36 @@ func unicastAddressesToIPNets(a []*UnicastAddress) []*net.IPNet {
 	return out
 }
 
+// unwrapIP returns the shortest version of ip.
+func unwrapIP(ip net.IP) net.IP {
+	if ip4 := ip.To4(); ip4 != nil {
+		return ip4
+	}
+	return ip
+}
+
+func v4Mask(m net.IPMask) net.IPMask {
+	if len(m) == 16 {
+		return m[12:]
+	}
+	return m
+}
+
 func netCompare(a, b net.IPNet) int {
-	v := bytes.Compare(a.IP, b.IP)
+	aip, bip := unwrapIP(a.IP), unwrapIP(b.IP)
+	v := bytes.Compare(aip, bip)
 	if v != 0 {
 		return v
 	}
 
+	amask, bmask := a.Mask, b.Mask
+	if len(aip) == 4 {
+		amask = v4Mask(a.Mask)
+		bmask = v4Mask(b.Mask)
+	}
+
 	// narrower first
-	return -bytes.Compare(a.Mask, b.Mask)
+	return -bytes.Compare(amask, bmask)
 }
 
 func sortNets(a []*net.IPNet) {
@@ -363,6 +388,7 @@ func sortNets(a []*net.IPNet) {
 	})
 }
 
+// deltaNets returns the changes to turn a into b.
 func deltaNets(a, b []*net.IPNet) (add, del []*net.IPNet) {
 	add = make([]*net.IPNet, 0, len(b))
 	del = make([]*net.IPNet, 0, len(a))
@@ -394,6 +420,17 @@ func deltaNets(a, b []*net.IPNet) (add, del []*net.IPNet) {
 	return
 }
 
+func excludeIPv6LinkLocal(in []*net.IPNet) (out []*net.IPNet) {
+	out = in[:0]
+	for _, n := range in {
+		if len(n.IP) == 16 && n.IP.IsLinkLocalUnicast() {
+			continue
+		}
+		out = append(out, n)
+	}
+	return out
+}
+
 // Returns all the interface's routes. Corresponds to GetIpForwardTable2 function, but filtered by interface.
 // (https://docs.microsoft.com/en-us/windows/desktop/api/netioapi/nf-netioapi-getipforwardtable2)
 func (ifc *Interface) GetRoutes(family AddressFamily) ([]*Route, error) {

interface_test.go

@@ -8,6 +8,7 @@ package winipcfg
 import (
 	"fmt"
 	"net"
+	"strings"
 	"testing"
 	"time"
 )
@@ -247,35 +248,79 @@ func ipnet4(ip string, bits int) *net.IPNet {
 	}
 }
 
-func TestInterface_DeltaNets(t *testing.T) {
-	a := []*net.IPNet{
-		ipnet4("1.2.3.4", 24),
-		ipnet4("1.2.3.4", 31),
-		ipnet4("1.2.3.3", 32),
-		ipnet4("10.0.1.1", 32),
-		ipnet4("100.0.1.1", 32),
-	}
-	b := []*net.IPNet{
-		ipnet4("10.0.1.1", 32),
-		ipnet4("100.0.2.1", 32),
-		ipnet4("1.2.3.3", 32),
-		ipnet4("1.2.3.4", 24),
-	}
-	add, del := deltaNets(a, b)
-
-	expect_add := []*net.IPNet{
-		ipnet4("100.0.2.1", 32),
-	}
-	expect_del := []*net.IPNet{
-		ipnet4("1.2.3.4", 31),
-		ipnet4("100.0.1.1", 32),
+// each cidr can end in "[4]" to mean To4 form.
+func nets(cidrs ...string) (ret []*net.IPNet) {
+	for _, s := range cidrs {
+		to4 := strings.HasSuffix(s, "[4]")
+		if to4 {
+			s = strings.TrimSuffix(s, "[4]")
+		}
+		ip, ipNet, err := net.ParseCIDR(s)
+		if err != nil {
+			panic(fmt.Sprintf("Bogus CIDR %q in test", s))
+		}
+		if to4 {
+			ip = ip.To4()
+		}
+		ipNet.IP = ip
+		ret = append(ret, ipNet)
 	}
+	return
+}
 
-	if !equalNetIPs(expect_add, add) {
-		t.Errorf("add:\n  want: %v\n   got: %v\n", expect_add, add)
+func TestInterface_DeltaNets(t *testing.T) {
+	tests := []struct {
+		a, b             []*net.IPNet
+		wantAdd, wantDel []*net.IPNet
+	}{
+		{
+			a:       nets("1.2.3.4/24", "1.2.3.4/31", "1.2.3.3/32", "10.0.1.1/32", "100.0.1.1/32"),
+			b:       nets("10.0.1.1/32", "100.0.2.1/32", "1.2.3.3/32", "1.2.3.4/24"),
+			wantAdd: nets("100.0.2.1/32"),
+			wantDel: nets("1.2.3.4/31", "100.0.1.1/32"),
+		},
+		{
+			a:       nets("fe80::99d0:ec2d:b2e7:536b/64", "100.84.36.11/32"),
+			b:       nets("100.84.36.11/32"),
+			wantDel: nets("fe80::99d0:ec2d:b2e7:536b/64"),
+		},
+		{
+			a:       nets("100.84.36.11/32", "fe80::99d0:ec2d:b2e7:536b/64"),
+			b:       nets("100.84.36.11/32"),
+			wantDel: nets("fe80::99d0:ec2d:b2e7:536b/64"),
+		},
+		{
+			a:       nets("100.84.36.11/32", "fe80::99d0:ec2d:b2e7:536b/64"),
+			b:       nets("100.84.36.11/32[4]"),
+			wantDel: nets("fe80::99d0:ec2d:b2e7:536b/64"),
+		},
+		{
+			a: excludeIPv6LinkLocal(nets("100.84.36.11/32", "fe80::99d0:ec2d:b2e7:536b/64")),
+			b: nets("100.84.36.11/32"),
+		},
+		{
+			a: []*net.IPNet{
+				{
+					IP:   net.ParseIP("1.2.3.4"),
+					Mask: net.IPMask{0xff, 0xff, 0xff, 0xff},
+				},
+			},
+			b: []*net.IPNet{
+				{
+					IP:   net.ParseIP("1.2.3.4"),
+					Mask: net.IPMask{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
+				},
+			},
+		},
 	}
-	if !equalNetIPs(expect_del, del) {
-		t.Errorf("del:\n  want: %v\n   got: %v\n", expect_del, del)
+	for i, tt := range tests {
+		add, del := deltaNets(tt.a, tt.b)
+		if !equalNetIPs(add, tt.wantAdd) {
+			t.Errorf("[%d] add:\n  got: %v\n want: %v\n", i, add, tt.wantAdd)
+		}
+		if !equalNetIPs(del, tt.wantDel) {
+			t.Errorf("[%d] del:\n  got: %v\n want: %v\n", i, del, tt.wantDel)
+		}
 	}
 }