rec: Do not assume the records are in a particular order when determining if an answer is

NODATA.

(cherry picked from commit fa5f61e94e1bd354d42923a844c59b3be232c29f)

diff --git a/pdns/recursordist/pdns_recursor.cc b/pdns/recursordist/pdns_recursor.cc
index 95ea4fd7d6739058b639c48ebb0cd5fcf09ea290..7d24f72a23c33972cc848a82f37b90a2db8bb8a5 100644 (file)
--- a/pdns/recursordist/pdns_recursor.cc
+++ b/pdns/recursordist/pdns_recursor.cc
@@ -776,30 +776,12 @@ int getFakePTRRecords(const DNSName& qname, vector<DNSRecord>& ret)
   return rcode;
 }
 
-static bool answerIsNOData(uint16_t requestedType, int rcode, const std::vector<DNSRecord>& records)
-{
-  if (rcode != RCode::NoError) {
-    return false;
-  }
-  for (const auto& rec : records) {
-    if (rec.d_place != DNSResourceRecord::ANSWER) {
-      /* no records in the answer section */
-      return true;
-    }
-    if (rec.d_type == requestedType) {
-      /* we have a record, of the right type, in the right section */
-      return false;
-    }
-  }
-  return true;
-}
-
 // RFC 6147 section 5.1 all rcodes except NXDomain should be candidate for dns64
 // for NoError, check if it is NoData
 static bool dns64Candidate(uint16_t requestedType, int rcode, const std::vector<DNSRecord>& records)
 {
   if (rcode == RCode::NoError) {
-    return answerIsNOData(requestedType, rcode, records);
+    return SyncRes::answerIsNOData(requestedType, rcode, records);
   }
   return rcode != RCode::NXDomain;
 }
