From e4894ce07a529e174b542d20eae327a026e171e8 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Wed, 13 Dec 2017 15:03:24 +0100 Subject: [PATCH] rec: Fix the computation of the closest encloser for positive answers When the positive answer is expanded from a wildcard with NSEC3, the closest encloser is not always parent of the qname, depending on the number of labels in the initial wildcard. --- pdns/recursordist/test-syncres_cc.cc | 17 +++++++++++------ pdns/syncres.cc | 12 +++++++----- pdns/syncres.hh | 4 ++-- pdns/validate.cc | 11 +++++++++-- pdns/validate.hh | 2 +- 5 files changed, 30 insertions(+), 16 deletions(-) diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc index b6cdc579f..e84944809 100644 --- a/pdns/recursordist/test-syncres_cc.cc +++ b/pdns/recursordist/test-syncres_cc.cc @@ -5307,7 +5307,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec3_wildcard) { setDNSSECValidation(sr, DNSSECMode::ValidateAll); primeHints(); - const DNSName target("www.powerdns.com."); + const DNSName target("www.sub.powerdns.com."); testkeysset_t keys; auto luaconfsCopy = g_luaconfs.getCopy(); @@ -5324,11 +5324,16 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec3_wildcard) { queriesCount++; if (type == QType::DS || type == QType::DNSKEY) { - if (type == QType::DS && domain == target) { + if (type == QType::DS && domain.isPartOf(DNSName("sub.powerdns.com"))) { setLWResult(res, RCode::NoError, true, false, true); addRecordToLW(res, DNSName("powerdns.com."), QType::SOA, "pdns-public-ns1.powerdns.com. pieter\\.lexis.powerdns.com. 2017032301 10800 3600 604800 3600", DNSResourceRecord::AUTHORITY, 3600); addRRSIG(keys, res->d_records, DNSName("powerdns.com."), 300); - addNSECRecordToLW(DNSName("www.powerdns.com."), DNSName("wwz.powerdns.com."), { QType::A, QType::NSEC, QType::RRSIG }, 600, res->d_records); + if (domain == DNSName("sub.powerdns.com")) { + addNSECRecordToLW(DNSName("sub.powerdns.com."), DNSName("sud.powerdns.com."), { QType::A, QType::NSEC, QType::RRSIG }, 600, res->d_records); + } + else if (domain == target) { + addNSECRecordToLW(DNSName("www.sub.powerdns.com."), DNSName("wwz.sub.powerdns.com."), { QType::A, QType::NSEC, QType::RRSIG }, 600, res->d_records); + } addRRSIG(keys, res->d_records, DNSName("powerdns.com"), 300); return 1; } @@ -5386,7 +5391,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec3_wildcard) { addNSEC3UnhashedRecordToLW(DNSName("powerdns.com."), DNSName("powerdns.com."), "whatever", { QType::A, QType::TXT, QType::RRSIG, QType::NSEC }, 600, res->d_records); addRRSIG(keys, res->d_records, DNSName("powerdns.com."), 300); /* then the next closer */ - addNSEC3NarrowRecordToLW(DNSName("www.powerdns.com."), DNSName("powerdns.com."), { QType::A, QType::TXT, QType::RRSIG, QType::NSEC }, 600, res->d_records); + addNSEC3NarrowRecordToLW(DNSName("sub.powerdns.com."), DNSName("powerdns.com."), { QType::A, QType::TXT, QType::RRSIG, QType::NSEC }, 600, res->d_records); addRRSIG(keys, res->d_records, DNSName("powerdns.com."), 300); } return 1; @@ -5401,7 +5406,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec3_wildcard) { BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), Secure); BOOST_REQUIRE_EQUAL(ret.size(), 6); - BOOST_CHECK_EQUAL(queriesCount, 9); + BOOST_CHECK_EQUAL(queriesCount, 10); /* again, to test the cache */ ret.clear(); @@ -5409,7 +5414,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec3_wildcard) { BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), Secure); BOOST_REQUIRE_EQUAL(ret.size(), 6); - BOOST_CHECK_EQUAL(queriesCount, 9); + BOOST_CHECK_EQUAL(queriesCount, 10); } BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec3_wildcard_too_many_iterations) { diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 812030209..b3fe433a7 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -1898,7 +1898,7 @@ vState SyncRes::validateRecordsWithSigs(unsigned int depth, const DNSName& qname return Bogus; } -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) +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) { tcache_t tcache; @@ -1944,6 +1944,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr if (rec.d_name == qname && rrsig->d_labels < labelCount) { LOG(prefix<d_labels; } // cerr<<"Got an RRSIG for "<d_type)<<" with name '"<& ret, set& nsset, DNSName& newtarget, DNSName& newauth, bool& realreferral, bool& negindic, vState& state, bool needWildcardProof) +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 done = false; @@ -2239,7 +2240,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co harvestNXRecords(lwr.d_records, ne, d_now.tv_sec, &lowestTTL); cspmap_t csp = harvestCSPFromNE(ne); - dState res = getDenial(csp, qname, ne.d_qtype.getCode(), false, false, false); + dState res = getDenial(csp, qname, ne.d_qtype.getCode(), false, false, false, wildcardLabelsCount); if (res != NXDOMAIN) { vState st = Bogus; if (res == INSECURE) { @@ -2483,7 +2484,8 @@ bool SyncRes::processAnswer(unsigned int depth, LWResult& lwr, const DNSName& qn } bool needWildcardProof = false; - *rcode = updateCacheFromRecords(depth, lwr, qname, qtype, auth, wasForwarded, ednsmask, state, needWildcardProof); + unsigned int wildcardLabelsCount; + *rcode = updateCacheFromRecords(depth, lwr, qname, qtype, auth, wasForwarded, ednsmask, state, needWildcardProof, wildcardLabelsCount); if (*rcode != RCode::NoError) { return true; } @@ -2495,7 +2497,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); + bool done = processRecords(prefix, qname, qtype, auth, lwr, sendRDQuery, ret, nsset, newtarget, newauth, realreferral, negindic, state, needWildcardProof, 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); - 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 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); + 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 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 doSpecialNamesResolve(const DNSName &qname, const QType &qtype, const uint16_t qclass, vector &ret); diff --git a/pdns/validate.cc b/pdns/validate.cc index f7f37e6fa..c866ccc1d 100644 --- a/pdns/validate.cc +++ b/pdns/validate.cc @@ -306,9 +306,12 @@ static bool provesNSEC3NoWildCard(DNSName wildcard, uint16_t const qtype, const useful when we have a positive answer synthetized from a wildcard and we only need to prove that the exact name does not exist. */ -dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16_t qtype, bool referralToUnsigned, bool wantsNoDataProof, bool needWildcardProof) +dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16_t qtype, bool referralToUnsigned, bool wantsNoDataProof, bool needWildcardProof, unsigned int wildcardLabelsCount) { bool nsec3Seen = false; + if (!needWildcardProof && wildcardLabelsCount == 0) { + throw std::runtime_error("Invalid wildcard labels count for the validation of a positive answer synthetized from a wildcard"); + } for(const auto& v : validrrsets) { LOG("Do have: "< 0 && closestEncloserLabelsCount > wildcardLabelsCount) { + closestEncloser.chopOff(); + closestEncloserLabelsCount--; + } } bool nextCloserFound = false; diff --git a/pdns/validate.hh b/pdns/validate.hh index 1c647c879..23ebfe877 100644 --- a/pdns/validate.hh +++ b/pdns/validate.hh @@ -71,7 +71,7 @@ vState getKeysFor(DNSRecordOracle& dro, const DNSName& zone, skeyset_t& keyset); bool getTrustAnchor(const map& anchors, const DNSName& zone, dsmap_t &res); bool haveNegativeTrustAnchor(const map& negAnchors, const DNSName& zone, std::string& reason); void validateDNSKeysAgainstDS(time_t now, const DNSName& zone, const dsmap_t& dsmap, const skeyset_t& tkeys, vector >& toSign, const vector >& sigs, skeyset_t& validkeys); -dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16_t qtype, bool referralToUnsigned, bool wantsNoDataProof, bool needsWildcardProof=true); +dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16_t qtype, bool referralToUnsigned, bool wantsNoDataProof, bool needsWildcardProof=true, unsigned int wildcardLabelsCount=0); bool isSupportedDS(const DSRecordContent& ds); DNSName getSigner(const std::vector >& signatures); bool denialProvesNoDelegation(const DNSName& zone, const std::vector& dsrecords); -- 2.40.0