net/portmapper: use the upstream goupnp library instead of our fork

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Iabf9fc4e158f45e4fc385f2b4c2e55ba01351c5c

cmd/tailscale/depaware.txt

@@ -22,6 +22,12 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         github.com/gorilla/csrf                                      from tailscale.com/client/web
         github.com/gorilla/securecookie                              from github.com/gorilla/csrf
         github.com/hdevalence/ed25519consensus                       from tailscale.com/tka+
+        github.com/huin/goupnp                                       from github.com/huin/goupnp/dcps/internetgateway2+
+        github.com/huin/goupnp/dcps/internetgateway2                 from tailscale.com/net/portmapper
+        github.com/huin/goupnp/httpu                                 from github.com/huin/goupnp+
+        github.com/huin/goupnp/scpd                                  from github.com/huin/goupnp
+        github.com/huin/goupnp/soap                                  from github.com/huin/goupnp+
+        github.com/huin/goupnp/ssdp                                  from github.com/huin/goupnp
    L    github.com/josharian/native                                  from github.com/mdlayher/netlink+
    L 💣 github.com/jsimonetti/rtnetlink                              from tailscale.com/net/interfaces+
    L    github.com/jsimonetti/rtnetlink/internal/unix                from github.com/jsimonetti/rtnetlink
@@ -47,12 +53,6 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
    W 💣 github.com/tailscale/go-winio/internal/socket                from github.com/tailscale/go-winio
    W    github.com/tailscale/go-winio/internal/stringbuffer          from github.com/tailscale/go-winio/internal/fs
    W    github.com/tailscale/go-winio/pkg/guid                       from github.com/tailscale/go-winio+
-        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
    L 💣 github.com/tailscale/netlink                                 from tailscale.com/util/linuxfw
         github.com/tailscale/web-client-prebuilt                     from tailscale.com/client/web
         github.com/tcnksm/go-httpstat                                from tailscale.com/net/netcheck
@@ -250,7 +250,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 github.com/tailscale/goupnp+
+        encoding/xml                                                 from github.com/huin/goupnp+
         errors                                                       from bufio+
         expvar                                                       from tailscale.com/derp+
         flag                                                         from github.com/peterbourgon/ff/v3+
@@ -293,7 +293,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         path                                                         from html/template+
         path/filepath                                                from crypto/x509+
         reflect                                                      from crypto/x509+
-        regexp                                                       from github.com/tailscale/goupnp/httpu+
+        regexp                                                       from github.com/coreos/go-iptables/iptables+
         regexp/syntax                                                from regexp
         runtime/debug                                                from tailscale.com/util/singleflight+
         slices                                                       from tailscale.com/cmd/tailscale/cli+

cmd/tailscaled/depaware.txt

@@ -98,6 +98,12 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         github.com/gorilla/csrf                                      from tailscale.com/client/web
         github.com/gorilla/securecookie                              from github.com/gorilla/csrf
         github.com/hdevalence/ed25519consensus                       from tailscale.com/tka+
+        github.com/huin/goupnp                                       from github.com/huin/goupnp/dcps/internetgateway2+
+        github.com/huin/goupnp/dcps/internetgateway2                 from tailscale.com/net/portmapper
+        github.com/huin/goupnp/httpu                                 from github.com/huin/goupnp+
+        github.com/huin/goupnp/scpd                                  from github.com/huin/goupnp
+        github.com/huin/goupnp/soap                                  from github.com/huin/goupnp+
+        github.com/huin/goupnp/ssdp                                  from github.com/huin/goupnp
    L 💣 github.com/illarion/gonotify                                 from tailscale.com/net/dns
    L    github.com/insomniacslk/dhcp/dhcpv4                          from tailscale.com/net/tstun
    L    github.com/insomniacslk/dhcp/iana                            from github.com/insomniacslk/dhcp/dhcpv4
@@ -145,12 +151,6 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
   LD 💣 github.com/tailscale/golang-x-crypto/internal/alias          from github.com/tailscale/golang-x-crypto/chacha20
   LD    github.com/tailscale/golang-x-crypto/ssh                     from tailscale.com/ipn/ipnlocal+
   LD    github.com/tailscale/golang-x-crypto/ssh/internal/bcrypt_pbkdf from github.com/tailscale/golang-x-crypto/ssh
-        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/tailscale/hujson                                  from tailscale.com/ipn/conffile
    L 💣 github.com/tailscale/netlink                                 from tailscale.com/wgengine/router+
         github.com/tailscale/web-client-prebuilt                     from tailscale.com/client/web
@@ -483,7 +483,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+
+        encoding/xml                                                 from github.com/aws/aws-sdk-go-v2/aws/protocol/xml+
         errors                                                       from bufio+
         expvar                                                       from tailscale.com/derp+
         flag                                                         from net/http/httptest+

go.mod

