From 4c64843d602ed1c557139e294753319b03bc6fdf Mon Sep 17 00:00:00 2001 From: Marcus B Spencer Date: Thu, 19 Jun 2025 23:24:45 -0500 Subject: [PATCH] feat(connections, nat): add UDP portmapping/pinhole for QUIC (fixes #7403) (#10171) Fixes #7403. Tested by enabling UPnP on the router, and checking on the router page that the external ports of the UDP mappings match what is shown in the logs and the internal ports matching the QUIC listening port. --- lib/connections/quic_listen.go | 31 +++++++++++++++++++++++++++---- lib/connections/tcp_listen.go | 19 ++----------------- lib/connections/util.go | 25 +++++++++++++++++++++++++ lib/nat/service.go | 12 ++++++------ 4 files changed, 60 insertions(+), 27 deletions(-) diff --git a/lib/connections/quic_listen.go b/lib/connections/quic_listen.go index 2418a0404..3868fd92c 100644 --- a/lib/connections/quic_listen.go +++ b/lib/connections/quic_listen.go @@ -49,9 +49,11 @@ type quicListener struct { registry *registry.Registry lanChecker *lanChecker - address *url.URL - laddr net.Addr - mut sync.Mutex + address *url.URL + natService *nat.Service + mapping *nat.Mapping + laddr net.Addr + mut sync.Mutex } func (t *quicListener) OnNATTypeChanged(natType stun.NATType) { @@ -126,7 +128,24 @@ func (t *quicListener) serve(ctx context.Context) error { l.Infof("QUIC listener (%v) starting", udpConn.LocalAddr()) defer l.Infof("QUIC listener (%v) shutting down", udpConn.LocalAddr()) + var ipVersion nat.IPVersion + switch t.uri.Scheme { + case "quic4": + ipVersion = nat.IPv4Only + case "quic6": + ipVersion = nat.IPv6Only + default: + ipVersion = nat.IPvAny + } + mapping := t.natService.NewMapping(nat.UDP, ipVersion, udpAddr.IP, udpAddr.Port) + mapping.OnChanged(func() { + t.notifyAddressesChanged(t) + }) + // Should be called after t.mapping is nil'ed out. + defer t.natService.RemoveMapping(mapping) + t.mut.Lock() + t.mapping = mapping t.laddr = udpConn.LocalAddr() t.mut.Unlock() defer func() { @@ -196,6 +215,9 @@ func (t *quicListener) WANAddresses() []*url.URL { if t.address != nil { uris = append(uris, t.address) } + + uris = append(uris, portMappingURIs(t.mapping, *t.uri)...) + t.mut.Unlock() return uris } @@ -232,12 +254,13 @@ func (*quicListenerFactory) Valid(config.Configuration) error { return nil } -func (f *quicListenerFactory) New(uri *url.URL, cfg config.Wrapper, tlsCfg *tls.Config, conns chan internalConn, _ *nat.Service, registry *registry.Registry, lanChecker *lanChecker) genericListener { +func (f *quicListenerFactory) New(uri *url.URL, cfg config.Wrapper, tlsCfg *tls.Config, conns chan internalConn, natService *nat.Service, registry *registry.Registry, lanChecker *lanChecker) genericListener { l := &quicListener{ uri: fixupPort(uri, config.DefaultQUICPort), cfg: cfg, tlsCfg: tlsCfg, conns: conns, + natService: natService, factory: f, registry: registry, lanChecker: lanChecker, diff --git a/lib/connections/tcp_listen.go b/lib/connections/tcp_listen.go index 2d0c2fc98..d487b5f20 100644 --- a/lib/connections/tcp_listen.go +++ b/lib/connections/tcp_listen.go @@ -175,24 +175,9 @@ func (t *tcpListener) WANAddresses() []*url.URL { uris := []*url.URL{ maybeReplacePort(t.uri, t.laddr), } - if t.mapping != nil { - addrs := t.mapping.ExternalAddresses() - for _, addr := range addrs { - uri := *t.uri - // Does net.JoinHostPort internally - uri.Host = addr.String() - uris = append(uris, &uri) - // For every address with a specified IP, add one without an IP, - // just in case the specified IP is still internal (router behind DMZ). - if len(addr.IP) != 0 && !addr.IP.IsUnspecified() { - zeroUri := *t.uri - addr.IP = nil - zeroUri.Host = addr.String() - uris = append(uris, &zeroUri) - } - } - } + uris = append(uris, portMappingURIs(t.mapping, *t.uri)...) + t.mut.RUnlock() // If we support ReusePort, add an unspecified zero port address, which will be resolved by the discovery server diff --git a/lib/connections/util.go b/lib/connections/util.go index a14b96475..eeabc3c94 100644 --- a/lib/connections/util.go +++ b/lib/connections/util.go @@ -12,6 +12,7 @@ import ( "strconv" "strings" + "github.com/syncthing/syncthing/lib/nat" "github.com/syncthing/syncthing/lib/osutil" ) @@ -130,3 +131,27 @@ func maybeReplacePort(uri *url.URL, laddr net.Addr) *url.URL { uriCopy.Host = net.JoinHostPort(host, lportStr) return &uriCopy } + +func portMappingURIs(mapping *nat.Mapping, listener_uri url.URL) []*url.URL { + var uris []*url.URL + if mapping != nil { + addrs := mapping.ExternalAddresses() + for _, addr := range addrs { + uri := listener_uri + // Does net.JoinHostPort internally + uri.Host = addr.String() + uris = append(uris, &uri) + + // For every address with a specified IP, add one without an IP, + // just in case the specified IP is still internal (router behind DMZ). + if len(addr.IP) != 0 && !addr.IP.IsUnspecified() { + zeroUri := listener_uri + addr.IP = nil + zeroUri.Host = addr.String() + uris = append(uris, &zeroUri) + } + } + } + + return uris +} diff --git a/lib/nat/service.go b/lib/nat/service.go index 2db3f1a08..9a196abf0 100644 --- a/lib/nat/service.go +++ b/lib/nat/service.go @@ -257,7 +257,7 @@ func (s *Service) verifyExistingLocked(ctx context.Context, mapping *Mapping, na // extAddrs either contains one IPv4 address, or possibly several // IPv6 addresses all using the same port. Therefore the first // entry always has the external port. - responseAddrs, err := s.tryNATDevice(ctx, nat, mapping.address, extAddrs[0].Port, leaseTime) + responseAddrs, err := s.tryNATDevice(ctx, nat, mapping.address, extAddrs[0].Port, mapping.protocol, leaseTime) if err != nil { l.Infof("Failed to renew %s -> %v open port on %s: %s", mapping, extAddrs, id, err) mapping.removeAddressLocked(id) @@ -309,7 +309,7 @@ func (s *Service) acquireNewLocked(ctx context.Context, mapping *Mapping, nats m continue } - addrs, err := s.tryNATDevice(ctx, nat, mapping.address, 0, leaseTime) + addrs, err := s.tryNATDevice(ctx, nat, mapping.address, 0, mapping.protocol, leaseTime) if err != nil { l.Infof("Failed to acquire %s open port on %s: %s", mapping, id, err) continue @@ -325,14 +325,14 @@ func (s *Service) acquireNewLocked(ctx context.Context, mapping *Mapping, nats m // tryNATDevice tries to acquire a port mapping for the given internal address to // the given external port. If external port is 0, picks a pseudo-random port. -func (s *Service) tryNATDevice(ctx context.Context, natd Device, intAddr Address, extPort int, leaseTime time.Duration) ([]Address, error) { +func (s *Service) tryNATDevice(ctx context.Context, natd Device, intAddr Address, extPort int, protocol Protocol, leaseTime time.Duration) ([]Address, error) { var err error var port int // For IPv6, we just try to create the pinhole. If it fails, nothing can be done (probably no IGDv2 support). // If it already exists, the relevant UPnP standard requires that the gateway recognizes this and updates the lease time. // Since we usually have a global unicast IPv6 address so no conflicting mappings, we just request the port we're running on if natd.SupportsIPVersion(IPv6Only) { - ipaddrs, err := natd.AddPinhole(ctx, TCP, intAddr, leaseTime) + ipaddrs, err := natd.AddPinhole(ctx, protocol, intAddr, leaseTime) var addrs []Address for _, ipaddr := range ipaddrs { addrs = append(addrs, Address{ @@ -354,7 +354,7 @@ func (s *Service) tryNATDevice(ctx context.Context, natd Device, intAddr Address if extPort != 0 { // First try renewing our existing mapping, if we have one. name := fmt.Sprintf("syncthing-%d", extPort) - port, err = natd.AddPortMapping(ctx, TCP, intAddr.Port, extPort, name, leaseTime) + port, err = natd.AddPortMapping(ctx, protocol, intAddr.Port, extPort, name, leaseTime) if err == nil { extPort = port goto findIP @@ -372,7 +372,7 @@ func (s *Service) tryNATDevice(ctx context.Context, natd Device, intAddr Address // Then try up to ten random ports. extPort = 1024 + predictableRand.Intn(65535-1024) name := fmt.Sprintf("syncthing-%d", extPort) - port, err = natd.AddPortMapping(ctx, TCP, intAddr.Port, extPort, name, leaseTime) + port, err = natd.AddPortMapping(ctx, protocol, intAddr.Port, extPort, name, leaseTime) if err == nil { extPort = port goto findIP