Bug 1077926: Make test certificate generation faster by reusing key, r=keeler
☠☠ backed out by 8995092bd954 ☠ ☠
authorBrian Smith <brian@briansmith.org>
Sun, 05 Oct 2014 00:29:43 -0700
changeset 209125 124b04c01c7127593f89408edf82db7e22122735
parent 209124 16fe1b9eb9e6f66d787cbcc72cdfa5328f08f6fa
child 209126 6dc2eaf50c04bfab8f98b13756bb28c421674cf7
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewerskeeler
bugs1077926
milestone35.0a1
Bug 1077926: Make test certificate generation faster by reusing key, r=keeler
security/pkix/test/gtest/pkixbuild_tests.cpp
security/pkix/test/gtest/pkixcert_extension_tests.cpp
security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp
security/pkix/test/lib/pkixtestnss.cpp
security/pkix/test/lib/pkixtestutil.cpp
security/pkix/test/lib/pkixtestutil.h
--- a/security/pkix/test/gtest/pkixbuild_tests.cpp
+++ b/security/pkix/test/gtest/pkixbuild_tests.cpp
@@ -35,18 +35,16 @@ using namespace mozilla::pkix::test;
 typedef ScopedPtr<CERTCertificate, CERT_DestroyCertificate>
           ScopedCERTCertificate;
 typedef ScopedPtr<CERTCertList, CERT_DestroyCertList> ScopedCERTCertList;
 
 static ByteString
 CreateCert(const char* issuerCN,
            const char* subjectCN,
            EndEntityOrCA endEntityOrCA,
-           /*optional*/ TestKeyPair* issuerKey,
-           /*out*/ ScopedTestKeyPair& subjectKey,
            /*out*/ ScopedCERTCertificate* subjectCert = nullptr)
 {
   static long serialNumberValue = 0;
   ++serialNumberValue;
   ByteString serialNumber(CreateEncodedSerialNumber(serialNumberValue));
   EXPECT_FALSE(ENCODING_FAILED(serialNumber));
 
   ByteString issuerDER(CNToDERName(issuerCN));
@@ -55,23 +53,22 @@ CreateCert(const char* issuerCN,
   ByteString extensions[2];
   if (endEntityOrCA == EndEntityOrCA::MustBeCA) {
     extensions[0] =
       CreateEncodedBasicConstraints(true, nullptr,
                                     ExtensionCriticality::Critical);
     EXPECT_FALSE(ENCODING_FAILED(extensions[0]));
   }
 
+  ScopedTestKeyPair reusedKey(CloneReusedKeyPair());
   ByteString certDER(CreateEncodedCertificate(
-                       v3, sha256WithRSAEncryption,
-                       serialNumber, issuerDER,
-                       oneDayBeforeNow, oneDayAfterNow,
-                       subjectDER, extensions, issuerKey,
-                       SignatureAlgorithm::rsa_pkcs1_with_sha256,
-                       subjectKey));
+                       v3, sha256WithRSAEncryption, serialNumber, issuerDER,
+                       oneDayBeforeNow, oneDayAfterNow, subjectDER,
+                       *reusedKey, extensions, *reusedKey,
+                       SignatureAlgorithm::rsa_pkcs1_with_sha256));
   EXPECT_FALSE(ENCODING_FAILED(certDER));
   if (subjectCert) {
     SECItem certDERItem = {
       siBuffer,
       const_cast<uint8_t*>(certDER.data()),
       static_cast<unsigned int>(certDER.length())
     };
     *subjectCert = CERT_NewTempCertificate(CERT_GetDefaultCertDB(),
@@ -95,17 +92,17 @@ public:
 
     static_assert(MOZILLA_PKIX_ARRAY_LENGTH(names) ==
                     MOZILLA_PKIX_ARRAY_LENGTH(certChainTail),
                   "mismatch in sizes of names and certChainTail arrays");
 
     for (size_t i = 0; i < MOZILLA_PKIX_ARRAY_LENGTH(names); ++i) {
       const char* issuerName = i == 0 ? names[0] : names[i-1];
       (void) CreateCert(issuerName, names[i], EndEntityOrCA::MustBeCA,
-                        leafCAKey.get(), leafCAKey, &certChainTail[i]);
+                        &certChainTail[i]);
     }
 
     return true;
   }
 
 private:
   virtual Result GetCertTrust(EndEntityOrCA, const CertPolicyId&,
                               Input candidateCert,
@@ -187,17 +184,16 @@ private:
     return TestCheckPublicKey(subjectPublicKeyInfo);
   }
 
   // We hold references to CERTCertificates in the cert chain tail so that we
   // CERT_CreateSubjectCertList can find them.
   ScopedCERTCertificate certChainTail[7];
 
 public:
