From b8d0297951d15de837bec88a11bb87a1ad6be2a5 Mon Sep 17 00:00:00 2001 From: sukun Date: Sat, 20 Jul 2024 17:45:04 +0530 Subject: [PATCH] add certhashes to addresses provided by AddrsFactory --- config/config.go | 9 +-- libp2p_test.go | 28 ++++++++-- p2p/host/basic/basic_host.go | 105 ++++++++++++++++++++--------------- 3 files changed, 90 insertions(+), 52 deletions(-) diff --git a/config/config.go b/config/config.go index 623cc473fc..07b3e12747 100644 --- a/config/config.go +++ b/config/config.go @@ -492,8 +492,9 @@ func (cfg *Config) NewNode() (host.Host, error) { return addrs } + // enable autorelay fxopts = append(fxopts, - fx.Invoke(func(h *bhost.BasicHost, lifecycle fx.Lifecycle) (*autorelay.AutoRelay, error) { + fx.Invoke(func(h *bhost.BasicHost, lifecycle fx.Lifecycle) error { oldAddrFactory = h.AddrsFactory if cfg.EnableAutoRelay { if !cfg.DisableMetrics { @@ -505,12 +506,12 @@ func (cfg *Config) NewNode() (host.Host, error) { ar, err := autorelay.NewAutoRelay(h, cfg.AutoRelayOpts...) if err != nil { - return nil, err + return err } lifecycle.Append(fx.StartStopHook(ar.Start, ar.Close)) - return ar, nil + return nil } - return nil, nil + return nil }), ) 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..b5845774db 100644 --- a/p2p/host/basic/basic_host.go +++ b/p2p/host/basic/basic_host.go @@ -284,6 +284,20 @@ func NewHost(n network.Network, opts *HostOpts) (*BasicHost, error) { if opts.AddrsFactory != nil { h.AddrsFactory = opts.AddrsFactory } + // This is a terrible hack. + // We want to use this AddrsFactory for autonat. Wrapping AddrsFactory here ensures + // that autonat receives addresses with the correct certhashes. + // + // This logic cannot be in Addrs method as the autorelay package updates AddrsFactory + // to only provide p2p-circuit addresses when reachability is Private. For the same reason, + // autonat cannot use Addrs method. + // + // 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 +818,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 +840,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 +936,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.