Bug 4321: ssl_bump terminate does not terminate at step1

The following trivial configuration should terminate all connections that
are subject to SslBumping:
  ssl_bump terminate all
but Squid either splices or bumps instead.

This patch fixes Squid to immediately close the connection.

Also this patch:
 - solves wrong use of Ssl::bumpNone in cases where the Ssl::bumpEnd
   (do not bump) or Ssl::bumpSplice (splice after peek/stare at step1)
   must be used.
 - updates %ssl::bump_mode documetation.
 - fixes %ssl::bump_mode formating code to print last bumping action

This is a Measurement Factory project

diff --git a/src/cf.data.pre b/src/cf.data.pre
index 015fbb400310fd4094cb277f7cad34a221be3d3d..046fa47dd6fbcb069474fc0592fba683b583ef28 100644 (file)
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -4311,13 +4311,14 @@ DOC_START
                                For CONNECT requests that initiated bumping of
                                a connection and for any request received on
                                an already bumped connection, Squid logs the
-                               corresponding SslBump mode ("server-first" or
-                               "client-first"). See the ssl_bump option for
-                               more information about these modes.
+                               corresponding SslBump mode ("splice", "bump",
+                               "peek", "stare", "terminate", "server-first"
+                               or "client-first"). See the ssl_bump option 
+                               for more information about these modes.
 
                                A "none" token is logged for requests that
                                triggered "ssl_bump" ACL evaluation matching
-                               either a "none" rule or no rules at all.
+                               a "none" rule.
 
                                In all other cases, a single dash ("-") is
                                logged.

diff --git a/src/client_side.cc b/src/client_side.cc
index 70edf253498b577d55e5a6cdafdd2324e6c01782..cf1eb77bef7c1a90a4433bfdf5832983a006d1f2 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -2727,8 +2727,6 @@ httpsEstablish(ConnStateData *connState, const Security::ContextPointer &ctx)
 
 /**
  * A callback function to use with the ACLFilledChecklist callback.
- * In the case of ACCESS_ALLOWED answer initializes a bumped SSL connection,
- * else reverts the connection to tunnel mode.
  */
 static void
 httpsSslBumpAccessCheckDone(allow_t answer, void *data)
@@ -2739,15 +2737,19 @@ httpsSslBumpAccessCheckDone(allow_t answer, void *data)
     if (!connState->isOpen())
         return;
 
-    // Require both a match and a positive bump mode to work around exceptional
-    // cases where ACL code may return ACCESS_ALLOWED with zero answer.kind.
-    if (answer == ACCESS_ALLOWED && answer.kind != Ssl::bumpNone) {
-        debugs(33, 2, "sslBump needed for " << connState->clientConnection << " method " << answer.kind);
+    if (answer == ACCESS_ALLOWED) {
+        debugs(33, 2, "sslBump action " << Ssl::bumpMode(answer.kind) << "needed for " << connState->clientConnection);
         connState->sslBumpMode = static_cast<Ssl::BumpMode>(answer.kind);
     } else {
-        debugs(33, 2, HERE << "sslBump not needed for " << connState->clientConnection);
-        connState->sslBumpMode = Ssl::bumpNone;
+        debugs(33, 3, "sslBump not needed for " << connState->clientConnection);
+        connState->sslBumpMode = Ssl::bumpSplice;
+    }
+
+    if (connState->sslBumpMode == Ssl::bumpTerminate) {
+        connState->clientConnection->close();
+        return;
     }
+
     if (!connState->fakeAConnectRequest("ssl-bump", connState->inBuf))
         connState->clientConnection->close();
 }
@@ -3168,7 +3170,8 @@ ConnStateData::parseTlsHandshake()
         Must(context && context->http);
         HttpRequest::Pointer request = context->http->request;
         debugs(83, 5, "Got something other than TLS Client Hello. Cannot SslBump.");
-        sslBumpMode = Ssl::bumpNone;
+        sslBumpMode = Ssl::bumpSplice;
+        context->http->al->ssl.bumpMode = Ssl::bumpSplice;
         if (!clientTunnelOnError(this, context, request, HttpRequestMethod(), ERR_PROTOCOL_UNKNOWN))
             clientConnection->close();
         return;
@@ -3204,6 +3207,9 @@ void httpsSslBumpStep2AccessCheckDone(allow_t answer, void *data)
 
     connState->serverBump()->act.step2 = bumpAction;
     connState->sslBumpMode = bumpAction;
+    Http::StreamPointer context = connState->pipeline.front();
+    if (ClientHttpRequest *http = (context ? context->http : nullptr))
+        http->al->ssl.bumpMode = bumpAction;
 
     if (bumpAction == Ssl::bumpTerminate) {
         connState->clientConnection->close();

diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index 75804ac67bbd1361de67a6cf85b16bb6d3710bc1..782002c286345ca6a6a92b8eda92f773d293a8b4 100644 (file)
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -1499,10 +1499,19 @@ ClientRequestContext::sslBumpAccessCheckDone(const allow_t &answer)
         return;
 
     const Ssl::BumpMode bumpMode = answer == ACCESS_ALLOWED ?
-                                   static_cast<Ssl::BumpMode>(answer.kind) : Ssl::bumpNone;
+                                   static_cast<Ssl::BumpMode>(answer.kind) : Ssl::bumpSplice;
     http->sslBumpNeed(bumpMode); // for processRequest() to bump if needed
     http->al->ssl.bumpMode = bumpMode; // for logging
 
+    if (bumpMode == Ssl::bumpTerminate) {
+        const Comm::ConnectionPointer clientConn = http->getConn() ? http->getConn()->clientConnection : nullptr;
+        if (Comm::IsConnOpen(clientConn)) {
+            debugs(85, 3, "closing after Ssl::bumpTerminate ");
+            clientConn->close();
+        }
+        return;
+    }
+
     http->doCallouts();
 }
 #endif

diff --git a/src/ssl/PeekingPeerConnector.cc b/src/ssl/PeekingPeerConnector.cc
index 645e000b3e67a5bdcecbdd45fe24e8dd171a4659..1fbffd56a92a73a2aa66764d21abd0e03833d35a 100644 (file)
--- a/src/ssl/PeekingPeerConnector.cc
+++ b/src/ssl/PeekingPeerConnector.cc
@@ -88,6 +88,7 @@ Ssl::PeekingPeerConnector::checkForPeekAndSpliceMatched(const Ssl::BumpMode acti
         request->clientConnectionManager->sslBumpMode = finalAction;
         request->clientConnectionManager->serverBump()->act.step3 = finalAction;
     }
+    al->ssl.bumpMode = finalAction;
 
     if (finalAction == Ssl::bumpTerminate) {
         serverConn->close();