From: Christos Tsantilas Date: Tue, 10 Jan 2012 10:41:18 +0000 (+0200) Subject: Check SSL server certificate on reconnect X-Git-Tag: BumpSslServerFirst.take04~12 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=e7bcc25fbd20d1a9557f004bc67829a482848fa8;p=thirdparty%2Fsquid.git Check SSL server certificate on reconnect In the case where the squid-to-server connection closed and we re-connect to server we need to check if the server certificate changed. Moreover we need to check stored certificates with the web server certificate to handle the case the web server certificate is updated. In this case a new certificate must generated to mimic the updated server certificate fields --- diff --git a/src/client_side.cc b/src/client_side.cc index 0fd1d5701a..7eb28aceda 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -3640,7 +3640,7 @@ ConnStateData::getSslContextStart() SSL_CTX * dynCtx = ssl_ctx_cache.find(host); if (dynCtx) { debugs(33, 5, HERE << "SSL certificate for " << host << " have found in cache"); - if (Ssl::verifySslCertificateDate(dynCtx)) { + if (Ssl::verifySslCertificate(dynCtx, bumpServerCert.get())) { debugs(33, 5, HERE << "Cached SSL certificate for " << host << " is valid"); getSslContextDone(dynCtx); return; diff --git a/src/forward.cc b/src/forward.cc index 38898f4538..5f940df8a0 100644 --- a/src/forward.cc +++ b/src/forward.cc @@ -773,6 +773,13 @@ FwdState::initiateSSL() SSL_set_ex_data(ssl, ssl_ex_index_cert_error_check, check); } + // store peeked cert to check SQUID_X509_V_ERR_CERT_CHANGE + X509 *peeked_cert; + if (request->clientConnectionManager.valid() && (peeked_cert = request->clientConnectionManager->getBumpServerCert())) { + CRYPTO_add(&(peeked_cert->references),1,CRYPTO_LOCK_X509); + SSL_set_ex_data(ssl, ssl_ex_index_ssl_peeked_cert, peeked_cert); + } + fd_table[fd].ssl = ssl; fd_table[fd].read_method = &ssl_read_method; fd_table[fd].write_method = &ssl_write_method; diff --git a/src/globals.h b/src/globals.h index c20e713b5e..048be61382 100644 --- a/src/globals.h +++ b/src/globals.h @@ -155,6 +155,7 @@ extern "C" { extern int ssl_ctx_ex_index_dont_verify_domain; /* -1 */ 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 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 8c2c2f8430..bb49b10815 100644 --- a/src/ssl/ErrorDetail.cc +++ b/src/ssl/ErrorDetail.cc @@ -16,6 +16,8 @@ typedef std::map SslErrors; SslErrors TheSslErrors; static SslErrorEntry TheSslErrorArray[] = { + {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/gadgets.cc b/src/ssl/gadgets.cc index 9c9512575f..4c08d1ce28 100644 --- a/src/ssl/gadgets.cc +++ b/src/ssl/gadgets.cc @@ -396,3 +396,93 @@ bool Ssl::sslDateIsInTheFuture(char const * date) return (X509_cmp_current_time(&tm) > 0); } + +/// Print the time represented by a ASN1_TIME struct to a string using GeneralizedTime format +static bool asn1timeToGeneralizedTimeStr(ASN1_TIME *aTime, char *buf, int bufLen) +{ + // ASN1_Time holds time to UTCTime or GeneralizedTime form. + // UTCTime has the form YYMMDDHHMMSS[Z | [+|-]offset] + // GeneralizedTime has the form YYYYMMDDHHMMSS[Z | [+|-] offset] + + // length should have space for data plus 2 extra bytes for the two extra year fields + // plus the '\0' char. + if ((aTime->length + 3) > bufLen) + return false; + + char *str; + if (aTime->type == V_ASN1_UTCTIME) { + if (aTime->data[0] > '5') { // RFC 2459, section 4.1.2.5.1 + buf[0] = '1'; + buf[1] = '9'; + } else { + buf[0] = '2'; + buf[1] = '0'; + } + str = buf +2; + } + else // if (aTime->type == V_ASN1_GENERALIZEDTIME) + str = buf; + + memcpy(str, aTime->data, aTime->length); + str[aTime->length] = '\0'; + return true; +} + +static int asn1time_cmp(ASN1_TIME *asnTime1, ASN1_TIME *asnTime2) +{ + char strTime1[64], strTime2[64]; + if (!asn1timeToGeneralizedTimeStr(asnTime1, strTime1, sizeof(strTime1))) + return -1; + if (!asn1timeToGeneralizedTimeStr(asnTime2, strTime2, sizeof(strTime2))) + return -1; + + return strcmp(strTime1, strTime2); +} + +bool Ssl::ssl_match_certificates(X509 *cert1, X509 *cert2) +{ + assert(cert1 && cert2); + X509_NAME *cert1_name = X509_get_subject_name(cert1); + X509_NAME *cert2_name = X509_get_subject_name(cert2); + if (X509_NAME_cmp(cert1_name, cert2_name) != 0) + return false; + + ASN1_TIME *aTime = X509_get_notBefore(cert1); + ASN1_TIME *bTime = X509_get_notBefore(cert2); + if (asn1time_cmp(aTime, bTime) != 0) + return false; + + aTime = X509_get_notAfter(cert1); + bTime = X509_get_notAfter(cert2); + if (asn1time_cmp(aTime, bTime) != 0) + return false; + + char *alStr1; + int alLen; + alStr1 = (char *)X509_alias_get0(cert1, &alLen); + char *alStr2 = (char *)X509_alias_get0(cert2, &alLen); + if ((!alStr1 && alStr2) || (alStr1 && !alStr2) || + (alStr1 && alStr2 && strcmp(alStr1, alStr2)) != 0) + return false; + + // Compare subjectAltName extension + STACK_OF(GENERAL_NAME) * cert1_altnames; + cert1_altnames = (STACK_OF(GENERAL_NAME)*)X509_get_ext_d2i(cert1, NID_subject_alt_name, NULL, NULL); + STACK_OF(GENERAL_NAME) * cert2_altnames; + cert2_altnames = (STACK_OF(GENERAL_NAME)*)X509_get_ext_d2i(cert2, NID_subject_alt_name, NULL, NULL); + bool match = true; + if (cert1_altnames) { + int numalts = sk_GENERAL_NAME_num(cert1_altnames); + for (int i = 0; match && i < numalts; i++) { + const GENERAL_NAME *aName = sk_GENERAL_NAME_value(cert1_altnames, i); + match = sk_GENERAL_NAME_find(cert2_altnames, aName); + } + } + else if (cert2_altnames) + match = false; + + sk_GENERAL_NAME_pop_free(cert1_altnames, GENERAL_NAME_free); + sk_GENERAL_NAME_pop_free(cert2_altnames, GENERAL_NAME_free); + + return match; +} diff --git a/src/ssl/gadgets.h b/src/ssl/gadgets.h index 25281837b8..97f60a129a 100644 --- a/src/ssl/gadgets.h +++ b/src/ssl/gadgets.h @@ -161,5 +161,11 @@ void readCertAndPrivateKeyFromFiles(X509_Pointer & cert, EVP_PKEY_Pointer & pkey */ bool sslDateIsInTheFuture(char const * date); +/** + \ingroup SslCrtdSslAPI + * Check if the major (mimicked) fields of the two certificates matches + \return true if the certificates matches false otherwise. +*/ +bool ssl_match_certificates(X509 *peer_cert, X509 *peeked_cert); } // namespace Ssl #endif // SQUID_SSL_GADGETS_H diff --git a/src/ssl/ssl_crtd.cc b/src/ssl/ssl_crtd.cc index cb2c97d1aa..1057284404 100644 --- a/src/ssl/ssl_crtd.cc +++ b/src/ssl/ssl_crtd.cc @@ -245,6 +245,15 @@ static bool proccessNewRequest(Ssl::CrtdMessage const & request_message, std::st db.find(cert_subject, cert, pkey); + if (cert.get() && certToMimic.get()) { + if (!Ssl::ssl_match_certificates(cert.get(), certToMimic.get())) { + // The certificate changed (renewed or other reason). + // Generete a new one with the updated fields. + cert.reset(NULL); + pkey.reset(NULL); + } + } + if (!cert || !pkey) { Ssl::X509_Pointer certToSign; Ssl::EVP_PKEY_Pointer pkeyToSign; diff --git a/src/ssl/support.cc b/src/ssl/support.cc index f763b010c8..8cb0d5e5bd 100644 --- a/src/ssl/support.cc +++ b/src/ssl/support.cc @@ -218,6 +218,7 @@ ssl_verify_cb(int ok, X509_STORE_CTX * ctx) const char *server = (const char *)SSL_get_ex_data(ssl, ssl_ex_index_server); void *dont_verify_domain = SSL_CTX_get_ex_data(sslctx, ssl_ctx_ex_index_dont_verify_domain); ACLChecklist *check = (ACLChecklist*)SSL_get_ex_data(ssl, ssl_ex_index_cert_error_check); + X509 *peeked_cert = (X509 *)SSL_get_ex_data(ssl, ssl_ex_index_ssl_peeked_cert); X509 *peer_cert = ctx->cert; X509_NAME_oneline(X509_get_subject_name(peer_cert), buffer, @@ -235,6 +236,15 @@ ssl_verify_cb(int ok, X509_STORE_CTX * ctx) } } + if (ok && peeked_cert) { + /*Check if the already peeked certificate match the new one*/ + if (X509_cmp(peer_cert, peeked_cert) != 0) { + debugs(83, 2, "SQUID_X509_V_ERR_CERT_CHANGE: Certificate " << buffer << " does not match peeked certificate"); + ok = 0; + error_no = SQUID_X509_V_ERR_CERT_CHANGE; + } + } + if (!ok) { if (const char *err_descr = Ssl::GetErrorDescr(error_no)) debugs(83, 5, err_descr << ": " << buffer); @@ -570,6 +580,15 @@ ssl_free_ErrorDetail(void *, void *ptr, CRYPTO_EX_DATA *, delete errDetail; } +// "free" function for X509 certificates +static void +ssl_free_X509(void *, void *ptr, CRYPTO_EX_DATA *, + int, long, void *) +{ + X509 *cert = static_cast (ptr); + X509_free(cert); +} + /// \ingroup ServerProtocolSSLInternal static void ssl_initialize(void) @@ -609,6 +628,7 @@ ssl_initialize(void) ssl_ctx_ex_index_dont_verify_domain = SSL_CTX_get_ex_new_index(0, (void *) "dont_verify_domain", NULL, NULL, NULL); ssl_ex_index_cert_error_check = SSL_get_ex_new_index(0, (void *) "cert_error_check", NULL, &ssl_dupAclChecklist, &ssl_freeAclChecklist); 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); } /// \ingroup ServerProtocolSSLInternal @@ -1235,7 +1255,7 @@ SSL_CTX * Ssl::generateSslContext(char const *host, Ssl::X509_Pointer const & mi return createSSLContext(cert, pkey); } -bool Ssl::verifySslCertificateDate(SSL_CTX * sslContext) +bool Ssl::verifySslCertificate(SSL_CTX * sslContext, X509 *checkCert) { // Temporary ssl for getting X509 certificate from SSL_CTX. Ssl::SSL_Pointer ssl(SSL_new(sslContext)); @@ -1243,7 +1263,13 @@ bool Ssl::verifySslCertificateDate(SSL_CTX * sslContext) ASN1_TIME * time_notBefore = X509_get_notBefore(cert); ASN1_TIME * time_notAfter = X509_get_notAfter(cert); bool ret = (X509_cmp_current_time(time_notBefore) < 0 && X509_cmp_current_time(time_notAfter) > 0); - return ret; + if (!ret) + return false; + + if (checkCert) + return ssl_match_certificates(cert, checkCert); + + return true; } bool diff --git a/src/ssl/support.h b/src/ssl/support.h index 057014a0ca..e2b25b15ed 100644 --- a/src/ssl/support.h +++ b/src/ssl/support.h @@ -56,10 +56,11 @@ */ // Custom SSL errors; assumes all official errors are positive +#define SQUID_X509_V_ERR_CERT_CHANGE -3 #define SQUID_ERR_SSL_HANDSHAKE -2 #define SQUID_X509_V_ERR_DOMAIN_MISMATCH -1 // All SSL errors range: from smallest (negative) custom to largest SSL error -#define SQUID_SSL_ERROR_MIN SQUID_ERR_SSL_HANDSHAKE +#define SQUID_SSL_ERROR_MIN SQUID_X509_V_ERR_CERT_CHANGE #define SQUID_SSL_ERROR_MAX INT_MAX namespace Ssl @@ -112,10 +113,12 @@ SSL_CTX *generateSslContext(char const *host, Ssl::X509_Pointer const & mimicCer /** \ingroup ServerProtocolSSLAPI - * Check date of certificate signature. If there is out of date error fucntion - * returns false, true otherwise. + * Check if the certificate of the given context is still valid + \param sslContext The context to check + \param checkCert Also check if the context certificate matches this certificate + \return true if the contexts certificate is valid, false otherwise */ -bool verifySslCertificateDate(SSL_CTX * sslContext); +bool verifySslCertificate(SSL_CTX * sslContext, X509 *checkCert = NULL); /** \ingroup ServerProtocolSSLAPI