From: Otto Moerbeek Date: Fri, 31 Mar 2023 08:02:31 +0000 (+0200) Subject: More fine grained capping of packet cache TTL X-Git-Tag: rec-4.9.0-alpha1~10^2~1 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=915059183767d9f6db36ed82387bca7b8ab949e4;p=thirdparty%2Fpdns.git More fine grained capping of packet cache TTL Distinguish between negative answers (NxDomain and NoData), failure to resolve (ServFail and completely empty answers) and ordinary answers when capping the packet cache TTL. A new setting (packetcache-negative-ttl) is created for that. NoData is: empty answer section but SOA record present in authority section. --- diff --git a/pdns/recursordist/docs/settings.rst b/pdns/recursordist/docs/settings.rst index bf7b41f6ec..4572674446 100644 --- a/pdns/recursordist/docs/settings.rst +++ b/pdns/recursordist/docs/settings.rst @@ -1586,6 +1586,19 @@ If an answer containing an NSEC3 record with more iterations is received, its DN Maximum number of seconds to cache an item in the packet cache, no matter what the original TTL specified. +.. _setting-packetcache-negative-ttl: + +``packetcache-negative-ttl`` +---------------------------- +.. versionadded:: 4.9.0 + +- Integer +- Default: 60 + +Maximum number of seconds to cache an ``NxDomain`` or ``NoData`` answer in the packetcache. +This setting's maximum is capped to `packetcache-ttl`_. +i.e. setting ``packetcache-ttl=15`` and keeping ``packetcache-negative-ttl`` at the default will lower ``packetcache-negative-ttl`` to ``15``. + .. _setting-packetcache-servfail-ttl: ``packetcache-servfail-ttl`` diff --git a/pdns/recursordist/pdns_recursor.cc b/pdns/recursordist/pdns_recursor.cc index 1c6ddf5ff2..69c70f5d7e 100644 --- a/pdns/recursordist/pdns_recursor.cc +++ b/pdns/recursordist/pdns_recursor.cc @@ -420,10 +420,14 @@ static void handleRPZCustom(const DNSRecord& spoofed, const QType& qtype, SyncRe } } -static bool addRecordToPacket(DNSPacketWriter& pw, const DNSRecord& rec, uint32_t& minTTL, uint32_t ttlCap, const uint16_t maxAnswerSize) +static bool addRecordToPacket(DNSPacketWriter& pw, const DNSRecord& rec, uint32_t& minTTL, uint32_t ttlCap, const uint16_t maxAnswerSize, bool& seenAuthSOA) { pw.startRecord(rec.d_name, rec.d_type, (rec.d_ttl > ttlCap ? ttlCap : rec.d_ttl), rec.d_class, rec.d_place); + if (rec.d_type == QType::SOA && rec.d_place == DNSResourceRecord::AUTHORITY) { + seenAuthSOA = true; + } + if (rec.d_type != QType::OPT) // their TTL ain't real minTTL = min(minTTL, rec.d_ttl); @@ -875,6 +879,20 @@ static void dumpTrace(const string& trace, const timeval& timev) // fclose by unique_ptr does implicit flush } +static uint32_t capPacketCacheTTL(const struct dnsheader& hdr, uint32_t ttl, bool seenAuthSOA) +{ + if (hdr.rcode == RCode::NXDomain || (hdr.rcode == RCode::NoError && hdr.ancount == 0 && seenAuthSOA)) { + ttl = std::min(ttl, SyncRes::s_packetcachenegativettl); + } + else if ((hdr.rcode != RCode::NoError && hdr.rcode != RCode::NXDomain) || (hdr.ancount == 0 && hdr.nscount == 0)) { + ttl = min(ttl, SyncRes::s_packetcacheservfailttl); + } + else { + ttl = std::min(ttl, SyncRes::s_packetcachettl); + } + return ttl; +} + void startDoResolve(void* p) { auto dc = std::unique_ptr(reinterpret_cast(p)); @@ -984,6 +1002,7 @@ void startDoResolve(void* p) If we have a TTL cap, this value can't be larger than the cap no matter what. */ uint32_t minTTL = dc->d_ttlCap; + bool seenAuthSOA = false; sr.d_eventTrace = std::move(dc->d_eventTrace); sr.setId(MT->getTid()); @@ -1413,7 +1432,7 @@ void startDoResolve(void* p) continue; } - if (!addRecordToPacket(pw, *i, minTTL, dc->d_ttlCap, maxanswersize)) { + if (!addRecordToPacket(pw, *i, minTTL, dc->d_ttlCap, maxanswersize, seenAuthSOA)) { needCommit = false; break; } @@ -1655,11 +1674,7 @@ void startDoResolve(void* p) } if (t_packetCache && !variableAnswer && !sr.wasVariable()) { - const auto& hdr = pw.getHeader(); - if ((hdr->rcode != RCode::NoError && hdr->rcode != RCode::NXDomain) || (hdr->ancount == 0 && hdr->nscount == 0)) { - minTTL = min(minTTL, SyncRes::s_packetcacheservfailttl); - } - minTTL = min(minTTL, SyncRes::s_packetcachettl); + minTTL = capPacketCacheTTL(*pw.getHeader(), minTTL, seenAuthSOA); t_packetCache->insertResponsePacket(dc->d_tag, dc->d_qhash, std::move(dc->d_query), dc->d_mdp.d_qname, dc->d_mdp.d_qtype, dc->d_mdp.d_qclass, string((const char*)&*packet.begin(), packet.size()), diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc index 82365a9135..4ea5166d23 100644 --- a/pdns/recursordist/rec-main.cc +++ b/pdns/recursordist/rec-main.cc @@ -1510,10 +1510,12 @@ static int serviceMain(int argc, char* argv[], Logr::log_t log) SyncRes::s_maxnegttl = ::arg().asNum("max-negative-ttl"); SyncRes::s_maxbogusttl = ::arg().asNum("max-cache-bogus-ttl"); SyncRes::s_maxcachettl = max(::arg().asNum("max-cache-ttl"), 15); + SyncRes::s_packetcachettl = ::arg().asNum("packetcache-ttl"); - // Cap the packetcache-servfail-ttl to the packetcache-ttl - uint32_t packetCacheServFailTTL = ::arg().asNum("packetcache-servfail-ttl"); - SyncRes::s_packetcacheservfailttl = (packetCacheServFailTTL > SyncRes::s_packetcachettl) ? SyncRes::s_packetcachettl : packetCacheServFailTTL; + // Cap the packetcache-servfail-ttl and packetcache-negative-ttl to packetcache-ttl + SyncRes::s_packetcacheservfailttl = std::min(static_cast(::arg().asNum("packetcache-servfail-ttl")), SyncRes::s_packetcachettl); + SyncRes::s_packetcachenegativettl = std::min(static_cast(::arg().asNum("packetcache-negative-ttl")), SyncRes::s_packetcachettl); + SyncRes::s_serverdownmaxfails = ::arg().asNum("server-down-max-fails"); SyncRes::s_serverdownthrottletime = ::arg().asNum("server-down-throttle-time"); SyncRes::s_nonresolvingnsmaxfails = ::arg().asNum("non-resolving-ns-max-fails"); @@ -2695,6 +2697,7 @@ int main(int argc, char** argv) ::arg().set("packetcache-ttl", "maximum number of seconds to keep a cached entry in packetcache") = "3600"; ::arg().set("max-packetcache-entries", "maximum number of entries to keep in the packetcache") = "500000"; ::arg().set("packetcache-servfail-ttl", "maximum number of seconds to keep a cached servfail entry in packetcache") = "60"; + ::arg().set("packetcache-negative-ttl", "maximum number of seconds to keep a cached NxDomain or NoData entry in packetcache") = "60"; ::arg().set("server-id", "Returned when queried for 'id.server' TXT or NSID, defaults to hostname, set custom or 'disabled'") = ""; ::arg().set("stats-ringbuffer-entries", "maximum number of packets to store statistics for") = "10000"; ::arg().set("version-string", "string reported on version.pdns or version.bind") = fullVersionString(); diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc index 88b5aaaaae..31654b06ef 100644 --- a/pdns/recursordist/syncres.cc +++ b/pdns/recursordist/syncres.cc @@ -436,6 +436,7 @@ unsigned int SyncRes::s_minimumTTL; unsigned int SyncRes::s_minimumECSTTL; unsigned int SyncRes::s_packetcachettl; unsigned int SyncRes::s_packetcacheservfailttl; +unsigned int SyncRes::s_packetcachenegativettl; unsigned int SyncRes::s_serverdownmaxfails; unsigned int SyncRes::s_serverdownthrottletime; unsigned int SyncRes::s_nonresolvingnsmaxfails; diff --git a/pdns/recursordist/syncres.hh b/pdns/recursordist/syncres.hh index 74693009a7..496ea28efe 100644 --- a/pdns/recursordist/syncres.hh +++ b/pdns/recursordist/syncres.hh @@ -518,6 +518,7 @@ public: static unsigned int s_maxcachettl; static unsigned int s_packetcachettl; static unsigned int s_packetcacheservfailttl; + static unsigned int s_packetcachenegativettl; static unsigned int s_serverdownmaxfails; static unsigned int s_serverdownthrottletime; static unsigned int s_nonresolvingnsmaxfails; diff --git a/regression-tests.recursor-dnssec/test_PacketCache.py b/regression-tests.recursor-dnssec/test_PacketCache.py index f797822d8a..80b015f6f9 100644 --- a/regression-tests.recursor-dnssec/test_PacketCache.py +++ b/regression-tests.recursor-dnssec/test_PacketCache.py @@ -21,6 +21,7 @@ class PacketCacheRecursorTest(RecursorTest): _apiKey = 'secretapikey' _config_template = """ packetcache-ttl=10 + packetcache-negative-ttl=8 packetcache-servfail-ttl=5 auth-zones=example=configs/%s/example.zone webserver=yes @@ -141,13 +142,13 @@ f 3600 IN CNAME f ; CNAME loop: dirty trick to get a ServFail in an a self.assertRRsetInAnswer(res, expected) self.checkPacketCacheMetrics(6, 4) - # NXDomain should get default packetcache TTL (10) + # NXDomain should get negative packetcache TTL (8) query = dns.message.make_query('nxdomain.example.', 'A', want_dnssec=True) res = self.sendUDPQuery(query) self.assertRcodeEqual(res, dns.rcode.NXDOMAIN) self.checkPacketCacheMetrics(6, 5) - # NoData should get default packetcache TTL (10) + # NoData should get negative packetcache TTL (8) query = dns.message.make_query('a.example.', 'AAAA', want_dnssec=True) res = self.sendUDPQuery(query) self.assertRcodeEqual(res, dns.rcode.NOERROR) @@ -166,8 +167,8 @@ f 3600 IN CNAME f ; CNAME loop: dirty trick to get a ServFail in an a try: ret = subprocess.check_output(rec_controlCmd, stderr=subprocess.STDOUT) self.assertTrue((b"a.example. 10 A ; tag 0 udp\n" in ret) or (b"a.example. 9 A ; tag 0 udp\n" in ret)) - self.assertTrue((b"nxdomain.example. 10 A ; tag 0 udp\n" in ret) or (b"nxdomain.example. 9 A ; tag 0 udp\n" in ret)) - self.assertTrue((b"a.example. 10 AAAA ; tag 0 udp\n" in ret) or (b"a.example. 9 AAAA ; tag 0 udp\n" in ret)) + self.assertTrue((b"nxdomain.example. 8 A ; tag 0 udp\n" in ret) or (b"nxdomain.example. 7 A ; tag 0 udp\n" in ret)) + self.assertTrue((b"a.example. 8 AAAA ; tag 0 udp\n" in ret) or (b"a.example. 7 AAAA ; tag 0 udp\n" in ret)) self.assertTrue((b"f.example. 5 A ; tag 0 udp\n" in ret) or (b"f.example. 4 A ; tag 0 udp\n" in ret)) except subprocess.CalledProcessError as e: