From 91d61952a82af3e8887e8ae532ecc19d87fe9073 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Tue, 1 Sep 2020 16:32:09 -0400 Subject: [PATCH 1/4] Simplify and clarify extra outbound peer counting --- src/init.cpp | 2 +- src/net.cpp | 10 +++++----- src/net.h | 2 +- src/net_processing.cpp | 2 +- src/test/fuzz/connman.cpp | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 5460c9b2b00..09be3d01fa9 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -200,7 +200,7 @@ void Shutdown(NodeContext& node) // using the other before destroying them. if (node.peerman) UnregisterValidationInterface(node.peerman.get()); // Follow the lock order requirements: - // * CheckForStaleTipAndEvictPeers locks cs_main before indirectly calling GetExtraOutboundCount + // * CheckForStaleTipAndEvictPeers locks cs_main before indirectly calling GetExtraFullOutboundCount // which locks cs_vNodes. // * ProcessMessage locks cs_main and g_cs_orphans before indirectly calling ForEachNode which // locks cs_vNodes. diff --git a/src/net.cpp b/src/net.cpp index bbb85694e74..fe1baf97a32 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1827,18 +1827,18 @@ void CConnman::SetTryNewOutboundPeer(bool flag) // Also exclude peers that haven't finished initial connection handshake yet // (so that we don't decide we're over our desired connection limit, and then // evict some peer that has finished the handshake) -int CConnman::GetExtraOutboundCount() +int CConnman::GetExtraFullOutboundCount() { - int nOutbound = 0; + int full_outbound_peers = 0; { LOCK(cs_vNodes); for (const CNode* pnode : vNodes) { - if (pnode->fSuccessfullyConnected && !pnode->fDisconnect && pnode->IsOutboundOrBlockRelayConn()) { - ++nOutbound; + if (pnode->fSuccessfullyConnected && !pnode->fDisconnect && pnode->IsFullOutboundConn()) { + ++full_outbound_peers; } } } - return std::max(nOutbound - m_max_outbound_full_relay - m_max_outbound_block_relay, 0); + return std::max(full_outbound_peers - m_max_outbound_full_relay, 0); } void CConnman::ThreadOpenConnections(const std::vector connect) diff --git a/src/net.h b/src/net.h index 504855c3863..e959718faba 100644 --- a/src/net.h +++ b/src/net.h @@ -336,7 +336,7 @@ public: // return a value less than (num_outbound_connections - num_outbound_slots) // in cases where some outbound connections are not yet fully connected, or // not yet fully disconnected. - int GetExtraOutboundCount(); + int GetExtraFullOutboundCount(); bool AddNode(const std::string& node); bool RemoveAddedNode(const std::string& node); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index cdddde85407..243ac6a34b6 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3910,7 +3910,7 @@ void PeerManager::ConsiderEviction(CNode& pto, int64_t time_in_seconds) void PeerManager::EvictExtraOutboundPeers(int64_t time_in_seconds) { // Check whether we have too many outbound peers - int extra_peers = m_connman.GetExtraOutboundCount(); + int extra_peers = m_connman.GetExtraFullOutboundCount(); if (extra_peers > 0) { // If we have more outbound peers than we target, disconnect one. // Pick the outbound peer that least recently announced diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index 6521c3f3b20..bb97f58cf26 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -145,7 +145,7 @@ void test_one_input(const std::vector& buffer) } (void)connman.GetAddedNodeInfo(); (void)connman.GetBestHeight(); - (void)connman.GetExtraOutboundCount(); + (void)connman.GetExtraFullOutboundCount(); (void)connman.GetLocalServices(); (void)connman.GetMaxOutboundTarget(); (void)connman.GetMaxOutboundTimeframe(); From 3cc8a7a0f5fa183cd7f0cf5e56f16f9a9d1f2441 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Tue, 1 Sep 2020 16:49:40 -0400 Subject: [PATCH 2/4] Use conn_type to identify block-relay peers, rather than m_tx_relay == nullptr --- src/net_processing.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 243ac6a34b6..ffbd2c17f93 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2466,7 +2466,7 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat LogPrintf("New outbound peer connected: version: %d, blocks=%d, peer=%d%s (%s)\n", pfrom.nVersion.load(), pfrom.nStartingHeight, pfrom.GetId(), (fLogIPs ? strprintf(", peeraddr=%s", pfrom.addr.ToString()) : ""), - pfrom.m_tx_relay == nullptr ? "block-relay" : "full-relay"); + pfrom.IsBlockOnlyConn() ? "block-relay" : "full-relay"); } if (pfrom.GetCommonVersion() >= SENDHEADERS_VERSION) { @@ -3923,13 +3923,11 @@ void PeerManager::EvictExtraOutboundPeers(int64_t time_in_seconds) AssertLockHeld(::cs_main); // Ignore non-outbound peers, or nodes marked for disconnect already - if (!pnode->IsOutboundOrBlockRelayConn() || pnode->fDisconnect) return; + if (!pnode->IsFullOutboundConn() || pnode->fDisconnect) return; CNodeState *state = State(pnode->GetId()); if (state == nullptr) return; // shouldn't be possible, but just in case // Don't evict our protected peers if (state->m_chain_sync.m_protect) return; - // Don't evict our block-relay-only peers. - if (pnode->m_tx_relay == nullptr) return; if (state->m_last_block_announcement < oldest_block_announcement || (state->m_last_block_announcement == oldest_block_announcement && pnode->GetId() > worst_peer)) { worst_peer = pnode->GetId(); oldest_block_announcement = state->m_last_block_announcement; From daffaf03fbede6c01287779e464379ee3acb005a Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Tue, 1 Sep 2020 17:05:47 -0400 Subject: [PATCH 3/4] Periodically make block-relay connections and sync headers To make eclipse attacks more difficult, regularly initiate outbound connections and stay connected long enough to sync headers and potentially learn of new blocks. If we learn a new block, rotate out an existing block-relay peer in favor of the new peer. This augments the existing outbound peer rotation that exists -- currently we make new full-relay connections when our tip is stale, which we disconnect after waiting a small time to see if we learn a new block. As block-relay connections use minimal bandwidth, we can make these connections regularly and not just when our tip is stale. Like feeler connections, these connections are not aggressive; whenever our timer fires (once every 5 minutes on average), we'll try to initiate a new block-relay connection as described, but if we fail to connect we just wait for our timer to fire again before repeating with a new peer. --- src/net.cpp | 44 +++++++++++++++++++++++++++++++++-- src/net.h | 15 ++++++++++++ src/net_processing.cpp | 52 ++++++++++++++++++++++++++++++++++++++++-- src/net_processing.h | 4 ++++ 4 files changed, 111 insertions(+), 4 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index fe1baf97a32..7cb91f13885 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1841,6 +1841,20 @@ int CConnman::GetExtraFullOutboundCount() return std::max(full_outbound_peers - m_max_outbound_full_relay, 0); } +int CConnman::GetExtraBlockRelayCount() +{ + int block_relay_peers = 0; + { + LOCK(cs_vNodes); + for (const CNode* pnode : vNodes) { + if (pnode->fSuccessfullyConnected && !pnode->fDisconnect && pnode->IsBlockOnlyConn()) { + ++block_relay_peers; + } + } + } + return std::max(block_relay_peers - m_max_outbound_block_relay, 0); +} + void CConnman::ThreadOpenConnections(const std::vector connect) { // Connect to specific addresses @@ -1869,6 +1883,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect) // Minimum time before next feeler connection (in microseconds). int64_t nNextFeeler = PoissonNextSend(nStart*1000*1000, FEELER_INTERVAL); + int64_t nNextExtraBlockRelay = PoissonNextSend(nStart*1000*1000, EXTRA_BLOCK_RELAY_ONLY_PEER_INTERVAL); while (!interruptNet) { ProcessAddrFetch(); @@ -1941,8 +1956,9 @@ void CConnman::ThreadOpenConnections(const std::vector connect) // until we hit our block-relay-only peer limit. // GetTryNewOutboundPeer() gets set when a stale tip is detected, so we // try opening an additional OUTBOUND_FULL_RELAY connection. If none of - // these conditions are met, check the nNextFeeler timer to decide if - // we should open a FEELER. + // these conditions are met, check to see if it's time to try an extra + // block-relay-only peer (to confirm our tip is current, see below) or the nNextFeeler + // timer to decide if we should open a FEELER. if (!m_anchors.empty() && (nOutboundBlockRelay < m_max_outbound_block_relay)) { conn_type = ConnectionType::BLOCK_RELAY; @@ -1953,6 +1969,30 @@ void CConnman::ThreadOpenConnections(const std::vector connect) conn_type = ConnectionType::BLOCK_RELAY; } else if (GetTryNewOutboundPeer()) { // OUTBOUND_FULL_RELAY + } else if (nTime > nNextExtraBlockRelay && m_start_extra_block_relay_peers) { + // Periodically connect to a peer (using regular outbound selection + // methodology from addrman) and stay connected long enough to sync + // headers, but not much else. + // + // Then disconnect the peer, if we haven't learned anything new. + // + // The idea is to make eclipse attacks very difficult to pull off, + // because every few minutes we're finding a new peer to learn headers + // from. + // + // This is similar to the logic for trying extra outbound (full-relay) + // peers, except: + // - we do this all the time on a poisson timer, rather than just when + // our tip is stale + // - we potentially disconnect our next-youngest block-relay-only peer, if our + // newest block-relay-only peer delivers a block more recently. + // See the eviction logic in net_processing.cpp. + // + // Because we can promote these connections to block-relay-only + // connections, they do not get their own ConnectionType enum + // (similar to how we deal with extra outbound peers). + nNextExtraBlockRelay = PoissonNextSend(nTime, EXTRA_BLOCK_RELAY_ONLY_PEER_INTERVAL); + conn_type = ConnectionType::BLOCK_RELAY; } else if (nTime > nNextFeeler) { nNextFeeler = PoissonNextSend(nTime, FEELER_INTERVAL); conn_type = ConnectionType::FEELER; diff --git a/src/net.h b/src/net.h index e959718faba..41f7fa93ff8 100644 --- a/src/net.h +++ b/src/net.h @@ -48,6 +48,8 @@ static const bool DEFAULT_WHITELISTFORCERELAY = false; static const int TIMEOUT_INTERVAL = 20 * 60; /** Run the feeler connection loop once every 2 minutes or 120 seconds. **/ static const int FEELER_INTERVAL = 120; +/** Run the extra block-relay-only connection loop once every 5 minutes. **/ +static const int EXTRA_BLOCK_RELAY_ONLY_PEER_INTERVAL = 300; /** The maximum number of addresses from our addrman to return in response to a getaddr message. */ static constexpr size_t MAX_ADDR_TO_SEND = 1000; /** Maximum length of incoming protocol messages (no message over 4 MB is currently acceptable). */ @@ -330,6 +332,11 @@ public: void SetTryNewOutboundPeer(bool flag); bool GetTryNewOutboundPeer(); + void StartExtraBlockRelayPeers() { + LogPrint(BCLog::NET, "net: enabling extra block-relay-only peers\n"); + m_start_extra_block_relay_peers = true; + } + // Return the number of outbound peers we have in excess of our target (eg, // if we previously called SetTryNewOutboundPeer(true), and have since set // to false, we may have extra peers that we wish to disconnect). This may @@ -337,6 +344,8 @@ public: // in cases where some outbound connections are not yet fully connected, or // not yet fully disconnected. int GetExtraFullOutboundCount(); + // Count the number of block-relay-only peers we have over our limit. + int GetExtraBlockRelayCount(); bool AddNode(const std::string& node); bool RemoveAddedNode(const std::string& node); @@ -594,6 +603,12 @@ private: * This takes the place of a feeler connection */ std::atomic_bool m_try_another_outbound_peer; + /** flag for initiating extra block-relay-only peer connections. + * this should only be enabled after initial chain sync has occurred, + * as these connections are intended to be short-lived and low-bandwidth. + */ + std::atomic_bool m_start_extra_block_relay_peers{false}; + std::atomic m_next_send_inv_to_incoming{0}; /** diff --git a/src/net_processing.cpp b/src/net_processing.cpp index ffbd2c17f93..7dee2db3c86 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3909,9 +3909,52 @@ void PeerManager::ConsiderEviction(CNode& pto, int64_t time_in_seconds) void PeerManager::EvictExtraOutboundPeers(int64_t time_in_seconds) { + // If we have any extra block-relay-only peers, disconnect the youngest unless + // it's given us a block -- in which case, compare with the second-youngest, and + // out of those two, disconnect the peer who least recently gave us a block. + // The youngest block-relay-only peer would be the extra peer we connected + // to temporarily in order to sync our tip; see net.cpp. + // Note that we use higher nodeid as a measure for most recent connection. + if (m_connman.GetExtraBlockRelayCount() > 0) { + std::pair youngest_peer{-1, 0}, next_youngest_peer{-1, 0}; + + m_connman.ForEachNode([&](CNode* pnode) { + if (!pnode->IsBlockOnlyConn() || pnode->fDisconnect) return; + if (pnode->GetId() > youngest_peer.first) { + next_youngest_peer = youngest_peer; + youngest_peer.first = pnode->GetId(); + youngest_peer.second = pnode->nLastBlockTime; + } + }); + NodeId to_disconnect = youngest_peer.first; + if (youngest_peer.second > next_youngest_peer.second) { + // Our newest block-relay-only peer gave us a block more recently; + // disconnect our second youngest. + to_disconnect = next_youngest_peer.first; + } + m_connman.ForNode(to_disconnect, [&](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { + AssertLockHeld(::cs_main); + // Make sure we're not getting a block right now, and that + // we've been connected long enough for this eviction to happen + // at all. + // Note that we only request blocks from a peer if we learn of a + // valid headers chain with at least as much work as our tip. + CNodeState *node_state = State(pnode->GetId()); + if (node_state == nullptr || + (time_in_seconds - pnode->nTimeConnected >= MINIMUM_CONNECT_TIME && node_state->nBlocksInFlight == 0)) { + pnode->fDisconnect = true; + LogPrint(BCLog::NET, "disconnecting extra block-relay-only peer=%d (last block received at time %d)\n", pnode->GetId(), pnode->nLastBlockTime); + return true; + } else { + LogPrint(BCLog::NET, "keeping block-relay-only peer=%d chosen for eviction (connect time: %d, blocks_in_flight: %d)\n", + pnode->GetId(), pnode->nTimeConnected, node_state->nBlocksInFlight); + } + return false; + }); + } + // Check whether we have too many outbound peers - int extra_peers = m_connman.GetExtraFullOutboundCount(); - if (extra_peers > 0) { + if (m_connman.GetExtraFullOutboundCount() > 0) { // If we have more outbound peers than we target, disconnect one. // Pick the outbound peer that least recently announced // us a new block, with ties broken by choosing the more recent @@ -3983,6 +4026,11 @@ void PeerManager::CheckForStaleTipAndEvictPeers() } m_stale_tip_check_time = time_in_seconds + STALE_CHECK_INTERVAL; } + + if (!m_initial_sync_finished && CanDirectFetch(m_chainparams.GetConsensus())) { + m_connman.StartExtraBlockRelayPeers(); + m_initial_sync_finished = true; + } } namespace { diff --git a/src/net_processing.h b/src/net_processing.h index d5b54dae561..12a4e9c38f8 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -206,6 +206,10 @@ private: //* Whether this node is running in blocks only mode */ const bool m_ignore_incoming_txs; + /** Whether we've completed initial sync yet, for determining when to turn + * on extra block-relay-only peers. */ + bool m_initial_sync_finished{false}; + /** Protects m_peer_map */ mutable Mutex m_peer_mutex; /** From b3a515c0bec97633a76bec101af47c3c90c0b749 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Thu, 3 Sep 2020 10:41:02 -0400 Subject: [PATCH 4/4] Clarify comments around outbound peer eviction --- src/net_processing.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 7dee2db3c86..200c286f5d7 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3953,10 +3953,10 @@ void PeerManager::EvictExtraOutboundPeers(int64_t time_in_seconds) }); } - // Check whether we have too many outbound peers + // Check whether we have too many OUTBOUND_FULL_RELAY peers if (m_connman.GetExtraFullOutboundCount() > 0) { - // If we have more outbound peers than we target, disconnect one. - // Pick the outbound peer that least recently announced + // If we have more OUTBOUND_FULL_RELAY peers than we target, disconnect one. + // Pick the OUTBOUND_FULL_RELAY peer that least recently announced // us a new block, with ties broken by choosing the more recent // connection (higher node id) NodeId worst_peer = -1; @@ -3965,7 +3965,8 @@ void PeerManager::EvictExtraOutboundPeers(int64_t time_in_seconds) m_connman.ForEachNode([&](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { AssertLockHeld(::cs_main); - // Ignore non-outbound peers, or nodes marked for disconnect already + // Only consider OUTBOUND_FULL_RELAY peers that are not already + // marked for disconnection if (!pnode->IsFullOutboundConn() || pnode->fDisconnect) return; CNodeState *state = State(pnode->GetId()); if (state == nullptr) return; // shouldn't be possible, but just in case