summaryrefslogtreecommitdiffhomepage
path: root/net
diff options
context:
space:
mode:
Diffstat (limited to 'net')
-rw-r--r--net/netcheck/netcheck.go7
-rw-r--r--net/portmapper/igd_test.go12
-rw-r--r--net/portmapper/portmapper.go77
-rw-r--r--net/portmapper/portmapper_test.go4
-rw-r--r--net/portmapper/portmappertype/portmappertype.go9
-rw-r--r--net/portmapper/upnp.go1
-rw-r--r--net/portmapper/upnp_test.go206
7 files changed, 276 insertions, 40 deletions
diff --git a/net/netcheck/netcheck.go b/net/netcheck/netcheck.go
index c5a3d2392..278f6a9f2 100644
--- a/net/netcheck/netcheck.go
+++ b/net/netcheck/netcheck.go
@@ -574,7 +574,6 @@ func nodeMight6(n *tailcfg.DERPNode) bool {
}
ip, _ := netip.ParseAddr(n.IPv6)
return ip.Is6()
-
}
// nodeMight4 reports whether n might reply to STUN over IPv4 based on
@@ -722,14 +721,14 @@ func (rs *reportState) setOptBool(b *opt.Bool, v bool) {
b.Set(v)
}
-func (rs *reportState) probePortMapServices() {
+func (rs *reportState) probePortMapServices(ctx context.Context) {
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())
+ res, err := rs.c.PortMapper.Probe(ctx)
if err != nil {
if !errors.Is(err, portmappertype.ErrGatewayRange) {
// "skipping portmap; gateway range likely lacks support"
@@ -900,7 +899,7 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap, opts *GetRe
if !c.SkipExternalNetwork && c.PortMapper != nil {
rs.waitPortMap.Add(1)
- go rs.probePortMapServices()
+ go rs.probePortMapServices(ctx)
}
var plan probePlan
diff --git a/net/portmapper/igd_test.go b/net/portmapper/igd_test.go
index 77015f5bf..33fc41efd 100644
--- a/net/portmapper/igd_test.go
+++ b/net/portmapper/igd_test.go
@@ -20,6 +20,7 @@ import (
"tailscale.com/tstest"
"tailscale.com/types/logger"
"tailscale.com/util/eventbus"
+ "tailscale.com/util/eventbus/eventbustest"
"tailscale.com/util/testenv"
)
@@ -213,7 +214,6 @@ func (d *TestIGD) handlePMPQuery(pkt []byte, src netip.AddrPort) {
return
}
d.inc(&d.counters.numPMPPublicAddrRecv)
-
}
// TODO
}
@@ -266,15 +266,15 @@ func (d *TestIGD) handlePCPQuery(pkt []byte, src netip.AddrPort) {
// A cleanup for the resulting client is also added to t.
func newTestClient(t *testing.T, igd *TestIGD, bus *eventbus.Bus) *Client {
if bus == nil {
- bus = eventbus.New()
+ bus = eventbustest.NewBus(t)
t.Log("Created empty event bus for test client")
- t.Cleanup(bus.Close)
}
var c *Client
c = NewClient(Config{
- Logf: tstest.WhileTestRunningLogger(t),
- NetMon: netmon.NewStatic(),
- EventBus: bus,
+ ShutdownCtx: t.Context(),
+ Logf: tstest.WhileTestRunningLogger(t),
+ NetMon: netmon.NewStatic(),
+ EventBus: bus,
OnChange: func() { // TODO(creachadair): Remove.
t.Logf("port map changed")
t.Logf("have mapping: %v", c.HaveMapping())
diff --git a/net/portmapper/portmapper.go b/net/portmapper/portmapper.go
index 16a981d1d..f3fa2b32b 100644
--- a/net/portmapper/portmapper.go
+++ b/net/portmapper/portmapper.go
@@ -31,6 +31,7 @@ import (
"tailscale.com/types/nettype"
"tailscale.com/util/clientmetric"
"tailscale.com/util/eventbus"
+ "tailscale.com/util/execqueue"
)
var (
@@ -40,6 +41,8 @@ var (
ErrPortMappingDisabled = portmappertype.ErrPortMappingDisabled
)
+var releaseTimeout = 10 * time.Second
+
var disablePortMapperEnv = envknob.RegisterBool("TS_DISABLE_PORTMAPPER")
// DebugKnobs contains debug configuration that can be provided when creating a
@@ -121,6 +124,7 @@ type Client struct {
debug DebugKnobs
testPxPPort uint16 // if non-zero, pxpPort to use for tests
testUPnPPort uint16 // if non-zero, uPnPPort to use for tests
+ shutdownCtx context.Context
mu syncs.Mutex // guards following, and all fields thereof
@@ -151,6 +155,8 @@ type Client struct {
localPort uint16
mapping mapping // non-nil if we have a mapping
+
+ actionQueue execqueue.ExecQueue
}
var _ portmappertype.Client = (*Client)(nil)
@@ -250,6 +256,9 @@ type Config struct {
// OnChange is called to run in a new goroutine whenever the port mapping
// status has changed. If nil, no callback is issued.
OnChange func()
+
+ // Context, which must be non-nil holds a shutdown context to derive other contexts from.
+ ShutdownCtx context.Context
}
// NewClient constructs a new portmapping [Client] from c. It will panic if any
@@ -262,9 +271,10 @@ func NewClient(c Config) *Client {
panic("nil EventBus")
}
ret := &Client{
- logf: c.Logf,
- netMon: c.NetMon,
- onChange: c.OnChange,
+ logf: c.Logf,
+ netMon: c.NetMon,
+ onChange: c.OnChange,
+ shutdownCtx: c.ShutdownCtx,
}
if buildfeatures.HasPortMapper {
// TODO(bradfitz): move this to method on netMon
@@ -391,13 +401,28 @@ func (c *Client) listenPacket(ctx context.Context, network, addr string) (nettyp
return pc.(*net.UDPConn), nil
}
-func (c *Client) invalidateMappingsLocked(releaseOld bool) {
+func (c *Client) releaseActiveMappingAsyncLocked(releaseOld bool) {
if c.mapping != nil {
+ mapping := c.mapping
+ c.mapping = nil
if releaseOld {
- c.mapping.Release(context.Background())
+ c.actionQueue.Add(func() {
+ ctx, cancel := context.WithTimeout(c.shutdownCtx, releaseTimeout)
+ defer cancel()
+ mapping.Release(ctx)
+ c.updates.Publish(portmappertype.Mapping{
+ External: mapping.External(),
+ GoodUntil: mapping.GoodUntil(),
+ Type: mapping.MappingType(),
+ Status: portmappertype.StatusRemovedFromGateway,
+ })
+ })
}
- c.mapping = nil
}
+}
+
+func (c *Client) invalidateMappingsLocked(releaseOld bool) {
+ c.releaseActiveMappingAsyncLocked(releaseOld)
c.pmpPubIP = netip.Addr{}
c.pmpPubIPTime = time.Time{}
@@ -499,12 +524,12 @@ func (c *Client) GetCachedMappingOrStartCreatingOne() (external netip.AddrPort,
func (c *Client) maybeStartMappingLocked() {
if !c.runningCreate {
c.runningCreate = true
- go c.createMapping()
+ c.actionQueue.Add(c.createMapping)
}
}
func (c *Client) createMapping() {
- ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+ ctx, cancel := context.WithTimeout(c.shutdownCtx, 5*time.Second)
defer cancel()
defer func() {
@@ -531,6 +556,7 @@ func (c *Client) createMapping() {
External: mapping.External(),
Type: mapping.MappingType(),
GoodUntil: mapping.GoodUntil(),
+ Status: portmappertype.StatusCreated,
})
// TODO(creachadair): Remove this entirely once there are no longer any
// places where the callback is set.
@@ -630,20 +656,28 @@ func (c *Client) createOrGetMapping(ctx context.Context) (mapping mapping, exter
prevPort = m.External().Port()
}
- if c.debug.DisablePCP() && c.debug.DisablePMP() {
- c.mu.Unlock()
- if external, ok := c.getUPnPPortMapping(ctx, gw, internalAddr, prevPort); ok {
- return nil, external, nil
- }
- c.vlogf("fallback to UPnP due to PCP and PMP being disabled failed")
- return nil, netip.AddrPort{}, NoMappingError{ErrNoPortMappingServices}
- }
+ pxpDisabled := c.debug.DisablePCP() && c.debug.DisablePMP()
// If we just did a Probe (e.g. via netchecker) but didn't
// find a PMP service, bail out early rather than probing
// again. Cuts down latency for most clients.
haveRecentPMP := c.sawPMPRecentlyLocked()
haveRecentPCP := c.sawPCPRecentlyLocked()
+ noRecentPXP := c.lastProbe.After(now.Add(-5*time.Second)) &&
+ !haveRecentPMP && !haveRecentPCP
+
+ if pxpDisabled || noRecentPXP {
+ c.mu.Unlock()
+ if external, ok := c.getUPnPPortMapping(ctx, gw, internalAddr, prevPort); ok {
+ return nil, external, nil
+ }
+ if pxpDisabled {
+ c.vlogf("fallback to UPnP due to PCP and PMP being disabled failed")
+ } else { // noRecentPXP
+ c.vlogf("fallback to UPnP due to no PCP and PMP failed")
+ }
+ return nil, netip.AddrPort{}, NoMappingError{ErrNoPortMappingServices}
+ }
// Since PMP mapping may require multiple calls, and it's not clear from the outset
// whether we're doing a PCP or PMP call, initialize the PMP mapping here,
@@ -659,15 +693,6 @@ func (c *Client) createOrGetMapping(ctx context.Context) (mapping mapping, exter
if haveRecentPMP {
m.external = netip.AddrPortFrom(c.pmpPubIP, m.external.Port())
}
- if c.lastProbe.After(now.Add(-5*time.Second)) && !haveRecentPMP && !haveRecentPCP {
- c.mu.Unlock()
- // fallback to UPnP portmapping
- if external, ok := c.getUPnPPortMapping(ctx, gw, internalAddr, prevPort); ok {
- return nil, external, nil
- }
- c.vlogf("fallback to UPnP due to no PCP and PMP failed")
- return nil, netip.AddrPort{}, NoMappingError{ErrNoPortMappingServices}
- }
c.mu.Unlock()
uc, err := c.listenPacket(ctx, "udp4", ":0")
@@ -888,7 +913,7 @@ func (c *Client) Probe(ctx context.Context) (res portmappertype.ProbeResult, err
}
}()
- uc, err := c.listenPacket(context.Background(), "udp4", ":0")
+ uc, err := c.listenPacket(ctx, "udp4", ":0")
if err != nil {
c.logf("ProbePCP: %v", err)
return res, err
diff --git a/net/portmapper/portmapper_test.go b/net/portmapper/portmapper_test.go
index a697a3908..a338f4596 100644
--- a/net/portmapper/portmapper_test.go
+++ b/net/portmapper/portmapper_test.go
@@ -102,7 +102,7 @@ func TestPCPIntegration(t *testing.T) {
defer igd.Close()
c := newTestClient(t, igd, nil)
- res, err := c.Probe(context.Background())
+ res, err := c.Probe(t.Context())
if err != nil {
t.Fatalf("probe failed: %v", err)
}
@@ -113,7 +113,7 @@ func TestPCPIntegration(t *testing.T) {
t.Fatalf("probe did not see pcp: %+v", res)
}
- _, external, err := c.createOrGetMapping(context.Background())
+ _, external, err := c.createOrGetMapping(t.Context())
if err != nil {
t.Fatalf("failed to get mapping: %v", err)
}
diff --git a/net/portmapper/portmappertype/portmappertype.go b/net/portmapper/portmappertype/portmappertype.go
index cc8358a4a..f3e0cea60 100644
--- a/net/portmapper/portmappertype/portmappertype.go
+++ b/net/portmapper/portmappertype/portmappertype.go
@@ -19,7 +19,8 @@ import (
// HookNewPortMapper is a hook to install the portmapper creation function.
// It must be set by an init function when buildfeatures.HasPortmapper is true.
-var HookNewPortMapper feature.Hook[func(logf logger.Logf,
+var HookNewPortMapper feature.Hook[func(ctx context.Context,
+ logf logger.Logf,
bus *eventbus.Bus,
netMon *netmon.Monitor,
disableUPnPOrNil,
@@ -82,7 +83,13 @@ type Client interface {
type Mapping struct {
External netip.AddrPort
Type string
+ Status string
GoodUntil time.Time
// TODO(creachadair): Record whether we reused an existing mapping?
}
+
+const (
+ StatusCreated = "StatusCreated"
+ StatusRemovedFromGateway = "StatusRemovedFromGateway"
+)
diff --git a/net/portmapper/upnp.go b/net/portmapper/upnp.go
index 34140e947..450cc5e00 100644
--- a/net/portmapper/upnp.go
+++ b/net/portmapper/upnp.go
@@ -110,6 +110,7 @@ func (u *upnpMapping) MappingDebug() string {
u.renewAfter.Unix(), u.goodUntil.Unix(),
u.loc)
}
+
func (u *upnpMapping) Release(ctx context.Context) {
u.client.DeletePortMappingCtx(ctx, "", u.external.Port(), upnpProtocolUDP)
}
diff --git a/net/portmapper/upnp_test.go b/net/portmapper/upnp_test.go
index a954b2bea..a92ce66ad 100644
--- a/net/portmapper/upnp_test.go
+++ b/net/portmapper/upnp_test.go
@@ -17,9 +17,11 @@ import (
"slices"
"sync/atomic"
"testing"
+ "time"
"tailscale.com/net/portmapper/portmappertype"
"tailscale.com/tstest"
+ "tailscale.com/util/eventbus/eventbustest"
)
// Google Wifi
@@ -628,6 +630,201 @@ func TestGetUPnPPortMapping(t *testing.T) {
}
}
+func TestReleaseUPnPPortMapping(t *testing.T) {
+ t.Run("GoodRouterReponse", func(t *testing.T) {
+ igd, err := NewTestIGD(t, TestIGDOptions{UPnP: true})
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer igd.Close()
+
+ // This is a very basic fake UPnP server handler.
+ var sawRequestWithLease atomic.Bool
+ handlers := map[string]any{
+ "AddPortMapping": func(body []byte) (int, string) {
+ // Decode a minimal body to determine whether we skip the request or not.
+ var req struct {
+ Protocol string `xml:"NewProtocol"`
+ InternalPort string `xml:"NewInternalPort"`
+ ExternalPort string `xml:"NewExternalPort"`
+ InternalClient string `xml:"NewInternalClient"`
+ LeaseDuration string `xml:"NewLeaseDuration"`
+ }
+ if err := xml.Unmarshal(body, &req); err != nil {
+ t.Errorf("bad request: %v", err)
+ return http.StatusBadRequest, "bad request"
+ }
+
+ if req.Protocol != "UDP" {
+ t.Errorf(`got Protocol=%q, want "UDP"`, req.Protocol)
+ }
+ if req.LeaseDuration != "0" {
+ // Return a fake error to ensure that we fall back to a permanent lease.
+ sawRequestWithLease.Store(true)
+ return http.StatusOK, testAddPortMappingPermanentLease
+ }
+
+ // Success!
+ return http.StatusOK, testAddPortMappingResponse
+ },
+ "GetExternalIPAddress": testGetExternalIPAddressResponse,
+ "GetStatusInfo": testGetStatusInfoResponse,
+ "DeletePortMapping": testDeletePortMappingResponse,
+ }
+
+ ctx := t.Context()
+
+ rootDesc := testRootDesc
+ igd.SetUPnPHandler(&upnpServer{
+ t: t,
+ Desc: rootDesc,
+ Control: map[string]map[string]any{
+ "/ctl/IPConn": handlers,
+ "/upnp/control/yomkmsnooi/wanipconn-1": handlers,
+ },
+ })
+
+ bus := eventbustest.NewBus(t)
+ c := newTestClient(t, igd, bus)
+ t.Logf("Listening on upnp=%v", c.testUPnPPort)
+
+ c.debug.VerboseLogs = true
+
+ var port uint16
+ sawRequestWithLease.Store(false)
+ mustProbeUPnP(t, ctx, c)
+
+ gw, myIP, ok := c.gatewayAndSelfIP()
+ if !ok {
+ t.Fatalf("could not get gateway and self IP")
+ }
+ t.Logf("gw=%v myIP=%v", gw, myIP)
+
+ ext, ok := c.getUPnPPortMapping(ctx, gw, netip.AddrPortFrom(myIP, 12345), port)
+ if !ok {
+ t.Fatal("could not get UPnP port mapping")
+ }
+ if got, want := ext.Addr(), netip.MustParseAddr("123.123.123.123"); got != want {
+ t.Errorf("bad external address; got %v want %v", got, want)
+ }
+ if !sawRequestWithLease.Load() {
+ t.Errorf("wanted request with lease, but didn't see one")
+ }
+ port = ext.Port()
+ t.Logf("external IP:port : %v:%v", ext, port)
+
+ tw := eventbustest.NewWatcher(t, bus)
+ c.mu.Lock()
+ c.invalidateMappingsLocked(true)
+ c.mu.Unlock()
+
+ err = eventbustest.Expect(tw, eventbustest.Type[portmappertype.Mapping]())
+ if err != nil {
+ t.Errorf("failed to release UPnP mapping: %v", err)
+ }
+ })
+
+ t.Run("NoRouterReponse", func(t *testing.T) {
+ igd, err := NewTestIGD(t, TestIGDOptions{UPnP: true})
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer igd.Close()
+ deleteWaitDoneCh := make(chan any)
+
+ // This is a very basic fake UPnP server handler.
+ var sawRequestWithLease atomic.Bool
+ handlers := map[string]any{
+ "AddPortMapping": func(body []byte) (int, string) {
+ // Decode a minimal body to determine whether we skip the request or not.
+ var req struct {
+ Protocol string `xml:"NewProtocol"`
+ InternalPort string `xml:"NewInternalPort"`
+ ExternalPort string `xml:"NewExternalPort"`
+ InternalClient string `xml:"NewInternalClient"`
+ LeaseDuration string `xml:"NewLeaseDuration"`
+ }
+ if err := xml.Unmarshal(body, &req); err != nil {
+ t.Errorf("bad request: %v", err)
+ return http.StatusBadRequest, "bad request"
+ }
+
+ if req.Protocol != "UDP" {
+ t.Errorf(`got Protocol=%q, want "UDP"`, req.Protocol)
+ }
+ if req.LeaseDuration != "0" {
+ // Return a fake error to ensure that we fall back to a permanent lease.
+ sawRequestWithLease.Store(true)
+ return http.StatusOK, testAddPortMappingPermanentLease
+ }
+
+ // Success!
+ return http.StatusOK, testAddPortMappingResponse
+ },
+ "GetExternalIPAddress": testGetExternalIPAddressResponse,
+ "GetStatusInfo": testGetStatusInfoResponse,
+ "DeletePortMapping": func(body []byte) (int, string) {
+ <-deleteWaitDoneCh
+ // return will happen after the end of the test, so just return OK
+ return http.StatusOK, ""
+ },
+ }
+
+ ctx := t.Context()
+
+ rootDesc := testRootDesc
+ igd.SetUPnPHandler(&upnpServer{
+ t: t,
+ Desc: rootDesc,
+ Control: map[string]map[string]any{
+ "/ctl/IPConn": handlers,
+ "/upnp/control/yomkmsnooi/wanipconn-1": handlers,
+ },
+ })
+
+ bus := eventbustest.NewBus(t)
+ c := newTestClient(t, igd, bus)
+ releaseTimeout = 10 * time.Millisecond
+ t.Logf("Listening on upnp=%v", c.testUPnPPort)
+
+ c.debug.VerboseLogs = true
+
+ var port uint16
+ sawRequestWithLease.Store(false)
+ mustProbeUPnP(t, ctx, c)
+
+ gw, myIP, ok := c.gatewayAndSelfIP()
+ if !ok {
+ t.Fatalf("could not get gateway and self IP")
+ }
+ t.Logf("gw=%v myIP=%v", gw, myIP)
+
+ ext, ok := c.getUPnPPortMapping(ctx, gw, netip.AddrPortFrom(myIP, 12345), port)
+ if !ok {
+ t.Fatal("could not get UPnP port mapping")
+ }
+ if got, want := ext.Addr(), netip.MustParseAddr("123.123.123.123"); got != want {
+ t.Errorf("bad external address; got %v want %v", got, want)
+ }
+ if !sawRequestWithLease.Load() {
+ t.Errorf("wanted request with lease, but didn't see one")
+ }
+ port = ext.Port()
+ t.Logf("external IP:port : %v:%v", ext, port)
+
+ tw := eventbustest.NewWatcher(t, bus)
+ c.mu.Lock()
+ c.invalidateMappingsLocked(true)
+ c.mu.Unlock()
+
+ err = eventbustest.Expect(tw, eventbustest.Type[portmappertype.Mapping]())
+ deleteWaitDoneCh <- nil
+ if err != nil {
+ t.Errorf("failed to release UPnP mapping: %v", err)
+ }
+ })
+}
+
func TestGetUPnPPortMapping_LeaseDuration(t *testing.T) {
testCases := []struct {
name string
@@ -639,7 +836,6 @@ func TestGetUPnPPortMapping_LeaseDuration(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
-
// This is a very basic fake UPnP server handler.
var sawRequestWithLease atomic.Bool
handlers := map[string]any{
@@ -1177,6 +1373,14 @@ const testGetStatusInfoResponse = `<?xml version="1.0"?>
</s:Envelope>
`
+const testDeletePortMappingResponse = `<?xml version="1.0"?>
+<s:Envelope xmlns:s="http://schemas.xmlsoap.org/soap/envelope/" s:encodingStyle="http://schemas.xmlsoap.org/soap/encoding/">
+ <s:Body><m:DeletePortMappingResponse xmlns:m="urn:schemas-upnp-org:service:WANIPConnection:1">
+ </m:DeletePortMappingResponse>
+ </s:Body>
+</s:Envelope>
+`
+
const testLegacyAddPortMappingResponse = `<?xml version="1.0"?>
<s:Envelope xmlns:s="http://schemas.xmlsoap.org/soap/envelope/" s:encodingStyle="http://schemas.xmlsoap.org/soap/encoding/">
<s:Body>