dnsdist: Fix incoming DoT when OpenSSL's read-ahead mode is enabled

diff --git a/pdns/dnsdist-tcp.cc b/pdns/dnsdist-tcp.cc
index 2f78c22dad7a81d8d20fec7a0bbe8c3825f356d6..c55574ef27d46fe3b3267b8d200577d67171b04f 100644 (file)
--- a/pdns/dnsdist-tcp.cc
+++ b/pdns/dnsdist-tcp.cc
@@ -372,7 +372,7 @@ void IncomingTCPConnectionState::terminateClientConnection()
   }
 }
 
-void IncomingTCPConnectionState::queueResponse(std::shared_ptr<IncomingTCPConnectionState>& state, const struct timeval& now, TCPResponse&& response)
+void IncomingTCPConnectionState::queueResponse(std::shared_ptr<IncomingTCPConnectionState>& state, const struct timeval& now, TCPResponse&& response, bool fromBackend)
 {
   // queue response
   state->d_queuedResponses.emplace_back(std::move(response));
@@ -400,6 +400,17 @@ void IncomingTCPConnectionState::queueResponse(std::shared_ptr<IncomingTCPConnec
     // for the same reason we need to update the state right away, nobody will do that for us
     if (state->active()) {
       updateIO(state, iostate, now);
+      // if we have not finished reading every available byte, we _need_ to do an actual read
+      // attempt before waiting for the socket to become readable again, because if there is
+      // buffered data available the socket might never become readable again.
+      // This is true as soon as we deal with TLS because TLS records are processed one by
+      // one and might not match what we see at the application layer, so data might already
+      // be available in the TLS library's buffers. This is especially true when OpenSSL's
+      // read-ahead mode is enabled because then it buffers even more than one SSL record
+      // for performance reasons.
+      if (fromBackend && !state->d_lastIOBlocked) {
+        state->handleIO();
+      }
     }
   }
 }
@@ -522,7 +533,7 @@ void IncomingTCPConnectionState::handleResponse(const struct timeval& now, TCPRe
   ++dnsdist::metrics::g_stats.responses;
   ++state->d_ci.cs->responses;
 
-  queueResponse(state, now, std::move(response));
+  queueResponse(state, now, std::move(response), true);
 }
 
 struct TCPCrossProtocolResponse
@@ -651,7 +662,7 @@ IncomingTCPConnectionState::QueryProcessingResult IncomingTCPConnectionState::ha
     TCPResponse response;
     d_state = State::idle;
     ++d_currentQueriesCount;
-    queueResponse(state, now, std::move(response));
+    queueResponse(state, now, std::move(response), false);
     return QueryProcessingResult::SelfAnswered;
   }
 
@@ -670,7 +681,7 @@ IncomingTCPConnectionState::QueryProcessingResult IncomingTCPConnectionState::ha
       response.d_buffer = std::move(query);
       d_state = State::idle;
       ++d_currentQueriesCount;
-      queueResponse(state, now, std::move(response));
+      queueResponse(state, now, std::move(response), false);
       return QueryProcessingResult::Empty;
     }
   }
@@ -751,7 +762,7 @@ IncomingTCPConnectionState::QueryProcessingResult IncomingTCPConnectionState::ha
 
     d_state = State::idle;
     ++d_currentQueriesCount;
-    queueResponse(state, now, std::move(response));
+    queueResponse(state, now, std::move(response), false);
     return QueryProcessingResult::SelfAnswered;
   }
 
@@ -1188,7 +1199,7 @@ void IncomingTCPConnectionState::handleXFRResponse(const struct timeval& now, TC
   }
 
   std::shared_ptr<IncomingTCPConnectionState> state = shared_from_this();
-  queueResponse(state, now, std::move(response));
+  queueResponse(state, now, std::move(response), true);
 }
 
 void IncomingTCPConnectionState::handleTimeout(std::shared_ptr<IncomingTCPConnectionState>& state, bool write)

