From 0ad3ff513e423e6d05f4a50995166d6440975cc6 Mon Sep 17 00:00:00 2001 From: Christos Tsantilas Date: Sat, 27 Jul 2013 16:37:29 +0300 Subject: [PATCH] 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 --- errors/templates/error-details.txt | 4 ++ src/cf.data.pre | 3 ++ src/globals.h | 1 + src/ssl/ErrorDetail.cc | 2 + src/ssl/support.cc | 77 +++++++++++++++++++++--------- src/ssl/support.h | 9 ++++ 6 files changed, 73 insertions(+), 23 deletions(-) diff --git a/errors/templates/error-details.txt b/errors/templates/error-details.txt index dc1d3b47b3..52b70617aa 100644 --- 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 f46459a465..9160ba3201 100644 --- 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 8ce56aa10d..c713470ee6 100644 --- 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 779520858f..3d5e65531b 100644 --- a/src/ssl/ErrorDetail.cc +++ b/src/ssl/ErrorDetail.cc @@ -19,6 +19,8 @@ typedef std::map 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 b66279a48a..584e7a2cdc 100644 --- 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(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 (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 0a183b606d..9eb9d848ef 100644 --- 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; -- 2.47.2