From: Rusty Bird <rustybird@net-c.com>
Date: Fri, 12 Jan 2018 14:46:39 +0000 (+0000)
Subject: Support sending SNI in all TLS connections opened by Squid (#123)
X-Git-Tag: SQUID_4_0_23~5
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e68114d4697413657436afc4d8b4f8822d841868;p=thirdparty%2Fsquid.git

Support sending SNI in all TLS connections opened by Squid (#123)

Add SNI support to regular (i.e., not SslBumped) TLS connections,
including transparent http->https rewrite and Secure ICAP.
---

diff --git a/src/adaptation/icap/Xaction.cc b/src/adaptation/icap/Xaction.cc
index 26dabf18b9..7f61ed4d2e 100644
--- a/src/adaptation/icap/Xaction.cc
+++ b/src/adaptation/icap/Xaction.cc
@@ -717,6 +717,7 @@ Ssl::IcapPeerConnector::initialize(Security::SessionPointer &serverSession)
 #if USE_OPENSSL
     SBuf *host = new SBuf(icapService->cfg().secure.sslDomain);
     SSL_set_ex_data(serverSession.get(), ssl_ex_index_server, host);
+    setClientSNI(serverSession.get(), host->c_str());
 
     ACLFilledChecklist *check = static_cast<ACLFilledChecklist *>(SSL_get_ex_data(serverSession.get(), ssl_ex_index_cert_error_check));
     if (check)
diff --git a/src/security/BlindPeerConnector.cc b/src/security/BlindPeerConnector.cc
index a570c9b5aa..00543d10c9 100644
--- a/src/security/BlindPeerConnector.cc
+++ b/src/security/BlindPeerConnector.cc
@@ -47,11 +47,13 @@ Security::BlindPeerConnector::initialize(Security::SessionPointer &serverSession
         // const loss is okay here, ssl_ex_index_server is only read and not assigned a destructor
         SBuf *host = new SBuf(peer->secure.sslDomain);
         SSL_set_ex_data(serverSession.get(), ssl_ex_index_server, host);
+        Ssl::setClientSNI(serverSession.get(), host->c_str());
 
         Security::SetSessionResumeData(serverSession, peer->sslSession);
     } else {
         SBuf *hostName = new SBuf(request->url.host());
         SSL_set_ex_data(serverSession.get(), ssl_ex_index_server, (void*)hostName);
+        Ssl::setClientSNI(serverSession.get(), hostName->c_str());
 #endif
     }
 
diff --git a/src/ssl/PeekingPeerConnector.cc b/src/ssl/PeekingPeerConnector.cc
index 4a1854d369..eb95af8d1a 100644
--- a/src/ssl/PeekingPeerConnector.cc
+++ b/src/ssl/PeekingPeerConnector.cc
@@ -186,17 +186,12 @@ Ssl::PeekingPeerConnector::initialize(Security::SessionPointer &serverSession)
             // Set client SSL options
             SSL_set_options(serverSession.get(), ::Security::ProxyOutgoingConfig.parsedOptions);
 
-            // Use SNI TLS extension only when we connect directly
-            // to the origin server and we know the server host name.
-            const char *sniServer = NULL;
             const bool redirected = request->flags.redirected && ::Config.onoff.redir_rewrites_host;
-            if (!hostName || redirected)
-                sniServer = !request->url.hostIsNumeric() ? request->url.host() : NULL;
-            else
-                sniServer = hostName->c_str();
-
+            const char *sniServer = (!hostName || redirected) ?
+                                    request->url.host() :
+                                    hostName->c_str();
             if (sniServer)
-                Ssl::setClientSNI(serverSession.get(), sniServer);
+                setClientSNI(serverSession.get(), sniServer);
         }
 
         if (Ssl::ServerBump *serverBump = csd->serverBump()) {
diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index 7180b7ee34..566d54907b 100644
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -923,9 +923,13 @@ Ssl::verifySslCertificate(Security::ContextPointer &ctx, CertificateProperties c
     return (X509_cmp_current_time(time_notBefore) < 0 && X509_cmp_current_time(time_notAfter) > 0);
 }
 
-bool
+void
 Ssl::setClientSNI(SSL *ssl, const char *fqdn)
 {
+    const Ip::Address test(fqdn);
+    if (!test.isAnyAddr())
+        return; // raw IP is inappropriate for SNI
+
     //The SSL_CTRL_SET_TLSEXT_HOSTNAME is a openssl macro which indicates
     // if the TLS servername extension (SNI) is enabled in openssl library.
 #if defined(SSL_CTRL_SET_TLSEXT_HOSTNAME)
@@ -933,12 +937,9 @@ Ssl::setClientSNI(SSL *ssl, const char *fqdn)
         const int ssl_error = ERR_get_error();
         debugs(83, 3,  "WARNING: unable to set TLS servername extension (SNI): " <<
                Security::ErrorString(ssl_error) << "\n");
-        return false;
     }
-    return true;
 #else
     debugs(83, 7,  "no support for TLS servername extension (SNI)");
-    return false;
 #endif
 }
 
diff --git a/src/ssl/support.h b/src/ssl/support.h
index deaca3d85a..33075d7ba2 100644
--- a/src/ssl/support.h
+++ b/src/ssl/support.h
@@ -320,9 +320,8 @@ int asn1timeToString(ASN1_TIME *tm, char *buf, int len);
    \ingroup ServerProtocolSSLAPI
    * Sets the hostname for the Server Name Indication (SNI) TLS extension
    * if supported by the used openssl toolkit.
-   \return true if SNI set false otherwise
 */
-bool setClientSNI(SSL *ssl, const char *fqdn);
+void setClientSNI(SSL *ssl, const char *fqdn);
 
 /**
   \ingroup ServerProtocolSSLAPI
diff --git a/src/tests/stub_libsslsquid.cc b/src/tests/stub_libsslsquid.cc
index c4314bd621..64601d7c42 100644
--- a/src/tests/stub_libsslsquid.cc
+++ b/src/tests/stub_libsslsquid.cc
@@ -75,7 +75,7 @@ void readCertChainAndPrivateKeyFromFiles(Security::CertPointer &, Security::Priv
 int matchX509CommonNames(X509 *peer_cert, void *check_data, int (*check_func)(void *check_data,  ASN1_STRING *cn_data)) STUB_RETVAL(0)
 bool checkX509ServerValidity(X509 *cert, const char *server) STUB_RETVAL(false)
 int asn1timeToString(ASN1_TIME *tm, char *buf, int len) STUB_RETVAL(0)
-bool setClientSNI(SSL *ssl, const char *fqdn) STUB_RETVAL(false)
+void setClientSNI(SSL *ssl, const char *fqdn) STUB
 } //namespace Ssl
 
 #endif