Do not send (potentially) invalid headers in response to getheaders

Nowhere else in the protocol do we send headers which are for
blocks we have not fully validated except in response to getheaders
messages with a null locator. On my public node I have not seen any
such request (whether for an invalid block or not) in at least two
years of debug.log output, indicating that this should have minimal
impact.
This commit is contained in:
Matt Corallo 2017-10-30 10:41:05 -04:00
parent bb9ab0fccf
commit 3788a8479b
1 changed files with 9 additions and 13 deletions

View File

@ -755,11 +755,13 @@ void Misbehaving(NodeId pnode, int howmuch)
// To prevent fingerprinting attacks, only send blocks/headers outside of the // To prevent fingerprinting attacks, only send blocks/headers outside of the
// active chain if they are no more than a month older (both in time, and in // active chain if they are no more than a month older (both in time, and in
// best equivalent proof of work) than the best header chain we know about. // best equivalent proof of work) than the best header chain we know about and
static bool StaleBlockRequestAllowed(const CBlockIndex* pindex, const Consensus::Params& consensusParams) // we fully-validated them at some point.
static bool BlockRequestAllowed(const CBlockIndex* pindex, const Consensus::Params& consensusParams)
{ {
AssertLockHeld(cs_main); AssertLockHeld(cs_main);
return (pindexBestHeader != nullptr) && if (chainActive.Contains(pindex)) return true;
return pindex->IsValid(BLOCK_VALID_SCRIPTS) && (pindexBestHeader != nullptr) &&
(pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() < STALE_RELAY_AGE_LIMIT) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() < STALE_RELAY_AGE_LIMIT) &&
(GetBlockProofEquivalentTime(*pindexBestHeader, *pindex, *pindexBestHeader, consensusParams) < STALE_RELAY_AGE_LIMIT); (GetBlockProofEquivalentTime(*pindexBestHeader, *pindex, *pindexBestHeader, consensusParams) < STALE_RELAY_AGE_LIMIT);
} }
@ -1038,14 +1040,9 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam
CValidationState dummy; CValidationState dummy;
ActivateBestChain(dummy, Params(), a_recent_block); ActivateBestChain(dummy, Params(), a_recent_block);
} }
if (chainActive.Contains(mi->second)) { send = BlockRequestAllowed(mi->second, consensusParams);
send = true; if (!send) {
} else { LogPrintf("%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom->GetId());
send = mi->second->IsValid(BLOCK_VALID_SCRIPTS) &&
StaleBlockRequestAllowed(mi->second, consensusParams);
if (!send) {
LogPrintf("%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom->GetId());
}
} }
} }
// disconnect node in case we have reached the outbound limit for serving historical blocks // disconnect node in case we have reached the outbound limit for serving historical blocks
@ -1986,8 +1983,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
return true; return true;
pindex = (*mi).second; pindex = (*mi).second;
if (!chainActive.Contains(pindex) && if (!BlockRequestAllowed(pindex, chainparams.GetConsensus())) {
!StaleBlockRequestAllowed(pindex, chainparams.GetConsensus())) {
LogPrintf("%s: ignoring request from peer=%i for old block header that isn't in the main chain\n", __func__, pfrom->GetId()); LogPrintf("%s: ignoring request from peer=%i for old block header that isn't in the main chain\n", __func__, pfrom->GetId());
return true; return true;
} }