Skip to content

Commit

Permalink
[backport] net: Allow connections from misbehavior banned peers
Browse files Browse the repository at this point in the history
Summary:
0297be61a Allow connections from misbehavior banned peers. (Gregory Maxwell)

---

Pull request description:

This allows incoming connections from peers which are only banned
due to an automatic misbehavior ban if doing so won't fill inbound.

These peers are preferred for eviction when inbound fills, but may
still be kept if they fall into the protected classes.  This
eviction preference lasts the entire life of the connection even
if the ban expires.

If they misbehave again they'll still get disconnected.

The main purpose of banning on misbehavior is to prevent our
connections from being wasted on unhelpful peers such as ones
running incompatible consensus rules.  For inbound peers this
can be better accomplished with eviction preferences.

A secondary purpose was to reduce resource waste from repeated
abuse but virtually any attacker can get a nearly unlimited
supply of addresses, so disconnection is about the best we can
do.

This can reduce the potential from negative impact due to incorrect misbehaviour bans.

---

This is a backport from Core PR14929 (bitcoin/bitcoin#14929)

Test Plan:
  ninja check
  cmake --build . --config Release --target check-functional -- -j 6

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, nakihito

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, nakihito

Subscribers: nakihito

Differential Revision: https://reviews.bitcoinabc.org/D4759
  • Loading branch information
jonasschnelli authored and jonspock committed Jan 10, 2020
1 parent d60121b commit 74494ed
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 2 deletions.
27 changes: 25 additions & 2 deletions src/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,7 @@ struct NodeEvictionCandidate {
bool fBloomFilter;
CAddress addr;
uint64_t nKeyedNetGroup;
bool prefer_evict;
};

static bool ReverseCompareNodeMinPingTime(const NodeEvictionCandidate &a,
Expand Down Expand Up @@ -897,7 +898,8 @@ bool CConnman::AttemptToEvictConnection() {
node->fRelayTxes,
node->pfilter != nullptr,
node->addr,
node->nKeyedNetGroup};
node->nKeyedNetGroup,
node->m_prefer_evict};
vEvictionCandidates.push_back(candidate);
}
}
Expand Down Expand Up @@ -927,6 +929,20 @@ bool CConnman::AttemptToEvictConnection() {
return false;
}

// If any remaining peers are preferred for eviction consider only them.
// This happens after the other preferences since if a peer is really the
// best by other criteria (esp relaying blocks)
// then we probably don't want to evict it no matter what.
if (std::any_of(
vEvictionCandidates.begin(), vEvictionCandidates.end(),
[](NodeEvictionCandidate const &n) { return n.prefer_evict; })) {
vEvictionCandidates.erase(
std::remove_if(
vEvictionCandidates.begin(), vEvictionCandidates.end(),
[](NodeEvictionCandidate const &n) { return !n.prefer_evict; }),
vEvictionCandidates.end());
}

// Identify the network group with the most connections and youngest member.
// (vEvictionCandidates is already sorted by reverse connect time)
uint64_t naMostConnections;
Expand Down Expand Up @@ -1015,7 +1031,12 @@ void CConnman::AcceptConnection(const ListenSocket &hListenSocket) {
// sockets on all platforms. Set it again here just to be sure.
SetSocketNoDelay(hSocket);

if (m_banman && m_banman->IsBanned(addr) && !whitelisted) {
int bannedlevel = m_banman ? m_banman->IsBannedLevel(addr) : 0;

// Don't accept connections from banned peers, but if our inbound slots
// aren't almost full, accept if the only banning reason was an automatic
// misbehavior ban.
if (!whitelisted && bannedlevel > ((nInbound + 1 < nMaxInbound) ? 1 : 0)) {
LogPrint(BCLog::NET, "connection from %s dropped (banned)\n",
addr.ToString());
CloseSocket(hSocket);
Expand Down Expand Up @@ -1043,6 +1064,8 @@ void CConnman::AcceptConnection(const ListenSocket &hListenSocket) {
CalculateKeyedNetGroup(addr), nonce, addr_bind, "", true);
pnode->AddRef();
pnode->fWhitelisted = whitelisted;

pnode->m_prefer_evict = bannedlevel > 0;
m_msgproc->InitializeNode(*config, pnode);

LogPrint(BCLog::NET, "connection from %s accepted\n", addr.ToString());
Expand Down
2 changes: 2 additions & 0 deletions src/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,8 @@ class CNode {
GUARDED_BY(cs_SubVer);
// Used for both cleanSubVer and strSubVer.
CCriticalSection cs_SubVer;
// This peer is preferred for eviction.
bool m_prefer_evict{false};
// This peer can bypass DoS banning.
bool fWhitelisted{false};
// If true this node is being used as a short lived feeler.
Expand Down

0 comments on commit 74494ed

Please sign in to comment.