------------------------------------------------------------ revno: 12609 revision-id: squid3@treenet.co.nz-20130910072244-ivf8w0zblh9w4cy1 parent: squid3@treenet.co.nz-20130909092928-63ye90p9zwjvowpq author: Christos Tsantilas committer: Amos Jeffries branch nick: 3.3 timestamp: Tue 2013-09-10 01:22:44 -0600 message: 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 ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: squid3@treenet.co.nz-20130910072244-ivf8w0zblh9w4cy1 # target_branch: http://bzr.squid-cache.org/bzr/squid3/3.3 # testament_sha1: 6734e7f9e58c882f1f253d465a9d2f5750bc0e4f # timestamp: 2013-09-10 07:28:24 +0000 # source_branch: http://bzr.squid-cache.org/bzr/squid3/3.3 # base_revision_id: squid3@treenet.co.nz-20130909092928-\ # 63ye90p9zwjvowpq # # Begin patch === modified file 'errors/templates/error-details.txt' --- errors/templates/error-details.txt 2011-11-08 13:40:26 +0000 +++ errors/templates/error-details.txt 2013-09-10 07:22:44 +0000 @@ -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" === modified file 'src/cf.data.pre' --- src/cf.data.pre 2013-09-09 07:17:19 +0000 +++ src/cf.data.pre 2013-09-10 07:22:44 +0000 @@ -2295,6 +2295,9 @@ 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 === modified file 'src/globals.h' --- src/globals.h 2013-09-09 07:17:19 +0000 +++ src/globals.h 2013-09-10 07:22:44 +0000 @@ -134,6 +134,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_validation_counter; /* -1 */ extern const char *external_acl_message; /* NULL */ extern int opt_send_signal; /* -1 */ === modified file 'src/ssl/ErrorDetail.cc' --- src/ssl/ErrorDetail.cc 2012-08-14 11:53:07 +0000 +++ src/ssl/ErrorDetail.cc 2013-09-10 07:22:44 +0000 @@ -19,8 +19,10 @@ 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, === modified file 'src/ssl/support.cc' --- src/ssl/support.cc 2013-07-04 04:42:07 +0000 +++ src/ssl/support.cc 2013-09-10 07:22:44 +0000 @@ -238,6 +238,23 @@ 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 @@ 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); + // 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); + } + // 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; - } else { - debugs(83, 5, "confirming SSL error " << error_no); - } - delete filledCheck->sslErrors; - filledCheck->sslErrors = NULL; + // 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 @@ 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_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 === modified file 'src/ssl/support.h' --- src/ssl/support.h 2013-09-09 07:17:19 +0000 +++ src/ssl/support.h 2013-09-10 07:22:44 +0000 @@ -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;