From 3528d28ed1cb2243807f2f45a594c12d7ca7f39c Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 22 Sep 2020 09:13:45 -0700 Subject: [PATCH] wgengine/router: move Tailscale's winipcfg additions into wgengine/router Part of unforking our winipcfg-go and using upstream (#760), move our additions into our repo. (We might upstream them later if upstream has interest) Originally these were: @apenwarr: "Add ifc.SyncAddresses() and SyncRoutes()." https://github.com/tailscale/winipcfg-go/commit/609dcf2df55fbb76effd430a51b9757676853390 @bradfitz: "winipcfg: make Interface.AddRoutes do as much as possible, return combined error" https://github.com/tailscale/winipcfg-go/commit/e9f93d53f33a7d2cadbdfa822761b8251af4d4bd @bradfitz: "prevent unnecessary Interface.SyncAddresses work; normalize IPNets in deltaNets" https://github.com/tailscale/winipcfg-go/commit/decb9ee8e17028db14c6951413cdd34ab46efca4 --- cmd/tailscale/depaware.txt | 2 +- cmd/tailscaled/depaware.txt | 2 +- wgengine/router/ifconfig_windows.go | 216 ++++++++++++++++++++++- wgengine/router/ifconfig_windows_test.go | 143 +++++++++++++++ 4 files changed, 359 insertions(+), 4 deletions(-) diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index 4e268d38f..96c8d09b1 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -5,7 +5,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep github.com/apenwarr/fixconsole from tailscale.com/cmd/tailscale W 💣 github.com/apenwarr/w32 from github.com/apenwarr/fixconsole L github.com/coreos/go-iptables/iptables from tailscale.com/wgengine/router - L github.com/go-multierror/multierror from tailscale.com/wgengine/router + LW github.com/go-multierror/multierror from tailscale.com/wgengine/router W 💣 github.com/go-ole/go-ole from github.com/go-ole/go-ole/oleutil+ W 💣 github.com/go-ole/go-ole/oleutil from tailscale.com/wgengine/winnet L 💣 github.com/godbus/dbus/v5 from tailscale.com/wgengine/router/dns diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 74bc04ce0..bf6772000 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -5,7 +5,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de github.com/apenwarr/fixconsole from tailscale.com/cmd/tailscaled W 💣 github.com/apenwarr/w32 from github.com/apenwarr/fixconsole L github.com/coreos/go-iptables/iptables from tailscale.com/wgengine/router - L github.com/go-multierror/multierror from tailscale.com/wgengine/router + LW github.com/go-multierror/multierror from tailscale.com/wgengine/router W 💣 github.com/go-ole/go-ole from github.com/go-ole/go-ole/oleutil+ W 💣 github.com/go-ole/go-ole/oleutil from tailscale.com/wgengine/winnet L 💣 github.com/godbus/dbus/v5 from tailscale.com/wgengine/router/dns diff --git a/wgengine/router/ifconfig_windows.go b/wgengine/router/ifconfig_windows.go index 67030ef74..36f7ba9cc 100644 --- a/wgengine/router/ifconfig_windows.go +++ b/wgengine/router/ifconfig_windows.go @@ -15,6 +15,7 @@ import ( "sort" "time" + "github.com/go-multierror/multierror" ole "github.com/go-ole/go-ole" winipcfg "github.com/tailscale/winipcfg-go" "github.com/tailscale/wireguard-go/tun" @@ -307,7 +308,7 @@ func configureInterface(cfg *Config, tun *tun.NativeTun) error { routes = append(routes, r) } - err = iface.SyncAddresses(addresses) + err = syncAddresses(iface, addresses) if err != nil { return err } @@ -327,7 +328,7 @@ func configureInterface(cfg *Config, tun *tun.NativeTun) error { } var errAcc error - err = iface.SyncRoutes(deduplicatedRoutes) + err = syncRoutes(iface, deduplicatedRoutes) if err != nil && errAcc == nil { log.Printf("setroutes: %v", err) errAcc = err @@ -412,3 +413,214 @@ func routeLess(ri, rj *winipcfg.RouteData) bool { } return false } + +// 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 { + 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(amask, bmask) +} + +func sortNets(a []*net.IPNet) { + sort.Slice(a, func(i, j int) bool { + return netCompare(*a[i], *a[j]) == -1 + }) +} + +// 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)) + sortNets(a) + sortNets(b) + + i := 0 + j := 0 + for i < len(a) && j < len(b) { + switch netCompare(*a[i], *b[j]) { + case -1: + // a < b, delete + del = append(del, a[i]) + i++ + case 0: + // a == b, no diff + i++ + j++ + case 1: + // a > b, add missing entry + add = append(add, b[j]) + j++ + default: + panic("unexpected compare result") + } + } + del = append(del, a[i:]...) + add = append(add, b[j:]...) + 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 +} + +// 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 syncAddresses(ifc *winipcfg.Interface, 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 { + erracc = err + } + } + + err := ifc.AddAddresses(add) + if err != nil { + erracc = err + } + + ifc.UnicastIPNets = make([]*net.IPNet, len(want)) + copy(ifc.UnicastIPNets, want) + return erracc +} + +func routeDataCompare(a, b *winipcfg.RouteData) int { + v := bytes.Compare(a.Destination.IP, b.Destination.IP) + if v != 0 { + return v + } + + // Narrower masks first + v = bytes.Compare(a.Destination.Mask, b.Destination.Mask) + if v != 0 { + return -v + } + + // No nexthop before non-empty nexthop + v = bytes.Compare(a.NextHop, b.NextHop) + if v != 0 { + return v + } + + // Lower metrics first + if a.Metric < b.Metric { + return -1 + } else if a.Metric > b.Metric { + return 1 + } + + return 0 +} + +func sortRouteData(a []*winipcfg.RouteData) { + sort.Slice(a, func(i, j int) bool { + return routeDataCompare(a[i], a[j]) < 0 + }) +} + +func deltaRouteData(a, b []*winipcfg.RouteData) (add, del []*winipcfg.RouteData) { + add = make([]*winipcfg.RouteData, 0, len(b)) + del = make([]*winipcfg.RouteData, 0, len(a)) + sortRouteData(a) + sortRouteData(b) + + i := 0 + j := 0 + for i < len(a) && j < len(b) { + switch routeDataCompare(a[i], b[j]) { + case -1: + // a < b, delete + del = append(del, a[i]) + i++ + case 0: + // a == b, no diff + i++ + j++ + case 1: + // a > b, add missing entry + add = append(add, b[j]) + j++ + default: + panic("unexpected compare result") + } + } + del = append(del, a[i:]...) + add = append(add, b[j:]...) + return +} + +// syncRoutes incrementally sets multiples routes on an interface. +// This avoids a full ifc.FlushRoutes call. +func syncRoutes(ifc *winipcfg.Interface, want []*winipcfg.RouteData) error { + routes, err := ifc.GetRoutes(windows.AF_INET) + if err != nil { + return err + } + + got := make([]*winipcfg.RouteData, 0, len(routes)) + for _, r := range routes { + v, err := r.ToRouteData() + if err != nil { + return err + } + got = append(got, v) + } + + add, del := deltaRouteData(got, want) + + var errs []error + for _, a := range del { + err := ifc.DeleteRoute(&a.Destination, &a.NextHop) + if err != nil { + errs = append(errs, fmt.Errorf("deleting route %v: %w", a.Destination, err)) + } + } + + for _, a := range add { + err := ifc.AddRoute(a) + if err != nil { + errs = append(errs, fmt.Errorf("adding route %v: %w", a.Destination, err)) + } + } + + return multierror.New(errs) +} diff --git a/wgengine/router/ifconfig_windows_test.go b/wgengine/router/ifconfig_windows_test.go index d8e0852e8..b43c5d91c 100644 --- a/wgengine/router/ifconfig_windows_test.go +++ b/wgengine/router/ifconfig_windows_test.go @@ -5,8 +5,10 @@ package router import ( + "fmt" "math/rand" "net" + "strings" "testing" winipcfg "github.com/tailscale/winipcfg-go" @@ -95,3 +97,144 @@ func TestRouteLessConsistent(t *testing.T) { } } } + +func equalNetIPs(a, b []*net.IPNet) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if netCompare(*a[i], *b[i]) != 0 { + return false + } + } + return true +} + +func ipnet4(ip string, bits int) *net.IPNet { + return &net.IPNet{ + IP: net.ParseIP(ip), + Mask: net.CIDRMask(bits, 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 +} + +func TestDeltaNets(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}, + }, + }, + }, + } + 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) + } + } +} + +func equalRouteDatas(a, b []*winipcfg.RouteData) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if routeDataCompare(a[i], b[i]) != 0 { + return false + } + } + return true +} + +func TestDeltaRouteData(t *testing.T) { + var h0 net.IP + h1 := net.ParseIP("99.99.99.99") + h2 := net.ParseIP("99.99.9.99") + + a := []*winipcfg.RouteData{ + {*ipnet4("1.2.3.4", 32), h0, 1}, + {*ipnet4("1.2.3.4", 24), h1, 2}, + {*ipnet4("1.2.3.4", 24), h2, 1}, + {*ipnet4("1.2.3.5", 32), h0, 1}, + } + b := []*winipcfg.RouteData{ + {*ipnet4("1.2.3.5", 32), h0, 1}, + {*ipnet4("1.2.3.4", 24), h1, 2}, + {*ipnet4("1.2.3.4", 24), h2, 2}, + } + add, del := deltaRouteData(a, b) + + wantAdd := []*winipcfg.RouteData{ + {*ipnet4("1.2.3.4", 24), h2, 2}, + } + wantDel := []*winipcfg.RouteData{ + {*ipnet4("1.2.3.4", 32), h0, 1}, + {*ipnet4("1.2.3.4", 24), h2, 1}, + } + + if !equalRouteDatas(add, wantAdd) { + t.Errorf("add:\n got: %v\n want: %v\n", add, wantAdd) + } + if !equalRouteDatas(del, wantDel) { + t.Errorf("del:\n got: %v\n want: %v\n", del, wantDel) + } +}