From: Christos Tsantilas <chtsanti@users.sourceforge.net>
Date: Mon, 17 Oct 2016 11:02:18 +0000 (+0300)
Subject: Avoid segfaults when we lack the server name for certificate validator.
X-Git-Tag: SQUID_4_0_16~18
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=cb171eaddfd37bd81c31aa3b5bc6a255ee7c6a9a;p=thirdparty%2Fsquid.git

Avoid segfaults when we lack the server name for certificate validator.

Squid could crash when transparently proxying a TLS client that does not
send SNI because the code composing certificate validator request
assumed that the intended server name is always available. There could
have been other use cases leading to the same kind of crash.

Fixed Squid sends an empty domain name to the certificate validator when
SSL_ex_data[ssl_ex_index_server] does not exist.

This is a Measurement Factory project.
---

diff --git a/src/Debug.h b/src/Debug.h
index f98d962893..c9f795e3aa 100644
--- a/src/Debug.h
+++ b/src/Debug.h
@@ -206,5 +206,35 @@ std::ostream &operator <<(std::ostream &os, const Raw &raw)
     return raw.print(os);
 }
 
+/// debugs objects pointed by possibly nil pointers: label=object
+template <class Pointer>
+class RawPointerT {
+public:
+    RawPointerT(const char *aLabel, const Pointer &aPtr):
+        label(aLabel), ptr(aPtr) {}
+    const char *label; /// the name or description of the being-debugged object
+    const Pointer &ptr; /// a possibly nil pointer to the being-debugged object
+};
+
+/// convenience wrapper for creating  RawPointerT<> objects
+template <class Pointer>
+inline RawPointerT<Pointer>
+RawPointer(const char *label, const Pointer &ptr)
+{
+    return RawPointerT<Pointer>(label, ptr);
+}
+
+/// prints RawPointerT<>, dereferencing the raw pointer if possible
+template <class Pointer>
+inline std::ostream &
+operator <<(std::ostream &os, const RawPointerT<Pointer> &pd)
+{
+    os << pd.label << '=';
+    if (pd.ptr)
+        return os << *pd.ptr;
+    else
+        return os << "[nil]";
+}
+
 #endif /* SQUID_DEBUG_H */
 
diff --git a/src/security/PeerConnector.cc b/src/security/PeerConnector.cc
index 24b6f5b948..51e77833bc 100644
--- a/src/security/PeerConnector.cc
+++ b/src/security/PeerConnector.cc
@@ -208,8 +208,8 @@ Security::PeerConnector::sslFinalized()
         // Ssl::CertValidationRequest object used only to pass data to
         // Ssl::CertValidationHelper::submit method.
         validationRequest.ssl = session.get();
-        SBuf *dName = (SBuf *)SSL_get_ex_data(session.get(), ssl_ex_index_server);
-        validationRequest.domainName = dName->c_str();
+        if (SBuf *dName = (SBuf *)SSL_get_ex_data(session.get(), ssl_ex_index_server))
+            validationRequest.domainName = dName->c_str();
         if (Security::CertErrors *errs = static_cast<Security::CertErrors *>(SSL_get_ex_data(session.get(), ssl_ex_index_ssl_errors)))
             // validationRequest disappears on return so no need to cbdataReference
             validationRequest.errors = errs;
@@ -253,7 +253,7 @@ Security::PeerConnector::sslCrtvdHandleReply(Ssl::CertValidationResponse::Pointe
     if (Debug::Enabled(83, 5)) {
         Security::SessionPointer ssl(fd_table[serverConnection()->fd].ssl);
         SBuf *server = static_cast<SBuf *>(SSL_get_ex_data(ssl.get(), ssl_ex_index_server));
-        debugs(83,5, *server << " cert validation result: " << validationResponse->resultCode);
+        debugs(83,5, RawPointer("host", server) << " cert validation result: " << validationResponse->resultCode);
     }
 
     if (validationResponse->resultCode == ::Helper::Error) {