RFC 6125 section 6.4.4 says the client should not check the
Common Name if the cert contains any DNS entries.

Flag if a GEN_DNS entry is found, and if none of them match, abort the
comparison.

Thanks to [email protected] for the security report.
---
This is 8 in the list evilrabbit sent.

Request for feedback: would this be better to put into master instead?
Is it serious enough to take a risk of unexpected consequences of not
comparing CN in that case?

 mutt_ssl.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/mutt_ssl.c b/mutt_ssl.c
index a9a4e8ae..70a62f97 100644
--- a/mutt_ssl.c
+++ b/mutt_ssl.c
@@ -957,7 +957,7 @@ static int check_host (X509 *x509cert, const char 
*hostname, char *err, size_t e
   int subj_alt_names_count;
   GENERAL_NAME *subj_alt_name;
   /* did we find a name matching hostname? */
-  int match_found;
+  int match_found, has_dns_entry = 0;
 
   /* Check if 'hostname' matches the one of the subjectAltName extensions of
    * type DNS or the Common Name (CN). */
@@ -982,6 +982,7 @@ static int check_host (X509 *x509cert, const char 
*hostname, char *err, size_t e
       subj_alt_name = sk_GENERAL_NAME_value(subj_alt_names, i);
       if (subj_alt_name->type == GEN_DNS)
       {
+        has_dns_entry = 1;
        if (subj_alt_name->d.ia5->length >= 0 &&
            mutt_strlen((char *)subj_alt_name->d.ia5->data) == 
(size_t)subj_alt_name->d.ia5->length &&
            (match_found = hostname_match(hostname_ascii,
@@ -994,6 +995,14 @@ static int check_host (X509 *x509cert, const char 
*hostname, char *err, size_t e
     GENERAL_NAMES_free(subj_alt_names);
   }
 
+  if (has_dns_entry && !match_found)
+  {
+    if (err && errlen)
+      snprintf(err, errlen, _("certificate owner does not match hostname %s"),
+               hostname);
+    goto out;
+  }
+
   if (!match_found)
   {
     /* Try the common name */
-- 
2.53.0

Reply via email to