From: Remi Gacogne Date: Mon, 5 Sep 2022 14:08:36 +0000 (+0200) Subject: dnsdist: Faster cache-lookups for DNS over HTTPS queries X-Git-Tag: dnsdist-1.8.0-rc1~286^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0c8759d8ad20baa60f784c4487f79cdf03102603;p=thirdparty%2Fpdns.git dnsdist: Faster cache-lookups for DNS over HTTPS queries For DoH queries we do two cache lookups: - a first one for responses received over UDP, that can be truncated, which we do not want here as the client likely would not what to do with that - then on a miss, a second one for responses received over TCP We do not want the first lookup to override the initial query in case of a hit for a TC=1 response, so we used to do a copy of the query (allocation + copy). This code move the TC=1 check to the cache itself so we do not have to do that copy, speeding things up. --- diff --git a/pdns/dnsdist-cache.cc b/pdns/dnsdist-cache.cc index 7222a29162..ec936be2b1 100644 --- a/pdns/dnsdist-cache.cc +++ b/pdns/dnsdist-cache.cc @@ -192,7 +192,7 @@ void DNSDistPacketCache::insert(uint32_t key, const boost::optional& su } } -bool DNSDistPacketCache::get(DNSQuestion& dq, uint16_t queryId, uint32_t* keyOut, boost::optional& subnet, bool dnssecOK, bool receivedOverUDP, uint32_t allowExpired, bool skipAging) +bool DNSDistPacketCache::get(DNSQuestion& dq, uint16_t queryId, uint32_t* keyOut, boost::optional& subnet, bool dnssecOK, bool receivedOverUDP, uint32_t allowExpired, bool skipAging, bool truncatedOK) { const auto& dnsQName = dq.qname->getStorage(); uint32_t key = getKey(dnsQName, dq.qname->wirelength(), dq.getData(), receivedOverUDP); @@ -245,6 +245,14 @@ bool DNSDistPacketCache::get(DNSQuestion& dq, uint16_t queryId, uint32_t* keyOut return false; } + if (!truncatedOK) { + dnsheader dh; + memcpy(&dh, value.value.data(), sizeof(dh)); + if (dh.tc != 0) { + return false; + } + } + response.resize(value.len); memcpy(&response.at(0), &queryId, sizeof(queryId)); memcpy(&response.at(sizeof(queryId)), &value.value.at(sizeof(queryId)), sizeof(dnsheader) - sizeof(queryId)); diff --git a/pdns/dnsdist-cache.hh b/pdns/dnsdist-cache.hh index 23b321375c..d7fcca8e52 100644 --- a/pdns/dnsdist-cache.hh +++ b/pdns/dnsdist-cache.hh @@ -38,7 +38,7 @@ public: DNSDistPacketCache(size_t maxEntries, uint32_t maxTTL=86400, uint32_t minTTL=0, uint32_t tempFailureTTL=60, uint32_t maxNegativeTTL=3600, uint32_t staleTTL=60, bool dontAge=false, uint32_t shards=1, bool deferrableInsertLock=true, bool parseECS=false); void insert(uint32_t key, const boost::optional& subnet, uint16_t queryFlags, bool dnssecOK, const DNSName& qname, uint16_t qtype, uint16_t qclass, const PacketBuffer& response, bool receivedOverUDP, uint8_t rcode, boost::optional tempFailureTTL); - bool get(DNSQuestion& dq, uint16_t queryId, uint32_t* keyOut, boost::optional& subnet, bool dnssecOK, bool receivedOverUDP, uint32_t allowExpired = 0, bool skipAging = false); + bool get(DNSQuestion& dq, uint16_t queryId, uint32_t* keyOut, boost::optional& subnet, bool dnssecOK, bool receivedOverUDP, uint32_t allowExpired = 0, bool skipAging = false, bool truncatedOK = true); size_t purgeExpired(size_t upTo, const time_t now); size_t expunge(size_t upTo=0); size_t expungeByName(const DNSName& name, uint16_t qtype=QType::ANY, bool suffixMatch=false); diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc index dcbfc9e1c6..82a9073200 100644 --- a/pdns/dnsdist.cc +++ b/pdns/dnsdist.cc @@ -1327,18 +1327,14 @@ ProcessQueryResult processQuery(DNSQuestion& dq, ClientState& cs, LocalHolders& return ProcessQueryResult::SendAnswer; } else if (dq.protocol == dnsdist::Protocol::DoH) { - /* do a second-lookup for UDP responses */ - /* we need to do a copy to be able to restore the query on a TC=1 cached answer */ + /* do a second-lookup for UDP responses, but we do not want TC=1 answers */ PacketBuffer initialQuery(dq.getData()); - if (dq.packetCache->get(dq, dq.getHeader()->id, &dq.cacheKeyUDP, dq.subnet, dq.dnssecOK, true, allowExpired)) { - if (dq.getHeader()->tc == 0) { - if (!prepareOutgoingResponse(holders, cs, dq, true)) { - return ProcessQueryResult::Drop; - } - - return ProcessQueryResult::SendAnswer; + if (dq.packetCache->get(dq, dq.getHeader()->id, &dq.cacheKeyUDP, dq.subnet, dq.dnssecOK, true, allowExpired, false)) { + if (!prepareOutgoingResponse(holders, cs, dq, true)) { + return ProcessQueryResult::Drop; } - dq.getMutableData() = std::move(initialQuery); + + return ProcessQueryResult::SendAnswer; } } diff --git a/pdns/test-dnsdistpacketcache_cc.cc b/pdns/test-dnsdistpacketcache_cc.cc index 4be01c2fcf..55a757cac1 100644 --- a/pdns/test-dnsdistpacketcache_cc.cc +++ b/pdns/test-dnsdistpacketcache_cc.cc @@ -156,7 +156,6 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheSharded) { pwR.startRecord(a, QType::AAAA, 7200, QClass::IN, DNSResourceRecord::ANSWER); ComboAddress v6("2001:db8::1"); pwR.xfrIP6(std::string(reinterpret_cast(v6.sin6.sin6_addr.s6_addr), 16)); - pwR.xfr32BitInt(0x01020304); pwR.commit(); uint32_t key = 0; @@ -250,7 +249,6 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheTCP) { pwR.startRecord(a, QType::AAAA, 7200, QClass::IN, DNSResourceRecord::ANSWER); ComboAddress v6("2001:db8::1"); pwR.xfrIP6(std::string(reinterpret_cast(v6.sin6.sin6_addr.s6_addr), 16)); - pwR.xfr32BitInt(0x01020304); pwR.commit(); { @@ -443,6 +441,59 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheNXDomainTTL) { } } +BOOST_AUTO_TEST_CASE(test_PacketCacheTruncated) { + const size_t maxEntries = 150000; + DNSDistPacketCache PC(maxEntries, /* maxTTL */ 86400, /* minTTL */ 1, /* tempFailureTTL */ 60, /* maxNegativeTTL */ 1); + + struct timespec queryTime; + gettime(&queryTime); // does not have to be accurate ("realTime") in tests + + ComboAddress remote; + bool dnssecOK = false; + + try { + DNSName name("truncated"); + PacketBuffer query; + GenericDNSPacketWriter pwQ(query, name, QType::A, QClass::IN, 0); + pwQ.getHeader()->rd = 1; + + PacketBuffer response; + GenericDNSPacketWriter pwR(response, name, 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(name, QType::A, 7200, QClass::IN, DNSResourceRecord::ANSWER); + pwR.xfr32BitInt(0x01020304); + pwR.commit(); + + uint32_t key = 0; + boost::optional subnet; + DNSQuestion dq(&name, QType::A, QClass::IN, &remote, &remote, query, dnsdist::Protocol::DoUDP, &queryTime); + bool found = PC.get(dq, 0, &key, subnet, dnssecOK, receivedOverUDP); + BOOST_CHECK_EQUAL(found, false); + BOOST_CHECK(!subnet); + + PC.insert(key, subnet, *(getFlagsFromDNSHeader(dq.getHeader())), dnssecOK, name, QType::A, QClass::IN, response, receivedOverUDP, RCode::NXDomain, boost::none); + + bool allowTruncated = true; + found = PC.get(dq, pwR.getHeader()->id, &key, subnet, dnssecOK, receivedOverUDP, 0, true, allowTruncated); + BOOST_CHECK_EQUAL(found, true); + BOOST_CHECK(!subnet); + + allowTruncated = false; + found = PC.get(dq, pwR.getHeader()->id, &key, subnet, dnssecOK, receivedOverUDP, 0, true, allowTruncated); + BOOST_CHECK_EQUAL(found, false); +} + catch(const PDNSException& e) { + cerr<<"Had error: "<