net: Consistent checksum handling

laanwj · Fuzzbawls · commit de87ea6b79c6 · 2020-09-02T00:42:01.000-07:00
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.
diff --git a/src/main.cpp b/src/main.cpp
@@ -6159,15 +6159,17 @@ bool ProcessMessages(CNode* pfrom, CConnman& connman, std::atomic<bool>& interru
     // Message size
     unsigned int nMessageSize = hdr.nMessageSize;
 
-    // Checksum
-    CDataStream& vRecv = msg.vRecv;
-    uint256 hash = Hash(vRecv.begin(), vRecv.begin() + nMessageSize);
-    unsigned int nChecksum = ReadLE32((unsigned char*)&hash);
-    if (nChecksum != hdr.nChecksum) {
-        LogPrintf("ProcessMessages(%s, %u bytes): CHECKSUM ERROR nChecksum=%08x hdr.nChecksum=%08x\n",
-            SanitizeString(strCommand), nMessageSize, nChecksum, hdr.nChecksum);
-        return fMoreWork;
-    }
+        // Checksum
+        CDataStream& vRecv = msg.vRecv;
+        uint256 hash = Hash(vRecv.begin(), vRecv.begin() + nMessageSize);
+        if (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0)
+        {
+            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));
+            return fMoreWork;
+        }
 
     // Process message
     bool fRet = false;
diff --git a/src/protocol.cpp b/src/protocol.cpp
@@ -124,7 +124,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)
@@ -133,7 +133,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
@@ -47,7 +47,7 @@ class CMessageHeader
         READWRITE(FLATDATA(pchMessageStart));
         READWRITE(FLATDATA(pchCommand));
         READWRITE(nMessageSize);
-        READWRITE(nChecksum);
+        READWRITE(FLATDATA(pchChecksum));
     }
 
     // TODO: make private (improves encapsulation)
@@ -64,7 +64,7 @@ class CMessageHeader
     char pchMessageStart[MESSAGE_START_SIZE];
     char pchCommand[COMMAND_SIZE];
     unsigned int nMessageSize;
-    unsigned int nChecksum;
+    uint8_t pchChecksum[CHECKSUM_SIZE];
 };
 
 /**
