<<< Date Index >>>     <<< Thread Index >>>

[PATCH] SSL intermediate certificates



Hi list,

I think mutt's (Open)SSL support is kinda poor.
It only checks a peer's direct signer/issuer.
As SSL has a recursive structure
(selfsigned root -> intermediate certs -> peer),
I hope mutt will check certs a little deeper.

Also, if a peer cert is not signed directly
by any of $certificate_file or system certs,
mutt asks me whether or not to trust it showing its
MD5 fingerprint although OpenSSL defaults to SHA1.
This is annoying, too.

If anyone is interested,
I wrote a patch (attached) to try to
- check cert_chain and cache trusted certs
- allow users to save intermediate certs
- show SHA1 fingerprint instead of MD5

Beware, I am no (Open)SSL expert either.


Example situation:

My POPS servers are secure.plala.or.jp (I don't
know how many servers this domain name points to,
but I'm given a different cert every time I
connect to it).

Their certs were all issued by
"VeriSign International Server CA - Class 3",
which was issued by VeriSign's Root CA,
"Class 3 Public Primary Certification Authority".

Most SSL clients need only the root cert.
But mutt complains that I don't have
"VeriSign International Server CA - Class 3"
in my $certificate_file.

Also note, the "missing" cert is actually sent
to mutt from the peer POPS server.
Mutt simply ignores it and complains to me.

Even if I compromise, I don't want to append
all the peer certs. I only want
"VeriSign International Server CA - Class 3"
in my $certificate_file. But how can I add it
to the file? Mutt asks me only for the peer.

Bye,
-- 
tamo
diff -r 10a1f06bc8aa globals.h
--- a/globals.h Tue Oct 07 19:22:53 2008 -0700
+++ b/globals.h Mon Oct 13 13:36:24 2008 +0900
@@ -129,7 +129,8 @@
 WHERE char *SslCertFile INITVAL (NULL);
 WHERE char *SslClientCert INITVAL (NULL);
 #ifdef USE_SSL_OPENSSL
-WHERE LIST *SslSessionCerts INITVAL (NULL);
+#include <openssl/ssl.h>
+WHERE STACK_OF(X509) *SslSessionCerts INITVAL (NULL);
 #endif
 WHERE char *SslEntropyFile INITVAL (NULL);
 #ifdef USE_SSL_GNUTLS
diff -r 10a1f06bc8aa mutt_ssl.c
--- a/mutt_ssl.c        Tue Oct 07 19:22:53 2008 -0700
+++ b/mutt_ssl.c        Mon Oct 13 13:36:24 2008 +0900
@@ -60,6 +60,14 @@
 #define HAVE_ENTROPY() (!access(DEVRANDOM, R_OK) || entropy_byte_count >= 16)
 #endif
 
+/* keep a handle on accepted certificates in case we want to
+ * open up another connection to the same server in this session */
+#define CACHE_TRUSTED(c) \
+       if (!SslSessionCerts) \
+         SslSessionCerts = sk_new_null(); \
+       sk_X509_push (SslSessionCerts, c); \
+       dprint (1, (debugfile, "trusted: %s\n", c->name))
+
 typedef struct _sslsockdata
 {
   SSL_CTX *ctx;
@@ -76,7 +84,8 @@
 static int ssl_socket_open (CONNECTION * conn);
 static int ssl_socket_close (CONNECTION * conn);
 static int tls_close (CONNECTION* conn);
-static int ssl_check_certificate (CONNECTION *conn, sslsockdata * data);
+static int ssl_check_certificate (CONNECTION *conn, sslsockdata * data, int);
+static int interactive_check_cert (X509 *cert);
 static void ssl_get_client_cert(sslsockdata *ssldata, CONNECTION *conn);
 static int ssl_passwd_cb(char *buf, int size, int rwflag, void *userdata);
 static int ssl_negotiate (CONNECTION *conn, sslsockdata*);
@@ -345,7 +354,7 @@
     return -1;
   }
 
-  if (!ssl_check_certificate (conn, ssldata))
+  if (!ssl_check_certificate (conn, ssldata, 1))
     return -1;
 
   mutt_message (_("SSL connection using %s (%s)"), 
@@ -415,7 +424,7 @@
   unsigned int n;
   int j;
 
-  if (!X509_digest (cert, EVP_md5 (), md, &n))
+  if (!X509_digest (cert, EVP_sha1 (), md, &n))
   {
     snprintf (s, l, _("[unable to calculate]"));
   }
@@ -424,7 +433,7 @@
     for (j = 0; j < (int) n; j++)
     {
       char ch[8];
-      snprintf (ch, 8, "%02X%s", md[j], (j % 2 ? " " : ""));
+      snprintf (ch, 8, "%02X%s", md[j], (j + 1 < (int) n ? ":" : ""));
       safe_strcat (s, l, ch);
     }
   }
