Skip to content

Commit

Permalink
Merge #11583: Do not make it trivial for inbound peers to generate lo…
Browse files Browse the repository at this point in the history
…g 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
  • Loading branch information
laanwj committed Dec 11, 2017
2 parents 8ab6c0b + be9f38c commit 37ffa16
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 14 deletions.
2 changes: 1 addition & 1 deletion src/net.cpp
Expand Up @@ -1102,7 +1102,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {

if (IsBanned(addr) && !whitelisted)
{
LogPrintf("connection from %s dropped (banned)\n", addr.ToString());
LogPrint(BCLog::NET, "connection from %s dropped (banned)\n", addr.ToString());
CloseSocket(hSocket);
return;
}
Expand Down
31 changes: 18 additions & 13 deletions src/net_processing.cpp
Expand Up @@ -1078,7 +1078,7 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam
}
send = BlockRequestAllowed(mi->second, consensusParams);
if (!send) {
LogPrintf("%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom->GetId());
LogPrint(BCLog::NET, "%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom->GetId());
}
}
// disconnect node in case we have reached the outbound limit for serving historical blocks
Expand Down Expand Up @@ -1569,7 +1569,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
if (nVersion < MIN_PEER_PROTO_VERSION)
{
// disconnect from peers older than this proto version
LogPrintf("peer=%d using obsolete version %i; disconnecting\n", pfrom->GetId(), nVersion);
LogPrint(BCLog::NET, "peer=%d using obsolete version %i; disconnecting\n", pfrom->GetId(), nVersion);
connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_OBSOLETE,
strprintf("Version must be %d or greater", MIN_PEER_PROTO_VERSION)));
pfrom->fDisconnect = true;
Expand Down Expand Up @@ -1669,7 +1669,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
if (fLogIPs)
remoteAddr = ", peeraddr=" + pfrom->addr.ToString();

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

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

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

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

if (!BlockRequestAllowed(pindex, chainparams.GetConsensus())) {
LogPrintf("%s: ignoring request from peer=%i for old block header that isn't in the main chain\n", __func__, pfrom->GetId());
LogPrint(BCLog::NET, "%s: ignoring request from peer=%i for old block header that isn't in the main chain\n", __func__, pfrom->GetId());
return true;
}
}
Expand Down Expand Up @@ -2296,10 +2299,12 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
int nDoS;
if (state.IsInvalid(nDoS)) {
if (nDoS > 0) {
LogPrintf("Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId());
LOCK(cs_main);
Misbehaving(pfrom->GetId(), nDoS);
} else {
LogPrint(BCLog::NET, "Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId());
}
LogPrintf("Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId());
return true;
}
}
Expand Down Expand Up @@ -2901,7 +2906,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
msg.SetVersion(pfrom->GetRecvVersion());
// Scan for message start
if (memcmp(msg.hdr.pchMessageStart, chainparams.MessageStart(), CMessageHeader::MESSAGE_START_SIZE) != 0) {
LogPrintf("PROCESSMESSAGE: INVALID MESSAGESTART %s peer=%d\n", SanitizeString(msg.hdr.GetCommand()), pfrom->GetId());
LogPrint(BCLog::NET, "PROCESSMESSAGE: INVALID MESSAGESTART %s peer=%d\n", SanitizeString(msg.hdr.GetCommand()), pfrom->GetId());
pfrom->fDisconnect = true;
return false;
}
Expand All @@ -2910,7 +2915,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
CMessageHeader& hdr = msg.hdr;
if (!hdr.IsValid(chainparams.MessageStart()))
{
LogPrintf("PROCESSMESSAGE: ERRORS IN HEADER %s peer=%d\n", SanitizeString(hdr.GetCommand()), pfrom->GetId());
LogPrint(BCLog::NET, "PROCESSMESSAGE: ERRORS IN HEADER %s peer=%d\n", SanitizeString(hdr.GetCommand()), pfrom->GetId());
return fMoreWork;
}
std::string strCommand = hdr.GetCommand();
Expand All @@ -2923,7 +2928,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
const uint256& hash = msg.GetMessageHash();
if (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0)
{
LogPrintf("%s(%s, %u bytes): CHECKSUM ERROR expected %s was %s\n", __func__,
LogPrint(BCLog::NET, "%s(%s, %u bytes): CHECKSUM ERROR expected %s was %s\n", __func__,
SanitizeString(strCommand), nMessageSize,
HexStr(hash.begin(), hash.begin()+CMessageHeader::CHECKSUM_SIZE),
HexStr(hdr.pchChecksum, hdr.pchChecksum+CMessageHeader::CHECKSUM_SIZE));
Expand All @@ -2946,17 +2951,17 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
if (strstr(e.what(), "end of data"))
{
// Allow exceptions from under-length message on vRecv
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());
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());
}
else if (strstr(e.what(), "size too large"))
{
// Allow exceptions from over-long size
LogPrintf("%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
}
else if (strstr(e.what(), "non-canonical ReadCompactSize()"))
{
// Allow exceptions from non-canonical encoding
LogPrintf("%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what());
}
else
{
Expand All @@ -2970,7 +2975,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
}

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

LOCK(cs_main);
Expand Down
4 changes: 4 additions & 0 deletions src/util.h
Expand Up @@ -134,6 +134,10 @@ template<typename T, typename... Args> static inline void MarkUsed(const T& t, c
MarkUsed(args...);
}

// Be conservative when using LogPrintf/error or other things which
// unconditionally log to debug.log! It should not be the case that an inbound
// peer can fill up a users disk with debug.log entries.

#ifdef USE_COVERAGE
#define LogPrintf(...) do { MarkUsed(__VA_ARGS__); } while(0)
#define LogPrint(category, ...) do { MarkUsed(__VA_ARGS__); } while(0)
Expand Down

0 comments on commit 37ffa16

Please sign in to comment.