TLS: 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 6fde62adef58d938ef860f1ba5d5c26e01517984..5cb2259f16003d7e4066cec73ce4bf4459f687cd 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -4993,6 +4993,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 != NULL);
+    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
@@ -5003,13 +5043,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 25ae9a400effebfe675fe4db47ad0ffb1df6ebc9..d441750c5fc43972884861ab522dc9b28ccb5b48 100644 (file)
--- a/src/client_side.h
+++ b/src/client_side.h
@@ -416,6 +416,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)