From: Remi Gacogne Date: Tue, 5 Feb 2019 15:53:12 +0000 (+0100) Subject: rec: Add a new 'max-cache-bogus-ttl' option X-Git-Tag: auth-4.2.0-beta1~12^2~2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b9473937476ad1c6f78f4ca12a5882b5817deae7;p=pdns rec: Add a new 'max-cache-bogus-ttl' option --- diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc index f4842e9e7..a95787e6f 100644 --- a/pdns/pdns_recursor.cc +++ b/pdns/pdns_recursor.cc @@ -3634,6 +3634,7 @@ static int serviceMain(int argc, char*argv[]) SyncRes::s_nopacketcache = ::arg().mustDo("disable-packetcache"); 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 @@ -4222,6 +4223,7 @@ int main(int argc, char **argv) ::arg().set("hint-file", "If set, load root hints from this file")=""; ::arg().set("max-cache-entries", "If set, maximum number of entries in the main cache")="1000000"; ::arg().set("max-negative-ttl", "maximum number of seconds to keep a negative cached entry in memory")="3600"; + ::arg().set("max-cache-bogus-ttl", "maximum number of seconds to keep a Bogus (positive or negative) cached entry in memory")="3600"; ::arg().set("max-cache-ttl", "maximum number of seconds to keep a cached entry in memory")="86400"; ::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"; diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc index 7dea10c39..ee963cf70 100644 --- a/pdns/recursor_cache.cc +++ b/pdns/recursor_cache.cc @@ -404,7 +404,7 @@ bool MemRecursorCache::doAgeCache(time_t now, const DNSName& name, uint16_t qtyp return false; } -bool MemRecursorCache::updateValidationStatus(time_t now, const DNSName &qname, const QType& qt, const ComboAddress& who, bool requireAuth, vState newState) +bool MemRecursorCache::updateValidationStatus(time_t now, const DNSName &qname, const QType& qt, const ComboAddress& who, bool requireAuth, vState newState, boost::optional capTTD) { bool updated = false; uint16_t qtype = qt.getCode(); @@ -415,6 +415,9 @@ bool MemRecursorCache::updateValidationStatus(time_t now, const DNSName &qname, } entry->d_state = newState; + if (capTTD) { + entry->d_ttd = std::min(entry->d_ttd, *capTTD); + } return true; } @@ -427,6 +430,9 @@ bool MemRecursorCache::updateValidationStatus(time_t now, const DNSName &qname, continue; i->d_state = newState; + if (capTTD) { + i->d_ttd = std::min(i->d_ttd, *capTTD); + } updated = true; if(qtype != QType::ANY && qtype != QType::ADDR) // normally if we have a hit, we are done diff --git a/pdns/recursor_cache.hh b/pdns/recursor_cache.hh index 9b4f3f6e0..e9a44c86c 100644 --- a/pdns/recursor_cache.hh +++ b/pdns/recursor_cache.hh @@ -64,7 +64,7 @@ public: int doWipeCache(const DNSName& name, bool sub, uint16_t qtype=0xffff); bool doAgeCache(time_t now, const DNSName& name, uint16_t qtype, uint32_t newTTL); - bool updateValidationStatus(time_t now, const DNSName &qname, const QType& qt, const ComboAddress& who, bool requireAuth, vState newState); + bool updateValidationStatus(time_t now, const DNSName &qname, const QType& qt, const ComboAddress& who, bool requireAuth, vState newState, boost::optional capTTD); uint64_t cacheHits, cacheMisses; @@ -89,7 +89,7 @@ private: DNSName d_qname; Netmask d_netmask; mutable vState d_state; - time_t d_ttd; + mutable time_t d_ttd; uint16_t d_qtype; bool d_auth; }; diff --git a/pdns/recursordist/docs/settings.rst b/pdns/recursordist/docs/settings.rst index 5d44334fd..2e6200539 100644 --- a/pdns/recursordist/docs/settings.rst +++ b/pdns/recursordist/docs/settings.rst @@ -712,6 +712,17 @@ Path to a lua file to manipulate the Recursor's answers. See :doc:`lua-scripting The interval between calls to the Lua user defined `maintenance()` function in seconds. See :ref:`hooks-maintenance-callback` +.. _setting-max-cache-bogus-ttl: + +``max-cache-bogus-ttl`` +----------------------- +.. versionadded:: 4.2.0 + +- Integer +- Default: 3600 + +Maximum number of seconds to cache an item in the DNS cache (negative or positive) if its DNSSEC validation failed, no matter what the original TTL specified, to reduce the impact of a broken domain. + .. _setting-max-cache-entries: ``max-cache-entries`` diff --git a/pdns/recursordist/negcache.cc b/pdns/recursordist/negcache.cc index 6552486b7..0f5325c33 100644 --- a/pdns/recursordist/negcache.cc +++ b/pdns/recursordist/negcache.cc @@ -111,11 +111,14 @@ void NegCache::add(const NegCacheEntry& ne) { * \param qtype The type of the entry to replace * \param newState The new validation state */ -void NegCache::updateValidationStatus(const DNSName& qname, const QType& qtype, const vState newState) { +void NegCache::updateValidationStatus(const DNSName& qname, const QType& qtype, const vState newState, boost::optional capTTD) { auto range = d_negcache.equal_range(tie(qname, qtype)); if (range.first != range.second) { range.first->d_validationState = newState; + if (capTTD) { + range.first->d_ttd = std::min(range.first->d_ttd, *capTTD); + } } } diff --git a/pdns/recursordist/negcache.hh b/pdns/recursordist/negcache.hh index b78951e21..63eae6305 100644 --- a/pdns/recursordist/negcache.hh +++ b/pdns/recursordist/negcache.hh @@ -49,7 +49,7 @@ class NegCache : public boost::noncopyable { DNSName d_name; // The denied name QType d_qtype; // The denied type DNSName d_auth; // The denying name (aka auth) - uint32_t d_ttd; // Timestamp when this entry should die + mutable uint32_t d_ttd; // Timestamp when this entry should die recordsAndSignatures authoritySOA; // The upstream SOA record and RRSIGs recordsAndSignatures DNSSECRecords; // The upstream NSEC(3) and RRSIGs mutable vState d_validationState{Indeterminate}; @@ -59,7 +59,7 @@ class NegCache : public boost::noncopyable { }; void add(const NegCacheEntry& ne); - void updateValidationStatus(const DNSName& qname, const QType& qtype, const vState newState); + void updateValidationStatus(const DNSName& qname, const QType& qtype, const vState newState, boost::optional capTTD); bool get(const DNSName& qname, const QType& qtype, const struct timeval& now, const NegCacheEntry** ne, bool typeMustMatch=false); bool getRootNXTrust(const DNSName& qname, const struct timeval& now, const NegCacheEntry** ne); uint64_t count(const DNSName& qname) const; diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc index eb38f362c..d65d623b2 100644 --- a/pdns/recursordist/test-syncres_cc.cc +++ b/pdns/recursordist/test-syncres_cc.cc @@ -121,6 +121,7 @@ static void init(bool debug=false) SyncRes::s_maxtotusec = 1000*7000; SyncRes::s_maxdepth = 40; SyncRes::s_maxnegttl = 3600; + SyncRes::s_maxbogusttl = 3600; SyncRes::s_maxcachettl = 86400; SyncRes::s_packetcachettl = 3600; SyncRes::s_packetcacheservfailttl = 60; @@ -1852,6 +1853,8 @@ BOOST_AUTO_TEST_CASE(test_root_nx_trust) { return 0; }); + SyncRes::s_maxnegttl = 3600; + vector ret; int res = sr->beginResolve(target1, QType(QType::A), QClass::IN, ret); BOOST_CHECK_EQUAL(res, RCode::NXDomain); @@ -1862,7 +1865,8 @@ BOOST_AUTO_TEST_CASE(test_root_nx_trust) { ret.clear(); res = sr->beginResolve(target2, QType(QType::A), QClass::IN, ret); BOOST_CHECK_EQUAL(res, RCode::NXDomain); - BOOST_CHECK_EQUAL(ret.size(), 1); + BOOST_REQUIRE_EQUAL(ret.size(), 1); + BOOST_CHECK_LE(ret[0].d_ttl, SyncRes::s_maxnegttl); /* one for target1 and one for the entire TLD */ BOOST_CHECK_EQUAL(SyncRes::getNegCacheSize(), 2); @@ -1926,8 +1930,8 @@ BOOST_AUTO_TEST_CASE(test_root_nx_trust_specific) { res = sr->beginResolve(target2, QType(QType::A), QClass::IN, ret); BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_REQUIRE_EQUAL(ret.size(), 1); - BOOST_REQUIRE(ret[0].d_type == QType::A); BOOST_CHECK_EQUAL(ret[0].d_name, target2); + BOOST_REQUIRE(ret[0].d_type == QType::A); BOOST_CHECK(getRR(ret[0])->getCA() == ComboAddress("192.0.2.2")); BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1); @@ -4414,7 +4418,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_no_rrsig) { char addr[] = "a.root-servers.net."; for (char idx = 'a'; idx <= 'm'; idx++) { addr[0] = idx; - addRecordToLW(res, domain, QType::NS, std::string(addr), DNSResourceRecord::ANSWER, 3600); + addRecordToLW(res, domain, QType::NS, std::string(addr), DNSResourceRecord::ANSWER, 86400); } /* No RRSIG */ @@ -4436,6 +4440,9 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_no_rrsig) { return 0; }); + SyncRes::s_maxcachettl = 86400; + SyncRes::s_maxbogusttl = 3600; + vector ret; int res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret); BOOST_CHECK_EQUAL(res, RCode::NoError); @@ -4451,6 +4458,10 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_no_rrsig) { BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus); BOOST_REQUIRE_EQUAL(ret.size(), 13); + /* check that we capped the TTL to max-cache-bogus-ttl */ + for (const auto& record : ret) { + BOOST_CHECK_LE(record.d_ttl, SyncRes::s_maxbogusttl); + } BOOST_CHECK_EQUAL(queriesCount, 1); } @@ -9321,6 +9332,76 @@ BOOST_AUTO_TEST_CASE(test_dnssec_rrsig_negcache_validity) { BOOST_CHECK_EQUAL(queriesCount, 4); } +BOOST_AUTO_TEST_CASE(test_dnssec_rrsig_negcache_bogus_validity) { + std::unique_ptr sr; + initSR(sr, true); + + setDNSSECValidation(sr, DNSSECMode::ValidateAll); + + primeHints(); + const DNSName target("com."); + testkeysset_t keys; + + auto luaconfsCopy = g_luaconfs.getCopy(); + luaconfsCopy.dsAnchors.clear(); + generateKeyMaterial(g_rootdnsname, DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys, luaconfsCopy.dsAnchors); + generateKeyMaterial(DNSName("com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys); + g_luaconfs.setState(luaconfsCopy); + + size_t queriesCount = 0; + const time_t fixedNow = sr->getNow().tv_sec; + + sr->setAsyncCallback([target,&queriesCount,keys,fixedNow](const ComboAddress& ip, const DNSName& domain, int type, bool doTCP, bool sendRDQuery, int EDNS0Level, struct timeval* now, boost::optional& srcmask, boost::optional context, LWResult* res, bool* chained) { + queriesCount++; + + DNSName auth = domain; + auth.chopOff(); + + if (type == QType::DS || type == QType::DNSKEY) { + return genericDSAndDNSKEYHandler(res, domain, auth, type, keys); + } + else { + setLWResult(res, RCode::NoError, true, false, true); + addRecordToLW(res, domain, QType::SOA, "pdns-public-ns1.powerdns.com. pieter\\.lexis.powerdns.com. 2017032301 10800 3600 604800 3600", DNSResourceRecord::AUTHORITY, 86400); + addRRSIG(keys, res->d_records, domain, 86400); + addNSECRecordToLW(domain, DNSName("z."), { QType::NSEC, QType::RRSIG }, 86400, res->d_records); + /* no RRSIG */ + return 1; + } + + return 0; + }); + + SyncRes::s_maxnegttl = 3600; + SyncRes::s_maxbogusttl = 360; + + vector ret; + int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus); + BOOST_REQUIRE_EQUAL(ret.size(), 3); + BOOST_CHECK_EQUAL(queriesCount, 4); + + /* check that the entry has not been negatively cached but not longer than s_maxbogusttl */ + const NegCache::NegCacheEntry* ne = nullptr; + BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1); + BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true); + BOOST_CHECK_EQUAL(ne->d_ttd, fixedNow + SyncRes::s_maxbogusttl); + BOOST_CHECK_EQUAL(ne->d_validationState, Bogus); + BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1); + BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1); + BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 1); + BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 0); + + /* again, to test the cache */ + ret.clear(); + res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus); + BOOST_REQUIRE_EQUAL(ret.size(), 3); + BOOST_CHECK_EQUAL(queriesCount, 4); +} + BOOST_AUTO_TEST_CASE(test_dnssec_rrsig_cache_validity) { std::unique_ptr sr; initSR(sr, true); @@ -9390,7 +9471,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cache_secure) { Validation is optional, and the first query does not ask for it, so the answer is cached as Indeterminate. The second query asks for validation, answer should be marked as - Secure. + Secure, after just-in-time validation. */ std::unique_ptr sr; initSR(sr, true); @@ -9549,7 +9630,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cache_bogus) { else { if (domain == target && type == QType::A) { setLWResult(res, 0, true, false, true); - addRecordToLW(res, target, QType::A, "192.0.2.1"); + addRecordToLW(res, target, QType::A, "192.0.2.1", DNSResourceRecord::ANSWER, 86400); /* no RRSIG */ return 1; } @@ -9558,6 +9639,8 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cache_bogus) { return 0; }); + SyncRes::s_maxbogusttl = 3600; + vector ret; /* first query does not require validation */ sr->setDNSSECValidationRequested(false); @@ -9567,6 +9650,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cache_bogus) { BOOST_REQUIRE_EQUAL(ret.size(), 1); for (const auto& record : ret) { BOOST_CHECK(record.d_type == QType::A); + BOOST_CHECK_EQUAL(record.d_ttl, 86400); } BOOST_CHECK_EQUAL(queriesCount, 1); @@ -9577,9 +9661,26 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cache_bogus) { res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus); + /* check that we correctly capped the TTD for a Bogus record after + just-in-time validation */ + BOOST_REQUIRE_EQUAL(ret.size(), 1); + for (const auto& record : ret) { + BOOST_CHECK(record.d_type == QType::A); + BOOST_CHECK_EQUAL(record.d_ttl, SyncRes::s_maxbogusttl); + } + BOOST_CHECK_EQUAL(queriesCount, 3); + + ret.clear(); + /* third time also _does_ require validation, so we + can check that the cache has been updated */ + sr->setDNSSECValidationRequested(true); + res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus); BOOST_REQUIRE_EQUAL(ret.size(), 1); for (const auto& record : ret) { BOOST_CHECK(record.d_type == QType::A); + BOOST_CHECK_EQUAL(record.d_ttl, SyncRes::s_maxbogusttl); } BOOST_CHECK_EQUAL(queriesCount, 3); } @@ -9763,13 +9864,13 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cname_cache_bogus) { else { if (domain == target && type == QType::A) { setLWResult(res, 0, true, false, true); - addRecordToLW(res, target, QType::CNAME, cnameTarget.toString()); - addRecordToLW(res, cnameTarget, QType::A, "192.0.2.1"); + addRecordToLW(res, target, QType::CNAME, cnameTarget.toString(), DNSResourceRecord::ANSWER, 86400); + addRecordToLW(res, cnameTarget, QType::A, "192.0.2.1", DNSResourceRecord::ANSWER, 86400); /* no RRSIG */ return 1; } else if (domain == cnameTarget && type == QType::A) { setLWResult(res, 0, true, false, true); - addRecordToLW(res, cnameTarget, QType::A, "192.0.2.1"); + addRecordToLW(res, cnameTarget, QType::A, "192.0.2.1", DNSResourceRecord::ANSWER, 86400); /* no RRSIG */ return 1; } @@ -9778,6 +9879,9 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cname_cache_bogus) { return 0; }); + SyncRes::s_maxbogusttl = 60; + SyncRes::s_maxnegttl = 3600; + vector ret; /* first query does not require validation */ sr->setDNSSECValidationRequested(false); @@ -9787,6 +9891,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cname_cache_bogus) { BOOST_REQUIRE_EQUAL(ret.size(), 2); for (const auto& record : ret) { BOOST_CHECK(record.d_type == QType::CNAME || record.d_type == QType::A); + BOOST_CHECK_EQUAL(record.d_ttl, 86400); } BOOST_CHECK_EQUAL(queriesCount, 2); @@ -9798,8 +9903,25 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cname_cache_bogus) { BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus); BOOST_REQUIRE_EQUAL(ret.size(), 2); + /* check that we correctly capped the TTD for a Bogus record after + just-in-time validation */ + for (const auto& record : ret) { + BOOST_CHECK(record.d_type == QType::CNAME || record.d_type == QType::A); + BOOST_CHECK_EQUAL(record.d_ttl, SyncRes::s_maxbogusttl); + } + BOOST_CHECK_EQUAL(queriesCount, 5); + + ret.clear(); + /* and a third time to make sure that the validation status (and TTL!) + was properly updated in the cache */ + sr->setDNSSECValidationRequested(true); + res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus); + BOOST_REQUIRE_EQUAL(ret.size(), 2); for (const auto& record : ret) { BOOST_CHECK(record.d_type == QType::CNAME || record.d_type == QType::A); + BOOST_CHECK_EQUAL(record.d_ttl, SyncRes::s_maxbogusttl); } BOOST_CHECK_EQUAL(queriesCount, 5); } @@ -10134,8 +10256,8 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus) { } else { setLWResult(res, RCode::NoError, true, false, true); - addRecordToLW(res, domain, QType::SOA, "pdns-public-ns1.powerdns.com. pieter\\.lexis.powerdns.com. 2017032301 10800 3600 604800 3600", DNSResourceRecord::AUTHORITY, 3600); - addRRSIG(keys, res->d_records, domain, 300); + addRecordToLW(res, domain, QType::SOA, "pdns-public-ns1.powerdns.com. pieter\\.lexis.powerdns.com. 2017032301 10800 3600 604800 3600", DNSResourceRecord::AUTHORITY, 86400); + addRRSIG(keys, res->d_records, domain, 86400); /* no denial */ return 1; } @@ -10143,6 +10265,10 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus) { return 0; }); + SyncRes::s_maxbogusttl = 60; + SyncRes::s_maxnegttl = 3600; + const auto now = sr->getNow().tv_sec; + vector ret; /* first query does not require validation */ sr->setDNSSECValidationRequested(false); @@ -10150,6 +10276,11 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus) { BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), Indeterminate); BOOST_REQUIRE_EQUAL(ret.size(), 2); + for (const auto& record : ret) { + if (record.d_type == QType::SOA) { + BOOST_CHECK_EQUAL(record.d_ttl, SyncRes::s_maxnegttl); + } + } BOOST_CHECK_EQUAL(queriesCount, 1); const NegCache::NegCacheEntry* ne = nullptr; BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1); @@ -10157,6 +10288,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus) { BOOST_CHECK_EQUAL(ne->d_validationState, Indeterminate); BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1); BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1); + BOOST_CHECK_EQUAL(ne->d_ttd, now + SyncRes::s_maxnegttl); BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 0); BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 0); @@ -10167,11 +10299,35 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus) { BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus); BOOST_REQUIRE_EQUAL(ret.size(), 2); + for (const auto& record : ret) { + BOOST_CHECK_EQUAL(record.d_ttl, SyncRes::s_maxbogusttl); + } + BOOST_CHECK_EQUAL(queriesCount, 4); + BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true); + BOOST_CHECK_EQUAL(ne->d_validationState, Bogus); + BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1); + BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1); + BOOST_CHECK_EQUAL(ne->d_ttd, now + SyncRes::s_maxbogusttl); + BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 0); + BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 0); + + ret.clear(); + /* third one _does_ not require validation, we just check that + the cache (status and TTL) has been correctly updated */ + sr->setDNSSECValidationRequested(false); + res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus); + BOOST_REQUIRE_EQUAL(ret.size(), 2); + for (const auto& record : ret) { + BOOST_CHECK_EQUAL(record.d_ttl, SyncRes::s_maxbogusttl); + } BOOST_CHECK_EQUAL(queriesCount, 4); BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true); BOOST_CHECK_EQUAL(ne->d_validationState, Bogus); BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1); BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1); + BOOST_CHECK_EQUAL(ne->d_ttd, now + SyncRes::s_maxbogusttl); BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 0); BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 0); } diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 52d3e1b65..a13d720c3 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -49,6 +49,7 @@ string SyncRes::s_serverID; SyncRes::LogMode SyncRes::s_lm; unsigned int SyncRes::s_maxnegttl; +unsigned int SyncRes::s_maxbogusttl; unsigned int SyncRes::s_maxcachettl; unsigned int SyncRes::s_maxqperq; unsigned int SyncRes::s_maxtotusec; @@ -912,6 +913,16 @@ DNSName SyncRes::getBestNSNamesFromCache(const DNSName &qname, const QType& qtyp return subdomain; } +void SyncRes::updateValidationStatusInCache(const DNSName &qname, const QType& qt, bool aa, vState newState) const +{ + if (newState == Bogus) { + t_RC->updateValidationStatus(d_now.tv_sec, qname, qt, d_cacheRemote, aa, newState, s_maxbogusttl + d_now.tv_sec); + } + else { + t_RC->updateValidationStatus(d_now.tv_sec, qname, qt, d_cacheRemote, aa, newState, boost::none); + } +} + bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector& ret, unsigned int depth, int &res, vState& state, bool wasAuthZone, bool wasForwardRecurse) { string prefix; @@ -931,6 +942,7 @@ bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector vector> signatures; vector> authorityRecs; bool wasAuth; + uint32_t capTTL = std::numeric_limits::max(); /* we don't require auth data for forward-recurse lookups */ if(t_RC->get(d_now.tv_sec, qname, QType(QType::CNAME), !wasForwardRecurse && d_requireAuthData, &cset, d_cacheRemote, d_doDNSSEC ? &signatures : nullptr, d_doDNSSEC ? &authorityRecs : nullptr, &d_wasVariable, &state, &wasAuth) > 0) { @@ -958,7 +970,10 @@ bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector state = SyncRes::validateRecordsWithSigs(depth, qname, QType(QType::CNAME), qname, cset, signatures); if (state != Indeterminate) { LOG(prefix<updateValidationStatus(d_now.tv_sec, qname, QType(QType::CNAME), d_cacheRemote, d_requireAuthData, state); + if (state == Bogus) { + capTTL = s_maxbogusttl; + } + updateValidationStatusInCache(qname, QType(QType::CNAME), wasAuth, state); } } } @@ -966,7 +981,9 @@ bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector LOG(prefix<d_content->getZoneRepresentation()<<"', validation state is "<d_ttl - d_now.tv_sec; + sigdr.d_ttl=ttl; sigdr.d_content=signature; sigdr.d_place=DNSResourceRecord::ANSWER; sigdr.d_class=QClass::IN; @@ -983,7 +1000,7 @@ bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector for(const auto& rec : authorityRecs) { DNSRecord authDR(*rec); - authDR.d_ttl=j->d_ttl - d_now.tv_sec; + authDR.d_ttl=ttl; ret.push_back(authDR); } @@ -1105,7 +1122,11 @@ void SyncRes::computeNegCacheValidationStatus(const NegCache::NegCacheEntry* ne, } if (state != Indeterminate) { /* validation succeeded, let's update the cache entry so we don't have to validate again */ - t_sstorage.negcache.updateValidationStatus(ne->d_name, ne->d_qtype, state); + boost::optional capTTD = boost::none; + if (state == Bogus) { + capTTD = d_now.tv_sec + s_maxbogusttl; + } + t_sstorage.negcache.updateValidationStatus(ne->d_name, ne->d_qtype, state, capTTD); } } @@ -1165,6 +1186,10 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w if (!wasAuthZone && shouldValidate() && state == Indeterminate) { LOG(prefix<> signatures; vector> authorityRecs; uint32_t ttl=0; + uint32_t capTTL = std::numeric_limits::max(); bool wasCachedAuth; if(t_RC->get(d_now.tv_sec, sqname, sqt, !wasForwardRecurse && d_requireAuthData, &cset, d_cacheRemote, d_doDNSSEC ? &signatures : nullptr, d_doDNSSEC ? &authorityRecs : nullptr, &d_wasVariable, &cachedState, &wasCachedAuth) > 0) { @@ -1212,7 +1238,10 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w if (cachedState != Indeterminate) { LOG(prefix<updateValidationStatus(d_now.tv_sec, sqname, sqt, d_cacheRemote, d_requireAuthData, cachedState); + if (cachedState == Bogus) { + capTTL = s_maxbogusttl; + } + updateValidationStatusInCache(sqname, sqt, wasCachedAuth, cachedState); } } @@ -1226,7 +1255,9 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w if(j->d_ttl>(unsigned int) d_now.tv_sec) { DNSRecord dr=*j; - ttl = (dr.d_ttl-=d_now.tv_sec); + dr.d_ttl -= d_now.tv_sec; + dr.d_ttl = std::min(dr.d_ttl, capTTL); + ttl = dr.d_ttl; ret.push_back(dr); LOG("[ttl="<second.records) { + record.d_ttl = std::min(record.d_ttl, static_cast(s_maxbogusttl + d_now.tv_sec)); + } + } + /* We don't need to store NSEC3 records in the positive cache because: - we don't allow direct NSEC3 queries - denial of existence proofs in wildcard expanded positive responses are stored in authorityRecs @@ -2448,7 +2486,6 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co ne.d_qtype = QType(0); // this encodes 'whole record' ne.d_auth = rec.d_name; harvestNXRecords(lwr.d_records, ne, d_now.tv_sec, &lowestTTL); - ne.d_ttd = d_now.tv_sec + lowestTTL; if (state == Secure) { dState denialState = getDenialValidationState(ne, state, NXDOMAIN, false); @@ -2458,6 +2495,11 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co ne.d_validationState = state; } + if (ne.d_validationState == Bogus) { + lowestTTL = min(lowestTTL, s_maxbogusttl); + } + + ne.d_ttd = d_now.tv_sec + lowestTTL; /* if we get an NXDomain answer with a CNAME, let's not cache the target, even the server was authoritative for it, and do an additional query for the CNAME target. @@ -2495,7 +2537,6 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co LOG(prefix<getZoneRepresentation()<<"|"<