-  ScopedTestKeyPair leafCAKey;
   CERTCertificate* GetLeafCACert() const
   {
     return certChainTail[MOZILLA_PKIX_ARRAY_LENGTH(certChainTail) - 1].get();
   }
 };
 
 class pkixbuild : public ::testing::Test
 {
@@ -233,64 +229,58 @@ TEST_F(pkixbuild, MaxAcceptableCertChain
                              EndEntityOrCA::MustBeCA,
                              KeyUsage::noParticularKeyUsageRequired,
                              KeyPurposeId::id_kp_serverAuth,
                              CertPolicyId::anyPolicy,
                              nullptr/*stapledOCSPResponse*/));
   }
 
   {
-    ScopedTestKeyPair unusedKeyPair;
     ScopedCERTCertificate cert;
     ByteString certDER(CreateCert("CA7", "Direct End-Entity",
-                                  EndEntityOrCA::MustBeEndEntity,
-                                  trustDomain.leafCAKey.get(), unusedKeyPair));
+                                  EndEntityOrCA::MustBeEndEntity));
     ASSERT_FALSE(ENCODING_FAILED(certDER));
     Input certDERInput;
     ASSERT_EQ(Success, certDERInput.Init(certDER.data(), certDER.length()));
     ASSERT_EQ(Success,
               BuildCertChain(trustDomain, certDERInput, Now(),
                              EndEntityOrCA::MustBeEndEntity,
                              KeyUsage::noParticularKeyUsageRequired,
                              KeyPurposeId::id_kp_serverAuth,
                              CertPolicyId::anyPolicy,
                              nullptr/*stapledOCSPResponse*/));
   }
 }
 
 TEST_F(pkixbuild, BeyondMaxAcceptableCertChainLength)
 {
   static char const* const caCertName = "CA Too Far";
-  ScopedTestKeyPair caKeyPair;
 
   // We need a CERTCertificate for caCert so that the trustdomain's FindIssuer
   // method can find it through the NSS cert DB.
   ScopedCERTCertificate caCert;
 
   {
     ByteString certDER(CreateCert("CA7", caCertName, EndEntityOrCA::MustBeCA,
-                                  trustDomain.leafCAKey.get(), caKeyPair,
                                   &caCert));
     ASSERT_FALSE(ENCODING_FAILED(certDER));
     Input certDERInput;
     ASSERT_EQ(Success, certDERInput.Init(certDER.data(), certDER.length()));
     ASSERT_EQ(Result::ERROR_UNKNOWN_ISSUER,
               BuildCertChain(trustDomain, certDERInput, Now(),
                              EndEntityOrCA::MustBeCA,
                              KeyUsage::noParticularKeyUsageRequired,
                              KeyPurposeId::id_kp_serverAuth,
                              CertPolicyId::anyPolicy,
                              nullptr/*stapledOCSPResponse*/));
   }
 
   {
-    ScopedTestKeyPair unusedKeyPair;
     ByteString certDER(CreateCert(caCertName, "End-Entity Too Far",
-                                  EndEntityOrCA::MustBeEndEntity,
-                                  caKeyPair.get(), unusedKeyPair));
+                                  EndEntityOrCA::MustBeEndEntity));
     ASSERT_FALSE(ENCODING_FAILED(certDER));
     Input certDERInput;
     ASSERT_EQ(Success, certDERInput.Init(certDER.data(), certDER.length()));
     ASSERT_EQ(Result::ERROR_UNKNOWN_ISSUER,
               BuildCertChain(trustDomain, certDERInput, Now(),
                              EndEntityOrCA::MustBeEndEntity,
                              KeyUsage::noParticularKeyUsageRequired,
                              KeyPurposeId::id_kp_serverAuth,
@@ -378,35 +368,33 @@ public:
 
 private:
   ByteString rootDER;
 };
 
 TEST_F(pkixbuild, NoRevocationCheckingForExpiredCert)
 {
   const char* rootCN = "Root CA";
-  ScopedTestKeyPair rootKey;
   ByteString rootDER(CreateCert(rootCN, rootCN, EndEntityOrCA::MustBeCA,
-                                nullptr, rootKey, nullptr));
+                                nullptr));
   EXPECT_FALSE(ENCODING_FAILED(rootDER));
   ExpiredCertTrustDomain expiredCertTrustDomain(rootDER);
 
   ByteString serialNumber(CreateEncodedSerialNumber(100));
   EXPECT_FALSE(ENCODING_FAILED(serialNumber));
   ByteString issuerDER(CNToDERName(rootCN));
   ByteString subjectDER(CNToDERName("Expired End-Entity Cert"));