@@ -1298,7 +1280,7 @@ void startDoResolve(void* p) // NOLINT(readability-function-cognitive-complexity
       bool luaHookHandled = false;
       if (dc->d_luaContext) {
         PolicyResult policyResult = PolicyResult::NoAction;
-        if (answerIsNOData(dc->d_mdp.d_qtype, res, ret)) {
+        if (SyncRes::answerIsNOData(dc->d_mdp.d_qtype, res, ret)) {
           if (dc->d_luaContext->nodata(dq, res, sr.d_eventTrace)) {
             luaHookHandled = true;
             shouldNotValidate = true;

diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc
index f5efdfcb26bbba359da4716b6b3ed1b8a79c8d1a..6f83d007476267ad8ff2b49091321cd8639f1d44 100644 (file)
--- a/pdns/recursordist/syncres.cc
+++ b/pdns/recursordist/syncres.cc
@@ -5953,3 +5953,26 @@ int SyncRes::getRootNS(struct timeval now, asyncresolve_t asyncCallback, unsigne
   }
   return res;
 }
+
+bool SyncRes::answerIsNOData(uint16_t requestedType, int rcode, const std::vector<DNSRecord>& records)
+{
+  if (rcode != RCode::NoError) {
+    return false;
+  }
+
+  // NOLINTNEXTLINE(readability-use-anyofallof)
+  for (const auto& rec : records) {
+    if (rec.d_place == DNSResourceRecord::ANSWER && rec.d_type == requestedType) {
+      /* we have a record, of the right type, in the right section */
+      return false;
+    }
+  }
+  return true;
+#if 0
+  // This code should be equivalent to the code above, clang-tidy prefers any_of()
+  // I have doubts if that is easier to read
+  return !std::any_of(records.begin(), records.end(), [=](const DNSRecord& rec) {
+    return rec.d_place == DNSResourceRecord::ANSWER && rec.d_type == requestedType;
+  });
+#endif
+}

diff --git a/pdns/recursordist/syncres.hh b/pdns/recursordist/syncres.hh
index 2b3a3082d1122c0288771c76f44bf2f6e5db6b98..c12edd807415c705e59eff3a277f43e374a05a2e 100644 (file)
--- a/pdns/recursordist/syncres.hh
+++ b/pdns/recursordist/syncres.hh
@@ -498,6 +498,8 @@ public:
     return false;
   }
 
+  static bool answerIsNOData(uint16_t requestedType, int rcode, const std::vector<DNSRecord>& records);
+
   static thread_local ThreadLocalStorage t_sstorage;
 
   static pdns::stat_t s_ecsqueries;

diff --git a/pdns/recursordist/test-syncres_cc10.cc b/pdns/recursordist/test-syncres_cc10.cc
index 4353d843fc443656317f74d97b28f782b622db5c..474c8cf8f458732650c5264fa4b0cf1dd6c4f2f7 100644 (file)
--- a/pdns/recursordist/test-syncres_cc10.cc
+++ b/pdns/recursordist/test-syncres_cc10.cc
@@ -1958,4 +1958,39 @@ BOOST_AUTO_TEST_CASE(test_locked_nonauth_update_to_auth)
   BOOST_CHECK_GT(secondTTL, 0);
 }
 
+BOOST_AUTO_TEST_CASE(test_nodata_ok)
+{
+  vector<DNSRecord> vec;
+  vec.emplace_back("nz.compass.com", nullptr, QType::CNAME, QClass::IN, 60, 0, DNSResourceRecord::ANSWER);
+  vec.emplace_back("nz.compass.com", nullptr, QType::RRSIG, QClass::IN, 60, 0, DNSResourceRecord::ANSWER);
+  vec.emplace_back("kslicmitv6qe1behk70g8q7e572vabp0.kompass.com", nullptr, QType::NSEC3, QClass::IN, 60, 0, DNSResourceRecord::AUTHORITY);
+  vec.emplace_back("kslicmitv6qe1behk70g8q7e572vabp0.kompass.com", nullptr, QType::RRSIG, QClass::IN, 60, 0, DNSResourceRecord::AUTHORITY);
+
+  BOOST_CHECK(SyncRes::answerIsNOData(QType::A, RCode::NoError, vec));
+}
+
+BOOST_AUTO_TEST_CASE(test_nodata_not)
+{
+  vector<DNSRecord> vec;
+  vec.emplace_back("kc-pro.westeurope.cloudapp.azure.com", nullptr, QType::A, QClass::IN, 60, 0, DNSResourceRecord::ANSWER);
+  vec.emplace_back("nz.compass.com", nullptr, QType::CNAME, QClass::IN, 60, 0, DNSResourceRecord::ANSWER);
+  vec.emplace_back("nz.compass.com", nullptr, QType::RRSIG, QClass::IN, 60, 0, DNSResourceRecord::ANSWER);
+  vec.emplace_back("kslicmitv6qe1behk70g8q7e572vabp0.kompass.com", nullptr, QType::NSEC3, QClass::IN, 60, 0, DNSResourceRecord::AUTHORITY);
+  vec.emplace_back("kslicmitv6qe1behk70g8q7e572vabp0.kompass.com", nullptr, QType::RRSIG, QClass::IN, 60, 0, DNSResourceRecord::AUTHORITY);
+
+  BOOST_CHECK(!SyncRes::answerIsNOData(QType::A, RCode::NoError, vec));
+}
+
+BOOST_AUTO_TEST_CASE(test_nodata_out_of_order)
+{
+  vector<DNSRecord> vec;
+  vec.emplace_back("nz.compass.com", nullptr, QType::CNAME, QClass::IN, 60, 0, DNSResourceRecord::ANSWER);
+  vec.emplace_back("nz.compass.com", nullptr, QType::RRSIG, QClass::IN, 60, 0, DNSResourceRecord::ANSWER);
+  vec.emplace_back("kslicmitv6qe1behk70g8q7e572vabp0.kompass.com", nullptr, QType::NSEC3, QClass::IN, 60, 0, DNSResourceRecord::AUTHORITY);
+  vec.emplace_back("kslicmitv6qe1behk70g8q7e572vabp0.kompass.com", nullptr, QType::RRSIG, QClass::IN, 60, 0, DNSResourceRecord::AUTHORITY);
+  vec.emplace_back("kc-pro.westeurope.cloudapp.azure.com", nullptr, QType::A, QClass::IN, 60, 0, DNSResourceRecord::ANSWER);
+
+  BOOST_CHECK(!SyncRes::answerIsNOData(QType::A, RCode::NoError, vec));
+}
+
 BOOST_AUTO_TEST_SUITE_END()