From 17d0b1e67010ee9813d2acddf3bef40fc6d78482 Mon Sep 17 00:00:00 2001 From: Peter van Dijk Date: Wed, 12 Dec 2012 12:03:37 +0000 Subject: [PATCH] auth packetcache now honours edns presence as a variable. Found while investigating a report from Winfried Angele. Patch by Ruben d'Arco. Includes test. Closes #630 git-svn-id: svn://svn.powerdns.com/pdns/trunk/pdns@2982 d19b8d6e-7fed-0310-83ef-9ca221ded41b --- pdns/dnspacket.cc | 5 +++ pdns/dnspacket.hh | 1 + pdns/packetcache.cc | 17 +++++---- pdns/packetcache.hh | 14 ++++--- .../edns-packet-cache/command | 37 +++++++++++++++++++ .../edns-packet-cache/description | 7 ++++ .../edns-packet-cache/expected_result | 16 ++++++++ .../edns-packet-cache/named.conf | 14 +++++++ 8 files changed, 97 insertions(+), 14 deletions(-) create mode 100755 regression-tests.nobackend/edns-packet-cache/command create mode 100644 regression-tests.nobackend/edns-packet-cache/description create mode 100644 regression-tests.nobackend/edns-packet-cache/expected_result create mode 100644 regression-tests.nobackend/edns-packet-cache/named.conf diff --git a/pdns/dnspacket.cc b/pdns/dnspacket.cc index 2c7582542..70ea974c7 100644 --- 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 231fe8d22..b52bd4974 100644 --- 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 a9b029415..75ba26d1b 100644 --- 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(maxttlqdomain, 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 '"<ttd > now); if(ret) diff --git a/pdns/packetcache.hh b/pdns/packetcache.hh index 72c37b42b..b60fccda6 100644 --- 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 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, member, member, - member + member, + member >, composite_key_compare, std::less, std::less, std::less, - std::less, std::less > + std::less, std::less, std::less > >, sequenced<> > diff --git a/regression-tests.nobackend/edns-packet-cache/command b/regression-tests.nobackend/edns-packet-cache/command new file mode 100755 index 000000000..d157d568b --- /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 index 000000000..6104eb02d --- /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 index 000000000..93d7fe40f --- /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 index 000000000..26dc6adfd --- /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"; +}; -- 2.40.0