net: correctly ban before the handshake is complete
7a8c251901
made a change to avoid getting into SendMessages() until the version handshake (VERSION + VERACK) is complete. That was done to avoid leaking out messages to nodes who could connect, but never bothered sending us their version/verack. Unfortunately, the ban tally and possible disconnect are done as part of SendMessages(). So after7a8c251901
, if a peer managed to do something bannable before completing the handshake (say send 100 non-version messages before their version), they wouldn't actually end up getting disconnected/banned. That's fixed here by checking the banscore as part of ProcessMessages() in addition to SendMessages().
This commit is contained in:
parent
d304fef374
commit
c45b9fb54c
1 changed files with 37 additions and 23 deletions
|
@ -2596,6 +2596,36 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
static bool SendRejectsAndCheckIfBanned(CNode* pnode, CConnman& connman)
|
||||||
|
{
|
||||||
|
AssertLockHeld(cs_main);
|
||||||
|
CNodeState &state = *State(pnode->GetId());
|
||||||
|
|
||||||
|
BOOST_FOREACH(const CBlockReject& reject, state.rejects) {
|
||||||
|
connman.PushMessage(pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, (std::string)NetMsgType::BLOCK, reject.chRejectCode, reject.strRejectReason, reject.hashBlock));
|
||||||
|
}
|
||||||
|
state.rejects.clear();
|
||||||
|
|
||||||
|
if (state.fShouldBan) {
|
||||||
|
state.fShouldBan = false;
|
||||||
|
if (pnode->fWhitelisted)
|
||||||
|
LogPrintf("Warning: not punishing whitelisted peer %s!\n", pnode->addr.ToString());
|
||||||
|
else if (pnode->fAddnode)
|
||||||
|
LogPrintf("Warning: not punishing addnoded peer %s!\n", pnode->addr.ToString());
|
||||||
|
else {
|
||||||
|
pnode->fDisconnect = true;
|
||||||
|
if (pnode->addr.IsLocal())
|
||||||
|
LogPrintf("Warning: not banning local peer %s!\n", pnode->addr.ToString());
|
||||||
|
else
|
||||||
|
{
|
||||||
|
connman.Ban(pnode->addr, BanReasonNodeMisbehaving);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
bool ProcessMessages(CNode* pfrom, CConnman& connman, const std::atomic<bool>& interruptMsgProc)
|
bool ProcessMessages(CNode* pfrom, CConnman& connman, const std::atomic<bool>& interruptMsgProc)
|
||||||
{
|
{
|
||||||
const CChainParams& chainparams = Params();
|
const CChainParams& chainparams = Params();
|
||||||
|
@ -2706,8 +2736,12 @@ bool ProcessMessages(CNode* pfrom, CConnman& connman, const std::atomic<bool>& i
|
||||||
PrintExceptionContinue(NULL, "ProcessMessages()");
|
PrintExceptionContinue(NULL, "ProcessMessages()");
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!fRet)
|
if (!fRet) {
|
||||||
LogPrintf("%s(%s, %u bytes) FAILED peer=%d\n", __func__, SanitizeString(strCommand), nMessageSize, pfrom->id);
|
LogPrintf("%s(%s, %u bytes) FAILED peer=%d\n", __func__, SanitizeString(strCommand), nMessageSize, pfrom->id);
|
||||||
|
}
|
||||||
|
|
||||||
|
LOCK(cs_main);
|
||||||
|
SendRejectsAndCheckIfBanned(pfrom, connman);
|
||||||
|
|
||||||
return fMoreWork;
|
return fMoreWork;
|
||||||
}
|
}
|
||||||
|
@ -2773,30 +2807,10 @@ bool SendMessages(CNode* pto, CConnman& connman, const std::atomic<bool>& interr
|
||||||
if (!lockMain)
|
if (!lockMain)
|
||||||
return true;
|
return true;
|
||||||
|
|
||||||
|
if (SendRejectsAndCheckIfBanned(pto, connman))
|
||||||
|
return true;
|
||||||
CNodeState &state = *State(pto->GetId());
|
CNodeState &state = *State(pto->GetId());
|
||||||
|
|
||||||
BOOST_FOREACH(const CBlockReject& reject, state.rejects)
|
|
||||||
connman.PushMessage(pto, msgMaker.Make(NetMsgType::REJECT, (std::string)NetMsgType::BLOCK, reject.chRejectCode, reject.strRejectReason, reject.hashBlock));
|
|
||||||
state.rejects.clear();
|
|
||||||
|
|
||||||
if (state.fShouldBan) {
|
|
||||||
state.fShouldBan = false;
|
|
||||||
if (pto->fWhitelisted)
|
|
||||||
LogPrintf("Warning: not punishing whitelisted peer %s!\n", pto->addr.ToString());
|
|
||||||
else if (pto->fAddnode)
|
|
||||||
LogPrintf("Warning: not punishing addnoded peer %s!\n", pto->addr.ToString());
|
|
||||||
else {
|
|
||||||
pto->fDisconnect = true;
|
|
||||||
if (pto->addr.IsLocal())
|
|
||||||
LogPrintf("Warning: not banning local peer %s!\n", pto->addr.ToString());
|
|
||||||
else
|
|
||||||
{
|
|
||||||
connman.Ban(pto->addr, BanReasonNodeMisbehaving);
|
|
||||||
}
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Address refresh broadcast
|
// Address refresh broadcast
|
||||||
int64_t nNow = GetTimeMicros();
|
int64_t nNow = GetTimeMicros();
|
||||||
if (!IsInitialBlockDownload() && pto->nNextLocalAddrSend < nNow) {
|
if (!IsInitialBlockDownload() && pto->nNextLocalAddrSend < nNow) {
|
||||||
|
|
Loading…
Reference in a new issue