Bug 1122841, Part 2: Centralize checking of public key, r=keeler
authorBrian Smith <brian@briansmith.org>
Mon, 02 Feb 2015 16:17:08 -0800
changeset 242197 3fe8d7d7f9f7373d0d3a3341d1a46347c06c85c7
parent 242196 75c440d6b2fff1191f4a48807243e5be1783c12a
child 242198 0db39f732328a2bb42c0bb2367a1d32d5edbd47a
push id649
push userwcosta@mozilla.com
push dateWed, 11 Feb 2015 16:57:44 +0000
reviewerskeeler
bugs1122841
milestone38.0a1
Bug 1122841, Part 2: Centralize checking of public key, r=keeler
security/apps/AppTrustDomain.cpp
security/apps/AppTrustDomain.h
security/certverifier/NSSCertDBTrustDomain.cpp
security/certverifier/NSSCertDBTrustDomain.h
security/manager/ssl/tests/unit/test_keysize_ev.js
security/manager/ssl/tests/unit/test_ocsp_stapling.js
security/pkix/include/pkix/Result.h
security/pkix/include/pkix/pkixnss.h
security/pkix/include/pkix/pkixtypes.h
security/pkix/lib/pkixbuild.cpp
security/pkix/lib/pkixcert.cpp
security/pkix/lib/pkixcheck.cpp
security/pkix/lib/pkixder.cpp
security/pkix/lib/pkixder.h
security/pkix/lib/pkixnss.cpp
security/pkix/test/gtest/pkixbuild_tests.cpp
security/pkix/test/gtest/pkixcert_extension_tests.cpp
security/pkix/test/gtest/pkixcert_signature_algorithm_tests.cpp
security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp
security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp
security/pkix/test/lib/pkixtestnss.cpp
security/pkix/test/lib/pkixtestutil.h
--- a/security/apps/AppTrustDomain.cpp
+++ b/security/apps/AppTrustDomain.cpp
@@ -212,25 +212,24 @@ AppTrustDomain::GetCertTrust(EndEntityOr
   trustLevel = TrustLevel::InheritsTrust;
   return Success;
 }
 
 Result
 AppTrustDomain::VerifySignedData(const SignedDataWithSignature& signedData,
                                  Input subjectPublicKeyInfo)
 {
-  return ::mozilla::pkix::VerifySignedDataNSS(signedData, subjectPublicKeyInfo,
-                                              mMinimumNonECCBits, mPinArg);
+  return VerifySignedDataNSS(signedData, subjectPublicKeyInfo, mPinArg);
 }
 
 Result
 AppTrustDomain::DigestBuf(Input item, /*out*/ uint8_t* digestBuf,
                           size_t digestBufLen)
 {
-  return ::mozilla::pkix::DigestBufNSS(item, digestBuf, digestBufLen);
+  return DigestBufNSS(item, digestBuf, digestBufLen);
 }
 
 Result
 AppTrustDomain::CheckRevocation(EndEntityOrCA, const CertID&, Time,
                                 /*optional*/ const Input*,
                                 /*optional*/ const Input*)
 {
   // We don't currently do revocation checking. If we need to distrust an Apps
@@ -245,15 +244,32 @@ AppTrustDomain::IsChainValid(const DERAr
                                                             mCertChain);
   if (srv != SECSuccess) {
     return MapPRErrorCodeToResult(PR_GetError());
   }
   return Success;
 }
 
 Result
-AppTrustDomain::CheckPublicKey(Input subjectPublicKeyInfo)
+AppTrustDomain::CheckRSAPublicKeyModulusSizeInBits(
+  EndEntityOrCA /*endEntityOrCA*/, unsigned int modulusSizeInBits)
 {
-  return ::mozilla::pkix::CheckPublicKeyNSS(subjectPublicKeyInfo,
-                                            mMinimumNonECCBits);
+  if (modulusSizeInBits < mMinimumNonECCBits) {
+    return Result::ERROR_INADEQUATE_KEY_SIZE;
+  }
+  return Success;
+}
+
+Result
+AppTrustDomain::CheckECDSACurveIsAcceptable(EndEntityOrCA /*endEntityOrCA*/,
+                                            NamedCurve curve)
+{
+  switch (curve) {
+    case NamedCurve::secp256r1: // fall through
+    case NamedCurve::secp384r1: // fall through
+    case NamedCurve::secp521r1:
+      return Success;
+  }
+
+  return Result::ERROR_UNSUPPORTED_ELLIPTIC_CURVE;
 }
 
 } } // namespace mozilla::psm
--- a/security/apps/AppTrustDomain.h
+++ b/security/apps/AppTrustDomain.h
@@ -33,18 +33,22 @@ public:
                             mozilla::pkix::Time time) MOZ_OVERRIDE;
   virtual Result CheckRevocation(mozilla::pkix::EndEntityOrCA endEntityOrCA,
                                  const mozilla::pkix::CertID& certID,
                                  mozilla::pkix::Time time,
                     /*optional*/ const mozilla::pkix::Input* stapledOCSPresponse,
                     /*optional*/ const mozilla::pkix::Input* aiaExtension) MOZ_OVERRIDE;
   virtual Result IsChainValid(const mozilla::pkix::DERArray& certChain,
                               mozilla::pkix::Time time) MOZ_OVERRIDE;
