From: Christos Tsantilas <chtsanti@users.sourceforge.net>
Date: Mon, 8 May 2017 05:40:15 +0000 (+1200)
Subject: SSL-Bump: Second adaptation missing for CONNECTs
X-Git-Tag: SQUID_4_0_20~21
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=f797c7ce73a102ed6ada75dc2dbda2b5a49e1872;p=thirdparty%2Fsquid.git

SSL-Bump: Second adaptation missing for CONNECTs

Squid does not send CONNECT request to adaptation services
if the "ssl_bump splice" rule matched at step 2. This adaptation
is important because the CONNECT request gains SNI information during
the second SslBump step. This is a regression bug, possibly caused by
the Squid bug 4529 fix (trunk commits r14913 and r14914).

This is a Measurement Factory project.
---

diff --git a/src/client_side.cc b/src/client_side.cc
index bba6dfe007..70edf25349 100644
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -3145,8 +3145,9 @@ ConnStateData::parseTlsHandshake()
 
     parsingTlsHandshake = false;
 
-    if (mayTunnelUnsupportedProto())
-        preservedClientData = inBuf;
+    // client data may be needed for splicing and for
+    // tunneling unsupportedProtocol after an error
+    preservedClientData = inBuf;
 
     // Even if the parser failed, each TLS detail should either be set
     // correctly or still be "unknown"; copying unknown detail is a no-op.
@@ -3228,9 +3229,23 @@ ConnStateData::splice()
     transferProtocol = Http::ProtocolVersion();
     assert(!pipeline.empty());
     Http::StreamPointer context = pipeline.front();
+    Must(context);
+    Must(context->http);
     ClientHttpRequest *http = context->http;
-    tunnelStart(http);
-    return true;
+    HttpRequest::Pointer request = http->request;
+    context->finished();
+    if (transparent()) {
+        // For transparent connections, make a new fake CONNECT request, now
+        // with SNI as target. doCallout() checks, adaptations may need that.
+        return fakeAConnectRequest("splice", preservedClientData);
+    } else {
+        // For non transparent connections  make a new tunneled CONNECT, which
+        // also sets the HttpRequest::flags::forceTunnel flag to avoid
+        // respond with "Connection Established" to the client.
+        // This fake CONNECT request required to allow use of SNI in
+        // doCallout() checks and adaptations.
+        return initiateTunneledRequest(request, Http::METHOD_CONNECT, "splice", preservedClientData);
+    }
 }
 
 void
diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index d32500bb43..5133c1d302 100644
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -1415,13 +1415,15 @@ ClientRequestContext::sslBumpAccessCheck()
         return false;
     }
 
+    const Ssl::BumpMode bumpMode = http->getConn()->sslBumpMode;
     if (http->request->flags.forceTunnel) {
         debugs(85, 5, "not needed; already decided to tunnel " << http->getConn());
+        if (bumpMode != Ssl::bumpEnd)
+            http->al->ssl.bumpMode = bumpMode; // inherited from bumped connection
         return false;
     }
 
     // If SSL connection tunneling or bumping decision has been made, obey it.
-    const Ssl::BumpMode bumpMode = http->getConn()->sslBumpMode;
     if (bumpMode != Ssl::bumpEnd) {
         debugs(85, 5, HERE << "SslBump already decided (" << bumpMode <<
                "), " << "ignoring ssl_bump for " << http->getConn());