From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Tue, 10 Jan 2023 13:27:27 +0000 (+0100)
Subject: rec: do not chain ecs enabled queries.
X-Git-Tag: dnsdist-1.8.0-rc1~112^2~3
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=5b90e5daf9d40aa8be77119c3ad01179267abc3f;p=thirdparty%2Fpdns.git

rec: do not chain ecs enabled queries.

asyncresolve() assumes the recieved ecs info corresponds to the one sent out.
---

diff --git a/pdns/recursordist/lwres.cc b/pdns/recursordist/lwres.cc
index 7cf9d7972d..40e44fc659 100644
--- a/pdns/recursordist/lwres.cc
+++ b/pdns/recursordist/lwres.cc
@@ -422,7 +422,6 @@ static LWResult::Result asyncresolve(const ComboAddress& ip, const DNSName& doma
       eo.source = *srcmask;
       outgoingECSBits = srcmask->getBits();
       outgoingECSAddr = srcmask->getNetwork();
-      //      cout<<"Adding request mask: "<<eo.source.toString()<<endl;
       opts.emplace_back(EDNSOptionCode::ECS, makeEDNSSubnetOptsString(eo));
       weWantEDNSSubnet = true;
     }
@@ -472,7 +471,7 @@ static LWResult::Result asyncresolve(const ComboAddress& ip, const DNSName& doma
       t_Counters.at(rec::Counter::ipv6queries)++;
     }
 
-    ret = asendto((const char*)&*vpacket.begin(), vpacket.size(), 0, ip, qid, domain, type, &queryfd);
+    ret = asendto((const char*)&*vpacket.begin(), vpacket.size(), 0, ip, qid, domain, type, weWantEDNSSubnet, &queryfd);
 
     if (ret != LWResult::Result::Success) {
       return ret;
@@ -595,7 +594,6 @@ static LWResult::Result asyncresolve(const ComboAddress& ip, const DNSName& doma
           if (opt.first == EDNSOptionCode::ECS) {
             EDNSSubnetOpts reso;
             if (getEDNSSubnetOptsFromString(opt.second, &reso)) {
-              //	    cerr<<"EDNS Subnet response: "<<reso.source.toString()<<", scope: "<<reso.scope.toString()<<", family = "<<reso.scope.getNetwork().sin4.sin_family<<endl;
               /* rfc7871 states that 0 "indicate[s] that the answer is suitable for all addresses in FAMILY",
                  so we might want to still pass the information along to be able to differentiate between
                  IPv4 and IPv6. Still I'm pretty sure it doesn't matter in real life, so let's not duplicate
diff --git a/pdns/recursordist/lwres.hh b/pdns/recursordist/lwres.hh
index b34f3790b9..1f4e22eb41 100644
--- a/pdns/recursordist/lwres.hh
+++ b/pdns/recursordist/lwres.hh
@@ -84,7 +84,7 @@ public:
 };
 
 LWResult::Result asendto(const char* data, size_t len, int flags, const ComboAddress& ip, uint16_t id,
-                         const DNSName& domain, uint16_t qtype, int* fd);
+                         const DNSName& domain, uint16_t qtype, bool ecs, int* fd);
 LWResult::Result arecvfrom(PacketBuffer& packet, int flags, const ComboAddress& ip, size_t* d_len, uint16_t id,
                            const DNSName& domain, uint16_t qtype, int fd, const struct timeval* now);
 
diff --git a/pdns/recursordist/pdns_recursor.cc b/pdns/recursordist/pdns_recursor.cc
index 6e9c4f9ac1..9f73f08a01 100644
--- a/pdns/recursordist/pdns_recursor.cc
+++ b/pdns/recursordist/pdns_recursor.cc
@@ -259,7 +259,7 @@ thread_local std::unique_ptr<UDPClientSocks> t_udpclientsocks;
 
 /* these two functions are used by LWRes */
 LWResult::Result asendto(const char* data, size_t len, int flags,
-                         const ComboAddress& toaddr, uint16_t id, const DNSName& domain, uint16_t qtype, int* fd)
+                         const ComboAddress& toaddr, uint16_t id, const DNSName& domain, uint16_t qtype, bool ecs, int* fd)
 {
 
   auto pident = std::make_shared<PacketID>();
@@ -267,18 +267,24 @@ LWResult::Result asendto(const char* data, size_t len, int flags,
   pident->remote = toaddr;
   pident->type = qtype;
 
-  // see if there is an existing outstanding request we can chain on to, using partial equivalence function looking for the same
-  // query (qname and qtype) to the same host, but with a different message ID
-  pair<MT_t::waiters_t::iterator, MT_t::waiters_t::iterator> chain = MT->d_waiters.equal_range(pident, PacketIDBirthdayCompare());
-
-  for (; chain.first != chain.second; chain.first++) {
-    // Line below detected an issue with the two ways of ordering PackeIDs (birtday and non-birthday)
-    assert(chain.first->key->domain == pident->domain);
-    if (chain.first->key->fd > -1 && !chain.first->key->closed) { // don't chain onto existing chained waiter or a chain already processed
-      // cerr << "Insert " << id << ' ' << pident << " into chain for  " << chain.first->key << endl;
-      chain.first->key->chain.insert(id); // we can chain
-      *fd = -1; // gets used in waitEvent / sendEvent later on
-      return LWResult::Result::Success;
+  // We might want to put the ecs netmask into the PacketID key, but we take the easy way:
+  // Avoid processing a chain with queries having different ECS data by lwres:asyncresolve() as it
+  // assumes the mask received corresponds to the mask sent out, so do not chain ecs queries.
+  if (!ecs) {
+    // See if there is an existing outstanding request we can chain on to, using partial equivalence
+    // function looking for the same query (qname and qtype) to the same host, but with a different
+    // message ID.
+    auto chain = MT->d_waiters.equal_range(pident, PacketIDBirthdayCompare());
+
+    for (; chain.first != chain.second; chain.first++) {
+      // Line below detected an issue with the two ways of ordering PackeIDs (birtday and non-birthday)
+      assert(chain.first->key->domain == pident->domain);
+      // don't chain onto existing chained waiter or a chain already processed
+      if (chain.first->key->fd > -1 && !chain.first->key->closed) {
+        chain.first->key->chain.insert(id); // we can chain
+        *fd = -1;                           // gets used in waitEvent / sendEvent later on
+        return LWResult::Result::Success;
+      }
     }
   }