]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
Parse ECS info if relevant and act on it if it mismatches
authorOtto Moerbeek <otto.moerbeek@open-xchange.com>
Wed, 23 Apr 2025 11:50:51 +0000 (13:50 +0200)
committerOtto Moerbeek <otto.moerbeek@open-xchange.com>
Mon, 14 Jul 2025 07:15:19 +0000 (09:15 +0200)
Moved slowParseEDNSOptions() from dnsdist specific code to common code

pdns/dnsdistdist/dnsdist-ecs.cc
pdns/ednsoptions.cc
pdns/ednsoptions.hh
pdns/recursordist/lwres.cc
pdns/recursordist/lwres.hh
pdns/recursordist/pdns_recursor.cc
pdns/recursordist/rec-tcounters.hh
pdns/recursordist/syncres.cc
pdns/recursordist/syncres.hh

index c8cc4f6585f6443a505a70a529f555a3d8666ae3..77ceafe28c4083d566cfc8695e622a04d9974974 100644 (file)
@@ -256,61 +256,6 @@ bool slowRewriteEDNSOptionInQueryWithRecords(const PacketBuffer& initialPacket,
   return true;
 }
 
-static bool slowParseEDNSOptions(const PacketBuffer& packet, EDNSOptionViewMap& options)
-{
-  if (packet.size() < sizeof(dnsheader)) {
-    return false;
-  }
-
-  const dnsheader_aligned dnsHeader(packet.data());
-
-  if (ntohs(dnsHeader->qdcount) == 0) {
-    return false;
-  }
-
-  if (ntohs(dnsHeader->arcount) == 0) {
-    throw std::runtime_error("slowParseEDNSOptions() should not be called for queries that have no EDNS");
-  }
-
-  try {
-    uint64_t numrecords = ntohs(dnsHeader->ancount) + ntohs(dnsHeader->nscount) + ntohs(dnsHeader->arcount);
-    // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast,cppcoreguidelines-pro-type-const-cast)
-    DNSPacketMangler dpm(const_cast<char*>(reinterpret_cast<const char*>(packet.data())), packet.size());
-    uint64_t index{};
-    for (index = 0; index < ntohs(dnsHeader->qdcount); ++index) {
-      dpm.skipDomainName();
-      /* type and class */
-      dpm.skipBytes(4);
-    }
-
-    for (index = 0; index < numrecords; ++index) {
-      dpm.skipDomainName();
-
-      uint8_t section = index < ntohs(dnsHeader->ancount) ? 1 : (index < (ntohs(dnsHeader->ancount) + ntohs(dnsHeader->nscount)) ? 2 : 3);
-      uint16_t dnstype = dpm.get16BitInt();
-      dpm.get16BitInt();
-      dpm.skipBytes(4); /* TTL */
-
-      if (section == 3 && dnstype == QType::OPT) {
-        uint32_t offset = dpm.getOffset();
-        if (offset >= packet.size()) {
-          return false;
-        }
-        /* if we survive this call, we can parse it safely */
-        dpm.skipRData();
-        // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
-        return getEDNSOptions(reinterpret_cast<const char*>(&packet.at(offset)), packet.size() - offset, options) == 0;
-      }
-      dpm.skipRData();
-    }
-  }
-  catch (...) {
-    return false;
-  }
-
-  return true;
-}
-
 int locateEDNSOptRR(const PacketBuffer& packet, uint16_t* optStart, size_t* optLen, bool* last)
 {
   if (optStart == nullptr || optLen == nullptr || last == nullptr) {
index 93c127e554c5992f11647097266d22d0e6a43963..2d0377b8a7b3c945a3edd9f5a6183cfdbdb974f3 100644 (file)
@@ -22,6 +22,7 @@
 #include "dns.hh"
 #include "ednsoptions.hh"
 #include "iputils.hh"
+#include "dnsparser.hh"
 
 bool getNextEDNSOption(const char* data, size_t dataLen, uint16_t& optionCode, uint16_t& optionLen)
 {
@@ -93,6 +94,61 @@ int getEDNSOption(const char* optRR, const size_t len, uint16_t wantedOption, si
   return ENOENT;
 }
 
+bool slowParseEDNSOptions(const PacketBuffer& packet, EDNSOptionViewMap& options)
+{
+  if (packet.size() < sizeof(dnsheader)) {
+    return false;
+  }
+
+  const dnsheader_aligned dnsHeader(packet.data());
+
+  if (ntohs(dnsHeader->qdcount) == 0) {
+    return false;
+  }
+
+  if (ntohs(dnsHeader->arcount) == 0) {
+    throw std::runtime_error("slowParseEDNSOptions() should not be called for queries that have no EDNS");
+  }
+
+  try {
+    uint64_t numrecords = ntohs(dnsHeader->ancount) + ntohs(dnsHeader->nscount) + ntohs(dnsHeader->arcount);
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast,cppcoreguidelines-pro-type-const-cast)
+    DNSPacketMangler dpm(const_cast<char*>(reinterpret_cast<const char*>(packet.data())), packet.size());
+    uint64_t index{};
+    for (index = 0; index < ntohs(dnsHeader->qdcount); ++index) {
+      dpm.skipDomainName();
+      /* type and class */
+      dpm.skipBytes(4);
+    }
+
+    for (index = 0; index < numrecords; ++index) {
+      dpm.skipDomainName();
+
+      uint8_t section = index < ntohs(dnsHeader->ancount) ? 1 : (index < (ntohs(dnsHeader->ancount) + ntohs(dnsHeader->nscount)) ? 2 : 3);
+      uint16_t dnstype = dpm.get16BitInt();
+      dpm.get16BitInt();
+      dpm.skipBytes(4); /* TTL */
+
+      if (section == 3 && dnstype == QType::OPT) {
+        uint32_t offset = dpm.getOffset();
+        if (offset >= packet.size()) {
+          return false;
+        }
+        /* if we survive this call, we can parse it safely */
+        dpm.skipRData();
+        // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
+        return getEDNSOptions(reinterpret_cast<const char*>(&packet.at(offset)), packet.size() - offset, options) == 0;
+      }
+      dpm.skipRData();
+    }
+  }
+  catch (...) {
+    return false;
+  }
+
+  return true;
+}
+
 /* extract all EDNS0 options from a pointer on the beginning rdLen of the OPT RR */
 int getEDNSOptions(const char* optRR, const size_t len, EDNSOptionViewMap& options)
 {
index fee6eb4afdd1cf65eb698e7129abb38600b17d97..b629382a670993a305e8ccb6264de5317945d31e 100644 (file)
@@ -22,6 +22,8 @@
 #pragma once
 #include "namespaces.hh"
 
+#include "noinitvector.hh"
+
 struct EDNSOptionCode
 {
   // Temporary code assigned for OpenTelemetry TraceID and SpanID
@@ -55,3 +57,4 @@ bool getEDNSOptionsFromContent(const std::string& content, std::vector<std::pair
 bool getNextEDNSOption(const char* data, size_t dataLen, uint16_t& optionCode, uint16_t& optionLen);
 
 void generateEDNSOption(uint16_t optionCode, const std::string& payload, std::string& res);
+bool slowParseEDNSOptions(const PacketBuffer& packet, EDNSOptionViewMap& options);
index d96e1cbb3081d550ee62339c064d13b9f8cfed9b..5923fb1c10a0f77b0bd50bcf148d476ddf059444 100644 (file)
@@ -58,6 +58,7 @@
 thread_local TCPOutConnectionManager t_tcp_manager;
 std::shared_ptr<Logr::Logger> g_slogout;
 bool g_paddingOutgoing;
+bool g_ECSHardening{false};
 
 void remoteLoggerQueueData(RemoteLoggerInterface& rli, const std::string& data)
 {
@@ -596,15 +597,16 @@ static LWResult::Result asyncresolve(const ComboAddress& address, const DNSName&
     if (EDNS0Level > 0 && getEDNSOpts(mdp, &edo)) {
       lwr->d_haveEDNS = true;
 
-      // If we sent out ECS, we can also expect to see a return with or without ECS, the absent case is
-      // not handled explicitly. If we do see a ECS in the reply, the source part *must* match with
-      // what we sent out. See https://www.rfc-editor.org/rfc/rfc7871#section-7.3
+      // If we sent out ECS, we can also expect to see a return with or without ECS, the absent case
+      // is not handled explicitly. In hardening mode, if we do see a ECS in the reply, the source
+      // part *must* match with what we sent out. See
+      // https://www.rfc-editor.org/rfc/rfc7871#section-7.3
       if (subnetOpts) {
         for (const auto& opt : edo.d_options) {
           if (opt.first == EDNSOptionCode::ECS) {
             EDNSSubnetOpts reso;
             if (EDNSSubnetOpts::getFromString(opt.second, &reso)) {
-              if (!(reso.getSource() == subnetOpts->getSource())) {
+              if (g_ECSHardening && !(reso.getSource() == subnetOpts->getSource())) {
                 g_slogout->info(Logr::Notice, "Incoming ECS does not match outgoing",
                                 "server", Logging::Loggable(address),
                                 "qname", Logging::Loggable(domain),
index d8fdae644f0c8a6244423d4adde2c91457a34a04..0c9ba45079dc7bd7ac9d7386aa4b2494355aca0f 100644 (file)
@@ -51,6 +51,7 @@ void remoteLoggerQueueData(RemoteLoggerInterface&, const std::string&);
 
 extern std::shared_ptr<Logr::Logger> g_slogout;
 extern bool g_paddingOutgoing;
+extern bool g_ECSHardening;
 
 class LWResException : public PDNSException
 {
@@ -71,6 +72,7 @@ public:
     OSLimitError = 3,
     Spoofed = 4, /* Spoofing attempt (too many near-misses) */
     ChainLimitError = 5,
+    ECSMissing = 6,
   };
 
   [[nodiscard]] static bool isLimitError(Result res)
index 37889a6f3b20b49af1f5bcc7732805c1aa89443d..0be8a735e1d04e87296b8e0e3e4228290f827965 100644 (file)
@@ -30,8 +30,8 @@
 #include "rec-taskqueue.hh"
 #include "shuffle.hh"
 #include "validate-recursor.hh"
-
 #include "ratelimitedlog.hh"
+#include "ednsoptions.hh"
 
 #ifdef HAVE_SYSTEMD
 #include <systemd/sd-daemon.h>
@@ -227,7 +227,6 @@ static void handleGenUDPQueryResponse(int fileDesc, FDMultiplexer::funcparam_t&
   else {
     PacketBuffer empty;
     g_multiTasker->sendEvent(pident, &empty);
-    //    cerr<<"Had some kind of error: "<<ret<<", "<<stringerror()<<endl;
   }
 }
 
@@ -290,37 +289,33 @@ LWResult::Result asendto(const void* data, size_t len, int /* flags */,
   if (ecs) {
     pident->ecsSubnet = ecs->getSource();
   }
-  // We cannot merge ECS-enabled queries based on the ECS source only, as the scope
-  // of the response might be narrower, so instead we do not chain ECS-enabled queries
-  // at all.
-  if (true || !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 = g_multiTasker->getWaiters().equal_range(pident, PacketIDBirthdayCompare());
 
-    for (; chain.first != chain.second; chain.first++) {
-      // Line below detected an issue with the two ways of ordering PacketIDs (birthday and non-birthday)
-      assert(chain.first->key->domain == pident->domain); // NOLINT
-      // don't chain onto existing chained waiter or a chain already processed
-      if (chain.first->key->fd > -1 && !chain.first->key->closed) {
-        auto currentChainSize = chain.first->key->authReqChain.size();
-        *fileDesc = -static_cast<int>(currentChainSize + 1); // value <= -1, gets used in waitEvent / sendEvent later on
-        if (g_maxChainLength > 0 && currentChainSize >= g_maxChainLength) {
-          return LWResult::Result::ChainLimitError;
-        }
-        assert(uSec(chain.first->key->creationTime) != 0); // NOLINT
-        auto age = now - chain.first->key->creationTime;
-        if (uSec(age) > static_cast<uint64_t>(1000) * authWaitTimeMSec(g_multiTasker) * 2 / 3) {
-          return LWResult::Result::ChainLimitError;
-        }
-        chain.first->key->authReqChain.emplace(*fileDesc, qid); // we can chain
-        auto maxLength = t_Counters.at(rec::Counter::maxChainLength);
-        if (currentChainSize + 1 > maxLength) {
-          t_Counters.at(rec::Counter::maxChainLength) = currentChainSize + 1;
-        }
-        return LWResult::Result::Success;
+  // See if there is an existing outstanding request we can chain on to, using partial equivalence
+  // function looking for the same query (qname, qtype and ecs if applicable) to the same host, but
+  // with a different message ID.
+  auto chain = g_multiTasker->getWaiters().equal_range(pident, PacketIDBirthdayCompare());
+
+  for (; chain.first != chain.second; chain.first++) {
+    // Line below detected an issue with the two ways of ordering PacketIDs (birthday and non-birthday)
+    assert(chain.first->key->domain == pident->domain); // NOLINT
+    // don't chain onto existing chained waiter or a chain already processed
+    if (chain.first->key->fd > -1 && !chain.first->key->closed) {
+      auto currentChainSize = chain.first->key->authReqChain.size();
+      *fileDesc = -static_cast<int>(currentChainSize + 1); // value <= -1, gets used in waitEvent / sendEvent later on
+      if (g_maxChainLength > 0 && currentChainSize >= g_maxChainLength) {
+        return LWResult::Result::ChainLimitError;
       }
+      assert(uSec(chain.first->key->creationTime) != 0); // NOLINT
+      auto age = now - chain.first->key->creationTime;
+      if (uSec(age) > static_cast<uint64_t>(1000) * authWaitTimeMSec(g_multiTasker) * 2 / 3) {
+        return LWResult::Result::ChainLimitError;
+      }
+      chain.first->key->authReqChain.emplace(*fileDesc, qid); // we can chain
+      auto maxLength = t_Counters.at(rec::Counter::maxChainLength);
+      if (currentChainSize + 1 > maxLength) {
+        t_Counters.at(rec::Counter::maxChainLength) = currentChainSize + 1;
+      }
+      return LWResult::Result::Success;
     }
   }
 
@@ -377,6 +372,10 @@ LWResult::Result arecvfrom(PacketBuffer& packet, int /* flags */, const ComboAdd
 
     len = packet.size();
 
+    if (g_ECSHardening && pident->ecsSubnet && !*pident->ecsReceived) {
+      t_Counters.at(rec::Counter::ecsMissingCount)++;
+      return LWResult::Result::ECSMissing;
+    }
     if (nearMissLimit > 0 && pident->nearMisses > nearMissLimit) {
       /* we have received more than nearMissLimit answers on the right IP and port, from the right source (we are using connected sockets),
          for the correct qname and qtype, but with an unexpected message ID. That looks like a spoofing attempt. */
@@ -2686,7 +2685,6 @@ static void handleNewUDPQuestion(int fileDesc, FDMultiplexer::funcparam_t& /* va
       }
     }
     else {
-      // cerr<<t_id<<" had error: "<<stringerror()<<endl;
       if (firstQuery && errno == EAGAIN) {
         t_Counters.at(rec::Counter::noPacketError)++;
       }
@@ -2913,7 +2911,7 @@ void distributeAsyncFunction(const string& packet, const pipefunc_t& func)
 }
 
 // resend event to everybody chained onto it
-static void doResends(MT_t::waiters_t::iterator& iter, const std::shared_ptr<PacketID>& resend, const PacketBuffer& content)
+static void doResends(MT_t::waiters_t::iterator& iter, const std::shared_ptr<PacketID>& resend, const PacketBuffer& content, const std::optional<bool>& ecsReceived)
 {
   // We close the chain for new entries, since they won't be processed anyway
   iter->key->closed = true;
@@ -2922,6 +2920,10 @@ static void doResends(MT_t::waiters_t::iterator& iter, const std::shared_ptr<Pac
     return;
   }
 
+  if (ecsReceived) {
+    iter->key->ecsReceived = ecsReceived;
+  }
+
   auto maxWeight = t_Counters.at(rec::Counter::maxChainWeight);
   auto weight = iter->key->authReqChain.size() * content.size();
   if (weight > maxWeight) {
@@ -2948,6 +2950,31 @@ void mthreadSleep(unsigned int jitterMsec)
   assert(g_multiTasker->waitEvent(neverHappens, nullptr, jitterMsec) != -1); // NOLINT
 }
 
+static bool checkIncomingECSSource(const PacketBuffer& packet, const Netmask& subnet)
+{
+  bool foundMatchingECS = false;
+
+  // We sent out ECS, check if the response has the expected ECS info
+  EDNSOptionViewMap ednsOptions;
+  if (slowParseEDNSOptions(packet, ednsOptions)) {
+    // check content
+    auto option = ednsOptions.find(EDNSOptionCode::ECS);
+    if (option != ednsOptions.end()) {
+      // found an ECS option
+      EDNSSubnetOpts ecs;
+      for (const auto& value : option->second.values) {
+        if (EDNSSubnetOpts::getFromString(value.content, value.size, &ecs)) {
+          if (ecs.getSource() == subnet) {
+            foundMatchingECS = true;
+          }
+        }
+        break; // only look at first
+      }
+    }
+  }
+  return foundMatchingECS;
+}
+
 static void handleUDPServerResponse(int fileDesc, FDMultiplexer::funcparam_t& var)
 {
   auto pid = boost::any_cast<std::shared_ptr<PacketID>>(var);
@@ -2967,7 +2994,7 @@ static void handleUDPServerResponse(int fileDesc, FDMultiplexer::funcparam_t& va
     PacketBuffer empty;
     auto iter = g_multiTasker->getWaiters().find(pid);
     if (iter != g_multiTasker->getWaiters().end()) {
-      doResends(iter, pid, empty);
+      doResends(iter, pid, empty, false);
     }
     g_multiTasker->sendEvent(pid, &empty); // this denotes error (does retry lookup using other NS)
     return;
@@ -3028,7 +3055,10 @@ static void handleUDPServerResponse(int fileDesc, FDMultiplexer::funcparam_t& va
   if (!pident->domain.empty()) {
     auto iter = g_multiTasker->getWaiters().find(pident);
     if (iter != g_multiTasker->getWaiters().end()) {
-      doResends(iter, pident, packet);
+      if (g_ECSHardening) {
+        iter->key->ecsReceived = iter->key->ecsSubnet && checkIncomingECSSource(packet, *iter->key->ecsSubnet);
+      }
+      doResends(iter, pident, packet, iter->key->ecsReceived);
     }
   }
 
@@ -3048,7 +3078,6 @@ retryWithName:
 
       // be a bit paranoid here since we're weakening our matching
       if (pident->domain.empty() && !d_waiter.key->domain.empty() && pident->type == 0 && d_waiter.key->type != 0 && pident->id == d_waiter.key->id && d_waiter.key->remote == pident->remote) {
-        // cerr<<"Empty response, rest matches though, sending to a waiter"<<endl;
         pident->domain = d_waiter.key->domain;
         pident->type = d_waiter.key->type;
         goto retryWithName; // note that this only passes on an error, lwres will still reject the packet NOLINT(cppcoreguidelines-avoid-goto)
index feeba0d6ab4ee7ac0d4c299e6c33829a65286e25..9f637971aa468d9f608743b9255c877902c167c2 100644 (file)
@@ -98,6 +98,7 @@ enum class Counter : uint8_t
   maxChainLength,
   maxChainWeight,
   chainLimits,
+  ecsMissingCount,
 
   numberOfCounters
 };
index e37d1589ee8ee38148fb3b683b842402527b8ff1..c7cc489591769927791b85aeda182f28a6272932 100644 (file)
@@ -5441,17 +5441,29 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname,
     LOG(prefix << qname << ": Query handled by Lua" << endl);
   }
   else {
-    ednsmask = getEDNSSubnetMask(qname, remoteIP);
-    if (ednsmask) {
-      LOG(prefix << qname << ": Adding EDNS Client Subnet Mask " << ednsmask->toString() << " to query" << endl);
-      s_ecsqueries++;
-    }
-    updateQueryCounts(prefix, qname, remoteIP, doTCP, doDoT);
-    auto match = d_eventTrace.add(RecEventTrace::AuthRequest, qname.toLogString() + '/' + qtype.toString(), true, 0);
-    resolveret = asyncresolveWrapper(remoteIP, d_doDNSSEC, qname, auth, qtype.getCode(),
-                                     doTCP, sendRDQuery, &d_now, ednsmask, &lwr, &chained, nsName); // <- we go out on the wire!
-    d_eventTrace.add(RecEventTrace::AuthRequest, static_cast<int64_t>(lwr.d_rcode), false, match);
-    ednsStats(ednsmask, qname, prefix);
+    bool sendECSIfRelevant = true;
+    for (int count = 0; count < 2; ++count) {
+      ednsmask = sendECSIfRelevant ? getEDNSSubnetMask(qname, remoteIP) : boost::none;
+      if (ednsmask) {
+        LOG(prefix << qname << ": Adding EDNS Client Subnet Mask " << ednsmask->toString() << " to query" << endl);
+        s_ecsqueries++;
+      }
+      updateQueryCounts(prefix, qname, remoteIP, doTCP, doDoT);
+      auto match = d_eventTrace.add(RecEventTrace::AuthRequest, qname.toLogString() + '/' + qtype.toString(), true, 0);
+      resolveret = asyncresolveWrapper(remoteIP, d_doDNSSEC, qname, auth, qtype.getCode(),
+                                       doTCP, sendRDQuery, &d_now, ednsmask, &lwr, &chained, nsName); // <- we go out on the wire!
+      d_eventTrace.add(RecEventTrace::AuthRequest, static_cast<int64_t>(lwr.d_rcode), false, match);
+      ednsStats(ednsmask, qname, prefix);
+      if (resolveret == LWResult::Result::ECSMissing) {
+        if (sendECSIfRelevant) {
+          sendECSIfRelevant = false;
+          LOG(prefix << qname << ": Answer has no ECS, trying again without EDNS Client Subnet Mask" << endl);
+          continue;
+        }
+        assert(0); // should not happen
+      }
+      break; // when no ECS shenanigans happened, only one pass through the loop
+    }
   }
 
   /* preoutquery killed the query by setting dq.rcode to -3 */
index d81c5c46f2313bbef82b4435796582163879ee51..bff0f855382fe540959ae9a77b4979c8cea01cf4 100644 (file)
@@ -803,6 +803,7 @@ struct PacketID
   bool inIncompleteOkay{false};
   uint16_t id{0}; // wait for a specific id/remote pair
   uint16_t type{0}; // and this is its type
+  std::optional<bool> ecsReceived; // only set in ecsHardened mode
   TCPAction highState{TCPAction::DoingRead};
   IOState lowState{IOState::NeedRead};