From c1f098242b6185b0ebe2697fb58f7236421d4aa7 Mon Sep 17 00:00:00 2001 From: Charles-Henri Bruyand Date: Wed, 6 Oct 2021 12:14:12 +0200 Subject: [PATCH] remove DNSDistPacketCache::setCookieHashing and clarify documentation and code about skipped options --- pdns/dnsdist-cache.cc | 18 +----------------- pdns/dnsdist-cache.hh | 4 +--- .../dnsdist-lua-bindings-packetcache.cc | 10 +++++----- pdns/dnsdistdist/docs/reference/config.rst | 2 +- pdns/fuzz_dnsdistcache.cc | 5 +++-- 5 files changed, 11 insertions(+), 28 deletions(-) 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& 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& 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& optionsToSkip); + void setSkippedOptions(const std::unordered_set& 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 optionsToSkip{}; + std::unordered_set optionsToSkip{EDNSOptionCode::COOKIE}; if (vars) { @@ -91,7 +90,9 @@ void setupLuaBindingsPacketCache(LuaContext& luaCtx, bool client) } if (vars->count("cookieHashing")) { - cookieHashing = boost::get((*vars)["cookieHashing"]); + if (boost::get((*vars)["cookieHashing"])) { + optionsToSkip.erase(EDNSOptionCode::COOKIE); + } } if (vars->count("skipOptions")) { for (auto option: boost::get>>(vars->at("skipOptions"))) { @@ -114,8 +115,7 @@ void setupLuaBindingsPacketCache(LuaContext& luaCtx, bool client) auto res = std::make_shared(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; -- 2.47.2