wgengine/magicsock: delete legacy AddrSet endpoints.

Instead of using the legacy codepath, teach discoEndpoint to handle
peers that have a home DERP, but no disco key. We can still communicate
with them, but only over DERP.

Signed-off-by: David Anderson <[email protected]>

cmd/tailscaled/depaware.txt

@@ -48,7 +48,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         golang.zx2c4.com/wireguard/ratelimiter                       from golang.zx2c4.com/wireguard/device
         golang.zx2c4.com/wireguard/replay                            from golang.zx2c4.com/wireguard/device
         golang.zx2c4.com/wireguard/rwcancel                          from golang.zx2c4.com/wireguard/device+
-        golang.zx2c4.com/wireguard/tai64n                            from golang.zx2c4.com/wireguard/device+
+        golang.zx2c4.com/wireguard/tai64n                            from golang.zx2c4.com/wireguard/device
      💣 golang.zx2c4.com/wireguard/tun                               from golang.zx2c4.com/wireguard/device+
    W 💣 golang.zx2c4.com/wireguard/tun/wintun                        from golang.zx2c4.com/wireguard/tun+
    W 💣 golang.zx2c4.com/wireguard/windows/tunnel/winipcfg           from tailscale.com/net/interfaces+
@@ -180,7 +180,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
    W 💣 tailscale.com/wgengine/winnet                                from tailscale.com/wgengine/router
         golang.org/x/crypto/acme                                     from tailscale.com/ipn/localapi
         golang.org/x/crypto/blake2b                                  from golang.org/x/crypto/nacl/box
-        golang.org/x/crypto/blake2s                                  from golang.zx2c4.com/wireguard/device+
+        golang.org/x/crypto/blake2s                                  from golang.zx2c4.com/wireguard/device
         golang.org/x/crypto/chacha20                                 from golang.org/x/crypto/chacha20poly1305
         golang.org/x/crypto/chacha20poly1305                         from crypto/tls+
         golang.org/x/crypto/cryptobyte                               from crypto/ecdsa+

wgengine/bench/bench_test.go

