Bump-server-first fails with SQUID_X509_V_ERR_DOMAIN_MISMATCH error

When the bump-server-first used we do not always know the hostname of the server
we are connecting to. Thus the the hostname validity check for the
SSL server certificate will fail with SQUID_X509_V_ERR_DOMAIN_MISMATCH error.

This patch does not check if server certificate is valid for the hostname on
bump-server-first case and move this check after the http request received
from the web client

diff --git a/src/client_side.cc b/src/client_side.cc
index 2df6815fe05448bf906481f392dc47091f134896..0fd1d5701a07e1e7972ac798b4d7d775461b8692 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -104,6 +104,7 @@
 #include "comm/Loops.h"
 #include "comm/Write.h"
 #include "comm/TcpAcceptor.h"
+#include "errorpage.h"
 #include "eui/Config.h"
 #include "fde.h"
 #include "HttpHdrContRange.h"
@@ -2444,6 +2445,67 @@ ConnStateData::clientAfterReadingRequests()
         readSomeData();
 }
 
+#if USE_SSL
+bool ConnStateData::serveDelayedError(ClientSocketContext *context)
+{
+    ClientHttpRequest *http = context->http;
+    StoreEntry *e = bumpServerFirstErrorEntry();
+    if (!e)
+        return false;
+
+    if (!e->isEmpty()) {
+        //Failed? Here we should get the error from conn and send it to client
+        // The error stored in ConnStateData::bumpFirstEntry, replace the
+        // ClientHttpRequest store entry with this.
+        clientStreamNode *node = context->getClientReplyContext();
+        clientReplyContext *repContext = dynamic_cast<clientReplyContext *>(node->data.getRaw());
+        assert (repContext);
+        debugs(33, 5, "Connection first has failed for " << http->uri << ". Respond with an error");
+        repContext->setReplyToStoreEntry(e);
+        context->pullData();
+        flags.readMore = false;
+        return true;
+    }
+
+    // We are in ssl-bump first mode. We have not the server connect name when
+    // we connected to server so we have to check certificates subject with our server name
+    if (X509 *server_cert = getBumpServerCert()) {
+        HttpRequest *request = http->request;
+        if (!Ssl::checkX509ServerValidity(server_cert, request->GetHost())) {
+            debugs(33, 2, "SQUID_X509_V_ERR_DOMAIN_MISMATCH: Certificate does not match domainname " << request->GetHost());
+
+            ACLFilledChecklist check(Config.ssl_client.cert_error, request, dash_str);
+            if (Comm::IsConnOpen(pinning.serverConnection))
+                check.fd(pinning.serverConnection->fd);
+
+            if (check.fastCheck() != ACCESS_ALLOWED) {
+                clientStreamNode *node = context->getClientReplyContext();
+                clientReplyContext *repContext = dynamic_cast<clientReplyContext *>(node->data.getRaw());
+                assert (repContext);
+
+                // Create an error object and fill it
+                ErrorState *err = errorCon(ERR_SECURE_CONNECT_FAIL, HTTP_SERVICE_UNAVAILABLE, request);
+                err->src_addr = clientConnection->remote;
+#ifdef EPROTO
+                err->xerrno = EPROTO;
+#else
+                err->xerrno = EACCES;
+#endif
+                Ssl::ErrorDetail *errDetail = new Ssl::ErrorDetail( SQUID_X509_V_ERR_DOMAIN_MISMATCH, server_cert);
+                err->detail = errDetail;
+                repContext->setReplyToError(request->method, err);
+                assert(context->http->out.offset == 0);
+                context->pullData();
+                flags.readMore = false;
+                return true;
+            }
+        }
+    }
+
+    return false;
+}
+#endif //USE_SSL
+
 static void
 clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *context, const HttpRequestMethod& method, HttpVersion http_ver)
 {
@@ -2645,22 +2707,8 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c
     }
 
 #if USE_SSL
-    if (conn->switchedToHttps() && conn->bumpServerFirstErrorEntry() &&
-        !Comm::IsConnOpen(conn->pinning.serverConnection)) {
-        //Failed? Here we should get the error from conn and send it to client
-        // The error stored in ConnStateData::bumpFirstEntry, replace the 
-        // ClientHttpRequest store entry with this.
-        clientStreamNode *node = context->getClientReplyContext();
-        clientReplyContext *repContext = dynamic_cast<clientReplyContext *>(node->data.getRaw());
-        assert (repContext);
-        debugs(33, 5, "Connection first has failed for " << http->uri << ". Respond with an error");
-        StoreEntry *e = conn->bumpServerFirstErrorEntry();
-        Must(e && !e->isEmpty());
-        repContext->setReplyToStoreEntry(e);
-        context->pullData();
-        conn->flags.readMore = false;
+    if (conn->switchedToHttps() && conn->serveDelayedError(context))
         goto finish;
-    }
 #endif
 
     /* Do we expect a request-body? */

diff --git a/src/client_side.h b/src/client_side.h
index 72c6207908fad723ed9a262344da6daf5ad82e94..8df9c59de7a9679e4253c795d829df058aa2c222 100644 (file)
--- a/src/client_side.h
+++ b/src/client_side.h
@@ -334,6 +334,8 @@ public:
     /// Holds the squid error reply in the case of bump server first error 
     StoreEntry *bumpServerFirstErrorEntry() const {return bumpErrorEntry;}
     void setBumpServerCert(X509 *serverCert) {bumpServerCert.reset(serverCert);}
