feat: core/transport: Add SkipResolver interface (#2989)

* Rebase on top of resolveAddrs refactor

* Add comments

* Sanitize address inputs when returning a reservation message (#3006)

core/transport/transport.go

@@ -50,6 +50,10 @@ type CapableConn interface {
 // shutdown. NOTE: `Dial` and `Listen` may be called after or concurrently with
 // `Close`.
 //
+// In addition to the Transport interface, transports may implement
+// Resolver or SkipResolver interface. When wrapping/embedding a transport, you should
+// ensure that the Resolver/SkipResolver interface is handled correctly.
+//
 // For a conceptual overview, see https://docs.libp2p.io/concepts/transport/
 type Transport interface {
 	// Dial dials a remote peer. It should try to reuse local listener
@@ -85,6 +89,14 @@ type Resolver interface {
 	Resolve(ctx context.Context, maddr ma.Multiaddr) ([]ma.Multiaddr, error)
 }
 
+// SkipResolver can be optionally implemented by transports that don't want to
+// resolve or transform the multiaddr. Useful for transports that indirectly
+// wrap other transports (e.g. p2p-circuit). This lets the inner transport
+// specify how a multiaddr is resolved later.
+type SkipResolver interface {
+	SkipResolve(ctx context.Context, maddr ma.Multiaddr) bool
+}
+
 // Listener is an interface closely resembling the net.Listener interface. The
 // only real difference is that Accept() returns Conn's of the type in this
 // package, and also exposes a Multiaddr method as opposed to a regular Addr

libp2p_test.go

@@ -2,10 +2,16 @@ package libp2p
 
 import (
 	"context"
+	"crypto/ecdsa"
+	"crypto/elliptic"
 	"crypto/rand"
+	"crypto/tls"
+	"crypto/x509"
+	"crypto/x509/pkix"
 	"errors"
 	"fmt"
 	"io"
+	"math/big"
 	"net"
 	"net/netip"
 	"regexp"
@@ -26,11 +32,12 @@ import (
 	"github.com/libp2p/go-libp2p/p2p/net/swarm"
 	"github.com/libp2p/go-libp2p/p2p/protocol/ping"
 	"github.com/libp2p/go-libp2p/p2p/security/noise"
-	tls "github.com/libp2p/go-libp2p/p2p/security/tls"
+	sectls "github.com/libp2p/go-libp2p/p2p/security/tls"
 	quic "github.com/libp2p/go-libp2p/p2p/transport/quic"
 	"github.com/libp2p/go-libp2p/p2p/transport/quicreuse"
 	"github.com/libp2p/go-libp2p/p2p/transport/tcp"
 	libp2pwebrtc "github.com/libp2p/go-libp2p/p2p/transport/webrtc"
+	"github.com/libp2p/go-libp2p/p2p/transport/websocket"
 	webtransport "github.com/libp2p/go-libp2p/p2p/transport/webtransport"
 	"go.uber.org/goleak"
 
@@ -256,7 +263,7 @@ func TestSecurityConstructor(t *testing.T) {
 	h, err := New(
 		Transport(tcp.NewTCPTransport),
 		Security("/noisy", noise.New),
-		Security("/tls", tls.New),
+		Security("/tls", sectls.New),
 		DefaultListenAddrs,
 		DisableRelay(),
 	)
@@ -655,3 +662,92 @@ func TestUseCorrectTransportForDialOut(t *testing.T) {
 		}
 	}
 }
+
+func TestCircuitBehindWSS(t *testing.T) {
+	relayTLSConf := getTLSConf(t, net.IPv4(127, 0, 0, 1), time.Now(), time.Now().Add(time.Hour))
+	serverNameChan := make(chan string, 2) // Channel that returns what server names the client hello specified
+	relayTLSConf.GetConfigForClient = func(chi *tls.ClientHelloInfo) (*tls.Config, error) {
+		serverNameChan <- chi.ServerName
+		return relayTLSConf, nil
+	}
+
+	relay, err := New(
+		EnableRelayService(),
+		ForceReachabilityPublic(),
+		Transport(websocket.New, websocket.WithTLSConfig(relayTLSConf)),
+		ListenAddrStrings("/ip4/127.0.0.1/tcp/0/wss"),
+	)
+	require.NoError(t, err)
+	defer relay.Close()
+
+	relayAddrPort, _ := relay.Addrs()[0].ValueForProtocol(ma.P_TCP)
+	relayAddrWithSNIString := fmt.Sprintf(
+		"/dns4/localhost/tcp/%s/wss", relayAddrPort,
+	)
+	relayAddrWithSNI := []ma.Multiaddr{ma.StringCast(relayAddrWithSNIString)}
+
+	h, err := New(
+		NoListenAddrs,
+		EnableRelay(),
+		Transport(websocket.New, websocket.WithTLSClientConfig(&tls.Config{InsecureSkipVerify: true})),
+		ForceReachabilityPrivate())
+	require.NoError(t, err)
+	defer h.Close()
+
+	peerBehindRelay, err := New(
+		NoListenAddrs,
+		Transport(websocket.New, websocket.WithTLSClientConfig(&tls.Config{InsecureSkipVerify: true})),
+		EnableRelay(),
+		EnableAutoRelayWithStaticRelays([]peer.AddrInfo{{ID: relay.ID(), Addrs: relayAddrWithSNI}}),
+		ForceReachabilityPrivate())
+	require.NoError(t, err)
+	defer peerBehindRelay.Close()
+
+	require.Equal(t,
+		"localhost",
+		<-serverNameChan, // The server connects to the relay
+	)
+
+	// Connect to the peer behind the relay
+	h.Connect(context.Background(), peer.AddrInfo{
+		ID: peerBehindRelay.ID(),
+		Addrs: []ma.Multiaddr{ma.StringCast(
+			fmt.Sprintf("%s/p2p/%s/p2p-circuit", relayAddrWithSNIString, relay.ID()),
+		)},
+	})
+	require.NoError(t, err)
+
+	require.Equal(t,
+		"localhost",
+		<-serverNameChan, // The client connects to the relay and sends the SNI
+	)
+}
+
+// getTLSConf is a helper to generate a self-signed TLS config
+func getTLSConf(t *testing.T, ip net.IP, start, end time.Time) *tls.Config {
+	t.Helper()
+	certTempl := &x509.Certificate{
+		SerialNumber:          big.NewInt(1234),
+		Subject:               pkix.Name{Organization: []string{"websocket"}},
+		NotBefore:             start,
+		NotAfter:              end,
+		IsCA:                  true,
+		ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth},
+		KeyUsage:              x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
+		BasicConstraintsValid: true,
+		IPAddresses:           []net.IP{ip},
+	}
+	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	require.NoError(t, err)
+	caBytes, err := x509.CreateCertificate(rand.Reader, certTempl, certTempl, &priv.PublicKey, priv)
+	require.NoError(t, err)
+	cert, err := x509.ParseCertificate(caBytes)
+	require.NoError(t, err)
+	return &tls.Config{
+		Certificates: []tls.Certificate{{
+			Certificate: [][]byte{cert.Raw},
+			PrivateKey:  priv,
+			Leaf:        cert,
+		}},
+	}
+}

p2p/net/swarm/swarm_dial.go

@@ -393,6 +393,32 @@ func (s *Swarm) resolveAddrs(ctx context.Context, pi peer.AddrInfo) []ma.Multiad
 			return s.multiaddrResolver.ResolveDNSAddr(ctx, pi.ID, maddr, maximumDNSADDRRecursion, outputLimit)
 		},
 	}
+
+	var skipped []ma.Multiaddr
+	skipResolver := resolver{
+		canResolve: func(addr ma.Multiaddr) bool {
+			tpt := s.TransportForDialing(addr)
+			if tpt == nil {
+				return false
+			}
+			_, ok := tpt.(transport.SkipResolver)
+			return ok
+
+		},
+		resolve: func(ctx context.Context, addr ma.Multiaddr, outputLimit int) ([]ma.Multiaddr, error) {
+			tpt := s.TransportForDialing(addr)
+			resolver, ok := tpt.(transport.SkipResolver)
+			if !ok {
+				return []ma.Multiaddr{addr}, nil
+			}
+			if resolver.SkipResolve(ctx, addr) {
+				skipped = append(skipped, addr)
+				return nil, nil
+			}
+			return []ma.Multiaddr{addr}, nil
+		},
+	}
+
 	tptResolver := resolver{
 		canResolve: func(addr ma.Multiaddr) bool {
 			tpt := s.TransportForDialing(addr)
@@ -418,14 +444,17 @@ func (s *Swarm) resolveAddrs(ctx context.Context, pi peer.AddrInfo) []ma.Multiad
 			return addrs, nil
 		},
 	}
+
 	dnsResolver := resolver{
 		canResolve: startsWithDNSComponent,
 		resolve:    s.multiaddrResolver.ResolveDNSComponent,
 	}
-	addrs, errs := chainResolvers(ctx, pi.Addrs, maximumResolvedAddresses, []resolver{dnsAddrResolver, tptResolver, dnsResolver})
+	addrs, errs := chainResolvers(ctx, pi.Addrs, maximumResolvedAddresses, []resolver{dnsAddrResolver, skipResolver, tptResolver, dnsResolver})
 	for _, err := range errs {
 		log.Warnf("Failed to resolve addr %s: %v", err.addr, err.err)
 	}
+	// Add skipped addresses back to the resolved addresses
+	addrs = append(addrs, skipped...)
 	return stripP2PComponent(addrs)
 }
 

p2p/protocol/circuitv2/client/transport.go

@@ -46,8 +46,20 @@ func AddTransport(h host.Host, upgrader transport.Upgrader) error {
 
 // Transport interface
 var _ transport.Transport = (*Client)(nil)
+
+// p2p-circuit implements the SkipResolver interface so that the underlying
+// transport can do the address resolution later. If you wrap this transport,
+// make sure you also implement SkipResolver as well.
+var _ transport.SkipResolver = (*Client)(nil)
 var _ io.Closer = (*Client)(nil)
 
+// SkipResolve returns true since we always defer to the inner transport for
+// the actual connection. By skipping resolution here, we let the inner
+// transport decide how to resolve the multiaddr
+func (c *Client) SkipResolve(ctx context.Context, maddr ma.Multiaddr) bool {
+	return true
+}
+
 func (c *Client) Dial(ctx context.Context, a ma.Multiaddr, p peer.ID) (transport.CapableConn, error) {
 	connScope, err := c.host.Network().ResourceManager().OpenConnection(network.DirOutbound, false, a)
 

p2p/protocol/circuitv2/relay/relay.go

@@ -9,6 +9,7 @@ import (
 	"sync/atomic"
 	"time"
 
+	"github.com/libp2p/go-libp2p/core/crypto"
 	"github.com/libp2p/go-libp2p/core/host"
 	"github.com/libp2p/go-libp2p/core/network"
 	"github.com/libp2p/go-libp2p/core/peer"
@@ -224,7 +225,13 @@ func (r *Relay) handleReserve(s network.Stream) pbv2.Status {
 	// Delivery of the reservation might fail for a number of reasons.
 	// For example, the stream might be reset or the connection might be closed before the reservation is received.
 	// In that case, the reservation will just be garbage collected later.
-	if err := r.writeResponse(s, pbv2.Status_OK, r.makeReservationMsg(p, expire), r.makeLimitMsg(p)); err != nil {
+	rsvp := makeReservationMsg(
+		r.host.Peerstore().PrivKey(r.host.ID()),
+		r.host.ID(),
+		r.host.Addrs(),
+		p,
+		expire)
+	if err := r.writeResponse(s, pbv2.Status_OK, rsvp, r.makeLimitMsg(p)); err != nil {
 		log.Debugf("error writing reservation response; retracting reservation for %s", p)
 		s.Reset()
 		return pbv2.Status_CONNECTION_FAILED
@@ -567,31 +574,54 @@ func (r *Relay) writeResponse(s network.Stream, status pbv2.Status, rsvp *pbv2.R
 	return wr.WriteMsg(&msg)
 }
 
-func (r *Relay) makeReservationMsg(p peer.ID, expire time.Time) *pbv2.Reservation {
+func makeReservationMsg(
+	signingKey crypto.PrivKey,
+	selfID peer.ID,
+	selfAddrs []ma.Multiaddr,
+	p peer.ID,
+	expire time.Time,
+) *pbv2.Reservation {
 	expireUnix := uint64(expire.Unix())
 
+	rsvp := &pbv2.Reservation{Expire: &expireUnix}
+
+	selfP2PAddr, err := ma.NewComponent("p2p", selfID.String())
+	if err != nil {
+		log.Errorf("error creating p2p component: %s", err)
+		return rsvp
+	}
+
 	var addrBytes [][]byte
-	for _, addr := range r.host.Addrs() {
+	for _, addr := range selfAddrs {
 		if !manet.IsPublicAddr(addr) {
 			continue
 		}
 
-		addr = addr.Encapsulate(r.selfAddr)
+		id, _ := peer.IDFromP2PAddr(addr)
+		switch {
+		case id == "":
+			// No ID, we'll add one to the address
+			addr = addr.Encapsulate(selfP2PAddr)
+		case id == selfID:
+		// This address already has our ID in it.
+		// Do nothing
+		case id != selfID:
+			// This address has a different ID in it. Skip it.
+			log.Warnf("skipping address %s: contains an unexpected ID", addr)
+			continue
+		}
 		addrBytes = append(addrBytes, addr.Bytes())
 	}
 
-	rsvp := &pbv2.Reservation{
-		Expire: &expireUnix,
-		Addrs:  addrBytes,
-	}
+	rsvp.Addrs = addrBytes
 
 	voucher := &proto.ReservationVoucher{
-		Relay:      r.host.ID(),
+		Relay:      selfID,
 		Peer:       p,
 		Expiration: expire,
 	}
 
-	envelope, err := record.Seal(voucher, r.host.Peerstore().PrivKey(r.host.ID()))
+	envelope, err := record.Seal(voucher, signingKey)
 	if err != nil {
 		log.Errorf("error sealing voucher for %s: %s", p, err)
 		return rsvp

p2p/protocol/circuitv2/relay/relay_priv_test.go

@@ -0,0 +1,53 @@
+package relay
+
+import (
+	"crypto/rand"
+	"testing"
+	"time"
+
+	"github.com/libp2p/go-libp2p/core/crypto"
+	"github.com/libp2p/go-libp2p/core/peer"
+	"github.com/stretchr/testify/require"
+
+	ma "github.com/multiformats/go-multiaddr"
+)
+
+func genKeyAndID(t *testing.T) (crypto.PrivKey, peer.ID) {
+	t.Helper()
+	key, _, err := crypto.GenerateEd25519Key(rand.Reader)
+	require.NoError(t, err)
+	id, err := peer.IDFromPrivateKey(key)
+	require.NoError(t, err)
+	return key, id
+}
+
+// TestMakeReservationWithP2PAddrs ensures that our reservation message builder
+// sanitizes the input addresses
+func TestMakeReservationWithP2PAddrs(t *testing.T) {
+	selfKey, selfID := genKeyAndID(t)
+	_, otherID := genKeyAndID(t)
+	_, reserverID := genKeyAndID(t)
+
+	addrs := []ma.Multiaddr{
+		ma.StringCast("/ip4/1.2.3.4/tcp/1234"),                         // No p2p part
+		ma.StringCast("/ip4/1.2.3.4/tcp/1235/p2p/" + selfID.String()),  // Already has p2p part
+		ma.StringCast("/ip4/1.2.3.4/tcp/1236/p2p/" + otherID.String()), // Some other peer (?? Not expected, but we could get anything in this func)
+	}
+
+	rsvp := makeReservationMsg(selfKey, selfID, addrs, reserverID, time.Now().Add(time.Minute))
+	require.NotNil(t, rsvp)
+
+	expectedAddrs := []string{
+		"/ip4/1.2.3.4/tcp/1234/p2p/" + selfID.String(),
+		"/ip4/1.2.3.4/tcp/1235/p2p/" + selfID.String(),
+	}
+
+	var addrsFromRsvp []string
+	for _, addr := range rsvp.GetAddrs() {
+		a, err := ma.NewMultiaddrBytes(addr)
+		require.NoError(t, err)
+		addrsFromRsvp = append(addrsFromRsvp, a.String())
+	}
+
+	require.Equal(t, expectedAddrs, addrsFromRsvp)
+}