Skip to content

Commit be9f38c

Browse files
TheBlueMattTheBlueMatt
committed
Do not make it trivial for inbound peers to generate log entries
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). To avoid removing logs for outbound peers, a new log is added to notify users when a new outbound peer is connected which mimics the version print.
1 parent 3788a84 commit be9f38c

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

11121112
if (IsBanned(addr) && !whitelisted)
11131113
{
1114-
LogPrintf("connection from %s dropped (banned)\n", addr.ToString());
1114+
LogPrint(BCLog::NET, "connection from %s dropped (banned)\n", addr.ToString());
11151115
CloseSocket(hSocket);
11161116
return;
11171117
}

‎src/net_processing.cpp

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,7 +1042,7 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam
10421042
}
10431043
send = BlockRequestAllowed(mi->second, consensusParams);
10441044
if (!send) {
1045-
LogPrintf("%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom->GetId());
1045+
LogPrint(BCLog::NET, "%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom->GetId());
10461046
}
10471047
}
10481048
// disconnect node in case we have reached the outbound limit for serving historical blocks
@@ -1511,7 +1511,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
15111511
if (nVersion < MIN_PEER_PROTO_VERSION)
15121512
{
15131513
// disconnect from peers older than this proto version
1514-
LogPrintf("peer=%d using obsolete version %i; disconnecting\n", pfrom->GetId(), nVersion);
1514+
LogPrint(BCLog::NET, "peer=%d using obsolete version %i; disconnecting\n", pfrom->GetId(), nVersion);
15151515
connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_OBSOLETE,
15161516
strprintf("Version must be %d or greater", MIN_PEER_PROTO_VERSION)));
15171517
pfrom->fDisconnect = true;
@@ -1611,7 +1611,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
16111611
if (fLogIPs)
16121612
remoteAddr = ", peeraddr=" + pfrom->addr.ToString();
16131613

1614-
LogPrintf("receive version message: %s: version %d, blocks=%d, us=%s, peer=%d%s\n",
1614+
LogPrint(BCLog::NET, "receive version message: %s: version %d, blocks=%d, us=%s, peer=%d%s\n",
16151615
cleanSubVer, pfrom->nVersion,
16161616
pfrom->nStartingHeight, addrMe.ToString(), pfrom->GetId(),
16171617
remoteAddr);
@@ -1654,6 +1654,9 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
16541654
// Mark this node as currently connected, so we update its timestamp later.
16551655
LOCK(cs_main);
16561656
State(pfrom->GetId())->fCurrentlyConnected = true;
1657+
LogPrintf("New outbound peer connected: version: %d, blocks=%d, peer=%d%s\n",
1658+
pfrom->nVersion.load(), pfrom->nStartingHeight, pfrom->GetId(),
1659+
(fLogIPs ? strprintf(", peeraddr=%s", pfrom->addr.ToString()) : ""));
16571660
}
16581661

