auth packetcache now honours edns presence as a variable. Found while investigating...

git-svn-id: svn://svn.powerdns.com/pdns/trunk/pdns@2982 d19b8d6e-7fed-0310-83ef-9ca221ded41b

diff --git a/pdns/dnspacket.cc b/pdns/dnspacket.cc
index 2c75825421c00f6465bd89a9aa7530602acb9293..70ea974c73bc306ab79f3def28ebf068b4d653ea 100644 (file)
--- a/pdns/dnspacket.cc
+++ b/pdns/dnspacket.cc
@@ -557,6 +557,11 @@ bool DNSPacket::hasEDNSSubnet()
   return d_haveednssubnet;
 }
 
+bool DNSPacket::hasEDNS() 
+{
+  return d_haveednssection;
+}
+
 Netmask DNSPacket::getRealRemote() const
 {
   if(d_haveednssubnet)

diff --git a/pdns/dnspacket.hh b/pdns/dnspacket.hh
index 231fe8d2224091d0e7911404b65120125b3cb1c3..b52bd49744cc5a10dbda931741314089374b1948 100644 (file)
--- a/pdns/dnspacket.hh
+++ b/pdns/dnspacket.hh
@@ -138,6 +138,7 @@ public:
 
   bool couldBeCached(); //!< returns 0 if this query should bypass the packet cache
   bool hasEDNSSubnet();
+  bool hasEDNS();
   //////// DATA !
 
   ComboAddress d_remote;

diff --git a/pdns/packetcache.cc b/pdns/packetcache.cc
index a9b029415f5d9ee06cabcd37924aaa218ce256a2..75ba26d1b17d23d451c25c5161f731cc943eca24 100644 (file)
--- a/pdns/packetcache.cc
+++ b/pdns/packetcache.cc
@@ -85,7 +85,7 @@ int PacketCache::get(DNSPacket *p, DNSPacket *cached)
     }
 
     uint16_t maxReplyLen = p->d_tcp ? 0xffff : p->getMaxReplyLen();
