diff options
Diffstat (limited to 'src')
33 files changed, 891 insertions, 390 deletions
diff --git a/src/Makefile.test.include b/src/Makefile.test.include index ef03f63579..570f011f7a 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -90,6 +90,7 @@ BITCOIN_TESTS =\ test/fs_tests.cpp \ test/getarg_tests.cpp \ test/hash_tests.cpp \ + test/i2p_tests.cpp \ test/interfaces_tests.cpp \ test/key_io_tests.cpp \ test/key_tests.cpp \ @@ -101,6 +102,7 @@ BITCOIN_TESTS =\ test/merkleblock_tests.cpp \ test/miner_tests.cpp \ test/multisig_tests.cpp \ + test/net_peer_eviction_tests.cpp \ test/net_tests.cpp \ test/netbase_tests.cpp \ test/pmt_tests.cpp \ @@ -238,6 +240,7 @@ test_fuzz_fuzz_SOURCES = \ test/fuzz/golomb_rice.cpp \ test/fuzz/hex.cpp \ test/fuzz/http_request.cpp \ + test/fuzz/i2p.cpp \ test/fuzz/integer.cpp \ test/fuzz/key.cpp \ test/fuzz/key_io.cpp \ diff --git a/src/Makefile.test_util.include b/src/Makefile.test_util.include index f7f393ccac..85e50ebf70 100644 --- a/src/Makefile.test_util.include +++ b/src/Makefile.test_util.include @@ -26,6 +26,7 @@ libtest_util_a_SOURCES = \ test/util/logging.cpp \ test/util/mining.cpp \ test/util/net.cpp \ + test/util/script.cpp \ test/util/setup_common.cpp \ test/util/str.cpp \ test/util/transaction_utils.cpp \ diff --git a/src/i2p.cpp b/src/i2p.cpp index d16c620d88..a44f09f043 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -20,6 +20,7 @@ #include <util/system.h> #include <chrono> +#include <memory> #include <stdexcept> #include <string> @@ -115,7 +116,8 @@ namespace sam { Session::Session(const fs::path& private_key_file, const CService& control_host, CThreadInterrupt* interrupt) - : m_private_key_file(private_key_file), m_control_host(control_host), m_interrupt(interrupt) + : m_private_key_file(private_key_file), m_control_host(control_host), m_interrupt(interrupt), + m_control_sock(std::make_unique<Sock>(INVALID_SOCKET)) { } @@ -145,7 +147,7 @@ bool Session::Accept(Connection& conn) try { while (!*m_interrupt) { Sock::Event occurred; - conn.sock.Wait(MAX_WAIT_FOR_IO, Sock::RECV, &occurred); + conn.sock->Wait(MAX_WAIT_FOR_IO, Sock::RECV, &occurred); if ((occurred & Sock::RECV) == 0) { // Timeout, no incoming connections within MAX_WAIT_FOR_IO. @@ -153,7 +155,7 @@ bool Session::Accept(Connection& conn) } const std::string& peer_dest = - conn.sock.RecvUntilTerminator('\n', MAX_WAIT_FOR_IO, *m_interrupt, MAX_MSG_SIZE); + conn.sock->RecvUntilTerminator('\n', MAX_WAIT_FOR_IO, *m_interrupt, MAX_MSG_SIZE); conn.peer = CService(DestB64ToAddr(peer_dest), Params().GetDefaultPort()); @@ -171,7 +173,7 @@ bool Session::Connect(const CService& to, Connection& conn, bool& proxy_error) proxy_error = true; std::string session_id; - Sock sock; + std::unique_ptr<Sock> sock; conn.peer = to; try { @@ -184,12 +186,12 @@ bool Session::Connect(const CService& to, Connection& conn, bool& proxy_error) } const Reply& lookup_reply = - SendRequestAndGetReply(sock, strprintf("NAMING LOOKUP NAME=%s", to.ToStringIP())); + SendRequestAndGetReply(*sock, strprintf("NAMING LOOKUP NAME=%s", to.ToStringIP())); const std::string& dest = lookup_reply.Get("VALUE"); const Reply& connect_reply = SendRequestAndGetReply( - sock, strprintf("STREAM CONNECT ID=%s DESTINATION=%s SILENT=false", session_id, dest), + *sock, strprintf("STREAM CONNECT ID=%s DESTINATION=%s SILENT=false", session_id, dest), false); const std::string& result = connect_reply.Get("RESULT"); @@ -271,7 +273,7 @@ Session::Reply Session::SendRequestAndGetReply(const Sock& sock, return reply; } -Sock Session::Hello() const +std::unique_ptr<Sock> Session::Hello() const { auto sock = CreateSock(m_control_host); @@ -279,13 +281,13 @@ Sock Session::Hello() const throw std::runtime_error("Cannot create socket"); } - if (!ConnectSocketDirectly(m_control_host, sock->Get(), nConnectTimeout, true)) { + if (!ConnectSocketDirectly(m_control_host, *sock, nConnectTimeout, true)) { throw std::runtime_error(strprintf("Cannot connect to %s", m_control_host.ToString())); } SendRequestAndGetReply(*sock, "HELLO VERSION MIN=3.1 MAX=3.1"); - return std::move(*sock); + return sock; } void Session::CheckControlSock() @@ -293,7 +295,7 @@ void Session::CheckControlSock() LOCK(m_mutex); std::string errmsg; - if (!m_control_sock.IsConnected(errmsg)) { + if (!m_control_sock->IsConnected(errmsg)) { Log("Control socket error: %s", errmsg); Disconnect(); } @@ -341,26 +343,26 @@ Binary Session::MyDestination() const void Session::CreateIfNotCreatedAlready() { std::string errmsg; - if (m_control_sock.IsConnected(errmsg)) { + if (m_control_sock->IsConnected(errmsg)) { return; } Log("Creating SAM session with %s", m_control_host.ToString()); - Sock sock = Hello(); + auto sock = Hello(); const auto& [read_ok, data] = ReadBinaryFile(m_private_key_file); if (read_ok) { m_private_key.assign(data.begin(), data.end()); } else { - GenerateAndSavePrivateKey(sock); + GenerateAndSavePrivateKey(*sock); } const std::string& session_id = GetRandHash().GetHex().substr(0, 10); // full is an overkill, too verbose in the logs const std::string& private_key_b64 = SwapBase64(EncodeBase64(m_private_key)); - SendRequestAndGetReply(sock, strprintf("SESSION CREATE STYLE=STREAM ID=%s DESTINATION=%s", - session_id, private_key_b64)); + SendRequestAndGetReply(*sock, strprintf("SESSION CREATE STYLE=STREAM ID=%s DESTINATION=%s", + session_id, private_key_b64)); m_my_addr = CService(DestBinToAddr(MyDestination()), Params().GetDefaultPort()); m_session_id = session_id; @@ -370,12 +372,12 @@ void Session::CreateIfNotCreatedAlready() m_my_addr.ToString()); } -Sock Session::StreamAccept() +std::unique_ptr<Sock> Session::StreamAccept() { - Sock sock = Hello(); + auto sock = Hello(); const Reply& reply = SendRequestAndGetReply( - sock, strprintf("STREAM ACCEPT ID=%s SILENT=false", m_session_id), false); + *sock, strprintf("STREAM ACCEPT ID=%s SILENT=false", m_session_id), false); const std::string& result = reply.Get("RESULT"); @@ -393,14 +395,14 @@ Sock Session::StreamAccept() void Session::Disconnect() { - if (m_control_sock.Get() != INVALID_SOCKET) { + if (m_control_sock->Get() != INVALID_SOCKET) { if (m_session_id.empty()) { Log("Destroying incomplete session"); } else { Log("Destroying session %s", m_session_id); } } - m_control_sock.Reset(); + m_control_sock->Reset(); m_session_id.clear(); } } // namespace sam @@ -12,6 +12,7 @@ #include <threadinterrupt.h> #include <util/sock.h> +#include <memory> #include <optional> #include <string> #include <unordered_map> @@ -29,7 +30,7 @@ using Binary = std::vector<uint8_t>; */ struct Connection { /** Connected socket. */ - Sock sock; + std::unique_ptr<Sock> sock; /** Our I2P address. */ CService me; @@ -166,7 +167,7 @@ private: * @return a connected socket * @throws std::runtime_error if an error occurs */ - Sock Hello() const EXCLUSIVE_LOCKS_REQUIRED(m_mutex); + std::unique_ptr<Sock> Hello() const EXCLUSIVE_LOCKS_REQUIRED(m_mutex); /** * Check the control socket for errors and possibly disconnect. @@ -204,10 +205,11 @@ private: /** * Open a new connection to the SAM proxy and issue "STREAM ACCEPT" request using the existing - * session id. Return the idle socket that is waiting for a peer to connect to us. + * session id. + * @return the idle socket that is waiting for a peer to connect to us * @throws std::runtime_error if an error occurs */ - Sock StreamAccept() EXCLUSIVE_LOCKS_REQUIRED(m_mutex); + std::unique_ptr<Sock> StreamAccept() EXCLUSIVE_LOCKS_REQUIRED(m_mutex); /** * Destroy the session, closing the internally used sockets. @@ -248,7 +250,7 @@ private: * connections and make outgoing ones. * See https://geti2p.net/en/docs/api/samv3 */ - Sock m_control_sock GUARDED_BY(m_mutex); + std::unique_ptr<Sock> m_control_sock GUARDED_BY(m_mutex); /** * Our .b32.i2p address. diff --git a/src/init.cpp b/src/init.cpp index 9fb3f1294c..17b216573f 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -228,6 +228,7 @@ void Shutdown(NodeContext& node) node.peerman.reset(); node.connman.reset(); node.banman.reset(); + node.addrman.reset(); if (node.mempool && node.mempool->IsLoaded() && node.args->GetArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { DumpMempool(*node.mempool); @@ -1402,10 +1403,12 @@ bool AppInitMain(const std::any& context, NodeContext& node, interfaces::BlockAn fDiscover = args.GetBoolArg("-discover", true); const bool ignores_incoming_txs{args.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY)}; + assert(!node.addrman); + node.addrman = std::make_unique<CAddrMan>(); assert(!node.banman); node.banman = std::make_unique<BanMan>(GetDataDir() / "banlist.dat", &uiInterface, args.GetArg("-bantime", DEFAULT_MISBEHAVING_BANTIME)); assert(!node.connman); - node.connman = std::make_unique<CConnman>(GetRand(std::numeric_limits<uint64_t>::max()), GetRand(std::numeric_limits<uint64_t>::max()), args.GetBoolArg("-networkactive", true)); + node.connman = std::make_unique<CConnman>(GetRand(std::numeric_limits<uint64_t>::max()), GetRand(std::numeric_limits<uint64_t>::max()), *node.addrman, args.GetBoolArg("-networkactive", true)); assert(!node.fee_estimator); // Don't initialize fee estimation with old data if we don't relay transactions, @@ -1421,7 +1424,7 @@ bool AppInitMain(const std::any& context, NodeContext& node, interfaces::BlockAn ChainstateManager& chainman = *Assert(node.chainman); assert(!node.peerman); - node.peerman = PeerManager::make(chainparams, *node.connman, node.banman.get(), + node.peerman = PeerManager::make(chainparams, *node.connman, *node.addrman, node.banman.get(), *node.scheduler, chainman, *node.mempool, ignores_incoming_txs); RegisterValidationInterface(node.peerman.get()); diff --git a/src/net.cpp b/src/net.cpp index 3b1ebede98..1dcb141421 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -31,6 +31,10 @@ #include <fcntl.h> #endif +#if HAVE_DECL_GETIFADDRS && HAVE_DECL_FREEIFADDRS +#include <ifaddrs.h> +#endif + #ifdef USE_POLL #include <poll.h> #endif @@ -432,7 +436,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo i2p::Connection conn; if (m_i2p_sam_session->Connect(addrConnect, conn, proxyConnectionFailed)) { connected = true; - sock = std::make_unique<Sock>(std::move(conn.sock)); + sock = std::move(conn.sock); addr_bind = CAddress{conn.me, NODE_NONE}; } } else if (GetProxy(addrConnect.GetNetwork(), proxy)) { @@ -448,7 +452,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo if (!sock) { return nullptr; } - connected = ConnectSocketDirectly(addrConnect, sock->Get(), nConnectTimeout, + connected = ConnectSocketDirectly(addrConnect, *sock, nConnectTimeout, conn_type == ConnectionType::MANUAL); } if (!proxyConnectionFailed) { @@ -840,6 +844,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; } @@ -870,13 +880,51 @@ static bool CompareNodeBlockRelayOnlyTime(const NodeEvictionCandidate &a, const return a.nTimeConnected > b.nTimeConnected; } -//! Sort an array by the specified comparator, then erase the last K elements. -template<typename T, typename Comparator> -static void EraseLastKElements(std::vector<T> &elements, Comparator comparator, size_t k) +//! Sort an array by the specified comparator, then erase the last K elements where predicate is true. +template <typename T, typename Comparator> +static void EraseLastKElements( + std::vector<T>& elements, Comparator comparator, size_t k, + std::function<bool(const NodeEvictionCandidate&)> predicate = [](const NodeEvictionCandidate& n) { return true; }) { std::sort(elements.begin(), elements.end(), comparator); size_t eraseSize = std::min(k, elements.size()); - elements.erase(elements.end() - eraseSize, elements.end()); + elements.erase(std::remove_if(elements.end() - eraseSize, elements.end(), predicate), elements.end()); +} + +void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& vEvictionCandidates) +{ + // 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 + // 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; + + 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. + total_protect_size -= initial_size - vEvictionCandidates.size(); + EraseLastKElements(vEvictionCandidates, ReverseCompareNodeTimeConnected, total_protect_size); } [[nodiscard]] std::optional<NodeId> SelectNodeToEvict(std::vector<NodeEvictionCandidate>&& vEvictionCandidates) @@ -893,30 +941,17 @@ static void EraseLastKElements(std::vector<T> &elements, Comparator comparator, // 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. - std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), CompareNodeBlockRelayOnlyTime); - size_t erase_size = std::min(size_t(8), vEvictionCandidates.size()); - vEvictionCandidates.erase(std::remove_if(vEvictionCandidates.end() - erase_size, vEvictionCandidates.end(), [](NodeEvictionCandidate const &n) { return !n.fRelayTxes && n.fRelevantServices; }), vEvictionCandidates.end()); + const size_t erase_size = std::min(size_t(8), vEvictionCandidates.size()); + EraseLastKElements(vEvictionCandidates, CompareNodeBlockRelayOnlyTime, erase_size, + [](const NodeEvictionCandidate& n) { return !n.fRelayTxes && n.fRelevantServices; }); // Protect 4 nodes that most recently sent us novel blocks. // An attacker cannot manipulate this metric without performing useful work. EraseLastKElements(vEvictionCandidates, CompareNodeBlockTime, 4); - // 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. - size_t initial_size = vEvictionCandidates.size(); - size_t total_protect_size = initial_size / 2; - - // Pick out up to 1/4 peers that are localhost, sorted by longest uptime. - std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), CompareLocalHostTimeConnected); - size_t local_erase_size = total_protect_size / 2; - vEvictionCandidates.erase(std::remove_if(vEvictionCandidates.end() - local_erase_size, vEvictionCandidates.end(), [](NodeEvictionCandidate const &n) { return n.m_is_local; }), vEvictionCandidates.end()); - // 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); + // Protect some of the remaining eviction candidates by ratios of desirable + // or disadvantaged characteristics. + ProtectEvictionCandidatesByRatio(vEvictionCandidates); if (vEvictionCandidates.empty()) return std::nullopt; @@ -937,7 +972,7 @@ static void EraseLastKElements(std::vector<T> &elements, Comparator comparator, for (const NodeEvictionCandidate &node : vEvictionCandidates) { std::vector<NodeEvictionCandidate> &group = mapNetGroupNodes[node.nKeyedNetGroup]; group.push_back(node); - int64_t grouptime = group[0].nTimeConnected; + const int64_t grouptime = group[0].nTimeConnected; if (group.size() > nMostConnections || (group.size() == nMostConnections && grouptime > nMostConnectionsTime)) { nMostConnections = group.size(); @@ -985,7 +1020,8 @@ bool CConnman::AttemptToEvictConnection() node->nLastBlockTime, node->nLastTXTime, HasAllDesirableServiceFlags(node->nServices), peer_relay_txes, peer_filter_not_null, node->nKeyedNetGroup, - node->m_prefer_evict, node->addr.IsLocal()}; + node->m_prefer_evict, node->addr.IsLocal(), + node->m_inbound_onion}; vEvictionCandidates.push_back(candidate); } } @@ -2221,7 +2257,7 @@ void CConnman::ThreadI2PAcceptIncoming() continue; } - CreateNodeFromAcceptedSocket(conn.sock.Release(), NetPermissionFlags::PF_NONE, + CreateNodeFromAcceptedSocket(conn.sock->Release(), NetPermissionFlags::PF_NONE, CAddress{conn.me, NODE_NONE}, CAddress{conn.peer, NODE_NONE}); } } @@ -2351,8 +2387,8 @@ void CConnman::SetNetworkActive(bool active) uiInterface.NotifyNetworkActiveChanged(fNetworkActive); } -CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In, bool network_active) - : nSeed0(nSeed0In), nSeed1(nSeed1In) +CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In, CAddrMan& addrman_in, bool network_active) + : addrman(addrman_in), nSeed0(nSeed0In), nSeed1(nSeed1In) { SetTryNewOutboundPeer(false); @@ -2621,11 +2657,7 @@ void CConnman::StopNodes() void CConnman::DeleteNode(CNode* pnode) { assert(pnode); - bool fUpdateConnectionTime = false; - m_msgproc->FinalizeNode(*pnode, fUpdateConnectionTime); - if (fUpdateConnectionTime) { - addrman.Connected(pnode->addr); - } + m_msgproc->FinalizeNode(*pnode); delete pnode; } @@ -2635,21 +2667,6 @@ CConnman::~CConnman() Stop(); } -void CConnman::SetServices(const CService &addr, ServiceFlags nServices) -{ - addrman.SetServices(addr, nServices); -} - -void CConnman::MarkAddressGood(const CAddress& addr) -{ - addrman.Good(addr); -} - -bool CConnman::AddNewAddresses(const std::vector<CAddress>& vAddr, const CAddress& addrFrom, int64_t nTimePenalty) -{ - return addrman.Add(vAddr, addrFrom, nTimePenalty); -} - std::vector<CAddress> CConnman::GetAddresses(size_t max_addresses, size_t max_pct) { std::vector<CAddress> addresses = addrman.GetAddr(max_addresses, max_pct); @@ -425,6 +425,7 @@ public: std::atomic<int64_t> nLastSend{0}; std::atomic<int64_t> nLastRecv{0}; + //! Unix epoch time at peer connection, in seconds. const int64_t nTimeConnected; std::atomic<int64_t> nTimeOffset{0}; // Address of this peer @@ -770,7 +771,7 @@ public: virtual void InitializeNode(CNode* pnode) = 0; /** Handle removal of a peer (clear state) */ - virtual void FinalizeNode(const CNode& node, bool& update_connection_time) = 0; + virtual void FinalizeNode(const CNode& node) = 0; /** * Process protocol messages received from a given node @@ -856,7 +857,7 @@ public: m_onion_binds = connOptions.onion_binds; } - CConnman(uint64_t seed0, uint64_t seed1, bool network_active = true); + CConnman(uint64_t seed0, uint64_t seed1, CAddrMan& addrman, bool network_active = true); ~CConnman(); bool Start(CScheduler& scheduler, const Options& options); @@ -921,9 +922,6 @@ public: }; // Addrman functions - void SetServices(const CService &addr, ServiceFlags nServices); - void MarkAddressGood(const CAddress& addr); - bool AddNewAddresses(const std::vector<CAddress>& vAddr, const CAddress& addrFrom, int64_t nTimePenalty = 0); std::vector<CAddress> GetAddresses(size_t max_addresses, size_t max_pct); /** * Cache is used to minimize topology leaks, so it should @@ -1130,7 +1128,7 @@ private: std::vector<ListenSocket> vhListenSocket; std::atomic<bool> fNetworkActive{true}; bool fAddressesInitialized{false}; - CAddrMan addrman; + CAddrMan& addrman; std::deque<std::string> m_addr_fetches GUARDED_BY(m_addr_fetches_mutex); RecursiveMutex m_addr_fetches_mutex; std::vector<std::string> vAddedNodes GUARDED_BY(cs_vAddedNodes); @@ -1281,8 +1279,39 @@ struct NodeEvictionCandidate uint64_t nKeyedNetGroup; bool prefer_evict; bool m_is_local; + bool m_is_onion; }; +/** + * 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 (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); +/** Protect desirable or disadvantaged inbound peers from eviction by ratio. + * + * This function protects half of the peers which have been connected the + * 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. + * + * 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. + * + * This function was extracted from SelectNodeToEvict() to be able to test the + * ratio-based protection logic deterministically. + */ +void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& vEvictionCandidates); + #endif // BITCOIN_NET_H diff --git a/src/net_processing.cpp b/src/net_processing.cpp index d327a69a93..e4054968c0 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -225,9 +225,9 @@ using PeerRef = std::shared_ptr<Peer>; class PeerManagerImpl final : public PeerManager { public: - PeerManagerImpl(const CChainParams& chainparams, CConnman& connman, BanMan* banman, - CScheduler& scheduler, ChainstateManager& chainman, CTxMemPool& pool, - bool ignore_incoming_txs); + PeerManagerImpl(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, + BanMan* banman, CScheduler& scheduler, ChainstateManager& chainman, + CTxMemPool& pool, bool ignore_incoming_txs); /** Overridden from CValidationInterface. */ void BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexConnected) override; @@ -238,7 +238,7 @@ public: /** Implement NetEventsInterface */ void InitializeNode(CNode* pnode) override; - void FinalizeNode(const CNode& node, bool& fUpdateConnectionTime) override; + void FinalizeNode(const CNode& node) override; bool ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt) override; bool SendMessages(CNode* pto) override EXCLUSIVE_LOCKS_REQUIRED(pto->cs_sendProcessing); @@ -322,6 +322,7 @@ private: const CChainParams& m_chainparams; CConnman& m_connman; + CAddrMan& m_addrman; /** Pointer to this node's banman. May be nullptr - check existence before dereferencing. */ BanMan* const m_banman; ChainstateManager& m_chainman; @@ -968,13 +969,13 @@ void PeerManagerImpl::ReattemptInitialBroadcast(CScheduler& scheduler) scheduler.scheduleFromNow([&] { ReattemptInitialBroadcast(scheduler); }, delta); } -void PeerManagerImpl::FinalizeNode(const CNode& node, bool& fUpdateConnectionTime) +void PeerManagerImpl::FinalizeNode(const CNode& node) { NodeId nodeid = node.GetId(); - fUpdateConnectionTime = false; - LOCK(cs_main); int misbehavior{0}; { + LOCK(cs_main); + { // We remove the PeerRef from g_peer_map here, but we don't always // destruct the Peer. Sometimes another thread is still holding a // PeerRef, so the refcount is >= 1. Be careful not to do any @@ -990,12 +991,6 @@ void PeerManagerImpl::FinalizeNode(const CNode& node, bool& fUpdateConnectionTim if (state->fSyncStarted) nSyncStarted--; - if (node.fSuccessfullyConnected && misbehavior == 0 && - !node.IsBlockOnlyConn() && !node.IsInboundConn()) { - // Only change visible addrman state for outbound, full-relay peers - fUpdateConnectionTime = true; - } - for (const QueuedBlock& entry : state->vBlocksInFlight) { mapBlocksInFlight.erase(entry.hash); } @@ -1020,6 +1015,14 @@ void PeerManagerImpl::FinalizeNode(const CNode& node, bool& fUpdateConnectionTim assert(m_wtxid_relay_peers == 0); assert(m_txrequest.Size() == 0); } + } // cs_main + if (node.fSuccessfullyConnected && misbehavior == 0 && + !node.IsBlockOnlyConn() && !node.IsInboundConn()) { + // Only change visible addrman state for full outbound peers. We don't + // call Connected() for feeler connections since they don't have + // fSuccessfullyConnected set. + m_addrman.Connected(node.addr); + } LogPrint(BCLog::NET, "Cleared nodestate for peer=%d\n", nodeid); } @@ -1201,18 +1204,19 @@ bool PeerManagerImpl::BlockRequestAllowed(const CBlockIndex* pindex) (GetBlockProofEquivalentTime(*pindexBestHeader, *pindex, *pindexBestHeader, m_chainparams.GetConsensus()) < STALE_RELAY_AGE_LIMIT); } -std::unique_ptr<PeerManager> PeerManager::make(const CChainParams& chainparams, CConnman& connman, BanMan* banman, - CScheduler& scheduler, ChainstateManager& chainman, CTxMemPool& pool, - bool ignore_incoming_txs) +std::unique_ptr<PeerManager> PeerManager::make(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, + BanMan* banman, CScheduler& scheduler, ChainstateManager& chainman, + CTxMemPool& pool, bool ignore_incoming_txs) { - return std::make_unique<PeerManagerImpl>(chainparams, connman, banman, scheduler, chainman, pool, ignore_incoming_txs); + return std::make_unique<PeerManagerImpl>(chainparams, connman, addrman, banman, scheduler, chainman, pool, ignore_incoming_txs); } -PeerManagerImpl::PeerManagerImpl(const CChainParams& chainparams, CConnman& connman, BanMan* banman, - CScheduler& scheduler, ChainstateManager& chainman, CTxMemPool& pool, - bool ignore_incoming_txs) +PeerManagerImpl::PeerManagerImpl(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, + BanMan* banman, CScheduler& scheduler, ChainstateManager& chainman, + CTxMemPool& pool, bool ignore_incoming_txs) : m_chainparams(chainparams), m_connman(connman), + m_addrman(addrman), m_banman(banman), m_chainman(chainman), m_mempool(pool), @@ -2330,7 +2334,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, nServices = ServiceFlags(nServiceInt); if (!pfrom.IsInboundConn()) { - m_connman.SetServices(pfrom.addr, nServices); + m_addrman.SetServices(pfrom.addr, nServices); } if (pfrom.ExpectServicesFromConn() && !HasAllDesirableServiceFlags(nServices)) { @@ -2474,7 +2478,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // // This moves an address from New to Tried table in Addrman, // resolves tried-table collisions, etc. - m_connman.MarkAddressGood(pfrom.addr); + m_addrman.Good(pfrom.addr); } std::string remoteAddr; @@ -2679,7 +2683,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (fReachable) vAddrOk.push_back(addr); } - m_connman.AddNewAddresses(vAddrOk, pfrom.addr, 2 * 60 * 60); + m_addrman.Add(vAddrOk, pfrom.addr, 2 * 60 * 60); if (vAddr.size() < 1000) pfrom.fGetAddr = false; if (pfrom.IsAddrFetchConn()) { diff --git a/src/net_processing.h b/src/net_processing.h index f6f2d73721..4556d32377 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -10,6 +10,7 @@ #include <sync.h> #include <validationinterface.h> +class CAddrMan; class CChainParams; class CTxMemPool; class ChainstateManager; @@ -36,9 +37,9 @@ struct CNodeStateStats { class PeerManager : public CValidationInterface, public NetEventsInterface { public: - static std::unique_ptr<PeerManager> make(const CChainParams& chainparams, CConnman& connman, BanMan* banman, - CScheduler& scheduler, ChainstateManager& chainman, CTxMemPool& pool, - bool ignore_incoming_txs); + static std::unique_ptr<PeerManager> make(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, + BanMan* banman, CScheduler& scheduler, ChainstateManager& chainman, + CTxMemPool& pool, bool ignore_incoming_txs); virtual ~PeerManager() { } /** Get statistics from node state */ diff --git a/src/netbase.cpp b/src/netbase.cpp index 49e455aa84..2980bdf459 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -537,12 +537,12 @@ static void LogConnectFailure(bool manual_connection, const char* fmt, const Arg } } -bool ConnectSocketDirectly(const CService &addrConnect, const SOCKET& hSocket, int nTimeout, bool manual_connection) +bool ConnectSocketDirectly(const CService &addrConnect, const Sock& sock, int nTimeout, bool manual_connection) { // Create a sockaddr from the specified service. struct sockaddr_storage sockaddr; socklen_t len = sizeof(sockaddr); - if (hSocket == INVALID_SOCKET) { + if (sock.Get() == INVALID_SOCKET) { LogPrintf("Cannot connect to %s: invalid socket\n", addrConnect.ToString()); return false; } @@ -552,8 +552,7 @@ bool ConnectSocketDirectly(const CService &addrConnect, const SOCKET& hSocket, i } // Connect to the addrConnect service on the hSocket socket. - if (connect(hSocket, (struct sockaddr*)&sockaddr, len) == SOCKET_ERROR) - { + if (sock.Connect(reinterpret_cast<struct sockaddr*>(&sockaddr), len) == SOCKET_ERROR) { int nErr = WSAGetLastError(); // WSAEINVAL is here because some legacy version of winsock uses it if (nErr == WSAEINPROGRESS || nErr == WSAEWOULDBLOCK || nErr == WSAEINVAL) @@ -561,46 +560,34 @@ bool ConnectSocketDirectly(const CService &addrConnect, const SOCKET& hSocket, i // Connection didn't actually fail, but is being established // asynchronously. Thus, use async I/O api (select/poll) // synchronously to check for successful connection with a timeout. -#ifdef USE_POLL - struct pollfd pollfd = {}; - pollfd.fd = hSocket; - pollfd.events = POLLIN | POLLOUT; - int nRet = poll(&pollfd, 1, nTimeout); -#else - struct timeval timeout = MillisToTimeval(nTimeout); - fd_set fdset; - FD_ZERO(&fdset); - FD_SET(hSocket, &fdset); - int nRet = select(hSocket + 1, nullptr, &fdset, nullptr, &timeout); -#endif - // Upon successful completion, both select and poll return the total - // number of file descriptors that have been selected. A value of 0 - // indicates that the call timed out and no file descriptors have - // been selected. - if (nRet == 0) - { - LogPrint(BCLog::NET, "connection to %s timeout\n", addrConnect.ToString()); + const Sock::Event requested = Sock::RECV | Sock::SEND; + Sock::Event occurred; + if (!sock.Wait(std::chrono::milliseconds{nTimeout}, requested, &occurred)) { + LogPrintf("wait for connect to %s failed: %s\n", + addrConnect.ToString(), + NetworkErrorString(WSAGetLastError())); return false; - } - if (nRet == SOCKET_ERROR) - { - LogPrintf("select() for %s failed: %s\n", addrConnect.ToString(), NetworkErrorString(WSAGetLastError())); + } else if (occurred == 0) { + LogPrint(BCLog::NET, "connection attempt to %s timed out\n", addrConnect.ToString()); return false; } - // Even if the select/poll was successful, the connect might not + // Even if the wait was successful, the connect might not // have been successful. The reason for this failure is hidden away // in the SO_ERROR for the socket in modern systems. We read it into - // nRet here. - socklen_t nRetSize = sizeof(nRet); - if (getsockopt(hSocket, SOL_SOCKET, SO_ERROR, (sockopt_arg_type)&nRet, &nRetSize) == SOCKET_ERROR) - { + // sockerr here. + int sockerr; + socklen_t sockerr_len = sizeof(sockerr); + if (sock.GetSockOpt(SOL_SOCKET, SO_ERROR, (sockopt_arg_type)&sockerr, &sockerr_len) == + SOCKET_ERROR) { LogPrintf("getsockopt() for %s failed: %s\n", addrConnect.ToString(), NetworkErrorString(WSAGetLastError())); return false; } - if (nRet != 0) - { - LogConnectFailure(manual_connection, "connect() to %s failed after select(): %s", addrConnect.ToString(), NetworkErrorString(nRet)); + if (sockerr != 0) { + LogConnectFailure(manual_connection, + "connect() to %s failed after wait: %s", + addrConnect.ToString(), + NetworkErrorString(sockerr)); return false; } } @@ -668,7 +655,7 @@ bool IsProxy(const CNetAddr &addr) { bool ConnectThroughProxy(const proxyType& proxy, const std::string& strDest, uint16_t port, const Sock& sock, int nTimeout, bool& outProxyConnectionFailed) { // first connect to proxy server - if (!ConnectSocketDirectly(proxy.proxy, sock.Get(), nTimeout, true)) { + if (!ConnectSocketDirectly(proxy.proxy, sock, nTimeout, true)) { outProxyConnectionFailed = true; return false; } diff --git a/src/netbase.h b/src/netbase.h index 08172b9984..1f35c29dcb 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -194,7 +194,7 @@ extern std::function<std::unique_ptr<Sock>(const CService&)> CreateSock; * Try to connect to the specified service on the specified socket. * * @param addrConnect The service to which to connect. - * @param hSocket The socket on which to connect. + * @param sock The socket on which to connect. * @param nTimeout Wait this many milliseconds for the connection to be * established. * @param manual_connection Whether or not the connection was manually requested @@ -202,7 +202,7 @@ extern std::function<std::unique_ptr<Sock>(const CService&)> CreateSock; * * @returns Whether or not a connection was successfully made. */ -bool ConnectSocketDirectly(const CService &addrConnect, const SOCKET& hSocket, int nTimeout, bool manual_connection); +bool ConnectSocketDirectly(const CService &addrConnect, const Sock& sock, int nTimeout, bool manual_connection); /** * Connect to a specified destination service through a SOCKS5 proxy by first diff --git a/src/node/context.cpp b/src/node/context.cpp index 958221a913..6d22a6b110 100644 --- a/src/node/context.cpp +++ b/src/node/context.cpp @@ -4,6 +4,7 @@ #include <node/context.h> +#include <addrman.h> #include <banman.h> #include <interfaces/chain.h> #include <net.h> diff --git a/src/node/context.h b/src/node/context.h index 9b611bf8f5..2be9a584e6 100644 --- a/src/node/context.h +++ b/src/node/context.h @@ -12,6 +12,7 @@ class ArgsManager; class BanMan; +class CAddrMan; class CBlockPolicyEstimator; class CConnman; class CScheduler; @@ -35,6 +36,7 @@ class WalletClient; //! any member functions. It should just be a collection of references that can //! be used without pulling in unwanted dependencies or functionality. struct NodeContext { + std::unique_ptr<CAddrMan> addrman; std::unique_ptr<CConnman> connman; std::unique_ptr<CTxMemPool> mempool; std::unique_ptr<CBlockPolicyEstimator> fee_estimator; diff --git a/src/randomenv.cpp b/src/randomenv.cpp index 9248db1539..fa2a3a0607 100644 --- a/src/randomenv.cpp +++ b/src/randomenv.cpp @@ -38,7 +38,7 @@ #include <sys/utsname.h> #include <unistd.h> #endif -#if HAVE_DECL_GETIFADDRS +#if HAVE_DECL_GETIFADDRS && HAVE_DECL_FREEIFADDRS #include <ifaddrs.h> #endif #if HAVE_SYSCTL @@ -361,7 +361,7 @@ void RandAddStaticEnv(CSHA512& hasher) hasher.Write((const unsigned char*)hname, strnlen(hname, 256)); } -#if HAVE_DECL_GETIFADDRS +#if HAVE_DECL_GETIFADDRS && HAVE_DECL_FREEIFADDRS // Network interfaces struct ifaddrs *ifad = NULL; getifaddrs(&ifad); diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index d4c1ab4b53..96533a50c8 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -907,8 +907,8 @@ static RPCHelpMan addpeeraddress() [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { NodeContext& node = EnsureNodeContext(request.context); - if (!node.connman) { - throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); + if (!node.addrman) { + throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Address manager functionality missing or disabled"); } UniValue obj(UniValue::VOBJ); @@ -925,7 +925,7 @@ static RPCHelpMan addpeeraddress() address.nTime = GetAdjustedTime(); // The source address is set equal to the address. This is equivalent to the peer // announcing itself. - if (!node.connman->AddNewAddresses({address}, address)) { + if (!node.addrman->Add(address, address)) { obj.pushKV("success", false); return obj; } diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index e75982bc6f..7557d4618a 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -67,9 +67,9 @@ BOOST_FIXTURE_TEST_SUITE(denialofservice_tests, TestingSetup) BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) { const CChainParams& chainparams = Params(); - auto connman = std::make_unique<CConnman>(0x1337, 0x1337); - auto peerLogic = PeerManager::make(chainparams, *connman, nullptr, *m_node.scheduler, - *m_node.chainman, *m_node.mempool, false); + auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman); + auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, + *m_node.scheduler, *m_node.chainman, *m_node.mempool, false); // Mock an outbound peer CAddress addr1(ip(0xa0b0c001), NODE_NONE); @@ -117,8 +117,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) BOOST_CHECK(dummyNode1.fDisconnect == true); SetMockTime(0); - bool dummy; - peerLogic->FinalizeNode(dummyNode1, dummy); + peerLogic->FinalizeNode(dummyNode1); } static void AddRandomOutboundPeer(std::vector<CNode *> &vNodes, PeerManager &peerLogic, CConnmanTest* connman) @@ -137,9 +136,9 @@ static void AddRandomOutboundPeer(std::vector<CNode *> &vNodes, PeerManager &pee BOOST_AUTO_TEST_CASE(stale_tip_peer_management) { const CChainParams& chainparams = Params(); - auto connman = std::make_unique<CConnmanTest>(0x1337, 0x1337); - auto peerLogic = PeerManager::make(chainparams, *connman, nullptr, *m_node.scheduler, - *m_node.chainman, *m_node.mempool, false); + auto connman = std::make_unique<CConnmanTest>(0x1337, 0x1337, *m_node.addrman); + auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, + *m_node.scheduler, *m_node.chainman, *m_node.mempool, false); constexpr int max_outbound_full_relay = MAX_OUTBOUND_FULL_RELAY_CONNECTIONS; CConnman::Options options; @@ -199,9 +198,8 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) BOOST_CHECK(vNodes[max_outbound_full_relay-1]->fDisconnect == true); BOOST_CHECK(vNodes.back()->fDisconnect == false); - bool dummy; for (const CNode *node : vNodes) { - peerLogic->FinalizeNode(*node, dummy); + peerLogic->FinalizeNode(*node); } connman->ClearNodes(); @@ -211,9 +209,9 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) { const CChainParams& chainparams = Params(); auto banman = std::make_unique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); - auto connman = std::make_unique<CConnman>(0x1337, 0x1337); - auto peerLogic = PeerManager::make(chainparams, *connman, banman.get(), *m_node.scheduler, - *m_node.chainman, *m_node.mempool, false); + auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman); + auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), + *m_node.scheduler, *m_node.chainman, *m_node.mempool, false); banman->ClearBanned(); CAddress addr1(ip(0xa0b0c001), NODE_NONE); @@ -249,18 +247,17 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) BOOST_CHECK(banman->IsDiscouraged(addr1)); // Expect both 1 and 2 BOOST_CHECK(banman->IsDiscouraged(addr2)); // to be discouraged now - bool dummy; - peerLogic->FinalizeNode(dummyNode1, dummy); - peerLogic->FinalizeNode(dummyNode2, dummy); + peerLogic->FinalizeNode(dummyNode1); + peerLogic->FinalizeNode(dummyNode2); } BOOST_AUTO_TEST_CASE(DoS_bantime) { const CChainParams& chainparams = Params(); auto banman = std::make_unique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); - auto connman = std::make_unique<CConnman>(0x1337, 0x1337); - auto peerLogic = PeerManager::make(chainparams, *connman, banman.get(), *m_node.scheduler, - *m_node.chainman, *m_node.mempool, false); + auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman); + auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), + *m_node.scheduler, *m_node.chainman, *m_node.mempool, false); banman->ClearBanned(); int64_t nStartTime = GetTime(); @@ -279,8 +276,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) } BOOST_CHECK(banman->IsDiscouraged(addr)); - bool dummy; - peerLogic->FinalizeNode(dummyNode, dummy); + peerLogic->FinalizeNode(dummyNode); } class TxOrphanageTest : public TxOrphanage diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index d951bda20f..b21d2eae79 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -230,11 +230,8 @@ FUZZ_TARGET_INIT(coins_view, initialize_coins_view) // consensus/tx_verify.cpp:171: bool Consensus::CheckTxInputs(const CTransaction &, TxValidationState &, const CCoinsViewCache &, int, CAmount &): Assertion `!coin.IsSpent()' failed. return; } - try { - (void)Consensus::CheckTxInputs(transaction, state, coins_view_cache, fuzzed_data_provider.ConsumeIntegralInRange<int>(0, std::numeric_limits<int>::max()), tx_fee_out); - assert(MoneyRange(tx_fee_out)); - } catch (const std::runtime_error&) { - } + (void)Consensus::CheckTxInputs(transaction, state, coins_view_cache, fuzzed_data_provider.ConsumeIntegralInRange<int>(0, std::numeric_limits<int>::max()), tx_fee_out); + assert(MoneyRange(tx_fee_out)); }, [&] { const CTransaction transaction{random_mutable_transaction}; diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index ae77a45e44..e07f25dedf 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -25,40 +25,25 @@ FUZZ_TARGET_INIT(connman, initialize_connman) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; SetMockTime(ConsumeTime(fuzzed_data_provider)); - CConnman connman{fuzzed_data_provider.ConsumeIntegral<uint64_t>(), fuzzed_data_provider.ConsumeIntegral<uint64_t>(), fuzzed_data_provider.ConsumeBool()}; - CAddress random_address; + CAddrMan addrman; + CConnman connman{fuzzed_data_provider.ConsumeIntegral<uint64_t>(), fuzzed_data_provider.ConsumeIntegral<uint64_t>(), addrman, fuzzed_data_provider.ConsumeBool()}; CNetAddr random_netaddr; CNode random_node = ConsumeNode(fuzzed_data_provider); - CService random_service; CSubNet random_subnet; std::string random_string; while (fuzzed_data_provider.ConsumeBool()) { CallOneOf( fuzzed_data_provider, [&] { - random_address = ConsumeAddress(fuzzed_data_provider); - }, - [&] { random_netaddr = ConsumeNetAddr(fuzzed_data_provider); }, [&] { - random_service = ConsumeService(fuzzed_data_provider); - }, - [&] { random_subnet = ConsumeSubNet(fuzzed_data_provider); }, [&] { random_string = fuzzed_data_provider.ConsumeRandomLengthString(64); }, [&] { - std::vector<CAddress> addresses; - while (fuzzed_data_provider.ConsumeBool()) { - addresses.push_back(ConsumeAddress(fuzzed_data_provider)); - } - // Limit nTimePenalty to int32_t to avoid signed integer overflow - (void)connman.AddNewAddresses(addresses, ConsumeAddress(fuzzed_data_provider), fuzzed_data_provider.ConsumeIntegral<int32_t>()); - }, - [&] { connman.AddNode(random_string); }, [&] { @@ -98,9 +83,6 @@ FUZZ_TARGET_INIT(connman, initialize_connman) (void)connman.GetNodeCount(fuzzed_data_provider.PickValueInArray({ConnectionDirection::None, ConnectionDirection::In, ConnectionDirection::Out, ConnectionDirection::Both})); }, [&] { - connman.MarkAddressGood(random_address); - }, - [&] { (void)connman.OutboundTargetReached(fuzzed_data_provider.ConsumeBool()); }, [&] { @@ -128,9 +110,6 @@ FUZZ_TARGET_INIT(connman, initialize_connman) connman.SetNetworkActive(fuzzed_data_provider.ConsumeBool()); }, [&] { - connman.SetServices(random_service, ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS)); - }, - [&] { connman.SetTryNewOutboundPeer(fuzzed_data_provider.ConsumeBool()); }); } diff --git a/src/test/fuzz/i2p.cpp b/src/test/fuzz/i2p.cpp new file mode 100644 index 0000000000..345d68502a --- /dev/null +++ b/src/test/fuzz/i2p.cpp @@ -0,0 +1,57 @@ +// Copyright (c) 2020-2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <i2p.h> +#include <netaddress.h> +#include <netbase.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> +#include <test/util/setup_common.h> +#include <threadinterrupt.h> +#include <util/system.h> + +void initialize_i2p() +{ + static const auto testing_setup = MakeNoLogFileContext<>(); +} + +FUZZ_TARGET_INIT(i2p, initialize_i2p) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + + // Mock CreateSock() to create FuzzedSock. + auto CreateSockOrig = CreateSock; + CreateSock = [&fuzzed_data_provider](const CService&) { + return std::make_unique<FuzzedSock>(fuzzed_data_provider); + }; + + const CService sam_proxy; + CThreadInterrupt interrupt; + + i2p::sam::Session sess{GetDataDir() / "fuzzed_i2p_private_key", sam_proxy, &interrupt}; + + i2p::Connection conn; + + if (sess.Listen(conn)) { + if (sess.Accept(conn)) { + try { + conn.sock->RecvUntilTerminator('\n', 10ms, interrupt, i2p::sam::MAX_MSG_SIZE); + } catch (const std::runtime_error&) { + } + } + } + + const CService to; + bool proxy_error; + + if (sess.Connect(to, conn, proxy_error)) { + try { + conn.sock->SendComplete("verack\n", 10ms, interrupt); + } catch (const std::runtime_error&) { + } + } + + CreateSock = CreateSockOrig; +} diff --git a/src/test/fuzz/node_eviction.cpp b/src/test/fuzz/node_eviction.cpp index 603d520cf5..70ffc6bf37 100644 --- a/src/test/fuzz/node_eviction.cpp +++ b/src/test/fuzz/node_eviction.cpp @@ -31,6 +31,7 @@ FUZZ_TARGET(node_eviction) /* nKeyedNetGroup */ fuzzed_data_provider.ConsumeIntegral<uint64_t>(), /* prefer_evict */ fuzzed_data_provider.ConsumeBool(), /* m_is_local */ fuzzed_data_provider.ConsumeBool(), + /* m_is_onion */ fuzzed_data_provider.ConsumeBool(), }); } // Make a copy since eviction_candidates may be in some valid but otherwise diff --git a/src/test/fuzz/script_flags.cpp b/src/test/fuzz/script_flags.cpp index 561230707c..387f9c069c 100644 --- a/src/test/fuzz/script_flags.cpp +++ b/src/test/fuzz/script_flags.cpp @@ -5,13 +5,11 @@ #include <pubkey.h> #include <script/interpreter.h> #include <streams.h> +#include <test/util/script.h> #include <version.h> #include <test/fuzz/fuzz.h> -/** Flags that are not forbidden by an assert */ -static bool IsValidFlagCombination(unsigned flags); - void initialize_script_flags() { static const ECCVerifyHandle verify_handle; @@ -74,10 +72,3 @@ FUZZ_TARGET_INIT(script_flags, initialize_script_flags) return; } } - -static bool IsValidFlagCombination(unsigned flags) -{ - if (flags & SCRIPT_VERIFY_CLEANSTACK && ~flags & (SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS)) return false; - if (flags & SCRIPT_VERIFY_WITNESS && ~flags & SCRIPT_VERIFY_P2SH) return false; - return true; -} diff --git a/src/test/fuzz/signature_checker.cpp b/src/test/fuzz/signature_checker.cpp index 7b57c5dfd8..6b86c8889d 100644 --- a/src/test/fuzz/signature_checker.cpp +++ b/src/test/fuzz/signature_checker.cpp @@ -6,6 +6,8 @@ #include <script/interpreter.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> +#include <test/util/script.h> #include <cstdint> #include <limits> @@ -56,17 +58,12 @@ FUZZ_TARGET_INIT(signature_checker, initialize_signature_checker) FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); const unsigned int flags = fuzzed_data_provider.ConsumeIntegral<unsigned int>(); const SigVersion sig_version = fuzzed_data_provider.PickValueInArray({SigVersion::BASE, SigVersion::WITNESS_V0}); - const std::string script_string_1 = fuzzed_data_provider.ConsumeRandomLengthString(65536); - const std::vector<uint8_t> script_bytes_1{script_string_1.begin(), script_string_1.end()}; - const std::string script_string_2 = fuzzed_data_provider.ConsumeRandomLengthString(65536); - const std::vector<uint8_t> script_bytes_2{script_string_2.begin(), script_string_2.end()}; + const auto script_1 = ConsumeScript(fuzzed_data_provider, 65536); + const auto script_2 = ConsumeScript(fuzzed_data_provider, 65536); std::vector<std::vector<unsigned char>> stack; - (void)EvalScript(stack, {script_bytes_1.begin(), script_bytes_1.end()}, flags, FuzzedSignatureChecker(fuzzed_data_provider), sig_version, nullptr); - if ((flags & SCRIPT_VERIFY_CLEANSTACK) != 0 && ((flags & SCRIPT_VERIFY_P2SH) == 0 || (flags & SCRIPT_VERIFY_WITNESS) == 0)) { + (void)EvalScript(stack, script_1, flags, FuzzedSignatureChecker(fuzzed_data_provider), sig_version, nullptr); + if (!IsValidFlagCombination(flags)) { return; } - if ((flags & SCRIPT_VERIFY_WITNESS) != 0 && (flags & SCRIPT_VERIFY_P2SH) == 0) { - return; - } - (void)VerifyScript({script_bytes_1.begin(), script_bytes_1.end()}, {script_bytes_2.begin(), script_bytes_2.end()}, nullptr, flags, FuzzedSignatureChecker(fuzzed_data_provider), nullptr); + (void)VerifyScript(script_1, script_2, nullptr, flags, FuzzedSignatureChecker(fuzzed_data_provider), nullptr); } diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index 93418ab1ff..d786ac1db1 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -80,7 +80,7 @@ CScriptWitness ConsumeScriptWitness(FuzzedDataProvider& fuzzed_data_provider, co CScript ConsumeScript(FuzzedDataProvider& fuzzed_data_provider, const size_t max_length, const bool maybe_p2wsh) noexcept { - const std::vector<uint8_t> b = ConsumeRandomLengthByteVector(fuzzed_data_provider); + const std::vector<uint8_t> b = ConsumeRandomLengthByteVector(fuzzed_data_provider, max_length); CScript r_script{b.begin(), b.end()}; if (maybe_p2wsh && fuzzed_data_provider.ConsumeBool()) { uint256 script_hash; diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index d5c54911c5..50d3ac66e5 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -567,36 +567,37 @@ class FuzzedSock : public Sock { FuzzedDataProvider& m_fuzzed_data_provider; + /** + * Data to return when `MSG_PEEK` is used as a `Recv()` flag. + * If `MSG_PEEK` is used, then our `Recv()` returns some random data as usual, but on the next + * `Recv()` call we must return the same data, thus we remember it here. + */ + mutable std::optional<uint8_t> m_peek_data; + public: explicit FuzzedSock(FuzzedDataProvider& fuzzed_data_provider) : m_fuzzed_data_provider{fuzzed_data_provider} { + m_socket = fuzzed_data_provider.ConsumeIntegral<SOCKET>(); } ~FuzzedSock() override { + // Sock::~Sock() will be called after FuzzedSock::~FuzzedSock() and it will call + // Sock::Reset() (not FuzzedSock::Reset()!) which will call CloseSocket(m_socket). + // Avoid closing an arbitrary file descriptor (m_socket is just a random number which + // may concide with a real opened file descriptor). + Reset(); } FuzzedSock& operator=(Sock&& other) override { - assert(false && "Not implemented yet."); + assert(false && "Move of Sock into FuzzedSock not allowed."); return *this; } - SOCKET Get() const override - { - assert(false && "Not implemented yet."); - return INVALID_SOCKET; - } - - SOCKET Release() override - { - assert(false && "Not implemented yet."); - return INVALID_SOCKET; - } - void Reset() override { - assert(false && "Not implemented yet."); + m_socket = INVALID_SOCKET; } ssize_t Send(const void* data, size_t len, int flags) const override @@ -633,10 +634,13 @@ public: ssize_t Recv(void* buf, size_t len, int flags) const override { + // Have a permanent error at recv_errnos[0] because when the fuzzed data is exhausted + // SetFuzzedErrNo() will always return the first element and we want to avoid Recv() + // returning -1 and setting errno to EAGAIN repeatedly. constexpr std::array recv_errnos{ + ECONNREFUSED, EAGAIN, EBADF, - ECONNREFUSED, EFAULT, EINTR, EINVAL, @@ -653,8 +657,26 @@ public: } return r; } - const std::vector<uint8_t> random_bytes = m_fuzzed_data_provider.ConsumeBytes<uint8_t>( - m_fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, len)); + std::vector<uint8_t> random_bytes; + bool pad_to_len_bytes{m_fuzzed_data_provider.ConsumeBool()}; + if (m_peek_data.has_value()) { + // `MSG_PEEK` was used in the preceding `Recv()` call, return `m_peek_data`. + random_bytes.assign({m_peek_data.value()}); + if ((flags & MSG_PEEK) == 0) { + m_peek_data.reset(); + } + pad_to_len_bytes = false; + } else if ((flags & MSG_PEEK) != 0) { + // New call with `MSG_PEEK`. + random_bytes = m_fuzzed_data_provider.ConsumeBytes<uint8_t>(1); + if (!random_bytes.empty()) { + m_peek_data = random_bytes[0]; + pad_to_len_bytes = false; + } + } else { + random_bytes = m_fuzzed_data_provider.ConsumeBytes<uint8_t>( + m_fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, len)); + } if (random_bytes.empty()) { const ssize_t r = m_fuzzed_data_provider.ConsumeBool() ? 0 : -1; if (r == -1) { @@ -663,7 +685,7 @@ public: return r; } std::memcpy(buf, random_bytes.data(), random_bytes.size()); - if (m_fuzzed_data_provider.ConsumeBool()) { + if (pad_to_len_bytes) { if (len > random_bytes.size()) { std::memset((char*)buf + random_bytes.size(), 0, len - random_bytes.size()); } @@ -675,10 +697,59 @@ public: return random_bytes.size(); } + int Connect(const sockaddr*, socklen_t) const override + { + // Have a permanent error at connect_errnos[0] because when the fuzzed data is exhausted + // SetFuzzedErrNo() will always return the first element and we want to avoid Connect() + // returning -1 and setting errno to EAGAIN repeatedly. + constexpr std::array connect_errnos{ + ECONNREFUSED, + EAGAIN, + ECONNRESET, + EHOSTUNREACH, + EINPROGRESS, + EINTR, + ENETUNREACH, + ETIMEDOUT, + }; + if (m_fuzzed_data_provider.ConsumeBool()) { + SetFuzzedErrNo(m_fuzzed_data_provider, connect_errnos); + return -1; + } + return 0; + } + + int GetSockOpt(int level, int opt_name, void* opt_val, socklen_t* opt_len) const override + { + constexpr std::array getsockopt_errnos{ + ENOMEM, + ENOBUFS, + }; + if (m_fuzzed_data_provider.ConsumeBool()) { + SetFuzzedErrNo(m_fuzzed_data_provider, getsockopt_errnos); + return -1; + } + if (opt_val == nullptr) { + return 0; + } + std::memcpy(opt_val, + ConsumeFixedLengthByteVector(m_fuzzed_data_provider, *opt_len).data(), + *opt_len); + return 0; + } + bool Wait(std::chrono::milliseconds timeout, Event requested, Event* occurred = nullptr) const override { return m_fuzzed_data_provider.ConsumeBool(); } + + bool IsConnected(std::string& errmsg) const override { + if (m_fuzzed_data_provider.ConsumeBool()) { + return true; + } + errmsg = "disconnected at random by the fuzzer"; + return false; + } }; [[nodiscard]] inline FuzzedSock ConsumeSock(FuzzedDataProvider& fuzzed_data_provider) diff --git a/src/test/i2p_tests.cpp b/src/test/i2p_tests.cpp new file mode 100644 index 0000000000..334f71106c --- /dev/null +++ b/src/test/i2p_tests.cpp @@ -0,0 +1,44 @@ +// Copyright (c) 2021-2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <i2p.h> +#include <netaddress.h> +#include <test/util/logging.h> +#include <test/util/net.h> +#include <test/util/setup_common.h> +#include <threadinterrupt.h> +#include <util/system.h> + +#include <boost/test/unit_test.hpp> + +#include <memory> +#include <string> + +BOOST_FIXTURE_TEST_SUITE(i2p_tests, BasicTestingSetup) + +BOOST_AUTO_TEST_CASE(unlimited_recv) +{ + auto CreateSockOrig = CreateSock; + + // Mock CreateSock() to create MockSock. + CreateSock = [](const CService&) { + return std::make_unique<StaticContentsSock>(std::string(i2p::sam::MAX_MSG_SIZE + 1, 'a')); + }; + + CThreadInterrupt interrupt; + i2p::sam::Session session(GetDataDir() / "test_i2p_private_key", CService{}, &interrupt); + + { + ASSERT_DEBUG_LOG("Creating SAM session"); + ASSERT_DEBUG_LOG("too many bytes without a terminator"); + + i2p::Connection conn; + bool proxy_error; + BOOST_REQUIRE(!session.Connect(CService{}, conn, proxy_error)); + } + + CreateSock = CreateSockOrig; +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/net_peer_eviction_tests.cpp b/src/test/net_peer_eviction_tests.cpp new file mode 100644 index 0000000000..31d391bf7d --- /dev/null +++ b/src/test/net_peer_eviction_tests.cpp @@ -0,0 +1,348 @@ +// Copyright (c) 2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <net.h> +#include <test/util/setup_common.h> + +#include <boost/test/unit_test.hpp> + +#include <algorithm> +#include <functional> +#include <optional> +#include <unordered_set> +#include <vector> + +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<NodeEvictionCandidate> GetRandomNodeEvictionCandidates(const int n_candidates, FastRandomContext& random_context) +{ + std::vector<NodeEvictionCandidate> candidates; + for (int id = 0; id < n_candidates; ++id) { + candidates.push_back({ + /* id */ id, + /* nTimeConnected */ static_cast<int64_t>(random_context.randrange(100)), + /* m_min_ping_time */ std::chrono::microseconds{random_context.randrange(100)}, + /* nLastBlockTime */ static_cast<int64_t>(random_context.randrange(100)), + /* nLastTXTime */ static_cast<int64_t>(random_context.randrange(100)), + /* fRelevantServices */ random_context.randbool(), + /* fRelayTxes */ random_context.randbool(), + /* fBloomFilter */ random_context.randbool(), + /* nKeyedNetGroup */ random_context.randrange(100), + /* prefer_evict */ random_context.randbool(), + /* m_is_local */ random_context.randbool(), + /* m_is_onion */ random_context.randbool(), + }); + } + return candidates; +} + +// Create `num_peers` random nodes, apply setup function `candidate_setup_fn`, +// call ProtectEvictionCandidatesByRatio() to apply protection logic, and then +// return true if all of `protected_peer_ids` and none of `unprotected_peer_ids` +// are protected from eviction, i.e. removed from the eviction candidates. +bool IsProtected(int num_peers, + std::function<void(NodeEvictionCandidate&)> candidate_setup_fn, + const std::unordered_set<NodeId>& protected_peer_ids, + const std::unordered_set<NodeId>& unprotected_peer_ids, + FastRandomContext& random_context) +{ + std::vector<NodeEvictionCandidate> candidates{GetRandomNodeEvictionCandidates(num_peers, random_context)}; + for (NodeEvictionCandidate& candidate : candidates) { + candidate_setup_fn(candidate); + } + Shuffle(candidates.begin(), candidates.end(), random_context); + + const size_t size{candidates.size()}; + const size_t expected{size - size / 2}; // Expect half the candidates will be protected. + ProtectEvictionCandidatesByRatio(candidates); + BOOST_CHECK_EQUAL(candidates.size(), expected); + + size_t unprotected_count{0}; + for (const NodeEvictionCandidate& candidate : candidates) { + if (protected_peer_ids.count(candidate.id)) { + // this peer should have been removed from the eviction candidates + BOOST_TEST_MESSAGE(strprintf("expected candidate to be protected: %d", candidate.id)); + return false; + } + if (unprotected_peer_ids.count(candidate.id)) { + // this peer remains in the eviction candidates, as expected + ++unprotected_count; + } + } + + const bool is_protected{unprotected_count == unprotected_peer_ids.size()}; + if (!is_protected) { + BOOST_TEST_MESSAGE(strprintf("unprotected: expected %d, actual %d", + unprotected_peer_ids.size(), unprotected_count)); + } + return is_protected; +} + +BOOST_AUTO_TEST_CASE(peer_protection_test) +{ + FastRandomContext random_context{true}; + int num_peers{12}; + + // Expect half of the peers with greatest uptime (the lowest nTimeConnected) + // to be protected from eviction. + BOOST_CHECK(IsProtected( + num_peers, [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + 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}, + random_context)); + + // Verify in the opposite direction. + BOOST_CHECK(IsProtected( + num_peers, [num_peers](NodeEvictionCandidate& c) { + c.nTimeConnected = num_peers - c.id; + 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}, + random_context)); + + // Test protection of onion and localhost peers... + + // Expect 1/4 onion peers to be protected from eviction, + // independently of other characteristics. + BOOST_CHECK(IsProtected( + num_peers, [](NodeEvictionCandidate& c) { + c.m_is_onion = (c.id == 3 || c.id == 8 || c.id == 9); + }, + /* 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). + 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); + }, + /* protected_peer_ids */ {0, 1, 2, 3, 8, 9}, + /* unprotected_peer_ids */ {4, 5, 6, 7, 10, 11}, + random_context)); + + // Expect 1/4 localhost peers to be protected from eviction, + // 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 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}, + /* 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. +bool IsEvicted(std::vector<NodeEvictionCandidate> candidates, const std::unordered_set<NodeId>& node_ids, FastRandomContext& random_context) +{ + Shuffle(candidates.begin(), candidates.end(), random_context); + const std::optional<NodeId> evicted_node_id = SelectNodeToEvict(std::move(candidates)); + if (!evicted_node_id) { + return false; + } + return node_ids.count(*evicted_node_id); +} + +// Create number_of_nodes random nodes, apply setup function candidate_setup_fn, +// apply eviction logic and then return true if any of the node ids in node_ids +// are selected for eviction. +bool IsEvicted(const int number_of_nodes, std::function<void(NodeEvictionCandidate&)> candidate_setup_fn, const std::unordered_set<NodeId>& node_ids, FastRandomContext& random_context) +{ + std::vector<NodeEvictionCandidate> candidates = GetRandomNodeEvictionCandidates(number_of_nodes, random_context); + for (NodeEvictionCandidate& candidate : candidates) { + candidate_setup_fn(candidate); + } + return IsEvicted(candidates, node_ids, random_context); +} + +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. [...]" + } + } +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 858b90b8b2..8eab26f3d5 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -857,147 +857,4 @@ BOOST_AUTO_TEST_CASE(LocalAddress_BasicLifecycle) BOOST_CHECK_EQUAL(IsLocal(addr), false); } -std::vector<NodeEvictionCandidate> GetRandomNodeEvictionCandidates(const int n_candidates, FastRandomContext& random_context) -{ - std::vector<NodeEvictionCandidate> candidates; - for (int id = 0; id < n_candidates; ++id) { - candidates.push_back({ - /* id */ id, - /* nTimeConnected */ static_cast<int64_t>(random_context.randrange(100)), - /* m_min_ping_time */ std::chrono::microseconds{random_context.randrange(100)}, - /* nLastBlockTime */ static_cast<int64_t>(random_context.randrange(100)), - /* nLastTXTime */ static_cast<int64_t>(random_context.randrange(100)), - /* fRelevantServices */ random_context.randbool(), - /* fRelayTxes */ random_context.randbool(), - /* fBloomFilter */ random_context.randbool(), - /* nKeyedNetGroup */ random_context.randrange(100), - /* prefer_evict */ random_context.randbool(), - /* m_is_local */ random_context.randbool(), - }); - } - return candidates; -} - -// Returns true if any of the node ids in node_ids are selected for eviction. -bool IsEvicted(std::vector<NodeEvictionCandidate> candidates, const std::vector<NodeId>& node_ids, FastRandomContext& random_context) -{ - Shuffle(candidates.begin(), candidates.end(), random_context); - const std::optional<NodeId> evicted_node_id = SelectNodeToEvict(std::move(candidates)); - if (!evicted_node_id) { - return false; - } - return std::find(node_ids.begin(), node_ids.end(), *evicted_node_id) != node_ids.end(); -} - -// Create number_of_nodes random nodes, apply setup function candidate_setup_fn, -// apply eviction logic and then return true if any of the node ids in node_ids -// are selected for eviction. -bool IsEvicted(const int number_of_nodes, std::function<void(NodeEvictionCandidate&)> candidate_setup_fn, const std::vector<NodeId>& node_ids, FastRandomContext& random_context) -{ - std::vector<NodeEvictionCandidate> candidates = GetRandomNodeEvictionCandidates(number_of_nodes, random_context); - for (NodeEvictionCandidate& candidate : candidates) { - candidate_setup_fn(candidate); - } - return IsEvicted(candidates, node_ids, random_context); -} - -namespace { -constexpr int NODE_EVICTION_TEST_ROUNDS{10}; -constexpr int NODE_EVICTION_TEST_UP_TO_N_NODES{200}; -} // namespace - -BOOST_AUTO_TEST_CASE(node_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: - // * "Protect the half of the remaining nodes which have been connected the longest. [...]" - // * "Pick out up to 1/4 peers that are localhost, sorted by longest uptime. [...]" - // * "If any remaining peers are preferred for eviction consider only them. [...]" - // * "Identify the network group with the most connections and youngest member. [...]" - } - } -} - BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util/net.h b/src/test/util/net.h index e25036be26..2b7988413f 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -5,7 +5,13 @@ #ifndef BITCOIN_TEST_UTIL_NET_H #define BITCOIN_TEST_UTIL_NET_H +#include <compat.h> #include <net.h> +#include <util/sock.h> + +#include <cassert> +#include <cstring> +#include <string> struct ConnmanTestMsg : public CConnman { using CConnman::CConnman; @@ -61,4 +67,67 @@ constexpr ConnectionType ALL_CONNECTION_TYPES[]{ ConnectionType::ADDR_FETCH, }; +/** + * 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 + * exhausted an EOF is returned by further reads. + */ +class StaticContentsSock : public Sock +{ +public: + explicit StaticContentsSock(const std::string& contents) : m_contents{contents}, m_consumed{0} + { + // Just a dummy number that is not INVALID_SOCKET. + static_assert(INVALID_SOCKET != 1000); + m_socket = 1000; + } + + ~StaticContentsSock() override { Reset(); } + + StaticContentsSock& operator=(Sock&& other) override + { + assert(false && "Move of Sock into MockSock not allowed."); + return *this; + } + + void Reset() override + { + m_socket = INVALID_SOCKET; + } + + ssize_t Send(const void*, size_t len, int) const override { return len; } + + ssize_t Recv(void* buf, size_t len, int flags) const override + { + const size_t consume_bytes{std::min(len, m_contents.size() - m_consumed)}; + std::memcpy(buf, m_contents.data() + m_consumed, consume_bytes); + if ((flags & MSG_PEEK) == 0) { + m_consumed += consume_bytes; + } + return consume_bytes; + } + + int Connect(const sockaddr*, socklen_t) const override { return 0; } + + int GetSockOpt(int level, int opt_name, void* opt_val, socklen_t* opt_len) const override + { + std::memset(opt_val, 0x0, *opt_len); + return 0; + } + + bool Wait(std::chrono::milliseconds timeout, + Event requested, + Event* occurred = nullptr) const override + { + if (occurred != nullptr) { + *occurred = requested; + } + return true; + } + +private: + const std::string m_contents; + mutable size_t m_consumed; +}; + #endif // BITCOIN_TEST_UTIL_NET_H diff --git a/src/test/util/script.cpp b/src/test/util/script.cpp new file mode 100644 index 0000000000..a5852daa60 --- /dev/null +++ b/src/test/util/script.cpp @@ -0,0 +1,13 @@ +// Copyright (c) 2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <script/interpreter.h> +#include <test/util/script.h> + +bool IsValidFlagCombination(unsigned flags) +{ + if (flags & SCRIPT_VERIFY_CLEANSTACK && ~flags & (SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS)) return false; + if (flags & SCRIPT_VERIFY_WITNESS && ~flags & SCRIPT_VERIFY_P2SH) return false; + return true; +} diff --git a/src/test/util/script.h b/src/test/util/script.h index abd14c2067..428b3e10b3 100644 --- a/src/test/util/script.h +++ b/src/test/util/script.h @@ -18,4 +18,7 @@ static const CScript P2WSH_OP_TRUE{ return hash; }())}; +/** Flags that are not forbidden by an assert in script validation */ +bool IsValidFlagCombination(unsigned flags); + #endif // BITCOIN_TEST_UTIL_SCRIPT_H diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index f866c2a1f9..bfb3466dcf 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -4,6 +4,7 @@ #include <test/util/setup_common.h> +#include <addrman.h> #include <banman.h> #include <chainparams.h> #include <consensus/consensus.h> @@ -155,6 +156,7 @@ ChainTestingSetup::~ChainTestingSetup() GetMainSignals().UnregisterBackgroundSignalScheduler(); m_node.connman.reset(); m_node.banman.reset(); + m_node.addrman.reset(); m_node.args = nullptr; UnloadBlockIndex(m_node.mempool.get(), *m_node.chainman); m_node.mempool.reset(); @@ -187,11 +189,12 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const throw std::runtime_error(strprintf("ActivateBestChain failed. (%s)", state.ToString())); } + m_node.addrman = std::make_unique<CAddrMan>(); m_node.banman = std::make_unique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); - m_node.connman = std::make_unique<CConnman>(0x1337, 0x1337); // Deterministic randomness for tests. - m_node.peerman = PeerManager::make(chainparams, *m_node.connman, m_node.banman.get(), - *m_node.scheduler, *m_node.chainman, *m_node.mempool, - false); + m_node.connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman); // Deterministic randomness for tests. + m_node.peerman = PeerManager::make(chainparams, *m_node.connman, *m_node.addrman, + m_node.banman.get(), *m_node.scheduler, *m_node.chainman, + *m_node.mempool, false); { CConnman::Options options; options.m_msgproc = m_node.peerman.get(); diff --git a/src/util/sock.cpp b/src/util/sock.cpp index f9ecfef5d4..0bc9795db3 100644 --- a/src/util/sock.cpp +++ b/src/util/sock.cpp @@ -66,6 +66,16 @@ ssize_t Sock::Recv(void* buf, size_t len, int flags) const return recv(m_socket, static_cast<char*>(buf), len, flags); } +int Sock::Connect(const sockaddr* addr, socklen_t addr_len) const +{ + return connect(m_socket, addr, addr_len); +} + +int Sock::GetSockOpt(int level, int opt_name, void* opt_val, socklen_t* opt_len) const +{ + return getsockopt(m_socket, level, opt_name, static_cast<char*>(opt_val), opt_len); +} + bool Sock::Wait(std::chrono::milliseconds timeout, Event requested, Event* occurred) const { #ifdef USE_POLL diff --git a/src/util/sock.h b/src/util/sock.h index 4b0618dcff..c4ad0cbc43 100644 --- a/src/util/sock.h +++ b/src/util/sock.h @@ -80,16 +80,29 @@ public: /** * send(2) wrapper. Equivalent to `send(this->Get(), data, len, flags);`. Code that uses this - * wrapper can be unit-tested if this method is overridden by a mock Sock implementation. + * wrapper can be unit tested if this method is overridden by a mock Sock implementation. */ virtual ssize_t Send(const void* data, size_t len, int flags) const; /** * recv(2) wrapper. Equivalent to `recv(this->Get(), buf, len, flags);`. Code that uses this - * wrapper can be unit-tested if this method is overridden by a mock Sock implementation. + * wrapper can be unit tested if this method is overridden by a mock Sock implementation. */ virtual ssize_t Recv(void* buf, size_t len, int flags) const; + /** + * connect(2) wrapper. Equivalent to `connect(this->Get(), addr, addrlen)`. Code that uses this + * wrapper can be unit tested if this method is overridden by a mock Sock implementation. + */ + virtual int Connect(const sockaddr* addr, socklen_t addr_len) const; + + /** + * getsockopt(2) wrapper. Equivalent to + * `getsockopt(this->Get(), level, opt_name, opt_val, opt_len)`. Code that uses this + * wrapper can be unit tested if this method is overridden by a mock Sock implementation. + */ + virtual int GetSockOpt(int level, int opt_name, void* opt_val, socklen_t* opt_len) const; + using Event = uint8_t; /** @@ -153,7 +166,7 @@ public: */ virtual bool IsConnected(std::string& errmsg) const; -private: +protected: /** * Contained socket. `INVALID_SOCKET` designates the object is empty. */ |