bug 1264761 - improve handling of x509 versions in certificate manager r=Cykesiopka
☠☠ backed out by 5c27d9625278 ☠ ☠
authorDavid Keeler <dkeeler@mozilla.com>
Mon, 18 Apr 2016 11:07:24 -0700
changeset 331810 bb60c7a0b0c54ae0be1316d44b7a84098b2985bd
parent 331809 27b5119d82e01dded303cd795021483480ecd2e4
child 331811 54715f7701edf6971457ae99cafd759f285a8ebc
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersCykesiopka
bugs1264761
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 1264761 - improve handling of x509 versions in certificate manager r=Cykesiopka MozReview-Commit-ID: B7EPx63ttlt
security/manager/locales/en-US/chrome/pipnss/pipnss.properties
security/manager/ssl/nsNSSCertHelper.cpp
security/manager/ssl/tests/unit/test_cert_version.js
--- a/security/manager/locales/en-US/chrome/pipnss/pipnss.properties
+++ b/security/manager/locales/en-US/chrome/pipnss/pipnss.properties
@@ -48,19 +48,18 @@ VerifyCAVerifier=CA Verifier
 VerifyStatusResponder=Status Responder Certificate
 HighGrade=High Grade
 MediumGrade=Medium Grade
 # LOCALIZATION NOTE (nick_template): $1s is the common name from a cert (e.g. "Mozilla"), $2s is the CA name (e.g. VeriSign)
 nick_template=%1$s's %2$s ID
 #These are the strings set for the ASN1 objects in a certificate.
 CertDumpCertificate=Certificate
 CertDumpVersion=Version
-CertDumpVersion1=Version 1
-CertDumpVersion2=Version 2
-CertDumpVersion3=Version 3
+# LOCALIZATION NOTE (CertDumpVersionValue): %S is a version number (e.g. "3" in "Version 3")
+CertDumpVersionValue=Version %S
 CertDumpSerialNo=Serial Number
 CertDumpMD2WithRSA=PKCS #1 MD2 With RSA Encryption
 CertDumpMD5WithRSA=PKCS #1 MD5 With RSA Encryption
 CertDumpSHA1WithRSA=PKCS #1 SHA-1 With RSA Encryption
 CertDumpSHA256WithRSA=PKCS #1 SHA-256 With RSA Encryption
 CertDumpSHA384WithRSA=PKCS #1 SHA-384 With RSA Encryption
 CertDumpSHA512WithRSA=PKCS #1 SHA-512 With RSA Encryption
 CertDumpDefOID=Object Identifier (%S)
--- a/security/manager/ssl/nsNSSCertHelper.cpp
+++ b/security/manager/ssl/nsNSSCertHelper.cpp
@@ -1,31 +1,30 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsNSSCertHelper.h"
 
 #include <algorithm>
 
+#include "ScopedNSSTypes.h"
 #include "mozilla/Snprintf.h"
 #include "mozilla/UniquePtr.h"
+#include "nsCOMPtr.h"
 #include "nsComponentManagerUtils.h"
-#include "nsCOMPtr.h"
 #include "nsDateTimeFormatCID.h"
 #include "nsIDateTimeFormat.h"
 #include "nsNSSASN1Object.h"
-#include "nsNSSCertificate.h"
 #include "nsNSSCertTrust.h"
 #include "nsNSSCertValidity.h"
+#include "nsNSSCertificate.h"
 #include "nsNSSComponent.h"
-#include "nsIDateTimeFormat.h"
 #include "nsServiceManagerUtils.h"
 #include "prerror.h"
-#include "ScopedNSSTypes.h"
 #include "secder.h"
 
 using namespace mozilla;
  
 /* Object Identifier constants */
 #define CONST_OID static const unsigned char
 #define MICROSOFT_OID 0x2b, 0x6, 0x1, 0x4, 0x1, 0x82, 0x37
 #define PKIX_OID 0x2b, 0x6, 0x01, 0x05, 0x05, 0x07
@@ -79,63 +78,56 @@ GetIntValue(SECItem *versionItem,
   if (srv != SECSuccess) {
     NS_ERROR("Could not decode version of cert");
     return NS_ERROR_FAILURE;
   }
   return NS_OK;
 }
 
 static nsresult
