Bug 1026261: Remove CERTCertificate from mozilla::pkix revocation checking API, r=keeler
authorBrian Smith <brian@briansmith.org>
Fri, 20 Jun 2014 10:10:51 -0700
changeset 210706 3d54fd14fb9c6fce3336ea14831ff51b6bbc6b5d
parent 210705 8112e25fa218836cd002d9102071d8b497d68dc8
child 210707 bf47f42c33c8bbe275903f9ace0effee6df02439
push id3857
push userraliiev@mozilla.com
push dateTue, 02 Sep 2014 16:39:23 +0000
treeherdermozilla-beta@5638b907b505 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1026261
milestone33.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 1026261: Remove CERTCertificate from mozilla::pkix revocation checking API, r=keeler
security/apps/AppTrustDomain.cpp
security/apps/AppTrustDomain.h
security/certverifier/NSSCertDBTrustDomain.cpp
security/certverifier/NSSCertDBTrustDomain.h
security/certverifier/OCSPCache.cpp
security/certverifier/OCSPCache.h
security/manager/ssl/tests/gtest/OCSPCacheTest.cpp
security/manager/ssl/tests/unit/tlsserver/lib/OCSPCommon.cpp
security/pkix/include/pkix/pkix.h
security/pkix/include/pkix/pkixtypes.h
security/pkix/lib/pkixbuild.cpp
security/pkix/lib/pkixkey.cpp
security/pkix/lib/pkixocsp.cpp
security/pkix/lib/pkixutil.h
security/pkix/test/gtest/pkix_cert_chain_length_tests.cpp
security/pkix/test/gtest/pkix_cert_extension_tests.cpp
security/pkix/test/gtest/pkix_ocsp_request_tests.cpp
security/pkix/test/lib/pkixtestutil.cpp
security/pkix/test/lib/pkixtestutil.h
--- a/security/apps/AppTrustDomain.cpp
+++ b/security/apps/AppTrustDomain.cpp
@@ -165,20 +165,18 @@ SECStatus
 AppTrustDomain::VerifySignedData(const CERTSignedData* signedData,
                                  const SECItem& subjectPublicKeyInfo)
 {
   return ::mozilla::pkix::VerifySignedData(signedData, subjectPublicKeyInfo,
                                            mPinArg);
 }
 
 SECStatus
-AppTrustDomain::CheckRevocation(EndEntityOrCA,
-                                const CERTCertificate*,
-                                /*const*/ CERTCertificate*,
-                                PRTime time,
+AppTrustDomain::CheckRevocation(EndEntityOrCA, const CertID&, PRTime time,
+                                /*optional*/ const SECItem*,
                                 /*optional*/ const SECItem*)
 {
   // We don't currently do revocation checking. If we need to distrust an Apps
   // certificate, we will use the active distrust mechanism.
   return SECSuccess;
 }
 
 } }
--- a/security/apps/AppTrustDomain.h
+++ b/security/apps/AppTrustDomain.h
@@ -26,20 +26,19 @@ public:
                  /*out*/ mozilla::pkix::TrustLevel* trustLevel) MOZ_OVERRIDE;
   SECStatus FindPotentialIssuers(const SECItem* encodedIssuerName,
                                  PRTime time,
                          /*out*/ mozilla::pkix::ScopedCERTCertList& results)
                                  MOZ_OVERRIDE;
   SECStatus VerifySignedData(const CERTSignedData* signedData,
                              const SECItem& subjectPublicKeyInfo) MOZ_OVERRIDE;
   SECStatus CheckRevocation(mozilla::pkix::EndEntityOrCA endEntityOrCA,
-                            const CERTCertificate* cert,
-                            /*const*/ CERTCertificate* issuerCertToDup,
-                            PRTime time,
-                            /*optional*/ const SECItem* stapledOCSPresponse);
+                            const mozilla::pkix::CertID& certID, PRTime time,
+                            /*optional*/ const SECItem* stapledOCSPresponse,
+                            /*optional*/ const SECItem* aiaExtension);
   SECStatus IsChainValid(const CERTCertList* certChain) { return SECSuccess; }
 
 private:
   void* mPinArg; // non-owning!
   mozilla::pkix::ScopedCERTCertificate mTrustedRoot;
 };
 
 } } // namespace mozilla::psm
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -170,40 +170,87 @@ OCSPFetchingTypeToTimeoutTime(NSSCertDBT
     case NSSCertDBTrustDomain::LocalOnlyOCSPForEV:
       PR_NOT_REACHED("we should never see this OCSPFetching type here");
     default:
       PR_NOT_REACHED("we're not handling every OCSPFetching type");
   }
   return PR_SecondsToInterval(2);
 }
 
+// Copied and modified from CERT_GetOCSPAuthorityInfoAccessLocation and
+// CERT_GetGeneralNameByType. Returns SECFailure on error, SECSuccess
+// with url == nullptr when an OCSP URI was not found, and SECSuccess with
+// url != nullptr when an OCSP URI was found. The output url will be owned
+// by the arena.
+static SECStatus
+GetOCSPAuthorityInfoAccessLocation(PLArenaPool* arena,
+                                   const SECItem& aiaExtension,
+                                   /*out*/ char const*& url)
+{
+  url = nullptr;
+
+  // TODO(bug 1028380): Remove the const_cast.
+  CERTAuthInfoAccess** aia = CERT_DecodeAuthInfoAccessExtension(
+                                arena,
+                                const_cast<SECItem*>(&aiaExtension));
+  if (!aia) {
+    PR_SetError(SEC_ERROR_CERT_BAD_ACCESS_LOCATION, 0);
+    return SECFailure;
+  }
+  for (size_t i = 0; aia[i]; ++i) {
+    if (SECOID_FindOIDTag(&aia[i]->method) == SEC_OID_PKIX_OCSP) {
+      // NSS chooses the **last** OCSP URL; we choose the **first**
+      CERTGeneralName* current = aia[i]->location;
+      if (!current) {
+        continue;
+      }
+      do {
+        if (current->type == certURI) {
+          const SECItem& location = current->name.other;
+          // (location.len + 1) must be small enough to fit into a uint32_t,
+          // but we limit it to a smaller bound to reduce OOM risk.
+          if (location.len > 1024 || memchr(location.data, 0, location.len)) {
+            // Reject embedded nulls. (NSS doesn't do this)
+            PR_SetError(SEC_ERROR_CERT_BAD_ACCESS_LOCATION, 0);
+            return SECFailure;
+          }
+          // Copy the non-null-terminated SECItem into a null-terminated string.
+          char* nullTerminatedURL(static_cast<char*>(
+                                    PORT_ArenaAlloc(arena, location.len + 1)));
+          if (!nullTerminatedURL) {
+            return SECFailure;
+          }
+          memcpy(nullTerminatedURL, location.data, location.len);
+          nullTerminatedURL[location.len] = 0;
+          url = nullTerminatedURL;
+          return SECSuccess;
+        }
+        current = CERT_GetNextGeneralName(current);
+      } while (current != aia[i]->location);
+    }
+  }
+
+  return SECSuccess;
+}
+
 SECStatus
