Unexpected SQUID_X509_V_ERR_DOMAIN_MISMATCH errors while accessing sites with valid...

A "const char *" pointer retrieved using the SBuf::c_str() method may attached
to an SSL object using the SSL_set_ex_data method as server name used to
validate server certificates. This pointer may become invalid, causing
the SQUID_X509_V_ERR_DOMAIN_MISMATCH errors.

This patch changes the type of the ssl_ex_index_server index used with the
SSL_set_ex_data function to be an SBuf object.

This is a Measurement Factory project

diff --git a/src/ssl/PeerConnector.cc b/src/ssl/PeerConnector.cc
index cba2ef56679e11b9f47dce93a747b5cb04adb212..9557b5118aeb0c977532fb84b73cb4ea37184e23 100644 (file)
--- a/src/ssl/PeerConnector.cc
+++ b/src/ssl/PeerConnector.cc
@@ -136,8 +136,8 @@ Ssl::PeerConnector::initializeSsl()
         assert(!peer->secure.sslDomain.isEmpty());
 
         // const loss is okay here, ssl_ex_index_server is only read and not assigned a destructor
-        const char *host = const_cast<SBuf*>(&peer->secure.sslDomain)->c_str();
-        SSL_set_ex_data(ssl, ssl_ex_index_server, const_cast<char*>(host));
+        SBuf *host = new SBuf(peer->secure.sslDomain);
+        SSL_set_ex_data(ssl, ssl_ex_index_server, host);
 
         if (peer->sslSession)
             SSL_set_session(ssl, peer->sslSession);
@@ -145,7 +145,7 @@ Ssl::PeerConnector::initializeSsl()
         // client connection is required in the case we need to splice
         // or terminate client and server connections
         assert(clientConn != NULL);
-        const char *hostName = NULL;
+        SBuf *hostName = NULL;
         Ssl::ClientBio *cltBio = NULL;
 
         //Enable Status_request tls extension, required to bump some clients
@@ -157,7 +157,7 @@ Ssl::PeerConnector::initializeSsl()
             cltBio = static_cast<Ssl::ClientBio *>(b->ptr);
             const Ssl::Bio::sslFeatures &features = cltBio->getFeatures();
             if (!features.serverName.isEmpty())
-                hostName = features.serverName.c_str();
+                hostName = new SBuf(features.serverName);
         }
 
         if (!hostName) {
@@ -166,7 +166,7 @@ Ssl::PeerConnector::initializeSsl()
             // unless it was the CONNECT request with a user-typed address.
             const bool isConnectRequest = !csd->port->flags.isIntercepted();
             if (!request->flags.sslPeek || isConnectRequest)
-                hostName = request->GetHost();
+                hostName = new SBuf(request->GetHost());
         }
 
         if (hostName)
@@ -194,7 +194,7 @@ Ssl::PeerConnector::initializeSsl()
 
             // Use SNI TLS extension only when we connect directly
             // to the origin server and we know the server host name.
-            const char *sniServer = hostName ? hostName :
+            const char *sniServer = hostName ? hostName->c_str() :
                                     (!request->GetHostIsNumeric() ? request->GetHost() : NULL);
             if (sniServer)
                 Ssl::setClientSNI(ssl, sniServer);

diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index 58279d88c85addb69cdb8a71432ed2c80937de1d..8af22eac293e8a7f2453eb27d8ddd1130a0162e4 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -222,7 +222,7 @@ ssl_verify_cb(int ok, X509_STORE_CTX * ctx)
     char buffer[256] = "";
     SSL *ssl = (SSL *)X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx());
     SSL_CTX *sslctx = SSL_get_SSL_CTX(ssl);
-    const char *server = (const char *)SSL_get_ex_data(ssl, ssl_ex_index_server);
+    SBuf *server = (SBuf *)SSL_get_ex_data(ssl, ssl_ex_index_server);
     void *dont_verify_domain = SSL_CTX_get_ex_data(sslctx, ssl_ctx_ex_index_dont_verify_domain);
     ACLChecklist *check = (ACLChecklist*)SSL_get_ex_data(ssl, ssl_ex_index_cert_error_check);
     X509 *peeked_cert = (X509 *)SSL_get_ex_data(ssl, ssl_ex_index_ssl_peeked_cert);
@@ -253,7 +253,7 @@ ssl_verify_cb(int ok, X509_STORE_CTX * ctx)
 
         // Check for domain mismatch only if the current certificate is the peer certificate.
         if (!dont_verify_domain && server && peer_cert == X509_STORE_CTX_get_current_cert(ctx)) {
-            if (!Ssl::checkX509ServerValidity(peer_cert, server)) {
+            if (!Ssl::checkX509ServerValidity(peer_cert, server->c_str())) {
                 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;
@@ -683,6 +683,15 @@ ssl_free_X509(void *, void *ptr, CRYPTO_EX_DATA *,
     X509_free(cert);
 }
 
+// "free" function for SBuf 
+static void
+ssl_free_SBuf(void *, void *ptr, CRYPTO_EX_DATA *,
+              int, long, void *)
+{
+    SBuf  *buf = static_cast <SBuf *>(ptr);
+    delete buf;
+}
+
 /// \ingroup ServerProtocolSSLInternal
 static void
 ssl_initialize(void)
@@ -717,7 +726,7 @@ ssl_initialize(void)
         fatalf("Sign hash '%s' is not supported\n", defName);
 
     ssl_ex_index_server = SSL_get_ex_new_index(0, (void *) "server", NULL, NULL, NULL);
-    ssl_ctx_ex_index_dont_verify_domain = SSL_CTX_get_ex_new_index(0, (void *) "dont_verify_domain", NULL, NULL, NULL);
+    ssl_ctx_ex_index_dont_verify_domain = SSL_CTX_get_ex_new_index(0, (void *) "dont_verify_domain", NULL, NULL, ssl_free_SBuf);
     ssl_ex_index_cert_error_check = SSL_get_ex_new_index(0, (void *) "cert_error_check", NULL, &ssl_dupAclChecklist, &ssl_freeAclChecklist);
     ssl_ex_index_ssl_error_detail = SSL_get_ex_new_index(0, (void *) "ssl_error_detail", NULL, NULL, &ssl_free_ErrorDetail);
     ssl_ex_index_ssl_peeked_cert  = SSL_get_ex_new_index(0, (void *) "ssl_peeked_cert", NULL, NULL, &ssl_free_X509);