]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Peek at the origin server SSL certificate when bumping intercepted HTTPS.
authorAlex Rousskov <rousskov@measurement-factory.com>
Wed, 14 Dec 2011 18:24:29 +0000 (11:24 -0700)
committerAlex Rousskov <rousskov@measurement-factory.com>
Wed, 14 Dec 2011 18:24:29 +0000 (11:24 -0700)
* Configuration changes:

Allow intercepted SSL connections to be bumped, in addition to the
tproxied SSL connections.

Honor and check ssl-bump flag in https_port. Earlier code apparently
assumed that the flag must be present in http_port and left
Ssl::TheGlobalContextStorage uninitialized if only https_port had the
flag.

* Client-side changes:

Added a new Ssl::ServerPeeker class to do client-side error handling
while peeking at the origin server certificate. Peeking is done at the
server-side. Server-side uses Store and store_client API to report
errors to the client-side. That works OK when the errors can be sent to
the client, but when we bump intercepted connections, the client does
not yet have a secure connection established with Squid so errors cannot
be sent (popular browsers do not display CONNECT-stage errors). Instead,
the errors must be accumulated and sent after the secure connection with
the client is established (in response to the first HTTP request on that
connection). Ssl::ServerPeeker needs work to support such accumulation.

Start Ssl::ServerPeeker job in ConnStateData::switchToHttps() and wait
for somebody to call the new ConnStateData::httpsPeeked() method back.
Needs more work to actually use the peeked certificate and handle
errors.

Changed ConnStateData::switchToHttps() profile to require destination
port number.  Without it, we cannot switch intercepted SSL connections
because they do not have a proper request structure that can supply port
details.

Polished pinned connection cleaning code: If our Comm close handler for
the pinned connection was called, do no try to remove the handler. If
the pinned connection was closed (e.g., by a server-side error), do not
try to close it again. If we already called unpinConnection(), do not
try to close the pinned connection again.

Do not assume we have a request when pinning a connection to the server.
Intercepted connections do not have requests at the connection pinning
stage.

* Server-side changes:

Bug 3243 (CVE 2009-0801: Bypass of browser same-origin access control in
intercepted communication) fix always created a new connection to the
origin server. I think it is safe (and possibly even safer!) to reuse a
pinned connection instead (if one is available). We now do that in the
new FwdState::selectPeerForIntercepted() method. If bump-server-first
does not reuse a pinned connection (left from certificate peeking),
Squid would be opening and closing to-server connections just to learn
the certificate, which is not kosher.

Added a new internal protocol type (PROTO_SSL_PEEK) to allow FwdState to
detect peeking requests and end processing after the certificate is
received (instead of proceeding with to httpStart()).

src/anyp/ProtocolType.h
src/client_side.cc
src/client_side.h
src/client_side_request.cc
src/forward.cc
src/forward.h
src/ssl/Makefile.am
src/ssl/ServerPeeker.cc [new file with mode: 0644]
src/ssl/ServerPeeker.h [new file with mode: 0644]

index 8a43b39fadc18ed7150f49e7e75ccbf94928133f..a3b4b2c6108da36001ee079c319934ac1310948a 100644 (file)
@@ -27,8 +27,11 @@ typedef enum {
 #endif
     PROTO_URN,
     PROTO_WHOIS,
-    PROTO_INTERNAL,
+    PROTO_INTERNAL, // miss on an internal object such as an icon
     PROTO_ICY,
+#if USE_SSL
+    PROTO_SSL_PEEK, // an internal request to peek at an HTTPS server
+#endif
     PROTO_UNKNOWN,
     PROTO_MAX
 } ProtocolType;
index 84bd32334ead8d4a71475d0071e2011a0bd0d481..636455d1661ae3cd11eaa24eb4fbd474ac51f95e 100644 (file)
 #if USE_SSL
 #include "ssl/context_storage.h"
 #include "ssl/helper.h"
+#include "ssl/ServerPeeker.h"
 #include "ssl/support.h"
 #include "ssl/gadgets.h"
 #endif
