From 1cde8005233d163723d4d5bf1bacf22e6cb7a07e Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Sat, 10 Apr 2021 18:05:47 +0200 Subject: p2p, refactor: rm redundant erase_size calculation in SelectNodeToEvict() as EraseLastKElements() called in the next line performs the same operation. Thanks to Martin Zumsande (lightlike) for seeing this while reviewing. Co-authored-by: Martin Zumsande --- src/net.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'src') diff --git a/src/net.cpp b/src/net.cpp index 6f9f17ed4e..901132daa3 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -944,8 +944,7 @@ void ProtectEvictionCandidatesByRatio(std::vector& vEvict // An attacker cannot manipulate this metric without performing useful work. EraseLastKElements(vEvictionCandidates, CompareNodeTXTime, 4); // Protect up to 8 non-tx-relay peers that have sent us novel blocks. - const size_t erase_size = std::min(size_t(8), vEvictionCandidates.size()); - EraseLastKElements(vEvictionCandidates, CompareNodeBlockRelayOnlyTime, erase_size, + EraseLastKElements(vEvictionCandidates, CompareNodeBlockRelayOnlyTime, 8, [](const NodeEvictionCandidate& n) { return !n.fRelayTxes && n.fRelevantServices; }); // Protect 4 nodes that most recently sent us novel blocks. -- cgit v1.2.3 From 519e76bb64d03ecac175ec33c31e37d0e90f037f Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Mon, 19 Apr 2021 15:08:25 +0200 Subject: test: speed up and simplify peer_eviction_test This speeds up the test significantly, which helps when running it repeatedly. Suggest reviewing the diff with: colorMoved = dimmed-zebra colorMovedWs = allow-indentation-change --- src/test/net_peer_eviction_tests.cpp | 171 +++++++++++++++++------------------ 1 file changed, 82 insertions(+), 89 deletions(-) (limited to 'src') diff --git a/src/test/net_peer_eviction_tests.cpp b/src/test/net_peer_eviction_tests.cpp index 31d391bf7d..3b981cf69c 100644 --- a/src/test/net_peer_eviction_tests.cpp +++ b/src/test/net_peer_eviction_tests.cpp @@ -15,11 +15,6 @@ BOOST_FIXTURE_TEST_SUITE(net_peer_eviction_tests, BasicTestingSetup) -namespace { -constexpr int NODE_EVICTION_TEST_ROUNDS{10}; -constexpr int NODE_EVICTION_TEST_UP_TO_N_NODES{200}; -} // namespace - std::vector GetRandomNodeEvictionCandidates(const int n_candidates, FastRandomContext& random_context) { std::vector candidates; @@ -257,91 +252,89 @@ BOOST_AUTO_TEST_CASE(peer_eviction_test) { FastRandomContext random_context{true}; - for (int i = 0; i < NODE_EVICTION_TEST_ROUNDS; ++i) { - for (int number_of_nodes = 0; number_of_nodes < NODE_EVICTION_TEST_UP_TO_N_NODES; ++number_of_nodes) { - // Four nodes with the highest keyed netgroup values should be - // protected from eviction. - BOOST_CHECK(!IsEvicted( - number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { - candidate.nKeyedNetGroup = number_of_nodes - candidate.id; - }, - {0, 1, 2, 3}, random_context)); - - // Eight nodes with the lowest minimum ping time should be protected - // from eviction. - BOOST_CHECK(!IsEvicted( - number_of_nodes, [](NodeEvictionCandidate& candidate) { - candidate.m_min_ping_time = std::chrono::microseconds{candidate.id}; - }, - {0, 1, 2, 3, 4, 5, 6, 7}, random_context)); - - // Four nodes that most recently sent us novel transactions accepted - // into our mempool should be protected from eviction. - BOOST_CHECK(!IsEvicted( - number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { - candidate.nLastTXTime = number_of_nodes - candidate.id; - }, - {0, 1, 2, 3}, random_context)); - - // Up to eight non-tx-relay peers that most recently sent us novel - // blocks should be protected from eviction. - BOOST_CHECK(!IsEvicted( - number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { - candidate.nLastBlockTime = number_of_nodes - candidate.id; - if (candidate.id <= 7) { - candidate.fRelayTxes = false; - candidate.fRelevantServices = true; - } - }, - {0, 1, 2, 3, 4, 5, 6, 7}, random_context)); - - // Four peers that most recently sent us novel blocks should be - // protected from eviction. - BOOST_CHECK(!IsEvicted( - number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { - candidate.nLastBlockTime = number_of_nodes - candidate.id; - }, - {0, 1, 2, 3}, random_context)); - - // Combination of the previous two tests. - BOOST_CHECK(!IsEvicted( - number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { - candidate.nLastBlockTime = number_of_nodes - candidate.id; - if (candidate.id <= 7) { - candidate.fRelayTxes = false; - candidate.fRelevantServices = true; - } - }, - {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}, random_context)); - - // Combination of all tests above. - BOOST_CHECK(!IsEvicted( - number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { - candidate.nKeyedNetGroup = number_of_nodes - candidate.id; // 4 protected - candidate.m_min_ping_time = std::chrono::microseconds{candidate.id}; // 8 protected - candidate.nLastTXTime = number_of_nodes - candidate.id; // 4 protected - candidate.nLastBlockTime = number_of_nodes - candidate.id; // 4 protected - }, - {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19}, random_context)); - - // An eviction is expected given >= 29 random eviction candidates. The eviction logic protects at most - // four peers by net group, eight by lowest ping time, four by last time of novel tx, up to eight non-tx-relay - // peers by last novel block time, and four more peers by last novel block time. - if (number_of_nodes >= 29) { - BOOST_CHECK(SelectNodeToEvict(GetRandomNodeEvictionCandidates(number_of_nodes, random_context))); - } - - // No eviction is expected given <= 20 random eviction candidates. The eviction logic protects at least - // four peers by net group, eight by lowest ping time, four by last time of novel tx and four peers by last - // novel block time. - if (number_of_nodes <= 20) { - BOOST_CHECK(!SelectNodeToEvict(GetRandomNodeEvictionCandidates(number_of_nodes, random_context))); - } - - // Cases left to test: - // * "If any remaining peers are preferred for eviction consider only them. [...]" - // * "Identify the network group with the most connections and youngest member. [...]" + for (int number_of_nodes = 0; number_of_nodes < 200; ++number_of_nodes) { + // Four nodes with the highest keyed netgroup values should be + // protected from eviction. + BOOST_CHECK(!IsEvicted( + number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { + candidate.nKeyedNetGroup = number_of_nodes - candidate.id; + }, + {0, 1, 2, 3}, random_context)); + + // Eight nodes with the lowest minimum ping time should be protected + // from eviction. + BOOST_CHECK(!IsEvicted( + number_of_nodes, [](NodeEvictionCandidate& candidate) { + candidate.m_min_ping_time = std::chrono::microseconds{candidate.id}; + }, + {0, 1, 2, 3, 4, 5, 6, 7}, random_context)); + + // Four nodes that most recently sent us novel transactions accepted + // into our mempool should be protected from eviction. + BOOST_CHECK(!IsEvicted( + number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { + candidate.nLastTXTime = number_of_nodes - candidate.id; + }, + {0, 1, 2, 3}, random_context)); + + // Up to eight non-tx-relay peers that most recently sent us novel + // blocks should be protected from eviction. + BOOST_CHECK(!IsEvicted( + number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { + candidate.nLastBlockTime = number_of_nodes - candidate.id; + if (candidate.id <= 7) { + candidate.fRelayTxes = false; + candidate.fRelevantServices = true; + } + }, + {0, 1, 2, 3, 4, 5, 6, 7}, random_context)); + + // Four peers that most recently sent us novel blocks should be + // protected from eviction. + BOOST_CHECK(!IsEvicted( + number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { + candidate.nLastBlockTime = number_of_nodes - candidate.id; + }, + {0, 1, 2, 3}, random_context)); + + // Combination of the previous two tests. + BOOST_CHECK(!IsEvicted( + number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { + candidate.nLastBlockTime = number_of_nodes - candidate.id; + if (candidate.id <= 7) { + candidate.fRelayTxes = false; + candidate.fRelevantServices = true; + } + }, + {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}, random_context)); + + // Combination of all tests above. + BOOST_CHECK(!IsEvicted( + number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { + candidate.nKeyedNetGroup = number_of_nodes - candidate.id; // 4 protected + candidate.m_min_ping_time = std::chrono::microseconds{candidate.id}; // 8 protected + candidate.nLastTXTime = number_of_nodes - candidate.id; // 4 protected + candidate.nLastBlockTime = number_of_nodes - candidate.id; // 4 protected + }, + {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19}, random_context)); + + // An eviction is expected given >= 29 random eviction candidates. The eviction logic protects at most + // four peers by net group, eight by lowest ping time, four by last time of novel tx, up to eight non-tx-relay + // peers by last novel block time, and four more peers by last novel block time. + if (number_of_nodes >= 29) { + BOOST_CHECK(SelectNodeToEvict(GetRandomNodeEvictionCandidates(number_of_nodes, random_context))); + } + + // No eviction is expected given <= 20 random eviction candidates. The eviction logic protects at least + // four peers by net group, eight by lowest ping time, four by last time of novel tx and four peers by last + // novel block time. + if (number_of_nodes <= 20) { + BOOST_CHECK(!SelectNodeToEvict(GetRandomNodeEvictionCandidates(number_of_nodes, random_context))); } + + // Cases left to test: + // * "If any remaining peers are preferred for eviction consider only them. [...]" + // * "Identify the network group with the most connections and youngest member. [...]" } } -- cgit v1.2.3 From 4a19f501abac4adb476a6f2a30dfdf1a35892ccc Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Mon, 17 May 2021 11:11:02 +0200 Subject: test: add ALL_NETWORKS to test utilities --- src/test/util/net.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'src') diff --git a/src/test/util/net.h b/src/test/util/net.h index 71685d437a..1b49a671bd 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -6,9 +6,11 @@ #define BITCOIN_TEST_UTIL_NET_H #include +#include #include #include +#include #include #include #include @@ -67,6 +69,16 @@ constexpr ConnectionType ALL_CONNECTION_TYPES[]{ ConnectionType::ADDR_FETCH, }; +constexpr auto ALL_NETWORKS = std::array{ + Network::NET_UNROUTABLE, + Network::NET_IPV4, + Network::NET_IPV6, + Network::NET_ONION, + Network::NET_I2P, + Network::NET_CJDNS, + Network::NET_INTERNAL, +}; + /** * A mocked Sock alternative that returns a statically contained data upon read and succeeds * and ignores all writes. The data to be returned is given to the constructor and when it is -- cgit v1.2.3 From ec590f1d91325404383d74098a5b42a2cd67dad9 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Thu, 10 Jun 2021 10:45:17 +0200 Subject: p2p, refactor: improve constness in ProtectEvictionCandidatesByRatio() --- src/net.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/net.cpp b/src/net.cpp index 901132daa3..f1fd9b6c91 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -903,7 +903,7 @@ void ProtectEvictionCandidatesByRatio(std::vector& vEvict // 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 total_protect_size{initial_size / 2}; const size_t onion_protect_size = total_protect_size / 2; if (onion_protect_size) { @@ -926,8 +926,8 @@ void ProtectEvictionCandidatesByRatio(std::vector& vEvict // Calculate how many we removed, and update our total number of peers that // we want to protect based on uptime accordingly. - total_protect_size -= initial_size - vEvictionCandidates.size(); - EraseLastKElements(vEvictionCandidates, ReverseCompareNodeTimeConnected, total_protect_size); + const size_t remaining_to_protect{total_protect_size - (initial_size - vEvictionCandidates.size())}; + EraseLastKElements(vEvictionCandidates, ReverseCompareNodeTimeConnected, remaining_to_protect); } [[nodiscard]] std::optional SelectNodeToEvict(std::vector&& vEvictionCandidates) -- cgit v1.2.3 From 7321e6f2fe1641eb331f30e68646f5984d4bcbb3 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Thu, 10 Jun 2021 10:51:39 +0200 Subject: p2p, refactor: rename vEvictionCandidates to eviction_candidates in ProtectEvictionCandidatesByRatio() per current style guide in doc/developer-notes.md --- src/net.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/net.cpp b/src/net.cpp index f1fd9b6c91..16c66d2470 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -894,7 +894,7 @@ static void EraseLastKElements( elements.erase(std::remove_if(elements.end() - eraseSize, elements.end(), predicate), elements.end()); } -void ProtectEvictionCandidatesByRatio(std::vector& vEvictionCandidates) +void ProtectEvictionCandidatesByRatio(std::vector& eviction_candidates) { // 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. @@ -902,13 +902,13 @@ void ProtectEvictionCandidatesByRatio(std::vector& vEvict // 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(); + const size_t initial_size = eviction_candidates.size(); const size_t total_protect_size{initial_size / 2}; const size_t onion_protect_size = total_protect_size / 2; 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, + EraseLastKElements(eviction_candidates, CompareOnionTimeConnected, onion_protect_size, [](const NodeEvictionCandidate& n) { return n.m_is_onion; }); } @@ -918,16 +918,16 @@ void ProtectEvictionCandidatesByRatio(std::vector& vEvict // 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 remaining_tor_slots{onion_protect_size - (initial_size - eviction_candidates.size())}; const size_t localhost_protect_size{std::max(remaining_tor_slots, localhost_min_protect_size)}; - EraseLastKElements(vEvictionCandidates, CompareLocalHostTimeConnected, localhost_protect_size, + EraseLastKElements(eviction_candidates, 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. - const size_t remaining_to_protect{total_protect_size - (initial_size - vEvictionCandidates.size())}; - EraseLastKElements(vEvictionCandidates, ReverseCompareNodeTimeConnected, remaining_to_protect); + const size_t remaining_to_protect{total_protect_size - (initial_size - eviction_candidates.size())}; + EraseLastKElements(eviction_candidates, ReverseCompareNodeTimeConnected, remaining_to_protect); } [[nodiscard]] std::optional SelectNodeToEvict(std::vector&& vEvictionCandidates) -- cgit v1.2.3 From 4ee7aec47ebf6b59b4d930e6e4025e91352c05b4 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Thu, 10 Jun 2021 11:44:55 +0200 Subject: p2p: add m_network to NodeEvictionCandidate struct --- src/net.cpp | 2 +- src/net.h | 1 + src/test/fuzz/node_eviction.cpp | 1 + src/test/net_peer_eviction_tests.cpp | 3 +++ 4 files changed, 6 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/net.cpp b/src/net.cpp index 16c66d2470..71e7754ad4 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1023,7 +1023,7 @@ bool CConnman::AttemptToEvictConnection() HasAllDesirableServiceFlags(node->nServices), peer_relay_txes, peer_filter_not_null, node->nKeyedNetGroup, node->m_prefer_evict, node->addr.IsLocal(), - node->m_inbound_onion}; + node->m_inbound_onion, node->ConnectedThroughNetwork()}; vEvictionCandidates.push_back(candidate); } } diff --git a/src/net.h b/src/net.h index b43916c55e..a0708c338b 100644 --- a/src/net.h +++ b/src/net.h @@ -1210,6 +1210,7 @@ struct NodeEvictionCandidate bool prefer_evict; bool m_is_local; bool m_is_onion; + Network m_network; }; /** diff --git a/src/test/fuzz/node_eviction.cpp b/src/test/fuzz/node_eviction.cpp index 70ffc6bf37..9f02b5271b 100644 --- a/src/test/fuzz/node_eviction.cpp +++ b/src/test/fuzz/node_eviction.cpp @@ -32,6 +32,7 @@ FUZZ_TARGET(node_eviction) /* prefer_evict */ fuzzed_data_provider.ConsumeBool(), /* m_is_local */ fuzzed_data_provider.ConsumeBool(), /* m_is_onion */ fuzzed_data_provider.ConsumeBool(), + /* m_network */ fuzzed_data_provider.PickValueInArray(ALL_NETWORKS), }); } // Make a copy since eviction_candidates may be in some valid but otherwise diff --git a/src/test/net_peer_eviction_tests.cpp b/src/test/net_peer_eviction_tests.cpp index 3b981cf69c..7e74b6f6a3 100644 --- a/src/test/net_peer_eviction_tests.cpp +++ b/src/test/net_peer_eviction_tests.cpp @@ -2,7 +2,9 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include +#include #include #include @@ -32,6 +34,7 @@ std::vector GetRandomNodeEvictionCandidates(const int n_c /* prefer_evict */ random_context.randbool(), /* m_is_local */ random_context.randbool(), /* m_is_onion */ random_context.randbool(), + /* m_network */ ALL_NETWORKS[random_context.randrange(ALL_NETWORKS.size())], }); } return candidates; -- cgit v1.2.3 From 38a81a8e20b0e5ad9fef0eae8abd914619f05b25 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Mon, 17 May 2021 11:09:08 +0200 Subject: p2p: add CompareNodeNetworkTime() comparator struct to compare and sort peer eviction candidates by the passed-in is_local (localhost status) and network arguments, and by longest uptime. --- src/net.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'src') diff --git a/src/net.cpp b/src/net.cpp index 71e7754ad4..9318d25359 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -883,6 +883,26 @@ static bool CompareNodeBlockRelayOnlyTime(const NodeEvictionCandidate &a, const return a.nTimeConnected > b.nTimeConnected; } +/** + * Sort eviction candidates by network/localhost and connection uptime. + * Candidates near the beginning are more likely to be evicted, and those + * near the end are more likely to be protected, e.g. less likely to be evicted. + * - First, nodes that are not `is_local` and that do not belong to `network`, + * sorted by increasing uptime (from most recently connected to connected longer). + * - Then, nodes that are `is_local` or belong to `network`, sorted by increasing uptime. + */ +struct CompareNodeNetworkTime { + const bool m_is_local; + const Network m_network; + CompareNodeNetworkTime(bool is_local, Network network) : m_is_local(is_local), m_network(network) {} + bool operator()(const NodeEvictionCandidate& a, const NodeEvictionCandidate& b) const + { + if (m_is_local && a.m_is_local != b.m_is_local) return b.m_is_local; + if ((a.m_network == m_network) != (b.m_network == m_network)) return b.m_network == m_network; + return a.nTimeConnected > b.nTimeConnected; + }; +}; + //! Sort an array by the specified comparator, then erase the last K elements where predicate is true. template static void EraseLastKElements( -- cgit v1.2.3 From 3f8105c4d251e0e81bdd31f0999004e36f8990b2 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Wed, 2 Jun 2021 23:15:01 +0200 Subject: test: remove combined onion/localhost eviction protection tests as we are about the change the behavior sufficiently that when we have multiple disadvantaged networks and a small number of peers under test, the number of protected peers per network can be different. --- src/test/net_peer_eviction_tests.cpp | 72 ------------------------------------ 1 file changed, 72 deletions(-) (limited to 'src') diff --git a/src/test/net_peer_eviction_tests.cpp b/src/test/net_peer_eviction_tests.cpp index 7e74b6f6a3..9e5b4a47d5 100644 --- a/src/test/net_peer_eviction_tests.cpp +++ b/src/test/net_peer_eviction_tests.cpp @@ -154,78 +154,6 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) /* protected_peer_ids */ {0, 1, 2, 7, 8, 9}, /* unprotected_peer_ids */ {3, 4, 5, 6, 10, 11}, random_context)); - - // Combined test: expect 1/4 onion and 2 localhost peers to be protected - // from eviction, sorted by longest uptime. - BOOST_CHECK(IsProtected( - num_peers, [](NodeEvictionCandidate& c) { - c.nTimeConnected = c.id; - c.m_is_onion = (c.id == 0 || c.id == 5 || c.id == 10); - c.m_is_local = (c.id == 1 || c.id == 9 || c.id == 11); - }, - /* protected_peer_ids */ {0, 1, 2, 5, 9, 10}, - /* unprotected_peer_ids */ {3, 4, 6, 7, 8, 11}, - random_context)); - - // Combined test: expect having only 1 onion to allow allocating the - // remaining 2 of the 1/4 to localhost peers, sorted by longest uptime. - BOOST_CHECK(IsProtected( - num_peers + 4, [](NodeEvictionCandidate& c) { - c.nTimeConnected = c.id; - c.m_is_onion = (c.id == 15); - c.m_is_local = (c.id > 6 && c.id < 11); - }, - /* protected_peer_ids */ {0, 1, 2, 3, 7, 8, 9, 15}, - /* unprotected_peer_ids */ {4, 5, 6, 10, 11, 12, 13, 14}, - random_context)); - - // Combined test: expect 2 onions (< 1/4) to allow allocating the minimum 2 - // localhost peers, sorted by longest uptime. - BOOST_CHECK(IsProtected( - num_peers, [](NodeEvictionCandidate& c) { - c.nTimeConnected = c.id; - c.m_is_onion = (c.id == 7 || c.id == 9); - c.m_is_local = (c.id == 6 || c.id == 11); - }, - /* protected_peer_ids */ {0, 1, 6, 7, 9, 11}, - /* unprotected_peer_ids */ {2, 3, 4, 5, 8, 10}, - random_context)); - - // Combined test: when > 1/4, expect max 1/4 onion and 2 localhost peers - // to be protected from eviction, sorted by longest uptime. - BOOST_CHECK(IsProtected( - num_peers, [](NodeEvictionCandidate& c) { - c.nTimeConnected = c.id; - c.m_is_onion = (c.id > 3 && c.id < 8); - c.m_is_local = (c.id > 7); - }, - /* protected_peer_ids */ {0, 4, 5, 6, 8, 9}, - /* unprotected_peer_ids */ {1, 2, 3, 7, 10, 11}, - random_context)); - - // Combined test: idem > 1/4 with only 8 peers: expect 2 onion and 2 - // localhost peers (1/4 + 2) to be protected, sorted by longest uptime. - BOOST_CHECK(IsProtected( - 8, [](NodeEvictionCandidate& c) { - c.nTimeConnected = c.id; - c.m_is_onion = (c.id > 1 && c.id < 5); - c.m_is_local = (c.id > 4); - }, - /* protected_peer_ids */ {2, 3, 5, 6}, - /* unprotected_peer_ids */ {0, 1, 4, 7}, - random_context)); - - // Combined test: idem > 1/4 with only 6 peers: expect 1 onion peer and no - // localhost peers (1/4 + 0) to be protected, sorted by longest uptime. - BOOST_CHECK(IsProtected( - 6, [](NodeEvictionCandidate& c) { - c.nTimeConnected = c.id; - c.m_is_onion = (c.id == 4 || c.id == 5); - c.m_is_local = (c.id == 3); - }, - /* protected_peer_ids */ {0, 1, 4}, - /* unprotected_peer_ids */ {2, 3, 5}, - random_context)); } // Returns true if any of the node ids in node_ids are selected for eviction. -- cgit v1.2.3 From 1e15acf478ae071234350c9b38dc823dfe2e3419 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Tue, 20 Apr 2021 13:22:20 +0200 Subject: p2p: make ProtectEvictionCandidatesByRatio() fully ratio-based with a more abstract framework to allow easily extending inbound eviction protection to peers connected through new higher-latency networks that are disadvantaged by our inbound eviction criteria, such as I2P and perhaps other BIP155 networks in the future like CJDNS. This is a change in behavior. The algorithm is a basically a multi-pass knapsack: - Count the number of eviction candidates in each of the disadvantaged privacy networks. - Sort the networks from lower to higher candidate counts, so that a network with fewer candidates will have the first opportunity for any unused slots remaining from the previous iteration. In the case of a tie in candidate counts, priority is given by array member order from first to last, guesstimated to favor more unusual networks. - Iterate through the networks in this order. On each iteration, allocate each network an equal number of protected slots targeting a total number of candidates to protect, provided any slots remain in the knapsack. - Protect the candidates in that network having the longest uptime, if any in that network are present. - Continue iterating as long as we have non-allocated slots remaining and candidates available to protect. Localhost peers are treated as a network like Tor or I2P by aliasing them to an unused Network enumerator: Network::NET_MAX. The goal is to favorise diversity of our inbound connections. Credit to Vasil Dimov for improving the algorithm from single-pass to multi-pass to better allocate unused protection slots. Co-authored-by: Vasil Dimov --- src/net.cpp | 74 ++++++++++++++++++++++++++---------- src/test/net_peer_eviction_tests.cpp | 15 +++++--- 2 files changed, 63 insertions(+), 26 deletions(-) (limited to 'src') diff --git a/src/net.cpp b/src/net.cpp index 9318d25359..b69a59fc1d 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -42,6 +42,7 @@ #endif #include +#include #include #include #include @@ -918,35 +919,66 @@ void ProtectEvictionCandidatesByRatio(std::vector& evicti { // 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. - // 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 + // To favorise the diversity of our peer connections, reserve up to half of these protected + // spots for Tor/onion and localhost peers, even if they're not longest uptime overall. + // This helps protect these higher-latency peers that tend to be otherwise // disadvantaged under our eviction criteria. const size_t initial_size = eviction_candidates.size(); const size_t total_protect_size{initial_size / 2}; - const size_t onion_protect_size = total_protect_size / 2; - if (onion_protect_size) { - // Pick out up to 1/4 peers connected via our onion service, sorted by longest uptime. - EraseLastKElements(eviction_candidates, 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 - eviction_candidates.size())}; - const size_t localhost_protect_size{std::max(remaining_tor_slots, localhost_min_protect_size)}; - EraseLastKElements(eviction_candidates, CompareLocalHostTimeConnected, localhost_protect_size, - [](const NodeEvictionCandidate& n) { return n.m_is_local; }); + // Disadvantaged networks to protect: localhost and Tor/onion. In case of equal counts, earlier + // array members have first opportunity to recover unused slots from the previous iteration. + struct Net { bool is_local; Network id; size_t count; }; + std::array networks{{{/* localhost */ true, NET_MAX, 0}, {false, NET_ONION, 0}}}; + + // Count and store the number of eviction candidates per network. + for (Net& n : networks) { + n.count = std::count_if(eviction_candidates.cbegin(), eviction_candidates.cend(), + [&n](const NodeEvictionCandidate& c) { + return n.is_local ? c.m_is_local : c.m_network == n.id; + }); + } + // Sort `networks` by ascending candidate count, to give networks having fewer candidates + // the first opportunity to recover unused protected slots from the previous iteration. + std::stable_sort(networks.begin(), networks.end(), [](Net a, Net b) { return a.count < b.count; }); + + // Protect up to 25% of the eviction candidates by disadvantaged network. + const size_t max_protect_by_network{total_protect_size / 2}; + size_t num_protected{0}; + + while (num_protected < max_protect_by_network) { + const size_t disadvantaged_to_protect{max_protect_by_network - num_protected}; + const size_t protect_per_network{ + std::max(disadvantaged_to_protect / networks.size(), static_cast(1))}; + + // Early exit flag if there are no remaining candidates by disadvantaged network. + bool protected_at_least_one{false}; + + for (const Net& n : networks) { + if (n.count == 0) continue; + const size_t before = eviction_candidates.size(); + EraseLastKElements(eviction_candidates, CompareNodeNetworkTime(n.is_local, n.id), + protect_per_network, [&n](const NodeEvictionCandidate& c) { + return n.is_local ? c.m_is_local : c.m_network == n.id; + }); + const size_t after = eviction_candidates.size(); + if (before > after) { + protected_at_least_one = true; + num_protected += before - after; + if (num_protected >= max_protect_by_network) { + break; + } + } + } + if (!protected_at_least_one) { + break; + } } // Calculate how many we removed, and update our total number of peers that // we want to protect based on uptime accordingly. - const size_t remaining_to_protect{total_protect_size - (initial_size - eviction_candidates.size())}; + assert(num_protected == initial_size - eviction_candidates.size()); + const size_t remaining_to_protect{total_protect_size - num_protected}; EraseLastKElements(eviction_candidates, ReverseCompareNodeTimeConnected, remaining_to_protect); } diff --git a/src/test/net_peer_eviction_tests.cpp b/src/test/net_peer_eviction_tests.cpp index 9e5b4a47d5..7dc2f2562a 100644 --- a/src/test/net_peer_eviction_tests.cpp +++ b/src/test/net_peer_eviction_tests.cpp @@ -93,6 +93,7 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) num_peers, [](NodeEvictionCandidate& c) { c.nTimeConnected = c.id; c.m_is_onion = c.m_is_local = false; + c.m_network = NET_IPV4; }, /* protected_peer_ids */ {0, 1, 2, 3, 4, 5}, /* unprotected_peer_ids */ {6, 7, 8, 9, 10, 11}, @@ -103,6 +104,7 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) num_peers, [num_peers](NodeEvictionCandidate& c) { c.nTimeConnected = num_peers - c.id; c.m_is_onion = c.m_is_local = false; + c.m_network = NET_IPV6; }, /* protected_peer_ids */ {6, 7, 8, 9, 10, 11}, /* unprotected_peer_ids */ {0, 1, 2, 3, 4, 5}, @@ -111,22 +113,23 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) // Test protection of onion and localhost peers... // Expect 1/4 onion peers to be protected from eviction, - // independently of other characteristics. + // if no localhost peers. BOOST_CHECK(IsProtected( num_peers, [](NodeEvictionCandidate& c) { - c.m_is_onion = (c.id == 3 || c.id == 8 || c.id == 9); + c.m_is_local = false; + c.m_network = (c.id == 3 || c.id == 8 || c.id == 9) ? NET_ONION : NET_IPV4; }, /* protected_peer_ids */ {3, 8, 9}, /* unprotected_peer_ids */ {}, random_context)); - // Expect 1/4 onion peers and 1/4 of the others to be protected - // from eviction, sorted by longest uptime (lowest nTimeConnected). + // Expect 1/4 onion peers and 1/4 of the other peers to be protected, + // sorted by longest uptime (lowest nTimeConnected), if no localhost peers. BOOST_CHECK(IsProtected( num_peers, [](NodeEvictionCandidate& c) { c.nTimeConnected = c.id; c.m_is_local = false; - c.m_is_onion = (c.id == 3 || c.id > 7); + c.m_network = (c.id == 3 || c.id > 7) ? NET_ONION : NET_IPV6; }, /* protected_peer_ids */ {0, 1, 2, 3, 8, 9}, /* unprotected_peer_ids */ {4, 5, 6, 7, 10, 11}, @@ -138,6 +141,7 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) num_peers, [](NodeEvictionCandidate& c) { c.m_is_onion = false; c.m_is_local = (c.id == 1 || c.id == 9 || c.id == 11); + c.m_network = NET_IPV4; }, /* protected_peer_ids */ {1, 9, 11}, /* unprotected_peer_ids */ {}, @@ -150,6 +154,7 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) c.nTimeConnected = c.id; c.m_is_onion = false; c.m_is_local = (c.id > 6); + c.m_network = NET_IPV6; }, /* protected_peer_ids */ {0, 1, 2, 7, 8, 9}, /* unprotected_peer_ids */ {3, 4, 5, 6, 10, 11}, -- cgit v1.2.3 From 787d46bb2a39fb39166882cc6e0afbc34424d88e Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Tue, 20 Apr 2021 12:17:40 +0200 Subject: p2p: update ProtectEvictionCandidatesByRatio() doxygen docs --- src/net.h | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/net.h b/src/net.h index a0708c338b..4e8e979f4a 100644 --- a/src/net.h +++ b/src/net.h @@ -1228,20 +1228,18 @@ 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 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. + * Half of these protected spots (1/4 of the total) are reserved for the + * following categories of peers, sorted by longest uptime, even if they're not + * longest uptime overall: * - * This helps protect onion peers, which tend to be otherwise disadvantaged - * under our eviction criteria for their higher min ping times relative to IPv4 - * and IPv6 peers, and favorise the diversity of peer connections. + * - onion peers connected via our tor control service * - * This function was extracted from SelectNodeToEvict() to be able to test the - * ratio-based protection logic deterministically. + * - localhost peers, as manually configured hidden services not using + * `-bind=addr[:port]=onion` will not be detected as inbound onion connections + * + * This helps protect these privacy network peers, which tend to be otherwise + * disadvantaged under our eviction criteria for their higher min ping times + * relative to IPv4/IPv6 peers, and favorise the diversity of peer connections. */ void ProtectEvictionCandidatesByRatio(std::vector& vEvictionCandidates); -- cgit v1.2.3 From 9e889e8a5c021b0ec7e4c4d17d418ab4a0accad4 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Fri, 11 Jun 2021 12:08:09 +0200 Subject: p2p: remove unused CompareOnionTimeConnected() --- src/net.cpp | 6 ------ 1 file changed, 6 deletions(-) (limited to 'src') diff --git a/src/net.cpp b/src/net.cpp index b69a59fc1d..7e5c3806c1 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -848,12 +848,6 @@ 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; } -- cgit v1.2.3 From 310fab49282d507e5fa710afb20d036604bbf3a2 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Fri, 11 Jun 2021 12:08:27 +0200 Subject: p2p: remove unused CompareLocalHostTimeConnected() --- src/net.cpp | 6 ------ 1 file changed, 6 deletions(-) (limited to 'src') diff --git a/src/net.cpp b/src/net.cpp index 7e5c3806c1..f4388b2b26 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -842,12 +842,6 @@ static bool ReverseCompareNodeTimeConnected(const NodeEvictionCandidate &a, cons return a.nTimeConnected > b.nTimeConnected; } -static bool CompareLocalHostTimeConnected(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) -{ - if (a.m_is_local != b.m_is_local) return b.m_is_local; - return a.nTimeConnected > b.nTimeConnected; -} - static bool CompareNetGroupKeyed(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) { return a.nKeyedNetGroup < b.nKeyedNetGroup; } -- cgit v1.2.3 From 045cb40192bf3dfa6c42916237e55f86bbc788d4 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Fri, 11 Jun 2021 11:54:08 +0200 Subject: p2p: remove unused m_is_onion member from NodeEvictionCandidate struct --- src/net.cpp | 2 +- src/net.h | 1 - src/test/fuzz/node_eviction.cpp | 1 - src/test/net_peer_eviction_tests.cpp | 7 ++----- 4 files changed, 3 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/net.cpp b/src/net.cpp index f4388b2b26..a69d13436d 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1063,7 +1063,7 @@ bool CConnman::AttemptToEvictConnection() HasAllDesirableServiceFlags(node->nServices), peer_relay_txes, peer_filter_not_null, node->nKeyedNetGroup, node->m_prefer_evict, node->addr.IsLocal(), - node->m_inbound_onion, node->ConnectedThroughNetwork()}; + node->ConnectedThroughNetwork()}; vEvictionCandidates.push_back(candidate); } } diff --git a/src/net.h b/src/net.h index 4e8e979f4a..e308ac5714 100644 --- a/src/net.h +++ b/src/net.h @@ -1209,7 +1209,6 @@ struct NodeEvictionCandidate uint64_t nKeyedNetGroup; bool prefer_evict; bool m_is_local; - bool m_is_onion; Network m_network; }; diff --git a/src/test/fuzz/node_eviction.cpp b/src/test/fuzz/node_eviction.cpp index 9f02b5271b..a3f71426fa 100644 --- a/src/test/fuzz/node_eviction.cpp +++ b/src/test/fuzz/node_eviction.cpp @@ -31,7 +31,6 @@ FUZZ_TARGET(node_eviction) /* nKeyedNetGroup */ fuzzed_data_provider.ConsumeIntegral(), /* prefer_evict */ fuzzed_data_provider.ConsumeBool(), /* m_is_local */ fuzzed_data_provider.ConsumeBool(), - /* m_is_onion */ fuzzed_data_provider.ConsumeBool(), /* m_network */ fuzzed_data_provider.PickValueInArray(ALL_NETWORKS), }); } diff --git a/src/test/net_peer_eviction_tests.cpp b/src/test/net_peer_eviction_tests.cpp index 7dc2f2562a..38be548b2f 100644 --- a/src/test/net_peer_eviction_tests.cpp +++ b/src/test/net_peer_eviction_tests.cpp @@ -33,7 +33,6 @@ std::vector GetRandomNodeEvictionCandidates(const int n_c /* nKeyedNetGroup */ random_context.randrange(100), /* prefer_evict */ random_context.randbool(), /* m_is_local */ random_context.randbool(), - /* m_is_onion */ random_context.randbool(), /* m_network */ ALL_NETWORKS[random_context.randrange(ALL_NETWORKS.size())], }); } @@ -92,7 +91,7 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) BOOST_CHECK(IsProtected( num_peers, [](NodeEvictionCandidate& c) { c.nTimeConnected = c.id; - c.m_is_onion = c.m_is_local = false; + c.m_is_local = false; c.m_network = NET_IPV4; }, /* protected_peer_ids */ {0, 1, 2, 3, 4, 5}, @@ -103,7 +102,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_onion = c.m_is_local = false; + c.m_is_local = false; c.m_network = NET_IPV6; }, /* protected_peer_ids */ {6, 7, 8, 9, 10, 11}, @@ -139,7 +138,6 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) // 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); c.m_network = NET_IPV4; }, @@ -152,7 +150,6 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) BOOST_CHECK(IsProtected( num_peers, [](NodeEvictionCandidate& c) { c.nTimeConnected = c.id; - c.m_is_onion = false; c.m_is_local = (c.id > 6); c.m_network = NET_IPV6; }, -- cgit v1.2.3 From 70bbc62711643ec57cce620f9f7a0e1fe5fb6346 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Thu, 22 Apr 2021 16:42:37 +0200 Subject: test: add combined onion/localhost eviction protection coverage --- src/test/net_peer_eviction_tests.cpp | 104 +++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) (limited to 'src') diff --git a/src/test/net_peer_eviction_tests.cpp b/src/test/net_peer_eviction_tests.cpp index 38be548b2f..ec4aa50173 100644 --- a/src/test/net_peer_eviction_tests.cpp +++ b/src/test/net_peer_eviction_tests.cpp @@ -156,6 +156,110 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) /* protected_peer_ids */ {0, 1, 2, 7, 8, 9}, /* unprotected_peer_ids */ {3, 4, 5, 6, 10, 11}, random_context)); + + // Tests with 2 networks... + + // Combined test: expect having 1 localhost and 1 onion peer out of 4 to + // protect 1 localhost, 0 onion and 1 other peer, sorted by longest uptime; + // stable sort breaks tie with array order of localhost first. + BOOST_CHECK(IsProtected( + 4, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id == 4); + c.m_network = (c.id == 3) ? NET_ONION : NET_IPV4; + }, + /* protected_peer_ids */ {0, 4}, + /* unprotected_peer_ids */ {1, 2}, + random_context)); + + // Combined test: expect having 1 localhost and 1 onion peer out of 7 to + // protect 1 localhost, 0 onion, and 2 other peers (3 total), sorted by + // uptime; stable sort breaks tie with array order of localhost first. + BOOST_CHECK(IsProtected( + 7, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id == 6); + c.m_network = (c.id == 5) ? NET_ONION : NET_IPV4; + }, + /* protected_peer_ids */ {0, 1, 6}, + /* unprotected_peer_ids */ {2, 3, 4, 5}, + random_context)); + + // Combined test: expect having 1 localhost and 1 onion peer out of 8 to + // protect protect 1 localhost, 1 onion and 2 other peers (4 total), sorted + // by uptime; stable sort breaks tie with array order of localhost first. + BOOST_CHECK(IsProtected( + 8, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id == 6); + c.m_network = (c.id == 5) ? NET_ONION : NET_IPV4; + }, + /* protected_peer_ids */ {0, 1, 5, 6}, + /* unprotected_peer_ids */ {2, 3, 4, 7}, + random_context)); + + // Combined test: expect having 3 localhost and 3 onion peers out of 12 to + // protect 2 localhost and 1 onion, plus 3 other peers, sorted by longest + // uptime; stable sort breaks ties with the array order of localhost first. + BOOST_CHECK(IsProtected( + num_peers, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id == 6 || c.id == 9 || c.id == 11); + c.m_network = (c.id == 7 || c.id == 8 || c.id == 10) ? NET_ONION : NET_IPV6; + }, + /* protected_peer_ids */ {0, 1, 2, 6, 7, 9}, + /* unprotected_peer_ids */ {3, 4, 5, 8, 10, 11}, + random_context)); + + // Combined test: expect having 4 localhost and 1 onion peer out of 12 to + // protect 2 localhost and 1 onion, plus 3 other peers, sorted by longest uptime. + BOOST_CHECK(IsProtected( + num_peers, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id > 4 && c.id < 9); + c.m_network = (c.id == 10) ? NET_ONION : NET_IPV4; + }, + /* protected_peer_ids */ {0, 1, 2, 5, 6, 10}, + /* unprotected_peer_ids */ {3, 4, 7, 8, 9, 11}, + random_context)); + + // Combined test: expect having 4 localhost and 2 onion peers out of 16 to + // protect 2 localhost and 2 onions, plus 4 other peers, sorted by longest uptime. + BOOST_CHECK(IsProtected( + 16, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id == 6 || c.id == 9 || c.id == 11 || c.id == 12); + c.m_network = (c.id == 8 || c.id == 10) ? NET_ONION : NET_IPV6; + }, + /* protected_peer_ids */ {0, 1, 2, 3, 6, 8, 9, 10}, + /* unprotected_peer_ids */ {4, 5, 7, 11, 12, 13, 14, 15}, + random_context)); + + // Combined test: expect having 5 localhost and 1 onion peer out of 16 to + // protect 3 localhost (recovering the unused onion slot), 1 onion, and 4 + // others, sorted by longest uptime. + BOOST_CHECK(IsProtected( + 16, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id > 10); + c.m_network = (c.id == 10) ? NET_ONION : NET_IPV4; + }, + /* protected_peer_ids */ {0, 1, 2, 3, 10, 11, 12, 13}, + /* unprotected_peer_ids */ {4, 5, 6, 7, 8, 9, 14, 15}, + random_context)); + + // Combined test: expect having 1 localhost and 4 onion peers out of 16 to + // protect 1 localhost and 3 onions (recovering the unused localhost slot), + // plus 4 others, sorted by longest uptime. + BOOST_CHECK(IsProtected( + 16, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id == 15); + c.m_network = (c.id > 6 && c.id < 11) ? NET_ONION : NET_IPV6; + }, + /* protected_peer_ids */ {0, 1, 2, 3, 7, 8, 9, 15}, + /* unprotected_peer_ids */ {5, 6, 10, 11, 12, 13, 14}, + random_context)); } // Returns true if any of the node ids in node_ids are selected for eviction. -- cgit v1.2.3 From ce02dd1ef1f7f54f33780b32f195d31c1cc87318 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Sat, 10 Apr 2021 16:11:03 +0200 Subject: p2p: extend inbound eviction protection by network to I2P peers This commit extends our inbound eviction protection to I2P peers to favorise the diversity of peer connections, as peers connected through the I2P network are otherwise disadvantaged by our eviction criteria for their higher latency (higher min ping times) relative to IPv4 and IPv6 peers, as well as relative to Tor onion peers. The `networks` array is order-dependent in the case of a tie in candidate counts between networks (earlier array members receive priority in the case of a tie). Therefore, we place I2P candidates before localhost and onion ones in terms of opportunity to recover unused remaining protected slots from the previous iteration, guesstimating that most nodes allowing both onion and I2P inbounds will have more onion peers, followed by localhost, then I2P, as I2P support is only being added in the upcoming v22.0 release. --- src/net.cpp | 7 ++++--- src/net.h | 2 ++ src/test/net_peer_eviction_tests.cpp | 10 +++++----- 3 files changed, 11 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/net.cpp b/src/net.cpp index a69d13436d..4d7c181330 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -908,16 +908,17 @@ void ProtectEvictionCandidatesByRatio(std::vector& evicti // 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. // To favorise the diversity of our peer connections, reserve up to half of these protected - // spots for Tor/onion and localhost peers, even if they're not longest uptime overall. + // spots for Tor/onion, localhost and I2P peers, even if they're not longest uptime overall. // This helps protect these higher-latency peers that tend to be otherwise // disadvantaged under our eviction criteria. const size_t initial_size = eviction_candidates.size(); const size_t total_protect_size{initial_size / 2}; - // Disadvantaged networks to protect: localhost and Tor/onion. In case of equal counts, earlier + // Disadvantaged networks to protect: I2P, localhost, Tor/onion. In case of equal counts, earlier // array members have first opportunity to recover unused slots from the previous iteration. struct Net { bool is_local; Network id; size_t count; }; - std::array networks{{{/* localhost */ true, NET_MAX, 0}, {false, NET_ONION, 0}}}; + std::array networks{ + {{false, NET_I2P, 0}, {/* localhost */ true, NET_MAX, 0}, {false, NET_ONION, 0}}}; // Count and store the number of eviction candidates per network. for (Net& n : networks) { diff --git a/src/net.h b/src/net.h index e308ac5714..01658e8973 100644 --- a/src/net.h +++ b/src/net.h @@ -1236,6 +1236,8 @@ struct NodeEvictionCandidate * - localhost peers, as manually configured hidden services not using * `-bind=addr[:port]=onion` will not be detected as inbound onion connections * + * - I2P peers + * * This helps protect these privacy network peers, which tend to be otherwise * disadvantaged under our eviction criteria for their higher min ping times * relative to IPv4/IPv6 peers, and favorise the diversity of peer connections. diff --git a/src/test/net_peer_eviction_tests.cpp b/src/test/net_peer_eviction_tests.cpp index ec4aa50173..bf56e1bea7 100644 --- a/src/test/net_peer_eviction_tests.cpp +++ b/src/test/net_peer_eviction_tests.cpp @@ -109,10 +109,10 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) /* unprotected_peer_ids */ {0, 1, 2, 3, 4, 5}, random_context)); - // Test protection of onion and localhost peers... + // Test protection of onion, localhost, and I2P peers... // Expect 1/4 onion peers to be protected from eviction, - // if no localhost peers. + // if no localhost or I2P peers. BOOST_CHECK(IsProtected( num_peers, [](NodeEvictionCandidate& c) { c.m_is_local = false; @@ -123,7 +123,7 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) random_context)); // Expect 1/4 onion peers and 1/4 of the other peers to be protected, - // sorted by longest uptime (lowest nTimeConnected), if no localhost peers. + // sorted by longest uptime (lowest nTimeConnected), if no localhost or I2P peers. BOOST_CHECK(IsProtected( num_peers, [](NodeEvictionCandidate& c) { c.nTimeConnected = c.id; @@ -135,7 +135,7 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) random_context)); // Expect 1/4 localhost peers to be protected from eviction, - // if no onion peers. + // if no onion or I2P peers. BOOST_CHECK(IsProtected( num_peers, [](NodeEvictionCandidate& c) { c.m_is_local = (c.id == 1 || c.id == 9 || c.id == 11); @@ -146,7 +146,7 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) random_context)); // 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. + // sorted by longest uptime (lowest nTimeConnected), if no onion or I2P peers. BOOST_CHECK(IsProtected( num_peers, [](NodeEvictionCandidate& c) { c.nTimeConnected = c.id; -- cgit v1.2.3 From 7c2284eda22a08dbf2a560894e496e245d026ee0 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Thu, 22 Apr 2021 16:46:21 +0200 Subject: test: add tests for inbound eviction protection of I2P peers --- src/test/net_peer_eviction_tests.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'src') diff --git a/src/test/net_peer_eviction_tests.cpp b/src/test/net_peer_eviction_tests.cpp index bf56e1bea7..35dfe6c4af 100644 --- a/src/test/net_peer_eviction_tests.cpp +++ b/src/test/net_peer_eviction_tests.cpp @@ -157,6 +157,29 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) /* unprotected_peer_ids */ {3, 4, 5, 6, 10, 11}, random_context)); + // Expect 1/4 I2P peers to be protected from eviction, + // if no onion or localhost peers. + BOOST_CHECK(IsProtected( + num_peers, [](NodeEvictionCandidate& c) { + c.m_is_local = false; + c.m_network = (c.id == 2 || c.id == 7 || c.id == 10) ? NET_I2P : NET_IPV4; + }, + /* protected_peer_ids */ {2, 7, 10}, + /* unprotected_peer_ids */ {}, + random_context)); + + // Expect 1/4 I2P peers and 1/4 of the other peers to be protected, + // sorted by longest uptime (lowest nTimeConnected), if no onion or localhost peers. + BOOST_CHECK(IsProtected( + num_peers, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = false; + c.m_network = (c.id == 4 || c.id > 8) ? NET_I2P : NET_IPV6; + }, + /* protected_peer_ids */ {0, 1, 2, 4, 9, 10}, + /* unprotected_peer_ids */ {3, 5, 6, 7, 8, 11}, + random_context)); + // Tests with 2 networks... // Combined test: expect having 1 localhost and 1 onion peer out of 4 to -- cgit v1.2.3 From 1b1088d52fbff8b1c9438d6aa8c6edcbdd471457 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Thu, 22 Apr 2021 19:21:37 +0200 Subject: test: add combined I2P/onion/localhost eviction protection tests --- src/test/net_peer_eviction_tests.cpp | 173 +++++++++++++++++++++++++++++++++++ 1 file changed, 173 insertions(+) (limited to 'src') diff --git a/src/test/net_peer_eviction_tests.cpp b/src/test/net_peer_eviction_tests.cpp index 35dfe6c4af..4bfd487b86 100644 --- a/src/test/net_peer_eviction_tests.cpp +++ b/src/test/net_peer_eviction_tests.cpp @@ -283,6 +283,179 @@ BOOST_AUTO_TEST_CASE(peer_protection_test) /* protected_peer_ids */ {0, 1, 2, 3, 7, 8, 9, 15}, /* unprotected_peer_ids */ {5, 6, 10, 11, 12, 13, 14}, random_context)); + + // Combined test: expect having 2 onion and 4 I2P out of 12 peers to protect + // 2 onion (prioritized for having fewer candidates) and 1 I2P, plus 3 + // others, sorted by longest uptime. + BOOST_CHECK(IsProtected( + num_peers, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = false; + if (c.id == 8 || c.id == 10) { + c.m_network = NET_ONION; + } else if (c.id == 6 || c.id == 9 || c.id == 11 || c.id == 12) { + c.m_network = NET_I2P; + } else { + c.m_network = NET_IPV4; + } + }, + /* protected_peer_ids */ {0, 1, 2, 6, 8, 10}, + /* unprotected_peer_ids */ {3, 4, 5, 7, 9, 11}, + random_context)); + + // Tests with 3 networks... + + // Combined test: expect having 1 localhost, 1 I2P and 1 onion peer out of 4 + // to protect 1 I2P, 0 localhost, 0 onion and 1 other peer (2 total), sorted + // by longest uptime; stable sort breaks tie with array order of I2P first. + BOOST_CHECK(IsProtected( + 4, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id == 3); + if (c.id == 4) { + c.m_network = NET_I2P; + } else if (c.id == 2) { + c.m_network = NET_ONION; + } else { + c.m_network = NET_IPV6; + } + }, + /* protected_peer_ids */ {0, 4}, + /* unprotected_peer_ids */ {1, 2}, + random_context)); + + // Combined test: expect having 1 localhost, 1 I2P and 1 onion peer out of 7 + // to protect 1 I2P, 0 localhost, 0 onion and 2 other peers (3 total) sorted + // by longest uptime; stable sort breaks tie with array order of I2P first. + BOOST_CHECK(IsProtected( + 7, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id == 4); + if (c.id == 6) { + c.m_network = NET_I2P; + } else if (c.id == 5) { + c.m_network = NET_ONION; + } else { + c.m_network = NET_IPV6; + } + }, + /* protected_peer_ids */ {0, 1, 6}, + /* unprotected_peer_ids */ {2, 3, 4, 5}, + random_context)); + + // Combined test: expect having 1 localhost, 1 I2P and 1 onion peer out of 8 + // to protect 1 I2P, 1 localhost, 0 onion and 2 other peers (4 total) sorted + // by uptime; stable sort breaks tie with array order of I2P then localhost. + BOOST_CHECK(IsProtected( + 8, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id == 6); + if (c.id == 5) { + c.m_network = NET_I2P; + } else if (c.id == 4) { + c.m_network = NET_ONION; + } else { + c.m_network = NET_IPV6; + } + }, + /* protected_peer_ids */ {0, 1, 5, 6}, + /* unprotected_peer_ids */ {2, 3, 4, 7}, + random_context)); + + // Combined test: expect having 4 localhost, 2 I2P, and 2 onion peers out of + // 16 to protect 1 localhost, 2 I2P, and 1 onion (4/16 total), plus 4 others + // for 8 total, sorted by longest uptime. + BOOST_CHECK(IsProtected( + 16, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id == 6 || c.id > 11); + if (c.id == 7 || c.id == 11) { + c.m_network = NET_I2P; + } else if (c.id == 9 || c.id == 10) { + c.m_network = NET_ONION; + } else { + c.m_network = NET_IPV4; + } + }, + /* protected_peer_ids */ {0, 1, 2, 3, 6, 7, 9, 11}, + /* unprotected_peer_ids */ {4, 5, 8, 10, 12, 13, 14, 15}, + random_context)); + + // Combined test: expect having 1 localhost, 8 I2P and 1 onion peer out of + // 24 to protect 1, 4, and 1 (6 total), plus 6 others for 12/24 total, + // sorted by longest uptime. + BOOST_CHECK(IsProtected( + 24, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id == 12); + if (c.id > 14 && c.id < 23) { // 4 protected instead of usual 2 + c.m_network = NET_I2P; + } else if (c.id == 23) { + c.m_network = NET_ONION; + } else { + c.m_network = NET_IPV6; + } + }, + /* protected_peer_ids */ {0, 1, 2, 3, 4, 5, 12, 15, 16, 17, 18, 23}, + /* unprotected_peer_ids */ {6, 7, 8, 9, 10, 11, 13, 14, 19, 20, 21, 22}, + random_context)); + + // Combined test: expect having 1 localhost, 3 I2P and 6 onion peers out of + // 24 to protect 1, 3, and 2 (6 total, I2P has fewer candidates and so gets the + // unused localhost slot), plus 6 others for 12/24 total, sorted by longest uptime. + BOOST_CHECK(IsProtected( + 24, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id == 15); + if (c.id == 12 || c.id == 14 || c.id == 17) { + c.m_network = NET_I2P; + } else if (c.id > 17) { // 4 protected instead of usual 2 + c.m_network = NET_ONION; + } else { + c.m_network = NET_IPV4; + } + }, + /* protected_peer_ids */ {0, 1, 2, 3, 4, 5, 12, 14, 15, 17, 18, 19}, + /* unprotected_peer_ids */ {6, 7, 8, 9, 10, 11, 13, 16, 20, 21, 22, 23}, + random_context)); + + // Combined test: expect having 1 localhost, 7 I2P and 4 onion peers out of + // 24 to protect 1 localhost, 2 I2P, and 3 onions (6 total), plus 6 others + // for 12/24 total, sorted by longest uptime. + BOOST_CHECK(IsProtected( + 24, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id == 13); + if (c.id > 16) { + c.m_network = NET_I2P; + } else if (c.id == 12 || c.id == 14 || c.id == 15 || c.id == 16) { + c.m_network = NET_ONION; + } else { + c.m_network = NET_IPV6; + } + }, + /* protected_peer_ids */ {0, 1, 2, 3, 4, 5, 12, 13, 14, 15, 17, 18}, + /* unprotected_peer_ids */ {6, 7, 8, 9, 10, 11, 16, 19, 20, 21, 22, 23}, + random_context)); + + // Combined test: expect having 8 localhost, 4 I2P, and 3 onion peers out of + // 24 to protect 2 of each (6 total), plus 6 others for 12/24 total, sorted + // by longest uptime. + BOOST_CHECK(IsProtected( + 24, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id > 15); + if (c.id > 10 && c.id < 15) { + c.m_network = NET_I2P; + } else if (c.id > 6 && c.id < 10) { + c.m_network = NET_ONION; + } else { + c.m_network = NET_IPV4; + } + }, + /* protected_peer_ids */ {0, 1, 2, 3, 4, 5, 7, 8, 11, 12, 16, 17}, + /* unprotected_peer_ids */ {6, 9, 10, 13, 14, 15, 18, 19, 20, 21, 22, 23}, + random_context)); } // Returns true if any of the node ids in node_ids are selected for eviction. -- cgit v1.2.3