backport 10555 to 4.4

diff --git a/pdns/recursordist/test-syncres_cc3.cc b/pdns/recursordist/test-syncres_cc3.cc
index 4fbc6d4aec12047f698226b3b350d72fb9ee5d92..7512194b9f5c6509a3b2be07e2d3e6e3a21ebc0e 100644 (file)
--- a/pdns/recursordist/test-syncres_cc3.cc
+++ b/pdns/recursordist/test-syncres_cc3.cc
@@ -106,8 +106,8 @@ BOOST_AUTO_TEST_CASE(test_unauth_any)
 
   vector<DNSRecord> ret;
   int res = sr->beginResolve(target, QType(QType::ANY), QClass::IN, ret);
-  BOOST_CHECK_EQUAL(res, RCode::ServFail);
-  BOOST_CHECK_EQUAL(ret.size(), 0U);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(ret.size(), 1U);
 }
 
 static void test_no_data_f(bool qmin)
@@ -310,14 +310,14 @@ BOOST_AUTO_TEST_CASE(test_answer_no_aa)
 
   vector<DNSRecord> ret;
   int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
-  BOOST_CHECK_EQUAL(res, RCode::ServFail);
-  BOOST_CHECK_EQUAL(ret.size(), 0U);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(ret.size(), 1U);
 
   /* check that the record in the answer section has not been cached */
   const ComboAddress who;
   vector<DNSRecord> cached;
   vector<std::shared_ptr<RRSIGRecordContent>> signatures;
-  BOOST_REQUIRE_EQUAL(s_RC->get(now, target, QType(QType::A), false, &cached, who, boost::none, &signatures), -1);
+  BOOST_REQUIRE_GT(s_RC->get(now, target, QType(QType::A), false, &cached, who), 0);
 }
 
 BOOST_AUTO_TEST_CASE(test_special_types)

diff --git a/pdns/recursordist/test-syncres_cc4.cc b/pdns/recursordist/test-syncres_cc4.cc
index 908c8b6304c3f3dbb9135ffb725a35c908b0ee66..32ded0f2fd19f378f0660175f02567b2bd7becf2 100644 (file)
--- a/pdns/recursordist/test-syncres_cc4.cc
+++ b/pdns/recursordist/test-syncres_cc4.cc
@@ -1111,6 +1111,82 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_no_rrsig)
   BOOST_CHECK_EQUAL(queriesCount, 1U);
 }
 
