dnsdist: allow skipping arbitrary EDNS options when computing packets hash

diff --git a/pdns/dnsdist-cache.cc b/pdns/dnsdist-cache.cc
index 8b41f6845480ab96ea24e45a272f56fc4148f965..4d14e2f24a90c15853a7be9f28258216d16c8e92 100644 (file)
--- a/pdns/dnsdist-cache.cc
+++ b/pdns/dnsdist-cache.cc
@@ -26,7 +26,6 @@
 #include "dnsparser.hh"
 #include "dnsdist-cache.hh"
 #include "dnsdist-ecs.hh"
-#include "ednsoptions.hh"
 #include "ednssubnet.hh"
 #include "packetcache.hh"
 
@@ -417,9 +416,9 @@ uint32_t DNSDistPacketCache::getKey(const DNSName::string_t& qname, size_t qname
     throw std::range_error("Computing packet cache key for an invalid packet (" + std::to_string(packet.size()) + " < " + std::to_string(sizeof(dnsheader) + qnameWireLength) + ")");
   }
   if (packet.size() > ((sizeof(dnsheader) + qnameWireLength))) {
-    if (!d_cookieHashing) {
-      /* skip EDNS Cookie options if any */
-      result = PacketCache::hashAfterQname(pdns_string_view(reinterpret_cast<const char*>(packet.data()), packet.size()), result, sizeof(dnsheader) + qnameWireLength, false);
+    if (!d_optionsToSkip.empty()) {
+      /* skip EDNS options if any */
+      result = PacketCache::hashAfterQname(pdns_string_view(reinterpret_cast<const char*>(packet.data()), packet.size()), result, sizeof(dnsheader) + qnameWireLength, d_optionsToSkip);
     }
     else {
       result = burtle(&packet.at(sizeof(dnsheader) + qnameWireLength), packet.size() - (sizeof(dnsheader) + qnameWireLength), result);
@@ -480,3 +479,24 @@ uint64_t DNSDistPacketCache::dump(int fd)
 
   return count;
 }
+
+bool DNSDistPacketCache::isCookieHashingEnabled() const
+{
+  return d_optionsToSkip.count(EDNSOptionCode::COOKIE) == 0;
+}
+
+void DNSDistPacketCache::setCookieHashing(bool hashing)
+{
+  if (hashing) {
+    d_optionsToSkip.erase(EDNSOptionCode::COOKIE);
+  } else {
+    d_optionsToSkip.insert(EDNSOptionCode::COOKIE);
+  }
+}
+
+void DNSDistPacketCache::skipOptions(const std::unordered_set<uint16_t>& optionsToSkip)
+{
+  bool cookieHasingEnabled = isCookieHashingEnabled();
+  d_optionsToSkip = optionsToSkip;
+  setCookieHashing(cookieHasingEnabled);
+}

diff --git a/pdns/dnsdist-cache.hh b/pdns/dnsdist-cache.hh
index 0fbcb772b922f8655f72ae482d440282a5666810..f2ae2a45ae7563ac065d817f818d5c70b95dc68c 100644 (file)
--- a/pdns/dnsdist-cache.hh
+++ b/pdns/dnsdist-cache.hh
@@ -28,6 +28,7 @@
 #include "lock.hh"
 #include "noinitvector.hh"
 #include "stat_t.hh"
+#include "ednsoptions.hh"
 
 struct DNSQuestion;
 
@@ -54,9 +55,11 @@ public:
   uint64_t getTTLTooShorts() const { return d_ttlTooShorts; }
   uint64_t getEntriesCount();
   uint64_t dump(int fd);
+  bool isCookieHashingEnabled() const;
+  void setCookieHashing(bool hashing);
+  void skipOptions(const std::unordered_set<uint16_t>& optionsToSkip);
 
   bool isECSParsingEnabled() const { return d_parseECS; }
-  bool isCookieHashingEnabled() const { return d_cookieHashing; }
 
   bool keepStaleData() const
   {
@@ -67,10 +70,6 @@ public:
     d_keepStaleData = keep;
   }
 
-  void setCookieHashing(bool hashing)
-  {
-    d_cookieHashing = hashing;
-  }
 
   void setECSParsingEnabled(bool enabled)
   {
@@ -144,5 +143,5 @@ private:
   bool d_deferrableInsertLock;
   bool d_parseECS;
   bool d_keepStaleData{false};
-  bool d_cookieHashing{false};
+  std::unordered_set<uint16_t> d_optionsToSkip{EDNSOptionCode::COOKIE};
 };

diff --git a/pdns/dnsdistdist/dnsdist-lua-bindings-packetcache.cc b/pdns/dnsdistdist/dnsdist-lua-bindings-packetcache.cc
index 7cadee528bf2b3d2380d8ea349fdc20b96693911..feddcef7401bed401d14036dcfa9fb5594568c79 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-lua-bindings-packetcache.cc
+++ b/pdns/dnsdistdist/dnsdist-lua-bindings-packetcache.cc
@@ -28,10 +28,12 @@
 #include "dnsdist.hh"
 #include "dnsdist-lua.hh"
 
+#include <boost/lexical_cast.hpp>
+
 void setupLuaBindingsPacketCache(LuaContext& luaCtx, bool client)
 {
   /* PacketCache */
-  luaCtx.writeFunction("newPacketCache", [client](size_t maxEntries, boost::optional<std::unordered_map<std::string, boost::variant<bool, size_t>>> vars) {
+  luaCtx.writeFunction("newPacketCache", [client](size_t maxEntries, boost::optional<std::unordered_map<std::string, boost::variant<bool, size_t, std::vector<std::pair<int, uint16_t>>>>> vars) {
 
       bool keepStaleData = false;
       size_t maxTTL = 86400;
@@ -44,6 +46,7 @@ void setupLuaBindingsPacketCache(LuaContext& luaCtx, bool client)
       bool deferrableInsertLock = true;
       bool ecsParsing = false;
       bool cookieHashing = false;
+      std::unordered_set<uint16_t> optionsToSkip{};
 
       if (vars) {
 
@@ -90,6 +93,11 @@ void setupLuaBindingsPacketCache(LuaContext& luaCtx, bool client)
         if (vars->count("cookieHashing")) {
           cookieHashing = boost::get<bool>((*vars)["cookieHashing"]);
         }
+        if (vars->count("skipOptions")) {
+          for (auto option: boost::get<std::vector<std::pair<int, uint16_t>>>(vars->at("skipOptions"))) {
+            optionsToSkip.insert(option.second);
+          }
+        }
       }
 
       if (maxEntries < numberOfShards) {
@@ -107,6 +115,7 @@ void setupLuaBindingsPacketCache(LuaContext& luaCtx, bool client)
 
       res->setKeepStaleData(keepStaleData);
       res->setCookieHashing(cookieHashing);
+      res->skipOptions(optionsToSkip);
 
       return res;
     });

diff --git a/pdns/dnsdistdist/docs/reference/config.rst b/pdns/dnsdistdist/docs/reference/config.rst
index b6c07bbd2d16d35ae065e7ca7946abe08f5f7936..2a010057f1c67b89bb34f95dfd2be2a25a69c706 100644 (file)
--- a/pdns/dnsdistdist/docs/reference/config.rst
+++ b/pdns/dnsdistdist/docs/reference/config.rst
@@ -777,6 +777,9 @@ See :doc:`../guides/cache` for a how to.
     ``cookieHashing`` parameter added.
     ``numberOfShards`` now defaults to 20.
 
+  .. versionchanged:: 1.7.0
+    ``skipOptions`` parameter added.
+
   Creates a new :class:`PacketCache` with the settings specified.
 
   :param int maxEntries: The maximum number of entries in this cache
@@ -794,6 +797,7 @@ See :doc:`../guides/cache` for a how to.
   * ``staleTTL=60``: int - When the backend servers are not reachable, and global configuration ``setStaleCacheEntriesTTL`` is set appropriately, TTL that will be used when a stale cache entry is returned.
   * ``temporaryFailureTTL=60``: int - On a SERVFAIL or REFUSED from the backend, cache for this amount of seconds..
   * ``cookieHashing=false``: bool - Whether EDNS Cookie values will be hashed, resulting in separate entries for different cookies in the packet cache. This is required if the backend is sending answers with EDNS Cookies, otherwise a client might receive an answer with the wrong cookie.
+  * ``skipOptions={}``: Extra list of EDNS option codes to skip when hashing the packet (see ``cookieHashing`` above).
 
 .. class:: PacketCache
 

diff --git a/pdns/packetcache.hh b/pdns/packetcache.hh
index 8aa6933f4d3288dc6ed7af0fb57c655ae3a36541..15f9d2de46428435afc3afdc6c6aefeba01d4c43 100644 (file)
--- a/pdns/packetcache.hh
+++ b/pdns/packetcache.hh
@@ -31,9 +31,9 @@ public:
   /* hash the packet from the provided position, which should point right after tje qname. This skips:
      - the query ID ;
      - EDNS Cookie options, if any ;
-     - EDNS Client Subnet options, if any and skipECS is true.
+     - Any given option code present in optionsToSkip
   */
-  static uint32_t hashAfterQname(const pdns_string_view& packet, uint32_t currentHash, size_t pos, bool skipECS)
+  static uint32_t hashAfterQname(const pdns_string_view& packet, uint32_t currentHash, size_t pos, const std::unordered_set<uint16_t>& optionsToSkip = {EDNSOptionCode::COOKIE})
   {
     const size_t packetSize = packet.size();
     assert(packetSize >= sizeof(dnsheader));
@@ -82,22 +82,12 @@ public:
         return currentHash;
       }
 
-      bool skip = false;
-      if (optionCode == EDNSOptionCode::COOKIE) {
-        skip = true;
-      }
-      else if (optionCode == EDNSOptionCode::ECS) {
-        if (skipECS) {
-          skip = true;
-        }
-      }
-
-      if (!skip) {
+      if (optionsToSkip.count(optionCode) == 0) {
         /* hash the option code, length and content */
         currentHash = burtle(reinterpret_cast<const unsigned char*>(&packet.at(pos)), 4 + optionLen, currentHash);
       }
       else {
-        /* hash the option code and length */
+        /* skip option: hash only its code and length */
         currentHash = burtle(reinterpret_cast<const unsigned char*>(&packet.at(pos)), 4, currentHash);
       }
 
@@ -140,9 +130,9 @@ public:
   /* hash the packet from the beginning, including the qname. This skips:
      - the query ID ;
      - EDNS Cookie options, if any ;
-     - EDNS Client Subnet options, if any and skipECS is true.
+     - Any given option code present in optionsToSkip
   */
-  static uint32_t canHashPacket(const std::string& packet, bool skipECS)
+  static uint32_t canHashPacket(const std::string& packet, const std::unordered_set<uint16_t>& optionsToSkip = {EDNSOptionCode::COOKIE})
   {
     size_t pos = 0;
     uint32_t currentHash = hashHeaderAndQName(packet, pos);
@@ -152,7 +142,7 @@ public:
       return currentHash;
     }
 
-    return hashAfterQname(packet, currentHash, pos, skipECS);
+    return hashAfterQname(packet, currentHash, pos, optionsToSkip);
   }
 
   static bool queryHeaderMatches(const std::string& cachedQuery, const std::string& query)

diff --git a/pdns/recpacketcache.cc b/pdns/recpacketcache.cc
index d22447f8364f6ed445d36cd61efa8829047db49c..a2d19c570e62178241c4d6bede7c3b3ea478ab98 100644 (file)
--- a/pdns/recpacketcache.cc
+++ b/pdns/recpacketcache.cc
@@ -121,10 +121,12 @@ bool RecursorPacketCache::getResponsePacket(unsigned int tag, const std::string&
   return getResponsePacket(tag, queryPacket, qname, qtype, qclass, now, responsePacket, age, &valState, qhash, nullptr, false);
 }
 
+static const std::unordered_set<uint16_t> s_skipOptions = {EDNSOptionCode::ECS, EDNSOptionCode::COOKIE};
+
 bool RecursorPacketCache::getResponsePacket(unsigned int tag, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass, time_t now,
                                             std::string* responsePacket, uint32_t* age, vState* valState, uint32_t* qhash, OptPBData* pbdata, bool tcp)
 {
-  *qhash = canHashPacket(queryPacket, true);
+  *qhash = canHashPacket(queryPacket, s_skipOptions);
   const auto& idx = d_packetCache.get<HashTag>();
   auto range = idx.equal_range(tie(tag, *qhash, tcp));
 
@@ -139,7 +141,7 @@ bool RecursorPacketCache::getResponsePacket(unsigned int tag, const std::string&
 bool RecursorPacketCache::getResponsePacket(unsigned int tag, const std::string& queryPacket, DNSName& qname, uint16_t* qtype, uint16_t* qclass, time_t now,
                                             std::string* responsePacket, uint32_t* age, vState* valState, uint32_t* qhash, OptPBData *pbdata, bool tcp)
 {
-  *qhash = canHashPacket(queryPacket, true);
+  *qhash = canHashPacket(queryPacket, s_skipOptions);
   const auto& idx = d_packetCache.get<HashTag>();
   auto range = idx.equal_range(tie(tag, *qhash, tcp));
 

diff --git a/pdns/test-packetcache_hh.cc b/pdns/test-packetcache_hh.cc
index 64c60e972ea122acd9bd75429b3d08139b9d1ec2..4914b055219eca9c9381c8ee555c2519871f4395 100644 (file)
--- a/pdns/test-packetcache_hh.cc
+++ b/pdns/test-packetcache_hh.cc
@@ -32,7 +32,7 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheAuthCollision) {
     pw1.getHeader()->qr = false;
     pw1.getHeader()->id = 0x42;
     string spacket1((const char*)&packet[0], packet.size());
-    auto hash1 = PacketCache::canHashPacket(spacket1, false);
+    auto hash1 = PacketCache::canHashPacket(spacket1, optionsToSkip);
 
     packet.clear();
     DNSPacketWriter pw2(packet, qname, qtype);
@@ -40,7 +40,7 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheAuthCollision) {
     pw2.getHeader()->qr = false;
     pw2.getHeader()->id = 0x84;
     string spacket2((const char*)&packet[0], packet.size());
-    auto hash2 = PacketCache::canHashPacket(spacket2, false);
+    auto hash2 = PacketCache::canHashPacket(spacket2, optionsToSkip);
 
     BOOST_CHECK_EQUAL(hash1, hash2);
     BOOST_CHECK(PacketCache::queryMatches(spacket1, spacket2, qname, optionsToSkip));
@@ -60,7 +60,7 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheAuthCollision) {
     pw1.commit();
 
     string spacket1((const char*)&packet[0], packet.size());
-    auto hash1 = PacketCache::canHashPacket(spacket1, false);
+    auto hash1 = PacketCache::canHashPacket(spacket1, optionsToSkip);
 
     packet.clear();
     DNSPacketWriter pw2(packet, qname, qtype);
@@ -74,7 +74,7 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheAuthCollision) {
     pw2.commit();
 
     string spacket2((const char*)&packet[0], packet.size());
-    auto hash2 = PacketCache::canHashPacket(spacket2, false);
+    auto hash2 = PacketCache::canHashPacket(spacket2, optionsToSkip);
 
     BOOST_CHECK_EQUAL(hash1, hash2);
     /* the hash is the same but we should _not_ match */
@@ -100,7 +100,7 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheAuthCollision) {
           ednsOptions.push_back(std::make_pair(EDNSOptionCode::ECS, makeEDNSSubnetOptsString(opt)));
           pwFQ.addOpt(512, 0, 0, ednsOptions);
           pwFQ.commit();
-          auto secondKey = PacketCache::canHashPacket(std::string(reinterpret_cast<const char *>(secondQuery.data()), secondQuery.size()), false);
+          auto secondKey = PacketCache::canHashPacket(std::string(reinterpret_cast<const char *>(secondQuery.data()), secondQuery.size()), optionsToSkip);
           auto pair = colMap.insert(std::make_pair(secondKey, opt.source));
           total++;
           if (!pair.second) {
@@ -132,7 +132,7 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheAuthCollision) {
     pw1.commit();
 
     string spacket1((const char*)&packet[0], packet.size());
-    auto hash1 = PacketCache::canHashPacket(spacket1, false);
+    auto hash1 = PacketCache::canHashPacket(spacket1, optionsToSkip);
 
     packet.clear();
     DNSPacketWriter pw2(packet, qname, qtype);
@@ -147,7 +147,7 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheAuthCollision) {
     pw2.commit();
 
     string spacket2((const char*)&packet[0], packet.size());
-    auto hash2 = PacketCache::canHashPacket(spacket2, false);
+    auto hash2 = PacketCache::canHashPacket(spacket2, optionsToSkip);
 
     BOOST_CHECK_EQUAL(hash1, hash2);
     /* the hash is the same but we should _not_ match */
@@ -170,7 +170,7 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheAuthCollision) {
     pw1.commit();
 
     string spacket1((const char*)&packet[0], packet.size());
-    auto hash1 = PacketCache::canHashPacket(spacket1, false);
+    auto hash1 = PacketCache::canHashPacket(spacket1, optionsToSkip);
 
     packet.clear();
     DNSPacketWriter pw2(packet, qname, qtype);
@@ -186,7 +186,7 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheAuthCollision) {
     pw2.commit();
 
     string spacket2((const char*)&packet[0], packet.size());
-    auto hash2 = PacketCache::canHashPacket(spacket2, false);
+    auto hash2 = PacketCache::canHashPacket(spacket2, optionsToSkip);
 
     BOOST_CHECK_EQUAL(hash1, hash2);
     /* the hash is the same but we should _not_ match */
@@ -214,7 +214,7 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheAuthCollision) {
           ednsOptions.push_back(std::make_pair(EDNSOptionCode::ECS, makeEDNSSubnetOptsString(opt)));
           pwFQ.addOpt(512, 0, 32768, ednsOptions);
           pwFQ.commit();
-          auto secondKey = PacketCache::canHashPacket(std::string(reinterpret_cast<const char *>(secondQuery.data()), secondQuery.size()), false);
+          auto secondKey = PacketCache::canHashPacket(std::string(reinterpret_cast<const char *>(secondQuery.data()), secondQuery.size()), optionsToSkip);
           colMap.insert(std::make_pair(secondKey, opt.source));
 
           secondQuery.clear();
@@ -227,7 +227,7 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheAuthCollision) {
           ednsOptions.push_back(std::make_pair(EDNSOptionCode::ECS, makeEDNSSubnetOptsString(opt)));
           pwSQ.addOpt(512, 0, 0, ednsOptions);
           pwSQ.commit();
-          secondKey = PacketCache::canHashPacket(std::string(reinterpret_cast<const char *>(secondQuery.data()), secondQuery.size()), false);
+          secondKey = PacketCache::canHashPacket(std::string(reinterpret_cast<const char *>(secondQuery.data()), secondQuery.size()), optionsToSkip);
 
           total++;
           if (colMap.count(secondKey)) {
@@ -252,6 +252,7 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheRecSimple) {
   uint16_t qtype = QType::AAAA;
   EDNSSubnetOpts opt;
   DNSPacketWriter::optvect_t ednsOptions;
+  static const std::unordered_set<uint16_t> optionsToSkip{ EDNSOptionCode::COOKIE, EDNSOptionCode::ECS };
 
   {
     vector<uint8_t> packet;
@@ -269,7 +270,7 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheRecSimple) {
     *(ptr + 1) = 255;
     /* truncate the end of the OPT header to try to trigger an out of bounds read */
     spacket1.resize(spacket1.size() - 6);
-    BOOST_CHECK_NO_THROW(PacketCache::canHashPacket(spacket1, true));
+    BOOST_CHECK_NO_THROW(PacketCache::canHashPacket(spacket1, optionsToSkip));
   }
 }
 
@@ -290,7 +291,7 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheRecCollision) {
     pw1.getHeader()->qr = false;
     pw1.getHeader()->id = 0x42;
     string spacket1((const char*)&packet[0], packet.size());
-    auto hash1 = PacketCache::canHashPacket(spacket1, true);
+    auto hash1 = PacketCache::canHashPacket(spacket1, optionsToSkip);
 
     packet.clear();
     DNSPacketWriter pw2(packet, qname, qtype);
@@ -298,7 +299,7 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheRecCollision) {
     pw2.getHeader()->qr = false;
     pw2.getHeader()->id = 0x84;
     string spacket2((const char*)&packet[0], packet.size());
-    auto hash2 = PacketCache::canHashPacket(spacket2, true);
+    auto hash2 = PacketCache::canHashPacket(spacket2, optionsToSkip);
 
     BOOST_CHECK_EQUAL(hash1, hash2);
     BOOST_CHECK(PacketCache::queryMatches(spacket1, spacket2, qname, optionsToSkip));
@@ -318,7 +319,7 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheRecCollision) {
     pw1.commit();
 
     string spacket1((const char*)&packet[0], packet.size());
-    auto hash1 = PacketCache::canHashPacket(spacket1, true);
+    auto hash1 = PacketCache::canHashPacket(spacket1, optionsToSkip);
 
     packet.clear();
     DNSPacketWriter pw2(packet, qname, qtype);
@@ -332,7 +333,7 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheRecCollision) {
     pw2.commit();
 
     string spacket2((const char*)&packet[0], packet.size());
-    auto hash2 = PacketCache::canHashPacket(spacket2, true);
+    auto hash2 = PacketCache::canHashPacket(spacket2, optionsToSkip);
 
     BOOST_CHECK_EQUAL(hash1, hash2);
     /* the hash is the same and we don't hash the ECS so we should match */
@@ -355,7 +356,7 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheRecCollision) {
     pw1.commit();
 
     string spacket1((const char*)&packet[0], packet.size());
-    auto hash1 = PacketCache::canHashPacket(spacket1, true);
+    auto hash1 = PacketCache::canHashPacket(spacket1, optionsToSkip);
 
     packet.clear();
     DNSPacketWriter pw2(packet, qname, qtype);
@@ -371,7 +372,7 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheRecCollision) {
     pw2.commit();
 
     string spacket2((const char*)&packet[0], packet.size());
-    auto hash2 = PacketCache::canHashPacket(spacket2, true);
+    auto hash2 = PacketCache::canHashPacket(spacket2, optionsToSkip);
 
     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 */