@@ -42,7 +42,7 @@ func BenchmarkBatchTCP(b *testing.B) {
 }
 
 func BenchmarkWireGuardTest(b *testing.B) {
-	b.Skip("setup code doesn't support disco yet")
+	b.Skip("https://github.com/tailscale/tailscale/issues/2716")
 	run(b, func(logf logger.Logf, traf *TrafficGen) {
 		setupWGTest(b, logf, traf, Addr1, Addr2)
 	})

wgengine/bench/wg.go

@@ -99,10 +99,8 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netadd
 		logf("e1 status: %v", *st)
 
 		var eps []string
-		var ipps []netaddr.IPPort
 		for _, ep := range st.LocalAddrs {
 			eps = append(eps, ep.Addr.String())
-			ipps = append(ipps, ep.Addr)
 		}
 		endpoint := wgcfg.Endpoints{
 			PublicKey: c1.PrivateKey.Public(),
@@ -142,10 +140,8 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netadd
 		logf("e2 status: %v", *st)
 
 		var eps []string
-		var ipps []netaddr.IPPort
 		for _, ep := range st.LocalAddrs {
 			eps = append(eps, ep.Addr.String())
-			ipps = append(ipps, ep.Addr)
 		}
 		endpoint := wgcfg.Endpoints{
 			PublicKey: c2.PrivateKey.Public(),

wgengine/magicsock/legacy.go

@@ -1,642 +0,0 @@
-// Copyright (c) 2019 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 magicsock
-
-import (
-	"bytes"
-	"crypto/hmac"
-	"crypto/subtle"
-	"encoding/binary"
-	"errors"
-	"hash"
-	"net"
-	"strings"
-	"sync"
-	"time"
-
-	"golang.org/x/crypto/blake2s"
-	"golang.org/x/crypto/chacha20poly1305"
-	"golang.org/x/crypto/poly1305"
-	"golang.zx2c4.com/wireguard/conn"
-	"golang.zx2c4.com/wireguard/tai64n"
-	"inet.af/netaddr"
-	"tailscale.com/ipn/ipnstate"
-	"tailscale.com/tstime/mono"
-	"tailscale.com/types/key"
-	"tailscale.com/types/logger"
-	"tailscale.com/types/wgkey"
-	"tailscale.com/wgengine/wgcfg"
-)
-
-var (
-	errNoDestinations = errors.New("magicsock: no destinations")
-	errDisabled       = errors.New("magicsock: legacy networking disabled")
-)
-
-// createLegacyEndpointLocked creates a new wireguard-go endpoint for a legacy connection.
-// pk is the public key of the remote peer. addrs is the ordered set of addresses for the remote peer.
-// rawDest is the encoded wireguard-go endpoint string. It should be treated as a black box.
-// It is provided so that addrSet.DstToString can return it when requested by wireguard-go.
-func (c *Conn) createLegacyEndpointLocked(pk key.Public, addrs wgcfg.IPPortSet, rawDest string) (conn.Endpoint, error) {
-	if c.disableLegacy {
-		return nil, errDisabled
-	}
-
-	a := &addrSet{
-		Logf:      c.logf,
-		publicKey: pk,
-		curAddr:   -1,
-		rawdst:    rawDest,
-	}
-	a.ipPorts = append(a.ipPorts, addrs.IPPorts()...)
-
-	// If this endpoint is being updated, remember its old set of
-	// endpoints so we can remove any (from c.addrsByUDP) that are
-	// not in the new set.
-	var oldIPP []netaddr.IPPort
-	if preva, ok := c.addrsByKey[pk]; ok {
-		oldIPP = preva.ipPorts
-	}
-	c.addrsByKey[pk] = a
-
-	// Add entries to c.addrsByUDP.
-	for _, ipp := range a.ipPorts {
-		if ipp.IP() == derpMagicIPAddr {
-			continue
-		}
-		c.addrsByUDP[ipp] = a
-	}
-
-	// Remove previous c.addrsByUDP entries that are no longer in the new set.
-	for _, ipp := range oldIPP {
-		if ipp.IP() != derpMagicIPAddr && c.addrsByUDP[ipp] != a {
-			delete(c.addrsByUDP, ipp)
-		}
-	}
-
-	return a, nil
-}
-
-func (c *Conn) findLegacyEndpointLocked(ipp netaddr.IPPort, packet []byte) conn.Endpoint {
-	if c.disableLegacy {
-		return nil
-	}
-
-	// Pre-disco: look up their addrSet.
-	if as, ok := c.addrsByUDP[ipp]; ok {
-		as.updateDst(ipp)
-		return as
-	}
-
-	// We don't know who this peer is. It's possible that it's one of
-	// our legitimate peers and they've roamed to an address we don't
-	// know. If this is a handshake packet, we can try to identify the
-	// peer in question.
-	if as := c.peerFromPacketLocked(packet); as != nil {
-		as.updateDst(ipp)
-		return as
-	}
-
-	// We have no idea who this is, drop the packet.
-	//
-	// In the past, when this magicsock implementation was the main
-	// one, we tried harder to find a match here: we would pass the
-	// packet into wireguard-go with a "singleEndpoint" implementation
-	// that wrapped the UDPAddr. Then, a patch we added to
-	// wireguard-go would call UpdateDst on that singleEndpoint after
-	// decrypting the packet and identifying the peer (if any),
-	// allowing us to update the relevant addrSet.
-	//
-	// This was a significant out of tree patch to wireguard-go, so we
-	// got rid of it, and instead switched to this logic you're
-	// reading now, which makes a best effort to identify sources for
-	// handshake packets (because they're relatively easy to turn into
-	// a peer public key statelessly), but otherwise drops packets
-	// that come from "roaming" addresses that aren't known to
-	// magicsock.
-	//
-	// The practical consequence of this is that some complex NAT
-	// traversal cases will now fail between a very old Tailscale
-	// client (0.96 and earlier) and a very new Tailscale
-	// client. However, those scenarios were likely also failing on
-	// all-old clients, because the probabilistic NAT opening didn't
-	// work reliably. So, in practice, this simplification means
-	// connectivity looks like this:
-	//
-	//  - old+old client: unchanged
-	//  - old+new client (easy network topology): unchanged
-	//  - old+new client (hard network topology): was bad, now a bit worse
-	//  - new+new client: unchanged
-	//
-	// This degradation is acceptable in that it continues to support
-	// the incremental upgrade of old clients that currently work
-	// well, which is our primary goal for the <100 clients still left
-	// on the oldest pre-DERP versions (as of 2021-01-12).
-	return nil
-}
-
-func (c *Conn) resetAddrSetStatesLocked() {
-	for _, as := range c.addrsByKey {
-		as.curAddr = -1
-		as.stopSpray = as.timeNow().Add(sprayPeriod)
-	}
-}
-
-func (c *Conn) sendAddrSet(b []byte, as *addrSet) error {
-	if c.disableLegacy {
-		return errDisabled
-	}
-
-	var addrBuf [8]netaddr.IPPort
-	dsts, roamAddr := as.appendDests(addrBuf[:0], b)
-
-	if len(dsts) == 0 {
-		return errNoDestinations
-	}
-
-	var success bool
-	var ret error
-	for _, addr := range dsts {
-		sent, err := c.sendAddr(addr, as.publicKey, b)
-		if sent {
-			success = true
-		} else if ret == nil {
-			ret = err
-		}
-		if err != nil && addr != roamAddr && c.sendLogLimit.Allow() {
-			if c.connCtx.Err() == nil { // don't log if we're closed
-				c.logf("magicsock: Conn.Send(%v): %v", addr, err)
-			}
-		}
-	}
-	if success {
-		return nil
-	}
-	return ret
-}
-
-// peerFromPacketLocked extracts returns the addrSet for the peer who sent
-// packet, if derivable.
-//
-// The derived addrSet is a hint, not a cryptographically strong
-// assertion. The returned value MUST NOT be used for any security
-// critical function. Callers MUST assume that the addrset can be
-// picked by a remote attacker.
-func (c *Conn) peerFromPacketLocked(packet []byte) *addrSet {
-	if len(packet) < 4 {
-		return nil
-	}
-	msgType := binary.LittleEndian.Uint32(packet[:4])
-	if msgType != messageInitiationType {
-		// Can't get peer out of a non-handshake packet.
-		return nil
-	}
-
-	var msg messageInitiation
-	reader := bytes.NewReader(packet)
-	err := binary.Read(reader, binary.LittleEndian, &msg)
-	if err != nil {
-		return nil
-	}
-
-	// Process just enough of the handshake to extract the long-term
-	// peer public key. We don't verify the handshake all the way, so
-	// this may be a spoofed packet. The extracted peer MUST NOT be
-	// used for any security critical function. In our case, we use it
-	// as a hint for roaming addresses.
-	var (
-		pub      = c.privateKey.Public()
-		hash     [blake2s.Size]byte
-		chainKey [blake2s.Size]byte
-		peerPK   key.Public
-		boxKey   [chacha20poly1305.KeySize]byte
-	)
-
-	mixHash(&hash, &initialHash, pub[:])
-	mixHash(&hash, &hash, msg.Ephemeral[:])
-	mixKey(&chainKey, &initialChainKey, msg.Ephemeral[:])
-
-	ss := c.privateKey.SharedSecret(key.Public(msg.Ephemeral))
-	if isZero(ss[:]) {
-		return nil
-	}
-
-	kdf2(&chainKey, &boxKey, chainKey[:], ss[:])
-	aead, _ := chacha20poly1305.New(boxKey[:])
-	_, err = aead.Open(peerPK[:0], zeroNonce[:], msg.Static[:], hash[:])
-	if err != nil {
-		return nil
-	}
-
-	return c.addrsByKey[peerPK]
-}
-
-func shouldSprayPacket(b []byte) bool {
-	if len(b) < 4 {
-		return false
-	}
-	msgType := binary.LittleEndian.Uint32(b[:4])
-	switch msgType {
-	case messageInitiationType,
-		messageResponseType,
-		messageCookieReplyType: // TODO: necessary?
-		return true
-	}
-	return false
-}
-
-const sprayPeriod = 3 * time.Second
-
-// appendDests appends to dsts the destinations that b should be
-// written to in order to reach as. Some of the returned IPPorts may
-// be fake addrs representing DERP servers.
-//
-// It also returns as's current roamAddr, if any.
-func (as *addrSet) appendDests(dsts []netaddr.IPPort, b []byte) (_ []netaddr.IPPort, roamAddr netaddr.IPPort) {
-	spray := shouldSprayPacket(b) // true for handshakes
-	now := as.timeNow()
-
-	as.mu.Lock()
-	defer as.mu.Unlock()
-
-	as.lastSend = now
-
-	// Spray logic.
-	//
-	// After exchanging a handshake with a peer, we send some outbound
-	// packets to every endpoint of that peer. These packets are spaced out
-	// over several seconds to make sure that our peer has an opportunity to
-	// send its own spray packet to us before we are done spraying.
-	//
-	// Multiple packets are necessary because we have to both establish the
-	// NAT mappings between two peers *and use* the mappings to switch away
-	// from DERP to a higher-priority UDP endpoint.
-	const sprayFreq = 250 * time.Millisecond
-	if spray {
-		as.lastSpray = now
-		as.stopSpray = now.Add(sprayPeriod)
-
-		// Reset our favorite route on new handshakes so we
-		// can downgrade to a worse path if our better path
-		// goes away. (https://github.com/tailscale/tailscale/issues/92)
-		as.curAddr = -1
-	} else if now.Before(as.stopSpray) {
-		// We are in the spray window. If it has been sprayFreq since we
-		// last sprayed a packet, spray this packet.
-		if now.Sub(as.lastSpray) >= sprayFreq {
-			spray = true
-			as.lastSpray = now
-		}
-	}
-
-	// Pick our destination address(es).
-	switch {
-	case spray:
-		// This packet is being sprayed to all addresses.
-		for i := range as.ipPorts {
-			dsts = append(dsts, as.ipPorts[i])
-		}
-		if as.roamAddr != nil {
-			dsts = append(dsts, *as.roamAddr)
-		}
-	case as.roamAddr != nil:
-		// We have a roaming address, prefer it over other addrs.
-		// TODO(danderson): this is not correct, there's no reason
-		// roamAddr should be special like this.
-		dsts = append(dsts, *as.roamAddr)
-	case as.curAddr != -1:
-		if as.curAddr >= len(as.ipPorts) {
-			as.Logf("[unexpected] magicsock bug: as.curAddr >= len(as.ipPorts): %d >= %d", as.curAddr, len(as.ipPorts))
-			break
-		}
-		// No roaming addr, but we've seen packets from a known peer
-		// addr, so keep using that one.
-		dsts = append(dsts, as.ipPorts[as.curAddr])
-	default:
-		// We know nothing about how to reach this peer, and we're not
-		// spraying. Use the first address in the array, which will
-		// usually be a DERP address that guarantees connectivity.
-		if len(as.ipPorts) > 0 {
-			dsts = append(dsts, as.ipPorts[0])
-		}
-	}
-
-	if logPacketDests {
-		as.Logf("spray=%v; roam=%v; dests=%v", spray, as.roamAddr, dsts)
-	}
-	if as.roamAddr != nil {
-		roamAddr = *as.roamAddr
-	}
-	return dsts, roamAddr
-}
-
-// addrSet is a set of UDP addresses that implements wireguard/conn.Endpoint.
-//
-// This is the legacy endpoint for peers that don't support discovery;
-// it predates discoEndpoint.
-type addrSet struct {
-	publicKey key.Public // peer public key used for DERP communication
-
-	// ipPorts is an ordered priority list provided by wgengine,
-	// sorted from expensive+slow+reliable at the begnining to
-	// fast+cheap at the end. More concretely, it's typically:
-	//
-	//     [DERP fakeip:node, Global IP:port, LAN ip:port]
-	//
-	// But there could be multiple or none of each.
-	ipPorts []netaddr.IPPort
-
-	// clock, if non-nil, is used in tests instead of time.Now.
-	clock func() mono.Time
-	Logf  logger.Logf // must not be nil
-
-	mu sync.Mutex // guards following fields
-
-	lastSend mono.Time
-
-	// roamAddr is non-nil if/when we receive a correctly signed
-	// WireGuard packet from an unexpected address. If so, we
-	// remember it and send responses there in the future, but
-	// this should hopefully never be used (or at least used
-	// rarely) in the case that all the components of Tailscale
-	// are correctly learning/sharing the network map details.
-	roamAddr *netaddr.IPPort
-
-	// curAddr is an index into addrs of the highest-priority
-	// address a valid packet has been received from so far.
-	// If no valid packet from addrs has been received, curAddr is -1.
-	curAddr int
-
-	// stopSpray is the time after which we stop spraying packets.
-	stopSpray mono.Time
-
-	// lastSpray is the last time we sprayed a packet.
-	lastSpray mono.Time
-
-	// loggedLogPriMask is a bit field of that tracks whether
-	// we've already logged about receiving a packet from a low
-	// priority ("low-pri") address when we already have curAddr
-	// set to a better one. This is only to suppress some
-	// redundant logs.
-	loggedLogPriMask uint32
-
-	// rawdst is the destination string from/for wireguard-go.
-	rawdst string
-}
-
-// derpID returns this addrSet's home DERP node, or 0 if none is found.
-func (as *addrSet) derpID() int {
-	for _, ua := range as.ipPorts {
-		if ua.IP() == derpMagicIPAddr {
-			return int(ua.Port())
-		}
-	}
-	return 0
-}
-
-func (as *addrSet) timeNow() mono.Time {
-	if as.clock != nil {
-		return as.clock()
-	}
-	return mono.Now()
-}
-
-var noAddr, _ = netaddr.FromStdAddr(net.ParseIP("127.127.127.127"), 127, "")
-
-func (a *addrSet) dst() netaddr.IPPort {
-	a.mu.Lock()
-	defer a.mu.Unlock()
-
-	if a.roamAddr != nil {
-		return *a.roamAddr
-	}
-	if len(a.ipPorts) == 0 {
-		return noAddr
-	}
-	i := a.curAddr
-	if i == -1 {
-		i = 0
-	}
-	return a.ipPorts[i]
-}
-
-func (a *addrSet) DstToBytes() []byte {
-	return packIPPort(a.dst())
-}
-func (a *addrSet) DstToString() string {
-	return a.rawdst
-}
-func (a *addrSet) DstIP() net.IP {
-	return a.dst().IP().IPAddr().IP // TODO: add netaddr accessor to cut an alloc here?
-}
-func (a *addrSet) SrcIP() net.IP       { return nil }
-func (a *addrSet) SrcToString() string { return "" }
-func (a *addrSet) ClearSrc()           {}
-
-// updateDst records receipt of a packet from new. This is used to
-// potentially update the transmit address used for this addrSet.
-func (a *addrSet) updateDst(new netaddr.IPPort) error {
-	if new.IP() == derpMagicIPAddr {
-		// Never consider DERP addresses as a viable candidate for
-		// either curAddr or roamAddr. It's only ever a last resort
-		// choice, never a preferred choice.
-		// This is a hot path for established connections.
-		return nil
-	}
-
-	a.mu.Lock()
-	defer a.mu.Unlock()
-
-	if a.roamAddr != nil && new == *a.roamAddr {
-		// Packet from the current roaming address, no logging.
-		// This is a hot path for established connections.
-		return nil
-	}
-	if a.roamAddr == nil && a.curAddr >= 0 && new == a.ipPorts[a.curAddr] {
-		// Packet from current-priority address, no logging.
-		// This is a hot path for established connections.
-		return nil
-	}
-
-	index := -1
-	for i := range a.ipPorts {
-		if new == a.ipPorts[i] {
-			index = i
-			break
-		}
-	}
-
-	publicKey := wgkey.Key(a.publicKey)
-	pk := publicKey.ShortString()
-	old := "<none>"
-	if a.curAddr >= 0 {
-		old = a.ipPorts[a.curAddr].String()
-	}
-
-	switch {
-	case index == -1:
-		if a.roamAddr == nil {
-			a.Logf("[v1] magicsock: rx %s from roaming address %s, set as new priority", pk, new)
-		} else {
-			a.Logf("[v1] magicsock: rx %s from roaming address %s, replaces roaming address %s", pk, new, a.roamAddr)
-		}
-		a.roamAddr = &new
-
-	case a.roamAddr != nil:
-		a.Logf("[v1] magicsock: rx %s from known %s (%d), replaces roaming address %s", pk, new, index, a.roamAddr)
-		a.roamAddr = nil
-		a.curAddr = index
-		a.loggedLogPriMask = 0
-
-	case a.curAddr == -1:
-		a.Logf("[v1] magicsock: rx %s from %s (%d/%d), set as new priority", pk, new, index, len(a.ipPorts))
-		a.curAddr = index
-		a.loggedLogPriMask = 0
-
-	case index < a.curAddr:
-		if 1 <= index && index <= 32 && (a.loggedLogPriMask&1<<(index-1)) == 0 {
-			a.Logf("[v1] magicsock: rx %s from low-pri %s (%d), keeping current %s (%d)", pk, new, index, old, a.curAddr)
-			a.loggedLogPriMask |= 1 << (index - 1)
-		}
-
-	default: // index > a.curAddr
-		a.Logf("[v1] magicsock: rx %s from %s (%d/%d), replaces old priority %s", pk, new, index, len(a.ipPorts), old)
-		a.curAddr = index
-		a.loggedLogPriMask = 0
-	}
-
-	return nil
-}
-
-func (a *addrSet) String() string {
-	a.mu.Lock()
-	defer a.mu.Unlock()
-
-	buf := new(strings.Builder)
-	buf.WriteByte('[')
-	if a.roamAddr != nil {
-		buf.WriteString("roam:")
-		sbPrintAddr(buf, *a.roamAddr)
-	}
-	for i, addr := range a.ipPorts {
-		if i > 0 || a.roamAddr != nil {
-			buf.WriteString(", ")
-		}
-		sbPrintAddr(buf, addr)
-		if a.curAddr == i {
-			buf.WriteByte('*')
-		}
-	}
-	buf.WriteByte(']')
-
-	return buf.String()
-}
-
-func (as *addrSet) populatePeerStatus(ps *ipnstate.PeerStatus) {
-	as.mu.Lock()
-	defer as.mu.Unlock()
-
-	ps.LastWrite = as.lastSend.WallTime()
-	for i, ua := range as.ipPorts {
-		if ua.IP() == derpMagicIPAddr {
-			continue
-		}
-		uaStr := ua.String()
-		ps.Addrs = append(ps.Addrs, uaStr)
-		if as.curAddr == i {
-			ps.CurAddr = uaStr
-		}
-	}
-	if as.roamAddr != nil {
-		ps.CurAddr = ippDebugString(*as.roamAddr)
-	}
-}
-
-// Message types copied from wireguard-go/device/noise-protocol.go
-const (
-	messageInitiationType  = 1
-	messageResponseType    = 2
-	messageCookieReplyType = 3
-)
-
-// Cryptographic constants copied from wireguard-go/device/noise-protocol.go
-var (
-	noiseConstruction = "Noise_IKpsk2_25519_ChaChaPoly_BLAKE2s"
-	wgIdentifier      = "WireGuard v1 zx2c4 [email protected]"
-	initialChainKey   [blake2s.Size]byte
-	initialHash       [blake2s.Size]byte
-	zeroNonce         [chacha20poly1305.NonceSize]byte
-)
-
-func init() {
-	initialChainKey = blake2s.Sum256([]byte(noiseConstruction))
-	mixHash(&initialHash, &initialChainKey, []byte(wgIdentifier))
-}
-
-// messageInitiation is the same as wireguard-go's MessageInitiation,
-// from wireguard-go/device/noise-protocol.go.
-type messageInitiation struct {
-	Type      uint32
-	Sender    uint32
-	Ephemeral wgkey.Key
-	Static    [wgkey.Size + poly1305.TagSize]byte
-	Timestamp [tai64n.TimestampSize + poly1305.TagSize]byte
-	MAC1      [blake2s.Size128]byte
-	MAC2      [blake2s.Size128]byte
-}
-
-func mixKey(dst *[blake2s.Size]byte, c *[blake2s.Size]byte, data []byte) {
-	kdf1(dst, c[:], data)
-}
-
-func mixHash(dst *[blake2s.Size]byte, h *[blake2s.Size]byte, data []byte) {
-	hash, _ := blake2s.New256(nil)
-	hash.Write(h[:])
-	hash.Write(data)
-	hash.Sum(dst[:0])
-	hash.Reset()
-}
-
-func hmac1(sum *[blake2s.Size]byte, key, in0 []byte) {
-	mac := hmac.New(func() hash.Hash {
-		h, _ := blake2s.New256(nil)
-		return h
-	}, key)
-	mac.Write(in0)
-	mac.Sum(sum[:0])
-}
-
-func hmac2(sum *[blake2s.Size]byte, key, in0, in1 []byte) {
-	mac := hmac.New(func() hash.Hash {
-		h, _ := blake2s.New256(nil)
-		return h
-	}, key)
-	mac.Write(in0)
-	mac.Write(in1)
-	mac.Sum(sum[:0])
-}
-
-func kdf1(t0 *[blake2s.Size]byte, key, input []byte) {
-	hmac1(t0, key, input)
-	hmac1(t0, t0[:], []byte{0x1})
-}
-
-func kdf2(t0, t1 *[blake2s.Size]byte, key, input []byte) {
-	var prk [blake2s.Size]byte
-	hmac1(&prk, key, input)
-	hmac1(t0, prk[:], []byte{0x1})
-	hmac2(t1, prk[:], t0[:], []byte{0x2})
-	for i := range prk[:] {
-		prk[i] = 0
-	}
-}
-
-func isZero(val []byte) bool {
-	acc := 1
-	for _, b := range val {
-		acc &= subtle.ConstantTimeByteEq(b, 0)
-	}
-	return acc == 1
-}

wgengine/magicsock/magicsock_test.go

@@ -9,7 +9,6 @@ import (
 	"context"
 	crand "crypto/rand"
 	"crypto/tls"
-	"encoding/binary"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -38,7 +37,6 @@ import (
 	"tailscale.com/tailcfg"
 	"tailscale.com/tstest"
 	"tailscale.com/tstest/natlab"
-	"tailscale.com/tstime/mono"
 	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/netmap"
@@ -137,7 +135,7 @@ type magicStack struct {
 // newMagicStack builds and initializes an idle magicsock and
 // friends. You need to call conn.SetNetworkMap and dev.Reconfig
 // before anything interesting happens.
-func newMagicStack(t testing.TB, logf logger.Logf, l nettype.PacketListener, derpMap *tailcfg.DERPMap, disableLegacy bool) *magicStack {
+func newMagicStack(t testing.TB, logf logger.Logf, l nettype.PacketListener, derpMap *tailcfg.DERPMap) *magicStack {
 	t.Helper()
 
 	privateKey, err := wgkey.NewPrivate()
@@ -152,8 +150,7 @@ func newMagicStack(t testing.TB, logf logger.Logf, l nettype.PacketListener, der
 		EndpointsFunc: func(eps []tailcfg.Endpoint) {
 			epCh <- eps
 		},
-		SimulatedNetwork:        l != nettype.Std{},
-		DisableLegacyNetworking: disableLegacy,
+		SimulatedNetwork: l != nettype.Std{},
 	})
 	if err != nil {
 		t.Fatalf("constructing magicsock: %v", err)
@@ -236,9 +233,7 @@ func (s *magicStack) IP() netaddr.IP {
 // and WireGuard configs into everyone to form a full mesh that has up
 // to date endpoint info. Think of it as an extremely stripped down
 // and purpose-built Tailscale control plane.
-//
-// meshStacks only supports disco connections, not legacy logic.
-func meshStacks(logf logger.Logf, ms []*magicStack) (cleanup func()) {
+func meshStacks(logf logger.Logf, mutateNetmap func(idx int, nm *netmap.NetworkMap), ms ...*magicStack) (cleanup func()) {
 	ctx, cancel := context.WithCancel(context.Background())
 
 	// Serialize all reconfigurations globally, just to keep things
@@ -273,6 +268,9 @@ func meshStacks(logf logger.Logf, ms []*magicStack) (cleanup func()) {
 			nm.Peers = append(nm.Peers, peer)
 		}
 
+		if mutateNetmap != nil {
+			mutateNetmap(myIdx, nm)
+		}
 		return nm
 	}
 
@@ -342,10 +340,9 @@ func TestNewConn(t *testing.T) {
 
 	port := pickPort(t)
 	conn, err := NewConn(Options{
-		Port:                    port,
-		EndpointsFunc:           epFunc,
-		Logf:                    t.Logf,
-		DisableLegacyNetworking: true,
+		Port:          port,
+		EndpointsFunc: epFunc,
+		Logf:          t.Logf,
 	})
 	if err != nil {
 		t.Fatal(err)
@@ -443,60 +440,9 @@ func TestPickDERPFallback(t *testing.T) {
 		t.Errorf("not sticky: got %v; want %v", got, someNode)
 	}
 
-	// But move if peers are elsewhere.
-	const otherNode = 789
-	c.addrsByKey = map[key.Public]*addrSet{
-		{1}: {ipPorts: []netaddr.IPPort{netaddr.IPPortFrom(derpMagicIPAddr, otherNode)}},
-	}
-	if got := c.pickDERPFallback(); got != otherNode {
-		t.Errorf("didn't join peers: got %v; want %v", got, someNode)
-	}
-}
-
-func makeConfigs(t *testing.T, addrs []netaddr.IPPort) []wgcfg.Config {
-	t.Helper()
-
-	var privKeys []wgkey.Private
-	var addresses [][]netaddr.IPPrefix
-
-	for i := range addrs {
-		privKey, err := wgkey.NewPrivate()
-		if err != nil {
-			t.Fatal(err)
-		}
-		privKeys = append(privKeys, wgkey.Private(privKey))
-
-		addresses = append(addresses, []netaddr.IPPrefix{
-			netaddr.MustParseIPPrefix(fmt.Sprintf("1.0.0.%d/32", i+1)),
-		})
-	}
-
-	var cfgs []wgcfg.Config
-	for i := range addrs {
-		cfg := wgcfg.Config{
-			Name:       fmt.Sprintf("peer%d", i+1),
-			PrivateKey: privKeys[i],
-			Addresses:  addresses[i],
-		}
-		for peerNum, addr := range addrs {
-			if peerNum == i {
-				continue
-			}
-			publicKey := privKeys[peerNum].Public()
-			peer := wgcfg.Peer{
-				PublicKey:  publicKey,
-				AllowedIPs: addresses[peerNum],
-				Endpoints: wgcfg.Endpoints{
-					PublicKey: publicKey,
-					IPPorts:   wgcfg.NewIPPortSet(addr),
-				},
-				PersistentKeepalive: 25,
-			}
-			cfg.Peers = append(cfg.Peers, peer)
-		}
-		cfgs = append(cfgs, cfg)
-	}
-	return cfgs
+	// TODO: test that disco-based clients changing to a new DERP
+	// region causes this fallback to also move, once disco clients
+	// have fixed DERP fallback logic.
 }
 
 // TestDeviceStartStop exercises the startup and shutdown logic of
@@ -510,9 +456,8 @@ func TestDeviceStartStop(t *testing.T) {
 	tstest.ResourceCheck(t)
 
 	conn, err := NewConn(Options{
-		EndpointsFunc:           func(eps []tailcfg.Endpoint) {},
-		Logf:                    t.Logf,
-		DisableLegacyNetworking: true,
+		EndpointsFunc: func(eps []tailcfg.Endpoint) {},
+		Logf:          t.Logf,
 	})
 	if err != nil {
 		t.Fatal(err)
@@ -552,12 +497,12 @@ func TestConnClosed(t *testing.T) {
 	derpMap, cleanup := runDERPAndStun(t, logf, d.stun, d.stunIP)
 	defer cleanup()
 
-	ms1 := newMagicStack(t, logger.WithPrefix(logf, "conn1: "), d.m1, derpMap, true)
+	ms1 := newMagicStack(t, logger.WithPrefix(logf, "conn1: "), d.m1, derpMap)
 	defer ms1.Close()
-	ms2 := newMagicStack(t, logger.WithPrefix(logf, "conn2: "), d.m2, derpMap, true)
+	ms2 := newMagicStack(t, logger.WithPrefix(logf, "conn2: "), d.m2, derpMap)
 	defer ms2.Close()
 
-	cleanup = meshStacks(t.Logf, []*magicStack{ms1, ms2})
+	cleanup = meshStacks(t.Logf, nil, ms1, ms2)
 	defer cleanup()
 
 	pkt := tuntest.Ping(ms2.IP().IPAddr().IP, ms1.IP().IPAddr().IP)
@@ -617,6 +562,54 @@ func TestTwoDevicePing(t *testing.T) {
 	testTwoDevicePing(t, n)
 }
 
+// Legacy clients appear to new code as peers that know about DERP and
+// WireGuard, but don't have a disco key. Check that we can still
+// communicate successfully with such peers.
+func TestNoDiscoKey(t *testing.T) {
+	tstest.PanicOnLog()
+	tstest.ResourceCheck(t)
+
+	derpMap, cleanup := runDERPAndStun(t, t.Logf, nettype.Std{}, netaddr.IPv4(127, 0, 0, 1))
+	defer cleanup()
+
+	m1 := newMagicStack(t, t.Logf, nettype.Std{}, derpMap)
+	defer m1.Close()
+	m2 := newMagicStack(t, t.Logf, nettype.Std{}, derpMap)
+	defer m2.Close()
+
+	removeDisco := func(idx int, nm *netmap.NetworkMap) {
+		for _, p := range nm.Peers {
+			p.DiscoKey = tailcfg.DiscoKey{}
+		}
+	}
+
+	cleanupMesh := meshStacks(t.Logf, removeDisco, m1, m2)
+	defer cleanupMesh()
+
+	// Wait for both peers to know about each other before we try to
+	// ping.
+	for {
+		if s1 := m1.Status(); len(s1.Peer) != 1 {
+			time.Sleep(10 * time.Millisecond)
+			continue
+		}
+		if s2 := m2.Status(); len(s2.Peer) != 1 {
+			time.Sleep(10 * time.Millisecond)
+			continue
+		}
+		break
+	}
+
+	pkt := tuntest.Ping(m2.IP().IPAddr().IP, m1.IP().IPAddr().IP)
+	m1.tun.Outbound <- pkt
+	select {
+	case <-m2.tun.Inbound:
+		t.Logf("ping m1>m2 ok")
+	case <-time.After(10 * time.Second):
+		t.Fatalf("timed out waiting for ping to transit")
+	}
+}
+
 func TestActiveDiscovery(t *testing.T) {
 	t.Run("simple_internet", func(t *testing.T) {
 		t.Parallel()
@@ -685,11 +678,11 @@ func TestActiveDiscovery(t *testing.T) {
 		inet := natlab.NewInternet()
 		lan1 := &natlab.Network{
 			Name:    "lan1",
-			Prefix4: mustPrefix("192.168.0.0/24"),
+			Prefix4: netaddr.MustParseIPPrefix("192.168.0.0/24"),
 		}
 		lan2 := &natlab.Network{
 			Name:    "lan2",
-			Prefix4: mustPrefix("192.168.1.0/24"),
+			Prefix4: netaddr.MustParseIPPrefix("192.168.1.0/24"),
 		}
 
 		sif := mstun.Attach("eth0", inet)
@@ -729,14 +722,6 @@ func TestActiveDiscovery(t *testing.T) {
 	})
 }
 
-func mustPrefix(s string) netaddr.IPPrefix {
-	pfx, err := netaddr.ParseIPPrefix(s)
-	if err != nil {
-		panic(err)
-	}
-	return pfx
-}
-
 type devices struct {
 	m1   nettype.PacketListener
 	m1IP netaddr.IP
@@ -838,12 +823,12 @@ func testActiveDiscovery(t *testing.T, d *devices) {
 	derpMap, cleanup := runDERPAndStun(t, logf, d.stun, d.stunIP)
 	defer cleanup()
 
-	m1 := newMagicStack(t, logger.WithPrefix(logf, "conn1: "), d.m1, derpMap, true)
+	m1 := newMagicStack(t, logger.WithPrefix(logf, "conn1: "), d.m1, derpMap)
 	defer m1.Close()
-	m2 := newMagicStack(t, logger.WithPrefix(logf, "conn2: "), d.m2, derpMap, true)
+	m2 := newMagicStack(t, logger.WithPrefix(logf, "conn2: "), d.m2, derpMap)
 	defer m2.Close()
 
-	cleanup = meshStacks(logf, []*magicStack{m1, m2})
+	cleanup = meshStacks(logf, nil, m1, m2)
 	defer cleanup()
 
 	m1IP := m1.IP()
@@ -894,21 +879,62 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 	derpMap, cleanup := runDERPAndStun(t, logf, d.stun, d.stunIP)
 	defer cleanup()
 
-	m1 := newMagicStack(t, logf, d.m1, derpMap, false)
+	m1 := newMagicStack(t, logf, d.m1, derpMap)
 	defer m1.Close()
-	m2 := newMagicStack(t, logf, d.m2, derpMap, false)
+	m2 := newMagicStack(t, logf, d.m2, derpMap)
 	defer m2.Close()
 
-	addrs := []netaddr.IPPort{
-		netaddr.IPPortFrom(d.m1IP, m1.conn.LocalPort()),
-		netaddr.IPPortFrom(d.m2IP, m2.conn.LocalPort()),
+	cleanupMesh := meshStacks(logf, nil, m1, m2)
+	defer cleanupMesh()
+
+	// Wait for magicsock to be told about peers from meshStacks.
+	tstest.WaitFor(10*time.Second, func() error {
+		if p := m1.Status().Peer[key.Public(m2.privateKey.Public())]; p == nil || !p.InMagicSock {
+			return errors.New("m1 not ready")
+		}
+		if p := m2.Status().Peer[key.Public(m1.privateKey.Public())]; p == nil || !p.InMagicSock {
+			return errors.New("m2 not ready")
+		}
+		return nil
+	})
+
+	m1cfg := &wgcfg.Config{
+		Name:       "peer1",
+		PrivateKey: m1.privateKey,
+		Addresses:  []netaddr.IPPrefix{netaddr.MustParseIPPrefix("1.0.0.1/32")},
+		Peers: []wgcfg.Peer{
+			wgcfg.Peer{
+				PublicKey:  m2.privateKey.Public(),
+				AllowedIPs: []netaddr.IPPrefix{netaddr.MustParseIPPrefix("1.0.0.2/32")},
+				Endpoints: wgcfg.Endpoints{
+					PublicKey: m2.privateKey.Public(),
+					DiscoKey:  m2.conn.DiscoPublicKey(),
+				},
+				PersistentKeepalive: 25,
+			},
+		},
+	}
+	m2cfg := &wgcfg.Config{
+		Name:       "peer2",
+		PrivateKey: m2.privateKey,
+		Addresses:  []netaddr.IPPrefix{netaddr.MustParseIPPrefix("1.0.0.2/32")},
+		Peers: []wgcfg.Peer{
+			wgcfg.Peer{
+				PublicKey:  m1.privateKey.Public(),
+				AllowedIPs: []netaddr.IPPrefix{netaddr.MustParseIPPrefix("1.0.0.1/32")},
+				Endpoints: wgcfg.Endpoints{
+					PublicKey: m1.privateKey.Public(),
+					DiscoKey:  m1.conn.DiscoPublicKey(),
+				},
+				PersistentKeepalive: 25,
+			},
+		},
 	}
-	cfgs := makeConfigs(t, addrs)
 
-	if err := m1.Reconfig(&cfgs[0]); err != nil {
+	if err := m1.Reconfig(m1cfg); err != nil {
 		t.Fatal(err)
 	}
-	if err := m2.Reconfig(&cfgs[1]); err != nil {
+	if err := m2.Reconfig(m2cfg); err != nil {
 		t.Fatal(err)
 	}
 
@@ -997,7 +1023,7 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 	t.Run("no-op dev1 reconfig", func(t *testing.T) {
 		setT(t)
 		defer setT(outerT)
-		if err := m1.Reconfig(&cfgs[0]); err != nil {
+		if err := m1.Reconfig(m1cfg); err != nil {
 			t.Fatal(err)
 		}
 		ping1(t)
@@ -1005,176 +1031,6 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 	})
 }
 
-// TestAddrSet tests addrSet appendDests and updateDst.
-func TestAddrSet(t *testing.T) {
-	tstest.PanicOnLog()
-	tstest.ResourceCheck(t)
-
-	mustIPPortPtr := func(s string) *netaddr.IPPort {
-		ipp := netaddr.MustParseIPPort(s)
-		return &ipp
-	}
-	ipps := func(ss ...string) (ret []netaddr.IPPort) {
-		t.Helper()
-		for _, s := range ss {
-			ret = append(ret, netaddr.MustParseIPPort(s))
-		}
-		return ret
-	}
-	joinUDPs := func(in []netaddr.IPPort) string {
-		var sb strings.Builder
-		for i, ua := range in {
-			if i > 0 {
-				sb.WriteByte(',')
-			}
-			sb.WriteString(ua.String())
-		}
-		return sb.String()
-	}
-	var (
-		regPacket   = []byte("some regular packet")
-		sprayPacket = []byte("0000")
-	)
-	binary.LittleEndian.PutUint32(sprayPacket[:4], device.MessageInitiationType)
-	if !shouldSprayPacket(sprayPacket) {
-		t.Fatal("sprayPacket should be classified as a spray packet for testing")
-	}
-
-	// A step is either a b+want appendDests tests, or an
-	// UpdateDst call, depending on which fields are set.
-	type step struct {
-		// advance is the time to advance the fake clock
-		// before the step.
-		advance time.Duration
-
-		// updateDst, if set, does an UpdateDst call and
-		// b+want are ignored.
-		updateDst *netaddr.IPPort
-
-		b    []byte
-		want string // comma-separated
-	}
-	tests := []struct {
-		name     string
-		as       *addrSet
-		steps    []step
-		logCheck func(t *testing.T, logged []byte)
-	}{
-		{
-			name: "reg_packet_no_curaddr",
-			as: &addrSet{
-				ipPorts:  ipps("127.3.3.40:1", "123.45.67.89:123", "10.0.0.1:123"),
-				curAddr:  -1, // unknown
-				roamAddr: nil,
-			},
-			steps: []step{
-				{b: regPacket, want: "127.3.3.40:1"},
-			},
-		},
-		{
-			name: "reg_packet_have_curaddr",
-			as: &addrSet{
-				ipPorts:  ipps("127.3.3.40:1", "123.45.67.89:123", "10.0.0.1:123"),
-				curAddr:  1, // global IP
-				roamAddr: nil,
-			},
-			steps: []step{
-				{b: regPacket, want: "123.45.67.89:123"},
-			},
-		},
-		{
-			name: "reg_packet_have_roamaddr",
-			as: &addrSet{
-				ipPorts:  ipps("127.3.3.40:1", "123.45.67.89:123", "10.0.0.1:123"),
-				curAddr:  2, // should be ignored
-				roamAddr: mustIPPortPtr("5.6.7.8:123"),
-			},
-			steps: []step{
-				{b: regPacket, want: "5.6.7.8:123"},
-				{updateDst: mustIPPortPtr("10.0.0.1:123")}, // no more roaming
-				{b: regPacket, want: "10.0.0.1:123"},
-			},
-		},
-		{
-			name: "start_roaming",
-			as: &addrSet{
-				ipPorts: ipps("127.3.3.40:1", "123.45.67.89:123", "10.0.0.1:123"),
-				curAddr: 2,
-			},
-			steps: []step{
-				{b: regPacket, want: "10.0.0.1:123"},
-				{updateDst: mustIPPortPtr("4.5.6.7:123")},
-				{b: regPacket, want: "4.5.6.7:123"},
-				{updateDst: mustIPPortPtr("5.6.7.8:123")},
-				{b: regPacket, want: "5.6.7.8:123"},
-				{updateDst: mustIPPortPtr("123.45.67.89:123")}, // end roaming
-				{b: regPacket, want: "123.45.67.89:123"},
-			},
-		},
-		{
-			name: "spray_packet",
-			as: &addrSet{
-				ipPorts:  ipps("127.3.3.40:1", "123.45.67.89:123", "10.0.0.1:123"),
-				curAddr:  2, // should be ignored
-				roamAddr: mustIPPortPtr("5.6.7.8:123"),
-			},
-			steps: []step{
-				{b: sprayPacket, want: "127.3.3.40:1,123.45.67.89:123,10.0.0.1:123,5.6.7.8:123"},
-				{advance: 300 * time.Millisecond, b: regPacket, want: "127.3.3.40:1,123.45.67.89:123,10.0.0.1:123,5.6.7.8:123"},
-				{advance: 300 * time.Millisecond, b: regPacket, want: "127.3.3.40:1,123.45.67.89:123,10.0.0.1:123,5.6.7.8:123"},
-				{advance: 3, b: regPacket, want: "5.6.7.8:123"},
-				{advance: 2 * time.Millisecond, updateDst: mustIPPortPtr("10.0.0.1:123")},
-				{advance: 3, b: regPacket, want: "10.0.0.1:123"},
-			},
-		},
-		{
-			name: "low_pri",
-			as: &addrSet{
-				ipPorts: ipps("127.3.3.40:1", "123.45.67.89:123", "10.0.0.1:123"),
-				curAddr: 2,
-			},
-			steps: []step{
-				{updateDst: mustIPPortPtr("123.45.67.89:123")},
-				{updateDst: mustIPPortPtr("123.45.67.89:123")},
-			},
-			logCheck: func(t *testing.T, logged []byte) {
-				if n := bytes.Count(logged, []byte(", keeping current ")); n != 1 {
-					t.Errorf("low-prio keeping current logged %d times; want 1", n)
-				}
-			},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			faket := mono.Time(0)
-			var logBuf bytes.Buffer
-			tt.as.Logf = func(format string, args ...interface{}) {
-				fmt.Fprintf(&logBuf, format, args...)
-				t.Logf(format, args...)
-			}
-			tt.as.clock = func() mono.Time { return faket }
-			for i, st := range tt.steps {
-				faket = faket.Add(st.advance)
-
-				if st.updateDst != nil {
-					if err := tt.as.updateDst(*st.updateDst); err != nil {
-						t.Fatal(err)
-					}
-					continue
-				}
-				got, _ := tt.as.appendDests(nil, st.b)
-				if gotStr := joinUDPs(got); gotStr != st.want {
-					t.Errorf("step %d: got %v; want %v", i, gotStr, st.want)
-				}
-			}
-			if tt.logCheck != nil {
-				tt.logCheck(t, logBuf.Bytes())
-			}
-		})
-	}
-}
-
 func TestDiscoMessage(t *testing.T) {
 	c := newConn()
 	c.logf = t.Logf
@@ -1182,16 +1038,15 @@ func TestDiscoMessage(t *testing.T) {
 
 	peer1Pub := c.DiscoPublicKey()
 	peer1Priv := c.discoPrivate
-	c.endpointOfDisco = map[tailcfg.DiscoKey]*discoEndpoint{
-		tailcfg.DiscoKey(peer1Pub): {
-			// ... (enough for this test)
-		},
-	}
-	c.nodeOfDisco = map[tailcfg.DiscoKey]*tailcfg.Node{
-		tailcfg.DiscoKey(peer1Pub): {
-			// ... (enough for this test)
-		},
-	}
+	n := &tailcfg.Node{
+		Key:      tailcfg.NodeKey(key.NewPrivate().Public()),
+		DiscoKey: peer1Pub,
+	}
+	c.peerMap.upsertNode(n)
+	c.peerMap.upsertDiscoEndpoint(&discoEndpoint{
+		publicKey: n.Key,
+		discoKey:  n.DiscoKey,
+	})
 
 	const payload = "why hello"
 
@@ -1242,8 +1097,8 @@ func Test32bitAlignment(t *testing.T) {
 	}
 }
 
-// newNonLegacyTestConn returns a new Conn with DisableLegacyNetworking set true.
-func newNonLegacyTestConn(t testing.TB) *Conn {
+// newTestConn returns a new Conn.
+func newTestConn(t testing.TB) *Conn {
 	t.Helper()
 	port := pickPort(t)
 	conn, err := NewConn(Options{
@@ -1252,7 +1107,6 @@ func newNonLegacyTestConn(t testing.TB) *Conn {
 		EndpointsFunc: func(eps []tailcfg.Endpoint) {
 			t.Logf("endpoints: %q", eps)
 		},
-		DisableLegacyNetworking: true,
 	})
 	if err != nil {
 		t.Fatal(err)
@@ -1301,7 +1155,7 @@ func addTestEndpoint(tb testing.TB, conn *Conn, sendConn net.PacketConn) (tailcf
 }
 
 func setUpReceiveFrom(tb testing.TB) (roundTrip func()) {
-	conn := newNonLegacyTestConn(tb)
+	conn := newTestConn(tb)
 	tb.Cleanup(func() { conn.Close() })
 	conn.logf = logger.Discard
 
@@ -1451,7 +1305,7 @@ func logBufWriter(buf *bytes.Buffer) logger.Logf {
 //
 // https://github.com/tailscale/tailscale/issues/1391
 func TestSetNetworkMapChangingNodeKey(t *testing.T) {
-	conn := newNonLegacyTestConn(t)
+	conn := newTestConn(t)
 	t.Cleanup(func() { conn.Close() })
 	var logBuf bytes.Buffer
 	conn.logf = logBufWriter(&logBuf)
@@ -1488,14 +1342,14 @@ func TestSetNetworkMapChangingNodeKey(t *testing.T) {
 		})
 	}
 
-	de := conn.endpointOfDisco[discoKey]
-	if de != nil && de.publicKey != nodeKey2 {
+	de, ok := conn.peerMap.discoEndpointForDiscoKey(discoKey)
+	if ok && de.publicKey != nodeKey2 {
 		t.Fatalf("discoEndpoint public key = %q; want %q", de.publicKey[:], nodeKey2[:])
 	}
 
 	log := logBuf.String()
 	wantSub := map[string]int{
-		"magicsock: got updated network map; 1 peers (1 with discokey)":                                                                           2,
+		"magicsock: got updated network map; 1 peers": 2,
 		"magicsock: disco key discokey:0000000000000000000000000000000000000000000000000000000000000001 changed from node key [TksxA] to [TksyA]": 1,
 	}
 	for sub, want := range wantSub {
@@ -1510,7 +1364,7 @@ func TestSetNetworkMapChangingNodeKey(t *testing.T) {
 }
 
 func TestRebindStress(t *testing.T) {
-	conn := newNonLegacyTestConn(t)
+	conn := newTestConn(t)
 
 	var logBuf bytes.Buffer
 	conn.logf = logBufWriter(&logBuf)

wgengine/wgcfg/clone.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.
 
-// Code generated by tailscale.com/cmd/cloner -type Config,Peer,Endpoints,IPPortSet; DO NOT EDIT.
+// Code generated by tailscale.com/cmd/cloner -type Config,Peer,Endpoints; DO NOT EDIT.
 
 package wgcfg
 
@@ -30,7 +30,7 @@ func (src *Config) Clone() *Config {
 }
 
 // A compilation failure here means this code must be regenerated, with command:
-//   tailscale.com/cmd/cloner -type Config,Peer,Endpoints,IPPortSet
+//   tailscale.com/cmd/cloner -type Config,Peer,Endpoints
 var _ConfigNeedsRegeneration = Config(struct {
 	Name       string
 	PrivateKey wgkey.Private
@@ -49,12 +49,11 @@ func (src *Peer) Clone() *Peer {
 	dst := new(Peer)
 	*dst = *src
 	dst.AllowedIPs = append(src.AllowedIPs[:0:0], src.AllowedIPs...)
-	dst.Endpoints = *src.Endpoints.Clone()
 	return dst
 }
 
 // A compilation failure here means this code must be regenerated, with command:
-//   tailscale.com/cmd/cloner -type Config,Peer,Endpoints,IPPortSet
+//   tailscale.com/cmd/cloner -type Config,Peer,Endpoints
 var _PeerNeedsRegeneration = Peer(struct {
 	PublicKey           wgkey.Key
 	AllowedIPs          []netaddr.IPPrefix
@@ -70,32 +69,12 @@ func (src *Endpoints) Clone() *Endpoints {
 	}
 	dst := new(Endpoints)
 	*dst = *src
-	dst.IPPorts = *src.IPPorts.Clone()
 	return dst
 }
 
 // A compilation failure here means this code must be regenerated, with command:
-//   tailscale.com/cmd/cloner -type Config,Peer,Endpoints,IPPortSet
+//   tailscale.com/cmd/cloner -type Config,Peer,Endpoints
 var _EndpointsNeedsRegeneration = Endpoints(struct {
 	PublicKey wgkey.Key
 	DiscoKey  tailcfg.DiscoKey
-	IPPorts   IPPortSet
-}{})
-
-// Clone makes a deep copy of IPPortSet.
-// The result aliases no memory with the original.
-func (src *IPPortSet) Clone() *IPPortSet {
-	if src == nil {
-		return nil
-	}
-	dst := new(IPPortSet)
-	*dst = *src
-	dst.ipp = append(src.ipp[:0:0], src.ipp...)
-	return dst
-}
-
-// A compilation failure here means this code must be regenerated, with command:
-//   tailscale.com/cmd/cloner -type Config,Peer,Endpoints,IPPortSet
-var _IPPortSetNeedsRegeneration = IPPortSet(struct {
-	ipp []netaddr.IPPort
 }{})

wgengine/wgcfg/config.go

@@ -6,15 +6,12 @@
 package wgcfg
 
 import (
-	"encoding/json"
-	"strings"
-
 	"inet.af/netaddr"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/wgkey"
 )
 
-//go:generate go run tailscale.com/cmd/cloner -type=Config,Peer,Endpoints,IPPortSet -output=clone.go
+//go:generate go run tailscale.com/cmd/cloner -type=Config,Peer,Endpoints -output=clone.go
 
 // Config is a WireGuard configuration.
 // It only supports the set of things Tailscale uses.
@@ -36,84 +33,13 @@ type Peer struct {
 
 // Endpoints represents the routes to reach a remote node.
 // It is serialized and provided to wireguard-go as a conn.Endpoint.
+//
+// TODO: change name, it's now just a pair of keys representing a peer.
 type Endpoints struct {
 	// PublicKey is the public key for the remote node.
 	PublicKey wgkey.Key `json:"pk"`
 	// DiscoKey is the disco key associated with the remote node.
 	DiscoKey tailcfg.DiscoKey `json:"dk,omitempty"`
-	// IPPorts is a set of possible ip+ports the remote node can be reached at.
-	// This is used only for legacy connections to pre-disco (pre-0.100) peers.
-	IPPorts IPPortSet `json:"ipp,omitempty"`
-}
-
-func (e Endpoints) Equal(f Endpoints) bool {
-	if e.PublicKey != f.PublicKey {
-		return false
-	}
-	if e.DiscoKey != f.DiscoKey {
-		return false
-	}
-	return e.IPPorts.EqualUnordered(f.IPPorts)
-}
-
-// IPPortSet is an immutable slice of netaddr.IPPorts.
-type IPPortSet struct {
-	ipp []netaddr.IPPort
-}
-
-// NewIPPortSet returns an IPPortSet containing the ports in ipp.
-func NewIPPortSet(ipps ...netaddr.IPPort) IPPortSet {
-	return IPPortSet{ipp: append(ipps[:0:0], ipps...)}
-}
-
-// String returns a comma-separated list of all IPPorts in s.
-func (s IPPortSet) String() string {
-	buf := new(strings.Builder)
-	for i, ipp := range s.ipp {
-		if i > 0 {
-			buf.WriteByte(',')
-		}
-		buf.WriteString(ipp.String())
-	}
-	return buf.String()
-}
-
-// IPPorts returns a slice of netaddr.IPPorts containing the IPPorts in s.
-func (s IPPortSet) IPPorts() []netaddr.IPPort {
-	return append(s.ipp[:0:0], s.ipp...)
-}
-
-// EqualUnordered reports whether s and t contain the same IPPorts, regardless of order.
-func (s IPPortSet) EqualUnordered(t IPPortSet) bool {
-	if len(s.ipp) != len(t.ipp) {
-		return false
-	}
-	// Check whether the endpoints are the same, regardless of order.
-	ipps := make(map[netaddr.IPPort]int, len(s.ipp))
-	for _, ipp := range s.ipp {
-		ipps[ipp]++
-	}
-	for _, ipp := range t.ipp {
-		ipps[ipp]--
-	}
-	for _, n := range ipps {
-		if n != 0 {
-			return false
-		}
-	}
-	return true
-}
-
-// MarshalJSON marshals s into JSON.
-// It is necessary so that IPPortSet's fields can be unexported, to guarantee immutability.
-func (s IPPortSet) MarshalJSON() ([]byte, error) {
-	return json.Marshal(s.ipp)
-}
-
-// UnmarshalJSON unmarshals s from JSON.
-// It is necessary so that IPPortSet's fields can be unexported, to guarantee immutability.
-func (s *IPPortSet) UnmarshalJSON(b []byte) error {
-	return json.Unmarshal(b, &s.ipp)
 }
 
 // PeerWithKey returns the Peer with key k and reports whether it was found.

wgengine/wgcfg/device_test.go

@@ -20,6 +20,7 @@ import (
 	"golang.zx2c4.com/wireguard/device"
 	"golang.zx2c4.com/wireguard/tun"
 	"inet.af/netaddr"
+	"tailscale.com/tailcfg"
 	"tailscale.com/types/wgkey"
 )
 
@@ -128,7 +129,7 @@ func TestDeviceConfig(t *testing.T) {
 	})
 
 	t.Run("device1 modify peer", func(t *testing.T) {
-		cfg1.Peers[0].Endpoints.IPPorts = NewIPPortSet(netaddr.MustParseIPPort("1.2.3.4:12345"))
+		cfg1.Peers[0].Endpoints.DiscoKey = tailcfg.DiscoKey{1}
 		if err := ReconfigDevice(device1, cfg1, t.Logf); err != nil {
 			t.Fatal(err)
 		}
@@ -136,7 +137,7 @@ func TestDeviceConfig(t *testing.T) {
 	})
 
 	t.Run("device1 replace endpoint", func(t *testing.T) {
-		cfg1.Peers[0].Endpoints.IPPorts = NewIPPortSet(netaddr.MustParseIPPort("1.1.1.1:123"))
+		cfg1.Peers[0].Endpoints.DiscoKey = tailcfg.DiscoKey{2}
 		if err := ReconfigDevice(device1, cfg1, t.Logf); err != nil {
 			t.Fatal(err)
 		}

wgengine/wgcfg/nmcfg/nmcfg.go

@@ -78,19 +78,6 @@ func WGCfg(nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags,
 		}
 
 		cpeer.Endpoints = wgcfg.Endpoints{PublicKey: wgkey.Key(peer.Key), DiscoKey: peer.DiscoKey}
-		if peer.DiscoKey.IsZero() {
-			// Legacy connection. Add IP+port endpoints.
-			var ipps []netaddr.IPPort
-			if err := appendEndpoint(cpeer, &ipps, peer.DERP); err != nil {
-				return nil, err
-			}
-			for _, ep := range peer.Endpoints {
-				if err := appendEndpoint(cpeer, &ipps, ep); err != nil {
-					return nil, err
-				}
-			}
-			cpeer.Endpoints.IPPorts = wgcfg.NewIPPortSet(ipps...)
-		}
 		didExitNodeWarn := false
 		for _, allowedIP := range peer.AllowedIPs {
 			if allowedIP.Bits() == 0 && peer.StableID != exitNode {
@@ -135,15 +122,3 @@ func WGCfg(nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags,
 
 	return cfg, nil
 }
-
-func appendEndpoint(peer *wgcfg.Peer, ipps *[]netaddr.IPPort, epStr string) error {
-	if epStr == "" {
-		return nil
-	}
-	ipp, err := netaddr.ParseIPPort(epStr)
-	if err != nil {
-		return fmt.Errorf("malformed endpoint %q for peer %v", epStr, peer.PublicKey.ShortString())
-	}
-	*ipps = append(*ipps, ipp)
-	return nil
-}

wgengine/wgcfg/writer.go

@@ -52,7 +52,7 @@ func (cfg *Config) ToUAPI(w io.Writer, prev *Config) error {
 		setPeer(p)
 		set("protocol_version", "1")
 
-		if !oldPeer.Endpoints.Equal(p.Endpoints) {
+		if oldPeer.Endpoints != p.Endpoints {
 			buf, err := json.Marshal(p.Endpoints)
 			if err != nil {
 				return err