From a34d1d2dd92eb1761eea27de5770ce77bd9557ce Mon Sep 17 00:00:00 2001 From: Christos Tsantilas Date: Wed, 25 May 2016 11:55:35 +0300 Subject: [PATCH] New Security::CertList type to replace Ssl::X509_STACK_Pointer references in HandshakeParser class. The Security::CertList type is an Security::CertPointer list. --- src/security/Handshake.cc | 34 +++++++++++---------------- src/security/Handshake.h | 12 +++------- src/security/forward.h | 2 ++ src/ssl/PeerConnector.cc | 8 +++---- src/ssl/bio.h | 2 +- src/ssl/support.cc | 49 +++++++++++++++++++++++---------------- src/ssl/support.h | 4 ++-- 7 files changed, 54 insertions(+), 57 deletions(-) diff --git a/src/security/Handshake.cc b/src/security/Handshake.cc index cd8f418d7f..7dfc7cc04f 100644 --- 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(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 747519f5fb..5a2eca6ab7 100644 --- 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 @@ -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 54d6b06fb3..e14f475a0f 100644 --- a/src/security/forward.h +++ b/src/security/forward.h @@ -58,6 +58,8 @@ typedef void *CrlPointer; typedef std::list CertRevokeList; +typedef std::list CertList; + #if USE_OPENSSL CtoCpp1(DH_free, DH *); typedef Security::LockingPointer DhePointer; diff --git a/src/ssl/PeerConnector.cc b/src/ssl/PeerConnector.cc index cf1cb16e07..e99e8b099a 100644 --- 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(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 75f8665ff0..ef4d0ba7af 100644 --- 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 99818d9ddb..eed7b39084 100644 --- 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 &URIs, Ssl::X509_STACK_Pointer const &serverCertificates) +Ssl::missingChainCertificatesUrls(std::queue &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 ada33bae6d..c638ddcf8f 100644 --- 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 &URIs, Ssl::X509_STACK_Pointer const &serverCertificates); +void missingChainCertificatesUrls(std::queue &URIs, Security::CertList const &serverCertificates); /** \ingroup ServerProtocolSSLAPI -- 2.47.2