From: Christos Tsantilas <chtsanti@users.sourceforge.net>
Date: Wed, 22 Jun 2011 08:54:17 +0000 (+0300)
Subject: Polish SSL certificate error handling
X-Git-Tag: take08~55^2~108
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7698a79c6482fa733e76d5bd18769c233a973b37;p=thirdparty%2Fsquid.git

Polish SSL certificate error handling

This patch
- Adds SQUID_X509_V_ERR_DOMAIN_MISMATCH to TheSslErrorArray and in
  errors/templates/error-details.txt template file to enable bypass and
  details reporting for that error.

- Fixes the ssl error details handling code, to report the first honored error
  in a certificate, not the first certification validation error (which could
  have been bypassed).

- Adjust ssl_verify_cb() to copy ctx->error to error_no in the beginning and
  never use ctx->error after that.
---

diff --git a/errors/templates/error-details.txt b/errors/templates/error-details.txt
index af0152b971..c7d0f0fed5 100644
--- a/errors/templates/error-details.txt
+++ b/errors/templates/error-details.txt
@@ -1,3 +1,7 @@
+name: SQUID_X509_V_ERR_DOMAIN_MISMATCH
+detail: %ssl_error_descr: %ssl_subject
+descr: Certificate does not match domainname
+
 name: X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT
 detail: "SSL Certficate error: certificate issuer (CA) not known: %ssl_ca_name"
 descr: "Unable to get issuer certificate"
diff --git a/src/ssl/ErrorDetail.cc b/src/ssl/ErrorDetail.cc
index 23233980cd..5ed8bfa3d2 100644
--- 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_DOMAIN_MISMATCH,
+     "SQUID_X509_V_ERR_DOMAIN_MISMATCH"},
     {X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT,
         "X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT"},
     {X509_V_ERR_UNABLE_TO_GET_CRL,
diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index a01707dcba..1b1a8177ee 100644
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -204,14 +204,16 @@ static int check_domain( void *check_data, ASN1_STRING *cn_data)
 static int
 ssl_verify_cb(int ok, X509_STORE_CTX * ctx)
 {
-    char buffer[256];
+    // preserve original ctx->error before SSL_ calls can overwrite it
+    Ssl::ssl_error_t error_no = ok ? SSL_ERROR_NONE : ctx->error;
+
+    char buffer[256] = "";
     SSL *ssl = (SSL *)X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx());
     SSL_CTX *sslctx = SSL_get_SSL_CTX(ssl);
     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 *peer_cert = ctx->cert;
-    Ssl::ssl_error_t error_no = SSL_ERROR_NONE;
 
     X509_NAME_oneline(X509_get_subject_name(peer_cert), buffer,
                       sizeof(buffer));
@@ -226,34 +228,30 @@ ssl_verify_cb(int ok, X509_STORE_CTX * ctx)
                 debugs(83, 2, "SQUID_X509_V_ERR_DOMAIN_MISMATCH: Certificate " << buffer << " does not match domainname " << server);
                 ok = 0;
                 error_no = SQUID_X509_V_ERR_DOMAIN_MISMATCH;
-
-                if (check)
-                    Filled(check)->ssl_error = SQUID_X509_V_ERR_DOMAIN_MISMATCH;
             }
         }
-    } else {
-        error_no = ctx->error;
-        if (const char *err_descr = Ssl::GetErrorDescr(ctx->error))
+    }
+    
+    if (!ok) {
+        if (const char *err_descr = Ssl::GetErrorDescr(error_no))
             debugs(83, 5, err_descr << ": " << buffer);
         else
-            debugs(83, 1, "SSL unknown certificate error " << ctx->error << " in " << buffer);
-
-        if (check)
-            Filled(check)->ssl_error = ctx->error;
-    }
-
-    if (!ok && check) {
-        if (check->fastCheck()) {
-            debugs(83, 3, "bypassing SSL error " << ctx->error << " in " << buffer);
-            ok = 1;
-        } else {
-            debugs(83, 5, "confirming SSL error " << ctx->error);
+            debugs(83, DBG_IMPORTANT, "SSL unknown certificate error " << error_no << " in " << buffer);
+
+        if (check) {
+            Filled(check)->ssl_error = error_no;
+            if (check->fastCheck()) {
+                debugs(83, 3, "bypassing SSL error " << error_no << " in " << buffer);
+                ok = 1;
+            } else {
+                debugs(83, 5, "confirming SSL error " << error_no);
+            }
         }
     }
 
     if (!dont_verify_domain && server) {}
 
-    if (error_no != SSL_ERROR_NONE && !SSL_get_ex_data(ssl, ssl_ex_index_ssl_error_detail) ) {
+    if (!ok && !SSL_get_ex_data(ssl, ssl_ex_index_ssl_error_detail) ) {
         Ssl::ErrorDetail *errDetail = new Ssl::ErrorDetail(error_no, peer_cert);
         if (!SSL_set_ex_data(ssl, ssl_ex_index_ssl_error_detail,  errDetail)) {
             debugs(83, 2, "Failed to set Ssl::ErrorDetail in ssl_verify_cb: Certificate " << buffer);