diff --git a/config/config.go b/config/config.go index a99da1232d..8a7b8455e1 100644 --- a/config/config.go +++ b/config/config.go @@ -407,9 +407,9 @@ func (cfg *Config) newBasicHost(swrm *swarm.Swarm, eventBus event.Bus) (*bhost.B // addresses by default. // // TODO: We shouldn't be doing this here. - oldFactory := h.AddrsFactory + originalAddrFactory := h.AddrsFactory h.AddrsFactory = func(addrs []ma.Multiaddr) []ma.Multiaddr { - return oldFactory(autorelay.Filter(addrs)) + return originalAddrFactory(autorelay.Filter(addrs)) } } return h, nil @@ -486,26 +486,36 @@ func (cfg *Config) NewNode() (host.Host, error) { ) } - // Note: h.AddrsFactory may be changed by relayFinder, but non-relay version is - // used by AutoNAT below. - if cfg.EnableAutoRelay { - if !cfg.DisableMetrics { - mt := autorelay.WithMetricsTracer( - autorelay.NewMetricsTracer(autorelay.WithRegisterer(cfg.PrometheusRegisterer))) - mtOpts := []autorelay.Option{mt} - cfg.AutoRelayOpts = append(mtOpts, cfg.AutoRelayOpts...) - } - fxopts = append(fxopts, - fx.Invoke(func(h *bhost.BasicHost, lifecycle fx.Lifecycle) (*autorelay.AutoRelay, error) { + // originalAddrFactory is the AddrFactory before it's modified by autorelay + // we need this for checking reachability via autonat + originalAddrFactory := func(addrs []ma.Multiaddr) []ma.Multiaddr { + return addrs + } + + // enable autorelay + fxopts = append(fxopts, + fx.Invoke(func(h *bhost.BasicHost) { + originalAddrFactory = h.AddrsFactory + }), + fx.Invoke(func(h *bhost.BasicHost, lifecycle fx.Lifecycle) error { + if cfg.EnableAutoRelay { + if !cfg.DisableMetrics { + mt := autorelay.WithMetricsTracer( + autorelay.NewMetricsTracer(autorelay.WithRegisterer(cfg.PrometheusRegisterer))) + mtOpts := []autorelay.Option{mt} + cfg.AutoRelayOpts = append(mtOpts, cfg.AutoRelayOpts...) + } + 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 + }), + ) var bh *bhost.BasicHost fxopts = append(fxopts, fx.Invoke(func(bho *bhost.BasicHost) { bh = bho })) @@ -523,7 +533,7 @@ func (cfg *Config) NewNode() (host.Host, error) { return nil, err } - if err := cfg.addAutoNAT(bh); err != nil { + if err := cfg.addAutoNAT(bh, originalAddrFactory); err != nil { app.Stop(context.Background()) if cfg.Routing != nil { rh.Close() @@ -539,8 +549,7 @@ func (cfg *Config) NewNode() (host.Host, error) { return &closableBasicHost{App: app, BasicHost: bh}, nil } -func (cfg *Config) addAutoNAT(h *bhost.BasicHost) error { - addrF := h.AddrsFactory +func (cfg *Config) addAutoNAT(h *bhost.BasicHost, addrF AddrsFactory) error { autonatOpts := []autonat.Option{ autonat.UsingAddresses(func() []ma.Multiaddr { return addrF(h.AllAddrs()) diff --git a/libp2p_test.go b/libp2p_test.go index ea52e56470..a5b4190d1e 100644 --- a/libp2p_test.go +++ b/libp2p_test.go @@ -465,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..84118564f6 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 autonat cannot use the Addrs method directly. + // The autorelay package updates AddrsFactory to only provide p2p-circuit addresses when + // reachability is Private. + // + // 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,13 @@ 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) - } - + // We don't need to append certhashes here, the user provided addrsFactory was + // wrapped with addCertHashes in the constructor. 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 + // Make a copy. Consumers can modify the slice elements + res := make([]ma.Multiaddr, len(addrs)) + copy(res, addrs) + return res } // NormalizeMultiaddr returns a multiaddr suitable for equality checks. @@ -864,8 +844,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 +940,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.