16591662
if (pfrom->nVersion >= SENDHEADERS_VERSION) {
@@ -1932,7 +1935,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
19321935

19331936
BlockMap::iterator it = mapBlockIndex.find(req.blockhash);
19341937
if (it == mapBlockIndex.end() || !(it->second->nStatus & BLOCK_HAVE_DATA)) {
1935-
LogPrintf("Peer %d sent us a getblocktxn for a block we don't have", pfrom->GetId());
1938+
LogPrint(BCLog::NET, "Peer %d sent us a getblocktxn for a block we don't have", pfrom->GetId());
19361939
return true;
19371940
}
19381941

@@ -1984,7 +1987,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
19841987
pindex = (*mi).second;
19851988

19861989
if (!BlockRequestAllowed(pindex, chainparams.GetConsensus())) {
1987-
LogPrintf("%s: ignoring request from peer=%i for old block header that isn't in the main chain\n", __func__, pfrom->GetId());
1990+
LogPrint(BCLog::NET, "%s: ignoring request from peer=%i for old block header that isn't in the main chain\n", __func__, pfrom->GetId());
19881991
return true;
19891992
}
19901993
}
@@ -2232,10 +2235,12 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
22322235
int nDoS;
22332236
if (state.IsInvalid(nDoS)) {
22342237
if (nDoS > 0) {
2238+
LogPrintf("Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId());
22352239
LOCK(cs_main);
22362240
Misbehaving(pfrom->GetId(), nDoS);
2241+
} else {
2242+
LogPrint(BCLog::NET, "Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId());
22372243
}
2238-
LogPrintf("Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId());
22392244
return true;
22402245
}
22412246
}
@@ -2837,7 +2842,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
28372842
msg.SetVersion(pfrom->GetRecvVersion());
28382843
// Scan for message start
28392844
if (memcmp(msg.hdr.pchMessageStart, chainparams.MessageStart(), CMessageHeader::MESSAGE_START_SIZE) != 0) {
2840-
LogPrintf("PROCESSMESSAGE: INVALID MESSAGESTART %s peer=%d\n", SanitizeString(msg.hdr.GetCommand()), pfrom->GetId());
2845+
LogPrint(BCLog::NET, "PROCESSMESSAGE: INVALID MESSAGESTART %s peer=%d\n", SanitizeString(msg.hdr.GetCommand()), pfrom->GetId());
28412846
pfrom->fDisconnect = true;
28422847
return false;
28432848
}
@@ -2846,7 +2851,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
28462851
CMessageHeader& hdr = msg.hdr;
28472852
if (!hdr.IsValid(chainparams.MessageStart()))
28482853
{
2849-
LogPrintf("PROCESSMESSAGE: ERRORS IN HEADER %s peer=%d\n", SanitizeString(hdr.GetCommand()), pfrom->GetId());
2854+
LogPrint(BCLog::NET, "PROCESSMESSAGE: ERRORS IN HEADER %s peer=%d\n", SanitizeString(hdr.GetCommand()), pfrom->GetId());
28502855
return fMoreWork;
28512856
}
28522857
std::string strCommand = hdr.GetCommand();
@@ -2859,7 +2864,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
28592864
const uint256& hash = msg.GetMessageHash();
28602865
if (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0)
28612866
{
2862-
LogPrintf("%s(%s, %u bytes): CHECKSUM ERROR expected %s was %s\n", __func__,
2867+
LogPrint(BCLog::NET, "%s(%s, %u bytes): CHECKSUM ERROR expected %s was %s\n", __func__,
28632868
SanitizeString(strCommand), nMessageSize,
28642869
HexStr(hash.begin(), hash.begin()+CMessageHeader::CHECKSUM_SIZE),
28652870
HexStr(hdr.pchChecksum, hdr.pchChecksum+CMessageHeader::CHECKSUM_SIZE));
@@ -2882,17 +2887,17 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
28822887
if (strstr(e.what(), "end of data"))
28832888
{
28842889
// Allow exceptions from under-length message on vRecv
2885-
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());
2890+
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());
28862891
}
28872892
else if (strstr(e.what(), "size too large"))
28882893
{
28892894
// Allow exceptions from over-long size
2890-
LogPrintf("%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
2895+
LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
28912896
}
28922897
else if (strstr(e.what(), "non-canonical ReadCompactSize()"))
28932898
{
28942899
// Allow exceptions from non-canonical encoding
2895-
LogPrintf("%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
2900+
LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
28962901
}
28972902
else
28982903
{
@@ -2906,7 +2911,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
29062911
}
29072912

29082913
if (!fRet) {
2909-
LogPrintf("%s(%s, %u bytes) FAILED peer=%d\n", __func__, SanitizeString(strCommand), nMessageSize, pfrom->GetId());
2914+
LogPrint(BCLog::NET, "%s(%s, %u bytes) FAILED peer=%d\n", __func__, SanitizeString(strCommand), nMessageSize, pfrom->GetId());
29102915
}
29112916

29122917
LOCK(cs_main);

‎src/util.h

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

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

0 commit comments

Comments
 (0)