rec: Throttle servers sending invalid data and rcodes

diff --git a/pdns/recursordist/test-syncres_cc1.cc b/pdns/recursordist/test-syncres_cc1.cc
index 72286460114cbfd86bdb0427849229c49dc4bdbe..6c298bf8cfe66b2ca37cf4379ba84a2e8c442b72 100644 (file)
--- a/pdns/recursordist/test-syncres_cc1.cc
+++ b/pdns/recursordist/test-syncres_cc1.cc
@@ -414,6 +414,242 @@ BOOST_AUTO_TEST_CASE(test_all_nss_network_error)
   }
 }
 
+BOOST_AUTO_TEST_CASE(test_all_nss_send_tc_then_garbage_over_tcp) {
+  std::unique_ptr<SyncRes> sr;
+  initSR(sr);
+
+  primeHints();
+
+  std::set<ComboAddress> downServers;
+
+  sr->setAsyncCallback([&downServers](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) {
+
+    if (isRootServer(ip)) {
+      setLWResult(res, 0, false, false, true);
+      addRecordToLW(res, "lock-up.", QType::NS, "a.gtld-servers.net.", DNSResourceRecord::AUTHORITY, 172800);
+      addRecordToLW(res, "a.gtld-servers.net.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600);
+      addRecordToLW(res, "a.gtld-servers.net.", QType::AAAA, "2001:DB8::1", DNSResourceRecord::ADDITIONAL, 3600);
+      return LWResult::Result::Success;
+    }
+
+    if (!doTCP) {
+      setLWResult(res, 0, false, true, false);
+      return LWResult::Result::Success;
+    }
+    else {
+      downServers.insert(ip);
+
+      setLWResult(res, RCode::FormErr, false, false, false);
+      res->d_validpacket = false;
+      return LWResult::Result::Success;
+    }
+  });
+
+  DNSName target("www.lock-up.");
+
+  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(downServers.size(), 2U);
+
+  for (const auto& server : downServers) {
+    BOOST_CHECK(SyncRes::isThrottled(time(nullptr), server, target, QType::A));
+    BOOST_CHECK_EQUAL(SyncRes::getNSSpeed(DNSName("a.gtld-servers.net."), server), 1000000U);
+  }
+}
+
+BOOST_AUTO_TEST_CASE(test_all_nss_send_garbage_over_udp) {
+  std::unique_ptr<SyncRes> sr;
+  initSR(sr);
+
+  primeHints();
+
+  std::set<ComboAddress> downServers;
+  size_t queriesCount = 0;
+
+  sr->setAsyncCallback([&queriesCount, &downServers](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) {
+
+    if (isRootServer(ip)) {
+      setLWResult(res, 0, false, false, true);
+      addRecordToLW(res, "lock-up.", QType::NS, "a.gtld-servers.net.", DNSResourceRecord::AUTHORITY, 172800);
+      addRecordToLW(res, "a.gtld-servers.net.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600);
+      addRecordToLW(res, "a.gtld-servers.net.", QType::AAAA, "2001:DB8::1", DNSResourceRecord::ADDITIONAL, 3600);
+      return LWResult::Result::Success;
+    }
+
+    ++queriesCount;
+    downServers.insert(ip);
+
+    setLWResult(res, RCode::FormErr, false, false, false);
+    res->d_validpacket = false;
+    return LWResult::Result::Success;
+  });
+
+  DNSName target("www.lock-up.");
+
+  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(downServers.size(), 2U);
+  /* two queries with EDNS, that's it */
+  BOOST_CHECK_EQUAL(queriesCount, 2U);
+
+  for (const auto& server : downServers) {
+    BOOST_CHECK(SyncRes::isThrottled(time(nullptr), server, target, QType::A));
+    BOOST_CHECK_EQUAL(SyncRes::getNSSpeed(DNSName("a.gtld-servers.net."), server), 1000000U);
+    BOOST_CHECK_EQUAL(SyncRes::getEDNSStatus(server), SyncRes::EDNSStatus::EDNSIGNORANT);
+  }
+}
+
+BOOST_AUTO_TEST_CASE(test_regular_ns_send_refused) {
+  std::unique_ptr<SyncRes> sr;
+  initSR(sr);
+
+  primeHints();
+
+  std::set<ComboAddress> downServers;
+  size_t queriesCount = 0;
+
+  sr->setAsyncCallback([&queriesCount, &downServers](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) {
+
+    if (isRootServer(ip)) {
+      setLWResult(res, 0, false, false, true);
+      addRecordToLW(res, "refused.", QType::NS, "a.gtld-servers.net.", DNSResourceRecord::AUTHORITY, 172800);
+      addRecordToLW(res, "a.gtld-servers.net.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600);
+      addRecordToLW(res, "a.gtld-servers.net.", QType::AAAA, "2001:DB8::1", DNSResourceRecord::ADDITIONAL, 3600);
+      return LWResult::Result::Success;
+    }
+
+    ++queriesCount;
+    downServers.insert(ip);
+
+    setLWResult(res, RCode::Refused, false, false, true);
+
+    return LWResult::Result::Success;
+  });
+
+  DNSName target("www.refused.");
+
+  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(downServers.size(), 2U);
+  BOOST_CHECK_EQUAL(queriesCount, 2U);
+
+  for (const auto& server : downServers) {
+    /* same as any other server */
+    BOOST_CHECK(SyncRes::isThrottled(time(nullptr), server, target, QType::A));
+    BOOST_CHECK_EQUAL(SyncRes::getNSSpeed(DNSName("a.gtld-servers.net."), server), 0U);
+    BOOST_CHECK_EQUAL(SyncRes::getEDNSStatus(server), SyncRes::EDNSStatus::EDNSOK);
+  }
+}
+
+BOOST_AUTO_TEST_CASE(test_forward_ns_send_refused) {
+  std::unique_ptr<SyncRes> sr;
+  initSR(sr);
+
+  primeHints();
+
+  std::set<ComboAddress> downServers;
+  size_t queriesCount = 0;
+
+  const DNSName target("www.refused.");
+
+  SyncRes::AuthDomain ad;
+  const std::vector<ComboAddress> forwardedNSs { ComboAddress("192.0.2.42:53"), ComboAddress("192.0.2.43:53") };
+  ad.d_rdForward = false;
+  ad.d_servers = forwardedNSs;
+  (*SyncRes::t_sstorage.domainmap)[target] = ad;
+
+  sr->setAsyncCallback([&queriesCount, &downServers](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) {
+
+    if (isRootServer(ip)) {
+      setLWResult(res, 0, false, false, true);
+      addRecordToLW(res, "refused.", QType::NS, "a.gtld-servers.net.", DNSResourceRecord::AUTHORITY, 172800);
+      addRecordToLW(res, "a.gtld-servers.net.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600);
+      addRecordToLW(res, "a.gtld-servers.net.", QType::AAAA, "2001:DB8::1", DNSResourceRecord::ADDITIONAL, 3600);
+      return LWResult::Result::Success;
+    }
+
+    ++queriesCount;
+    downServers.insert(ip);
+
+    setLWResult(res, RCode::Refused, false, false, true);
+
+    return LWResult::Result::Success;
+  });
+
+  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(), 0);
+  BOOST_CHECK_EQUAL(downServers.size(), 2U);
+  BOOST_CHECK_EQUAL(queriesCount, 2U);
+
+  for (const auto& server : forwardedNSs) {
+    BOOST_CHECK_EQUAL(downServers.count(server), 1U);
+    /* same as any other server */
+    BOOST_CHECK(SyncRes::isThrottled(time(nullptr), server, target, QType::A));
+    BOOST_CHECK_EQUAL(SyncRes::getNSSpeed(DNSName("a.gtld-servers.net."), server), 0U);
+    BOOST_CHECK_EQUAL(SyncRes::getEDNSStatus(server), SyncRes::EDNSStatus::EDNSOK);
+  }
+}
+
+BOOST_AUTO_TEST_CASE(test_forward_ns_send_servfail) {
+  std::unique_ptr<SyncRes> sr;
+  initSR(sr);
+
+  primeHints();
+
+  std::set<ComboAddress> downServers;
+  size_t queriesCount = 0;
+
+  const DNSName target("www.refused.");
+
+  SyncRes::AuthDomain ad;
+  const std::vector<ComboAddress> forwardedNSs { ComboAddress("192.0.2.42:53"), ComboAddress("192.0.2.43:53") };
+  ad.d_rdForward = false;
+  ad.d_servers = forwardedNSs;
+  (*SyncRes::t_sstorage.domainmap)[DNSName("refused.")] = ad;
+
+  sr->setAsyncCallback([&queriesCount, &downServers](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) {
+
+    if (isRootServer(ip)) {
+      setLWResult(res, 0, false, false, true);
+      addRecordToLW(res, "refused.", QType::NS, "a.gtld-servers.net.", DNSResourceRecord::AUTHORITY, 172800);
+      addRecordToLW(res, "a.gtld-servers.net.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600);
+      addRecordToLW(res, "a.gtld-servers.net.", QType::AAAA, "2001:DB8::1", DNSResourceRecord::ADDITIONAL, 3600);
+      return LWResult::Result::Success;
+    }
+
+    ++queriesCount;
+    downServers.insert(ip);
+
+    setLWResult(res, RCode::ServFail, false, false, true);
+
+    return LWResult::Result::Success;
+  });
+
+  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(), 0);
+  BOOST_CHECK_EQUAL(downServers.size(), 2U);
+  BOOST_CHECK_EQUAL(queriesCount, 2U);
+
+  for (const auto& server : forwardedNSs) {
+    BOOST_CHECK_EQUAL(downServers.count(server), 1U);
+    /* on servfail from a server we forward to we only increase the NS speed so
+       that a different server might be tried instead, but we don't throttle */
+    BOOST_CHECK(!SyncRes::isThrottled(time(nullptr), server, target, QType::A));
+    BOOST_CHECK_EQUAL(SyncRes::getNSSpeed(DNSName(server.toStringWithPort()), server), 1000000U);
+    BOOST_CHECK_EQUAL(SyncRes::getEDNSStatus(server), SyncRes::EDNSStatus::EDNSOK);
+  }
+}
+
 BOOST_AUTO_TEST_CASE(test_only_one_ns_up_resolving_itself_with_glue)
 {
   std::unique_ptr<SyncRes> sr;

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 172c20d021f893b29d2108147341a6ae2debfbf0..3fb329d3f4ea2e6ae90ac30dd3eb806563cb46e8 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -628,10 +628,11 @@ LWResult::Result SyncRes::asyncresolveWrapper(const ComboAddress& ip, bool ednsM
         t_sstorage.ednsstatus.setMode(ind, ednsstatus, EDNSStatus::EDNSOK);
        //      cerr<<"We find that "<<ip.toString()<<" is EDNS OK!"<<endl;
       }
-      
     }
-    if (oldmode != *mode || !ednsstatus->modeSetAt)
+
+    if (oldmode != *mode || !ednsstatus->modeSetAt) {
       t_sstorage.ednsstatus.setTS(ind, ednsstatus, d_now.tv_sec);
+    }
     //    cerr<<"Result: ret="<<ret<<", EDNS-level: "<<EDNSLevel<<", haveEDNS: "<<res->d_haveEDNS<<", new mode: "<<mode<<endl;  
     return LWResult::Result::Success;
   }
@@ -3506,7 +3507,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
   return done;
 }
 
-bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname, const QType& qtype, LWResult& lwr, boost::optional<Netmask>& ednsmask, const DNSName& auth, bool const sendRDQuery, const DNSName& nsName, const ComboAddress& remoteIP, bool doTCP, bool* truncated)
+bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname, const QType& qtype, LWResult& lwr, boost::optional<Netmask>& ednsmask, const DNSName& auth, bool const sendRDQuery, const bool wasForwarded, const DNSName& nsName, const ComboAddress& remoteIP, bool doTCP, bool* truncated)
 {
   bool chained = false;
   LWResult::Result resolveret = LWResult::Result::Success;
@@ -3622,14 +3623,41 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname,
     return false;
   }
 
