From: Christos Tsantilas <chtsanti@users.sourceforge.net>
Date: Thu, 23 Feb 2012 19:48:07 +0000 (+0200)
Subject: Certificate for error messages
X-Git-Tag: BumpSslServerFirst.take05~15
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=59a49556276875029ec5752103f928774660ccf9;p=thirdparty%2Fsquid.git

Certificate for error messages

If a certificate verification error is honored by Squid (i.e., a Squid error
page is returned to the user), the user gets a browser warning and then, after
ignoring the warning, the squid error message (about the same kind of problem). This happens because the fake certificate mimics details of the true server
certificate, which is "broken".

When Squid serves its own error page it is no longer trying to hide its presencefrom the user. Thus, it is probably better to serve all Squid-generated SSL
handshake and validation error responses with a trusted certificate containing
minimal details, like we already do for the DNS_FAIL errors.

This patch fixing the above problem.

NOTE: The fix will not work for domain mismatch errors. We are checking
for domain mismatch after we have establish the SSL connection with the web
client , and after we got the http request, because we need the HTTP request
Host header to check for SSL domain mismatch errors.
---

diff --git a/src/client_side.cc b/src/client_side.cc
index bfb52111e1..17a6493c3f 100644
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -3661,51 +3661,61 @@ void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &cer
     if (!bumpServerFirstErrorEntry())
         return;
 
