net/portmapper: fix UPnP probing, work against all ports

Prior to Tailscale 1.12 it detected UPnP on any port.
Starting with Tailscale 1.11.x, it stopped detecting UPnP on all ports.

Then start plumbing its discovered Location header port number to the
code that was assuming port 5000.

Fixes #2109

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

cmd/tailscaled/debug.go

@@ -14,18 +14,23 @@ import (
 	"io"
 	"io/ioutil"
 	"log"
+	"net"
 	"net/http"
 	"net/http/httptrace"
 	"net/url"
 	"os"
+	"strings"
 	"time"
 
+	"inet.af/netaddr"
 	"tailscale.com/derp/derphttp"
 	"tailscale.com/ipn"
 	"tailscale.com/net/interfaces"
+	"tailscale.com/net/portmapper"
 	"tailscale.com/net/tshttpproxy"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
+	"tailscale.com/types/logger"
 	"tailscale.com/wgengine/monitor"
 )
 
@@ -33,6 +38,7 @@ var debugArgs struct {
 	monitor   bool
 	getURL    string
 	derpCheck string
+	portmap   bool
 }
 
 var debugModeFunc = debugMode // so it can be addressable
@@ -40,6 +46,7 @@ var debugModeFunc = debugMode // so it can be addressable
 func debugMode(args []string) error {
 	fs := flag.NewFlagSet("debug", flag.ExitOnError)
 	fs.BoolVar(&debugArgs.monitor, "monitor", false, "If true, run link monitor forever. Precludes all other options.")
+	fs.BoolVar(&debugArgs.portmap, "portmap", false, "If true, run portmap debugging. Precludes all other options.")
 	fs.StringVar(&debugArgs.getURL, "get-url", "", "If non-empty, fetch provided URL.")
 	fs.StringVar(&debugArgs.derpCheck, "derp", "", "if non-empty, test a DERP ping via named region code")
 	if err := fs.Parse(args); err != nil {
@@ -55,6 +62,9 @@ func debugMode(args []string) error {
 	if debugArgs.monitor {
 		return runMonitor(ctx)
 	}
+	if debugArgs.portmap {
+		return debugPortmap(ctx)
+	}
 	if debugArgs.getURL != "" {
 		return getURL(ctx, debugArgs.getURL)
 	}
@@ -191,3 +201,83 @@ func checkDerp(ctx context.Context, derpRegion string) error {
 	log.Printf("ok")
 	return err
 }
+
+func debugPortmap(ctx context.Context) error {
+	ctx, cancel := context.WithTimeout(ctx, 3*time.Second)
+	defer cancel()
+
+	portmapper.VerboseLogs = true
+
+	done := make(chan bool, 1)
+
+	var c *portmapper.Client
+	logf := log.Printf
+	c = portmapper.NewClient(logger.WithPrefix(logf, "portmapper: "), func() {
+		logf("portmapping changed.")
+		logf("have mapping: %v", c.HaveMapping())
+
+		if ext, ok := c.GetCachedMappingOrStartCreatingOne(); ok {
+			logf("cb: mapping: %v", ext)
+			select {
+			case done <- true:
+			default:
+			}
+			return
+		}
+		logf("cb: no mapping")
+	})
+	linkMon, err := monitor.New(logger.WithPrefix(logf, "monitor: "))
+	if err != nil {
+		return err
+	}
+
+	gatewayAndSelfIP := func() (gw, self netaddr.IP, ok bool) {
+		if v := os.Getenv("TS_DEBUG_GW_SELF"); strings.Contains(v, "/") {
+			i := strings.Index(v, "/")
+			gw = netaddr.MustParseIP(v[:i])
+			self = netaddr.MustParseIP(v[i+1:])
+			return gw, self, true
+		}
+		return linkMon.GatewayAndSelfIP()
+	}
+
+	c.SetGatewayLookupFunc(gatewayAndSelfIP)
+
+	gw, selfIP, ok := gatewayAndSelfIP()
+	if !ok {
+		logf("no gateway or self IP; %v", linkMon.InterfaceState())
+		return nil
+	}
+	logf("gw=%v; self=%v", gw, selfIP)
+
+	res, err := c.Probe(ctx)
+	if err != nil {
+		return fmt.Errorf("Probe: %v", err)
+	}
+	logf("Probe: %+v", res)
+
+	if !res.PCP && !res.PMP && !res.UPnP {
+		logf("no portmapping services available")
+		return nil
+	}
+
+	uc, err := net.ListenPacket("udp", "0.0.0.0:0")
+	if err != nil {
+		return err
+	}
+	defer uc.Close()
+	c.SetLocalPort(uint16(uc.LocalAddr().(*net.UDPAddr).Port))
+
+	if ext, ok := c.GetCachedMappingOrStartCreatingOne(); ok {
+		logf("mapping: %v", ext)
+	} else {
+		logf("no mapping")
+	}
+
+	select {
+	case <-done:
+		return nil
+	case <-ctx.Done():
+		return ctx.Err()
+	}
+}

cmd/tailscaled/tailscaled.go

@@ -210,6 +210,9 @@ func run() error {
 	logf = logger.RateLimitedFn(logf, 5*time.Second, 5, 100)
 
 	if args.cleanup {
+		if os.Getenv("TS_PLEASE_PANIC") != "" {
+			panic("TS_PLEASE_PANIC asked us to panic")
+		}
 		dns.Cleanup(logf, args.tunname)
 		router.Cleanup(logf, args.tunname)
 		return nil

derp/derp_server.go

@@ -167,7 +167,7 @@ type PacketForwarder interface {
 // Conn is the subset of the underlying net.Conn the DERP Server needs.
 // It is a defined type so that non-net connections can be used.
 type Conn interface {
-	io.Closer
+	io.WriteCloser
 
 	// The *Deadline methods follow the semantics of net.Conn.
 
@@ -470,8 +470,9 @@ func (s *Server) addWatcher(c *sclient) {
 }
 
 func (s *Server) accept(nc Conn, brw *bufio.ReadWriter, remoteAddr string, connNum int64) error {
-	br, bw := brw.Reader, brw.Writer
+	br := brw.Reader
 	nc.SetDeadline(time.Now().Add(10 * time.Second))
+	bw := &lazyBufioWriter{w: nc, lbw: brw.Writer}
 	if err := s.sendServerKey(bw); err != nil {
 		return fmt.Errorf("send server key: %v", err)
 	}
@@ -534,7 +535,7 @@ func (s *Server) accept(nc Conn, brw *bufio.ReadWriter, remoteAddr string, connN
 	}
 	defer s.unregisterClient(c)
 
-	err = s.sendServerInfo(bw, clientKey)
+	err = s.sendServerInfo(c.bw, clientKey)
 	if err != nil {
 		return fmt.Errorf("send server info: %v", err)
 	}
@@ -737,7 +738,7 @@ func (c *sclient) handleFrameSendPacket(ft frameType, fl uint32) error {
 // dropReason is why we dropped a DERP frame.
 type dropReason int
 
-//go:generate go run tailscale.com/cmd/addlicense -year 2021 -file dropreason_string.go stringer -type=dropReason -trimprefix=dropReason
+//go:generate go run tailscale.com/cmd/addlicense -year 2021 -file dropreason_string.go go run golang.org/x/tools/cmd/stringer -type=dropReason -trimprefix=dropReason
 
 const (
 	dropReasonUnknownDest      dropReason = iota // unknown destination pubkey
@@ -846,18 +847,20 @@ func (s *Server) verifyClient(clientKey key.Public, info *clientInfo) error {
 	return nil
 }
 
-func (s *Server) sendServerKey(bw *bufio.Writer) error {
+func (s *Server) sendServerKey(lw *lazyBufioWriter) error {
 	buf := make([]byte, 0, len(magic)+len(s.publicKey))
 	buf = append(buf, magic...)
 	buf = append(buf, s.publicKey[:]...)
-	return writeFrame(bw, frameServerKey, buf)
+	err := writeFrame(lw.bw(), frameServerKey, buf)
+	lw.Flush() // redundant (no-op) flush to release bufio.Writer
+	return err
 }
 
 type serverInfo struct {
 	Version int `json:"version,omitempty"`
 }
 
-func (s *Server) sendServerInfo(bw *bufio.Writer, clientKey key.Public) error {
+func (s *Server) sendServerInfo(bw *lazyBufioWriter, clientKey key.Public) error {
 	var nonce [24]byte
 	if _, err := crand.Read(nonce[:]); err != nil {
 		return err
@@ -868,7 +871,7 @@ func (s *Server) sendServerInfo(bw *bufio.Writer, clientKey key.Public) error {
 	}
 
 	msgbox := box.Seal(nil, msg, &nonce, clientKey.B32(), s.privateKey.B32())
-	if err := writeFrameHeader(bw, frameServerInfo, nonceLen+uint32(len(msgbox))); err != nil {
+	if err := writeFrameHeader(bw.bw(), frameServerInfo, nonceLen+uint32(len(msgbox))); err != nil {
 		return err
 	}
 	if _, err := bw.Write(nonce[:]); err != nil {
@@ -1002,7 +1005,7 @@ type sclient struct {
 	preferred   bool
 
 	// Owned by sender, not thread-safe.
-	bw *bufio.Writer
+	bw *lazyBufioWriter
 
 	// Guarded by s.mu
 	//
@@ -1164,14 +1167,14 @@ func (c *sclient) setWriteDeadline() {
 // sendKeepAlive sends a keep-alive frame, without flushing.
 func (c *sclient) sendKeepAlive() error {
 	c.setWriteDeadline()
-	return writeFrameHeader(c.bw, frameKeepAlive, 0)
+	return writeFrameHeader(c.bw.bw(), frameKeepAlive, 0)
 }
 
 // sendPeerGone sends a peerGone frame, without flushing.
 func (c *sclient) sendPeerGone(peer key.Public) error {
 	c.s.peerGoneFrames.Add(1)
 	c.setWriteDeadline()
-	if err := writeFrameHeader(c.bw, framePeerGone, keyLen); err != nil {
+	if err := writeFrameHeader(c.bw.bw(), framePeerGone, keyLen); err != nil {
 		return err
 	}
 	_, err := c.bw.Write(peer[:])
@@ -1181,7 +1184,7 @@ func (c *sclient) sendPeerGone(peer key.Public) error {
 // sendPeerPresent sends a peerPresent frame, without flushing.
 func (c *sclient) sendPeerPresent(peer key.Public) error {
 	c.setWriteDeadline()
-	if err := writeFrameHeader(c.bw, framePeerPresent, keyLen); err != nil {
+	if err := writeFrameHeader(c.bw.bw(), framePeerPresent, keyLen); err != nil {
 		return err
 	}
 	_, err := c.bw.Write(peer[:])
@@ -1254,11 +1257,11 @@ func (c *sclient) sendPacket(srcKey key.Public, contents []byte) (err error) {
 	if withKey {
 		pktLen += len(srcKey)
 	}
-	if err = writeFrameHeader(c.bw, frameRecvPacket, uint32(pktLen)); err != nil {
+	if err = writeFrameHeader(c.bw.bw(), frameRecvPacket, uint32(pktLen)); err != nil {
 		return err
 	}
 	if withKey {
-		err := writePublicKey(c.bw, &srcKey)
+		err := writePublicKey(c.bw.bw(), &srcKey)
 		if err != nil {
 			return err
 		}
@@ -1573,3 +1576,45 @@ func (s *Server) ServeDebugTraffic(w http.ResponseWriter, r *http.Request) {
 		time.Sleep(minTimeBetweenLogs)
 	}
 }
+
+var bufioWriterPool = &sync.Pool{
+	New: func() interface{} {
+		return bufio.NewWriterSize(ioutil.Discard, 2<<10)
+	},
+}
+
+// lazyBufioWriter is a bufio.Writer-like wrapping writer that lazily
+// allocates its actual bufio.Writer from a sync.Pool, releasing it to
+// the pool upon flush.
+//
+// We do this to reduce memory overhead; most DERP connections are
+// idle and the idle bufio.Writers were 30% of overall memory usage.
+type lazyBufioWriter struct {
+	w   io.Writer     // underlying
+	lbw *bufio.Writer // lazy; nil means it needs an associated buffer
+}
+
+func (w *lazyBufioWriter) bw() *bufio.Writer {
+	if w.lbw == nil {
+		w.lbw = bufioWriterPool.Get().(*bufio.Writer)
+		w.lbw.Reset(w.w)
+	}
+	return w.lbw
+}
+
+func (w *lazyBufioWriter) Available() int { return w.bw().Available() }
+
+func (w *lazyBufioWriter) Write(p []byte) (int, error) { return w.bw().Write(p) }
+
+func (w *lazyBufioWriter) Flush() error {
+	if w.lbw == nil {
+		return nil
+	}
+	err := w.lbw.Flush()
+
+	w.lbw.Reset(ioutil.Discard)
+	bufioWriterPool.Put(w.lbw)
+	w.lbw = nil
+
+	return err
+}

go.mod

@@ -31,7 +31,7 @@ require (
 	github.com/pkg/sftp v1.13.0
 	github.com/tailscale/certstore v0.0.0-20210528134328-066c94b793d3
 	github.com/tailscale/depaware v0.0.0-20201214215404-77d1e9757027
-	github.com/tailscale/goupnp v1.0.1-0.20210710010003-1cf2d718bbb2
+	github.com/tailscale/goupnp v1.0.1-0.20210804011211-c64d0f06ea05
 	github.com/tailscale/hujson v0.0.0-20200924210142-dde312d0d6a2
 	github.com/tcnksm/go-httpstat v0.2.0
 	github.com/toqueteos/webbrowser v1.2.0

go.sum

@@ -581,8 +581,8 @@ github.com/tailscale/certstore v0.0.0-20210528134328-066c94b793d3 h1:fEubocuQkrl
 github.com/tailscale/certstore v0.0.0-20210528134328-066c94b793d3/go.mod h1:2P+hpOwd53e7JMX/L4f3VXkv1G+33ES6IWZSrkIeWNs=
 github.com/tailscale/depaware v0.0.0-20201214215404-77d1e9757027 h1:lK99QQdH3yBWY6aGilF+IRlQIdmhzLrsEmF6JgN+Ryw=
 github.com/tailscale/depaware v0.0.0-20201214215404-77d1e9757027/go.mod h1:p9lPsd+cx33L3H9nNoecRRxPssFKUwwI50I3pZ0yT+8=
-github.com/tailscale/goupnp v1.0.1-0.20210710010003-1cf2d718bbb2 h1:AIJ8AF9O7jBmCwilP0ydwJMIzW5dw48Us8f3hLJhYBY=
-github.com/tailscale/goupnp v1.0.1-0.20210710010003-1cf2d718bbb2/go.mod h1:PdCqy9JzfWMJf1H5UJW2ip33/d4YkoKN0r67yKH1mG8=
+github.com/tailscale/goupnp v1.0.1-0.20210804011211-c64d0f06ea05 h1:4chzWmimtJPxRs2O36yuGRW3f9SYV+bMTTvMBI0EKio=
+github.com/tailscale/goupnp v1.0.1-0.20210804011211-c64d0f06ea05/go.mod h1:PdCqy9JzfWMJf1H5UJW2ip33/d4YkoKN0r67yKH1mG8=
 github.com/tailscale/hujson v0.0.0-20200924210142-dde312d0d6a2 h1:reREUgl2FG+o7YCsrZB8XLjnuKv5hEIWtnOdAbRAXZI=
 github.com/tailscale/hujson v0.0.0-20200924210142-dde312d0d6a2/go.mod h1:STqf+YV0ADdzk4ejtXFsGqDpATP9JoL0OB+hiFQbkdE=
 github.com/tcnksm/go-httpstat v0.2.0 h1:rP7T5e5U2HfmOBmZzGgGZjBQ5/GluWUylujl0tJ04I0=

logpolicy/logpolicy.go

@@ -187,6 +187,10 @@ func runningUnderSystemd() bool {
 	return false
 }
 
+func redirectStderrToLogPanics() bool {
+	return runningUnderSystemd() || os.Getenv("TS_PLEASE_PANIC") != ""
+}
+
 // tryFixLogStateLocation is a temporary fixup for
 // https://github.com/tailscale/tailscale/issues/247 . We accidentally
 // wrote logging state files to /, and then later to $CACHE_DIRECTORY
@@ -435,9 +439,14 @@ func New(collection string) *Policy {
 		c.HTTPC = &http.Client{Transport: newLogtailTransport(u.Host)}
 	}
 
-	filchBuf, filchErr := filch.New(filepath.Join(dir, cmdName), filch.Options{})
+	filchBuf, filchErr := filch.New(filepath.Join(dir, cmdName), filch.Options{
+		ReplaceStderr: redirectStderrToLogPanics(),
+	})
 	if filchBuf != nil {
 		c.Buffer = filchBuf
+		if filchBuf.OrigStderr != nil {
+			c.Stderr = filchBuf.OrigStderr
+		}
 	}
 	lw := logtail.NewLogger(c, log.Printf)
 	log.SetFlags(0) // other logflags are set on console, not here

logtail/logtail.go

@@ -118,9 +118,10 @@ type Logger struct {
 	bo             *backoff.Backoff
 	zstdEncoder    Encoder
 	uploadCancel   func()
+	explainedRaw   bool
 
 	shutdownStart chan struct{} // closed when shutdown begins
-	shutdownDone  chan struct{} // closd when shutdown complete
+	shutdownDone  chan struct{} // closed when shutdown complete
 }
 
 // SetVerbosityLevel controls the verbosity level that should be
@@ -230,6 +231,14 @@ func (l *Logger) drainPending() (res []byte) {
 			// outside of the logtail logger. Encode it.
 			// Do not add a client time, as it could have been
 			// been written a long time ago.
+			if !l.explainedRaw {
+				fmt.Fprintf(l.stderr, "RAW-STDERR: ***\n")
+				fmt.Fprintf(l.stderr, "RAW-STDERR: *** Lines prefixed with RAW-STDERR below bypassed logtail and probably come from a previous run of the program\n")
+				fmt.Fprintf(l.stderr, "RAW-STDERR: ***\n")
+				fmt.Fprintf(l.stderr, "RAW-STDERR:\n")
+				l.explainedRaw = true
+			}
+			fmt.Fprintf(l.stderr, "RAW-STDERR: %s", b)
 			b = l.encodeText(b, true)
 		}
 

net/dns/manager_windows.go

@@ -293,7 +293,7 @@ func (m windowsManager) SetDNS(cfg OSConfig) error {
 		}
 
 		t0 = time.Now()
-		m.logf("running ipconfig /registerdns ...")
+		m.logf("running ipconfig /flushdns ...")
 		cmd = exec.Command("ipconfig", "/flushdns")
 		cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true}
 		err = cmd.Run()

net/dns/resolver/doh_test.go

@@ -44,7 +44,9 @@ func TestDoH(t *testing.T) {
 		t.Fatal("no known DoH")
 	}
 
-	f := new(forwarder)
+	f := &forwarder{
+		dohSem: make(chan struct{}, 10),
+	}
 
 	for ip := range knownDoH {
 		t.Run(ip.String(), func(t *testing.T) {

net/dns/resolver/forwarder_test.go

@@ -0,0 +1,90 @@
+// Copyright (c) 2021 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 resolver
+
+import (
+	"fmt"
+	"reflect"
+	"strings"
+	"testing"
+	"time"
+
+	"inet.af/netaddr"
+)
+
+func (rr resolverAndDelay) String() string {
+	return fmt.Sprintf("%v+%v", rr.ipp, rr.startDelay)
+}
+
+func TestResolversWithDelays(t *testing.T) {
+	// query
+	q := func(ss ...string) (ipps []netaddr.IPPort) {
+		for _, s := range ss {
+			ipps = append(ipps, netaddr.MustParseIPPort(s))
+		}
+		return
+	}
+	// output
+	o := func(ss ...string) (rr []resolverAndDelay) {
+		for _, s := range ss {
+			var d time.Duration
+			if i := strings.Index(s, "+"); i != -1 {
+				var err error
+				d, err = time.ParseDuration(s[i+1:])
+				if err != nil {
+					panic(fmt.Sprintf("parsing duration in %q: %v", s, err))
+				}
+				s = s[:i]
+			}
+			rr = append(rr, resolverAndDelay{
+				ipp:        netaddr.MustParseIPPort(s),
+				startDelay: d,
+			})
+		}
+		return
+	}
+
+	tests := []struct {
+		name string
+		in   []netaddr.IPPort
+		want []resolverAndDelay
+	}{
+		{
+			name: "unknown-no-delays",
+			in:   q("1.2.3.4:53", "2.3.4.5:53"),
+			want: o("1.2.3.4:53", "2.3.4.5:53"),
+		},
+		{
+			name: "google-all-ipv4",
+			in:   q("8.8.8.8:53", "8.8.4.4:53"),
+			want: o("8.8.8.8:53", "8.8.4.4:53+200ms"),
+		},
+		{
+			name: "google-only-ipv6",
+			in:   q("[2001:4860:4860::8888]:53", "[2001:4860:4860::8844]:53"),
+			want: o("[2001:4860:4860::8888]:53", "[2001:4860:4860::8844]:53+200ms"),
+		},
+		{
+			name: "google-all-four",
+			in:   q("8.8.8.8:53", "8.8.4.4:53", "[2001:4860:4860::8888]:53", "[2001:4860:4860::8844]:53"),
+			want: o("8.8.8.8:53", "8.8.4.4:53+200ms", "[2001:4860:4860::8888]:53+2.5s", "[2001:4860:4860::8844]:53+2.7s"),
+		},
+		{
+			name: "quad9-one-v4-one-v6",
+			in:   q("9.9.9.9:53", "[2620:fe::fe]:53"),
+			want: o("9.9.9.9:53", "[2620:fe::fe]:53+200ms"),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := resolversWithDelays(tt.in)
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("got %v; want %v", got, tt.want)
+			}
+		})
+	}
+
+}

net/dnsfallback/dns-fallback-servers.json

@@ -89,20 +89,6 @@
 			"RegionCode": "r4",
 			"RegionName": "r4",
 			"Nodes": [
-				{
-					"Name": "4a",
-					"RegionID": 4,
-					"HostName": "derp4.tailscale.com",
-					"IPv4": "167.172.182.26",
-					"IPv6": "2a03:b0c0:3:e0::36e:9001"
-				},
-				{
-					"Name": "4b",
-					"RegionID": 4,
-					"HostName": "derp4b.tailscale.com",
-					"IPv4": "157.230.25.0",
-					"IPv6": "2a03:b0c0:3:e0::58f:3001"
-				},
 				{
 					"Name": "4c",
 					"RegionID": 4,
@@ -173,19 +159,26 @@
 			"RegionCode": "r8",
 			"RegionName": "r8",
 			"Nodes": [
-				{
-					"Name": "8a",
-					"RegionID": 8,
-					"HostName": "derp8.tailscale.com",
-					"IPv4": "167.71.139.179",
-					"IPv6": "2a03:b0c0:1:e0::3cc:e001"
-				},
 				{
 					"Name": "8b",
 					"RegionID": 8,
 					"HostName": "derp8b.tailscale.com",
 					"IPv4": "46.101.74.201",
 					"IPv6": "2a03:b0c0:1:d0::ec1:e001"
+				},
+				{
+					"Name": "8c",
+					"RegionID": 8,
+					"HostName": "derp8c.tailscale.com",
+					"IPv4": "206.189.16.32",
+					"IPv6": "2a03:b0c0:1:d0::e1f:4001"
+				},
+				{
+					"Name": "8d",
+					"RegionID": 8,
+					"HostName": "derp8d.tailscale.com",
+					"IPv4": "178.62.44.132",
+					"IPv6": "2a03:b0c0:1:d0::e08:e001"
 				}
 			]
 		},

net/interfaces/interfaces.go

@@ -134,7 +134,7 @@ func LocalAddresses() (regular, loopback []netaddr.IP, err error) {
 					// but their OS supports IPv6 so they have an fe80::
 					// address. We don't want to report all of those
 					// IPv6 LL to Control.
-				} else if ip.Is6() && tsaddr.IsULA(ip) {
+				} else if ip.Is6() && ip.IsPrivate() {
 					// Google Cloud Run uses NAT with IPv6 Unique
 					// Local Addresses to provide IPv6 connectivity.
 					ula6 = append(ula6, ip)
@@ -548,7 +548,7 @@ func isUsableV4(ip netaddr.IP) bool {
 // (fc00::/7) are in some environments used with address translation.
 func isUsableV6(ip netaddr.IP) bool {
 	return v6Global1.Contains(ip) ||
-		(tsaddr.IsULA(ip) && !tsaddr.TailscaleULARange().Contains(ip))
+		(ip.Is6() && ip.IsPrivate() && !tsaddr.TailscaleULARange().Contains(ip))
 }
 
 var (

net/interfaces/interfaces_test.go

@@ -58,6 +58,8 @@ func TestIsUsableV6(t *testing.T) {
 		{"zeros", "0000:0000:0000:0000:0000:0000:0000:0000", false},
 		{"Link Local", "fe80::1", false},
 		{"Global", "2602::1", true},
+		{"IPv4 public", "192.0.2.1", false},
+		{"IPv4 private", "192.168.1.1", false},
 	}
 
 	for _, test := range tests {

net/portmapper/disabled_stubs.go

@@ -15,10 +15,6 @@ import (
 
 type upnpClient interface{}
 
-func getUPnPClient(ctx context.Context, gw netaddr.IP) (upnpClient, error) {
-	return nil, nil
-}
-
 func (c *Client) getUPnPPortMapping(
 	ctx context.Context,
 	gw netaddr.IP,

net/portmapper/portmapper.go

@@ -14,9 +14,11 @@ import (
 	"fmt"
 	"io"
 	"net"
+	"net/http"
 	"sync"
 	"time"
 
+	"go4.org/mem"
 	"inet.af/netaddr"
 	"tailscale.com/net/interfaces"
 	"tailscale.com/net/netns"
@@ -62,8 +64,11 @@ type Client struct {
 	pmpPubIPTime time.Time  // time pmpPubIP last verified
 	pmpLastEpoch uint32
 
-	pcpSawTime  time.Time // time we last saw PCP was available
-	uPnPSawTime time.Time // time we last saw UPnP was available
+	pcpSawTime time.Time // time we last saw PCP was available
+
+	uPnPSawTime    time.Time         // time we last saw UPnP was available
+	uPnPMeta       uPnPDiscoResponse // Location header from UPnP UDP discovery response
+	uPnPHTTPClient *http.Client      // nil until needed
 
 	localPort uint16
 
@@ -560,27 +565,9 @@ func (c *Client) Probe(ctx context.Context) (res ProbeResult, err error) {
 	defer cancel()
 	defer closeCloserOnContextDone(ctx, uc)()
 
-	if c.sawUPnPRecently() {
-		res.UPnP = true
-	} else {
-		hasUPnP := make(chan bool, 1)
-		defer func() {
-			res.UPnP = <-hasUPnP
-		}()
-		go func() {
-			client, err := getUPnPClient(ctx, gw)
-			if err == nil && client != nil {
-				hasUPnP <- true
-				c.mu.Lock()
-				c.uPnPSawTime = time.Now()
-				c.mu.Unlock()
-			}
-			close(hasUPnP)
-		}()
-	}
-
 	pcpAddr := netaddr.IPPortFrom(gw, pcpPort).UDPAddr()
 	pmpAddr := netaddr.IPPortFrom(gw, pmpPort).UDPAddr()
+	upnpAddr := netaddr.IPPortFrom(gw, upnpPort).UDPAddr()
 
 	// Don't send probes to services that we recently learned (for
 	// the same gw/myIP) are available. See
@@ -595,11 +582,16 @@ func (c *Client) Probe(ctx context.Context) (res ProbeResult, err error) {
 	} else {
 		uc.WriteTo(pcpAnnounceRequest(myIP), pcpAddr)
 	}
+	if c.sawUPnPRecently() {
+		res.UPnP = true
+	} else {
+		uc.WriteTo(uPnPPacket, upnpAddr)
+	}
 
 	buf := make([]byte, 1500)
 	pcpHeard := false // true when we get any PCP response
 	for {
-		if pcpHeard && res.PMP {
+		if pcpHeard && res.PMP && res.UPnP {
 			// Nothing more to discover.
 			return res, nil
 		}
@@ -612,6 +604,19 @@ func (c *Client) Probe(ctx context.Context) (res ProbeResult, err error) {
 		}
 		port := addr.(*net.UDPAddr).Port
 		switch port {
+		case upnpPort:
+			if mem.Contains(mem.B(buf[:n]), mem.S(":InternetGatewayDevice:")) {
+				meta, err := parseUPnPDiscoResponse(buf[:n])
+				if err != nil {
+					c.logf("unrecognized UPnP discovery response; ignoring")
+				}
+				// log.Printf("UPnP reply %+v, %q", meta, buf[:n])
+				res.UPnP = true
+				c.mu.Lock()
+				c.uPnPSawTime = time.Now()
+				c.uPnPMeta = meta
+				c.mu.Unlock()
+			}
 		case pcpPort: // same as pmpPort
 			if pres, ok := parsePCPResponse(buf[:n]); ok {
 				if pres.OpCode == pcpOpReply|pcpOpAnnounce {
@@ -724,3 +729,13 @@ func parsePCPResponse(b []byte) (res pcpResponse, ok bool) {
 }
 
 var pmpReqExternalAddrPacket = []byte{0, 0} // version 0, opcode 0 = "Public address request"
+
+const (
+	upnpPort = 1900
+)
+
+var uPnPPacket = []byte("M-SEARCH * HTTP/1.1\r\n" +
+	"HOST: 239.255.255.250:1900\r\n" +
+	"ST: ssdp:all\r\n" +
+	"MAN: \"ssdp:discover\"\r\n" +
+	"MX: 2\r\n\r\n")

net/portmapper/upnp.go

@@ -8,17 +8,27 @@
 package portmapper
 
 import (
+	"bufio"
+	"bytes"
 	"context"
 	"fmt"
+	"log"
 	"math/rand"
+	"net/http"
 	"net/url"
 	"time"
 
+	"github.com/tailscale/goupnp"
 	"github.com/tailscale/goupnp/dcps/internetgateway2"
 	"inet.af/netaddr"
 	"tailscale.com/control/controlknobs"
+	"tailscale.com/net/netns"
 )
 
+// VerboseLogs controls verbose debug logging.
+// It exists for use by "tailscaled debug --portmap".
+var VerboseLogs bool
+
 // References:
 //
 // WANIP Connection v2: http://upnp.org/specs/gw/UPnP-gw-WANIPConnection-v2-Service.pdf
@@ -44,7 +54,8 @@ func (u *upnpMapping) Release(ctx context.Context) {
 }
 
 // upnpClient is an interface over the multiple different clients exported by goupnp,
-// exposing the functions we need for portmapping. They are auto-generated from XML-specs.
+// exposing the functions we need for portmapping. Those clients are auto-generated from XML-specs,
+// which is why they're not very idiomatic.
 type upnpClient interface {
 	AddPortMapping(
 		ctx context.Context,
@@ -77,7 +88,7 @@ type upnpClient interface {
 		// greater than 0. From the spec, it appears if it is set to 0, it will switch to using
 		// 604800 seconds, but not sure why this is desired. The recommended time is 3600 seconds.
 		leaseDurationSec uint32,
-	) (err error)
+	) error
 
 	DeletePortMapping(ctx context.Context, remoteHost string, externalPort uint16, protocol string) error
 	GetExternalIPAddress(ctx context.Context) (externalIPAddress string, err error)
@@ -92,6 +103,8 @@ const tsPortMappingDesc = "tailscale-portmap"
 // behavior of calling AddPortMapping with port = 0 to specify a wildcard port.
 // It returns the new external port (which may not be identical to the external port specified),
 // or an error.
+//
+// TODO(bradfitz): also returned the actual lease duration obtained. and check it regularly.
 func addAnyPortMapping(
 	ctx context.Context,
 	upnp upnpClient,
@@ -130,51 +143,76 @@ func addAnyPortMapping(
 	return externalPort, err
 }
 
-// getUPnPClients gets a client for interfacing with UPnP, ignoring the underlying protocol for
+// getUPnPClient gets a client for interfacing with UPnP, ignoring the underlying protocol for
 // now.
 // Adapted from https://github.com/huin/goupnp/blob/master/GUIDE.md.
-func getUPnPClient(ctx context.Context, gw netaddr.IP) (upnpClient, error) {
+//
+// The gw is the detected gateway.
+//
+// The meta is the most recently parsed UDP discovery packet response
+// from the Internet Gateway Device.
+//
+// The provided ctx is not retained in the returned upnpClient, but
+// its associated HTTP client is (if set via goupnp.WithHTTPClient).
+func getUPnPClient(ctx context.Context, gw netaddr.IP, meta uPnPDiscoResponse) (upnpClient, error) {
 	if controlknobs.DisableUPnP() {
 		return nil, nil
 	}
-	ctx, cancel := context.WithTimeout(ctx, 250*time.Millisecond)
-	defer cancel()
-	// Attempt to connect over the multiple available connection types concurrently,
-	// returning the fastest.
 
-	// TODO(jknodt): this url seems super brittle? maybe discovery is better but this is faster
-	u, err := url.Parse(fmt.Sprintf("http://%s:5000/rootDesc.xml", gw))
+	if meta.Location == "" {
+		return nil, nil
+	}
+
+	if VerboseLogs {
+		log.Printf("fetching %v", meta.Location)
+	}
+	u, err := url.Parse(meta.Location)
 	if err != nil {
 		return nil, err
 	}
 
-	clients := make(chan upnpClient, 3)
-	go func() {
-		var err error
-		ip1Clients, err := internetgateway2.NewWANIPConnection1ClientsByURL(ctx, u)
-		if err == nil && len(ip1Clients) > 0 {
-			clients <- ip1Clients[0]
-		}
-	}()
-	go func() {
-		ip2Clients, err := internetgateway2.NewWANIPConnection2ClientsByURL(ctx, u)
-		if err == nil && len(ip2Clients) > 0 {
-			clients <- ip2Clients[0]
-		}
-	}()
-	go func() {
-		ppp1Clients, err := internetgateway2.NewWANPPPConnection1ClientsByURL(ctx, u)
-		if err == nil && len(ppp1Clients) > 0 {
-			clients <- ppp1Clients[0]
-		}
-	}()
-
-	select {
-	case client := <-clients:
-		return client, nil
-	case <-ctx.Done():
-		return nil, ctx.Err()
+	ipp, err := netaddr.ParseIPPort(u.Host)
+	if err != nil {
+		return nil, fmt.Errorf("unexpected host %q in %q", u.Host, meta.Location)
 	}
+	if ipp.IP() != gw {
+		return nil, fmt.Errorf("UPnP discovered root %q does not match gateway IP %v; ignoring UPnP",
+			meta.Location, gw)
+	}
+
+	ctx, cancel := context.WithTimeout(ctx, 500*time.Millisecond)
+	defer cancel()
+
+	// This part does a network fetch.
+	root, err := goupnp.DeviceByURL(ctx, u)
+	if err != nil {
+		return nil, err
+	}
+
+	// These parts don't do a network fetch.
+	// Pick the best service type available.
+	if cc, _ := internetgateway2.NewWANIPConnection2ClientsFromRootDevice(ctx, root, u); len(cc) > 0 {
+		return cc[0], nil
+	}
+	if cc, _ := internetgateway2.NewWANIPConnection1ClientsFromRootDevice(ctx, root, u); len(cc) > 0 {
+		return cc[0], nil
+	}
+	if cc, _ := internetgateway2.NewWANPPPConnection1ClientsFromRootDevice(ctx, root, u); len(cc) > 0 {
+		return cc[0], nil
+	}
+	return nil, nil
+}
+
+func (c *Client) upnpHTTPClientLocked() *http.Client {
+	if c.uPnPHTTPClient == nil {
+		c.uPnPHTTPClient = &http.Client{
+			Transport: &http.Transport{
+				DialContext:     netns.NewDialer().DialContext,
+				IdleConnTimeout: 2 * time.Second, // LAN is cheap
+			},
+		}
+	}
+	return c.uPnPHTTPClient
 }
 
 // getUPnPPortMapping attempts to create a port-mapping over the UPnP protocol. On success,
@@ -199,11 +237,17 @@ func (c *Client) getUPnPPortMapping(
 	var err error
 	c.mu.Lock()
 	oldMapping, ok := c.mapping.(*upnpMapping)
+	meta := c.uPnPMeta
+	httpClient := c.upnpHTTPClientLocked()
 	c.mu.Unlock()
 	if ok && oldMapping != nil {
 		client = oldMapping.client
 	} else {
-		client, err = getUPnPClient(ctx, gw)
+		ctx := goupnp.WithHTTPClient(ctx, httpClient)
+		client, err = getUPnPClient(ctx, gw, meta)
+		if VerboseLogs {
+			log.Printf("getUPnPClient: %T, %v", client, err)
+		}
 		if err != nil {
 			return netaddr.IPPort{}, false
 		}
@@ -221,11 +265,17 @@ func (c *Client) getUPnPPortMapping(
 		internal.IP().String(),
 		time.Second*pmpMapLifetimeSec,
 	)
+	if VerboseLogs {
+		log.Printf("addAnyPortMapping: %v, %v", newPort, err)
+	}
 	if err != nil {
 		return netaddr.IPPort{}, false
 	}
 	// TODO cache this ip somewhere?
 	extIP, err := client.GetExternalIPAddress(ctx)
+	if VerboseLogs {
+		log.Printf("client.GetExternalIPAddress: %v, %v", extIP, err)
+	}
 	if err != nil {
 		// TODO this doesn't seem right
 		return netaddr.IPPort{}, false
@@ -246,3 +296,18 @@ func (c *Client) getUPnPPortMapping(
 	c.localPort = newPort
 	return upnp.external, true
 }
+
+type uPnPDiscoResponse struct {
+	Location string
+}
+
+// parseUPnPDiscoResponse parses a UPnP HTTP-over-UDP discovery response.
+func parseUPnPDiscoResponse(body []byte) (uPnPDiscoResponse, error) {
+	var r uPnPDiscoResponse
+	res, err := http.ReadResponse(bufio.NewReaderSize(bytes.NewReader(body), 128), nil)
+	if err != nil {
+		return r, err
+	}
+	r.Location = res.Header.Get("Location")
+	return r, nil
+}

net/portmapper/upnp_test.go

@@ -0,0 +1,95 @@
+// Copyright (c) 2021 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 portmapper
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"reflect"
+	"testing"
+
+	"inet.af/netaddr"
+)
+
+// Google Wifi
+const (
+	googleWifiUPnPDisco = "HTTP/1.1 200 OK\r\nCACHE-CONTROL: max-age=120\r\nST: urn:schemas-upnp-org:device:InternetGatewayDevice:2\r\nUSN: uuid:a9708184-a6c0-413a-bbac-11bcf7e30ece::urn:schemas-upnp-org:device:InternetGatewayDevice:2\r\nEXT:\r\nSERVER: Linux/5.4.0-1034-gcp UPnP/1.1 MiniUPnPd/1.9\r\nLOCATION: http://192.168.86.1:5000/rootDesc.xml\r\nOPT: \"http://schemas.upnp.org/upnp/1/0/\"; ns=01\r\n01-NLS: 1\r\nBOOTID.UPNP.ORG: 1\r\nCONFIGID.UPNP.ORG: 1337\r\n\r\n"
+
+	googleWifiRootDescXML = `<?xml version="1.0"?>
+<root xmlns="urn:schemas-upnp-org:device-1-0"><specVersion><major>1</major><minor>0</minor></specVersion><device><deviceType>urn:schemas-upnp-org:device:InternetGatewayDevice:2</deviceType><friendlyName>OnHub</friendlyName><manufacturer>Google</manufacturer><manufacturerURL>http://google.com/</manufacturerURL><modelDescription>Wireless Router</modelDescription><modelName>OnHub</modelName><modelNumber>1</modelNumber><modelURL>https://on.google.com/hub/</modelURL><serialNumber>00000000</serialNumber><UDN>uuid:a9708184-a6c0-413a-bbac-11bcf7e30ece</UDN><serviceList><service><serviceType>urn:schemas-upnp-org:service:Layer3Forwarding:1</serviceType><serviceId>urn:upnp-org:serviceId:Layer3Forwarding1</serviceId><controlURL>/ctl/L3F</controlURL><eventSubURL>/evt/L3F</eventSubURL><SCPDURL>/L3F.xml</SCPDURL></service><service><serviceType>urn:schemas-upnp-org:service:DeviceProtection:1</serviceType><serviceId>urn:upnp-org:serviceId:DeviceProtection1</serviceId><controlURL>/ctl/DP</controlURL><eventSubURL>/evt/DP</eventSubURL><SCPDURL>/DP.xml</SCPDURL></service></serviceList><deviceList><device><deviceType>urn:schemas-upnp-org:device:WANDevice:2</deviceType><friendlyName>WANDevice</friendlyName><manufacturer>MiniUPnP</manufacturer><manufacturerURL>http://miniupnp.free.fr/</manufacturerURL><modelDescription>WAN Device</modelDescription><modelName>WAN Device</modelName><modelNumber>20210414</modelNumber><modelURL>http://miniupnp.free.fr/</modelURL><serialNumber>00000000</serialNumber><UDN>uuid:a9708184-a6c0-413a-bbac-11bcf7e30ecf</UDN><UPC>000000000000</UPC><serviceList><service><serviceType>urn:schemas-upnp-org:service:WANCommonInterfaceConfig:1</serviceType><serviceId>urn:upnp-org:serviceId:WANCommonIFC1</serviceId><controlURL>/ctl/CmnIfCfg</controlURL><eventSubURL>/evt/CmnIfCfg</eventSubURL><SCPDURL>/WANCfg.xml</SCPDURL></service></serviceList><deviceList><device><deviceType>urn:schemas-upnp-org:device:WANConnectionDevice:2</deviceType><friendlyName>WANConnectionDevice</friendlyName><manufacturer>MiniUPnP</manufacturer><manufacturerURL>http://miniupnp.free.fr/</manufacturerURL><modelDescription>MiniUPnP daemon</modelDescription><modelName>MiniUPnPd</modelName><modelNumber>20210414</modelNumber><modelURL>http://miniupnp.free.fr/</modelURL><serialNumber>00000000</serialNumber><UDN>uuid:a9708184-a6c0-413a-bbac-11bcf7e30ec0</UDN><UPC>000000000000</UPC><serviceList><service><serviceType>urn:schemas-upnp-org:service:WANIPConnection:2</serviceType><serviceId>urn:upnp-org:serviceId:WANIPConn1</serviceId><controlURL>/ctl/IPConn</controlURL><eventSubURL>/evt/IPConn</eventSubURL><SCPDURL>/WANIPCn.xml</SCPDURL></service></serviceList></device></deviceList></device></deviceList><presentationURL>http://testwifi.here/</presentationURL></device></root>`
+)
+
+// pfSense 2.5.0-RELEASE / FreeBSD 12.2-STABLE
+const (
+	pfSenseUPnPDisco = "HTTP/1.1 200 OK\r\nCACHE-CONTROL: max-age=120\r\nST: urn:schemas-upnp-org:device:InternetGatewayDevice:1\r\nUSN: uuid:bee7052b-49e8-3597-b545-55a1e38ac11::urn:schemas-upnp-org:device:InternetGatewayDevice:1\r\nEXT:\r\nSERVER: FreeBSD/12.2-STABLE UPnP/1.1 MiniUPnPd/2.2.1\r\nLOCATION: http://192.168.1.1:2189/rootDesc.xml\r\nOPT: \"http://schemas.upnp.org/upnp/1/0/\"; ns=01\r\n01-NLS: 1627958564\r\nBOOTID.UPNP.ORG: 1627958564\r\nCONFIGID.UPNP.ORG: 1337\r\n\r\n"
+
+	pfSenseRootDescXML = `<?xml version="1.0"?>
+<root xmlns="urn:schemas-upnp-org:device-1-0" configId="1337"><specVersion><major>1</major><minor>1</minor></specVersion><device><deviceType>urn:schemas-upnp-org:device:InternetGatewayDevice:1</deviceType><friendlyName>FreeBSD router</friendlyName><manufacturer>FreeBSD</manufacturer><manufacturerURL>http://www.freebsd.org/</manufacturerURL><modelDescription>FreeBSD router</modelDescription><modelName>FreeBSD router</modelName><modelNumber>2.5.0-RELEASE</modelNumber><modelURL>http://www.freebsd.org/</modelURL><serialNumber>BEE7052B</serialNumber><UDN>uuid:bee7052b-49e8-3597-b545-55a1e38ac11</UDN><serviceList><service><serviceType>urn:schemas-upnp-org:service:Layer3Forwarding:1</serviceType><serviceId>urn:upnp-org:serviceId:L3Forwarding1</serviceId><SCPDURL>/L3F.xml</SCPDURL><controlURL>/ctl/L3F</controlURL><eventSubURL>/evt/L3F</eventSubURL></service></serviceList><deviceList><device><deviceType>urn:schemas-upnp-org:device:WANDevice:1</deviceType><friendlyName>WANDevice</friendlyName><manufacturer>MiniUPnP</manufacturer><manufacturerURL>http://miniupnp.free.fr/</manufacturerURL><modelDescription>WAN Device</modelDescription><modelName>WAN Device</modelName><modelNumber>20210205</modelNumber><modelURL>http://miniupnp.free.fr/</modelURL><serialNumber>BEE7052B</serialNumber><UDN>uuid:bee7052b-49e8-3597-b545-55a1e38ac12</UDN><UPC>000000000000</UPC><serviceList><service><serviceType>urn:schemas-upnp-org:service:WANCommonInterfaceConfig:1</serviceType><serviceId>urn:upnp-org:serviceId:WANCommonIFC1</serviceId><SCPDURL>/WANCfg.xml</SCPDURL><controlURL>/ctl/CmnIfCfg</controlURL><eventSubURL>/evt/CmnIfCfg</eventSubURL></service></serviceList><deviceList><device><deviceType>urn:schemas-upnp-org:device:WANConnectionDevice:1</deviceType><friendlyName>WANConnectionDevice</friendlyName><manufacturer>MiniUPnP</manufacturer><manufacturerURL>http://miniupnp.free.fr/</manufacturerURL><modelDescription>MiniUPnP daemon</modelDescription><modelName>MiniUPnPd</modelName><modelNumber>20210205</modelNumber><modelURL>http://miniupnp.free.fr/</modelURL><serialNumber>BEE7052B</serialNumber><UDN>uuid:bee7052b-49e8-3597-b545-55a1e38ac13</UDN><UPC>000000000000</UPC><serviceList><service><serviceType>urn:schemas-upnp-org:service:WANIPConnection:1</serviceType><serviceId>urn:upnp-org:serviceId:WANIPConn1</serviceId><SCPDURL>/WANIPCn.xml</SCPDURL><controlURL>/ctl/IPConn</controlURL><eventSubURL>/evt/IPConn</eventSubURL></service></serviceList></device></deviceList></device></deviceList><presentationURL>https://192.168.1.1/</presentationURL></device></root>`
+)
+
+func TestParseUPnPDiscoResponse(t *testing.T) {
+	tests := []struct {
+		name    string
+		headers string
+		want    uPnPDiscoResponse
+	}{
+		{"google", googleWifiUPnPDisco, uPnPDiscoResponse{
+			Location: "http://192.168.86.1:5000/rootDesc.xml",
+		}},
+		{"pfsense", pfSenseUPnPDisco, uPnPDiscoResponse{
+			Location: "http://192.168.1.1:2189/rootDesc.xml",
+		}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := parseUPnPDiscoResponse([]byte(tt.headers))
+			if err != nil {
+				t.Fatal(err)
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("unexpected result:\n got: %+v\nwant: %+v\n", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestGetUPnPClient(t *testing.T) {
+	tests := []struct {
+		name    string
+		xmlBody string
+		want    string
+	}{
+		{"google", googleWifiRootDescXML, "*internetgateway2.WANIPConnection2"},
+		{"pfsense", pfSenseRootDescXML, "*internetgateway2.WANIPConnection1"},
+		// TODO(bradfitz): find a PPP one in the wild
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				if r.RequestURI == "/rootDesc.xml" {
+					io.WriteString(w, tt.xmlBody)
+					return
+				}
+				http.NotFound(w, r)
+			}))
+			defer ts.Close()
+			gw, _ := netaddr.FromStdIP(ts.Listener.Addr().(*net.TCPAddr).IP)
+			c, err := getUPnPClient(context.Background(), gw, uPnPDiscoResponse{
+				Location: ts.URL + "/rootDesc.xml",
+			})
+			if err != nil {
+				t.Fatal(err)
+			}
+			got := fmt.Sprintf("%T", c)
+			if got != tt.want {
+				t.Errorf("got %v; want %v", got, tt.want)
+			}
+		})
+	}
+}

net/tsaddr/tsaddr.go

@@ -105,11 +105,6 @@ func Tailscale4To6(ipv4 netaddr.IP) netaddr.IP {
 	return netaddr.IPFrom16(ret)
 }
 
-func IsULA(ip netaddr.IP) bool {
-	ulaRange.Do(func() { mustPrefix(&ulaRange.v, "fc00::/7") })
-	return ulaRange.v.Contains(ip)
-}
-
 func mustPrefix(v *netaddr.IPPrefix, prefix string) {
 	var err error
 	*v, err = netaddr.ParseIPPrefix(prefix)

net/tsaddr/tsaddr_test.go

@@ -43,28 +43,6 @@ func TestCGNATRange(t *testing.T) {
 	}
 }
 
-func TestIsUla(t *testing.T) {
-	tests := []struct {
-		name string
-		ip   string
-		want bool
-	}{
-		{"first ULA", "fc00::1", true},
-		{"not ULA", "fb00::1", false},
-		{"Tailscale", "fd7a:115c:a1e0::1", true},
-		{"Cloud Run", "fddf:3978:feb1:d745::1", true},
-		{"zeros", "0000:0000:0000:0000:0000:0000:0000:0000", false},
-		{"Link Local", "fe80::1", false},
-		{"Global", "2602::1", false},
-	}
-
-	for _, test := range tests {
-		if got := IsULA(netaddr.MustParseIP(test.ip)); got != test.want {
-			t.Errorf("IsULA(%s) = %v, want %v", test.name, got, test.want)
-		}
-	}
-}
-
 func TestNewContainsIPFunc(t *testing.T) {
 	f := NewContainsIPFunc([]netaddr.IPPrefix{netaddr.MustParseIPPrefix("10.0.0.0/8")})
 	if f(netaddr.MustParseIP("8.8.8.8")) {

tstest/integration/integration_test.go

@@ -84,6 +84,40 @@ func TestOneNodeUp_NoAuth(t *testing.T) {
 	t.Logf("number of HTTP logcatcher requests: %v", env.LogCatcher.numRequests())
 }
 
+func TestCollectPanic(t *testing.T) {
+	t.Parallel()
+	bins := BuildTestBinaries(t)
+
+	env := newTestEnv(t, bins)
+	defer env.Close()
+
+	n := newTestNode(t, env)
+
+	cmd := exec.Command(n.env.Binaries.Daemon, "--cleanup")
+	cmd.Env = append(os.Environ(),
+		"TS_PLEASE_PANIC=1",
+		"TS_LOG_TARGET="+n.env.LogCatcherServer.URL,
+	)
+	got, _ := cmd.CombinedOutput() // we expect it to fail, ignore err
+	t.Logf("initial run: %s", got)
+
+	// Now we run it again, and on start, it will upload the logs to logcatcher.
+	cmd = exec.Command(n.env.Binaries.Daemon, "--cleanup")
+	cmd.Env = append(os.Environ(), "TS_LOG_TARGET="+n.env.LogCatcherServer.URL)
+	if out, err := cmd.CombinedOutput(); err != nil {
+		t.Fatalf("cleanup failed: %v: %q", err, out)
+	}
+	if err := tstest.WaitFor(20*time.Second, func() error {
+		const sub = `panic`
+		if !n.env.LogCatcher.logsContains(mem.S(sub)) {
+			return fmt.Errorf("log catcher didn't see %#q; got %s", sub, n.env.LogCatcher.logsString())
+		}
+		return nil
+	}); err != nil {
+		t.Fatal(err)
+	}
+}
+
 // test Issue 2321: Start with UpdatePrefs should save prefs to disk
 func TestStateSavedOnStart(t *testing.T) {
 	t.Parallel()

tstest/integration/tailscaled_deps_test_darwin.go

@@ -41,6 +41,7 @@ import (
 	_ "tailscale.com/logpolicy"
 	_ "tailscale.com/net/dns"
 	_ "tailscale.com/net/interfaces"
+	_ "tailscale.com/net/portmapper"
 	_ "tailscale.com/net/socks5/tssocks"
 	_ "tailscale.com/net/tshttpproxy"
 	_ "tailscale.com/net/tstun"

tstest/integration/tailscaled_deps_test_freebsd.go

@@ -39,6 +39,7 @@ import (
 	_ "tailscale.com/logpolicy"
 	_ "tailscale.com/net/dns"
 	_ "tailscale.com/net/interfaces"
+	_ "tailscale.com/net/portmapper"
 	_ "tailscale.com/net/socks5/tssocks"
 	_ "tailscale.com/net/tshttpproxy"
 	_ "tailscale.com/net/tstun"

tstest/integration/tailscaled_deps_test_linux.go

@@ -39,6 +39,7 @@ import (
 	_ "tailscale.com/logpolicy"
 	_ "tailscale.com/net/dns"
 	_ "tailscale.com/net/interfaces"
+	_ "tailscale.com/net/portmapper"
 	_ "tailscale.com/net/socks5/tssocks"
 	_ "tailscale.com/net/tshttpproxy"
 	_ "tailscale.com/net/tstun"

tstest/integration/tailscaled_deps_test_openbsd.go

@@ -39,6 +39,7 @@ import (
 	_ "tailscale.com/logpolicy"
 	_ "tailscale.com/net/dns"
 	_ "tailscale.com/net/interfaces"
+	_ "tailscale.com/net/portmapper"
 	_ "tailscale.com/net/socks5/tssocks"
 	_ "tailscale.com/net/tshttpproxy"
 	_ "tailscale.com/net/tstun"

tstest/integration/tailscaled_deps_test_windows.go

@@ -45,6 +45,7 @@ import (
 	_ "tailscale.com/logtail/backoff"
 	_ "tailscale.com/net/dns"
 	_ "tailscale.com/net/interfaces"
+	_ "tailscale.com/net/portmapper"
 	_ "tailscale.com/net/socks5/tssocks"
 	_ "tailscale.com/net/tshttpproxy"
 	_ "tailscale.com/net/tstun"

util/deephash/deephash.go

@@ -21,7 +21,6 @@ import (
 	"hash"
 	"math"
 	"reflect"
-	"strconv"
 	"sync"
 	"time"
 	"unsafe"
@@ -38,21 +37,15 @@ type hasher struct {
 	visitStack visitStack
 }
 
-// newHasher initializes a new hasher, for use by hasherPool.
-func newHasher() *hasher {
-	h := &hasher{h: sha256.New()}
-	h.bw = bufio.NewWriterSize(h.h, h.h.BlockSize())
-	return h
-}
-
-// setBufioWriter switches the bufio writer to w after flushing
-// any output to the old one. It then also returns the old one, so
-// the caller can switch back to it.
-func (h *hasher) setBufioWriter(w *bufio.Writer) (old *bufio.Writer) {
-	old = h.bw
-	old.Flush()
-	h.bw = w
-	return old
+func (h *hasher) reset() {
+	if h.h == nil {
+		h.h = sha256.New()
+	}
+	if h.bw == nil {
+		h.bw = bufio.NewWriterSize(h.h, h.h.BlockSize())
+	}
+	h.bw.Flush()
+	h.h.Reset()
 }
 
 // Sum is an opaque checksum type that is comparable.
@@ -60,6 +53,12 @@ type Sum struct {
 	sum [sha256.Size]byte
 }
 
+func (s1 *Sum) xor(s2 Sum) {
+	for i := 0; i < sha256.Size; i++ {
+		s1.sum[i] ^= s2.sum[i]
+	}
+}
+
 func (s Sum) String() string {
 	return hex.EncodeToString(s.sum[:])
 }
@@ -69,34 +68,31 @@ var (
 	seed uint64
 )
 
-// Hash returns the hash of v.
-func (h *hasher) Hash(v interface{}) (hash Sum) {
-	h.bw.Flush()
-	h.h.Reset()
-	once.Do(func() {
-		seed = uint64(time.Now().UnixNano())
-	})
-	h.uint(seed)
-	h.print(reflect.ValueOf(v))
+func (h *hasher) sum() (s Sum) {
 	h.bw.Flush()
 	// Sum into scratch & copy out, as hash.Hash is an interface
 	// so the slice necessarily escapes, and there's no sha256
 	// concrete type exported and we don't want the 'hash' result
 	// parameter to escape to the heap:
-	h.h.Sum(h.scratch[:0])
-	copy(hash.sum[:], h.scratch[:])
-	return
+	copy(s.sum[:], h.h.Sum(h.scratch[:0]))
+	return s
 }
 
 var hasherPool = &sync.Pool{
-	New: func() interface{} { return newHasher() },
+	New: func() interface{} { return new(hasher) },
 }
 
 // Hash returns the hash of v.
-func Hash(v interface{}) Sum {
+func Hash(v interface{}) (s Sum) {
 	h := hasherPool.Get().(*hasher)
 	defer hasherPool.Put(h)
-	return h.Hash(v)
+	h.reset()
+	once.Do(func() {
+		seed = uint64(time.Now().UnixNano())
+	})
+	h.hashUint64(seed)
+	h.hashValue(reflect.ValueOf(v))
+	return h.sum()
 }
 
 // Update sets last to the hash of v and reports whether its value changed.
@@ -116,19 +112,25 @@ type appenderTo interface {
 	AppendTo([]byte) []byte
 }
 
-func (h *hasher) uint(i uint64) {
-	binary.BigEndian.PutUint64(h.scratch[:8], i)
-	h.bw.Write(h.scratch[:8])
+func (h *hasher) hashUint8(i uint8) {
+	h.bw.WriteByte(i)
 }
-
-func (h *hasher) int(i int) {
-	binary.BigEndian.PutUint64(h.scratch[:8], uint64(i))
+func (h *hasher) hashUint16(i uint16) {
+	binary.LittleEndian.PutUint16(h.scratch[:2], i)
+	h.bw.Write(h.scratch[:2])
+}
+func (h *hasher) hashUint32(i uint32) {
+	binary.LittleEndian.PutUint32(h.scratch[:4], i)
+	h.bw.Write(h.scratch[:4])
+}
+func (h *hasher) hashUint64(i uint64) {
+	binary.LittleEndian.PutUint64(h.scratch[:8], i)
 	h.bw.Write(h.scratch[:8])
 }
 
 var uint8Type = reflect.TypeOf(byte(0))
 
-func (h *hasher) print(v reflect.Value) {
+func (h *hasher) hashValue(v reflect.Value) {
 	if !v.IsValid() {
 		return
 	}
@@ -155,33 +157,33 @@ func (h *hasher) print(v reflect.Value) {
 		panic(fmt.Sprintf("unhandled kind %v for type %v", v.Kind(), v.Type()))
 	case reflect.Ptr:
 		if v.IsNil() {
-			w.WriteByte(0) // indicates nil
+			h.hashUint8(0) // indicates nil
 			return
 		}
 
 		// Check for cycle.
 		ptr := pointerOf(v)
 		if idx, ok := h.visitStack.seen(ptr); ok {
-			w.WriteByte(2) // indicates cycle
-			h.uint(uint64(idx))
+			h.hashUint8(2) // indicates cycle
+			h.hashUint64(uint64(idx))
 			return
 		}
 		h.visitStack.push(ptr)
 		defer h.visitStack.pop(ptr)
 
-		w.WriteByte(1) // indicates visiting a pointer
-		h.print(v.Elem())
+		h.hashUint8(1) // indicates visiting a pointer
+		h.hashValue(v.Elem())
 	case reflect.Struct:
 		w.WriteString("struct")
-		h.int(v.NumField())
+		h.hashUint64(uint64(v.NumField()))
 		for i, n := 0, v.NumField(); i < n; i++ {
-			h.int(i)
-			h.print(v.Field(i))
+			h.hashUint64(uint64(i))
+			h.hashValue(v.Field(i))
 		}
 	case reflect.Slice, reflect.Array:
 		vLen := v.Len()
 		if v.Kind() == reflect.Slice {
-			h.int(vLen)
+			h.hashUint64(uint64(vLen))
 		}
 		if v.Type().Elem() == uint8Type && v.CanInterface() {
 			if vLen > 0 && vLen <= scratchSize {
@@ -200,96 +202,91 @@ func (h *hasher) print(v reflect.Value) {
 			// TODO(dsnet): Perform cycle detection for slices,
 			// which is functionally a list of pointers.
 			// See https://github.com/google/go-cmp/blob/402949e8139bb890c71a707b6faf6dd05c92f4e5/cmp/compare.go#L438-L450
-			h.int(i)
-			h.print(v.Index(i))
+			h.hashUint64(uint64(i))
+			h.hashValue(v.Index(i))
 		}
 	case reflect.Interface:
 		if v.IsNil() {
-			w.WriteByte(0) // indicates nil
+			h.hashUint8(0) // indicates nil
 			return
 		}
 		v = v.Elem()
 
-		w.WriteByte(1) // indicates visiting interface value
+		h.hashUint8(1) // indicates visiting interface value
 		h.hashType(v.Type())
-		h.print(v)
+		h.hashValue(v)
 	case reflect.Map:
 		// Check for cycle.
 		ptr := pointerOf(v)
 		if idx, ok := h.visitStack.seen(ptr); ok {
-			w.WriteByte(2) // indicates cycle
-			h.uint(uint64(idx))
+			h.hashUint8(2) // indicates cycle
+			h.hashUint64(uint64(idx))
 			return
 		}
 		h.visitStack.push(ptr)
 		defer h.visitStack.pop(ptr)
 
-		w.WriteByte(1) // indicates visiting a map
+		h.hashUint8(1) // indicates visiting a map
 		h.hashMap(v)
 	case reflect.String:
-		h.int(v.Len())
-		w.WriteString(v.String())
+		s := v.String()
+		h.hashUint64(uint64(len(s)))
+		w.WriteString(s)
 	case reflect.Bool:
-		w.Write(strconv.AppendBool(h.scratch[:0], v.Bool()))
-	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
-		w.Write(strconv.AppendInt(h.scratch[:0], v.Int(), 10))
-	case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
-		h.uint(v.Uint())
-	case reflect.Float32, reflect.Float64:
-		w.Write(strconv.AppendUint(h.scratch[:0], math.Float64bits(v.Float()), 10))
-	case reflect.Complex64, reflect.Complex128:
-		fmt.Fprintf(w, "%v", v.Complex())
+		if v.Bool() {
+			h.hashUint8(1)
+		} else {
+			h.hashUint8(0)
+		}
+	case reflect.Int8:
+		h.hashUint8(uint8(v.Int()))
+	case reflect.Int16:
+		h.hashUint16(uint16(v.Int()))
+	case reflect.Int32:
+		h.hashUint32(uint32(v.Int()))
+	case reflect.Int64, reflect.Int:
+		h.hashUint64(uint64(v.Int()))
+	case reflect.Uint8:
+		h.hashUint8(uint8(v.Uint()))
+	case reflect.Uint16:
+		h.hashUint16(uint16(v.Uint()))
+	case reflect.Uint32:
+		h.hashUint32(uint32(v.Uint()))
+	case reflect.Uint64, reflect.Uint, reflect.Uintptr:
+		h.hashUint64(uint64(v.Uint()))
+	case reflect.Float32:
+		h.hashUint32(math.Float32bits(float32(v.Float())))
+	case reflect.Float64:
+		h.hashUint64(math.Float64bits(float64(v.Float())))
+	case reflect.Complex64:
+		h.hashUint32(math.Float32bits(real(complex64(v.Complex()))))
+		h.hashUint32(math.Float32bits(imag(complex64(v.Complex()))))
+	case reflect.Complex128:
+		h.hashUint64(math.Float64bits(real(complex128(v.Complex()))))
+		h.hashUint64(math.Float64bits(imag(complex128(v.Complex()))))
 	}
 }
 
 type mapHasher struct {
-	xbuf [sha256.Size]byte // XOR'ed accumulated buffer
-	ebuf [sha256.Size]byte // scratch buffer
-	s256 hash.Hash         // sha256 hash.Hash
-	bw   *bufio.Writer     // to hasher into ebuf
-	val  valueCache        // re-usable values for map iteration
-	iter *reflect.MapIter  // re-usable map iterator
-}
-
-func (mh *mapHasher) Reset() {
-	for i := range mh.xbuf {
-		mh.xbuf[i] = 0
-	}
-}
-
-func (mh *mapHasher) startEntry() {
-	for i := range mh.ebuf {
-		mh.ebuf[i] = 0
-	}
-	mh.bw.Flush()
-	mh.s256.Reset()
-}
-
-func (mh *mapHasher) endEntry() {
-	mh.bw.Flush()
-	for i, b := range mh.s256.Sum(mh.ebuf[:0]) {
-		mh.xbuf[i] ^= b
-	}
+	h    hasher
+	val  valueCache      // re-usable values for map iteration
+	iter reflect.MapIter // re-usable map iterator
 }
 
 var mapHasherPool = &sync.Pool{
-	New: func() interface{} {
-		mh := new(mapHasher)
-		mh.s256 = sha256.New()
-		mh.bw = bufio.NewWriter(mh.s256)
-		mh.val = make(valueCache)
-		mh.iter = new(reflect.MapIter)
-		return mh
-	},
+	New: func() interface{} { return new(mapHasher) },
 }
 
 type valueCache map[reflect.Type]reflect.Value
 
-func (c valueCache) get(t reflect.Type) reflect.Value {
-	v, ok := c[t]
+func (c *valueCache) get(t reflect.Type) reflect.Value {
+	v, ok := (*c)[t]
 	if !ok {
 		v = reflect.New(t).Elem()
-		c[t] = v
+		if *c == nil {
+			*c = make(valueCache)
+		}
+		(*c)[t] = v
 	}
 	return v
 }
@@ -301,25 +298,22 @@ func (c valueCache) get(t reflect.Type) reflect.Value {
 func (h *hasher) hashMap(v reflect.Value) {
 	mh := mapHasherPool.Get().(*mapHasher)
 	defer mapHasherPool.Put(mh)
-	mh.Reset()
-	iter := mapIter(mh.iter, v)
-	defer mapIter(mh.iter, reflect.Value{}) // avoid pinning v from mh.iter when we return
-
-	// Temporarily switch to the map hasher's bufio.Writer.
-	oldw := h.setBufioWriter(mh.bw)
-	defer h.setBufioWriter(oldw)
+	iter := mapIter(&mh.iter, v)
+	defer mapIter(&mh.iter, reflect.Value{}) // avoid pinning v from mh.iter when we return
 
+	var sum Sum
 	k := mh.val.get(v.Type().Key())
 	e := mh.val.get(v.Type().Elem())
+	mh.h.visitStack = h.visitStack // always use the parent's visit stack to avoid cycles
 	for iter.Next() {
 		key := iterKey(iter, k)
 		val := iterVal(iter, e)
-		mh.startEntry()
-		h.print(key)
-		h.print(val)
-		mh.endEntry()
+		mh.h.reset()
+		mh.h.hashValue(key)
+		mh.h.hashValue(val)
+		sum.xor(mh.h.sum())
 	}
-	oldw.Write(mh.xbuf[:])
+	h.bw.Write(append(h.scratch[:0], sum.sum[:]...)) // append into scratch to avoid heap allocation
 }
 
 // visitStack is a stack of pointers visited.
@@ -361,5 +355,5 @@ func (h *hasher) hashType(t reflect.Type) {
 	// that maps reflect.Type to some arbitrary and unique index.
 	// While safer, it requires global state with memory that can never be GC'd.
 	rtypeAddr := reflect.ValueOf(t).Pointer() // address of *reflect.rtype
-	h.uint(uint64(rtypeAddr))
+	h.hashUint64(uint64(rtypeAddr))
 }

util/deephash/deephash_test.go

@@ -9,6 +9,7 @@ import (
 	"bufio"
 	"bytes"
 	"fmt"
+	"math"
 	"reflect"
 	"testing"
 
@@ -31,12 +32,56 @@ func (p appendBytes) AppendTo(b []byte) []byte {
 func TestHash(t *testing.T) {
 	type tuple [2]interface{}
 	type iface struct{ X interface{} }
+	type scalars struct {
+		I8   int8
+		I16  int16
+		I32  int32
+		I64  int64
+		I    int
+		U8   uint8
+		U16  uint16
+		U32  uint32
+		U64  uint64
+		U    uint
+		UP   uintptr
+		F32  float32
+		F64  float64
+		C64  complex64
+		C128 complex128
+	}
 	type MyBool bool
 	type MyHeader tar.Header
 	tests := []struct {
 		in     tuple
 		wantEq bool
 	}{
+		{in: tuple{false, true}, wantEq: false},
+		{in: tuple{true, true}, wantEq: true},
+		{in: tuple{false, false}, wantEq: true},
+		{
+			in: tuple{
+				scalars{-8, -16, -32, -64, -1234, 8, 16, 32, 64, 1234, 5678, 32.32, 64.64, 32 + 32i, 64 + 64i},
+				scalars{-8, -16, -32, -64, -1234, 8, 16, 32, 64, 1234, 5678, 32.32, 64.64, 32 + 32i, 64 + 64i},
+			},
+			wantEq: true,
+		},
+		{in: tuple{scalars{I8: math.MinInt8}, scalars{I8: math.MinInt8 / 2}}, wantEq: false},
+		{in: tuple{scalars{I16: math.MinInt16}, scalars{I16: math.MinInt16 / 2}}, wantEq: false},
+		{in: tuple{scalars{I32: math.MinInt32}, scalars{I32: math.MinInt32 / 2}}, wantEq: false},
+		{in: tuple{scalars{I64: math.MinInt64}, scalars{I64: math.MinInt64 / 2}}, wantEq: false},
+		{in: tuple{scalars{I: -1234}, scalars{I: -1234 / 2}}, wantEq: false},
+		{in: tuple{scalars{U8: math.MaxUint8}, scalars{U8: math.MaxUint8 / 2}}, wantEq: false},
+		{in: tuple{scalars{U16: math.MaxUint16}, scalars{U16: math.MaxUint16 / 2}}, wantEq: false},
+		{in: tuple{scalars{U32: math.MaxUint32}, scalars{U32: math.MaxUint32 / 2}}, wantEq: false},
+		{in: tuple{scalars{U64: math.MaxUint64}, scalars{U64: math.MaxUint64 / 2}}, wantEq: false},
+		{in: tuple{scalars{U: 1234}, scalars{U: 1234 / 2}}, wantEq: false},
+		{in: tuple{scalars{UP: 5678}, scalars{UP: 5678 / 2}}, wantEq: false},
+		{in: tuple{scalars{F32: 32.32}, scalars{F32: math.Nextafter32(32.32, 0)}}, wantEq: false},
+		{in: tuple{scalars{F64: 64.64}, scalars{F64: math.Nextafter(64.64, 0)}}, wantEq: false},
+		{in: tuple{scalars{F32: float32(math.NaN())}, scalars{F32: float32(math.NaN())}}, wantEq: true},
+		{in: tuple{scalars{F64: float64(math.NaN())}, scalars{F64: float64(math.NaN())}}, wantEq: true},
+		{in: tuple{scalars{C64: 32 + 32i}, scalars{C64: complex(math.Nextafter32(32, 0), 32)}}, wantEq: false},
+		{in: tuple{scalars{C128: 64 + 64i}, scalars{C128: complex(math.Nextafter(64, 0), 64)}}, wantEq: false},
 		{in: tuple{[]appendBytes{{}, {0, 0, 0, 0, 0, 0, 0, 1}}, []appendBytes{{}, {0, 0, 0, 0, 0, 0, 0, 1}}}, wantEq: true},
 		{in: tuple{[]appendBytes{{}, {0, 0, 0, 0, 0, 0, 0, 1}}, []appendBytes{{0, 0, 0, 0, 0, 0, 0, 1}, {}}}, wantEq: false},
 		{in: tuple{iface{MyBool(true)}, iface{MyBool(true)}}, wantEq: true},
@@ -47,9 +92,6 @@ func TestHash(t *testing.T) {
 		{in: tuple{iface{&MyHeader{}}, iface{&tar.Header{}}}, wantEq: false},
 		{in: tuple{iface{[]map[string]MyBool{}}, iface{[]map[string]MyBool{}}}, wantEq: true},
 		{in: tuple{iface{[]map[string]bool{}}, iface{[]map[string]MyBool{}}}, wantEq: false},
-		{in: tuple{false, true}, wantEq: false},
-		{in: tuple{true, true}, wantEq: true},
-		{in: tuple{false, false}, wantEq: true},
 		{
 			in: func() tuple {
 				i1 := 1
@@ -225,10 +267,10 @@ func TestPrintArray(t *testing.T) {
 	var got bytes.Buffer
 	bw := bufio.NewWriter(&got)
 	h := &hasher{bw: bw}
-	h.print(reflect.ValueOf(x))
+	h.hashValue(reflect.ValueOf(x))
 	bw.Flush()
 	const want = "struct" +
-		"\x00\x00\x00\x00\x00\x00\x00\x01" + // 1 field
+		"\x01\x00\x00\x00\x00\x00\x00\x00" + // 1 field
 		"\x00\x00\x00\x00\x00\x00\x00\x00" + // 0th field
 		// the 32 bytes:
 		"\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x1f"

wgengine/magicsock/magicsock.go

@@ -3490,7 +3490,7 @@ func (de *discoEndpoint) sendDiscoPing(ep netaddr.IPPort, txid stun.TxID, logLev
 // discoPingPurpose is the reason why a discovery ping message was sent.
 type discoPingPurpose int
 
-//go:generate go run tailscale.com/cmd/addlicense -year 2020 -file discopingpurpose_string.go stringer -type=discoPingPurpose -trimprefix=ping
+//go:generate go run tailscale.com/cmd/addlicense -year 2020 -file discopingpurpose_string.go go run golang.org/x/tools/cmd/stringer -type=discoPingPurpose -trimprefix=ping
 const (
 	// pingDiscovery means that purpose of a ping was to see if a
 	// path was valid.