Skip to content

Commit

Permalink
cleanup nat addr handling; add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@sukunrt
sukunrt committed Dec 2, 2024
1 parent 567854e commit c930864
Show file tree
Hide file tree
Showing 2 changed files with 171 additions and 57 deletions.
130 changes: 73 additions & 57 deletions p2p/host/basic/address_service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@ type addressService struct {
ctx context.Context
ctxCancel context.CancelFunc

wg sync.WaitGroup
updateLocalIPv4Backoff backoff.ExpBackoff
updateLocalIPv6Backoff backoff.ExpBackoff
wg sync.WaitGroup

ifaceAddrs *interfaceAddrsCache
}
Expand Down Expand Up @@ -227,6 +225,7 @@ func (a *addressService) AllAddrs() []ma.Multiaddr {
}

filteredIfaceAddrs := a.ifaceAddrs.Filtered()
allIfaceAddrs := a.ifaceAddrs.All()

// Iterate over all _unresolved_ listen addresses, resolving our primary
// interface only to avoid advertising too many addresses.
Expand All @@ -238,7 +237,6 @@ func (a *addressService) AllAddrs() []ma.Multiaddr {
} else {
finalAddrs = append(finalAddrs, resolved...)
}

finalAddrs = ma.Unique(finalAddrs)

