------------------------------------------------------------ revno: 13693 revision-id: squid3@treenet.co.nz-20141218125858-szq763plofwyl65u parent: squid3@treenet.co.nz-20141218125702-dmzpopynf9mk570o author: Christos Tsantilas committer: Amos Jeffries branch nick: 3.5 timestamp: Thu 2014-12-18 04:58:58 -0800 message: Support http_access denials of SslBump "peeked" connections. If an SSL connection is "peeked", it is currently not possible to deny it with http_access. For example, the following configuration denies all plain HTTP requests as expected but allows all CONNECTs (and all subsequent encrypted/spliced HTTPS requests inside the allowed CONNECT tunnels): http_access deny all ssl_bump peek all ssl_bump splice all The bug results in insecure bumping configurations and/or forces admins to abuse ssl_bump directive (during step1 of bumping) for access control (as a partial workaround). This change sends all SSL tunnels (CONNECT and transparent) through http_access (and adaptation, etc.) checks during bumping step1. If (real or fake) CONNECT is denied during step1, then Squid does not connect to the SSL server, but bumps the client connection, and then delivers an error page (in response to the first decrypted GET). The behavior is similar to what Squid has already been doing for server certificate validation errors. Technical notes ---------------- Before these changes: * When a transparent SSL connection is being bumped, if we decide to splice during step1, then we splice the connections without any http_access checks. The (spliced) connection is always established. * When a CONNECT tunnel is being bumped at step1, if peek/stare/server-first mode is selected, and our http_access check fails, then: 1) We create an error page and proceeding with SSL bump, expecting to serve the error after the client SSL connection is negotiated. 2) We start forwarding SSL Hello to the server, to peek/stare at (or server-first bump) the server connection. 3) If we then decide to splice the connection during step2 or step3, then we splice, and the error page never gets served to the client! After these changes: * During transparent SSL bumping, if we decide to splice at step1, do not splice the connection immediately, but create a fake CONNECT request first and send it through the callout code (http_access check, ICAP/ECAP, etc.). If that fake CONNECT is denied, the code path described below kicks in. * When an error page is generated during CONNECT or transparent bumping (e.g. because an http_access check has failed), we switch to the "client-first" bumping mode and then serve the error page to the client (upon receiving the first regular request on the bumped connection). This is a Measurement Factory project. ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: squid3@treenet.co.nz-20141218125858-szq763plofwyl65u # target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 # testament_sha1: 27e20468a6c6ee33948b90bd347c3042999e41df # timestamp: 2014-12-18 13:51:00 +0000 # source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 # base_revision_id: squid3@treenet.co.nz-20141218125702-\ # dmzpopynf9mk570o # # Begin patch === modified file 'src/client_side.cc' --- src/client_side.cc 2014-12-09 10:18:36 +0000 +++ src/client_side.cc 2014-12-18 12:58:58 +0000 @@ -3859,26 +3859,25 @@ if (answer == ACCESS_ALLOWED && (answer.kind != Ssl::bumpNone && answer.kind != Ssl::bumpSplice)) { debugs(33, 2, "sslBump needed for " << connState->clientConnection << " method " << answer.kind); connState->sslBumpMode = static_cast(answer.kind); - httpsEstablish(connState, NULL, (Ssl::BumpMode)answer.kind); } else { debugs(33, 2, HERE << "sslBump not needed for " << connState->clientConnection); connState->sslBumpMode = Ssl::bumpNone; - - // fake a CONNECT request to force connState to tunnel - static char ip[MAX_IPSTRLEN]; - connState->clientConnection->local.toUrl(ip, sizeof(ip)); - // Pre-pend this fake request to the TLS bits already in the buffer - SBuf retStr; - retStr.append("CONNECT ").append(ip).append(" HTTP/1.1\r\nHost: ").append(ip).append("\r\n\r\n"); - connState->in.buf = retStr.append(connState->in.buf); - bool ret = connState->handleReadData(); - if (ret) - ret = connState->clientParseRequests(); - - if (!ret) { - debugs(33, 2, HERE << "Failed to start fake CONNECT request for ssl bumped connection: " << connState->clientConnection); - connState->clientConnection->close(); - } + } + + // fake a CONNECT request to force connState to tunnel + static char ip[MAX_IPSTRLEN]; + connState->clientConnection->local.toUrl(ip, sizeof(ip)); + // Pre-pend this fake request to the TLS bits already in the buffer + SBuf retStr; + retStr.append("CONNECT ").append(ip).append(" HTTP/1.1\r\nHost: ").append(ip).append("\r\n\r\n"); + connState->in.buf = retStr.append(connState->in.buf); + bool ret = connState->handleReadData(); + if (ret) + ret = connState->clientParseRequests(); + + if (!ret) { + debugs(33, 2, "Failed to start fake CONNECT request for SSL bumped connection: " << connState->clientConnection); + connState->clientConnection->close(); } } === modified file 'src/client_side_request.cc' --- src/client_side_request.cc 2014-09-25 10:34:22 +0000 +++ src/client_side_request.cc 2014-12-18 12:58:58 +0000 @@ -20,6 +20,7 @@ #include "acl/FilledChecklist.h" #include "acl/Gadgets.h" #include "anyp/PortCfg.h" +#include "base/AsyncJobCalls.h" #include "client_side.h" #include "client_side_reply.h" #include "client_side_request.h" @@ -1417,6 +1418,7 @@ if (bumpMode != Ssl::bumpEnd) { debugs(85, 5, HERE << "SslBump already decided (" << bumpMode << "), " << "ignoring ssl_bump for " << http->getConn()); + http->sslBumpNeed(bumpMode); // for processRequest() to bump if needed http->al->ssl.bumpMode = bumpMode; // inherited from bumped connection return false; } @@ -1572,12 +1574,21 @@ "-bumped CONNECT tunnel on FD " << getConn()->clientConnection); getConn()->sslBumpMode = sslBumpNeed_; + AsyncCall::Pointer bumpCall = commCbCall(85, 5, "ClientSocketContext::sslBumpEstablish", + CommIoCbPtrFun(&SslBumpEstablish, this)); + + if (request->flags.interceptTproxy || request->flags.intercepted) { + CommIoCbParams ¶ms = GetCommParams(bumpCall); + params.flag = Comm::OK; + params.conn = getConn()->clientConnection; + ScheduleCallHere(bumpCall); + return; + } + // send an HTTP 200 response to kick client SSL negotiation // TODO: Unify with tunnel.cc and add a Server(?) header static const char *const conn_established = "HTTP/1.1 200 Connection established\r\n\r\n"; - AsyncCall::Pointer call = commCbCall(85, 5, "ClientSocketContext::sslBumpEstablish", - CommIoCbPtrFun(&SslBumpEstablish, this)); - Comm::Write(getConn()->clientConnection, conn_established, strlen(conn_established), call, NULL); + Comm::Write(getConn()->clientConnection, conn_established, strlen(conn_established), bumpCall, NULL); } #endif @@ -1781,6 +1792,8 @@ StoreEntry *e= storeCreateEntry(storeUri, storeUri, request->flags, request->method); #if USE_OPENSSL if (sslBumpNeeded()) { + // We have to serve an error, so bump the client first. + sslBumpNeed(Ssl::bumpClientFirst); // set final error but delay sending until we bump Ssl::ServerBump *srvBump = new Ssl::ServerBump(request, e); errorAppendEntry(e, calloutContext->error);