-  ScopedTestKeyPair unusedSubjectKey;
+  ScopedTestKeyPair reusedKey(CloneReusedKeyPair());
   ByteString certDER(CreateEncodedCertificate(
                        v3, sha256WithRSAEncryption,
                        serialNumber, issuerDER,
                        oneDayBeforeNow - Time::ONE_DAY_IN_SECONDS,
                        oneDayBeforeNow,
-                       subjectDER, nullptr, rootKey.get(),
-                       SignatureAlgorithm::rsa_pkcs1_with_sha256,
-                       unusedSubjectKey));
+                       subjectDER, *reusedKey, nullptr, *reusedKey,
+                       SignatureAlgorithm::rsa_pkcs1_with_sha256));
   EXPECT_FALSE(ENCODING_FAILED(certDER));
 
   Input cert;
   ASSERT_EQ(Success, cert.Init(certDER.data(), certDER.length()));
   ASSERT_EQ(Result::ERROR_EXPIRED_CERTIFICATE,
             BuildCertChain(expiredCertTrustDomain, cert, Now(),
                            EndEntityOrCA::MustBeEndEntity,
                            KeyUsage::noParticularKeyUsageRequired,
--- a/security/pkix/test/gtest/pkixcert_extension_tests.cpp
+++ b/security/pkix/test/gtest/pkixcert_extension_tests.cpp
@@ -38,23 +38,23 @@ CreateCert(const char* subjectCN,
   static long serialNumberValue = 0;
   ++serialNumberValue;
   ByteString serialNumber(CreateEncodedSerialNumber(serialNumberValue));
   EXPECT_FALSE(ENCODING_FAILED(serialNumber));
   ByteString issuerDER(CNToDERName(subjectCN));
   EXPECT_FALSE(ENCODING_FAILED(issuerDER));
   ByteString subjectDER(CNToDERName(subjectCN));
   EXPECT_FALSE(ENCODING_FAILED(subjectDER));
+  subjectKey = CloneReusedKeyPair();
   return CreateEncodedCertificate(v3, sha256WithRSAEncryption,
                                   serialNumber, issuerDER,
                                   oneDayBeforeNow, oneDayAfterNow,
-                                  subjectDER, extensions,
-                                  nullptr,
-                                  SignatureAlgorithm::rsa_pkcs1_with_sha256,
-                                  subjectKey);
+                                  subjectDER, *subjectKey, extensions,
+                                  *subjectKey,
+                                  SignatureAlgorithm::rsa_pkcs1_with_sha256);
 }
 
 // Creates a self-signed certificate with the given extension.
 static ByteString
 CreateCert(const char* subjectStr,
            const ByteString& extension,
            /*out*/ ScopedTestKeyPair& subjectKey)
 {
--- a/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp
+++ b/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp
@@ -387,22 +387,22 @@ protected:
 
     const ByteString extensions[] = {
       signerEKUDER
         ? CreateEncodedEKUExtension(*signerEKUDER,
                                     ExtensionCriticality::NotCritical)
         : ByteString(),
       ByteString()
     };
-    ScopedTestKeyPair signerKeyPair;
+    ScopedTestKeyPair signerKeyPair(GenerateKeyPair());
     ByteString signerDER(CreateEncodedCertificate(
                            ++rootIssuedCount, rootName,
                            oneDayBeforeNow, oneDayAfterNow, certSubjectName,
-                           signerEKUDER ? extensions : nullptr,
-                           rootKeyPair.get(), signerKeyPair));
+                           *signerKeyPair, signerEKUDER ? extensions : nullptr,
+                           *rootKeyPair));
     EXPECT_FALSE(ENCODING_FAILED(signerDER));
     if (signerDEROut) {
       *signerDEROut = signerDER;
     }
 
     ByteString signerNameDER;
     if (signerName) {
       signerNameDER = CNToDERName(signerName);
@@ -416,19 +416,19 @@ protected:
                                                &oneDayAfterNow, certs);
   }
 
   static ByteString CreateEncodedCertificate(uint32_t serialNumber,
                                              const char* issuer,
                                              time_t notBefore,
                                              time_t notAfter,
                                              const char* subject,
+                                             const TestKeyPair& subjectKeyPair,
                                 /*optional*/ const ByteString* extensions,
-                                /*optional*/ TestKeyPair* signerKeyPair,
-                                     /*out*/ ScopedTestKeyPair& keyPair)
+                                             const TestKeyPair& signerKeyPair)
   {
     ByteString serialNumberDER(CreateEncodedSerialNumber(serialNumber));
     if (ENCODING_FAILED(serialNumberDER)) {
       return ByteString();
     }
     ByteString issuerDER(CNToDERName(issuer));
     if (ENCODING_FAILED(issuerDER)) {
       return ByteString();
@@ -436,20 +436,19 @@ protected:
     ByteString subjectDER(CNToDERName(subject));
     if (ENCODING_FAILED(subjectDER)) {
       return ByteString();
     }
     return ::mozilla::pkix::test::CreateEncodedCertificate(
                                     v3,
                                     sha256WithRSAEncryption,
                                     serialNumberDER, issuerDER, notBefore,
-                                    notAfter, subjectDER, extensions,
-                                    signerKeyPair,
-                                    SignatureAlgorithm::rsa_pkcs1_with_sha256,
-                                    keyPair);
+                                    notAfter, subjectDER, subjectKeyPair,
+                                    extensions, signerKeyPair,
+                                    SignatureAlgorithm::rsa_pkcs1_with_sha256);
   }
 
   static const Input OCSPSigningEKUDER;
 };
 
 /*static*/ const Input pkixocsp_VerifyEncodedResponse_DelegatedResponder::
   OCSPSigningEKUDER(tlv_id_kp_OCSPSigning);
 
