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 f46459a465ce141c274d9f4e2ecf041000fac2fd..9160ba3201fef9ffe223947895912d994117f0cc 100644 (file)
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -2453,6 +2453,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 8ce56aa10d936fe5dfae3285fea28a41ebd03c5f..c713470ee679a86507621fde882a6699990ae3f5 100644 (file)
--- a/src/globals.h
+++ b/src/globals.h
@@ -137,6 +137,7 @@ 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_cert_chain;  /* -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 779520858f1baf52060205659fced1de26f35c2a..3d5e65531b500d079404d4d7ca4956e3958eb3c6 100644 (file)
--- a/src/ssl/ErrorDetail.cc
+++ b/src/ssl/ErrorDetail.cc
@@ -19,6 +19,8 @@ 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_ERR_SSL_HANDSHAKE,

diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index b66279a48a8da74d5a2e0198e416b261e91e022a..584e7a2cdc6d4885252417cf8e6b919f70219424 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -239,6 +239,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);
 
@@ -282,30 +299,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::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);
+        // 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;
-            filledCheck->serverCert.reset(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);
+            // 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);
+                }
             }
         }
     }
@@ -651,6 +672,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;
+}
+
 /// \ingroup ServerProtocolSSLInternal
 /// Callback handler function to release STACK_OF(X509) "ex" data stored
 /// in an SSL object.
@@ -713,6 +743,7 @@ ssl_initialize(void)
     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_cert_chain = SSL_get_ex_new_index(0, (void *) "ssl_cert_chain", NULL, NULL, &ssl_free_CertChain);
+    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 0a183b606ded91f021a4bb7294bfad553b4f4f09..9eb9d848ef6f004d82fe24a1122a3dba86ebf43d 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;