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 1ecaf8e21fccec3265eceb496954b9865bc5014b..1bfaac7973397677f748f1d9ffb5483fa57b68e6 100644 (file)
--- 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 95266d990b82cfc6f21ce715dc6d2d889b9be487..fc1dcdc638c1d1089dfa03febe80f2d6766fdada 100644 (file)
--- 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);