-NSSCertDBTrustDomain::CheckRevocation(
-  mozilla::pkix::EndEntityOrCA endEntityOrCA,
-  const CERTCertificate* cert,
-  /*const*/ CERTCertificate* issuerCert,
-  PRTime time,
-  /*optional*/ const SECItem* stapledOCSPResponse)
+NSSCertDBTrustDomain::CheckRevocation(EndEntityOrCA endEntityOrCA,
+                                      const CertID& certID, PRTime time,
+                         /*optional*/ const SECItem* stapledOCSPResponse,
+                         /*optional*/ const SECItem* aiaExtension)
 {
   // Actively distrusted certificates will have already been blocked by
   // GetCertTrust.
 
   // TODO: need to verify that IsRevoked isn't called for trust anchors AND
   // that that fact is documented in mozillapkix.
 
   PR_LOG(gCertVerifierLog, PR_LOG_DEBUG,
          ("NSSCertDBTrustDomain: Top of CheckRevocation\n"));
 
-  PORT_Assert(cert);
-  PORT_Assert(issuerCert);
-  if (!cert || !issuerCert) {
-    PORT_SetError(SEC_ERROR_INVALID_ARGS);
-    return SECFailure;
-  }
-
   // Bug 991815: The BR allow OCSP for intermediates to be up to one year old.
   // Since this affects EV there is no reason why DV should be more strict
   // so all intermediatates are allowed to have OCSP responses up to one year
   // old.
   uint16_t maxOCSPLifetimeInDays = 10;
   if (endEntityOrCA == EndEntityOrCA::MustBeCA) {
     maxOCSPLifetimeInDays = 365;
   }
@@ -213,20 +260,19 @@ NSSCertDBTrustDomain::CheckRevocation(
   // exception for expired responses because some servers, nginx in particular,
   // are known to serve expired responses due to bugs.
   // We keep track of the result of verifying the stapled response but don't
   // immediately return failure if the response has expired.
   PRErrorCode stapledOCSPResponseErrorCode = 0;
   if (stapledOCSPResponse) {
     PR_ASSERT(endEntityOrCA == EndEntityOrCA::MustBeEndEntity);
     bool expired;
-    SECStatus rv = VerifyAndMaybeCacheEncodedOCSPResponse(cert, issuerCert,
-                                                          time,
+    SECStatus rv = VerifyAndMaybeCacheEncodedOCSPResponse(certID, time,
                                                           maxOCSPLifetimeInDays,
-                                                          stapledOCSPResponse,
+                                                          *stapledOCSPResponse,
                                                           ResponseWasStapled,
                                                           expired);
     if (rv == SECSuccess) {
       // stapled OCSP response present and good
       Telemetry::Accumulate(Telemetry::SSL_OCSP_STAPLING, 1);
       PR_LOG(gCertVerifierLog, PR_LOG_DEBUG,
              ("NSSCertDBTrustDomain: stapled OCSP response: good"));
       return rv;
@@ -249,17 +295,17 @@ NSSCertDBTrustDomain::CheckRevocation(
     // no stapled OCSP response
     Telemetry::Accumulate(Telemetry::SSL_OCSP_STAPLING, 2);
     PR_LOG(gCertVerifierLog, PR_LOG_DEBUG,
            ("NSSCertDBTrustDomain: no stapled OCSP response"));
   }
 
   PRErrorCode cachedResponseErrorCode = 0;
   PRTime cachedResponseValidThrough = 0;
-  bool cachedResponsePresent = mOCSPCache.Get(cert, issuerCert,
+  bool cachedResponsePresent = mOCSPCache.Get(certID,
                                               cachedResponseErrorCode,
                                               cachedResponseValidThrough);
   if (cachedResponsePresent) {
     if (cachedResponseErrorCode == 0 && cachedResponseValidThrough >= time) {
       PR_LOG(gCertVerifierLog, PR_LOG_DEBUG,
              ("NSSCertDBTrustDomain: cached OCSP response: good"));
       return SECSuccess;
     }
@@ -327,18 +373,29 @@ NSSCertDBTrustDomain::CheckRevocation(
   }
 
   if (mOCSPFetching == LocalOnlyOCSPForEV) {
     PR_SetError(cachedResponseErrorCode != 0 ? cachedResponseErrorCode
                                              : SEC_ERROR_OCSP_UNKNOWN_CERT, 0);
     return SECFailure;
   }
 
-  ScopedPtr<char, PORT_Free_string>
-    url(CERT_GetOCSPAuthorityInfoAccessLocation(cert));
+  ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+  if (!arena) {
+    return SECFailure;
+  }
+
+  const char* url = nullptr; // owned by the arena
+
+  if (aiaExtension) {
+    if (GetOCSPAuthorityInfoAccessLocation(arena.get(), *aiaExtension, url)
+          != SECSuccess) {
+      return SECFailure;
+    }
+  }
 
   if (!url) {
     if (mOCSPFetching == FetchOCSPForEV ||
         cachedResponseErrorCode == SEC_ERROR_OCSP_UNKNOWN_CERT) {
       PR_SetError(SEC_ERROR_OCSP_UNKNOWN_CERT, 0);
       return SECFailure;
     }
     if (cachedResponseErrorCode == SEC_ERROR_OCSP_OLD_RESPONSE) {
@@ -352,45 +409,39 @@ NSSCertDBTrustDomain::CheckRevocation(
 
     // Nothing to do if we don't have an OCSP responder URI for the cert; just
     // assume it is good. Note that this is the confusing, but intended,
     // interpretation of "strict" revocation checking in the face of a
     // certificate that lacks an OCSP responder URI.
     return SECSuccess;
   }
 
-  ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
-  if (!arena) {
-    return SECFailure;
-  }
-
   // Only request a response if we didn't have a cached indication of failure
   // (don't keep requesting responses from a failing server).
   const SECItem* response = nullptr;
   if (cachedResponseErrorCode == 0 ||
       cachedResponseErrorCode == SEC_ERROR_OCSP_UNKNOWN_CERT ||
       cachedResponseErrorCode == SEC_ERROR_OCSP_OLD_RESPONSE) {
-    const SECItem* request(CreateEncodedOCSPRequest(arena.get(), cert,
-                                                    issuerCert));
+    const SECItem* request(CreateEncodedOCSPRequest(arena.get(), certID));
     if (!request) {
       return SECFailure;
     }
 
-    response = DoOCSPRequest(arena.get(), url.get(), request,
+    response = DoOCSPRequest(arena.get(), url, request,
                              OCSPFetchingTypeToTimeoutTime(mOCSPFetching),
                              mOCSPGetConfig == CertVerifier::ocsp_get_enabled);
   }
 
   if (!response) {
     PRErrorCode error = PR_GetError();
     if (error == 0) {
       error = cachedResponseErrorCode;
     }
     PRTime timeout = time + ServerFailureDelay;
-    if (mOCSPCache.Put(cert, issuerCert, error, time, timeout) != SECSuccess) {
+    if (mOCSPCache.Put(certID, error, time, timeout) != SECSuccess) {
       return SECFailure;
     }
     PR_SetError(error, 0);
     if (mOCSPFetching != FetchOCSPForDVSoftFail) {
       PR_LOG(gCertVerifierLog, PR_LOG_DEBUG,
              ("NSSCertDBTrustDomain: returning SECFailure after "
               "OCSP request failure"));
       return SECFailure;
@@ -415,19 +466,19 @@ NSSCertDBTrustDomain::CheckRevocation(
             "OCSP request failure"));
     return SECSuccess; // Soft fail -> success :(
   }
 
   // If the response from the network has expired but indicates a revoked
   // or unknown certificate, PR_GetError() will return the appropriate error.
   // We actually ignore expired here.
   bool expired;
-  SECStatus rv = VerifyAndMaybeCacheEncodedOCSPResponse(cert, issuerCert, time,
+  SECStatus rv = VerifyAndMaybeCacheEncodedOCSPResponse(certID, time,
                                                         maxOCSPLifetimeInDays,
-                                                        response,
+                                                        *response,
                                                         ResponseIsFromNetwork,
                                                         expired);
   if (rv == SECSuccess || mOCSPFetching != FetchOCSPForDVSoftFail) {
     PR_LOG(gCertVerifierLog, PR_LOG_DEBUG,
       ("NSSCertDBTrustDomain: returning after VerifyEncodedOCSPResponse"));
     return rv;
   }
 
@@ -448,23 +499,23 @@ NSSCertDBTrustDomain::CheckRevocation(
   PR_LOG(gCertVerifierLog, PR_LOG_DEBUG,
          ("NSSCertDBTrustDomain: end of CheckRevocation"));
 
   return SECSuccess; // Soft fail -> success :(
 }
 
 SECStatus
 NSSCertDBTrustDomain::VerifyAndMaybeCacheEncodedOCSPResponse(
-  const CERTCertificate* cert, CERTCertificate* issuerCert, PRTime time,
-  uint16_t maxLifetimeInDays, const SECItem* encodedResponse,
-  EncodedResponseSource responseSource, /*out*/ bool& expired)
+  const CertID& certID, PRTime time, uint16_t maxLifetimeInDays,
+  const SECItem& encodedResponse, EncodedResponseSource responseSource,
+  /*out*/ bool& expired)
 {
   PRTime thisUpdate = 0;
   PRTime validThrough = 0;
-  SECStatus rv = VerifyEncodedOCSPResponse(*this, cert, issuerCert, time,
+  SECStatus rv = VerifyEncodedOCSPResponse(*this, certID, time,
                                            maxLifetimeInDays, encodedResponse,
                                            expired, &thisUpdate, &validThrough);
   PRErrorCode error = (rv == SECSuccess ? 0 : PR_GetError());
   // If a response was stapled and expired, we don't want to cache it. Return
   // early to simplify the logic here.
   if (responseSource == ResponseWasStapled && expired) {
     PR_ASSERT(rv != SECSuccess);
     return rv;
@@ -478,18 +529,17 @@ NSSCertDBTrustDomain::VerifyAndMaybeCach
     validThrough = time + ServerFailureDelay;
   }
   if (responseSource == ResponseIsFromNetwork ||
       rv == SECSuccess ||
       error == SEC_ERROR_REVOKED_CERTIFICATE ||
       error == SEC_ERROR_OCSP_UNKNOWN_CERT) {
     PR_LOG(gCertVerifierLog, PR_LOG_DEBUG,
            ("NSSCertDBTrustDomain: caching OCSP response"));
-    if (mOCSPCache.Put(cert, issuerCert, error, thisUpdate, validThrough)
-          != SECSuccess) {
+    if (mOCSPCache.Put(certID, error, thisUpdate, validThrough) != SECSuccess) {
       return SECFailure;
     }
   }
 
   // If the verification failed, re-set to that original error
   // (the call to Put may have un-set it).
   if (rv != SECSuccess) {
     PR_SetError(error, 0);
--- a/security/certverifier/NSSCertDBTrustDomain.h
+++ b/security/certverifier/NSSCertDBTrustDomain.h
@@ -70,32 +70,32 @@ public:
                                  const mozilla::pkix::CertPolicyId& policy,
                                  const SECItem& candidateCertDER,
                          /*out*/ mozilla::pkix::TrustLevel* trustLevel);
 
   virtual SECStatus VerifySignedData(const CERTSignedData* signedData,
                                      const SECItem& subjectPublicKeyInfo);
 
   virtual SECStatus CheckRevocation(mozilla::pkix::EndEntityOrCA endEntityOrCA,
-                                    const CERTCertificate* cert,
-                          /*const*/ CERTCertificate* issuerCert,
+                                    const mozilla::pkix::CertID& certID,
                                     PRTime time,
-                       /*optional*/ const SECItem* stapledOCSPResponse);
+                       /*optional*/ const SECItem* stapledOCSPResponse,
+                       /*optional*/ const SECItem* aiaExtension);
 
   virtual SECStatus IsChainValid(const CERTCertList* certChain);
 
 private:
   enum EncodedResponseSource {
     ResponseIsFromNetwork = 1,
     ResponseWasStapled = 2
   };
   static const PRTime ServerFailureDelay = 5 * 60 * PR_USEC_PER_SEC;
   SECStatus VerifyAndMaybeCacheEncodedOCSPResponse(
-    const CERTCertificate* cert, CERTCertificate* issuerCert, PRTime time,
-    uint16_t maxLifetimeInDays, const SECItem* encodedResponse,
+    const mozilla::pkix::CertID& certID, PRTime time,
+    uint16_t maxLifetimeInDays, const SECItem& encodedResponse,
     EncodedResponseSource responseSource, /*out*/ bool& expired);
 
   const SECTrustType mCertDBTrustType;
   const OCSPFetching mOCSPFetching;
   OCSPCache& mOCSPCache; // non-owning!
   void* mPinArg; // non-owning!
   const CertVerifier::ocsp_get_config mOCSPGetConfig;
   CERTChainVerifyCallback* mCheckChainCallback; // non-owning!
--- a/security/certverifier/OCSPCache.cpp
+++ b/security/certverifier/OCSPCache.cpp
@@ -23,22 +23,25 @@
  */
 
 #include "OCSPCache.h"
 
 #include <limits>
 
 #include "NSSCertDBTrustDomain.h"
 #include "pk11pub.h"
+#include "pkix/pkixtypes.h"
 #include "secerr.h"
 
 #ifdef PR_LOGGING
 extern PRLogModuleInfo* gCertVerifierLog;
 #endif
 
+using namespace mozilla::pkix;
+
 namespace mozilla { namespace psm {
 
 void
 MozillaPKIX_PK11_DestroyContext_true(PK11Context* context)
 {
   PK11_DestroyContext(context, true);
 }
 
@@ -53,206 +56,177 @@ typedef mozilla::pkix::ScopedPtr<PK11Con
 // Because the DER encodings include the length of the data encoded,
 // there do not exist A(derIssuerA, derPublicKeyA, serialNumberA) and
 // B(derIssuerB, derPublicKeyB, serialNumberB) such that the concatenation of
 // each triplet results in the same string of bytes but where each part in A is
 // not equal to its counterpart in B. This is important because as a result it
 // is computationally infeasible to find collisions that would subvert this
 // cache (given that SHA384 is a cryptographically-secure hash function).
 static SECStatus
-CertIDHash(SHA384Buffer& buf, const CERTCertificate* aCert,
-       const CERTCertificate* aIssuerCert)
+CertIDHash(SHA384Buffer& buf, const CertID& certID)
 {
   ScopedPK11Context context(PK11_CreateDigestContext(SEC_OID_SHA384));
   if (!context) {
     return SECFailure;
   }
   SECStatus rv = PK11_DigestBegin(context.get());
   if (rv != SECSuccess) {
     return rv;
   }
-  rv = PK11_DigestOp(context.get(), aCert->derIssuer.data,
-                     aCert->derIssuer.len);
+  rv = PK11_DigestOp(context.get(), certID.issuer.data, certID.issuer.len);
   if (rv != SECSuccess) {
     return rv;
   }
-  rv = PK11_DigestOp(context.get(), aIssuerCert->derPublicKey.data,
-                     aIssuerCert->derPublicKey.len);
+  rv = PK11_DigestOp(context.get(), certID.issuerSubjectPublicKeyInfo.data,
+                     certID.issuerSubjectPublicKeyInfo.len);
   if (rv != SECSuccess) {
     return rv;
   }
-  rv = PK11_DigestOp(context.get(), aCert->serialNumber.data,
-                     aCert->serialNumber.len);
+  rv = PK11_DigestOp(context.get(), certID.serialNumber.data,
+                     certID.serialNumber.len);
   if (rv != SECSuccess) {
     return rv;
   }
   uint32_t outLen = 0;
   rv = PK11_DigestFinal(context.get(), buf, &outLen, SHA384_LENGTH);
   if (outLen != SHA384_LENGTH) {
     return SECFailure;
   }
   return rv;
 }
 
 SECStatus
-OCSPCache::Entry::Init(const CERTCertificate* aCert,
-                       const CERTCertificate* aIssuerCert,
-                       PRErrorCode aErrorCode,
-                       PRTime aThisUpdate,
-                       PRTime aValidThrough)
+OCSPCache::Entry::Init(const CertID& aCertID, PRErrorCode aErrorCode,
+                       PRTime aThisUpdate, PRTime aValidThrough)
 {
   mErrorCode = aErrorCode;
   mThisUpdate = aThisUpdate;
   mValidThrough = aValidThrough;
-  return CertIDHash(mIDHash, aCert, aIssuerCert);
+  return CertIDHash(mIDHash, aCertID);
 }
 
 OCSPCache::OCSPCache()
   : mMutex("OCSPCache-mutex")
 {
 }
 
 OCSPCache::~OCSPCache()
 {
   Clear();
 }
 
 // Returns false with index in an undefined state if no matching entry was
 // found.
 bool
-OCSPCache::FindInternal(const CERTCertificate* aCert,
-                        const CERTCertificate* aIssuerCert,
-                        /*out*/ size_t& index,
+OCSPCache::FindInternal(const CertID& aCertID, /*out*/ size_t& index,
                         const MutexAutoLock& /* aProofOfLock */)
 {
   if (mEntries.length() == 0) {
     return false;
   }
 
   SHA384Buffer idHash;
-  SECStatus rv = CertIDHash(idHash, aCert, aIssuerCert);
+  SECStatus rv = CertIDHash(idHash, aCertID);
   if (rv != SECSuccess) {
     return false;
   }
 
   // mEntries is sorted with the most-recently-used entry at the end.
   // Thus, searching from the end will often be fastest.
   index = mEntries.length();
   while (index > 0) {
     --index;
     if (memcmp(mEntries[index]->mIDHash, idHash, SHA384_LENGTH) == 0) {
       return true;
     }
   }
   return false;
 }
 
-void
-OCSPCache::LogWithCerts(const char* aMessage, const CERTCertificate* aCert,
-                        const CERTCertificate* aIssuerCert)
+static inline void
+LogWithCertID(const char* aMessage, const CertID& aCertID)
 {
-#ifdef PR_LOGGING
-  if (PR_LOG_TEST(gCertVerifierLog, PR_LOG_DEBUG)) {
-    mozilla::pkix::ScopedPtr<char, mozilla::psm::PORT_Free_string>
-      cn(CERT_GetCommonName(&aCert->subject));
-    mozilla::pkix::ScopedPtr<char, mozilla::psm::PORT_Free_string>
-      cnIssuer(CERT_GetCommonName(&aIssuerCert->subject));
-    PR_LOG(gCertVerifierLog, PR_LOG_DEBUG, (aMessage, cn.get(), cnIssuer.get()));
-  }
-#endif
+  PR_LOG(gCertVerifierLog, PR_LOG_DEBUG, (aMessage, &aCertID));
 }
 
 void
 OCSPCache::MakeMostRecentlyUsed(size_t aIndex,
                                 const MutexAutoLock& /* aProofOfLock */)
 {
   Entry* entry = mEntries[aIndex];
   // Since mEntries is sorted with the most-recently-used entry at the end,
   // aIndex is likely to be near the end, so this is likely to be fast.
   mEntries.erase(mEntries.begin() + aIndex);
   mEntries.append(entry);
 }
 
 bool
-OCSPCache::Get(const CERTCertificate* aCert,
-               const CERTCertificate* aIssuerCert,
-               PRErrorCode& aErrorCode,
+OCSPCache::Get(const CertID& aCertID, PRErrorCode& aErrorCode,
                PRTime& aValidThrough)
 {
-  PR_ASSERT(aCert);
-  PR_ASSERT(aIssuerCert);
-
   MutexAutoLock lock(mMutex);
 
   size_t index;
-  if (!FindInternal(aCert, aIssuerCert, index, lock)) {
-    LogWithCerts("OCSPCache::Get(%s, %s) not in cache", aCert, aIssuerCert);
+  if (!FindInternal(aCertID, index, lock)) {
+    LogWithCertID("OCSPCache::Get(%p) not in cache", aCertID);
     return false;
   }
-  LogWithCerts("OCSPCache::Get(%s, %s) in cache", aCert, aIssuerCert);
+  LogWithCertID("OCSPCache::Get(%p) in cache", aCertID);
   aErrorCode = mEntries[index]->mErrorCode;
   aValidThrough = mEntries[index]->mValidThrough;
   MakeMostRecentlyUsed(index, lock);
   return true;
 }
 
 SECStatus
-OCSPCache::Put(const CERTCertificate* aCert,
-               const CERTCertificate* aIssuerCert,
-               PRErrorCode aErrorCode,
-               PRTime aThisUpdate,
-               PRTime aValidThrough)
+OCSPCache::Put(const CertID& aCertID, PRErrorCode aErrorCode,
+               PRTime aThisUpdate, PRTime aValidThrough)
 {
-  PR_ASSERT(aCert);
-  PR_ASSERT(aIssuerCert);
-
   MutexAutoLock lock(mMutex);
 
   size_t index;
-  if (FindInternal(aCert, aIssuerCert, index, lock)) {
+  if (FindInternal(aCertID, index, lock)) {
     // Never replace an entry indicating a revoked certificate.
     if (mEntries[index]->mErrorCode == SEC_ERROR_REVOKED_CERTIFICATE) {
-      LogWithCerts("OCSPCache::Put(%s, %s) already in cache as revoked - "
-                   "not replacing", aCert, aIssuerCert);
+      LogWithCertID("OCSPCache::Put(%p) already in cache as revoked - "
+                    "not replacing", aCertID);
       MakeMostRecentlyUsed(index, lock);
       return SECSuccess;
     }
 
     // Never replace a newer entry with an older one unless the older entry
     // indicates a revoked certificate, which we want to remember.
     if (mEntries[index]->mThisUpdate > aThisUpdate &&
         aErrorCode != SEC_ERROR_REVOKED_CERTIFICATE) {
-      LogWithCerts("OCSPCache::Put(%s, %s) already in cache with more recent "
-                   "validity - not replacing", aCert, aIssuerCert);
+      LogWithCertID("OCSPCache::Put(%p) already in cache with more recent "
+                    "validity - not replacing", aCertID);
       MakeMostRecentlyUsed(index, lock);
       return SECSuccess;
     }
 
     // Only known good responses or responses indicating an unknown
     // or revoked certificate should replace previously known responses.
     if (aErrorCode != 0 && aErrorCode != SEC_ERROR_OCSP_UNKNOWN_CERT &&
         aErrorCode != SEC_ERROR_REVOKED_CERTIFICATE) {
-      LogWithCerts("OCSPCache::Put(%s, %s) already in cache - not replacing "
-                   "with less important status", aCert, aIssuerCert);
+      LogWithCertID("OCSPCache::Put(%p) already in cache - not replacing "
+                    "with less important status", aCertID);
       MakeMostRecentlyUsed(index, lock);
       return SECSuccess;
     }
 
-    LogWithCerts("OCSPCache::Put(%s, %s) already in cache - replacing",
-                 aCert, aIssuerCert);
+    LogWithCertID("OCSPCache::Put(%p) already in cache - replacing", aCertID);
     mEntries[index]->mErrorCode = aErrorCode;
     mEntries[index]->mThisUpdate = aThisUpdate;
     mEntries[index]->mValidThrough = aValidThrough;
     MakeMostRecentlyUsed(index, lock);
     return SECSuccess;
   }
 
   if (mEntries.length() == MaxEntries) {
-    LogWithCerts("OCSPCache::Put(%s, %s) too full - evicting an entry", aCert,
-                 aIssuerCert);
+    LogWithCertID("OCSPCache::Put(%p) too full - evicting an entry", aCertID);
     for (Entry** toEvict = mEntries.begin(); toEvict != mEntries.end();
          toEvict++) {
       // Never evict an entry that indicates a revoked or unknokwn certificate,
       // because revoked responses are more security-critical to remember.
       if ((*toEvict)->mErrorCode != SEC_ERROR_REVOKED_CERTIFICATE &&
           (*toEvict)->mErrorCode != SEC_ERROR_OCSP_UNKNOWN_CERT) {
         delete *toEvict;
         mEntries.erase(toEvict);
@@ -278,24 +252,24 @@ OCSPCache::Put(const CERTCertificate* aC
   Entry* newEntry = new Entry();
   // Normally we don't have to do this in Gecko, because OOM is fatal.
   // However, if we want to embed this in another project, OOM might not
   // be fatal, so handle this case.
   if (!newEntry) {
     PR_SetError(SEC_ERROR_NO_MEMORY, 0);
     return SECFailure;
   }
-  SECStatus rv = newEntry->Init(aCert, aIssuerCert, aErrorCode, aThisUpdate,
+  SECStatus rv = newEntry->Init(aCertID, aErrorCode, aThisUpdate,
                                 aValidThrough);
   if (rv != SECSuccess) {
     delete newEntry;
     return rv;
   }
   mEntries.append(newEntry);
-  LogWithCerts("OCSPCache::Put(%s, %s) added to cache", aCert, aIssuerCert);
+  LogWithCertID("OCSPCache::Put(%p) added to cache", aCertID);
   return SECSuccess;
 }
 
 void
 OCSPCache::Clear()
 {
   MutexAutoLock lock(mMutex);
   PR_LOG(gCertVerifierLog, PR_LOG_DEBUG, ("OCSPCache::Clear: clearing cache"));
--- a/security/certverifier/OCSPCache.h
+++ b/security/certverifier/OCSPCache.h
@@ -20,24 +20,27 @@
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
 
 #ifndef mozilla_psm_OCSPCache_h
 #define mozilla_psm_OCSPCache_h
 
-#include "certt.h"
 #include "hasht.h"
-#include "pkix/pkixtypes.h"
 #include "mozilla/Mutex.h"
 #include "mozilla/Vector.h"
 #include "prerror.h"
+#include "prtime.h"
 #include "seccomon.h"
 
+namespace mozilla { namespace pkix {
+struct CertID;
+} } // namespace mozilla::pkix
+
 namespace mozilla { namespace psm {
 
 // make SHA384Buffer be of type "array of uint8_t of length SHA384_LENGTH"
 typedef uint8_t SHA384Buffer[SHA384_LENGTH];
 
 // OCSPCache can store and retrieve OCSP response verification results. Each
 // result is keyed on the certificate that purportedly corresponds to it (where
 // certificates are distinguished based on serial number, issuer, and
@@ -49,62 +52,53 @@ class OCSPCache
 public:
   OCSPCache();
   ~OCSPCache();
 
   // Returns true if the status of the given certificate (issued by the given
   // issuer) is in the cache, and false otherwise.
   // If it is in the cache, returns by reference the error code of the cached
   // status and the time through which the status is considered trustworthy.
-  bool Get(const CERTCertificate* aCert, const CERTCertificate* aIssuerCert,
+  bool Get(const mozilla::pkix::CertID& aCertID,
            /* out */ PRErrorCode& aErrorCode, /* out */ PRTime& aValidThrough);
 
   // Caches the status of the given certificate (issued by the given issuer).
   // The status is considered trustworthy through the given time.
   // A status with an error code of SEC_ERROR_REVOKED_CERTIFICATE will not
   // be replaced or evicted.
   // A status with an error code of SEC_ERROR_OCSP_UNKNOWN_CERT will not
   // be evicted when the cache is full.
   // A status with a more recent thisUpdate will not be replaced with a
   // status with a less recent thisUpdate unless the less recent status
   // indicates the certificate is revoked.
-  SECStatus Put(const CERTCertificate* aCert,
-                const CERTCertificate* aIssuerCert,
-                PRErrorCode aErrorCode,
-                PRTime aThisUpdate,
-                PRTime aValidThrough);
+  SECStatus Put(const mozilla::pkix::CertID& aCertID, PRErrorCode aErrorCode,
+                PRTime aThisUpdate, PRTime aValidThrough);
 
   // Removes everything from the cache.
   void Clear();
 
 private:
   class Entry
   {
   public:
-    SECStatus Init(const CERTCertificate* aCert,
-                   const CERTCertificate* aIssuerCert,
-                   PRErrorCode aErrorCode, PRTime aThisUpdate,
-                   PRTime aValidThrough);
+    SECStatus Init(const mozilla::pkix::CertID& aCertID, PRErrorCode aErrorCode,
+                   PRTime aThisUpdate, PRTime aValidThrough);
 
     PRErrorCode mErrorCode;
     PRTime mThisUpdate;
     PRTime mValidThrough;
     // The SHA-384 hash of the concatenation of the DER encodings of the
     // issuer name and issuer key, followed by the serial number.
     // See the documentation for CertIDHash in OCSPCache.cpp.
     SHA384Buffer mIDHash;
   };
 
-  bool FindInternal(const CERTCertificate* aCert,
-                    const CERTCertificate* aIssuerCert,
-                    /*out*/ size_t& index,
+  bool FindInternal(const mozilla::pkix::CertID& aCertID, /*out*/ size_t& index,
                     const MutexAutoLock& aProofOfLock);
   void MakeMostRecentlyUsed(size_t aIndex, const MutexAutoLock& aProofOfLock);
-  void LogWithCerts(const char* aMessage, const CERTCertificate* aCert,
-                    const CERTCertificate* aIssuerCert);
 
   Mutex mMutex;
   static const size_t MaxEntries = 1024;
   // Sorted with the most-recently-used entry at the end.
   // Using 256 here reserves as much possible inline storage as the vector
   // implementation will give us. 1024 bytes is the maximum it allows,
   // which results in 256 Entry pointers or 128 Entry pointers, depending
   // on the size of a pointer.
--- a/security/manager/ssl/tests/gtest/OCSPCacheTest.cpp
+++ b/security/manager/ssl/tests/gtest/OCSPCacheTest.cpp
@@ -1,283 +1,315 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* 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 "CertVerifier.h"
 #include "OCSPCache.h"
+#include "gtest/gtest.h"
 #include "nss.h"
+#include "pkix/pkixtypes.h"
 #include "prerr.h"
 #include "prprf.h"
 #include "secerr.h"
 
-#include "gtest/gtest.h"
+using namespace mozilla::pkix;
+using namespace mozilla::psm;
 
 const int MaxCacheEntries = 1024;
 
 class OCSPCacheTest : public ::testing::Test
 {
   protected:
     static void SetUpTestCase()
     {
       NSS_NoDB_Init(nullptr);
       mozilla::psm::InitCertVerifierLog();
     }
 
     mozilla::psm::OCSPCache cache;
 };
 
-// Makes a fake certificate with just the fields we need for testing here.
-// (And those values are almost entirely bogus.)
-// stackCert should be stack-allocated memory.
 static void
-MakeFakeCert(CERTCertificate* stackCert, const char* subjectValue,
-             const char* issuerValue, const char* serialNumberValue,
-             const char* publicKeyValue)
-{
-  stackCert->derSubject.data = (unsigned char*)subjectValue;
-  stackCert->derSubject.len = strlen(subjectValue);
-  stackCert->derIssuer.data = (unsigned char*)issuerValue;
-  stackCert->derIssuer.len = strlen(issuerValue);
-  stackCert->serialNumber.data = (unsigned char*)serialNumberValue;
-  stackCert->serialNumber.len = strlen(serialNumberValue);
-  stackCert->derPublicKey.data = (unsigned char*)publicKeyValue;
-  stackCert->derPublicKey.len = strlen(publicKeyValue);
-  CERTName *subject = CERT_AsciiToName(subjectValue); // TODO: this will leak...
-  ASSERT_TRUE(subject);
-  stackCert->subject.arena = subject->arena;
-  stackCert->subject.rdns = subject->rdns;
-}
-
-static void
-PutAndGet(mozilla::psm::OCSPCache& cache, CERTCertificate* subject,
-          CERTCertificate* issuer,
-          PRErrorCode error, PRTime time)
+PutAndGet(OCSPCache& cache, const CertID& certID, PRErrorCode error,
+          PRTime time)
 {
   // The first time is thisUpdate. The second is validUntil.
   // The caller is expecting the validUntil returned with Get
   // to be equal to the passed-in time. Since these values will
   // be different in practice, make thisUpdate less than validUntil.
   ASSERT_TRUE(time >= 10);
-  SECStatus rv = cache.Put(subject, issuer, error, time - 10, time);
+  SECStatus rv = cache.Put(certID, error, time - 10, time);
   ASSERT_TRUE(rv == SECSuccess);
   PRErrorCode errorOut;
   PRTime timeOut;
-  ASSERT_TRUE(cache.Get(subject, issuer, errorOut, timeOut));
+  ASSERT_TRUE(cache.Get(certID, errorOut, timeOut));
   ASSERT_TRUE(error == errorOut && time == timeOut);
 }
 
+static const SECItem fakeIssuer1 = {
+  siBuffer,
+  const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>("CN=issuer1")),
+  10
+};
+static const SECItem fakeKey000 = {
+  siBuffer,
+  const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>("key000")),
+  6
+};
+static const SECItem fakeKey001 = {
+  siBuffer,
+  const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>("key001")),
+  6
+};
+static const SECItem fakeSerial0000 = {
+  siBuffer,
+  const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>("0000")),
+  4
+};
+
 TEST_F(OCSPCacheTest, TestPutAndGet)
 {
+  static const SECItem fakeSerial000 = {
+    siBuffer,
+    const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>("000")),
+    3
+  };
+  static const SECItem fakeSerial001 = {
+    siBuffer,
+    const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>("001")),
+    3
+  };
+
   SCOPED_TRACE("");
-  CERTCertificate subject;
-  MakeFakeCert(&subject, "CN=subject1", "CN=issuer1", "001", "key001");
-  CERTCertificate issuer;
-  MakeFakeCert(&issuer, "CN=issuer1", "CN=issuer1", "000", "key000");
-  PutAndGet(cache, &subject, &issuer, 0, PR_Now());
+  PutAndGet(cache, CertID(fakeIssuer1, fakeKey000, fakeSerial001), 0, PR_Now());
   PRErrorCode errorOut;
   PRTime timeOut;
-  ASSERT_FALSE(cache.Get(&issuer, &issuer, errorOut, timeOut));
+  ASSERT_FALSE(cache.Get(CertID(fakeIssuer1, fakeKey001, fakeSerial000),
+                         errorOut, timeOut));
 }
 
 TEST_F(OCSPCacheTest, TestVariousGets)
 {
   SCOPED_TRACE("");
-  CERTCertificate issuer;
-  MakeFakeCert(&issuer, "CN=issuer1", "CN=issuer1", "000", "key000");
   PRTime timeIn = PR_Now();
   for (int i = 0; i < MaxCacheEntries; i++) {
-    CERTCertificate subject;
-    char subjectBuf[64];
-    PR_snprintf(subjectBuf, sizeof(subjectBuf), "CN=subject%04d", i);
     char serialBuf[8];
     PR_snprintf(serialBuf, sizeof(serialBuf), "%04d", i);
-    MakeFakeCert(&subject, subjectBuf, "CN=issuer1", serialBuf, "key000");
-    PutAndGet(cache, &subject, &issuer, 0, timeIn + i);
+    const SECItem fakeSerial = {
+      siBuffer,
+      const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>(serialBuf)),
+      4
+    };
+    PutAndGet(cache, CertID(fakeIssuer1, fakeKey000, fakeSerial), 0, timeIn + i);
   }
-  CERTCertificate subject;
-  // This will be at the end of the list in the cache
+
   PRErrorCode errorOut;
   PRTime timeOut;
-  MakeFakeCert(&subject, "CN=subject0000", "CN=issuer1", "0000", "key000");
-  ASSERT_TRUE(cache.Get(&subject, &issuer, errorOut, timeOut));
+
+  // This will be at the end of the list in the cache
+  CertID cert0000(fakeIssuer1, fakeKey000, fakeSerial0000);
+  ASSERT_TRUE(cache.Get(cert0000, errorOut, timeOut));
   ASSERT_TRUE(errorOut == 0 && timeOut == timeIn);
   // Once we access it, it goes to the front
-  ASSERT_TRUE(cache.Get(&subject, &issuer, errorOut, timeOut));
+  ASSERT_TRUE(cache.Get(cert0000, errorOut, timeOut));
   ASSERT_TRUE(errorOut == 0 && timeOut == timeIn);
-  MakeFakeCert(&subject, "CN=subject0512", "CN=issuer1", "0512", "key000");
+
   // This will be in the middle
-  ASSERT_TRUE(cache.Get(&subject, &issuer, errorOut, timeOut));
+  static const SECItem fakeSerial0512 = {
+    siBuffer,
+    const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>("0512")),
+    4
+  };
+  CertID cert0512(fakeIssuer1, fakeKey000, fakeSerial0512);
+  ASSERT_TRUE(cache.Get(cert0512, errorOut, timeOut));
   ASSERT_TRUE(errorOut == 0 && timeOut == timeIn + 512);
-  ASSERT_TRUE(cache.Get(&subject, &issuer, errorOut, timeOut));
+  ASSERT_TRUE(cache.Get(cert0512, errorOut, timeOut));
   ASSERT_TRUE(errorOut == 0 && timeOut == timeIn + 512);
+
   // We've never seen this certificate
-  MakeFakeCert(&subject, "CN=subject1111", "CN=issuer1", "1111", "key000");
-  ASSERT_FALSE(cache.Get(&subject, &issuer, errorOut, timeOut));
+  static const SECItem fakeSerial1111 = {
+    siBuffer,
+    const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>("1111")),
+    4
+  };
+  ASSERT_FALSE(cache.Get(CertID(fakeIssuer1, fakeKey000, fakeSerial1111),
+                         errorOut, timeOut));
 }
 
 TEST_F(OCSPCacheTest, TestEviction)
 {
   SCOPED_TRACE("");
-  CERTCertificate issuer;
   PRTime timeIn = PR_Now();
-  MakeFakeCert(&issuer, "CN=issuer1", "CN=issuer1", "000", "key000");
+
   // By putting more distinct entries in the cache than it can hold,
   // we cause the least recently used entry to be evicted.
   for (int i = 0; i < MaxCacheEntries + 1; i++) {
-    CERTCertificate subject;
-    char subjectBuf[64];
-    PR_snprintf(subjectBuf, sizeof(subjectBuf), "CN=subject%04d", i);
     char serialBuf[8];
     PR_snprintf(serialBuf, sizeof(serialBuf), "%04d", i);
-    MakeFakeCert(&subject, subjectBuf, "CN=issuer1", serialBuf, "key000");
-    PutAndGet(cache, &subject, &issuer, 0, timeIn + i);
+    const SECItem fakeSerial = {
+      siBuffer,
+      const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>(serialBuf)),
+      4
+    };
+    PutAndGet(cache, CertID(fakeIssuer1, fakeKey000, fakeSerial), 0, timeIn + i);
   }
-  CERTCertificate evictedSubject;
-  MakeFakeCert(&evictedSubject, "CN=subject0000", "CN=issuer1", "0000", "key000");
+
   PRErrorCode errorOut;
   PRTime timeOut;
-  ASSERT_FALSE(cache.Get(&evictedSubject, &issuer, errorOut, timeOut));
+  ASSERT_FALSE(cache.Get(CertID(fakeIssuer1, fakeKey001, fakeSerial0000),
+                         errorOut, timeOut));
 }
 
 TEST_F(OCSPCacheTest, TestNoEvictionForRevokedResponses)
 {
   SCOPED_TRACE("");
-  CERTCertificate issuer;
-  MakeFakeCert(&issuer, "CN=issuer1", "CN=issuer1", "000", "key000");
-  CERTCertificate notEvictedSubject;
-  MakeFakeCert(&notEvictedSubject, "CN=subject0000", "CN=issuer1", "0000", "key000");
   PRTime timeIn = PR_Now();
-  PutAndGet(cache, &notEvictedSubject, &issuer, SEC_ERROR_REVOKED_CERTIFICATE, timeIn);
+  CertID notEvicted(fakeIssuer1, fakeKey000, fakeSerial0000);
+  PutAndGet(cache, notEvicted, SEC_ERROR_REVOKED_CERTIFICATE, timeIn);
   // By putting more distinct entries in the cache than it can hold,
   // we cause the least recently used entry that isn't revoked to be evicted.
   for (int i = 1; i < MaxCacheEntries + 1; i++) {
-    CERTCertificate subject;
-    char subjectBuf[64];
-    PR_snprintf(subjectBuf, sizeof(subjectBuf), "CN=subject%04d", i);
     char serialBuf[8];
     PR_snprintf(serialBuf, sizeof(serialBuf), "%04d", i);
-    MakeFakeCert(&subject, subjectBuf, "CN=issuer1", serialBuf, "key000");
-    PutAndGet(cache, &subject, &issuer, 0, timeIn + i);
+    const SECItem fakeSerial = {
+      siBuffer,
+      reinterpret_cast<uint8_t*>(serialBuf),
+      4
+    };
+    PutAndGet(cache, CertID(fakeIssuer1, fakeKey000, fakeSerial), 0, timeIn + i);
   }
   PRErrorCode errorOut;
   PRTime timeOut;
-  ASSERT_TRUE(cache.Get(&notEvictedSubject, &issuer, errorOut, timeOut));
+  ASSERT_TRUE(cache.Get(notEvicted, errorOut, timeOut));
   ASSERT_TRUE(errorOut == SEC_ERROR_REVOKED_CERTIFICATE && timeOut == timeIn);
-  CERTCertificate evictedSubject;
-  MakeFakeCert(&evictedSubject, "CN=subject0001", "CN=issuer1", "0001", "key000");
-  ASSERT_FALSE(cache.Get(&evictedSubject, &issuer, errorOut, timeOut));
+
+  const SECItem fakeSerial0001 = {
+    siBuffer,
+    const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>("0001")),
+    4
+  };
+  CertID evicted(fakeIssuer1, fakeKey000, fakeSerial0001);
+  ASSERT_FALSE(cache.Get(evicted, errorOut, timeOut));
 }
 
 TEST_F(OCSPCacheTest, TestEverythingIsRevoked)
 {
   SCOPED_TRACE("");
-  CERTCertificate issuer;
-  MakeFakeCert(&issuer, "CN=issuer1", "CN=issuer1", "000", "key000");
   PRTime timeIn = PR_Now();
   // Fill up the cache with revoked responses.
   for (int i = 0; i < MaxCacheEntries; i++) {
-    CERTCertificate subject;
-    char subjectBuf[64];
-    PR_snprintf(subjectBuf, sizeof(subjectBuf), "CN=subject%04d", i);
     char serialBuf[8];
     PR_snprintf(serialBuf, sizeof(serialBuf), "%04d", i);
-    MakeFakeCert(&subject, subjectBuf, "CN=issuer1", serialBuf, "key000");
-    PutAndGet(cache, &subject, &issuer, SEC_ERROR_REVOKED_CERTIFICATE, timeIn + i);
+    const SECItem fakeSerial = {
+      siBuffer,
+      reinterpret_cast<uint8_t*>(serialBuf),
+      4
+    };
+    PutAndGet(cache, CertID(fakeIssuer1, fakeKey000, fakeSerial),
+              SEC_ERROR_REVOKED_CERTIFICATE, timeIn + i);
   }
-  CERTCertificate goodSubject;
-  MakeFakeCert(&goodSubject, "CN=subject1025", "CN=issuer1", "1025", "key000");
+  const SECItem fakeSerial1025 = {
+    siBuffer,
+    const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>("1025")),
+    4
+  };
+  CertID good(fakeIssuer1, fakeKey000, fakeSerial1025);
   // This will "succeed", allowing verification to continue. However,
   // nothing was actually put in the cache.
-  SECStatus result = cache.Put(&goodSubject, &issuer, 0, timeIn + 1025 - 50,
-                               timeIn + 1025);
+  SECStatus result = cache.Put(good, 0, timeIn + 1025 - 50, timeIn + 1025);
   ASSERT_TRUE(result == SECSuccess);
   PRErrorCode errorOut;
   PRTime timeOut;
-  ASSERT_FALSE(cache.Get(&goodSubject, &issuer, errorOut, timeOut));
+  ASSERT_FALSE(cache.Get(good, errorOut, timeOut));
 
-  CERTCertificate revokedSubject;
-  MakeFakeCert(&revokedSubject, "CN=subject1026", "CN=issuer1", "1026", "key000");
+  const SECItem fakeSerial1026 = {
+    siBuffer,
+    const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>("1026")),
+    4
+  };
+  CertID revoked(fakeIssuer1, fakeKey000, fakeSerial1026);
   // This will fail, causing verification to fail.
-  result = cache.Put(&revokedSubject, &issuer, SEC_ERROR_REVOKED_CERTIFICATE,
+  result = cache.Put(revoked, SEC_ERROR_REVOKED_CERTIFICATE,
                      timeIn + 1026 - 50, timeIn + 1026);
   PRErrorCode error = PR_GetError();
   ASSERT_TRUE(result == SECFailure);
   ASSERT_TRUE(error == SEC_ERROR_REVOKED_CERTIFICATE);
 }
 
 TEST_F(OCSPCacheTest, VariousIssuers)
 {
   SCOPED_TRACE("");
-  CERTCertificate issuer1;
-  MakeFakeCert(&issuer1, "CN=issuer1", "CN=issuer1", "000", "key000");
-  CERTCertificate issuer2;
-  MakeFakeCert(&issuer2, "CN=issuer2", "CN=issuer2", "000", "key001");
-  CERTCertificate issuer3;
-  // Note: same CN as issuer1
-  MakeFakeCert(&issuer3, "CN=issuer1", "CN=issuer3", "000", "key003");
-  CERTCertificate subject;
-  MakeFakeCert(&subject, "CN=subject", "CN=issuer1", "001", "key002");
+  static const SECItem fakeIssuer2 = {
+    siBuffer,
+    const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>("CN=issuer2")),
+    10
+  };
+  static const SECItem fakeSerial001 = {
+    siBuffer,
+    const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>("001")),
+    3
+  };
+
   PRTime timeIn = PR_Now();
-  PutAndGet(cache, &subject, &issuer1, 0, timeIn);
+  CertID subject(fakeIssuer1, fakeKey000, fakeSerial001);
+  PutAndGet(cache, subject, 0, timeIn);
   PRErrorCode errorOut;
   PRTime timeOut;
-  ASSERT_TRUE(cache.Get(&subject, &issuer1, errorOut, timeOut));
+  ASSERT_TRUE(cache.Get(subject, errorOut, timeOut));
   ASSERT_TRUE(errorOut == 0 && timeOut == timeIn);
-  ASSERT_FALSE(cache.Get(&subject, &issuer2, errorOut, timeOut));
-  ASSERT_FALSE(cache.Get(&subject, &issuer3, errorOut, timeOut));
+  // Test that we don't match a different issuer DN
+  ASSERT_FALSE(cache.Get(CertID(fakeIssuer2, fakeKey000, fakeSerial001),
+                         errorOut, timeOut));
+  // Test that we don't match a different issuer key
+  ASSERT_FALSE(cache.Get(CertID(fakeIssuer1, fakeKey001, fakeSerial001),
+                         errorOut, timeOut));
 }
 
 TEST_F(OCSPCacheTest, Times)
 {
   SCOPED_TRACE("");
-  CERTCertificate subject;
-  MakeFakeCert(&subject, "CN=subject1", "CN=issuer1", "001", "key001");
-  CERTCertificate issuer;
-  MakeFakeCert(&issuer, "CN=issuer1", "CN=issuer1", "000", "key000");
-  PutAndGet(cache, &subject, &issuer, SEC_ERROR_OCSP_UNKNOWN_CERT, 100);
-  PutAndGet(cache, &subject, &issuer, 0, 200);
+  CertID certID(fakeIssuer1, fakeKey000, fakeSerial0000);
+  PutAndGet(cache, certID, SEC_ERROR_OCSP_UNKNOWN_CERT, 100);
+  PutAndGet(cache, certID, 0, 200);
   // This should not override the more recent entry.
-  SECStatus rv = cache.Put(&subject, &issuer, SEC_ERROR_OCSP_UNKNOWN_CERT, 100, 100);
-  ASSERT_TRUE(rv == SECSuccess);
+  ASSERT_EQ(SECSuccess, cache.Put(certID, SEC_ERROR_OCSP_UNKNOWN_CERT, 100,
+                                  100));
   PRErrorCode errorOut;
   PRTime timeOut;
-  ASSERT_TRUE(cache.Get(&subject, &issuer, errorOut, timeOut));
+  ASSERT_TRUE(cache.Get(certID, errorOut, timeOut));
   // Here we see the more recent time.
   ASSERT_TRUE(errorOut == 0 && timeOut == 200);
 
   // SEC_ERROR_REVOKED_CERTIFICATE overrides everything
-  PutAndGet(cache, &subject, &issuer, SEC_ERROR_REVOKED_CERTIFICATE, 50);
+  PutAndGet(cache, certID, SEC_ERROR_REVOKED_CERTIFICATE, 50);
 }
 
 TEST_F(OCSPCacheTest, NetworkFailure)
 {
   SCOPED_TRACE("");
-  CERTCertificate subject;
-  MakeFakeCert(&subject, "CN=subject1", "CN=issuer1", "001", "key001");
-  CERTCertificate issuer;
-  MakeFakeCert(&issuer, "CN=issuer1", "CN=issuer1", "000", "key000");
-  PutAndGet(cache, &subject, &issuer, PR_CONNECT_REFUSED_ERROR, 100);
-  PutAndGet(cache, &subject, &issuer, 0, 200);
+  CertID certID(fakeIssuer1, fakeKey000, fakeSerial0000);
+  PutAndGet(cache, certID, PR_CONNECT_REFUSED_ERROR, 100);
+  PutAndGet(cache, certID, 0, 200);
   // This should not override the already present entry.
-  SECStatus rv = cache.Put(&subject, &issuer, PR_CONNECT_REFUSED_ERROR, 300, 350);
+  SECStatus rv = cache.Put(certID, PR_CONNECT_REFUSED_ERROR, 300, 350);
   ASSERT_TRUE(rv == SECSuccess);
   PRErrorCode errorOut;
   PRTime timeOut;
-  ASSERT_TRUE(cache.Get(&subject, &issuer, errorOut, timeOut));
+  ASSERT_TRUE(cache.Get(certID, errorOut, timeOut));
   ASSERT_TRUE(errorOut == 0 && timeOut == 200);
 
-  PutAndGet(cache, &subject, &issuer, SEC_ERROR_OCSP_UNKNOWN_CERT, 400);
+  PutAndGet(cache, certID, SEC_ERROR_OCSP_UNKNOWN_CERT, 400);
   // This should not override the already present entry.
-  rv = cache.Put(&subject, &issuer, PR_CONNECT_REFUSED_ERROR, 500, 550);
+  rv = cache.Put(certID, PR_CONNECT_REFUSED_ERROR, 500, 550);
   ASSERT_TRUE(rv == SECSuccess);
-  ASSERT_TRUE(cache.Get(&subject, &issuer, errorOut, timeOut));
+  ASSERT_TRUE(cache.Get(certID, errorOut, timeOut));
   ASSERT_TRUE(errorOut == SEC_ERROR_OCSP_UNKNOWN_CERT && timeOut == 400);
 
-  PutAndGet(cache, &subject, &issuer, SEC_ERROR_REVOKED_CERTIFICATE, 600);
+  PutAndGet(cache, certID, SEC_ERROR_REVOKED_CERTIFICATE, 600);
   // This should not override the already present entry.
-  rv = cache.Put(&subject, &issuer, PR_CONNECT_REFUSED_ERROR, 700, 750);
+  rv = cache.Put(certID, PR_CONNECT_REFUSED_ERROR, 700, 750);
   ASSERT_TRUE(rv == SECSuccess);
-  ASSERT_TRUE(cache.Get(&subject, &issuer, errorOut, timeOut));
+  ASSERT_TRUE(cache.Get(certID, errorOut, timeOut));
   ASSERT_TRUE(errorOut == SEC_ERROR_REVOKED_CERTIFICATE && timeOut == 600);
 }
--- a/security/manager/ssl/tests/unit/tlsserver/lib/OCSPCommon.cpp
+++ b/security/manager/ssl/tests/unit/tlsserver/lib/OCSPCommon.cpp
@@ -1,26 +1,26 @@
 /* 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 "OCSPCommon.h"
 
 #include <stdio.h>
 
+#include "pkixtestutil.h"
 #include "ScopedNSSTypes.h"
 #include "TLSServer.h"
-#include "pkixtestutil.h"
 #include "secder.h"
 #include "secerr.h"
 
 using namespace mozilla;
-using namespace mozilla::test;
+using namespace mozilla::pkix;
 using namespace mozilla::pkix::test;
-
+using namespace mozilla::test;
 
 SECItemArray *
 GetOCSPResponseForType(OCSPResponseType aORT, CERTCertificate *aCert,
                        PLArenaPool *aArena, const char *aAdditionalCertName)
 {
   if (aORT == ORTNone) {
     if (gDebugLevel >= DEBUG_WARNINGS) {
       fprintf(stderr, "GetOCSPResponseForType called with type ORTNone, "
@@ -35,35 +35,36 @@ GetOCSPResponseForType(OCSPResponseType 
     arr->items[0].len = 0;
     return arr;
   }
 
   PRTime now = PR_Now();
   PRTime oneDay = 60*60*24 * (PRTime)PR_USEC_PER_SEC;
   PRTime oldNow = now - (8 * oneDay);
 
-  OCSPResponseContext context(aArena, aCert, now);
+  mozilla::ScopedCERTCertificate cert(CERT_DupCertificate(aCert));
 
   if (aORT == ORTGoodOtherCert) {
-    context.cert = PK11_FindCertFromNickname(aAdditionalCertName, nullptr);
-    if (!context.cert) {
+    cert = PK11_FindCertFromNickname(aAdditionalCertName, nullptr);
+    if (!cert) {
       PrintPRError("PK11_FindCertFromNickname failed");
       return nullptr;
     }
   }
   // XXX CERT_FindCertIssuer uses the old, deprecated path-building logic
-  ScopedCERTCertificate issuerCert(CERT_FindCertIssuer(aCert, now,
-                                                       certUsageSSLCA));
+  mozilla::ScopedCERTCertificate
+    issuerCert(CERT_FindCertIssuer(aCert, now, certUsageSSLCA));
   if (!issuerCert) {
     PrintPRError("CERT_FindCertIssuer failed");
     return nullptr;
   }
-  context.issuerNameDER = &issuerCert->derSubject;
-  context.issuerSPKI = &issuerCert->subjectPublicKeyInfo;
-  ScopedCERTCertificate signerCert;
+  CertID certID(cert->derIssuer, issuerCert->derPublicKey, cert->serialNumber);
+  OCSPResponseContext context(aArena, certID, now);
+
+  mozilla::ScopedCERTCertificate signerCert;
   if (aORT == ORTGoodOtherCA || aORT == ORTDelegatedIncluded ||
       aORT == ORTDelegatedIncludedLast || aORT == ORTDelegatedMissing ||
       aORT == ORTDelegatedMissingMultiple) {
     signerCert = PK11_FindCertFromNickname(aAdditionalCertName, nullptr);
     if (!signerCert) {
       PrintPRError("PK11_FindCertFromNickname failed");
       return nullptr;
     }
@@ -72,17 +73,17 @@ GetOCSPResponseForType(OCSPResponseType 
   const SECItem* certs[5] = { nullptr, nullptr, nullptr, nullptr, nullptr };
 
   if (aORT == ORTDelegatedIncluded) {
     certs[0] = &signerCert->derCert;
     context.certs = certs;
   }
   if (aORT == ORTDelegatedIncludedLast || aORT == ORTDelegatedMissingMultiple) {
     certs[0] = &issuerCert->derCert;
-    certs[1] = &context.cert->derCert;
+    certs[1] = &cert->derCert;
     certs[2] = &issuerCert->derCert;
     if (aORT != ORTDelegatedMissingMultiple) {
       certs[3] = &signerCert->derCert;
     }
     context.certs = certs;
   }
 
   switch (aORT) {
--- a/security/pkix/include/pkix/pkix.h
+++ b/security/pkix/include/pkix/pkix.h
@@ -100,36 +100,32 @@ SECStatus BuildCertChain(TrustDomain& tr
                  /*out*/ ScopedCERTCertList& results);
 
 // Verify the given signed data using the given public key.
 SECStatus VerifySignedData(const CERTSignedData* sd,
                            const SECItem& subjectPublicKeyInfo,
                            void* pkcs11PinArg);
 
 // The return value, if non-null, is owned by the arena and MUST NOT be freed.
-SECItem* CreateEncodedOCSPRequest(PLArenaPool* arena,
-                                  const CERTCertificate* cert,
-                                  const CERTCertificate* issuerCert);
+SECItem* CreateEncodedOCSPRequest(PLArenaPool* arena, const CertID& certID);
 
 // The out parameter expired will be true if the response has expired. If the
 // response also indicates a revoked or unknown certificate, that error
 // will be returned by PR_GetError(). Otherwise, SEC_ERROR_OCSP_OLD_RESPONSE
 // will be returned by PR_GetError() for an expired response.
 // The optional parameter thisUpdate will be the thisUpdate value of
 // the encoded response if it is considered trustworthy. Only
 // good, unknown, or revoked responses that verify correctly are considered
 // trustworthy. If the response is not trustworthy, thisUpdate will be 0.
 // Similarly, the optional parameter validThrough will be the time through
 // which the encoded response is considered trustworthy (that is, if a response had a
 // thisUpdate time of validThrough, it would be considered trustworthy).
 SECStatus VerifyEncodedOCSPResponse(TrustDomain& trustDomain,
-                                    const CERTCertificate* cert,
-                                    CERTCertificate* issuerCert,
-                                    PRTime time,
+                                    const CertID& certID, PRTime time,
                                     uint16_t maxLifetimeInDays,
-                                    const SECItem* encodedResponse,
+                                    const SECItem& encodedResponse,
                           /* out */ bool& expired,
-                 /* optional out */ PRTime* thisUpdate,
-                 /* optional out */ PRTime* validThrough);
+                 /* optional out */ PRTime* thisUpdate = nullptr,
+                 /* optional out */ PRTime* validThrough = nullptr);
 
 } } // namespace mozilla::pkix
 
 #endif // mozilla_pkix__pkix_h
--- a/security/pkix/include/pkix/pkixtypes.h
+++ b/security/pkix/include/pkix/pkixtypes.h
@@ -20,21 +20,21 @@
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
 
 #ifndef mozilla_pkix__pkixtypes_h
 #define mozilla_pkix__pkixtypes_h
 
+#include "cert.h"
 #include "pkix/enumclass.h"
 #include "pkix/ScopedPtr.h"
-#include "plarena.h"
-#include "cert.h"
-#include "keyhi.h"
+#include "seccomon.h"
+#include "secport.h"
 #include "stdint.h"
 
 namespace mozilla { namespace pkix {
 
 inline void
 PORT_FreeArena_false(PLArenaPool* arena) {
   // PL_FreeArenaPool can't be used because it doesn't actually free the
   // memory, which doesn't work well with memory analysis tools
@@ -83,16 +83,44 @@ struct CertPolicyId {
 
 MOZILLA_PKIX_ENUM_CLASS TrustLevel {
   TrustAnchor = 1,        // certificate is a trusted root CA certificate or
                           // equivalent *for the given policy*.
   ActivelyDistrusted = 2, // certificate is known to be bad
   InheritsTrust = 3       // certificate must chain to a trust anchor
 };
 
+// CertID references the information needed to do revocation checking for the
+// certificate issued by the given issuer with the given serial number.
+//
+// issuer must be the DER-encoded issuer field from the certificate for which
+// revocation checking is being done, **NOT** the subject field of the issuer
+// certificate. (Those two fields must be equal to each other, but they may not
+// be encoded exactly the same, and the encoding matters for OCSP.)
+// issuerSubjectPublicKeyInfo is the entire DER-encoded subjectPublicKeyInfo
+// field from the issuer's certificate. serialNumber is the entire DER-encoded
+// serial number from the subject certificate (the certificate for which we are
+// checking the revocation status).
+struct CertID
+{
+public:
+  CertID(const SECItem& issuer, const SECItem& issuerSubjectPublicKeyInfo,
+         const SECItem& serialNumber)
+    : issuer(issuer)
+    , issuerSubjectPublicKeyInfo(issuerSubjectPublicKeyInfo)
+    , serialNumber(serialNumber)
+  {
+  }
+  const SECItem& issuer;
+  const SECItem& issuerSubjectPublicKeyInfo;
+  const SECItem& serialNumber;
+private:
+  void operator=(const CertID&) /*= delete*/;
+};
+
 // Applications control the behavior of path building and verification by
 // implementing the TrustDomain interface. The TrustDomain is used for all
 // cryptography and for determining which certificates are trusted or
 // distrusted.
 class TrustDomain
 {
 public:
   virtual ~TrustDomain() { }
@@ -131,20 +159,19 @@ public:
   // Most implementations of this function should probably forward the call
   // directly to mozilla::pkix::VerifySignedData.
   virtual SECStatus VerifySignedData(const CERTSignedData* signedData,
                                      const SECItem& subjectPublicKeyInfo) = 0;
 
   // issuerCertToDup is only non-const so CERT_DupCertificate can be called on
   // it.
   virtual SECStatus CheckRevocation(EndEntityOrCA endEntityOrCA,
-                                    const CERTCertificate* cert,
-                          /*const*/ CERTCertificate* issuerCertToDup,
-                                    PRTime time,
-                       /*optional*/ const SECItem* stapledOCSPresponse) = 0;
+                                    const CertID& certID, PRTime time,
+                       /*optional*/ const SECItem* stapledOCSPresponse,
+                       /*optional*/ const SECItem* aiaExtension) = 0;
 
   // Called as soon as we think we have a valid chain but before revocation
   // checks are done. Called to compute additional chain level checks, by the
   // TrustDomain.
   virtual SECStatus IsChainValid(const CERTCertList* certChain) = 0;
 
 protected:
   TrustDomain() { }
--- a/security/pkix/lib/pkixbuild.cpp
+++ b/security/pkix/lib/pkixbuild.cpp
@@ -64,17 +64,16 @@ BackCert::Init(const SECItem& certDER)
   //    not desirable.
   if (! (nssCert->version.len == 1 &&
       nssCert->version.data[0] == mozilla::pkix::der::Version::v3)) {
     return Fail(RecoverableError, SEC_ERROR_EXTENSION_VALUE_INVALID);
   }
 
   const SECItem* dummyEncodedSubjectKeyIdentifier = nullptr;
   const SECItem* dummyEncodedAuthorityKeyIdentifier = nullptr;
-  const SECItem* dummyEncodedAuthorityInfoAccess = nullptr;
   const SECItem* dummyEncodedSubjectAltName = nullptr;
 
   for (const CERTCertExtension* ext = *exts; ext; ext = *++exts) {
     const SECItem** out = nullptr;
 
     // python DottedOIDToCode.py id-ce 2.5.29
     static const uint8_t id_ce[] = {
       0x55, 0x1d
@@ -99,17 +98,17 @@ BackCert::Init(const SECItem& certDER)
         case 54: out = &encodedInhibitAnyPolicy; break; // Bug 989051
       }
     } else if (ext->id.len == PR_ARRAY_SIZE(id_pe_authorityInfoAccess) &&
                !memcmp(ext->id.data, id_pe_authorityInfoAccess,
                        PR_ARRAY_SIZE(id_pe_authorityInfoAccess))) {
       // We should remember the value of the encoded AIA extension here, but
       // since our TrustDomain implementations get the OCSP URI using
       // CERT_GetOCSPAuthorityInfoAccessLocation, we currently don't need to.
-      out = &dummyEncodedAuthorityInfoAccess;
+      out = &encodedAuthorityInfoAccess;
     }
 
     // If this is an extension we don't understand and it's marked critical,
     // we must reject this certificate.
     // (The only valid explicit value of the critical flag is TRUE because
     // it is defined as BOOLEAN DEFAULT FALSE, so we just assume it is true.)
     if (!out && ext->critical.data && ext->critical.len > 0) {
       return Fail(RecoverableError, SEC_ERROR_UNKNOWN_CRITICAL_EXTENSION);
@@ -278,17 +277,17 @@ BuildForward(TrustDomain& trustDomain,
     ++subCACount;
   } else {
     PR_ASSERT(subCACount == 0);
   }
 
   // Find a trusted issuer.
   // TODO(bug 965136): Add SKI/AKI matching optimizations
   ScopedCERTCertList candidates;
-  if (trustDomain.FindPotentialIssuers(&subject.GetNSSCert()->derIssuer, time,
+  if (trustDomain.FindPotentialIssuers(&subject.GetIssuer(), time,
                                        candidates) != SECSuccess) {
     return MapSECStatus(SECFailure);
   }
   if (!candidates) {
     return Fail(RecoverableError, SEC_ERROR_UNKNOWN_ISSUER);
   }
 
   PRErrorCode errorToReturn = 0;
@@ -300,20 +299,22 @@ BuildForward(TrustDomain& trustDomain,
                            results);
     if (rv == Success) {
       // If we found a valid chain but deferred reporting an error with the
       // end-entity certificate, report it now.
       if (deferredEndEntityError != 0) {
         return Fail(FatalError, deferredEndEntityError);
       }
 
-      SECStatus srv = trustDomain.CheckRevocation(endEntityOrCA,
-                                                  subject.GetNSSCert(),
-                                                  n->cert, time,
-                                                  stapledOCSPResponse);
+      CertID certID(subject.GetIssuer(), n->cert->derPublicKey,
+                    subject.GetSerialNumber());
+      SECStatus srv = trustDomain.CheckRevocation(
+                                    endEntityOrCA, certID, time,
+                                    stapledOCSPResponse,
+                                    subject.encodedAuthorityInfoAccess);
       if (srv != SECSuccess) {
         return MapSECStatus(SECFailure);
       }
 
       // We found a trusted issuer. At this point, we know the cert is valid
       // and results contains the complete cert chain.
       return Success;
     }
--- a/security/pkix/lib/pkixkey.cpp
+++ b/security/pkix/lib/pkixkey.cpp
@@ -17,23 +17,23 @@
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
 
-#include "pkix/pkix.h"
-
 #include <limits>
 #include <stdint.h>
 
 #include "cert.h"
 #include "cryptohi.h"
+#include "keyhi.h"
+#include "pkix/pkix.h"
 #include "prerror.h"
 #include "secerr.h"
 
 namespace mozilla { namespace pkix {
 
 SECStatus
 VerifySignedData(const CERTSignedData* sd, const SECItem& subjectPublicKeyInfo,
                  void* pkcs11PinArg)
--- a/security/pkix/lib/pkixocsp.cpp
+++ b/security/pkix/lib/pkixocsp.cpp
@@ -19,25 +19,23 @@
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
 
 #include <limits>
 
+#include "hasht.h"
+#include "pk11pub.h"
 #include "pkix/bind.h"
 #include "pkix/pkix.h"
 #include "pkixcheck.h"
 #include "pkixder.h"
 
-#include "hasht.h"
-#include "pk11pub.h"
-#include "secder.h"
-
 // TODO: use typed/qualified typedefs everywhere?
 // TODO: When should we return SEC_ERROR_OCSP_UNAUTHORIZED_RESPONSE?
 
 namespace mozilla { namespace pkix {
 
 static const PRTime ONE_DAY
   = INT64_C(24) * INT64_C(60) * INT64_C(60) * PR_USEC_PER_SEC;
 static const PRTime SLOP = ONE_DAY;
@@ -47,47 +45,38 @@ MOZILLA_PKIX_ENUM_CLASS CertStatus : uin
   Good = der::CONTEXT_SPECIFIC | 0,
   Revoked = der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 1,
   Unknown = der::CONTEXT_SPECIFIC | 2
 };
 
 class Context
 {
 public:
-  Context(TrustDomain& trustDomain,
-          const SECItem& certSerialNumber,
-          const SECItem& issuerSubject,
-          const SECItem& issuerSubjectPublicKeyInfo,
-          PRTime time,
-          uint16_t maxLifetimeInDays,
-          PRTime* thisUpdate,
-          PRTime* validThrough)
+  Context(TrustDomain& trustDomain, const CertID& certID, PRTime time,
+          uint16_t maxLifetimeInDays, /*optional out*/ PRTime* thisUpdate,
+          /*optional out*/ PRTime* validThrough)
     : trustDomain(trustDomain)
-    , certSerialNumber(certSerialNumber)
-    , issuerSubject(issuerSubject)
-    , issuerSubjectPublicKeyInfo(issuerSubjectPublicKeyInfo)
+    , certID(certID)
     , time(time)
     , maxLifetimeInDays(maxLifetimeInDays)
     , certStatus(CertStatus::Unknown)
     , thisUpdate(thisUpdate)
     , validThrough(validThrough)
     , expired(false)
   {
     if (thisUpdate) {
       *thisUpdate = 0;
     }
     if (validThrough) {
       *validThrough = 0;
     }
   }
 
   TrustDomain& trustDomain;
-  const SECItem& certSerialNumber;
-  const SECItem& issuerSubject;
-  const SECItem& issuerSubjectPublicKeyInfo;
+  const CertID& certID;
   const PRTime time;
   const uint16_t maxLifetimeInDays;
   CertStatus certStatus;
   PRTime* thisUpdate;
   PRTime* validThrough;
   bool expired;
 
 private:
@@ -192,16 +181,19 @@ static inline der::Result SingleResponse
                                           Context& context);
 static inline der::Result CheckExtensionsForCriticality(der::Input&);
 static inline der::Result CertID(der::Input& input,
                                   const Context& context,
                                   /*out*/ bool& match);
 static Result MatchKeyHash(const SECItem& issuerKeyHash,
                            const SECItem& issuerSubjectPublicKeyInfo,
                            /*out*/ bool& match);
+static Result KeyHash(const SECItem& subjectPublicKeyInfo,
+                      /*out*/ uint8_t* hashBuf, size_t hashBufSize);
+
 
 static Result
 MatchResponderID(ResponderIDType responderIDType,
                  const SECItem& responderIDItem,
                  const SECItem& potentialSignerSubject,
                  const SECItem& potentialSignerSubjectPublicKeyInfo,
                  /*out*/ bool& match)
 {
@@ -256,24 +248,25 @@ VerifyOCSPSignedData(TrustDomain& trustD
 // *directly* to issuerCert.
 static Result
 VerifySignature(Context& context, ResponderIDType responderIDType,
                 const SECItem& responderID, const SECItem* certs,
                 size_t numCerts, const CERTSignedData& signedResponseData)
 {
   bool match;
   Result rv = MatchResponderID(responderIDType, responderID,
-                               context.issuerSubject,
-                               context.issuerSubjectPublicKeyInfo, match);
+                               context.certID.issuer,
+                               context.certID.issuerSubjectPublicKeyInfo,
+                               match);
   if (rv != Success) {
     return rv;
   }
   if (match) {
     return VerifyOCSPSignedData(context.trustDomain, signedResponseData,
-                                context.issuerSubjectPublicKeyInfo);
+                                context.certID.issuerSubjectPublicKeyInfo);
   }
 
   for (size_t i = 0; i < numCerts; ++i) {
     BackCert cert(nullptr, BackCert::IncludeCN::No);
     rv = cert.Init(certs[i]);
     if (rv != Success) {
       return rv;
     }
@@ -284,18 +277,18 @@ VerifySignature(Context& context, Respon
       return rv;
     }
     if (rv == RecoverableError) {
       continue;
     }
 
     if (match) {
       rv = CheckOCSPResponseSignerCert(context.trustDomain, cert,
-                                       context.issuerSubject,
-                                       context.issuerSubjectPublicKeyInfo,
+                                       context.certID.issuer,
+                                       context.certID.issuerSubjectPublicKeyInfo,
                                        context.time);
       if (rv == FatalError) {
         return rv;
       }
       if (rv == RecoverableError) {
         continue;
       }
 
@@ -311,44 +304,32 @@ static inline void
 SetErrorToMalformedResponseOnBadDERError()
 {
   if (PR_GetError() == SEC_ERROR_BAD_DER) {
     PR_SetError(SEC_ERROR_OCSP_MALFORMED_RESPONSE, 0);
   }
 }
 
 SECStatus
-VerifyEncodedOCSPResponse(TrustDomain& trustDomain,
-                          const CERTCertificate* cert,
-                          CERTCertificate* issuerCert, PRTime time,
-                          uint16_t maxOCSPLifetimeInDays,
-                          const SECItem* encodedResponse,
+VerifyEncodedOCSPResponse(TrustDomain& trustDomain, const struct CertID& certID,
+                          PRTime time, uint16_t maxOCSPLifetimeInDays,
+                          const SECItem& encodedResponse,
                           bool& expired,
-                          PRTime* thisUpdate,
-                          PRTime* validThrough)
+                          /*optional out*/ PRTime* thisUpdate,
+                          /*optional out*/ PRTime* validThrough)
 {
-  PR_ASSERT(cert);
-  PR_ASSERT(issuerCert);
-  // TODO: PR_Assert(pinArg)
-  PR_ASSERT(encodedResponse);
-  if (!cert || !issuerCert || !encodedResponse || !encodedResponse->data) {
-    PR_SetError(SEC_ERROR_INVALID_ARGS, 0);
-    return SECFailure;
-  }
-
   // Always initialize this to something reasonable.
   expired = false;
 
   der::Input input;
-  if (input.Init(encodedResponse->data, encodedResponse->len) != der::Success) {
+  if (input.Init(encodedResponse.data, encodedResponse.len) != der::Success) {
     SetErrorToMalformedResponseOnBadDERError();
     return SECFailure;
   }
-  Context context(trustDomain, cert->serialNumber, issuerCert->derSubject,
-                  issuerCert->derPublicKey, time, maxOCSPLifetimeInDays,
+  Context context(trustDomain, certID, time, maxOCSPLifetimeInDays,
                   thisUpdate, validThrough);
 
   if (der::Nested(input, der::SEQUENCE,
                   bind(OCSPResponse, _1, ref(context))) != der::Success) {
     SetErrorToMalformedResponseOnBadDERError();
     return SECFailure;
   }
 
@@ -721,17 +702,17 @@ CertID(der::Input& input, const Context&
     return der::Failure;
   }
 
   SECItem serialNumber;
   if (der::CertificateSerialNumber(input, serialNumber) != der::Success) {
     return der::Failure;
   }
 
-  if (!SECITEM_ItemsAreEqual(&serialNumber, &context.certSerialNumber)) {
+  if (!SECITEM_ItemsAreEqual(&serialNumber, &context.certID.serialNumber)) {
     // This does not reference the certificate we're interested in.
     // Consume the rest of the input and return successfully to
     // potentially continue processing other responses.
     input.SkipToEnd();
     return der::Success;
   }
 
   // TODO: support SHA-2 hashes.
@@ -746,27 +727,27 @@ CertID(der::Input& input, const Context&
   if (issuerNameHash.len != SHA1_LENGTH) {
     return der::Fail(SEC_ERROR_OCSP_MALFORMED_RESPONSE);
   }
 
   // From http://tools.ietf.org/html/rfc6960#section-4.1.1:
   // "The hash shall be calculated over the DER encoding of the
   // issuer's name field in the certificate being checked."
   uint8_t hashBuf[SHA1_LENGTH];
-  if (HashBuf(context.issuerSubject, hashBuf, sizeof(hashBuf))
+  if (HashBuf(context.certID.issuer, hashBuf, sizeof(hashBuf))
         != der::Success) {
     return der::Failure;
   }
   if (memcmp(hashBuf, issuerNameHash.data, issuerNameHash.len)) {
     // Again, not interested in this response. Consume input, return success.
     input.SkipToEnd();
     return der::Success;
   }
 
-  if (MatchKeyHash(issuerKeyHash, context.issuerSubjectPublicKeyInfo,
+  if (MatchKeyHash(issuerKeyHash, context.certID.issuerSubjectPublicKeyInfo,
                    match) != Success) {
     return der::Failure;
   }
 
   return der::Success;
 }
 
 // From http://tools.ietf.org/html/rfc6960#section-4.1.1:
@@ -781,18 +762,33 @@ CertID(der::Input& input, const Context&
 //                          -- bits] in the responder's certificate)
 static Result
 MatchKeyHash(const SECItem& keyHash, const SECItem& subjectPublicKeyInfo,
              /*out*/ bool& match)
 {
   if (keyHash.len != SHA1_LENGTH)  {
     return Fail(RecoverableError, SEC_ERROR_OCSP_MALFORMED_RESPONSE);
   }
+  static uint8_t hashBuf[SHA1_LENGTH];
+  Result rv = KeyHash(subjectPublicKeyInfo, hashBuf, sizeof hashBuf);
+  if (rv != Success) {
+    return rv;
+  }
+  match = !memcmp(hashBuf, keyHash.data, keyHash.len);
+  return Success;
+}
 
-  // TODO(bug 966856): support SHA-2 hashes
+// TODO(bug 966856): support SHA-2 hashes
+Result
+KeyHash(const SECItem& subjectPublicKeyInfo, /*out*/ uint8_t* hashBuf,
+        size_t hashBufSize)
+{
+  if (!hashBuf || hashBufSize != SHA1_LENGTH) {
+    return Fail(FatalError, SEC_ERROR_LIBRARY_FAILURE);
+  }
 
   // RFC 5280 Section 4.1
   //
   // SubjectPublicKeyInfo  ::=  SEQUENCE  {
   //    algorithm            AlgorithmIdentifier,
   //    subjectPublicKey     BIT STRING  }
 
   der::Input spki;
@@ -831,21 +827,19 @@ MatchKeyHash(const SECItem& keyHash, con
 
   // Assume/require that the number of unused bits in the public key is zero.
   if (subjectPublicKey.len == 0 || subjectPublicKey.data[0] != 0) {
     return Fail(RecoverableError, SEC_ERROR_BAD_DER);
   }
   ++subjectPublicKey.data;
   --subjectPublicKey.len;
 
-  static uint8_t hashBuf[SHA1_LENGTH];
-  if (HashBuf(subjectPublicKey, hashBuf, sizeof(hashBuf)) != der::Success) {
+  if (HashBuf(subjectPublicKey, hashBuf, hashBufSize) != der::Success) {
     return MapSECStatus(SECFailure);
   }
-  match = !memcmp(hashBuf, keyHash.data, keyHash.len);
   return Success;
 }
 
 // Extension  ::=  SEQUENCE  {
 //      extnID      OBJECT IDENTIFIER,
 //      critical    BOOLEAN DEFAULT FALSE,
 //      extnValue   OCTET STRING
 //      }
@@ -898,21 +892,19 @@ CheckExtensionsForCriticality(der::Input
 //   SHOULD be considered unreliable.
 //
 //   If nextUpdate is not set, the responder is indicating that newer
 //   revocation information is available all the time.
 //
 // http://tools.ietf.org/html/rfc5019#section-4
 
 SECItem*
-CreateEncodedOCSPRequest(PLArenaPool* arena,
-                         const CERTCertificate* cert,
-                         const CERTCertificate* issuerCert)
+CreateEncodedOCSPRequest(PLArenaPool* arena, const struct CertID& certID)
 {
-  if (!arena || !cert || !issuerCert) {
+  if (!arena) {
     PR_SetError(SEC_ERROR_INVALID_ARGS, 0);
     return nullptr;
   }
 
   // We do not add any extensions to the request.
 
   // RFC 6960 says "An OCSP client MAY wish to specify the kinds of response
   // types it understands. To do so, it SHOULD use an extension with the OID
@@ -947,23 +939,23 @@ CreateEncodedOCSPRequest(PLArenaPool* ar
     + 2;                            //           serialNumber (header)
 
   // The only way we could have a request this large is if the serialNumber was
   // ridiculously and unreasonably large. RFC 5280 says "Conforming CAs MUST
   // NOT use serialNumber values longer than 20 octets." With this restriction,
   // we allow for some amount of non-conformance with that requirement while
   // still ensuring we can encode the length values in the ASN.1 TLV structures
   // in a single byte.
-  if (cert->serialNumber.len > 127u - totalLenWithoutSerialNumberData) {
+  if (certID.serialNumber.len > 127u - totalLenWithoutSerialNumberData) {
     PR_SetError(SEC_ERROR_BAD_DATA, 0);
     return nullptr;
   }
 
   uint8_t totalLen = static_cast<uint8_t>(totalLenWithoutSerialNumberData +
-    cert->serialNumber.len);
+    certID.serialNumber.len);
 
   SECItem* encodedRequest = SECITEM_AllocItem(arena, nullptr, totalLen);
   if (!encodedRequest) {
     return nullptr;
   }
 
   uint8_t* d = encodedRequest->data;
   *d++ = 0x30; *d++ = totalLen - 2u;  // OCSPRequest (SEQUENCE)
@@ -975,36 +967,34 @@ CreateEncodedOCSPRequest(PLArenaPool* ar
   // reqCert.hashAlgorithm
   for (size_t i = 0; i < PR_ARRAY_SIZE(hashAlgorithm); ++i) {
     *d++ = hashAlgorithm[i];
   }
 
   // reqCert.issuerNameHash (OCTET STRING)
   *d++ = 0x04;
   *d++ = hashLen;
-  if (HashBuf(issuerCert->derSubject, d, hashLen) != der::Success) {
+  if (HashBuf(certID.issuer, d, hashLen) != der::Success) {
     return nullptr;
   }
   d += hashLen;
 
   // reqCert.issuerKeyHash (OCTET STRING)
   *d++ = 0x04;
   *d++ = hashLen;
-  SECItem key = issuerCert->subjectPublicKeyInfo.subjectPublicKey;
-  DER_ConvertBitString(&key);
-  if (HashBuf(key, d, hashLen) != der::Success) {
+  if (KeyHash(certID.issuerSubjectPublicKeyInfo, d, hashLen) != Success) {
     return nullptr;
   }
   d += hashLen;
 
   // reqCert.serialNumber (INTEGER)
   *d++ = 0x02; // INTEGER
-  *d++ = static_cast<uint8_t>(cert->serialNumber.len);
-  for (size_t i = 0; i < cert->serialNumber.len; ++i) {
-    *d++ = cert->serialNumber.data[i];
+  *d++ = static_cast<uint8_t>(certID.serialNumber.len);
+  for (size_t i = 0; i < certID.serialNumber.len; ++i) {
+    *d++ = certID.serialNumber.data[i];
   }
 
   PR_ASSERT(d == encodedRequest->data + totalLen);
 
   return encodedRequest;
 }
 
 } } // namespace mozilla::pkix
--- a/security/pkix/lib/pkixutil.h
+++ b/security/pkix/lib/pkixutil.h
@@ -91,41 +91,44 @@ class BackCert
 public:
   // IncludeCN::No means that GetConstrainedNames won't include the subject CN
   // in its results. IncludeCN::Yes means that GetConstrainedNames will include
   // the subject CN in its results.
   MOZILLA_PKIX_ENUM_CLASS IncludeCN { No = 0, Yes = 1 };
 
   // nssCert and childCert must be valid for the lifetime of BackCert
   BackCert(BackCert* childCert, IncludeCN includeCN)
-    : encodedBasicConstraints(nullptr)
+    : encodedAuthorityInfoAccess(nullptr)
+    , encodedBasicConstraints(nullptr)
     , encodedCertificatePolicies(nullptr)
     , encodedExtendedKeyUsage(nullptr)
     , encodedKeyUsage(nullptr)
     , encodedNameConstraints(nullptr)
     , encodedInhibitAnyPolicy(nullptr)
     , childCert(childCert)
     , constrainedNames(nullptr)
     , includeCN(includeCN)
   {
   }
 
   Result Init(const SECItem& certDER);
 
   const SECItem& GetDER() const { return nssCert->derCert; }
   const SECItem& GetIssuer() const { return nssCert->derIssuer; }
+  const SECItem& GetSerialNumber() const { return nssCert->serialNumber; }
   const SECItem& GetSubject() const { return nssCert->derSubject; }
   const SECItem& GetSubjectPublicKeyInfo() const
   {
     return nssCert->derPublicKey;
   }
 
   Result VerifyOwnSignatureWithKey(TrustDomain& trustDomain,
                                    const SECItem& subjectPublicKeyInfo) const;
 
+  const SECItem* encodedAuthorityInfoAccess;
   const SECItem* encodedBasicConstraints;
   const SECItem* encodedCertificatePolicies;
   const SECItem* encodedExtendedKeyUsage;
   const SECItem* encodedKeyUsage;
   const SECItem* encodedNameConstraints;
   const SECItem* encodedInhibitAnyPolicy;
 
   BackCert* const childCert;
--- a/security/pkix/test/gtest/pkix_cert_chain_length_tests.cpp
+++ b/security/pkix/test/gtest/pkix_cert_chain_length_tests.cpp
@@ -137,18 +137,18 @@ private:
 
   SECStatus VerifySignedData(const CERTSignedData* signedData,
                              const SECItem& subjectPublicKeyInfo)
   {
     return ::mozilla::pkix::VerifySignedData(signedData, subjectPublicKeyInfo,
                                              nullptr);
   }
 
-  SECStatus CheckRevocation(EndEntityOrCA, const CERTCertificate*,
-                            /*const*/ CERTCertificate*, PRTime,
+  SECStatus CheckRevocation(EndEntityOrCA, const CertID&, PRTime,
+                            /*optional*/ const SECItem*,
                             /*optional*/ const SECItem*)
   {
     return SECSuccess;
   }
 
   virtual SECStatus IsChainValid(const CERTCertList*)
   {
     return SECSuccess;
--- a/security/pkix/test/gtest/pkix_cert_extension_tests.cpp
+++ b/security/pkix/test/gtest/pkix_cert_extension_tests.cpp
@@ -90,18 +90,18 @@ private:
 
   SECStatus VerifySignedData(const CERTSignedData* signedData,
                              const SECItem& subjectPublicKeyInfo)
   {
     return ::mozilla::pkix::VerifySignedData(signedData, subjectPublicKeyInfo,
                                              nullptr);
   }
 
-  SECStatus CheckRevocation(EndEntityOrCA, const CERTCertificate*,
-                            /*const*/ CERTCertificate*, PRTime,
+  SECStatus CheckRevocation(EndEntityOrCA, const CertID&, PRTime,
+                            /*optional*/ const SECItem*,
                             /*optional*/ const SECItem*)
   {
     return SECSuccess;
   }
 
   virtual SECStatus IsChainValid(const CERTCertList*)
   {
     return SECSuccess;
--- a/security/pkix/test/gtest/pkix_ocsp_request_tests.cpp
+++ b/security/pkix/test/gtest/pkix_ocsp_request_tests.cpp
@@ -31,17 +31,16 @@
 using namespace mozilla::pkix;
 using namespace mozilla::pkix::test;
 
 class pkix_ocsp_request_tests : public NSSTest
 {
 protected:
   // These SECItems are allocated in arena, and so will be auto-cleaned.
   SECItem* unsupportedLongSerialNumber;
-  SECItem* shortSerialNumber;
   SECItem* longestRequiredSerialNumber;
 
   void SetUp()
   {
     static const uint8_t UNSUPPORTED_LEN = 128; // must be larger than 127
     // tag + length + value is 1 + 2 + UNSUPPORTED_LEN
     unsupportedLongSerialNumber = SECITEM_AllocItem(arena.get(), nullptr,
                                                     1 + 2 + UNSUPPORTED_LEN);
@@ -49,115 +48,69 @@ protected:
            unsupportedLongSerialNumber->len);
     unsupportedLongSerialNumber->data[0] = der::INTEGER;
     // Encoding the length takes two bytes: one byte to indicate that a
     // second byte follows, and the second byte to indicate the length.
     unsupportedLongSerialNumber->data[1] = 0x80 + 1;
     unsupportedLongSerialNumber->data[2] = UNSUPPORTED_LEN;
     unsupportedLongSerialNumber->data[3] = 0x01; // value is 0x010000...00
 
-    // Each of tag, length, and value here are 1 byte: the total length is 3.
-    shortSerialNumber = SECITEM_AllocItem(arena.get(), nullptr, 3);
-    shortSerialNumber->data[0] = der::INTEGER;
-    shortSerialNumber->data[1] = 0x01; // length of value is 1
-    shortSerialNumber->data[2] = 0x01; // value is 1
-
     static const uint8_t LONGEST_REQUIRED_LEN = 20;
     // tag + length + value is 1 + 1 + LONGEST_REQUIRED_LEN
     longestRequiredSerialNumber = SECITEM_AllocItem(arena.get(), nullptr,
                                     1 + 1 + LONGEST_REQUIRED_LEN);
     memset(longestRequiredSerialNumber->data, 0,
            longestRequiredSerialNumber->len);
     longestRequiredSerialNumber->data[0] = der::INTEGER;
     longestRequiredSerialNumber->data[1] = LONGEST_REQUIRED_LEN;
     longestRequiredSerialNumber->data[2] = 0x01; // value is 0x010000...00
   }
 
-  void MakeTwoCerts(const char* issuerCN, SECItem* issuerSerial,
-                    /*out*/ ScopedCERTCertificate& issuer,
-                    const char* childCN, SECItem* childSerial,
-                    /*out*/ ScopedCERTCertificate& child)
+  // The resultant issuerDER is owned by the arena.
+  SECStatus MakeIssuerCertIDComponents(const char* issuerASCII,
+                                       /*out*/ const SECItem*& issuerDER,
+                                       /*out*/ ScopedSECItem& issuerSPKI)
   {
-    const SECItem* issuerNameDer = ASCIIToDERName(arena.get(), issuerCN);
-    ASSERT_TRUE(issuerNameDer);
-    ScopedSECKEYPrivateKey issuerKey;
-    SECItem* issuerCertDer(CreateEncodedCertificate(arena.get(), v3,
-                             SEC_OID_SHA256, issuerSerial, issuerNameDer,
-                             oneDayBeforeNow, oneDayAfterNow, issuerNameDer,
-                             nullptr, nullptr, SEC_OID_SHA256, issuerKey));
-    ASSERT_TRUE(issuerCertDer);
-    const SECItem* childNameDer = ASCIIToDERName(arena.get(), childCN);
-    ASSERT_TRUE(childNameDer);
-    ScopedSECKEYPrivateKey childKey;
-    SECItem* childDer(CreateEncodedCertificate(arena.get(), v3,
-                        SEC_OID_SHA256, childSerial, issuerNameDer,
-                        oneDayBeforeNow, oneDayAfterNow, childNameDer, nullptr,
-                        issuerKey.get(), SEC_OID_SHA256, childKey));
-    ASSERT_TRUE(childDer);
-    issuer = CERT_NewTempCertificate(CERT_GetDefaultCertDB(), issuerCertDer,
-                                     nullptr, false, true);
-    ASSERT_TRUE(issuer);
-    child = CERT_NewTempCertificate(CERT_GetDefaultCertDB(), childDer, nullptr,
-                                    false, true);
-    ASSERT_TRUE(child);
+    issuerDER = ASCIIToDERName(arena.get(), issuerASCII);
+    if (!issuerDER) {
+      return SECFailure;
+    }
+    ScopedSECKEYPublicKey issuerPublicKey;
+    ScopedSECKEYPrivateKey issuerPrivateKey;
+    if (GenerateKeyPair(issuerPublicKey, issuerPrivateKey) != SECSuccess) {
+      return SECFailure;
+    }
+    issuerSPKI = SECKEY_EncodeDERSubjectPublicKeyInfo(issuerPublicKey.get());
+    if (!issuerSPKI) {
+      return SECFailure;
+    }
+
+    return SECSuccess;
   }
 
 };
 
-// Test that the large length of the issuer serial number doesn't cause
-// CreateEncodedOCSPRequest to fail when called for the child certificate.
-TEST_F(pkix_ocsp_request_tests, IssuerCertLongSerialNumberTest)
-{
-  const char* issuerCN = "CN=Long Serial Number CA";
-  const char* childCN = "CN=Short Serial Number EE";
-  ScopedCERTCertificate issuer;
-  ScopedCERTCertificate child;
-  {
-    SCOPED_TRACE("IssuerCertLongSerialNumberTest");
-    MakeTwoCerts(issuerCN, unsupportedLongSerialNumber, issuer,
-                 childCN, shortSerialNumber, child);
-  }
-  ASSERT_TRUE(issuer);
-  ASSERT_TRUE(child);
-  ASSERT_TRUE(CreateEncodedOCSPRequest(arena.get(), child.get(),
-                                       issuer.get()));
-  ASSERT_EQ(0, PR_GetError());
-}
-
 // Test that the large length of the child serial number causes
 // CreateEncodedOCSPRequest to fail.
 TEST_F(pkix_ocsp_request_tests, ChildCertLongSerialNumberTest)
 {
-  const char* issuerCN = "CN=Short Serial Number CA";
-  const char* childCN = "CN=Long Serial Number EE";
-  ScopedCERTCertificate issuer;
-  ScopedCERTCertificate child;
-  {
-    SCOPED_TRACE("ChildCertLongSerialNumberTest");
-    MakeTwoCerts(issuerCN, shortSerialNumber, issuer,
-                 childCN, unsupportedLongSerialNumber, child);
-  }
-  ASSERT_TRUE(issuer);
-  ASSERT_TRUE(child);
-  ASSERT_FALSE(CreateEncodedOCSPRequest(arena.get(), child.get(),
-                                        issuer.get()));
+  const SECItem* issuerDER;
+  ScopedSECItem issuerSPKI;
+  ASSERT_EQ(SECSuccess,
+            MakeIssuerCertIDComponents("CN=CA", issuerDER, issuerSPKI));
+  ASSERT_FALSE(CreateEncodedOCSPRequest(arena.get(),
+                                        CertID(*issuerDER, *issuerSPKI,
+                                               *unsupportedLongSerialNumber)));
   ASSERT_EQ(SEC_ERROR_BAD_DATA, PR_GetError());
 }
 
 // Test that CreateEncodedOCSPRequest handles the longest serial number that
 // it's required to support (i.e. 20 octets).
 TEST_F(pkix_ocsp_request_tests, LongestSupportedSerialNumberTest)
 {
-  const char* issuerCN = "CN=Short Serial Number CA";
-  const char* childCN = "CN=Longest Serial Number Supported EE";
-  ScopedCERTCertificate issuer;
-  ScopedCERTCertificate child;
-  {
-    SCOPED_TRACE("LongestSupportedSerialNumberTest");
-    MakeTwoCerts(issuerCN, shortSerialNumber, issuer,
-                 childCN, longestRequiredSerialNumber, child);
-  }
-  ASSERT_TRUE(issuer);
-  ASSERT_TRUE(child);
-  ASSERT_TRUE(CreateEncodedOCSPRequest(arena.get(), child.get(),
-                                       issuer.get()));
-  ASSERT_EQ(0, PR_GetError());
+  const SECItem* issuerDER;
+  ScopedSECItem issuerSPKI;
+  ASSERT_EQ(SECSuccess,
+            MakeIssuerCertIDComponents("CN=CA", issuerDER, issuerSPKI));
+  ASSERT_TRUE(CreateEncodedOCSPRequest(arena.get(),
+                                        CertID(*issuerDER, *issuerSPKI,
+                                               *longestRequiredSerialNumber)));
 }
--- a/security/pkix/test/lib/pkixtestutil.cpp
+++ b/security/pkix/test/lib/pkixtestutil.cpp
@@ -173,24 +173,21 @@ private:
   size_t numItems;
   size_t length;
 
   Output(const Output&) /* = delete */;
   void operator=(const Output&) /* = delete */;
 };
 
 OCSPResponseContext::OCSPResponseContext(PLArenaPool* arena,
-                                         CERTCertificate* cert,
-                                         PRTime time)
+                                         const CertID& certID, PRTime time)
   : arena(arena)
-  , cert(CERT_DupCertificate(cert))
+  , certID(certID)
   , responseStatus(successful)
   , skipResponseBytes(false)
-  , issuerNameDER(nullptr)
-  , issuerSPKI(nullptr)
   , signerNameDER(nullptr)
   , producedAt(time)
   , extensions(nullptr)
   , includeEmptyExtensions(false)
   , badSignature(false)
   , certs(nullptr)
 
   , certIDHashAlg(SEC_OID_SHA1)
@@ -238,41 +235,41 @@ HashAlgorithmToLength(SECOidTag hashAlg)
     default:
       PR_NOT_REACHED("HashAlgorithmToLength: bad hashAlg");
       PR_Abort();
   }
   return 0;
 }
 
 static SECItem*
-HashedOctetString(PLArenaPool* arena, const SECItem* bytes, SECOidTag hashAlg)
+HashedOctetString(PLArenaPool* arena, const SECItem& bytes, SECOidTag hashAlg)
 {
   size_t hashLen = HashAlgorithmToLength(hashAlg);
   if (hashLen == 0) {
     return nullptr;
   }
   SECItem* hashBuf = SECITEM_AllocItem(arena, nullptr, hashLen);
   if (!hashBuf) {
     return nullptr;
   }
-  if (PK11_HashBuf(hashAlg, hashBuf->data, bytes->data, bytes->len)
+  if (PK11_HashBuf(hashAlg, hashBuf->data, bytes.data, bytes.len)
         != SECSuccess) {
     return nullptr;
   }
 
   return EncodeNested(arena, der::OCTET_STRING, hashBuf);
 }
 
 static SECItem*
 KeyHashHelper(PLArenaPool* arena, const CERTSubjectPublicKeyInfo* spki)
 {
   // We only need a shallow copy here.
   SECItem spk = spki->subjectPublicKey;
   DER_ConvertBitString(&spk); // bits to bytes
-  return HashedOctetString(arena, &spk, SEC_OID_SHA1);
+  return HashedOctetString(arena, spk, SEC_OID_SHA1);
 }
 
 static SECItem*
 AlgorithmIdentifier(PLArenaPool* arena, SECOidTag algTag)
 {
   SECAlgorithmIDStr aid;
   aid.algorithm.data = nullptr;
   aid.algorithm.len = 0;
@@ -914,18 +911,17 @@ SECItem*
 CreateEncodedOCSPResponse(OCSPResponseContext& context)
 {
   if (!context.arena) {
     PR_SetError(SEC_ERROR_INVALID_ARGS, 0);
     return nullptr;
   }
 
   if (!context.skipResponseBytes) {
-    if (!context.cert || !context.issuerNameDER || !context.issuerSPKI ||
-        !context.signerPrivateKey) {
+    if (!context.signerPrivateKey) {
       PR_SetError(SEC_ERROR_INVALID_ARGS, 0);
       return nullptr;
     }
   }
 
   // OCSPResponse ::= SEQUENCE {
   //    responseStatus          OCSPResponseStatus,
   //    responseBytes       [0] EXPLICIT ResponseBytes OPTIONAL }
@@ -1255,31 +1251,39 @@ SECItem*
 CertID(OCSPResponseContext& context)
 {
   SECItem* hashAlgorithm = AlgorithmIdentifier(context.arena,
                                                context.certIDHashAlg);
   if (!hashAlgorithm) {
     return nullptr;
   }
   SECItem* issuerNameHash = HashedOctetString(context.arena,
-                                              context.issuerNameDER,
+                                              context.certID.issuer,
                                               context.certIDHashAlg);
   if (!issuerNameHash) {
     return nullptr;
   }
-  SECItem* issuerKeyHash = KeyHashHelper(context.arena, context.issuerSPKI);
+
+  ScopedPtr<CERTSubjectPublicKeyInfo, SECKEY_DestroySubjectPublicKeyInfo>
+    spki(SECKEY_DecodeDERSubjectPublicKeyInfo(
+          &context.certID.issuerSubjectPublicKeyInfo));
+  if (!spki) {
+    return nullptr;
+  }
+  SECItem* issuerKeyHash(KeyHashHelper(context.arena, spki.get()));
   if (!issuerKeyHash) {
     return nullptr;
   }
+
   static const SEC_ASN1Template serialTemplate[] = {
-    { SEC_ASN1_INTEGER, offsetof(CERTCertificate, serialNumber) },
+    { SEC_ASN1_INTEGER, 0 },
     { 0 }
   };
   SECItem* serialNumber = SEC_ASN1EncodeItem(context.arena, nullptr,
-                                             context.cert.get(),
+                                             &context.certID.serialNumber,
                                              serialTemplate);
   if (!serialNumber) {
     return nullptr;
   }
 
   Output output;
   if (output.Add(hashAlgorithm) != der::Success) {
     return nullptr;
--- a/security/pkix/test/lib/pkixtestutil.h
+++ b/security/pkix/test/lib/pkixtestutil.h
@@ -23,16 +23,17 @@
  */
 
 #ifndef mozilla_pkix_test__pkixtestutils_h
 #define mozilla_pkix_test__pkixtestutils_h
 
 #include <stdint.h>
 #include <stdio.h>
 
+#include "keyhi.h"
 #include "pkix/enumclass.h"
 #include "pkix/pkixtypes.h"
 #include "pkix/ScopedPtr.h"
 #include "seccomon.h"
 
 namespace mozilla { namespace pkix { namespace test {
 
 namespace {
@@ -123,21 +124,21 @@ public:
   bool critical;
   SECItem value;
   OCSPResponseExtension* next;
 };
 
 class OCSPResponseContext
 {
 public:
-  OCSPResponseContext(PLArenaPool* arena, CERTCertificate* cert, PRTime time);
+  OCSPResponseContext(PLArenaPool* arena, const CertID& certID, PRTime time);
 
   PLArenaPool* arena;
+  const CertID& certID;
   // TODO(bug 980538): add a way to specify what certificates are included.
-  pkix::ScopedCERTCertificate cert; // The subject of the OCSP response
 
   // The fields below are in the order that they appear in an OCSP response.
 
   // By directly using the issuer name & SPKI and signer name & private key,
   // instead of extracting those things out of CERTCertificate objects, we
   // avoid poor interactions with the NSS CERTCertificate caches. In
   // particular, there are some tests in which it is important that we know
   // that the issuer and/or signer certificates are NOT in the NSS caches
@@ -155,18 +156,16 @@ public:
     // 4 is not used
     sigRequired = 5,
     unauthorized = 6,
   };
   uint8_t responseStatus; // an OCSPResponseStatus or an invalid value
   bool skipResponseBytes; // If true, don't include responseBytes
 
   // responderID
-  const SECItem* issuerNameDER; // non-owning
-  const CERTSubjectPublicKeyInfo* issuerSPKI; // non-owning pointer
   const SECItem* signerNameDER; // If set, responderID will use the byName
                                 // form; otherwise responderID will use the
                                 // byKeyHash form.
 
   PRTime producedAt;
 
   OCSPResponseExtension* extensions;
   bool includeEmptyExtensions; // If true, include the extension wrapper