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 0fd1d5701a07e1e7972ac798b4d7d775461b8692..7eb28aceda43249e908a39a40dac85278568f7fa 100644 (file)
--- 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 38898f453827a4e2cecca2be3983d2da8bbcf4dc..5f940df8a0bc8a23b08a9aac6236a22f48dcc2be 100644 (file)
--- 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 c20e713b5e0b2decd9dd7c35e13e9e349a15de0a..048be61382d7b8e00bc1e8a56025b9fa33f9e142 100644 (file)
--- 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 8c2c2f8430236779fdef542d60c2ea96e1696336..bb49b10815758884a57920c4c5cf7a51fc1bab99 100644 (file)
--- a/src/ssl/ErrorDetail.cc
+++ b/src/ssl/ErrorDetail.cc
@@ -16,6 +16,8 @@ typedef std::map<Ssl::ssl_error_t, const SslErrorEntry *> 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 9c9512575f0d7eda92d3c9ba0addc2d371e2b38c..4c08d1ce2831a38339bd77e8c9b6954d91f6349e 100644 (file)
--- 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 25281837b882a3e810ebb7cb0473e868c126444a..97f60a129ac4cac8ff663f5876fef064a6128d7b 100644 (file)
--- 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 cb2c97d1aa13b36a7bfce0fb695df503373843f2..105728440447c5a8262be54039a201ac054288b9 100644 (file)
--- 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 f763b010c8f32d9d19f89a774f01754a41b8de76..8cb0d5e5bd60b54e4533a9b9864d64dca81020f0 100644 (file)
--- 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 <X509 *>(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 057014a0ca8083e61ca744d8d0327e117ec35709..e2b25b15edac80cbe68f4ea34ff4c74a8d3faa7f 100644 (file)
--- 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