From 4fe338ab3ed73b3ffb20eedf95500c56ec2920e1 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Mon, 19 Oct 2020 09:31:51 -0400 Subject: [PATCH] Call CAddrMan::Good() on block-relay-only peer addresses Being able to invoke Good() is important for address management (new vs tried table, tried table eviction via test-before-evict). We mitigate potential information leaks by not calling Connected() on these peer addresses. --- src/net_processing.cpp | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index a4dfab62e2c..e57ddbb1168 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2407,14 +2407,8 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat // empty and no one will know who we are, so these mechanisms are // important to help us connect to the network. // - // We also update the addrman to record connection success for - // these peers (which include OUTBOUND_FULL_RELAY and FEELER - // connections) so that addrman will have an up-to-date notion of - // which peers are online and available. - // - // We skip these operations for BLOCK_RELAY peers to avoid - // potentially leaking information about our BLOCK_RELAY - // connections via the addrman or address relay. + // We skip this for BLOCK_RELAY peers to avoid potentially leaking + // information about our BLOCK_RELAY connections via address relay. if (fListen && !::ChainstateActive().IsInitialBlockDownload()) { CAddress addr = GetLocalAddress(&pfrom.addr, pfrom.GetLocalServices()); @@ -2433,9 +2427,23 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat // Get recent addresses m_connman.PushMessage(&pfrom, CNetMsgMaker(greatest_common_version).Make(NetMsgType::GETADDR)); pfrom.fGetAddr = true; + } - // Moves address from New to Tried table in Addrman, resolves - // tried-table collisions, etc. + if (!pfrom.IsInboundConn()) { + // For non-inbound connections, we update the addrman to record + // connection success so that addrman will have an up-to-date + // notion of which peers are online and available. + // + // While we strive to not leak information about block-relay-only + // connections via the addrman, not moving an address to the tried + // table is also potentially detrimental because new-table entries + // are subject to eviction in the event of addrman collisions. We + // mitigate the information-leak by never calling + // CAddrMan::Connected() on block-relay-only peers; see + // FinalizeNode(). + // + // This moves an address from New to Tried table in Addrman, + // resolves tried-table collisions, etc. m_connman.MarkAddressGood(pfrom.addr); }