From: Christos Tsantilas Date: Thu, 25 May 2017 11:47:23 +0000 (+1200) Subject: Bug 4321: ssl_bump terminate does not terminate at step1 X-Git-Tag: SQUID_4_0_20~15 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=409d9a832397a80adb21f3f59398ff48c381e6d0;p=thirdparty%2Fsquid.git 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 015fbb4003..046fa47dd6 100644 --- 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 70edf25349..cf1eb77bef 100644 --- 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(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 75804ac67b..782002c286 100644 --- 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(answer.kind) : Ssl::bumpNone; + static_cast(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 645e000b3e..1fbffd56a9 100644 --- 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();