]> granicus.if.org Git - pdns/commitdiff
Do full packet comparison in the packet caches in addition to the hash
authorRemi Gacogne <remi.gacogne@powerdns.com>
Thu, 23 Aug 2018 14:21:39 +0000 (16:21 +0200)
committerRemi Gacogne <remi.gacogne@powerdns.com>
Thu, 11 Oct 2018 09:37:30 +0000 (11:37 +0200)
(cherry picked from commit c407246da8addf5443c3ac79c692917c3f7b3f35)

pdns/pdns_recursor.cc
pdns/recpacketcache.cc
pdns/recpacketcache.hh
pdns/test-recpacketcache_cc.cc

index 8d33a8b658a92e3fadf2cf6d016e073e2aa241a9..96782f647461b0af80d338e10b35b45fcead7f39 100644 (file)
@@ -1134,7 +1134,7 @@ void startDoResolve(void *p)
       if(sendmsg(dc->d_socket, &msgh, 0) < 0 && g_logCommonErrors) 
         L<<Logger::Warning<<"Sending UDP reply to client "<<dc->d_remote.toStringWithPort()<<" failed with: "<<strerror(errno)<<endl;
       if(!SyncRes::s_nopacketcache && !variableAnswer && !sr.wasVariable() ) {
-        t_packetCache->insertResponsePacket(dc->d_tag, dc->d_mdp.d_qname, dc->d_mdp.d_qtype, dc->d_query,
+        t_packetCache->insertResponsePacket(dc->d_tag, dc->d_mdp.d_qname, dc->d_mdp.d_qtype, dc->d_mdp.d_qclass, dc->d_query,
                                             string((const char*)&*packet.begin(), packet.size()),
                                             g_now.tv_sec,
                                             pw.getHeader()->rcode == RCode::ServFail ? SyncRes::s_packetcacheservfailttl :
index 1d24831795c796248be2d8294e93789c00a94cc3..7137842f6ab0da13c1304c3df52cc3488878ba40 100644 (file)
@@ -42,32 +42,78 @@ int RecursorPacketCache::doWipePacketCache(const DNSName& name, uint16_t qtype,
   return count;
 }
 
+static bool queryHeaderMatches(const std::string& cachedQuery, const std::string& query)
+{
+  if (cachedQuery.size() != query.size()) {
+    return false;
+  }
+
+  return (cachedQuery.compare(/* skip the ID */ 2, sizeof(dnsheader) - 2, query, 2, sizeof(dnsheader) - 2) == 0);
+}
+
+bool RecursorPacketCache::queryMatches(const std::string& cachedQuery, const std::string& query, const DNSName& qname, uint16_t ecsBegin, uint16_t ecsEnd)
+{
+  if (!queryHeaderMatches(cachedQuery, query)) {
+    return false;
+  }
+
+  size_t pos = sizeof(dnsheader) + qname.wirelength();
+
+  if (ecsBegin != 0 && ecsBegin >= pos && ecsEnd > ecsBegin) {
+    if (cachedQuery.compare(pos, ecsBegin - pos, query, pos, ecsBegin - pos) != 0) {
+      return false;
+    }
+
+    if (cachedQuery.compare(ecsEnd, cachedQuery.size() - ecsEnd, query, ecsEnd, query.size() - ecsEnd) != 0) {
+      return false;
+    }
+  }
+  else {
+    if (cachedQuery.compare(pos, cachedQuery.size() - pos, query, pos, query.size() - pos) != 0) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
 // one day this function could be really fast by doing only a case insensitive compare
-static bool qrMatch(const std::string& query, const std::string& response)
+bool RecursorPacketCache::qrMatch(const packetCache_t::index<HashTag>::type::iterator& iter, const std::string& queryPacket, uint16_t ecsBegin, uint16_t ecsEnd)
 {
-  uint16_t rqtype, rqclass, qqtype, qqclass;
-  DNSName queryname(query.c_str(), query.length(), sizeof(dnsheader), false, &qqtype, &qqclass, 0);
-  DNSName respname(response.c_str(), response.length(), sizeof(dnsheader), false, &rqtype, &rqclass, 0);
+  uint16_t qqtype, qqclass;
+  DNSName queryname(queryPacket.c_str(), queryPacket.length(), sizeof(dnsheader), false, &qqtype, &qqclass, 0);
   // this ignores checking on the EDNS subnet flags! 
-  return queryname==respname && rqtype == qqtype && rqclass == qqclass;
+  if (qqtype != iter->d_type || qqclass != iter->d_class || queryname != iter->d_name) {
+    return false;
+  }
+
+  if (iter->d_ecsBegin != ecsBegin || iter->d_ecsEnd != ecsEnd) {
+    return false;
+  }
+
+  return queryMatches(iter->d_query, queryPacket, queryname, ecsBegin, ecsEnd);
 }
 
-uint32_t RecursorPacketCache::canHashPacket(const std::string& origPacket)
+uint32_t RecursorPacketCache::canHashPacket(const std::string& origPacket, uint16_t* ecsBegin, uint16_t* ecsEnd)
 {
   //  return 42; // should still work if you do this!
   uint32_t ret=0;
-  ret=burtle((const unsigned char*)origPacket.c_str() + 2, 10, ret); // rest of dnsheader, skip id
+  ret = burtle(reinterpret_cast<const unsigned char*>(origPacket.c_str()) + 2, sizeof(dnsheader) - 2, ret); // rest of dnsheader, skip id
   const char* end = origPacket.c_str() + origPacket.size();
-  const char* p = origPacket.c_str() + 12;
+  const char* p = origPacket.c_str() + sizeof(dnsheader);
 
   for(; p < end && *p; ++p) { // XXX if you embed a 0 in your qname we'll stop lowercasing there
     const char l = dns_tolower(*p); // label lengths can safely be lower cased
     ret=burtle((const unsigned char*)&l, 1, ret);
   }                           // XXX the embedded 0 in the qname will break the subnet stripping
 
-  struct dnsheader* dh = (struct dnsheader*)origPacket.c_str();
+  const struct dnsheader* dh = reinterpret_cast<const struct dnsheader*>(origPacket.c_str());
   const char* skipBegin = p;
   const char* skipEnd = p;
+  if (ecsBegin != nullptr && ecsEnd != nullptr) {
+    *ecsBegin = 0;
+    *ecsEnd = 0;
+  }
   /* we need at least 1 (final empty label) + 2 (QTYPE) + 2 (QCLASS)
      + OPT root label (1), type (2), class (2) and ttl (4)
      + the OPT RR rdlen (2)
@@ -78,33 +124,33 @@ uint32_t RecursorPacketCache::canHashPacket(const std::string& origPacket)
     size_t optionLen = 0;
     /* skip the final empty label (1), the qtype (2), qclass (2) */
     /* root label (1), type (2), class (2) and ttl (4) */
-    int res = getEDNSOption((char*) p + 14, end - (p + 14), EDNSOptionCode::ECS, &optionBegin, &optionLen);
+    int res = getEDNSOption(const_cast<char*>(reinterpret_cast<const char*>(p)) + 14, end - (p + 14), EDNSOptionCode::ECS, &optionBegin, &optionLen);
     if (res == 0) {
       skipBegin = optionBegin;
       skipEnd = optionBegin + optionLen;
+      if (ecsBegin != nullptr && ecsEnd != nullptr) {
+        *ecsBegin = optionBegin - origPacket.c_str();
+        *ecsEnd = *ecsBegin + optionLen;
+      }
     }
   }
   if (skipBegin > p) {
     //cout << "Hashing from " << (p-origPacket.c_str()) << " for " << skipBegin-p << "bytes, end is at "<< end-origPacket.c_str() << endl;
-    ret = burtle((const unsigned char*)p, skipBegin-p, ret);
+    ret = burtle(reinterpret_cast<const unsigned char*>(p), skipBegin-p, ret);
   }
   if (skipEnd < end) {
     //cout << "Hashing from " << (skipEnd-origPacket.c_str()) << " for " << end-skipEnd << "bytes, end is at " << end-origPacket.c_str() << endl;
-    ret = burtle((const unsigned char*) skipEnd, end-skipEnd, ret);
+    ret = burtle(reinterpret_cast<const unsigned char*>(skipEnd), end-skipEnd, ret);
   }
   return ret;
 }
 
-bool RecursorPacketCache::getResponsePacket(unsigned int tag, const std::string& queryPacket, time_t now,
-                                            std::string* responsePacket, uint32_t* age)
-{
-  return getResponsePacket(tag, queryPacket, now, responsePacket, age, nullptr);
-}
-
 bool RecursorPacketCache::getResponsePacket(unsigned int tag, const std::string& queryPacket, time_t now,
                                             std::string* responsePacket, uint32_t* age, RecProtoBufMessage* protobufMessage)
 {
-  uint32_t h = canHashPacket(queryPacket);
+  uint16_t ecsBegin = 0;
+  uint16_t ecsEnd = 0;
+  uint32_t h = canHashPacket(queryPacket, &ecsBegin, &ecsEnd);
   auto& idx = d_packetCache.get<HashTag>();
   auto range = idx.equal_range(tie(tag,h)); 
 
@@ -112,11 +158,13 @@ bool RecursorPacketCache::getResponsePacket(unsigned int tag, const std::string&
     d_misses++;
     return false;
   }
-    
+
   for(auto iter = range.first ; iter != range.second ; ++ iter) {
     // the possibility is VERY real that we get hits that are not right - birthday paradox
-    if(!qrMatch(queryPacket, iter->d_packet))
+    if(!qrMatch(iter, queryPacket, ecsBegin, ecsEnd)) {
       continue;
+    }
+
     if((uint32_t)now < iter->d_ttd) { // it is right, it is fresh!
       *age = now - iter->d_creation;
       *responsePacket = iter->d_packet;
@@ -153,27 +201,30 @@ bool RecursorPacketCache::getResponsePacket(unsigned int tag, const std::string&
 }
 
 
-void RecursorPacketCache::insertResponsePacket(unsigned int tag, const DNSName& qname, uint16_t qtype, const std::string& queryPacket, const std::string& responsePacket, time_t now, uint32_t ttl)
-{
-  insertResponsePacket(tag, qname, qtype, queryPacket, responsePacket, now, ttl, nullptr);
-}
-
-void RecursorPacketCache::insertResponsePacket(unsigned int tag, const DNSName& qname, uint16_t qtype, const std::string& queryPacket, const std::string& responsePacket, time_t now, uint32_t ttl, const RecProtoBufMessage* protobufMessage)
+void RecursorPacketCache::insertResponsePacket(unsigned int tag, const DNSName& qname, uint16_t qtype, uint16_t qclass, const std::string& queryPacket, const std::string& responsePacket, time_t now, uint32_t ttl, const RecProtoBufMessage* protobufMessage)
 {
-  auto qhash = canHashPacket(queryPacket);
+  uint16_t ecsBegin = 0;
+  uint16_t ecsEnd = 0;
+  auto qhash = canHashPacket(queryPacket, &ecsBegin, &ecsEnd);
   auto& idx = d_packetCache.get<HashTag>();
   auto range = idx.equal_range(tie(tag,qhash));
   auto iter = range.first;
 
   for( ; iter != range.second ; ++iter)  {
-    if(iter->d_type != qtype)
+    if(iter->d_type != qtype || iter->d_class != qclass) {
       continue;
+    }
+
     // this only happens on insert which is relatively rare and does not need to be super fast
     DNSName respname(iter->d_packet.c_str(), iter->d_packet.length(), sizeof(dnsheader), false, 0, 0, 0);
-    if(qname != respname)
+    if(qname != respname) {
       continue;
+    }
     moveCacheItemToBack(d_packetCache, iter);
     iter->d_packet = responsePacket;
+    iter->d_query = queryPacket;
+    iter->d_ecsBegin = ecsBegin;
+    iter->d_ecsEnd = ecsEnd;
     iter->d_ttd = now + ttl;
     iter->d_creation = now;
 #ifdef HAVE_PROTOBUF
@@ -188,9 +239,13 @@ void RecursorPacketCache::insertResponsePacket(unsigned int tag, const DNSName&
   if(iter == range.second) { // nothing to refresh
     struct Entry e;
     e.d_packet = responsePacket;
+    e.d_query = queryPacket;
     e.d_name = qname;
     e.d_qhash = qhash;
     e.d_type = qtype;
+    e.d_class = qclass;
+    e.d_ecsBegin = ecsBegin;
+    e.d_ecsEnd = ecsEnd;
     e.d_ttd = now+ttl;
     e.d_creation = now;
     e.d_tag = tag;
index 07af0c5d5fd623656a9a605e9f603bfa7920ed51..7caf03871946b5028c94fdd41f77a148565d2e12 100644 (file)
@@ -50,10 +50,8 @@ class RecursorPacketCache
 {
 public:
   RecursorPacketCache();
-  bool getResponsePacket(unsigned int tag, const std::string& queryPacket, time_t now, std::string* responsePacket, uint32_t* age);
-  void insertResponsePacket(unsigned int tag, const DNSName& qname, uint16_t qtype, const std::string& queryPacket, const std::string& responsePacket, time_t now, uint32_t ttd);
   bool getResponsePacket(unsigned int tag, const std::string& queryPacket, time_t now, std::string* responsePacket, uint32_t* age, RecProtoBufMessage* protobufMessage);
-  void insertResponsePacket(unsigned int tag, const DNSName& qname, uint16_t qtype, const std::string& queryPacket, const std::string& responsePacket, time_t now, uint32_t ttd, const RecProtoBufMessage* protobufMessage);
+  void insertResponsePacket(unsigned int tag, const DNSName& qname, uint16_t qtype, uint16_t qclass, const std::string& queryPacket, const std::string& responsePacket, time_t now, uint32_t ttd, const RecProtoBufMessage* protobufMessage);
   void doPruneTo(unsigned int maxSize=250000);
   uint64_t doDump(int fd);
   int doWipePacketCache(const DNSName& name, uint16_t qtype=0xffff, bool subtree=false);
@@ -68,16 +66,21 @@ private:
   struct NameTag {};
   struct Entry 
   {
-    mutable uint32_t d_ttd;
-    mutable uint32_t d_creation; // so we can 'age' our packets
     DNSName d_name;
-    uint16_t d_type;
     mutable std::string d_packet; // "I know what I am doing"
+    mutable std::string d_query;
 #ifdef HAVE_PROTOBUF
     mutable RecProtoBufMessage d_protobufMessage;
 #endif
+    mutable uint32_t d_ttd;
+    mutable uint32_t d_creation; // so we can 'age' our packets
     uint32_t d_qhash;
     uint32_t d_tag;
+    mutable uint16_t d_ecsBegin;
+    mutable uint16_t d_ecsEnd;
+    uint16_t d_type;
+    uint16_t d_class;
+
     inline bool operator<(const struct Entry& rhs) const;
     
     uint32_t getTTD() const
@@ -85,7 +88,7 @@ private:
       return d_ttd;
     }
   };
-  uint32_t canHashPacket(const std::string& origPacket);
+
   typedef multi_index_container<
     Entry,
     indexed_by  <
@@ -96,6 +99,11 @@ private:
   > packetCache_t;
   
   packetCache_t d_packetCache;
+
+public:
+  static bool queryMatches(const std::string& cachedQuery, const std::string& query, const DNSName& qname, uint16_t ecsBegin, uint16_t ecsEnd);
+  static bool qrMatch(const packetCache_t::index<HashTag>::type::iterator& iter, const std::string& queryPacket, uint16_t ecsBegin, uint16_t ecsEnd);
+  static uint32_t canHashPacket(const std::string& origPacket, uint16_t* ecsBegin, uint16_t* ecsEnd);
 };
 
 #endif
index f6cd9bb2cea4f7124c731a87bfe17b9199c720d4..5e971c0f20fc29b12f5f0cc03cc06e209688f009 100644 (file)
@@ -9,9 +9,29 @@
 #include "dnswriter.hh"
 #include "dnsrecords.hh"
 #include "dns_random.hh"
+#include "ednsoptions.hh"
+#include "ednssubnet.hh"
 #include "iputils.hh"
 #include <utility>
 
+struct EDNSCookiesOpt
+{
+  string client;
+  string server;
+};
+
+static string makeEDNSCookiesOptString(const EDNSCookiesOpt& eco)
+{
+  string ret;
+  if (eco.client.length() != 8)
+    return ret;
+  if (eco.server.length() != 0 && (eco.server.length() < 8 || eco.server.length() > 32))
+    return ret;
+  ret.assign(eco.client);
+  if (eco.server.length() != 0)
+    ret.append(eco.server);
+  return ret;
+}
 
 BOOST_AUTO_TEST_SUITE(recpacketcache_cc)
 
@@ -33,19 +53,19 @@ BOOST_AUTO_TEST_CASE(test_recPacketCacheSimple) {
   pw.commit();
   string rpacket((const char*)&packet[0], packet.size());
 
-  rpc.insertResponsePacket(0,qname, QType::A, qpacket, rpacket, time(0), 3600);
+  rpc.insertResponsePacket(0,qname, QType::A, QClass::IN, qpacket, rpacket, time(0), 3600, nullptr);
   BOOST_CHECK_EQUAL(rpc.size(), 1);
   rpc.doPruneTo(0);
   BOOST_CHECK_EQUAL(rpc.size(), 0);
-  rpc.insertResponsePacket(0,qname, QType::A, qpacket, rpacket, time(0), 3600);
+  rpc.insertResponsePacket(0,qname, QType::A, QClass::IN, qpacket, rpacket, time(0), 3600, nullptr);
   BOOST_CHECK_EQUAL(rpc.size(), 1);
   rpc.doWipePacketCache(qname);
   BOOST_CHECK_EQUAL(rpc.size(), 0);
 
-  rpc.insertResponsePacket(0,qname, QType::A, qpacket, rpacket, time(0), 3600);
+  rpc.insertResponsePacket(0,qname, QType::A, QClass::IN, qpacket, rpacket, time(0), 3600, nullptr);
   uint32_t age=0;
   string fpacket;
-  bool found = rpc.getResponsePacket(0, qpacket, time(0), &fpacket, &age);
+  bool found = rpc.getResponsePacket(0, qpacket, time(0), &fpacket, &age, nullptr);
   BOOST_CHECK_EQUAL(found, 1);
   BOOST_CHECK_EQUAL(fpacket, rpacket);
 
@@ -57,7 +77,7 @@ BOOST_AUTO_TEST_CASE(test_recPacketCacheSimple) {
   pw2.getHeader()->qr=false;
   pw2.getHeader()->id=random();
   qpacket.assign((const char*)&packet[0], packet.size());
-  found = rpc.getResponsePacket(0, qpacket, time(0), &fpacket, &age);
+  found = rpc.getResponsePacket(0, qpacket, time(0), &fpacket, &age, nullptr);
   BOOST_CHECK_EQUAL(found, 0);
 
   rpc.doWipePacketCache(DNSName("com"), 0xffff, true);
@@ -67,4 +87,140 @@ BOOST_AUTO_TEST_CASE(test_recPacketCacheSimple) {
 
 } 
 
+BOOST_AUTO_TEST_CASE(test_PacketCacheRecCollision) {
+
+  /* rec version (ECS is processed, we hash the whole query except for the ID and the ECS value, while lowercasing the qname) */
+  const DNSName qname("www.powerdns.com.");
+  uint16_t qtype = QType::AAAA;
+  EDNSSubnetOpts opt;
+  DNSPacketWriter::optvect_t ednsOptions;
+  uint16_t ecsBegin;
+  uint16_t ecsEnd;
+
+  {
+    /* same query, different IDs */
+    vector<uint8_t> packet;
+    DNSPacketWriter pw1(packet, qname, qtype);
+    pw1.getHeader()->rd = true;
+    pw1.getHeader()->qr = false;
+    pw1.getHeader()->id = 0x42;
+    string spacket1((const char*)&packet[0], packet.size());
+    auto hash1 = RecursorPacketCache::canHashPacket(spacket1, &ecsBegin, &ecsEnd);
+    /* no ECS */
+    BOOST_CHECK_EQUAL(ecsBegin, 0);
+    BOOST_CHECK_EQUAL(ecsEnd, 0);
+
+    packet.clear();
+    DNSPacketWriter pw2(packet, qname, qtype);
+    pw2.getHeader()->rd = true;
+    pw2.getHeader()->qr = false;
+    pw2.getHeader()->id = 0x84;
+    string spacket2((const char*)&packet[0], packet.size());
+    auto hash2 = RecursorPacketCache::canHashPacket(spacket2, &ecsBegin, &ecsEnd);
+    /* no ECS */
+    BOOST_CHECK_EQUAL(ecsBegin, 0);
+    BOOST_CHECK_EQUAL(ecsEnd, 0);
+
+    BOOST_CHECK_EQUAL(hash1, hash2);
+    BOOST_CHECK(RecursorPacketCache::queryMatches(spacket1, spacket2, qname, ecsBegin, ecsEnd));
+  }
+
+  {
+    /* same query, different IDs, different ECS, still hashes to the same value */
+    vector<uint8_t> packet;
+    DNSPacketWriter pw1(packet, qname, qtype);
+    pw1.getHeader()->rd = true;
+    pw1.getHeader()->qr = false;
+    pw1.getHeader()->id = 0x42;
+    opt.source = Netmask("10.0.18.199/32");
+    ednsOptions.clear();
+    ednsOptions.push_back(std::make_pair(EDNSOptionCode::ECS, makeEDNSSubnetOptsString(opt)));
+    pw1.addOpt(512, 0, 0, ednsOptions);
+    pw1.commit();
+
+    string spacket1((const char*)&packet[0], packet.size());
+    auto hash1 = RecursorPacketCache::canHashPacket(spacket1, &ecsBegin, &ecsEnd);
+    /* ECS value */
+    BOOST_CHECK_EQUAL(ecsBegin, sizeof(dnsheader) + qname.wirelength() + ( 2 * sizeof(uint16_t)) /* qtype */ + (2 * sizeof(uint16_t)) /* qclass */ + /* OPT root label */ 1 + sizeof(uint32_t) /* TTL */ + DNS_RDLENGTH_SIZE);
+    BOOST_CHECK_EQUAL(ecsEnd, ecsBegin + EDNS_OPTION_CODE_SIZE + EDNS_OPTION_LENGTH_SIZE + 2 /* family */ + 1 /* scope length */ + 1 /* source length */ + 4 /* IPv4 */);
+
+    packet.clear();
+    DNSPacketWriter pw2(packet, qname, qtype);
+    pw2.getHeader()->rd = true;
+    pw2.getHeader()->qr = false;
+    pw2.getHeader()->id = 0x84;
+    opt.source = Netmask("10.0.131.66/32");
+    ednsOptions.clear();
+    ednsOptions.push_back(std::make_pair(EDNSOptionCode::ECS, makeEDNSSubnetOptsString(opt)));
+    pw2.addOpt(512, 0, 0, ednsOptions);
+    pw2.commit();
+
+    string spacket2((const char*)&packet[0], packet.size());
+    auto hash2 = RecursorPacketCache::canHashPacket(spacket2, &ecsBegin, &ecsEnd);
+    /* ECS value */
+    BOOST_CHECK_EQUAL(ecsBegin, sizeof(dnsheader) + qname.wirelength() + ( 2 * sizeof(uint16_t)) /* qtype */ + (2 * sizeof(uint16_t)) /* qclass */ + /* OPT root label */ 1 + sizeof(uint32_t) /* TTL */ + DNS_RDLENGTH_SIZE);
+    BOOST_CHECK_EQUAL(ecsEnd, ecsBegin + EDNS_OPTION_CODE_SIZE + EDNS_OPTION_LENGTH_SIZE + 2 /* family */ + 1 /* scope length */ + 1 /* source length */ + 4 /* IPv4 */);
+
+    BOOST_CHECK_EQUAL(hash1, hash2);
+    /* the hash is the same and we don't hash the ECS so we should match */
+    BOOST_CHECK(RecursorPacketCache::queryMatches(spacket1, spacket2, qname, ecsBegin, ecsEnd));
+  }
+
+  {
+    /* same query but different cookies, still hashes to the same value */
+    vector<uint8_t> packet;
+    DNSPacketWriter pw1(packet, qname, qtype);
+    pw1.getHeader()->rd = true;
+    pw1.getHeader()->qr = false;
+    pw1.getHeader()->id = 0x42;
+    opt.source = Netmask("192.0.2.1/32");
+    ednsOptions.clear();
+    ednsOptions.push_back(std::make_pair(EDNSOptionCode::ECS, makeEDNSSubnetOptsString(opt)));
+    EDNSCookiesOpt cookiesOpt;
+    cookiesOpt.client = string("deadbeef");
+    cookiesOpt.server = string("deadbeef");
+    cookiesOpt.server[4] = -20;
+    cookiesOpt.server[5] = -114;
+    cookiesOpt.server[6] = 0;
+    cookiesOpt.server[7] = 0;
+    ednsOptions.push_back(std::make_pair(EDNSOptionCode::COOKIE, makeEDNSCookiesOptString(cookiesOpt)));
+    pw1.addOpt(512, 0, EDNSOpts::DNSSECOK, ednsOptions);
+    pw1.commit();
+
+    string spacket1((const char*)&packet[0], packet.size());
+    auto hash1 = RecursorPacketCache::canHashPacket(spacket1, &ecsBegin, &ecsEnd);
+    /* ECS value */
+    BOOST_CHECK_EQUAL(ecsBegin, sizeof(dnsheader) + qname.wirelength() + ( 2 * sizeof(uint16_t)) /* qtype */ + (2 * sizeof(uint16_t)) /* qclass */ + /* OPT root label */ 1 + sizeof(uint32_t) /* TTL */ + DNS_RDLENGTH_SIZE);
+    BOOST_CHECK_EQUAL(ecsEnd, ecsBegin + EDNS_OPTION_CODE_SIZE + EDNS_OPTION_LENGTH_SIZE + 2 /* family */ + 1 /* scope length */ + 1 /* source length */ + 4 /* IPv4 */);
+
+    packet.clear();
+    DNSPacketWriter pw2(packet, qname, qtype);
+    pw2.getHeader()->rd = true;
+    pw2.getHeader()->qr = false;
+    pw2.getHeader()->id = 0x84;
+    opt.source = Netmask("192.0.2.1/32");
+    ednsOptions.clear();
+    ednsOptions.push_back(std::make_pair(EDNSOptionCode::ECS, makeEDNSSubnetOptsString(opt)));
+    cookiesOpt.client = string("deadbeef");
+    cookiesOpt.server = string("deadbeef");
+    cookiesOpt.server[4] = 103;
+    cookiesOpt.server[5] = 68;
+    cookiesOpt.server[6] = 0;
+    cookiesOpt.server[7] = 0;
+    ednsOptions.push_back(std::make_pair(EDNSOptionCode::COOKIE, makeEDNSCookiesOptString(cookiesOpt)));
+    pw2.addOpt(512, 0, EDNSOpts::DNSSECOK, ednsOptions);
+    pw2.commit();
+
+    string spacket2((const char*)&packet[0], packet.size());
+    auto hash2 = RecursorPacketCache::canHashPacket(spacket2, &ecsBegin, &ecsEnd);
+    /* ECS value */
+    BOOST_CHECK_EQUAL(ecsBegin, sizeof(dnsheader) + qname.wirelength() + ( 2 * sizeof(uint16_t)) /* qtype */ + (2 * sizeof(uint16_t)) /* qclass */ + /* OPT root label */ 1 + sizeof(uint32_t) /* TTL */ + DNS_RDLENGTH_SIZE);
+    BOOST_CHECK_EQUAL(ecsEnd, ecsBegin + EDNS_OPTION_CODE_SIZE + EDNS_OPTION_LENGTH_SIZE + 2 /* family */ + 1 /* scope length */ + 1 /* source length */ + 4 /* IPv4 */);
+
+    BOOST_CHECK_EQUAL(hash1, hash2);
+    /* the hash is the same but we should _not_ match, even though we skip the ECS part, because the cookies are different */
+    BOOST_CHECK(!RecursorPacketCache::queryMatches(spacket1, spacket2, qname, ecsBegin, ecsEnd));
+  }
+}
+
 BOOST_AUTO_TEST_SUITE_END()