Validate the shortest certificate chain (#84)

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 0baa345a68e2df991f06a182162c4d2a56c9073e..e1435884cc5f1ec97123a52f617e2f74de4ee3a6 100644 (file)
--- a/src/security/PeerConnector.cc
+++ b/src/security/PeerConnector.cc
@@ -656,8 +656,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);
@@ -694,7 +695,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 9347f5d596a430277c1f2fd29d47df80bc701de6..b23006dca8cb8b265a06cd568d4257ab7f37eb75 100644 (file)
--- 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<SBuf> &URIs, Security::CertList const &serverCertificates)
+Ssl::missingChainCertificatesUrls(std::queue<SBuf> &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 1f7c5e0f43420ac1f48c06fd3cc5e65319b7ab55..7c0cf2e16c38d2de924c491697fbbee82daa88ae 100644 (file)
--- 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<SBuf> &URIs, Security::CertList const &serverCertificates);
+void missingChainCertificatesUrls(std::queue<SBuf> &URIs, Security::CertList const &serverCertificates, const Security::ContextPointer &context);
 
 /**
   \ingroup ServerProtocolSSLAPI