@@ -3435,7 +3436,7 @@ httpsEstablish(ConnStateData *connState,  SSL_CTX *sslContext)
     else {
         char buf[MAX_IPSTRLEN];
         debugs(33, 4, HERE << details << " try to generate a Dynamic SSL CTX");
-        connState->switchToHttps(details->local.NtoA(buf, sizeof(buf)));
+        connState->switchToHttps(details->local.NtoA(buf, sizeof(buf)), details->local.GetPort());
     }
 }
 
@@ -3511,7 +3512,7 @@ httpsAccept(const CommAcceptCbParams &params)
         // using tproxy-provided destination IP and port.
         HttpRequest *request = new HttpRequest();
         static char ip[MAX_IPSTRLEN];
-        assert(params.conn->flags & COMM_TRANSPARENT);
+        assert(params.conn->flags & (COMM_TRANSPARENT | COMM_INTERCEPTION));
         request->SetHost(params.conn->local.NtoA(ip, sizeof(ip)));
         request->port = params.conn->local.GetPort();
         request->myportname = s->name;
@@ -3644,7 +3645,7 @@ ConnStateData::getSslContextDone(SSL_CTX * sslContext, bool isNew)
 }
 
 void
-ConnStateData::switchToHttps(const char *host)
+ConnStateData::switchToHttps(const char *host, const int port)
 {
     assert(!switchedToHttps_);
 
@@ -3655,10 +3656,36 @@ ConnStateData::switchToHttps(const char *host)
     freeAllContexts();
     //currentobject->connIsFinished();
 
+    /* careful: freeAllContexts() above frees request, host, etc. */
+
     // We are going to read new request
     flags.readMore = true;
     debugs(33, 5, HERE << "converting " << clientConnection << " to SSL");
 
+    const bool alwaysBumpServerFirst = true;
+    if (alwaysBumpServerFirst) {
+        Must(!httpsPeeker.set());
+        httpsPeeker = AsyncJob::Start(new Ssl::ServerPeeker(
+                                      this, sslHostName.termedBuf(), port));
+        // will call httpsPeeked() with certificate and connection, eventually
+        return;
+    }
+
+    // otherwise, use sslHostName
+    getSslContextStart();
+}
+
+void
+ConnStateData::httpsPeeked(Comm::ConnectionPointer serverConnection)
+{
+    Must(httpsPeeker.set());
+    // XXX: handle httpsPeeker errors
+    
+    pinConnection(serverConnection, NULL, NULL, false);
+
+    // XXX: change sslHostName based on httpsPeeker results
+    debugs(33, 5, HERE << "bumped HTTPS server: " << sslHostName);
+    httpsPeeker.clear();
     getSslContextStart();
 }
 
@@ -3759,6 +3786,22 @@ clientHttpsConnectionsOpen(void)
             continue;
         }
 
+        // TODO: merge with similar code in clientHttpConnectionsOpen()
+        if (s->sslBump && !Config.accessList.ssl_bump) {
+            debugs(33, DBG_IMPORTANT, "WARNING: No ssl_bump configured. Disabling ssl-bump on " << s->protocol << "_port " << s->http.s);
+            s->sslBump = 0;
+        }
+
+        if (s->sslBump && !s->staticSslContext && !s->generateHostCertificates) {
+            debugs(1, DBG_IMPORTANT, "Will not bump SSL at http_port " << s->http.s << " due to SSL initialization failure.");
+            s->sslBump = 0;
+        }
+
+        if (s->sslBump) {
+            // Create ssl_ctx cache for this port.
+            Ssl::TheGlobalContextStorage.addLocalStorage(s->s, s->dynamicCertMemCacheSize == std::numeric_limits<size_t>::max() ? 4194304 : s->dynamicCertMemCacheSize);
+        }
+
         // Fill out a Comm::Connection which IPC will open as a listener for us
         s->http.listenConn = new Comm::Connection;
         s->http.listenConn->local = s->http.s;
@@ -4099,10 +4142,11 @@ ConnStateData::sendControlMsg(HttpControlMsg msg)
     clientConnection->close();
 }
 
-/* This is a comm call normally scheduled by comm_close() */
+/// Our close handler called by Comm when the pinned connection is closed
 void
 ConnStateData::clientPinnedConnectionClosed(const CommCloseCbParams &io)
 {
+    pinning.closeHandler = NULL; // Comm unregisters handlers before calling
     unpinConnection();
 }
 
