Disconnect outbound peers relaying invalid headers

This commit is contained in:
Suhas Daftuar 2017-10-26 14:54:33 -04:00
parent 4637f18522
commit 37886d5e2f
3 changed files with 56 additions and 12 deletions

View File

@ -1205,7 +1205,7 @@ inline void static SendBlockTransactions(const CBlock& block, const BlockTransac
connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCKTXN, resp)); connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCKTXN, resp));
} }
bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::vector<CBlockHeader>& headers, const CChainParams& chainparams) bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::vector<CBlockHeader>& headers, const CChainParams& chainparams, bool punish_duplicate_invalid)
{ {
const CNetMsgMaker msgMaker(pfrom->GetSendVersion()); const CNetMsgMaker msgMaker(pfrom->GetSendVersion());
size_t nCount = headers.size(); size_t nCount = headers.size();
@ -1258,13 +1258,48 @@ bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::ve
} }
CValidationState state; CValidationState state;
if (!ProcessNewBlockHeaders(headers, state, chainparams, &pindexLast)) { CBlockHeader first_invalid_header;
if (!ProcessNewBlockHeaders(headers, state, chainparams, &pindexLast, &first_invalid_header)) {
int nDoS; int nDoS;
if (state.IsInvalid(nDoS)) { if (state.IsInvalid(nDoS)) {
if (nDoS > 0) { if (nDoS > 0) {
LOCK(cs_main); LOCK(cs_main);
Misbehaving(pfrom->GetId(), nDoS); Misbehaving(pfrom->GetId(), nDoS);
} }
if (punish_duplicate_invalid && mapBlockIndex.find(first_invalid_header.GetHash()) != mapBlockIndex.end()) {
// Goal: don't allow outbound peers to use up our outbound
// connection slots if they are on incompatible chains.
//
// We ask the caller to set punish_invalid appropriately based
// on the peer and the method of header delivery (compact
// blocks are allowed to be invalid in some circumstances,
// under BIP 152).
// Here, we try to detect the narrow situation that we have a
// valid block header (ie it was valid at the time the header
// was received, and hence stored in mapBlockIndex) but know the
// block is invalid, and that a peer has announced that same
// block as being on its active chain.
// Disconnect the peer in such a situation.
//
// Note: if the header that is invalid was not accepted to our
// mapBlockIndex at all, that may also be grounds for
// disconnecting the peer, as the chain they are on is likely
// to be incompatible. However, there is a circumstance where
// that does not hold: if the header's timestamp is more than
// 2 hours ahead of our current time. In that case, the header
// may become valid in the future, and we don't want to
// disconnect a peer merely for serving us one too-far-ahead
// block header, to prevent an attacker from splitting the
// network by mining a block right at the 2 hour boundary.
//
// TODO: update the DoS logic (or, rather, rewrite the
// DoS-interface between validation and net_processing) so that
// the interface is cleaner, and so that we disconnect on all the
// reasons that a peer's headers chain is incompatible
// with ours (eg block->nVersion softforks, MTP violations,
// etc), and not just the duplicate-invalid case.
pfrom->fDisconnect = true;
}
return error("invalid header received"); return error("invalid header received");
} }
} }
@ -2219,7 +2254,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
// If we end up treating this as a plain headers message, call that as well // If we end up treating this as a plain headers message, call that as well
// without cs_main. // without cs_main.
bool fRevertToHeaderProcessing = false; bool fRevertToHeaderProcessing = false;
CDataStream vHeadersMsg(SER_NETWORK, PROTOCOL_VERSION);
// Keep a CBlock for "optimistic" compactblock reconstructions (see // Keep a CBlock for "optimistic" compactblock reconstructions (see
// below) // below)
@ -2336,10 +2370,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
return true; return true;
} else { } else {
// If this was an announce-cmpctblock, we want the same treatment as a header message // If this was an announce-cmpctblock, we want the same treatment as a header message
// Dirty hack to process as if it were just a headers message (TODO: move message handling into their own functions)
std::vector<CBlock> headers;
headers.push_back(cmpctblock.header);
vHeadersMsg << headers;
fRevertToHeaderProcessing = true; fRevertToHeaderProcessing = true;
} }
} }
@ -2348,8 +2378,14 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
if (fProcessBLOCKTXN) if (fProcessBLOCKTXN)
return ProcessMessage(pfrom, NetMsgType::BLOCKTXN, blockTxnMsg, nTimeReceived, chainparams, connman, interruptMsgProc); return ProcessMessage(pfrom, NetMsgType::BLOCKTXN, blockTxnMsg, nTimeReceived, chainparams, connman, interruptMsgProc);
if (fRevertToHeaderProcessing) if (fRevertToHeaderProcessing) {
return ProcessMessage(pfrom, NetMsgType::HEADERS, vHeadersMsg, nTimeReceived, chainparams, connman, interruptMsgProc); // Headers received from HB compact block peers are permitted to be
// relayed before full validation (see BIP 152), so we don't want to disconnect
// the peer if the header turns out to be for an invalid block.
// Note that if a peer tries to build on an invalid chain, that
// will be detected and the peer will be banned.
return ProcessHeadersMessage(pfrom, connman, {cmpctblock.header}, chainparams, /*punish_duplicate_invalid=*/false);
}
if (fBlockReconstructed) { if (fBlockReconstructed) {
// If we got here, we were able to optimistically reconstruct a // If we got here, we were able to optimistically reconstruct a
@ -2480,7 +2516,12 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
ReadCompactSize(vRecv); // ignore tx count; assume it is 0. ReadCompactSize(vRecv); // ignore tx count; assume it is 0.
} }
return ProcessHeadersMessage(pfrom, connman, headers, chainparams); // Headers received via a HEADERS message should be valid, and reflect
// the chain the peer is on. If we receive a known-invalid header,
// disconnect the peer if it is using one of our outbound connection
// slots.
bool should_punish = !pfrom->fInbound && !pfrom->m_manual_connection;
return ProcessHeadersMessage(pfrom, connman, headers, chainparams, should_punish);
} }
else if (strCommand == NetMsgType::BLOCK && !fImporting && !fReindex) // Ignore blocks received while importing else if (strCommand == NetMsgType::BLOCK && !fImporting && !fReindex) // Ignore blocks received while importing

