From 8f5b52e5715f9216a3a9574df7ee8576d02c65b1 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Thu, 10 Sep 2020 18:40:02 +0000 Subject: [PATCH] net/netns: add windows support. Also remove rebinding logic from the windows router. Magicsock will instead rebind based on link change signals. Signed-off-by: David Anderson --- cmd/tailscale/depaware.txt | 7 +- cmd/tailscaled/depaware.txt | 7 +- control/controlclient/netmap.go | 5 - ipn/local.go | 6 - net/interfaces/interfaces_windows.go | 21 ++++ net/netns/netns_default.go | 2 +- net/netns/netns_windows.go | 123 +++++++++++++++++++++ tsconst/interface.go | 11 ++ wgengine/router/ifconfig_windows.go | 158 ++++++++++++--------------- wgengine/router/router_windows.go | 6 +- 10 files changed, 235 insertions(+), 111 deletions(-) create mode 100644 net/netns/netns_windows.go create mode 100644 tsconst/interface.go diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index 7eecaafd2..2be5a346a 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -15,7 +15,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep L 💣 github.com/mdlayher/netlink/nlenc from github.com/jsimonetti/rtnetlink+ github.com/peterbourgon/ff/v2 from github.com/peterbourgon/ff/v2/ffcli github.com/peterbourgon/ff/v2/ffcli from tailscale.com/cmd/tailscale/cli - W 💣 github.com/tailscale/winipcfg-go from tailscale.com/wgengine/router + W 💣 github.com/tailscale/winipcfg-go from tailscale.com/net/interfaces+ 💣 github.com/tailscale/wireguard-go/conn from github.com/tailscale/wireguard-go/device+ 💣 github.com/tailscale/wireguard-go/device from tailscale.com/wgengine+ github.com/tailscale/wireguard-go/device/tokenbucket from github.com/tailscale/wireguard-go/device @@ -55,7 +55,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep tailscale.com/net/dnscache from tailscale.com/cmd/tailscale/cli+ tailscale.com/net/interfaces from tailscale.com/cmd/tailscale/cli+ tailscale.com/net/netcheck from tailscale.com/cmd/tailscale/cli+ - tailscale.com/net/netns from tailscale.com/control/controlclient+ + 💣 tailscale.com/net/netns from tailscale.com/control/controlclient+ tailscale.com/net/stun from tailscale.com/net/netcheck+ tailscale.com/net/tlsdial from tailscale.com/control/controlclient+ tailscale.com/net/tsaddr from tailscale.com/ipn+ @@ -66,6 +66,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep 💣 tailscale.com/syncs from tailscale.com/net/interfaces+ tailscale.com/tailcfg from tailscale.com/cmd/tailscale/cli+ DW tailscale.com/tempfork/osexec from tailscale.com/portlist + W tailscale.com/tsconst from tailscale.com/net/interfaces tailscale.com/types/empty from tailscale.com/control/controlclient+ tailscale.com/types/key from tailscale.com/cmd/tailscale/cli+ tailscale.com/types/logger from tailscale.com/cmd/tailscale/cli+ @@ -81,7 +82,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep tailscale.com/wgengine/magicsock from tailscale.com/wgengine 💣 tailscale.com/wgengine/monitor from tailscale.com/cmd/tailscale/cli+ tailscale.com/wgengine/packet from tailscale.com/wgengine+ - 💣 tailscale.com/wgengine/router from tailscale.com/cmd/tailscale/cli+ + tailscale.com/wgengine/router from tailscale.com/cmd/tailscale/cli+ 💣 tailscale.com/wgengine/router/dns from tailscale.com/ipn+ tailscale.com/wgengine/tsdns from tailscale.com/ipn+ tailscale.com/wgengine/tstun from tailscale.com/wgengine diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index aa3e947fc..496e6910e 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -19,7 +19,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de L 💣 github.com/mdlayher/netlink from github.com/jsimonetti/rtnetlink+ L 💣 github.com/mdlayher/netlink/nlenc from github.com/jsimonetti/rtnetlink+ github.com/pborman/getopt/v2 from tailscale.com/cmd/tailscaled - W 💣 github.com/tailscale/winipcfg-go from tailscale.com/wgengine/router + W 💣 github.com/tailscale/winipcfg-go from tailscale.com/net/interfaces+ 💣 github.com/tailscale/wireguard-go/conn from github.com/tailscale/wireguard-go/device+ 💣 github.com/tailscale/wireguard-go/device from tailscale.com/wgengine+ github.com/tailscale/wireguard-go/device/tokenbucket from github.com/tailscale/wireguard-go/device @@ -60,7 +60,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/net/dnscache from tailscale.com/derp/derphttp+ tailscale.com/net/interfaces from tailscale.com/ipn+ tailscale.com/net/netcheck from tailscale.com/wgengine/magicsock - tailscale.com/net/netns from tailscale.com/control/controlclient+ + 💣 tailscale.com/net/netns from tailscale.com/control/controlclient+ 💣 tailscale.com/net/netstat from tailscale.com/ipn/ipnserver tailscale.com/net/stun from tailscale.com/net/netcheck+ tailscale.com/net/tlsdial from tailscale.com/control/controlclient+ @@ -73,6 +73,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de 💣 tailscale.com/syncs from tailscale.com/net/interfaces+ tailscale.com/tailcfg from tailscale.com/control/controlclient+ DW tailscale.com/tempfork/osexec from tailscale.com/portlist + W tailscale.com/tsconst from tailscale.com/net/interfaces tailscale.com/types/empty from tailscale.com/control/controlclient+ tailscale.com/types/key from tailscale.com/derp+ tailscale.com/types/logger from tailscale.com/cmd/tailscaled+ @@ -89,7 +90,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/wgengine/magicsock from tailscale.com/cmd/tailscaled+ 💣 tailscale.com/wgengine/monitor from tailscale.com/wgengine tailscale.com/wgengine/packet from tailscale.com/wgengine+ - 💣 tailscale.com/wgengine/router from tailscale.com/cmd/tailscaled+ + tailscale.com/wgengine/router from tailscale.com/cmd/tailscaled+ 💣 tailscale.com/wgengine/router/dns from tailscale.com/ipn+ tailscale.com/wgengine/tsdns from tailscale.com/ipn+ tailscale.com/wgengine/tstun from tailscale.com/wgengine diff --git a/control/controlclient/netmap.go b/control/controlclient/netmap.go index 422feafbf..2f7f39a5b 100644 --- a/control/controlclient/netmap.go +++ b/control/controlclient/netmap.go @@ -219,7 +219,6 @@ func (nm *NetworkMap) JSON() string { AllowSingleHosts WGConfigFlags = 1 << iota AllowSubnetRoutes AllowDefaultRoute - HackDefaultRoute ) // EndpointDiscoSuffix is appended to the hex representation of a peer's discovery key @@ -274,10 +273,6 @@ func (nm *NetworkMap) WGCfg(logf logger.Logf, flags WGConfigFlags) (*wgcfg.Confi logf("wgcfg: %v skipping default route", peer.Key.ShortString()) continue } - if (flags & HackDefaultRoute) != 0 { - allowedIP = wgcfg.CIDR{IP: wgcfg.IPv4(10, 0, 0, 0), Mask: 8} - logf("wgcfg: %v converting default route => %v", peer.Key.ShortString(), allowedIP.String()) - } } else if allowedIP.Mask < 32 { if (flags & AllowSubnetRoutes) == 0 { logf("wgcfg: %v skipping subnet route", peer.Key.ShortString()) diff --git a/ipn/local.go b/ipn/local.go index 1d61ffa25..2e67f0ac7 100644 --- a/ipn/local.go +++ b/ipn/local.go @@ -953,12 +953,6 @@ func (b *LocalBackend) authReconfig() { flags |= controlclient.AllowDefaultRoute // TODO(apenwarr): Make subnet routes a different pref? flags |= controlclient.AllowSubnetRoutes - // TODO(apenwarr): Remove this once we sort out subnet routes. - // Right now default routes are broken in Windows, but - // controlclient doesn't properly send subnet routes. So - // let's convert a default route into a subnet route in order - // to allow experimentation. - flags |= controlclient.HackDefaultRoute } if uc.AllowSingleHosts { flags |= controlclient.AllowSingleHosts diff --git a/net/interfaces/interfaces_windows.go b/net/interfaces/interfaces_windows.go index 5eae126d2..97b250e52 100644 --- a/net/interfaces/interfaces_windows.go +++ b/net/interfaces/interfaces_windows.go @@ -8,8 +8,10 @@ "os/exec" "syscall" + "github.com/tailscale/winipcfg-go" "go4.org/mem" "inet.af/netaddr" + "tailscale.com/tsconst" "tailscale.com/util/lineread" ) @@ -71,3 +73,22 @@ func likelyHomeRouterIPWindows() (ret netaddr.IP, ok bool) { }) return ret, !ret.IsZero() } + +// NonTailscaleMTUs returns a map of interface LUID to interface MTU, +// for all interfaces except Tailscale tunnels. +func NonTailscaleMTUs() (map[uint64]uint32, error) { + ifs, err := winipcfg.GetInterfaces() + if err != nil { + return nil, err + } + + ret := map[uint64]uint32{} + for _, iface := range ifs { + if iface.Description == tsconst.WintunInterfaceDesc { + continue + } + ret[iface.Luid] = iface.Mtu + } + + return ret, nil +} diff --git a/net/netns/netns_default.go b/net/netns/netns_default.go index 7de7d9fe8..e794fccb7 100644 --- a/net/netns/netns_default.go +++ b/net/netns/netns_default.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// +build !linux +// +build !linux,!windows package netns diff --git a/net/netns/netns_windows.go b/net/netns/netns_windows.go new file mode 100644 index 000000000..b67cac6a6 --- /dev/null +++ b/net/netns/netns_windows.go @@ -0,0 +1,123 @@ +// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package netns + +import ( + "encoding/binary" + "syscall" + "unsafe" + + "github.com/tailscale/winipcfg-go" + "golang.org/x/sys/windows" + "tailscale.com/net/interfaces" +) + +// control binds c to the Windows interface that holds a default +// route, and is not the Tailscale WinTun interface. +func control(network, address string, c syscall.RawConn) error { + canV4, canV6 := false, false + switch network { + case "tcp", "udp": + canV4, canV6 = true, true + case "tcp4", "udp4": + canV4 = true + case "tcp6", "udp6": + canV6 = true + } + + if canV4 { + if4, err := getDefaultInterface(winipcfg.AF_INET) + if err != nil { + return err + } + if err := bindSocket4(c, if4); err != nil { + return err + } + } + + if canV6 { + if6, err := getDefaultInterface(winipcfg.AF_INET6) + if err != nil { + return err + } + if err := bindSocket6(c, if6); err != nil { + return err + } + } + + return nil +} + +// getDefaultInterface returns the index of the interface that has the +// non-Tailscale default route for the given address family. +func getDefaultInterface(family winipcfg.AddressFamily) (ifidx uint32, err error) { + ifs, err := interfaces.NonTailscaleMTUs() + if err != nil { + return 0, err + } + + routes, err := winipcfg.GetRoutes(family) + if err != nil { + return 0, err + } + + bestMetric := ^uint32(0) + // The zero index means "unspecified". If we end up passing zero + // to bindSocket*(), it unsets the binding and lets the socket + // behave as normal again, which is what we want if there's no + // default route we can use. + var index uint32 + for _, route := range routes { + if route.DestinationPrefix.PrefixLength != 0 || ifs[route.InterfaceLuid] == 0 { + continue + } + if route.Metric < bestMetric { + bestMetric = route.Metric + index = route.InterfaceIndex + } + } + + return index, nil +} + +const sockoptBoundInterface = 31 + +// bindSocket4 binds the given RawConn to the network interface with +// index ifidx, for IPv4 traffic only. +func bindSocket4(c syscall.RawConn, ifidx uint32) error { + // For v4 the interface index must be passed as a big-endian + // integer, regardless of platform endianness. + index := nativeToBigEndian(ifidx) + var controlErr error + err := c.Control(func(fd uintptr) { + controlErr = windows.SetsockoptInt(windows.Handle(fd), windows.IPPROTO_IP, sockoptBoundInterface, int(index)) + }) + if err != nil { + return err + } + return controlErr +} + +// bindSocket6 binds the given RawConn to the network interface with +// index ifidx, for IPv6 traffic only. +func bindSocket6(c syscall.RawConn, ifidx uint32) error { + var controlErr error + err := c.Control(func(fd uintptr) { + controlErr = windows.SetsockoptInt(windows.Handle(fd), windows.IPPROTO_IPV6, sockoptBoundInterface, int(ifidx)) + }) + if err != nil { + return err + } + return controlErr +} + +// nativeToBigEndian returns i converted into big-endian +// representation, suitable for passing to Windows APIs that require a +// mangled uint32. +func nativeToBigEndian(i uint32) uint32 { + var b [4]byte + binary.BigEndian.PutUint32(b[:], i) + return *(*uint32)(unsafe.Pointer(&b[0])) +} diff --git a/tsconst/interface.go b/tsconst/interface.go new file mode 100644 index 000000000..ae68e7355 --- /dev/null +++ b/tsconst/interface.go @@ -0,0 +1,11 @@ +// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package tsconst exports some constants used elsewhere in the +// codebase. +package tsconst + +// WintunInterfaceDesc is the description attached to Tailscale +// interfaces on Windows. This is set by our modified WinTun driver. +const WintunInterfaceDesc = "Tailscale Tunnel" diff --git a/wgengine/router/ifconfig_windows.go b/wgengine/router/ifconfig_windows.go index d6e101e42..83f689e78 100644 --- a/wgengine/router/ifconfig_windows.go +++ b/wgengine/router/ifconfig_windows.go @@ -7,115 +7,48 @@ import ( "bytes" - "encoding/binary" "errors" "fmt" "log" "net" "sort" "time" - "unsafe" ole "github.com/go-ole/go-ole" winipcfg "github.com/tailscale/winipcfg-go" - "github.com/tailscale/wireguard-go/conn" - "github.com/tailscale/wireguard-go/device" "github.com/tailscale/wireguard-go/tun" "golang.org/x/sys/windows" + "tailscale.com/net/interfaces" "tailscale.com/wgengine/winnet" ) -const ( - sockoptIP_UNICAST_IF = 31 - sockoptIPV6_UNICAST_IF = 31 -) - -func htonl(val uint32) uint32 { - bytes := make([]byte, 4) - binary.BigEndian.PutUint32(bytes, val) - return *(*uint32)(unsafe.Pointer(&bytes[0])) -} - -func bindSocketRoute(family winipcfg.AddressFamily, device *device.Device, ourLuid uint64, lastLuid *uint64) error { - routes, err := winipcfg.GetRoutes(family) - if err != nil { - return err - } - lowestMetric := ^uint32(0) - index := uint32(0) // Zero is "unspecified", which for IP_UNICAST_IF resets the value, which is what we want. - luid := uint64(0) // Hopefully luid zero is unspecified, but hard to find docs saying so. - for _, route := range routes { - if route.DestinationPrefix.PrefixLength != 0 || route.InterfaceLuid == ourLuid { - continue - } - if route.Metric < lowestMetric { - lowestMetric = route.Metric - index = route.InterfaceIndex - luid = route.InterfaceLuid - } - } - if luid == *lastLuid { - return nil - } - *lastLuid = luid - if false { - bind, ok := device.Bind().(conn.BindSocketToInterface) - if !ok { - return fmt.Errorf("unexpected device.Bind type %T", device.Bind()) - } - // TODO(apenwarr): doesn't work with magic socket yet. - if family == winipcfg.AF_INET { - return bind.BindSocketToInterface4(index, false) - } else if family == winipcfg.AF_INET6 { - return bind.BindSocketToInterface6(index, false) - } - } else { - log.Printf("WARNING: skipping windows socket binding.") - } - return nil -} - -func monitorDefaultRoutes(device *device.Device, autoMTU bool, tun *tun.NativeTun) (*winipcfg.RouteChangeCallback, error) { +// monitorDefaultRoutes subscribes to route change events and updates +// the Tailscale tunnel interface's MTU to match that of the +// underlying default route. +// +// This is an attempt at making the MTU mostly correct, but in +// practice this entire piece of code ends up just using the 1280 +// value passed in at device construction time. This code might make +// the MTU go lower due to very low-MTU IPv4 interfaces. +// +// TODO: this code is insufficient to control the MTU correctly. The +// correct way to do it is per-peer PMTU discovery, and synthesizing +// ICMP fragmentation-needed messages within tailscaled. This code may +// address a few rare corner cases, but is unlikely to significantly +// help with MTU issues compared to a static 1280B implementation. +func monitorDefaultRoutes(tun *tun.NativeTun) (*winipcfg.RouteChangeCallback, error) { guid := tun.GUID() ourLuid, err := winipcfg.InterfaceGuidToLuid(&guid) - lastLuid4 := uint64(0) - lastLuid6 := uint64(0) lastMtu := uint32(0) if err != nil { return nil, err } doIt := func() error { - err = bindSocketRoute(winipcfg.AF_INET, device, ourLuid, &lastLuid4) + mtu, err := getDefaultRouteMTU() if err != nil { return err } - err = bindSocketRoute(winipcfg.AF_INET6, device, ourLuid, &lastLuid6) - if err != nil { - log.Printf("bindSocketRoute(AF_INET6): %v", err) - return err - } - if !autoMTU { - return nil - } - mtu := uint32(0) - if lastLuid4 != 0 { - iface, err := winipcfg.InterfaceFromLUID(lastLuid4) - if err != nil { - return err - } - if iface.Mtu > 0 { - mtu = iface.Mtu - } - } - if lastLuid6 != 0 { - iface, err := winipcfg.InterfaceFromLUID(lastLuid6) - if err != nil { - return err - } - if iface.Mtu > 0 && iface.Mtu < mtu { - mtu = iface.Mtu - } - } + if mtu > 0 && (lastMtu == 0 || lastMtu != mtu) { iface, err := winipcfg.GetIpInterface(ourLuid, winipcfg.AF_INET) if err != nil { @@ -136,7 +69,7 @@ func monitorDefaultRoutes(device *device.Device, autoMTU bool, tun *tun.NativeTu if err != nil { return err } - tun.ForceMTU(int(iface.NlMtu)) //TODO: it sort of breaks the model with v6 mtu and v4 mtu being different. Just set v4 one for now. + tun.ForceMTU(int(iface.NlMtu)) iface, err = winipcfg.GetIpInterface(ourLuid, winipcfg.AF_INET6) if err != nil { if !isMissingIPv6Err(err) { @@ -172,6 +105,56 @@ func monitorDefaultRoutes(device *device.Device, autoMTU bool, tun *tun.NativeTu return cb, nil } +func getDefaultRouteMTU() (uint32, error) { + mtus, err := interfaces.NonTailscaleMTUs() + if err != nil { + return 0, err + } + + routes, err := winipcfg.GetRoutes(winipcfg.AF_INET) + if err != nil { + return 0, err + } + best := ^uint32(0) + mtu := uint32(0) + for _, route := range routes { + if route.DestinationPrefix.PrefixLength != 0 { + continue + } + routeMTU := mtus[route.InterfaceLuid] + if routeMTU == 0 { + continue + } + if route.Metric < best { + best = route.Metric + mtu = routeMTU + } + } + + routes, err = winipcfg.GetRoutes(winipcfg.AF_INET6) + if err != nil { + return 0, err + } + best = ^uint32(0) + for _, route := range routes { + if route.DestinationPrefix.PrefixLength != 0 { + continue + } + routeMTU := mtus[route.InterfaceLuid] + if routeMTU == 0 { + continue + } + if route.Metric < best { + best = route.Metric + if routeMTU < mtu { + mtu = routeMTU + } + } + } + + return mtu, nil +} + func setFirewall(ifcGUID *windows.GUID) (bool, error) { c := ole.Connection{} err := c.Initialize() @@ -232,7 +215,6 @@ func setFirewall(ifcGUID *windows.GUID) (bool, error) { func configureInterface(cfg *Config, tun *tun.NativeTun) error { const mtu = 0 guid := tun.GUID() - log.Printf("wintun GUID is %v", guid) iface, err := winipcfg.InterfaceFromGUID(&guid) if err != nil { return err @@ -332,7 +314,6 @@ func configureInterface(cfg *Config, tun *tun.NativeTun) error { } deduplicatedRoutes = append(deduplicatedRoutes, &routes[i]) } - log.Printf("routes: %v", routes) var errAcc error err = iface.SyncRoutes(deduplicatedRoutes) @@ -346,7 +327,6 @@ func configureInterface(cfg *Config, tun *tun.NativeTun) error { log.Printf("getipif: %v", err) return err } - log.Printf("foundDefault4: %v", foundDefault4) if foundDefault4 { ipif.UseAutomaticMetric = false ipif.Metric = 0 diff --git a/wgengine/router/router_windows.go b/wgengine/router/router_windows.go index 9f67d51ff..1f36e5c7c 100644 --- a/wgengine/router/router_windows.go +++ b/wgengine/router/router_windows.go @@ -49,13 +49,10 @@ func newUserspaceRouter(logf logger.Logf, wgdev *device.Device, tundev tun.Devic } func (r *winRouter) Up() error { - // MonitorDefaultRoutes handles making sure our wireguard UDP - // traffic goes through the old route, not recursively through the VPN. - r.removeFirewallAcceptRule() var err error - r.routeChangeCallback, err = monitorDefaultRoutes(r.wgdev, true, r.nativeTun) + r.routeChangeCallback, err = monitorDefaultRoutes(r.nativeTun) if err != nil { log.Fatalf("MonitorDefaultRoutes: %v", err) } @@ -116,6 +113,7 @@ func (r *winRouter) Close() error { if r.routeChangeCallback != nil { r.routeChangeCallback.Unregister() } + return nil }