From: bert hubert <bert.hubert@netherlabs.nl>
Date: Thu, 12 Nov 2015 10:08:24 +0000 (+0100)
Subject: fix up cache dumping, plus make sure we don't packet cache edns subnet variable answers
X-Git-Tag: dnsdist-1.0.0-alpha1~230^2~1
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=3762e8215b8af7028603195b16f315f3a102925a;p=pdns

fix up cache dumping, plus make sure we don't packet cache edns subnet variable answers
---

diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index b9ab0829f..f018abce8 100644
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -828,7 +828,7 @@ void startDoResolve(void *p)
       else
         msgh.msg_control=NULL;
       sendmsg(dc->d_socket, &msgh, 0);
-      if(!SyncRes::s_nopacketcache && !variableAnswer ) {
+      if(!SyncRes::s_nopacketcache && !variableAnswer && !sr.wasVariable() ) {
         t_packetCache->insertResponsePacket(string((const char*)&*packet.begin(), packet.size()),
                                             g_now.tv_sec,
                                             min(minTTL,
@@ -836,6 +836,7 @@ void startDoResolve(void *p)
                                             )
         );
       }
+      //      else cerr<<"Not putting in packet cache: "<<sr.wasVariable()<<endl;
     }
     else {
       char buf[2];
diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc
index d80771856..3d898a447 100644
--- a/pdns/recursor_cache.cc
+++ b/pdns/recursor_cache.cc
@@ -61,8 +61,7 @@ int MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, vec
          ) {
 
 	ttd = i->d_ttd;	
-	auto records = &i->d_records;
-	for(auto k=records->begin(); k != records->end(); ++k) {
+	for(auto k=i->d_records.begin(); k != i->d_records.end(); ++k) {
 	  if(res) {
 	    DNSRecord dr;
 	    dr.d_name = qname;
@@ -128,22 +127,12 @@ void MemRecursorCache::replace(time_t now, const DNSName &qname, const QType& qt
   d_cachecachevalid=false;
 
   cache_t::iterator stored;
-  if(ednsmask) {
-    auto key=boost::make_tuple(qname, qt.getCode(), *ednsmask);
-    stored=d_cache.find(key);
-    if(stored == d_cache.end()) {
-      stored=d_cache.insert(CacheEntry(key,CacheEntry::records_t(), auth)).first;
-    }
-  }
-  else {
-    auto key=boost::make_tuple(qname, qt.getCode(),Netmask());
-    stored=d_cache.find(key);
-    if(stored == d_cache.end()) {
-      stored=d_cache.insert(CacheEntry(key,CacheEntry::records_t(), auth)).first;
-    }
+  auto key=boost::make_tuple(qname, qt.getCode(), ednsmask ? *ednsmask : Netmask());
+  stored=d_cache.find(key);
+  if(stored == d_cache.end()) {
+    stored=d_cache.insert(CacheEntry(key,CacheEntry::records_t(), auth)).first;
   }
 
-
   uint32_t maxTTD=UINT_MAX;
   CacheEntry ce=*stored;
   ce.d_qtype=qt.getCode();
@@ -179,7 +168,7 @@ void MemRecursorCache::replace(time_t now, const DNSName &qname, const QType& qt
 
 
   for(auto i=content.cbegin(); i != content.cend(); ++i) {
-    // cerr<<"To store: "<<i->content<<" with ttl/ttd "<<i->ttl<<endl;
+    //    cerr<<"To store: "<<i->d_content->getZoneRepresentation()<<" with ttl/ttd "<<i->d_ttl<<endl;
     ce.d_ttd=min(maxTTD, i->d_ttl);   // XXX this does weird things if TTLs differ in the set
     ce.d_records.push_back(i->d_content);
     // there was code here that did things with TTL and auth. Unsure if it was good. XXX
@@ -282,7 +271,6 @@ uint64_t MemRecursorCache::doDump(int fd)
     return 0;
   }
   fprintf(fp, "; main record cache dump from thread follows\n;\n");
-
   auto& sidx=d_cache.get<0>();
 
   uint64_t count=0;
@@ -291,10 +279,10 @@ uint64_t MemRecursorCache::doDump(int fd)
     for(auto j=i->d_records.cbegin(); j != i->d_records.cend(); ++j) {
       count++;
       try {
-        fprintf(fp, "%s %d IN %s %s ; %s\n", i->d_qname.toString().c_str(), (int32_t)(i->d_ttd - now), DNSRecordContent::NumberToType(i->d_qtype).c_str(), (*j)->getZoneRepresentation().c_str(), i->d_netmask.toString().c_str());
+        fprintf(fp, "%s %d IN %s %s ; %s\n", i->d_qname.toString().c_str(), (int32_t)(i->d_ttd - now), DNSRecordContent::NumberToType(i->d_qtype).c_str(), (*j)->getZoneRepresentation().c_str(), i->d_netmask.empty() ? "" : i->d_netmask.toString().c_str());
       }
       catch(...) {
-        fprintf(fp, "; error printing '%s'\n", i->d_qname.toString().c_str());
+        fprintf(fp, "; error printing '%s'\n", i->d_qname.empty() ? "EMPTY" : i->d_qname.toString().c_str());
       }
     }
   }
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index f274ccdb9..b50772e7a 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -123,6 +123,7 @@ SyncRes::SyncRes(const struct timeval& now) :  d_outqueries(0), d_tcpoutqueries(
 int SyncRes::beginResolve(const DNSName &qname, const QType &qtype, uint16_t qclass, vector<DNSRecord>&ret)
 {
   s_queries++;
+  d_wasVariable=false;
 
   if( (qtype.getCode() == QType::AXFR))
     return -1;
@@ -1175,6 +1176,8 @@ int SyncRes::doResolveAt(set<DNSName> nameservers, DNSName auth, bool flawedNSSe
 	//	cout<<"Have "<<i->second.records.size()<<" records and "<<i->second.signatures.size()<<" signatures for "<<i->first.first.toString();
 	//	cout<<'|'<<DNSRecordContent::NumberToType(i->first.second.getCode())<<endl;
         t_RC->replace(d_now.tv_sec, i->first.name, QType(i->first.type), i->second.records, i->second.signatures, lwr.d_aabit, i->first.place == DNSResourceRecord::ANSWER ? ednsmask : boost::optional<Netmask>());
+	if(i->first.place == DNSResourceRecord::ANSWER && ednsmask)
+	  d_wasVariable=true;
       }
       set<DNSName> nsset;
       LOG(prefix<<qname.toString()<<": determining status after receiving this packet"<<endl);
diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 90326011f..e0251d467 100644
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -287,6 +287,10 @@ public:
     d_pdl = pdl;
   }
 
+  bool wasVariable() const
+  {
+    return d_wasVariable;
+  }
 
   int asyncresolveWrapper(const ComboAddress& ip, const DNSName& domain, int type, bool doTCP, bool sendRDQuery, struct timeval* now, boost::optional<Netmask>& srcmask, LWResult* res);
 
@@ -314,7 +318,9 @@ public:
   unsigned int d_totUsec;
   ComboAddress d_requestor;
   bool d_doDNSSEC;
-
+  
+  bool d_wasVariable{false};
+  
   typedef multi_index_container <
     NegCacheEntry,
     indexed_by <
@@ -431,7 +437,6 @@ public:
   static bool s_nopacketcache;
   static string s_serverID;
 
-
   struct StaticStorage {
     negcache_t negcache;
     nsspeeds_t nsSpeeds;