fix: circuitv2: sanitize address inputs: check address peer id when returning a reservation message

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)
+}