From 2a4a7f2fc73676deabaa4c2abe479f46eefa7993 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 10 Jan 2025 21:24:37 -0800 Subject: [PATCH] lanscaping: remove portmapper (goupnp, xml, ...) -rwxr-xr-x@ 1 bradfitz staff 13941426 Jan 10 21:24 /Users/bradfitz/bin/tailscaled.min -rwxr-xr-x@ 1 bradfitz staff 15204504 Jan 10 21:24 /Users/bradfitz/bin/tailscaled.minlinux Change-Id: I3c324d4203ba8543295e0503b84874710b8afeb6 Signed-off-by: Brad Fitzpatrick --- cmd/tailscale/depaware-minlinux.txt | 2 +- cmd/tailscaled/depaware-minlinux.txt | 10 +- ipn/localapi/localapi.go | 154 --------------------------- net/netcheck/netcheck.go | 37 ------- wgengine/magicsock/magicsock.go | 31 +----- 5 files changed, 3 insertions(+), 231 deletions(-) diff --git a/cmd/tailscale/depaware-minlinux.txt b/cmd/tailscale/depaware-minlinux.txt index 5d41b91a2..1ecdabd85 100644 --- a/cmd/tailscale/depaware-minlinux.txt +++ b/cmd/tailscale/depaware-minlinux.txt @@ -86,7 +86,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep tailscale.com/net/netutil from tailscale.com/client/tailscale+ tailscale.com/net/packet from tailscale.com/wgengine/capture tailscale.com/net/ping from tailscale.com/net/netcheck - tailscale.com/net/portmapper from tailscale.com/cmd/tailscale/cli+ + tailscale.com/net/portmapper from tailscale.com/cmd/tailscale/cli tailscale.com/net/sockstats from tailscale.com/control/controlhttp+ tailscale.com/net/stun from tailscale.com/net/netcheck L tailscale.com/net/tcpinfo from tailscale.com/derp diff --git a/cmd/tailscaled/depaware-minlinux.txt b/cmd/tailscaled/depaware-minlinux.txt index 7c1e47f38..da092ca23 100644 --- a/cmd/tailscaled/depaware-minlinux.txt +++ b/cmd/tailscaled/depaware-minlinux.txt @@ -37,12 +37,6 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de github.com/tailscale/golang-x-crypto/internal/poly1305 from github.com/tailscale/golang-x-crypto/ssh github.com/tailscale/golang-x-crypto/ssh from tailscale.com/ipn/ipnlocal 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/net/routetable+ L 💣 github.com/tailscale/netlink/nl from github.com/tailscale/netlink @@ -123,7 +117,6 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/net/packet from tailscale.com/net/connstats+ tailscale.com/net/packet/checksum from tailscale.com/net/tstun tailscale.com/net/ping from tailscale.com/net/netcheck+ - tailscale.com/net/portmapper from tailscale.com/ipn/localapi+ tailscale.com/net/proxymux from tailscale.com/cmd/tailscaled tailscale.com/net/routetable from tailscale.com/doctor/routetable tailscale.com/net/socks5 from tailscale.com/cmd/tailscaled @@ -307,7 +300,6 @@ 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 archive/tar+ expvar from tailscale.com/derp+ flag from net/http/httptest+ @@ -319,7 +311,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de html from net/http/pprof+ io from archive/tar+ io/fs from archive/tar+ - io/ioutil from github.com/digitalocean/go-smbios/smbios+ + L io/ioutil from github.com/digitalocean/go-smbios/smbios+ iter from github.com/go-json-experiment/json/jsontext+ log from expvar+ log/internal from log diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index d1b69fad4..c55351458 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -40,9 +40,7 @@ import ( "tailscale.com/ipn/ipnlocal" "tailscale.com/ipn/ipnstate" "tailscale.com/logtail" - "tailscale.com/net/netmon" "tailscale.com/net/netutil" - "tailscale.com/net/portmapper" "tailscale.com/tailcfg" "tailscale.com/taildrop" "tailscale.com/tstime" @@ -93,7 +91,6 @@ var handler = map[string]localAPIHandler{ "debug-packet-filter-matches": (*Handler).serveDebugPacketFilterMatches, "debug-packet-filter-rules": (*Handler).serveDebugPacketFilterRules, "debug-peer-endpoint-changes": (*Handler).serveDebugPeerEndpointChanges, - "debug-portmap": (*Handler).serveDebugPortmap, "derpmap": (*Handler).serveDERPMap, "dev-set-state-store": (*Handler).serveDevSetStateStore, "dial": (*Handler).serveDial, @@ -715,157 +712,6 @@ func (h *Handler) serveDebugPacketFilterMatches(w http.ResponseWriter, r *http.R enc.Encode(nm.PacketFilter) } -func (h *Handler) serveDebugPortmap(w http.ResponseWriter, r *http.Request) { - if !h.PermitWrite { - http.Error(w, "debug access denied", http.StatusForbidden) - return - } - w.Header().Set("Content-Type", "text/plain") - - dur, err := time.ParseDuration(r.FormValue("duration")) - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - - gwSelf := r.FormValue("gateway_and_self") - - // Update portmapper debug flags - debugKnobs := &portmapper.DebugKnobs{VerboseLogs: true} - switch r.FormValue("type") { - case "": - case "pmp": - debugKnobs.DisablePCP = true - debugKnobs.DisableUPnP = true - case "pcp": - debugKnobs.DisablePMP = true - debugKnobs.DisableUPnP = true - case "upnp": - debugKnobs.DisablePCP = true - debugKnobs.DisablePMP = true - default: - http.Error(w, "unknown portmap debug type", http.StatusBadRequest) - return - } - - if defBool(r.FormValue("log_http"), false) { - debugKnobs.LogHTTP = true - } - - var ( - logLock sync.Mutex - handlerDone bool - ) - logf := func(format string, args ...any) { - if !strings.HasSuffix(format, "\n") { - format = format + "\n" - } - - logLock.Lock() - defer logLock.Unlock() - - // The portmapper can call this log function after the HTTP - // handler returns, which is not allowed and can cause a panic. - // If this happens, ignore the log lines since this typically - // occurs due to a client disconnect. - if handlerDone { - return - } - - // Write and flush each line to the client so that output is streamed - fmt.Fprintf(w, format, args...) - if f, ok := w.(http.Flusher); ok { - f.Flush() - } - } - defer func() { - logLock.Lock() - handlerDone = true - logLock.Unlock() - }() - - ctx, cancel := context.WithTimeout(r.Context(), dur) - defer cancel() - - done := make(chan bool, 1) - - var c *portmapper.Client - c = portmapper.NewClient(logger.WithPrefix(logf, "portmapper: "), h.b.NetMon(), debugKnobs, h.b.ControlKnobs(), 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") - }) - defer c.Close() - - netMon, err := netmon.New(logger.WithPrefix(logf, "monitor: ")) - if err != nil { - logf("error creating monitor: %v", err) - return - } - - gatewayAndSelfIP := func() (gw, self netip.Addr, ok bool) { - if a, b, ok := strings.Cut(gwSelf, "/"); ok { - gw = netip.MustParseAddr(a) - self = netip.MustParseAddr(b) - return gw, self, true - } - return netMon.GatewayAndSelfIP() - } - - c.SetGatewayLookupFunc(gatewayAndSelfIP) - - gw, selfIP, ok := gatewayAndSelfIP() - if !ok { - logf("no gateway or self IP; %v", netMon.InterfaceState()) - return - } - logf("gw=%v; self=%v", gw, selfIP) - - uc, err := net.ListenPacket("udp", "0.0.0.0:0") - if err != nil { - return - } - defer uc.Close() - c.SetLocalPort(uint16(uc.LocalAddr().(*net.UDPAddr).Port)) - - res, err := c.Probe(ctx) - if err != nil { - logf("error in Probe: %v", err) - return - } - logf("Probe: %+v", res) - - if !res.PCP && !res.PMP && !res.UPnP { - logf("no portmapping services available") - return - } - - if ext, ok := c.GetCachedMappingOrStartCreatingOne(); ok { - logf("mapping: %v", ext) - } else { - logf("no mapping") - } - - select { - case <-done: - case <-ctx.Done(): - if r.Context().Err() == nil { - logf("serveDebugPortmap: context done: %v", ctx.Err()) - } else { - h.logf("serveDebugPortmap: context done: %v", ctx.Err()) - } - } -} - func (h *Handler) serveComponentDebugLogging(w http.ResponseWriter, r *http.Request) { if !h.PermitWrite { http.Error(w, "debug access denied", http.StatusForbidden) diff --git a/net/netcheck/netcheck.go b/net/netcheck/netcheck.go index f05a77ebf..5c7deebcb 100644 --- a/net/netcheck/netcheck.go +++ b/net/netcheck/netcheck.go @@ -30,7 +30,6 @@ import ( "tailscale.com/net/netmon" "tailscale.com/net/netns" "tailscale.com/net/ping" - "tailscale.com/net/portmapper" "tailscale.com/net/sockstats" "tailscale.com/net/stun" "tailscale.com/syncs" @@ -221,10 +220,6 @@ type Client struct { // in tests to avoid probing the local LAN's router, etc. SkipExternalNetwork bool - // PortMapper, if non-nil, is used for portmap queries. - // If nil, portmap discovery is not done. - PortMapper *portmapper.Client // lazily initialized on first use - // UseDNSCache controls whether this client should use a // *dnscache.Resolver to resolve DERP hostnames, when no IP address is // provided in the DERP map. Note that Tailscale-provided DERP servers @@ -726,29 +721,6 @@ func (rs *reportState) setOptBool(b *opt.Bool, v bool) { b.Set(v) } -func (rs *reportState) probePortMapServices() { - defer rs.waitPortMap.Done() - - rs.setOptBool(&rs.report.UPnP, false) - rs.setOptBool(&rs.report.PMP, false) - rs.setOptBool(&rs.report.PCP, false) - - res, err := rs.c.PortMapper.Probe(context.Background()) - if err != nil { - if !errors.Is(err, portmapper.ErrGatewayRange) { - // "skipping portmap; gateway range likely lacks support" - // is not very useful, and too spammy on cloud systems. - // If there are other errors, we want to log those. - rs.c.logf("probePortMapServices: %v", err) - } - return - } - - rs.setOptBool(&rs.report.UPnP, res.UPnP) - rs.setOptBool(&rs.report.PMP, res.PMP) - rs.setOptBool(&rs.report.PCP, res.PCP) -} - func newReport() *Report { return &Report{ RegionLatency: make(map[int]time.Duration), @@ -887,11 +859,6 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap, opts *GetRe v6udp.Close() } - if !c.SkipExternalNetwork && c.PortMapper != nil { - rs.waitPortMap.Add(1) - go rs.probePortMapServices() - } - var plan probePlan if opts == nil || !opts.OnlyTCP443 { plan = makeProbePlan(dm, ifState, last, preferredDERP) @@ -966,10 +933,6 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap, opts *GetRe captivePortalStop() } - if !c.SkipExternalNetwork && c.PortMapper != nil { - rs.waitPortMap.Wait() - c.vlogf("portMap done") - } rs.stopTimers() // Try HTTPS and ICMP latency check if all STUN probes failed due to diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index d3075f55d..8f8f80a38 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -40,7 +40,6 @@ import ( "tailscale.com/net/netns" "tailscale.com/net/packet" "tailscale.com/net/ping" - "tailscale.com/net/portmapper" "tailscale.com/net/sockstats" "tailscale.com/net/stun" "tailscale.com/net/tstun" @@ -174,10 +173,6 @@ type Conn struct { // conditions, including the closest DERP relay and NAT mappings. netChecker *netcheck.Client - // portMapper is the NAT-PMP/PCP/UPnP prober/client, for requesting - // port mappings from NAT devices. - portMapper *portmapper.Client - // derpRecvCh is used by receiveDERP to read DERP messages. // It must have buffer size > 0; see issue 3736. derpRecvCh chan derpReadResult @@ -533,11 +528,6 @@ func NewConn(opts Options) (*Conn, error) { c.idleFunc = opts.IdleFunc c.testOnlyPacketListener = opts.TestOnlyPacketListener c.noteRecvActivity = opts.NoteRecvActivity - portMapOpts := &portmapper.DebugKnobs{ - DisableAll: func() bool { return opts.DisablePortMapper || c.onlyTCP443.Load() }, - } - c.portMapper = portmapper.NewClient(logger.WithPrefix(c.logf, "portmapper: "), opts.NetMon, portMapOpts, opts.ControlKnobs, c.onPortMapChanged) - c.portMapper.SetGatewayLookupFunc(opts.NetMon.GatewayAndSelfIP) c.netMon = opts.NetMon c.health = opts.HealthTracker c.onPortUpdate = opts.OnPortUpdate @@ -554,7 +544,6 @@ func NewConn(opts Options) (*Conn, error) { NetMon: c.netMon, SendPacket: c.sendUDPNetcheck, SkipExternalNetwork: inTest(), - PortMapper: c.portMapper, UseDNSCache: true, } @@ -844,7 +833,7 @@ func (c *Conn) updateNetInfo(ctx context.Context) (*netcheck.Report, error) { UPnP: report.UPnP, PMP: report.PMP, PCP: report.PCP, - HavePortMap: c.portMapper.HaveMapping(), + HavePortMap: false, } for rid, d := range report.RegionV4Latency { ni.DERPLatency[fmt.Sprintf("%d-v4", rid)] = d.Seconds() @@ -1005,12 +994,6 @@ func (c *Conn) DiscoPublicKey() key.DiscoPublic { // // c.mu must NOT be held. func (c *Conn) determineEndpoints(ctx context.Context) ([]tailcfg.Endpoint, error) { - var havePortmap bool - var portmapExt netip.AddrPort - if runtime.GOOS != "js" { - portmapExt, havePortmap = c.portMapper.GetCachedMappingOrStartCreatingOne() - } - nr, err := c.updateNetInfo(ctx) if err != nil { c.logf("magicsock.Conn.determineEndpoints: updateNetInfo: %v", err) @@ -1046,15 +1029,6 @@ func (c *Conn) determineEndpoints(ctx context.Context) ([]tailcfg.Endpoint, erro } } - // If we didn't have a portmap earlier, maybe it's done by now. - if !havePortmap { - portmapExt, havePortmap = c.portMapper.GetCachedMappingOrStartCreatingOne() - } - if havePortmap { - addAddr(portmapExt, tailcfg.EndpointPortmapped) - c.setNetInfoHavePortMap() - } - v4Addrs, v6Addrs := nr.GetGlobalAddrs() for _, addr := range v4Addrs { addAddr(addr, tailcfg.EndpointSTUN) @@ -1975,7 +1949,6 @@ func (c *Conn) SetNetworkUp(up bool) { if up { c.startDerpHomeConnectLocked() } else { - c.portMapper.NoteNetworkDown() c.closeAllDerpLocked("network-down") } } @@ -2484,7 +2457,6 @@ func (c *Conn) Close() error { c.derpCleanupTimer.Stop() } c.stopPeriodicReSTUNTimerLocked() - c.portMapper.Close() c.peerMap.forEachEndpoint(func(ep *endpoint) { ep.stopAndReset() @@ -2739,7 +2711,6 @@ func (c *Conn) rebind(curPortFate currentPortFate) error { if err := c.bindSocket(&c.pconn4, "udp4", curPortFate); err != nil { return fmt.Errorf("magicsock: Rebind IPv4 failed: %w", err) } - c.portMapper.SetLocalPort(c.LocalPort()) c.UpdatePMTUD() return nil }