@@ -108,6 +108,7 @@ require (
 	github.com/Microsoft/go-winio v0.6.1 // indirect
 	github.com/google/gnostic-models v0.6.9-0.20230804172637-c7be7c783f49 // indirect
 	github.com/gorilla/securecookie v1.1.1 // indirect
+	github.com/huin/goupnp v1.3.0 // indirect
 )
 
 require (

go.sum

@@ -524,6 +524,8 @@ github.com/hexops/gotextdiff v1.0.3/go.mod h1:pSWU5MAI3yDq+fZBTazCSJysOMbxWL1BSo
 github.com/huandu/xstrings v1.3.3/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq4ovT0aE=
 github.com/huandu/xstrings v1.4.0 h1:D17IlohoQq4UcpqD7fDk80P7l+lwAmlFaBHgOipl2FU=
 github.com/huandu/xstrings v1.4.0/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq4ovT0aE=
+github.com/huin/goupnp v1.3.0 h1:UvLUlWDNpoUdYzb2TCn+MuTWtcjXKSza2n6CBdQ0xXc=
+github.com/huin/goupnp v1.3.0/go.mod h1:gnGPsThkYa7bFi/KWmEysQRf48l2dvR5bxr2OFckNX8=
 github.com/iancoleman/strcase v0.3.0 h1:nTXanmYxhfFAMjZL34Ov6gkzEsSJZ5DbhxWjvSASxEI=
 github.com/iancoleman/strcase v0.3.0/go.mod h1:iwCmte+B7n89clKwxIoIXy/HfoL7AsD47ZCWhYzw7ho=
 github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=

net/portmapper/upnp.go

@@ -23,9 +23,9 @@ import (
 	"sync/atomic"
 	"time"
 
-	"github.com/tailscale/goupnp"
+	"github.com/huin/goupnp"
-	"github.com/tailscale/goupnp/dcps/internetgateway2"
+	"github.com/huin/goupnp/dcps/internetgateway2"
-	"github.com/tailscale/goupnp/soap"
+	"github.com/huin/goupnp/soap"
 	"tailscale.com/envknob"
 	"tailscale.com/net/netns"
 	"tailscale.com/types/logger"
@@ -62,48 +62,27 @@ func (u *upnpMapping) GoodUntil() time.Time     { return u.goodUntil }
 func (u *upnpMapping) RenewAfter() time.Time    { return u.renewAfter }
 func (u *upnpMapping) External() netip.AddrPort { return u.external }
 func (u *upnpMapping) Release(ctx context.Context) {
-	u.client.DeletePortMapping(ctx, "", u.external.Port(), upnpProtocolUDP)
+	u.client.DeletePortMappingCtx(ctx, "", u.external.Port(), upnpProtocolUDP)
 }
 
 // upnpClient is an interface over the multiple different clients exported by goupnp,
 // 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(
+	AddPortMappingCtx(
 		ctx context.Context,
-
+		NewRemoteHost string,
-		// remoteHost is the remote device sending packets to this device, in the format of x.x.x.x.
+		NewExternalPort uint16,
-		// The empty string, "", means any host out on the internet can send packets in.
+		NewProtocol string,
-		remoteHost string,
+		NewInternalPort uint16,
-
+		NewInternalClient string,
-		// externalPort is the exposed port of this port mapping. Visible during NAT operations.
+		NewEnabled bool,
-		// 0 will let the router select the port, but there is an additional call,
+		NewPortMappingDescription string,
-		// `AddAnyPortMapping`, which is available on 1 of the 3 possible protocols,
+		NewLeaseDuration uint32,
-		// 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,
 	) error
 
-	DeletePortMapping(ctx context.Context, remoteHost string, externalPort uint16, protocol string) error
+	DeletePortMappingCtx(ctx context.Context, remoteHost string, externalPort uint16, protocol string) error
-	GetExternalIPAddress(ctx context.Context) (externalIPAddress string, err error)
+	GetExternalIPAddressCtx(ctx context.Context) (externalIPAddress string, err error)
 }
 
 // tsPortMappingDesc gets sent to UPnP clients as a human-readable label for the portmapping.
@@ -153,7 +132,7 @@ func addAnyPortMapping(
 	// First off, try using AddAnyPortMapping; if there's a conflict, the
 	// router will pick another port and return it.
 	if upnp, ok := upnp.(*internetgateway2.WANIPConnection2); ok {
-		return upnp.AddAnyPortMapping(
+		return upnp.AddAnyPortMappingCtx(
 			ctx,
 			"",
 			externalPort,
@@ -168,7 +147,7 @@ func addAnyPortMapping(
 
 	// Fall back to using AddPortMapping, which requests a mapping to/from
 	// a specific external port.
-	err = upnp.AddPortMapping(
+	err = upnp.AddPortMappingCtx(
 		ctx,
 		"",
 		externalPort,
@@ -182,8 +161,8 @@ func addAnyPortMapping(
 	return externalPort, err
 }
 
-// getUPnPClient gets a client for interfacing with UPnP, ignoring the underlying protocol for
+// getUPnPClient gets a client for interfacing with UPnP, ignoring the
-// now.
+// underlying protocol for now.
 // Adapted from https://github.com/huin/goupnp/blob/master/GUIDE.md.
 //
 // The gw is the detected gateway.
@@ -191,9 +170,11 @@ func addAnyPortMapping(
 // 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
+// If set, the provided http.Client is copied and used as the HTTP client for
-// its associated HTTP client is (if set via goupnp.WithHTTPClient).
+// SOAP requests.
-func getUPnPClient(ctx context.Context, logf logger.Logf, debug DebugKnobs, gw netip.Addr, meta uPnPDiscoResponse) (client upnpClient, err error) {
+//
+// The provided ctx is not retained in the returned upnpClient.
+func getUPnPClient(ctx context.Context, logf logger.Logf, httpc *http.Client, debug DebugKnobs, gw netip.Addr, meta uPnPDiscoResponse) (client upnpClient, err error) {
 	if debug.DisableUPnP {
 		return nil, nil
 	}
@@ -229,12 +210,16 @@ func getUPnPClient(ctx context.Context, logf logger.Logf, debug DebugKnobs, gw n
 	defer cancel()
 
 	// This part does a network fetch.
-	root, err := goupnp.DeviceByURL(ctx, u)
+	root, err := goupnp.DeviceByURLCtx(ctx, u)
 	if err != nil {
 		return nil, err
 	}
 
+	var soapClient *soap.SOAPClient
 	defer func() {
+		if soapClient != nil && httpc != nil {
+			soapClient.HTTPClient = *httpc
+		}
 		if client == nil {
 			return
 		}
@@ -245,13 +230,16 @@ func getUPnPClient(ctx context.Context, logf logger.Logf, debug DebugKnobs, gw n
 
 	// These parts don't do a network fetch.
 	// Pick the best service type available.
-	if cc, _ := internetgateway2.NewWANIPConnection2ClientsFromRootDevice(ctx, root, u); len(cc) > 0 {
+	if cc, _ := internetgateway2.NewWANIPConnection2ClientsFromRootDevice(root, u); len(cc) > 0 {
+		soapClient = cc[0].ServiceClient.SOAPClient
 		return cc[0], nil
 	}
-	if cc, _ := internetgateway2.NewWANIPConnection1ClientsFromRootDevice(ctx, root, u); len(cc) > 0 {
+	if cc, _ := internetgateway2.NewWANIPConnection1ClientsFromRootDevice(root, u); len(cc) > 0 {
+		soapClient = cc[0].ServiceClient.SOAPClient
 		return cc[0], nil
 	}
-	if cc, _ := internetgateway2.NewWANPPPConnection1ClientsFromRootDevice(ctx, root, u); len(cc) > 0 {
+	if cc, _ := internetgateway2.NewWANPPPConnection1ClientsFromRootDevice(root, u); len(cc) > 0 {
+		soapClient = cc[0].ServiceClient.SOAPClient
 		return cc[0], nil
 	}
 	return nil, nil
@@ -305,8 +293,7 @@ func (c *Client) getUPnPPortMapping(
 	if ok && oldMapping != nil {
 		client = oldMapping.client
 	} else {
-		ctx := goupnp.WithHTTPClient(ctx, httpClient)
+		client, err = getUPnPClient(ctx, c.logf, httpClient, c.debug, gw, meta)
-		client, err = getUPnPClient(ctx, c.logf, c.debug, gw, meta)
 		if c.debug.VerboseLogs {
 			c.logf("getUPnPClient: %T, %v", client, err)
 		}
@@ -363,7 +350,7 @@ func (c *Client) getUPnPPortMapping(
 	}
 
 	// TODO cache this ip somewhere?
-	extIP, err := client.GetExternalIPAddress(ctx)
+	extIP, err := client.GetExternalIPAddressCtx(ctx)
 	if c.debug.VerboseLogs {
 		c.logf("client.GetExternalIPAddress: %v, %v", extIP, err)
 	}

net/portmapper/upnp_test.go

@@ -114,7 +114,7 @@ func TestGetUPnPClient(t *testing.T) {
 			gw, _ := netip.AddrFromSlice(ts.Listener.Addr().(*net.TCPAddr).IP)
 			gw = gw.Unmap()
 			var logBuf tstest.MemLogger
-			c, err := getUPnPClient(context.Background(), logBuf.Logf, DebugKnobs{}, gw, uPnPDiscoResponse{
+			c, err := getUPnPClient(context.Background(), logBuf.Logf, nil, DebugKnobs{}, gw, uPnPDiscoResponse{
 				Location: ts.URL + "/rootDesc.xml",
 			})
 			if err != nil {

tstest/jsdeps/jsdeps_test.go

@@ -14,11 +14,11 @@ func TestDeps(t *testing.T) {
 		GOOS:   "js",
 		GOARCH: "wasm",
 		BadDeps: map[string]string{
-			"runtime/pprof":               "bloat",
+			"runtime/pprof":              "bloat",
-			"golang.org/x/net/http2/h2c":  "bloat",
+			"golang.org/x/net/http2/h2c": "bloat",
-			"net/http/pprof":              "bloat",
+			"net/http/pprof":             "bloat",
-			"golang.org/x/net/proxy":      "bloat",
+			"golang.org/x/net/proxy":     "bloat",
-			"github.com/tailscale/goupnp": "bloat, which can't work anyway in wasm",
+			"github.com/huin/goupnp":     "bloat, which can't work anyway in wasm",
 		},
 	}.Check(t)
 }