From: Christos Tsantilas <chtsanti@users.sourceforge.net>
Date: Thu, 9 Jul 2015 13:12:10 +0000 (+0300)
Subject: Errors served using invalid certificates when dealing with SSL server errors.
X-Git-Tag: merge-candidate-3-v1~52^2~1
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=ce74a2ebb78c728378d8b9520f0c8489d9415aaa;p=thirdparty%2Fsquid.git

Errors served using invalid certificates when dealing with SSL server errors.

When bumping Squid needs to send an Squid-generated error "page" over a
secure connection, Squid needs to generate a certificate for that connection.
Prior to these changes, several scenarios could lead to Squid generating
a certificate that clients could not validate. In those cases, the user would
get a cryptic and misleading browser error instead of a Squid-generated
error page with useful details about the problem.

For example, is a server certificate that is rejected by the certificate
validation helper. Squid no longer uses CN from that certificate to generate
a fake certificate.

Another example is a user accessing an origin server using one of its
"alternative names" and getting a Squid-generated certificate containing just
the server common name (CN).

These changes make sure that certificate for error pages is generated using
SNI (when peeking or staring, if available) or CONNECT host name (including
server-first bumping mode). We now update the ConnStateData::sslCommonName
field (used as CN field for generated certificates) only _after_ the server
certificate is successfully validated.

This is a Measurement Factory project.
---

diff --git a/src/ssl/PeerConnector.cc b/src/ssl/PeerConnector.cc
index 1ecaf8e21f..1bfaac7973 100644
--- a/src/ssl/PeerConnector.cc
+++ b/src/ssl/PeerConnector.cc
@@ -739,8 +739,11 @@ Ssl::PeekingPeerConnector::noteNegotiationDone(ErrorState *error)
         }
     }
 
-    if (!error && splice)
-        switchToTunnel(request.getRaw(), clientConn, serverConn);
+    if (!error) {
+        serverCertificateVerified();
+        if (splice)
+            switchToTunnel(request.getRaw(), clientConn, serverConn);
+    }
 }
 
 void
@@ -820,10 +823,6 @@ Ssl::PeekingPeerConnector::handleServerCertificate()
 
         serverCertificateHandled = true;
 
-        csd->resetSslCommonName(Ssl::CommonHostName(serverCert.get()));
-        debugs(83, 5, "HTTPS server CN: " << csd->sslCommonName() <<
-               " bumped: " << *serverConnection());
-
         // remember the server certificate for later use
         if (Ssl::ServerBump *serverBump = csd->serverBump()) {
             serverBump->serverCert.reset(serverCert.release());
@@ -831,3 +830,24 @@ Ssl::PeekingPeerConnector::handleServerCertificate()
     }
 }
 
+void
+Ssl::PeekingPeerConnector::serverCertificateVerified()
+{
+    if (ConnStateData *csd = request->clientConnectionManager.valid()) {
+        Ssl::X509_Pointer serverCert;
+        if(Ssl::ServerBump *serverBump = csd->serverBump())
+            serverCert.resetAndLock(serverBump->serverCert.get());
+        else {
+            const int fd = serverConnection()->fd;
+            SSL *ssl = fd_table[fd].ssl;
+            serverCert.reset(SSL_get_peer_certificate(ssl));
+        }
+        if (serverCert.get()) {
+            csd->resetSslCommonName(Ssl::CommonHostName(serverCert.get()));
+            debugs(83, 5, "HTTPS server CN: " << csd->sslCommonName() <<
+                   " bumped: " << *serverConnection());
+        }
+    }
+}
+
+
diff --git a/src/ssl/PeerConnector.h b/src/ssl/PeerConnector.h
index 95266d990b..fc1dcdc638 100644
--- a/src/ssl/PeerConnector.h
+++ b/src/ssl/PeerConnector.h
@@ -241,6 +241,10 @@ public:
     /// Handles the final bumping decision.
     void checkForPeekAndSpliceDone(Ssl::BumpMode const);
 
+    /// Runs after the server certificate verified to update client
+    /// connection manager members
+    void serverCertificateVerified();
+
     /// A wrapper function for checkForPeekAndSpliceDone for use with acl
     static void cbCheckForPeekAndSpliceDone(allow_t answer, void *data);