]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
Chain on ECS matching, and consider a mismatch in returned ECS as a spoof attempt
authorOtto Moerbeek <otto.moerbeek@open-xchange.com>
Wed, 15 Jan 2025 13:23:04 +0000 (14:23 +0100)
committerOtto Moerbeek <otto.moerbeek@open-xchange.com>
Mon, 14 Jul 2025 07:10:08 +0000 (09:10 +0200)
pdns/recursordist/lwres.cc
pdns/recursordist/lwres.hh
pdns/recursordist/pdns_recursor.cc
pdns/recursordist/syncres.hh

index 2ecac1162d1d7f62e54c51ba9f5a8500d0cc3746..06096bb8970dbe55b7c072c10098eb92522e5736 100644 (file)
@@ -420,18 +420,12 @@ static LWResult::Result asyncresolve(const ComboAddress& address, const DNSName&
   pw.getHeader()->cd = (sendRDQuery && g_dnssecmode != DNSSECMode::Off);
 
   string ping;
-  bool weWantEDNSSubnet = false;
-  uint8_t outgoingECSBits = 0;
-  ComboAddress outgoingECSAddr;
   std::optional<EDNSSubnetOpts> subnetOpts = std::nullopt;
   if (EDNS0Level > 0) {
     DNSPacketWriter::optvect_t opts;
     if (srcmask) {
       subnetOpts->setSource(*srcmask);
-      outgoingECSBits = srcmask->getBits();
-      outgoingECSAddr = srcmask->getNetwork();
       opts.emplace_back(EDNSOptionCode::ECS, subnetOpts->makeOptString());
-      weWantEDNSSubnet = true;
     }
 
     if (dnsOverTLS && g_paddingOutgoing) {
@@ -500,7 +494,7 @@ static LWResult::Result asyncresolve(const ComboAddress& address, const DNSName&
 #endif /* HAVE_FSTRM */
 
     // sleep until we see an answer to this, interface to mtasker
-    ret = arecvfrom(buf, 0, address, len, qid, domain, type, queryfd, *now);
+    ret = arecvfrom(buf, 0, address, len, qid, domain, type, queryfd, subnetOpts, *now);
   }
   else {
     bool isNew;
@@ -601,17 +595,29 @@ static LWResult::Result asyncresolve(const ComboAddress& address, const DNSName&
     if (EDNS0Level > 0 && getEDNSOpts(mdp, &edo)) {
       lwr->d_haveEDNS = true;
 
-      if (weWantEDNSSubnet) {
+      // 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 (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())) {
+                g_slogout->info(Logr::Notice, "Incoming ECS does not match outgoing",
+                                "server", Logging::Loggable(address),
+                                "qname", Logging::Loggable(domain),
+                                "outgoing", Logging::Loggable(subnetOpts->getSource()),
+                                "incoming", Logging::Loggable(reso.getSource()));
+                return LWResult::Result::Spoofed; // XXXXX OK?
+              }
               /* 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
                  entries in our cache. */
               if (reso.getScopePrefixLength() != 0) {
-                uint8_t bits = std::min(reso.getScopePrefixLength(), outgoingECSBits);
+                uint8_t bits = std::min(reso.getScopePrefixLength(), subnetOpts->getSourcePrefixLength());
+                auto outgoingECSAddr = subnetOpts->getSource().getNetwork();
                 outgoingECSAddr.truncate(bits);
                 srcmask = Netmask(outgoingECSAddr, bits);
               }
index 8b9e7321badebff84f65072d1e388f59be1bca72..d8fdae644f0c8a6244423d4adde2c91457a34a04 100644 (file)
@@ -91,6 +91,6 @@ class EDNSSubnetOpts;
 LWResult::Result asendto(const void* data, size_t len, int flags, const ComboAddress& toAddress, uint16_t qid,
                          const DNSName& domain, uint16_t qtype, const std::optional<EDNSSubnetOpts>& ecs, int* fileDesc, timeval& now);
 LWResult::Result arecvfrom(PacketBuffer& packet, int flags, const ComboAddress& fromAddr, size_t& len, uint16_t qid,
-                           const DNSName& domain, uint16_t qtype, int fileDesc, const struct timeval& now);
+                           const DNSName& domain, uint16_t qtype, int fileDesc, const std::optional<EDNSSubnetOpts>& ecs, const struct timeval& now);
 
 LWResult::Result asyncresolve(const ComboAddress& address, const DNSName& domain, int type, bool doTCP, bool sendRDQuery, int EDNS0Level, struct timeval* now, boost::optional<Netmask>& srcmask, const ResolveContext& context, const std::shared_ptr<std::vector<std::unique_ptr<RemoteLogger>>>& outgoingLoggers, const std::shared_ptr<std::vector<std::unique_ptr<FrameStreamLogger>>>& fstrmLoggers, const std::set<uint16_t>& exportTypes, LWResult* lwr, bool* chained);
index 5d30c4404de0f4363c00837fac47c39f1d81c4bb..37889a6f3b20b49af1f5bcc7732805c1aa89443d 100644 (file)
@@ -287,11 +287,13 @@ LWResult::Result asendto(const void* data, size_t len, int /* flags */,
   pident->domain = domain;
   pident->remote = toAddress;
   pident->type = qtype;
-
+  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 (!ecs) {
+  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.
@@ -345,7 +347,7 @@ LWResult::Result asendto(const void* data, size_t len, int /* flags */,
 }
 
 LWResult::Result arecvfrom(PacketBuffer& packet, int /* flags */, const ComboAddress& fromAddr, size_t& len,
-                           uint16_t qid, const DNSName& domain, uint16_t qtype, int fileDesc, const struct timeval& now)
+                           uint16_t qid, const DNSName& domain, uint16_t qtype, int fileDesc, const std::optional<EDNSSubnetOpts>& ecs, const struct timeval& now)
 {
   static const unsigned int nearMissLimit = ::arg().asNum("spoof-nearmiss-max");
 
@@ -356,7 +358,13 @@ LWResult::Result arecvfrom(PacketBuffer& packet, int /* flags */, const ComboAdd
   pident->type = qtype;
   pident->remote = fromAddr;
   pident->creationTime = now;
-
+  if (ecs) {
+    // We sent out the query using ecs
+    // We expect incoming source ECS to match, see https://www.rfc-editor.org/rfc/rfc7871#section-7.3
+    // But there's also section 11-2, which says we should treat absent incoming ecs as scope zero
+    // We fill in the search key with the ecs we sent out, so both cases are covered and accepted here.
+    pident->ecsSubnet = ecs->getSource();
+  }
   int ret = g_multiTasker->waitEvent(pident, &packet, authWaitTimeMSec(g_multiTasker), &now);
   len = 0;
 
index 91e800627125a9eff4cbff9a236f0d90142a89aa..ec139815ecddfe0b0165dc7f8a6afc820ec73a98 100644 (file)
@@ -792,6 +792,7 @@ struct PacketID
   mutable chain_t authReqChain;
   shared_ptr<TCPIOHandler> tcphandler{nullptr};
   timeval creationTime{};
+  std::optional<Netmask> ecsSubnet;
   string::size_type inPos{0}; // how far are we along in the inMSG
   size_t inWanted{0}; // if this is set, we'll read until inWanted bytes are read
   string::size_type outPos{0}; // how far we are along in the outMSG
@@ -831,10 +832,10 @@ struct PacketIDCompare
 {
   bool operator()(const std::shared_ptr<PacketID>& lhs, const std::shared_ptr<PacketID>& rhs) const
   {
-    if (std::tie(lhs->remote, lhs->tcpsock, lhs->type) < std::tie(rhs->remote, rhs->tcpsock, rhs->type)) {
+    if (std::tie(lhs->remote, lhs->tcpsock, lhs->type, lhs->ecsSubnet) < std::tie(rhs->remote, rhs->tcpsock, rhs->type, rhs->ecsSubnet)) {
       return true;
     }
-    if (std::tie(lhs->remote, lhs->tcpsock, lhs->type) > std::tie(rhs->remote, rhs->tcpsock, rhs->type)) {
+    if (std::tie(lhs->remote, lhs->tcpsock, lhs->type, lhs->ecsSubnet) > std::tie(rhs->remote, rhs->tcpsock, rhs->type, rhs->ecsSubnet)) {
       return false;
     }
 
@@ -846,10 +847,10 @@ struct PacketIDBirthdayCompare
 {
   bool operator()(const std::shared_ptr<PacketID>& lhs, const std::shared_ptr<PacketID>& rhs) const
   {
-    if (std::tie(lhs->remote, lhs->tcpsock, lhs->type) < std::tie(rhs->remote, rhs->tcpsock, rhs->type)) {
+    if (std::tie(lhs->remote, lhs->tcpsock, lhs->type, lhs->ecsSubnet) < std::tie(rhs->remote, rhs->tcpsock, rhs->type, rhs->ecsSubnet)) {
       return true;
     }
-    if (std::tie(lhs->remote, lhs->tcpsock, lhs->type) > std::tie(rhs->remote, rhs->tcpsock, rhs->type)) {
+    if (std::tie(lhs->remote, lhs->tcpsock, lhs->type, lhs->ecsSubnet) > std::tie(rhs->remote, rhs->tcpsock, rhs->type, rhs->ecsSubnet)) {
       return false;
     }
     return lhs->domain < rhs->domain;