Skip to content

Commit 37ffa16

Browse files
laanwjlaanwj
committed
Merge #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 #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
2 parents 8ab6c0b + be9f38c commit 37ffa16

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
@@ -1102,7 +1102,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
11021102

11031103
if (IsBanned(addr) && !whitelisted)
11041104
{
1105-
LogPrintf("connection from %s dropped (banned)\n", addr.ToString());
1105+
LogPrint(BCLog::NET, "connection from %s dropped (banned)\n", addr.ToString());
11061106
CloseSocket(hSocket);
11071107
return;
11081108
}

‎src/net_processing.cpp

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1078,7 +1078,7 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam
10781078
}
10791079
send = BlockRequestAllowed(mi->second, consensusParams);
10801080
if (!send) {
1081-
LogPrintf("%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom->GetId());
1081+
LogPrint(BCLog::NET, "%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom->GetId());
10821082
}
10831083
}
10841084
// disconnect node in case we have reached the outbound limit for serving historical blocks
@@ -1569,7 +1569,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
15691569
if (nVersion < MIN_PEER_PROTO_VERSION)
15701570
{
15711571
// disconnect from peers older than this proto version
1572-
LogPrintf("peer=%d using obsolete version %i; disconnecting\n", pfrom->GetId(), nVersion);
1572+
LogPrint(BCLog::NET, "peer=%d using obsolete version %i; disconnecting\n", pfrom->GetId(), nVersion);
15731573
connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_OBSOLETE,
15741574
strprintf("Version must be %d or greater", MIN_PEER_PROTO_VERSION)));
15751575
pfrom->fDisconnect = true;
@@ -1669,7 +1669,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
16691669
if (fLogIPs)
16701670
remoteAddr = ", peeraddr=" + pfrom->addr.ToString();
16711671

1672-
LogPrintf("receive version message: %s: version %d, blocks=%d, us=%s, peer=%d%s\n",
1672+
LogPrint(BCLog::NET, "receive version message: %s: version %d, blocks=%d, us=%s, peer=%d%s\n",
16731673
cleanSubVer, pfrom->nVersion,
16741674
pfrom->nStartingHeight, addrMe.ToString(), pfrom->GetId(),
16751675
remoteAddr);
@@ -1712,6 +1712,9 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
17121712
// Mark this node as currently connected, so we update its timestamp later.
17131713
LOCK(cs_main);
17141714
State(pfrom->GetId())->fCurrentlyConnected = true;
1715+
LogPrintf("New outbound peer connected: version: %d, blocks=%d, peer=%d%s\n",
1716+
pfrom->nVersion.load(), pfrom->nStartingHeight, pfrom->GetId(),
1717+
(fLogIPs ? strprintf(", peeraddr=%s", pfrom->addr.ToString()) : ""));
17151718
}
17161719