-  virtual Result CheckPublicKey(mozilla::pkix::Input subjectPublicKeyInfo)
-                                MOZ_OVERRIDE;
+  virtual Result CheckRSAPublicKeyModulusSizeInBits(
+                   mozilla::pkix::EndEntityOrCA endEntityOrCA,
+                   unsigned int modulusSizeInBits) MOZ_OVERRIDE;
+  virtual Result CheckECDSACurveIsAcceptable(
+                   mozilla::pkix::EndEntityOrCA endEntityOrCA,
+                   mozilla::pkix::NamedCurve curve) MOZ_OVERRIDE;
   virtual Result VerifySignedData(
            const mozilla::pkix::SignedDataWithSignature& signedData,
            mozilla::pkix::Input subjectPublicKeyInfo) MOZ_OVERRIDE;
   virtual Result DigestBuf(mozilla::pkix::Input item,
                            /*out*/ uint8_t* digestBuf,
                            size_t digestBufLen) MOZ_OVERRIDE;
 
 private:
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -255,25 +255,24 @@ NSSCertDBTrustDomain::GetCertTrust(EndEn
   trustLevel = TrustLevel::InheritsTrust;
   return Success;
 }
 
 Result
 NSSCertDBTrustDomain::VerifySignedData(const SignedDataWithSignature& signedData,
                                        Input subjectPublicKeyInfo)
 {
-  return ::mozilla::pkix::VerifySignedDataNSS(signedData, subjectPublicKeyInfo,
-                                              mMinimumNonECCBits, mPinArg);
+  return VerifySignedDataNSS(signedData, subjectPublicKeyInfo, mPinArg);
 }
 
 Result
 NSSCertDBTrustDomain::DigestBuf(Input item,
                                 /*out*/ uint8_t* digestBuf, size_t digestBufLen)
 {
-  return ::mozilla::pkix::DigestBufNSS(item, digestBuf, digestBufLen);
+  return DigestBufNSS(item, digestBuf, digestBufLen);
 }
 
 
 static PRIntervalTime
 OCSPFetchingTypeToTimeoutTime(NSSCertDBTrustDomain::OCSPFetching ocspFetching)
 {
   switch (ocspFetching) {
     case NSSCertDBTrustDomain::FetchOCSPForDVSoftFail:
@@ -716,20 +715,37 @@ NSSCertDBTrustDomain::IsChainValid(const
   if (mBuiltChain) {
     *mBuiltChain = certList.forget();
   }
 
   return Success;
 }
 
 Result
-NSSCertDBTrustDomain::CheckPublicKey(Input subjectPublicKeyInfo)
+NSSCertDBTrustDomain::CheckRSAPublicKeyModulusSizeInBits(
+  EndEntityOrCA /*endEntityOrCA*/, unsigned int modulusSizeInBits)
 {
-  return ::mozilla::pkix::CheckPublicKeyNSS(subjectPublicKeyInfo,
-                                            mMinimumNonECCBits);
+  if (modulusSizeInBits < mMinimumNonECCBits) {
+    return Result::ERROR_INADEQUATE_KEY_SIZE;
+  }
+  return Success;
+}
+
+Result
+NSSCertDBTrustDomain::CheckECDSACurveIsAcceptable(
+  EndEntityOrCA /*endEntityOrCA*/, NamedCurve curve)
+{
+  switch (curve) {
+    case NamedCurve::secp256r1: // fall through
+    case NamedCurve::secp384r1: // fall through
+    case NamedCurve::secp521r1:
+      return Success;
+  }
+
+  return Result::ERROR_UNSUPPORTED_ELLIPTIC_CURVE;
 }
 
 namespace {
 
 static char*
 nss_addEscape(const char* string, char quote)
 {
   char* newString = 0;
--- a/security/certverifier/NSSCertDBTrustDomain.h
+++ b/security/certverifier/NSSCertDBTrustDomain.h
@@ -65,18 +65,23 @@ public:
                             mozilla::pkix::Time time) MOZ_OVERRIDE;
 
   virtual Result GetCertTrust(mozilla::pkix::EndEntityOrCA endEntityOrCA,
                               const mozilla::pkix::CertPolicyId& policy,
                               mozilla::pkix::Input candidateCertDER,
                               /*out*/ mozilla::pkix::TrustLevel& trustLevel)
                               MOZ_OVERRIDE;
 
-  virtual Result CheckPublicKey(mozilla::pkix::Input subjectPublicKeyInfo)
-                                MOZ_OVERRIDE;
+  virtual Result CheckRSAPublicKeyModulusSizeInBits(
+                   mozilla::pkix::EndEntityOrCA endEntityOrCA,
+                   unsigned int modulusSizeInBits) MOZ_OVERRIDE;
+
+  virtual Result CheckECDSACurveIsAcceptable(
+                   mozilla::pkix::EndEntityOrCA endEntityOrCA,
+                   mozilla::pkix::NamedCurve curve) MOZ_OVERRIDE;
 
   virtual Result VerifySignedData(
                    const mozilla::pkix::SignedDataWithSignature& signedData,
                    mozilla::pkix::Input subjectPublicKeyInfo)
                    MOZ_OVERRIDE;
 
   virtual Result DigestBuf(mozilla::pkix::Input item,
                            /*out*/ uint8_t* digestBuf,
--- a/security/manager/ssl/tests/unit/test_keysize_ev.js
+++ b/security/manager/ssl/tests/unit/test_keysize_ev.js
@@ -122,27 +122,28 @@ function checkRSAChains(inadequateKeySiz
   expectedNamesForOCSP = [ eeFullName ];
   addKeySizeTestForEV(expectedNamesForOCSP, rootNotOKName,
                       [ intFullName ], eeFullName, false);
 
   // Chain with an intermediate cert that has an inadequate size for EV, but
   // adequate size for DV
   intFullName = intNotOKName + "-" + rootOKName;
   eeFullName = eeOKName + "-" + intNotOKName + "-" + rootOKName;
-  expectedNamesForOCSP = gEVExpected
-                       ? [ intFullName ]
-                       : [ eeFullName ];
+  expectedNamesForOCSP = [ eeFullName ];
   addKeySizeTestForEV(expectedNamesForOCSP, rootOKCertFileName,
                       [ intFullName ], eeFullName, false);
 
   // Chain with an end entity cert that has an inadequate size for EV, but
   // adequate size for DV
   intFullName = intOKName + "-" + rootOKName;
   eeFullName = eeNotOKName + "-" + intOKName + "-" + rootOKName;
-  expectedNamesForOCSP = [ eeFullName ];
+  expectedNamesForOCSP = gEVExpected
+                       ? [ intFullName,
+                           eeFullName ]
+                       : [ eeFullName ];
   addKeySizeTestForEV(expectedNamesForOCSP, rootOKCertFileName,
                       [ intFullName ], eeFullName, false);
 }
 
 function run_test() {
   Services.prefs.setCharPref("network.dns.localDomains", "www.example.com");
 
   checkRSAChains(2040, 2048);
--- a/security/manager/ssl/tests/unit/test_ocsp_stapling.js
+++ b/security/manager/ssl/tests/unit/test_ocsp_stapling.js
@@ -136,17 +136,17 @@ function add_tests(certDB, otherTestCA) 
 
   // ocsp-stapling-expired.example.com and
   // ocsp-stapling-expired-fresh-ca.example.com are handled in
   // test_ocsp_stapling_expired.js
 
   // Check that OCSP responder certificates with key sizes below 1024 bits are
   // rejected, even when the main certificate chain keys are at least 1024 bits.
   add_ocsp_test("keysize-ocsp-delegated.example.com",
-                getXPCOMStatusFromNSS(MOZILLA_PKIX_ERROR_INADEQUATE_KEY_SIZE),
+                getXPCOMStatusFromNSS(SEC_ERROR_OCSP_INVALID_SIGNING_CERT),
                 true);
 
   add_ocsp_test("revoked-ca-cert-used-as-end-entity.example.com",
                 getXPCOMStatusFromNSS(SEC_ERROR_REVOKED_CERTIFICATE), true);
 }
 
 function check_ocsp_stapling_telemetry() {
   let histogram = Cc["@mozilla.org/base/telemetry;1"]
--- a/security/pkix/include/pkix/Result.h
+++ b/security/pkix/include/pkix/Result.h
@@ -172,16 +172,18 @@ static const unsigned int FATAL_ERROR_FL
     MOZILLA_PKIX_MAP(ERROR_NO_RFC822NAME_MATCH, 43, \
                      MOZILLA_PKIX_ERROR_NO_RFC822NAME_MATCH) \
     MOZILLA_PKIX_MAP(ERROR_UNSUPPORTED_ELLIPTIC_CURVE, 44, \
                      SEC_ERROR_UNSUPPORTED_ELLIPTIC_CURVE) \
     MOZILLA_PKIX_MAP(ERROR_NOT_YET_VALID_CERTIFICATE, 45, \
                      MOZILLA_PKIX_ERROR_NOT_YET_VALID_CERTIFICATE) \
     MOZILLA_PKIX_MAP(ERROR_NOT_YET_VALID_ISSUER_CERTIFICATE, 46, \
                      MOZILLA_PKIX_ERROR_NOT_YET_VALID_ISSUER_CERTIFICATE) \
+    MOZILLA_PKIX_MAP(ERROR_UNSUPPORTED_EC_POINT_FORM, 47, \
+                     SEC_ERROR_UNSUPPORTED_EC_POINT_FORM) \
     MOZILLA_PKIX_MAP(FATAL_ERROR_INVALID_ARGS, FATAL_ERROR_FLAG | 1, \
                      SEC_ERROR_INVALID_ARGS) \
     MOZILLA_PKIX_MAP(FATAL_ERROR_INVALID_STATE, FATAL_ERROR_FLAG | 2, \
                      PR_INVALID_STATE_ERROR) \
     MOZILLA_PKIX_MAP(FATAL_ERROR_LIBRARY_FAILURE, FATAL_ERROR_FLAG | 3, \
                      SEC_ERROR_LIBRARY_FAILURE) \
     MOZILLA_PKIX_MAP(FATAL_ERROR_NO_MEMORY, FATAL_ERROR_FLAG | 4, \
                      SEC_ERROR_NO_MEMORY) \
--- a/security/pkix/include/pkix/pkixnss.h
+++ b/security/pkix/include/pkix/pkixnss.h
@@ -29,39 +29,32 @@
 #include "prerror.h"
 #include "seccomon.h"
 
 namespace mozilla { namespace pkix {
 
 // Verify the given signed data using the given public key.
 Result VerifySignedDataNSS(const SignedDataWithSignature& sd,
                            Input subjectPublicKeyInfo,
-                           unsigned int minimumNonECCBits,
                            void* pkcs11PinArg);
 
 // Computes the SHA-1 hash of the data in the current item.
 //
 // item contains the data to hash.
 // digestBuf must point to a buffer to where the SHA-1 hash will be written.
 // digestBufLen must be 20 (the length of a SHA-1 hash,
 //              TrustDomain::DIGEST_LENGTH).
 //
 // TODO(bug 966856): Add SHA-2 support
 // TODO: Taking the output buffer as (uint8_t*, size_t) is counter to our
 // other, extensive, memory safety efforts in mozilla::pkix, and we should find
 // a way to provide a more-obviously-safe interface.
 Result DigestBufNSS(Input item, /*out*/ uint8_t* digestBuf,
                     size_t digestBufLen);
 
-// Checks, for RSA keys, that the modulus is at least the given number of bits.
-// Checks, for ECC keys, that the curve used is one of the NIST P-256, P-384,
-// or P-521 curves.
-Result CheckPublicKeyNSS(Input subjectPublicKeyInfo,
-                         unsigned int minimumNonECCBits);
-
 Result MapPRErrorCodeToResult(PRErrorCode errorCode);
 PRErrorCode MapResultToPRErrorCode(Result result);
 
 // The error codes within each module must fit in 16 bits. We want these
 // errors to fit in the same module as the NSS errors but not overlap with
 // any of them. Converting an NSS SEC, NSS SSL, or PSM error to an NS error
 // involves negating the value of the error and then synthesizing an error
 // in the NS_ERROR_MODULE_SECURITY module. Hence, PSM errors will start at
--- a/security/pkix/include/pkix/pkixtypes.h
+++ b/security/pkix/include/pkix/pkixtypes.h
@@ -297,31 +297,43 @@ public:
   // certChain.GetDER(0) is the trust anchor.
   virtual Result IsChainValid(const DERArray& certChain, Time time) = 0;
 
   virtual Result CheckRevocation(EndEntityOrCA endEntityOrCA,
                                  const CertID& certID, Time time,
                     /*optional*/ const Input* stapledOCSPresponse,
                     /*optional*/ const Input* aiaExtension) = 0;
 
-  // Check that the key size, algorithm, elliptic curve used (if applicable),
-  // and parameters of the given public key are acceptable.
+  // Check that the RSA public key size is acceptable.
   //
-  // VerifySignedData() should do the same checks that this function does, but
-  // mainly for efficiency, some keys are not passed to VerifySignedData().
-  // This function is called instead for those keys.
-  virtual Result CheckPublicKey(Input subjectPublicKeyInfo) = 0;
+  // Return Success if the key size is acceptable,
+  // Result::ERROR_INADEQUATE_KEY_SIZE if the key size is not acceptable,
+  // or another error code if another error occurred.
+  virtual Result CheckRSAPublicKeyModulusSizeInBits(
+                   EndEntityOrCA endEntityOrCA,
+                   unsigned int modulusSizeInBits) = 0;
+
+  // Check that the given named ECC curve is acceptable for ECDSA signatures.
+  //
+  // Return Success if the curve is acceptable,
+  // Result::ERROR_UNSUPPORTED_ELLIPTIC_CURVE if the curve is not acceptable,
+  // or another error code if another error occurred.
+  virtual Result CheckECDSACurveIsAcceptable(EndEntityOrCA endEntityOrCA,
+                                             NamedCurve curve) = 0;
 
   // Verify the given signature using the given public key.
   //
   // Most implementations of this function should probably forward the call
   // directly to mozilla::pkix::VerifySignedData.
   //
-  // In any case, the implementation must perform checks on the public key
-  // similar to how mozilla::pkix::CheckPublicKey() does.
+  // CheckRSAPublicKeyModulusSizeInBits or CheckECDSACurveIsAcceptable will
+  // be called before calling this function, so it is not necessary to repeat
+  // those checks in VerifySignedData. However, VerifySignedData *is*
+  // responsible for doing the mathematical verification of the public key
+  // validity as specified in NIST SP 800-56A.
   virtual Result VerifySignedData(const SignedDataWithSignature& signedData,
                                   Input subjectPublicKeyInfo) = 0;
 
   // Compute the SHA-1 hash of the data in the current item.
   //
   // item contains the data to hash.
   // digestBuf must point to a buffer to where the SHA-1 hash will be written.
   // digestBufLen must be DIGEST_LENGTH (20, the length of a SHA-1 hash).
--- a/security/pkix/lib/pkixbuild.cpp
+++ b/security/pkix/lib/pkixbuild.cpp
@@ -322,22 +322,14 @@ BuildCertChain(TrustDomain& trustDomain,
   // XXX: Support the legacy use of the subject CN field for indicating the
   // domain name the certificate is valid for.
   BackCert cert(certDER, endEntityOrCA, nullptr);
   Result rv = cert.Init();
   if (rv != Success) {
     return rv;
   }
 
-  // See documentation for CheckPublicKey() in pkixtypes.h for why the public
-  // key also needs to be checked here when trustDomain.VerifySignedData()
-  // should already be doing it.
-  rv = trustDomain.CheckPublicKey(cert.GetSubjectPublicKeyInfo());
-  if (rv != Success) {
-    return rv;
-  }
-
   return BuildForward(trustDomain, cert, time, requiredKeyUsageIfPresent,
                       requiredEKUIfPresent, requiredPolicy, stapledOCSPResponse,
                       0/*subCACount*/);
 }
 
 } } // namespace mozilla::pkix
--- a/security/pkix/lib/pkixcert.cpp
+++ b/security/pkix/lib/pkixcert.cpp
@@ -98,23 +98,16 @@ BackCert::Init()
   // TODO(bug XXXXXXX): We rely on the the caller of mozilla::pkix to validate
   // that the name is syntactically valid, if they care. In Gecko we do this
   // implicitly by parsing the certificate into a CERTCertificate object.
   // Instead of relying on the caller to do this, we should do it ourselves.
   rv = der::ExpectTagAndGetTLV(tbsCertificate, der::SEQUENCE, subject);
   if (rv != Success) {
     return rv;
   }
-  // TODO(bug XXXXXXX): We defer parsing/validating subjectPublicKeyInfo to
-  // the point where the public key is needed. For end-entity certificates, we
-  // assume that the caller will extract the public key and use it somehow; if
-  // they don't do that then we'll never know whether the key is invalid. On
-  // the other hand, if the caller never uses the key then in some ways it
-  // doesn't matter. Regardless, we should parse and validate
-  // subjectPublicKeyKeyInfo internally.
   rv = der::ExpectTagAndGetTLV(tbsCertificate, der::SEQUENCE,
                                subjectPublicKeyInfo);
   if (rv != Success) {
     return rv;
   }
 
   static const uint8_t CSC = der::CONTEXT_SPECIFIC | der::CONSTRUCTED;
 
--- a/security/pkix/lib/pkixcheck.cpp
+++ b/security/pkix/lib/pkixcheck.cpp
@@ -24,16 +24,18 @@
 
 #include "pkixcheck.h"
 
 #include "pkixder.h"
 #include "pkixutil.h"
 
 namespace mozilla { namespace pkix {
 
+// 4.1.2.5 Validity
+
 Result
 CheckValidity(Input encodedValidity, Time time)
 {
   Reader validity(encodedValidity);
   Time notBefore(Time::uninitialized);
   if (der::TimeChoice(validity, notBefore) != Success) {
     return Result::ERROR_EXPIRED_CERTIFICATE;
   }
@@ -47,16 +49,197 @@ CheckValidity(Input encodedValidity, Tim
   }
   if (time > notAfter) {
     return Result::ERROR_EXPIRED_CERTIFICATE;
   }
 
   return der::End(validity);
 }
 
+// 4.1.2.7 Subject Public Key Info
+
+Result
+CheckSubjectPublicKeyInfo(Reader& input, TrustDomain& trustDomain,
+                          EndEntityOrCA endEntityOrCA)
+{
+  // Here, we validate the syntax and do very basic semantic validation of the
+  // public key of the certificate. The intention here is to filter out the
+  // types of bad inputs that are most likely to trigger non-mathematical
+  // security vulnerabilities in the TrustDomain, like buffer overflows or the
+  // use of unsafe elliptic curves.
+  //
+  // We don't check (all of) the mathematical properties of the public key here
+  // because it is more efficient for the TrustDomain to do it during signature
+  // verification and/or other use of the public key. In particular, we
+  // delegate the arithmetic validation of the public key, as specified in
+  // NIST SP800-56A section 5.6.2, to the TrustDomain, at least for now.
+
+  Reader algorithm;
+  Input subjectPublicKey;
+  Result rv = der::ExpectTagAndGetValue(input, der::SEQUENCE, algorithm);
+  if (rv != Success) {
+    return rv;
+  }
+  rv = der::BitStringWithNoUnusedBits(input, subjectPublicKey);
+  if (rv != Success) {
+    return rv;
+  }
+  rv = der::End(input);
+  if (rv != Success) {
+    return rv;
+  }
+
+  Reader subjectPublicKeyReader(subjectPublicKey);
+
+  Reader algorithmOID;
+  rv = der::ExpectTagAndGetValue(algorithm, der::OIDTag, algorithmOID);
+  if (rv != Success) {
+    return rv;
+  }
+
+  // RFC 3279 Section 2.3.1
+  // python DottedOIDToCode.py rsaEncryption 1.2.840.113549.1.1.1
+  static const uint8_t rsaEncryption[] = {
+    0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x01
+  };
+
+  // RFC 3279 Section 2.3.5 and RFC 5480 Section 2.1.1
+  // python DottedOIDToCode.py id-ecPublicKey 1.2.840.10045.2.1
+  static const uint8_t id_ecPublicKey[] = {
+    0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, 0x01
+  };
+
+  if (algorithmOID.MatchRest(id_ecPublicKey)) {
+    // An id-ecPublicKey AlgorithmIdentifier has a parameter that identifes
+    // the curve being used. Although RFC 5480 specifies multiple forms, we
+    // only supported the NamedCurve form, where the curve is identified by an
+    // OID.
+
+    Reader namedCurveOIDValue;
+    rv = der::ExpectTagAndGetValue(algorithm, der::OIDTag,
+                                   namedCurveOIDValue);
+    if (rv != Success) {
+      return rv;
+    }
+
+    // RFC 5480
+    // python DottedOIDToCode.py secp256r1 1.2.840.10045.3.1.7
+    static const uint8_t secp256r1[] = {
+      0x2a, 0x86, 0x48, 0xce, 0x3d, 0x03, 0x01, 0x07
+    };
+
+    // RFC 5480
+    // python DottedOIDToCode.py secp384r1 1.3.132.0.34
+    static const uint8_t secp384r1[] = {
+      0x2b, 0x81, 0x04, 0x00, 0x22
+    };
+
+    // RFC 5480
+    // python DottedOIDToCode.py secp521r1 1.3.132.0.35
+    static const uint8_t secp521r1[] = {
+      0x2b, 0x81, 0x04, 0x00, 0x23
+    };
+
+    // Matching is attempted based on a rough estimate of the commonality of the
+    // elliptic curve, to minimize the number of MatchRest calls.
+    NamedCurve curve;
+    unsigned int bits;
+    if (namedCurveOIDValue.MatchRest(secp256r1)) {
+      curve = NamedCurve::secp256r1;
+      bits = 256;
+    } else if (namedCurveOIDValue.MatchRest(secp384r1)) {
+      curve = NamedCurve::secp384r1;
+      bits = 384;
+    } else if (namedCurveOIDValue.MatchRest(secp521r1)) {
+      curve = NamedCurve::secp521r1;
+      bits = 521;
+    } else {
+      return Result::ERROR_UNSUPPORTED_ELLIPTIC_CURVE;
+    }
+
+    rv = trustDomain.CheckECDSACurveIsAcceptable(endEntityOrCA, curve);
+    if (rv != Success) {
+      return rv;
+    }
+
+    // RFC 5480 Section 2.2 says that the first octet will be 0x04 to indicate
+    // an uncompressed point, which is the only encoding we support.
+    uint8_t compressedOrUncompressed;
+    rv = subjectPublicKeyReader.Read(compressedOrUncompressed);
+    if (rv != Success) {
+      return rv;
+    }
+    if (compressedOrUncompressed != 0x04) {
+      return Result::ERROR_UNSUPPORTED_EC_POINT_FORM;
+    }
+
+    // The point is encoded as two raw (not DER-encoded) integers, each padded
+    // to the bit length (rounded up to the nearest byte).
+    Input point;
+    rv = subjectPublicKeyReader.SkipToEnd(point);
+    if (rv != Success) {
+      return rv;
+    }
+    if (point.GetLength() != ((bits + 7) / 8u) * 2u) {
+      return Result::ERROR_BAD_DER;
+    }
+
+    // XXX: We defer the mathematical verification of the validity of the point
+    // until signature verification. This means that if we never verify a
+    // signature, we'll never fully check whether the public key is valid.
+  } else if (algorithmOID.MatchRest(rsaEncryption)) {
+    // RFC 3279 Section 2.3.1 says "The parameters field MUST have ASN.1 type
+    // NULL for this algorithm identifier."
+    rv = der::ExpectTagAndEmptyValue(algorithm, der::NULLTag);
+    if (rv != Success) {
+      return rv;
+    }
+
+    // RSAPublicKey :: = SEQUENCE{
+    //    modulus            INTEGER,    --n
+    //    publicExponent     INTEGER  }  --e
+    rv = der::Nested(subjectPublicKeyReader, der::SEQUENCE,
+                     [&trustDomain, endEntityOrCA](Reader& r) {
+      Input modulus;
+      Input::size_type modulusSignificantBytes;
+      Result rv = der::PositiveInteger(r, modulus, &modulusSignificantBytes);
+      if (rv != Success) {
+        return rv;
+      }
+      // XXX: Should we do additional checks of the modulus?
+      rv = trustDomain.CheckRSAPublicKeyModulusSizeInBits(
+             endEntityOrCA, modulusSignificantBytes * 8u);
+      if (rv != Success) {
+        return rv;
+      }
+
+      // XXX: We don't allow the TrustDomain to validate the exponent.
+      // XXX: We don't do our own sanity checking of the exponent.
+      Input exponent;
+      return der::PositiveInteger(r, exponent);
+    });
+    if (rv != Success) {
+      return rv;
+    }
+  } else {
+    return Result::ERROR_UNSUPPORTED_KEYALG;
+  }
+
+  rv = der::End(algorithm);
+  if (rv != Success) {
+    return rv;
+  }
+  rv = der::End(subjectPublicKeyReader);
+  if (rv != Success) {
+    return rv;
+  }
+
+  return Success;
+}
+
 // 4.2.1.3. Key Usage (id-ce-keyUsage)
 
 // As explained in the comment in CheckKeyUsage, bit 0 is the most significant
 // bit and bit 7 is the least significant bit.
 inline uint8_t KeyUsageToBitMask(KeyUsage keyUsage)
 {
   assert(keyUsage != KeyUsage::noParticularKeyUsageRequired);
   return 0x80u >> static_cast<uint8_t>(keyUsage);
@@ -551,16 +734,31 @@ CheckIssuerIndependentProperties(TrustDo
     return Result::ERROR_UNTRUSTED_CERT;
   }
   if (trustLevel != TrustLevel::TrustAnchor &&
       trustLevel != TrustLevel::InheritsTrust) {
     // The TrustDomain returned a trust level that we weren't expecting.
     return Result::FATAL_ERROR_INVALID_STATE;
   }
 
+  // Check the SPKI first, because it is one of the most selective properties
+  // of the certificate due to SHA-1 deprecation and the deprecation of
+  // certificates with keys weaker than RSA 2048.
+  Reader spki(cert.GetSubjectPublicKeyInfo());
+  rv = der::Nested(spki, der::SEQUENCE, [&](Reader& r) {
+    return CheckSubjectPublicKeyInfo(r, trustDomain, endEntityOrCA);
+  });
+  if (rv != Success) {
+    return rv;
+  }
+  rv = der::End(spki);
+  if (rv != Success) {
+    return rv;
+  }
+
   // 4.2.1.1. Authority Key Identifier is ignored (see bug 965136).
 
   // 4.2.1.2. Subject Key Identifier is ignored (see bug 965136).
 
   // 4.2.1.3. Key Usage
   rv = CheckKeyUsage(endEntityOrCA, cert.GetKeyUsage(),
                      requiredKeyUsageIfPresent);
   if (rv != Success) {
--- a/security/pkix/lib/pkixder.cpp
+++ b/security/pkix/lib/pkixder.cpp
@@ -218,52 +218,16 @@ SignatureAlgorithmOIDValue(Reader& algor
     algorithm = SignatureAlgorithm::rsa_pkcs1_with_sha1;
   } else {
     algorithm = SignatureAlgorithm::unsupported_algorithm;
   }
 
   return Success;
 }
 
-static Result
-NamedCurveOIDValue(Reader& namedCurveID, /*out*/ NamedCurve& namedCurve)
-{
-  // RFC 5480
-  // python DottedOIDToCode.py secp256r1 1.2.840.10045.3.1.7
-  static const uint8_t secp256r1[] = {
-    0x2a, 0x86, 0x48, 0xce, 0x3d, 0x03, 0x01, 0x07
-  };
-
-  // RFC 5480
-  // python DottedOIDToCode.py id-secp384r1 1.3.132.0.34
-  static const uint8_t secp384r1[] = {
-    0x2b, 0x81, 0x04, 0x00, 0x22
-  };
-
-  // RFC 5480
-  // python DottedOIDToCode.py id-secp521r1 1.3.132.0.35
-  static const uint8_t secp521r1[] = {
-    0x2b, 0x81, 0x04, 0x00, 0x23
-  };
-
-  // Matching is attempted based on a rough estimate of the commonality of the
-  // named curve, to minimize the number of MatchRest calls.
-  if (namedCurveID.MatchRest(secp256r1)) {
-    namedCurve = NamedCurve::secp256r1;
-  } else if (namedCurveID.MatchRest(secp384r1)) {
-    namedCurve = NamedCurve::secp384r1;
-  } else if (namedCurveID.MatchRest(secp521r1)) {
-    namedCurve = NamedCurve::secp521r1;
-  } else {
-    return Result::ERROR_UNSUPPORTED_ELLIPTIC_CURVE;
-  }
-
-  return Success;
-}
-
 template <typename OidValueParser, typename Algorithm>
 Result
 AlgorithmIdentifier(OidValueParser oidValueParser, Reader& input,
                     /*out*/ Algorithm& algorithm)
 {
   Reader value;
   Result rv = ExpectTagAndGetValue(input, SEQUENCE, value);
   if (rv != Success) {
@@ -299,33 +263,16 @@ SignatureAlgorithmIdentifier(Reader& inp
 
 Result
 DigestAlgorithmIdentifier(Reader& input, /*out*/ DigestAlgorithm& algorithm)
 {
   return AlgorithmIdentifier(DigestAlgorithmOIDValue, input, algorithm);
 }
 
 Result
-NamedCurveOID(Reader& input, /*out*/ NamedCurve& namedCurve)
-{
-  Reader namedCurveID;
-  Result rv = ExpectTagAndGetValue(input, der::OIDTag, namedCurveID);
-  if (rv != Success) {
-    return rv;
-  }
-
-  rv = NamedCurveOIDValue(namedCurveID, namedCurve);
-  if (rv != Success) {
-    return rv;
-  }
-
-  return Success;
-}
-
-Result
 SignedData(Reader& input, /*out*/ Reader& tbs,
            /*out*/ SignedDataWithSignature& signedData)
 {
   Reader::Mark mark(input.GetMark());
 
   Result rv;
   rv = ExpectTagAndGetValue(input, SEQUENCE, tbs);
   if (rv != Success) {
--- a/security/pkix/lib/pkixder.h
+++ b/security/pkix/lib/pkixder.h
@@ -598,18 +598,16 @@ OptionalExtensions(Reader& input, uint8_
 }
 
 Result DigestAlgorithmIdentifier(Reader& input,
                                  /*out*/ DigestAlgorithm& algorithm);
 
 Result SignatureAlgorithmIdentifier(Reader& input,
                                     /*out*/ SignatureAlgorithm& algorithm);
 
-Result NamedCurveOID(Reader& input, /*out*/ NamedCurve& namedCurve);
-
 // Parses a SEQUENCE into tbs and then parses an AlgorithmIdentifier followed
 // by a BIT STRING into signedData. This handles the commonality between
 // parsing the signed/signature fields of certificates and OCSP responses. In
 // the case of an OCSP response, the caller needs to parse the certs
 // separately.
 //
 // Certificate  ::=  SEQUENCE  {
 //        tbsCertificate       TBSCertificate,
--- a/security/pkix/lib/pkixnss.cpp
+++ b/security/pkix/lib/pkixnss.cpp
@@ -21,124 +21,30 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
 
 #include "pkix/pkixnss.h"
 
 #include <limits>
 
-#include "cert.h"
 #include "cryptohi.h"
 #include "keyhi.h"
 #include "pk11pub.h"
 #include "pkix/pkix.h"
 #include "pkix/ScopedPtr.h"
-#include "pkixder.h"
 #include "pkixutil.h"
 #include "secerr.h"
 #include "sslerr.h"
 
 namespace mozilla { namespace pkix {
 
-typedef ScopedPtr<SECKEYPublicKey, SECKEY_DestroyPublicKey> ScopedSECKeyPublicKey;
-
-static Result
-CheckPublicKeySize(Input subjectPublicKeyInfo, unsigned int minimumNonECCBits,
-                   /*out*/ ScopedSECKeyPublicKey& publicKey)
-{
-  SECItem subjectPublicKeyInfoSECItem =
-    UnsafeMapInputToSECItem(subjectPublicKeyInfo);
-  ScopedPtr<CERTSubjectPublicKeyInfo, SECKEY_DestroySubjectPublicKeyInfo>
-    spki(SECKEY_DecodeDERSubjectPublicKeyInfo(&subjectPublicKeyInfoSECItem));
-  if (!spki) {
-    return MapPRErrorCodeToResult(PR_GetError());
-  }
-  publicKey = SECKEY_ExtractPublicKey(spki.get());
-  if (!publicKey) {
-    return MapPRErrorCodeToResult(PR_GetError());
-  }
-
-  // Some compilers complain if if we don't explicitly list every case. That is
-  // usually what we want, but in this case we really want to support an
-  // open-ended set of key types that might be expanded by future NSS versions.
-#if defined(__clang__)
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wswitch-enum"
-#elif defined(_MSC_VER)
-#pragma warning(push)
-#pragma warning(disable: 4061)
-#endif
-
-  switch (publicKey.get()->keyType) {
-    case ecKey:
-    {
-      SECKEYECParams* encodedParams = &publicKey.get()->u.ec.DEREncodedParams;
-      if (!encodedParams) {
-        return Result::ERROR_UNSUPPORTED_ELLIPTIC_CURVE;
-      }
-
-      Input input;
-      Result rv = input.Init(encodedParams->data, encodedParams->len);
-      if (rv != Success) {
-        return rv;
-      }
-
-      Reader reader(input);
-      NamedCurve namedCurve;
-      rv = der::NamedCurveOID(reader, namedCurve);
-      if (rv != Success) {
-        return rv;
-      }
-
-      rv = der::End(reader);
-      if (rv != Success) {
-        return rv;
-      }
-
-      switch (namedCurve) {
-        case NamedCurve::secp256r1: // fall through
-        case NamedCurve::secp384r1: // fall through
-        case NamedCurve::secp521r1:
-          break;
-      }
-
-      return Success;
-    }
-
-    case rsaKey:
-      if (SECKEY_PublicKeyStrengthInBits(publicKey.get()) < minimumNonECCBits) {
-        return Result::ERROR_INADEQUATE_KEY_SIZE;
-      }
-      break;
-
-    default:
-      return Result::ERROR_UNSUPPORTED_KEYALG;
-  }
-
-#if defined(__clang__)
-#pragma clang diagnostic pop
-#elif defined(_MSC_VER)
-#pragma warning(pop)
-#endif
-
-  return Success;
-}
-
-Result
-CheckPublicKeyNSS(Input subjectPublicKeyInfo, unsigned int minimumNonECCBits)
-{
-  ScopedSECKeyPublicKey unused;
-  return CheckPublicKeySize(subjectPublicKeyInfo, minimumNonECCBits, unused);
-}
-
 Result
 VerifySignedDataNSS(const SignedDataWithSignature& sd,
-                    Input subjectPublicKeyInfo, unsigned int minimumNonECCBits,
-                    void* pkcs11PinArg)
+                    Input subjectPublicKeyInfo, void* pkcs11PinArg)
 {
   SECOidTag pubKeyAlg;
   SECOidTag digestAlg;
   switch (sd.algorithm) {
     case SignatureAlgorithm::ecdsa_with_sha512:
       pubKeyAlg = SEC_OID_ANSIX962_EC_PUBLIC_KEY;
       digestAlg = SEC_OID_SHA512;
       break;
@@ -171,21 +77,27 @@ VerifySignedDataNSS(const SignedDataWith
       digestAlg = SEC_OID_SHA1;
       break;
     case SignatureAlgorithm::unsupported_algorithm: // fall through
       return NotReached("unknown signature algorithm",
                         Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED);
     MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM
   }
 
-  Result rv;
-  ScopedSECKeyPublicKey pubKey;
-  rv = CheckPublicKeySize(subjectPublicKeyInfo, minimumNonECCBits, pubKey);
-  if (rv != Success) {
-    return rv;
+  SECItem subjectPublicKeyInfoSECItem =
+    UnsafeMapInputToSECItem(subjectPublicKeyInfo);
+  ScopedPtr<CERTSubjectPublicKeyInfo, SECKEY_DestroySubjectPublicKeyInfo>
+    spki(SECKEY_DecodeDERSubjectPublicKeyInfo(&subjectPublicKeyInfoSECItem));
+  if (!spki) {
+    return MapPRErrorCodeToResult(PR_GetError());
+  }
+  ScopedPtr<SECKEYPublicKey, SECKEY_DestroyPublicKey>
+    pubKey(SECKEY_ExtractPublicKey(spki.get()));
+  if (!pubKey) {
+    return MapPRErrorCodeToResult(PR_GetError());
   }
 
   // The static_cast is safe as long as the length of the data in sd.data can
   // fit in an int. Right now that length is stored as a uint16_t, so this
   // works. In the future this may change, hence the assertion.
   // See also bug 921585.
   static_assert(sizeof(decltype(sd.data.GetLength())) < sizeof(int),
                 "sd.data.GetLength() must fit in an int");
--- a/security/pkix/test/gtest/pkixbuild_tests.cpp
+++ b/security/pkix/test/gtest/pkixbuild_tests.cpp
@@ -160,21 +160,28 @@ private:
   }
 
   Result DigestBuf(Input, /*out*/ uint8_t*, size_t) override
   {
     ADD_FAILURE();
     return Result::FATAL_ERROR_LIBRARY_FAILURE;
   }
 
-  Result CheckPublicKey(Input subjectPublicKeyInfo) override
+  Result CheckRSAPublicKeyModulusSizeInBits(EndEntityOrCA, unsigned int)
+                                            override
   {
-    return TestCheckPublicKey(subjectPublicKeyInfo);
+    return Success;
   }
 
+  Result CheckECDSACurveIsAcceptable(EndEntityOrCA, NamedCurve) override
+  {
+    return Success;
+  }
+
+
   std::map<ByteString, ByteString> subjectDERToCertDER;
   ByteString leafCACertDER;
   ByteString rootCACertDER;
 };
 
 class pkixbuild : public ::testing::Test
 {
 public:
@@ -322,19 +329,25 @@ public:
   }
 
   Result DigestBuf(Input, /*out*/uint8_t*, size_t) override
   {
     ADD_FAILURE();
     return Result::FATAL_ERROR_LIBRARY_FAILURE;
   }
 
-  Result CheckPublicKey(Input subjectPublicKeyInfo) override
+  Result CheckRSAPublicKeyModulusSizeInBits(EndEntityOrCA, unsigned int)
+                                            override
   {
-    return TestCheckPublicKey(subjectPublicKeyInfo);
+    return Success;
+  }
+
+  Result CheckECDSACurveIsAcceptable(EndEntityOrCA, NamedCurve) override
+  {
+    return Success;
   }
 
 private:
   ByteString rootDER;
 };
 
 TEST_F(pkixbuild, NoRevocationCheckingForExpiredCert)
 {
@@ -405,19 +418,25 @@ public:
   }
 
   Result DigestBuf(Input, /*out*/uint8_t*, size_t) override
   {
     ADD_FAILURE();
     return Result::FATAL_ERROR_LIBRARY_FAILURE;
   }
 
-  Result CheckPublicKey(Input subjectPublicKeyInfo) override
+  Result CheckRSAPublicKeyModulusSizeInBits(EndEntityOrCA, unsigned int)
+                                            override
   {
-    return TestCheckPublicKey(subjectPublicKeyInfo);
+    return Success;
+  }
+
+  Result CheckECDSACurveIsAcceptable(EndEntityOrCA, NamedCurve) override
+  {
+    return Success;
   }
 };
 
 class pkixbuild_DSS : public ::testing::Test { };
 
 TEST_F(pkixbuild_DSS, DSSEndEntityKeyNotAccepted)
 {
   DSSTrustDomain trustDomain;
@@ -502,19 +521,25 @@ public:
   }
 
   Result DigestBuf(Input, /*out*/uint8_t*, size_t) override
   {
     ADD_FAILURE();
     return Result::FATAL_ERROR_LIBRARY_FAILURE;
   }
 
-  Result CheckPublicKey(Input subjectPublicKeyInfo) override
+  Result CheckRSAPublicKeyModulusSizeInBits(EndEntityOrCA, unsigned int)
+                                            override
   {
-    return TestCheckPublicKey(subjectPublicKeyInfo);
+    return Success;
+  }
+
+  Result CheckECDSACurveIsAcceptable(EndEntityOrCA, NamedCurve) override
+  {
+    return Success;
   }
 
 private:
   const ByteString issuer;
   const bool expectedKeepGoing;
 };
 
 struct IssuerNameCheckParams
--- a/security/pkix/test/gtest/pkixcert_extension_tests.cpp
+++ b/security/pkix/test/gtest/pkixcert_extension_tests.cpp
@@ -96,19 +96,25 @@ private:
   }
 
   Result DigestBuf(Input, /*out*/ uint8_t*, size_t) override
   {
     ADD_FAILURE();
     return Result::FATAL_ERROR_LIBRARY_FAILURE;
   }
 
-  Result CheckPublicKey(Input subjectPublicKeyInfo) override
+  Result CheckRSAPublicKeyModulusSizeInBits(EndEntityOrCA, unsigned int)
+                                            override
   {
-    return TestCheckPublicKey(subjectPublicKeyInfo);
+    return Success;
+  }
+
+  Result CheckECDSACurveIsAcceptable(EndEntityOrCA, NamedCurve) override
+  {
+    return Success;
   }
 };
 
 // python DottedOIDToCode.py --tlv unknownExtensionOID 1.3.6.1.4.1.13769.666.666.666.1.500.9.3
 static const uint8_t tlv_unknownExtensionOID[] = {
   0x06, 0x12, 0x2b, 0x06, 0x01, 0x04, 0x01, 0xeb, 0x49, 0x85, 0x1a, 0x85, 0x1a,
   0x85, 0x1a, 0x01, 0x83, 0x74, 0x09, 0x03
 };
--- a/security/pkix/test/gtest/pkixcert_signature_algorithm_tests.cpp
+++ b/security/pkix/test/gtest/pkixcert_signature_algorithm_tests.cpp
@@ -111,19 +111,25 @@ private:
   }
 
   Result DigestBuf(Input, uint8_t*, size_t) override
   {
     ADD_FAILURE();
     return Result::FATAL_ERROR_LIBRARY_FAILURE;
   }
 
-  Result CheckPublicKey(Input subjectPublicKeyInfo) override
+  Result CheckRSAPublicKeyModulusSizeInBits(EndEntityOrCA, unsigned int)
+                                            override
   {
-    return TestCheckPublicKey(subjectPublicKeyInfo);
+    return Success;
+  }
+
+  Result CheckECDSACurveIsAcceptable(EndEntityOrCA, NamedCurve) override
+  {
+    return Success;
   }
 
   ByteString rootDER;
   ByteString rootSubjectDER;
   ByteString intDER;
   ByteString intSubjectDER;
 };
 
--- a/security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp
+++ b/security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp
@@ -66,20 +66,25 @@ private:
   }
 
   Result DigestBuf(Input item, /*out*/ uint8_t *digestBuf, size_t digestBufLen)
                    override
   {
     return TestDigestBuf(item, digestBuf, digestBufLen);
   }
 
-  Result CheckPublicKey(Input) override
+  Result CheckRSAPublicKeyModulusSizeInBits(EndEntityOrCA, unsigned int)
+                                            override
   {
-    ADD_FAILURE();
-    return Result::FATAL_ERROR_LIBRARY_FAILURE;
+    return Success;
+  }
+
+  Result CheckECDSACurveIsAcceptable(EndEntityOrCA, NamedCurve) override
+  {
+    return Success;
   }
 };
 
 class pkixocsp_CreateEncodedOCSPRequest : public ::testing::Test
 {
 protected:
   void MakeIssuerCertIDComponents(const char* issuerASCII,
                                   /*out*/ ByteString& issuerDER,
--- a/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp
+++ b/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp
@@ -77,19 +77,25 @@ public:
   }
 
   Result DigestBuf(Input item, /*out*/ uint8_t* digestBuf, size_t digestBufLen)
                    final override
   {
     return TestDigestBuf(item, digestBuf, digestBufLen);
   }
 
-  Result CheckPublicKey(Input subjectPublicKeyInfo) final override
+  Result CheckRSAPublicKeyModulusSizeInBits(EndEntityOrCA, unsigned int)
+                                            final override
   {
-    return TestCheckPublicKey(subjectPublicKeyInfo);
+    return Success;
+  }
+
+  Result CheckECDSACurveIsAcceptable(EndEntityOrCA, NamedCurve) final override
+  {
+    return Success;
   }
 
   OCSPTestTrustDomain(const OCSPTestTrustDomain&) = delete;
   void operator=(const OCSPTestTrustDomain&) = delete;
 };
 
 namespace {
 char const* const rootName = "Test CA 1";
--- a/security/pkix/test/lib/pkixtestnss.cpp
+++ b/security/pkix/test/lib/pkixtestnss.cpp
@@ -380,29 +380,21 @@ SHA1(const ByteString& toHash)
                                static_cast<int32_t>(toHash.length()));
   if (srv != SECSuccess) {
     return ByteString();
   }
   return ByteString(digestBuf, sizeof(digestBuf));
 }
 
 Result
-TestCheckPublicKey(Input subjectPublicKeyInfo)
-{
-  InitNSSIfNeeded();
-  return CheckPublicKeyNSS(subjectPublicKeyInfo, MINIMUM_TEST_KEY_BITS);
-}
-
-Result
 TestVerifySignedData(const SignedDataWithSignature& signedData,
                      Input subjectPublicKeyInfo)
 {
   InitNSSIfNeeded();
-  return VerifySignedDataNSS(signedData, subjectPublicKeyInfo,
-                             MINIMUM_TEST_KEY_BITS, nullptr);
+  return VerifySignedDataNSS(signedData, subjectPublicKeyInfo, nullptr);
 }
 
 Result
 TestDigestBuf(Input item, /*out*/ uint8_t* digestBuf, size_t digestBufLen)
 {
   InitNSSIfNeeded();
   return DigestBufNSS(item, digestBuf, digestBufLen);
 }
--- a/security/pkix/test/lib/pkixtestutil.h
+++ b/security/pkix/test/lib/pkixtestutil.h
@@ -27,18 +27,16 @@
 
 #include <ctime>
 #include <stdint.h> // Some Mozilla-supported compilers lack <cstdint>
 #include <string>
 
 #include "pkix/pkixtypes.h"
 #include "pkix/ScopedPtr.h"
 
-static const unsigned int MINIMUM_TEST_KEY_BITS = 1024;
-
 namespace mozilla { namespace pkix { namespace test {
 
 typedef std::basic_string<uint8_t> ByteString;
 
 inline bool ENCODING_FAILED(const ByteString& bs) { return bs.empty(); }
 
 // XXX: Ideally, we should define this instead:
 //