@@ -535,23 +534,23 @@ TEST_F(pkixocsp_VerifyEncodedResponse_De
   static const char* signerName = "good_indirect_expired";
 
   const ByteString extensions[] = {
     CreateEncodedEKUExtension(OCSPSigningEKUDER,
                               ExtensionCriticality::NotCritical),
     ByteString()
   };
 
-  ScopedTestKeyPair signerKeyPair;
+  ScopedTestKeyPair signerKeyPair(GenerateKeyPair());
   ByteString signerDER(CreateEncodedCertificate(
                           ++rootIssuedCount, rootName,
                           now - (10 * Time::ONE_DAY_IN_SECONDS),
                           now - (2 * Time::ONE_DAY_IN_SECONDS),
-                          signerName, extensions, rootKeyPair.get(),
-                          signerKeyPair));
+                          signerName, *signerKeyPair, extensions,
+                          *rootKeyPair));
   ASSERT_FALSE(ENCODING_FAILED(signerDER));
 
   ByteString certs[] = { signerDER, ByteString() };
   ByteString responseString(
                CreateEncodedOCSPSuccessfulResponse(
                          OCSPResponseContext::good, *endEntityCertID,
                          signerName, *signerKeyPair, oneDayBeforeNow,
                          oneDayBeforeNow, &oneDayAfterNow, certs));
@@ -570,23 +569,23 @@ TEST_F(pkixocsp_VerifyEncodedResponse_De
   static const char* signerName = "good_indirect_future";
 
   const ByteString extensions[] = {
     CreateEncodedEKUExtension(OCSPSigningEKUDER,
                               ExtensionCriticality::NotCritical),
     ByteString()
   };
 
-  ScopedTestKeyPair signerKeyPair;
+  ScopedTestKeyPair signerKeyPair(GenerateKeyPair());
   ByteString signerDER(CreateEncodedCertificate(
                          ++rootIssuedCount, rootName,
                          now + (2 * Time::ONE_DAY_IN_SECONDS),
                          now + (10 * Time::ONE_DAY_IN_SECONDS),
-                         signerName, extensions, rootKeyPair.get(),
-                         signerKeyPair));
+                         signerName, *signerKeyPair, extensions,
+                         *rootKeyPair));
   ASSERT_FALSE(ENCODING_FAILED(signerDER));
 
   ByteString certs[] = { signerDER, ByteString() };
   ByteString responseString(
                CreateEncodedOCSPSuccessfulResponse(
                          OCSPResponseContext::good, *endEntityCertID,
                          signerName, *signerKeyPair, oneDayBeforeNow,
                          oneDayBeforeNow, &oneDayAfterNow, certs));
