net/portmapper: add upnp port mapping

Add in UPnP portmapping, using goupnp library in order to get the UPnP client and run the
portmapping functions. This rips out anywhere where UPnP used to be in portmapping, and has a
flow separate from PMP and PCP.

RELNOTE=portmapper now supports UPnP mappings

Fixes #682
Updates #2109

Signed-off-by: julianknodt <[email protected]>

cmd/tailscale/depaware.txt

@@ -7,6 +7,12 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
      💣 github.com/mitchellh/go-ps                                   from tailscale.com/cmd/tailscale/cli
         github.com/peterbourgon/ff/v2                                from github.com/peterbourgon/ff/v2/ffcli
         github.com/peterbourgon/ff/v2/ffcli                          from tailscale.com/cmd/tailscale/cli
+        github.com/tailscale/goupnp                                  from github.com/tailscale/goupnp/dcps/internetgateway2
+        github.com/tailscale/goupnp/dcps/internetgateway2            from tailscale.com/net/portmapper
+        github.com/tailscale/goupnp/httpu                            from github.com/tailscale/goupnp+
+        github.com/tailscale/goupnp/scpd                             from github.com/tailscale/goupnp
+        github.com/tailscale/goupnp/soap                             from github.com/tailscale/goupnp+
+        github.com/tailscale/goupnp/ssdp                             from github.com/tailscale/goupnp
         github.com/tcnksm/go-httpstat                                from tailscale.com/net/netcheck
         github.com/toqueteos/webbrowser                              from tailscale.com/cmd/tailscale/cli
      💣 go4.org/intern                                               from inet.af/netaddr
@@ -19,6 +25,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         tailscale.com/client/tailscale                               from tailscale.com/cmd/tailscale/cli+
         tailscale.com/client/tailscale/apitype                       from tailscale.com/client/tailscale+
         tailscale.com/cmd/tailscale/cli                              from tailscale.com/cmd/tailscale
+        tailscale.com/control/controlknobs                           from tailscale.com/net/portmapper
         tailscale.com/derp                                           from tailscale.com/derp/derphttp
         tailscale.com/derp/derphttp                                  from tailscale.com/net/netcheck
         tailscale.com/disco                                          from tailscale.com/derp
@@ -77,7 +84,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         golang.org/x/net/idna                                        from golang.org/x/net/http/httpguts+
         golang.org/x/net/proxy                                       from tailscale.com/net/netns
    D    golang.org/x/net/route                                       from net+
-        golang.org/x/sync/errgroup                                   from tailscale.com/derp
+        golang.org/x/sync/errgroup                                   from tailscale.com/derp+
         golang.org/x/sync/singleflight                               from tailscale.com/net/dnscache
         golang.org/x/sys/cpu                                         from golang.org/x/crypto/blake2b+
   LD    golang.org/x/sys/unix                                        from tailscale.com/net/netns+
@@ -127,7 +134,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         encoding/hex                                                 from crypto/x509+
         encoding/json                                                from expvar+
         encoding/pem                                                 from crypto/tls+
-        encoding/xml                                                 from tailscale.com/cmd/tailscale/cli
+        encoding/xml                                                 from tailscale.com/cmd/tailscale/cli+
         errors                                                       from bufio+
         expvar                                                       from tailscale.com/derp+
         flag                                                         from github.com/peterbourgon/ff/v2+

cmd/tailscaled/depaware.txt

@@ -23,6 +23,12 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
    L 💣 github.com/mdlayher/socket                                   from github.com/mdlayher/netlink
    W    github.com/pkg/errors                                        from github.com/tailscale/certstore
    W 💣 github.com/tailscale/certstore                               from tailscale.com/control/controlclient
+        github.com/tailscale/goupnp                                  from github.com/tailscale/goupnp/dcps/internetgateway2
+        github.com/tailscale/goupnp/dcps/internetgateway2            from tailscale.com/net/portmapper
+        github.com/tailscale/goupnp/httpu                            from github.com/tailscale/goupnp+
+        github.com/tailscale/goupnp/scpd                             from github.com/tailscale/goupnp
+        github.com/tailscale/goupnp/soap                             from github.com/tailscale/goupnp+
+        github.com/tailscale/goupnp/ssdp                             from github.com/tailscale/goupnp
         github.com/tcnksm/go-httpstat                                from tailscale.com/net/netcheck
      💣 go4.org/intern                                               from inet.af/netaddr
      💣 go4.org/mem                                                  from tailscale.com/derp+
