From 379e1d7683543e39764ae795ccf7c0b6a1de62cc Mon Sep 17 00:00:00 2001 From: Otto Moerbeek Date: Mon, 12 Oct 2020 10:13:39 +0200 Subject: [PATCH] Backport of "Prevent updating the status of all cached records for a name" to 4.3.x --- pdns/recursor_cache.cc | 89 +++++++++++++++++++----- pdns/recursor_cache.hh | 2 +- pdns/recursordist/test-syncres_cc8.cc | 98 +++++++++++++++++++++++++++ pdns/syncres.cc | 64 +++++++++++------ pdns/validate.cc | 21 ++++++ pdns/validate.hh | 1 + 6 files changed, 236 insertions(+), 39 deletions(-) diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc index ca1f8bd1cb..7e85de2db1 100644 --- a/pdns/recursor_cache.cc +++ b/pdns/recursor_cache.cc @@ -38,7 +38,39 @@ unsigned int MemRecursorCache::bytes() const return ret; } -int32_t MemRecursorCache::handleHit(MemRecursorCache::OrderedTagIterator_t& entry, const DNSName& qname, const ComboAddress& who, vector* res, vector>* signatures, std::vector>* authorityRecs, bool* variable, vState* state, bool* wasAuth) +static void updateDNSSECValidationStateFromCache(boost::optional& state, const vState stateUpdate) +{ + // if there was no state it's easy */ + if (state == boost::none) { + state = stateUpdate; + return; + } + + if (stateUpdate == vState::TA) { + state = vState::Secure; + } + else if (stateUpdate == vState::NTA) { + state = vState::Insecure; + } + else if (stateUpdate == vState::Bogus) { + state = stateUpdate; + } + else if (stateUpdate == vState::Indeterminate) { + state = stateUpdate; + } + else if (stateUpdate == vState::Insecure) { + if (*state != vState::Bogus && *state != vState::Indeterminate) { + state = stateUpdate; + } + } + else if (stateUpdate == vState::Secure) { + if (*state != vState::Bogus && *state != vState::Indeterminate) { + state = stateUpdate; + } + } +} + +int32_t MemRecursorCache::handleHit(MemRecursorCache::OrderedTagIterator_t& entry, const DNSName& qname, const ComboAddress& who, vector* res, vector>* signatures, std::vector>* authorityRecs, bool* variable, boost::optional& state, bool* wasAuth) { int32_t ttd = entry->d_ttd; @@ -62,20 +94,18 @@ int32_t MemRecursorCache::handleHit(MemRecursorCache::OrderedTagIterator_t& entr } } - if(signatures) { // if you do an ANY lookup you are hosed XXXX - *signatures = entry->d_signatures; + if (signatures) { + signatures->insert(signatures->end(), entry->d_signatures.begin(), entry->d_signatures.end()); } - if(authorityRecs) { - *authorityRecs = entry->d_authorityRecs; + if (authorityRecs) { + authorityRecs->insert(authorityRecs->end(), entry->d_authorityRecs.begin(), entry->d_authorityRecs.end()); } - if (state) { - *state = entry->d_state; - } + updateDNSSECValidationStateFromCache(state, entry->d_state); if (wasAuth) { - *wasAuth = entry->d_auth; + *wasAuth = *wasAuth && entry->d_auth; } moveCacheItemToBack(d_cache, entry); @@ -173,6 +203,7 @@ bool MemRecursorCache::entryMatches(MemRecursorCache::OrderedTagIterator_t& entr // returns -1 for no hits int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, bool requireAuth, vector* res, const ComboAddress& who, vector>* signatures, std::vector>* authorityRecs, bool* variable, vState* state, bool* wasAuth) { + boost::optional cachedState{boost::none}; time_t ttd=0; // cerr<<"looking up "<< qname<<"|"+qt.getName()<<"\n"; if(res) { @@ -180,6 +211,12 @@ int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, } const uint16_t qtype = qt.getCode(); + if (wasAuth) { + // we might retrieve more than one entry, we need to set that to true + // so it will be set to false if at least one entry is not auth + *wasAuth = true; + } + /* If we don't have any netmask-specific entries at all, let's just skip this to be able to use the nice d_cachecache hack. */ if (qtype != QType::ANY && !d_ecsIndex.empty()) { @@ -188,23 +225,32 @@ int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, auto entryA = getEntryUsingECSIndex(now, qname, QType::A, requireAuth, who); if (entryA != d_cache.end()) { - ret = handleHit(entryA, qname, who, res, signatures, authorityRecs, variable, state, wasAuth); + ret = handleHit(entryA, qname, who, res, signatures, authorityRecs, variable, cachedState, wasAuth); } auto entryAAAA = getEntryUsingECSIndex(now, qname, QType::AAAA, requireAuth, who); if (entryAAAA != d_cache.end()) { - int32_t ttdAAAA = handleHit(entryAAAA, qname, who, res, signatures, authorityRecs, variable, state, wasAuth); + int32_t ttdAAAA = handleHit(entryAAAA, qname, who, res, signatures, authorityRecs, variable, cachedState, wasAuth); if (ret > 0) { ret = std::min(ret, ttdAAAA); } else { ret = ttdAAAA; } } + + if (state && cachedState) { + *state = *cachedState; + } return ret > 0 ? static_cast(ret-now) : ret; } else { auto entry = getEntryUsingECSIndex(now, qname, qtype, requireAuth, who); if (entry != d_cache.end()) { - return static_cast(handleHit(entry, qname, who, res, signatures, authorityRecs, variable, state, wasAuth) - now); + int32_t ret = handleHit(entry, qname, who, res, signatures, authorityRecs, variable, cachedState, wasAuth); + if (state && cachedState) { + *state = *cachedState; + } + return static_cast(ret-now); + } return -1; } @@ -224,12 +270,14 @@ int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, if (!entryMatches(firstIndexIterator, qtype, requireAuth, who)) continue; - ttd = handleHit(firstIndexIterator, qname, who, res, signatures, authorityRecs, variable, state, wasAuth); + ttd = handleHit(firstIndexIterator, qname, who, res, signatures, authorityRecs, variable, cachedState, wasAuth); if(qt.getCode()!=QType::ANY && qt.getCode()!=QType::ADDR) // normally if we have a hit, we are done break; } - + if (state && cachedState) { + *state = *cachedState; + } // cerr<<"time left : "<size() : 0) <<"\n"; return static_cast(ttd-now); } @@ -408,7 +456,15 @@ bool MemRecursorCache::updateValidationStatus(time_t now, const DNSName &qname, { bool updated = false; uint16_t qtype = qt.getCode(); - if (qtype != QType::ANY && qtype != QType::ADDR && !d_ecsIndex.empty()) { + if (qtype == QType::ANY) { + throw std::runtime_error("Trying to update the DNSSEC validation status of all (via ANY) records for " + qname.toLogString()); + } + if (qtype == QType::ADDR) { + throw std::runtime_error("Trying to update the DNSSEC validation status of several (via ADDR) records for " + qname.toLogString()); + } + + + if (!d_ecsIndex.empty()) { auto entry = getEntryUsingECSIndex(now, qname, qtype, requireAuth, who); if (entry == d_cache.end()) { return false; @@ -435,8 +491,7 @@ bool MemRecursorCache::updateValidationStatus(time_t now, const DNSName &qname, } updated = true; - if(qtype != QType::ANY && qtype != QType::ADDR) // normally if we have a hit, we are done - break; + break; } return updated; diff --git a/pdns/recursor_cache.hh b/pdns/recursor_cache.hh index e9a44c86c7..22ffcdfbd0 100644 --- a/pdns/recursor_cache.hh +++ b/pdns/recursor_cache.hh @@ -200,7 +200,7 @@ private: bool entryMatches(OrderedTagIterator_t& entry, uint16_t qt, bool requireAuth, const ComboAddress& who); std::pair getEntries(const DNSName &qname, const QType& qt); cache_t::const_iterator getEntryUsingECSIndex(time_t now, const DNSName &qname, uint16_t qtype, bool requireAuth, const ComboAddress& who); - int32_t handleHit(OrderedTagIterator_t& entry, const DNSName& qname, const ComboAddress& who, vector* res, vector>* signatures, std::vector>* authorityRecs, bool* variable, vState* state, bool* wasAuth); + int32_t handleHit(OrderedTagIterator_t& entry, const DNSName& qname, const ComboAddress& who, vector* res, vector>* signatures, std::vector>* authorityRecs, bool* variable, boost::optional& state, bool* wasAuth); public: void preRemoval(const CacheEntry& entry) diff --git a/pdns/recursordist/test-syncres_cc8.cc b/pdns/recursordist/test-syncres_cc8.cc index a1f0b85e8e..739a05f5b0 100644 --- a/pdns/recursordist/test-syncres_cc8.cc +++ b/pdns/recursordist/test-syncres_cc8.cc @@ -1050,4 +1050,102 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cache_bogus) BOOST_CHECK_EQUAL(queriesCount, 3U); } +BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cache_secure_any) +{ + /* + Validation is optional, and the first two queries (A, AAAA) do not ask for it, + so the answer are cached as Indeterminate. + The third query asks for validation, and is for ANY, so the answer should be marked as + Secure, after just-in-time validation. + The last query also requests validation but is for AAAA only. + */ + std::unique_ptr sr; + initSR(sr, true); + + setDNSSECValidation(sr, DNSSECMode::Process); + + primeHints(); + const DNSName target("com."); + testkeysset_t keys; + + auto luaconfsCopy = g_luaconfs.getCopy(); + luaconfsCopy.dsAnchors.clear(); + generateKeyMaterial(g_rootdnsname, DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys, luaconfsCopy.dsAnchors); + g_luaconfs.setState(luaconfsCopy); + + size_t queriesCount = 0; + + sr->setAsyncCallback([target, &queriesCount, keys](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++; + + if (type == QType::DS || type == QType::DNSKEY) { + return genericDSAndDNSKEYHandler(res, domain, domain, type, keys, false); + } + else { + if (domain == target && type == QType::A) { + setLWResult(res, 0, true, false, true); + addRecordToLW(res, target, QType::A, "192.0.2.1"); + addRRSIG(keys, res->d_records, DNSName("."), 300); + return 1; + } + else if (domain == target && type == QType::AAAA) { + setLWResult(res, 0, true, false, true); + addRecordToLW(res, target, QType::AAAA, "2001:db8::1"); + addRRSIG(keys, res->d_records, DNSName("."), 300); + return 1; + } + } + + return 0; + }); + + vector ret; + /* first query does not require validation */ + sr->setDNSSECValidationRequested(false); + int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Indeterminate); + BOOST_REQUIRE_EQUAL(ret.size(), 2U); + for (const auto& record : ret) { + BOOST_CHECK(record.d_type == QType::A || record.d_type == QType::RRSIG); + } + BOOST_CHECK_EQUAL(queriesCount, 1U); + + ret.clear(); + /* second query does not require validation either */ + sr->setDNSSECValidationRequested(false); + res = sr->beginResolve(target, QType(QType::AAAA), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Indeterminate); + BOOST_REQUIRE_EQUAL(ret.size(), 2U); + for (const auto& record : ret) { + BOOST_CHECK(record.d_type == QType::AAAA || record.d_type == QType::RRSIG); + } + BOOST_CHECK_EQUAL(queriesCount, 2U); + + ret.clear(); + /* third one _does_ require validation */ + sr->setDNSSECValidationRequested(true); + res = sr->beginResolve(target, QType(QType::ANY), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Secure); + BOOST_REQUIRE_EQUAL(ret.size(), 4U); + for (const auto& record : ret) { + BOOST_CHECK(record.d_type == QType::A || record.d_type == QType::AAAA || record.d_type == QType::RRSIG); + } + BOOST_CHECK_EQUAL(queriesCount, 4U); + + ret.clear(); + /* last one also requires validation */ + sr->setDNSSECValidationRequested(true); + res = sr->beginResolve(target, QType(QType::AAAA), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Secure); + BOOST_REQUIRE_EQUAL(ret.size(), 2U); + for (const auto& record : ret) { + BOOST_CHECK(record.d_type == QType::AAAA || record.d_type == QType::RRSIG); + } + BOOST_CHECK_EQUAL(queriesCount, 4U); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/pdns/syncres.cc b/pdns/syncres.cc index f31137df72..e8c40fb848 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -1161,6 +1161,11 @@ DNSName SyncRes::getBestNSNamesFromCache(const DNSName &qname, const QType& qtyp void SyncRes::updateValidationStatusInCache(const DNSName &qname, const QType& qt, bool aa, vState newState) const { + if (qt == QType::ANY || qt == QType::ADDR) { + // not doing that + return; + } + if (newState == Bogus) { t_RC->updateValidationStatus(d_now.tv_sec, qname, qt, d_cacheRemote, aa, newState, s_maxbogusttl + d_now.tv_sec); } @@ -1397,6 +1402,20 @@ static void reapRecordsFromNegCacheEntryForValidation(tcache_t& tcache, const ve } } +static void reapRecordsForValidation(std::map& entries, const vector& records) +{ + for (const auto& rec : records) { + entries[rec.d_type].records.push_back(rec); + } +} + +static void reapSignaturesForValidation(std::map& entries, const vector>& signatures) +{ + for (const auto& sig : signatures) { + entries[sig->d_type].signatures.push_back(sig); + } +} + /*! * Convience function to push the records from records into ret with a new TTL * @@ -1601,7 +1620,25 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w cachedState = validateDNSKeys(sqname, cset, signatures, depth); } else { - cachedState = SyncRes::validateRecordsWithSigs(depth, sqname, sqt, sqname, cset, signatures); + if (sqt == QType::ANY) { + std::map types; + reapRecordsForValidation(types, cset); + reapSignaturesForValidation(types, signatures); + + for (const auto& type : types) { + vState cachedRecordState; + if (type.first == QType::DNSKEY) { + cachedRecordState = validateDNSKeys(sqname, type.second.records, type.second.signatures, depth); + } + else { + cachedRecordState = SyncRes::validateRecordsWithSigs(depth, sqname, QType(type.first), sqname, type.second.records, type.second.signatures); + } + updateDNSSECValidationState(cachedState, cachedRecordState); + } + } + else { + cachedState = SyncRes::validateRecordsWithSigs(depth, sqname, sqt, sqname, cset, signatures); + } } } else { @@ -1613,7 +1650,9 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w if (cachedState == Bogus) { capTTL = s_maxbogusttl; } - updateValidationStatusInCache(sqname, sqt, wasCachedAuth, cachedState); + if (sqt != QType::ANY && sqt != QType::ADDR) { + updateValidationStatusInCache(sqname, sqt, wasCachedAuth, cachedState); + } } } @@ -1962,24 +2001,7 @@ uint32_t SyncRes::computeLowestTTD(const std::vector& records, const void SyncRes::updateValidationState(vState& state, const vState stateUpdate) { LOG(d_prefix<<"validation state was "< >& signa return DNSName(); } + +void updateDNSSECValidationState(vState& state, const vState stateUpdate) +{ + if (stateUpdate == vState::TA) { + state = vState::Secure; + } + else if (stateUpdate == vState::NTA) { + state = vState::Insecure; + } + else if (stateUpdate == vState::Bogus) { + state = vState::Bogus; + } + else if (state == vState::Indeterminate) { + state = stateUpdate; + } + else if (stateUpdate == vState::Insecure) { + if (state != vState::Bogus) { + state = vState::Insecure; + } + } +} diff --git a/pdns/validate.hh b/pdns/validate.hh index 76e9de3d04..0f5e4bf5b7 100644 --- a/pdns/validate.hh +++ b/pdns/validate.hh @@ -80,3 +80,4 @@ bool denialProvesNoDelegation(const DNSName& zone, const std::vector& bool isRRSIGNotExpired(const time_t now, const shared_ptr sig); bool isWildcardExpanded(unsigned int labelCount, const std::shared_ptr& sign); bool isWildcardExpandedOntoItself(const DNSName& owner, unsigned int labelCount, const std::shared_ptr& sign); +void updateDNSSECValidationState(vState& state, const vState stateUpdate); -- 2.47.2