From: Remi Gacogne Date: Thu, 4 Mar 2021 15:02:48 +0000 (+0100) Subject: rec: Fix the validation status computation with negative indication X-Git-Tag: rec-4.5.0-beta1~4^2~7 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b4ac45b6f4ee59879d3f9ff3b8a644c1f1829563;p=thirdparty%2Fpdns.git rec: Fix the validation status computation with negative indication We need to know whether the denial had signatures to decide whether we need to lool for a zone cut. --- diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 4d31ce86f5..3694571d89 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -1999,25 +1999,26 @@ static const set nsecTypes = {QType::NSEC, QType::NSEC3}; * \param ne The NegCacheEntry to be filled out (will not be cleared, only appended to */ static void harvestNXRecords(const vector& records, NegCache::NegCacheEntry& ne, const time_t now, uint32_t* lowestTTL) { - for(const auto& rec : records) { - if(rec.d_place != DNSResourceRecord::AUTHORITY) + for (const auto& rec : records) { + if (rec.d_place != DNSResourceRecord::AUTHORITY) { // RFC 4035 section 3.1.3. indicates that NSEC records MUST be placed in // the AUTHORITY section. Section 3.1.1 indicates that that RRSIGs for // records MUST be in the same section as the records they cover. // Hence, we ignore all records outside of the AUTHORITY section. continue; + } - if(rec.d_type == QType::RRSIG) { + if (rec.d_type == QType::RRSIG) { auto rrsig = getRR(rec); - if(rrsig) { - if(rrsig->d_type == QType::SOA) { + if (rrsig) { + if (rrsig->d_type == QType::SOA) { ne.authoritySOA.signatures.push_back(rec); if (lowestTTL && isRRSIGNotExpired(now, rrsig)) { *lowestTTL = min(*lowestTTL, rec.d_ttl); *lowestTTL = min(*lowestTTL, getRRSIGTTL(now, rrsig)); } } - if(nsecTypes.count(rrsig->d_type)) { + if (nsecTypes.count(rrsig->d_type)) { ne.DNSSECRecords.signatures.push_back(rec); if (lowestTTL && isRRSIGNotExpired(now, rrsig)) { *lowestTTL = min(*lowestTTL, rec.d_ttl); @@ -2027,14 +2028,14 @@ static void harvestNXRecords(const vector& records, NegCache::NegCach } continue; } - if(rec.d_type == QType::SOA) { + if (rec.d_type == QType::SOA) { ne.authoritySOA.records.push_back(rec); if (lowestTTL) { *lowestTTL = min(*lowestTTL, rec.d_ttl); } continue; } - if(nsecTypes.count(rec.d_type)) { + if (nsecTypes.count(rec.d_type)) { ne.DNSSECRecords.records.push_back(rec); if (lowestTTL) { *lowestTTL = min(*lowestTTL, rec.d_ttl); @@ -3287,6 +3288,10 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr } } + if (seenAuth.empty() && !i->second.signatures.empty()) { + seenAuth = getSigner(i->second.signatures); + } + if (g_aggressiveNSECCache && (i->first.type == QType::NSEC || i->first.type == QType::NSEC3) && recordState == vState::Secure && !seenAuth.empty()) { // Good candidate for NSEC{,3} caching g_aggressiveNSECCache->insertNSEC(seenAuth, i->first.name, i->second.records.at(0), i->second.signatures, i->first.type == QType::NSEC3); @@ -3342,7 +3347,7 @@ dState SyncRes::getDenialValidationState(const NegCache::NegCacheEntry& ne, cons return getDenial(csp, ne.d_name, ne.d_qtype.getCode(), referralToUnsigned, expectedState == dState::NXQTYPE); } -bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, const QType qtype, const DNSName& auth, LWResult& lwr, const bool sendRDQuery, vector& ret, set& nsset, DNSName& newtarget, DNSName& newauth, bool& realreferral, bool& negindic, vState& state, const bool needWildcardProof, const bool gatherWildcardProof, const unsigned int wildcardLabelsCount, int& rcode, unsigned int depth) +bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, const QType qtype, const DNSName& auth, LWResult& lwr, const bool sendRDQuery, vector& ret, set& nsset, DNSName& newtarget, DNSName& newauth, bool& realreferral, bool& negindic, vState& state, const bool needWildcardProof, const bool gatherWildcardProof, const unsigned int wildcardLabelsCount, int& rcode, bool& negIndicHasSignatures, unsigned int depth) { bool done = false; DNSName dnameTarget, dnameOwner; @@ -3387,7 +3392,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co else { /* here we need to get the validation status of the zone telling us that the domain does not exist, ie the owner of the SOA */ - auto recordState = getValidationStatus(rec.d_name, !ne.authoritySOA.signatures.empty(), false, depth); + auto recordState = getValidationStatus(rec.d_name, !ne.authoritySOA.signatures.empty() || !ne.DNSSECRecords.signatures.empty(), false, depth); if (recordState == vState::Secure) { dState denialState = getDenialValidationState(ne, dState::NXDOMAIN, false); updateDenialValidationState(ne.d_validationState, ne.d_name, state, denialState, dState::NXDOMAIN); @@ -3416,6 +3421,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co } } + negIndicHasSignatures = !ne.authoritySOA.signatures.empty() || !ne.DNSSECRecords.signatures.empty(); negindic = true; } else if (rec.d_place == DNSResourceRecord::ANSWER && s_redirectionQTypes.count(rec.d_type) > 0 && // CNAME or DNAME answer @@ -3486,7 +3492,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co ne.d_validationState = state; } else { - auto recordState = getValidationStatus(qname, !ne.authoritySOA.signatures.empty(), false, depth); + auto recordState = getValidationStatus(qname, !ne.authoritySOA.signatures.empty() || !ne.DNSSECRecords.signatures.empty(), false, depth); if (recordState == vState::Secure) { /* We have a positive answer synthesized from a wildcard, we need to check that we have @@ -3551,7 +3557,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co harvestNXRecords(lwr.d_records, ne, d_now.tv_sec, &lowestTTL); if (!vStateIsBogus(state)) { - auto recordState = getValidationStatus(newauth, !ne.authoritySOA.signatures.empty(), true, depth); + auto recordState = getValidationStatus(newauth, !ne.authoritySOA.signatures.empty() || !ne.DNSSECRecords.signatures.empty(), true, depth); if (recordState == vState::Secure) { ne.d_auth = auth; @@ -3576,6 +3582,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co if (qname == newauth && qtype == QType::DS) { /* we are actually done! */ negindic = true; + negIndicHasSignatures = !ne.authoritySOA.signatures.empty() || !ne.DNSSECRecords.signatures.empty(); nsset.clear(); } } @@ -3603,7 +3610,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co ne.d_validationState = state; } else { - auto recordState = getValidationStatus(qname, ne.authoritySOA.signatures.empty(), qtype == QType::DS, depth); + auto recordState = getValidationStatus(qname, !ne.authoritySOA.signatures.empty() || !ne.DNSSECRecords.signatures.empty(), qtype == QType::DS, depth); if (recordState == vState::Secure) { dState denialState = getDenialValidationState(ne, dState::NXQTYPE, false); updateDenialValidationState(ne.d_validationState, ne.d_name, state, denialState, dState::NXQTYPE); @@ -3627,6 +3634,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co ret.push_back(rec); negindic = true; + negIndicHasSignatures = !ne.authoritySOA.signatures.empty() || !ne.DNSSECRecords.signatures.empty(); } } } @@ -3910,11 +3918,13 @@ bool SyncRes::processAnswer(unsigned int depth, LWResult& lwr, const DNSName& qn LOG(prefix< nsset; - bool realreferral=false, negindic=false; + bool realreferral = false; + bool negindic = false; + bool negIndicHasSignatures = false; DNSName newauth; DNSName newtarget; - bool done = processRecords(prefix, qname, qtype, auth, lwr, sendRDQuery, ret, nsset, newtarget, newauth, realreferral, negindic, state, needWildcardProof, gatherWildcardProof, wildcardLabelsCount, *rcode, depth); + bool done = processRecords(prefix, qname, qtype, auth, lwr, sendRDQuery, ret, nsset, newtarget, newauth, realreferral, negindic, state, needWildcardProof, gatherWildcardProof, wildcardLabelsCount, *rcode, negIndicHasSignatures, depth); if (done){ LOG(prefix<, vState& state, bool& needWildcardProof, bool& gatherWildcardProof, unsigned int& wildcardLabelsCount, bool sendRDQuery, const ComboAddress& remoteIP); - bool processRecords(const std::string& prefix, const DNSName& qname, const QType qtype, const DNSName& auth, LWResult& lwr, const bool sendRDQuery, vector& ret, set& nsset, DNSName& newtarget, DNSName& newauth, bool& realreferral, bool& negindic, vState& state, const bool needWildcardProof, const bool gatherwildcardProof, const unsigned int wildcardLabelsCount, int& rcode, unsigned int depth); + bool processRecords(const std::string& prefix, const DNSName& qname, const QType qtype, const DNSName& auth, LWResult& lwr, const bool sendRDQuery, vector& ret, set& nsset, DNSName& newtarget, DNSName& newauth, bool& realreferral, bool& negindic, vState& state, const bool needWildcardProof, const bool gatherwildcardProof, const unsigned int wildcardLabelsCount, int& rcode, bool& negIndicHasSignatures, unsigned int depth); bool doSpecialNamesResolve(const DNSName &qname, QType qtype, const uint16_t qclass, vector &ret);