-  /* we got an answer */
-  if(lwr.d_rcode==RCode::ServFail || lwr.d_rcode==RCode::Refused) {
-    LOG(prefix<<qname<<": "<<nsName<<" ("<<remoteIP.toString()<<") returned a "<< (lwr.d_rcode==RCode::ServFail ? "ServFail" : "Refused") << ", trying sibling IP or NS"<<endl);
+  if (lwr.d_validpacket == false) {
+    LOG(prefix<<qname<<": "<<nsName<<" ("<<remoteIP.toString()<<") returned a packet we could not parse over " << (doTCP ? "TCP" : "UDP") << ", trying sibling IP or NS"<<endl);
     if (!chained && !dontThrottle) {
-      t_sstorage.throttle.throttle(d_now.tv_sec, boost::make_tuple(remoteIP, qname, qtype.getCode()), 60, 3);
+
+      // let's make sure we prefer a different server for some time, if there is one available
+      t_sstorage.nsSpeeds[nsName.empty()? DNSName(remoteIP.toStringWithPort()) : nsName].submit(remoteIP, 1000000, d_now); // 1 sec
+
+      if (doTCP) {
+        // we can be more heavy-handed over TCP
+        t_sstorage.throttle.throttle(d_now.tv_sec, boost::make_tuple(remoteIP, qname, qtype.getCode()), 60, 10);
+      }
+      else {
+        t_sstorage.throttle.throttle(d_now.tv_sec, boost::make_tuple(remoteIP, qname, qtype.getCode()), 10, 2);
+      }
     }
     return false;
   }
+  else {
+    /* we got an answer */
+    if (lwr.d_rcode != RCode::NoError && lwr.d_rcode != RCode::NXDomain) {
+      LOG(prefix<<qname<<": "<<nsName<<" ("<<remoteIP.toString()<<") returned a "<< RCode::to_s(lwr.d_rcode) << ", trying sibling IP or NS"<<endl);
+      if (!chained && !dontThrottle) {
+        if (wasForwarded && lwr.d_rcode == RCode::ServFail) {
+          // rather than throttling what could be the only server we have for this destination, let's make sure we try a different one if there is one available
+          // on the other hand, we might keep hammering a server under attack if there is no other alternative, or the alternative is overwhelmed as well, but
+          // at the very least we will detect that if our packets stop being answered
+          t_sstorage.nsSpeeds[nsName.empty()? DNSName(remoteIP.toStringWithPort()) : nsName].submit(remoteIP, 1000000, d_now); // 1 sec
+        }
+        else {
+          t_sstorage.throttle.throttle(d_now.tv_sec, boost::make_tuple(remoteIP, qname, qtype.getCode()), 60, 3);
+        }
+      }
+      return false;
+    }
+  }
 
   /* this server sent a valid answer, mark it backup up if it was down */
   if(s_serverdownmaxfails > 0) {
@@ -3956,11 +3984,11 @@ int SyncRes::doResolveAt(NsSet &nameservers, DNSName auth, bool flawedNSSet, con
           }
 
           bool truncated = false;
-          bool gotAnswer = doResolveAtThisIP(prefix, qname, qtype, lwr, ednsmask, auth, sendRDQuery,
+          bool gotAnswer = doResolveAtThisIP(prefix, qname, qtype, lwr, ednsmask, auth, sendRDQuery, wasForwarded,
                                              tns->first, *remoteIP, false, &truncated);
           if (gotAnswer && truncated ) {
             /* retry, over TCP this time */
-            gotAnswer = doResolveAtThisIP(prefix, qname, qtype, lwr, ednsmask, auth, sendRDQuery,
+            gotAnswer = doResolveAtThisIP(prefix, qname, qtype, lwr, ednsmask, auth, sendRDQuery, wasForwarded,
                                           tns->first, *remoteIP, true, &truncated);
           }
 

diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 1880ba12bd2de17ec3ecb5dda7cd8351287039d0..b38203bada4df1a6dca38973aec9573f716e9a2f 100644 (file)
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -493,6 +493,10 @@ public:
   {
     t_sstorage.nsSpeeds.clear();
   }
+  static float getNSSpeed(const DNSName& server, const ComboAddress& ca)
+  {
+    return t_sstorage.nsSpeeds[server].d_collection[ca].peek();
+  }
   static EDNSStatus::EDNSMode getEDNSStatus(const ComboAddress& server)
   {
     const auto& it = t_sstorage.ednsstatus.find(server);
@@ -804,7 +808,7 @@ private:
 
   int doResolveAt(NsSet &nameservers, DNSName auth, bool flawedNSSet, const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret,
                   unsigned int depth, set<GetBestNSAnswer>&beenthere, vState& state, StopAtDelegation* stopAtDelegation);
-  bool doResolveAtThisIP(const std::string& prefix, const DNSName& qname, const QType& qtype, LWResult& lwr, boost::optional<Netmask>& ednsmask, const DNSName& auth, bool const sendRDQuery, const DNSName& nsName, const ComboAddress& remoteIP, bool doTCP, bool* truncated);
+  bool doResolveAtThisIP(const std::string& prefix, const DNSName& qname, const QType& qtype, LWResult& lwr, boost::optional<Netmask>& ednsmask, const DNSName& auth, bool const sendRDQuery, const bool wasForwarded, const DNSName& nsName, const ComboAddress& remoteIP, bool doTCP, bool* truncated);
   bool processAnswer(unsigned int depth, LWResult& lwr, const DNSName& qname, const QType& qtype, DNSName& auth, bool wasForwarded, const boost::optional<Netmask> ednsmask, bool sendRDQuery, NsSet &nameservers, std::vector<DNSRecord>& ret, const DNSFilterEngine& dfe, bool* gotNewServers, int* rcode, vState& state);
 
   int doResolve(const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, unsigned int depth, set<GetBestNSAnswer>& beenthere, vState& state);