Skip to content

Commit f4fb2c1

Browse files
laanwjPastaPastaPasta
laanwj
authored and
PastaPastaPasta
committed
Merge bitcoin#11583: Do not make it trivial for inbound peers to generate log entries
be9f38c Do not make it trivial for inbound peers to generate log entries (Matt Corallo) Pull request description: Based on bitcoin#11580 because I'm lazy. We should generally avoid writing to debug.log unconditionally for inbound peers which misbehave (the peer being about to be banned being an exception, since they cannot do this twice). Tree-SHA512: 8e59c8d08d00b1527951b30f4842d010a4c2fc440503ade112baa2c1b9afd0e0d1c5c2df83dde25183a242af45089cf9b9f873b71796771232ffb6c5fc6cc0cc
1 parent fccf28b commit f4fb2c1

File tree

3 files changed

+23
-14
lines changed

3 files changed

+23
-14
lines changed

‎src/net.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1199,7 +1199,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
11991199

12001200
if (IsBanned(addr) && !whitelisted)
12011201
{
1202-
LogPrintf("%s (banned)\n", strDropped);
1202+
LogPrint(BCLog::NET, "%s (banned)\n", strDropped);
12031203
CloseSocket(hSocket);
12041204
return;
12051205
}

‎src/net_processing.cpp‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1167,7 +1167,7 @@ void static ProcessGetBlockData(CNode* pfrom, const Consensus::Params& consensus
11671167
if (mi != mapBlockIndex.end()) {
11681168
send = BlockRequestAllowed(mi->second, consensusParams);
11691169
if (!send) {
1170-
LogPrintf("%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom->GetId());
1170+
LogPrint(BCLog::NET,"%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom->GetId());
11711171
}
11721172
}
11731173
const CNetMsgMaker msgMaker(pfrom->GetSendVersion());
@@ -1832,7 +1832,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
18321832
if (nVersion < MIN_PEER_PROTO_VERSION)
18331833
{
18341834
// disconnect from peers older than this proto version
1835-
LogPrintf("peer=%d using obsolete version %i; disconnecting\n", pfrom->GetId(), nVersion);
1835+
LogPrint(BCLog::NET, "peer=%d using obsolete version %i; disconnecting\n", pfrom->GetId(), nVersion);
18361836
connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_OBSOLETE,
18371837
strprintf("Version must be %d or greater", MIN_PEER_PROTO_VERSION)));
18381838
pfrom->fDisconnect = true;
@@ -1943,7 +1943,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
19431943
if (fLogIPs)
19441944
remoteAddr = ", peeraddr=" + pfrom->addr.ToString();
19451945

1946-
LogPrintf("receive version message: %s: version %d, blocks=%d, us=%s, peer=%d%s\n",
1946+
LogPrint(BCLog::NET, "receive version message: %s: version %d, blocks=%d, us=%s, peer=%d%s\n",
19471947
cleanSubVer, pfrom->nVersion,
19481948
pfrom->nStartingHeight, addrMe.ToString(), pfrom->GetId(),
19491949
remoteAddr);
@@ -1978,6 +1978,9 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
19781978
// Mark this node as currently connected, so we update its timestamp later.
19791979
LOCK(cs_main);
19801980
State(pfrom->GetId())->fCurrentlyConnected = true;
1981+
LogPrintf("New outbound peer connected: version: %d, blocks=%d, peer=%d%s\n",
1982+
pfrom->nVersion.load(), pfrom->nStartingHeight, pfrom->GetId(),
1983+
(fLogIPs ? strprintf(", peeraddr=%s", pfrom->addr.ToString()) : ""));
19811984
}
19821985

19831986
if (pfrom->nVersion >= LLMQS_PROTO_VERSION) {
@@ -2316,7 +2319,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
23162319

23172320
BlockMap::iterator it = mapBlockIndex.find(req.blockhash);
23182321
if (it == mapBlockIndex.end() || !(it->second->nStatus & BLOCK_HAVE_DATA)) {
2319-
LogPrintf("Peer %d sent us a getblocktxn for a block we don't have", pfrom->GetId());
2322+
LogPrint(BCLog::NET, "Peer %d sent us a getblocktxn for a block we don't have", pfrom->GetId());
23202323
return true;
23212324
}
23222325

@@ -2367,7 +2370,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
23672370
pindex = (*mi).second;
23682371

23692372
if (!BlockRequestAllowed(pindex, chainparams.GetConsensus())) {
2370-
LogPrintf("%s: ignoring request from peer=%i for old block header that isn't in the main chain\n", __func__, pfrom->GetId());
2373+
LogPrint(BCLog::NET, "%s: ignoring request from peer=%i for old block header that isn't in the main chain\n", __func__, pfrom->GetId());
23712374
return true;
23722375
}
23732376
}
@@ -2625,10 +2628,12 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
26252628
int nDoS;
26262629
if (state.IsInvalid(nDoS)) {
26272630
if (nDoS > 0) {
2631+
LogPrintf("Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId());
26282632
LOCK(cs_main);
26292633
Misbehaving(pfrom->GetId(), nDoS);
2634+
} else {
2635+
LogPrint(BCLog::NET, "Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId());
26302636
}
2631-
LogPrintf("Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId());
26322637
return true;
26332638
}
26342639
}
@@ -3270,7 +3275,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
32703275
msg.SetVersion(pfrom->GetRecvVersion());
32713276
// Scan for message start
32723277
if (memcmp(msg.hdr.pchMessageStart, chainparams.MessageStart(), CMessageHeader::MESSAGE_START_SIZE) != 0) {
3273-
LogPrintf("PROCESSMESSAGE: INVALID MESSAGESTART %s peer=%d\n", SanitizeString(msg.hdr.GetCommand()), pfrom->GetId());
3278+
LogPrint(BCLog::NET, "PROCESSMESSAGE: INVALID MESSAGESTART %s peer=%d\n", SanitizeString(msg.hdr.GetCommand()), pfrom->GetId());
32743279
pfrom->fDisconnect = true;
32753280
return false;
32763281
}
@@ -3279,7 +3284,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
32793284
CMessageHeader& hdr = msg.hdr;
32803285
if (!hdr.IsValid(chainparams.MessageStart()))
32813286
{
3282-
LogPrintf("PROCESSMESSAGE: ERRORS IN HEADER %s peer=%d\n", SanitizeString(hdr.GetCommand()), pfrom->GetId());
3287+
LogPrint(BCLog::NET, "PROCESSMESSAGE: ERRORS IN HEADER %s peer=%d\n", SanitizeString(hdr.GetCommand()), pfrom->GetId());
32833288
return fMoreWork;
32843289
}
32853290
std::string strCommand = hdr.GetCommand();
@@ -3292,7 +3297,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
32923297
const uint256& hash = msg.GetMessageHash();
32933298
if (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0)
32943299
{
3295-
LogPrintf("%s(%s, %u bytes): CHECKSUM ERROR expected %s was %s\n", __func__,
3300+
LogPrint(BCLog::NET, "%s(%s, %u bytes): CHECKSUM ERROR expected %s was %s\n", __func__,
32963301
SanitizeString(strCommand), nMessageSize,
32973302
HexStr(hash.begin(), hash.begin()+CMessageHeader::CHECKSUM_SIZE),
32983303
HexStr(hdr.pchChecksum, hdr.pchChecksum+CMessageHeader::CHECKSUM_SIZE));
@@ -3315,17 +3320,17 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
33153320
if (strstr(e.what(), "end of data"))
33163321
{
33173322
// Allow exceptions from under-length message on vRecv
3318-
LogPrintf("%s(%s, %u bytes): Exception '%s' caught, normally caused by a message being shorter than its stated length\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
3323+
LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' caught, normally caused by a message being shorter than its stated length\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
33193324
}
33203325
else if (strstr(e.what(), "size too large"))
33213326
{
33223327
// Allow exceptions from over-long size
3323-
LogPrintf("%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
3328+
LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
33243329
}
33253330
else if (strstr(e.what(), "non-canonical ReadCompactSize()"))
33263331
{
33273332
// Allow exceptions from non-canonical encoding
3328-
LogPrintf("%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
3333+
LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
33293334
}
33303335
else
33313336
{
@@ -3336,7 +3341,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
33363341
}
33373342

33383343
if (!fRet) {
3339-
LogPrintf("%s(%s, %u bytes) FAILED peer=%d\n", __func__, SanitizeString(strCommand), nMessageSize, pfrom->GetId());
3344+
LogPrint(BCLog::NET, "%s(%s, %u bytes) FAILED peer=%d\n", __func__, SanitizeString(strCommand), nMessageSize, pfrom->GetId());
33403345
}
33413346

33423347
LOCK(cs_main);

‎src/util.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,10 @@ template<typename T, typename... Args> static inline void MarkUsed(const T& t, c
184184
MarkUsed(args...);
185185
}
186186

187+
// Be conservative when using LogPrintf/error or other things which
188+
// unconditionally log to debug.log! It should not be the case that an inbound
189+
// peer can fill up a users disk with debug.log entries.
190+
187191
#ifdef USE_COVERAGE
188192
#define LogPrintf(...) do { MarkUsed(__VA_ARGS__); } while(0)
189193
#define LogPrint(category, ...) do { MarkUsed(__VA_ARGS__); } while(0)

0 commit comments

Comments
 (0)