mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-01-19 06:43:45 +01:00
Merge #9720: net: fix banning and disallow sending messages before receiving verack
d943491qa: add a test to detect leaky p2p messages (Cory Fields)8650bbbqa: Expose on-connection to mininode listeners (Matt Corallo)5b5e4f8qa: mininode learns when a socket connects, not its first action (Matt Corallo)cbfc5a6net: require a verack before responding to anything else (Cory Fields)8502e7anet: parse reject earlier (Cory Fields)c45b9fbnet: correctly ban before the handshake is complete (Cory Fields)
This commit is contained in:
@@ -1190,8 +1190,31 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
|
||||
}
|
||||
}
|
||||
|
||||
if (strCommand == NetMsgType::REJECT)
|
||||
{
|
||||
if (fDebug) {
|
||||
try {
|
||||
std::string strMsg; unsigned char ccode; std::string strReason;
|
||||
vRecv >> LIMITED_STRING(strMsg, CMessageHeader::COMMAND_SIZE) >> ccode >> LIMITED_STRING(strReason, MAX_REJECT_MESSAGE_LENGTH);
|
||||
|
||||
if (strCommand == NetMsgType::VERSION)
|
||||
std::ostringstream ss;
|
||||
ss << strMsg << " code " << itostr(ccode) << ": " << strReason;
|
||||
|
||||
if (strMsg == NetMsgType::BLOCK || strMsg == NetMsgType::TX)
|
||||
{
|
||||
uint256 hash;
|
||||
vRecv >> hash;
|
||||
ss << ": hash " << hash.ToString();
|
||||
}
|
||||
LogPrint("net", "Reject %s\n", SanitizeString(ss.str()));
|
||||
} catch (const std::ios_base::failure&) {
|
||||
// Avoid feedback loops by preventing reject messages from triggering a new reject message.
|
||||
LogPrint("net", "Unparseable reject message received\n");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
else if (strCommand == NetMsgType::VERSION)
|
||||
{
|
||||
// Each connection can only send one version message
|
||||
if (pfrom->nVersion != 0)
|
||||
@@ -1402,6 +1425,13 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
|
||||
pfrom->fSuccessfullyConnected = true;
|
||||
}
|
||||
|
||||
else if (!pfrom->fSuccessfullyConnected)
|
||||
{
|
||||
// Must have a verack message before anything else
|
||||
LOCK(cs_main);
|
||||
Misbehaving(pfrom->GetId(), 1);
|
||||
return false;
|
||||
}
|
||||
|
||||
else if (strCommand == NetMsgType::ADDR)
|
||||
{
|
||||
@@ -2549,31 +2579,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
|
||||
pfrom->fRelayTxes = true;
|
||||
}
|
||||
|
||||
|
||||
else if (strCommand == NetMsgType::REJECT)
|
||||
{
|
||||
if (fDebug) {
|
||||
try {
|
||||
std::string strMsg; unsigned char ccode; std::string strReason;
|
||||
vRecv >> LIMITED_STRING(strMsg, CMessageHeader::COMMAND_SIZE) >> ccode >> LIMITED_STRING(strReason, MAX_REJECT_MESSAGE_LENGTH);
|
||||
|
||||
std::ostringstream ss;
|
||||
ss << strMsg << " code " << itostr(ccode) << ": " << strReason;
|
||||
|
||||
if (strMsg == NetMsgType::BLOCK || strMsg == NetMsgType::TX)
|
||||
{
|
||||
uint256 hash;
|
||||
vRecv >> hash;
|
||||
ss << ": hash " << hash.ToString();
|
||||
}
|
||||
LogPrint("net", "Reject %s\n", SanitizeString(ss.str()));
|
||||
} catch (const std::ios_base::failure&) {
|
||||
// Avoid feedback loops by preventing reject messages from triggering a new reject message.
|
||||
LogPrint("net", "Unparseable reject message received\n");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
else if (strCommand == NetMsgType::FEEFILTER) {
|
||||
CAmount newFeeFilter = 0;
|
||||
vRecv >> newFeeFilter;
|
||||
@@ -2601,6 +2606,36 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
|
||||
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)
|
||||
{
|
||||
const CChainParams& chainparams = Params();
|
||||
@@ -2711,8 +2746,12 @@ bool ProcessMessages(CNode* pfrom, CConnman& connman, const std::atomic<bool>& i
|
||||
PrintExceptionContinue(NULL, "ProcessMessages()");
|
||||
}
|
||||
|
||||
if (!fRet)
|
||||
if (!fRet) {
|
||||
LogPrintf("%s(%s, %u bytes) FAILED peer=%d\n", __func__, SanitizeString(strCommand), nMessageSize, pfrom->id);
|
||||
}
|
||||
|
||||
LOCK(cs_main);
|
||||
SendRejectsAndCheckIfBanned(pfrom, connman);
|
||||
|
||||
return fMoreWork;
|
||||
}
|
||||
@@ -2778,30 +2817,10 @@ bool SendMessages(CNode* pto, CConnman& connman, const std::atomic<bool>& interr
|
||||
if (!lockMain)
|
||||
return true;
|
||||
|
||||
if (SendRejectsAndCheckIfBanned(pto, connman))
|
||||
return true;
|
||||
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
|
||||
int64_t nNow = GetTimeMicros();
|
||||
if (!IsInitialBlockDownload() && pto->nNextLocalAddrSend < nNow) {
|
||||
|
||||
Reference in New Issue
Block a user