From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Fri, 2 Oct 2020 09:49:11 +0000 (+0200)
Subject: rec: Reply with NOTIMP instead of terminating the TCP connection
X-Git-Tag: auth-4.4.0-alpha2~35^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=28647bcf8d64d5ab2038482750e343ff4fe023a3;p=thirdparty%2Fpdns.git

rec: Reply with NOTIMP instead of terminating the TCP connection

For qdcount == 0 and UPDATE/NOTIFY opcodes.
---

diff --git a/pdns/dnsparser.cc b/pdns/dnsparser.cc
index 193cb1f3d2..3f84fa91cf 100644
--- a/pdns/dnsparser.cc
+++ b/pdns/dnsparser.cc
@@ -339,6 +339,20 @@ void MOADNSParser::init(bool query, const std::string& packet)
   }
 }
 
+bool MOADNSParser::hasEDNS() const
+{
+  if (d_header.arcount == 0 || d_answers.empty()) {
+    return false;
+  }
+
+  for (const auto& record : d_answers) {
+    if (record.first.d_place == DNSResourceRecord::ADDITIONAL && record.first.d_type == QType::OPT) {
+      return true;
+    }
+  }
+
+  return false;
+}
 
 void PacketReader::getDnsrecordheader(struct dnsrecordheader &ah)
 {
diff --git a/pdns/dnsparser.hh b/pdns/dnsparser.hh
index d15be9e776..ea0d2568d3 100644
--- a/pdns/dnsparser.hh
+++ b/pdns/dnsparser.hh
@@ -388,6 +388,9 @@ public:
   {
     return d_tsigPos;
   }
+
+  bool hasEDNS() const;
+
 private:
   void init(bool query, const std::string& packet);
   uint16_t d_tsigPos;
diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index ff27c1c6fb..3d5ad8bb52 100644
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -762,6 +762,67 @@ static void terminateTCPConnection(int fd)
   }
 }
 
+static bool sendResponseOverTCP(const std::unique_ptr<DNSComboWriter>& dc, const std::vector<uint8_t>& packet)
+{
+  char buf[2];
+  buf[0] = packet.size() / 256;
+  buf[1] = packet.size() % 256;
+
+  Utility::iovec iov[2];
+  iov[0].iov_base=(void*)buf;              iov[0].iov_len=2;
+  iov[1].iov_base=(void*)&*packet.begin(); iov[1].iov_len = packet.size();
+
+  int wret = Utility::writev(dc->d_socket, iov, 2);
+  bool hadError = true;
+
+  if (wret == 0) {
+    g_log<<Logger::Warning<<"EOF writing TCP answer to "<<dc->getRemote()<<endl;
+  } else if (wret < 0 ) {
+    int err = errno;
+    g_log << Logger::Warning << "Error writing TCP answer to " << dc->getRemote() << ": " << strerror(err) << endl;
+  } else if ((unsigned int)wret != 2 + packet.size()) {
+    g_log<<Logger::Warning<<"Oops, partial answer sent to "<<dc->getRemote()<<" for "<<dc->d_mdp.d_qname<<" (size="<< (2 + packet.size()) <<", sent "<<wret<<")"<<endl;
+  } else {
+    hadError = false;
+  }
+
+  return hadError;
+}
+
+static void sendErrorOverTCP(std::unique_ptr<DNSComboWriter>& dc, int rcode)
+{
+  std::vector<uint8_t> packet;
+  if (dc->d_mdp.d_header.qdcount == 0) {
+    /* header-only */
+    packet.resize(sizeof(dnsheader));
+  }
+  else {
+    DNSPacketWriter pw(packet, dc->d_mdp.d_qname, dc->d_mdp.d_qtype, dc->d_mdp.d_qclass);
+    if (dc->d_mdp.hasEDNS()) {
+      /* we try to add the EDNS OPT RR even for truncated answers,
+         as rfc6891 states:
+         "The minimal response MUST be the DNS header, question section, and an
+         OPT record.  This MUST also occur when a truncated response (using
+         the DNS header's TC bit) is returned."
+      */
+      pw.addOpt(512, 0, 0);
+      pw.commit();
+    }
+  }
+
+  dnsheader& header = reinterpret_cast<dnsheader&>(packet.at(0));;
+  header.aa = 0;
+  header.ra = 1;
+  header.qr = 1;
+  header.tc = 0;
+  header.id = dc->d_mdp.d_header.id;
+  header.rd = dc->d_mdp.d_header.rd;
+  header.cd = dc->d_mdp.d_header.cd;
+  header.rcode = rcode;
+
+  sendResponseOverTCP(dc, packet);
+}
+
 static void handleRunningTCPQuestion(int fd, FDMultiplexer::funcparam_t& var);
 
 // the idea is, only do things that depend on the *response* here. Incoming accounting is on incoming.
@@ -1860,28 +1921,8 @@ static void startDoResolve(void *p)
       //      else cerr<<"Not putting in packet cache: "<<sr.wasVariable()<<endl;
     }
     else {
-      char buf[2];
-      buf[0]=packet.size()/256;
-      buf[1]=packet.size()%256;
-
-      Utility::iovec iov[2];
-
-      iov[0].iov_base=(void*)buf;              iov[0].iov_len=2;
-      iov[1].iov_base=(void*)&*packet.begin(); iov[1].iov_len = packet.size();
-
-      int wret=Utility::writev(dc->d_socket, iov, 2);
-      bool hadError=true;
+      bool hadError = sendResponseOverTCP(dc, packet);
 
-      if (wret == 0) {
-        g_log<<Logger::Warning<<"EOF writing TCP answer to "<<dc->getRemote()<<endl;
-      } else if (wret < 0 ) {
-        int err = errno;
-        g_log << Logger::Warning << "Error writing TCP answer to " << dc->getRemote() << ": " << strerror(err) << endl;
-      } else if ((unsigned int)wret != 2 + packet.size()) {
-        g_log<<Logger::Warning<<"Oops, partial answer sent to "<<dc->getRemote()<<" for "<<dc->d_mdp.d_qname<<" (size="<< (2 + packet.size()) <<", sent "<<wret<<")"<<endl;
-      } else {
-        hadError=false;
-      }
       // update tcp connection status, closing if needed and doing the fd multiplexer accounting
       if  (dc->d_tcpConnection->d_requestsInFlight > 0) {
         dc->d_tcpConnection->d_requestsInFlight--;
@@ -2396,7 +2437,7 @@ static void handleRunningTCPQuestion(int fd, FDMultiplexer::funcparam_t& var)
         if (g_logCommonErrors) {
           g_log<<Logger::Error<<"Ignoring non-query opcode from TCP client "<< dc->getRemote() <<" on server socket!"<<endl;
         }
-        terminateTCPConnection(fd);
+        sendErrorOverTCP(dc, RCode::NotImp);
         return;
       }
       else if (dh->qdcount == 0) {
@@ -2404,7 +2445,7 @@ static void handleRunningTCPQuestion(int fd, FDMultiplexer::funcparam_t& var)
         if (g_logCommonErrors) {
           g_log<<Logger::Error<<"Ignoring empty (qdcount == 0) query from "<< dc->getRemote() <<" on server socket!"<<endl;
         }
-        terminateTCPConnection(fd);
+        sendErrorOverTCP(dc, RCode::NotImp);
         return;
       }
       else {