changeset: 6931:2632bc4f5b20 user: Kevin McCarthy <ke...@8t8.us> date: Sun Feb 12 09:59:41 2017 -0800 link: http://dev.mutt.org/hg/mutt/rev/2632bc4f5b20
Filter expired local certs for OpenSSL verification. OpenSSL has trouble establishing the chain and verifying when duplicate expired certs are loaded in from $certificate_file. A warning about this is mentioned in SSL_CTX_load_verify_locations(3SSL). Filter out expired certs when loading verify certs. Note that the full certicates file is still used for verification in check_certificate_by_digest(). changeset: 6932:48a1f145c269 user: Matthias Andree <matthias.and...@gmx.de> date: Sun Feb 12 09:59:48 2017 -0800 link: http://dev.mutt.org/hg/mutt/rev/48a1f145c269 Plug memory leak in weed-expired-certs code. X509_STORE_add_cert() creates a copy of the certificate we're offering, so we need to free our copy afterwards. This isn't documented, but from observed behaviour in OpenSSL 1.0.2 and its master branch source code. Change PEM_read_X509() call to reuse cert to avoid free/reallocation overhead. changeset: 6933:2350d7d61b34 user: Kevin McCarthy <ke...@8t8.us> date: Sun Feb 12 12:24:51 2017 -0800 link: http://dev.mutt.org/hg/mutt/rev/2350d7d61b34 Fix potential cert memory leak in check_certificate_by_digest(). Thanks to Matthias Andree's debugging, it appears the cert is not freed when PEM_read_X509() encounters EOF. Change the return value check to not overwrite cert. It is already updated via the second parameter. diffs (89 lines): diff -r 7c97a8af8718 -r 2350d7d61b34 mutt_ssl.c --- a/mutt_ssl.c Fri Feb 10 13:01:21 2017 -0800 +++ b/mutt_ssl.c Sun Feb 12 12:24:51 2017 -0800 @@ -86,6 +86,49 @@ static int ssl_passwd_cb(char *buf, int size, int rwflag, void *userdata); static int ssl_negotiate (CONNECTION *conn, sslsockdata*); +/* ssl certificate verification can behave strangely if there are expired + * certs loaded into the trusted store. This function filters out expired + * certs. + * Previously the code used this form: + * SSL_CTX_load_verify_locations (ssldata->ctx, SslCertFile, NULL); + */ +static int ssl_load_certificates (SSL_CTX *ctx) +{ + FILE *fp; + X509 *cert = NULL; + X509_STORE *store; + char buf[STRING]; + + dprint (2, (debugfile, "ssl_load_certificates: loading trusted certificates\n")); + store = SSL_CTX_get_cert_store (ctx); + if (!store) + { + store = X509_STORE_new (); + SSL_CTX_set_cert_store (ctx, store); + } + + if ((fp = fopen (SslCertFile, "rt")) == NULL) + return 0; + + while (NULL != PEM_read_X509 (fp, &cert, NULL, NULL)) + { + if ((X509_cmp_current_time (X509_get_notBefore (cert)) >= 0) || + (X509_cmp_current_time (X509_get_notAfter (cert)) <= 0)) + { + dprint (2, (debugfile, "ssl_load_certificates: filtering expired cert: %s\n", + X509_NAME_oneline (X509_get_subject_name (cert), buf, sizeof (buf)))); + } + else + { + X509_STORE_add_cert (store, cert); + } + } + X509_free (cert); + safe_fclose (&fp); + + return 1; +} + /* mutt_ssl_starttls: Negotiate TLS over an already opened connection. * TODO: Merge this code better with ssl_socket_open. */ int mutt_ssl_starttls (CONNECTION* conn) @@ -144,7 +187,7 @@ } } - if (SslCertFile && ! SSL_CTX_load_verify_locations (ssldata->ctx, SslCertFile, NULL)) + if (SslCertFile && !ssl_load_certificates (ssldata->ctx)) dprint (1, (debugfile, "mutt_ssl_starttls: Error loading trusted certificates\n")); ssl_get_client_cert(ssldata, conn); @@ -394,7 +437,7 @@ } } - if (SslCertFile && ! SSL_CTX_load_verify_locations (data->ctx, SslCertFile, NULL)) + if (SslCertFile && !ssl_load_certificates (data->ctx)) dprint (1, (debugfile, "ssl_socket_open: Error loading trusted certificates\n")); ssl_get_client_cert(data, conn); @@ -704,7 +747,7 @@ } if (X509_cmp_current_time (X509_get_notAfter (peercert)) <= 0) { - dprint (2, (debugfile, "Server certificate has expired")); + dprint (2, (debugfile, "Server certificate has expired\n")); mutt_error (_("Server certificate has expired")); mutt_sleep (2); return 0; @@ -720,7 +763,7 @@ return 0; } - while ((cert = PEM_read_X509 (fp, &cert, NULL, NULL)) != NULL) + while (PEM_read_X509 (fp, &cert, NULL, NULL) != NULL) { pass = compare_certificates (cert, peercert, peermd, peermdlen) ? 0 : 1;
