From bed6dcaae5cc081a35c8cba4d1cb0ca3474e50b6 Mon Sep 17 00:00:00 2001 From: sukun Date: Fri, 9 Jun 2023 12:58:08 +0530 Subject: [PATCH 1/3] nat: add HasNAT method for checking NAT environments --- p2p/host/basic/basic_host.go | 5 ++--- p2p/host/basic/natmgr.go | 7 +++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/p2p/host/basic/basic_host.go b/p2p/host/basic/basic_host.go index 431137911e..557bc4e78c 100644 --- a/p2p/host/basic/basic_host.go +++ b/p2p/host/basic/basic_host.go @@ -843,11 +843,10 @@ func (h *BasicHost) AllAddrs() []ma.Multiaddr { finalAddrs = network.DedupAddrs(finalAddrs) - // natmgr is nil if we do not use nat option; - if h.natmgr != nil { + // use nat mappings if we have them + if h.natmgr != nil && h.natmgr.HasNAT() { // We have successfully mapped ports on our NAT. Use those // instead of observed addresses (mostly). - // Next, apply this mapping to our addresses. for _, listen := range listenAddrs { extMaddr := h.natmgr.GetMapping(listen) diff --git a/p2p/host/basic/natmgr.go b/p2p/host/basic/natmgr.go index 120e2b826d..ea9febf200 100644 --- a/p2p/host/basic/natmgr.go +++ b/p2p/host/basic/natmgr.go @@ -21,6 +21,7 @@ import ( // and tries to obtain port mappings for those. type NATManager interface { GetMapping(ma.Multiaddr) ma.Multiaddr + HasNAT() bool io.Closer } @@ -86,6 +87,12 @@ func (nmgr *natManager) Close() error { return nil } +func (nmgr *natManager) HasNAT() bool { + nmgr.natMx.RLock() + defer nmgr.natMx.RUnlock() + return nmgr.nat != nil +} + func (nmgr *natManager) background(ctx context.Context) { defer nmgr.refCount.Done() From ef757859b63b298dcff477693282816c596a5f37 Mon Sep 17 00:00:00 2001 From: sukun Date: Fri, 9 Jun 2023 18:33:06 +0530 Subject: [PATCH 2/3] use observed addresses in case we have a nat and no mappings --- p2p/host/basic/basic_host.go | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/p2p/host/basic/basic_host.go b/p2p/host/basic/basic_host.go index 557bc4e78c..aa3280a3f3 100644 --- a/p2p/host/basic/basic_host.go +++ b/p2p/host/basic/basic_host.go @@ -845,13 +845,12 @@ func (h *BasicHost) AllAddrs() []ma.Multiaddr { // use nat mappings if we have them if h.natmgr != nil && h.natmgr.HasNAT() { - // We have successfully mapped ports on our NAT. Use those - // instead of observed addresses (mostly). - // Next, apply this mapping to our addresses. + // Use nat mappings for our addresses, if we have them. + var unmapped []ma.Multiaddr for _, listen := range listenAddrs { extMaddr := h.natmgr.GetMapping(listen) if extMaddr == nil { - // not mapped + unmapped = append(unmapped, listen) continue } @@ -870,16 +869,18 @@ func (h *BasicHost) AllAddrs() []ma.Multiaddr { } // 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, allIfaceAddrs) + // In case the router gives us a wrong address or we're behind a double-NAT, + // also add observed addresses. Check for both the listen address + // and the resolved addresses. It is important to check for the listen address + // as the local address on connections for some transports is unspecified. + addrs, err := manet.ResolveUnspecifiedAddress(listen, allIfaceAddrs) if err != nil { // This can happen if we try to resolve /ip6/::/... // without any IPv6 interface addresses. continue } - - for _, addr := range resolved { + addrs = append(addrs, listen) + for _, addr := range addrs { // 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. @@ -903,6 +904,19 @@ func (h *BasicHost) AllAddrs() []ma.Multiaddr { } } } + // Add observed addresses for umapped addresses + for _, listen := range unmapped { + addrs, err := manet.ResolveUnspecifiedAddress(listen, allIfaceAddrs) + if err != nil { + // This can happen if we try to resolve /ip6/::/... + // without any IPv6 interface addresses. + continue + } + addrs = append(addrs, listen) + for _, addr := range addrs { + finalAddrs = append(finalAddrs, h.ids.ObservedAddrsFor(addr)...) + } + } } else { var observedAddrs []ma.Multiaddr if h.ids != nil { From 203b54fe93b66f696af6197d2e468c2402b97406 Mon Sep 17 00:00:00 2001 From: sukun Date: Fri, 9 Jun 2023 23:53:32 +0530 Subject: [PATCH 3/3] add tests --- p2p/host/basic/basic_host_test.go | 91 +++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/p2p/host/basic/basic_host_test.go b/p2p/host/basic/basic_host_test.go index 5c1babd9d5..7468ee22ad 100644 --- a/p2p/host/basic/basic_host_test.go +++ b/p2p/host/basic/basic_host_test.go @@ -19,9 +19,12 @@ import ( "github.com/libp2p/go-libp2p/core/protocol" "github.com/libp2p/go-libp2p/core/record" "github.com/libp2p/go-libp2p/p2p/host/autonat" + blhost "github.com/libp2p/go-libp2p/p2p/host/blank" "github.com/libp2p/go-libp2p/p2p/host/eventbus" swarmt "github.com/libp2p/go-libp2p/p2p/net/swarm/testing" "github.com/libp2p/go-libp2p/p2p/protocol/identify" + "github.com/libp2p/go-libp2p/p2p/protocol/identify/pb" + "github.com/libp2p/go-msgio/pbio" ma "github.com/multiformats/go-multiaddr" @@ -241,6 +244,94 @@ func TestAllAddrs(t *testing.T) { require.True(t, ma.Contains(h.AllAddrs(), firstAddr), "should still contain the original addr") } +func TestAllAddrsIdentify(t *testing.T) { + oldThreshold := identify.ActivationThresh + identify.ActivationThresh = 1 + defer func() { identify.ActivationThresh = oldThreshold }() + + h1, err := NewHost(swarmt.GenSwarm(t, swarmt.OptDialOnly), nil) + require.NoError(t, err) + defer h1.Close() + laddr := ma.StringCast("/ip4/0.0.0.0/udp/0/quic-v1") + require.NoError(t, h1.Network().Listen(laddr)) + + h1.IDService().Start() + defer h1.IDService().Close() + + var wg sync.WaitGroup + wg.Add(1) + // inform the host that its address is 1.2.3.4:1234 + obsAddr := ma.StringCast("/ip4/1.2.3.4/udp/1234/quic-v1") + idHandler := func(s network.Stream) { + msg := &pb.Identify{} + msg.ObservedAddr = obsAddr.Bytes() + writer := pbio.NewDelimitedWriter(s) + if err := writer.WriteMsg(msg); err != nil { + t.Fatal(err) + } + s.Close() + wg.Done() + } + h2 := blhost.NewBlankHost(swarmt.GenSwarm(t)) + h2.SetStreamHandler(identify.ID, idHandler) + + err = h2.Connect(context.Background(), peer.AddrInfo{ID: h1.ID(), Addrs: h1.Addrs()}) + require.NoError(t, err) + wg.Wait() + + require.Eventually(t, + func() bool { return ma.Contains(h1.AllAddrs(), obsAddr) }, + 5*time.Second, + 100*time.Millisecond, + ) +} + +func TestAllAddrsNAT(t *testing.T) { + oldThreshold := identify.ActivationThresh + identify.ActivationThresh = 1 + defer func() { identify.ActivationThresh = oldThreshold }() + + h1, err := NewHost(swarmt.GenSwarm(t, swarmt.OptDialOnly), &HostOpts{ + NATManager: func(net network.Network) NATManager { return NewNATManager(net) }, + }) + require.NoError(t, err) + defer h1.Close() + + h1.IDService().Start() + defer h1.IDService().Close() + + laddr := ma.StringCast("/ip4/0.0.0.0/udp/0/quic-v1") + require.NoError(t, h1.Network().Listen(laddr)) + + var wg sync.WaitGroup + wg.Add(1) + + // inform the host that its address is 1.2.3.4:1234 + obsAddr := ma.StringCast("/ip4/1.2.3.4/udp/1234/quic-v1") + idHandler := func(s network.Stream) { + msg := &pb.Identify{} + msg.ObservedAddr = obsAddr.Bytes() + writer := pbio.NewDelimitedWriter(s) + if err := writer.WriteMsg(msg); err != nil { + t.Fatal(err) + } + s.Close() + wg.Done() + } + h2 := blhost.NewBlankHost(swarmt.GenSwarm(t)) + h2.SetStreamHandler(identify.ID, idHandler) + err = h2.Connect(context.Background(), peer.AddrInfo{ID: h1.ID(), Addrs: h1.Addrs()}) + require.NoError(t, err) + + wg.Wait() + + require.Eventually(t, + func() bool { return ma.Contains(h1.AllAddrs(), obsAddr) }, + 5*time.Second, + 100*time.Millisecond, + ) +} + // getHostPair gets a new pair of hosts. // The first host initiates the connection to the second host. func getHostPair(t *testing.T) (host.Host, host.Host) {