From ad7c6db871e4cda0f93dbad2e1c873d8d2e86941 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 27 Sep 2016 14:05:24 +0200 Subject: [PATCH 1/2] net: Consistent checksum handling In principle, the checksums of P2P packets are simply 4-byte blobs which are the first four bytes of SHA256(SHA256(payload)). Currently they are handled as little-endian 32-bit integers half of the time, as blobs the other half, sometimes copying the one to the other, resulting in somewhat confused code. This PR changes the handling to be consistent both at packet creation and receiving, making it (I think) easier to understand. --- src/main.cpp | 9 +++++---- src/protocol.cpp | 4 ++-- src/protocol.h | 4 ++-- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index a4c6802d5607a..4db6e54f33aee 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -6498,11 +6498,12 @@ bool ProcessMessages(CNode* pfrom, CConnman& connman) // Checksum CDataStream& vRecv = msg.vRecv; uint256 hash = Hash(vRecv.begin(), vRecv.begin() + nMessageSize); - unsigned int nChecksum = ReadLE32((unsigned char*)&hash); - if (nChecksum != hdr.nChecksum) + if (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0) { - LogPrintf("%s(%s, %u bytes): CHECKSUM ERROR nChecksum=%08x hdr.nChecksum=%08x\n", __func__, - SanitizeString(strCommand), nMessageSize, nChecksum, hdr.nChecksum); + LogPrintf("%s(%s, %u bytes): CHECKSUM ERROR expected %s was %s\n", __func__, + SanitizeString(strCommand), nMessageSize, + HexStr(hash.begin(), hash.begin()+CMessageHeader::CHECKSUM_SIZE), + HexStr(hdr.pchChecksum, hdr.pchChecksum+CMessageHeader::CHECKSUM_SIZE)); continue; } diff --git a/src/protocol.cpp b/src/protocol.cpp index 8c35f681493be..d955f219bea5d 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -152,7 +152,7 @@ CMessageHeader::CMessageHeader(const MessageStartChars& pchMessageStartIn) memcpy(pchMessageStart, pchMessageStartIn, MESSAGE_START_SIZE); memset(pchCommand, 0, sizeof(pchCommand)); nMessageSize = -1; - nChecksum = 0; + memset(pchChecksum, 0, CHECKSUM_SIZE); } CMessageHeader::CMessageHeader(const MessageStartChars& pchMessageStartIn, const char* pszCommand, unsigned int nMessageSizeIn) @@ -161,7 +161,7 @@ CMessageHeader::CMessageHeader(const MessageStartChars& pchMessageStartIn, const memset(pchCommand, 0, sizeof(pchCommand)); strncpy(pchCommand, pszCommand, COMMAND_SIZE); nMessageSize = nMessageSizeIn; - nChecksum = 0; + memset(pchChecksum, 0, CHECKSUM_SIZE); } std::string CMessageHeader::GetCommand() const diff --git a/src/protocol.h b/src/protocol.h index 36d7775abb1b1..6655db3ddcdd6 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -45,7 +45,7 @@ class CMessageHeader READWRITE(FLATDATA(pchMessageStart)); READWRITE(FLATDATA(pchCommand)); READWRITE(nMessageSize); - READWRITE(nChecksum); + READWRITE(FLATDATA(pchChecksum)); } // TODO: make private (improves encapsulation) @@ -62,7 +62,7 @@ class CMessageHeader char pchMessageStart[MESSAGE_START_SIZE]; char pchCommand[COMMAND_SIZE]; unsigned int nMessageSize; - unsigned int nChecksum; + uint8_t pchChecksum[CHECKSUM_SIZE]; }; /** From fcb20b8d8ad0afcf2ee55efe9f660537eb5b5540 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 28 Sep 2016 15:33:45 +0200 Subject: [PATCH 2/2] net: Hardcode protocol sizes and use fixed-size types The P2P network uses a fixed protocol, these sizes shouldn't change based on what happens to be the architecture. --- src/protocol.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/protocol.h b/src/protocol.h index 6655db3ddcdd6..bc0ed47a8bff0 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -52,8 +52,8 @@ class CMessageHeader public: enum { COMMAND_SIZE = 12, - MESSAGE_SIZE_SIZE = sizeof(int), - CHECKSUM_SIZE = sizeof(int), + MESSAGE_SIZE_SIZE = 4, + CHECKSUM_SIZE = 4, MESSAGE_SIZE_OFFSET = MESSAGE_START_SIZE + COMMAND_SIZE, CHECKSUM_OFFSET = MESSAGE_SIZE_OFFSET + MESSAGE_SIZE_SIZE, @@ -61,7 +61,7 @@ class CMessageHeader }; char pchMessageStart[MESSAGE_START_SIZE]; char pchCommand[COMMAND_SIZE]; - unsigned int nMessageSize; + uint32_t nMessageSize; uint8_t pchChecksum[CHECKSUM_SIZE]; };