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.

diff --git a/src/client_side.cc b/src/client_side.cc
index 03b1b04045995f2cabf7c0c405791d41dc3fe91f..298828d42a8cd3a76f739268aad33c798f3e71ba 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -3681,26 +3681,25 @@ httpsSslBumpAccessCheckDone(allow_t answer, void *data)
     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<Ssl::BumpMode>(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();
     }
 }
 

diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index f9f6801a982383f353907a9544328e950a9cc519..ff136c0ce995b900c742244a709401a7445e8d3e 100644 (file)
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -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"
@@ -1413,6 +1414,7 @@ ClientRequestContext::sslBumpAccessCheck()
     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;
     }
@@ -1568,12 +1570,21 @@ ClientHttpRequest::sslBumpStart()
            "-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 &params = GetCommParams<CommIoCbParams>(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
@@ -1777,6 +1788,8 @@ ClientHttpRequest::doCallouts()
         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);