-ProcessVersion(SECItem         *versionItem,
-               nsINSSComponent *nssComponent,
-               nsIASN1PrintableItem **retItem)
+ProcessVersion(SECItem* versionItem, nsINSSComponent* nssComponent,
+               nsIASN1PrintableItem** retItem)
 {
-  nsresult rv;
   nsAutoString text;
+  nssComponent->GetPIPNSSBundleString("CertDumpVersion", text);
   nsCOMPtr<nsIASN1PrintableItem> printableItem = new nsNSSASN1PrintableItem();
- 
-  nssComponent->GetPIPNSSBundleString("CertDumpVersion", text);
-  rv = printableItem->SetDisplayName(text);
-  if (NS_FAILED(rv))
+  nsresult rv = printableItem->SetDisplayName(text);
+  if (NS_FAILED(rv)) {
     return rv;
+  }
 
   // Now to figure out what version this certificate is.
   unsigned long version;
-
   if (versionItem->data) {
     rv = GetIntValue(versionItem, &version);
-    if (NS_FAILED(rv))
+    if (NS_FAILED(rv)) {
       return rv;
+    }
   } else {
-    // If there is no version present in the cert, then rfc2459
-    // says we default to v1 (0)
+    // If there is no version present in the cert, then RFC 5280 says we
+    // default to v1 (0).
     version = 0;
   }
 
-  switch (version){
-  case 0:
-    rv = nssComponent->GetPIPNSSBundleString("CertDumpVersion1", text);
-    break;
-  case 1:
-    rv = nssComponent->GetPIPNSSBundleString("CertDumpVersion2", text);
-    break;
-  case 2:
-    rv = nssComponent->GetPIPNSSBundleString("CertDumpVersion3", text);
-    break;
-  default:
-    NS_ERROR("Bad value for cert version");
-    rv = NS_ERROR_FAILURE;
+  // A value of n actually corresponds to version n + 1
+  nsAutoString versionString;
+  versionString.AppendInt(version + 1);
+  const char16_t* params[1] = { versionString.get() };
+  rv = nssComponent->PIPBundleFormatStringFromName("CertDumpVersionValue",
+                                                   params,
+                                                   MOZ_ARRAY_LENGTH(params),
+                                                   text);
+  if (NS_FAILED(rv)) {
+    return rv;
   }
-    
-  if (NS_FAILED(rv))
-    return rv;
 
   rv = printableItem->SetDisplayValue(text);
-  if (NS_FAILED(rv))
+  if (NS_FAILED(rv)) {
     return rv;
+  }
 
   printableItem.forget(retItem);
   return NS_OK;
 }
 
 static nsresult 
 ProcessSerialNumberDER(SECItem         *serialItem, 
                        nsINSSComponent *nssComponent,
--- a/security/manager/ssl/tests/unit/test_cert_version.js
+++ b/security/manager/ssl/tests/unit/test_cert_version.js
@@ -44,16 +44,26 @@ function loadCertWithTrust(certName, tru
 function checkEndEntity(cert, expectedResult) {
   checkCertErrorGeneric(certdb, cert, expectedResult, certificateUsageSSLServer);
 }
 
 function checkIntermediate(cert, expectedResult) {
   checkCertErrorGeneric(certdb, cert, expectedResult, certificateUsageSSLCA);
 }
 
+// Test that the code that decodes certificates to display them in the
+// certificate manager correctly handles the version field.
+function checkCertVersion(cert, expectedVersionString) {
+  let asn1 = cert.ASN1Structure.QueryInterface(Ci.nsIASN1Sequence);
+  let tbsCertificate = asn1.ASN1Objects.queryElementAt(0, Ci.nsIASN1Sequence);
+  let version = tbsCertificate.ASN1Objects.queryElementAt(0, Ci.nsIASN1Object);
+  equal(version.displayValue, expectedVersionString,
+        "Actual and expected version strings should match");
+}
+
 function run_test() {
   loadCertWithTrust("ca", "CTu,,");
 
   // Section for CAs lacking the basicConstraints extension entirely:
   loadCertWithTrust("int-v1-noBC_ca", ",,");
   checkIntermediate(certFromFile("int-v1-noBC_ca"), MOZILLA_PKIX_ERROR_V1_CERT_USED_AS_CA);
   checkEndEntity(certFromFile("ee_int-v1-noBC"), MOZILLA_PKIX_ERROR_V1_CERT_USED_AS_CA);
   // A v1 certificate with no basicConstraints extension may issue certificates
@@ -167,9 +177,14 @@ function run_test() {
   checkEndEntity(certFromFile("ss-v2-BC-not-cA"), SEC_ERROR_UNKNOWN_ISSUER);
   checkEndEntity(certFromFile("ss-v3-BC-not-cA"), SEC_ERROR_UNKNOWN_ISSUER);
   checkEndEntity(certFromFile("ss-v4-BC-not-cA"), SEC_ERROR_UNKNOWN_ISSUER);
 
   checkEndEntity(certFromFile("ss-v1-BC-cA"), SEC_ERROR_UNKNOWN_ISSUER);
   checkEndEntity(certFromFile("ss-v2-BC-cA"), SEC_ERROR_UNKNOWN_ISSUER);
   checkEndEntity(certFromFile("ss-v3-BC-cA"), SEC_ERROR_UNKNOWN_ISSUER);
   checkEndEntity(certFromFile("ss-v4-BC-cA"), SEC_ERROR_UNKNOWN_ISSUER);
+
+  checkCertVersion(certFromFile("ss-v1-noBC"), "Version 1");
+  checkCertVersion(certFromFile("int-v2-BC-cA_ca"), "Version 2");
+  checkCertVersion(certFromFile("ee-v3-BC-not-cA_ca"), "Version 3");
+  checkCertVersion(certFromFile("int-v4-BC-not-cA_ca"), "Version 4");
 }