Bug 1015973 - Improve cert error decoding. r=keeler, a=abillings
authorCamilo Viecco <cviecco@mozilla.com>
Mon, 02 Jun 2014 09:04:39 -0700
changeset 207332 85b450997e1f5a5350aa10c4446d924148b7bd64
parent 207331 75cffcdac1397acc64a2b641b1a9533c09447ef1
child 207333 8f6ec528e890f395f5ad35ecc4a5828c0d7851f9
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler, abillings
bugs1015973
milestone32.0a2
Bug 1015973 - Improve cert error decoding. r=keeler, a=abillings
security/manager/ssl/src/TransportSecurityInfo.cpp
--- a/security/manager/ssl/src/TransportSecurityInfo.cpp
+++ b/security/manager/ssl/src/TransportSecurityInfo.cpp
@@ -15,16 +15,17 @@
 #include "nsDateTimeFormatCID.h"
 #include "nsICertOverrideService.h"
 #include "nsIObjectInputStream.h"
 #include "nsIObjectOutputStream.h"
 #include "nsNSSCertHelper.h"
 #include "nsIProgrammingLanguage.h"
 #include "nsIArray.h"
 #include "nsComponentManagerUtils.h"
+#include "nsReadableUtils.h"
 #include "nsServiceManagerUtils.h"
 #include "PSMRunnable.h"
 
 #include "secerr.h"
 
 //#define DEBUG_SSL_VERBOSE //Enable this define to get minimal 
                             //reports when doing SSL read/write
                             
@@ -619,22 +620,31 @@ GetSubjectAltNames(CERTCertificate *nssC
 
   SECITEM_FreeItem(&altNameExtension, false);
 
   CERTGeneralName *current = sanNameList;
   do {
     nsAutoString name;
     switch (current->type) {
       case certDNSName:
-        name.AssignASCII((char*)current->name.other.data, current->name.other.len);
-        if (!allNames.IsEmpty()) {
-          allNames.AppendLiteral(", ");
+        {
+          nsDependentCSubstring nameFromCert(reinterpret_cast<char*>
+                                              (current->name.other.data),
+                                              current->name.other.len);
+          // dNSName fields are defined as type IA5String and thus should
+          // be limited to ASCII characters.
+          if (IsASCII(nameFromCert)) {
+            name.Assign(NS_ConvertASCIItoUTF16(nameFromCert));
+            if (!allNames.IsEmpty()) {
+              allNames.AppendLiteral(", ");
+            }
+            ++nameCount;
+            allNames.Append(name);
+          }
         }
-        ++nameCount;
-        allNames.Append(name);
         break;
 
       case certIPAddress:
         {
           char buf[INET6_ADDRSTRLEN];
           PRNetAddr addr;
           if (current->name.other.len == 4) {
             addr.inet.family = PR_AF_INET;
@@ -704,18 +714,25 @@ AppendErrorTextMismatch(const nsString &
   bool useSAN = false;
 
   if (nssCert)
     useSAN = GetSubjectAltNames(nssCert.get(), component, allNames, nameCount);
 
   if (!useSAN) {
     char *certName = CERT_GetCommonName(&nssCert->subject);
     if (certName) {
-      ++nameCount;
-      allNames.Assign(NS_ConvertUTF8toUTF16(certName));
+      nsDependentCSubstring commonName(certName, strlen(certName));
+      if (IsUTF8(commonName)) {
+        // Bug 1024781
+        // We should actually check that the common name is a valid dns name or
+        // ip address and not any string value before adding it to the display
+        // list.
+        ++nameCount;
+        allNames.Assign(NS_ConvertUTF8toUTF16(commonName));
+      }
       PORT_Free(certName);
     }
   }
 
   if (nameCount > 1) {
     nsString message;
     rv = component->GetPIPNSSBundleString("certErrorMismatchMultiple", 
                                           message);