Page MenuHomePhabricator
Files F14362649

D1778.id4945.diff
No OneTemporary
Actions

D1778.id4945.diff
View Options

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1395,7 +1395,8 @@
static bool ProcessHeadersMessage(const Config &config, CNode *pfrom,
CConnman *connman,
- const std::vector<CBlockHeader> &headers) {
+ const std::vector<CBlockHeader> &headers,
+ bool punish_duplicate_invalid) {
const CChainParams &chainparams = config.GetChainParams();
const CNetMsgMaker msgMaker(pfrom->GetSendVersion());
size_t nCount = headers.size();
@@ -1459,13 +1460,51 @@
}
CValidationState state;
- if (!ProcessNewBlockHeaders(config, headers, state, &pindexLast)) {
+ CBlockHeader first_invalid_header;
+ if (!ProcessNewBlockHeaders(config, headers, state, &pindexLast,
+ &first_invalid_header)) {
int nDoS;
if (state.IsInvalid(nDoS)) {
if (nDoS > 0) {
LOCK(cs_main);
Misbehaving(pfrom, nDoS, state.GetRejectReason());
}
+ 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");
}
}
@@ -2524,7 +2563,6 @@
// well
// without cs_main.
bool fRevertToHeaderProcessing = false;
- CDataStream vHeadersMsg(SER_NETWORK, PROTOCOL_VERSION);
// Keep a CBlock for "optimistic" compactblock reconstructions (see
// below)
@@ -2669,12 +2707,7 @@
return true;
} else {
// 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;
+ // treatment as a header message.
fRevertToHeaderProcessing = true;
}
}
@@ -2687,9 +2720,15 @@
}
if (fRevertToHeaderProcessing) {
- return ProcessMessage(config, pfrom, NetMsgType::HEADERS,
- vHeadersMsg, nTimeReceived, 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(config, pfrom, connman,
+ {cmpctblock.header},
+ /*punish_duplicate_invalid=*/false);
}
if (fBlockReconstructed) {
@@ -2822,7 +2861,13 @@
ReadCompactSize(vRecv);
}
- return ProcessHeadersMessage(config, pfrom, connman, headers);
+ // 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(config, pfrom, connman, headers,
+ should_punish);
}
else if (strCommand == NetMsgType::BLOCK && !fImporting && !fReindex) {
diff --git a/src/validation.h b/src/validation.h
--- a/src/validation.h
+++ b/src/validation.h
@@ -313,18 +313,20 @@
*
* Call without cs_main held.
*
- * @param[in] config The global config.
- * @param[in] block The block headers themselves.
- * @param[out] state This may be set to an Error state if any error occurred
- * processing them.
- * @param[out] ppindex If set, the pointer will be set to point to the last new
- * block index object for the given headers.
+ * @param[in] config The config.
+ * @param[in] block The block headers themselves.
+ * @param[out] state This may be set to an Error state if any error
+ * occurred processing them.
+ * @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.
* @return True if block headers were accepted as valid.
*/
bool ProcessNewBlockHeaders(const Config &config,
const std::vector<CBlockHeader> &block,
CValidationState &state,
- const CBlockIndex **ppindex = nullptr);
+ const CBlockIndex **ppindex = nullptr,
+ CBlockHeader *first_invalid = nullptr);
/**
* Check whether enough disk space is available for an incoming block.
diff --git a/src/validation.cpp b/src/validation.cpp
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -3393,13 +3393,21 @@
bool ProcessNewBlockHeaders(const Config &config,
const std::vector<CBlockHeader> &headers,
CValidationState &state,
- const CBlockIndex **ppindex) {
+ const CBlockIndex **ppindex,
+ CBlockHeader *first_invalid) {
+ if (first_invalid != nullptr) {
+ first_invalid->SetNull();
+ }
+
{
LOCK(cs_main);
for (const CBlockHeader &header : headers) {
// Use a temp pindex instead of ppindex to avoid a const_cast
CBlockIndex *pindex = nullptr;
if (!AcceptBlockHeader(config, header, state, &pindex)) {
+ if (first_invalid) {
+ *first_invalid = header;
+ }
return false;
}

File Metadata

Mime Type
text/plain
Expires
Mon, May 12, 01:38 (4 h, 42 s)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
5777028
Default Alt Text
D1778.id4945.diff (8 KB)

Event Timeline