From f2e5cac661b1990afaa821acefd99cb607a94c0b Mon Sep 17 00:00:00 2001 From: Brendan Cully Date: Wed, 27 May 2009 22:40:34 -0700 Subject: [PATCH] Fix TLS certificate chain validation for openssl. --- ChangeLog | 12 ++++++++++++ mutt_ssl.c | 45 +++++++++++++++------------------------------ 2 files changed, 27 insertions(+), 30 deletions(-) diff --git a/ChangeLog b/ChangeLog index cf1bccbac..d652e6d5b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +2009-05-25 17:31 -0700 Brendan Cully (8f11dd00c770) + + * mutt_ssl_gnutls.c: Fix a serious oversight validating TLS + certificates. If any certificate in a chain presented by a server + was accepted, the connection was allowed without verifying that the + presented certificate was actually signed by the certificate in the + chain. + +2009-05-27 22:13 -0700 Petr Písař (05bc65d6ae70) + + * po/cs.po: Updated Czech translation. + 2009-05-27 10:13 +0200 Rocco Rutte (97305eeb91ce) * doc/manual.xml.head, doc/mutt.man: Document that -- is always diff --git a/mutt_ssl.c b/mutt_ssl.c index 04e9a44c3..9cf7cadb1 100644 --- a/mutt_ssl.c +++ b/mutt_ssl.c @@ -739,8 +739,9 @@ static int ssl_cache_trusted_cert (X509 *c) return (sk_X509_push (SslSessionCerts, X509_dup(c))); } -/* check whether cert is preauthorized */ -static int ssl_check_preauth (X509 *cert, CONNECTION *conn) +/* check whether cert is preauthorized. If host is not null, verify that + * it matches the certificate */ +static int ssl_check_preauth (X509 *cert, const char* host) { char buf[SHORT_STRING]; @@ -752,9 +753,9 @@ static int ssl_check_preauth (X509 *cert, CONNECTION *conn) } buf[0] = 0; - if (option (OPTSSLVERIFYHOST) != M_NO) + if (host && option (OPTSSLVERIFYHOST) != M_NO) { - if (!check_host (cert, conn->account.host, buf, sizeof (buf))) + if (!check_host (cert, host, buf, sizeof (buf))) { mutt_error (_("Certificate host check failed: %s"), buf); mutt_sleep (2); @@ -785,44 +786,28 @@ static int ssl_check_certificate (CONNECTION *conn, sslsockdata *data) STACK_OF(X509) *chain; X509 *cert; - if ((preauthrc = ssl_check_preauth (data->cert, conn)) > 0) + if ((preauthrc = ssl_check_preauth (data->cert, conn->account.host)) > 0) return preauthrc; chain = SSL_get_peer_cert_chain (data->ssl); chain_len = sk_X509_num (chain); - if (!chain || (chain_len < 1)) + if (!chain || (chain_len <= 1)) return interactive_check_cert (data->cert, 0, 0); - /* check the chain from root to peer */ + /* check the chain from root to peer. */ for (i = chain_len-1; i >= 0; i--) { 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 */ || !preauthrc) + + /* if the certificate validates or is manually accepted, then add it to + * the trusted set and recheck the peer certificate */ + if (ssl_check_preauth (cert, NULL) + || interactive_check_cert (cert, i, chain_len)) { - if (check_certificate_by_signer (cert)) - { - dprint (2, (debugfile, "ssl chain: checked by signer: %s\n", cert->name)); - ssl_cache_trusted_cert (cert); + ssl_cache_trusted_cert (cert); + if (ssl_check_preauth (data->cert, conn->account.host)) return 1; - } - else if (SslCertFile && check_certificate_by_digest (cert)) - { - dprint (2, (debugfile, "ssl chain: trusted with file: %s\n", cert->name)); - ssl_cache_trusted_cert (cert); - return 1; - } - else /* allow users to shoot their foot */ - { - dprint (2, (debugfile, "ssl chain: check failed: %s\n", cert->name)); - if (interactive_check_cert (cert, i, chain_len)) - return 1; - } } - else /* highly suspicious because (i==0 && preauthrc < 0) */ - if (interactive_check_cert (cert, i, chain_len)) - return 1; } return 0; -- 2.40.0