Skip to content

Commit

Permalink
Backport Bitcoin PR#8822: net: Consistent checksum handling (#1565)
Browse files Browse the repository at this point in the history
* 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.

* 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.
  • Loading branch information
OlegGirko authored and UdjinM6 committed Aug 8, 2017
1 parent 87707c0 commit bcf5455
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 11 deletions.
9 changes: 5 additions & 4 deletions src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions src/protocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
10 changes: 5 additions & 5 deletions src/protocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,24 +45,24 @@ class CMessageHeader
READWRITE(FLATDATA(pchMessageStart));
READWRITE(FLATDATA(pchCommand));
READWRITE(nMessageSize);
READWRITE(nChecksum);
READWRITE(FLATDATA(pchChecksum));
}

// TODO: make private (improves encapsulation)
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,
HEADER_SIZE = MESSAGE_START_SIZE + COMMAND_SIZE + MESSAGE_SIZE_SIZE + CHECKSUM_SIZE
};
char pchMessageStart[MESSAGE_START_SIZE];
char pchCommand[COMMAND_SIZE];
unsigned int nMessageSize;
unsigned int nChecksum;
uint32_t nMessageSize;
uint8_t pchChecksum[CHECKSUM_SIZE];
};

/**
Expand Down

0 comments on commit bcf5455

Please sign in to comment.