New Security::CertList type to replace Ssl::X509_STACK_Pointer references

in HandshakeParser class.

The Security::CertList type is an Security::CertPointer list.

diff --git a/src/security/Handshake.cc b/src/security/Handshake.cc
index cd8f418d7f81ffce03cffd2d88514db9eb86d2e4..7dfc7cc04f6976bc6ef150d7e3a7ec1e7c14277d 100644 (file)
--- a/src/security/Handshake.cc
+++ b/src/security/Handshake.cc
@@ -535,17 +535,19 @@ Security::HandshakeParser::parseHello(const SBuf &data)
     return false; // unreached
 }
 
-#if USE_OPENSSL
-X509 *
-Security::HandshakeParser::ParseCertificate(const SBuf &raw)
+void
+Security::HandshakeParser::ParseCertificate(const SBuf &raw, Security::CertPointer &pCert)
 {
+#if USE_OPENSSL
     typedef const unsigned char *x509Data;
     const x509Data x509Start = reinterpret_cast<x509Data>(raw.rawContent());
     x509Data x509Pos = x509Start;
     X509 *x509 = d2i_X509(nullptr, &x509Pos, raw.length());
     Must(x509); // successfully parsed
     Must(x509Pos == x509Start + raw.length()); // no leftovers
-    return x509;
+    pCert.resetAndLock(x509);
+#else
+#endif
 }
 
 void
@@ -557,11 +559,10 @@ Security::HandshakeParser::parseServerCertificates(const SBuf &raw)
 
     Parser::BinaryTokenizer tkItems(clist);
     while (!tkItems.atEnd()) {
-        X509 *cert = ParseCertificate(tkItems.pstring24("Certificate"));
-        if (!serverCertificates.get())
-            serverCertificates.reset(sk_X509_new_null());
-        sk_X509_push(serverCertificates.get(), cert);
-        debugs(83, 7, "parsed " << sk_X509_num(serverCertificates.get()) << " certificates so far");
+        Security::CertPointer cert;
+        ParseCertificate(tkItems.pstring24("Certificate"), cert);
+        serverCertificates.push_back(cert);
+        debugs(83, 7, "parsed " << serverCertificates.size() << " certificates so far");
     }
 
 }
@@ -571,6 +572,8 @@ static
 Security::Extensions
 Security::SupportedExtensions()
 {
+#if USE_OPENSSL
+
     // optimize lookup speed by reserving the number of values x3, approximately
     Security::Extensions extensions(64);
 
@@ -654,20 +657,9 @@ Security::SupportedExtensions()
 #endif
 
     return extensions; // might be empty
-}
-
 #else
 
-void
-Security::HandshakeParser::parseServerCertificates(const SBuf &raw)
-{
-}
-
-static
-Security::Extensions
-Security::SupportedExtensions()
-{
     return Extensions(); // no extensions are supported without OpenSSL
-}
 #endif
+}
 

diff --git a/src/security/Handshake.h b/src/security/Handshake.h
index 747519f5fba1070dfda6f7aeae72e5b7a41dde43..5a2eca6ab74b3f95100068feac5ea520374282f0 100644 (file)
--- a/src/security/Handshake.h
+++ b/src/security/Handshake.h
@@ -12,9 +12,7 @@
 #include "anyp/ProtocolVersion.h"
 #include "base/YesNoNone.h"
 #include "parser/BinaryTokenizer.h"
-#if USE_OPENSSL
-#include "ssl/gadgets.h"
-#endif
+#include "security/forward.h"
 
 #include <unordered_set>
 
@@ -71,9 +69,7 @@ public:
 
     TlsDetails::Pointer details; ///< TLS handshake meta info or nil.
 
-#if USE_OPENSSL
-    Ssl::X509_STACK_Pointer serverCertificates; ///< parsed certificates chain
-#endif
+    Security::CertList serverCertificates; ///< parsed certificates chain
 
     ParserState state; ///< current parsing state.
 
@@ -105,9 +101,7 @@ private:
     void parseV23Ciphers(const SBuf &raw);
 
     void parseServerCertificates(const SBuf &raw);
-#if USE_OPENSSL
-    static X509 *ParseCertificate(const SBuf &raw);
-#endif
+    static void ParseCertificate(const SBuf &raw, CertPointer &cert);
 
     unsigned int currentContentType; ///< The current TLS/SSL record content type
 

