diff --git a/libp2p_test.go b/libp2p_test.go index e8339705d8..a5b4190d1e 100644 --- a/libp2p_test.go +++ b/libp2p_test.go @@ -377,10 +377,7 @@ func TestRoutedHost(t *testing.T) { } func TestAutoNATService(t *testing.T) { - // h, err := New(EnableNATService()) - // require.NoError(t, err) - // h.Close() - h, err := New(EnableNATService(), EnableAutoRelayWithStaticRelays([]peer.AddrInfo{{ID: peer.ID("hello")}})) + h, err := New(EnableNATService()) require.NoError(t, err) h.Close() } @@ -468,3 +465,26 @@ func TestDialCircuitAddrWithWrappedResourceManager(t *testing.T) { require.NoError(t, res.Error) defer cancel() } + +func TestHostAddrsFactoryAddsCerthashes(t *testing.T) { + addr := ma.StringCast("/ip4/1.2.3.4/udp/1/quic-v1/webtransport") + h, err := New( + AddrsFactory(func(m []ma.Multiaddr) []ma.Multiaddr { + return []ma.Multiaddr{addr} + }), + ) + require.NoError(t, err) + require.Eventually(t, func() bool { + addrs := h.Addrs() + for _, a := range addrs { + first, last := ma.SplitFunc(a, func(c ma.Component) bool { + return c.Protocol().Code == ma.P_CERTHASH + }) + if addr.Equal(first) && last != nil { + return true + } + } + return false + }, 5*time.Second, 50*time.Millisecond) + h.Close() +} diff --git a/p2p/host/basic/basic_host.go b/p2p/host/basic/basic_host.go index a5eba01d72..1defcb3464 100644 --- a/p2p/host/basic/basic_host.go +++ b/p2p/host/basic/basic_host.go @@ -284,6 +284,18 @@ func NewHost(n network.Network, opts *HostOpts) (*BasicHost, error) { if opts.AddrsFactory != nil { h.AddrsFactory = opts.AddrsFactory } + // This is a terrible hack to ensure that certhashes are added to addresses + // provided by AddrsFactory. This ensures that autonat receives addresses + // with the correct certhashes. We need to do it here and not in Addrs + // method as the autorelay package updates the AddrsFactory. Autonat + // cannot use Addrs directly because when the node is private, the autorelay + // provided addrsFactory only returns p2p-circuit addresses. Wrapping it here + // allows us to provide the wrapped AddrsFactory to autonat before autorelay + // updates it. + addrFactory := h.AddrsFactory + h.AddrsFactory = func(addrs []ma.Multiaddr) []ma.Multiaddr { + return h.addCertHashes(addrFactory(addrs)) + } if opts.NATManager != nil { h.natmgr = opts.NATManager(n) @@ -804,47 +816,9 @@ func (h *BasicHost) ConnManager() connmgr.ConnManager { // Addrs returns listening addresses that are safe to announce to the network. // The output is the same as AllAddrs, but processed by AddrsFactory. func (h *BasicHost) Addrs() []ma.Multiaddr { - // This is a temporary workaround/hack that fixes #2233. Once we have a - // proper address pipeline, rework this. See the issue for more context. - type transportForListeninger interface { - TransportForListening(a ma.Multiaddr) transport.Transport - } - - type addCertHasher interface { - AddCertHashes(m ma.Multiaddr) (ma.Multiaddr, bool) - } - - addrs := h.AddrsFactory(h.AllAddrs()) - - s, ok := h.Network().(transportForListeninger) - if !ok { - return addrs - } - - // Copy addrs slice since we'll be modifying it. - addrsOld := addrs - addrs = make([]ma.Multiaddr, len(addrsOld)) - copy(addrs, addrsOld) - - for i, addr := range addrs { - wtOK, wtN := libp2pwebtransport.IsWebtransportMultiaddr(addr) - webrtcOK, webrtcN := libp2pwebrtc.IsWebRTCDirectMultiaddr(addr) - if (wtOK && wtN == 0) || (webrtcOK && webrtcN == 0) { - t := s.TransportForListening(addr) - tpt, ok := t.(addCertHasher) - if !ok { - continue - } - addrWithCerthash, added := tpt.AddCertHashes(addr) - if !added { - log.Debugf("Couldn't add certhashes to multiaddr: %s", addr) - continue - } - addrs[i] = addrWithCerthash - } - } - - return addrs + // We don't need to append certhashes here, the user provided addrsFactory was + // wrapped with addCertHashes in the constructor. + return h.AddrsFactory(h.AllAddrs()) } // NormalizeMultiaddr returns a multiaddr suitable for equality checks. @@ -864,8 +838,9 @@ func (h *BasicHost) NormalizeMultiaddr(addr ma.Multiaddr) ma.Multiaddr { return addr } -// AllAddrs returns all the addresses of BasicHost at this moment in time. -// It's ok to not include addresses if they're not available to be used now. +// AllAddrs returns all the addresses the host is listening on except circuit addresses. +// The output has webtransport addresses inferred from quic addresses. +// All the addresses have the correct func (h *BasicHost) AllAddrs() []ma.Multiaddr { listenAddrs := h.Network().ListenAddresses() if len(listenAddrs) == 0 { @@ -959,10 +934,50 @@ func (h *BasicHost) AllAddrs() []ma.Multiaddr { } finalAddrs = ma.Unique(finalAddrs) finalAddrs = inferWebtransportAddrsFromQuic(finalAddrs) - return finalAddrs } +func (h *BasicHost) addCertHashes(addrs []ma.Multiaddr) []ma.Multiaddr { + // This is a temporary workaround/hack that fixes #2233. Once we have a + // proper address pipeline, rework this. See the issue for more context. + type transportForListeninger interface { + TransportForListening(a ma.Multiaddr) transport.Transport + } + + type addCertHasher interface { + AddCertHashes(m ma.Multiaddr) (ma.Multiaddr, bool) + } + + s, ok := h.Network().(transportForListeninger) + if !ok { + return addrs + } + + // Copy addrs slice since we'll be modifying it. + addrsOld := addrs + addrs = make([]ma.Multiaddr, len(addrsOld)) + copy(addrs, addrsOld) + + for i, addr := range addrs { + wtOK, wtN := libp2pwebtransport.IsWebtransportMultiaddr(addr) + webrtcOK, webrtcN := libp2pwebrtc.IsWebRTCDirectMultiaddr(addr) + if (wtOK && wtN == 0) || (webrtcOK && webrtcN == 0) { + t := s.TransportForListening(addr) + tpt, ok := t.(addCertHasher) + if !ok { + continue + } + addrWithCerthash, added := tpt.AddCertHashes(addr) + if !added { + log.Debugf("Couldn't add certhashes to multiaddr: %s", addr) + continue + } + addrs[i] = addrWithCerthash + } + } + return addrs +} + var wtComponent = ma.StringCast("/webtransport") // inferWebtransportAddrsFromQuic infers more webtransport addresses from QUIC addresses.