// use nat mappings if we have them
Expand All @@ -248,58 +246,10 @@ func (a *addressService) AllAddrs() []ma.Multiaddr {
// Next, apply this mapping to our addresses.
for _, listen := range listenAddrs {
extMaddr := a.natmgr.GetMapping(listen)
if extMaddr == nil {
// not mapped
continue
}

// if the router reported a sane address
if !manet.IsIPUnspecified(extMaddr) {
// Add in the mapped addr.
finalAddrs = append(finalAddrs, extMaddr)
} else {
log.Warn("NAT device reported an unspecified IP as it's external address")
}

// Did the router give us a routable public addr?
if manet.IsPublicAddr(extMaddr) {
// well done
continue
}

// No.
// in case the router gives us a wrong address or we're behind a double-NAT.
// also add observed addresses
resolved, err := manet.ResolveUnspecifiedAddress(listen, a.ifaceAddrs.All())
if err != nil {
// This can happen if we try to resolve /ip6/::/...
// without any IPv6 interface addresses.
continue
}

for _, addr := range resolved {
// Now, check if we have any observed addresses that
// differ from the one reported by the router. Routers
// don't always give the most accurate information.
observed := a.ids.ObservedAddrsFor(addr)

if len(observed) == 0 {
continue
}

// Drop the IP from the external maddr
_, extMaddrNoIP := ma.SplitFirst(extMaddr)

for _, obsMaddr := range observed {
// Extract a public observed addr.
ip, _ := ma.SplitFirst(obsMaddr)
if ip == nil || !manet.IsPublicAddr(ip) {
continue
}

finalAddrs = append(finalAddrs, ma.Join(ip, extMaddrNoIP))
}
}
finalAddrs = append(finalAddrs, appendNATAddrs(
finalAddrs, listen, extMaddr, a.ids.ObservedAddrsFor,
allIfaceAddrs,
)...)
}
} else {
var observedAddrs []ma.Multiaddr
Expand All @@ -310,7 +260,7 @@ func (a *addressService) AllAddrs() []ma.Multiaddr {
}
finalAddrs = ma.Unique(finalAddrs)
// Remove /p2p-circuit addresses from the list.
// The p2p-circuit tranport listener reports its address as just /p2p-circuit
// The p2p-circuit transport listener reports its address as just /p2p-circuit
// This is useless for dialing. Users need to manage their circuit addresses themselves,
// or use AutoRelay.
finalAddrs = slices.DeleteFunc(finalAddrs, func(a ma.Multiaddr) bool {
Expand Down Expand Up @@ -536,3 +486,69 @@ func (i *interfaceAddrsCache) updateUnlocked() {
}
}
}

func appendNATAddrs(result []ma.Multiaddr, listenAddr ma.Multiaddr, natMapping ma.Multiaddr,
obsAddrsFunc func(ma.Multiaddr) []ma.Multiaddr,
ifaceAddrs []ma.Multiaddr) []ma.Multiaddr {
if natMapping == nil {
// If the nat mapping fails, use the observed addrs
resolved, err := manet.ResolveUnspecifiedAddress(listenAddr, ifaceAddrs)
if err != nil {
log.Errorf("failed to resolve unspec addr %s, %s: %s", listenAddr, ifaceAddrs, err)
return result
}
allAddrs := append(resolved, listenAddr)
for _, a := range allAddrs {
result = append(result, obsAddrsFunc(a)...)
}
return result
}

// if the router reported a sane address, use it.
if !manet.IsIPUnspecified(natMapping) {
// Add in the mapped addr.
result = append(result, natMapping)
} else {
log.Warn("NAT device reported an unspecified IP as it's external address")
}

// If the router gave us a public address, use it ignoring observed addresses
if manet.IsPublicAddr(natMapping) {
return result
}

// No.
// in case the router gives us a wrong address or we're behind a double-NAT.
// also add observed addresses
resolved, err := manet.ResolveUnspecifiedAddress(listenAddr, ifaceAddrs)
if err != nil {
log.Errorf("failed to resolve unspec addr %s, %s: %s", listenAddr, ifaceAddrs, err)
return result
}
// Add the listen addr too. UDP observed addresses are mapped to unspecified listen addresses.
allAddrs := append(resolved, listenAddr)

// Drop the IP from the external maddr
_, extMaddrNoIP := ma.SplitFirst(natMapping)
if extMaddrNoIP == nil {
return result
}
for _, addr := range allAddrs {
// Now, check if we have any observed addresses.
observed := obsAddrsFunc(addr)
if len(observed) == 0 {
continue
}

for _, obsMaddr := range observed {
// Extract a public observed addr.
ip, _ := ma.SplitFirst(obsMaddr)
if ip == nil || !manet.IsPublicAddr(ip) {
continue
}

result = append(result, ma.Join(ip, extMaddrNoIP))
}
}
return result
}
98 changes: 98 additions & 0 deletions p2p/host/basic/address_service_test.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package basichost

import (
"testing"

ma "github.com/multiformats/go-multiaddr"
manet "github.com/multiformats/go-multiaddr/net"
"github.com/stretchr/testify/require"
)

func TestAppendNATAddrs(t *testing.T) {
if1, if2 := ma.StringCast("/ip4/192.168.0.100"), ma.StringCast("/ip4/1.1.1.1")
ifaceAddrs := []ma.Multiaddr{if1, if2}
tcpListenAddr, udpListenAddr := ma.StringCast("/ip4/0.0.0.0/tcp/1"), ma.StringCast("/ip4/0.0.0.0/udp/2/quic-v1")
cases := []struct {
Name string
Listen ma.Multiaddr
Nat ma.Multiaddr
ObsAddrFunc func(ma.Multiaddr) []ma.Multiaddr
Expected []ma.Multiaddr
}{
{
Name: "nat map success",
// nat mapping success, obsaddress ignored
Listen: ma.StringCast("/ip4/0.0.0.0/udp/1/quic-v1"),
Nat: ma.StringCast("/ip4/1.1.1.1/udp/10/quic-v1"),
ObsAddrFunc: func(m ma.Multiaddr) []ma.Multiaddr {
return []ma.Multiaddr{ma.StringCast("/ip4/2.2.2.2/udp/100/quic-v1")}
},
Expected: []ma.Multiaddr{ma.StringCast("/ip4/1.1.1.1/udp/10/quic-v1")},
},
{
Name: "nat map failure",
//nat mapping fails, obs addresses added
Listen: ma.StringCast("/ip4/0.0.0.0/tcp/1"),
Nat: nil,
ObsAddrFunc: func(a ma.Multiaddr) []ma.Multiaddr {
ip, _ := ma.SplitFirst(a)
if ip == nil {
return nil
}
if ip.Equal(if1) {
return []ma.Multiaddr{ma.StringCast("/ip4/2.2.2.2/tcp/100")}
} else {
return []ma.Multiaddr{ma.StringCast("/ip4/3.3.3.3/tcp/100")}
}
},
Expected: []ma.Multiaddr{ma.StringCast("/ip4/2.2.2.2/tcp/100"), ma.StringCast("/ip4/3.3.3.3/tcp/100")},
},
{
Name: "nat map success but CGNAT",
//nat addr added, obs address added with nat provided port
Listen: tcpListenAddr,
Nat: ma.StringCast("/ip4/100.100.1.1/tcp/100"),
ObsAddrFunc: func(a ma.Multiaddr) []ma.Multiaddr {
ip, _ := ma.SplitFirst(a)
if ip == nil {
return nil
}
if ip.Equal(if1) {
return []ma.Multiaddr{ma.StringCast("/ip4/2.2.2.2/tcp/20")}
} else {
return []ma.Multiaddr{ma.StringCast("/ip4/3.3.3.3/tcp/30")}
}
},
Expected: []ma.Multiaddr{
ma.StringCast("/ip4/100.100.1.1/tcp/100"),
ma.StringCast("/ip4/2.2.2.2/tcp/100"),
ma.StringCast("/ip4/3.3.3.3/tcp/100"),
},
},
{
Name: "uses unspecified address for obs address",
// observed address manager should be queries with both specified and unspecified addresses
// udp observed addresses are mapped to unspecified addresses
Listen: udpListenAddr,
Nat: nil,
ObsAddrFunc: func(a ma.Multiaddr) []ma.Multiaddr {
if manet.IsIPUnspecified(a) {
return []ma.Multiaddr{ma.StringCast("/ip4/3.3.3.3/udp/20/quic-v1")}
}
return []ma.Multiaddr{ma.StringCast("/ip4/2.2.2.2/udp/20/quic-v1")}
},
Expected: []ma.Multiaddr{
ma.StringCast("/ip4/2.2.2.2/udp/20/quic-v1"),
ma.StringCast("/ip4/3.3.3.3/udp/20/quic-v1"),
},
},
}
for _, tc := range cases {
t.Run(tc.Name, func(t *testing.T) {
res := appendNATAddrs(nil,
tc.Listen, tc.Nat, tc.ObsAddrFunc, ifaceAddrs)
res = ma.Unique(res)
require.ElementsMatch(t, tc.Expected, res, "%s\n%s", tc.Expected, res)
})
}
}

0 comments on commit c930864

Please sign in to comment.