@@ -452,7 +461,7 @@
 {
   X509_STORE_CTX xsc;
   X509_STORE *ctx;
-  int pass = 0;
+  int pass = 0, i;
 
   ctx = X509_STORE_new ();
   if (ctx == NULL) return 0;
@@ -470,6 +479,9 @@
   else
     dprint (2, (debugfile, "X509_STORE_load_locations_failed\n"));
 
+  for (i = 0; i < sk_X509_num (SslSessionCerts); i++)
+    pass += (X509_STORE_add_cert (ctx, sk_X509_value (SslSessionCerts, i)) != 
0);
+
   if (pass == 0)
   {
     /* nothing to do */
@@ -477,7 +489,7 @@
     return 0;
   }
 
-  X509_STORE_CTX_init (&xsc, ctx, peercert, NULL);
+  X509_STORE_CTX_init (&xsc, ctx, peercert, SslSessionCerts);
 
   pass = (X509_verify_cert (&xsc) > 0);
 #ifdef DEBUG
@@ -490,6 +502,7 @@
     snprintf (buf, sizeof (buf), "%s (%d)", 
        X509_verify_cert_error_string(err), err);
     dprint (2, (debugfile, "X509_verify_cert: %s\n", buf));
+    dprint (2, (debugfile, " [%s]\n", peercert->name));
   }
 #endif
   X509_STORE_CTX_cleanup (&xsc);
@@ -525,16 +538,17 @@
   unsigned char peermd[EVP_MAX_MD_SIZE];
   unsigned int peermdlen;
   X509 *cert;
-  LIST *scert;
+  int i;
 
-  if (!X509_digest (peercert, EVP_sha1(), peermd, &peermdlen))
+  if (!X509_digest (peercert, EVP_sha1(), peermd, &peermdlen)
+      || !SslSessionCerts)
   {
     return 0;
   }
   