@@ -4114,22 +4158,30 @@ ConnStateData::pinConnection(const Comm::ConnectionPointer &pinServer, HttpReque
     if (Comm::IsConnOpen(pinning.serverConnection)) {
         if (pinning.serverConnection->fd == pinServer->fd)
             return;
+    }
 
-        unpinConnection(); // clears fields ready for re-use. Prevent close() scheduling our close handler.
-        pinning.serverConnection->close();
-    } else
-        unpinConnection(); // clears fields ready for re-use.
+    unpinConnection(); // closes pinned connection, if any, and resets fields.
 
     pinning.serverConnection = pinServer;
-    pinning.host = xstrdup(request->GetHost());
-    pinning.port = request->port;
+
+    // when pinning an SSL bumped connection, the request may be NULL
+    const char *pinnedHost = "[unknown]";
+    if (request) {
+        pinning.host = xstrdup(request->GetHost());
+        pinning.port = request->port;
+        pinnedHost = pinning.host;
+    } else {
+        pinning.port = pinServer->remote.GetPort();
+    }
     pinning.pinned = true;
     if (aPeer)
         pinning.peer = cbdataReference(aPeer);
     pinning.auth = auth;
     char stmp[MAX_IPSTRLEN];
     snprintf(desc, FD_DESC_SZ, "%s pinned connection for %s (%d)",
-             (auth || !aPeer) ? request->GetHost() : aPeer->name, clientConnection->remote.ToURL(stmp,MAX_IPSTRLEN), clientConnection->fd);
+             (auth || !aPeer) ? pinnedHost : aPeer->name,
+             clientConnection->remote.ToURL(stmp,MAX_IPSTRLEN),
+             clientConnection->fd);
     fd_note(pinning.serverConnection->fd, desc);
 
     typedef CommCbMemFunT<ConnStateData, CommCloseCbParams> Dialer;
@@ -4171,12 +4223,16 @@ ConnStateData::unpinConnection()
     if (pinning.peer)
         cbdataReferenceDone(pinning.peer);
 