@@ -672,21 +671,21 @@ TEST_F(pkixocsp_VerifyEncodedResponse_De
   ASSERT_TRUE(unknownKeyPair);
 
   // Delegated responder cert signed by unknown issuer
   const ByteString extensions[] = {
     CreateEncodedEKUExtension(OCSPSigningEKUDER,
                               ExtensionCriticality::NotCritical),
     ByteString()
   };
-  ScopedTestKeyPair signerKeyPair;
+  ScopedTestKeyPair signerKeyPair(GenerateKeyPair());
   ByteString signerDER(CreateEncodedCertificate(
                          1, subCAName, oneDayBeforeNow, oneDayAfterNow,
-                         signerName, extensions, unknownKeyPair.get(),
-                         signerKeyPair));
+                         signerName, *signerKeyPair, extensions,
+                         *unknownKeyPair));
   ASSERT_FALSE(ENCODING_FAILED(signerDER));
 
   // OCSP response signed by that delegated responder
   ByteString certs[] = { signerDER, ByteString() };
   ByteString responseString(
                CreateEncodedOCSPSuccessfulResponse(
                          OCSPResponseContext::good, *endEntityCertID,
                          signerName, *signerKeyPair, oneDayBeforeNow,
@@ -711,35 +710,35 @@ TEST_F(pkixocsp_VerifyEncodedResponse_De
   static const char* subCAName = "good_indirect_subca_1_first sub-CA";
   static const char* signerName = "good_indirect_subca_1_first OCSP signer";
 
   // sub-CA of root (root is the direct issuer of endEntity)
   const ByteString subCAExtensions[] = {
     CreateEncodedBasicConstraints(true, 0, ExtensionCriticality::NotCritical),
     ByteString()
   };
-  ScopedTestKeyPair subCAKeyPair;
+  ScopedTestKeyPair subCAKeyPair(GenerateKeyPair());
   ByteString subCADER(CreateEncodedCertificate(
                         ++rootIssuedCount, rootName,
                         oneDayBeforeNow, oneDayAfterNow,
-                        subCAName, subCAExtensions, rootKeyPair.get(),
-                        subCAKeyPair));
+                        subCAName, *subCAKeyPair, subCAExtensions,
+                        *rootKeyPair));
   ASSERT_FALSE(ENCODING_FAILED(subCADER));
 
   // Delegated responder cert signed by that sub-CA
   const ByteString extensions[] = {
     CreateEncodedEKUExtension(OCSPSigningEKUDER,
                               ExtensionCriticality::NotCritical),
     ByteString(),
   };
-  ScopedTestKeyPair signerKeyPair;
+  ScopedTestKeyPair signerKeyPair(GenerateKeyPair());
   ByteString signerDER(CreateEncodedCertificate(
                          1, subCAName, oneDayBeforeNow, oneDayAfterNow,
-                         signerName, extensions, subCAKeyPair.get(),
-                         signerKeyPair));
+                         signerName, *signerKeyPair, extensions,
+                         *subCAKeyPair));
   ASSERT_FALSE(ENCODING_FAILED(signerDER));
 
   // OCSP response signed by the delegated responder issued by the sub-CA
   // that is trying to impersonate the root.
   ByteString certs[] = { subCADER, signerDER, ByteString() };
   ByteString responseString(
                CreateEncodedOCSPSuccessfulResponse(
                          OCSPResponseContext::good, *endEntityCertID,
@@ -765,35 +764,34 @@ TEST_F(pkixocsp_VerifyEncodedResponse_De
   static const char* subCAName = "good_indirect_subca_1_second sub-CA";
   static const char* signerName = "good_indirect_subca_1_second OCSP signer";
 
   // sub-CA of root (root is the direct issuer of endEntity)
   const ByteString subCAExtensions[] = {
     CreateEncodedBasicConstraints(true, 0, ExtensionCriticality::NotCritical),
     ByteString()
   };
-  ScopedTestKeyPair subCAKeyPair;
+  ScopedTestKeyPair subCAKeyPair(GenerateKeyPair());
   ByteString subCADER(CreateEncodedCertificate(++rootIssuedCount, rootName,
                                                oneDayBeforeNow, oneDayAfterNow,
-                                               subCAName, subCAExtensions,
-                                               rootKeyPair.get(),
-                                               subCAKeyPair));
+                                               subCAName, *subCAKeyPair,
+                                               subCAExtensions, *rootKeyPair));
   ASSERT_FALSE(ENCODING_FAILED(subCADER));
 
   // Delegated responder cert signed by that sub-CA
   const ByteString extensions[] = {
     CreateEncodedEKUExtension(OCSPSigningEKUDER,
                               ExtensionCriticality::NotCritical),
     ByteString()
   };
-  ScopedTestKeyPair signerKeyPair;
+  ScopedTestKeyPair signerKeyPair(GenerateKeyPair());
   ByteString signerDER(CreateEncodedCertificate(
                          1, subCAName, oneDayBeforeNow, oneDayAfterNow,
-                         signerName, extensions, subCAKeyPair.get(),
-                         signerKeyPair));
+                         signerName, *signerKeyPair, extensions,
+                         *subCAKeyPair));
   ASSERT_FALSE(ENCODING_FAILED(signerDER));
 
   // OCSP response signed by the delegated responder issued by the sub-CA
   // that is trying to impersonate the root.
   ByteString certs[] = { signerDER, subCADER, ByteString() };
   ByteString responseString(
                  CreateEncodedOCSPSuccessfulResponse(
                          OCSPResponseContext::good, *endEntityCertID,
--- a/security/pkix/test/lib/pkixtestnss.cpp
+++ b/security/pkix/test/lib/pkixtestnss.cpp
@@ -26,16 +26,17 @@
 
 #include <limits>
 
 #include "cryptohi.h"
 #include "keyhi.h"
 #include "nss.h"
 #include "pk11pub.h"
 #include "pkix/pkixnss.h"
+#include "prinit.h"
 #include "secerr.h"
 #include "secitem.h"
 
 namespace mozilla { namespace pkix { namespace test {
 
 namespace {
 
 typedef ScopedPtr<SECKEYPublicKey, SECKEY_DestroyPublicKey>
@@ -46,20 +47,38 @@ typedef ScopedPtr<SECKEYPrivateKey, SECK
 inline void
 SECITEM_FreeItem_true(SECItem* item)
 {
   SECITEM_FreeItem(item, true);
 }
 
 typedef mozilla::pkix::ScopedPtr<SECItem, SECITEM_FreeItem_true> ScopedSECItem;
 
+TestKeyPair* GenerateKeyPairInner();
+
+static ScopedTestKeyPair reusedKeyPair;
+
+PRStatus
+init()
+{
+  if (NSS_NoDB_Init(nullptr) != SECSuccess) {
+    abort();
+  }
+
+  reusedKeyPair = GenerateKeyPairInner();
+  assert(reusedKeyPair);
+
+  return PR_SUCCESS;
+}
+
 Result
 InitNSSIfNeeded()
 {
-  if (NSS_NoDB_Init(nullptr) != SECSuccess) {
+  static PRCallOnceType initCallOnce;
+  if (PR_CallOnce(&initCallOnce, init) != PR_SUCCESS) {
     return MapPRErrorCodeToResult(PR_GetError());
   }
   return Success;
 }
 
 class NSSTestKeyPair : public TestKeyPair
 {
 public:
@@ -120,26 +139,24 @@ private:
 // Ownership of privateKey is transfered.
 TestKeyPair* CreateTestKeyPair(const ByteString& spki,
                                const ByteString& spk,
                                SECKEYPrivateKey* privateKey)
 {
   return new (std::nothrow) NSSTestKeyPair(spki, spk, privateKey);
 }
 
+namespace {
+
 TestKeyPair*
-GenerateKeyPair()
+GenerateKeyPairInner()
 {
-  if (InitNSSIfNeeded() != Success) {
-    return nullptr;
-  }
-
   ScopedPtr<PK11SlotInfo, PK11_FreeSlot> slot(PK11_GetInternalSlot());
   if (!slot) {
-    return nullptr;
+    abort();
   }
 
   // Bug 1012786: PK11_GenerateKeyPair can fail if there is insufficient
   // entropy to generate a random key. Attempting to add some entropy and
   // retrying appears to solve this issue.
   for (uint32_t retries = 0; retries < 10; retries++) {
     PK11RSAGenParams params;
     params.keySizeInBits = 2048;
@@ -149,22 +166,22 @@ GenerateKeyPair()
       privateKey(PK11_GenerateKeyPair(slot.get(), CKM_RSA_PKCS_KEY_PAIR_GEN,
                                       &params, &publicKeyTemp, false, true,
                                       nullptr));
     ScopedSECKEYPublicKey publicKey(publicKeyTemp);
     if (privateKey) {
       ScopedSECItem
         spkiDER(SECKEY_EncodeDERSubjectPublicKeyInfo(publicKey.get()));
       if (!spkiDER) {
-        return nullptr;
+        break;
       }
       ScopedPtr<CERTSubjectPublicKeyInfo, SECKEY_DestroySubjectPublicKeyInfo>
         spki(SECKEY_CreateSubjectPublicKeyInfo(publicKey.get()));
       if (!spki) {
-        return nullptr;
+        break;
       }
       SECItem spkDER = spki->subjectPublicKey;
       DER_ConvertBitString(&spkDER); // bits to bytes
       return CreateTestKeyPair(ByteString(spkiDER->data, spkiDER->len),
                                ByteString(spkDER.data, spkDER.len),
                                privateKey.release());
     }
 
@@ -179,17 +196,45 @@ GenerateKeyPair()
     // https://xkcd.com/221/
     static const uint8_t RANDOM_NUMBER[] = { 4, 4, 4, 4, 4, 4, 4, 4 };
     if (PK11_RandomUpdate((void*) &RANDOM_NUMBER,
                           sizeof(RANDOM_NUMBER)) != SECSuccess) {
       break;
     }
   }
 
+  abort();
+#if defined(_MSC_VER) && (_MSC_VER < 1700)
+  // Older versions of MSVC don't know that abort() never returns, so silence
+  // its warning by adding a redundant and never-reached return. But, only do
+  // it for that ancient compiler, because some other compilers will rightly
+  // warn that the return statement is unreachable.
   return nullptr;
+#endif
+}
+
+} // unnamed namespace
+
+TestKeyPair*
+GenerateKeyPair()
+{
+  if (InitNSSIfNeeded() != Success) {
+    abort();
+  }
+  return GenerateKeyPairInner();
+}
+
+TestKeyPair*
+CloneReusedKeyPair()
+{
+  if (InitNSSIfNeeded() != Success) {
+    abort();
+  }
+  assert(reusedKeyPair);
+  return reusedKeyPair->Clone();
 }
 
 ByteString
 SHA1(const ByteString& toHash)
 {
   if (InitNSSIfNeeded() != Success) {
     return ByteString();
   }
--- a/security/pkix/test/lib/pkixtestutil.cpp
+++ b/security/pkix/test/lib/pkixtestutil.cpp
@@ -336,26 +336,23 @@ YMDHMS(int16_t year, int16_t month, int1
   totalSeconds += hour * 60 * 60;
   totalSeconds += minutes * 60;
   totalSeconds += seconds;
   return TimeFromElapsedSecondsAD(totalSeconds);
 }
 
 static ByteString
 SignedData(const ByteString& tbsData,
-           /*optional*/ TestKeyPair* keyPair,
+           const TestKeyPair& keyPair,
            SignatureAlgorithm signatureAlgorithm,
            bool corrupt, /*optional*/ const ByteString* certs)
 {
   ByteString signature;
-  if (keyPair) {
-    if (keyPair->SignData(tbsData, signatureAlgorithm, signature)
-          != Success) {
-       return ByteString();
-     }
+  if (keyPair.SignData(tbsData, signatureAlgorithm, signature) != Success) {
+    return ByteString();
   }
 
   ByteString signatureAlgorithmDER;
   switch (signatureAlgorithm) {
     case SignatureAlgorithm::rsa_pkcs1_with_sha256:
       signatureAlgorithmDER.assign(alg_sha256WithRSAEncryption,
                                    sizeof(alg_sha256WithRSAEncryption));
       break;
@@ -460,50 +457,38 @@ static ByteString TBSCertificate(long ve
 //         signatureAlgorithm   AlgorithmIdentifier,
 //         signatureValue       BIT STRING  }
 ByteString
 CreateEncodedCertificate(long version, Input signature,
                          const ByteString& serialNumber,
                          const ByteString& issuerNameDER,
                          time_t notBefore, time_t notAfter,
                          const ByteString& subjectNameDER,
+                         const TestKeyPair& subjectKeyPair,
                          /*optional*/ const ByteString* extensions,
-                         /*optional*/ TestKeyPair* issuerKeyPair,
-                         SignatureAlgorithm signatureAlgorithm,
-                         /*out*/ ScopedTestKeyPair& keyPairResult)
+                         const TestKeyPair& issuerKeyPair,
+                         SignatureAlgorithm signatureAlgorithm)
 {
-  // It may be the case that privateKeyResult references the same TestKeyPair
-  // as issuerKeyPair. Thus, we can't set keyPairResult until after we're done
-  // with issuerKeyPair.
-  ScopedTestKeyPair subjectKeyPair(GenerateKeyPair());
-  if (!subjectKeyPair) {
-    return ByteString();
-  }
-
   ByteString tbsCertificate(TBSCertificate(version, serialNumber,
                                            signature, issuerNameDER, notBefore,
                                            notAfter, subjectNameDER,
-                                           subjectKeyPair->subjectPublicKeyInfo,
+                                           subjectKeyPair.subjectPublicKeyInfo,
                                            extensions));
   if (ENCODING_FAILED(tbsCertificate)) {
     return ByteString();
   }
 
-  ByteString result(SignedData(tbsCertificate,
-                               issuerKeyPair ? issuerKeyPair
-                                             : subjectKeyPair.get(),
+  ByteString result(SignedData(tbsCertificate, issuerKeyPair,
                                signatureAlgorithm, false, nullptr));
   if (ENCODING_FAILED(result)) {
     return ByteString();
   }
 
   MaybeLogOutput(result, "cert");
 
-  keyPairResult = subjectKeyPair.release();
-
   return result;
 }
 
 // TBSCertificate  ::=  SEQUENCE  {
 //      version         [0]  Version DEFAULT v1,
 //      serialNumber         CertificateSerialNumber,
 //      signature            AlgorithmIdentifier,
 //      issuer               Name,
@@ -759,18 +744,17 @@ ResponseBytes(OCSPResponseContext& conte
 ByteString
 BasicOCSPResponse(OCSPResponseContext& context)
 {
   ByteString tbsResponseData(ResponseData(context));
   if (ENCODING_FAILED(tbsResponseData)) {
     return ByteString();
   }
 
-  // TODO(bug 980538): certs
-  return SignedData(tbsResponseData, context.signerKeyPair.get(),
+  return SignedData(tbsResponseData, *context.signerKeyPair,
                     SignatureAlgorithm::rsa_pkcs1_with_sha256,
                     context.badSignature, context.certs);
 }
 
 // Extension ::= SEQUENCE {
 //   id               OBJECT IDENTIFIER,
 //   critical         BOOLEAN DEFAULT FALSE
 //   value            OCTET STRING
--- a/security/pkix/test/lib/pkixtestutil.h
+++ b/security/pkix/test/lib/pkixtestutil.h
@@ -106,16 +106,21 @@ protected:
     , subjectPublicKey(spk)
   {
   }
 
   TestKeyPair(const TestKeyPair&) /*= delete*/;
   void operator=(const TestKeyPair&) /*= delete*/;
 };
 
+// If the objective of the test doesn't involve verifying that signature
+// verification is done correctly then use the keypair returned from
+// CloneReusedKeyPair to make the test run much faster.
+TestKeyPair* CloneReusedKeyPair();
+
 TestKeyPair* GenerateKeyPair();
 inline void DeleteTestKeyPair(TestKeyPair* keyPair) { delete keyPair; }
 typedef ScopedPtr<TestKeyPair, DeleteTestKeyPair> ScopedTestKeyPair;
 
 ByteString SHA1(const ByteString& toHash);
 
 Result TestCheckPublicKey(Input subjectPublicKeyInfo);
 Result TestVerifySignedData(const SignedDataWithSignature& signedData,
@@ -141,29 +146,25 @@ enum Version { v1 = 0, v2 = 1, v3 = 2 };
 
 // signature is assumed to be the DER encoding of an AlgorithmIdentifer.
 // serialNumber is assumed to be the DER encoding of an INTEGER.
 //
 // If extensions is null, then no extensions will be encoded. Otherwise,
 // extensions must point to an array of ByteStrings, terminated with an empty
 // ByteString. (If the first item of the array is empty then an empty
 // Extensions sequence will be encoded.)
-//
-// If issuerPrivateKey is null, then the certificate will be self-signed.
-// Parameter order is based on the order of the attributes of the certificate
-// in RFC 5280.
 ByteString CreateEncodedCertificate(long version, Input signature,
                                     const ByteString& serialNumber,
                                     const ByteString& issuerNameDER,
                                     time_t notBefore, time_t notAfter,
                                     const ByteString& subjectNameDER,
+                                    const TestKeyPair& subjectKeyPair,
                                     /*optional*/ const ByteString* extensions,
-                                    /*optional*/ TestKeyPair* issuerKeyPair,
-                                    SignatureAlgorithm signatureAlgorithm,
-                                    /*out*/ ScopedTestKeyPair& keyPairResult);
+                                    const TestKeyPair& issuerKeyPair,
+                                    SignatureAlgorithm signatureAlgorithm);
 
 ByteString CreateEncodedSerialNumber(long value);
 
 MOZILLA_PKIX_ENUM_CLASS ExtensionCriticality { NotCritical = 0, Critical = 1 };
 
 ByteString CreateEncodedBasicConstraints(bool isCA,
                                          /*optional*/ long* pathLenConstraint,
                                          ExtensionCriticality criticality);