dnsdist: Use the correct cache key for DoH UDP responses

diff --git a/pdns/dnsdist-idstate.hh b/pdns/dnsdist-idstate.hh
index 69d882b74657a5f59d961cd0109f8ea7f3a7accb..580db0d9d2ce12f5961b4bd11cb83a0d57d0a4d5 100644 (file)
--- a/pdns/dnsdist-idstate.hh
+++ b/pdns/dnsdist-idstate.hh
@@ -89,7 +89,7 @@ struct IDState
 {
   IDState(): sentTime(true), tempFailureTTL(boost::none) { origDest.sin4.sin_family = 0;}
   IDState(const IDState& orig) = delete;
-  IDState(IDState&& rhs): subnet(rhs.subnet), origRemote(rhs.origRemote), origDest(rhs.origDest), hopRemote(rhs.hopRemote), hopLocal(rhs.hopLocal), qname(std::move(rhs.qname)), sentTime(rhs.sentTime), dnsCryptQuery(std::move(rhs.dnsCryptQuery)), packetCache(std::move(rhs.packetCache)), qTag(std::move(rhs.qTag)), tempFailureTTL(rhs.tempFailureTTL), cs(rhs.cs), du(std::move(rhs.du)), cacheKey(rhs.cacheKey), cacheKeyNoECS(rhs.cacheKeyNoECS), origFD(rhs.origFD), delayMsec(rhs.delayMsec), qtype(rhs.qtype), qclass(rhs.qclass), origID(rhs.origID), origFlags(rhs.origFlags), cacheFlags(rhs.cacheFlags), protocol(rhs.protocol), ednsAdded(rhs.ednsAdded), ecsAdded(rhs.ecsAdded), skipCache(rhs.skipCache), destHarvested(rhs.destHarvested), dnssecOK(rhs.dnssecOK), useZeroScope(rhs.useZeroScope)
+  IDState(IDState&& rhs): subnet(rhs.subnet), origRemote(rhs.origRemote), origDest(rhs.origDest), hopRemote(rhs.hopRemote), hopLocal(rhs.hopLocal), qname(std::move(rhs.qname)), sentTime(rhs.sentTime), dnsCryptQuery(std::move(rhs.dnsCryptQuery)), packetCache(std::move(rhs.packetCache)), qTag(std::move(rhs.qTag)), tempFailureTTL(rhs.tempFailureTTL), cs(rhs.cs), du(std::move(rhs.du)), cacheKey(rhs.cacheKey), cacheKeyNoECS(rhs.cacheKeyNoECS), cacheKeyUDP(rhs.cacheKeyUDP), origFD(rhs.origFD), delayMsec(rhs.delayMsec), qtype(rhs.qtype), qclass(rhs.qclass), origID(rhs.origID), origFlags(rhs.origFlags), cacheFlags(rhs.cacheFlags), protocol(rhs.protocol), ednsAdded(rhs.ednsAdded), ecsAdded(rhs.ecsAdded), skipCache(rhs.skipCache), destHarvested(rhs.destHarvested), dnssecOK(rhs.dnssecOK), useZeroScope(rhs.useZeroScope)
   {
     if (rhs.isInUse()) {
       throw std::runtime_error("Trying to move an in-use IDState");
@@ -128,6 +128,7 @@ struct IDState
     du = std::move(rhs.du);
     cacheKey = rhs.cacheKey;
     cacheKeyNoECS = rhs.cacheKeyNoECS;
+    cacheKeyUDP = rhs.cacheKeyUDP;
     origFD = rhs.origFD;
     delayMsec = rhs.delayMsec;
 #ifdef __SANITIZE_THREAD__
@@ -235,6 +236,8 @@ struct IDState
   std::atomic<uint32_t> generation{0}; // increased every time a state is used, to be able to detect an ABA issue    // 4
   uint32_t cacheKey{0};                                       // 4
   uint32_t cacheKeyNoECS{0};                                  // 4
+  // DoH-only */
+  uint32_t cacheKeyUDP{0};                                    // 4
   int origFD{-1};                                             // 4
   int delayMsec{0};
 #ifdef __SANITIZE_THREAD__

diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc
index 8e63525deb9ed248a382628a36e92bb141dca0fb..931d239b47c448267870349c017a9a1572d81513 100644 (file)
--- a/pdns/dnsdist.cc
+++ b/pdns/dnsdist.cc
@@ -463,8 +463,16 @@ bool processResponse(PacketBuffer& response, LocalStateHolder<vector<DNSDistResp
       */
       zeroScope = false;
     }
-    // if zeroScope, pass the pre-ECS hash-key and do not pass the subnet to the cache
-    dr.packetCache->insert(zeroScope ? dr.cacheKeyNoECS : dr.cacheKey, zeroScope ? boost::none : dr.subnet, dr.cacheFlags, dr.dnssecOK, *dr.qname, dr.qtype, dr.qclass, response, receivedOverUDP, dr.getHeader()->rcode, dr.tempFailureTTL);
+    uint32_t cacheKey = dr.cacheKey;
+    if (dr.protocol == dnsdist::Protocol::DoH && receivedOverUDP) {
+      cacheKey = dr.cacheKeyUDP;
+    }
+    else if (zeroScope) {
+      // if zeroScope, pass the pre-ECS hash-key and do not pass the subnet to the cache
+      cacheKey = dr.cacheKeyNoECS;
+    }
+
+    dr.packetCache->insert(cacheKey, zeroScope ? boost::none : dr.subnet, dr.cacheFlags, dr.dnssecOK, *dr.qname, dr.qtype, dr.qclass, response, receivedOverUDP, dr.getHeader()->rcode, dr.tempFailureTTL);
   }
 
 #ifdef HAVE_DNSCRYPT
@@ -1232,10 +1240,9 @@ ProcessQueryResult processQuery(DNSQuestion& dq, ClientState& cs, LocalHolders&
       }
       else if (dq.protocol == dnsdist::Protocol::DoH) {
         /* do a second-lookup for UDP responses */
-        uint32_t udpCacheKey = 0;
         /* we need to do a copy to be able to restore the query on a TC=1 cached answer */
         PacketBuffer initialQuery(dq.getData());
-        if (dq.packetCache->get(dq, dq.getHeader()->id, &udpCacheKey, dq.subnet, dq.dnssecOK, true, allowExpired)) {
+        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;

diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh
index b34e1d9f839cedf498a6995f0412063db1714892..8a9565c0acd82cd819d8539c4b13aad6b2ce0d91 100644 (file)
--- a/pdns/dnsdist.hh
+++ b/pdns/dnsdist.hh
@@ -149,6 +149,8 @@ public:
   boost::optional<uint32_t> tempFailureTTL;
   uint32_t cacheKeyNoECS{0};
   uint32_t cacheKey{0};
+  /* for DoH */
+  uint32_t cacheKeyUDP{0};
   const uint16_t qtype;
   const uint16_t qclass;
   uint16_t ecsPrefixLength;

diff --git a/pdns/dnsdistdist/dnsdist-idstate.cc b/pdns/dnsdistdist/dnsdist-idstate.cc
index f34e79de587293badb7877903d0b690362188605..0f3eeb247bfebb6e0d2262887eeb699d33c3f3b1 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-idstate.cc
+++ b/pdns/dnsdistdist/dnsdist-idstate.cc
@@ -14,6 +14,7 @@ DNSResponse makeDNSResponseFromIDState(IDState& ids, PacketBuffer& data)
   dr.skipCache = ids.skipCache;
   dr.cacheKey = ids.cacheKey;
   dr.cacheKeyNoECS = ids.cacheKeyNoECS;
+  dr.cacheKeyUDP = ids.cacheKeyUDP;
   dr.dnssecOK = ids.dnssecOK;
   dr.tempFailureTTL = ids.tempFailureTTL;
   dr.qTag = std::move(ids.qTag);
@@ -45,6 +46,7 @@ void setIDStateFromDNSQuestion(IDState& ids, DNSQuestion& dq, DNSName&& qname)
   ids.cacheFlags = dq.cacheFlags;
   ids.cacheKey = dq.cacheKey;
   ids.cacheKeyNoECS = dq.cacheKeyNoECS;
+  ids.cacheKeyUDP = dq.cacheKeyUDP;
   ids.subnet = dq.subnet;
   ids.skipCache = dq.skipCache;
   ids.packetCache = dq.packetCache;