-    haveSomething=getEntryLocked(p->qdomain, p->qtype, PacketCache::PACKETCACHE, value, -1, packetMeritsRecursion, maxReplyLen, p->d_dnssecOk);
+    haveSomething=getEntryLocked(p->qdomain, p->qtype, PacketCache::PACKETCACHE, value, -1, packetMeritsRecursion, maxReplyLen, p->d_dnssecOk, p->hasEDNS());
   }
   if(haveSomething) {
     (*d_statnumhit)++;
@@ -128,12 +128,12 @@ void PacketCache::insert(DNSPacket *q, DNSPacket *r, unsigned int maxttl)
   if(maxttl<ourttl)
     ourttl=maxttl;
   insert(q->qdomain, q->qtype, PacketCache::PACKETCACHE, r->getString(), ourttl, -1, packetMeritsRecursion,
-    maxReplyLen, q->d_dnssecOk);
+    maxReplyLen, q->d_dnssecOk, q->hasEDNS());
 }
 
 // universal key appears to be: qname, qtype, kind (packet, query cache), optionally zoneid, meritsRecursion
 void PacketCache::insert(const string &qname, const QType& qtype, CacheEntryType cet, const string& value, unsigned int ttl, int zoneID, 
-  bool meritsRecursion, unsigned int maxReplyLen, bool dnssecOk)
+  bool meritsRecursion, unsigned int maxReplyLen, bool dnssecOk, bool EDNS)
 {
   if(!((++d_ops) % 300000)) {
     cleanup();
@@ -142,7 +142,7 @@ void PacketCache::insert(const string &qname, const QType& qtype, CacheEntryType
   if(!ttl)
     return;
   
-  //cerr<<"Inserting qname '"<<qname<<"', cet: "<<(int)cet<<", value: '"<< (cet ? value : "PACKET") <<"', qtype: "<<qtype.getName()<<", ttl: "<<ttl<<", maxreplylen: "<<maxReplyLen<<endl;
+  //cerr<<"Inserting qname '"<<qname<<"', cet: "<<(int)cet<<", qtype: "<<qtype.getName()<<", ttl: "<<ttl<<", maxreplylen: "<<maxReplyLen<<", hasEDNS: "<<EDNS<<endl;
   CacheEntry val;
   val.ttd=time(0)+ttl;
   val.qname=qname;
@@ -153,6 +153,7 @@ void PacketCache::insert(const string &qname, const QType& qtype, CacheEntryType
   val.maxReplyLen = maxReplyLen;
   val.dnssecOk = dnssecOk;
   val.zoneID = zoneID;
+  val.hasEDNS = EDNS;
   
   TryWriteLock l(&d_mut);
   if(l.gotIt()) { 
@@ -254,7 +255,7 @@ int PacketCache::purge(const string &match)
 }
 // called from ueberbackend
 bool PacketCache::getEntry(const string &qname, const QType& qtype, CacheEntryType cet, string& value, int zoneID, bool meritsRecursion, 
-  unsigned int maxReplyLen, bool dnssecOk)
+  unsigned int maxReplyLen, bool dnssecOk, bool hasEDNS)
 {
   if(d_ttl<0) 
     getTTLS();
@@ -269,16 +270,16 @@ bool PacketCache::getEntry(const string &qname, const QType& qtype, CacheEntryTy
     return false;
   }
 
-  return getEntryLocked(qname, qtype, cet, value, zoneID, meritsRecursion, maxReplyLen, dnssecOk);
+  return getEntryLocked(qname, qtype, cet, value, zoneID, meritsRecursion, maxReplyLen, dnssecOk, hasEDNS);
 }
 
 
 bool PacketCache::getEntryLocked(const string &qname, const QType& qtype, CacheEntryType cet, string& value, int zoneID, bool meritsRecursion,
-  unsigned int maxReplyLen, bool dnssecOK)
+  unsigned int maxReplyLen, bool dnssecOK, bool hasEDNS)
 {
   uint16_t qt = qtype.getCode();
   //cerr<<"Lookup for maxReplyLen: "<<maxReplyLen<<endl;
-  cmap_t::const_iterator i=d_map.find(tie(qname, qt, cet, zoneID, meritsRecursion, maxReplyLen, dnssecOK));
+  cmap_t::const_iterator i=d_map.find(tie(qname, qt, cet, zoneID, meritsRecursion, maxReplyLen, dnssecOK, hasEDNS));
   time_t now=time(0);
   bool ret=(i!=d_map.end() && i->ttd > now);
   if(ret)

diff --git a/pdns/packetcache.hh b/pdns/packetcache.hh
index 72c37b42b261fa7053f276f9772f05591fe352df..b60fccda63efb0b1ad249bf7a103b28bbb708506 100644 (file)
--- a/pdns/packetcache.hh
+++ b/pdns/packetcache.hh
@@ -72,11 +72,11 @@ public:
   void insert(DNSPacket *q, DNSPacket *r, unsigned int maxttl=UINT_MAX);  //!< We copy the contents of *p into our cache. Do not needlessly call this to insert questions already in the cache as it wastes resources
 
   void insert(const string &qname, const QType& qtype, CacheEntryType cet, const string& value, unsigned int ttl, int zoneID=-1, bool meritsRecursion=false,
-    unsigned int maxReplyLen=512, bool dnssecOk=false);
+    unsigned int maxReplyLen=512, bool dnssecOk=false, bool EDNS=false);
 
   int get(DNSPacket *p, DNSPacket *q); //!< We return a dynamically allocated copy out of our cache. You need to delete it. You also need to spoof in the right ID with the DNSPacket.spoofID() method.
   bool getEntry(const string &content, const QType& qtype, CacheEntryType cet, string& entry, int zoneID=-1, 
-    bool meritsRecursion=false, unsigned int maxReplyLen=512, bool dnssecOk=false);
+    bool meritsRecursion=false, unsigned int maxReplyLen=512, bool dnssecOk=false, bool hasEDNS=false);
 
   int size(); //!< number of entries in the cache
   void cleanup(); //!< force the cache to preen itself from expired packets
@@ -86,10 +86,10 @@ public:
   map<char,int> getCounts();
 private:
   bool getEntryLocked(const string &content, const QType& qtype, CacheEntryType cet, string& entry, int zoneID=-1, 
-    bool meritsRecursion=false, unsigned int maxReplyLen=512, bool dnssecOk=false);
+    bool meritsRecursion=false, unsigned int maxReplyLen=512, bool dnssecOk=false, bool hasEDNS=false);
   struct CacheEntry
   {
-    CacheEntry() { qtype = ctype = 0; zoneID = -1; meritsRecursion=false; dnssecOk=false;}
+    CacheEntry() { qtype = ctype = 0; zoneID = -1; meritsRecursion=false; dnssecOk=false; hasEDNS=false;}
 
     string qname;
     uint16_t qtype;
@@ -99,6 +99,7 @@ private:
     bool meritsRecursion;
     unsigned int maxReplyLen;
     bool dnssecOk;
+    bool hasEDNS;
     string value;
   };
 
@@ -116,10 +117,11 @@ private:
                         member<CacheEntry,int, &CacheEntry::zoneID>,
                         member<CacheEntry,bool, &CacheEntry::meritsRecursion>,
                         member<CacheEntry,unsigned int, &CacheEntry::maxReplyLen>,
-                        member<CacheEntry,bool, &CacheEntry::dnssecOk>
+                        member<CacheEntry,bool, &CacheEntry::dnssecOk>,
+                        member<CacheEntry,bool, &CacheEntry::hasEDNS>
                         >,
                         composite_key_compare<CIBackwardsStringCompare, std::less<uint16_t>, std::less<uint16_t>, std::less<int>, std::less<bool>, 
