From 68920ad862cc12cb6404a8fc3b2371a3e7afb6e1 Mon Sep 17 00:00:00 2001 From: Christos Tsantilas Date: Wed, 22 Apr 2015 22:23:08 +0300 Subject: [PATCH] Unexpected SQUID_X509_V_ERR_DOMAIN_MISMATCH errors while accessing sites with valid certificates 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 --- src/ssl/PeerConnector.cc | 12 ++++++------ src/ssl/support.cc | 15 ++++++++++++--- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/ssl/PeerConnector.cc b/src/ssl/PeerConnector.cc index cba2ef5667..9557b5118a 100644 --- 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(&peer->secure.sslDomain)->c_str(); - SSL_set_ex_data(ssl, ssl_ex_index_server, const_cast(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(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 58279d88c8..8af22eac29 100644 --- 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 (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); -- 2.47.2