From 57c9e17e6cfef9bc0e8ab9fa8edf9d131a233e57 Mon Sep 17 00:00:00 2001
From: Micah Cowan <micah@cowan.name>
Date: Wed, 19 Aug 2009 01:15:27 -0700
Subject: [PATCH] Only warn of attack if the hostname would have matched.

---
 src/ChangeLog |  6 +++++
 src/openssl.c | 63 ++++++++++++++++++++++++++++++---------------------
 2 files changed, 43 insertions(+), 26 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index 9d3a64d4..9795bbc4 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,9 @@
+2009-08-19  Micah Cowan  <micah@cowan.name>
+
+	* openssl.c (ssl_check_certificate): Only warn about an attack if
+	the hostname would otherwise have matched. Also some formatting
+	cleanup.
+
 2009-08-19  Joao Ferreira  <joao@joaoff.com>
 
 	* openssl.c (ssl_check_certificate): Detect embedded NUL
diff --git a/src/openssl.c b/src/openssl.c
index dd3a62a4..de335c08 100644
--- a/src/openssl.c
+++ b/src/openssl.c
@@ -571,32 +571,8 @@ ssl_check_certificate (int fd, const char *host)
 
   X509_NAME *xname = X509_get_subject_name(cert);
   common_name[0] = '\0';
-  X509_NAME_get_text_by_NID (xname,
-                             NID_commonName, common_name, sizeof (common_name));
-
-  /* We now determine the length of the ASN1 string. If it differs from
-   * common_name's length, then there is a \0 before the string terminates.
-   * This can be an instance of a null-prefix attack [0].
-   *
-   * [0] https://www.blackhat.com/html/bh-usa-09/bh-usa-09-archives.html#Marlinspike
-   * */
-
-  int i=-1,j;
-  if(xname) {
-	  for(;(j=X509_NAME_get_index_by_NID (xname, NID_commonName, i))!=-1;i=j);
-  }
-
-  X509_NAME_ENTRY *xentry = X509_NAME_get_entry(xname,i);
-  ASN1_STRING *sdata = X509_NAME_ENTRY_get_data(xentry);
-
-  if (strlen(common_name) != ASN1_STRING_length(sdata)) 
-    {
-      logprintf (LOG_NOTQUIET, _("\
-%s: certificate common name is invalid. It is possible that someone is \
-eavesdropping on you (man-in-the-middle attack)!\n"),
-                 severity);
-      success = false;
-    }
+  X509_NAME_get_text_by_NID (xname, NID_commonName, common_name,
+                             sizeof (common_name));
 
   if (!pattern_match (common_name, host))
     {
@@ -605,6 +581,41 @@ eavesdropping on you (man-in-the-middle attack)!\n"),
                  severity, quote (common_name), quote (host));
       success = false;
     }
+  else
+    {
+      /* We now determine the length of the ASN1 string. If it differs from
+       * common_name's length, then there is a \0 before the string terminates.
+       * This can be an instance of a null-prefix attack.
+       *
+       * https://www.blackhat.com/html/bh-usa-09/bh-usa-09-archives.html#Marlinspike
+       * */
+
+      int i = -1, j;
+      X509_NAME_ENTRY *xentry;
+      ASN1_STRING *sdata;
+
+      if (xname) {
+        for (;;)
+          {
+            j = X509_NAME_get_index_by_NID (xname, NID_commonName, i);
+            if (j == -1) break;
+            i = j;
+          }
+      }
+
+      xentry = X509_NAME_get_entry(xname,i);
+      sdata = X509_NAME_ENTRY_get_data(xentry);
+      if (strlen (common_name) != ASN1_STRING_length (sdata)) 
+        {
+          logprintf (LOG_NOTQUIET, _("\
+%s: certificate common name is invalid (contains a NUL character).\n\
+This may be an indication that the host is not who it claims to be\n\
+(that is, it is not the real %s).\n"),
+                     severity, quote (host));
+          success = false;
+        }
+    }
+  
 
   if (success)
     DEBUGP (("X509 certificate successfully verified and matches host %s\n",