From 7b95317cdbbc0ffa2825637e5fd5246b69960453 Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Fri, 15 Nov 2024 09:56:57 -0800 Subject: [PATCH] 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. --- p2p/protocol/identify/obsaddr.go | 13 ++++++++++++- p2p/protocol/identify/obsaddr_glass_test.go | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/p2p/protocol/identify/obsaddr.go b/p2p/protocol/identify/obsaddr.go index ffe60345e1..06e54bf5fd 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() diff --git a/p2p/protocol/identify/obsaddr_glass_test.go b/p2p/protocol/identify/obsaddr_glass_test.go index 31fd4f5726..3211aa5f54 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")