From f2fc08ba498f6e27bcb9053bbb23ee2808f1b7aa Mon Sep 17 00:00:00 2001 From: jolavillette Date: Sat, 18 Jul 2026 18:34:49 +0200 Subject: [PATCH] Reduce log noise: gate startup keyring dump, throttle banned-identity drops Two more high-volume, non-error log sources, both left behaving exactly as before (nothing is hidden): - RNPPGPHandler::initCertificateInfo(): gate the per-key keyring dump (one "type/Key id/fingerprint" line + one "N signers" line per key) behind DEBUG_PGP_KEYRING_DUMP, off by default. With a large keyring this is thousands of lines of pure inventory at every startup. Key parse errors throw, so gating hides nothing; the "Loaded N public keys" summary is still printed unconditionally. - DistributedChatService: a single locally-banned identity can flood a lobby with thousands of items, and the code logged one WARN per dropped item. Every item is still dropped (correct, expected behaviour); logBannedIdentityDrop() now rate-limits the log to one line when the flood starts plus one summary per identity per 60s carrying the dropped count. Signature mismatches and other genuine problems remain logged separately and unconditionally. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/chat/distributedchat.cc | 38 +++++++++++++++++++++++++++++++++++-- src/pgp/rnppgphandler.cc | 10 ++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/chat/distributedchat.cc b/src/chat/distributedchat.cc index e4cb4e0ba..f932ec0f6 100644 --- a/src/chat/distributedchat.cc +++ b/src/chat/distributedchat.cc @@ -62,6 +62,40 @@ static const uint32_t MAX_MESSAGES_PER_SECONDS_PERIOD = 10 ; // duration #define EXTRACT_PRIVACY_FLAGS(flags) (ChatLobbyFlags(flags.toUInt32()) * (RS_CHAT_LOBBY_FLAGS_PUBLIC | RS_CHAT_LOBBY_FLAGS_PGP_SIGNED)) +// A single locally-banned identity can flood a lobby with thousands of items. We still drop every +// one of them (that is the correct, expected behaviour, NOT an error), but one WARN per dropped item +// buries the rest of the log. This rate-limits the logging: one line when the flood from a given +// identity starts, then one summary line per identity per REPORT_INTERVAL carrying the number of +// items dropped in between. No error is hidden: signature mismatches and other genuine problems are +// logged separately and unconditionally. +static void logBannedIdentityDrop(const RsGxsId& keyId) +{ + static const rstime_t REPORT_INTERVAL = 60; // seconds + static RsMutex banned_drop_log_mtx("bannedDropLog"); + static std::map > stats; // id -> (drops since last report, last report time) + + RS_STACK_MUTEX(banned_drop_log_mtx); + + rstime_t now = time(nullptr); + auto it = stats.find(keyId); + + if(it == stats.end()) + { + RsWarn() << "Dropping lobby message(s) from banned identity " << keyId << " (identity is locally banned). Further drops summarized every " << REPORT_INTERVAL << "s." ; + stats[keyId] = std::make_pair((uint32_t)0,now); + return; + } + + ++it->second.first; + + if(now - it->second.second >= REPORT_INTERVAL) + { + RsWarn() << "Dropped " << it->second.first << " further lobby message(s) from banned identity " << keyId << " in the last " << (now - it->second.second) << "s (identity is locally banned)." ; + it->second.first = 0; + it->second.second = now; + } +} + DistributedChatService::DistributedChatService(uint32_t serv_type,p3ServiceControl *sc,p3HistoryMgr *hm, RsGixs *is) : mServType(serv_type),mDistributedChatMtx("Distributed Chat"), mServControl(sc), mHistMgr(hm),mGixs(is) { @@ -140,7 +174,7 @@ bool DistributedChatService::handleRecvChatLobbyMsgItem(RsChatMsgItem *ci) if( rsReputations->overallReputationLevel(cli->signature.keyId) == RsReputationLevel::LOCALLY_NEGATIVE ) { - std::cerr << "(WW) Received lobby msg/item from banned identity " << cli->signature.keyId << ". Dropping it." << std::endl; + logBannedIdentityDrop(cli->signature.keyId); return false ; } if(!checkSignature(cli,cli->PeerId())) // check the object's signature and possibly request missing keys @@ -730,7 +764,7 @@ void DistributedChatService::handleRecvChatLobbyEventItem(RsChatLobbyEventItem * if( rsReputations->overallReputationLevel(item->signature.keyId) == RsReputationLevel::LOCALLY_NEGATIVE ) { - std::cerr << "(WW) Received lobby msg/item from banned identity " << item->signature.keyId << ". Dropping it." << std::endl; + logBannedIdentityDrop(item->signature.keyId); return ; } if(!checkSignature(item,item->PeerId())) // check the object's signature and possibly request missing keys diff --git a/src/pgp/rnppgphandler.cc b/src/pgp/rnppgphandler.cc index 9451bebf7..ddd8dd210 100644 --- a/src/pgp/rnppgphandler.cc +++ b/src/pgp/rnppgphandler.cc @@ -54,6 +54,12 @@ static const uint32_t PGP_CERTIFICATE_LIMIT_MAX_PASSWD_SIZE = 1024 ; //#define DEBUG_PGPHANDLER 1 //#define PGPHANDLER_DSA_SUPPORT +// When set, dumps the full parsed keyring at load time: one "type/Key id/fingerprint" line +// plus one "N signers" line per key. That is ~2 lines per key of pure inventory (thousands of +// lines with a large keyring) and hides no error, since key-parse failures throw. The +// "Loaded N public keys" summary is always printed regardless. Off by default. +//#define DEBUG_PGP_KEYRING_DUMP 1 + #define DEBUG_RNP 1 #define NOT_IMPLEMENTED RsErr() << " function " << __PRETTY_FUNCTION__ << " Not implemented yet." << std::endl; assert(false); return false; @@ -344,7 +350,9 @@ void RNPPGPHandler::initCertificateInfo(const rnp_key_handle_t& key_handle) bool have_secret = false; rnp_key_have_secret(key_handle,&have_secret); +#ifdef DEBUG_PGP_KEYRING_DUMP RsInfo() << (have_secret?" [SECRET]":" ") << " type: " << key_alg << "-" << key_bits << " Key id: " << key_id<< " fingerprint: " << key_fprint << " Username: \"" << key_uid << "\"" ; +#endif auto fill_cert = [key_alg,key_fprint](PGPCertificateInfo& cert,char *key_uid,const std::set& signers) { @@ -375,7 +383,9 @@ void RNPPGPHandler::initCertificateInfo(const rnp_key_handle_t& key_handle) size_t signature_count = 0; rnp_key_get_signature_count(key_handle,&signature_count); +#ifdef DEBUG_PGP_KEYRING_DUMP RsDbg() << "Key " << key_id << " has " << signature_count << " signers." ; +#endif for(size_t i=0;i