From: Remi Gacogne Date: Thu, 29 Apr 2021 11:27:55 +0000 (+0200) Subject: dnsdist: Use the correct cache key for DoH UDP responses X-Git-Tag: dnsdist-1.7.0-alpha1~45^2~33 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2e372f54e36db80d370b3c28e40d7e4e7a827383;p=thirdparty%2Fpdns.git dnsdist: Use the correct cache key for DoH UDP responses --- diff --git a/pdns/dnsdist-idstate.hh b/pdns/dnsdist-idstate.hh index 69d882b746..580db0d9d2 100644 --- 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 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 8e63525deb..931d239b47 100644 --- a/pdns/dnsdist.cc +++ b/pdns/dnsdist.cc @@ -463,8 +463,16 @@ bool processResponse(PacketBuffer& response, LocalStateHolderinsert(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 b34e1d9f83..8a9565c0ac 100644 --- a/pdns/dnsdist.hh +++ b/pdns/dnsdist.hh @@ -149,6 +149,8 @@ public: boost::optional 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 f34e79de58..0f3eeb247b 100644 --- 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;