From 2716647ebf60cea05fc9edce6a18dcce4e7727ad Mon Sep 17 00:00:00 2001 From: Troy Giorshev Date: Mon, 29 Jun 2020 14:09:42 -0400 Subject: Give V1TransportDeserializer an m_node_id member This is intended to only be used for logging. This will allow log messages in the following commits to keep recording the peer's ID, even when logging is moved into V1TransportDeserializer. --- src/test/fuzz/p2p_transport_deserializer.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/test/fuzz') diff --git a/src/test/fuzz/p2p_transport_deserializer.cpp b/src/test/fuzz/p2p_transport_deserializer.cpp index 6fba2bfaba..732136330b 100644 --- a/src/test/fuzz/p2p_transport_deserializer.cpp +++ b/src/test/fuzz/p2p_transport_deserializer.cpp @@ -19,7 +19,8 @@ void initialize() void test_one_input(const std::vector& buffer) { - V1TransportDeserializer deserializer{Params().MessageStart(), SER_NETWORK, INIT_PROTO_VERSION}; + // Construct deserializer, with a dummy NodeId + V1TransportDeserializer deserializer{Params().MessageStart(), (NodeId)0, SER_NETWORK, INIT_PROTO_VERSION}; const char* pch = (const char*)buffer.data(); size_t n_bytes = buffer.size(); while (n_bytes > 0) { -- cgit v1.2.3 From 890b1d7c2b8312d41d048d2db124586c5dbc8a49 Mon Sep 17 00:00:00 2001 From: Troy Giorshev Date: Mon, 29 Jun 2020 14:15:06 -0400 Subject: Move checksum check from net_processing to net This removes the m_valid_checksum member from CNetMessage. Instead, GetMessage() returns an Optional. Additionally, GetMessage() has been given an out parameter to be used to hold error information. For now it is specifically a uint32_t used to hold the raw size of the corrupt message. The checksum check is now done in GetMessage. --- src/test/fuzz/p2p_transport_deserializer.cpp | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) (limited to 'src/test/fuzz') diff --git a/src/test/fuzz/p2p_transport_deserializer.cpp b/src/test/fuzz/p2p_transport_deserializer.cpp index 732136330b..3e9cd3af38 100644 --- a/src/test/fuzz/p2p_transport_deserializer.cpp +++ b/src/test/fuzz/p2p_transport_deserializer.cpp @@ -32,16 +32,19 @@ void test_one_input(const std::vector& buffer) n_bytes -= handled; if (deserializer.Complete()) { const std::chrono::microseconds m_time{std::numeric_limits::max()}; - const CNetMessage msg = deserializer.GetMessage(Params().MessageStart(), m_time); - assert(msg.m_command.size() <= CMessageHeader::COMMAND_SIZE); - assert(msg.m_raw_message_size <= buffer.size()); - assert(msg.m_raw_message_size == CMessageHeader::HEADER_SIZE + msg.m_message_size); - assert(msg.m_time == m_time); - if (msg.m_valid_header) { - assert(msg.m_valid_netmagic); - } - if (!msg.m_valid_netmagic) { - assert(!msg.m_valid_header); + uint32_t out_err_raw_size{0}; + Optional result{deserializer.GetMessage(Params().MessageStart(), m_time, out_err_raw_size)}; + if (result) { + assert(result->m_command.size() <= CMessageHeader::COMMAND_SIZE); + assert(result->m_raw_message_size <= buffer.size()); + assert(result->m_raw_message_size == CMessageHeader::HEADER_SIZE + result->m_message_size); + assert(result->m_time == m_time); + if (result->m_valid_header) { + assert(result->m_valid_netmagic); + } + if (!result->m_valid_netmagic) { + assert(!result->m_valid_header); + } } } } -- cgit v1.2.3 From 5bceef6b12fa16d20287693be377dace3dfec3e5 Mon Sep 17 00:00:00 2001 From: Troy Giorshev Date: Mon, 8 Jun 2020 22:37:55 -0400 Subject: Change CMessageHeader Constructor This commit removes the single-parameter contructor of CMessageHeader and replaces it with a default constructor. The single parameter contructor isn't used anywhere except for tests. There is no reason to initialize a CMessageHeader with a particular messagestart. This messagestart should always be replaced when deserializing an actual message header so that we can run checks on it. The default constructor initializes it to zero, just like the command and checksum. This also removes a parameter of a V1TransportDeserializer constructor, as it was only used for this purpose. --- src/test/fuzz/deserialize.cpp | 2 +- src/test/fuzz/p2p_transport_deserializer.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'src/test/fuzz') diff --git a/src/test/fuzz/deserialize.cpp b/src/test/fuzz/deserialize.cpp index 54793c890f..f87b7576a4 100644 --- a/src/test/fuzz/deserialize.cpp +++ b/src/test/fuzz/deserialize.cpp @@ -190,7 +190,7 @@ void test_one_input(const std::vector& buffer) AssertEqualAfterSerializeDeserialize(s); #elif MESSAGEHEADER_DESERIALIZE const CMessageHeader::MessageStartChars pchMessageStart = {0x00, 0x00, 0x00, 0x00}; - CMessageHeader mh(pchMessageStart); + CMessageHeader mh; DeserializeFromFuzzingInput(buffer, mh); (void)mh.IsValid(pchMessageStart); #elif ADDRESS_DESERIALIZE diff --git a/src/test/fuzz/p2p_transport_deserializer.cpp b/src/test/fuzz/p2p_transport_deserializer.cpp index 3e9cd3af38..5349fd3f68 100644 --- a/src/test/fuzz/p2p_transport_deserializer.cpp +++ b/src/test/fuzz/p2p_transport_deserializer.cpp @@ -20,7 +20,7 @@ void initialize() void test_one_input(const std::vector& buffer) { // Construct deserializer, with a dummy NodeId - V1TransportDeserializer deserializer{Params().MessageStart(), (NodeId)0, SER_NETWORK, INIT_PROTO_VERSION}; + V1TransportDeserializer deserializer{(NodeId)0, SER_NETWORK, INIT_PROTO_VERSION}; const char* pch = (const char*)buffer.data(); size_t n_bytes = buffer.size(); while (n_bytes > 0) { -- cgit v1.2.3 From 52d4ae46ab822d0f54e246a6f2364415cda149bd Mon Sep 17 00:00:00 2001 From: Troy Giorshev Date: Mon, 8 Jun 2020 22:26:22 -0400 Subject: Give V1TransportDeserializer CChainParams& member This adds a CChainParams& member to V1TransportDeserializer member, and use it in place of many Params() calls. In addition to reducing the number of calls to a global, this removes a parameter from GetMessage (and will later allow us to remove one from CMessageHeader::IsValid()) --- src/test/fuzz/p2p_transport_deserializer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/test/fuzz') diff --git a/src/test/fuzz/p2p_transport_deserializer.cpp b/src/test/fuzz/p2p_transport_deserializer.cpp index 5349fd3f68..6252b8e91b 100644 --- a/src/test/fuzz/p2p_transport_deserializer.cpp +++ b/src/test/fuzz/p2p_transport_deserializer.cpp @@ -20,7 +20,7 @@ void initialize() void test_one_input(const std::vector& buffer) { // Construct deserializer, with a dummy NodeId - V1TransportDeserializer deserializer{(NodeId)0, SER_NETWORK, INIT_PROTO_VERSION}; + V1TransportDeserializer deserializer{Params(), (NodeId)0, SER_NETWORK, INIT_PROTO_VERSION}; const char* pch = (const char*)buffer.data(); size_t n_bytes = buffer.size(); while (n_bytes > 0) { @@ -33,7 +33,7 @@ void test_one_input(const std::vector& buffer) if (deserializer.Complete()) { const std::chrono::microseconds m_time{std::numeric_limits::max()}; uint32_t out_err_raw_size{0}; - Optional result{deserializer.GetMessage(Params().MessageStart(), m_time, out_err_raw_size)}; + Optional result{deserializer.GetMessage(m_time, out_err_raw_size)}; if (result) { assert(result->m_command.size() <= CMessageHeader::COMMAND_SIZE); assert(result->m_raw_message_size <= buffer.size()); -- cgit v1.2.3 From deb52711a17236d0fca302701b5af585341ab42a Mon Sep 17 00:00:00 2001 From: Troy Giorshev Date: Tue, 26 May 2020 17:01:57 -0400 Subject: Remove header checks out of net_processing This moves header size and netmagic checking out of net_processing and into net. This check now runs in ReadHeader, so that net can exit early out of receiving bytes from the peer. IsValid is now slimmed down, so it no longer needs a MessageStartChars& parameter. Additionally this removes the rest of the m_valid_* members from CNetMessage. --- src/test/fuzz/deserialize.cpp | 3 +-- src/test/fuzz/p2p_transport_deserializer.cpp | 6 ------ 2 files changed, 1 insertion(+), 8 deletions(-) (limited to 'src/test/fuzz') diff --git a/src/test/fuzz/deserialize.cpp b/src/test/fuzz/deserialize.cpp index f87b7576a4..b799d3b43b 100644 --- a/src/test/fuzz/deserialize.cpp +++ b/src/test/fuzz/deserialize.cpp @@ -189,10 +189,9 @@ void test_one_input(const std::vector& buffer) DeserializeFromFuzzingInput(buffer, s); AssertEqualAfterSerializeDeserialize(s); #elif MESSAGEHEADER_DESERIALIZE - const CMessageHeader::MessageStartChars pchMessageStart = {0x00, 0x00, 0x00, 0x00}; CMessageHeader mh; DeserializeFromFuzzingInput(buffer, mh); - (void)mh.IsValid(pchMessageStart); + (void)mh.IsCommandValid(); #elif ADDRESS_DESERIALIZE CAddress a; DeserializeFromFuzzingInput(buffer, a); diff --git a/src/test/fuzz/p2p_transport_deserializer.cpp b/src/test/fuzz/p2p_transport_deserializer.cpp index 6252b8e91b..7e216e16fe 100644 --- a/src/test/fuzz/p2p_transport_deserializer.cpp +++ b/src/test/fuzz/p2p_transport_deserializer.cpp @@ -39,12 +39,6 @@ void test_one_input(const std::vector& buffer) assert(result->m_raw_message_size <= buffer.size()); assert(result->m_raw_message_size == CMessageHeader::HEADER_SIZE + result->m_message_size); assert(result->m_time == m_time); - if (result->m_valid_header) { - assert(result->m_valid_netmagic); - } - if (!result->m_valid_netmagic) { - assert(!result->m_valid_header); - } } } } -- cgit v1.2.3