17171720
if (pfrom->nVersion >= SENDHEADERS_VERSION) {
@@ -1990,7 +1993,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
19901993

19911994
BlockMap::iterator it = mapBlockIndex.find(req.blockhash);
19921995
if (it == mapBlockIndex.end() || !(it->second->nStatus & BLOCK_HAVE_DATA)) {
1993-
LogPrintf("Peer %d sent us a getblocktxn for a block we don't have", pfrom->GetId());
1996+
LogPrint(BCLog::NET, "Peer %d sent us a getblocktxn for a block we don't have", pfrom->GetId());
19941997
return true;
19951998
}
19961999

@@ -2042,7 +2045,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
20422045
pindex = (*mi).second;
20432046

20442047
if (!BlockRequestAllowed(pindex, chainparams.GetConsensus())) {
2045-
LogPrintf("%s: ignoring request from peer=%i for old block header that isn't in the main chain\n", __func__, pfrom->GetId());
2048+
LogPrint(BCLog::NET, "%s: ignoring request from peer=%i for old block header that isn't in the main chain\n", __func__, pfrom->GetId());
20462049
return true;
20472050
}
20482051
}
@@ -2296,10 +2299,12 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
22962299
int nDoS;
22972300
if (state.IsInvalid(nDoS)) {
22982301
if (nDoS > 0) {
2302+
LogPrintf("Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId());
22992303
LOCK(cs_main);
23002304
Misbehaving(pfrom->GetId(), nDoS);
2305+
} else {
2306+
LogPrint(BCLog::NET, "Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId());
23012307
}
2302-
LogPrintf("Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId());
23032308
return true;
23042309
}
23052310
}
@@ -2901,7 +2906,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
29012906
msg.SetVersion(pfrom->GetRecvVersion());
29022907
// Scan for message start
29032908
if (memcmp(msg.hdr.pchMessageStart, chainparams.MessageStart(), CMessageHeader::MESSAGE_START_SIZE) != 0) {
2904-
LogPrintf("PROCESSMESSAGE: INVALID MESSAGESTART %s peer=%d\n", SanitizeString(msg.hdr.GetCommand()), pfrom->GetId());
2909+
LogPrint(BCLog::NET, "PROCESSMESSAGE: INVALID MESSAGESTART %s peer=%d\n", SanitizeString(msg.hdr.GetCommand()), pfrom->GetId());
29052910
pfrom->fDisconnect = true;
29062911
return false;
29072912
}
@@ -2910,7 +2915,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
29102915
CMessageHeader& hdr = msg.hdr;
29112916
if (!hdr.IsValid(chainparams.MessageStart()))
29122917
{
2913-
LogPrintf("PROCESSMESSAGE: ERRORS IN HEADER %s peer=%d\n", SanitizeString(hdr.GetCommand()), pfrom->GetId());
2918+
LogPrint(BCLog::NET, "PROCESSMESSAGE: ERRORS IN HEADER %s peer=%d\n", SanitizeString(hdr.GetCommand()), pfrom->GetId());
29142919
return fMoreWork;
29152920
}
29162921
std::string strCommand = hdr.GetCommand();
@@ -2923,7 +2928,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
29232928
const uint256& hash = msg.GetMessageHash();
29242929
if (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0)
29252930
{
2926-
LogPrintf("%s(%s, %u bytes): CHECKSUM ERROR expected %s was %s\n", __func__,
2931+
LogPrint(BCLog::NET, "%s(%s, %u bytes): CHECKSUM ERROR expected %s was %s\n", __func__,
29272932
SanitizeString(strCommand), nMessageSize,
29282933
HexStr(hash.begin(), hash.begin()+CMessageHeader::CHECKSUM_SIZE),
29292934
HexStr(hdr.pchChecksum, hdr.pchChecksum+CMessageHeader::CHECKSUM_SIZE));
@@ -2946,17 +2951,17 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
29462951
if (strstr(e.what(), "end of data"))
29472952
{
29482953
// Allow exceptions from under-length message on vRecv
2949-
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());
2954+
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());
29502955
}
29512956
else if (strstr(e.what(), "size too large"))
29522957
{
29532958
// Allow exceptions from over-long size
2954-
LogPrintf("%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
2959+
LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
29552960
}
29562961
else if (strstr(e.what(), "non-canonical ReadCompactSize()"))
29572962
{
29582963
// Allow exceptions from non-canonical encoding
2959-
LogPrintf("%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
2964+
LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
29602965
}
29612966
else
29622967
{
@@ -2970,7 +2975,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
29702975
}
29712976

29722977
if (!fRet) {
2973-
LogPrintf("%s(%s, %u bytes) FAILED peer=%d\n", __func__, SanitizeString(strCommand), nMessageSize, pfrom->GetId());
2978+
LogPrint(BCLog::NET, "%s(%s, %u bytes) FAILED peer=%d\n", __func__, SanitizeString(strCommand), nMessageSize, pfrom->GetId());
29742979
}
29752980

29762981
LOCK(cs_main);

‎src/util.h

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

137+
// Be conservative when using LogPrintf/error or other things which
138+
// unconditionally log to debug.log! It should not be the case that an inbound
139+
// peer can fill up a users disk with debug.log entries.
140+
137141
#ifdef USE_COVERAGE
138142
#define LogPrintf(...) do { MarkUsed(__VA_ARGS__); } while(0)
139143
#define LogPrint(category, ...) do { MarkUsed(__VA_ARGS__); } while(0)

0 commit comments

Comments
 (0)