Bug 1085074 - Part 2 - Use explicit bit sizes for key size cert file names. r=briansmith

security/certverifier/ExtendedValidation.cpp

@@ -123,16 +123,16 @@ static struct nsMyTrustedEVInfo myTrustedEVInfos[] = {
   },
   {
     // The RSA root with an inadequate key size used for EV key size checking
-    // O=ev-rsa-caBad,CN=XPCShell Key Size Testing rsa 2040-bit (EV)
+    // O=ev_root_rsa_2040,CN=XPCShell Key Size Testing rsa 2040-bit (EV)
     "1.3.6.1.4.1.13769.666.666.666.1.500.9.1",
     "DEBUGtesting EV OID",
     SEC_OID_UNKNOWN,
-    { 0x0E, 0xE2, 0x7A, 0x44, 0xD3, 0xAB, 0x66, 0x1A, 0x31, 0xBF, 0x0C,
-      0x1C, 0xFC, 0xAA, 0xD9, 0xD6, 0x27, 0x75, 0xC2, 0xDB, 0xC5, 0x69,
-      0xD7, 0x1C, 0xDE, 0x9C, 0x7E, 0xD5, 0x86, 0x88, 0x6C, 0xB7 },
-    "ME0xNDAyBgNVBAMMK1hQQ1NoZWxsIEtleSBTaXplIFRlc3RpbmcgcnNhIDIwNDAt"
-    "Yml0IChFVikxFTATBgNVBAoMDGV2LXJzYS1jYUJhZA==",
-    "PCQ3",
+    { 0xA9, 0xCF, 0x93, 0x7B, 0x12, 0x9E, 0x39, 0xD2, 0x43, 0x10, 0x33,
+      0x6B, 0xC6, 0xAD, 0x86, 0xA2, 0x7A, 0x9D, 0xA4, 0x5B, 0x67, 0xB2,
+      0xB7, 0xC1, 0xDC, 0x47, 0x8E, 0xD8, 0xA9, 0x6E, 0x2D, 0x6A },
+    "MFExNDAyBgNVBAMMK1hQQ1NoZWxsIEtleSBTaXplIFRlc3RpbmcgcnNhIDIwNDAt"
+    "Yml0IChFVikxGTAXBgNVBAoMEGV2X3Jvb3RfcnNhXzIwNDA=",
+    "ASt16w==",
     nullptr
   },
 #endif

security/manager/ssl/tests/unit/test_keysize.js

@@ -53,28 +53,42 @@ function check_fail_ca(cert) {
                                 certificateUsageSSLCA);
 }
 
