From: Peter van Dijk <peter.van.dijk@powerdns.com>
Date: Fri, 18 May 2018 15:10:18 +0000 (+0200)
Subject: implement the EDNS camel diet (draft-spacek-edns-camel-diet)
X-Git-Tag: dnsdist-1.3.1~52^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=269ac73da16469d6d52451971efbed44dfa95bdc;p=pdns

implement the EDNS camel diet (draft-spacek-edns-camel-diet)
---

diff --git a/pdns/lwres.cc b/pdns/lwres.cc
index 218bb2ad2..f97dac18e 100644
--- a/pdns/lwres.cc
+++ b/pdns/lwres.cc
@@ -239,6 +239,7 @@ int asyncresolve(const ComboAddress& ip, const DNSName& domain, int type, bool d
         logIncomingResponse(outgoingLogger, context ? context->d_initialRequestId : boost::none, uuid, ip, domain, type, qid, doTCP, len, lwr->d_rcode, lwr->d_records, queryTime);
       }
 #endif
+      lwr->d_validpacket=true;
       return 1; // this is "success", the error is set in lwr->d_rcode
     }
 
@@ -283,6 +284,7 @@ int asyncresolve(const ComboAddress& ip, const DNSName& domain, int type, bool d
       logIncomingResponse(outgoingLogger, context ? context->d_initialRequestId : boost::none, uuid, ip, domain, type, qid, doTCP, len, lwr->d_rcode, lwr->d_records, queryTime);
     }
 #endif
+    lwr->d_validpacket=true;
     return 1;
   }
   catch(std::exception &mde) {
@@ -295,6 +297,7 @@ int asyncresolve(const ComboAddress& ip, const DNSName& domain, int type, bool d
       logIncomingResponse(outgoingLogger, context ? context->d_initialRequestId : boost::none, uuid, ip, domain, type, qid, doTCP, len, lwr->d_rcode, lwr->d_records, queryTime);
     }
 #endif
+    lwr->d_validpacket=false;
     return 1; // success - oddly enough
   }
   catch(...) {
diff --git a/pdns/lwres.hh b/pdns/lwres.hh
index ad9855d76..96e931dd2 100644
--- a/pdns/lwres.hh
+++ b/pdns/lwres.hh
@@ -62,6 +62,7 @@ public:
 
   vector<DNSRecord> d_records;
   int d_rcode{0};
+  bool d_validpacket{false};
   bool d_aabit{false}, d_tcbit{false};
   uint32_t d_usec{0};
   bool d_haveEDNS{false};
diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index b959e8a04..68871d2c6 100644
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -200,12 +200,13 @@ static void setDNSSECValidation(std::unique_ptr<SyncRes>& sr, const DNSSECMode&
   g_dnssecmode = mode;
 }
 
-static void setLWResult(LWResult* res, int rcode, bool aa=false, bool tc=false, bool edns=false)
+static void setLWResult(LWResult* res, int rcode, bool aa=false, bool tc=false, bool edns=false, bool validpacket=true)
 {
   res->d_rcode = rcode;
   res->d_aabit = aa;
   res->d_tcbit = tc;
   res->d_haveEDNS = edns;
+  res->d_validpacket = validpacket;
 }
 
 static void addRecordToLW(LWResult* res, const DNSName& name, uint16_t type, const std::string& content, DNSResourceRecord::Place place=DNSResourceRecord::ANSWER, uint32_t ttl=60)
@@ -616,42 +617,6 @@ BOOST_AUTO_TEST_CASE(test_edns_formerr_fallback) {
   BOOST_CHECK_EQUAL(SyncRes::getEDNSStatus(noEDNSServer), SyncRes::EDNSStatus::NOEDNS);
 }
 
-BOOST_AUTO_TEST_CASE(test_edns_notimp_fallback) {
-  std::unique_ptr<SyncRes> sr;
-  initSR(sr);
-
-  size_t queriesWithEDNS = 0;
-  size_t queriesWithoutEDNS = 0;
-
-  sr->setAsyncCallback([&queriesWithEDNS, &queriesWithoutEDNS](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++;
-        setLWResult(res, RCode::NotImp);
-        return 1;
-      }
-
-      queriesWithoutEDNS++;
-
-      if (domain == DNSName("powerdns.com") && type == QType::A && !doTCP) {
-        setLWResult(res, 0, true, false, false);
-        addRecordToLW(res, domain, QType::A, "192.0.2.1");
-        return 1;
-      }
-
-      return 0;
-    });
-
-  primeHints();
-
-  /* fake that the NS doesn't handle EDNS, check that we fallback */
-  vector<DNSRecord> ret;
-  int res = sr->beginResolve(DNSName("powerdns.com."), QType(QType::A), QClass::IN, ret);
-  BOOST_CHECK_EQUAL(res, RCode::NoError);
-  BOOST_CHECK_EQUAL(ret.size(), 1);
-  BOOST_CHECK_EQUAL(queriesWithEDNS, 1);
-  BOOST_CHECK_EQUAL(queriesWithoutEDNS, 1);
-}
-
 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 431b5be51..694f3d4c2 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -418,7 +418,7 @@ int SyncRes::asyncresolveWrapper(const ComboAddress& ip, bool ednsMANDATORY, con
      0) UNKNOWN Unknown state 
      1) EDNS: Honors EDNS0
      2) EDNSIGNORANT: Ignores EDNS0, gives replies without EDNS0
-     3) NOEDNS: Generates FORMERR/NOTIMP on EDNS queries
+     3) NOEDNS: Generates FORMERR on EDNS queries
 
      Everybody starts out assumed to be '0'.
      If '0', send out EDNS0
@@ -477,7 +477,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_validpacket && res->d_rcode == RCode::FormErr)  {
 	//	cerr<<"Downgrading to NOEDNS because of "<<RCode::to_s(res->d_rcode)<<" for query to "<<ip.toString()<<" for '"<<domain<<"'"<<endl;
         mode = EDNSStatus::NOEDNS;
         continue;
@@ -485,7 +485,7 @@ int SyncRes::asyncresolveWrapper(const ComboAddress& ip, bool ednsMANDATORY, con
       else if(!res->d_haveEDNS) {
         if(mode != EDNSStatus::EDNSIGNORANT) {
           mode = EDNSStatus::EDNSIGNORANT;
-	  //	  cerr<<"We find that "<<ip.toString()<<" is an EDNS-ignorer for '"<<domain<<"', moving to mode 3"<<endl;
+	  //	  cerr<<"We find that "<<ip.toString()<<" is an EDNS-ignorer for '"<<domain<<"', moving to mode 2"<<endl;
 	}
       }
       else {