+    if (Comm::IsConnOpen(pinning.serverConnection)) {
     if (pinning.closeHandler != NULL) {
         comm_remove_close_handler(pinning.serverConnection->fd, pinning.closeHandler);
         pinning.closeHandler = NULL;
     }
     /// also close the server side socket, we should not use it for any future requests...
+    // TODO: do not close if called from our close handler?
     pinning.serverConnection->close();
+    }
+
     safe_free(pinning.host);
 
     /* NOTE: pinning.pinned should be kept. This combined with fd == -1 at the end of a request indicates that the host
index f0960f138d866b10f065c56a10eba8b78f954b5b..295bc4c9f8b374aa3158c685e315fd5dd3e70867 100644 (file)
@@ -306,6 +306,9 @@ public:
     virtual void swanSong();
 
 #if USE_SSL
+    /// called by Ssl::ServerPeeker when it is done bumping the server
+    void httpsPeeked(Comm::ConnectionPointer serverConnection);
+
     /// Start to create dynamic SSL_CTX for host or uses static port SSL context.
     void getSslContextStart();
     /**
@@ -319,7 +322,7 @@ public:
     /// Proccess response from ssl_crtd.
     void sslCrtdHandleReply(const char * reply);
 
-    void switchToHttps(const char *host);
+    void switchToHttps(const char *host, const int port);
     bool switchedToHttps() const { return switchedToHttps_; }
 #else
     bool switchedToHttps() const { return false; }
@@ -343,8 +346,14 @@ private:
     CBDATA_CLASS2(ConnStateData);
     bool closing_;
 
+#if USE_SSL
     bool switchedToHttps_;
     String sslHostName; ///< Host name for SSL certificate generation
+
+    /// a job that connects to the HTTPS server to get its SSL certificate
+    AsyncJob::Pointer httpsPeeker;
+#endif
+
     AsyncCall::Pointer reader; ///< set when we are reading
     BodyPipe::Pointer bodyPipe; // set when we are reading request body
 };
index 27b8e842d7339d3fd6541441472f4b43e210c2ce..ef6640f73c3cf17f0ae332f4198532cb03e6e269 100644 (file)
@@ -1373,7 +1373,7 @@ ClientHttpRequest::sslBumpEstablish(comm_err_t errflag)
         return;
     }
 
-    getConn()->switchToHttps(request->GetHost());
+    getConn()->switchToHttps(request->GetHost(), request->port);
 }
 
 void
index 56f3c65471cdf70e7533e00e588469d9566cfb63..449124a10e61df9b6b9e63c33b1753a0959224f2 100644 (file)
@@ -122,11 +122,7 @@ void FwdState::start(Pointer aSelf)
     // To resolve this we must force DIRECT and only to the original client destination.
     if (Config.onoff.client_dst_passthru && request && !request->flags.redirected &&
             (request->flags.intercepted || request->flags.spoof_client_ip)) {
-        Comm::ConnectionPointer p = new Comm::Connection();
-        p->remote = clientConn->local;
-        p->peerType = ORIGINAL_DST;
-        getOutgoingAddress(request, p);
-        serverDestinations.push_back(p);
+        selectPeerForIntercepted();
 
         // destination "found". continue with the forwarding.
         startConnectionOrFail();
@@ -136,6 +132,31 @@ void FwdState::start(Pointer aSelf)
     }
 }
 
+/// bypasses peerSelect() when dealing with intercepted requests
+void
+FwdState::selectPeerForIntercepted()
+{
+    // use pinned connection if available
+    Comm::ConnectionPointer p;
+    if (ConnStateData *client = request->pinnedConnection())
+        p = client->validatePinnedConnection(request, NULL);
+
+    if (p != NULL && Comm::IsConnOpen(p)) {
+        debugs(17, 3, HERE << "reusing a pinned conn: " << *p);
+        /* duplicate peerSelectPinned() effects */
+        p->peerType = PINNED;
+        entry->ping_status = PING_DONE;     /* Skip ICP */
+    } else {
+        debugs(17, 3, HERE << "opening a new conn: " << *p);
+        p = new Comm::Connection();
+        p->peerType = ORIGINAL_DST;
+        p->remote = clientConn->local;
+        getOutgoingAddress(request, p);
+    }
+
+    serverDestinations.push_back(p);
+}
+
 void
 FwdState::completed()
 {
@@ -755,7 +776,8 @@ FwdState::connectDone(const Comm::ConnectionPointer &conn, comm_err_t status, in
 
 #if USE_SSL
     if ((serverConnection()->getPeer() && serverConnection()->getPeer()->use_ssl) ||
-            (!serverConnection()->getPeer() && request->protocol == AnyP::PROTO_HTTPS)) {
+            (!serverConnection()->getPeer() &&
+                (request->protocol == AnyP::PROTO_HTTPS || request->protocol == AnyP::PROTO_SSL_PEEK))) {
         initiateSSL();
         return;
     }
@@ -959,6 +981,16 @@ FwdState::dispatch()
     }
 #endif
 
+#if USE_SSL
+    if (request->protocol == AnyP::PROTO_SSL_PEEK) {
+        CallJobHere1(17, 4, request->clientConnectionManager, ConnStateData,
+                    ConnStateData::httpsPeeked, serverConnection());
+        unregister(serverConn); // async call owns it now
+        complete(); // destroys us
+        return;
+       }
+#endif
+
     if (serverConnection()->getPeer() != NULL) {
         serverConnection()->getPeer()->stats.fetches++;
         request->peer_login = serverConnection()->getPeer()->login;
@@ -992,6 +1024,10 @@ FwdState::dispatch()
 
         case AnyP::PROTO_INTERNAL:
 
+#if USE_SSL
+        case AnyP::PROTO_SSL_PEEK:
+#endif
+
         case AnyP::PROTO_URN:
             fatal_dump("Should never get here");
             break;
index 01cd931f21e56abffae8b041f5bf6de0ac7de455..505eff5df844e51469c6d03dcfc8a4df3c91d342 100644 (file)
@@ -69,6 +69,7 @@ private:
     FwdState(const Comm::ConnectionPointer &client, StoreEntry *, HttpRequest *);
     void start(Pointer aSelf);
 
+    void selectPeerForIntercepted();
     static void logReplyStatus(int tries, http_status status);
     void doneWithRetries();
     void completed();
index e73e9db05902dbcc0718df2434c5d6cae767e553..4cde18b11ef4c2f327cd897cfc2e2f26d906fef9 100644 (file)
@@ -30,6 +30,8 @@ libsslsquid_la_SOURCES = \
        ErrorDetail.h \
        ErrorDetailManager.cc \
        ErrorDetailManager.h \
+       ServerPeeker.cc \
+       ServerPeeker.h \
        support.cc \
        support.h
 
diff --git a/src/ssl/ServerPeeker.cc b/src/ssl/ServerPeeker.cc
new file mode 100644 (file)
index 0000000..21c04c2
--- /dev/null
@@ -0,0 +1,59 @@
+/*
+ * $Id$
+ *
+ * DEBUG: section 33    Client-side Routines
+ *
+ */
+
+#include "config.h"
+
+#include "client_side.h"
+#include "forward.h"
+#include "ssl/ServerPeeker.h"
+#include "Store.h"
+
+
+CBDATA_NAMESPACED_CLASS_INIT(Ssl, ServerPeeker);
+
+
+Ssl::ServerPeeker::ServerPeeker(ConnStateData *anInitiator,
+    const char *host, const int port):
+    AsyncJob("Ssl::ServerPeeker"),
+    initiator(anInitiator),
+    clientConnection(anInitiator->clientConnection),
+    request(new HttpRequest),
+    entry(NULL)
+{
+    debugs(33, 4, HERE << "will peek at " << host << ':' << port);
+
+    request->SetHost(host);
+    request->port = port;
+    request->protocol = AnyP::PROTO_SSL_PEEK;
+    request->clientConnectionManager = initiator;
+}
+
+void
+Ssl::ServerPeeker::start()
+{
+    const char *uri = urlCanonical(request);
+    entry = storeCreateEntry(uri, uri, request->flags, request->method);
+
+    FwdState::fwdStart(clientConnection, entry, request);
+
+    // XXX: wait for FwdState to tell us the connection is ready
+
+    // TODO: send our answer to the initiator
+    // CallJobHere(33, 4, initiator, ConnStateData, ConnStateData::httpsPeeked);
+    initiator.clear(); // will trigger the end of the job
+}
+
+bool
+Ssl::ServerPeeker::doneAll() const
+{
+    return !initiator.valid() && AsyncJob::doneAll();
+}
+
+void
+Ssl::ServerPeeker::swanSong()
+{
+}
diff --git a/src/ssl/ServerPeeker.h b/src/ssl/ServerPeeker.h
new file mode 100644 (file)
index 0000000..e0e4620
--- /dev/null
@@ -0,0 +1,57 @@
+#ifndef _SQUID_SSL_PEEKER_H
+#define _SQUID_SSL_PEEKER_H
+
+#include "base/AsyncJob.h"
+#include "base/CbcPointer.h"
+#include "comm/forward.h"
+#include "HttpRequest.h"
+#include "ip/Address.h"
+
+class ConnStateData;
+
+namespace Ssl
+{
+
+/**
+  \ingroup ServerProtocolSSLAPI
+ * A job to facilitate connecting to the HTTPS server to learn its certificate.
+ *
+ * The Peeker job calls FwdState::fwdStart(). There are two possible outcomes:
+ *
+ * Success: FwdState calls ConnStateData which pins the establihsed connection
+ *          for future bumped HTTP requests (TODO: and stops this job).
+ *    
+ * Error: FwdState Stores the error (TODO: and this job preserves it for
+ *        for serving to the client in response to the first bumped request).
+ */
+class ServerPeeker: public AsyncJob
+{
+public:
+    typedef CbcPointer<ServerPeeker> Pointer;
+
+    explicit ServerPeeker(ConnStateData *anInitiator, const char *host, const int port);
+
+    /* AsyncJob API */
+    //virtual ~ServerPeeker();
+    virtual void start();
+    virtual bool doneAll() const;
+    virtual void swanSong();
+
+private:
+    /// connection manager waiting for peeked server info
+    CbcPointer<ConnStateData> initiator;
+
+    /// client-Squid connection which triggered this job
+    Comm::ConnectionPointer clientConnection;
+
+    /// faked, minimal request; required by server-side API
+    HttpRequest::Pointer request;
+
+    StoreEntry *entry; ///< for receiving Squid-generated error messages
+
+    CBDATA_CLASS2(ServerPeeker);
+};
+
+} // namespace Ssl
+
+#endif