TheArchive/tailscale
1
0
Fork 0
mirror of https://github.com/tailscale/tailscale.git synced 2024-11-25 19:15:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

net/portmapper: don't return unspecified/local external IPs

Browse Source 
We were previously not checking that the external IP that we got back
from a UPnP portmap was a valid endpoint; add minimal validation that
this endpoint is something that is routeable by another host.

Updates tailscale/corp#23538

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Id9649e7683394aced326d5348f4caa24d0efd532
This commit is contained in:
Andrew Dunham 2024-10-01 12:17:59 -04:00
parent 1eaad7d3de
commit 16ef88754d
2 changed files with 92 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
net/portmapper/upnp.go
View File

@ -638,6 +638,19 @@ func (c *Client) tryUPnPPortmapWithDevice(
return netip.AddrPort{}, nil, err
}
// Do a bit of validation on the external IP; we've seen cases where
// UPnP devices return the public IP 0.0.0.0, which obviously doesn't
// work as an endpoint.
//
// See: https://github.com/tailscale/corp/issues/23538
if externalIP.IsUnspecified() {
c.logf("UPnP returned unspecified external IP %v", externalIP)
return netip.AddrPort{}, nil, fmt.Errorf("UPnP returned unspecified external IP")
} else if externalIP.IsLoopback() {
c.logf("UPnP returned loopback external IP %v", externalIP)
return netip.AddrPort{}, nil, fmt.Errorf("UPnP returned loopback external IP")
}
return netip.AddrPortFrom(externalIP, newPort), client, nil
}

100
net/portmapper/upnp_test.go
View File

@ -599,13 +599,7 @@ func TestGetUPnPPortMapping(t *testing.T) {
)
for i := range 2 {
sawRequestWithLease.Store(false)
res, err := c.Probe(ctx)
if err != nil {
t.Fatalf("Probe: %v", err)
}
if !res.UPnP {
t.Errorf("didn't detect UPnP")
}
mustProbeUPnP(t, ctx, c)
gw, myIP, ok := c.gatewayAndSelfIP()
if !ok {
@ -656,13 +650,7 @@ func TestGetUPnPPortMapping_NoValidServices(t *testing.T) {
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")
}
mustProbeUPnP(t, ctx, c)
gw, myIP, ok := c.gatewayAndSelfIP()
if !ok {
@ -705,13 +693,7 @@ func TestGetUPnPPortMapping_Legacy(t *testing.T) {
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")
}
mustProbeUPnP(t, ctx, c)
gw, myIP, ok := c.gatewayAndSelfIP()
if !ok {
@ -838,6 +820,58 @@ func TestProcessUPnPResponses(t *testing.T) {
}
}
// See: https://github.com/tailscale/corp/issues/23538
func TestGetUPnPPortMapping_Invalid(t *testing.T) {
for _, responseAddr := range []string{
"0.0.0.0",
"127.0.0.1",
} {
t.Run(responseAddr, func(t *testing.T) {
igd, err := NewTestIGD(t.Logf, TestIGDOptions{UPnP: true})
if err != nil {
t.Fatal(err)
}
defer igd.Close()
// This is a very basic fake UPnP server handler.
handlers := map[string]any{
"AddPortMapping": testAddPortMappingResponse,
"GetExternalIPAddress": makeGetExternalIPAddressResponse(responseAddr),
"GetStatusInfo": testGetStatusInfoResponse,
"DeletePortMapping": "", // Do nothing for test
}
igd.SetUPnPHandler(&upnpServer{
t: t,
Desc: huaweiRootDescXML,
Control: map[string]map[string]any{
"/ctrlt/WANPPPConnection_1": handlers,
},
})
c := newTestClient(t, igd)
defer c.Close()
c.debug.VerboseLogs = true
ctx := context.Background()
mustProbeUPnP(t, ctx, c)
gw, myIP, ok := c.gatewayAndSelfIP()
if !ok {
t.Fatalf("could not get gateway and self IP")
}
ext, ok := c.getUPnPPortMapping(ctx, gw, netip.AddrPortFrom(myIP, 12345), 0)
if ok {
t.Fatal("did not expect to get UPnP port mapping")
}
if ext.IsValid() {
t.Fatalf("expected no external address; got %v", ext)
}
})
}
}
type upnpServer struct {
t *testing.T
Desc string // root device XML
@ -921,6 +955,18 @@ func (u *upnpServer) handleControl(w http.ResponseWriter, r *http.Request, handl
}
}
func mustProbeUPnP(tb testing.TB, ctx context.Context, c *Client) ProbeResult {
tb.Helper()
res, err := c.Probe(ctx)
if err != nil {
tb.Fatalf("Probe: %v", err)
}
if !res.UPnP {
tb.Fatalf("didn't detect UPnP")
}
return res
}
const testRootDesc = `<?xml version="1.0"?>
<root xmlns="urn:schemas-upnp-org:device-1-0" configId="1337">
<specVersion>
@ -1058,3 +1104,15 @@ func (u *upnpServer) handleControl(w http.ResponseWriter, r *http.Request, handl
</s:Body>
</s:Envelope>
`
func makeGetExternalIPAddressResponse(ip string) string {
return fmt.Sprintf(`<?xml version="1.0"?>
<s:Envelope xmlns:s="http://schemas.xmlsoap.org/soap/envelope/" s:encodingStyle="http://schemas.xmlsoap.org/soap/encoding/">
<s:Body>
<u:GetExternalIPAddressResponse xmlns:u="urn:schemas-upnp-org:service:WANIPConnection:1">
<NewExternalIPAddress>%s</NewExternalIPAddress>
</u:GetExternalIPAddressResponse>
</s:Body>
</s:Envelope>
`, ip)
}