From 76256d22d865b6d003147af275e8c3973091901f Mon Sep 17 00:00:00 2001 From: James Tucker Date: Tue, 14 Jun 2022 11:16:01 -0700 Subject: [PATCH] wgengine/router: windows: set SkipAsSource on IPv6 LL addresses Link-local addresses on the Tailscale interface are not routable. Ideally they would be removed, however, a concern exists that the operating system will attempt to re-add them which would lead to thrashing. Setting SkipAsSource attempts to avoid production of packets using the address as a source in any default behaviors. Before, in powershell: `ping (hostname)` would ping the link-local address of the Tailscale interface, and fail. After: `ping (hostname)` now pings the link-local address on the next highest priority metric local interface. Fixes #4647 Signed-off-by: James Tucker --- wgengine/router/ifconfig_windows.go | 53 ++++++++++++++---------- wgengine/router/ifconfig_windows_test.go | 5 --- 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/wgengine/router/ifconfig_windows.go b/wgengine/router/ifconfig_windows.go index c5770afab..6e5c23907 100644 --- a/wgengine/router/ifconfig_windows.go +++ b/wgengine/router/ifconfig_windows.go @@ -574,26 +574,8 @@ 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() { - // Windows creates a fixed link-local address for wintun, - // which doesn't seem to route correctly. Unfortunately, LLMNR returns this - // address for lookups by the hostname, and Windows prefers using it. - // This means that local traffic addressed to the machine's hostname breaks. - // - // While we otherwise preserve link-local addresses, we delete - // this one to force lookups to use a working address. - // - // See: https://github.com/tailscale/tailscale/issues/4647 - if ip, ok := netaddr.FromStdIP(n.IP); !ok || wintunLinkLocal != ip { - continue // filter this IPNet - } - } - out = append(out, n) - } - return out +func isIPv6LinkLocal(in *net.IPNet) bool { + return len(in.IP) == 16 && in.IP.IsLinkLocalUnicast() } // ipAdapterUnicastAddressToIPNet converts windows.IpAdapterUnicastAddress to net.IPNet. @@ -622,14 +604,27 @@ func unicastIPNets(ifc *winipcfg.IPAdapterAddresses) []*net.IPNet { // doing the minimum number of AddAddresses & DeleteAddress calls. // This avoids the full FlushAddresses. // -// Any IPv6 link-local addresses are not deleted. +// Any IPv6 link-local addresses are not deleted out of caution as some +// configurations may repeatedly re-add them. Link-local addresses are adjusted +// to set SkipAsSource. SkipAsSource prevents the addresses from being addded to +// DNS locally or remotely and from being picked as a source address for +// outgoing packets with unspecified sources. See #4647 and +// https://web.archive.org/web/20200912120956/https://devblogs.microsoft.com/scripting/use-powershell-to-change-ip-behavior-with-skipassource/ func syncAddresses(ifc *winipcfg.IPAdapterAddresses, want []*net.IPNet) error { var erracc error got := unicastIPNets(ifc) add, del := deltaNets(got, want) - del = excludeIPv6LinkLocal(del) + + ll := make([]*net.IPNet, 0) for _, a := range del { + // do not delete link-local addresses, and collect them for later + // applying SkipAsSource. + if isIPv6LinkLocal(a) { + ll = append(ll, a) + continue + } + err := ifc.LUID.DeleteIPAddress(*a) if err != nil { erracc = fmt.Errorf("deleting IP %q: %w", *a, err) @@ -643,6 +638,20 @@ func syncAddresses(ifc *winipcfg.IPAdapterAddresses, want []*net.IPNet) error { } } + for _, a := range ll { + mib, err := ifc.LUID.IPAddress(a.IP) + if err != nil { + erracc = fmt.Errorf("setting skip-as-source on IP %q: unable to retrieve MIB: %w", *a, err) + continue + } + if !mib.SkipAsSource { + mib.SkipAsSource = true + if err := mib.Set(); err != nil { + erracc = fmt.Errorf("setting skip-as-source on IP %q: unable to set MIB: %w", *a, err) + } + } + } + return erracc } diff --git a/wgengine/router/ifconfig_windows_test.go b/wgengine/router/ifconfig_windows_test.go index 0b64d44ec..d6d8cc8ff 100644 --- a/wgengine/router/ifconfig_windows_test.go +++ b/wgengine/router/ifconfig_windows_test.go @@ -163,11 +163,6 @@ func TestDeltaNets(t *testing.T) { 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"), - wantDel: nets("fe80::99d0:ec2d:b2e7:536b/64"), - }, { a: []*net.IPNet{ {