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 f918e84d1ba8be87ade37fae46fc6c0974c94829..78f96a901e0c89a64be1228140306cfec20933ca 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -3146,8 +3146,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.
@@ -3229,9 +3230,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 d2940097fafa5345285e83e9888bcd64c83f42cf..9356429505a04018305fe7c9227b5dd296122006 100644 (file)
--- 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());