swarm: fix recursive resolving of DNS multiaddrs (#2564)

* swarm: fix recursive resolving of DNS multiaddrs

* add a test case

Author: marten-seemann

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)

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")

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