From: Remi Gacogne Date: Thu, 11 Apr 2019 13:25:10 +0000 (+0200) Subject: rec: Fix DNSSEC validation of wildcards expanded onto themselves X-Git-Tag: rec-4.2.0-beta1~3^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F7714%2Fhead;p=thirdparty%2Fpdns.git rec: Fix DNSSEC validation of wildcards expanded onto themselves --- diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 1b71d3c190..72b67e56d2 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -2162,7 +2162,7 @@ void SyncRes::sanitizeRecords(const std::string& prefix, LWResult& lwr, const DN } } -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, bool& needWildcardProof, unsigned int& wildcardLabelsCount, bool rdQuery) +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, bool& needWildcardProof, bool& gatherWildcardProof, unsigned int& wildcardLabelsCount, bool rdQuery) { bool wasForwardRecurse = wasForwarded && rdQuery; tcache_t tcache; @@ -2190,7 +2190,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr /* if we have a positive answer synthetized from a wildcard, we need to store the corresponding NSEC/NSEC3 records proving that the exact name did not exist in the negative cache */ - if(needWildcardProof) { + if(gatherWildcardProof) { if (nsecTypes.count(rec.d_type)) { authorityRecs.push_back(std::make_shared(rec)); } @@ -2208,9 +2208,20 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr count can be lower than the name's label count if it was synthetized from the wildcard. Note that the difference might be > 1. */ - if (rec.d_name == qname && rrsig->d_labels < labelCount) { - LOG(prefix<d_labels; } @@ -2483,7 +2494,7 @@ dState SyncRes::getDenialValidationState(const NegCache::NegCacheEntry& ne, cons return getDenial(csp, ne.d_name, ne.d_qtype.getCode(), referralToUnsigned, expectedState == 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 unsigned int wildcardLabelsCount) +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) { bool done = false; @@ -2554,7 +2565,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co /* if we have a positive answer synthetized from a wildcard, we need to return the corresponding NSEC/NSEC3 records from the AUTHORITY section proving that the exact name did not exist */ - else if(needWildcardProof && (rec.d_type==QType::RRSIG || rec.d_type==QType::NSEC || rec.d_type==QType::NSEC3) && rec.d_place==DNSResourceRecord::AUTHORITY) { + else if(gatherWildcardProof && (rec.d_type==QType::RRSIG || rec.d_type==QType::NSEC || rec.d_type==QType::NSEC3) && rec.d_place==DNSResourceRecord::AUTHORITY) { ret.push_back(rec); // enjoy your DNSSEC } // for ANY answers we *must* have an authoritative answer, unless we are forwarding recursively @@ -2857,8 +2868,9 @@ bool SyncRes::processAnswer(unsigned int depth, LWResult& lwr, const DNSName& qn } bool needWildcardProof = false; + bool gatherWildcardProof = false; unsigned int wildcardLabelsCount; - *rcode = updateCacheFromRecords(depth, lwr, qname, qtype, auth, wasForwarded, ednsmask, state, needWildcardProof, wildcardLabelsCount, sendRDQuery); + *rcode = updateCacheFromRecords(depth, lwr, qname, qtype, auth, wasForwarded, ednsmask, state, needWildcardProof, gatherWildcardProof, wildcardLabelsCount, sendRDQuery); if (*rcode != RCode::NoError) { return true; } @@ -2870,7 +2882,7 @@ bool SyncRes::processAnswer(unsigned int depth, LWResult& lwr, const DNSName& qn DNSName newauth; DNSName newtarget; - bool done = processRecords(prefix, qname, qtype, auth, lwr, sendRDQuery, ret, nsset, newtarget, newauth, realreferral, negindic, state, needWildcardProof, wildcardLabelsCount); + bool done = processRecords(prefix, qname, qtype, auth, lwr, sendRDQuery, ret, nsset, newtarget, newauth, realreferral, negindic, state, needWildcardProof, gatherWildcardProof, wildcardLabelsCount); if(done){ LOG(prefix< 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, bool cacheOnly); void sanitizeRecords(const std::string& prefix, LWResult& lwr, const DNSName& qname, const QType& qtype, const DNSName& auth, bool wasForwarded, bool rdQuery); - RCode::rcodes_ updateCacheFromRecords(unsigned int depth, LWResult& lwr, const DNSName& qname, const QType& qtype, const DNSName& auth, bool wasForwarded, const boost::optional, vState& state, bool& needWildcardProof, unsigned int& wildcardLabelsCount, bool sendRDQuery); - 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 unsigned int wildcardLabelsCount); + RCode::rcodes_ updateCacheFromRecords(unsigned int depth, LWResult& lwr, const DNSName& qname, const QType& qtype, const DNSName& auth, bool wasForwarded, const boost::optional, vState& state, bool& needWildcardProof, bool& gatherWildcardProof, unsigned int& wildcardLabelsCount, bool sendRDQuery); + 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); bool doSpecialNamesResolve(const DNSName &qname, const QType &qtype, const uint16_t qclass, vector &ret); diff --git a/pdns/validate.cc b/pdns/validate.cc index c9d17c86e2..ab7fa3d9e2 100644 --- a/pdns/validate.cc +++ b/pdns/validate.cc @@ -112,6 +112,15 @@ bool denialProvesNoDelegation(const DNSName& zone, const std::vector& Labels field of the covering RRSIG RR, then the RRset and its covering RRSIG RR were created as a result of wildcard expansion." */ +bool isWildcardExpanded(unsigned int labelCount, const std::shared_ptr& sign) +{ + if (sign && sign->d_labels < labelCount) { + return true; + } + + return false; +} + static bool isWildcardExpanded(const DNSName& owner, const std::vector >& signatures) { if (signatures.empty()) { @@ -120,13 +129,29 @@ static bool isWildcardExpanded(const DNSName& owner, const std::vectord_labels < labelsCount) { + return isWildcardExpanded(labelsCount, sign); +} + +bool isWildcardExpandedOntoItself(const DNSName& owner, unsigned int labelCount, const std::shared_ptr& sign) +{ + if (owner.isWildcard() && (labelCount - 1) == sign->d_labels) { + /* this is a wildcard alright, but it has not been expanded */ return true; } - return false; } +static bool isWildcardExpandedOntoItself(const DNSName& owner, const std::vector >& signatures) +{ + if (signatures.empty()) { + return false; + } + + const auto& sign = signatures.at(0); + unsigned int labelsCount = owner.countLabels(); + return isWildcardExpandedOntoItself(owner, labelsCount, sign); +} + /* if this is a wildcard NSEC, the owner name has been modified to match the name. Make sure we use the original '*' form. */ static DNSName getNSECOwnerName(const DNSName& initialOwner, const std::vector >& signatures) @@ -381,7 +406,7 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16 /* we know that the name exists (but this qtype doesn't) so except if the answer was generated by a wildcard expansion, no wildcard could have matched (rfc4035 section 5.4 bullet 1) */ - if (!isWildcardExpanded(owner, v.second.signatures)) { + if (!isWildcardExpanded(owner, v.second.signatures) || isWildcardExpandedOntoItself(owner, v.second.signatures)) { needWildcardProof = false; } @@ -395,6 +420,7 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16 /* check if the whole NAME is denied existing */ if(isCoveredByNSEC(qname, owner, nsec->d_next)) { + LOG(qname<<" is covered "); /* if the name is an ENT and we received a NODATA answer, we are fine with a NSEC proving that the name does not exist. */ if (wantsNoDataProof && nsecProvesENT(qname, owner, nsec->d_next)) { @@ -403,15 +429,19 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16 } if (!needWildcardProof) { + LOG("and we did not need a wildcard proof"< >& signatures); bool denialProvesNoDelegation(const DNSName& zone, const std::vector& dsrecords); 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);