From 5d0ca81f7422b7eac0a5129c6fbccad77e36b85d Mon Sep 17 00:00:00 2001 From: Gregory Maxwell Date: Sun, 22 May 2016 05:55:15 +0000 Subject: Add recently accepted blocks and txn to AttemptToEvictConnection. This protects any not-already-protected peers who were the most recent four to relay transactions and most recent four to send blocks to us. --- src/main.cpp | 16 +++++++++++----- src/main.h | 2 +- src/net.cpp | 48 ++++++++++++++++++++++++++++++++++++++++++++---- src/net.h | 5 +++++ 4 files changed, 61 insertions(+), 10 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 6d006e8789..24ae61b74d 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3449,8 +3449,9 @@ static bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state } /** Store block on disk. If dbp is non-NULL, the file is known to already reside on disk */ -static bool AcceptBlock(const CBlock& block, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex, bool fRequested, const CDiskBlockPos* dbp) +static bool AcceptBlock(const CBlock& block, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex, bool fRequested, const CDiskBlockPos* dbp, bool* fNewBlock) { + if (fNewBlock) *fNewBlock = false; AssertLockHeld(cs_main); CBlockIndex *pindexDummy = NULL; @@ -3479,6 +3480,7 @@ static bool AcceptBlock(const CBlock& block, CValidationState& state, const CCha if (!fHasMoreWork) return true; // Don't process less-work chains if (fTooFarAhead) return true; // Block height is too high } + if (fNewBlock) *fNewBlock = true; if ((!CheckBlock(block, state, chainparams.GetConsensus(), GetAdjustedTime())) || !ContextualCheckBlock(block, state, pindex->pprev)) { if (state.IsInvalid() && !state.CorruptionPossible()) { @@ -3526,7 +3528,7 @@ static bool IsSuperMajority(int minVersion, const CBlockIndex* pstart, unsigned } -bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, const CNode* pfrom, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp) +bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, CNode* pfrom, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp) { { LOCK(cs_main); @@ -3535,9 +3537,11 @@ bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, c // Store to disk CBlockIndex *pindex = NULL; - bool ret = AcceptBlock(*pblock, state, chainparams, &pindex, fRequested, dbp); + bool fNewBlock = false; + bool ret = AcceptBlock(*pblock, state, chainparams, &pindex, fRequested, dbp, &fNewBlock); if (pindex && pfrom) { mapBlockSource[pindex->GetBlockHash()] = pfrom->GetId(); + if (fNewBlock) pfrom->nLastBlockTime = GetTime(); } CheckBlockIndex(chainparams.GetConsensus()); if (!ret) @@ -4107,7 +4111,7 @@ bool LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, CDiskB if (mapBlockIndex.count(hash) == 0 || (mapBlockIndex[hash]->nStatus & BLOCK_HAVE_DATA) == 0) { LOCK(cs_main); CValidationState state; - if (AcceptBlock(block, state, chainparams, NULL, true, dbp)) + if (AcceptBlock(block, state, chainparams, NULL, true, dbp, NULL)) nLoaded++; if (state.IsError()) break; @@ -4140,7 +4144,7 @@ bool LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, CDiskB head.ToString()); LOCK(cs_main); CValidationState dummy; - if (AcceptBlock(block, dummy, chainparams, NULL, true, &it->second)) + if (AcceptBlock(block, dummy, chainparams, NULL, true, &it->second, NULL)) { nLoaded++; queue.push_back(block.GetHash()); @@ -5040,6 +5044,8 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, RelayTransaction(tx); vWorkQueue.push_back(inv.hash); + pfrom->nLastTXTime = GetTime(); + LogPrint("mempool", "AcceptToMemoryPool: peer=%d: accepted %s (poolsz %u txn, %u kB)\n", pfrom->id, tx.GetHash().ToString(), diff --git a/src/main.h b/src/main.h index 9b99ae7c87..e2bfdfdf6e 100644 --- a/src/main.h +++ b/src/main.h @@ -215,7 +215,7 @@ void UnregisterNodeSignals(CNodeSignals& nodeSignals); * @param[out] dbp The already known disk position of pblock, or NULL if not yet stored. * @return True if state.IsValid() */ -bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, const CNode* pfrom, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp); +bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, CNode* pfrom, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp); /** Check whether enough disk space is available for an incoming block */ bool CheckDiskSpace(uint64_t nAdditionalBytes = 0); /** Open a block file (blk?????.dat) */ diff --git a/src/net.cpp b/src/net.cpp index 173eba57c8..ec547e8c99 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -838,6 +838,11 @@ struct NodeEvictionCandidate NodeId id; int64_t nTimeConnected; int64_t nMinPingUsecTime; + int64_t nLastBlockTime; + int64_t nLastTXTime; + bool fNetworkNode; + bool fRelayTxes; + bool fBloomFilter; CAddress addr; uint64_t nKeyedNetGroup; }; @@ -854,7 +859,24 @@ static bool ReverseCompareNodeTimeConnected(const NodeEvictionCandidate &a, cons static bool CompareNetGroupKeyed(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) { return a.nKeyedNetGroup < b.nKeyedNetGroup; -}; +} + +static bool CompareNodeBlockTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) +{ + // There is a fall-through here because it is common for a node to have many peers which have not yet relayed a block. + if (a.nLastBlockTime != b.nLastBlockTime) return a.nLastBlockTime < b.nLastBlockTime; + if (a.fNetworkNode != b.fNetworkNode) return b.fNetworkNode; + return a.nTimeConnected > b.nTimeConnected; +} + +static bool CompareNodeTXTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) +{ + // There is a fall-through here because it is common for a node to have more than a few peers that have not yet relayed txn. + if (a.nLastTXTime != b.nLastTXTime) return a.nLastTXTime < b.nLastTXTime; + if (a.fRelayTxes != b.fRelayTxes) return b.fRelayTxes; + if (a.fBloomFilter != b.fBloomFilter) return a.fBloomFilter; + return a.nTimeConnected > b.nTimeConnected; +} /** Try to find a connection to evict when the node is full. * Extreme care must be taken to avoid opening the node to attacker @@ -864,7 +886,7 @@ static bool CompareNetGroupKeyed(const NodeEvictionCandidate &a, const NodeEvict * to forge. In order to partition a node the attacker must be * simultaneously better at all of them than honest peers. */ -static bool AttemptToEvictConnection(bool fPreferNewConnection) { +static bool AttemptToEvictConnection() { std::vector vEvictionCandidates; { LOCK(cs_vNodes); @@ -876,7 +898,9 @@ static bool AttemptToEvictConnection(bool fPreferNewConnection) { continue; if (node->fDisconnect) continue; - NodeEvictionCandidate candidate = {node->id, node->nTimeConnected, node->nMinPingUsecTime, node->addr, node->nKeyedNetGroup}; + NodeEvictionCandidate candidate = {node->id, node->nTimeConnected, node->nMinPingUsecTime, + node->nLastBlockTime, node->nLastTXTime, node->fNetworkNode, + node->fRelayTxes, node->pfilter != NULL, node->addr, node->nKeyedNetGroup}; vEvictionCandidates.push_back(candidate); } } @@ -899,6 +923,20 @@ static bool AttemptToEvictConnection(bool fPreferNewConnection) { if (vEvictionCandidates.empty()) return false; + // Protect 4 nodes that most recently sent us transactions. + // An attacker cannot manipulate this metric without performing useful work. + std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), CompareNodeTXTime); + vEvictionCandidates.erase(vEvictionCandidates.end() - std::min(4, static_cast(vEvictionCandidates.size())), vEvictionCandidates.end()); + + if (vEvictionCandidates.empty()) return false; + + // Protect 4 nodes that most recently sent us blocks. + // An attacker cannot manipulate this metric without performing useful work. + std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), CompareNodeBlockTime); + vEvictionCandidates.erase(vEvictionCandidates.end() - std::min(4, static_cast(vEvictionCandidates.size())), vEvictionCandidates.end()); + + if (vEvictionCandidates.empty()) return false; + // Protect the half of the remaining nodes which have been connected the longest. // This replicates the non-eviction implicit behavior, and precludes attacks that start later. std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), ReverseCompareNodeTimeConnected); @@ -999,7 +1037,7 @@ static void AcceptConnection(const ListenSocket& hListenSocket) { if (nInbound >= nMaxInbound) { - if (!AttemptToEvictConnection(whitelisted)) { + if (!AttemptToEvictConnection()) { // No connection to evict, disconnect the new connection LogPrint("net", "failed to find an eviction candidate - connection dropped (full)\n"); CloseSocket(hSocket); @@ -2358,6 +2396,8 @@ CNode::CNode(SOCKET hSocketIn, const CAddress& addrIn, const std::string& addrNa fSentAddr = false; pfilter = new CBloomFilter(); timeLastMempoolReq = 0; + nLastBlockTime = 0; + nLastTXTime = 0; nPingNonceSent = 0; nPingUsecStart = 0; nPingUsecTime = 0; diff --git a/src/net.h b/src/net.h index 5c1f7e3e89..2f96c58e30 100644 --- a/src/net.h +++ b/src/net.h @@ -416,6 +416,11 @@ public: // Last time a "MEMPOOL" request was serviced. std::atomic timeLastMempoolReq; + + // Block and TXN accept times + std::atomic nLastBlockTime; + std::atomic nLastTXTime; + // Ping time measurement: // The pong reply we're expecting, or 0 if no pong expected. uint64_t nPingNonceSent; -- cgit v1.2.3 From 6ee7f05622c32431a9815a96b31a6a65a821fdcc Mon Sep 17 00:00:00 2001 From: Gregory Maxwell Date: Fri, 10 Jun 2016 03:02:01 +0000 Subject: Allow disconnecting a netgroup with only one member in eviction. With the latest additions there are enough protective measures that we can take the training wheels off. --- src/net.cpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index ec547e8c99..89eb55ae94 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -965,13 +965,6 @@ static bool AttemptToEvictConnection() { // Reduce to the network group with the most connections vEvictionCandidates = std::move(mapAddrCounts[naMostConnections]); - // Do not disconnect peers if there is only one unprotected connection from their network group. - // This step excessively favors netgroup diversity, and should be removed once more protective criteria are established. - if (vEvictionCandidates.size() <= 1) - // unless we prefer the new connection (for whitelisted peers) - if (!fPreferNewConnection) - return false; - // Disconnect from the network group with the most connections NodeId evicted = vEvictionCandidates.front().id; LOCK(cs_vNodes); -- cgit v1.2.3