From: Christos Tsantilas Date: Mon, 4 Dec 2017 10:07:10 +0000 (+0200) Subject: Validate the shortest certificate chain X-Git-Tag: SQUID_4_0_22~1 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=998063281edb62893a6cf522adbca8b4e5092ecc;p=thirdparty%2Fsquid.git Validate the shortest certificate chain Do not download remote certificate for issuer X if the received server certificate (signed by X) can be validated using a locally available CA certificate. According to our tests, a typical browser does not follow 'CA Issuers' references to download 'missing' certificates when the browser can validate the origin server certificate (or its chain) using a local CA certificate. Avoiding unnecessary validations and downloads not only saves time, but can prevent validation failures as well! This is Measurement Factory project --- diff --git a/src/security/PeerConnector.cc b/src/security/PeerConnector.cc index 4aec20c415..d3c9c16dd8 100644 --- a/src/security/PeerConnector.cc +++ b/src/security/PeerConnector.cc @@ -660,8 +660,9 @@ Security::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)); + ContextPointer ctx(getTlsContext()); const Security::CertList &certsList = srvBio->serverCertificatesIfAny(); - if (const char *issuerUri = Ssl::uriOfIssuerIfMissing(cert, certsList)) { + if (const char *issuerUri = Ssl::uriOfIssuerIfMissing(cert, certsList, ctx)) { urlsOfMissingCerts.push(SBuf(issuerUri)); } Ssl::SSL_add_untrusted_cert(session.get(), cert); @@ -698,7 +699,8 @@ Security::PeerConnector::checkForMissingCertificates() if (certs.size()) { debugs(83, 5, "SSL server sent " << certs.size() << " certificates"); - Ssl::missingChainCertificatesUrls(urlsOfMissingCerts, certs); + ContextPointer ctx(getTlsContext()); + Ssl::missingChainCertificatesUrls(urlsOfMissingCerts, certs, ctx); if (urlsOfMissingCerts.size()) { startCertDownloading(urlsOfMissingCerts.front()); urlsOfMissingCerts.pop(); diff --git a/src/ssl/support.cc b/src/ssl/support.cc index 9347f5d596..b23006dca8 100644 --- a/src/ssl/support.cc +++ b/src/ssl/support.cc @@ -1046,18 +1046,49 @@ findCertIssuer(Security::CertList const &list, X509 *cert) return false; } +/// \return true if the cert issuer exist in the certificates stored in connContext +static bool +issuerExistInCaDb(X509 *cert, const Security::ContextPointer &connContext) +{ + if (!connContext) + return false; + + X509_STORE_CTX *storeCtx = X509_STORE_CTX_new(); + if (!storeCtx) { + debugs(83, DBG_IMPORTANT, "Failed to allocate STORE_CTX object"); + return false; + } + + bool gotIssuer = false; + X509_STORE *store = SSL_CTX_get_cert_store(connContext.get()); + if (X509_STORE_CTX_init(storeCtx, store, nullptr, nullptr)) { + X509 *issuer = nullptr; + gotIssuer = (X509_STORE_CTX_get1_issuer(&issuer, storeCtx, cert) > 0); + if (issuer) + X509_free(issuer); + } else { + const int ssl_error = ERR_get_error(); + debugs(83, DBG_IMPORTANT, "Failed to initialize STORE_CTX object: " << Security::ErrorString(ssl_error)); + } + X509_STORE_CTX_free(storeCtx); + + return gotIssuer; +} + const char * -Ssl::uriOfIssuerIfMissing(X509 *cert, Security::CertList const &serverCertificates) +Ssl::uriOfIssuerIfMissing(X509 *cert, Security::CertList const &serverCertificates, const Security::ContextPointer &context) { if (!cert || !serverCertificates.size()) return nullptr; if (!findCertIssuer(serverCertificates, cert)) { //if issuer is missing - if (!findCertIssuerFast(SquidUntrustedCerts, cert)) { - // and issuer not found in local untrusted certificates database - if (const char *issuerUri = hasAuthorityInfoAccessCaIssuers(cert)) { - // There is a URI where we can download a certificate. + if (const char *issuerUri = hasAuthorityInfoAccessCaIssuers(cert)) { + // There is a URI where we can download a certificate. + if (!findCertIssuerFast(SquidUntrustedCerts, cert) && + !issuerExistInCaDb(cert, context)) { + // and issuer not found in local databases containing + // untrusted certificates and trusted CA certificates return issuerUri; } } @@ -1066,13 +1097,13 @@ Ssl::uriOfIssuerIfMissing(X509 *cert, Security::CertList const &serverCertificat } void -Ssl::missingChainCertificatesUrls(std::queue &URIs, Security::CertList const &serverCertificates) +Ssl::missingChainCertificatesUrls(std::queue &URIs, Security::CertList const &serverCertificates, const Security::ContextPointer &context) { if (!serverCertificates.size()) return; for (const auto &i : serverCertificates) { - if (const char *issuerUri = uriOfIssuerIfMissing(i.get(), serverCertificates)) + if (const char *issuerUri = uriOfIssuerIfMissing(i.get(), serverCertificates, context)) URIs.push(SBuf(issuerUri)); } } diff --git a/src/ssl/support.h b/src/ssl/support.h index 1f7c5e0f43..7c0cf2e16c 100644 --- a/src/ssl/support.h +++ b/src/ssl/support.h @@ -173,13 +173,13 @@ 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, Security::CertList const &serverCertificates); +const char *uriOfIssuerIfMissing(X509 *cert, Security::CertList const &serverCertificates, const Security::ContextPointer &context); /** * 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, Security::CertList const &serverCertificates); +void missingChainCertificatesUrls(std::queue &URIs, Security::CertList const &serverCertificates, const Security::ContextPointer &context); /** \ingroup ServerProtocolSSLAPI