diff options
| -rw-r--r-- | src/net_processing.cpp | 82 | ||||
| -rw-r--r-- | src/net_processing.h | 1 | ||||
| -rw-r--r-- | src/rpc/net.cpp | 2 | ||||
| -rwxr-xr-x | test/functional/p2p_addr_relay.py | 82 | ||||
| -rwxr-xr-x | test/functional/test_framework/p2p.py | 1 |
5 files changed, 144 insertions, 24 deletions
diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 2538904ade..005fe1bf0c 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -225,9 +225,31 @@ struct Peer { /** A vector of addresses to send to the peer, limited to MAX_ADDR_TO_SEND. */ std::vector<CAddress> m_addrs_to_send; - /** Probabilistic filter of addresses that this peer already knows. - * Used to avoid relaying addresses to this peer more than once. */ - const std::unique_ptr<CRollingBloomFilter> m_addr_known; + /** 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). + * + * Presence of this filter must correlate with m_addr_relay_enabled. + **/ + std::unique_ptr<CRollingBloomFilter> m_addr_known; + /** Whether we are participating in address relay with this connection. + * + * We set this bool to true for outbound peers (other than + * block-relay-only connections), or when an inbound peer sends us an + * address related message (ADDR, ADDRV2, GETADDR). + * + * We use this bool to decide whether a peer is eligible for gossiping + * addr messages. This avoids relaying to peers that are unlikely to + * forward them, effectively blackholing self announcements. Reasons + * peers might support addr relay on the link include that they connected + * to us as a block-relay-only peer or they are a light client. + * + * This field must correlate with whether m_addr_known has been + * initialized.*/ + std::atomic_bool m_addr_relay_enabled{false}; /** Whether a getaddr request to this peer is outstanding. */ bool m_getaddr_sent{false}; /** Guards address sending timers. */ @@ -259,9 +281,8 @@ struct Peer { /** Work queue of items requested by this peer **/ std::deque<CInv> 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<CRollingBloomFilter>(5000, 0.001) : nullptr} {} }; @@ -624,6 +645,14 @@ private: * @param[in] vRecv The raw message received */ void ProcessGetCFCheckPt(CNode& peer, CDataStream& vRecv); + + /** Checks if address relay is permitted with peer. If needed, initializes + * the m_addr_known bloom filter and sets m_addr_relay_enabled to true. + * + * @return True if address relay is enabled with peer + * False if address relay is disallowed + */ + bool SetupAddressRelay(CNode& node, Peer& peer); }; } // namespace @@ -744,11 +773,6 @@ static CNodeState *State(NodeId pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return &it->second; } -static bool RelayAddrsWithPeer(const Peer& peer) -{ - return peer.m_addr_known != nullptr; -} - /** * Whether the peer supports the address. For example, a peer that does not * implement BIP155 cannot receive Tor v3 addresses because it requires @@ -1129,9 +1153,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<Peer>(nodeid, /* addr_relay = */ !pnode->IsBlockOnlyConn()); + PeerRef peer = std::make_shared<Peer>(nodeid); LOCK(m_peer_mutex); m_peer_map.emplace_hint(m_peer_map.end(), nodeid, std::move(peer)); } @@ -1270,6 +1292,7 @@ bool PeerManagerImpl::GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) c stats.m_ping_wait = ping_wait; stats.m_addr_processed = peer->m_addr_processed.load(); stats.m_addr_rate_limited = peer->m_addr_rate_limited.load(); + stats.m_addr_relay_enabled = peer->m_addr_relay_enabled.load(); return true; } @@ -1684,7 +1707,7 @@ void PeerManagerImpl::RelayAddress(NodeId originator, LOCK(m_peer_mutex); for (auto& [id, peer] : m_peer_map) { - if (RelayAddrsWithPeer(*peer) && id != originator && IsAddrCompatible(*peer, addr)) { + if (peer->m_addr_relay_enabled && id != originator && IsAddrCompatible(*peer, addr)) { uint64_t hashKey = CSipHasher(hasher).Write(id).Finalize(); for (unsigned int i = 0; i < nRelayNodes; i++) { if (hashKey > best[i].first) { @@ -2574,7 +2597,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 @@ -2583,8 +2607,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()); @@ -2782,10 +2807,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())); @@ -3718,6 +3744,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) { @@ -4305,7 +4333,7 @@ void PeerManagerImpl::MaybeSendPing(CNode& node_to, Peer& peer, std::chrono::mic void PeerManagerImpl::MaybeSendAddr(CNode& node, Peer& peer, std::chrono::microseconds current_time) { // Nothing to do for non-address-relay peers - if (!RelayAddrsWithPeer(peer)) return; + if (!peer.m_addr_relay_enabled) return; LOCK(peer.m_addr_send_times_mutex); // Periodically advertise our local address to the peer. @@ -4433,6 +4461,22 @@ public: }; } +bool PeerManagerImpl::SetupAddressRelay(CNode& node, Peer& peer) +{ + // We don't participate in addr relay with outbound block-relay-only + // connections to prevent providing adversaries with the additional + // information of addr traffic to infer the link. + if (node.IsBlockOnlyConn()) return false; + + if (!peer.m_addr_relay_enabled.exchange(true)) { + // First addr message we have received from the peer, initialize + // m_addr_known + peer.m_addr_known = std::make_unique<CRollingBloomFilter>(5000, 0.001); + } + + return true; +} + bool PeerManagerImpl::SendMessages(CNode* pto) { PeerRef peer = GetPeerRef(pto->GetId()); diff --git a/src/net_processing.h b/src/net_processing.h index c537efb5db..4532a0505e 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -31,6 +31,7 @@ struct CNodeStateStats { std::vector<int> vHeightInFlight; uint64_t m_addr_processed = 0; uint64_t m_addr_rate_limited = 0; + bool m_addr_relay_enabled{false}; }; class PeerManager : public CValidationInterface, public NetEventsInterface diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index dba0f971b2..3962a13924 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -118,6 +118,7 @@ static RPCHelpMan getpeerinfo() {RPCResult::Type::STR, "addr", "(host:port) The IP address and port of the peer"}, {RPCResult::Type::STR, "addrbind", "(ip:port) Bind address of the connection to the peer"}, {RPCResult::Type::STR, "addrlocal", "(ip:port) Local address as reported by the peer"}, + {RPCResult::Type::BOOL, "addr_relay_enabled", "Whether we participate in address relay with this peer"}, {RPCResult::Type::STR, "network", "Network (" + Join(GetNetworkNames(/* append_unroutable */ true), ", ") + ")"}, {RPCResult::Type::NUM, "mapped_as", "The AS in the BGP route to the peer used for diversifying\n" "peer selection (only available if the asmap config flag is set)"}, @@ -201,6 +202,7 @@ static RPCHelpMan getpeerinfo() if (!(stats.addrLocal.empty())) { obj.pushKV("addrlocal", stats.addrLocal); } + obj.pushKV("addr_relay_enabled", statestats.m_addr_relay_enabled); obj.pushKV("network", GetNetworkName(stats.m_network)); if (stats.m_mapped_as != 0) { obj.pushKV("mapped_as", uint64_t(stats.m_mapped_as)); diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py index ff1d85a9be..95743a1bf5 100755 --- a/test/functional/p2p_addr_relay.py +++ b/test/functional/p2p_addr_relay.py @@ -11,14 +11,15 @@ from test_framework.messages import ( NODE_NETWORK, NODE_WITNESS, msg_addr, - msg_getaddr + msg_getaddr, + msg_verack ) from test_framework.p2p import ( P2PInterface, p2p_lock, ) from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal +from test_framework.util import assert_equal, assert_greater_than import random import time @@ -27,10 +28,12 @@ class AddrReceiver(P2PInterface): num_ipv4_received = 0 test_addr_contents = False _tokens = 1 + send_getaddr = True - def __init__(self, test_addr_contents=False): + def __init__(self, test_addr_contents=False, send_getaddr=True): super().__init__() self.test_addr_contents = test_addr_contents + self.send_getaddr = send_getaddr def on_addr(self, message): for addr in message.addrs: @@ -60,6 +63,11 @@ class AddrReceiver(P2PInterface): def addr_received(self): return self.num_ipv4_received != 0 + def on_version(self, message): + self.send_message(msg_verack()) + if (self.send_getaddr): + self.send_message(msg_getaddr()) + def getaddr_received(self): return self.message_count['getaddr'] > 0 @@ -75,6 +83,10 @@ class AddrTest(BitcoinTestFramework): def run_test(self): self.oversized_addr_test() self.relay_tests() + self.inbound_blackhole_tests() + + # This test populates the addrman, which can impact the node's behavior + # in subsequent tests self.getaddr_tests() self.blocksonly_mode_tests() self.rate_limit_tests() @@ -156,7 +168,7 @@ class AddrTest(BitcoinTestFramework): self.nodes[0].disconnect_p2ps() self.log.info('Check relay of addresses received from outbound peers') - inbound_peer = self.nodes[0].add_p2p_connection(AddrReceiver(test_addr_contents=True)) + inbound_peer = self.nodes[0].add_p2p_connection(AddrReceiver(test_addr_contents=True, send_getaddr=False)) full_outbound_peer = self.nodes[0].add_outbound_p2p_connection(AddrReceiver(), p2p_idx=0, connection_type="outbound-full-relay") msg = self.setup_addr_msg(2) self.send_addr_msg(full_outbound_peer, msg, [inbound_peer]) @@ -167,6 +179,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]) @@ -184,7 +199,64 @@ class AddrTest(BitcoinTestFramework): self.nodes[0].disconnect_p2ps() + def sum_addr_messages(self, msgs_dict): + return sum(bytes_received for (msg, bytes_received) in msgs_dict.items() if msg in ['addr', 'addrv2', 'getaddr']) + + def inbound_blackhole_tests(self): + self.log.info('Check that we only relay addresses to inbound peers who have previously sent us addr related messages') + + addr_source = self.nodes[0].add_p2p_connection(P2PInterface()) + receiver_peer = self.nodes[0].add_p2p_connection(AddrReceiver()) + blackhole_peer = self.nodes[0].add_p2p_connection(AddrReceiver(send_getaddr=False)) + initial_addrs_received = receiver_peer.num_ipv4_received + + peerinfo = self.nodes[0].getpeerinfo() + assert_equal(peerinfo[0]['addr_relay_enabled'], True) # addr_source + assert_equal(peerinfo[1]['addr_relay_enabled'], True) # receiver_peer + assert_equal(peerinfo[2]['addr_relay_enabled'], False) # blackhole_peer + + # addr_source sends 2 addresses to node0 + msg = self.setup_addr_msg(2) + addr_source.send_and_ping(msg) + self.mocktime += 30 * 60 + self.nodes[0].setmocktime(self.mocktime) + receiver_peer.sync_with_ping() + blackhole_peer.sync_with_ping() + + peerinfo = self.nodes[0].getpeerinfo() + + # Confirm node received addr-related messages from receiver peer + assert_greater_than(self.sum_addr_messages(peerinfo[1]['bytesrecv_per_msg']), 0) + # And that peer received addresses + assert_equal(receiver_peer.num_ipv4_received - initial_addrs_received, 2) + + # Confirm node has not received addr-related messages from blackhole peer + assert_equal(self.sum_addr_messages(peerinfo[2]['bytesrecv_per_msg']), 0) + # And that peer did not receive addresses + assert_equal(blackhole_peer.num_ipv4_received, 0) + + self.log.info("After blackhole peer sends addr message, it becomes eligible for addr gossip") + blackhole_peer.send_and_ping(msg_addr()) + + # Confirm node has now received addr-related messages from blackhole peer + assert_greater_than(self.sum_addr_messages(peerinfo[1]['bytesrecv_per_msg']), 0) + assert_equal(self.nodes[0].getpeerinfo()[2]['addr_relay_enabled'], True) + + msg = self.setup_addr_msg(2) + self.send_addr_msg(addr_source, msg, [receiver_peer, blackhole_peer]) + + # And that peer received addresses + assert_equal(blackhole_peer.num_ipv4_received, 2) + + self.nodes[0].disconnect_p2ps() + def getaddr_tests(self): + # In the previous tests, the node answered GETADDR requests with an + # empty addrman. Due to GETADDR response caching (see + # CConnman::GetAddresses), the node would continue to provide 0 addrs + # in response until enough time has passed or the node is restarted. + self.restart_node(0) + self.log.info('Test getaddr behavior') self.log.info('Check that we send a getaddr message upon connecting to an outbound-full-relay peer') full_outbound_peer = self.nodes[0].add_outbound_p2p_connection(AddrReceiver(), p2p_idx=0, connection_type="outbound-full-relay") @@ -197,7 +269,7 @@ class AddrTest(BitcoinTestFramework): assert_equal(block_relay_peer.getaddr_received(), False) self.log.info('Check that we answer getaddr messages only from inbound peers') - inbound_peer = self.nodes[0].add_p2p_connection(AddrReceiver()) + inbound_peer = self.nodes[0].add_p2p_connection(AddrReceiver(send_getaddr=False)) inbound_peer.sync_with_ping() # Add some addresses to addrman diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index cc80b543cd..b7d5bd8fab 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 |