+    X509 *getBumpServerCert() {return bumpServerCert.get();}
+    bool serveDelayedError(ClientSocketContext *context);
 #else
     bool switchedToHttps() const { return false; }
 #endif

diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc
index 0e3ff46bb9c8c17f4688767495d16d8a5553b437..288f2b58a57b69ed9dbb1e7719d276b7c9fa0299 100644 (file)
--- a/src/client_side_reply.cc
+++ b/src/client_side_reply.cc
@@ -113,16 +113,21 @@ clientReplyContext::setReplyToError(
     if (unparsedrequest)
         errstate->request_hdrs = xstrdup(unparsedrequest);
 
-    if (status == HTTP_NOT_IMPLEMENTED && http->request)
+#if USE_AUTH
+    errstate->auth_user_request = auth_user_request;
+#endif
+    setReplyToError(method, errstate);
+}
+
+void clientReplyContext::setReplyToError(const HttpRequestMethod& method, ErrorState *errstate)
+{
+    if (errstate->httpStatus == HTTP_NOT_IMPLEMENTED && http->request)
         /* prevent confusion over whether we default to persistent or not */
         http->request->flags.proxy_keepalive = 0;
 
     http->al.http.code = errstate->httpStatus;
 
     createStoreEntry(method, request_flags());
-#if USE_AUTH
-    errstate->auth_user_request = auth_user_request;
-#endif
     assert(errstate->callback_data == NULL);
     errorAppendEntry(http->storeEntry(), errstate);
     /* Now the caller reads to get this */

diff --git a/src/client_side_reply.h b/src/client_side_reply.h
index e92f41f1832228c9da27d4ffc6e6e1b5bc8a9374..a298815ccf91aeac5995b12643cdc5276e63329b 100644 (file)
--- a/src/client_side_reply.h
+++ b/src/client_side_reply.h
@@ -79,6 +79,7 @@ public:
 #else
                          void * unused);
 #endif
+    void setReplyToError(const HttpRequestMethod& method, ErrorState *errstate);
     void createStoreEntry(const HttpRequestMethod& m, request_flags flags);
     void removeStoreReference(store_client ** scp, StoreEntry ** ep);
     void removeClientStoreReference(store_client **scp, ClientHttpRequest *http);

diff --git a/src/forward.cc b/src/forward.cc
index fa71abf38bbd488301e9cb4ad3d0e95d89a07e0b..38898f453827a4e2cecca2be3983d2da8bbcf4dc 100644 (file)
--- a/src/forward.cc
+++ b/src/forward.cc
@@ -756,7 +756,9 @@ FwdState::initiateSSL()
             SSL_set_session(ssl, peer->sslSession);
 
     } else {
-        SSL_set_ex_data(ssl, ssl_ex_index_server, (void*)request->GetHost());
+        if (request->protocol != AnyP::PROTO_SSL_PEEK)
+            SSL_set_ex_data(ssl, ssl_ex_index_server, (void*)request->GetHost());
+        // else  we do not have the ssl server name yet, but only its IP address.
 
         // We need to set SNI TLS extension only in the case we are
         // connecting direct to origin server

diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index 3ef5ba4d09066fb36b7bff8c106102ab6564e61b..f763b010c8f32d9d19f89a774f01754a41b8de76 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -200,6 +200,11 @@ static int check_domain( void *check_data, ASN1_STRING *cn_data)
     return matchDomainName(server, cn[0] == '*' ? cn + 1 : cn);
 }
 
+bool Ssl::checkX509ServerValidity(X509 *cert, const char *server)
+{
+    return matchX509CommonNames(cert, (void *)server, check_domain);
+}
+
 /// \ingroup ServerProtocolSSLInternal
 static int
 ssl_verify_cb(int ok, X509_STORE_CTX * ctx)
@@ -222,9 +227,7 @@ ssl_verify_cb(int ok, X509_STORE_CTX * ctx)
         debugs(83, 5, "SSL Certificate signature OK: " << buffer);
 
         if (server) {
-            int found = Ssl::matchX509CommonNames(peer_cert, (void *)server, check_domain);
-
-            if (!found) {
+            if (!Ssl::checkX509ServerValidity(peer_cert, server)) {
                 debugs(83, 2, "SQUID_X509_V_ERR_DOMAIN_MISMATCH: Certificate " << buffer << " does not match domainname " << server);
                 ok = 0;
                 error_no = SQUID_X509_V_ERR_DOMAIN_MISMATCH;

diff --git a/src/ssl/support.h b/src/ssl/support.h
index 3bf0f3d79938f87ba895fa530917a5c69d3c89a1..057014a0ca8083e61ca744d8d0327e117ec35709 100644 (file)
--- a/src/ssl/support.h
+++ b/src/ssl/support.h
@@ -150,6 +150,15 @@ void readCertChainAndPrivateKeyFromFiles(X509_Pointer & cert, EVP_PKEY_Pointer &
  */
 int matchX509CommonNames(X509 *peer_cert, void *check_data, int (*check_func)(void *check_data,  ASN1_STRING *cn_data));
 
+/**
+   \ingroup ServerProtocolSSLAPI
+   * Check if the certificate is valid for a server
+   \param cert  The X509 cert to check.
+   \param server The server name.
+   \return   true if the certificate is valid for the server or false otherwise.
+ */
+bool checkX509ServerValidity(X509 *cert, const char *server);
+
 /**
    \ingroup ServerProtocolSSLAPI
    * Convert a given ASN1_TIME to a string form.