diff --git a/pdns/dnsdistdist/dnsdist-tcp-downstream.cc b/pdns/dnsdistdist/dnsdist-tcp-downstream.cc
index 89b3a2e5d81200d6db0cf5b189bda7acefaf8f8f..8dc6cc0a99ad21a0a81ec2dca1c98e3ba9426268 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-tcp-downstream.cc
+++ b/pdns/dnsdistdist/dnsdist-tcp-downstream.cc
@@ -362,7 +362,7 @@ void TCPConnectionToBackend::handleIO(std::shared_ptr<TCPConnectionToBackend>& c
             iostate = conn->handleResponse(conn, now);
           }
           catch (const std::exception& e) {
-            vinfolog("Got an exception while handling TCP response from %s (client is %s): %s", conn->d_ds ? conn->d_ds->getName() : "unknown", conn->d_currentQuery.d_query.d_idstate.origRemote.toStringWithPort(), e.what());
+            vinfolog("Got an exception while handling TCP response from %s (client is %s): %s", conn->d_ds ? conn->d_ds->getNameWithAddr() : "unknown", conn->d_currentQuery.d_query.d_idstate.origRemote.toStringWithPort(), e.what());
             ioGuard.release();
             conn->release();
             return;
@@ -726,7 +726,14 @@ IOState TCPConnectionToBackend::handleResponse(std::shared_ptr<TCPConnectionToBa
     d_state = State::idle;
     t_downstreamTCPConnectionsManager.moveToIdle(conn);
   }
+  else if (!d_pendingResponses.empty()) {
+    d_currentPos = 0;
+    d_state = State::waitingForResponseFromBackend;
+  }
 
+  // be very careful that handleResponse() might trigger new queries being assigned to us,
+  // which may reset our d_currentPos, d_state and/or d_responseBuffer, so we cannot assume
+  // anything without checking first
   auto shared = conn;
   if (sender->active()) {
     DEBUGLOG("passing response to client connection for "<<ids.qname);
@@ -741,9 +748,6 @@ IOState TCPConnectionToBackend::handleResponse(std::shared_ptr<TCPConnectionToBa
   }
   else if (!d_pendingResponses.empty()) {
     DEBUGLOG("still have some responses to read");
-    d_state = State::waitingForResponseFromBackend;
-    d_currentPos = 0;
-    d_responseBuffer.resize(sizeof(uint16_t));
     return IOState::NeedRead;
   }
   else {

diff --git a/pdns/dnsdistdist/dnsdist-tcp-upstream.hh b/pdns/dnsdistdist/dnsdist-tcp-upstream.hh
index f1c49e93e4e2ede74c98f39b945ac559f78ff925..e3ee319b11a1c7bb09368d4a2c9954aa79deb26c 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-tcp-upstream.hh
+++ b/pdns/dnsdistdist/dnsdist-tcp-upstream.hh
@@ -126,7 +126,7 @@ public:
   static void handleAsyncReady(int fd, FDMultiplexer::funcparam_t& param);
   static void updateIO(std::shared_ptr<IncomingTCPConnectionState>& state, IOState newState, const struct timeval& now);
 
-  static void queueResponse(std::shared_ptr<IncomingTCPConnectionState>& state, const struct timeval& now, TCPResponse&& response);
+  static void queueResponse(std::shared_ptr<IncomingTCPConnectionState>& state, const struct timeval& now, TCPResponse&& response, bool fromBackend);
   static void handleTimeout(std::shared_ptr<IncomingTCPConnectionState>& state, bool write);
 
   virtual void handleIO();

diff --git a/pdns/tcpiohandler.cc b/pdns/tcpiohandler.cc
index 0b960134971bffc4e8b59d21a10c61484fa7edea..6be1eea47680ec6c1bea34b4190296e8eee3bc15 100644 (file)
--- a/pdns/tcpiohandler.cc
+++ b/pdns/tcpiohandler.cc
@@ -435,6 +435,7 @@ public:
   bool hasBufferedData() const override
   {
     if (d_conn) {
+      /* this is broken when read-ahead is set, unfortunately */
       return SSL_pending(d_conn.get()) > 0;
     }