From b8aa7eba90ab3024343f22bb67d755f5ac85e2b9 Mon Sep 17 00:00:00 2001 From: merge-script Date: Thu, 16 May 2024 11:18:26 +0800 Subject: [PATCH] Merge bitcoin/bitcoin#30085: p2p: detect addnode cjdns peers in GetAddedNodeInfo() d0b047494c28381942c09d0cca45baa323bfcffc test: add GetAddedNodeInfo() CJDNS regression unit test (Jon Atack) 684da9707040ce25d95b2954eda50b811136d92c p2p, bugfix: detect addnode cjdns peers in GetAddedNodeInfo() (Jon Atack) Pull request description: Addnode peers connected to us via the cjdns network are currently not detected by `CConnman::GetAddedNodeInfo()`, i.e. `fConnected` is always false. This causes the following issues: - RPC `getaddednodeinfo` incorrectly shows them as not connected - `CConnman::ThreadOpenAddedConnections()` continually retries to connect them Fix the issue and add a unit regression test. Extracted from #28248. Suggest running the test with: `./src/test/test_bitcoin -t net_peer_connection_tests -l test_suite` ACKs for top commit: mzumsande: utACK d0b047494c28381942c09d0cca45baa323bfcffc brunoerg: crACK d0b047494c28381942c09d0cca45baa323bfcffc pinheadmz: ACK d0b047494c28381942c09d0cca45baa323bfcffc Tree-SHA512: a4d81425f79558f5792585611f3fe8ab999b82144daeed5c3ec619861c69add934c2b2afdad24c8488a0ade94f5ce8112f5555d60a1ce913d4f5a1cf5dbba55a --- src/net.cpp | 2 +- src/test/net_peer_connection_tests.cpp | 163 +++++++++++++++++++++++++ 2 files changed, 164 insertions(+), 1 deletion(-) create mode 100644 src/test/net_peer_connection_tests.cpp diff --git a/src/net.cpp b/src/net.cpp index 1b0e7369a3..5cd7e0e764 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2221,7 +2221,7 @@ std::vector CConnman::GetAddedNodeInfo() const } for (const std::string& strAddNode : lAddresses) { - CService service(LookupNumeric(strAddNode, Params().GetDefaultPort(strAddNode))); + CService service{MaybeFlipIPv6toCJDNS(LookupNumeric(strAddNode, Params().GetDefaultPort(strAddNode)))}; AddedNodeInfo addedNode{strAddNode, CService(), false, false}; if (service.IsValid()) { // strAddNode is an IP:port diff --git a/src/test/net_peer_connection_tests.cpp b/src/test/net_peer_connection_tests.cpp new file mode 100644 index 0000000000..00bc1fdb6a --- /dev/null +++ b/src/test/net_peer_connection_tests.cpp @@ -0,0 +1,163 @@ +// Copyright (c) 2023-present 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 +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +#include + +struct LogIPsTestingSetup : public TestingSetup { + LogIPsTestingSetup() + : TestingSetup{ChainType::MAIN, /*extra_args=*/{"-logips"}} {} +}; + +BOOST_FIXTURE_TEST_SUITE(net_peer_connection_tests, LogIPsTestingSetup) + +static CService ip(uint32_t i) +{ + struct in_addr s; + s.s_addr = i; + return CService{CNetAddr{s}, Params().GetDefaultPort()}; +} + +/** Create a peer and connect to it. If the optional `address` (IP/CJDNS only) isn't passed, a random address is created. */ +static void AddPeer(NodeId& id, std::vector& nodes, PeerManager& peerman, ConnmanTestMsg& connman, ConnectionType conn_type, bool onion_peer = false, std::optional address = std::nullopt) +{ + CAddress addr{}; + + if (address.has_value()) { + addr = CAddress{MaybeFlipIPv6toCJDNS(LookupNumeric(address.value(), Params().GetDefaultPort())), NODE_NONE}; + } else if (onion_peer) { + auto tor_addr{g_insecure_rand_ctx.randbytes(ADDR_TORV3_SIZE)}; + BOOST_REQUIRE(addr.SetSpecial(OnionToString(tor_addr))); + } + + while (!addr.IsLocal() && !addr.IsRoutable()) { + addr = CAddress{ip(g_insecure_rand_ctx.randbits(32)), NODE_NONE}; + } + + BOOST_REQUIRE(addr.IsValid()); + + const bool inbound_onion{onion_peer && conn_type == ConnectionType::INBOUND}; + + nodes.emplace_back(new CNode{++id, + /*sock=*/nullptr, + addr, + /*nKeyedNetGroupIn=*/0, + /*nLocalHostNonceIn=*/0, + CAddress{}, + /*addrNameIn=*/"", + conn_type, + /*inbound_onion=*/inbound_onion}); + CNode& node = *nodes.back(); + node.SetCommonVersion(PROTOCOL_VERSION); + + peerman.InitializeNode(node, ServiceFlags(NODE_NETWORK | NODE_WITNESS)); + node.fSuccessfullyConnected = true; + + connman.AddTestNode(node); +} + +BOOST_AUTO_TEST_CASE(test_addnode_getaddednodeinfo_and_connection_detection) +{ + auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()); + auto peerman = PeerManager::make(*connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, {}); + NodeId id{0}; + std::vector nodes; + + // Connect a localhost peer. + { + ASSERT_DEBUG_LOG("Added connection to 127.0.0.1:8333 peer=1"); + AddPeer(id, nodes, *peerman, *connman, ConnectionType::MANUAL, /*onion_peer=*/false, /*address=*/"127.0.0.1"); + BOOST_REQUIRE(nodes.back() != nullptr); + } + + // Call ConnectNode(), which is also called by RPC addnode onetry, for a localhost + // address that resolves to multiple IPs, including that of the connected peer. + // The connection attempt should consistently fail due to the check in ConnectNode(). + for (int i = 0; i < 10; ++i) { + ASSERT_DEBUG_LOG("Not opening a connection to localhost, already connected to 127.0.0.1:8333"); + BOOST_CHECK(!connman->ConnectNodePublic(*peerman, "localhost", ConnectionType::MANUAL)); + } + + // Add 3 more peer connections. + AddPeer(id, nodes, *peerman, *connman, ConnectionType::OUTBOUND_FULL_RELAY); + AddPeer(id, nodes, *peerman, *connman, ConnectionType::BLOCK_RELAY, /*onion_peer=*/true); + AddPeer(id, nodes, *peerman, *connman, ConnectionType::INBOUND); + + // Add a CJDNS peer connection. + AddPeer(id, nodes, *peerman, *connman, ConnectionType::INBOUND, /*onion_peer=*/false, + /*address=*/"[fc00:3344:5566:7788:9900:aabb:ccdd:eeff]:1234"); + BOOST_CHECK(nodes.back()->IsInboundConn()); + BOOST_CHECK_EQUAL(nodes.back()->ConnectedThroughNetwork(), Network::NET_CJDNS); + + BOOST_TEST_MESSAGE("Call AddNode() for all the peers"); + for (auto node : connman->TestNodes()) { + BOOST_CHECK(connman->AddNode({/*m_added_node=*/node->addr.ToStringAddrPort(), /*m_use_v2transport=*/true})); + BOOST_TEST_MESSAGE(strprintf("peer id=%s addr=%s", node->GetId(), node->addr.ToStringAddrPort())); + } + + BOOST_TEST_MESSAGE("\nCall AddNode() with 2 addrs resolving to existing localhost addnode entry; neither should be added"); + BOOST_CHECK(!connman->AddNode({/*m_added_node=*/"127.0.0.1", /*m_use_v2transport=*/true})); + // OpenBSD doesn't support the IPv4 shorthand notation with omitted zero-bytes. +#if !defined(__OpenBSD__) + BOOST_CHECK(!connman->AddNode({/*m_added_node=*/"127.1", /*m_use_v2transport=*/true})); +#endif + + BOOST_TEST_MESSAGE("\nExpect GetAddedNodeInfo to return expected number of peers with `include_connected` true/false"); + BOOST_CHECK_EQUAL(connman->GetAddedNodeInfo(/*include_connected=*/true).size(), nodes.size()); + BOOST_CHECK(connman->GetAddedNodeInfo(/*include_connected=*/false).empty()); + + // Test AddedNodesContain() + for (auto node : connman->TestNodes()) { + BOOST_CHECK(connman->AddedNodesContain(node->addr)); + } + AddPeer(id, nodes, *peerman, *connman, ConnectionType::OUTBOUND_FULL_RELAY); + BOOST_CHECK(!connman->AddedNodesContain(nodes.back()->addr)); + + BOOST_TEST_MESSAGE("\nPrint GetAddedNodeInfo contents:"); + for (const auto& info : connman->GetAddedNodeInfo(/*include_connected=*/true)) { + BOOST_TEST_MESSAGE(strprintf("\nadded node: %s", info.m_params.m_added_node)); + BOOST_TEST_MESSAGE(strprintf("connected: %s", info.fConnected)); + if (info.fConnected) { + BOOST_TEST_MESSAGE(strprintf("IP address: %s", info.resolvedAddress.ToStringAddrPort())); + BOOST_TEST_MESSAGE(strprintf("direction: %s", info.fInbound ? "inbound" : "outbound")); + } + } + + BOOST_TEST_MESSAGE("\nCheck that all connected peers are correctly detected as connected"); + for (auto node : connman->TestNodes()) { + BOOST_CHECK(connman->AlreadyConnectedPublic(node->addr)); + } + + // Clean up + for (auto node : connman->TestNodes()) { + peerman->FinalizeNode(*node); + } + connman->ClearTestNodes(); +} + +BOOST_AUTO_TEST_SUITE_END()