tailcfg: add Endpoint, EndpointType, MapRequest.EndpointType

Track endpoints internally with a new tailcfg.Endpoint type that
includes a typed netaddr.IPPort (instead of just a string) and
includes a type for how that endpoint was discovered (STUN, local,
etc).

Use []tailcfg.Endpoint instead of []string internally.

At the last second, send it to the control server as the existing
[]string for endpoints, but also include a new parallel
MapRequest.EndpointType []tailcfg.EndpointType, so the control server
can start filtering out less-important endpoint changes from
new-enough clients. Notably, STUN-discovered endpoints can be filtered
out from 1.6+ clients, as they can discover them amongst each other
via CallMeMaybe disco exchanges started over DERP. And STUN endpoints
change a lot, causing a lot of MapResposne updates. But portmapped
endpoints are worth keeping for now, as they they work right away
without requiring the firewall traversal extra RTT dance.

End result will be less control->client bandwidth. (despite negligible
increase in client->control bandwidth)

Updates tailscale/corp#1543

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

wgengine/magicsock/magicsock.go

@@ -116,7 +116,7 @@ type Conn struct {
 
 	logf             logger.Logf
 	port             uint16 // the preferred port from opts.Port; 0 means auto
-	epFunc           func(endpoints []string)
+	epFunc           func([]tailcfg.Endpoint)
 	derpActiveFunc   func()
 	idleFunc         func() time.Duration // nil means unknown
 	packetListener   nettype.PacketListener
@@ -201,7 +201,7 @@ type Conn struct {
 	// lastEndpoints records the endpoints found during the previous
 	// endpoint discovery. It's used to avoid duplicate endpoint
 	// change notifications.
-	lastEndpoints []string
+	lastEndpoints []tailcfg.Endpoint
 
 	// lastEndpointsTime is the last time the endpoints were updated,
 	// even if there was no change.
@@ -381,7 +381,7 @@ type Options struct {
 
 	// EndpointsFunc optionally provides a func to be called when
 	// endpoints change. The called func does not own the slice.
-	EndpointsFunc func(endpoint []string)
+	EndpointsFunc func([]tailcfg.Endpoint)
 
 	// DERPActiveFunc optionally provides a func to be called when
 	// a connection is made to a DERP server.
@@ -432,9 +432,9 @@ func (o *Options) logf() logger.Logf {
 	return o.Logf
 }
 
-func (o *Options) endpointsFunc() func([]string) {
+func (o *Options) endpointsFunc() func([]tailcfg.Endpoint) {
 	if o == nil || o.EndpointsFunc == nil {
-		return func([]string) {}
+		return func([]tailcfg.Endpoint) {}
 	}
 	return o.EndpointsFunc
 }
@@ -578,7 +578,7 @@ func (c *Conn) updateEndpoints(why string) {
 	}()
 	c.logf("[v1] magicsock: starting endpoint update (%s)", why)
 
-	endpoints, reasons, err := c.determineEndpoints(c.connCtx)
+	endpoints, err := c.determineEndpoints(c.connCtx)
 	if err != nil {
 		c.logf("magicsock: endpoint update (%s) failed: %v", why, err)
 		// TODO(crawshaw): are there any conditions under which
@@ -586,18 +586,18 @@ func (c *Conn) updateEndpoints(why string) {
 		return
 	}
 
-	if c.setEndpoints(endpoints, reasons) {
-		c.logEndpointChange(endpoints, reasons)
+	if c.setEndpoints(endpoints) {
+		c.logEndpointChange(endpoints)
 		c.epFunc(endpoints)
 	}
 }
 
 // setEndpoints records the new endpoints, reporting whether they're changed.
 // It takes ownership of the slice.
-func (c *Conn) setEndpoints(endpoints []string, reasons map[string]string) (changed bool) {
+func (c *Conn) setEndpoints(endpoints []tailcfg.Endpoint) (changed bool) {
 	anySTUN := false
-	for _, reason := range reasons {
-		if reason == "stun" {
+	for _, ep := range endpoints {
+		if ep.Type == tailcfg.EndpointSTUN {
 			anySTUN = true
 		}
 	}
@@ -625,7 +625,7 @@ func (c *Conn) setEndpoints(endpoints []string, reasons map[string]string) (chan
 		delete(c.onEndpointRefreshed, de)
 	}
 
-	if stringSetsEqual(endpoints, c.lastEndpoints) {
+	if endpointSetsEqual(endpoints, c.lastEndpoints) {
 		return false
 	}
 	c.lastEndpoints = endpoints
@@ -975,35 +975,39 @@ func (c *Conn) goDerpConnect(node int) {
 // does a STUN lookup (via netcheck) to determine its public address.
 //
 // c.mu must NOT be held.
-func (c *Conn) determineEndpoints(ctx context.Context) (ipPorts []string, reasons map[string]string, err error) {
+func (c *Conn) determineEndpoints(ctx context.Context) ([]tailcfg.Endpoint, error) {
 	nr, err := c.updateNetInfo(ctx)
 	if err != nil {
 		c.logf("magicsock.Conn.determineEndpoints: updateNetInfo: %v", err)
-		return nil, nil, err
+		return nil, err
 	}
 
-	already := make(map[string]string) // endpoint -> how it was found
-	var eps []string                   // unique endpoints
+	already := make(map[netaddr.IPPort]tailcfg.EndpointType) // endpoint -> how it was found
+	var eps []tailcfg.Endpoint                               // unique endpoints
 
-	addAddr := func(s, reason string) {
-		if debugOmitLocalAddresses && (reason == "localAddresses" || reason == "socket") {
+	ipp := func(s string) (ipp netaddr.IPPort) {
+		ipp, _ = netaddr.ParseIPPort(s)
+		return
+	}
+	addAddr := func(ipp netaddr.IPPort, et tailcfg.EndpointType) {
+		if ipp.IsZero() || (debugOmitLocalAddresses && et == tailcfg.EndpointLocal) {
 			return
 		}
-		if _, ok := already[s]; !ok {
-			already[s] = reason
-			eps = append(eps, s)
+		if _, ok := already[ipp]; !ok {
+			already[ipp] = et
+			eps = append(eps, tailcfg.Endpoint{Addr: ipp, Type: et})
 		}
 	}
 
 	if ext, err := c.portMapper.CreateOrGetMapping(ctx); err == nil {
-		addAddr(ext.String(), "portmap")
+		addAddr(ext, tailcfg.EndpointPortmapped)
 		c.setNetInfoHavePortMap()
 	} else if !portmapper.IsNoMappingError(err) {
 		c.logf("portmapper: %v", err)
 	}
 
 	if nr.GlobalV4 != "" {
-		addAddr(nr.GlobalV4, "stun")
+		addAddr(ipp(nr.GlobalV4), tailcfg.EndpointSTUN)
 
 		// If they're behind a hard NAT and are using a fixed
 		// port locally, assume they might've added a static
@@ -1012,12 +1016,12 @@ func (c *Conn) determineEndpoints(ctx context.Context) (ipPorts []string, reason
 		// it's an invalid candidate mapping.
 		if nr.MappingVariesByDestIP.EqualBool(true) && c.port != 0 {
 			if ip, _, err := net.SplitHostPort(nr.GlobalV4); err == nil {
-				addAddr(net.JoinHostPort(ip, strconv.Itoa(int(c.port))), "port_in")
+				addAddr(ipp(net.JoinHostPort(ip, strconv.Itoa(int(c.port)))), tailcfg.EndpointSTUN4LocalPort)
 			}
 		}
 	}
 	if nr.GlobalV6 != "" {
-		addAddr(nr.GlobalV6, "stun")
+		addAddr(ipp(nr.GlobalV6), tailcfg.EndpointSTUN)
 	}
 
 	c.ignoreSTUNPackets()
@@ -1025,9 +1029,8 @@ func (c *Conn) determineEndpoints(ctx context.Context) (ipPorts []string, reason
 	if localAddr := c.pconn4.LocalAddr(); localAddr.IP.IsUnspecified() {
 		ips, loopback, err := interfaces.LocalAddresses()
 		if err != nil {
-			return nil, nil, err
+			return nil, err
 		}
-		reason := "localAddresses"
 		if len(ips) == 0 && len(eps) == 0 {
 			// Only include loopback addresses if we have no
 			// interfaces at all to use as endpoints and don't
@@ -1035,15 +1038,14 @@ func (c *Conn) determineEndpoints(ctx context.Context) (ipPorts []string, reason
 			// for localhost testing when you're on a plane and
 			// offline, for example.
 			ips = loopback
-			reason = "loopback"
 		}
 		for _, ip := range ips {
-			addAddr(netaddr.IPPort{IP: ip, Port: uint16(localAddr.Port)}.String(), reason)
+			addAddr(netaddr.IPPort{IP: ip, Port: uint16(localAddr.Port)}, tailcfg.EndpointLocal)
 		}
 	} else {
 		// Our local endpoint is bound to a particular address.
 		// Do not offer addresses on other local interfaces.
-		addAddr(localAddr.String(), "socket")
+		addAddr(ipp(localAddr.String()), tailcfg.EndpointLocal)
 	}
 
 	// Note: the endpoints are intentionally returned in priority order,
@@ -1056,14 +1058,17 @@ func (c *Conn) determineEndpoints(ctx context.Context) (ipPorts []string, reason
 	// The STUN address(es) are always first so that legacy wireguard
 	// can use eps[0] as its only known endpoint address (although that's
 	// obviously non-ideal).
-	return eps, already, nil
+	//
+	// Despite this sorting, though, clients since 0.100 haven't relied
+	// on the sorting order for any decisions.
+	return eps, nil
 }
 
-// stringSetsEqual reports whether x and y represent the same set of
-// strings. The order doesn't matter.
+// endpointSetsEqual reports whether x and y represent the same set of
+// endpoints. The order doesn't matter.
 //
 // It does not mutate the slices.
-func stringSetsEqual(x, y []string) bool {
+func endpointSetsEqual(x, y []tailcfg.Endpoint) bool {
 	if len(x) == len(y) {
 		orderMatches := true
 		for i := range x {
@@ -1076,7 +1081,7 @@ func stringSetsEqual(x, y []string) bool {
 			return true
 		}
 	}
-	m := map[string]int{}
+	m := map[tailcfg.Endpoint]int{}
 	for _, v := range x {
 		m[v] |= 1
 	}
@@ -1988,9 +1993,7 @@ func (c *Conn) enqueueCallMeMaybe(derpAddr netaddr.IPPort, de *discoEndpoint) {
 
 	eps := make([]netaddr.IPPort, 0, len(c.lastEndpoints))
 	for _, ep := range c.lastEndpoints {
-		if ipp, err := netaddr.ParseIPPort(ep); err == nil {
-			eps = append(eps, ipp)
-		}
+		eps = append(eps, ep.Addr)
 	}
 	go de.sendDiscoMessage(derpAddr, &disco.CallMeMaybe{MyNumber: eps}, discoLog)
 }
@@ -2299,13 +2302,13 @@ func (c *Conn) logActiveDerpLocked() {
 	}))
 }
 
-func (c *Conn) logEndpointChange(endpoints []string, reasons map[string]string) {
+func (c *Conn) logEndpointChange(endpoints []tailcfg.Endpoint) {
 	c.logf("magicsock: endpoints changed: %s", logger.ArgWriter(func(buf *bufio.Writer) {
 		for i, ep := range endpoints {
 			if i > 0 {
 				buf.WriteString(", ")
 			}
-			fmt.Fprintf(buf, "%s (%s)", ep, reasons[ep])
+			fmt.Fprintf(buf, "%s (%s)", ep.Addr, ep.Type)
 		}
 	}))
 }
@@ -2968,7 +2971,10 @@ func (c *Conn) UpdateStatus(sb *ipnstate.StatusBuilder) {
 
 	sb.MutateSelfStatus(func(ss *ipnstate.PeerStatus) {
 		ss.PublicKey = c.privateKey.Public()
-		ss.Addrs = c.lastEndpoints
+		ss.Addrs = make([]string, 0, len(c.lastEndpoints))
+		for _, ep := range c.lastEndpoints {
+			ss.Addrs = append(ss.Addrs, ep.Addr.String())
+		}
 		ss.OS = version.OS()
 		if c.netMap != nil {
 			ss.HostName = c.netMap.Hostinfo.Hostname

wgengine/magicsock/magicsock_test.go

@@ -126,12 +126,12 @@ func runDERPAndStun(t *testing.T, logf logger.Logf, l nettype.PacketListener, st
 // happiness.
 type magicStack struct {
 	privateKey wgkey.Private
-	epCh       chan []string       // endpoint updates produced by this peer
-	conn       *Conn               // the magicsock itself
-	tun        *tuntest.ChannelTUN // TUN device to send/receive packets
-	tsTun      *tstun.Wrapper      // wrapped tun that implements filtering and wgengine hooks
-	dev        *device.Device      // the wireguard-go Device that connects the previous things
-	wgLogger   *wglog.Logger       // wireguard-go log wrapper
+	epCh       chan []tailcfg.Endpoint // endpoint updates produced by this peer
+	conn       *Conn                   // the magicsock itself
+	tun        *tuntest.ChannelTUN     // TUN device to send/receive packets
+	tsTun      *tstun.Wrapper          // wrapped tun that implements filtering and wgengine hooks
+	dev        *device.Device          // the wireguard-go Device that connects the previous things
+	wgLogger   *wglog.Logger           // wireguard-go log wrapper
 }
 
 // newMagicStack builds and initializes an idle magicsock and
@@ -145,11 +145,11 @@ func newMagicStack(t testing.TB, logf logger.Logf, l nettype.PacketListener, der
 		t.Fatalf("generating private key: %v", err)
 	}
 
-	epCh := make(chan []string, 100) // arbitrary
+	epCh := make(chan []tailcfg.Endpoint, 100) // arbitrary
 	conn, err := NewConn(Options{
 		Logf:           logf,
 		PacketListener: l,
-		EndpointsFunc: func(eps []string) {
+		EndpointsFunc: func(eps []tailcfg.Endpoint) {
 			epCh <- eps
 		},
 		SimulatedNetwork:        l != nettype.Std{},
@@ -245,7 +245,7 @@ func meshStacks(logf logger.Logf, ms []*magicStack) (cleanup func()) {
 	// simpler.
 	var (
 		mu  sync.Mutex
-		eps = make([][]string, len(ms))
+		eps = make([][]tailcfg.Endpoint, len(ms))
 	)
 
 	buildNetmapLocked := func(myIdx int) *netmap.NetworkMap {
@@ -267,7 +267,7 @@ func meshStacks(logf logger.Logf, ms []*magicStack) (cleanup func()) {
 				DiscoKey:   peer.conn.DiscoPublicKey(),
 				Addresses:  addrs,
 				AllowedIPs: addrs,
-				Endpoints:  eps[i],
+				Endpoints:  epStrings(eps[i]),
 				DERP:       "127.3.3.40:1",
 			}
 			nm.Peers = append(nm.Peers, peer)
@@ -276,7 +276,7 @@ func meshStacks(logf logger.Logf, ms []*magicStack) (cleanup func()) {
 		return nm
 	}
 
-	updateEps := func(idx int, newEps []string) {
+	updateEps := func(idx int, newEps []tailcfg.Endpoint) {
 		mu.Lock()
 		defer mu.Unlock()
 
@@ -331,9 +331,9 @@ func TestNewConn(t *testing.T) {
 	tstest.ResourceCheck(t)
 
 	epCh := make(chan string, 16)
-	epFunc := func(endpoints []string) {
+	epFunc := func(endpoints []tailcfg.Endpoint) {
 		for _, ep := range endpoints {
-			epCh <- ep
+			epCh <- ep.Addr.String()
 		}
 	}
 
@@ -503,7 +503,7 @@ func TestDeviceStartStop(t *testing.T) {
 	tstest.ResourceCheck(t)
 
 	conn, err := NewConn(Options{
-		EndpointsFunc:           func(eps []string) {},
+		EndpointsFunc:           func(eps []tailcfg.Endpoint) {},
 		Logf:                    t.Logf,
 		DisableLegacyNetworking: true,
 	})
@@ -1392,7 +1392,7 @@ func newNonLegacyTestConn(t testing.TB) *Conn {
 	conn, err := NewConn(Options{
 		Logf: t.Logf,
 		Port: port,
-		EndpointsFunc: func(eps []string) {
+		EndpointsFunc: func(eps []tailcfg.Endpoint) {
 			t.Logf("endpoints: %q", eps)
 		},
 		DisableLegacyNetworking: true,
@@ -1694,15 +1694,17 @@ func TestRebindStress(t *testing.T) {
 	}
 }
 
-func TestStringSetsEqual(t *testing.T) {
-	s := func(nn ...int) (ret []string) {
-		for _, n := range nn {
-			ret = append(ret, strconv.Itoa(n))
+func TestEndpointSetsEqual(t *testing.T) {
+	s := func(ports ...uint16) (ret []tailcfg.Endpoint) {
+		for _, port := range ports {
+			ret = append(ret, tailcfg.Endpoint{
+				Addr: netaddr.IPPort{Port: port},
+			})
 		}
 		return
 	}
 	tests := []struct {
-		a, b []string
+		a, b []tailcfg.Endpoint
 		want bool
 	}{
 		{
@@ -1745,7 +1747,7 @@ func TestStringSetsEqual(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
-		if got := stringSetsEqual(tt.a, tt.b); got != tt.want {
+		if got := endpointSetsEqual(tt.a, tt.b); got != tt.want {
 			t.Errorf("%q vs %q = %v; want %v", tt.a, tt.b, got, tt.want)
 		}
 	}
@@ -1804,3 +1806,10 @@ func TestBetterAddr(t *testing.T) {
 	}
 
 }
+
+func epStrings(eps []tailcfg.Endpoint) (ret []string) {
+	for _, ep := range eps {
+		ret = append(ret, ep.Addr.String())
+	}
+	return
+}