net/tsdial: give netstack a Dialer, start refactoring name resolution

This starts to refactor tsdial.Dialer's name resolution to have
different stages: in-memory MagicDNS vs system resolution. A future
change will plug in ExitDNS resolution.

This also plumbs a Dialer into netstack and unexports the dnsMap
internals.

And it removes some of the async AddNetworkMapCallback usage and
replaces it with synchronous updates of the Dialer's netmap
from LocalBackend, since the LocalBackend has the Dialer too.

Updates #3475

Change-Id: Idcb7b1169878c74f0522f5151031ccbc49fe4cb4
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

cmd/tailscaled/tailscaled.go

@@ -317,7 +317,7 @@ func run() error {
 		panic("internal error: exit node resolver not wired up")
 	}
 
-	ns, err := newNetstack(logf, e)
+	ns, err := newNetstack(logf, dialer, e)
 	if err != nil {
 		return fmt.Errorf("newNetstack: %w", err)
 	}
@@ -525,12 +525,12 @@ func runDebugServer(mux *http.ServeMux, addr string) {
 	}
 }
 
-func newNetstack(logf logger.Logf, e wgengine.Engine) (*netstack.Impl, error) {
+func newNetstack(logf logger.Logf, dialer *tsdial.Dialer, e wgengine.Engine) (*netstack.Impl, error) {
 	tunDev, magicConn, ok := e.(wgengine.InternalsGetter).GetInternals()
 	if !ok {
 		return nil, fmt.Errorf("%T is not a wgengine.InternalsGetter", e)
 	}
-	return netstack.Create(logf, tunDev, e, magicConn)
+	return netstack.Create(logf, tunDev, e, magicConn, dialer)
 }
 
 // mustStartProxyListeners creates listeners for local SOCKS and HTTP

cmd/tailscaled/tailscaled_windows.go

@@ -32,6 +32,7 @@
 	"tailscale.com/ipn/ipnserver"
 	"tailscale.com/logpolicy"
 	"tailscale.com/net/dns"
+	"tailscale.com/net/tsdial"
 	"tailscale.com/net/tstun"
 	"tailscale.com/safesocket"
 	"tailscale.com/types/logger"
@@ -182,6 +183,7 @@ func startIPNServer(ctx context.Context, logid string) error {
 	if err != nil {
 		return err
 	}
+	dialer := new(tsdial.Dialer)
 
 	getEngineRaw := func() (wgengine.Engine, error) {
 		dev, devName, err := tstun.New(logf, "Tailscale")
@@ -208,13 +210,14 @@ func startIPNServer(ctx context.Context, logid string) error {
 			DNS:         d,
 			ListenPort:  41641,
 			LinkMonitor: linkMon,
+			Dialer:      dialer,
 		})
 		if err != nil {
 			r.Close()
 			dev.Close()
 			return nil, fmt.Errorf("engine: %w", err)
 		}
-		ns, err := newNetstack(logf, eng)
+		ns, err := newNetstack(logf, dialer, eng)
 		if err != nil {
 			return nil, fmt.Errorf("newNetstack: %w", err)
 		}
@@ -294,7 +297,7 @@ type engineOrError struct {
 		return fmt.Errorf("safesocket.Listen: %v", err)
 	}
 
-	err = ipnserver.Run(ctx, logf, ln, store, linkMon, logid, getEngine, ipnServerOpts())
+	err = ipnserver.Run(ctx, logf, ln, store, linkMon, dialer, logid, getEngine, ipnServerOpts())
 	if err != nil {
 		logf("ipnserver.Run: %v", err)
 	}

ipn/ipnlocal/local.go

@@ -165,9 +165,6 @@ func NewLocalBackend(logf logger.Logf, logid string, store ipn.StateStore, diale
 	if dialer == nil {
 		dialer = new(tsdial.Dialer)
 	}
-	e.AddNetworkMapCallback(func(nm *netmap.NetworkMap) {
-		dialer.SetDNSMap(tsdial.DNSMapFromNetworkMap(nm))
-	})
 
 	osshare.SetFileSharingEnabled(false, logf)
 
@@ -2679,6 +2676,7 @@ func hasCapability(nm *netmap.NetworkMap, cap string) bool {
 }
 
 func (b *LocalBackend) setNetMapLocked(nm *netmap.NetworkMap) {
+	b.dialer.SetNetMap(nm)
 	var login string
 	if nm != nil {
 		login = nm.UserProfiles[nm.User].LoginName

ipn/ipnserver/server.go

@@ -43,7 +43,6 @@
 	"tailscale.com/safesocket"
 	"tailscale.com/smallzstd"
 	"tailscale.com/types/logger"
-	"tailscale.com/types/netmap"
 	"tailscale.com/util/groupmember"
 	"tailscale.com/util/pidowner"
 	"tailscale.com/util/systemd"
@@ -654,7 +653,7 @@ func StateStore(path string, logf logger.Logf) (ipn.StateStore, error) {
 // The getEngine func is called repeatedly, once per connection, until it returns an engine successfully.
 //
 // Deprecated: use New and Server.Run instead.
-func Run(ctx context.Context, logf logger.Logf, ln net.Listener, store ipn.StateStore, linkMon *monitor.Mon, logid string, getEngine func() (wgengine.Engine, error), opts Options) error {
+func Run(ctx context.Context, logf logger.Logf, ln net.Listener, store ipn.StateStore, linkMon *monitor.Mon, dialer *tsdial.Dialer, logid string, getEngine func() (wgengine.Engine, error), opts Options) error {
 	getEngine = getEngineUntilItWorksWrapper(getEngine)
 	runDone := make(chan struct{})
 	defer close(runDone)
@@ -738,11 +737,6 @@ func Run(ctx context.Context, logf logger.Logf, ln net.Listener, store ipn.State
 		}
 	}
 
-	dialer := new(tsdial.Dialer)
-	eng.AddNetworkMapCallback(func(nm *netmap.NetworkMap) {
-		dialer.SetDNSMap(tsdial.DNSMapFromNetworkMap(nm))
-	})
-
 	server, err := New(logf, logid, store, eng, dialer, serverModeUser, opts)
 	if err != nil {
 		return err

ipn/ipnserver/server_test.go

@@ -13,6 +13,7 @@
 
 	"tailscale.com/ipn"
 	"tailscale.com/ipn/ipnserver"
+	"tailscale.com/net/tsdial"
 	"tailscale.com/safesocket"
 	"tailscale.com/wgengine"
 )
@@ -72,6 +73,6 @@ func TestRunMultipleAccepts(t *testing.T) {
 	}
 	defer ln.Close()
 
-	err = ipnserver.Run(ctx, logTriggerTestf, ln, store, nil /* mon */, "dummy_logid", ipnserver.FixedEngine(eng), opts)
+	err = ipnserver.Run(ctx, logTriggerTestf, ln, store, nil /* mon */, new(tsdial.Dialer), "dummy_logid", ipnserver.FixedEngine(eng), opts)
 	t.Logf("ipnserver.Run = %v", err)
 }

net/tsdial/dnsmap.go

@@ -6,6 +6,7 @@
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net"
 	"strconv"
@@ -16,15 +17,18 @@
 	"tailscale.com/util/dnsname"
 )
 
-// DNSMap maps MagicDNS names (both base + FQDN) to their first IP.
+// dnsMap maps MagicDNS names (both base + FQDN) to their first IP.
 // It must not be mutated once created.
 //
 // Example keys are "foo.domain.tld.beta.tailscale.net" and "foo",
 // both without trailing dots.
-type DNSMap map[string]netaddr.IP
+type dnsMap map[string]netaddr.IP
 
-func DNSMapFromNetworkMap(nm *netmap.NetworkMap) DNSMap {
+func dnsMapFromNetworkMap(nm *netmap.NetworkMap) dnsMap {
-	ret := make(DNSMap)
+	if nm == nil {
+		return nil
+	}
+	ret := make(dnsMap)
 	suffix := nm.MagicDNSSuffix()
 	have4 := false
 	if nm.Name != "" && len(nm.Addresses) > 0 {
@@ -68,26 +72,35 @@ func DNSMapFromNetworkMap(nm *netmap.NetworkMap) DNSMap {
 	return ret
 }
 
+// errUnresolved is a sentinel error returned by dnsMap.resolveMemory.
+var errUnresolved = errors.New("address well formed but not resolved")
+
+func splitHostPort(addr string) (host string, port uint16, err error) {
+	host, portStr, err := net.SplitHostPort(addr)
+	if err != nil {
+		return "", 0, err
+	}
+	port16, err := strconv.ParseUint(portStr, 10, 16)
+	if err != nil {
+		return "", 0, fmt.Errorf("invalid port in address %q", addr)
+	}
+	return host, uint16(port16), nil
+}
+
 // Resolve resolves addr into an IP:port using first the MagicDNS contents
 // of m, else using the system resolver.
-func (m DNSMap) Resolve(ctx context.Context, addr string) (netaddr.IPPort, error) {
+//
-	ipp, pippErr := netaddr.ParseIPPort(addr)
+// The error is [exactly] errUnresolved if the addr is a name that isn't known
-	if pippErr == nil {
+// in the map.
-		return ipp, nil
+func (m dnsMap) resolveMemory(ctx context.Context, network, addr string) (_ netaddr.IPPort, err error) {
-	}
+	host, port, err := splitHostPort(addr)
-	host, port, err := net.SplitHostPort(addr)
 	if err != nil {
-		// addr is malformed.
+		// addr malformed or invalid port.
 		return netaddr.IPPort{}, err
 	}
-	if _, err := netaddr.ParseIP(host); err == nil {
+	if ip, err := netaddr.ParseIP(host); err == nil {
-		// The host part of addr was an IP, so the netaddr.ParseIPPort above should've
+		// addr was literal ip:port.
-		// passed. Must've been a bad port number. Return the original error.
+		return netaddr.IPPortFrom(ip, port), nil
-		return netaddr.IPPort{}, pippErr
-	}
-	port16, err := strconv.ParseUint(port, 10, 16)
-	if err != nil {
-		return netaddr.IPPort{}, fmt.Errorf("invalid port in address %q", addr)
 	}
 
 	// Host is not an IP, so assume it's a DNS name.
@@ -95,20 +108,8 @@ func (m DNSMap) Resolve(ctx context.Context, addr string) (netaddr.IPPort, error
 	// Try MagicDNS first, otherwise a real DNS lookup.
 	ip := m[host]
 	if !ip.IsZero() {
-		return netaddr.IPPortFrom(ip, uint16(port16)), nil
+		return netaddr.IPPortFrom(ip, port), nil
 	}
 
-	// TODO(bradfitz): wire up net/dnscache too.
+	return netaddr.IPPort{}, errUnresolved
-
-	// No MagicDNS name so try real DNS.
-	var r net.Resolver
-	ips, err := r.LookupIP(ctx, "ip", host)
-	if err != nil {
-		return netaddr.IPPort{}, err
-	}
-	if len(ips) == 0 {
-		return netaddr.IPPort{}, fmt.Errorf("DNS lookup returned no results for %q", host)
-	}
-	ip, _ = netaddr.FromStdIP(ips[0])
-	return netaddr.IPPortFrom(ip, uint16(port16)), nil
 }

net/tsdial/dnsmap_test.go

@@ -19,7 +19,7 @@ func TestDNSMapFromNetworkMap(t *testing.T) {
 	tests := []struct {
 		name string
 		nm   *netmap.NetworkMap
-		want DNSMap
+		want dnsMap
 	}{
 		{
 			name: "self",
@@ -30,7 +30,7 @@ func TestDNSMapFromNetworkMap(t *testing.T) {
 					pfx("100::123/128"),
 				},
 			},
-			want: DNSMap{
+			want: dnsMap{
 				"foo":         ip("100.102.103.104"),
 				"foo.tailnet": ip("100.102.103.104"),
 			},
@@ -59,7 +59,7 @@ func TestDNSMapFromNetworkMap(t *testing.T) {
 					},
 				},
 			},
-			want: DNSMap{
+			want: dnsMap{
 				"foo":         ip("100.102.103.104"),
 				"foo.tailnet": ip("100.102.103.104"),
 				"a":           ip("100.0.0.201"),
@@ -91,7 +91,7 @@ func TestDNSMapFromNetworkMap(t *testing.T) {
 					},
 				},
 			},
-			want: DNSMap{
+			want: dnsMap{
 				"foo":         ip("100::123"),
 				"foo.tailnet": ip("100::123"),
 				"a":           ip("100::201"),
@@ -103,7 +103,7 @@ func TestDNSMapFromNetworkMap(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := DNSMapFromNetworkMap(tt.nm)
+			got := dnsMapFromNetworkMap(tt.nm)
 			if !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("mismatch:\n got %v\nwant %v\n", got, tt.want)
 			}

net/tsdial/tsdial.go

@@ -18,6 +18,7 @@
 
 	"inet.af/netaddr"
 	"tailscale.com/net/netknob"
+	"tailscale.com/types/netmap"
 	"tailscale.com/wgengine/monitor"
 )
 
@@ -43,7 +44,7 @@ type Dialer struct {
 	peerDialer     *net.Dialer
 
 	mu      sync.Mutex
-	dns     DNSMap
+	dns     dnsMap
 	tunName string // tun device name
 	linkMon *monitor.Mon
 }
@@ -102,26 +103,55 @@ func (d *Dialer) PeerDialControlFunc() func(network, address string, c syscall.R
 	return peerDialControlFunc(d)
 }
 
-// SetDNSMap sets the current map of DNS names learned from the netmap.
+// SetNetMap sets the current network map and notably, the DNS names
-func (d *Dialer) SetDNSMap(m DNSMap) {
+// in its DNS configuration.
-	// TODO(bradfitz): update this to be aware of DNSConfig
+func (d *Dialer) SetNetMap(nm *netmap.NetworkMap) {
-	// ExtraNames and CertDomains.
+	m := dnsMapFromNetworkMap(nm)
+
 	d.mu.Lock()
 	defer d.mu.Unlock()
 	d.dns = m
 }
 
-func (d *Dialer) resolve(ctx context.Context, addr string) (netaddr.IPPort, error) {
+func (d *Dialer) Resolve(ctx context.Context, network, addr string) (netaddr.IPPort, error) {
 	d.mu.Lock()
 	dns := d.dns
 	d.mu.Unlock()
-	return dns.Resolve(ctx, addr)
+
+	// MagicDNS or otherwise baked in to the NetworkMap? Try that first.
+	ipp, err := dns.resolveMemory(ctx, network, addr)
+
+	if err != errUnresolved {
+		return ipp, err
+	}
+
+	// Otherwise, hit the network.
+
+	// TODO(bradfitz): use ExitDNS (Issue 3475)
+	// TODO(bradfitz): wire up net/dnscache too.
+
+	host, port, err := splitHostPort(addr)
+	if err != nil {
+		// addr is malformed.
+		return netaddr.IPPort{}, err
+	}
+
+	var r net.Resolver
+	ips, err := r.LookupIP(ctx, network, host)
+	if err != nil {
+		return netaddr.IPPort{}, err
+	}
+	if len(ips) == 0 {
+		return netaddr.IPPort{}, fmt.Errorf("DNS lookup returned no results for %q", host)
+	}
+	ip, _ := netaddr.FromStdIP(ips[0])
+	return netaddr.IPPortFrom(ip, port), nil
 }
 
 // UserDial connects to the provided network address as if a user were initiating the dial.
 // (e.g. from a SOCKS or HTTP outbound proxy)
 func (d *Dialer) UserDial(ctx context.Context, network, addr string) (net.Conn, error) {
-	ipp, err := d.resolve(ctx, addr)
+	ipp, err := d.Resolve(ctx, network, addr)
 	if err != nil {
 		return nil, err
 	}

tsnet/tsnet.go

@@ -132,7 +132,7 @@ func (s *Server) start() error {
 		return fmt.Errorf("%T is not a wgengine.InternalsGetter", eng)
 	}
 
-	ns, err := netstack.Create(logf, tunDev, eng, magicConn)
+	ns, err := netstack.Create(logf, tunDev, eng, magicConn, dialer)
 	if err != nil {
 		return fmt.Errorf("netstack.Create: %w", err)
 	}

wgengine/netstack/netstack.go

@@ -71,6 +71,7 @@ type Impl struct {
 	e       wgengine.Engine
 	mc      *magicsock.Conn
 	logf    logger.Logf
+	dialer  *tsdial.Dialer
 
 	// atomicIsLocalIPFunc holds a func that reports whether an IP
 	// is a local (non-subnet) Tailscale IP address of this
@@ -79,7 +80,6 @@ type Impl struct {
 	atomicIsLocalIPFunc atomic.Value // of func(netaddr.IP) bool
 
 	mu sync.Mutex
-	dns tsdial.DNSMap
 	// connsOpenBySubnetIP keeps track of number of connections open
 	// for each subnet IP temporarily registered on netstack for active
 	// TCP connections, so they can be unregistered when connections are
@@ -91,7 +91,7 @@ type Impl struct {
 const mtu = 1500
 
 // Create creates and populates a new Impl.
-func Create(logf logger.Logf, tundev *tstun.Wrapper, e wgengine.Engine, mc *magicsock.Conn) (*Impl, error) {
+func Create(logf logger.Logf, tundev *tstun.Wrapper, e wgengine.Engine, mc *magicsock.Conn, dialer *tsdial.Dialer) (*Impl, error) {
 	if mc == nil {
 		return nil, errors.New("nil magicsock.Conn")
 	}
@@ -104,6 +104,9 @@ func Create(logf logger.Logf, tundev *tstun.Wrapper, e wgengine.Engine, mc *magi
 	if e == nil {
 		return nil, errors.New("nil Engine")
 	}
+	if dialer == nil {
+		return nil, errors.New("nil Dialer")
+	}
 	ipstack := stack.New(stack.Options{
 		NetworkProtocols:   []stack.NetworkProtocolFactory{ipv4.NewProtocol, ipv6.NewProtocol},
 		TransportProtocols: []stack.TransportProtocolFactory{tcp.NewProtocol, udp.NewProtocol, icmp.NewProtocol4, icmp.NewProtocol6},
@@ -139,6 +142,7 @@ func Create(logf logger.Logf, tundev *tstun.Wrapper, e wgengine.Engine, mc *magi
 		tundev:              tundev,
 		e:                   e,
 		mc:                  mc,
+		dialer:              dialer,
 		connsOpenBySubnetIP: make(map[netaddr.IP]int),
 	}
 	ns.atomicIsLocalIPFunc.Store(tsaddr.NewContainsIPFunc(nil))
@@ -179,12 +183,6 @@ func (ns *Impl) Start() error {
 	return nil
 }
 
-func (ns *Impl) updateDNS(nm *netmap.NetworkMap) {
-	ns.mu.Lock()
-	defer ns.mu.Unlock()
-	ns.dns = tsdial.DNSMapFromNetworkMap(nm)
-}
-
 func (ns *Impl) addSubnetAddress(ip netaddr.IP) {
 	ns.mu.Lock()
 	ns.connsOpenBySubnetIP[ip]++
@@ -230,7 +228,6 @@ func ipPrefixToAddressWithPrefix(ipp netaddr.IPPrefix) tcpip.AddressWithPrefix {
 
 func (ns *Impl) updateIPs(nm *netmap.NetworkMap) {
 	ns.atomicIsLocalIPFunc.Store(tsaddr.NewContainsIPFunc(nm.Addresses))
-	ns.updateDNS(nm)
 
 	oldIPs := make(map[tcpip.AddressWithPrefix]bool)
 	for _, protocolAddr := range ns.ipstack.AllAddresses()[nicID] {
@@ -299,11 +296,7 @@ func (ns *Impl) updateIPs(nm *netmap.NetworkMap) {
 }
 
 func (ns *Impl) DialContextTCP(ctx context.Context, addr string) (*gonet.TCPConn, error) {
-	ns.mu.Lock()
+	remoteIPPort, err := ns.dialer.Resolve(ctx, "tcp", addr)
-	dnsMap := ns.dns
-	ns.mu.Unlock()
-
-	remoteIPPort, err := dnsMap.Resolve(ctx, addr)
 	if err != nil {
 		return nil, err
 	}
@@ -323,11 +316,7 @@ func (ns *Impl) DialContextTCP(ctx context.Context, addr string) (*gonet.TCPConn
 }
 
 func (ns *Impl) DialContextUDP(ctx context.Context, addr string) (*gonet.UDPConn, error) {
-	ns.mu.Lock()
+	remoteIPPort, err := ns.dialer.Resolve(ctx, "udp", addr)
-	dnsMap := ns.dns
-	ns.mu.Unlock()
-
-	remoteIPPort, err := dnsMap.Resolve(ctx, addr)
 	if err != nil {
 		return nil, err
 	}