Handle infinite certificate validation loops caused by OpenSSL bug #3090.

If OpenSSL is stuck in a validation loop, Squid breaks the loop and
triggers a
new custom SQUID_X509_V_ERR_INFINITE_VALIDATION SSL validation error.
That
error cannot be bypassed using sslproxy_cert_error because to break the
loop
Squid has to tell OpenSSL that the certificate is invalid, which
terminates
the SSL connection.

Validation loops exceeding SQUID_CERT_VALIDATION_ITERATION_MAX
iterations
are deemed infinite. That macro is defined to be 16384, but that default
can
be overwritten using CPPFLAGS.

  This is a Measurement Factory project

diff --git a/errors/templates/error-details.txt b/errors/templates/error-details.txt
index dc1d3b47b395c6699c31e45eab4474e4ac84e88e..52b70617aa5592e4d5da08a9e10ade84082fb129 100644 (file)
--- a/errors/templates/error-details.txt
+++ b/errors/templates/error-details.txt
@@ -1,3 +1,7 @@
+name: SQUID_X509_V_ERR_INFINITE_VALIDATION
+detail: "%ssl_error_descr: %ssl_subject"
+descr: "Cert validation infinite loop detected"
+
 name: SQUID_ERR_SSL_HANDSHAKE
 detail: "%ssl_error_descr: %ssl_lib_error"
 descr: "Handshake with SSL server failed"

diff --git a/src/cf.data.pre b/src/cf.data.pre
index 05bc028ec2c370346ad8d237d39006f768aae1c8..6557bf7a58facc59b196ba2ce599847fa5e91037 100644 (file)
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -2295,6 +2295,9 @@ DOC_START
        Without this option, all server certificate validation errors
        terminate the transaction to protect Squid and the client.
 
+       SQUID_X509_V_ERR_INFINITE_VALIDATION error cannot be bypassed
+       but should not happen unless your OpenSSL library is buggy.
+
        SECURITY WARNING:
                Bypassing validation errors is dangerous because an
                error usually implies that the server cannot be trusted

diff --git a/src/globals.h b/src/globals.h
index 211dcf84ce28f0949ec633a0f5b57e0ad41d1630..ed13fbcbf812b89231517bdca5efc1c8900901ed 100644 (file)
--- a/src/globals.h
+++ b/src/globals.h
@@ -134,6 +134,7 @@ extern int ssl_ex_index_cert_error_check;   /* -1 */
 extern int ssl_ex_index_ssl_error_detail;      /* -1 */
 extern int ssl_ex_index_ssl_peeked_cert;      /* -1 */
 extern int ssl_ex_index_ssl_errors;   /* -1 */
+extern int ssl_ex_index_ssl_validation_counter;  /* -1 */
 
 extern const char *external_acl_message;      /* NULL */
 extern int opt_send_signal;    /* -1 */