View File

@ -3079,13 +3079,15 @@ static bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state
} }
// Exposed wrapper for AcceptBlockHeader // Exposed wrapper for AcceptBlockHeader
bool ProcessNewBlockHeaders(const std::vector<CBlockHeader>& headers, CValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex) bool ProcessNewBlockHeaders(const std::vector<CBlockHeader>& headers, CValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex, CBlockHeader *first_invalid)
{ {
if (first_invalid != nullptr) first_invalid->SetNull();
{ {
LOCK(cs_main); LOCK(cs_main);
for (const CBlockHeader& header : headers) { for (const CBlockHeader& header : headers) {
CBlockIndex *pindex = nullptr; // Use a temp pindex instead of ppindex to avoid a const_cast CBlockIndex *pindex = nullptr; // Use a temp pindex instead of ppindex to avoid a const_cast
if (!AcceptBlockHeader(header, state, chainparams, &pindex)) { if (!AcceptBlockHeader(header, state, chainparams, &pindex)) {
if (first_invalid) *first_invalid = header;
return false; return false;
} }
if (ppindex) { if (ppindex) {

View File

@ -247,8 +247,9 @@ bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr<cons
* @param[out] state This may be set to an Error state if any error occurred processing them * @param[out] state This may be set to an Error state if any error occurred processing them
* @param[in] chainparams The params for the chain we want to connect to * @param[in] chainparams The params for the chain we want to connect to
* @param[out] ppindex If set, the pointer will be set to point to the last new block index object for the given headers * @param[out] ppindex If set, the pointer will be set to point to the last new block index object for the given headers
* @param[out] first_invalid First header that fails validation, if one exists
*/ */
bool ProcessNewBlockHeaders(const std::vector<CBlockHeader>& block, CValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex=nullptr); bool ProcessNewBlockHeaders(const std::vector<CBlockHeader>& block, CValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex=nullptr, CBlockHeader *first_invalid=nullptr);
/** Check whether enough disk space is available for an incoming block */ /** Check whether enough disk space is available for an incoming block */
bool CheckDiskSpace(uint64_t nAdditionalBytes = 0); bool CheckDiskSpace(uint64_t nAdditionalBytes = 0);