[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);
}