From: Remi Gacogne Date: Thu, 10 Apr 2025 15:05:21 +0000 (+0200) Subject: dnsdist: Add an option to cache truncated answers X-Git-Tag: dnsdist-2.0.0-alpha2~83^2~1 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9dc39e62803b6ccbfb6a0cfab9baf7fc3f47b0f1;p=thirdparty%2Fpdns.git dnsdist: Add an option to cache truncated answers --- diff --git a/pdns/dnsdistdist/dnsdist-cache.cc b/pdns/dnsdistdist/dnsdist-cache.cc index 12eb6e81ee..6aeee8a3ab 100644 --- a/pdns/dnsdistdist/dnsdist-cache.cc +++ b/pdns/dnsdistdist/dnsdist-cache.cc @@ -151,7 +151,14 @@ void DNSDistPacketCache::insert(uint32_t key, const boost::optional& su /* no TTL found, we don't want to cache this */ if (minTTL == std::numeric_limits::max()) { - return; + if (d_settings.d_truncatedTTL == 0) { + return; + } + dnsheader_aligned dh_aligned(response.data()); + if (dh_aligned->tc == 0) { + return; + } + minTTL = d_settings.d_truncatedTTL; } if (rcode == RCode::NXDomain || (rcode == RCode::NoError && seenAuthSOA)) { @@ -267,9 +274,8 @@ bool DNSDistPacketCache::get(DNSQuestion& dnsQuestion, uint16_t queryId, uint32_ } if (!truncatedOK) { - dnsheader dnsHeader{}; - memcpy(&dnsHeader, value.value.data(), sizeof(dnsHeader)); - if (dnsHeader.tc != 0) { + dnsheader_aligned dh_aligned(value.value.data()); + if (dh_aligned->tc != 0) { return false; } } @@ -548,9 +554,8 @@ std::set DNSDistPacketCache::getDomainsContainingRecords(const ComboAdd continue; } - dnsheader dnsHeader{}; - memcpy(&dnsHeader, value.value.data(), sizeof(dnsheader)); - if (dnsHeader.rcode != RCode::NoError || (dnsHeader.ancount == 0 && dnsHeader.nscount == 0 && dnsHeader.arcount == 0)) { + dnsheader_aligned dnsHeader(value.value.data()); + if (dnsHeader->rcode != RCode::NoError || (dnsHeader->ancount == 0 && dnsHeader->nscount == 0 && dnsHeader->arcount == 0)) { continue; } @@ -606,13 +611,12 @@ std::set DNSDistPacketCache::getRecordsForDomain(const DNSName& do continue; } - dnsheader dnsHeader{}; if (value.len < sizeof(dnsheader)) { continue; } - memcpy(&dnsHeader, value.value.data(), sizeof(dnsheader)); - if (dnsHeader.rcode != RCode::NoError || (dnsHeader.ancount == 0 && dnsHeader.nscount == 0 && dnsHeader.arcount == 0)) { + dnsheader_aligned dnsHeader(value.value.data()); + if (dnsHeader->rcode != RCode::NoError || (dnsHeader->ancount == 0 && dnsHeader->nscount == 0 && dnsHeader->arcount == 0)) { continue; } diff --git a/pdns/dnsdistdist/dnsdist-cache.hh b/pdns/dnsdistdist/dnsdist-cache.hh index 41b342aaad..5a068c1035 100644 --- a/pdns/dnsdistdist/dnsdist-cache.hh +++ b/pdns/dnsdistdist/dnsdist-cache.hh @@ -44,6 +44,7 @@ public: uint32_t d_minTTL{0}; uint32_t d_tempFailureTTL{60}; uint32_t d_maxNegativeTTL{3600}; + uint32_t d_truncatedTTL{0}; uint32_t d_staleTTL{60}; uint32_t d_shardCount{1}; bool d_dontAge{false}; diff --git a/pdns/dnsdistdist/dnsdist-lua-bindings-packetcache.cc b/pdns/dnsdistdist/dnsdist-lua-bindings-packetcache.cc index 6f47577d9a..b379e5080e 100644 --- a/pdns/dnsdistdist/dnsdist-lua-bindings-packetcache.cc +++ b/pdns/dnsdistdist/dnsdist-lua-bindings-packetcache.cc @@ -51,6 +51,7 @@ void setupLuaBindingsPacketCache(LuaContext& luaCtx, bool client) getOptionalValue(vars, "parseECS", settings.d_parseECS); getOptionalValue(vars, "staleTTL", settings.d_staleTTL); getOptionalValue(vars, "temporaryFailureTTL", settings.d_tempFailureTTL); + getOptionalValue(vars, "truncatedTTL", settings.d_truncatedTTL); getOptionalValue(vars, "cookieHashing", cookieHashing); getOptionalValue(vars, "maximumEntrySize", maximumEntrySize); diff --git a/pdns/dnsdistdist/dnsdist-rust-lib/rust/src/lib.rs b/pdns/dnsdistdist/dnsdist-rust-lib/rust/src/lib.rs index 9452e1fcdc..121b595808 100644 --- a/pdns/dnsdistdist/dnsdist-rust-lib/rust/src/lib.rs +++ b/pdns/dnsdistdist/dnsdist-rust-lib/rust/src/lib.rs @@ -2033,6 +2033,8 @@ mod dnsdistsettings { #[serde(default = "crate::U32::<60>::value", skip_serializing_if = "crate::U32::<60>::is_equal")] temporary_failure_ttl: u32, #[serde(default, skip_serializing_if = "crate::is_default")] + truncated_ttl: u32, + #[serde(default, skip_serializing_if = "crate::is_default")] cookie_hashing: bool, #[serde(default = "crate::U32::<4096>::value", skip_serializing_if = "crate::U32::<4096>::is_equal")] maximum_entry_size: u32, diff --git a/pdns/dnsdistdist/dnsdist-settings-definitions.yml b/pdns/dnsdistdist/dnsdist-settings-definitions.yml index 018ad8580c..c01f86881a 100644 --- a/pdns/dnsdistdist/dnsdist-settings-definitions.yml +++ b/pdns/dnsdistdist/dnsdist-settings-definitions.yml @@ -1834,6 +1834,10 @@ packet_cache: type: "u32" default: "60" description: "On a SERVFAIL or REFUSED from the backend, cache for this amount of seconds" + - name: "truncated_ttl" + type: "u32" + default: 0 + description: "On a truncated (TC=1, no records) response from the backend, cache for this amount of seconds. 0, the default, means that truncated answers are not cached" - name: "cookie_hashing" type: "bool" default: "false" diff --git a/pdns/dnsdistdist/docs/reference/config.rst b/pdns/dnsdistdist/docs/reference/config.rst index 39d36bbecb..c49f30b62d 100644 --- a/pdns/dnsdistdist/docs/reference/config.rst +++ b/pdns/dnsdistdist/docs/reference/config.rst @@ -996,6 +996,9 @@ See :doc:`../guides/cache` for a how to. .. versionchanged:: 1.9.0 ``maximumEntrySize`` parameter added. + .. versionchanged:: 2.0.0 + ``truncatedTTL`` parameter added. + Creates a new :class:`PacketCache` with the settings specified. :param int maxEntries: The maximum number of entries in this cache @@ -1011,7 +1014,8 @@ See :doc:`../guides/cache` for a how to. * ``numberOfShards=20``: int - Number of shards to divide the cache into, to reduce lock contention. Used to be 1 (no shards) before 1.6.0, and is now 20. * ``parseECS=false``: bool - Whether any EDNS Client Subnet option present in the query should be extracted and stored to be able to detect hash collisions involving queries with the same qname, qtype and qclass but a different incoming ECS value. Enabling this option adds a parsing cost and only makes sense if at least one backend might send different responses based on the ECS value, so it's disabled by default. Enabling this option is required for the :doc:`../advanced/zero-scope` option to work * ``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.. + * ``temporaryFailureTTL=60``: int - On a SERVFAIL or REFUSED from the backend, cache for this amount of seconds. + * ``truncatedTTL=0``: int - On a truncated (TC=1, no records) response from the backend, cache for this amount of seconds. 0, the default, means that truncated answers are not cached. * ``cookieHashing=false``: bool - If true, 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 (if ``cookieHashing`` above is false, EDNS cookie option number will be added to this list internally). * ``maximumEntrySize=4096``: int - The maximum size, in bytes, of a DNS packet that can be inserted into the packet cache. Default is 4096 bytes, which was the fixed size before 1.9.0, and is also a hard limit for UDP responses. diff --git a/pdns/dnsdistdist/docs/reference/yaml-settings.rst b/pdns/dnsdistdist/docs/reference/yaml-settings.rst index 5ac476f506..bcdf8b549f 100644 --- a/pdns/dnsdistdist/docs/reference/yaml-settings.rst +++ b/pdns/dnsdistdist/docs/reference/yaml-settings.rst @@ -738,6 +738,7 @@ Packet-cache settings - **parse_ecs**: Boolean ``(false)`` - Whether any EDNS Client Subnet option present in the query should be extracted and stored to be able to detect hash collisions involving queries with the same qname, qtype and qclass but a different incoming ECS value. Enabling this option adds a parsing cost and only makes sense if at least one backend might send different responses based on the ECS value, so it's disabled by default. Enabling this option is required for the :doc:`../advanced/zero-scope` option to work - **stale_ttl**: Unsigned integer ``(60)`` - 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 - **temporary_failure_ttl**: Unsigned integer ``(60)`` - On a SERVFAIL or REFUSED from the backend, cache for this amount of seconds +- **truncated_ttl**: Unsigned integer ``(0)`` - On a truncated (TC=1, no records) response from the backend, cache for this amount of seconds. 0, the default, means that truncated answers are not cached - **cookie_hashing**: Boolean ``(false)`` - If true, 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 - **maximum_entry_size**: Unsigned integer ``(4096)`` - The maximum size, in bytes, of a DNS packet that can be inserted into the packet cache - **options_to_skip**: Sequence of String ``("")`` - Extra list of EDNS option codes to skip when hashing the packet (if ``cookie_hashing`` above is false, EDNS cookie option number will be added to this list internally) diff --git a/pdns/dnsdistdist/test-dnsdistpacketcache_cc.cc b/pdns/dnsdistdist/test-dnsdistpacketcache_cc.cc index 73754fc506..d275f8b4bb 100644 --- a/pdns/dnsdistdist/test-dnsdistpacketcache_cc.cc +++ b/pdns/dnsdistdist/test-dnsdistpacketcache_cc.cc @@ -495,62 +495,82 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheNXDomainTTL) BOOST_AUTO_TEST_CASE(test_PacketCacheTruncated) { - const DNSDistPacketCache::CacheSettings settings{ - .d_maxEntries = 150000, - .d_maxTTL = 86400, - .d_minTTL = 1, - .d_tempFailureTTL = 60, - .d_maxNegativeTTL = 1, - }; - DNSDistPacketCache localCache(settings); - InternalQueryState ids; ids.qtype = QType::A; ids.qclass = QClass::IN; ids.protocol = dnsdist::Protocol::DoUDP; ids.queryRealTime.start(); // does not have to be accurate ("realTime") in tests + ids.qname = DNSName("truncated"); bool dnssecOK = false; + PacketBuffer query; + GenericDNSPacketWriter pwQ(query, ids.qname, QType::A, QClass::IN, 0); + pwQ.getHeader()->rd = 1; - try { - ids.qname = DNSName("truncated"); - PacketBuffer query; - GenericDNSPacketWriter pwQ(query, ids.qname, QType::A, QClass::IN, 0); - pwQ.getHeader()->rd = 1; + PacketBuffer response; + GenericDNSPacketWriter pwR(response, ids.qname, QType::A, QClass::IN, 0); + pwR.getHeader()->rd = 1; + pwR.getHeader()->ra = 0; + pwR.getHeader()->qr = 1; + pwR.getHeader()->tc = 1; + pwR.getHeader()->rcode = RCode::NoError; + pwR.getHeader()->id = pwQ.getHeader()->id; + pwR.commit(); - PacketBuffer response; - GenericDNSPacketWriter pwR(response, ids.qname, QType::A, QClass::IN, 0); - pwR.getHeader()->rd = 1; - pwR.getHeader()->ra = 0; - pwR.getHeader()->qr = 1; - pwR.getHeader()->tc = 1; - pwR.getHeader()->rcode = RCode::NoError; - pwR.getHeader()->id = pwQ.getHeader()->id; - pwR.commit(); - pwR.startRecord(ids.qname, QType::A, 7200, QClass::IN, DNSResourceRecord::ANSWER); - pwR.xfr32BitInt(0x01020304); - pwR.commit(); + uint32_t key = 0; + boost::optional subnet; + DNSQuestion dnsQuestion(ids, query); + bool allowTruncated = true; - uint32_t key = 0; - boost::optional subnet; - DNSQuestion dnsQuestion(ids, query); - bool found = localCache.get(dnsQuestion, 0, &key, subnet, dnssecOK, receivedOverUDP); - BOOST_CHECK_EQUAL(found, false); + { + /* truncated answers are not cached by default */ + const DNSDistPacketCache::CacheSettings settings{ + .d_maxEntries = 150000, + .d_maxTTL = 86400, + .d_minTTL = 1, + .d_tempFailureTTL = 60, + .d_maxNegativeTTL = 1, + }; + DNSDistPacketCache localCache(settings); + BOOST_CHECK_EQUAL(localCache.getSize(), 0U); + + bool found = localCache.get(dnsQuestion, 0, &key, subnet, dnssecOK, receivedOverUDP, 0, false, allowTruncated); + BOOST_REQUIRE_EQUAL(found, false); BOOST_CHECK(!subnet); - localCache.insert(key, subnet, *(getFlagsFromDNSHeader(dnsQuestion.getHeader().get())), dnssecOK, ids.qname, QType::A, QClass::IN, response, receivedOverUDP, RCode::NXDomain, boost::none); + localCache.insert(key, subnet, *(getFlagsFromDNSHeader(dnsQuestion.getHeader().get())), dnssecOK, ids.qname, QType::A, QClass::IN, response, receivedOverUDP, 0, boost::none); - bool allowTruncated = true; - found = localCache.get(dnsQuestion, pwR.getHeader()->id, &key, subnet, dnssecOK, receivedOverUDP, 0, true, allowTruncated); - BOOST_CHECK_EQUAL(found, true); + found = localCache.get(dnsQuestion, pwR.getHeader()->id, &key, subnet, dnssecOK, receivedOverUDP, 0, false, allowTruncated); + BOOST_REQUIRE_EQUAL(found, false); + } + + { + /* enable caching of truncated answers */ + const DNSDistPacketCache::CacheSettings settings{ + .d_maxEntries = 150000, + .d_maxTTL = 86400, + .d_minTTL = 1, + .d_truncatedTTL = 60, + }; + DNSDistPacketCache localCache(settings); + BOOST_CHECK_EQUAL(localCache.getSize(), 0U); + + bool found = localCache.get(dnsQuestion, 0, &key, subnet, dnssecOK, receivedOverUDP, 0, false, allowTruncated); + BOOST_REQUIRE_EQUAL(found, false); BOOST_CHECK(!subnet); + localCache.insert(key, subnet, *(getFlagsFromDNSHeader(dnsQuestion.getHeader().get())), dnssecOK, ids.qname, QType::A, QClass::IN, response, receivedOverUDP, 0, boost::none); + allowTruncated = false; - found = localCache.get(dnsQuestion, pwR.getHeader()->id, &key, subnet, dnssecOK, receivedOverUDP, 0, true, allowTruncated); - BOOST_CHECK_EQUAL(found, false); - } - catch (const PDNSException& e) { - cerr << "Had error: " << e.reason << endl; - throw; + found = localCache.get(dnsQuestion, pwR.getHeader()->id, &key, subnet, dnssecOK, receivedOverUDP, 0, false, allowTruncated); + BOOST_REQUIRE_EQUAL(found, false); + + allowTruncated = true; + found = localCache.get(dnsQuestion, pwR.getHeader()->id, &key, subnet, dnssecOK, receivedOverUDP, 0, false, allowTruncated); + BOOST_REQUIRE_EQUAL(found, true); + BOOST_REQUIRE_EQUAL(dnsQuestion.getData().size(), response.size()); + int match = memcmp(dnsQuestion.getData().data(), response.data(), dnsQuestion.getData().size()); + BOOST_CHECK_EQUAL(match, 0); + BOOST_CHECK(!subnet); } } diff --git a/regression-tests.dnsdist/test_Caching.py b/regression-tests.dnsdist/test_Caching.py index 0f0f1195b4..89ea4f833d 100644 --- a/regression-tests.dnsdist/test_Caching.py +++ b/regression-tests.dnsdist/test_Caching.py @@ -80,6 +80,23 @@ class TestCaching(DNSDistTest): self.assertEqual(total, 1) + def testEmptyTruncated(self): + """ + Cache: Empty TC=1 is not cached by default + """ + name = 'empty-tc.cache.tests.powerdns.com.' + query = dns.message.make_query(name, 'AAAA', 'IN') + response = dns.message.make_response(query) + response.flags |= dns.flags.TC + + for _ in range(2): + (receivedQuery, receivedResponse) = self.sendUDPQuery(query, response) + self.assertTrue(receivedQuery) + self.assertTrue(receivedResponse) + receivedQuery.id = query.id + self.assertEqual(query, receivedQuery) + self.assertEqual(receivedResponse, response) + def testDOCached(self): """ Cache: Served from cache, query has DO bit set @@ -3020,3 +3037,38 @@ class TestCachingOfVeryLargeAnswers(DNSDistTest): self.assertFalse(receivedResponse) receivedQuery.id = query.id self.assertEqual(query, receivedQuery) + +class TestCacheEmptyTC(DNSDistTest): + + _truncated_ttl = 42 + _config_template = """ + pc = newPacketCache(100, {maxTTL=86400, minTTL=1, truncatedTTL=%d}) + getPool(""):setCache(pc) + newServer{address="127.0.0.1:%d"} + """ + _config_params = ['_truncated_ttl', '_testServerPort'] + + def testEmptyTruncated(self): + """ + Cache: Empty TC=1 should be cached + """ + name = 'cache-empty-tc.cache.tests.powerdns.com.' + query = dns.message.make_query(name, 'AAAA', 'IN') + response = dns.message.make_response(query) + response.flags |= dns.flags.TC + + # first to fill the cache + for method in ("sendUDPQuery", "sendTCPQuery"): + sender = getattr(self, method) + (receivedQuery, receivedResponse) = sender(query, response) + self.assertTrue(receivedQuery) + self.assertTrue(receivedResponse) + receivedQuery.id = query.id + self.assertEqual(query, receivedQuery) + self.assertEqual(receivedResponse, response) + + # now it should be cached + for method in ("sendUDPQuery", "sendTCPQuery"): + sender = getattr(self, method) + (_, receivedResponse) = sender(query, response=None, useQueue=False) + self.assertEqual(receivedResponse, response)