Merge pull request #4882 from rgacogne/spurious-rrs

Don't parse spurious RRs in queries when we don't need them

diff --git a/docs/markdown/recursor/settings.md b/docs/markdown/recursor/settings.md
index 220177b5b0dbfb3eb297d47624df4c9546439104..95f4b57aa598f8883c330ad77eabc58825ea62f3 100644 (file)
--- a/docs/markdown/recursor/settings.md
+++ b/docs/markdown/recursor/settings.md
@@ -616,6 +616,14 @@ In practice, caches can become saturated with hundreds of thousands of hosts
 which are tried only once. This setting, which defaults to 3600 seconds, puts a
 maximum on the amount of time negative entries are cached.
 
+## `max-recursion-depth`
+* Integer
+* Default: 40 (since 4.1.0), unlimited (before 4.1.0)
+
+Total maximum number of internal recursion calls the server may use to answer
+a single query. 0 means unlimited. The value of `stack-size` should be increased
+together with this one to prevent the stack from overflowing.
+
 ## `max-tcp-clients`
 * Integer
 * Default: 128
@@ -639,7 +647,7 @@ Maximum number of DNS queries in a TCP connection.
 * Integer
 * Default: 7000
 
-Total maximum number of miliseconds of wallclock time the servermay use to answer
+Total maximum number of miliseconds of wallclock time the server may use to answer
 a single query.
 
 ## `minimum-ttl-override`

diff --git a/docs/markdown/recursor/upgrading.md b/docs/markdown/recursor/upgrading.md
index 813eb6f2e3b63a74904c80656afc290e404aaebc..b79c752d630ea3e595b6fada7cd5aa0278ee778c 100644 (file)
--- a/docs/markdown/recursor/upgrading.md
+++ b/docs/markdown/recursor/upgrading.md
@@ -1,6 +1,11 @@
 Before upgrading, it is advised to read the [changelog](../changelog.md).
 When upgrading several versions, please read **all** notes applying to the upgrade.
 
