From: Christos Tsantilas Date: Tue, 10 Sep 2013 07:22:44 +0000 (-0600) Subject: Handle infinite certificate validation loops caused by OpenSSL bug #3090. X-Git-Tag: SQUID_3_3_10~40 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c943f8b62678779abf589618706174a8aff01f30;p=thirdparty%2Fsquid.git 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 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 05bc028ec2..6557bf7a58 100644 --- 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 211dcf84ce..ed13fbcbf8 100644 --- 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 bc83b57710..6ff62ce4e0 100644 --- a/src/ssl/ErrorDetail.cc +++ b/src/ssl/ErrorDetail.cc @@ -19,8 +19,10 @@ 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_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 a8c1a1bd35..dda2a3ab11 100644 --- 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(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 (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 efff316471..894d39deb7 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;