Security::HandshakeParser::parseServerCertificates builds cert list with nils (#42)

... if squid does not compiled with OpenSSL support.
This patch fixes:
  * HandshakeParser::ParseCertificate() to return a Security::Pointer
  * HandshakeParser:: parseServerCertificates() to be a no-op if OpenSSL is
    not used
  * Fix compile error if squid compiled without openssl but with gnutls enabled

This is a Measurement Factory project

diff --git a/src/security/Handshake.cc b/src/security/Handshake.cc
index 73c7a70d6568ace121e3308e36ff7dc58bf64bce..cccd9937c7c7bf751464869b138ba2173c012e5d 100644 (file)
--- a/src/security/Handshake.cc
+++ b/src/security/Handshake.cc
@@ -535,9 +535,12 @@ Security::HandshakeParser::parseHello(const SBuf &data)
     return false; // unreached
 }
 
-void
-Security::HandshakeParser::ParseCertificate(const SBuf &raw, Security::CertPointer &pCert)
+/// Creates and returns a certificate by parsing a DER-encoded X509 structure.
+/// Throws on failures.
+Security::CertPointer
+Security::HandshakeParser::ParseCertificate(const SBuf &raw)
 {
+    Security::CertPointer pCert;
 #if USE_OPENSSL
     auto x509Start = reinterpret_cast<const unsigned char *>(raw.rawContent());
     auto x509Pos = x509Start;
@@ -546,27 +549,31 @@ Security::HandshakeParser::ParseCertificate(const SBuf &raw, Security::CertPoint
     Must(x509); // successfully parsed
     Must(x509Pos == x509Start + raw.length()); // no leftovers
 #else
-    // workaround GCC -O3 error with unused variables. see bug 4663.
-    pCert = nullptr;
-    debugs(83, 2, "TLS parsing is not supported without OpenSSL. " << raw);
+    assert(false);  // this code should never be reached
+    pCert = Security::CertPointer(nullptr); // avoid warnings about uninitialized pCert; XXX: Fix CertPoint declaration.
+    (void)raw; // avoid warnings about unused method parameter; TODO: Add a SimulateUse() macro.
 #endif
+    assert(pCert);
+    return pCert;
 }
 
 void
 Security::HandshakeParser::parseServerCertificates(const SBuf &raw)
 {
+#if USE_OPENSSL
     Parser::BinaryTokenizer tkList(raw);
     const SBuf clist = tkList.pstring24("CertificateList");
     Must(tkList.atEnd()); // no leftovers after all certificates
 
     Parser::BinaryTokenizer tkItems(clist);
     while (!tkItems.atEnd()) {
-        Security::CertPointer cert;
-        ParseCertificate(tkItems.pstring24("Certificate"), cert);
-        serverCertificates.push_back(cert);
+        if (Security::CertPointer cert = ParseCertificate(tkItems.pstring24("Certificate")))
+            serverCertificates.push_back(cert);
         debugs(83, 7, "parsed " << serverCertificates.size() << " certificates so far");
     }
-
+#else
+    debugs(83, 7, "no support for CertificateList parsing; ignoring " << raw.length() << " bytes");
+#endif
 }
 
 /// A helper function to create a set of all supported TLS extensions

diff --git a/src/security/Handshake.h b/src/security/Handshake.h
index 82a88e55503d371deac65a619ab7930ce8983df8..0aff1ee6b1d08488040684dbdb3dcbde74f89c9d 100644 (file)
--- a/src/security/Handshake.h
+++ b/src/security/Handshake.h
@@ -101,7 +101,7 @@ private:
     void parseV23Ciphers(const SBuf &raw);
 
     void parseServerCertificates(const SBuf &raw);
-    static void ParseCertificate(const SBuf &raw, CertPointer &cert);
+    static CertPointer ParseCertificate(const SBuf &raw);
 
     unsigned int currentContentType; ///< The current TLS/SSL record content type