From: Otto Moerbeek Date: Thu, 27 Nov 2025 08:49:42 +0000 (+0100) Subject: rec: count cumulative answer sizes for a single client query X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=12cc61abd1ef7065e7529c35529a5a14241bd50a;p=thirdparty%2Fpdns.git rec: count cumulative answer sizes for a single client query Signed-off-by: Otto Moerbeek --- diff --git a/pdns/recursordist/lwres.cc b/pdns/recursordist/lwres.cc index c94be0fbcd..f0d1da1438 100644 --- a/pdns/recursordist/lwres.cc +++ b/pdns/recursordist/lwres.cc @@ -789,6 +789,8 @@ static LWResult::Result asyncresolve(const OptLog& log, const ComboAddress& addr return ret; } + lwr->d_bytesReceived = len; + if (*chained) { auto msec = lwr->d_usec / 1000; if (msec > g_networkTimeoutMsec * 2 / 3) { diff --git a/pdns/recursordist/lwres.hh b/pdns/recursordist/lwres.hh index d3b05e062b..465b9735af 100644 --- a/pdns/recursordist/lwres.hh +++ b/pdns/recursordist/lwres.hh @@ -84,6 +84,7 @@ public: vector d_records; uint32_t d_usec{0}; + uint32_t d_bytesReceived{0}; int d_rcode{0}; bool d_validpacket{false}; bool d_aabit{false}, d_tcbit{false}; diff --git a/pdns/recursordist/pdns_recursor.cc b/pdns/recursordist/pdns_recursor.cc index a8b47c07a2..e65289eb2c 100644 --- a/pdns/recursordist/pdns_recursor.cc +++ b/pdns/recursordist/pdns_recursor.cc @@ -1894,6 +1894,7 @@ void startDoResolve(void* arg) // NOLINT(readability-function-cognitive-complexi "answers", Logging::Loggable(ntohs(packetWriter.getHeader()->ancount)), "additional", Logging::Loggable(ntohs(packetWriter.getHeader()->arcount)), "outqueries", Logging::Loggable(resolver.d_outqueries), + "received", Logging::Loggable(resolver.d_bytesReceived), "netms", Logging::Loggable(resolver.d_totUsec / 1000.0), "totms", Logging::Loggable(static_cast(spentUsec) / 1000.0), "throttled", Logging::Loggable(resolver.d_throttledqueries), diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc index 8abf028781..457ab09b2c 100644 --- a/pdns/recursordist/rec-main.cc +++ b/pdns/recursordist/rec-main.cc @@ -1752,6 +1752,7 @@ static int initSyncRes(Logr::log_t log) SyncRes::s_serverID = ::arg()["server-id"]; // This bound is dynamically adjusted in SyncRes, depending on qname minimization being active SyncRes::s_maxqperq = ::arg().asNum("max-qperq"); + SyncRes::s_maxbytesperq = ::arg().asNum("max-bytesperq"); SyncRes::s_maxnsperresolve = ::arg().asNum("max-ns-per-resolve"); SyncRes::s_maxnsaddressqperq = ::arg().asNum("max-ns-address-qperq"); SyncRes::s_maxtotusec = 1000 * ::arg().asNum("max-total-msec"); diff --git a/pdns/recursordist/rec-rust-lib/table.py b/pdns/recursordist/rec-rust-lib/table.py index 2a0e63cb2f..c114135ff0 100644 --- a/pdns/recursordist/rec-rust-lib/table.py +++ b/pdns/recursordist/rec-rust-lib/table.py @@ -1601,13 +1601,25 @@ Maximum number of Packet Cache entries. Sharded and shared by all threads since 'section' : 'outgoing', 'type' : LType.Uint64, 'default' : '50', - 'help' : 'Maximum outgoing queries per query', + 'help' : 'Maximum outgoing queries per client query', 'doc' : ''' The maximum number of outgoing queries that will be sent out during the resolution of a single client query. This is used to avoid cycles resolving names. ''', 'versionchanged': ('5.1.0', 'The default used to be 60, with an extra allowance if qname minimization was enabled. Having better algorithms allows for a lower default limit.'), }, + { + 'name' : 'max_bytesperq', + 'section' : 'outgoing', + 'type' : LType.Uint64, + 'default' : '100000', + 'help' : 'Maximum number of received bytes per client query', + 'doc' : ''' +The maximum number of cumulative bytes that will be accepted during the resolution of a single client query. +This is useful to limit amplification attacks. + ''', + 'versionadded': '5.4.0', + }, { 'name' : 'max_cnames_followed', 'section' : 'recursor', diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc index ed6df9974e..e4ea06d98d 100644 --- a/pdns/recursordist/syncres.cc +++ b/pdns/recursordist/syncres.cc @@ -351,6 +351,7 @@ unsigned int SyncRes::s_maxnegttl; unsigned int SyncRes::s_maxbogusttl; unsigned int SyncRes::s_maxcachettl; unsigned int SyncRes::s_maxqperq; +unsigned int SyncRes::s_maxbytesperq; unsigned int SyncRes::s_maxnsperresolve; unsigned int SyncRes::s_maxnsaddressqperq; unsigned int SyncRes::s_maxtotusec; @@ -461,7 +462,7 @@ static inline void accountAuthLatency(uint64_t usec, int family) } SyncRes::SyncRes(const struct timeval& now) : - d_authzonequeries(0), d_outqueries(0), d_tcpoutqueries(0), d_dotoutqueries(0), d_throttledqueries(0), d_timeouts(0), d_unreachables(0), d_totUsec(0), d_fixednow(now), d_now(now), d_cacheonly(false), d_doDNSSEC(false), d_doEDNS0(false), d_qNameMinimization(s_qnameminimization), d_lm(s_lm) + d_authzonequeries(0), d_outqueries(0), d_tcpoutqueries(0), d_dotoutqueries(0), d_throttledqueries(0), d_timeouts(0), d_unreachables(0), d_bytesReceived(0), d_totUsec(0), d_fixednow(now), d_now(now), d_cacheonly(false), d_doDNSSEC(false), d_doEDNS0(false), d_qNameMinimization(s_qnameminimization), d_lm(s_lm) { d_validationContext.d_nsec3IterationsRemainingQuota = s_maxnsec3iterationsperq > 0 ? s_maxnsec3iterationsperq : std::numeric_limits::max(); } @@ -3493,7 +3494,10 @@ vector SyncRes::retrieveAddressesForNS(const std::string& prefix, void SyncRes::checkMaxQperQ(const DNSName& qname) const { if (d_outqueries + d_throttledqueries > s_maxqperq) { - throw ImmediateServFailException("more than " + std::to_string(s_maxqperq) + " (max-qperq) queries sent or throttled while resolving " + qname.toLogString()); + throw ImmediateServFailException("More than " + std::to_string(s_maxqperq) + " (outgoing.max_qperq) queries sent or throttled while resolving " + qname.toLogString()); + } + if (d_bytesReceived > s_maxbytesperq) { + throw ImmediateServFailException("More than " + std::to_string(s_maxbytesperq) + " (outgoing.max_bytesperq) bytes received while resolving " + qname.toLogString()); } } @@ -5500,6 +5504,7 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname, throw ImmediateServFailException("Query killed by policy"); } + d_bytesReceived += lwr.d_bytesReceived; d_totUsec += lwr.d_usec; if (resolveret == LWResult::Result::Spoofed || resolveret == LWResult::Result::BadCookie) { diff --git a/pdns/recursordist/syncres.hh b/pdns/recursordist/syncres.hh index 870bd946d4..aa905b2ea1 100644 --- a/pdns/recursordist/syncres.hh +++ b/pdns/recursordist/syncres.hh @@ -529,6 +529,7 @@ public: static unsigned int s_minimumTTL; static unsigned int s_minimumECSTTL; static unsigned int s_maxqperq; + static unsigned int s_maxbytesperq; static unsigned int s_maxnsperresolve; static unsigned int s_maxnsaddressqperq; static unsigned int s_maxtotusec; @@ -604,9 +605,10 @@ public: unsigned int d_throttledqueries; unsigned int d_timeouts; unsigned int d_unreachables; + unsigned int d_bytesReceived; unsigned int d_totUsec; unsigned int d_maxdepth{0}; - // Initialized ony once, as opposed to d_now which gets updated after outgoing requests + // Initialized only once, as opposed to d_now which gets updated after outgoing requests struct timeval d_fixednow; private: diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc index c5565ff8ad..27e652766e 100644 --- a/pdns/recursordist/test-syncres_cc.cc +++ b/pdns/recursordist/test-syncres_cc.cc @@ -151,6 +151,7 @@ void initSR(bool debug) SyncRes::s_maxqperq = 50; SyncRes::s_maxnsaddressqperq = 10; + SyncRes::s_maxbytesperq = 100000; SyncRes::s_maxtotusec = 1000 * 7000; SyncRes::s_maxdepth = 40; SyncRes::s_maxnegttl = 3600; diff --git a/pdns/recursordist/test-syncres_cc1.cc b/pdns/recursordist/test-syncres_cc1.cc index ec62d84ff0..bca758dcfc 100644 --- a/pdns/recursordist/test-syncres_cc1.cc +++ b/pdns/recursordist/test-syncres_cc1.cc @@ -358,6 +358,50 @@ BOOST_AUTO_TEST_CASE(test_edns_formerr_but_edns_enabled) } } +BOOST_AUTO_TEST_CASE(test_edns_formerr_but_edns_enabled_limit_bytes) +{ + std::unique_ptr sr; + initSR(sr); + + /* in this test, the auth answers with FormErr to an EDNS-enabled + query, but the response does contain EDNS so we should not mark + it as EDNS ignorant or intolerant. + + We are MISUING this test to test max_bytesperq limit + */ + size_t queriesWithEDNS = 0; + size_t queriesWithoutEDNS = 0; + std::set usedServers; + + sr->setAsyncCallback([&](const ComboAddress& address, const DNSName& /* domain */, int type, bool /* doTCP */, bool /* sendRDQuery */, int EDNS0Level, struct timeval* /* now */, std::optional& /* srcmask */, const ResolveContext& /* context */, LWResult* res, bool* /* chained */) { + if (EDNS0Level > 0) { + queriesWithEDNS++; + } + else { + queriesWithoutEDNS++; + } + usedServers.insert(address); + + if (type == QType::DNAME) { + setLWResult(res, RCode::FormErr); + if (EDNS0Level > 0) { + res->d_haveEDNS = true; + } + res->d_bytesReceived = 10000; + return LWResult::Result::Success; + } + + return LWResult::Result::Timeout; + }); + + primeHints(); + + vector ret; + BOOST_CHECK_EXCEPTION(sr->beginResolve(DNSName("powerdns.com."), QType(QType::DNAME), QClass::IN, ret), ImmediateServFailException, [&](const ImmediateServFailException& isfe) { + return isfe.reason.substr(0, 9) == "More than"; + }); +} + BOOST_AUTO_TEST_CASE(test_meta_types) { std::unique_ptr sr;