Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 2 commits into from Nov 9, 2017
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3788a84
Do not send (potentially) invalid headers in response to getheaders
TheBlueMatt Oct 30, 2017
725b79a
[test] Verify node doesn't send headers that haven't been fully valid…
ryanofsky Nov 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 9 additions & 13 deletions src/net_processing.cpp
View file
Open in desktop
Expand Up @@ -755,11 +755,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) &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the function comment to describe this condition.

(pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() < STALE_RELAY_AGE_LIMIT) &&
(GetBlockProofEquivalentTime(*pindexBestHeader, *pindex, *pindexBestHeader, consensusParams) < STALE_RELAY_AGE_LIMIT);
}
Expand Down Expand Up @@ -1038,14 +1040,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
Expand Down Expand Up @@ -1986,8 +1983,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;
}
Expand Down
34 changes: 34 additions & 0 deletions test/functional/sendheaders.py
View file
Open in desktop
Expand Up @@ -10,6 +10,17 @@
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]
Expand Down Expand Up @@ -229,6 +240,29 @@ def run_test(self):
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
Expand Down