Ticket #3087 (closed defect: fixed)

Opened 2 months ago

Last modified 7 days ago

No server hostname validation in SSL certificate processing

Reported by: gkloepfer Owned by: mutt-dev
Priority: major Milestone:
Component: crypto Version: 1.5.16
Keywords: certificate server validation patch Cc:

Description

The SSL X509 certificate handling in mutt does not check the CN= against the FQDN that the user entered, and as such there is no indication that the certificate that mutt receives from a SSL-based server actually belongs to the server in question.

This could allow a malicious person to redirect (via DNS manipulation or otherwise) a user to a different server than intended and, using a valid server certificate from any host, permit the connection to succeed normally with no indication to the user that the certificate is invalid for the specified server.

I am attaching a patch against mutt 1.5.16 that looks like it will address the problem. The behavior the patch implements mimics the behavior in Mozilla-based e-mail clients.

Attachments

mutt_hostname_patch.txt (1.7 kB) - added by gkloepfer 2 months ago.
Patch to correct hostname validation
fix-3087.diff (4.3 kB) - added by pdmef 8 days ago.

Change History

Changed 2 months ago by gkloepfer

Patch to correct hostname validation

Changed 2 months ago by gkloepfer

  • version set to 1.5.16

Changed 2 months ago by gkloepfer

  • cc mutt0708@… removed

Changed 7 weeks ago by pdmef

  • status changed from new to closed
  • resolution set to fixed

(In [934a802dff7f]) Verify hostname in (Open)SSL certificate validation

This is based on the patch by gkloepfer attached to #3087 but passes the proper connection as argument (avoiding adding hostname to struct sslsockdata) and validates the hostname even in case OpenSSL cannot find the local issuer certificate. GnuTLS already supports hostname checking. Closes #3087.

Changed 2 weeks ago by pdmef

  • priority changed from critical to major
  • status changed from closed to reopened
  • resolution deleted

The hostname check added in [934a802dff7f] is likely incomplete.

First, http://www.openssl.org/docs/crypto/X509_NAME_get_index_by_NID.html says X509_NAME_get_text_by_NID() is a legacy function with limitations. Second, it seems that simply matching full hostnames is not enough as other verification implementations (including GnuTLS) seem to support pattern and domain name matching as well as extracting all hostnames provided in the certificate.

The hostname verification used in msmtp together with OpenSSL could be a candiate implementation for mutt as it seems to check more than the first CN and does support pattern matching.

Changed 8 days ago by pdmef

Changed 8 days ago by pdmef

  • keywords patch added

Here we go: attached is a patch that ports the hostname check from msmtp for OpenSSL to mutt. For the only IMAP server I have it works (certificate is for *.domain.tld, server is mail.domain.tld).

I'm no OpenSSL expert, but the port is very straight forward, and annotating the source at sourceforge suggest the code in question is untouched for at least 3 years. So I think this is mature enough to include into mutt.

Msmtp is GPLv3 so I think that won't be a problem, either.

Changed 7 days ago by pdmef

For clarification on gplv2 vs. gplv3: The code in msmtp hasn't changed in years so I think we'll easily find a gplv2 licensed version. Second, the standard header in mutt sources states 'either version 2 of the License, or (at your option) any later version.' so that I don't think inclusion would really be a problem.

As a side note: The file COPYRIGHT in the source states that parts may be under different licenses or in public domain. Maybe we should try to find out what code mutt includes and put that in a file somewhere or the manual? I think we should give more credit than just comments in the source in case we include bigger parts (like the regex engine, the various compat sources, the MD5/SHA1 implementations, etc.)

Changed 7 days ago by brendan

The patch looks fine to me, as long as you can confirm that the code we are borrowing from msmtp is gplv2. I'd rather not have explicitly gplv3 code added. I'm not sure why we need to advertise the source of the other borrowed code more heavily? It should certainly be noted in the source, but I think that should do. Anyone wanting to reuse the source for their own purposes should find the correct attribution immediately.

Changed 7 days ago by Rocco Rutte <pdmef@…>

  • status changed from reopened to closed
  • resolution set to fixed

(In [9373afa9278f]) Port certificate host checking from msmtp to mutt. It supports IDN, wildcards and extracting the hostname from subject alternative field as well as common name which should be the same gnutls supports. Closes #3087.

Changed 7 days ago by pdmef

I can verify that in the CVS at sourceforge the code we borrowed wasn't touched quite some time before the license switch.

Note: See TracTickets for help on using tickets.