From daf55531260833d597ee599e2d289ea1be0b1d9c Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Sat, 24 Oct 2020 03:16:04 -0400 Subject: Avoid calling CAddrMan::Connected() on block-relay-only peer addresses Connected() updates the time we serve in addr messages, so avoid leaking block-relay-only peer connections by avoiding these calls. --- src/net_processing.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 94d4052fa..a4dfab62e 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -837,7 +837,8 @@ void PeerManager::ReattemptInitialBroadcast(CScheduler& scheduler) const scheduler.scheduleFromNow([&] { ReattemptInitialBroadcast(scheduler); }, delta); } -void PeerManager::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTime) { +void PeerManager::FinalizeNode(const CNode& node, bool& fUpdateConnectionTime) { + NodeId nodeid = node.GetId(); fUpdateConnectionTime = false; LOCK(cs_main); int misbehavior{0}; @@ -854,7 +855,8 @@ void PeerManager::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTime) { if (state->fSyncStarted) nSyncStarted--; - if (misbehavior == 0 && state->fCurrentlyConnected) { + if (misbehavior == 0 && state->fCurrentlyConnected && !node.IsBlockOnlyConn()) { + // Note: we avoid changing visible addrman state for block-relay-only peers fUpdateConnectionTime = true; } -- cgit v1.2.3 From 4fe338ab3ed73b3ffb20eedf95500c56ec2920e1 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Mon, 19 Oct 2020 09:31:51 -0400 Subject: 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(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index a4dfab62e..e57ddbb11 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); } -- cgit v1.2.3