-    if (X509 *mimicCert = bumpServerCert.get())
-        certProperties.mimicCert.resetAndLock(mimicCert);
-
-    HttpRequest *fakeRequest =  new HttpRequest();
-    fakeRequest->SetHost(sslConnectHostOrIp.termedBuf());
-    fakeRequest->port = clientConnection->local.GetPort();
-    fakeRequest->protocol = AnyP::PROTO_HTTPS;
+    // In the case of error while connecting to secure server, use a fake trusted certificate,
+    // with no mimicked fields and no adaptation algorithms
+    if (bumpServerFirstErrorEntry()->isEmpty()) {
+        if (X509 *mimicCert = bumpServerCert.get())
+            certProperties.mimicCert.resetAndLock(mimicCert);
+
+        HttpRequest *fakeRequest =  new HttpRequest();
+        fakeRequest->SetHost(sslConnectHostOrIp.termedBuf());
+        fakeRequest->port = clientConnection->local.GetPort();
+        fakeRequest->protocol = AnyP::PROTO_HTTPS;
  
-    ACLFilledChecklist checklist(NULL, fakeRequest, 
-                                 clientConnection != NULL ? clientConnection->rfc931 : dash_str);
-    checklist.conn(this);
-    checklist.src_addr = clientConnection->remote;
-    checklist.my_addr = clientConnection->local;
-    checklist.sslErrorList = cbdataReference(bumpSslErrorNoList);
-
-    for (sslproxy_cert_adapt *ca = Config.ssl_client.cert_adapt; ca != NULL; ca = ca->next) {
-        if (ca->aclList && checklist.fastCheck(ca->aclList) == ACCESS_ALLOWED) {
-            const char *alg = Ssl::CertAdaptAlgorithmStr[ca->alg];
-            const char *param = ca->param;
+        ACLFilledChecklist checklist(NULL, fakeRequest, 
+                                     clientConnection != NULL ? clientConnection->rfc931 : dash_str);
+        checklist.conn(this);
+        checklist.src_addr = clientConnection->remote;
+        checklist.my_addr = clientConnection->local;
+        checklist.sslErrorList = cbdataReference(bumpSslErrorNoList);
+
+        for (sslproxy_cert_adapt *ca = Config.ssl_client.cert_adapt; ca != NULL; ca = ca->next) {
+            if (ca->aclList && checklist.fastCheck(ca->aclList) == ACCESS_ALLOWED) {
+                const char *alg = Ssl::CertAdaptAlgorithmStr[ca->alg];
+                const char *param = ca->param;
   
-            // if not param defined for Common Name adaptation use hostname from 
-            // the CONNECT request
-            if (ca->alg == Ssl::algSetCommonName && !certProperties.setCommonName) {
-                if (!param)
-                    param = sslConnectHostOrIp.termedBuf();
-                certProperties.commonName = param;
-                certProperties.setCommonName = true;
+                // if not param defined for Common Name adaptation use hostname from 
+                // the CONNECT request
+                if (ca->alg == Ssl::algSetCommonName && !certProperties.setCommonName) {
+                    if (!param)
+                        param = sslConnectHostOrIp.termedBuf();
+                    certProperties.commonName = param;
+                    certProperties.setCommonName = true;
+                }
+                else if(ca->alg == Ssl::algSetValidAfter && !certProperties.setValidAfter)
+                    certProperties.setValidAfter = true;
+                else if(ca->alg == Ssl::algSetValidBefore && !certProperties.setValidBefore)
+                    certProperties.setValidBefore = true;
+
+                assert(alg && param);
+                debugs(33, 5, HERE << "Matches certificate adaptation aglorithm: " << 
+                       alg << " param: " << param);
             }
-            else if(ca->alg == Ssl::algSetValidAfter && !certProperties.setValidAfter)
-                certProperties.setValidAfter = true;
-            else if(ca->alg == Ssl::algSetValidBefore && !certProperties.setValidBefore)
-                certProperties.setValidBefore = true;
-
-            assert(alg && param);
-            debugs(33, 5, HERE << "Matches certificate adaptation aglorithm: " << 
-                   alg << " param: " << param);
         }
-    }
 
-    certProperties.signAlgorithm = Ssl::algSignEnd;
-    for (sslproxy_cert_sign *sg = Config.ssl_client.cert_sign; sg != NULL; sg = sg->next) {
-        if (sg->aclList && checklist.fastCheck(sg->aclList) == ACCESS_ALLOWED) {
-            certProperties.signAlgorithm = (Ssl::CertSignAlgorithm)sg->alg;
-            break;
+        certProperties.signAlgorithm = Ssl::algSignEnd;
+        for (sslproxy_cert_sign *sg = Config.ssl_client.cert_sign; sg != NULL; sg = sg->next) {
+            if (sg->aclList && checklist.fastCheck(sg->aclList) == ACCESS_ALLOWED) {
+                certProperties.signAlgorithm = (Ssl::CertSignAlgorithm)sg->alg;
+                break;
+            }
         }
+    } else {// if (!bumpServerFirstErrorEntry()->isEmpty())
+        // Use trusted certificate for a Squid-generated error
+        // or the user would have to add a security exception
+        // just to see the error page. We will close the connection
+        // so that the trust is not extended to non-Squid content.
+        certProperties.signAlgorithm = Ssl::algSignTrusted;
     }
 
     assert(certProperties.signAlgorithm != Ssl::algSignEnd);
@@ -3860,15 +3870,17 @@ ConnStateData::httpsPeeked(Comm::ConnectionPointer serverConnection)
         Ssl::X509_Pointer serverCert(SSL_get_peer_certificate(ssl));
         assert(serverCert.get() != NULL);
         sslCommonName = Ssl::CommonHostName(serverCert.get());
-        assert(sslCommonName.defined());
-        debugs(33, 5, HERE << "found HTTPS server CN " << sslCommonName << " at bumped " <<
-               *serverConnection);
+        debugs(33, 5, HERE << "HTTPS server CN: " << sslCommonName << 
+               " bumped: " << *serverConnection);
 
         pinConnection(serverConnection, NULL, NULL, false);
 
         debugs(33, 5, HERE << "bumped HTTPS server: " << sslConnectHostOrIp);
-    } else
+    } else {
         debugs(33, 5, HERE << "Error while bumping: " << sslConnectHostOrIp);
+        if (bumpServerCert.get())
+            sslCommonName = Ssl::CommonHostName(bumpServerCert.get());
+    }
 
     if (httpsPeeker.valid())
         httpsPeeker->noteHttpsPeeked(serverConnection);