@@ -79,6 +85,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/client/tailscale                               from tailscale.com/derp
         tailscale.com/client/tailscale/apitype                       from tailscale.com/ipn/ipnlocal+
         tailscale.com/control/controlclient                          from tailscale.com/ipn/ipnlocal+
+        tailscale.com/control/controlknobs                           from tailscale.com/control/controlclient+
         tailscale.com/derp                                           from tailscale.com/derp/derphttp+
         tailscale.com/derp/derphttp                                  from tailscale.com/net/netcheck+
         tailscale.com/disco                                          from tailscale.com/derp+
@@ -182,7 +189,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         golang.org/x/net/ipv6                                        from golang.zx2c4.com/wireguard/device+
         golang.org/x/net/proxy                                       from tailscale.com/net/netns
    D    golang.org/x/net/route                                       from net+
-        golang.org/x/sync/errgroup                                   from tailscale.com/derp
+        golang.org/x/sync/errgroup                                   from tailscale.com/derp+
         golang.org/x/sync/singleflight                               from tailscale.com/net/dnscache
         golang.org/x/sys/cpu                                         from golang.org/x/crypto/blake2b+
   LD    golang.org/x/sys/unix                                        from github.com/mdlayher/netlink+
@@ -236,6 +243,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         encoding/hex                                                 from crypto/x509+
         encoding/json                                                from expvar+
         encoding/pem                                                 from crypto/tls+
+        encoding/xml                                                 from github.com/tailscale/goupnp+
         errors                                                       from bufio+
         expvar                                                       from tailscale.com/derp+
         flag                                                         from tailscale.com/cmd/tailscaled+

control/controlclient/direct.go

@@ -31,6 +31,7 @@ import (
 
 	"golang.org/x/crypto/nacl/box"
 	"inet.af/netaddr"
+	"tailscale.com/control/controlknobs"
 	"tailscale.com/health"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/log/logheap"
@@ -797,7 +798,10 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netm
 			continue
 		}
 
