bug 1264761 - improve handling of x509 versions in certificate manager r?Cykesiopka
MozReview-Commit-ID: B7EPx63ttlt
--- 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,57 @@ 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;
-
+ unsigned int version;
if (versionItem->data) {
- rv = GetIntValue(versionItem, &version);
- if (NS_FAILED(rv))
- return rv;
+ // Filter out totally bogus version values/encodings.
+ if (versionItem->len != 1) {
+ return NS_ERROR_FAILURE;
+ }
+ version = *reinterpret_cast<const uint8_t*>(versionItem->data);
} 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");
}