From: Christos Tsantilas <chtsanti@users.sourceforge.net>
Date: Mon, 8 Feb 2016 17:44:43 +0000 (+0200)
Subject: Fixed step3 splicing.
X-Git-Tag: SQUID_4_0_5~4
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=56753478d497d40d27dc8817d029fcd3aea63f53;p=thirdparty%2Fsquid.git

Fixed step3 splicing.

The information about PeekingPeerConnector splicing the connections
was lost in some cases, resulting in two different bugs:

 - With a certificate validator, the PeekingPeerConnector class calls
   back FwdState, which calls the ConnStateData class, which then tries
   secure the connection with the already tunneled SSL client and
   closes the connection on negotiating errors.

 - Without a certificate validator, the PeekingPeerConnector class
   never calls FwdState class, and both PeekingPeerConnector and
   FwdState objects stall until finishing tunnelState closes server
   and client connections.

Now, PeerConnector always calls FwdState back, marking spliced
connections as such. This has the following positive side-effects:

 - When FwdState learns about spliced connections, it does not call
   ConnStateData back. Instead, it terminates and gets destroyed.
   The tunnel continues uninterrupted.

 - The PeekingPeerConnector job ends and is destroyed instead of
   waiting to call FwdState.

This is a Measurement Factory project.
---

diff --git a/src/FwdState.cc b/src/FwdState.cc
index 6a878b3d88..004097e787 100644
--- a/src/FwdState.cc
+++ b/src/FwdState.cc
@@ -733,6 +733,16 @@ FwdState::connectedToPeer(Security::EncryptorAnswer &answer)
         return;
     }
 
+    if (answer.tunneled) {
+        // TODO: When ConnStateData establishes tunnels, its state changes
+        // [in ways that may affect logging?]. Consider informing
+        // ConnStateData about our tunnel or otherwise unifying tunnel
+        // establishment [side effects].
+        unregister(serverConn); // async call owns it now
+        complete(); // destroys us
+        return;
+    }
+
     // should reach ConnStateData before the dispatched Client job starts
     CallJobHere1(17, 4, request->clientConnectionManager, ConnStateData,
                  ConnStateData::notePeerConnection, serverConnection());
diff --git a/src/security/EncryptorAnswer.h b/src/security/EncryptorAnswer.h
index e652a34939..cfd56df038 100644
--- a/src/security/EncryptorAnswer.h
+++ b/src/security/EncryptorAnswer.h
@@ -21,12 +21,16 @@ namespace Security {
 class EncryptorAnswer
 {
 public:
+    EncryptorAnswer(): tunneled(false) {}
     ~EncryptorAnswer(); ///< deletes error if it is still set
     Comm::ConnectionPointer conn; ///< peer connection (secured on success)
 
     /// answer recepients must clear the error member in order to keep its info
     /// XXX: We should refcount ErrorState instead of cbdata-protecting it.
     CbcPointer<ErrorState> error; ///< problem details (nil on success)
+
+    /// whether we spliced the connections instead of negotiating encryption
+    bool tunneled;
 };
 
 std::ostream &operator <<(std::ostream &, const Security::EncryptorAnswer &);
diff --git a/src/ssl/PeekingPeerConnector.cc b/src/ssl/PeekingPeerConnector.cc
index 7914ce86ce..96a8efecd3 100644
--- a/src/ssl/PeekingPeerConnector.cc
+++ b/src/ssl/PeekingPeerConnector.cc
@@ -104,6 +104,7 @@ Ssl::PeekingPeerConnector::checkForPeekAndSpliceMatched(const Ssl::BumpMode acti
         // and if done, switch to tunnel mode
         if (sslFinalized()) {
             debugs(83,5, "Abort NegotiateSSL on FD " << serverConn->fd << " and splice the connection");
+            callBack();
         }
     }
 }
@@ -268,6 +269,7 @@ Ssl::PeekingPeerConnector::noteNegotiationDone(ErrorState *error)
             auto clientSsl = fd_table[clientConn->fd].ssl.get();
             clientConn->tlsNegotiations()->fillWith(clientSsl);
             switchToTunnel(request.getRaw(), clientConn, serverConn);
+            tunnelInsteadOfNegotiating();
         }
     }
 }
@@ -376,3 +378,14 @@ Ssl::PeekingPeerConnector::serverCertificateVerified()
     }
 }
 
+void
+Ssl::PeekingPeerConnector::tunnelInsteadOfNegotiating()
+{
+    Must(callback != NULL);
+    CbDialer *dialer = dynamic_cast<CbDialer*>(callback->getDialer());
+    Must(dialer);
+    dialer->answer().tunneled = true;
+    debugs(83, 5, "The SSL negotiation with server aborted");
+}
+
+
diff --git a/src/ssl/PeekingPeerConnector.h b/src/ssl/PeekingPeerConnector.h
index d4b58fb7a0..280229bbb4 100644
--- a/src/ssl/PeekingPeerConnector.h
+++ b/src/ssl/PeekingPeerConnector.h
@@ -68,6 +68,10 @@ public:
     static void cbCheckForPeekAndSpliceDone(allow_t answer, void *data);
 
 private:
+
+    /// Inform caller class that the SSL negotiation aborted
+    void tunnelInsteadOfNegotiating();
+
     Comm::ConnectionPointer clientConn; ///< TCP connection to the client
     AsyncCall::Pointer callback; ///< we call this with the results
     AsyncCall::Pointer closeHandler; ///< we call this when the connection closed
diff --git a/src/ssl/PeerConnector.h b/src/ssl/PeerConnector.h
index 913de9dcd9..a6752b3336 100644
--- a/src/ssl/PeerConnector.h
+++ b/src/ssl/PeerConnector.h
@@ -148,6 +148,10 @@ protected:
 
     void bail(ErrorState *error); ///< Return an error to the PeerConnector caller
 
+    /// Callback the caller class, and pass the ready to communicate secure
+    /// connection or an error if PeerConnector failed.
+    void callBack();
+
     /// If called the certificates validator will not used
     void bypassCertValidator() {useCertValidator_ = false;}
 
@@ -161,10 +165,6 @@ private:
     PeerConnector(const PeerConnector &); // not implemented
     PeerConnector &operator =(const PeerConnector &); // not implemented
 
-    /// Callback the caller class, and pass the ready to communicate secure
-    /// connection or an error if PeerConnector failed.
-    void callBack();
-
     /// Process response from cert validator helper
     void sslCrtvdHandleReply(Ssl::CertValidationResponsePointer);