Repository: libp2p/go-libp2p
Pull request #3043 (2024-11-15 17:58:52)

fix(obsaddr): do not record observations over relayed conn
These observations are telling us what the relay's IP address is. This isn't useful for us at all.

This also solves an issue during hole punching where we may report the relay's address as our own.


Patch:
diff --git a/p2p/protocol/identify/obsaddr.go b/p2p/protocol/identify/obsaddr.go
index ffe60345..06e54bf5 100644
--- a/p2p/protocol/identify/obsaddr.go
+++ b/p2p/protocol/identify/obsaddr.go
@@ -335,6 +335,11 @@ func (o *ObservedAddrManager) worker() {
 	}
 }
 
+func isRelayedAddress(a ma.Multiaddr) bool {
+	_, err := a.ValueForProtocol(ma.P_CIRCUIT)
+	return err == nil
+}
+
 func (o *ObservedAddrManager) shouldRecordObservation(conn connMultiaddrs, observed ma.Multiaddr) (shouldRecord bool, localTW thinWaist, observedTW thinWaist) {
 	if conn == nil || observed == nil {
 		return false, thinWaist{}, thinWaist{}
@@ -350,6 +355,12 @@ func (o *ObservedAddrManager) shouldRecordObservation(conn connMultiaddrs, obser
 		return false, thinWaist{}, thinWaist{}
 	}
 
+	// Ignore p2p-circuit addresses. These are the observed address of the relay.
+	// Not useful for us.
+	if isRelayedAddress(observed) {
+		return false, thinWaist{}, thinWaist{}
+	}
+
 	// we should only use ObservedAddr when our connection's LocalAddr is one
 	// of our ListenAddrs. If we Dial out using an ephemeral addr, knowing that
 	// address's external mapping is not very useful because the port will not be
@@ -410,7 +421,7 @@ func (o *ObservedAddrManager) maybeRecordObservation(conn connMultiaddrs, observ
 	if !shouldRecord {
 		return
 	}
-	log.Debugw("added own observed listen addr", "observed", observed)
+	log.Debugw("added own observed listen addr", "conn", conn, "observed", observed)
 
 	o.mu.Lock()
 	defer o.mu.Unlock()


Test patch:
diff --git a/p2p/protocol/identify/obsaddr_glass_test.go b/p2p/protocol/identify/obsaddr_glass_test.go
index 31fd4f57..3211aa5f 100644
--- a/p2p/protocol/identify/obsaddr_glass_test.go
+++ b/p2p/protocol/identify/obsaddr_glass_test.go
@@ -53,6 +53,24 @@ func TestShouldRecordObservationWithWebTransport(t *testing.T) {
 	require.True(t, shouldRecord)
 }
 
+func TestShouldNotRecordObservationWithRelayedAddr(t *testing.T) {
+	listenAddr := ma.StringCast("/ip4/1.2.3.4/udp/8888/quic-v1/p2p-circuit")
+	ifaceAddr := ma.StringCast("/ip4/10.0.0.2/udp/9999/quic-v1")
+	listenAddrs := func() []ma.Multiaddr { return []ma.Multiaddr{listenAddr} }
+	ifaceListenAddrs := func() ([]ma.Multiaddr, error) { return []ma.Multiaddr{ifaceAddr}, nil }
+	addrs := func() []ma.Multiaddr { return []ma.Multiaddr{listenAddr} }
+
+	c := &mockConn{
+		local:  listenAddr,
+		remote: ma.StringCast("/ip4/1.2.3.6/udp/1236/quic-v1/p2p-circuit"),
+	}
+	observedAddr := ma.StringCast("/ip4/1.2.3.4/udp/1231/quic-v1/p2p-circuit")
+	o, err := NewObservedAddrManager(listenAddrs, addrs, ifaceListenAddrs, normalize)
+	require.NoError(t, err)
+	shouldRecord, _, _ := o.shouldRecordObservation(c, observedAddr)
+	require.False(t, shouldRecord)
+}
+
 func TestShouldRecordObservationWithNAT64Addr(t *testing.T) {
 	listenAddr1 := ma.StringCast("/ip4/0.0.0.0/tcp/1234")
 	ifaceAddr1 := ma.StringCast("/ip4/10.0.0.2/tcp/4321")