From 1d1ef2db7ea0d93c7dab4a9800ec74afa7a019eb Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Wed, 24 Mar 2021 15:01:05 -0700 Subject: [PATCH] [net_processing] Defer initializing m_addr_known Use SetupAddressRelay to only initialize `m_addr_known` as needed. For outbound peers, we initialize the filter before sending our self announcement (not applicable for block-relay-only connections). For inbound peers, we initialize the filter when we get an addr related message (ADDR, ADDRV2, GETADDR). These changes intend to mitigate address blackholes. Since an inbound peer has to send us an addr related message to become eligible as a candidate for addr relay, this should reduce our likelihood of sending them self-announcements. --- src/net_processing.cpp | 29 +++++++++++++++++---------- test/functional/p2p_addr_relay.py | 3 +++ test/functional/test_framework/p2p.py | 1 + 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 1d53d1a1b77..fe20421a216 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -224,8 +224,13 @@ struct Peer { /** A vector of addresses to send to the peer, limited to MAX_ADDR_TO_SEND. */ std::vector m_addrs_to_send; - /** Probabilistic filter of addresses that this peer already knows. - * Used to avoid relaying addresses to this peer more than once. */ + /** Probabilistic filter to track recent addr messages relayed with this + * peer. Used to avoid relaying redundant addresses to this peer. + * + * We initialize this filter for outbound peers (other than + * block-relay-only connections) or when an inbound peer sends us an + * address related message (ADDR, ADDRV2, GETADDR). + **/ std::unique_ptr m_addr_known; /** Whether a getaddr request to this peer is outstanding. */ bool m_getaddr_sent{false}; @@ -258,9 +263,8 @@ struct Peer { /** Work queue of items requested by this peer **/ std::deque m_getdata_requests GUARDED_BY(m_getdata_requests_mutex); - explicit Peer(NodeId id, bool addr_relay) + explicit Peer(NodeId id) : m_id(id) - , m_addr_known{addr_relay ? std::make_unique(5000, 0.001) : nullptr} {} }; @@ -1125,9 +1129,7 @@ void PeerManagerImpl::InitializeNode(CNode *pnode) assert(m_txrequest.Count(nodeid) == 0); } { - // Addr relay is disabled for outbound block-relay-only peers to - // prevent adversaries from inferring these links from addr traffic. - PeerRef peer = std::make_shared(nodeid, /* addr_relay = */ !pnode->IsBlockOnlyConn()); + PeerRef peer = std::make_shared(nodeid); LOCK(m_peer_mutex); m_peer_map.emplace_hint(m_peer_map.end(), nodeid, std::move(peer)); } @@ -2580,7 +2582,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, UpdatePreferredDownload(pfrom, State(pfrom.GetId())); } - if (!pfrom.IsInboundConn() && !pfrom.IsBlockOnlyConn()) { + // Self advertisement & GETADDR logic + if (!pfrom.IsInboundConn() && SetupAddressRelay(pfrom, *peer)) { // For outbound peers, we try to relay our address (so that other // nodes can try to find us more quickly, as we have no guarantee // that an outbound peer is even aware of how to reach us) and do a @@ -2589,8 +2592,9 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // empty and no one will know who we are, so these mechanisms are // important to help us connect to the network. // - // We skip this for block-relay-only peers to avoid potentially leaking - // information about our block-relay-only connections via address relay. + // We skip this for block-relay-only peers. We want to avoid + // potentially leaking addr information and we do not want to + // indicate to the peer that we will participate in addr relay. if (fListen && !m_chainman.ActiveChainstate().IsInitialBlockDownload()) { CAddress addr = GetLocalAddress(&pfrom.addr, pfrom.GetLocalServices()); @@ -2788,10 +2792,11 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, s >> vAddr; - if (!RelayAddrsWithPeer(*peer)) { + if (!SetupAddressRelay(pfrom, *peer)) { LogPrint(BCLog::NET, "ignoring %s message from %s peer=%d\n", msg_type, pfrom.ConnectionTypeAsString(), pfrom.GetId()); return; } + if (vAddr.size() > MAX_ADDR_TO_SEND) { Misbehaving(pfrom.GetId(), 20, strprintf("%s message size = %u", msg_type, vAddr.size())); @@ -3725,6 +3730,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, return; } + SetupAddressRelay(pfrom, *peer); + // Only send one GetAddr response per connection to reduce resource waste // and discourage addr stamping of INV announcements. if (peer->m_getaddr_recvd) { diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py index 7b7bcfc917a..c8c1120462e 100755 --- a/test/functional/p2p_addr_relay.py +++ b/test/functional/p2p_addr_relay.py @@ -175,6 +175,9 @@ class AddrTest(BitcoinTestFramework): # of the outbound peer which is often sent before the GETADDR response. assert_equal(inbound_peer.num_ipv4_received, 0) + # Send an empty ADDR message to intialize address relay on this connection. + inbound_peer.send_and_ping(msg_addr()) + self.log.info('Check that subsequent addr messages sent from an outbound peer are relayed') msg2 = self.setup_addr_msg(2) self.send_addr_msg(full_outbound_peer, msg2, [inbound_peer]) diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index cc80b543cd7..b7d5bd8fab5 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -438,6 +438,7 @@ class P2PInterface(P2PConnection): self.send_message(msg_sendaddrv2()) self.send_message(msg_verack()) self.nServices = message.nServices + self.send_message(msg_getaddr()) # Connection helper methods