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 126dd95b220209f28384a16271bef3956fe00e43..98c779d021c24994ec9ecba5f4f864dd9b3f75b9 100644 (file)
--- a/src/ssl/PeerConnector.cc
+++ b/src/ssl/PeerConnector.cc
@@ -131,18 +131,8 @@ Ssl::PeerConnector::initializeSsl()
     }
 
     if (peer) {
-        if (peer->ssldomain)
-            SSL_set_ex_data(ssl, ssl_ex_index_server, peer->ssldomain);
-
-#if NOT_YET
-
-        else if (peer->name)
-            SSL_set_ex_data(ssl, ssl_ex_index_server, peer->name);
-
-#endif
-
-        else
-            SSL_set_ex_data(ssl, ssl_ex_index_server, peer->host);
+        SBuf *host = new SBuf(peer->ssldomain ? peer->ssldomain : peer->host);
+        SSL_set_ex_data(ssl, ssl_ex_index_server, host);
 
         if (peer->sslSession)
             SSL_set_session(ssl, peer->sslSession);
@@ -150,7 +140,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
@@ -162,7 +152,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) {
@@ -171,7 +161,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)
@@ -199,7 +189,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 4225fa4b86c94b542641697881972e1e91eb735c..d9eb052fb07e635a69cf8906e0830d88eb5249fa 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -221,7 +221,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);
@@ -252,7 +252,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;
@@ -698,6 +698,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)
@@ -731,7 +740,7 @@ ssl_initialize(void)
     if (!Ssl::DefaultSignHash)
         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_ex_index_server = SSL_get_ex_new_index(0, (void *) "server", NULL, NULL, ssl_free_SBuf);
     ssl_ctx_ex_index_dont_verify_domain = SSL_CTX_get_ex_new_index(0, (void *) "dont_verify_domain", NULL, NULL, NULL);
     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);