-  for (scert = SslSessionCerts; scert; scert = scert->next)
+  for (i = sk_X509_num (SslSessionCerts); i-- > 0;)
   {
-    cert = *(X509**)scert->data;
+    cert = sk_X509_value (SslSessionCerts, i);
     if (!compare_certificates (cert, peercert, peermd, peermdlen))
     {
       return 1;
@@ -717,16 +731,12 @@
   return rc;
 }
 
-static int ssl_check_certificate (CONNECTION *conn, sslsockdata * data)
+static int ssl_check_certificate (CONNECTION *conn, sslsockdata * data, int 
interactive)
 {
-  char *part[] =
-  {"/CN=", "/Email=", "/O=", "/OU=", "/L=", "/ST=", "/C="};
-  char helpstr[LONG_STRING];
   char buf[SHORT_STRING];
-  MUTTMENU *menu;
-  int done, row, i, certerr_hostname = 0;
-  FILE *fp;
-  char *name = NULL, *c;
+  int i, certerr_hostname = 0, chain_len;
+  STACK_OF(X509) *chain;
+  X509 *cert;
 
   /* check session cache first */
   if (check_certificate_cache (data->cert))
@@ -760,8 +770,60 @@
     return 1;
   }
 
-  /* interactive check from user */
-  menu = mutt_new_menu ();
+  if (!interactive)
+    return 0;
+
+  chain = SSL_get_peer_cert_chain (data->ssl);
+  chain_len = sk_X509_num (chain);
+  if (!chain || (chain_len < 1))
+    return interactive_check_cert (data->cert);
+  else
+  {
+    /* check the chain from root to peer */
+    for (i = chain_len; i-- > 0;)
+    {
+      cert = sk_X509_value (chain, i);
+      if (check_certificate_cache (cert))
+       dprint (2, (debugfile, "ssl chain: already cached: %s\n", cert->name));
+      else if (i /* 0 is the peer */ || !certerr_hostname)
+      {
+       if (check_certificate_by_signer (cert))
+       {
+         dprint (2, (debugfile, "ssl chain: checked by signer: %s\n", 
cert->name));
+         CACHE_TRUSTED (cert);
+       }
+       else if (SslCertFile && check_certificate_by_digest (cert))
+       {
+         dprint (2, (debugfile, "ssl chain: trusted with file: %s\n", 
cert->name));
+         CACHE_TRUSTED (cert);
+       }
+       else /* allow users to shoot their foot */
+       {
+         dprint (2, (debugfile, "ssl chain: check failed: %s\n", cert->name));
+         interactive_check_cert (cert);
+       }
+      }
+      else /* highly suspicious because (i==0 && certerr_hostname) */
+       interactive_check_cert (cert);
+    }
+    /* retry with the trusted chain */
+    return ssl_check_certificate (conn, data, 0);
+  }
+}
+
+static int interactive_check_cert (X509 *cert)
+{
+  char *part[] =
+  {"/CN=", "/Email=", "/O=", "/OU=", "/L=", "/ST=", "/C="};
+  char helpstr[LONG_STRING];
+  char buf[SHORT_STRING];
+  MUTTMENU *menu = mutt_new_menu();
+  int done, row, i;
+  FILE *fp;
+  char *name = NULL, *c;
+
+  dprint (2, (debugfile, "ssl interactive_check_cert: %s\n", cert->name));
+
   menu->max = 19;
   menu->dialog = (char **) safe_calloc (1, menu->max * sizeof (char *));
   for (i = 0; i < menu->max; i++)
@@ -770,7 +832,7 @@
   row = 0;
   strfcpy (menu->dialog[row], _("This certificate belongs to:"), SHORT_STRING);
   row++;
-  name = X509_NAME_oneline (X509_get_subject_name (data->cert),
+  name = X509_NAME_oneline (X509_get_subject_name (cert),
                            buf, sizeof (buf));
   for (i = 0; i < 5; i++)
   {
@@ -781,7 +843,7 @@
   row++;
   strfcpy (menu->dialog[row], _("This certificate was issued by:"), 
SHORT_STRING);
   row++;
-  name = X509_NAME_oneline (X509_get_issuer_name (data->cert),
+  name = X509_NAME_oneline (X509_get_issuer_name (cert),
                            buf, sizeof (buf));
   for (i = 0; i < 5; i++)
   {
@@ -792,18 +854,18 @@
   row++;
   snprintf (menu->dialog[row++], SHORT_STRING, _("This certificate is valid"));
   snprintf (menu->dialog[row++], SHORT_STRING, _("   from %s"), 
-      asn1time_to_string (X509_get_notBefore (data->cert)));
+      asn1time_to_string (X509_get_notBefore (cert)));
   snprintf (menu->dialog[row++], SHORT_STRING, _("     to %s"), 
-      asn1time_to_string (X509_get_notAfter (data->cert)));
+      asn1time_to_string (X509_get_notAfter (cert)));
 
   row++;
   buf[0] = '\0';
-  x509_fingerprint (buf, sizeof (buf), data->cert);
+  x509_fingerprint (buf, sizeof (buf), cert);
   snprintf (menu->dialog[row++], SHORT_STRING, _("Fingerprint: %s"), buf);
 
   menu->title = _("SSL Certificate check");
-  if (SslCertFile && X509_cmp_current_time (X509_get_notAfter (data->cert)) >= 0
-      && X509_cmp_current_time (X509_get_notBefore (data->cert)) < 0)
+  if (SslCertFile && X509_cmp_current_time (X509_get_notAfter (cert)) >= 0
+      && X509_cmp_current_time (X509_get_notBefore (cert)) < 0)
   {
     menu->prompt = _("(r)eject, accept (o)nce, (a)ccept always");
     menu->keys = _("roa");
@@ -836,7 +898,7 @@
         done = 0;
         if ((fp = fopen (SslCertFile, "a")))
        {
-         if (PEM_write_X509 (fp, data->cert))
+         if (PEM_write_X509 (fp, cert))
            done = 1;
          fclose (fp);
        }
@@ -853,15 +915,13 @@
         /* fall through */
       case OP_MAX + 2:         /* accept once */
         done = 2;
-        /* keep a handle on accepted certificates in case we want to
-         * open up another connection to the same server in this session */
-        SslSessionCerts = mutt_add_list_n (SslSessionCerts, &data->cert,
-                                           sizeof (X509 **));
+       CACHE_TRUSTED (cert);
         break;
     }
   }
   unset_option(OPTUNBUFFEREDINPUT);
   mutt_menuDestroy (&menu);
+  dprint (2, (debugfile, "ssl interactive_check_cert: done=%d\n", done));
   return (done == 2);
 }