Merge #11580: Do not send (potentially) invalid headers in response to getheaders
725b79a
[test] Verify node doesn't send headers that haven't been fully validated (Russell Yanofsky)3788a84
Do not send (potentially) invalid headers in response to getheaders (Matt Corallo) Pull request description: 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. Tree-SHA512: c1f6e0cdcdfb78ea577d555f9b3ceb1b4b60eff4f6cf313bfd8b576c9562d797bea73abc23f7011f249ae36dd539c715f3d20487ac03ace60e84e1b77c0c1e1a
This commit is contained in:
commit
1f4375f8e7
2 changed files with 43 additions and 13 deletions
|
@ -781,11 +781,13 @@ void Misbehaving(NodeId pnode, int howmuch)
|
|||
|
||||
// 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
|
||||
// best equivalent proof of work) than the best header chain we know about.
|
||||
static bool StaleBlockRequestAllowed(const CBlockIndex* pindex, const Consensus::Params& consensusParams)
|
||||
// best equivalent proof of work) than the best header chain we know about and
|
||||
// we fully-validated them at some point.
|
||||
static bool BlockRequestAllowed(const CBlockIndex* pindex, const Consensus::Params& consensusParams)
|
||||
{
|
||||
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) &&
|
||||
(GetBlockProofEquivalentTime(*pindexBestHeader, *pindex, *pindexBestHeader, consensusParams) < STALE_RELAY_AGE_LIMIT);
|
||||
}
|
||||
|
@ -1074,14 +1076,9 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam
|
|||
CValidationState dummy;
|
||||
ActivateBestChain(dummy, Params(), a_recent_block);
|
||||
}
|
||||
if (chainActive.Contains(mi->second)) {
|
||||
send = true;
|
||||
} else {
|
||||
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());
|
||||
}
|
||||
send = BlockRequestAllowed(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
|
||||
|
@ -2034,8 +2031,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
|
|||
return true;
|
||||
pindex = (*mi).second;
|
||||
|
||||
if (!chainActive.Contains(pindex) &&
|
||||
!StaleBlockRequestAllowed(pindex, chainparams.GetConsensus())) {
|
||||
if (!BlockRequestAllowed(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());
|
||||
return true;
|
||||
}
|
||||
|
|
|
@ -10,6 +10,17 @@ Setup:
|
|||
receive inv's (omitted from testing description below, this is our control).
|
||||
Second node is used for creating reorgs.
|
||||
|
||||
test_null_locators
|
||||
==================
|
||||
|
||||
Sends two getheaders requests with null locator values. First request's hashstop
|
||||
value refers to validated block, while second request's hashstop value refers to
|
||||
a block which hasn't been validated. Verifies only the first request returns
|
||||
headers.
|
||||
|
||||
test_nonnull_locators
|
||||
=====================
|
||||
|
||||
Part 1: No headers announcements before "sendheaders"
|
||||
a. node mines a block [expect: inv]
|
||||
send getdata for the block [expect: block]
|
||||
|
@ -221,6 +232,29 @@ class SendHeadersTest(BitcoinTestFramework):
|
|||
inv_node.sync_with_ping()
|
||||
test_node.sync_with_ping()
|
||||
|
||||
self.test_null_locators(test_node)
|
||||
self.test_nonnull_locators(test_node, inv_node)
|
||||
|
||||
def test_null_locators(self, test_node):
|
||||
tip = self.nodes[0].getblockheader(self.nodes[0].generate(1)[0])
|
||||
tip_hash = int(tip["hash"], 16)
|
||||
|
||||
self.log.info("Verify getheaders with null locator and valid hashstop returns headers.")
|
||||
test_node.clear_last_announcement()
|
||||
test_node.get_headers(locator=[], hashstop=tip_hash)
|
||||
assert_equal(test_node.check_last_announcement(headers=[tip_hash]), True)
|
||||
|
||||
self.log.info("Verify getheaders with null locator and invalid hashstop does not return headers.")
|
||||
block = create_block(int(tip["hash"], 16), create_coinbase(tip["height"] + 1), tip["mediantime"] + 1)
|
||||
block.solve()
|
||||
test_node.send_header_for_blocks([block])
|
||||
test_node.clear_last_announcement()
|
||||
test_node.get_headers(locator=[], hashstop=int(block.hash, 16))
|
||||
test_node.sync_with_ping()
|
||||
assert_equal(test_node.block_announced, False)
|
||||
test_node.send_message(msg_block(block))
|
||||
|
||||
def test_nonnull_locators(self, test_node, inv_node):
|
||||
tip = int(self.nodes[0].getbestblockhash(), 16)
|
||||
|
||||
# PART 1
|
||||
|
|
Loading…
Reference in a new issue