Page MenuHomePhabricator
Files F13711291

D10933.id32067.diff
No OneTemporary
Actions

D10933.id32067.diff
View Options

diff --git a/src/net_processing.h b/src/net_processing.h
--- a/src/net_processing.h
+++ b/src/net_processing.h
@@ -43,6 +43,7 @@
int m_starting_height = -1;
std::chrono::microseconds m_ping_wait;
std::vector<int> vHeightInFlight;
+ bool m_addr_relay_enabled{false};
};
class PeerManager : public CValidationInterface, public NetEventsInterface {
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -369,10 +369,33 @@
*/
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.
+ * 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.
*/
- const std::unique_ptr<CRollingBloomFilter> m_addr_known;
+ 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. */
@@ -402,11 +425,7 @@
/** 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)
- : m_id(id), m_addr_known{
- addr_relay
- ? std::make_unique<CRollingBloomFilter>(5000, 0.001)
- : nullptr} {}
+ explicit Peer(NodeId id) : m_id(id) {}
};
using PeerRef = std::shared_ptr<Peer>;
@@ -632,6 +651,15 @@
* their own locks.
*/
std::map<NodeId, PeerRef> m_peer_map GUARDED_BY(m_peer_mutex);
+
+ /**
+ * 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
@@ -915,10 +943,6 @@
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
@@ -1427,10 +1451,7 @@
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));
}
@@ -1604,6 +1625,7 @@
}
stats.m_ping_wait = ping_wait;
+ stats.m_addr_relay_enabled = peer->m_addr_relay_enabled.load();
return true;
}
@@ -2273,7 +2295,7 @@
LOCK(m_peer_mutex);
for (auto &[id, peer] : m_peer_map) {
- if (RelayAddrsWithPeer(*peer) && id != originator &&
+ 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++) {
@@ -3453,7 +3475,8 @@
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
@@ -3462,8 +3485,9 @@
// 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 peers to avoid potentially leaking
- // information about our BLOCK_RELAY 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 && !::ChainstateActive().IsInitialBlockDownload()) {
CAddress addr =
GetLocalAddress(&pfrom.addr, pfrom.GetLocalServices());
@@ -3632,11 +3656,12 @@
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() > GetMaxAddrToSend()) {
Misbehaving(
pfrom, 20,
@@ -4952,6 +4977,8 @@
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) {
@@ -5718,7 +5745,7 @@
void PeerManagerImpl::MaybeSendAddr(CNode &node, Peer &peer,
std::chrono::microseconds current_time) {
// Nothing to do for non-address-relay peers
- if (!RelayAddrsWithPeer(peer)) {
+ if (!peer.m_addr_relay_enabled) {
return;
}
@@ -5813,6 +5840,23 @@
};
} // namespace
+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(const Config &config, CNode *pto) {
PeerRef peer = GetPeerRef(pto->GetId());
if (!peer) {
diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -106,6 +106,8 @@
"(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 (ipv4, ipv6, or onion) the peer connected "
"through"},
@@ -232,6 +234,8 @@
if (!(stats.addrLocal.empty())) {
obj.pushKV("addrlocal", stats.addrLocal);
}
+ obj.pushKV("addr_relay_enabled",
+ statestats.m_addr_relay_enabled);
obj.pushKV("network", 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
--- a/test/functional/p2p_addr_relay.py
+++ b/test/functional/p2p_addr_relay.py
@@ -13,19 +13,22 @@
CAddress,
msg_addr,
msg_getaddr,
+ msg_verack,
)
from test_framework.p2p import P2PInterface
from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import assert_equal
+from test_framework.util import assert_equal, assert_greater_than
class AddrReceiver(P2PInterface):
num_ipv4_received = 0
test_addr_contents = False
+ 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:
@@ -42,6 +45,11 @@
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
@@ -56,6 +64,10 @@
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()
@@ -132,7 +144,7 @@
self.log.info('Check relay of addresses received from outbound peers')
inbound_peer = self.nodes[0].add_p2p_connection(
- AddrReceiver(test_addr_contents=True))
+ 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)
@@ -146,6 +158,10 @@
# 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)
@@ -169,7 +185,82 @@
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')
@@ -187,7 +278,8 @@
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
--- a/test/functional/test_framework/p2p.py
+++ b/test/functional/test_framework/p2p.py
@@ -488,6 +488,7 @@
if self.support_addrv2:
self.send_message(msg_sendaddrv2())
self.nServices = message.nServices
+ self.send_message(msg_getaddr())
# Connection helper methods

File Metadata

Mime Type
text/plain
Expires
Sat, Apr 26, 11:23 (16 h, 28 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
5570118
Default Alt Text
D10933.id32067.diff (15 KB)

Event Timeline