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 228533 3fe8d7d7f9f7373d0d3a3341d1a46347c06c85c7
parent 228532 75c440d6b2fff1191f4a48807243e5be1783c12a
child 228534 0db39f732328a2bb42c0bb2367a1d32d5edbd47a
push id28264
push usercbook@mozilla.com
push dateWed, 11 Feb 2015 13:58:35 +0000
treeherdermozilla-central@38058cb42a0e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1122841
milestone38.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 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:
 //