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 6a878b3d8828871459d88f2922f80978d8858f70..004097e787ed2ed449613b2bcfb2521a3970c787 100644 (file)
--- 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 e652a3493904ce57c15bef5079d40b9eecf9978a..cfd56df03819d486ebbe7d4fb8a0b13e6ddcd38d 100644 (file)
--- 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 7914ce86ce3aa88fe94184d1adbbfbae6b8796b5..96a8efecd3be57af1f322d4eb101a21a4c128f11 100644 (file)
--- 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 d4b58fb7a05599ca0cd8661408fedde05559069a..280229bbb41f2e31c2e9103b60a52d563f7ea1cc 100644 (file)
--- 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 913de9dcd9249161f14168452623e42fcbcf60ea..a6752b3336dcc9d271143fe5375444325806caca 100644 (file)
--- 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);