-		if resp.Debug != nil {
+		hasDebug := resp.Debug != nil
+		// being conservative here, if Debug not present set to False
+		controlknobs.SetDisableUPnP(resp.Debug.DisableUPnP.And(hasDebug))
+		if hasDebug {
 			if resp.Debug.LogHeapPprof {
 				go logheap.LogHeap(resp.Debug.LogHeapURL)
 			}

control/controlknobs/controlknobs.go

@@ -0,0 +1,41 @@
+// 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 controlknobs contains client options configurable from control which can be turned on
+// or off. The ability to turn options on and off is for incrementally adding features in.
+package controlknobs
+
+import (
+	"os"
+	"strconv"
+	"sync/atomic"
+
+	"tailscale.com/types/opt"
+)
+
+// disableUPnP indicates whether to attempt UPnP mapping.
+var disableUPnP atomic.Value
+
+func init() {
+	v, _ := strconv.ParseBool(os.Getenv("TS_DISABLE_UPNP"))
+	var toStore opt.Bool
+	toStore.Set(v)
+	disableUPnP.Store(toStore)
+}
+
+// DisableUPnP reports the last reported value from control
+// whether UPnP portmapping should be disabled.
+func DisableUPnP() opt.Bool {
+	v, _ := disableUPnP.Load().(opt.Bool)
+	return v
+}
+
+// SetDisableUPnP will set whether UPnP connections are permitted or not,
+// intended to be set from control.
+func SetDisableUPnP(v opt.Bool) {
+	old, ok := disableUPnP.Load().(opt.Bool)
+	if !ok || old != v {
+		disableUPnP.Store(v)
+	}
+}

go.mod

@@ -29,6 +29,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 // indirect
 	github.com/tcnksm/go-httpstat v0.2.0
 	github.com/toqueteos/webbrowser v1.2.0
 	go4.org/mem v0.0.0-20201119185036-c04c5a6ff174

go.sum

@@ -580,6 +580,12 @@ 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.20210629174436-7df6c9efe30c h1:F+pROyGPs+9wdB7jBPHr9IZEF8SKj9YUCFFShnyLNZM=
+github.com/tailscale/goupnp v1.0.1-0.20210629174436-7df6c9efe30c/go.mod h1:PdCqy9JzfWMJf1H5UJW2ip33/d4YkoKN0r67yKH1mG8=
+github.com/tailscale/goupnp v1.0.1-0.20210629175715-39c5a55db683 h1:ZXmZQuVebYllEJL/dpttpIDGx723ezC5GJkHIu0YKrM=
+github.com/tailscale/goupnp v1.0.1-0.20210629175715-39c5a55db683/go.mod h1:PdCqy9JzfWMJf1H5UJW2ip33/d4YkoKN0r67yKH1mG8=
+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/tcnksm/go-httpstat v0.2.0 h1:rP7T5e5U2HfmOBmZzGgGZjBQ5/GluWUylujl0tJ04I0=
 github.com/tcnksm/go-httpstat v0.2.0/go.mod h1:s3JVJFtQxtBEBC9dwcdTTXS9xFnM3SXAZwPG41aurT8=
 github.com/tdakkota/asciicheck v0.0.0-20200416190851-d7f85be797a2/go.mod h1:yHp0ai0Z9gUljN3o0xMhYJnH/IcvkdTBOX2fmJ93JEM=
@@ -686,6 +692,7 @@ golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73r
 golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
+golang.org/x/net v0.0.0-20181011144130-49bb7cea24b1/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20181023162649-9b4f9f5ad519/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20181114220301-adae6a3d119a/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20181201002055-351d144fa1fc/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
@@ -732,6 +739,7 @@ golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJ
 golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20210220032951-036812b2e83c h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ=

net/portmapper/portmapper.go

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// Package portmapper is a UDP port mapping client. It currently only does
-// NAT-PMP, but will likely do UPnP and perhaps PCP later.
+// Package portmapper is a UDP port mapping client. It currently allows for mapping over
+// NAT-PMP and UPnP, but will perhaps do PCP later.
 package portmapper
 
 import (
@@ -17,7 +17,6 @@ import (
 	"sync"
 	"time"
 
-	"go4.org/mem"
 	"inet.af/netaddr"
 	"tailscale.com/net/interfaces"
 	"tailscale.com/net/netns"
@@ -66,15 +65,34 @@ type Client struct {
 	pcpSawTime  time.Time // time we last saw PCP was available
 	uPnPSawTime time.Time // time we last saw UPnP was available
 
-	localPort  uint16
-	pmpMapping *pmpMapping // non-nil if we have a PMP mapping
+	localPort uint16
+
+	mapping mapping // non-nil if we have a mapping
+}
+
+// mapping represents a created port-mapping over some protocol.  It specifies a lease duration,
+// how to release the mapping, and whether the map is still valid.
+//
+// After a mapping is created, it should be immutable, and thus reads should be safe across
+// concurrent goroutines.
+type mapping interface {
+	// Release will attempt to unmap the established port mapping. It will block until completion,
+	// but can be called asynchronously. Release should be idempotent, and thus even if called
+	// multiple times should not cause additional side-effects.
+	Release(context.Context)
+	// goodUntil will return the lease time that the mapping is valid for.
+	GoodUntil() time.Time
+	// renewAfter returns the earliest time that the mapping should be renewed.
+	RenewAfter() time.Time
+	// externalIPPort indicates what port the mapping can be reached from on the outside.
+	External() netaddr.IPPort
 }
 
 // HaveMapping reports whether we have a current valid mapping.
 func (c *Client) HaveMapping() bool {
 	c.mu.Lock()
 	defer c.mu.Unlock()
-	return c.pmpMapping != nil && c.pmpMapping.goodUntil.After(time.Now())
+	return c.mapping != nil && c.mapping.GoodUntil().After(time.Now())
 }
 
 // pmpMapping is an already-created PMP mapping.
@@ -94,9 +112,13 @@ func (m *pmpMapping) externalValid() bool {
 	return !m.external.IP().IsZero() && m.external.Port() != 0
 }
 
-// release does a best effort fire-and-forget release of the PMP mapping m.
-func (m *pmpMapping) release() {
-	uc, err := netns.Listener().ListenPacket(context.Background(), "udp4", ":0")
+func (p *pmpMapping) GoodUntil() time.Time     { return p.goodUntil }
+func (p *pmpMapping) RenewAfter() time.Time    { return p.renewAfter }
+func (p *pmpMapping) External() netaddr.IPPort { return p.external }
+
+// Release does a best effort fire-and-forget release of the PMP mapping m.
+func (m *pmpMapping) Release(ctx context.Context) {
+	uc, err := netns.Listener().ListenPacket(ctx, "udp4", ":0")
 	if err != nil {
 		return
 	}
@@ -166,7 +188,6 @@ func (c *Client) gatewayAndSelfIP() (gw, myIP netaddr.IP, ok bool) {
 		gw = netaddr.IP{}
 		myIP = netaddr.IP{}
 	}
-
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
@@ -179,11 +200,11 @@ func (c *Client) gatewayAndSelfIP() (gw, myIP netaddr.IP, ok bool) {
 }
 
 func (c *Client) invalidateMappingsLocked(releaseOld bool) {
-	if c.pmpMapping != nil {
+	if c.mapping != nil {
 		if releaseOld {
-			c.pmpMapping.release()
+			c.mapping.Release(context.Background())
 		}
-		c.pmpMapping = nil
+		c.mapping = nil
 	}
 	c.pmpPubIP = netaddr.IP{}
 	c.pmpPubIPTime = time.Time{}
@@ -262,12 +283,12 @@ func (c *Client) GetCachedMappingOrStartCreatingOne() (external netaddr.IPPort,
 
 	// Do we have an existing mapping that's valid?
 	now := time.Now()
-	if m := c.pmpMapping; m != nil {
-		if now.Before(m.goodUntil) {
-			if now.After(m.renewAfter) {
+	if m := c.mapping; m != nil {
+		if now.Before(m.GoodUntil()) {
+			if now.After(m.RenewAfter()) {
 				c.maybeStartMappingLocked()
 			}
-			return m.external, true
+			return m.External(), true
 		}
 	}
 
@@ -315,9 +336,10 @@ func (c *Client) createOrGetMapping(ctx context.Context) (external netaddr.IPPor
 
 	c.mu.Lock()
 	localPort := c.localPort
+	internalAddr := netaddr.IPPortFrom(myIP, localPort)
 	m := &pmpMapping{
 		gw:       gw,
-		internal: netaddr.IPPortFrom(myIP, localPort),
+		internal: internalAddr,
 	}
 
 	// prevPort is the port we had most previously, if any. We try
@@ -326,13 +348,13 @@ func (c *Client) createOrGetMapping(ctx context.Context) (external netaddr.IPPor
 
 	// Do we have an existing mapping that's valid?
 	now := time.Now()
-	if m := c.pmpMapping; m != nil {
-		if now.Before(m.renewAfter) {
+	if m := c.mapping; m != nil {
+		if now.Before(m.RenewAfter()) {
 			defer c.mu.Unlock()
-			return m.external, nil
+			return m.External(), nil
 		}
 		// The mapping might still be valid, so just try to renew it.
-		prevPort = m.external.Port()
+		prevPort = m.External().Port()
 	}
 
 	// If we just did a Probe (e.g. via netchecker) but didn't
@@ -344,6 +366,10 @@ func (c *Client) createOrGetMapping(ctx context.Context) (external netaddr.IPPor
 	}
 	if c.lastProbe.After(now.Add(-5*time.Second)) && !haveRecentPMP {
 		c.mu.Unlock()
+		// fallback to UPnP portmapping
+		if mapping, ok := c.getUPnPPortMapping(ctx, gw, internalAddr, prevPort); ok {
+			return mapping, nil
+		}
 		return netaddr.IPPort{}, NoMappingError{ErrNoPortMappingServices}
 	}
 
@@ -381,6 +407,10 @@ func (c *Client) createOrGetMapping(ctx context.Context) (external netaddr.IPPor
 			if ctx.Err() == context.Canceled {
 				return netaddr.IPPort{}, err
 			}
+			// fallback to UPnP portmapping
+			if mapping, ok := c.getUPnPPortMapping(ctx, gw, internalAddr, prevPort); ok {
+				return mapping, nil
+			}
 			return netaddr.IPPort{}, NoMappingError{ErrNoPortMappingServices}
 		}
 		srcu := srci.(*net.UDPAddr)
@@ -413,7 +443,7 @@ func (c *Client) createOrGetMapping(ctx context.Context) (external netaddr.IPPor
 		if m.externalValid() {
 			c.mu.Lock()
 			defer c.mu.Unlock()
-			c.pmpMapping = m
+			c.mapping = m
 			return m.external, nil
 		}
 	}
@@ -530,9 +560,27 @@ 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
@@ -547,16 +595,11 @@ 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 && res.UPnP {
+		if pcpHeard && res.PMP {
 			// Nothing more to discover.
 			return res, nil
 		}
@@ -569,13 +612,6 @@ 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:")) {
-				res.UPnP = true
-				c.mu.Lock()
-				c.uPnPSawTime = time.Now()
-				c.mu.Unlock()
-			}
 		case pcpPort: // same as pmpPort
 			if pres, ok := parsePCPResponse(buf[:n]); ok {
 				if pres.OpCode == pcpOpReply|pcpOpAnnounce {
@@ -687,14 +723,4 @@ func parsePCPResponse(b []byte) (res pcpResponse, ok bool) {
 	return res, true
 }
 
-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")
-
 var pmpReqExternalAddrPacket = []byte{0, 0} // version 0, opcode 0 = "Public address request"

net/portmapper/portmapper_test.go

@@ -17,6 +17,7 @@ func TestCreateOrGetMapping(t *testing.T) {
 		t.Skip("skipping test without HIT_NETWORK=1")
 	}
 	c := NewClient(t.Logf, nil)
+	defer c.Close()
 	c.SetLocalPort(1234)
 	for i := 0; i < 2; i++ {
 		if i > 0 {
@@ -32,12 +33,13 @@ func TestClientProbe(t *testing.T) {
 		t.Skip("skipping test without HIT_NETWORK=1")
 	}
 	c := NewClient(t.Logf, nil)
-	for i := 0; i < 2; i++ {
+	defer c.Close()
+	for i := 0; i < 3; i++ {
 		if i > 0 {
 			time.Sleep(100 * time.Millisecond)
 		}
 		res, err := c.Probe(context.Background())
-		t.Logf("Got: %+v, %v", res, err)
+		t.Logf("Got(t=%dms): %+v, %v", i*100, res, err)
 	}
 }
 
@@ -46,6 +48,7 @@ func TestClientProbeThenMap(t *testing.T) {
 		t.Skip("skipping test without HIT_NETWORK=1")
 	}
 	c := NewClient(t.Logf, nil)
+	defer c.Close()
 	c.SetLocalPort(1234)
 	res, err := c.Probe(context.Background())
 	t.Logf("Probe: %+v, %v", res, err)

net/portmapper/upnp.go

@@ -0,0 +1,239 @@
+// 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"
+	"net/url"
+	"time"
+
+	"github.com/tailscale/goupnp/dcps/internetgateway2"
+	"inet.af/netaddr"
+	"tailscale.com/control/controlknobs"
+)
+
+// References:
+//
+// WANIP Connection v2: http://upnp.org/specs/gw/UPnP-gw-WANIPConnection-v2-Service.pdf
+
+// upnpMapping is a port mapping over the upnp protocol. After being created it is immutable,
+// but the client field may be shared across mapping instances.
+type upnpMapping struct {
+	gw         netaddr.IP
+	external   netaddr.IPPort
+	internal   netaddr.IPPort
+	goodUntil  time.Time
+	renewAfter time.Time
+
+	// client is a connection to a upnp device, and may be reused across different UPnP mappings.
+	client upnpClient
+}
+
+func (u *upnpMapping) GoodUntil() time.Time     { return u.goodUntil }
+func (u *upnpMapping) RenewAfter() time.Time    { return u.renewAfter }
+func (u *upnpMapping) External() netaddr.IPPort { return u.external }
+func (u *upnpMapping) Release(ctx context.Context) {
+	u.client.DeletePortMapping(ctx, "", u.external.Port(), "udp")
+}
+
+// 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.
+type upnpClient interface {
+	AddPortMapping(
+		ctx context.Context,
+
+		// remoteHost is the remote device sending packets to this device, in the format of x.x.x.x.
+		// The empty string, "", means any host out on the internet can send packets in.
+		remoteHost string,
+
+		// externalPort is the exposed port of this port mapping. Visible during NAT operations.
+		// 0 will let the router select the port, but there is an additional call,
+		// `AddAnyPortMapping`, which is available on 1 of the 3 possible protocols,
+		// which should be used if available. See `addAnyPortMapping` below, which calls this if
+		// `AddAnyPortMapping` is not supported.
+		externalPort uint16,
+
+		// protocol is whether this is over TCP or UDP. Either "tcp" or "udp".
+		protocol string,
+
+		// internalPort is the port that the gateway device forwards the traffic to.
+		internalPort uint16,
+		// internalClient is the IP address that packets will be forwarded to for this mapping.
+		// Internal client is of the form "x.x.x.x".
+		internalClient string,
+
+		// enabled is whether this portmapping should be enabled or disabled.
+		enabled bool,
+		// portMappingDescription is a user-readable description of this portmapping.
+		portMappingDescription string,
+		// leaseDurationSec is the duration of this portmapping. The value of this argument must be
+		// 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)
+
+	DeletePortMapping(ctx context.Context, remoteHost string, externalPort uint16, protocol string) error
+	GetExternalIPAddress(ctx context.Context) (externalIPAddress string, err error)
+}
+
+// tsPortMappingDesc gets sent to UPnP clients as a human-readable label for the portmapping.
+// It is not used for anything other than labelling.
+const tsPortMappingDesc = "tailscale-portmap"
+
+// addAnyPortMapping abstracts over different UPnP client connections, calling the available
+// AddAnyPortMapping call if available for WAN IP connection v2, otherwise defaulting to the old
+// behavior of calling AddPortMapping with port = 0 to specify a wildcard port.
+func addAnyPortMapping(
+	ctx context.Context,
+	upnp upnpClient,
+	externalPort uint16,
+	internalPort uint16,
+	internalClient string,
+	leaseDuration time.Duration,
+) (newPort uint16, err error) {
+	if upnp, ok := upnp.(*internetgateway2.WANIPConnection2); ok {
+		return upnp.AddAnyPortMapping(
+			ctx,
+			"",
+			externalPort,
+			"udp",
+			internalPort,
+			internalClient,
+			true,
+			tsPortMappingDesc,
+			uint32(leaseDuration.Seconds()),
+		)
+	}
+	err = upnp.AddPortMapping(
+		ctx,
+		"",
+		externalPort,
+		"udp",
+		internalPort,
+		internalClient,
+		true,
+		tsPortMappingDesc,
+		uint32(leaseDuration.Seconds()),
+	)
+	return internalPort, err
+}
+
+// getUPnPClients 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) {
+	if dis, ok := controlknobs.DisableUPnP().Get(); ok && dis {
+		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 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()
+	}
+}
+
+// getUPnPPortMapping attempts to create a port-mapping over the UPnP protocol. On success,
+// it will return the externally exposed IP and port. Otherwise, it will return a zeroed IP and
+// port and an error.
+func (c *Client) getUPnPPortMapping(
+	ctx context.Context,
+	gw netaddr.IP,
+	internal netaddr.IPPort,
+	prevPort uint16,
+) (external netaddr.IPPort, ok bool) {
+	if dis, ok := controlknobs.DisableUPnP().Get(); ok && dis {
+		return netaddr.IPPort{}, false
+	}
+	now := time.Now()
+	upnp := &upnpMapping{
+		gw:       gw,
+		internal: internal,
+	}
+
+	var client upnpClient
+	var err error
+	c.mu.Lock()
+	oldMapping, ok := c.mapping.(*upnpMapping)
+	c.mu.Unlock()
+	if ok && oldMapping != nil {
+		client = oldMapping.client
+	} else {
+		client, err = getUPnPClient(ctx, gw)
+		if err != nil {
+			return netaddr.IPPort{}, false
+		}
+	}
+	if client == nil {
+		return netaddr.IPPort{}, false
+	}
+
+	var newPort uint16
+	newPort, err = addAnyPortMapping(
+		ctx,
+		client,
+		prevPort,
+		internal.Port(),
+		internal.IP().String(),
+		time.Second*pmpMapLifetimeSec,
+	)
+	if err != nil {
+		return netaddr.IPPort{}, false
+	}
+	// TODO cache this ip somewhere?
+	extIP, err := client.GetExternalIPAddress(ctx)
+	if err != nil {
+		// TODO this doesn't seem right
+		return netaddr.IPPort{}, false
+	}
+	externalIP, err := netaddr.ParseIP(extIP)
+	if err != nil {
+		return netaddr.IPPort{}, false
+	}
+
+	upnp.external = netaddr.IPPortFrom(externalIP, newPort)
+	d := time.Duration(pmpMapLifetimeSec) * time.Second
+	upnp.goodUntil = now.Add(d)
+	upnp.renewAfter = now.Add(d / 2)
+	upnp.client = client
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	c.mapping = upnp
+	c.localPort = newPort
+	return upnp.external, true
+}

tailcfg/tailcfg.go

@@ -1060,6 +1060,13 @@ type Debug struct {
 	// :0 to get a random local port, ignoring any configured
 	// fixed port.
 	RandomizeClientPort bool `json:",omitempty"`
+
+	/// DisableUPnP is whether the client will attempt to perform a UPnP portmapping.
+	// By default, we want to enable it to see if it works on more clients.
+	//
+	// If UPnP catastrophically fails for people, this should be set to True to kill
+	// new attempts at UPnP connections.
+	DisableUPnP opt.Bool `json:",omitempty"`
 }
 
 func (k MachineKey) String() string                   { return fmt.Sprintf("mkey:%x", k[:]) }

tstest/integration/integration_test.go

@@ -47,6 +47,8 @@ var (
 var mainError atomic.Value // of error
 
 func TestMain(m *testing.M) {
+	// Have to disable UPnP which hits the network, otherwise it fails due to HTTP proxy.
+	os.Setenv("TS_DISABLE_UPNP", "true")
 	flag.Parse()
 	v := m.Run()
 	if v != 0 {

tstest/integration/testcontrol/testcontrol.go

@@ -636,6 +636,9 @@ func (s *Server) MapResponse(req *tailcfg.MapRequest) (res *tailcfg.MapResponse,
 		Domain:          string(user.Domain),
 		CollectServices: "true",
 		PacketFilter:    tailcfg.FilterAllowAll,
+		Debug: &tailcfg.Debug{
+			DisableUPnP: "true",
+		},
 	}
 	for _, p := range s.AllNodes() {
 		if p.StableID != node.StableID {

types/opt/bool.go

@@ -29,6 +29,13 @@ func (b Bool) Get() (v bool, ok bool) {
 	return v, err == nil
 }
 
+func (b Bool) And(v bool) Bool {
+	if v {
+		return b
+	}
+	return "false"
+}
+
 // EqualBool reports whether b is equal to v.
 // If b is empty or not a valid bool, it reports false.
 func (b Bool) EqualBool(v bool) bool {

types/opt/bool_test.go

@@ -87,3 +87,26 @@ func TestBoolEqualBool(t *testing.T) {
 	}
 
 }
+
+func TestBoolAnd(t *testing.T) {
+	tests := []struct {
+		lhs  Bool
+		rhs  bool
+		want Bool
+	}{
+		{"true", true, "true"},
+		{"true", false, "false"},
+
+		{"false", true, "false"},
+		{"false", false, "false"},
+
+		{"", true, ""},
+		{"", false, "false"},
+	}
+	for _, tt := range tests {
+		if got := tt.lhs.And(tt.rhs); got != tt.want {
+			t.Errorf("(%q).And(%v) = %v; want %v", string(tt.lhs), tt.rhs, got, tt.want)
+		}
+	}
+
+}