+BOOST_AUTO_TEST_CASE(test_dnssec_bogus_no_rrsig_noaa)
+{
+  std::unique_ptr<SyncRes> sr;
+  initSR(sr, true);
+
+  setDNSSECValidation(sr, DNSSECMode::ValidateAll);
+
+  primeHints();
+  const DNSName target(".");
+  testkeysset_t keys;
+
+  auto luaconfsCopy = g_luaconfs.getCopy();
+  luaconfsCopy.dsAnchors.clear();
+  generateKeyMaterial(target, DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys, luaconfsCopy.dsAnchors);
+  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 (domain == target && type == QType::NS) {
+
+      /* We are not setting AA! */
+      setLWResult(res, 0, false, false, true);
+      char addr[] = "a.root-servers.net.";
+      for (char idx = 'a'; idx <= 'm'; idx++) {
+        addr[0] = idx;
+        addRecordToLW(res, domain, QType::NS, std::string(addr), DNSResourceRecord::ANSWER, 86400);
+      }
+
+      /* No RRSIG */
+
+      addRecordToLW(res, "a.root-servers.net.", QType::A, "198.41.0.4", DNSResourceRecord::ADDITIONAL, 3600);
+      addRecordToLW(res, "a.root-servers.net.", QType::AAAA, "2001:503:ba3e::2:30", DNSResourceRecord::ADDITIONAL, 3600);
+
+      return 1;
+    }
+    else if (domain == target && type == QType::DNSKEY) {
+
+      setLWResult(res, 0, true, false, true);
+
+      addDNSKEY(keys, domain, 300, res->d_records);
+      addRRSIG(keys, res->d_records, domain, 300);
+
+      return 1;
+    }
+
+    return 0;
+  });
+
+  SyncRes::s_maxcachettl = 86400;
+  SyncRes::s_maxbogusttl = 3600;
+
+  vector<DNSRecord> ret;
+  int res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), vState::BogusNoRRSIG);
+  /* 13 NS + 0 RRSIG */
+  BOOST_REQUIRE_EQUAL(ret.size(), 13U);
+  /* no RRSIG so no query for DNSKEYs */
+  BOOST_CHECK_EQUAL(queriesCount, 1U);
+
+  /* again, to test the cache */
+  ret.clear();
+  res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), vState::BogusNoRRSIG);
+  BOOST_REQUIRE_EQUAL(ret.size(), 13U);
+  /* check that we capped the TTL to max-cache-bogus-ttl */
+  for (const auto& record : ret) {
+    BOOST_CHECK_LE(record.d_ttl, SyncRes::s_maxbogusttl);
+  }
+  BOOST_CHECK_EQUAL(queriesCount, 1U);
+}
+
 BOOST_AUTO_TEST_CASE(test_dnssec_insecure_unknown_ds_algorithm)
 {
   std::unique_ptr<SyncRes> sr;

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 625478a474354dbb0314577856ef04bfa19d05c6..0761de6d803976231cf4d21d10af2284f226fe0b 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -2789,6 +2789,46 @@ vState SyncRes::validateRecordsWithSigs(unsigned int depth, const DNSName& qname
   return state;
 }
 
+/* This function will check whether the answer should have the AA bit set, and will set if it should be set and isn't.
+   This is unfortunately needed to deal with very crappy so-called DNS servers */
+void SyncRes::fixupAnswer(const std::string& prefix, LWResult& lwr, const DNSName& qname, const QType qtype, const DNSName& auth, bool wasForwarded, bool rdQuery)
+{
+  const bool wasForwardRecurse = wasForwarded && rdQuery;
+
+  if (wasForwardRecurse || lwr.d_aabit) {
+    /* easy */
+    return;
+  }
+
+  for (const auto& rec : lwr.d_records) {
+
+    if (rec.d_type == QType::OPT) {
+      continue;
+    }
+
+    if (rec.d_class != QClass::IN) {
+      continue;
+    }
+
+    if (rec.d_type == QType::ANY) {
+      continue;
+    }
+
+    if (rec.d_place == DNSResourceRecord::ANSWER && (QType(rec.d_type) == qtype || rec.d_type == QType::CNAME || qtype == QType::ANY) && rec.d_name == qname && rec.d_name.isPartOf(auth)) {
+      /* This is clearly an answer to the question we were asking, from an authoritative server that is allowed to send it.
+         We are going to assume this server is broken and does not know it should set the AA bit, even though it is DNS 101 */
+      LOG(prefix<<"Received a record for "<<rec.d_name<<"|"<<DNSRecordContent::NumberToType(rec.d_type)<<" in the answer section from "<<auth<<", without the AA bit set. Assuming this server is clueless and setting the AA bit."<<endl);
+      lwr.d_aabit = true;
+      return;
+    }
+
+    if (rec.d_place != DNSResourceRecord::ANSWER) {
+      /* we have scanned all the records in the answer section, if any, we are done */
+      return;
+    }
+  }
+}
+
 static bool allowAdditionalEntry(std::unordered_set<DNSName>& allowedAdditionals, const DNSRecord& rec)
 {
   switch(rec.d_type) {
@@ -2956,6 +2996,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
     prefix.append(depth, ' ');
   }
 
+  fixupAnswer(prefix, lwr, qname, qtype, auth, wasForwarded, rdQuery);
   sanitizeRecords(prefix, lwr, qname, qtype, auth, wasForwarded, rdQuery);
 
   std::vector<std::shared_ptr<DNSRecord>> authorityRecs;

diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 547946832ae4bd8c80c5ef248c8a8afa37a1d120..3ec00af6667641330d7204b61824770d1b139fd1 100644 (file)
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -850,6 +850,9 @@ private:
 
   vector<ComboAddress> retrieveAddressesForNS(const std::string& prefix, const DNSName& qname, vector<std::pair<DNSName, float>>::const_iterator& tns, const unsigned int depth, set<GetBestNSAnswer>& beenthere, const vector<std::pair<DNSName, float>>& rnameservers, NsSet& nameservers, bool& sendRDQuery, bool& pierceDontQuery, bool& flawedNSSet, bool cacheOnly, unsigned int& addressQueriesForNS);
 
+/* This function will check whether the answer should have the AA bit set, and will set if it should be set and isn't.
+   This is unfortunately needed to deal with very crappy so-called DNS servers */
+  void fixupAnswer(const std::string& prefix, LWResult& lwr, const DNSName& qname, const QType qtype, const DNSName& auth, bool wasForwarded, bool rdQuery);
   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<Netmask>, vState& state, bool& needWildcardProof, bool& gatherWildcardProof, unsigned int& wildcardLabelsCount, bool sendRDQuery, const ComboAddress& remoteIP);
   bool processRecords(const std::string& prefix, const DNSName& qname, const QType& qtype, const DNSName& auth, LWResult& lwr, const bool sendRDQuery, vector<DNSRecord>& ret, set<DNSName>& nsset, DNSName& newtarget, DNSName& newauth, bool& realreferral, bool& negindic, vState& state, const bool needWildcardProof, const bool gatherwildcardProof, const unsigned int wildcardLabelsCount, int& rcode, unsigned int depth);