From: bert hubert <bert.hubert@netherlabs.nl>
Date: Wed, 11 Nov 2015 11:33:35 +0000 (+0100)
Subject: Merge remote-tracking branch 'origin/master' into ednssubnet
X-Git-Tag: dnsdist-1.0.0-alpha1~230^2~4
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6c674e9ac972306831916d84cfb04fb565c397c2;p=pdns

Merge remote-tracking branch 'origin/master' into ednssubnet

Conflicts:
	modules/geoipbackend/geoipbackend.cc
---

6c674e9ac972306831916d84cfb04fb565c397c2
diff --cc pdns/recursor_cache.cc
index ff1226e49,8ec82fc66..af1562b5c
--- a/pdns/recursor_cache.cc
+++ b/pdns/recursor_cache.cc
@@@ -46,23 -46,14 +46,23 @@@ int MemRecursorCache::get(time_t now, c
    if(res)
      res->clear();
  
++  bool haveSubnetSpecific=false;
    if(d_cachecache.first!=d_cachecache.second) {
++    for(cache_t::const_iterator i=d_cachecache.first; i != d_cachecache.second; ++i) {
++      if(!i->d_netmask.empty()) {
++	cout<<"Had a subnet specific hit: "<<i->d_netmask.toString()<<", query was for "<<who.toString()<<": match "<<i->d_netmask.match(who)<<endl;
++	haveSubnetSpecific=true;
++      }
++    }
      for(cache_t::const_iterator i=d_cachecache.first; i != d_cachecache.second; ++i)
--      if(i->d_ttd > now && (i->d_qtype == qt.getCode() || qt.getCode()==QType::ANY ||
--			    (qt.getCode()==QType::ADDR && (i->d_qtype == QType::A || i->d_qtype == QType::AAAA) ) )
++      if(i->d_ttd > now && ((i->d_qtype == qt.getCode() || qt.getCode()==QType::ANY ||
++			    (qt.getCode()==QType::ADDR && (i->d_qtype == QType::A || i->d_qtype == QType::AAAA) )) 
++			    && (!haveSubnetSpecific || i->d_netmask.match(who)))
           ) {
  
  	ttd = i->d_ttd;	
 -	for(auto k=i->d_records.begin(); k != i->d_records.end(); ++k) {
 +	auto records = &i->d_records;
- 	if(!i->d_subnetspecific.empty()) {
- 	  for(const auto& p : i->d_subnetspecific) {
- 	    if(p.first.match(who)) {
- 	      records = &p.second;
- 	      break;
- 	    }
- 	  }
- 	}
 +	for(auto k=records->begin(); k != records->end(); ++k) {
  	  if(res) {
  	    DNSRecord dr;
  	    dr.d_name = qname;
@@@ -123,51 -114,28 +123,42 @@@ bool MemRecursorCache::attemptToRefresh
    return true;
  }
  
 -void MemRecursorCache::replace(time_t now, const DNSName &qname, const QType& qt,  const vector<DNSRecord>& content, const vector<shared_ptr<RRSIGRecordContent>>& signatures, bool auth)
 +void MemRecursorCache::replace(time_t now, const DNSName &qname, const QType& qt,  const vector<DNSRecord>& content, const vector<shared_ptr<RRSIGRecordContent>>& signatures, bool auth, boost::optional<Netmask> ednsmask)
  {
 +  if(ednsmask) {
 +    cerr<<"This data is actually subnet mask specific!!"<<endl;
 +  }
    d_cachecachevalid=false;
--  boost::tuple<DNSName, uint16_t> key=boost::make_tuple(qname, qt.getCode());
--  cache_t::iterator stored=d_cache.find(key);
--  uint32_t maxTTD=UINT_MAX;
  
--  if(stored == d_cache.end()) {
--    stored=d_cache.insert(CacheEntry(key,CacheEntry::records_t(), auth)).first;
++  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;
++    }
++  }
++
++
++  uint32_t maxTTD=UINT_MAX;
    CacheEntry ce=*stored;
    ce.d_qtype=qt.getCode();
    ce.d_signatures=signatures;
  
--  //  cerr<<"asked to store "<< qname<<"|"+qt.getName()<<" -> '"<<content.begin()->d_content->getZoneRepresentation()<<"', auth="<<auth<<", ce.auth="<<ce.d_auth<<"\n";
++  cerr<<"asked to store "<< (qname.empty() ? "EMPTY" : qname.toString()) <<"|"+qt.getName()<<" -> '"<<content.begin()->d_content->getZoneRepresentation()<<"', auth="<<auth<<", ce.auth="<<ce.d_auth<<", "<< (ednsmask ? ednsmask->toString() : "")<<endl;
  
-   auto records = &ce.d_records;
-   if(ednsmask) {
-     records=nullptr; // there, we use nullptr!
- 
-     /* so the logic here is.. if you update a specific mask, we'll update too. 
-        If you supply anything more generic, we'll store that next to your more
-        specific answers. 
-        If you supply more specific answers, we'll store those too. */
-     for(auto &p : ce.d_subnetspecific) {
-       if(p.first == *ednsmask) {
- 	records = &p.second;
- 	break;
-       }
-     }
-     if(records==nullptr) {
-       ce.d_subnetspecific.push_back({*ednsmask, CacheEntry::records_t()});
-       records = &ce.d_subnetspecific.rbegin()->second;
-     }
-       
-   }
-   records->clear();
+   ce.d_records.clear();
  
    if(!auth && ce.d_auth) {  // unauth data came in, we have some auth data, but is it fresh?
      if(ce.d_ttd > now) { // we still have valid data, ignore unauth data
--      //      cerr<<"\tStill hold valid auth data, and the new data is unauth, return\n";
++      cerr<<"\tStill hold valid auth data, and the new data is unauth, return\n";
        return;
      }
      else {
@@@ -182,22 -150,50 +173,22 @@@
    }
  
    // make sure that we CAN refresh the root
--  if(auth && ((qname == DNSName()) || !attemptToRefreshNSTTL(qt, content, ce) ) ) {
++  if(auth && (qname.isRoot() || !attemptToRefreshNSTTL(qt, content, ce) ) ) {
      // cerr<<"\tGot auth data, and it was not refresh attempt of an unchanged NS set, nuking storage"<<endl;
-     records->clear(); // clear non-auth data
+     ce.d_records.clear(); // clear non-auth data
      ce.d_auth = true;
    }
  //  else cerr<<"\tNot nuking"<<endl;
  
  
--  // cerr<<"\tHave "<<content.size()<<" records to store\n";
++  cerr<<"\tHave "<<content.size()<<" records to store\n";
    for(auto i=content.cbegin(); i != content.cend(); ++i) {
      // cerr<<"To store: "<<i->content<<" with ttl/ttd "<<i->ttl<<endl;
      ce.d_ttd=min(maxTTD, i->d_ttl);   // XXX this does weird things if TTLs differ in the set
-     records->push_back(i->d_content);
+     ce.d_records.push_back(i->d_content);
 -
 -    /*
 -    else {
 -      range=equal_range(ce.d_records.begin(), ce.d_records.end(), dr);
 -
 -      if(range.first != range.second) {
 -       // cerr<<"\t\tMay need to modify TTL of stored record\n";
 -        for(vector<StoredRecord>::iterator j=range.first ; j!=range.second; ++j) {
 -          // see http://mailman.powerdns.com/pipermail/pdns-users/2006-May/003413.html
 -          if(j->d_ttd > (unsigned int) now && i->ttl > j->d_ttd && qt.getCode()==QType::NS && auth) { // don't allow auth servers to *raise* TTL of an NS record
 -            //~ cerr<<"\t\tNot doing so, trying to raise TTL NS\n";
 -            continue;
 -          }
 -          if(i->ttl > j->d_ttd || (auth) ) { // authoritative packets can override the TTL to be lower
 -            //~ cerr<<"\t\tUpdating the ttl, diff="<<j->d_ttd - i->ttl<<endl;;
 -            j->d_ttd=i->ttl;
 -          }
 -          else {
 -            //~ cerr<<"\t\tNOT updating the ttl, old= " <<j->d_ttd - now <<", new: "<<i->ttl - now <<endl;
 -          }
 -        }
 -      }
 -      else {
 -        //~ cerr<<"\t\tThere was no exact copy of this record, so adding & sorting\n";
 -        ce.d_records.push_back(dr);
 -        sort(ce.d_records.begin(), ce.d_records.end());
 -	}
 -    }
 -    */
 +    // there was code here that did things with TTL and auth. Unsure if it was good. XXX
    }
--
++  cerr<<"Calling replace"<<endl;
    d_cache.replace(stored, ce);
  }
  
@@@ -304,7 -300,7 +295,7 @@@ uint64_t MemRecursorCache::doDump(int f
      for(auto j=i->d_records.cbegin(); j != i->d_records.cend(); ++j) {
        count++;
        try {
--        fprintf(fp, "%s %d IN %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());
++        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());
        }
        catch(...) {
          fprintf(fp, "; error printing '%s'\n", i->d_qname.toString().c_str());
diff --cc pdns/recursor_cache.hh
index 93d36bea1,b71b6fee0..66a8b3b7b
--- a/pdns/recursor_cache.hh
+++ b/pdns/recursor_cache.hh
@@@ -48,8 -48,8 +48,8 @@@ private
  
    struct CacheEntry
    {
--    CacheEntry(const boost::tuple<DNSName, uint16_t>& key, const vector<shared_ptr<DNSRecordContent>>& records, bool auth) : 
-       d_qname(key.get<0>()), d_qtype(key.get<1>()), d_auth(auth), d_ttd(0), d_records(records)
 -      d_qname(key.get<0>()), d_qtype(key.get<1>()), d_auth(auth), d_records(records), d_ttd(0)
++    CacheEntry(const boost::tuple<DNSName, uint16_t, Netmask>& key, const vector<shared_ptr<DNSRecordContent>>& records, bool auth) : 
++      d_qname(key.get<0>()), d_qtype(key.get<1>()), d_auth(auth), d_ttd(0), d_records(records), d_netmask(key.get<2>())
      {}
  
      typedef vector<std::shared_ptr<DNSRecordContent>> records_t;
@@@ -59,12 -59,11 +59,12 @@@
        return d_ttd;
      }
  
 -    DNSName d_qname;
 +    DNSName d_qname; 
      uint16_t d_qtype;
      bool d_auth;
 -    records_t d_records;
      uint32_t d_ttd;
 +    records_t d_records;
-     vector<pair<Netmask, records_t> > d_subnetspecific;
++    Netmask d_netmask;
    };
  
    typedef multi_index_container<
@@@ -74,9 -73,9 +74,10 @@@
                        composite_key< 
                          CacheEntry,
                          member<CacheEntry,DNSName,&CacheEntry::d_qname>,
--                        member<CacheEntry,uint16_t,&CacheEntry::d_qtype>
++                        member<CacheEntry,uint16_t,&CacheEntry::d_qtype>,
++                        member<CacheEntry,Netmask,&CacheEntry::d_netmask>
                        >,
--                      composite_key_compare<CanonDNSNameCompare, std::less<uint16_t> >
++		  composite_key_compare<CanonDNSNameCompare, std::less<uint16_t>, std::less<Netmask> >
                  >,
                 sequenced<>
                 >
diff --cc pdns/syncres.cc
index f3537e369,d4a696776..afa650072
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@@ -856,14 -854,14 +856,6 @@@ inline vector<DNSName> SyncRes::shuffle
    return rnameservers;
  }
  
--struct TCacheComp
--{
--  bool operator()(const pair<DNSName, QType>& a, const pair<DNSName, QType>& b) const
--  {
--    return tie(a.first, a.second) < tie(b.first, b.second);
--  }
--};
--
  static bool magicAddrMatch(const QType& query, const QType& answer)
  {
    if(query.getCode() != QType::ADDR)
@@@ -1096,7 -1089,7 +1088,16 @@@ int SyncRes::doResolveAt(set<DNSName> n
  	vector<DNSRecord> records;
  	vector<shared_ptr<RRSIGRecordContent>> signatures;
        };
--      typedef map<pair<DNSName, QType>, CachePair, TCacheComp > tcache_t;
++      struct CacheKey
++      {
++	DNSName name;
++	uint16_t type;
++	DNSResourceRecord::Place place;
++	bool operator<(const CacheKey& rhs) const {
++	  return tie(name, type) < tie(rhs.name, rhs.type);
++	}
++      };
++      typedef map<CacheKey, CachePair> tcache_t;
        tcache_t tcache;
  
        if(d_doDNSSEC) {
@@@ -1104,7 -1097,7 +1105,7 @@@
  	  if(rec.d_type == QType::RRSIG) {
  	    auto rrsig = std::dynamic_pointer_cast<RRSIGRecordContent>(rec.d_content);
  	    //	    cerr<<"Got an RRSIG for "<<DNSRecordContent::NumberToType(rrsig->d_type)<<" with name '"<<rec.d_name<<"'"<<endl;
--	    tcache[make_pair(rec.d_name, QType(rrsig->d_type))].signatures.push_back(rrsig);
++	    tcache[{rec.d_name, rrsig->d_type, rec.d_place}].signatures.push_back(rrsig);
  	  }
  	}
        }
@@@ -1155,8 -1148,8 +1156,8 @@@
              dr.d_place=DNSResourceRecord::ANSWER;
  
              dr.d_ttl += d_now.tv_sec;
--
--            tcache[make_pair(rec.d_name,QType(rec.d_type))].records.push_back(dr);
++	    // we should note the PLACE and not store ECS subnet details for non-answer records
++            tcache[{rec.d_name,rec.d_type,rec.d_place}].records.push_back(dr);
            }
          }
          else
@@@ -1176,7 -1169,7 +1177,7 @@@
  
  	//	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.first, i->first.second, i->second.records, i->second.signatures, lwr.d_aabit, ednsmask);
 -        t_RC->replace(d_now.tv_sec, i->first.first, i->first.second, i->second.records, i->second.signatures, lwr.d_aabit);
++        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>());
        }
        set<DNSName> nsset;
        LOG(prefix<<qname.toString()<<": determining status after receiving this packet"<<endl);