From: Otto <otto.moerbeek@open-xchange.com>
Date: Wed, 19 May 2021 10:29:38 +0000 (+0200)
Subject: Handle IOState::NeedWrite/NeedRead by flipping the status
X-Git-Tag: dnsdist-1.7.0-alpha1~138^2~11
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=66242d006ab49ac4c94a50e452a109377de0dd64;p=thirdparty%2Fpdns.git

Handle IOState::NeedWrite/NeedRead by flipping the status
---

diff --git a/pdns/lwres.cc b/pdns/lwres.cc
index 8c3e431b7a..706b3926a4 100644
--- a/pdns/lwres.cc
+++ b/pdns/lwres.cc
@@ -349,8 +349,8 @@ LWResult::Result asyncresolve(const ComboAddress& ip, const DNSName& domain, int
       s.bind(localip);
 
       std::shared_ptr<TLSCtx> tlsCtx{nullptr};
-      TCPIOHandler handler("", s.releaseHandle(), timeout, tlsCtx, now->tv_sec);
-      IOState state = handler.tryConnect(SyncRes::s_tcp_fast_open_connect, ip);
+      auto handler = std::make_shared<TCPIOHandler>("", s.releaseHandle(), timeout, tlsCtx, now->tv_sec);
+      /* auto state = */ handler->tryConnect(SyncRes::s_tcp_fast_open_connect, ip);
 
       uint16_t tlen=htons(vpacket.size());
       char *lenP=(char*)&tlen;
diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index 49d59b01dc..551eb6c122 100644
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -396,52 +396,25 @@ static bool isHandlerThread()
   return s_threadInfos.at(t_id).isHandler;
 }
 
-static void handleTCPClientWritable(int fd, FDMultiplexer::funcparam_t& var);
-
-LWResult::Result asendtcp(const PacketBuffer& data, Socket* sock)
-{
-  PacketID pident;
-  pident.tcpsock=sock->getHandle();
-  pident.outMSG = data;
-
-  t_fdm->addWriteFD(sock->getHandle(), handleTCPClientWritable, pident);
-  PacketBuffer packet;
-
-  int ret = MT->waitEvent(pident, &packet, g_networkTimeoutMsec);
-  if (ret == 0) { //timeout
-    t_fdm->removeWriteFD(sock->getHandle());
-    return LWResult::Result::Timeout;
-  }
-  else if (ret == -1) { // error
-    t_fdm->removeWriteFD(sock->getHandle());
-    return LWResult::Result::PermanentError;
-  }
-  else if (packet.size() != data.size()) { // main loop tells us what it sent out, or empty in case of an error
-    return LWResult::Result::PermanentError;
-  }
-
-  return LWResult::Result::Success;
-}
-
 static void TCPIOHandlerWritable(int fd, FDMultiplexer::funcparam_t& var);
 