+# 4.0.3 to 4.0.4
+One setting has been added to limit the risk of overflowing the stack:
+
+ - [`max-recursion-depth`](settings.md#max-recursion-depth) defaults to 40 and was unlimited before
+
 # 4.0.0 to 4.0.1
 Two settings have changed defaults, these new defaults decrease CPU usage:
 

diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index 57cef38b5556d3366c75545b002013b1976705aa..16c11466733ad0669b10c54379d5585a9428fb16 100644 (file)
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -2760,6 +2760,7 @@ int serviceMain(int argc, char*argv[])
   SyncRes::s_serverID=::arg()["server-id"];
   SyncRes::s_maxqperq=::arg().asNum("max-qperq");
   SyncRes::s_maxtotusec=1000*::arg().asNum("max-total-msec");
+  SyncRes::s_maxdepth=::arg().asNum("max-recursion-depth");
   SyncRes::s_rootNXTrust = ::arg().mustDo( "root-nx-trust");
   if(SyncRes::s_serverID.empty()) {
     char tmp[128];
@@ -3118,6 +3119,7 @@ int main(int argc, char **argv)
     ::arg().set("minimum-ttl-override", "Set under adverse conditions, a minimum TTL")="0";
     ::arg().set("max-qperq", "Maximum outgoing queries per query")="50";
     ::arg().set("max-total-msec", "Maximum total wall-clock time per query in milliseconds, 0 for unlimited")="7000";
+    ::arg().set("max-recursion-depth", "Maximum number of internal recursion calls per query, 0 for unlimited")="40";
 
     ::arg().set("include-dir","Include *.conf files from this directory")="";
     ::arg().set("security-poll-suffix","Domain name from which to query security update notifications")="secpoll.powerdns.com.";

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 404dcc8c5bf543341c9cc9e2a3ef58f754f4423f..4fabbd37b71bc50344517fec5a4e13e533f8fd0e 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -74,6 +74,7 @@ bool SyncRes::s_nopacketcache;
 bool SyncRes::s_rootNXTrust;
 unsigned int SyncRes::s_maxqperq;
 unsigned int SyncRes::s_maxtotusec;
+unsigned int SyncRes::s_maxdepth;
 string SyncRes::s_serverID;
 SyncRes::LogMode SyncRes::s_lm;
 
@@ -182,7 +183,7 @@ int SyncRes::beginResolve(const DNSName &qname, const QType &qtype, uint16_t qcl
 }
 
 //! This is the 'out of band resolver', in other words, the authoritative server
-bool SyncRes::doOOBResolve(const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, int depth, int& res)
+bool SyncRes::doOOBResolve(const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, unsigned int depth, int& res)
 {
   string prefix;
   if(doLog()) {
@@ -407,7 +408,7 @@ int SyncRes::asyncresolveWrapper(const ComboAddress& ip, bool ednsMANDATORY, con
   return ret;
 }
 
-int SyncRes::doResolve(const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, int depth, set<GetBestNSAnswer>& beenthere)
+int SyncRes::doResolve(const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, unsigned int depth, set<GetBestNSAnswer>& beenthere)
 {
   string prefix;
   if(doLog()) {
@@ -417,6 +418,9 @@ int SyncRes::doResolve(const DNSName &qname, const QType &qtype, vector<DNSRecor
 
   LOG(prefix<<qname<<": Wants "<< (d_doDNSSEC ? "" : "NO ") << "DNSSEC processing in query for "<<qtype.getName()<<endl);
 
+  if(s_maxdepth && depth > s_maxdepth)
+    throw ImmediateServFailException("More than "+std::to_string(s_maxdepth)+" (max-recursion-depth) levels of recursion needed while resolving "+qname.toLogString());
+
   int res=0;
   if(!(d_nocache && qtype.getCode()==QType::NS && qname.isRoot())) {
     if(d_cacheonly) { // very limited OOB support
@@ -494,7 +498,7 @@ static bool ipv6First(const ComboAddress& a, const ComboAddress& b)
 
 /** This function explicitly goes out for A or AAAA addresses
 */
-vector<ComboAddress> SyncRes::getAddrs(const DNSName &qname, int depth, set<GetBestNSAnswer>& beenthere)
+vector<ComboAddress> SyncRes::getAddrs(const DNSName &qname, unsigned int depth, set<GetBestNSAnswer>& beenthere)
 {
   typedef vector<DNSRecord> res_t;
   res_t res;
@@ -569,7 +573,7 @@ vector<ComboAddress> SyncRes::getAddrs(const DNSName &qname, int depth, set<GetB
   return ret;
 }
 
-void SyncRes::getBestNSFromCache(const DNSName &qname, const QType& qtype, vector<DNSRecord>& bestns, bool* flawedNSSet, int depth, set<GetBestNSAnswer>& beenthere)
+void SyncRes::getBestNSFromCache(const DNSName &qname, const QType& qtype, vector<DNSRecord>& bestns, bool* flawedNSSet, unsigned int depth, set<GetBestNSAnswer>& beenthere)
 {
   string prefix;
   DNSName subdomain(qname);
@@ -657,7 +661,7 @@ SyncRes::domainmap_t::const_iterator SyncRes::getBestAuthZone(DNSName* qname)
 }
 
 /** doesn't actually do the work, leaves that to getBestNSFromCache */
-DNSName SyncRes::getBestNSNamesFromCache(const DNSName &qname, const QType& qtype, NsSet& nsset, bool* flawedNSSet, int depth, set<GetBestNSAnswer>&beenthere)
+DNSName SyncRes::getBestNSNamesFromCache(const DNSName &qname, const QType& qtype, NsSet& nsset, bool* flawedNSSet, unsigned int depth, set<GetBestNSAnswer>&beenthere)
 {
   DNSName subdomain(qname);
   DNSName authdomain(qname);
@@ -688,7 +692,7 @@ DNSName SyncRes::getBestNSNamesFromCache(const DNSName &qname, const QType& qtyp
   return subdomain;
 }
 
-bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector<DNSRecord>& ret, int depth, int &res)
+bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector<DNSRecord>& ret, unsigned int depth, int &res)
 {
   string prefix;
   if(doLog()) {
@@ -747,7 +751,7 @@ static const DNSName getLastLabel(const DNSName& qname)
 }
 
 
-bool SyncRes::doCacheCheck(const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, int depth, int &res)
+bool SyncRes::doCacheCheck(const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, unsigned int depth, int &res)
 {
   bool giveNegative=false;
 
@@ -964,7 +968,7 @@ static void addNXNSECS(vector<DNSRecord>&ret, const vector<DNSRecord>& records)
  */
 int SyncRes::doResolveAt(NsSet &nameservers, DNSName auth, bool flawedNSSet, const DNSName &qname, const QType &qtype,
                          vector<DNSRecord>&ret,
-                         int depth, set<GetBestNSAnswer>&beenthere)
+                         unsigned int depth, set<GetBestNSAnswer>&beenthere)
 {
   string prefix;
   if(doLog()) {

diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 2ee54d827a4f15d208c60a520346745235f0828e..0bbb9bacc880e5f88ce2fcf096478c69ca21b4ca 100644 (file)
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -370,6 +370,7 @@ public:
   static bool s_doIPv6;
   static unsigned int s_maxqperq;
   static unsigned int s_maxtotusec;
+  static unsigned int s_maxdepth;
   std::unordered_map<std::string,bool> d_discardedPolicies;
   DNSFilterEngine::Policy d_appliedPolicy;
 #ifdef HAVE_PROTOBUF
@@ -518,18 +519,18 @@ public:
 private:
   struct GetBestNSAnswer;
   int doResolveAt(NsSet &nameservers, DNSName auth, bool flawedNSSet, const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret,
-                  int depth, set<GetBestNSAnswer>&beenthere);
-  int doResolve(const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, int depth, set<GetBestNSAnswer>& beenthere);
-  bool doOOBResolve(const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, int depth, int &res);
+                  unsigned int depth, set<GetBestNSAnswer>&beenthere);
+  int doResolve(const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, unsigned int depth, set<GetBestNSAnswer>& beenthere);
+  bool doOOBResolve(const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, unsigned int depth, int &res);
   domainmap_t::const_iterator getBestAuthZone(DNSName* qname);
-  bool doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, int depth, int &res);
-  bool doCacheCheck(const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, int depth, int &res);
-  void getBestNSFromCache(const DNSName &qname, const QType &qtype, vector<DNSRecord>&bestns, bool* flawedNSSet, int depth, set<GetBestNSAnswer>& beenthere);
-  DNSName getBestNSNamesFromCache(const DNSName &qname, const QType &qtype, NsSet& nsset, bool* flawedNSSet, int depth, set<GetBestNSAnswer>&beenthere);
+  bool doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, unsigned int depth, int &res);
+  bool doCacheCheck(const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, unsigned int depth, int &res);
+  void getBestNSFromCache(const DNSName &qname, const QType &qtype, vector<DNSRecord>&bestns, bool* flawedNSSet, unsigned int depth, set<GetBestNSAnswer>& beenthere);
+  DNSName getBestNSNamesFromCache(const DNSName &qname, const QType &qtype, NsSet& nsset, bool* flawedNSSet, unsigned int depth, set<GetBestNSAnswer>&beenthere);
 
   inline vector<DNSName> shuffleInSpeedOrder(NsSet &nameservers, const string &prefix);
   bool moreSpecificThan(const DNSName& a, const DNSName &b);
-  vector<ComboAddress> getAddrs(const DNSName &qname, int depth, set<GetBestNSAnswer>& beenthere);
+  vector<ComboAddress> getAddrs(const DNSName &qname, unsigned int depth, set<GetBestNSAnswer>& beenthere);
 private:
   ostringstream d_trace;
   shared_ptr<RecursorLua4> d_pdl;