-                          std::less<unsigned int>, std::less<bool> >
+                          std::less<unsigned int>, std::less<bool>, std::less<bool> >
                             >,
                            sequenced<>
                            >

diff --git a/regression-tests.nobackend/edns-packet-cache/command b/regression-tests.nobackend/edns-packet-cache/command
new file mode 100755 (executable)
index 0000000..d157d56
--- /dev/null
+++ b/regression-tests.nobackend/edns-packet-cache/command
@@ -0,0 +1,37 @@
+#!/bin/bash -ex
+
+bindwait ()
+{
+       configname=$1
+       domcount=1
+       while sleep 1
+       do
+               done=$( (../pdns/pdns_control --config-name=$configname --socket-dir=. --no-config bind-domain-status || true) | grep -c 'parsed into memory' || true )
+               if [ $done = $domcount ]
+               then
+                       return
+               fi
+       done
+}
+
+port=5501
+rm -f pdns*.pid
+
+../pdns/pdns_server --daemon=no --local-port=$port --socket-dir=./          \
+       --no-shuffle --launch=bind --bind-config=edns-packet-cache/named.conf   \
+       --fancy-records --send-root-referral                                    \
+       --cache-ttl=60 --no-config &
+bindwait
+       
+# prime cache without EDNS
+../pdns/sdig 127.0.0.1 5501 minimal.com SOA | LC_ALL=C sort
+# expect EDNS in identical query with EDNS
+SDIGBUFSIZE=512 ../pdns/sdig 127.0.0.1 5501 minimal.com SOA | LC_ALL=C sort
+
+# prime cache with EDNS
+SDIGBUFSIZE=512 ../pdns/sdig 127.0.0.1 5501 minimal.com NS | LC_ALL=C sort
+# expect no EDNS in identical query without EDNS
+../pdns/sdig 127.0.0.1 5501 minimal.com NS | LC_ALL=C sort
+
+kill $(cat pdns*.pid)
+rm pdns*.pid

diff --git a/regression-tests.nobackend/edns-packet-cache/description b/regression-tests.nobackend/edns-packet-cache/description
new file mode 100644 (file)
index 0000000..6104eb0
--- /dev/null
+++ b/regression-tests.nobackend/edns-packet-cache/description
@@ -0,0 +1,7 @@
+The authoritative packet cache does not check whether a cached packet and the
+response is it being matched again, have the same EDNS status (present vs.
+not-present).
+
+Because it does take max reply length into account, the impact of this is
+limited - non-EDNS clients would only get EDNS replies from the cache if the
+EDNS bufsize happened to be 512.

diff --git a/regression-tests.nobackend/edns-packet-cache/expected_result b/regression-tests.nobackend/edns-packet-cache/expected_result
new file mode 100644 (file)
index 0000000..93d7fe4
--- /dev/null
+++ b/regression-tests.nobackend/edns-packet-cache/expected_result
@@ -0,0 +1,16 @@
+0      minimal.com.    IN      SOA     120     ns1.example.com. ahu.example.com. 2000081501 28800 7200 604800 86400
+Rcode: 0, RD: 0, QR: 1, TC: 0, AA: 1, opcode: 0
+Reply to question for qname='minimal.com.', qtype=SOA
+0      minimal.com.    IN      SOA     120     ns1.example.com. ahu.example.com. 2000081501 28800 7200 604800 86400
+2      .       IN      OPT     0       
+Rcode: 0, RD: 0, QR: 1, TC: 0, AA: 1, opcode: 0
+Reply to question for qname='minimal.com.', qtype=SOA
+0      minimal.com.    IN      NS      120     ns1.example.com.
+0      minimal.com.    IN      NS      120     ns2.example.com.
+2      .       IN      OPT     0       
+Rcode: 0, RD: 0, QR: 1, TC: 0, AA: 1, opcode: 0
+Reply to question for qname='minimal.com.', qtype=NS
+0      minimal.com.    IN      NS      120     ns1.example.com.
+0      minimal.com.    IN      NS      120     ns2.example.com.
+Rcode: 0, RD: 0, QR: 1, TC: 0, AA: 1, opcode: 0
+Reply to question for qname='minimal.com.', qtype=NS

diff --git a/regression-tests.nobackend/edns-packet-cache/named.conf b/regression-tests.nobackend/edns-packet-cache/named.conf
new file mode 100644 (file)
index 0000000..26dc6ad
--- /dev/null
+++ b/regression-tests.nobackend/edns-packet-cache/named.conf
@@ -0,0 +1,14 @@
+options {
+       directory "../regression-tests/";
+       recursion no;
+       listen-on port 5300 {
+               127.0.0.1;
+       };
+       version "Meow!Meow!";
+       minimal-responses yes;
+};
+
+zone "minimal.com"{
+       type master;
+       file "./minimal.com";
+};