]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Validate the shortest certificate chain
authorChristos Tsantilas <christos@chtsanti.net>
Mon, 4 Dec 2017 10:07:10 +0000 (12:07 +0200)
committerAmos Jeffries <yadij@users.noreply.github.com>
Mon, 4 Dec 2017 15:09:29 +0000 (04:09 +1300)
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

src/security/PeerConnector.cc
src/ssl/support.cc
src/ssl/support.h

index 4aec20c415c30af3f5cf5890214d9144c3f2713d..d3c9c16dd8946638cb6018c2fbe9930dba69f630 100644 (file)
@@ -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();
index 9347f5d596a430277c1f2fd29d47df80bc701de6..b23006dca8cb8b265a06cd568d4257ab7f37eb75 100644 (file)
@@ -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));
     }
 }
index 1f7c5e0f43420ac1f48c06fd3cc5e65319b7ab55..7c0cf2e16c38d2de924c491697fbbee82daa88ae 100644 (file)
@@ -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