Record SSL client and SSL server details (supported TLS version/requested

TLS version) for fast-sni branch

Currently the fast-SNI-peek branch does not parse SSL client hello in
Client-First and Server-First bumping modes. This patch always "peeks" the
SSL client hello message and apply the squid TLS parser for Client-First
and Server-First modes to.

Also this patch moves the code which retrieves SSL server details from
PeekingPeerConnector class to PeerConnector class t retrieve details  for all
SSL server side connections.

diff --git a/src/client_side.cc b/src/client_side.cc
index 38f8b2bada0af1b9190a01ba512953c2f84b4ce7..416f81e179ac263d8f56c8a83d12a12816cd8585 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -2196,9 +2196,7 @@ ConnStateData::afterClientRead()
             }
         }
         atTlsPeek = false;
-        Must(sslServerBump);
-        Must(sslServerBump->act.step1 == Ssl::bumpPeek || sslServerBump->act.step1 == Ssl::bumpStare);
-        startPeekAndSplice();
+        afterClientHelloPeeked();
         return;
     }
 #endif
@@ -3105,10 +3103,14 @@ ConnStateData::getSslContextDone(Security::ContextPtr sslContext, bool isNew)
                                      this, ConnStateData::requestTimeout);
     commSetConnTimeout(clientConnection, Config.Timeout.request, timeoutCall);
 
-    // Disable the client read handler until CachePeer selection is complete
-    Comm::SetSelect(clientConnection->fd, COMM_SELECT_READ, NULL, NULL, 0);
-    Comm::SetSelect(clientConnection->fd, COMM_SELECT_READ, clientNegotiateSSL, this, 0);
     switchedToHttps_ = true;
+
+    auto ssl = fd_table[clientConnection->fd].ssl.get();
+    BIO *b = SSL_get_rbio(ssl);
+    Ssl::ClientBio *bio = static_cast<Ssl::ClientBio *>(b->ptr);
+    bio->setReadBufData(inBuf);
+    inBuf.clear();
+    clientNegotiateSSL(clientConnection->fd, this);
 }
 
 void
@@ -3133,31 +3135,53 @@ ConnStateData::switchToHttps(HttpRequest *request, Ssl::BumpMode bumpServerMode)
     if (bumpServerMode == Ssl::bumpServerFirst && !sslServerBump) {
         request->flags.sslPeek = true;
         sslServerBump = new Ssl::ServerBump(request);
-
-        // will call httpsPeeked() with certificate and connection, eventually
-        FwdState::fwdStart(clientConnection, sslServerBump->entry, sslServerBump->request.getRaw());
-        return;
     } else if (bumpServerMode == Ssl::bumpPeek || bumpServerMode == Ssl::bumpStare) {
         request->flags.sslPeek = true;
         sslServerBump = new Ssl::ServerBump(request, NULL, bumpServerMode);
+    }
 
-        // commSetConnTimeout() was called for this request before we switched.
-        // Fix timeout to request_start_timeout
-        typedef CommCbMemFunT<ConnStateData, CommTimeoutCbParams> TimeoutDialer;
-        AsyncCall::Pointer timeoutCall =  JobCallback(33, 5,
-                                                      TimeoutDialer, this, ConnStateData::requestTimeout);
-        commSetConnTimeout(clientConnection, Config.Timeout.request_start_timeout, timeoutCall);
-        // Also reset receivedFirstByte_ flag to allow this timeout work in the case we have
-        // a bumbed "connect" request on non transparent port.
-        receivedFirstByte_ = false;
-        // Get more data to peek at Tls
-        atTlsPeek = true;
-        readSomeData();
+    // commSetConnTimeout() was called for this request before we switched.
+    // Fix timeout to request_start_timeout
+    typedef CommCbMemFunT<ConnStateData, CommTimeoutCbParams> TimeoutDialer;
+    AsyncCall::Pointer timeoutCall =  JobCallback(33, 5,
+                                                  TimeoutDialer, this, ConnStateData::requestTimeout);
+    commSetConnTimeout(clientConnection, Config.Timeout.request_start_timeout, timeoutCall);
+    // Also reset receivedFirstByte_ flag to allow this timeout work in the case we have
+    // a bumbed "connect" request on non transparent port.
+    receivedFirstByte_ = false;
+    // Get more data to peek at Tls
+    atTlsPeek = true;
+    readSomeData();
+}
+
+void
+ConnStateData::afterClientHelloPeeked()
+{
+    receivedFirstByte();
+
+    // Avoid the check for tlsParser error, in the case of bug in our tls parser.
+    // The bumpServerFirst and bumpClientFirst should not depend on tlsParser
+    // result. For the peek-and-splice mode, the tlsParser.parseError checked and
+    // handled inside startPeekAndsSplice method.
+
+    // Record parsed info
+    if (!tlsParser.parseError)
+        clientConnection->tlsNegotiations()->retrieveParsedInfo(tlsParser.details);
+
+    // We should disable read/write handlers
+    Comm::SetSelect(clientConnection->fd, COMM_SELECT_READ, NULL, NULL, 0);
+    Comm::SetSelect(clientConnection->fd, COMM_SELECT_WRITE, NULL, NULL, 0);
+
+    if (!sslServerBump) { // BumpClientFirst mode does not use this member
+        getSslContextStart();
         return;
+    } else if (sslServerBump->act.step1 == Ssl::bumpServerFirst) {
+        // will call httpsPeeked() with certificate and connection, eventually
+        FwdState::fwdStart(clientConnection, sslServerBump->entry, sslServerBump->request.getRaw());
+    } else {
+        Must(sslServerBump->act.step1 == Ssl::bumpPeek || sslServerBump->act.step1 == Ssl::bumpStare);
+        startPeekAndSplice();
     }
-
-    // otherwise, use sslConnectHostOrIp
-    getSslContextStart();
 }
 
 bool
