From: Charles-Henri Bruyand <charles-henri.bruyand@open-xchange.com>
Date: Wed, 6 Oct 2021 10:14:12 +0000 (+0200)
Subject: remove DNSDistPacketCache::setCookieHashing and clarify documentation and code about... 
X-Git-Tag: dnsdist-1.7.0-alpha2~34^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c1f098242b6185b0ebe2697fb58f7236421d4aa7;p=thirdparty%2Fpdns.git

remove DNSDistPacketCache::setCookieHashing and clarify documentation and code about skipped options
---

diff --git a/pdns/dnsdist-cache.cc b/pdns/dnsdist-cache.cc
index 4d14e2f24a..d023dee942 100644
--- a/pdns/dnsdist-cache.cc
+++ b/pdns/dnsdist-cache.cc
@@ -480,23 +480,7 @@ uint64_t DNSDistPacketCache::dump(int fd)
   return count;
 }
 
-bool DNSDistPacketCache::isCookieHashingEnabled() const
+void DNSDistPacketCache::setSkippedOptions(const std::unordered_set<uint16_t>& optionsToSkip)
 {
-  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 a9462420e6..3309459ecb 100644
--- a/pdns/dnsdist-cache.hh
+++ b/pdns/dnsdist-cache.hh
@@ -55,9 +55,7 @@ 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);
+  void setSkippedOptions(const std::unordered_set<uint16_t>& optionsToSkip);
 
   bool isECSParsingEnabled() const { return d_parseECS; }
 
diff --git a/pdns/dnsdistdist/dnsdist-lua-bindings-packetcache.cc b/pdns/dnsdistdist/dnsdist-lua-bindings-packetcache.cc
index feddcef740..f77051e417 100644
--- a/pdns/dnsdistdist/dnsdist-lua-bindings-packetcache.cc
+++ b/pdns/dnsdistdist/dnsdist-lua-bindings-packetcache.cc
@@ -45,8 +45,7 @@ void setupLuaBindingsPacketCache(LuaContext& luaCtx, bool client)
       bool dontAge = false;
       bool deferrableInsertLock = true;
       bool ecsParsing = false;
-      bool cookieHashing = false;
-      std::unordered_set<uint16_t> optionsToSkip{};
+      std::unordered_set<uint16_t> optionsToSkip{EDNSOptionCode::COOKIE};
 
       if (vars) {
 
@@ -91,7 +90,9 @@ void setupLuaBindingsPacketCache(LuaContext& luaCtx, bool client)
         }
 
         if (vars->count("cookieHashing")) {
-          cookieHashing = boost::get<bool>((*vars)["cookieHashing"]);
+          if (boost::get<bool>((*vars)["cookieHashing"])) {
+            optionsToSkip.erase(EDNSOptionCode::COOKIE);
+          }
         }
         if (vars->count("skipOptions")) {
           for (auto option: boost::get<std::vector<std::pair<int, uint16_t>>>(vars->at("skipOptions"))) {
@@ -114,8 +115,7 @@ void setupLuaBindingsPacketCache(LuaContext& luaCtx, bool client)
       auto res = std::make_shared<DNSDistPacketCache>(maxEntries, maxTTL, minTTL, tempFailTTL, maxNegativeTTL, staleTTL, dontAge, numberOfShards, deferrableInsertLock, ecsParsing);
 
       res->setKeepStaleData(keepStaleData);
-      res->setCookieHashing(cookieHashing);
-      res->skipOptions(optionsToSkip);
+      res->setSkippedOptions(optionsToSkip);
 
       return res;
     });
diff --git a/pdns/dnsdistdist/docs/reference/config.rst b/pdns/dnsdistdist/docs/reference/config.rst
index 2a010057f1..e001042d9e 100644
--- a/pdns/dnsdistdist/docs/reference/config.rst
+++ b/pdns/dnsdistdist/docs/reference/config.rst
@@ -797,7 +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).
+  * ``skipOptions={}``: Extra list of EDNS option codes to skip when hashing the packet (if ``cookieHashing`` above is false, EDNS cookie option number will already be added to this list).
 
 .. class:: PacketCache
 
diff --git a/pdns/fuzz_dnsdistcache.cc b/pdns/fuzz_dnsdistcache.cc
index eb68950bdd..c224449533 100644
--- a/pdns/fuzz_dnsdistcache.cc
+++ b/pdns/fuzz_dnsdistcache.cc
@@ -32,12 +32,13 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
 
   /* dnsdist's version */
   DNSDistPacketCache pcSkipCookies(10000);
+  // By default, cookies are not hashed
   pcSkipCookies.setECSParsingEnabled(true);
-  pcSkipCookies.setCookieHashing(false);
 
   DNSDistPacketCache pcHashCookies(10000);
   pcHashCookies.setECSParsingEnabled(true);
-  pcHashCookies.setCookieHashing(true);
+  // Do not skip cookies
+  pcHashCookies.setSkippedOptions({});
 
   try {
     uint16_t qtype;