diff --git a/src/security/forward.h b/src/security/forward.h
index 54d6b06fb34fccf967cdc2ace88a7fd46dce1110..e14f475a0f514a0c51191ea34181dbbc427a07e1 100644 (file)
--- a/src/security/forward.h
+++ b/src/security/forward.h
@@ -58,6 +58,8 @@ typedef void *CrlPointer;
 
 typedef std::list<Security::CrlPointer> CertRevokeList;
 
+typedef std::list<Security::CertPointer> CertList;
+
 #if USE_OPENSSL
 CtoCpp1(DH_free, DH *);
 typedef Security::LockingPointer<DH, DH_free_cpp, CRYPTO_LOCK_DH> DhePointer;

diff --git a/src/ssl/PeerConnector.cc b/src/ssl/PeerConnector.cc
index cf1cb16e0705a427366229e59bd1c94fc8290980..e99e8b099a526c9f2c47e18cda4763416c2f190f 100644 (file)
--- a/src/ssl/PeerConnector.cc
+++ b/src/ssl/PeerConnector.cc
@@ -564,7 +564,7 @@ Ssl::PeerConnector::certDownloadingDone(SBuf &obj, int downloadStatus)
     if (X509 *cert = d2i_X509(NULL, &raw, obj.length())) {
         char buffer[1024];
         debugs(81, 5, "Retrieved certificate: " << X509_NAME_oneline(X509_get_subject_name(cert), buffer, 1024));
-        const Ssl::X509_STACK_Pointer &certsList = srvBio->serverCertificatesIfAny();
+        const Security::CertList &certsList = srvBio->serverCertificatesIfAny();
         if (const char *issuerUri = Ssl::uriOfIssuerIfMissing(cert,  certsList)) {
             urlsOfMissingCerts.push(SBuf(issuerUri));
         }
@@ -596,10 +596,10 @@ Ssl::PeerConnector::checkForMissingCertificates ()
     Security::SessionPtr ssl = fd_table[fd].ssl.get();
     BIO *b = SSL_get_rbio(ssl);
     Ssl::ServerBio *srvBio = static_cast<Ssl::ServerBio *>(b->ptr);
-    const Ssl::X509_STACK_Pointer &certs = srvBio->serverCertificatesIfAny();
+    const Security::CertList &certs = srvBio->serverCertificatesIfAny();
 
-    if (certs.get() && sk_X509_num(certs.get())) {
-        debugs(83, 5, "SSL server sent " << sk_X509_num(certs.get()) << " certificates");
+    if (certs.size()) {
+        debugs(83, 5, "SSL server sent " << certs.size() << " certificates");
         Ssl::missingChainCertificatesUrls(urlsOfMissingCerts, certs);
         if (urlsOfMissingCerts.size()) {
             startCertDownloading(urlsOfMissingCerts.front());

diff --git a/src/ssl/bio.h b/src/ssl/bio.h
index 75f8665ff00e9f061bce9cba67ce830ec4a21d6b..ef4d0ba7afa8b7666a1fd3509fe886e891c96c39 100644 (file)
--- a/src/ssl/bio.h
+++ b/src/ssl/bio.h
@@ -158,7 +158,7 @@ public:
     /// Return true if the Server Hello parsing failed
     bool gotHelloFailed() const { return (parsedHandshake && parseError); }
 
-    const Ssl::X509_STACK_Pointer &serverCertificatesIfAny() { return parser_.serverCertificates; } /* XXX: may be nil */
+    const Security::CertList &serverCertificatesIfAny() { return parser_.serverCertificates; }
 
     /// \return the TLS Details advertised by TLS server.
     const Security::TlsDetails::Pointer &receivedHelloDetails() const {return parser_.details;}

diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index 99818d9ddbe218a5d478504b15536d0a1e898cbe..eed7b39084042e9781b068ec4128072906c5d04c 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -1170,28 +1170,23 @@ findCertByIssuerFast(Ssl::CertsIndexedList &list, X509 *cert)
 }
 
 /// slowly find a certificate with a given issuer using linear search
-static X509 *
-findCertByIssuerSlowly(STACK_OF(X509) *sk, X509 *cert)
+static bool
+findCertIssuer(Security::CertList const &list, X509 *cert)
 {
-    if (!sk)
-        return NULL;
-
-    const int skItemsNum = sk_X509_num(sk);
-    for (int i = 0; i < skItemsNum; ++i) {
-        X509 *issuer = sk_X509_value(sk, i);
-        if (X509_check_issued(issuer, cert) == X509_V_OK)
-            return issuer;
+    for (Security::CertList::const_iterator it = list.begin(); it != list.end(); ++it) {
+        if (X509_check_issued(it->get(), cert) == X509_V_OK)
+            return true;
     }
-    return NULL;
+    return false;
 }
 
 const char *
-Ssl::uriOfIssuerIfMissing(X509 *cert,  Ssl::X509_STACK_Pointer const &serverCertificates)
+Ssl::uriOfIssuerIfMissing(X509 *cert, Security::CertList const &serverCertificates)
 {
-    if (!cert || !serverCertificates.get())
+    if (!cert || !serverCertificates.size())
         return nullptr;
 
-    if (!findCertByIssuerSlowly(serverCertificates.get(), cert)) {
+    if (!findCertIssuer(serverCertificates, cert)) {
         //if issuer is missing
         if (!findCertByIssuerFast(SquidUntrustedCerts, cert)) {
             // and issuer not found in local untrusted certificates database 
@@ -1206,14 +1201,13 @@ Ssl::uriOfIssuerIfMissing(X509 *cert,  Ssl::X509_STACK_Pointer const &serverCert
 }
 
 void
-Ssl::missingChainCertificatesUrls(std::queue<SBuf> &URIs, Ssl::X509_STACK_Pointer const &serverCertificates)
+Ssl::missingChainCertificatesUrls(std::queue<SBuf> &URIs, Security::CertList const &serverCertificates)
 {
-    if (!serverCertificates.get())
+    if (!serverCertificates.size())
         return;
 
-    for (int i = 0; i < sk_X509_num(serverCertificates.get()); ++i) {
-        X509 *cert = sk_X509_value(serverCertificates.get(), i);
-        if (const char *issuerUri = uriOfIssuerIfMissing(cert, serverCertificates))
+    for (Security::CertList::const_iterator it = serverCertificates.begin(); it != serverCertificates.end(); ++it) {
+        if (const char *issuerUri = uriOfIssuerIfMissing(it->get(), serverCertificates))
             URIs.push(SBuf(issuerUri));
     }
 }
@@ -1232,6 +1226,21 @@ Ssl::SSL_add_untrusted_cert(SSL *ssl, X509 *cert)
     sk_X509_push(untrustedStack, cert);
 }
 
+static X509 *
+sk_x509_findCertByIssuer(STACK_OF(X509) *sk, X509 *cert)
+{
+    if (!sk)
+        return NULL;
+
+    const int skItemsNum = sk_X509_num(sk);
+    for (int i = 0; i < skItemsNum; ++i) {
+        X509 *issuer = sk_X509_value(sk, i);
+        if (X509_check_issued(issuer, cert) == X509_V_OK)
+            return issuer;
+    }
+    return NULL;
+}
+
 /// add missing issuer certificates to untrustedCerts
 static void
 completeIssuers(X509_STORE_CTX *ctx, STACK_OF(X509) *untrustedCerts)
@@ -1249,7 +1258,7 @@ completeIssuers(X509_STORE_CTX *ctx, STACK_OF(X509) *untrustedCerts)
         }
 
         // untrustedCerts is short, not worth indexing
-        X509 *issuer = findCertByIssuerSlowly(untrustedCerts, current);
+        X509 *issuer = sk_x509_findCertByIssuer(untrustedCerts, current);
         if (!issuer) {
             if ((issuer = findCertByIssuerFast(SquidUntrustedCerts, current)))
                 sk_X509_push(untrustedCerts, issuer);

diff --git a/src/ssl/support.h b/src/ssl/support.h
index ada33bae6dfc14b93104c8a85b000fe428d0477c..c638ddcf8f1d5ee93cc7d59ffdcec742b55a564f 100644 (file)
--- a/src/ssl/support.h
+++ b/src/ssl/support.h
@@ -220,14 +220,14 @@ void SSL_add_untrusted_cert(SSL *ssl, X509 *cert);
  * Searches in serverCertificates list for the cert issuer and if not found
  * and Authority Info Access of cert provides a URI return it.
  */
-const char *uriOfIssuerIfMissing(X509 *cert,  Ssl::X509_STACK_Pointer const &serverCertificates);
+const char *uriOfIssuerIfMissing(X509 *cert,  Security::CertList const &serverCertificates);
 
 /**
  \ingroup ServerProtocolSSLAPI
  * Fill URIs queue with the uris of missing certificates from serverCertificate chain
  * if this information provided by Authority Info Access.
  */
-void missingChainCertificatesUrls(std::queue<SBuf> &URIs, Ssl::X509_STACK_Pointer const &serverCertificates);
+void missingChainCertificatesUrls(std::queue<SBuf> &URIs, Security::CertList const &serverCertificates);
 
 /**
   \ingroup ServerProtocolSSLAPI