-LWResult::Result asendtcp(const PacketBuffer& data, TCPIOHandler& handler)
+LWResult::Result asendtcp(const PacketBuffer& data, shared_ptr<TCPIOHandler>& handler)
 {
   PacketID pident;
-  pident.tcphandler = &handler;
-  pident.tcpsock = handler.getDescriptor();
+  pident.tcphandler = handler;
+  pident.tcpsock = handler->getDescriptor();
   pident.outMSG = data;
 
-  t_fdm->addWriteFD(handler.getDescriptor(), TCPIOHandlerWritable, pident);
+  t_fdm->addWriteFD(handler->getDescriptor(), TCPIOHandlerWritable, pident);
   PacketBuffer packet;
 
   int ret = MT->waitEvent(pident, &packet, g_networkTimeoutMsec);
   if (ret == 0) { //timeout
-    t_fdm->removeWriteFD(handler.getDescriptor());
+    t_fdm->removeWriteFD(handler->getDescriptor());
     return LWResult::Result::Timeout;
   }
   else if (ret == -1) { // error
-    t_fdm->removeWriteFD(handler.getDescriptor());
+    t_fdm->removeWriteFD(handler->getDescriptor());
     return LWResult::Result::PermanentError;
   }
   else if (packet.size() != data.size()) { // main loop tells us what it sent out, or empty in case of an error
@@ -451,53 +424,26 @@ LWResult::Result asendtcp(const PacketBuffer& data, TCPIOHandler& handler)
   return LWResult::Result::Success;
 }
 
-static void handleTCPClientReadable(int fd, FDMultiplexer::funcparam_t& var);
-
-LWResult::Result arecvtcp(PacketBuffer& data, const size_t len, Socket* sock, const bool incompleteOkay)
-{
-  data.clear();
-  PacketID pident;
-  pident.tcpsock=sock->getHandle();
-  pident.inNeeded=len;
-  pident.inIncompleteOkay=incompleteOkay;
-  t_fdm->addReadFD(sock->getHandle(), handleTCPClientReadable, pident);
-
-  int ret = MT->waitEvent(pident, &data, g_networkTimeoutMsec);
-  if (ret == 0) {
-    t_fdm->removeReadFD(sock->getHandle());
-    return LWResult::Result::Timeout;
-  }
-  else if (ret == -1) {
-    t_fdm->removeWriteFD(sock->getHandle());
-    return LWResult::Result::PermanentError;
-  }
-  else if (data.empty()) {// error, EOF or other
-    return LWResult::Result::PermanentError;
-  }
-
-  return LWResult::Result::Success;
-}
-
 static void TCPIOHandlerReadable(int fd, FDMultiplexer::funcparam_t& var);
 
-LWResult::Result arecvtcp(PacketBuffer& data, const size_t len, TCPIOHandler& handler, const bool incompleteOkay)
+LWResult::Result arecvtcp(PacketBuffer& data, const size_t len, shared_ptr<TCPIOHandler>& handler, const bool incompleteOkay)
 {
   data.clear();
 
   PacketID pident;
-  pident.tcphandler = &handler;
-  pident.tcpsock = handler.getDescriptor();
+  pident.tcphandler = handler;
+  pident.tcpsock = handler->getDescriptor();
   pident.inNeeded = len;
   pident.inIncompleteOkay = incompleteOkay;
-  t_fdm->addReadFD(handler.getDescriptor(), TCPIOHandlerReadable, pident);
+  t_fdm->addReadFD(handler->getDescriptor(), TCPIOHandlerReadable, pident);
 
   int ret = MT->waitEvent(pident, &data, g_networkTimeoutMsec);
   if (ret == 0) {
-    t_fdm->removeReadFD(handler.getDescriptor());
+    t_fdm->removeReadFD(handler->getDescriptor());
     return LWResult::Result::Timeout;
   }
   else if (ret == -1) {
-    t_fdm->removeWriteFD(handler.getDescriptor());
+    t_fdm->removeWriteFD(handler->getDescriptor());
     return LWResult::Result::PermanentError;
   }
   else if (data.empty()) {// error, EOF or other
@@ -4078,37 +4024,6 @@ static void handleRCC(int fd, FDMultiplexer::funcparam_t& var)
   }
 }
 
-static void handleTCPClientReadable(int fd, FDMultiplexer::funcparam_t& var)
-{
-  PacketID* pident=boost::any_cast<PacketID>(&var);
-  //  cerr<<"handleTCPClientReadable called for fd "<<fd<<", pident->inNeeded: "<<pident->inNeeded<<", "<<pident->sock->getHandle()<<endl;
-
-  boost::shared_array<char> buffer(new char[pident->inNeeded]);
-
-  ssize_t ret=recv(fd, buffer.get(), pident->inNeeded,0);
-  if(ret > 0) {
-    pident->inMSG.insert(pident->inMSG.end(), &buffer[0],  &buffer[ret]);
-    pident->inNeeded -= (size_t)ret;
-    if(!pident->inNeeded || pident->inIncompleteOkay) {
-      //      cerr<<"Got entire load of "<<pident->inMSG.size()<<" bytes"<<endl;
-      PacketID pid=*pident;
-      PacketBuffer msg = pident->inMSG;
-
-      t_fdm->removeReadFD(fd);
-      MT->sendEvent(pid, &msg);
-    }
-    else {
-      //      cerr<<"Still have "<<pident->inNeeded<<" left to go"<<endl;
-    }
-  }
-  else {
-    PacketID tmp=*pident;
-    t_fdm->removeReadFD(fd); // pident might now be invalid (it isn't, but still)
-    PacketBuffer empty;
-    MT->sendEvent(tmp, &empty); // this conveys error status
-  }
-}
-
 static void TCPIOHandlerReadable(int fd, FDMultiplexer::funcparam_t& var)
 {
   PacketID* pident=boost::any_cast<PacketID>(&var);
@@ -4129,14 +4044,14 @@ static void TCPIOHandlerReadable(int fd, FDMultiplexer::funcparam_t& var)
       pident->inMSG.insert(pident->inMSG.end(), buffer.data(), buffer.data() + pos);
       pident->inNeeded -= pos;
       if (pident->inNeeded == 0 || pident->inIncompleteOkay) {
+        // removeReadFD seems to clobber PacketID, so take a copy
         PacketID pid = *pident;
-        PacketBuffer msg = pident->inMSG;
         t_fdm->removeReadFD(fd);
-        MT->sendEvent(pid, &msg);
+        MT->sendEvent(pid, &pid.inMSG);
       }
       break;
     case IOState::NeedWrite:
-      // What to do?
+      t_fdm->alterFDToWrite(fd, TCPIOHandlerWritable, var);
       break;
     }
   }
