diff options
author | fanquake <fanquake@gmail.com> | 2021-12-14 20:37:28 +0800 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2021-12-14 20:40:58 +0800 |
commit | 498fe4b7808e1e655953794f017b5f26d0bce8de (patch) | |
tree | e8d8c7ff0f401bf5c09263b908db07270355508d /src | |
parent | aaaceb7fb14fa69d34194e805bc384088f55d25c (diff) | |
parent | fa19bab90a3ccc2f76c20aa805292d6a9c5d8071 (diff) | |
download | bitcoin-498fe4b7808e1e655953794f017b5f26d0bce8de.tar.xz |
Merge bitcoin/bitcoin#23575: fuzz: Rework FillNode
fa19bab90a3ccc2f76c20aa805292d6a9c5d8071 fuzz: Rework FillNode (MarcoFalke)
fae6e31df7c6df04f41fc8401e2a9781a4d75be7 refactor: Set fSuccessfullyConnected in FillNode (MarcoFalke)
fa3583f856e34b6c6134745da14f5206cf71fa3e fuzz: Avoid negative NodeId in ConsumeNode (MarcoFalke)
Pull request description:
Currently `FillNode` is a bit clumsy because it directly modifies memory of `CNode`. This gets in the way of moving that memory to `Peer`. Also, it isn't particularly consistent. See for example https://github.com/bitcoin/bitcoin/pull/21160#discussion_r739206139 .
Fix all issues by sending a `version`/`verack` in `FillNode` and let net_processing figure out the internal details.
ACKs for top commit:
jnewbery:
Strong concept ACK and light code review ACK fa19bab90a3ccc2f76c20aa805292d6a9c5d8071
Tree-SHA512: 33261d857c3fa6d5d39d742624009a29178ad5a15eb3fd062da741affa5a4854fd45ed20d59a6bba2fb068cf7b39cad6f95b2910be7cb6afdc27cd7917955b67
Diffstat (limited to 'src')
-rw-r--r-- | src/test/fuzz/process_message.cpp | 4 | ||||
-rw-r--r-- | src/test/fuzz/process_messages.cpp | 5 | ||||
-rw-r--r-- | src/test/fuzz/util.cpp | 51 | ||||
-rw-r--r-- | src/test/fuzz/util.h | 6 |
4 files changed, 50 insertions, 16 deletions
diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 94a71859e9..1763cd8af3 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -79,11 +79,9 @@ void fuzz_target(FuzzBufferType buffer, const std::string& LIMIT_TO_MESSAGE_TYPE } CNode& p2p_node = *ConsumeNodeAsUniquePtr(fuzzed_data_provider).release(); - const bool successfully_connected{fuzzed_data_provider.ConsumeBool()}; - p2p_node.fSuccessfullyConnected = successfully_connected; connman.AddTestNode(p2p_node); g_setup->m_node.peerman->InitializeNode(&p2p_node); - FillNode(fuzzed_data_provider, p2p_node, /*init_version=*/successfully_connected); + FillNode(fuzzed_data_provider, connman, *g_setup->m_node.peerman, p2p_node); const auto mock_time = ConsumeTime(fuzzed_data_provider); SetMockTime(mock_time); diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index 21a959315e..e1c11e1afd 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -46,11 +46,8 @@ FUZZ_TARGET_INIT(process_messages, initialize_process_messages) peers.push_back(ConsumeNodeAsUniquePtr(fuzzed_data_provider, i).release()); CNode& p2p_node = *peers.back(); - const bool successfully_connected{fuzzed_data_provider.ConsumeBool()}; - p2p_node.fSuccessfullyConnected = successfully_connected; - p2p_node.fPauseSend = false; g_setup->m_node.peerman->InitializeNode(&p2p_node); - FillNode(fuzzed_data_provider, p2p_node, /*init_version=*/successfully_connected); + FillNode(fuzzed_data_provider, connman, *g_setup->m_node.peerman, p2p_node); connman.AddTestNode(p2p_node); } diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index ae5f7a379e..843b29b911 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -3,6 +3,8 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <consensus/amount.h> +#include <net_processing.h> +#include <netmessagemaker.h> #include <pubkey.h> #include <test/fuzz/util.h> #include <test/util/script.h> @@ -200,22 +202,57 @@ bool FuzzedSock::IsConnected(std::string& errmsg) const return false; } -void FillNode(FuzzedDataProvider& fuzzed_data_provider, CNode& node, bool init_version) noexcept +void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, PeerManager& peerman, CNode& node) noexcept { + const bool successfully_connected{fuzzed_data_provider.ConsumeBool()}; const ServiceFlags remote_services = ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS); const NetPermissionFlags permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS); const int32_t version = fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(MIN_PEER_PROTO_VERSION, std::numeric_limits<int32_t>::max()); const bool filter_txs = fuzzed_data_provider.ConsumeBool(); - node.nServices = remote_services; - node.m_permissionFlags = permission_flags; - if (init_version) { - node.nVersion = version; - node.SetCommonVersion(std::min(version, PROTOCOL_VERSION)); + const CNetMsgMaker mm{0}; + + CSerializedNetMsg msg_version{ + mm.Make(NetMsgType::VERSION, + version, // + Using<CustomUintFormatter<8>>(remote_services), // + int64_t{}, // dummy time + int64_t{}, // ignored service bits + CService{}, // dummy + int64_t{}, // ignored service bits + CService{}, // ignored + uint64_t{1}, // dummy nonce + std::string{}, // dummy subver + int32_t{}, // dummy starting_height + filter_txs), + }; + + (void)connman.ReceiveMsgFrom(node, msg_version); + node.fPauseSend = false; + connman.ProcessMessagesOnce(node); + { + LOCK(node.cs_sendProcessing); + peerman.SendMessages(&node); } + if (node.fDisconnect) return; + assert(node.nVersion == version); + assert(node.GetCommonVersion() == std::min(version, PROTOCOL_VERSION)); + assert(node.nServices == remote_services); if (node.m_tx_relay != nullptr) { LOCK(node.m_tx_relay->cs_filter); - node.m_tx_relay->fRelayTxes = filter_txs; + assert(node.m_tx_relay->fRelayTxes == filter_txs); + } + node.m_permissionFlags = permission_flags; + if (successfully_connected) { + CSerializedNetMsg msg_verack{mm.Make(NetMsgType::VERACK)}; + (void)connman.ReceiveMsgFrom(node, msg_verack); + node.fPauseSend = false; + connman.ProcessMessagesOnce(node); + { + LOCK(node.cs_sendProcessing); + peerman.SendMessages(&node); + } + assert(node.fSuccessfullyConnected == true); } } diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index 40aaeac63f..7937315822 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -36,6 +36,8 @@ #include <string> #include <vector> +class PeerManager; + template <typename... Callables> size_t CallOneOf(FuzzedDataProvider& fuzzed_data_provider, Callables... callables) { @@ -257,7 +259,7 @@ inline CAddress ConsumeAddress(FuzzedDataProvider& fuzzed_data_provider) noexcep template <bool ReturnUniquePtr = false> auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<NodeId>& node_id_in = std::nullopt) noexcept { - const NodeId node_id = node_id_in.value_or(fuzzed_data_provider.ConsumeIntegral<NodeId>()); + const NodeId node_id = node_id_in.value_or(fuzzed_data_provider.ConsumeIntegralInRange<NodeId>(0, std::numeric_limits<NodeId>::max())); const ServiceFlags local_services = ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS); const SOCKET socket = INVALID_SOCKET; const CAddress address = ConsumeAddress(fuzzed_data_provider); @@ -275,7 +277,7 @@ auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<N } inline std::unique_ptr<CNode> ConsumeNodeAsUniquePtr(FuzzedDataProvider& fdp, const std::optional<NodeId>& node_id_in = std::nullopt) { return ConsumeNode<true>(fdp, node_id_in); } -void FillNode(FuzzedDataProvider& fuzzed_data_provider, CNode& node, bool init_version) noexcept; +void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, PeerManager& peerman, CNode& node) noexcept; class FuzzedFileProvider { |