@@ -3185,10 +3209,6 @@ void ConnStateData::startPeekAndSplice()
             clientConnection->close();
         return;
     }
-    receivedFirstByte();
-
-    // Record parsed info
-    clientConnection->tlsNegotiations()->retrieveParsedInfo(tlsParser.details);
 
     if (serverBump()) {
         Security::TlsDetails::Pointer const &details = tlsParser.details;
@@ -3198,10 +3218,6 @@ void ConnStateData::startPeekAndSplice()
         }
     }
 
-    // We should disable read/write handlers
-    Comm::SetSelect(clientConnection->fd, COMM_SELECT_READ, NULL, NULL, 0);
-    Comm::SetSelect(clientConnection->fd, COMM_SELECT_WRITE, NULL, NULL, 0);
-
     startPeekAndSpliceDone();
 }
 

diff --git a/src/client_side.h b/src/client_side.h
index d34f6b9ceb4b88de9618a4da06983f20abe5782e..ea83290ac3e163d909b8c4aa5eee83c6161e699c 100644 (file)
--- a/src/client_side.h
+++ b/src/client_side.h
@@ -235,6 +235,7 @@ public:
     void sslCrtdHandleReply(const Helper::Reply &reply);
 
     void switchToHttps(HttpRequest *request, Ssl::BumpMode bumpServerMode);
+    void afterClientHelloPeeked();
     bool switchedToHttps() const { return switchedToHttps_; }
     Ssl::ServerBump *serverBump() {return sslServerBump;}
     inline void setServerBump(Ssl::ServerBump *srvBump) {

diff --git a/src/ssl/PeekingPeerConnector.cc b/src/ssl/PeekingPeerConnector.cc
index 964b6a7b5dbaa78752404f83baa29f546349ec19..2b611bd5636ab62abf8661f359711a0fc4948270 100644 (file)
--- a/src/ssl/PeekingPeerConnector.cc
+++ b/src/ssl/PeekingPeerConnector.cc
@@ -252,14 +252,6 @@ Ssl::PeekingPeerConnector::noteNegotiationDone(ErrorState *error)
         }
     }
 
-    // retrieve TLS server negotiated information if any
-    serverConnection()->tlsNegotiations()->retrieveNegotiatedInfo(ssl);
-    // retrieve TLS parsed extra info
-    BIO *b = SSL_get_rbio(ssl);
-    Ssl::ServerBio *bio = static_cast<Ssl::ServerBio *>(b->ptr);
-    if (const Security::TlsDetails::Pointer &details = bio->receivedHelloDetails())
-        serverConnection()->tlsNegotiations()->retrieveParsedInfo(details);
-
     if (!error) {
         serverCertificateVerified();
         if (splice) {

diff --git a/src/ssl/PeerConnector.cc b/src/ssl/PeerConnector.cc
index 740745b3d7d9d7cb6b2454347e61bc9674ed2294..73973794b44abfbacd55debe910255f88c16456e 100644 (file)
--- a/src/ssl/PeerConnector.cc
+++ b/src/ssl/PeerConnector.cc
@@ -14,6 +14,7 @@
 #include "errorpage.h"
 #include "fde.h"
 #include "HttpRequest.h"
+#include "security/NegotiationHistory.h"
 #include "SquidConfig.h"
 #include "ssl/bio.h"
 #include "ssl/cert_validate_message.h"
@@ -134,6 +135,21 @@ Ssl::PeerConnector::setReadTimeout()
     commSetConnTimeout(serverConnection(), timeToRead, nil);
 }
 
+void
+Ssl::PeerConnector::recordNegotiationDetails()
+{
+    const int fd = serverConnection()->fd;
+    Security::SessionPtr ssl = fd_table[fd].ssl.get();
+
+    // retrieve TLS server negotiated information if any
+    serverConnection()->tlsNegotiations()->retrieveNegotiatedInfo(ssl);
+    // retrieve TLS parsed extra info
+    BIO *b = SSL_get_rbio(ssl);
+    Ssl::ServerBio *bio = static_cast<Ssl::ServerBio *>(b->ptr);
+    if (const Security::TlsDetails::Pointer &details = bio->receivedHelloDetails())
+        serverConnection()->tlsNegotiations()->retrieveParsedInfo(details);
+}
+
 void
 Ssl::PeerConnector::negotiateSsl()
 {
@@ -148,6 +164,8 @@ Ssl::PeerConnector::negotiateSsl()
         return; // we might be gone by now
     }
 
+    recordNegotiationDetails();
+
     if (!sslFinalized())
         return;
 
@@ -333,6 +351,9 @@ Ssl::PeerConnector::handleNegotiateError(const int ret)
         // no special error handling for all other errors
         break;
     }
+
+    // Log connections details if there is any
+    recordNegotiationDetails();
     noteSslNegotiationError(ret, ssl_error, ssl_lib_error);
 }
 

diff --git a/src/ssl/PeerConnector.h b/src/ssl/PeerConnector.h
index 7a8f481d715289996d148fb62fcb935c4d974a55..551c0b8560acfa77ffec1bca4c1735a24a318166 100644 (file)
--- a/src/ssl/PeerConnector.h
+++ b/src/ssl/PeerConnector.h
@@ -155,6 +155,10 @@ protected:
     /// If called the certificates validator will not used
     void bypassCertValidator() {useCertValidator_ = false;}
 
+    /// Called after negotiation finishes to record connection details for
+    /// logging
+    void recordNegotiationDetails();
+
     HttpRequestPointer request; ///< peer connection trigger or cause
     Comm::ConnectionPointer serverConn; ///< TCP connection to the peer
     AccessLogEntryPointer al; ///< info for the future access.log entry