diff --git a/net/portmapper/upnp.go b/net/portmapper/upnp.go
index 67d9cbbab..0f44463e6 100644
--- a/net/portmapper/upnp.go
+++ b/net/portmapper/upnp.go
@@ -252,7 +252,8 @@ func getUPnPRootDevice(ctx context.Context, logf logger.Logf, debug DebugKnobs,
}
// selectBestService picks the "best" service from the given UPnP root device
-// to use to create a port mapping.
+// to use to create a port mapping. It may return (nil, nil) if no supported
+// service was found in the provided *goupnp.RootDevice.
//
// loc is the parsed location that was used to fetch the given RootDevice.
//
@@ -559,6 +560,20 @@ func (c *Client) tryUPnPPortmapWithDevice(
return netip.AddrPort{}, nil, err
}
+ // If we have no client, we cannot continue; this can happen if we get
+ // a valid UPnP response that does not contain any of the service types
+ // that we know how to use.
+ if client == nil {
+ // For debugging, print all available services that we aren't
+ // using because they're not supported; use c.vlogf so we don't
+ // spam the logs unless verbose debugging is turned on.
+ rootDev.Device.VisitServices(func(s *goupnp.Service) {
+ c.vlogf("unsupported UPnP service: Type=%q ID=%q ControlURL=%q", s.ServiceType, s.ServiceId, s.ControlURL.Str)
+ })
+
+ return netip.AddrPort{}, nil, fmt.Errorf("no supported UPnP clients")
+ }
+
// Start by trying to make a temporary lease with a duration.
var newPort uint16
newPort, err = addAnyPortMapping(
diff --git a/net/portmapper/upnp_test.go b/net/portmapper/upnp_test.go
index 8748cf427..4ca332ff1 100644
--- a/net/portmapper/upnp_test.go
+++ b/net/portmapper/upnp_test.go
@@ -165,6 +165,172 @@ const (
http://10.0.0.1:2828
+`
+
+ // Huawei, https://github.com/tailscale/tailscale/issues/10911
+ huaweiRootDescXML = `
+
+
+ 1
+ 0
+
+
+ urn:dslforum-org:device:InternetGatewayDevice:1
+ HG531 V1
+ Huawei Technologies Co., Ltd.
+ http://www.huawei.com
+ Huawei Home Gateway
+ HG531 V1
+ Huawei Model
+ http://www.huawei.com
+ G6J8W15326003974
+ uuid:00e0fc37-2626-2828-2600-587f668bdd9a
+ 000000000001
+
+
+ urn:www-huawei-com:service:DeviceConfig:1
+ urn:www-huawei-com:serviceId:DeviceConfig1
+ /desc/DevCfg.xml
+ /ctrlt/DeviceConfig_1
+ /evt/DeviceConfig_1
+
+
+ urn:dslforum-org:service:LANConfigSecurity:1
+ urn:dslforum-org:serviceId:LANConfigSecurity1
+ /desc/LANSec.xml
+ /ctrlt/LANConfigSecurity_1
+ /evt/LANConfigSecurity_1
+
+
+ urn:dslforum-org:service:Layer3Forwarding:1
+ urn:dslforum-org:serviceId:Layer3Forwarding1
+ /desc/L3Fwd.xml
+ /ctrlt/Layer3Forwarding_1
+ /evt/Layer3Forwarding_1
+
+
+
+
+ urn:dslforum-org:device:WANDevice:1
+ WANDevice
+ Huawei Technologies Co., Ltd.
+ http://www.huawei.com
+ Huawei Home Gateway
+ HG531 V1
+ Huawei Model
+ http://www.huawei.com
+ G6J8W15326003974
+ uuid:00e0fc37-2626-2828-2601-587f668bdd9a
+ 000000000001
+
+
+ urn:dslforum-org:service:WANDSLInterfaceConfig:1
+ urn:dslforum-org:serviceId:WANDSLInterfaceConfig1
+ /desc/WanDslIfCfg.xml
+ /ctrlt/WANDSLInterfaceConfig_1
+ /evt/WANDSLInterfaceConfig_1
+
+
+ urn:dslforum-org:service:WANCommonInterfaceConfig:1
+ urn:dslforum-org:serviceId:WANCommonInterfaceConfig1
+ /desc/WanCommonIfc1.xml
+ /ctrlt/WANCommonInterfaceConfig_1
+ /evt/WANCommonInterfaceConfig_1
+
+
+
+
+ urn:dslforum-org:device:WANConnectionDevice:1
+ WANConnectionDevice
+ Huawei Technologies Co., Ltd.
+ http://www.huawei.com
+ Huawei Home Gateway
+ HG531 V1
+ Huawei Model
+ http://www.huawei.com
+ G6J8W15326003974
+ uuid:00e0fc37-2626-2828-2603-587f668bdd9a
+ 000000000001
+
+
+ urn:dslforum-org:service:WANPPPConnection:1
+ urn:dslforum-org:serviceId:WANPPPConnection1
+ /desc/WanPppConn.xml
+ /ctrlt/WANPPPConnection_1
+ /evt/WANPPPConnection_1
+
+
+ urn:dslforum-org:service:WANEthernetConnectionManagement:1
+ urn:dslforum-org:serviceId:WANEthernetConnectionManagement1
+ /desc/WanEthConnMgt.xml
+ /ctrlt/WANEthernetConnectionManagement_1
+ /evt/WANEthernetConnectionManagement_1
+
+
+ urn:dslforum-org:service:WANDSLLinkConfig:1
+ urn:dslforum-org:serviceId:WANDSLLinkConfig1
+ /desc/WanDslLink.xml
+ /ctrlt/WANDSLLinkConfig_1
+ /evt/WANDSLLinkConfig_1
+
+
+
+
+
+
+ urn:dslforum-org:device:LANDevice:1
+ LANDevice
+ Huawei Technologies Co., Ltd.
+ http://www.huawei.com
+ Huawei Home Gateway
+ HG531 V1
+ Huawei Model
+ http://www.huawei.com
+ G6J8W15326003974
+ uuid:00e0fc37-2626-2828-2602-587f668bdd9a
+ 000000000001
+
+
+ urn:dslforum-org:service:WLANConfiguration:1
+ urn:dslforum-org:serviceId:WLANConfiguration4
+ /desc/WLANCfg.xml
+ /ctrlt/WLANConfiguration_4
+ /evt/WLANConfiguration_4
+
+
+ urn:dslforum-org:service:WLANConfiguration:1
+ urn:dslforum-org:serviceId:WLANConfiguration3
+ /desc/WLANCfg.xml
+ /ctrlt/WLANConfiguration_3
+ /evt/WLANConfiguration_3
+
+
+ urn:dslforum-org:service:WLANConfiguration:1
+ urn:dslforum-org:serviceId:WLANConfiguration2
+ /desc/WLANCfg.xml
+ /ctrlt/WLANConfiguration_2
+ /evt/WLANConfiguration_2
+
+
+ urn:dslforum-org:service:WLANConfiguration:1
+ urn:dslforum-org:serviceId:WLANConfiguration1
+ /desc/WLANCfg.xml
+ /ctrlt/WLANConfiguration_1
+ /evt/WLANConfiguration_1
+
+
+ urn:dslforum-org:service:LANHostConfigManagement:1
+ urn:dslforum-org:serviceId:LANHostConfigManagement1
+ /desc/LanHostCfgMgmt.xml
+ /ctrlt/LANHostConfigManagement_1
+ /evt/LANHostConfigManagement_1
+
+
+
+
+ http://127.0.0.1
+
+
`
)
@@ -233,6 +399,14 @@ func TestGetUPnPClient(t *testing.T) {
"*internetgateway2.WANIPConnection1",
"saw UPnP type WANIPConnection1 at http://127.0.0.1:NNN/rootDesc.xml; MikroTik Router (MikroTik), method=none\n",
},
+ {
+ "huawei",
+ huaweiRootDescXML,
+ // services not supported and thus returns nil, but shouldn't crash
+ "",
+ "",
+ },
+
// TODO(bradfitz): find a PPP one in the wild
}
for _, tt := range tests {
@@ -375,6 +549,48 @@ func TestGetUPnPPortMapping(t *testing.T) {
}
}
+// TestGetUPnPPortMapping_NoValidServices tests that getUPnPPortMapping doesn't
+// crash when a valid UPnP response with no supported services is discovered
+// and parsed.
+//
+// See https://github.com/tailscale/tailscale/issues/10911
+func TestGetUPnPPortMapping_NoValidServices(t *testing.T) {
+ igd, err := NewTestIGD(t.Logf, TestIGDOptions{UPnP: true})
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer igd.Close()
+
+ igd.SetUPnPHandler(&upnpServer{
+ t: t,
+ Desc: huaweiRootDescXML,
+ })
+
+ c := newTestClient(t, igd)
+ defer c.Close()
+ c.debug.VerboseLogs = true
+
+ ctx := context.Background()
+ res, err := c.Probe(ctx)
+ if err != nil {
+ t.Fatalf("Probe: %v", err)
+ }
+ if !res.UPnP {
+ t.Errorf("didn't detect UPnP")
+ }
+
+ gw, myIP, ok := c.gatewayAndSelfIP()
+ if !ok {
+ t.Fatalf("could not get gateway and self IP")
+ }
+
+ // This shouldn't panic
+ _, ok = c.getUPnPPortMapping(ctx, gw, netip.AddrPortFrom(myIP, 12345), 0)
+ if ok {
+ t.Fatal("did not expect to get UPnP port mapping")
+ }
+}
+
func TestGetUPnPPortMappingNoResponses(t *testing.T) {
igd, err := NewTestIGD(t.Logf, TestIGDOptions{UPnP: true})
if err != nil {