From 81d5c0c835582c2d230718835cc48240aafaad57 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 25 Oct 2023 17:40:20 +0700 Subject: [PATCH] swarm: fix recursive resolving of DNS multiaddrs (#2564) * swarm: fix recursive resolving of DNS multiaddrs * add a test case --- p2p/net/swarm/swarm_dial.go | 49 +++++++++++++------------ p2p/net/swarm/swarm_dial_test.go | 53 ++++++++++++++++++---------- p2p/transport/websocket/websocket.go | 2 +- 3 files changed, 59 insertions(+), 45 deletions(-) diff --git a/p2p/net/swarm/swarm_dial.go b/p2p/net/swarm/swarm_dial.go index 288ad9cc7d..3fb15383a2 100644 --- a/p2p/net/swarm/swarm_dial.go +++ b/p2p/net/swarm/swarm_dial.go @@ -301,27 +301,8 @@ func (s *Swarm) addrsForDial(ctx context.Context, p peer.ID) (goodAddrs []ma.Mul return nil, nil, ErrNoAddresses } - peerAddrsAfterTransportResolved := make([]ma.Multiaddr, 0, len(peerAddrs)) - for _, a := range peerAddrs { - tpt := s.TransportForDialing(a) - resolver, ok := tpt.(transport.Resolver) - if ok { - resolvedAddrs, err := resolver.Resolve(ctx, a) - if err != nil { - log.Warnf("Failed to resolve multiaddr %s by transport %v: %v", a, tpt, err) - continue - } - peerAddrsAfterTransportResolved = append(peerAddrsAfterTransportResolved, resolvedAddrs...) - } else { - peerAddrsAfterTransportResolved = append(peerAddrsAfterTransportResolved, a) - } - } - // Resolve dns or dnsaddrs - resolved, err := s.resolveAddrs(ctx, peer.AddrInfo{ - ID: p, - Addrs: peerAddrsAfterTransportResolved, - }) + resolved, err := s.resolveAddrs(ctx, peer.AddrInfo{ID: p, Addrs: peerAddrs}) if err != nil { return nil, nil, err } @@ -342,21 +323,19 @@ func (s *Swarm) addrsForDial(ctx context.Context, p peer.ID) (goodAddrs []ma.Mul } func (s *Swarm) resolveAddrs(ctx context.Context, pi peer.AddrInfo) ([]ma.Multiaddr, error) { - proto := ma.ProtocolWithCode(ma.P_P2P).Name - p2paddr, err := ma.NewMultiaddr("/" + proto + "/" + pi.ID.String()) + p2paddr, err := ma.NewMultiaddr("/" + ma.ProtocolWithCode(ma.P_P2P).Name + "/" + pi.ID.String()) if err != nil { return nil, err } - resolveSteps := 0 - + var resolveSteps int // Recursively resolve all addrs. // // While the toResolve list is non-empty: // * Pop an address off. // * If the address is fully resolved, add it to the resolved list. // * Otherwise, resolve it and add the results to the "to resolve" list. - toResolve := append(([]ma.Multiaddr)(nil), pi.Addrs...) + toResolve := append([]ma.Multiaddr{}, pi.Addrs...) resolved := make([]ma.Multiaddr, 0, len(pi.Addrs)) for len(toResolve) > 0 { // pop the last addr off. @@ -383,6 +362,26 @@ func (s *Swarm) resolveAddrs(ctx context.Context, pi peer.AddrInfo) ([]ma.Multia continue } + tpt := s.TransportForDialing(addr) + resolver, ok := tpt.(transport.Resolver) + if ok { + resolvedAddrs, err := resolver.Resolve(ctx, addr) + if err != nil { + log.Warnf("Failed to resolve multiaddr %s by transport %v: %v", addr, tpt, err) + continue + } + var added bool + for _, a := range resolvedAddrs { + if !addr.Equal(a) { + toResolve = append(toResolve, a) + added = true + } + } + if added { + continue + } + } + // otherwise, resolve it reqaddr := addr.Encapsulate(p2paddr) resaddrs, err := s.maResolver.Resolve(ctx, reqaddr) diff --git a/p2p/net/swarm/swarm_dial_test.go b/p2p/net/swarm/swarm_dial_test.go index a0579b6a10..061e131af6 100644 --- a/p2p/net/swarm/swarm_dial_test.go +++ b/p2p/net/swarm/swarm_dial_test.go @@ -197,16 +197,9 @@ func TestAddrResolution(t *testing.T) { } func TestAddrResolutionRecursive(t *testing.T) { - ctx := context.Background() + p1 := test.RandPeerIDFatal(t) + p2 := test.RandPeerIDFatal(t) - p1, err := test.RandPeerID() - if err != nil { - t.Error(err) - } - p2, err := test.RandPeerID() - if err != nil { - t.Error(err) - } addr1 := ma.StringCast("/dnsaddr/example.com") addr2 := ma.StringCast("/ip4/192.0.2.1/tcp/123") p2paddr1 := ma.StringCast("/dnsaddr/example.com/p2p/" + p1.String()) @@ -221,25 +214,19 @@ func TestAddrResolutionRecursive(t *testing.T) { "dnsaddr=" + p2paddr1i.String(), "dnsaddr=" + p2paddr2i.String(), }, - "_dnsaddr.foo.example.com": { - "dnsaddr=" + p2paddr1f.String(), - }, - "_dnsaddr.bar.example.com": { - "dnsaddr=" + p2paddr2i.String(), - }, + "_dnsaddr.foo.example.com": {"dnsaddr=" + p2paddr1f.String()}, + "_dnsaddr.bar.example.com": {"dnsaddr=" + p2paddr2i.String()}, }, } resolver, err := madns.NewResolver(madns.WithDefaultResolver(backend)) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) s := newTestSwarmWithResolver(t, resolver) pi1, err := peer.AddrInfoFromP2pAddr(p2paddr1) require.NoError(t, err) - tctx, cancel := context.WithTimeout(ctx, time.Millisecond*100) + tctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*100) defer cancel() s.Peerstore().AddAddrs(pi1.ID, pi1.Addrs, peerstore.TempAddrTTL) _, _, err = s.addrsForDial(tctx, p1) @@ -263,6 +250,34 @@ func TestAddrResolutionRecursive(t *testing.T) { require.Contains(t, addrs2, addr1) } +// see https://github.com/libp2p/go-libp2p/issues/2562 +func TestAddrResolutionRecursiveTransportSpecific(t *testing.T) { + p := test.RandPeerIDFatal(t) + + backend := &madns.MockResolver{ + IP: map[string][]net.IPAddr{ + "sub.example.com": {net.IPAddr{IP: net.IPv4(1, 2, 3, 4)}}, + }, + TXT: map[string][]string{ + "_dnsaddr.example.com": {"dnsaddr=/dns4/sub.example.com/tcp/443/wss/p2p/" + p.String()}, + }, + } + resolver, err := madns.NewResolver(madns.WithDefaultResolver(backend)) + require.NoError(t, err) + + s := newTestSwarmWithResolver(t, resolver) + pi1, err := peer.AddrInfoFromP2pAddr(ma.StringCast("/dnsaddr/example.com/p2p/" + p.String())) + require.NoError(t, err) + + tctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*100) + defer cancel() + s.Peerstore().AddAddrs(pi1.ID, pi1.Addrs, peerstore.TempAddrTTL) + addrs, _, err := s.addrsForDial(tctx, p) + require.NoError(t, err) + require.Len(t, addrs, 1) + require.Equal(t, addrs[0].String(), "/ip4/1.2.3.4/tcp/443/tls/sni/sub.example.com/ws") +} + func TestAddrsForDialFiltering(t *testing.T) { q1 := ma.StringCast("/ip4/1.2.3.4/udp/1/quic-v1") q1v1 := ma.StringCast("/ip4/1.2.3.4/udp/1/quic-v1") diff --git a/p2p/transport/websocket/websocket.go b/p2p/transport/websocket/websocket.go index 0054df99ee..5142ca97a1 100644 --- a/p2p/transport/websocket/websocket.go +++ b/p2p/transport/websocket/websocket.go @@ -120,7 +120,7 @@ func (t *WebsocketTransport) Proxy() bool { return false } -func (t *WebsocketTransport) Resolve(ctx context.Context, maddr ma.Multiaddr) ([]ma.Multiaddr, error) { +func (t *WebsocketTransport) Resolve(_ context.Context, maddr ma.Multiaddr) ([]ma.Multiaddr, error) { parsed, err := parseWebsocketMultiaddr(maddr) if err != nil { return nil, err