diff --git a/src/net.cpp b/src/net.cpp index 490b766cb1d..5e6bc86372a 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3496,7 +3496,7 @@ CConnman::~CConnman() Stop(); } -std::vector CConnman::GetAddresses(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered) const +std::vector CConnman::GetAddressesUnsafe(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered) const { std::vector addresses = addrman.GetAddr(max_addresses, max_pct, network, filtered); if (m_banman) { @@ -3521,7 +3521,7 @@ std::vector CConnman::GetAddresses(CNode& requestor, size_t max_addres auto r = m_addr_response_caches.emplace(cache_id, CachedAddrResponse{}); CachedAddrResponse& cache_entry = r.first->second; if (cache_entry.m_cache_entry_expiration < current_time) { // If emplace() added new one it has expiration 0. - cache_entry.m_addrs_response_cache = GetAddresses(max_addresses, max_pct, /*network=*/std::nullopt); + cache_entry.m_addrs_response_cache = GetAddressesUnsafe(max_addresses, max_pct, /*network=*/std::nullopt); // Choosing a proper cache lifetime is a trade-off between the privacy leak minimization // and the usefulness of ADDR responses to honest users. // @@ -3964,8 +3964,8 @@ void CConnman::PerformReconnections() void CConnman::ASMapHealthCheck() { - const std::vector v4_addrs{GetAddresses(/*max_addresses=*/ 0, /*max_pct=*/ 0, Network::NET_IPV4, /*filtered=*/ false)}; - const std::vector v6_addrs{GetAddresses(/*max_addresses=*/ 0, /*max_pct=*/ 0, Network::NET_IPV6, /*filtered=*/ false)}; + const std::vector v4_addrs{GetAddressesUnsafe(/*max_addresses=*/0, /*max_pct=*/0, Network::NET_IPV4, /*filtered=*/false)}; + const std::vector v6_addrs{GetAddressesUnsafe(/*max_addresses=*/0, /*max_pct=*/0, Network::NET_IPV6, /*filtered=*/false)}; std::vector clearnet_addrs; clearnet_addrs.reserve(v4_addrs.size() + v6_addrs.size()); std::transform(v4_addrs.begin(), v4_addrs.end(), std::back_inserter(clearnet_addrs), diff --git a/src/net.h b/src/net.h index 4cb4cb906e2..fa29c4c5e20 100644 --- a/src/net.h +++ b/src/net.h @@ -1169,19 +1169,30 @@ public: // Addrman functions /** - * Return all or many randomly selected addresses, optionally by network. + * Return randomly selected addresses. This function does not use the address response cache and + * should only be used in trusted contexts. + * + * An untrusted caller (e.g. from p2p) should instead use @ref GetAddresses to use the cache. * * @param[in] max_addresses Maximum number of addresses to return (0 = all). * @param[in] max_pct Maximum percentage of addresses to return (0 = all). Value must be from 0 to 100. * @param[in] network Select only addresses of this network (nullopt = all). * @param[in] filtered Select only addresses that are considered high quality (false = all). */ - std::vector GetAddresses(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const; + std::vector GetAddressesUnsafe(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const; /** - * Cache is used to minimize topology leaks, so it should - * be used for all non-trusted calls, for example, p2p. - * A non-malicious call (from RPC or a peer with addr permission) should - * call the function without a parameter to avoid using the cache. + * Return addresses from the per-requestor cache. If no cache entry exists, it is populated with + * randomly selected addresses. This function can be used in untrusted contexts. + * + * A trusted caller (e.g. from RPC or a peer with addr permission) can use + * @ref GetAddressesUnsafe to avoid using the cache. + * + * @param[in] requestor The requesting peer. Used to key the cache to prevent privacy leaks. + * @param[in] max_addresses Maximum number of addresses to return (0 = all). Ignored when cache + * already contains an entry for requestor. + * @param[in] max_pct Maximum percentage of addresses to return (0 = all). Value must be + * from 0 to 100. Ignored when cache already contains an entry for + * requestor. */ std::vector GetAddresses(CNode& requestor, size_t max_addresses, size_t max_pct); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 10fbae5e7ed..0c4a89c44cb 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4715,7 +4715,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, peer->m_addrs_to_send.clear(); std::vector vAddr; if (pfrom.HasPermission(NetPermissionFlags::Addr)) { - vAddr = m_connman.GetAddresses(MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND, /*network=*/std::nullopt); + vAddr = m_connman.GetAddressesUnsafe(MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND, /*network=*/std::nullopt); } else { vAddr = m_connman.GetAddresses(pfrom, MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND); } diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 2739b0d7dd0..2432d634593 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -953,7 +953,7 @@ static RPCHelpMan getnodeaddresses() } // returns a shuffled list of CAddress - const std::vector vAddr{connman.GetAddresses(count, /*max_pct=*/0, network)}; + const std::vector vAddr{connman.GetAddressesUnsafe(count, /*max_pct=*/0, network)}; UniValue ret(UniValue::VARR); for (const CAddress& addr : vAddr) { diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index a62d227da8e..c7509c54947 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -113,7 +113,7 @@ FUZZ_TARGET(connman, .init = initialize_connman) auto max_addresses = fuzzed_data_provider.ConsumeIntegral(); auto max_pct = fuzzed_data_provider.ConsumeIntegralInRange(0, 100); auto filtered = fuzzed_data_provider.ConsumeBool(); - (void)connman.GetAddresses(max_addresses, max_pct, /*network=*/std::nullopt, filtered); + (void)connman.GetAddressesUnsafe(max_addresses, max_pct, /*network=*/std::nullopt, filtered); }, [&] { auto max_addresses = fuzzed_data_provider.ConsumeIntegral();