diff --git a/src/ssl/ErrorDetail.cc b/src/ssl/ErrorDetail.cc
index bc83b577104ff2d20a0cb607272c0cf23f48cd9b..6ff62ce4e0a6ff7c773454a4be0fc17dd63f9bef 100644 (file)
--- a/src/ssl/ErrorDetail.cc
+++ b/src/ssl/ErrorDetail.cc
@@ -19,8 +19,10 @@ typedef std::map<Ssl::ssl_error_t, const SslErrorEntry *> SslErrors;
 SslErrors TheSslErrors;
 
 static SslErrorEntry TheSslErrorArray[] = {
+    {SQUID_X509_V_ERR_INFINITE_VALIDATION,
+        "SQUID_X509_V_ERR_INFINITE_VALIDATION"},
     {SQUID_X509_V_ERR_CERT_CHANGE,
-        "SQUID_X509_V_ERR_CERT_CHANGE"},
+     "SQUID_X509_V_ERR_CERT_CHANGE"},
     {SQUID_ERR_SSL_HANDSHAKE,
      "SQUID_ERR_SSL_HANDSHAKE"},
     {SQUID_X509_V_ERR_DOMAIN_MISMATCH,

diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index a8c1a1bd3594b3334f574e5c1de69bfbbb11646f..dda2a3ab11b32e87487adde79857c552d572b320 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -238,6 +238,23 @@ ssl_verify_cb(int ok, X509_STORE_CTX * ctx)
     X509_NAME_oneline(X509_get_subject_name(peer_cert), buffer,
                       sizeof(buffer));
 
+    // detect infinite loops
+    uint32_t *validationCounter = static_cast<uint32_t *>(SSL_get_ex_data(ssl, ssl_ex_index_ssl_validation_counter));
+    if (!validationCounter) {
+        validationCounter = new uint32_t(1);
+        SSL_set_ex_data(ssl, ssl_ex_index_ssl_validation_counter, validationCounter);
+    } else {
+        // overflows allowed if SQUID_CERT_VALIDATION_ITERATION_MAX >= UINT32_MAX
+        (*validationCounter)++;
+    }
+
+    if ((*validationCounter) >= SQUID_CERT_VALIDATION_ITERATION_MAX) {
+        ok = 0; // or the validation loop will never stop
+        error_no = SQUID_X509_V_ERR_INFINITE_VALIDATION;
+        debugs(83, 2, "SQUID_X509_V_ERR_INFINITE_VALIDATION: " <<
+               *validationCounter << " iterations while checking " << buffer);
+    }
+
     if (ok) {
         debugs(83, 5, "SSL Certificate signature OK: " << buffer);
 
@@ -276,18 +293,34 @@ ssl_verify_cb(int ok, X509_STORE_CTX * ctx)
         else
             debugs(83, DBG_IMPORTANT, "SSL unknown certificate error " << error_no << " in " << buffer);
 
-        if (check) {
-            ACLFilledChecklist *filledCheck = Filled(check);
-            assert(!filledCheck->sslErrors);
-            filledCheck->sslErrors = new Ssl::Errors(error_no);
-            if (check->fastCheck() == ACCESS_ALLOWED) {
-                debugs(83, 3, "bypassing SSL error " << error_no << " in " << buffer);
-                ok = 1;
-            } else {
-                debugs(83, 5, "confirming SSL error " << error_no);
+        // Check if the certificate error can be bypassed.
+        // Infinity validation loop errors can not bypassed.
+        if (error_no != SQUID_X509_V_ERR_INFINITE_VALIDATION) {
+            if (check) {
+                ACLFilledChecklist *filledCheck = Filled(check);
+                assert(!filledCheck->sslErrors);
+                filledCheck->sslErrors = new Ssl::CertErrors(Ssl::CertError(error_no, broken_cert));
+                filledCheck->serverCert.resetAndLock(peer_cert);
+                if (check->fastCheck() == ACCESS_ALLOWED) {
+                    debugs(83, 3, "bypassing SSL error " << error_no << " in " << buffer);
+                    ok = 1;
+                } else {
+                    debugs(83, 5, "confirming SSL error " << error_no);
+                }
+                delete filledCheck->sslErrors;
+                filledCheck->sslErrors = NULL;
+                filledCheck->serverCert.reset(NULL);
             }
-            delete filledCheck->sslErrors;
-            filledCheck->sslErrors = NULL;
+            // If the certificate validator is used then we need to allow all errors and
+            // pass them to certficate validator for more processing
+            else if (Ssl::TheConfig.ssl_crt_validator) {
+                ok = 1;
+                // Check if we have stored certificates chain. Store if not.
+                if (!SSL_get_ex_data(ssl, ssl_ex_index_ssl_cert_chain)) {
+                    STACK_OF(X509) *certStack = X509_STORE_CTX_get1_chain(ctx);
+                    if (certStack && !SSL_set_ex_data(ssl, ssl_ex_index_ssl_cert_chain, certStack))
+                        sk_X509_pop_free(certStack, X509_free);
+                }
         }
     }
 
@@ -632,6 +665,15 @@ ssl_free_SslErrors(void *, void *ptr, CRYPTO_EX_DATA *,
     delete errs;
 }
 
+// "free" function for SSL_get_ex_new_index("ssl_ex_index_ssl_validation_counter")
+static void
+ssl_free_int(void *, void *ptr, CRYPTO_EX_DATA *,
+             int, long, void *)
+{
+    uint32_t *counter = static_cast <uint32_t *>(ptr);
+    delete counter;
+}
+
 // "free" function for X509 certificates
 static void
 ssl_free_X509(void *, void *ptr, CRYPTO_EX_DATA *,
@@ -682,6 +724,7 @@ ssl_initialize(void)
     ssl_ex_index_ssl_error_detail = SSL_get_ex_new_index(0, (void *) "ssl_error_detail", NULL, NULL, &ssl_free_ErrorDetail);
     ssl_ex_index_ssl_peeked_cert  = SSL_get_ex_new_index(0, (void *) "ssl_peeked_cert", NULL, NULL, &ssl_free_X509);
     ssl_ex_index_ssl_errors =  SSL_get_ex_new_index(0, (void *) "ssl_errors", NULL, NULL, &ssl_free_SslErrors);
+    ssl_ex_index_ssl_validation_counter = SSL_get_ex_new_index(0, (void *) "ssl_validation_counter", NULL, NULL, &ssl_free_int);
 }
 
 /// \ingroup ServerProtocolSSLInternal

diff --git a/src/ssl/support.h b/src/ssl/support.h
index efff316471e26bcabc93b20e607047a51498911f..894d39deb79f46cee7a6a7c04ea28e7ef62daa69 100644 (file)
--- a/src/ssl/support.h
+++ b/src/ssl/support.h
@@ -55,6 +55,7 @@
  */
 
 // Custom SSL errors; assumes all official errors are positive
+#define SQUID_X509_V_ERR_INFINITE_VALIDATION -4
 #define SQUID_X509_V_ERR_CERT_CHANGE -3
 #define SQUID_ERR_SSL_HANDSHAKE -2
 #define SQUID_X509_V_ERR_DOMAIN_MISMATCH -1
@@ -62,6 +63,14 @@
 #define SQUID_SSL_ERROR_MIN SQUID_X509_V_ERR_CERT_CHANGE
 #define SQUID_SSL_ERROR_MAX INT_MAX
 
+// Maximum certificate validation callbacks. OpenSSL versions exceeding this
+// limit are deemed stuck in an infinite validation loop (OpenSSL bug #3090)
+// and will trigger the SQUID_X509_V_ERR_INFINITE_VALIDATION error.
+// Can be set to a number up to UINT32_MAX
+#ifndef SQUID_CERT_VALIDATION_ITERATION_MAX
+#define SQUID_CERT_VALIDATION_ITERATION_MAX 16384
+#endif
+
 namespace AnyP
 {
 class PortCfg;