From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Thu, 20 Sep 2018 12:46:11 +0000 (+0200)
Subject: rec: Keep the EDNS status of a server on FormErr with EDNS
X-Git-Tag: rec-4.1.5^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a237278908f4557cac27f2bcf822f356e33960eb;p=pdns

rec: Keep the EDNS status of a server on FormErr with EDNS

Note that the choice of DNAME in the unit test is an arbitrary
choice, we could even have used A here.

(cherry picked from commit 6fb756b6cd49d61eacf7865ce48d0edb62730710)
(cherry picked from commit d8b9d57103f1e1496767d9fac3955b1973e04302)
---

diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index 03f424ca2..b45362456 100644
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -647,6 +647,54 @@ BOOST_AUTO_TEST_CASE(test_edns_notimp_fallback) {
   BOOST_CHECK_EQUAL(queriesWithoutEDNS, 1);
 }
 
+BOOST_AUTO_TEST_CASE(test_edns_formerr_but_edns_enabled) {
+  std::unique_ptr<SyncRes> sr;
+  initSR(sr);
+
+  /* in this test, the auth answers with FormErr to an EDNS-enabled
+     query, but the response does contain EDNS so we should not mark
+     it as EDNS ignorant or intolerant.
+  */
+  size_t queriesWithEDNS = 0;
+  size_t queriesWithoutEDNS = 0;
+  std::set<ComboAddress> usedServers;
+
+  sr->setAsyncCallback([&queriesWithEDNS, &queriesWithoutEDNS, &usedServers](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, std::shared_ptr<RemoteLogger> outgoingLogger, LWResult* res, bool* chained) {
+
+      if (EDNS0Level > 0) {
+        queriesWithEDNS++;
+      }
+      else {
+        queriesWithoutEDNS++;
+      }
+      usedServers.insert(ip);
+
+      if (type == QType::DNAME) {
+        setLWResult(res, RCode::FormErr);
+        if (EDNS0Level > 0) {
+          res->d_haveEDNS = true;
+        }
+        return 1;
+      }
+
+      return 0;
+    });
+
+  primeHints();
+
+  vector<DNSRecord> ret;
+  int res = sr->beginResolve(DNSName("powerdns.com."), QType(QType::DNAME), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::ServFail);
+  BOOST_CHECK_EQUAL(ret.size(), 0);
+  BOOST_CHECK_EQUAL(queriesWithEDNS, 26);
+  BOOST_CHECK_EQUAL(queriesWithoutEDNS, 0);
+  BOOST_CHECK_EQUAL(SyncRes::getEDNSStatusesSize(), 26);
+  BOOST_CHECK_EQUAL(usedServers.size(), 26);
+  for (const auto& server : usedServers) {
+    BOOST_CHECK_EQUAL(SyncRes::getEDNSStatus(server), SyncRes::EDNSStatus::EDNSOK);
+  }
+}
+
 BOOST_AUTO_TEST_CASE(test_tc_fallback_to_tcp) {
   std::unique_ptr<SyncRes> sr;
   initSR(sr);
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index f2813b4a8..78a7ca4cf 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -479,7 +479,7 @@ int SyncRes::asyncresolveWrapper(const ComboAddress& ip, bool ednsMANDATORY, con
       return ret;
     }
     else if(mode==EDNSStatus::UNKNOWN || mode==EDNSStatus::EDNSOK || mode == EDNSStatus::EDNSIGNORANT ) {
-      if(res->d_rcode == RCode::FormErr || res->d_rcode == RCode::NotImp)  {
+      if(!res->d_haveEDNS && (res->d_rcode == RCode::FormErr || res->d_rcode == RCode::NotImp)) {
 	//	cerr<<"Downgrading to NOEDNS because of "<<RCode::to_s(res->d_rcode)<<" for query to "<<ip.toString()<<" for '"<<domain<<"'"<<endl;
         mode = EDNSStatus::NOEDNS;
         continue;