@@ -4148,26 +4063,6 @@ static void TCPIOHandlerReadable(int fd, FDMultiplexer::funcparam_t& var)
   }
 }
 
-static void handleTCPClientWritable(int fd, FDMultiplexer::funcparam_t& var)
-{
-  PacketID* pid = boost::any_cast<PacketID>(&var);
-  ssize_t ret = send(fd, pid->outMSG.data() + pid->outPos, pid->outMSG.size() - pid->outPos,0);
-  if (ret > 0) {
-    pid->outPos += (ssize_t)ret;
-    if (pid->outPos == pid->outMSG.size()) {
-      PacketID tmp=*pid;
-      t_fdm->removeWriteFD(fd);
-      MT->sendEvent(tmp, &tmp.outMSG);  // send back what we sent to convey everything is ok
-    }
-  }
-  else {  // error or EOF
-    PacketID tmp(*pid);
-    t_fdm->removeWriteFD(fd);
-    PacketBuffer sent;
-    MT->sendEvent(tmp, &sent);         // we convey error status by sending empty string
-  }
-}
-
 static void TCPIOHandlerWritable(int fd, FDMultiplexer::funcparam_t& var)
 {
   PacketID* pid = boost::any_cast<PacketID>(&var);
@@ -4178,6 +4073,7 @@ static void TCPIOHandlerWritable(int fd, FDMultiplexer::funcparam_t& var)
     IOState state = pid->tcphandler->tryWrite(pid->outMSG, pid->outPos, pid->outMSG.size());
     switch (state) {
     case IOState::Done: {
+      // removeWriteFD seems to clobber PacketID, so take a copy
       PacketID tmp = *pid;
       t_fdm->removeWriteFD(fd);
       MT->sendEvent(tmp, &tmp.outMSG);  // send back what we sent to convey everything is ok
@@ -4187,11 +4083,12 @@ static void TCPIOHandlerWritable(int fd, FDMultiplexer::funcparam_t& var)
       // We'll get back later
     break;
     case IOState::NeedRead:
-      // What to do?
+      t_fdm->alterFDToRead(fd, TCPIOHandlerReadable, var);
       break;
     }
   }
   catch (const std::runtime_error& e) {
+    // removeWriteFD seems to clobber PacketID, so take a copy
     PacketID tmp = *pid;
     t_fdm->removeWriteFD(fd);
     PacketBuffer sent;
@@ -5881,3 +5778,5 @@ int main(int argc, char **argv)
 
   return ret;
 }
+
+bool g_verbose; // XXX FIX ME XXX, see tcpiohandler.cc
diff --git a/pdns/rec-carbon.cc b/pdns/rec-carbon.cc
index 07d117bdbc..6dbbf4b8d9 100644
--- a/pdns/rec-carbon.cc
+++ b/pdns/rec-carbon.cc
@@ -49,16 +49,18 @@ try
   for(const auto& carbonServer: carbonServers) {
     ComboAddress remote(carbonServer, 2003);
     Socket s(remote.sin4.sin_family, SOCK_STREAM);
-
     s.setNonBlocking();
-    s.connect(remote);  // we do the connect so the first attempt happens while we gather stats
- 
+    std::shared_ptr<TLSCtx> tlsCtx{nullptr};
+    const int timeout = (g_networkTimeoutMsec + 999) / 1000;    // XXX tcpiohandler's unit is seconds
+    auto handler = std::make_shared<TCPIOHandler>("", s.releaseHandle(), timeout, tlsCtx, time(nullptr));
+     handler->tryConnect(SyncRes::s_tcp_fast_open_connect, remote);// we do the connect so the first attempt happens while we gather stats
+
     if(msg.empty()) {
       auto all = getAllStatsMap(StatComponent::Carbon);
-      
+
       ostringstream str;
       time_t now=time(0);
-      
+
       for(const auto& val : all) {
         str<<namespace_name<<'.'<<hostname<<'.'<<instance_name<<'.'<<val.first<<' '<<val.second.d_value<<' '<<now<<"\r\n";
       }
@@ -66,7 +68,7 @@ try
       msg.insert(msg.end(), x.cbegin(), x.cend());
     }
 
-    auto ret = asendtcp(msg, &s);     // this will actually do the right thing waiting on the connect
+    auto ret = asendtcp(msg, handler);     // this will actually do the right thing waiting on the connect
     if (ret == LWResult::Result::Timeout) {
       g_log<<Logger::Warning<<"Timeout connecting/writing carbon data to "<<remote.toStringWithPort()<<endl;
     }
diff --git a/pdns/recursordist/Makefile.am b/pdns/recursordist/Makefile.am
index ecdc4632f4..4626e59f9e 100644
--- a/pdns/recursordist/Makefile.am
+++ b/pdns/recursordist/Makefile.am
@@ -116,6 +116,7 @@ pdns_recursor_SOURCES = \
 	dnssecinfra.hh dnssecinfra.cc \
 	dnsseckeeper.hh \
 	dnswriter.cc dnswriter.hh \
+	dolog.hh \
 	ednsextendederror.cc ednsextendederror.hh \
 	ednsoptions.cc ednsoptions.hh \
 	ednspadding.cc ednspadding.hh \
@@ -184,7 +185,7 @@ pdns_recursor_SOURCES = \
 	svc-records.cc svc-records.hh \
 	syncres.cc syncres.hh \
 	taskqueue.cc taskqueue.hh \
-	tcpiohandler.hh \
+	tcpiohandler.cc tcpiohandler.hh \
 	threadname.hh threadname.cc \
 	tsigverifier.cc tsigverifier.hh \
 	ueberbackend.hh \
@@ -410,6 +411,18 @@ pdns_recursor_LDADD += \
 	$(FSTRM_LIBS)
 endif
 
+if HAVE_DNS_OVER_TLS
+if HAVE_LIBSSL
+AM_CPPFLAGS += $(LIBSSL_CFLAGS)
+pdns_recursor_LDADD += $(LIBSSL_LIBS)
+endif
+
+if HAVE_GNUTLS
+AM_CPPFLAGS += $(GNUTLS_CFLAGS)
+pdns_recursor_LDADD += $(GNUTLS_LIBS)
+endif
+endif
+
 rec_control_SOURCES = \
 	arguments.cc arguments.hh \
 	dnslabeltext.cc \
diff --git a/pdns/recursordist/configure.ac b/pdns/recursordist/configure.ac
index ca6088bfc6..2727cbb517 100644
--- a/pdns/recursordist/configure.ac
+++ b/pdns/recursordist/configure.ac
@@ -88,6 +88,20 @@ PDNS_WITH_LIBCAP
 
 PDNS_WITH_NET_SNMP
 
+AM_CONDITIONAL([HAVE_GNUTLS], [false])
+AM_CONDITIONAL([HAVE_LIBSSL], [false])
+
+PDNS_ENABLE_DNS_OVER_TLS
+
+AS_IF([test "x$enable_dns_over_tls" != "xno"], [
+  PDNS_WITH_LIBSSL
+  PDNS_WITH_GNUTLS
+
+  AS_IF([test "x$HAVE_GNUTLS" != "x1" -a "x$HAVE_LIBSSL" != "x1"], [
+    AC_MSG_ERROR([DNS over TLS support requested but neither GnuTLS nor OpenSSL are available])
+  ])
+])
+
 # check for tools we might need
 PDNS_CHECK_RAGEL([pdns/dnslabeltext.cc], [www.powerdns.com])
 PDNS_CHECK_CURL
@@ -224,5 +238,21 @@ AM_COND_IF([FSTRM],
   [AC_MSG_NOTICE([dnstap: yes])],
   [AC_MSG_NOTICE([dnstap: no])]
 )
+AS_IF([test "x$enable_dns_over_tls" != "xno"],
+  [AC_MSG_NOTICE([DNS over TLS: yes])],
+  [AC_MSG_NOTICE([DNS over TLS: no])]
+)
+AS_IF([test "x$enable_dns_over_tls" != "xno"], [
+  AS_IF([test "x$GNUTLS_LIBS" != "x"],
+    [AC_MSG_NOTICE([GnuTLS: yes])],
+    [AC_MSG_NOTICE([GnuTLS: no])]
+  )]
+)
+AS_IF([test "x$enable_dns_over_tls" != "xno"], [
+  AS_IF([test "x$LIBSSL_LIBS" != "x"],
+    [AC_MSG_NOTICE([OpenSSL: yes])],
+    [AC_MSG_NOTICE([OpenSSL: no])]
+  )]
+)
 AC_MSG_NOTICE([Context library: $pdns_context_library])
 AC_MSG_NOTICE([])
diff --git a/pdns/recursordist/dolog.hh b/pdns/recursordist/dolog.hh
new file mode 120000
index 0000000000..e458b07d27
--- /dev/null
+++ b/pdns/recursordist/dolog.hh
@@ -0,0 +1 @@
+../dolog.hh
\ No newline at end of file
diff --git a/pdns/recursordist/m4/pdns_enable_tls.m4 b/pdns/recursordist/m4/pdns_enable_tls.m4
new file mode 120000
index 0000000000..6e0eb49006
--- /dev/null
+++ b/pdns/recursordist/m4/pdns_enable_tls.m4
@@ -0,0 +1 @@
+../../../m4/pdns_enable_tls.m4
\ No newline at end of file
diff --git a/pdns/recursordist/m4/pdns_with_gnutls.m4 b/pdns/recursordist/m4/pdns_with_gnutls.m4
new file mode 120000
index 0000000000..b892c7fb49
--- /dev/null
+++ b/pdns/recursordist/m4/pdns_with_gnutls.m4
@@ -0,0 +1 @@
+../../../m4/pdns_with_gnutls.m4
\ No newline at end of file
diff --git a/pdns/recursordist/m4/pdns_with_libssl.m4 b/pdns/recursordist/m4/pdns_with_libssl.m4
new file mode 120000
index 0000000000..7a8a381775
--- /dev/null
+++ b/pdns/recursordist/m4/pdns_with_libssl.m4
@@ -0,0 +1 @@
+../../../m4/pdns_with_libssl.m4
\ No newline at end of file
diff --git a/pdns/recursordist/tcpiohandler.cc b/pdns/recursordist/tcpiohandler.cc
new file mode 120000
index 0000000000..a583875518
--- /dev/null
+++ b/pdns/recursordist/tcpiohandler.cc
@@ -0,0 +1 @@
+../tcpiohandler.cc
\ No newline at end of file
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index aabf85c2e9..086630bc75 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -4241,17 +4241,13 @@ int SyncRes::doResolveAt(NsSet &nameservers, DNSName auth, bool flawedNSSet, con
           bool truncated = false;
           bool spoofed = false;
           bool gotAnswer = false;
+          bool forceTCP = remoteIP->getPort() == 853;
 
-// Option below is for debugging purposes ony
-#define USE_TCP_ONLY 0
-
-#if !USE_TCP_ONLY
-          gotAnswer = doResolveAtThisIP(prefix, qname, qtype, lwr, ednsmask, auth, sendRDQuery, wasForwarded,
-                                        tns->first, *remoteIP, false, truncated, spoofed);
-          if (spoofed || (gotAnswer && truncated)) {
-#else
-          {
-#endif
+          if (!forceTCP) {
+            gotAnswer = doResolveAtThisIP(prefix, qname, qtype, lwr, ednsmask, auth, sendRDQuery, wasForwarded,
+                                          tns->first, *remoteIP, false, truncated, spoofed);
+          }
+          if (forceTCP || (spoofed || (gotAnswer && truncated))) {
             /* retry, over TCP this time */
             gotAnswer = doResolveAtThisIP(prefix, qname, qtype, lwr, ednsmask, auth, sendRDQuery, wasForwarded,
                                           tns->first, *remoteIP, true, truncated, spoofed);
diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 3a5e71a76d..19c7706133 100644
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -919,12 +919,9 @@ private:
   LogMode d_lm;
 };
 
-class Socket;
 /* external functions, opaque to us */
-LWResult::Result asendtcp(const PacketBuffer& data, Socket* sock);
-LWResult::Result arecvtcp(PacketBuffer& data, size_t len, Socket* sock, bool incompleteOkay);
-LWResult::Result asendtcp(const PacketBuffer& data, TCPIOHandler&);
-LWResult::Result arecvtcp(PacketBuffer& data, size_t len, TCPIOHandler&, bool incompleteOkay);
+LWResult::Result asendtcp(const PacketBuffer& data, shared_ptr<TCPIOHandler>&);
+LWResult::Result arecvtcp(PacketBuffer& data, size_t len, shared_ptr<TCPIOHandler>&, bool incompleteOkay);
 
 struct PacketID
 {
@@ -941,7 +938,7 @@ struct PacketID
 
   typedef set<uint16_t > chain_t;
   mutable chain_t chain;
-  TCPIOHandler *tcphandler{nullptr};
+  shared_ptr<TCPIOHandler> tcphandler{nullptr};
   size_t inNeeded{0}; // if this is set, we'll read until inNeeded bytes are read
   string::size_type outPos{0};    // how far we are along in the outMSG
   mutable uint32_t nearMisses{0}; // number of near misses - host correct, id wrong
diff --git a/pdns/ws-recursor.cc b/pdns/ws-recursor.cc
index ed6e4e2c5e..4be72fc98e 100644
--- a/pdns/ws-recursor.cc
+++ b/pdns/ws-recursor.cc
@@ -42,6 +42,7 @@
 #include "rec-lua-conf.hh"
 #include "rpzloader.hh"
 #include "uuid-utils.hh"
+#include "tcpiohandler.hh"
 
 extern thread_local FDMultiplexer* t_fdm;
 
@@ -1249,10 +1250,14 @@ void AsyncWebServer::serveConnection(std::shared_ptr<Socket> client) const {
     yarl.initialize(&req);
     client->setNonBlocking();
 
+    const int timeout = (g_networkTimeoutMsec + 999) / 1000; // XXX tcpiohandler's unit is seconds
+    std::shared_ptr<TLSCtx> tlsCtx{nullptr};
+    auto handler = std::make_shared<TCPIOHandler>("", client->releaseHandle(), timeout, tlsCtx, time(nullptr));
+
     PacketBuffer data;
     try {
       while(!req.complete) {
-        auto ret = arecvtcp(data, 16384, client.get(), true);
+        auto ret = arecvtcp(data, 16384, handler, true);
         if (ret == LWResult::Result::Success) {
           string str(reinterpret_cast<const char*>(data.data()), data.size());
           req.complete = yarl.feed(str);
@@ -1282,7 +1287,7 @@ void AsyncWebServer::serveConnection(std::shared_ptr<Socket> client) const {
     logResponse(resp, remote, logprefix);
 
     // now send the reply
-    if (asendtcp(reply, client.get()) != LWResult::Result::Success || reply.empty()) {
+    if (asendtcp(reply, handler) != LWResult::Result::Success || reply.empty()) {
       g_log<<Logger::Error<<logprefix<<"Failed sending reply to HTTP client"<<endl;
     }
   }