From 15f44f1d015c66eed54de286fcb9d69fbcbc5368 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Mon, 22 May 2017 11:54:20 +0200 Subject: [PATCH] rec: Prevent a loop while fetching DNSKEY If some records on the DNSKEY answer are signed with the same signer, we could end up in a DNSKEY retrieval loop since we haven't added the DNSKEY to the cache yet. --- pdns/syncres.cc | 14 +++++++++----- pdns/syncres.hh | 4 ++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 1e23b3d85..45a1551f8 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -1617,12 +1617,16 @@ vState SyncRes::getDNSKeys(const DNSName& signer, skeyset_t& keys, unsigned int return Bogus; } -vState SyncRes::validateRecordsWithSigs(unsigned int depth, const DNSName& name, const std::vector& records, const std::vector >& signatures) +vState SyncRes::validateRecordsWithSigs(unsigned int depth, const DNSName& qname, const QType& qtype, const DNSName& name, const std::vector& records, const std::vector >& signatures) { skeyset_t keys; if (!signatures.empty()) { const DNSName signer = getSigner(signatures); if (!signer.empty() && name.isPartOf(signer)) { + if (qtype == QType::DNSKEY && signer == qname) { + /* we are already retrieving those keys, sorry */ + return Indeterminate; + } vState state = getDNSKeys(signer, keys, depth); if (state != Secure) { return state; @@ -1648,7 +1652,7 @@ vState SyncRes::validateRecordsWithSigs(unsigned int depth, const DNSName& name, return Bogus; } -RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr, const DNSName& qname, const DNSName& auth, bool wasForwarded, const boost::optional ednsmask, vState& state, const SyncRes::zonesStates_t& cuts, bool& needWildcardProof) +RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr, const DNSName& qname, const QType& qtype, const DNSName& auth, bool wasForwarded, const boost::optional ednsmask, vState& state, const SyncRes::zonesStates_t& cuts, bool& needWildcardProof) { struct CacheEntry { @@ -1797,7 +1801,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr } else { LOG("Validating non-additional record for "<first.name<first.name, i->second.records, i->second.signatures); + recordState = validateRecordsWithSigs(depth, qname, qtype, i->first.name, i->second.records, i->second.signatures); } } } @@ -1805,7 +1809,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr /* for non authoritative answer, we only care about the DS record (or lack of) */ if ((i->first.type == QType::DS || i->first.type == QType::NSEC || i->first.type == QType::NSEC3) && i->first.place == DNSResourceRecord::AUTHORITY) { LOG("Validating DS record for "<first.name<first.name, i->second.records, i->second.signatures); + recordState = validateRecordsWithSigs(depth, qname, qtype, i->first.name, i->second.records, i->second.signatures); } } updateValidationState(state, recordState); @@ -2112,7 +2116,7 @@ bool SyncRes::processAnswer(unsigned int depth, LWResult& lwr, const DNSName& qn } bool needWildcardProof = false; - *rcode = updateCacheFromRecords(depth, lwr, qname, auth, wasForwarded, ednsmask, state, cuts, needWildcardProof); + *rcode = updateCacheFromRecords(depth, lwr, qname, qtype, auth, wasForwarded, ednsmask, state, cuts, needWildcardProof); if (*rcode != RCode::NoError) { return true; } diff --git a/pdns/syncres.hh b/pdns/syncres.hh index f43d49271..247555fd4 100644 --- a/pdns/syncres.hh +++ b/pdns/syncres.hh @@ -724,7 +724,7 @@ private: bool throttledOrBlocked(const std::string& prefix, const ComboAddress& remoteIP, const DNSName& qname, const QType& qtype, bool pierceDontQuery); vector retrieveAddressesForNS(const std::string& prefix, const DNSName& qname, vector::const_iterator& tns, const unsigned int depth, set& beenthere, const vector& rnameservers, NsSet& nameservers, bool& sendRDQuery, bool& pierceDontQuery, bool& flawedNSSet); - RCode::rcodes_ updateCacheFromRecords(unsigned int depth, LWResult& lwr, const DNSName& qname, const DNSName& auth, bool wasForwarded, const boost::optional, vState& state, const zonesStates_t& cuts, bool& needWildcardProof); + RCode::rcodes_ updateCacheFromRecords(unsigned int depth, LWResult& lwr, const DNSName& qname, const QType& qtype, const DNSName& auth, bool wasForwarded, const boost::optional, vState& state, const zonesStates_t& cuts, bool& needWildcardProof); 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, bool needWildcardProof); bool doSpecialNamesResolve(const DNSName &qname, const QType &qtype, const uint16_t qclass, vector &ret); @@ -737,7 +737,7 @@ private: uint32_t computeLowestTTD(const std::vector& records, const std::vector >& signatures, uint32_t signaturesTTL) const; void updateValidationState(vState& state, const vState stateUpdate); void updateValidationStatusAfterReferral(const DNSName& qname, const DNSName& oldauth, const DNSName& newauth, vState& state, unsigned int depth); - vState validateRecordsWithSigs(unsigned int depth, const DNSName& name, const std::vector& records, const std::vector >& signatures); + vState validateRecordsWithSigs(unsigned int depth, const DNSName& qname, const QType& qtype, const DNSName& name, const std::vector& records, const std::vector >& signatures); vState validateDNSKeys(const DNSName& zone, const std::vector& dnskeys, const std::vector >& signatures, unsigned int depth); vState getDSRecords(const DNSName& zone, dsmap_t& ds, bool onlyTA, unsigned int depth); vState getDNSKeys(const DNSName& signer, skeyset_t& keys, unsigned int depth); -- 2.40.0