From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Fri, 24 Apr 2020 09:48:16 +0000 (+0200)
Subject: rec: Fix DNSSEC validation of completely empty NXDomain answers
X-Git-Tag: dnsdist-1.5.0-rc3~50^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4bba0ec04aacbec08fe585ad790e2e8e0cb7b04a;p=thirdparty%2Fpdns.git

rec: Fix DNSSEC validation of completely empty NXDomain answers

If the answer has no SOA and no NSEC inside a DNSSEC-secure zone,
we should go Bogus.
---

diff --git a/pdns/recursordist/test-syncres_cc7.cc b/pdns/recursordist/test-syncres_cc7.cc
index 0ed32254fd..d1ef7a7a0c 100644
--- a/pdns/recursordist/test-syncres_cc7.cc
+++ b/pdns/recursordist/test-syncres_cc7.cc
@@ -1403,4 +1403,57 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_nodata)
   BOOST_CHECK_EQUAL(queriesCount, 4U);
 }
 
+BOOST_AUTO_TEST_CASE(test_dnssec_bogus_nxdomain)
+{
+  std::unique_ptr<SyncRes> sr;
+  initSR(sr, true);
+
+  setDNSSECValidation(sr, DNSSECMode::ValidateAll);
+
+  primeHints();
+  const DNSName target("powerdns.com.");
+  testkeysset_t keys;
+
+  auto luaconfsCopy = g_luaconfs.getCopy();
+  luaconfsCopy.dsAnchors.clear();
+  generateKeyMaterial(DNSName("."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys, luaconfsCopy.dsAnchors);
+  generateKeyMaterial(DNSName("com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys, luaconfsCopy.dsAnchors);
+  generateKeyMaterial(DNSName("powerdns.com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys);
+  g_luaconfs.setState(luaconfsCopy);
+
+  size_t queriesCount = 0;
+
+  sr->setAsyncCallback([target, &queriesCount, keys](const ComboAddress& ip, const DNSName& domain, int type, bool doTCP, bool sendRDQuery, int EDNS0Level, struct timeval* now, boost::optional<Netmask>& srcmask, boost::optional<const ResolveContext&> context, LWResult* res, bool* chained) {
+    queriesCount++;
+
+    if (type == QType::DS || type == QType::DNSKEY) {
+      return genericDSAndDNSKEYHandler(res, domain, domain, type, keys);
+    }
+    else {
+
+      setLWResult(res, RCode::NXDomain, true, false, true);
+      return 1;
+    }
+
+    return 0;
+  });
+
+  vector<DNSRecord> ret;
+  int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NXDomain);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus);
+  BOOST_REQUIRE_EQUAL(ret.size(), 0U);
+  /* com|NS, powerdns.com|NS, powerdns.com|A */
+  BOOST_CHECK_EQUAL(queriesCount, 3U);
+
+  /* again, to test the cache */
+  ret.clear();
+  res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NXDomain);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus);
+  BOOST_REQUIRE_EQUAL(ret.size(), 0U);
+  /* we don't store empty results */
+  BOOST_CHECK_EQUAL(queriesCount, 4U);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 872878e940..fe8ec039e6 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -3396,6 +3396,10 @@ bool SyncRes::processAnswer(unsigned int depth, LWResult& lwr, const DNSName& qn
   if(lwr.d_rcode == RCode::NXDomain) {
     LOG(prefix<<qname<<": status=NXDOMAIN, we are done "<<(negindic ? "(have negative SOA)" : "")<<endl);
 
+    if (state == Secure && (lwr.d_aabit || sendRDQuery) && !negindic) {
+      updateValidationState(state, Bogus);
+    }
+
     if(d_doDNSSEC)
       addNXNSECS(ret, lwr.d_records);