Handshake Problem during Renegotiation

Here is what happens:

   - Squid receives TLS Hello from the client (TCP connection A).
   - Squid successfully negotiates an TLS connection with the origin server
     (TCP connection B).
   - Squid successfully negotiates an TLS connection with the client
     (TCP connection A).
   - Squid marks connection B as "idle" and waits an HTTP request from
     connection A.
   - The origin server continues talking to Squid (TCP connection B).
     Squid detects a network read on an idle connection and closes TCP
     connection B (and then the associated TCP connection A as well).

This patch:
   - When squid detects a network read on server idle connection do an
     SSL_read to:
       a) see if application data received from server and abort in this case
       b) detect possible TLS error, or TLS shutdown message from server
       c) or ignore if only TLS protocol related packets received.

This is a Measurement Factory project

diff --git a/src/client_side.cc b/src/client_side.cc
index 005876415c71f66d97d07bf3701750bd39dc2ef9..a5e511c792cd59112ca774cff5b24849e21fd97b 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -4744,6 +4744,46 @@ ConnStateData::stopPinnedConnectionMonitoring()
     }
 }
 
+#if USE_OPENSSL
+bool
+ConnStateData::handleIdleClientPinnedTlsRead()
+{
+    // A ready-for-reading connection means that the TLS server either closed
+    // the connection, sent us some unexpected HTTP data, or started TLS
+    // renegotiations. We should close the connection except for the last case.
+
+    Must(pinning.serverConnection != nullptr);
+    SSL *ssl = fd_table[pinning.serverConnection->fd].ssl;
+    if (!ssl)
+        return false;
+
+    char buf[1];
+    const int readResult = SSL_read(ssl, buf, sizeof(buf));
+
+    if (readResult > 0 || SSL_pending(ssl) > 0) {
+        debugs(83, 2, pinning.serverConnection << " TLS application data read");
+        return false;
+    }
+
+    switch(const int error = SSL_get_error(ssl, readResult)) {
+    case SSL_ERROR_WANT_WRITE:
+        debugs(83, DBG_IMPORTANT, pinning.serverConnection << " TLS SSL_ERROR_WANT_WRITE request for idle pinned connection");
+        // fall through to restart monitoring, for now
+    case SSL_ERROR_NONE:
+    case SSL_ERROR_WANT_READ:
+        startPinnedConnectionMonitoring();
+        return true;
+
+    default:
+        debugs(83, 2, pinning.serverConnection << " TLS error: " << error);
+        return false;
+    }
+
+    // not reached
+    return true;
+}
+#endif
+
 /// Our read handler called by Comm when the server either closes an idle pinned connection or
 /// perhaps unexpectedly sends something on that idle (from Squid p.o.v.) connection.
 void
@@ -4754,13 +4794,19 @@ ConnStateData::clientPinnedConnectionRead(const CommIoCbParams &io)
     if (io.flag == Comm::ERR_CLOSING)
         return; // close handler will clean up
 
+    Must(pinning.serverConnection == io.conn);
+
+#if USE_OPENSSL
+    if (handleIdleClientPinnedTlsRead())
+        return;
+#endif
+
     // We could use getConcurrentRequestCount(), but this may be faster.
     const bool clientIsIdle = !getCurrentContext();
 
     debugs(33, 3, "idle pinned " << pinning.serverConnection << " read " <<
            io.size << (clientIsIdle ? " with idle client" : ""));
 
-    assert(pinning.serverConnection == io.conn);
     pinning.serverConnection->close();
 
     // If we are still sending data to the client, do not close now. When we are done sending,

diff --git a/src/client_side.h b/src/client_side.h
index 743f827550f06244992d751d0adf764effc7f926..72e2120cf5b63bc1fce504592fb0f865ed100e2f 100644 (file)
--- a/src/client_side.h
+++ b/src/client_side.h
@@ -414,6 +414,12 @@ protected:
 
     void startPinnedConnectionMonitoring();
     void clientPinnedConnectionRead(const CommIoCbParams &io);
+#if USE_OPENSSL
+    /// Handles a ready-for-reading TLS squid-to-server connection that
+    /// we thought was idle.
+    /// \return false if and only if the connection should be closed.
+    bool handleIdleClientPinnedTlsRead();
+#endif
 
     /// parse input buffer prefix into a single transfer protocol request
     /// return NULL to request more header bytes (after checking any limits)