From: Remi Gacogne Date: Thu, 24 Sep 2020 16:01:16 +0000 (+0200) Subject: rec: Fix validation when more than one cached record is returned X-Git-Tag: auth-4.4.0-alpha2~39^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8e706e7b3d5d7bac9ee8777c958ca19e5757628f;p=thirdparty%2Fpdns.git rec: Fix validation when more than one cached record is returned We need to validate them RRSet by RRSet. --- diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc index 2169670d69..d819dac29b 100644 --- a/pdns/recursor_cache.cc +++ b/pdns/recursor_cache.cc @@ -79,7 +79,39 @@ size_t MemRecursorCache::bytes() return ret; } -int32_t MemRecursorCache::handleHit(MapCombo& map, MemRecursorCache::OrderedTagIterator_t& entry, const DNSName& qname, 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(MapCombo& map, MemRecursorCache::OrderedTagIterator_t& entry, const DNSName& qname, vector* res, vector>* signatures, std::vector>* authorityRecs, bool* variable, boost::optional& state, bool* wasAuth) { // MUTEX SHOULD BE ACQUIRED int32_t ttd = entry->d_ttd; @@ -112,9 +144,7 @@ int32_t MemRecursorCache::handleHit(MapCombo& map, MemRecursorCache::OrderedTagI authorityRecs->insert(authorityRecs->end(), entry->d_authorityRecs.begin(), entry->d_authorityRecs.end()); } - if (state) { - updateDNSSECValidationState(*state, entry->d_state); - } + updateDNSSECValidationStateFromCache(state, entry->d_state); if (wasAuth) { *wasAuth = *wasAuth && entry->d_auth; @@ -218,6 +248,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, const OptTag& routingTag, 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) { @@ -229,9 +260,6 @@ int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, // so it will be set to false if at least one entry is not auth *wasAuth = true; } - if (state) { - *state = vState::Indeterminate; - } auto& map = getMap(qname); const lock l(map); @@ -244,23 +272,32 @@ int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, auto entryA = getEntryUsingECSIndex(map, now, qname, QType::A, requireAuth, who); if (entryA != map.d_map.end()) { - ret = handleHit(map, entryA, qname, res, signatures, authorityRecs, variable, state, wasAuth); + ret = handleHit(map, entryA, qname, res, signatures, authorityRecs, variable, cachedState, wasAuth); } auto entryAAAA = getEntryUsingECSIndex(map, now, qname, QType::AAAA, requireAuth, who); if (entryAAAA != map.d_map.end()) { - int32_t ttdAAAA = handleHit(map, entryAAAA, qname, res, signatures, authorityRecs, variable, state, wasAuth); + int32_t ttdAAAA = handleHit(map, entryAAAA, qname, 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(map, now, qname, qtype, requireAuth, who); if (entry != map.d_map.end()) { - return static_cast(handleHit(map, entry, qname, res, signatures, authorityRecs, variable, state, wasAuth) - now); + int32_t ret = handleHit(map, entry, qname, res, signatures, authorityRecs, variable, cachedState, wasAuth); + if (state && cachedState) { + *state = *cachedState; + } + return static_cast(ret-now); } return -1; } @@ -282,12 +319,15 @@ int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, continue; } - ttd = handleHit(map, firstIndexIterator, qname, res, signatures, authorityRecs, variable, state, wasAuth); + ttd = handleHit(map, firstIndexIterator, qname, 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; + } return static_cast(ttd-now); } } @@ -307,12 +347,15 @@ int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, continue; } - ttd = handleHit(map, firstIndexIterator, qname, res, signatures, authorityRecs, variable, state, wasAuth); + ttd = handleHit(map, firstIndexIterator, qname, 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; + } return static_cast(ttd-now); } return -1; diff --git a/pdns/recursor_cache.hh b/pdns/recursor_cache.hh index 8166ddab8a..b2cbca1125 100644 --- a/pdns/recursor_cache.hh +++ b/pdns/recursor_cache.hh @@ -230,7 +230,7 @@ private: bool entryMatches(OrderedTagIterator_t& entry, uint16_t qt, bool requireAuth, const ComboAddress& who); Entries getEntries(MapCombo& map, const DNSName &qname, const QType& qt, const OptTag& rtag); cache_t::const_iterator getEntryUsingECSIndex(MapCombo& map, time_t now, const DNSName &qname, uint16_t qtype, bool requireAuth, const ComboAddress& who); - int32_t handleHit(MapCombo& map, OrderedTagIterator_t& entry, const DNSName& qname, vector* res, vector>* signatures, std::vector>* authorityRecs, bool* variable, vState* state, bool* wasAuth); + int32_t handleHit(MapCombo& map, OrderedTagIterator_t& entry, const DNSName& qname, vector* res, vector>* signatures, std::vector>* authorityRecs, bool* variable, boost::optional& state, bool* wasAuth); public: struct lock { diff --git a/pdns/syncres.cc b/pdns/syncres.cc index b1049fc0d2..87db2e7c9a 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -1551,6 +1551,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); + } +} + /*! * Convenience function to push the records from records into ret with a new TTL * @@ -1748,7 +1762,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 { @@ -2661,7 +2693,7 @@ vState SyncRes::validateRecordsWithSigs(unsigned int depth, const DNSName& qname recordcontents.insert(record.d_content); } - LOG(d_prefix<<"Going to validate "<