-function check_for_key_type(key_type) {
+function checkForKeyType(keyType, inadequateKeySize, adequateKeySize) {
+  let rootOKName = "root_" + keyType + "_" + adequateKeySize;
+  let rootNotOKName = "root_" + keyType + "_" + inadequateKeySize;
+  let intOKName = "int_" + keyType + "_" + adequateKeySize;
+  let intNotOKName = "int_" + keyType + "_" + inadequateKeySize;
+  let eeOKName = "ee_" + keyType + "_" + adequateKeySize;
+  let eeNotOKName = "ee_" + keyType + "_" + inadequateKeySize;
+
   // Chain with certs that have adequate sizes for DV
-  check_ok_ca(load_cert(key_type + "-caOK", "CTu,CTu,CTu"));
-  check_ok_ca(load_cert(key_type + "-intOK-caOK", ",,"));
-  check_ok(certFromFile(key_type + "-eeOK-intOK-caOK.der"));
+  let intFullName = intOKName + "-" + rootOKName;
+  let eeFullName = eeOKName + "-" + intOKName + "-" + rootOKName;
+  check_ok_ca(load_cert(rootOKName, "CTu,CTu,CTu"));
+  check_ok_ca(load_cert(intFullName, ",,"));
+  check_ok(certFromFile(eeFullName + ".der"));
 
   // Chain with a root cert that has an inadequate size for DV
-  check_fail_ca(load_cert(key_type + "-caBad", "CTu,CTu,CTu"));
-  check_fail_ca(load_cert(key_type + "-intOK-caBad", ",,"));
-  check_fail(certFromFile(key_type + "-eeOK-intOK-caBad.der"));
+  intFullName = intOKName + "-" + rootNotOKName;
+  eeFullName = eeOKName + "-" + intOKName + "-" + rootNotOKName;
+  check_fail_ca(load_cert(rootNotOKName, "CTu,CTu,CTu"));
+  check_fail_ca(load_cert(intFullName, ",,"));
+  check_fail(certFromFile(eeFullName + ".der"));
 
   // Chain with an intermediate cert that has an inadequate size for DV
-  check_fail_ca(load_cert(key_type + "-intBad-caOK", ",,"));
-  check_fail(certFromFile(key_type + "-eeOK-intBad-caOK.der"));
+  intFullName = intNotOKName + "-" + rootOKName;
+  eeFullName = eeOKName + "-" + intNotOKName + "-" + rootOKName;
+  check_fail_ca(load_cert(intFullName, ",,"));
+  check_fail(certFromFile(eeFullName + ".der"));
 
   // Chain with an end entity cert that has an inadequate size for DV
-  check_fail(certFromFile(key_type + "-eeBad-intOK-caOK.der"));
+  eeFullName = eeNotOKName + "-" + intOKName + "-" + rootOKName;
+  check_fail(certFromFile(eeFullName + ".der"));
 }
 
 function run_test() {
-  check_for_key_type("rsa");
-  check_for_key_type("dsa");
+  checkForKeyType("rsa", 1016, 1024);
+  checkForKeyType("dsa", 960, 1024);
 
   run_next_test();
 }

security/manager/ssl/tests/unit/test_keysize/generate.py

@@ -33,15 +33,17 @@ mozilla_testing_ev_policy = ('certificatePolicies = @v3_ca_ev_cp\n\n' +
 
 generated_ev_root_filenames = []
 
-def generate_and_maybe_import_cert(key_type, cert_name_suffix, base_ext_text,
-                                   signer_key_filename, signer_cert_filename,
-                                   dsa_param_filename, key_size, generate_ev):
+def generate_and_maybe_import_cert(key_type, cert_name_prefix, cert_name_suffix,
+                                   base_ext_text, signer_key_filename,
+                                   signer_cert_filename, dsa_param_filename,
+                                   key_size, generate_ev):
     """
     Generates a certificate and imports it into the NSS DB if appropriate.
 
     Arguments:
       key_type -- the type of key generated: potential values: 'rsa', 'dsa',
                   or any of the curves found by 'openssl ecparam -list_curves'
+      cert_name_prefix -- prefix of the generated cert name
       cert_name_suffix -- suffix of the generated cert name
       base_ext_text -- the base text for the x509 extensions to be added to the
                        certificate (extra extensions will be added if generating
@@ -58,15 +60,21 @@ def generate_and_maybe_import_cert(key_type, cert_name_suffix, base_ext_text,
       generate_ev -- whether an EV cert should be generated
 
     Output:
+      cert_name -- the resultant (nick)name of the certificate
       key_filename -- the filename of the key file (PEM format)
       cert_filename -- the filename of the certificate (DER format)
     """
-    cert_name = key_type + cert_name_suffix
+    cert_name = cert_name_prefix + '_' + key_type + '_' + key_size
+
+    # If the suffix is not the empty string, add a hyphen for visual separation
+    if cert_name_suffix:
+        cert_name += '-' + cert_name_suffix
+
     ev_ext_text = ''
     subject_string = ('/CN=XPCShell Key Size Testing %s %s-bit' %
                       (key_type, key_size))
     if generate_ev:
-        cert_name = 'ev-' + cert_name
+        cert_name = 'ev_' + cert_name
         ev_ext_text = (aia_prefix + cert_name + aia_suffix +
                        mozilla_testing_ev_policy)
         subject_string += ' (EV)'
@@ -99,7 +107,7 @@ def generate_and_maybe_import_cert(key_type, cert_name_suffix, base_ext_text,
         if not signer_key_filename:
             generated_ev_root_filenames.append(cert_filename)
 
-    return [key_filename, cert_filename]
+    return [cert_name, key_filename, cert_filename]
 
 def generate_certs(key_type, inadequate_key_size, adequate_key_size, generate_ev):
     """
@@ -121,16 +129,17 @@ def generate_certs(key_type, inadequate_key_size, adequate_key_size, generate_ev
     # Generate chain with certs that have adequate sizes
     if generate_ev and key_type == 'rsa':
         # Reuse the existing RSA EV root
-        caOK_cert_name = 'evroot'
+        rootOK_nick = 'evroot'
         caOK_key = '../test_ev_certs/evroot.key'
         caOK_cert = '../test_ev_certs/evroot.der'
         caOK_pkcs12_filename = '../test_ev_certs/evroot.p12'
         CertUtils.import_cert_and_pkcs12(srcdir, caOK_cert, caOK_pkcs12_filename,
-                                         caOK_cert_name, ',,')
+                                         rootOK_nick, ',,')
     else:
-        [caOK_key, caOK_cert] = generate_and_maybe_import_cert(
+        [rootOK_nick, caOK_key, caOK_cert] = generate_and_maybe_import_cert(
             key_type,
-            '-caOK',
+            'root',
+            '',
             ca_ext_text,
             '',
             '',
@@ -138,9 +147,10 @@ def generate_certs(key_type, inadequate_key_size, adequate_key_size, generate_ev
             adequate_key_size,
             generate_ev)
 
-    [intOK_key, intOK_cert] = generate_and_maybe_import_cert(
+    [intOK_nick, intOK_key, intOK_cert] = generate_and_maybe_import_cert(
         key_type,
-        '-intOK-caOK',
+        'int',
+        rootOK_nick,
         ca_ext_text,
         caOK_key,
         caOK_cert,
@@ -150,7 +160,8 @@ def generate_certs(key_type, inadequate_key_size, adequate_key_size, generate_ev
 
     generate_and_maybe_import_cert(
         key_type,
-        '-eeOK-intOK-caOK',
+        'ee',
+        intOK_nick,
         ee_ext_text,
         intOK_key,
         intOK_cert,
@@ -159,9 +170,10 @@ def generate_certs(key_type, inadequate_key_size, adequate_key_size, generate_ev
         generate_ev)
 
     # Generate chain with a root cert that has an inadequate size
-    [rootNotOK_key, rootNotOK_cert] = generate_and_maybe_import_cert(
+    [rootNotOK_nick, rootNotOK_key, rootNotOK_cert] = generate_and_maybe_import_cert(
         key_type,
-        '-caBad',
+        'root',
+        '',
         ca_ext_text,
         '',
         '',
@@ -169,9 +181,10 @@ def generate_certs(key_type, inadequate_key_size, adequate_key_size, generate_ev
         inadequate_key_size,
         generate_ev)
 
-    [int_key, int_cert] = generate_and_maybe_import_cert(
+    [int_nick, int_key, int_cert] = generate_and_maybe_import_cert(
         key_type,
-        '-intOK-caBad',
+        'int',
+        rootNotOK_nick,
         ca_ext_text,
         rootNotOK_key,
         rootNotOK_cert,
@@ -181,7 +194,8 @@ def generate_certs(key_type, inadequate_key_size, adequate_key_size, generate_ev
 
     generate_and_maybe_import_cert(
         key_type,
-        '-eeOK-intOK-caBad',
+        'ee',
+        int_nick,
         ee_ext_text,
         int_key,
         int_cert,
@@ -190,9 +204,10 @@ def generate_certs(key_type, inadequate_key_size, adequate_key_size, generate_ev
         generate_ev)
 
     # Generate chain with an intermediate cert that has an inadequate size
-    [intNotOK_key, intNotOK_cert] = generate_and_maybe_import_cert(
+    [intNotOK_nick, intNotOK_key, intNotOK_cert] = generate_and_maybe_import_cert(
         key_type,
-        '-intBad-caOK',
+        'int',
+        rootOK_nick,
         ca_ext_text,
         caOK_key,
         caOK_cert,
@@ -202,7 +217,8 @@ def generate_certs(key_type, inadequate_key_size, adequate_key_size, generate_ev
 
     generate_and_maybe_import_cert(
         key_type,
-        '-eeOK-intBad-caOK',
+        'ee',
+        intNotOK_nick,
         ee_ext_text,
         intNotOK_key,
         intNotOK_cert,
@@ -213,7 +229,8 @@ def generate_certs(key_type, inadequate_key_size, adequate_key_size, generate_ev
     # Generate chain with an end entity cert that has an inadequate size
     generate_and_maybe_import_cert(
         key_type,
-        '-eeBad-intOK-caOK',
+        'ee',
+        intOK_nick,
         ee_ext_text,
         intOK_key,
         intOK_cert,

security/manager/ssl/tests/unit/test_keysize_ev.js

@@ -46,10 +46,7 @@ function checkEVStatus(cert, usage, isEVExpected) {
  * Adds a single EV key size test.
  *
  * @param {Array} expectedNamesForOCSP
- *        An array of nicknames of the certs to be responded to. The cert name
- *        prefix is not added to the nicknames in this array.
- * @param {String} certNamePrefix
- *        The prefix to prepend to the passed in cert names.
+ *        An array of nicknames of the certs to be responded to.
  * @param {String} rootCACertFileName
  *        The file name of the root CA cert. Can begin with ".." to reference
  *        certs in folders other than "test_keysize/".
@@ -60,7 +57,7 @@ function checkEVStatus(cert, usage, isEVExpected) {
  * @param {Boolean} expectedResult
  *        Whether the chain is expected to validate as EV.
  */
-function addKeySizeTestForEV(expectedNamesForOCSP, certNamePrefix,
+function addKeySizeTestForEV(expectedNamesForOCSP,
                              rootCACertFileName, subCACertFileNames,
                              endEntityCertFileName, expectedResult)
 {
@@ -68,16 +65,11 @@ function addKeySizeTestForEV(expectedNamesForOCSP, certNamePrefix,
     clearOCSPCache();
     let ocspResponder = getOCSPResponder(expectedNamesForOCSP);
 
-    // Don't prepend the cert name prefix if rootCACertFileName starts with ".."
-    // to support reusing certs in other directories.
-    let rootCertNamePrefix = rootCACertFileName.startsWith("..")
-                           ? ""
-                           : certNamePrefix;
-    loadCert(rootCertNamePrefix + rootCACertFileName, "CTu,CTu,CTu");
+    loadCert(rootCACertFileName, "CTu,CTu,CTu");
     for (let subCACertFileName of subCACertFileNames) {
-      loadCert(certNamePrefix + subCACertFileName, ",,");
+      loadCert(subCACertFileName, ",,");
     }
-    checkEVStatus(certFromFile(certNamePrefix + endEntityCertFileName + ".der"),
+    checkEVStatus(certFromFile(endEntityCertFileName + ".der"),
                   certificateUsageSSLServer, expectedResult);
 
     ocspResponder.stop(run_next_test);
@@ -97,62 +89,70 @@ function addKeySizeTestForEV(expectedNamesForOCSP, certNamePrefix,
  *
  * @param {String} keyType
  *        The key type to check (e.g. "rsa").
+ * @param {Number} inadequateKeySize
+ *        The inadequate key size of the generated certs.
+ * @param {Number} adequateKeySize
+ *        The adequate key size of the generated certs.
  */
-function checkForKeyType(keyType) {
-  let certNamePrefix = "ev-" + keyType;
-
+function checkForKeyType(keyType, inadequateKeySize, adequateKeySize) {
   // Reuse the existing test RSA EV root
-  let rootCAOKCertFileName = keyType == "rsa" ? "../test_ev_certs/evroot"
-                                              : "-caOK";
+  let rootOKCertFileName = keyType == "rsa"
+                         ? "../test_ev_certs/evroot"
+                         : "ev_root_" + keyType + "_" + adequateKeySize;
+  let rootOKName = keyType == "rsa"
+                 ? "evroot"
+                 : "ev_root_" + keyType + "_" + adequateKeySize;
+  let rootNotOKName = "ev_root_" + keyType + "_" + inadequateKeySize;
+  let intOKName = "ev_int_" + keyType + "_" + adequateKeySize;
+  let intNotOKName = "ev_int_" + keyType + "_" + inadequateKeySize;
+  let eeOKName = "ev_ee_" + keyType + "_" + adequateKeySize;
+  let eeNotOKName = "ev_ee_" + keyType + "_" + inadequateKeySize;
 
   // Chain with certs that have adequate sizes for EV and DV
   // In opt builds, this chain is only validated for DV. Hence, an OCSP fetch
-  // will not be done for the "-intOK-caOK" intermediate in such a build.
+  // will for example not be done for the "ev_int_rsa_2048-evroot" intermediate
+  // in such a build.
+  let intFullName = intOKName + "-" + rootOKName;
+  let eeFullName = eeOKName + "-" + intOKName + "-" + rootOKName;
   let expectedNamesForOCSP = isDebugBuild
-                           ? [ certNamePrefix + "-intOK-caOK",
-                               certNamePrefix + "-eeOK-intOK-caOK" ]
-                           : [ certNamePrefix + "-eeOK-intOK-caOK" ];
-  addKeySizeTestForEV(expectedNamesForOCSP, certNamePrefix,
-                      rootCAOKCertFileName,
-                      ["-intOK-caOK"],
-                      "-eeOK-intOK-caOK",
-                      isDebugBuild);
+                           ? [ intFullName,
+                               eeFullName ]
+                           : [ eeFullName ];
+  addKeySizeTestForEV(expectedNamesForOCSP, rootOKCertFileName,
+                      [ intFullName ], eeFullName, isDebugBuild);
 
   // Chain with a root cert that has an inadequate size for EV, but
   // adequate size for DV
-  expectedNamesForOCSP = [ certNamePrefix + "-eeOK-intOK-caBad" ];
-  addKeySizeTestForEV(expectedNamesForOCSP, certNamePrefix,
-                      "-caBad",
-                      ["-intOK-caBad"],
-                      "-eeOK-intOK-caBad",
-                      false);
+  intFullName = intOKName + "-" + rootNotOKName;
+  eeFullName = eeOKName + "-" + intOKName + "-" + rootNotOKName;
+  expectedNamesForOCSP = [ eeFullName ];
+  addKeySizeTestForEV(expectedNamesForOCSP, rootNotOKName,
+                      [ intFullName ], eeFullName, false);
 
   // Chain with an intermediate cert that has an inadequate size for EV, but
   // adequate size for DV
+  intFullName = intNotOKName + "-" + rootOKName;
+  eeFullName = eeOKName + "-" + intNotOKName + "-" + rootOKName;
   expectedNamesForOCSP = isDebugBuild
-                       ? [ certNamePrefix + "-intBad-caOK" ]
-                       : [ certNamePrefix + "-eeOK-intBad-caOK" ];
-  addKeySizeTestForEV(expectedNamesForOCSP, certNamePrefix,
-                      rootCAOKCertFileName,
-                      ["-intBad-caOK"],
-                      "-eeOK-intBad-caOK",
-                      false);
+                       ? [ intFullName ]
+                       : [ eeFullName ];
+  addKeySizeTestForEV(expectedNamesForOCSP, rootOKCertFileName,
+                      [ intFullName ], eeFullName, false);
 
   // Chain with an end entity cert that has an inadequate size for EV, but
   // adequate size for DV
-  expectedNamesForOCSP = [ certNamePrefix + "-eeBad-intOK-caOK" ];
-  addKeySizeTestForEV(expectedNamesForOCSP, certNamePrefix,
-                      rootCAOKCertFileName,
-                      ["-intOK-caOK"],
-                      "-eeBad-intOK-caOK",
-                      false);
+  intFullName = intOKName + "-" + rootOKName;
+  eeFullName = eeNotOKName + "-" + intOKName + "-" + rootOKName;
+  expectedNamesForOCSP = [ eeFullName ];
+  addKeySizeTestForEV(expectedNamesForOCSP, rootOKCertFileName,
+                      [ intFullName ], eeFullName, false);
 }
 
 function run_test() {
   // Setup OCSP responder
   Services.prefs.setCharPref("network.dns.localDomains", "www.example.com");
 
-  checkForKeyType("rsa");
+  checkForKeyType("rsa", 2040, 2048);
 
   run_next_test();
 }