diff options
| -rw-r--r-- | src/net.cpp | 35 | ||||
| -rw-r--r-- | src/net.h | 16 | ||||
| -rw-r--r-- | src/test/net_peer_eviction_tests.cpp | 12 |
3 files changed, 45 insertions, 18 deletions
diff --git a/src/net.cpp b/src/net.cpp index 7efa9ea10e..66d81b115f 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -840,6 +840,12 @@ static bool CompareLocalHostTimeConnected(const NodeEvictionCandidate &a, const return a.nTimeConnected > b.nTimeConnected; } +static bool CompareOnionTimeConnected(const NodeEvictionCandidate& a, const NodeEvictionCandidate& b) +{ + if (a.m_is_onion != b.m_is_onion) return b.m_is_onion; + return a.nTimeConnected > b.nTimeConnected; +} + static bool CompareNetGroupKeyed(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) { return a.nKeyedNetGroup < b.nKeyedNetGroup; } @@ -885,16 +891,31 @@ void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& vEvict { // Protect the half of the remaining nodes which have been connected the longest. // This replicates the non-eviction implicit behavior, and precludes attacks that start later. - // Reserve half of these protected spots for localhost peers, even if - // they're not longest-uptime overall. This helps protect tor peers, which - // tend to be otherwise disadvantaged under our eviction criteria. + // To favorise the diversity of our peer connections, reserve up to (half + 2) of + // these protected spots for onion and localhost peers, if any, even if they're not + // longest uptime overall. This helps protect tor peers, which tend to be otherwise + // disadvantaged under our eviction criteria. const size_t initial_size = vEvictionCandidates.size(); size_t total_protect_size = initial_size / 2; + const size_t onion_protect_size = total_protect_size / 2; - // Pick out up to 1/4 peers that are localhost, sorted by longest uptime. - const size_t local_erase_size = total_protect_size / 2; - EraseLastKElements(vEvictionCandidates, CompareLocalHostTimeConnected, local_erase_size, - [](const NodeEvictionCandidate& n) { return n.m_is_local; }); + if (onion_protect_size) { + // Pick out up to 1/4 peers connected via our onion service, sorted by longest uptime. + EraseLastKElements(vEvictionCandidates, CompareOnionTimeConnected, onion_protect_size, + [](const NodeEvictionCandidate& n) { return n.m_is_onion; }); + } + + const size_t localhost_min_protect_size{2}; + if (onion_protect_size >= localhost_min_protect_size) { + // Allocate any remaining slots of the 1/4, or minimum 2 additional slots, + // to localhost peers, sorted by longest uptime, as manually configured + // hidden services not using `-bind=addr[:port]=onion` will not be detected + // as inbound onion connections. + const size_t remaining_tor_slots{onion_protect_size - (initial_size - vEvictionCandidates.size())}; + const size_t localhost_protect_size{std::max(remaining_tor_slots, localhost_min_protect_size)}; + EraseLastKElements(vEvictionCandidates, CompareLocalHostTimeConnected, localhost_protect_size, + [](const NodeEvictionCandidate& n) { return n.m_is_local; }); + } // Calculate how many we removed, and update our total number of peers that // we want to protect based on uptime accordingly. @@ -1288,9 +1288,9 @@ struct NodeEvictionCandidate /** * Select an inbound peer to evict after filtering out (protecting) peers having * distinct, difficult-to-forge characteristics. The protection logic picks out - * fixed numbers of desirable peers per various criteria, followed by ratios of - * desirable or disadvantaged peers. If any eviction candidates remain, the - * selection logic chooses a peer to evict. + * fixed numbers of desirable peers per various criteria, followed by (mostly) + * ratios of desirable or disadvantaged peers. If any eviction candidates + * remain, the selection logic chooses a peer to evict. */ [[nodiscard]] std::optional<NodeId> SelectNodeToEvict(std::vector<NodeEvictionCandidate>&& vEvictionCandidates); @@ -1300,9 +1300,13 @@ struct NodeEvictionCandidate * longest, to replicate the non-eviction implicit behavior and preclude attacks * that start later. * - * Half of these protected spots (1/4 of the total) are reserved for localhost - * peers, if any, sorted by longest uptime, even if they're not longest uptime - * overall. + * Half of these protected spots (1/4 of the total) are reserved for onion peers + * connected via our tor control service, if any, sorted by longest uptime, even + * if they're not longest uptime overall. Any remaining slots of the 1/4 are + * then allocated to protect localhost peers, if any (or up to 2 localhost peers + * if no slots remain and 2 or more onion peers were protected), sorted by + * longest uptime, as manually configured hidden services not using + * `-bind=addr[:port]=onion` will not be detected as inbound onion connections. * * This helps protect onion peers, which tend to be otherwise disadvantaged * under our eviction criteria for their higher min ping times relative to IPv4 diff --git a/src/test/net_peer_eviction_tests.cpp b/src/test/net_peer_eviction_tests.cpp index 517474bad4..0bd0aefcee 100644 --- a/src/test/net_peer_eviction_tests.cpp +++ b/src/test/net_peer_eviction_tests.cpp @@ -94,7 +94,7 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) BOOST_CHECK(IsProtected( num_peers, [](NodeEvictionCandidate& c) { c.nTimeConnected = c.id; - c.m_is_local = false; + c.m_is_onion = c.m_is_local = false; }, /* protected_peer_ids */ {0, 1, 2, 3, 4, 5}, /* unprotected_peer_ids */ {6, 7, 8, 9, 10, 11}, @@ -104,7 +104,7 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) BOOST_CHECK(IsProtected( num_peers, [num_peers](NodeEvictionCandidate& c) { c.nTimeConnected = num_peers - c.id; - c.m_is_local = false; + c.m_is_onion = c.m_is_local = false; }, /* protected_peer_ids */ {6, 7, 8, 9, 10, 11}, /* unprotected_peer_ids */ {0, 1, 2, 3, 4, 5}, @@ -113,20 +113,22 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) // Test protection of localhost peers... // Expect 1/4 localhost peers to be protected from eviction, - // independently of other characteristics. + // if no onion peers. BOOST_CHECK(IsProtected( num_peers, [](NodeEvictionCandidate& c) { + c.m_is_onion = false; c.m_is_local = (c.id == 1 || c.id == 9 || c.id == 11); }, /* protected_peer_ids */ {1, 9, 11}, /* unprotected_peer_ids */ {}, random_context)); - // Expect 1/4 localhost peers and 1/4 of the others to be protected - // from eviction, sorted by longest uptime (lowest nTimeConnected). + // Expect 1/4 localhost peers and 1/4 of the other peers to be protected, + // sorted by longest uptime (lowest nTimeConnected), if no onion peers. BOOST_CHECK(IsProtected( num_peers, [](NodeEvictionCandidate& c) { c.nTimeConnected = c.id; + c.m_is_onion = false; c.m_is_local = (c.id > 6); }, /* protected_peer_ids */ {0, 1, 2, 7, 8, 9}, |