net/portmapper: relax handling of UPnP resp (#6946)

Gateway devices operating as an HA pair w/VRRP or CARP may send UPnP
replies from static addresses rather than the floating gateway address.
This commit relaxes our source address verification such that we parse
responses from non-gateway IPs, and re-point the UPnP root desc
URL to the gateway IP. This ensures we are still interfacing with the
gateway device (assuming L2 security intact), even though we got a
root desc from a non-gateway address.

This relaxed handling is required for ANY port mapping to work on certain
OPNsense/pfsense distributions using CARP at the time of writing, as
miniupnpd may only listen on the static, non-gateway interface address
for PCP and PMP.

Fixes #5502

Signed-off-by: Jordan Whited <jordan@tailscale.com>
This commit is contained in:
Jordan Whited 2023-01-12 16:57:02 -08:00 committed by GitHub
parent b76dffa594
commit 25a0091f69
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 12 additions and 5 deletions

View File

@ -100,11 +100,11 @@ type mapping interface {
// but can be called asynchronously. Release should be idempotent, and thus even if called
// multiple times should not cause additional side-effects.
Release(context.Context)
// goodUntil will return the lease time that the mapping is valid for.
// GoodUntil will return the lease time that the mapping is valid for.
GoodUntil() time.Time
// renewAfter returns the earliest time that the mapping should be renewed.
// RenewAfter returns the earliest time that the mapping should be renewed.
RenewAfter() time.Time
// externalIPPort indicates what port the mapping can be reached from on the outside.
// External indicates what port the mapping can be reached from on the outside.
External() netip.AddrPort
}
@ -797,7 +797,11 @@ func (c *Client) Probe(ctx context.Context) (res ProbeResult, err error) {
switch port {
case c.upnpPort():
metricUPnPResponse.Add(1)
if ip == gw && mem.Contains(mem.B(buf[:n]), mem.S(":InternetGatewayDevice:")) {
if mem.Contains(mem.B(buf[:n]), mem.S(":InternetGatewayDevice:")) {
if ip != gw {
// https://github.com/tailscale/tailscale/issues/5502
c.logf("UPnP discovery response from %v, but gateway IP is %v", ip, gw)
}
meta, err := parseUPnPDiscoResponse(buf[:n])
if err != nil {
metricUPnPParseErr.Add(1)

View File

@ -14,6 +14,7 @@ import (
"context"
"fmt"
"math/rand"
"net"
"net/http"
"net/netip"
"net/url"
@ -174,8 +175,10 @@ func getUPnPClient(ctx context.Context, logf logger.Logf, gw netip.Addr, meta uP
return nil, fmt.Errorf("unexpected host %q in %q", u.Host, meta.Location)
}
if ipp.Addr() != gw {
return nil, fmt.Errorf("UPnP discovered root %q does not match gateway IP %v; ignoring UPnP",
// https://github.com/tailscale/tailscale/issues/5502
logf("UPnP discovered root %q does not match gateway IP %v; repointing at gateway which is assumed to be floating",
meta.Location, gw)
u.Host = net.JoinHostPort(gw.String(), u.Port())
}
// We're fetching a smallish XML document over plain HTTP