From 34d2f5a3d9d8c7325e4b854d9e6808855a222169 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 12 Apr 2021 13:24:29 -0700 Subject: [PATCH] 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 --- control/controlclient/auto.go | 2 +- control/controlclient/direct.go | 46 +++++++++------ control/controlclient/direct_test.go | 14 ++++- ipn/ipnlocal/local.go | 4 +- ipn/ipnlocal/loglines_test.go | 1 - tailcfg/tailcfg.go | 46 ++++++++++++++- tailcfg/tailcfg_test.go | 18 ++++++ wgengine/magicsock/magicsock.go | 88 +++++++++++++++------------- wgengine/magicsock/magicsock_test.go | 51 +++++++++------- wgengine/userspace.go | 6 +- wgengine/wgengine.go | 4 +- 11 files changed, 186 insertions(+), 94 deletions(-) diff --git a/control/controlclient/auto.go b/control/controlclient/auto.go index 123e615f1..cc92e443d 100644 --- a/control/controlclient/auto.go +++ b/control/controlclient/auto.go @@ -738,7 +738,7 @@ func (c *Client) Logout(ctx context.Context) error { // // The localPort field is unused except for integration tests in // another repo. -func (c *Client) UpdateEndpoints(localPort uint16, endpoints []string) { +func (c *Client) UpdateEndpoints(localPort uint16, endpoints []tailcfg.Endpoint) { changed := c.direct.SetEndpoints(localPort, endpoints) if changed { c.sendNewMapRequest() diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 50dc6b62e..880fc04bc 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -76,7 +76,7 @@ type Direct struct { expiry *time.Time // hostinfo is mutated in-place while mu is held. hostinfo *tailcfg.Hostinfo // always non-nil - endpoints []string + endpoints []tailcfg.Endpoint everEndpoints bool // whether we've ever had non-empty endpoints localPort uint16 // or zero to mean auto } @@ -506,7 +506,7 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new return false, resp.AuthURL, nil } -func sameStrings(a, b []string) bool { +func sameEndpoints(a, b []tailcfg.Endpoint) bool { if len(a) != len(b) { return false } @@ -522,15 +522,19 @@ func sameStrings(a, b []string) bool { // whether they've changed. // // It does not retain the provided slice. -func (c *Direct) newEndpoints(localPort uint16, endpoints []string) (changed bool) { +func (c *Direct) newEndpoints(localPort uint16, endpoints []tailcfg.Endpoint) (changed bool) { c.mu.Lock() defer c.mu.Unlock() // Nothing new? - if c.localPort == localPort && sameStrings(c.endpoints, endpoints) { + if c.localPort == localPort && sameEndpoints(c.endpoints, endpoints) { return false // unchanged } - c.logf("client.newEndpoints(%v, %v)", localPort, endpoints) + var epStrs []string + for _, ep := range endpoints { + epStrs = append(epStrs, ep.Addr.String()) + } + c.logf("client.newEndpoints(%v, %v)", localPort, epStrs) c.localPort = localPort c.endpoints = append(c.endpoints[:0], endpoints...) if len(endpoints) > 0 { @@ -542,7 +546,7 @@ func (c *Direct) newEndpoints(localPort uint16, endpoints []string) (changed boo // SetEndpoints updates the list of locally advertised endpoints. // It won't be replicated to the server until a *fresh* call to PollNetMap(). // You don't need to restart PollNetMap if we return changed==false. -func (c *Direct) SetEndpoints(localPort uint16, endpoints []string) (changed bool) { +func (c *Direct) SetEndpoints(localPort uint16, endpoints []tailcfg.Endpoint) (changed bool) { // (no log message on function entry, because it clutters the logs // if endpoints haven't changed. newEndpoints() will log it.) return c.newEndpoints(localPort, endpoints) @@ -575,7 +579,12 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netm hostinfo := c.hostinfo.Clone() backendLogID := hostinfo.BackendLogID localPort := c.localPort - ep := append([]string(nil), c.endpoints...) + var epStrs []string + var epTypes []tailcfg.EndpointType + for _, ep := range c.endpoints { + epStrs = append(epStrs, ep.Addr.String()) + epTypes = append(epTypes, ep.Type) + } everEndpoints := c.everEndpoints c.mu.Unlock() @@ -595,7 +604,7 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netm } allowStream := maxPolls != 1 - c.logf("[v1] PollNetMap: stream=%v :%v ep=%v", allowStream, localPort, ep) + c.logf("[v1] PollNetMap: stream=%v :%v ep=%v", allowStream, localPort, epStrs) vlogf := logger.Discard if Debug.NetMap { @@ -605,15 +614,16 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netm } request := &tailcfg.MapRequest{ - Version: tailcfg.CurrentMapRequestVersion, - KeepAlive: c.keepAlive, - NodeKey: tailcfg.NodeKey(persist.PrivateNodeKey.Public()), - DiscoKey: c.discoPubKey, - Endpoints: ep, - Stream: allowStream, - Hostinfo: hostinfo, - DebugFlags: c.debugFlags, - OmitPeers: cb == nil, + Version: tailcfg.CurrentMapRequestVersion, + KeepAlive: c.keepAlive, + NodeKey: tailcfg.NodeKey(persist.PrivateNodeKey.Public()), + DiscoKey: c.discoPubKey, + Endpoints: epStrs, + EndpointTypes: epTypes, + Stream: allowStream, + Hostinfo: hostinfo, + DebugFlags: c.debugFlags, + OmitPeers: cb == nil, } var extraDebugFlags []string if hostinfo != nil && c.linkMon != nil && !c.skipIPForwardingCheck && @@ -641,7 +651,7 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netm // TODO(bradfitz): we skip this optimization in tests, though, // because the e2e tests are currently hyperspecific about the // ordering of things. The e2e tests need love. - if len(ep) == 0 && !everEndpoints && !inTest() { + if len(epStrs) == 0 && !everEndpoints && !inTest() { request.ReadOnly = true } diff --git a/control/controlclient/direct_test.go b/control/controlclient/direct_test.go index 603614aba..484eeffca 100644 --- a/control/controlclient/direct_test.go +++ b/control/controlclient/direct_test.go @@ -11,6 +11,7 @@ "strings" "testing" + "inet.af/netaddr" "tailscale.com/tailcfg" "tailscale.com/types/wgkey" ) @@ -144,7 +145,7 @@ func TestNewDirect(t *testing.T) { t.Errorf("c.SetHostinfo(hi) want true got %v", changed) } - endpoints := []string{"1", "2", "3"} + endpoints := fakeEndpoints(1, 2, 3) changed = c.newEndpoints(12, endpoints) if !changed { t.Errorf("c.newEndpoints(12) want true got %v", changed) @@ -157,13 +158,22 @@ func TestNewDirect(t *testing.T) { if !changed { t.Errorf("c.newEndpoints(13) want true got %v", changed) } - endpoints = []string{"4", "5", "6"} + endpoints = fakeEndpoints(4, 5, 6) changed = c.newEndpoints(13, endpoints) if !changed { t.Errorf("c.newEndpoints(13) want true got %v", changed) } } +func fakeEndpoints(ports ...uint16) (ret []tailcfg.Endpoint) { + for _, port := range ports { + ret = append(ret, tailcfg.Endpoint{ + Addr: netaddr.IPPort{Port: port}, + }) + } + return +} + func TestNewHostinfo(t *testing.T) { hi := NewHostinfo() if hi == nil { diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index e42065ac9..67b2d02b9 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -111,7 +111,7 @@ type LocalBackend struct { nodeByAddr map[netaddr.IP]*tailcfg.Node activeLogin string // last logged LoginName from netMap engineStatus ipn.EngineStatus - endpoints []string + endpoints []tailcfg.Endpoint blocked bool authURL string interact bool @@ -544,7 +544,7 @@ func (b *LocalBackend) setWgengineStatus(s *wgengine.Status, err error) { es := b.parseWgStatusLocked(s) cc := b.cc b.engineStatus = es - b.endpoints = append([]string{}, s.LocalAddrs...) + b.endpoints = append([]tailcfg.Endpoint{}, s.LocalAddrs...) b.mu.Unlock() if cc != nil { diff --git a/ipn/ipnlocal/loglines_test.go b/ipn/ipnlocal/loglines_test.go index 812e69037..66cc60a8d 100644 --- a/ipn/ipnlocal/loglines_test.go +++ b/ipn/ipnlocal/loglines_test.go @@ -82,7 +82,6 @@ func TestLocalLogLines(t *testing.T) { LastHandshake: time.Now(), NodeKey: tailcfg.NodeKey(key.NewPrivate()), }}, - LocalAddrs: []string{"idk an address"}, } lb.mu.Lock() lb.parseWgStatusLocked(status) diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index c255de54b..4e86bfc9a 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -636,6 +636,42 @@ type RegisterResponse struct { AuthURL string // if set, authorization pending } +// EndpointType distinguishes different sources of MapRequest.Endpoint values. +type EndpointType int + +const ( + EndpointUnknownType = EndpointType(0) + EndpointLocal = EndpointType(1) + EndpointSTUN = EndpointType(2) + EndpointPortmapped = EndpointType(3) + EndpointSTUN4LocalPort = EndpointType(4) // hard NAT: STUN'ed IPv4 address + local fixed port +) + +func (et EndpointType) String() string { + switch et { + case EndpointUnknownType: + return "?" + case EndpointLocal: + return "local" + case EndpointSTUN: + return "stun" + case EndpointPortmapped: + return "portmap" + case EndpointSTUN4LocalPort: + return "stun4localport" + } + return "other" +} + +// Endpoint is an endpoint IPPort and an associated type. +// It doesn't currently go over the wire as is but is instead +// broken up into two parallel slices in MapReqeust, for compatibility +// reasons. But this type is used in the codebase. +type Endpoint struct { + Addr netaddr.IPPort + Type EndpointType +} + // MapRequest is sent by a client to start a long-poll network map updates. // The request includes a copy of the client's current set of WireGuard // endpoints and general host information. @@ -655,11 +691,15 @@ type MapRequest struct { KeepAlive bool // whether server should send keep-alives back to us NodeKey NodeKey DiscoKey DiscoKey - Endpoints []string // caller's endpoints (IPv4 or IPv6) - IncludeIPv6 bool `json:",omitempty"` // include IPv6 endpoints in returned Node Endpoints (for Version 4 clients) - Stream bool // if true, multiple MapResponse objects are returned + IncludeIPv6 bool `json:",omitempty"` // include IPv6 endpoints in returned Node Endpoints (for Version 4 clients) + Stream bool // if true, multiple MapResponse objects are returned Hostinfo *Hostinfo + // Endpoints are the client's magicsock UDP ip:port endpoints (IPv4 or IPv6). + Endpoints []string + // EndpointTypes are the types of the corresponding endpoints in Endpoints. + EndpointTypes []EndpointType `json:",omitempty"` + // ReadOnly is whether the client just wants to fetch the // MapResponse, without updating their Endpoints. The // Endpoints field will be ignored and LastSeen will not be diff --git a/tailcfg/tailcfg_test.go b/tailcfg/tailcfg_test.go index 140374c92..fba103844 100644 --- a/tailcfg/tailcfg_test.go +++ b/tailcfg/tailcfg_test.go @@ -500,3 +500,21 @@ func TestUserProfileJSONMarshalForMac(t *testing.T) { t.Fatalf("Unmarshal: %v", err) } } + +func TestEndpointTypeMarshal(t *testing.T) { + eps := []EndpointType{ + EndpointUnknownType, + EndpointLocal, + EndpointSTUN, + EndpointPortmapped, + EndpointSTUN4LocalPort, + } + got, err := json.Marshal(eps) + if err != nil { + t.Fatal(err) + } + const want = `[0,1,2,3,4]` + if string(got) != want { + t.Errorf("got %s; want %s", got, want) + } +} diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 9f52d44f5..2db796ea2 100644 --- a/wgengine/magicsock/magicsock.go +++ b/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 diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index 33a571029..8490dee66 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/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 +} diff --git a/wgengine/userspace.go b/wgengine/userspace.go index c39532d4d..964afa88a 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -114,7 +114,7 @@ type userspaceEngine struct { closing bool // Close was called (even if we're still closing) statusCallback StatusCallback peerSequence []wgkey.Key - endpoints []string + endpoints []tailcfg.Endpoint pingers map[wgkey.Key]*pinger // legacy pingers for pre-discovery peers pendOpen map[flowtrack.Tuple]*pendingOpenFlow // see pendopen.go networkMapCallbacks map[*someHandle]NetworkMapCallback @@ -263,7 +263,7 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) closePool.addFunc(unregisterMonWatch) e.linkMonUnregister = unregisterMonWatch - endpointsFn := func(endpoints []string) { + endpointsFn := func(endpoints []tailcfg.Endpoint) { e.mu.Lock() e.endpoints = append(e.endpoints[:0], endpoints...) e.mu.Unlock() @@ -1181,7 +1181,7 @@ func (e *userspaceEngine) getStatus() (*Status, error) { } return &Status{ - LocalAddrs: append([]string(nil), e.endpoints...), + LocalAddrs: append([]tailcfg.Endpoint(nil), e.endpoints...), Peers: peers, DERPs: derpConns, }, nil diff --git a/wgengine/wgengine.go b/wgengine/wgengine.go index fc25072ed..192126e2a 100644 --- a/wgengine/wgengine.go +++ b/wgengine/wgengine.go @@ -23,8 +23,8 @@ // TODO(bradfitz): remove this, subset of ipnstate? Need to migrate users. type Status struct { Peers []ipnstate.PeerStatusLite - LocalAddrs []string // the set of possible endpoints for the magic conn - DERPs int // number of active DERP connections + LocalAddrs []tailcfg.Endpoint // the set of possible endpoints for the magic conn + DERPs int // number of active DERP connections } // StatusCallback is the type of status callbacks used by