Bug 1036107, Part 1: Stop using CERTSignedData in mozilla::pkix, r=keeler
authorBrian Smith <brian@briansmith.org>
Thu, 10 Jul 2014 19:00:32 -0700
changeset 207753 578899c0b81952253e829c2e6fa8de1b83afe624
parent 207752 1aa504e620340f66568e113e25f8d08d9c509a2f
child 207754 96d7c79707e590b91a38020bb5a11aec8070e447
push idunknown
push userunknown
push dateunknown
reviewerskeeler
bugs1036107
milestone33.0a1
Bug 1036107, Part 1: Stop using CERTSignedData in mozilla::pkix, r=keeler
security/apps/AppTrustDomain.cpp
security/apps/AppTrustDomain.h
security/build/nss.def
security/certverifier/NSSCertDBTrustDomain.cpp
security/certverifier/NSSCertDBTrustDomain.h
security/pkix/include/pkix/pkix.h
security/pkix/include/pkix/pkixtypes.h
security/pkix/lib/pkixcert.cpp
security/pkix/lib/pkixcheck.cpp
security/pkix/lib/pkixder.cpp
security/pkix/lib/pkixder.h
security/pkix/lib/pkixkey.cpp
security/pkix/lib/pkixocsp.cpp
security/pkix/lib/pkixutil.h
security/pkix/test/gtest/pkixbuild_tests.cpp
security/pkix/test/gtest/pkixcert_extension_tests.cpp
security/pkix/test/gtest/pkixder_pki_types_tests.cpp
security/pkix/test/lib/pkixtestutil.h
--- a/security/apps/AppTrustDomain.cpp
+++ b/security/apps/AppTrustDomain.cpp
@@ -184,17 +184,17 @@ AppTrustDomain::GetCertTrust(EndEntityOr
     return SECSuccess;
   }
 
   *trustLevel = TrustLevel::InheritsTrust;
   return SECSuccess;
 }
 
 SECStatus
-AppTrustDomain::VerifySignedData(const CERTSignedData& signedData,
+AppTrustDomain::VerifySignedData(const SignedDataWithSignature& signedData,
                                  const SECItem& subjectPublicKeyInfo)
 {
   return ::mozilla::pkix::VerifySignedData(signedData, subjectPublicKeyInfo,
                                            mPinArg);
 }
 
 SECStatus
 AppTrustDomain::CheckRevocation(EndEntityOrCA, const CertID&, PRTime time,
--- a/security/apps/AppTrustDomain.h
+++ b/security/apps/AppTrustDomain.h
@@ -22,18 +22,19 @@ public:
   SECStatus SetTrustedRoot(AppTrustedRoot trustedRoot);
 
   SECStatus GetCertTrust(mozilla::pkix::EndEntityOrCA endEntityOrCA,
                          const mozilla::pkix::CertPolicyId& policy,
                          const SECItem& candidateCertDER,
                  /*out*/ mozilla::pkix::TrustLevel* trustLevel) MOZ_OVERRIDE;
   SECStatus FindIssuer(const SECItem& encodedIssuerName,
                        IssuerChecker& checker, PRTime time) MOZ_OVERRIDE;
-  SECStatus VerifySignedData(const CERTSignedData& signedData,
-                             const SECItem& subjectPublicKeyInfo) MOZ_OVERRIDE;
+  SECStatus VerifySignedData(
+              const mozilla::pkix::SignedDataWithSignature& signedData,
+              const SECItem& subjectPublicKeyInfo) MOZ_OVERRIDE;
   SECStatus CheckRevocation(mozilla::pkix::EndEntityOrCA endEntityOrCA,
                             const mozilla::pkix::CertID& certID, PRTime time,
                             /*optional*/ const SECItem* stapledOCSPresponse,
                             /*optional*/ const SECItem* aiaExtension);
   SECStatus IsChainValid(const mozilla::pkix::DERArray& certChain);
 
 private:
   /*out*/ ScopedCERTCertList& mCertChain;
--- a/security/build/nss.def
+++ b/security/build/nss.def
@@ -647,9 +647,10 @@ SSL_VersionRangeGet
 SSL_VersionRangeSet
 SSL_VersionRangeSetDefault
 UTIL_SetForkState
 VFY_Begin
 VFY_CreateContext
 VFY_DestroyContext
 VFY_End
 VFY_Update
+VFY_VerifyDataDirect
 VFY_VerifyDataWithAlgorithmID
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -212,17 +212,17 @@ NSSCertDBTrustDomain::GetCertTrust(EndEn
     }
   }
 
   *trustLevel = TrustLevel::InheritsTrust;
   return SECSuccess;
 }
 
 SECStatus
-NSSCertDBTrustDomain::VerifySignedData(const CERTSignedData& signedData,
+NSSCertDBTrustDomain::VerifySignedData(const SignedDataWithSignature& signedData,
                                        const SECItem& subjectPublicKeyInfo)
 {
   return ::mozilla::pkix::VerifySignedData(signedData, subjectPublicKeyInfo,
                                            mPinArg);
 }
 
 static PRIntervalTime
 OCSPFetchingTypeToTimeoutTime(NSSCertDBTrustDomain::OCSPFetching ocspFetching)
--- a/security/certverifier/NSSCertDBTrustDomain.h
+++ b/security/certverifier/NSSCertDBTrustDomain.h
@@ -57,18 +57,19 @@ public:
   virtual SECStatus FindIssuer(const SECItem& encodedIssuerName,
                                IssuerChecker& checker, PRTime time);
 
   virtual SECStatus GetCertTrust(mozilla::pkix::EndEntityOrCA endEntityOrCA,
                                  const mozilla::pkix::CertPolicyId& policy,
                                  const SECItem& candidateCertDER,
                          /*out*/ mozilla::pkix::TrustLevel* trustLevel);
 
-  virtual SECStatus VerifySignedData(const CERTSignedData& signedData,
-                                     const SECItem& subjectPublicKeyInfo);
+  virtual SECStatus VerifySignedData(
+                      const mozilla::pkix::SignedDataWithSignature& signedData,
+                      const SECItem& subjectPublicKeyInfo);
 
   virtual SECStatus CheckRevocation(mozilla::pkix::EndEntityOrCA endEntityOrCA,
                                     const mozilla::pkix::CertID& certID,
                                     PRTime time,
                        /*optional*/ const SECItem* stapledOCSPResponse,
                        /*optional*/ const SECItem* aiaExtension);
 
   virtual SECStatus IsChainValid(const mozilla::pkix::DERArray& certChain);
--- a/security/pkix/include/pkix/pkix.h
+++ b/security/pkix/include/pkix/pkix.h
@@ -93,17 +93,17 @@ namespace mozilla { namespace pkix {
 SECStatus BuildCertChain(TrustDomain& trustDomain, const SECItem& cert,
                          PRTime time, EndEntityOrCA endEntityOrCA,
                          KeyUsage requiredKeyUsageIfPresent,
                          KeyPurposeId requiredEKUIfPresent,
                          const CertPolicyId& requiredPolicy,
             /*optional*/ const SECItem* stapledOCSPResponse);
 
 // Verify the given signed data using the given public key.
-SECStatus VerifySignedData(const CERTSignedData& sd,
+SECStatus VerifySignedData(const SignedDataWithSignature& 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 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
--- a/security/pkix/include/pkix/pkixtypes.h
+++ b/security/pkix/include/pkix/pkixtypes.h
@@ -20,24 +20,84 @@
  * 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/nullptr.h"
+#include "prtime.h"
 #include "seccomon.h"
-#include "secport.h"
 #include "stdint.h"
 
 namespace mozilla { namespace pkix {
 
+MOZILLA_PKIX_ENUM_CLASS DigestAlgorithm
+{
+  sha512 = 1,
+  sha384 = 2,
+  sha256 = 3,
+  sha1 = 4,
+};
+
+// Named ECC Curves:
+//   * secp521r1 (OID 1.3.132.0.35, RFC 5480)
+//   * secp384r1 (OID 1.3.132.0.34, RFC 5480)
+//   * secp256r1 (OID 1.2.840.10045.3.17, RFC 5480)
+MOZILLA_PKIX_ENUM_CLASS SignatureAlgorithm
+{
+  // ecdsa-with-SHA512 (OID 1.2.840.10045.4.3.4, RFC 5758 Section 3.2)
+  ecdsa_with_sha512 = 1,
+
+  // ecdsa-with-SHA384 (OID 1.2.840.10045.4.3.3, RFC 5758 Section 3.2)
+  ecdsa_with_sha384 = 4,
+
+  // ecdsa-with-SHA256 (OID 1.2.840.10045.4.3.2, RFC 5758 Section 3.2)
+  ecdsa_with_sha256 = 7,
+
+  // ecdsa-with-SHA1 (OID 1.2.840.10045.4.1, RFC 3279 Section 2.2.3)
+  ecdsa_with_sha1 = 10,
+
+  // sha512WithRSAEncryption (OID 1.2.840.113549.1.1.13, RFC 4055 Section 5)
+  rsa_pkcs1_with_sha512 = 13,
+
+  // sha384WithRSAEncryption (OID 1.2.840.113549.1.1.12, RFC 4055 Section 5)
+  rsa_pkcs1_with_sha384 = 14,
+
+  // sha256WithRSAEncryption (OID 1.2.840.113549.1.1.11, RFC 4055 Section 5)
+  rsa_pkcs1_with_sha256 = 15,
+
+  // sha-1WithRSAEncryption (OID 1.2.840.113549.1.1.5, RFC 3279 Section 2.2.1)
+  rsa_pkcs1_with_sha1 = 16,
+
+  // id-dsa-with-sha256 (OID 2.16.840.1.101.3.4.3.2, RFC 5758 Section 3.1)
+  dsa_with_sha256 = 17,
+
+  // id-dsa-with-sha1 (OID 1.2.840.10040.4.3, RFC 3279 Section 2.2.2)
+  dsa_with_sha1 = 18,
+};
+
+struct SignedDataWithSignature
+{
+public:
+  SignedDataWithSignature()
+  {
+    data.data = nullptr;
+    data.len = 0;
+    signature.data = nullptr;
+    signature.len = 0;
+  }
+  SECItem data; // non-owning
+  SignatureAlgorithm algorithm;
+  SECItem signature; // non-owning
+};
+
 MOZILLA_PKIX_ENUM_CLASS EndEntityOrCA { MustBeEndEntity = 0, MustBeCA = 1 };
 
 MOZILLA_PKIX_ENUM_CLASS KeyUsage : uint8_t {
   digitalSignature = 0,
   nonRepudiation   = 1,
   keyEncipherment  = 2,
   dataEncipherment = 3,
   keyAgreement     = 4,
@@ -203,23 +263,16 @@ public:
   //                    IssuerChecker::Check
   //                      [...]
   //
   // checker.Check is responsible for limiting the recursion to a reasonable
   // limit.
   virtual SECStatus FindIssuer(const SECItem& encodedIssuerName,
                                IssuerChecker& checker, PRTime time) = 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.
-  virtual SECStatus VerifySignedData(const CERTSignedData& signedData,
-                                     const SECItem& subjectPublicKeyInfo) = 0;
-
   // Called as soon as we think we have a valid chain but before revocation
   // checks are done. This function can be used to compute additional checks,
   // especilaly checks that require the entire certificate chain. This callback
   // can also be used to save a copy of the built certificate chain for later
   // use.
   //
   // This function may be called multiple times, regardless of whether it
   // returns SECSuccess or SECFailure. It is guaranteed that BuildCertChain
@@ -240,16 +293,22 @@ public:
 
   // issuerCertToDup is only non-const so CERT_DupCertificate can be called on
   // it.
   virtual SECStatus CheckRevocation(EndEntityOrCA endEntityOrCA,
                                     const CertID& certID, PRTime time,
                        /*optional*/ const SECItem* stapledOCSPresponse,
                        /*optional*/ const SECItem* aiaExtension) = 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.
+  virtual SECStatus VerifySignedData(const SignedDataWithSignature& signedData,
+                                     const SECItem& subjectPublicKeyInfo) = 0;
 protected:
   TrustDomain() { }
 
 private:
   TrustDomain(const TrustDomain&) /* = delete */;
   void operator=(const TrustDomain&) /* = delete */;
 };
 
--- a/security/pkix/lib/pkixcert.cpp
+++ b/security/pkix/lib/pkixcert.cpp
@@ -81,18 +81,19 @@ BackCert::Init()
   }
   if (der::CertificateSerialNumber(tbsCertificate, serialNumber)
         != der::Success) {
     return MapSECStatus(SECFailure);
   }
   // XXX: Ignored. What are we supposed to check? This seems totally redundant
   // with Certificate.signatureAlgorithm. Is it important to check that they
   // are consistent with each other? It doesn't seem to matter!
-  SECAlgorithmID signature;
-  if (der::AlgorithmIdentifier(tbsCertificate, signature) != der::Success) {
+  SignatureAlgorithm signature;
+  if (der::SignatureAlgorithmIdentifier(tbsCertificate, signature)
+        != der::Success) {
     return MapSECStatus(SECFailure);
   }
   if (der::ExpectTagAndGetTLV(tbsCertificate, der::SEQUENCE, issuer)
         != der::Success) {
     return MapSECStatus(SECFailure);
   }
   if (der::ExpectTagAndGetValue(tbsCertificate, der::SEQUENCE, validity)
         != der::Success) {
--- a/security/pkix/lib/pkixcheck.cpp
+++ b/security/pkix/lib/pkixcheck.cpp
@@ -17,18 +17,17 @@
  *
  * 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 <limits>
-
+#include "cert.h"
 #include "pkix/bind.h"
 #include "pkix/pkix.h"
 #include "pkix/ScopedPtr.h"
 #include "pkixcheck.h"
 #include "pkixder.h"
 #include "pkixutil.h"
 
 namespace mozilla { namespace pkix {
--- a/security/pkix/lib/pkixder.cpp
+++ b/security/pkix/lib/pkixder.cpp
@@ -87,30 +87,215 @@ ExpectTagAndGetLength(Input& input, uint
   }
 
   // Ensure the input is long enough for the length it says it has.
   return input.EnsureLength(length);
 }
 
 } // namespace internal
 
+static Result
+OptionalNull(Input& input)
+{
+  if (input.Peek(NULLTag)) {
+    return Null(input);
+  }
+  return Success;
+}
+
+namespace {
+
 Result
-SignedData(Input& input, /*out*/ Input& tbs, /*out*/ CERTSignedData& signedData)
+DigestAlgorithmOIDValue(Input& algorithmID, /*out*/ DigestAlgorithm& algorithm)
+{
+  // RFC 4055 Section 2.1
+  // python DottedOIDToCode.py id-sha1 1.3.14.3.2.26
+  static const uint8_t id_sha1[] = {
+    0x2b, 0x0e, 0x03, 0x02, 0x1a
+  };
+  // python DottedOIDToCode.py id-sha256 2.16.840.1.101.3.4.2.1
+  static const uint8_t id_sha256[] = {
+    0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01
+  };
+  // python DottedOIDToCode.py id-sha384 2.16.840.1.101.3.4.2.2
+  static const uint8_t id_sha384[] = {
+    0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x02
+  };
+  // python DottedOIDToCode.py id-sha512 2.16.840.1.101.3.4.2.3
+  static const uint8_t id_sha512[] = {
+    0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03
+  };
+
+  // Matching is attempted based on a rough estimate of the commonality of the
+  // algorithm, to minimize the number of MatchRest calls.
+  if (algorithmID.MatchRest(id_sha1)) {
+    algorithm = DigestAlgorithm::sha1;
+  } else if (algorithmID.MatchRest(id_sha256)) {
+    algorithm = DigestAlgorithm::sha256;
+  } else if (algorithmID.MatchRest(id_sha384)) {
+    algorithm = DigestAlgorithm::sha384;
+  } else if (algorithmID.MatchRest(id_sha512)) {
+    algorithm = DigestAlgorithm::sha512;
+  } else {
+    return Fail(SEC_ERROR_INVALID_ALGORITHM);
+  }
+
+  return Success;
+}
+
+Result
+SignatureAlgorithmOIDValue(Input& algorithmID,
+                           /*out*/ SignatureAlgorithm& algorithm)
+{
+  // RFC 5758 Section 3.1 (id-dsa-with-sha224 is intentionally excluded)
+  // python DottedOIDToCode.py id-dsa-with-sha256 2.16.840.1.101.3.4.3.2
+  static const uint8_t id_dsa_with_sha256[] = {
+    0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x03, 0x02
+  };
+
+  // RFC 5758 Section 3.2 (ecdsa-with-SHA224 is intentionally excluded)
+  // python DottedOIDToCode.py ecdsa-with-SHA256 1.2.840.10045.4.3.2
+  static const uint8_t ecdsa_with_SHA256[] = {
+    0x2a, 0x86, 0x48, 0xce, 0x3d, 0x04, 0x03, 0x02
+  };
+  // python DottedOIDToCode.py ecdsa-with-SHA384 1.2.840.10045.4.3.3
+  static const uint8_t ecdsa_with_SHA384[] = {
+    0x2a, 0x86, 0x48, 0xce, 0x3d, 0x04, 0x03, 0x03
+  };
+  // python DottedOIDToCode.py ecdsa-with-SHA512 1.2.840.10045.4.3.4
+  static const uint8_t ecdsa_with_SHA512[] = {
+    0x2a, 0x86, 0x48, 0xce, 0x3d, 0x04, 0x03, 0x04
+  };
+
+  // RFC 4055 Section 5 (sha224WithRSAEncryption is intentionally excluded)
+  // python DottedOIDToCode.py sha256WithRSAEncryption 1.2.840.113549.1.1.11
+  static const uint8_t sha256WithRSAEncryption[] = {
+    0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0b
+  };
+  // python DottedOIDToCode.py sha384WithRSAEncryption 1.2.840.113549.1.1.12
+  static const uint8_t sha384WithRSAEncryption[] = {
+    0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0c
+  };
+  // python DottedOIDToCode.py sha512WithRSAEncryption 1.2.840.113549.1.1.13
+  static const uint8_t sha512WithRSAEncryption[] = {
+    0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0d
+  };
+
+  // RFC 3279 Section 2.2.1
+  // python DottedOIDToCode.py sha-1WithRSAEncryption 1.2.840.113549.1.1.5
+  static const uint8_t sha_1WithRSAEncryption[] = {
+    0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x05
+  };
+
+  // RFC 3279 Section 2.2.2
+  // python DottedOIDToCode.py id-dsa-with-sha1 1.2.840.10040.4.3
+  static const uint8_t id_dsa_with_sha1[] = {
+    0x2a, 0x86, 0x48, 0xce, 0x38, 0x04, 0x03
+  };
+
+  // RFC 3279 Section 2.2.3
+  // python DottedOIDToCode.py ecdsa-with-SHA1 1.2.840.10045.4.1
+  static const uint8_t ecdsa_with_SHA1[] = {
+    0x2a, 0x86, 0x48, 0xce, 0x3d, 0x04, 0x01
+  };
+
+  // RFC 5758 Section 3.1 (DSA with SHA-2), RFC 3279 Section 2.2.2 (DSA with
+  // SHA-1), RFC 5758 Section 3.2 (ECDSA with SHA-2), and RFC 3279
+  // Section 2.2.3 (ECDSA with SHA-1) all say that parameters must be omitted.
+  //
+  // RFC 4055 Section 5 and RFC 3279 Section 2.2.1 both say that parameters for
+  // RSA must be encoded as NULL; we relax that requirement by allowing the
+  // NULL to be omitted, to match all the other signature algorithms we support
+  // and for compatibility.
+
+  // Matching is attempted based on a rough estimate of the commonality of the
+  // algorithm, to minimize the number of MatchRest calls.
+  if (algorithmID.MatchRest(sha256WithRSAEncryption)) {
+    algorithm = SignatureAlgorithm::rsa_pkcs1_with_sha256;
+  } else if (algorithmID.MatchRest(ecdsa_with_SHA256)) {
+    algorithm = SignatureAlgorithm::ecdsa_with_sha256;
+  } else if (algorithmID.MatchRest(sha_1WithRSAEncryption)) {
+    algorithm = SignatureAlgorithm::rsa_pkcs1_with_sha1;
+  } else if (algorithmID.MatchRest(ecdsa_with_SHA1)) {
+    algorithm = SignatureAlgorithm::ecdsa_with_sha1;
+  } else if (algorithmID.MatchRest(ecdsa_with_SHA384)) {
+    algorithm = SignatureAlgorithm::ecdsa_with_sha384;
+  } else if (algorithmID.MatchRest(ecdsa_with_SHA512)) {
+    algorithm = SignatureAlgorithm::ecdsa_with_sha512;
+  } else if (algorithmID.MatchRest(sha384WithRSAEncryption)) {
+    algorithm = SignatureAlgorithm::rsa_pkcs1_with_sha384;
+  } else if (algorithmID.MatchRest(sha512WithRSAEncryption)) {
+    algorithm = SignatureAlgorithm::rsa_pkcs1_with_sha512;
+  } else if (algorithmID.MatchRest(id_dsa_with_sha1)) {
+    algorithm = SignatureAlgorithm::dsa_with_sha1;
+  } else if (algorithmID.MatchRest(id_dsa_with_sha256)) {
+    algorithm = SignatureAlgorithm::dsa_with_sha256;
+  } else {
+    // Any MD5-based signature algorithm, or any unknown signature algorithm.
+    return Fail(SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED);
+  }
+
+  return Success;
+}
+
+template <typename OidValueParser, typename Algorithm>
+Result
+AlgorithmIdentifier(OidValueParser oidValueParser, Input& input,
+                    /*out*/ Algorithm& algorithm)
+{
+  Input value;
+  if (ExpectTagAndGetValue(input, SEQUENCE, value) != Success) {
+    return Failure;
+  }
+
+  Input algorithmID;
+  if (ExpectTagAndGetValue(value, der::OIDTag, algorithmID) != Success) {
+    return Failure;
+  }
+  if (oidValueParser(algorithmID, algorithm) != Success) {
+    return Failure;
+  }
+
+  if (OptionalNull(value) != Success) {
+    return Failure;
+  }
+
+  return End(value);
+}
+
+} // unnamed namespace
+
+Result
+SignatureAlgorithmIdentifier(Input& input,
+                             /*out*/ SignatureAlgorithm& algorithm)
+{
+  return AlgorithmIdentifier(SignatureAlgorithmOIDValue, input, algorithm);
+}
+
+Result
+DigestAlgorithmIdentifier(Input& input, /*out*/ DigestAlgorithm& algorithm)
+{
+  return AlgorithmIdentifier(DigestAlgorithmOIDValue, input, algorithm);
+}
+
+Result
+SignedData(Input& input, /*out*/ Input& tbs,
+           /*out*/ SignedDataWithSignature& signedData)
 {
   Input::Mark mark(input.GetMark());
 
   if (ExpectTagAndGetValue(input, SEQUENCE, tbs) != Success) {
     return Failure;
   }
 
   if (input.GetSECItem(siBuffer, mark, signedData.data) != Success) {
     return Failure;
   }
 
-  if (AlgorithmIdentifier(input, signedData.signatureAlgorithm) != Success) {
+  if (SignatureAlgorithmIdentifier(input, signedData.algorithm) != Success) {
     return Failure;
   }
 
   if (ExpectTagAndGetValue(input, BIT_STRING, signedData.signature)
         != Success) {
     return Failure;
   }
   if (signedData.signature.len == 0) {
@@ -123,17 +308,16 @@ SignedData(Input& input, /*out*/ Input& 
   // various places, so we enforce it too in order to simplify this code. If we
   // find compatibility issues, we'll know we're wrong and we'll have to figure
   // out how to shift the bits around.
   if (unusedBitsAtEnd != 0) {
     return Fail(SEC_ERROR_BAD_SIGNATURE);
   }
   ++signedData.signature.data;
   --signedData.signature.len;
-  signedData.signature.len = (signedData.signature.len << 3); // Bytes to bits
 
   return Success;
 }
 
 static inline Result
 ReadDigit(Input& input, /*out*/ int& value)
 {
   uint8_t b;
--- a/security/pkix/lib/pkixder.h
+++ b/security/pkix/lib/pkixder.h
@@ -34,17 +34,17 @@
 // input does not match the criteria.
 //
 // Skip* functions unconditionally advance the input mark and return Success if
 // they are able to do so; otherwise they return Failure with the input mark in
 // an undefined state.
 
 #include "pkix/enumclass.h"
 #include "pkix/nullptr.h"
-
+#include "pkix/pkixtypes.h"
 #include "prerror.h"
 #include "prtime.h"
 #include "secerr.h"
 #include "secoidt.h"
 #include "stdint.h"
 
 typedef struct CERTSignedDataStr CERTSignedData;
 
@@ -595,39 +595,16 @@ OID(Input& input, const uint8_t (&expect
     return Failure;
   }
 
   return input.Expect(expectedOid, Len);
 }
 
 // PKI-specific types
 
-// AlgorithmIdentifier  ::=  SEQUENCE  {
-//         algorithm               OBJECT IDENTIFIER,
-//         parameters              ANY DEFINED BY algorithm OPTIONAL  }
-inline Result
-AlgorithmIdentifier(Input& input, SECAlgorithmID& algorithmID)
-{
-  Input value;
-  if (ExpectTagAndGetValue(input, der::SEQUENCE, value) != Success) {
-    return Failure;
-  }
-  if (ExpectTagAndGetValue(value, OIDTag, algorithmID.algorithm) != Success) {
-    return Failure;
-  }
-  algorithmID.parameters.data = nullptr;
-  algorithmID.parameters.len = 0;
-  if (!value.AtEnd()) {
-    if (Null(value) != Success) {
-      return Failure;
-    }
-  }
-  return End(value);
-}
-
 inline Result
 CertificateSerialNumber(Input& input, /*out*/ SECItem& value)
 {
   // http://tools.ietf.org/html/rfc5280#section-4.1.2.2:
   //
   // * "The serial number MUST be a positive integer assigned by the CA to
   //   each certificate."
   // * "Certificate users MUST be able to handle serialNumber values up to 20
@@ -761,30 +738,36 @@ OptionalExtensions(Input& input, uint8_t
     if (critical && !understood) {
       return Fail(SEC_ERROR_UNKNOWN_CRITICAL_EXTENSION);
     }
   }
 
   return Success;
 }
 
+Result DigestAlgorithmIdentifier(Input& input,
+                                 /*out*/ DigestAlgorithm& algorithm);
+
+Result SignatureAlgorithmIdentifier(Input& input,
+                                    /*out*/ SignatureAlgorithm& algorithm);
+
 // 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,
 //        signatureAlgorithm   AlgorithmIdentifier,
 //        signatureValue       BIT STRING  }
 //
 // BasicOCSPResponse       ::= SEQUENCE {
 //    tbsResponseData      ResponseData,
 //    signatureAlgorithm   AlgorithmIdentifier,
 //    signature            BIT STRING,
 //    certs            [0] EXPLICIT SEQUENCE OF Certificate OPTIONAL }
-Result
-SignedData(Input& input, /*out*/ Input& tbs, /*out*/ CERTSignedData& signedData);
+Result SignedData(Input& input, /*out*/ Input& tbs,
+                  /*out*/ SignedDataWithSignature& signedDataWithSignature);
 
 } } } // namespace mozilla::pkix::der
 
 #endif // mozilla_pkix__pkixder_h
--- a/security/pkix/lib/pkixkey.cpp
+++ b/security/pkix/lib/pkixkey.cpp
@@ -31,68 +31,91 @@
 #include "pkix/pkix.h"
 #include "pkix/ScopedPtr.h"
 #include "prerror.h"
 #include "secerr.h"
 
 namespace mozilla { namespace pkix {
 
 SECStatus
-VerifySignedData(const CERTSignedData& sd, const SECItem& subjectPublicKeyInfo,
-                 void* pkcs11PinArg)
+VerifySignedData(const SignedDataWithSignature& sd,
+                 const SECItem& subjectPublicKeyInfo, void* pkcs11PinArg)
 {
-  if (!sd.data.data || !sd.signatureAlgorithm.algorithm.data ||
-      !sd.signature.data) {
+  if (!sd.data.data || !sd.signature.data) {
     PR_NOT_REACHED("invalid args to VerifySignedData");
     PR_SetError(SEC_ERROR_INVALID_ARGS, 0);
     return SECFailure;
   }
 
   // See bug 921585.
   if (sd.data.len > static_cast<unsigned int>(std::numeric_limits<int>::max())) {
     PR_SetError(SEC_ERROR_INVALID_ARGS, 0);
     return SECFailure;
   }
 
-  // convert sig->len from bit counts to byte count.
-  SECItem sig = sd.signature;
-  DER_ConvertBitString(&sig);
+  SECOidTag pubKeyAlg;
+  SECOidTag digestAlg;
+  switch (sd.algorithm) {
+    case SignatureAlgorithm::ecdsa_with_sha512:
+      pubKeyAlg = SEC_OID_ANSIX962_EC_PUBLIC_KEY;
+      digestAlg = SEC_OID_SHA512;
+      break;
+    case SignatureAlgorithm::ecdsa_with_sha384:
+      pubKeyAlg = SEC_OID_ANSIX962_EC_PUBLIC_KEY;
+      digestAlg = SEC_OID_SHA384;
+      break;
+    case SignatureAlgorithm::ecdsa_with_sha256:
+      pubKeyAlg = SEC_OID_ANSIX962_EC_PUBLIC_KEY;
+      digestAlg = SEC_OID_SHA256;
+      break;
+    case SignatureAlgorithm::ecdsa_with_sha1:
+      pubKeyAlg = SEC_OID_ANSIX962_EC_PUBLIC_KEY;
+      digestAlg = SEC_OID_SHA1;
+      break;
+    case SignatureAlgorithm::rsa_pkcs1_with_sha512:
+      pubKeyAlg = SEC_OID_PKCS1_RSA_ENCRYPTION;
+      digestAlg = SEC_OID_SHA512;
+      break;
+    case SignatureAlgorithm::rsa_pkcs1_with_sha384:
+      pubKeyAlg = SEC_OID_PKCS1_RSA_ENCRYPTION;
+      digestAlg = SEC_OID_SHA384;
+      break;
+    case SignatureAlgorithm::rsa_pkcs1_with_sha256:
+      pubKeyAlg = SEC_OID_PKCS1_RSA_ENCRYPTION;
+      digestAlg = SEC_OID_SHA256;
+      break;
+    case SignatureAlgorithm::rsa_pkcs1_with_sha1:
+      pubKeyAlg = SEC_OID_PKCS1_RSA_ENCRYPTION;
+      digestAlg = SEC_OID_SHA1;
+      break;
+    case SignatureAlgorithm::dsa_with_sha256:
+      pubKeyAlg = SEC_OID_ANSIX9_DSA_SIGNATURE;
+      digestAlg = SEC_OID_SHA256;
+      break;
+    case SignatureAlgorithm::dsa_with_sha1:
+      pubKeyAlg = SEC_OID_ANSIX9_DSA_SIGNATURE;
+      digestAlg = SEC_OID_SHA1;
+      break;
+    default:
+      PR_NOT_REACHED("unknown signature algorithm");
+      PR_SetError(SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED, 0);
+      return SECFailure;
+  }
 
   ScopedPtr<CERTSubjectPublicKeyInfo, SECKEY_DestroySubjectPublicKeyInfo>
     spki(SECKEY_DecodeDERSubjectPublicKeyInfo(&subjectPublicKeyInfo));
   if (!spki) {
     return SECFailure;
   }
   ScopedPtr<SECKEYPublicKey, SECKEY_DestroyPublicKey>
     pubKey(SECKEY_ExtractPublicKey(spki.get()));
   if (!pubKey) {
     return SECFailure;
   }
 
-  SECOidTag hashAlg;
-  if (VFY_VerifyDataWithAlgorithmID(sd.data.data, static_cast<int>(sd.data.len),
-                                    pubKey.get(), &sig, &sd.signatureAlgorithm,
-                                    &hashAlg, pkcs11PinArg) != SECSuccess) {
-    return SECFailure;
-  }
-
-  // TODO: Ideally, we would do this check before we call
-  // VFY_VerifyDataWithAlgorithmID. But, VFY_VerifyDataWithAlgorithmID gives us
-  // the hash algorithm so it is more convenient to do things in this order.
-  uint32_t policy;
-  if (NSS_GetAlgorithmPolicy(hashAlg, &policy) != SECSuccess) {
-    return SECFailure;
-  }
-
-  // XXX: I'm not sure why there isn't NSS_USE_ALG_IN_SSL_SIGNATURE, but there
-  // isn't. Since we don't know the context in which we're being called, be as
-  // strict as we can be given the NSS API that is available.
-  static const uint32_t requiredPolicy = NSS_USE_ALG_IN_CERT_SIGNATURE |
-                                         NSS_USE_ALG_IN_CMS_SIGNATURE;
-  if ((policy & requiredPolicy) != requiredPolicy) {
-    PR_SetError(SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED, 0);
-    return SECFailure;
-  }
-
-  return SECSuccess;
+  // The static_cast is safe according to the check above that references
+  // bug 921585.
+  return VFY_VerifyDataDirect(sd.data.data, static_cast<int>(sd.data.len),
+                              pubKey.get(), &sd.signature, pubKeyAlg,
+                              digestAlg, nullptr, pkcs11PinArg);
 }
 
 } } // namespace mozilla::pkix
--- a/security/pkix/lib/pkixocsp.cpp
+++ b/security/pkix/lib/pkixocsp.cpp
@@ -174,17 +174,17 @@ MOZILLA_PKIX_ENUM_CLASS ResponderIDType 
   byKey = der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 2
 };
 
 static inline der::Result OCSPResponse(der::Input&, Context&);
 static inline der::Result ResponseBytes(der::Input&, Context&);
 static inline der::Result BasicResponse(der::Input&, Context&);
 static inline der::Result ResponseData(
                               der::Input& tbsResponseData, Context& context,
-                              const CERTSignedData& signedResponseData,
+                              const SignedDataWithSignature& signedResponseData,
                               /*const*/ SECItem* certs, size_t numCerts);
 static inline der::Result SingleResponse(der::Input& input,
                                           Context& context);
 static der::Result ExtensionNotUnderstood(der::Input& extnID,
                                           const SECItem& extnValue,
                                           /*out*/ bool& understood);
 static inline der::Result CertID(der::Input& input,
                                   const Context& context,
@@ -229,17 +229,17 @@ MatchResponderID(ResponderIDType respond
 
     default:
       return Fail(RecoverableError, SEC_ERROR_OCSP_MALFORMED_RESPONSE);
   }
 }
 
 static Result
 VerifyOCSPSignedData(TrustDomain& trustDomain,
-                     const CERTSignedData& signedResponseData,
+                     const SignedDataWithSignature& signedResponseData,
                      const SECItem& spki)
 {
   SECStatus srv = trustDomain.VerifySignedData(signedResponseData, spki);
   if (srv != SECSuccess) {
     if (PR_GetError() == SEC_ERROR_BAD_SIGNATURE) {
       PR_SetError(SEC_ERROR_OCSP_BAD_SIGNATURE, 0);
     }
   }
@@ -250,17 +250,18 @@ VerifyOCSPSignedData(TrustDomain& trustD
 // the cert or it must be a delegated OCSP response signing cert directly
 // issued by the issuer. If the OCSP responder is a delegated OCSP response
 // signer, then its certificate is (probably) embedded within the OCSP
 // response and we'll need to verify that it is a valid certificate that chains
 // *directly* to issuerCert.
 static Result
 VerifySignature(Context& context, ResponderIDType responderIDType,
                 const SECItem& responderID, const SECItem* certs,
-                size_t numCerts, const CERTSignedData& signedResponseData)
+                size_t numCerts,
+                const SignedDataWithSignature& signedResponseData)
 {
   bool match;
   Result rv = MatchResponderID(responderIDType, responderID,
                                context.certID.issuer,
                                context.certID.issuerSubjectPublicKeyInfo,
                                match);
   if (rv != Success) {
     return rv;
@@ -423,17 +424,17 @@ ResponseBytes(der::Input& input, Context
 //    tbsResponseData      ResponseData,
 //    signatureAlgorithm   AlgorithmIdentifier,
 //    signature            BIT STRING,
 //    certs            [0] EXPLICIT SEQUENCE OF Certificate OPTIONAL }
 der::Result
 BasicResponse(der::Input& input, Context& context)
 {
   der::Input tbsResponseData;
-  CERTSignedData signedData;
+  SignedDataWithSignature signedData;
   if (der::SignedData(input, tbsResponseData, signedData) != der::Success) {
     if (PR_GetError() == SEC_ERROR_BAD_SIGNATURE) {
       PR_SetError(SEC_ERROR_OCSP_BAD_SIGNATURE, 0);
     }
     return der::Failure;
   }
 
   // Parse certificates, if any
@@ -478,17 +479,17 @@ BasicResponse(der::Input& input, Context
 // ResponseData ::= SEQUENCE {
 //    version             [0] EXPLICIT Version DEFAULT v1,
 //    responderID             ResponderID,
 //    producedAt              GeneralizedTime,
 //    responses               SEQUENCE OF SingleResponse,
 //    responseExtensions  [1] EXPLICIT Extensions OPTIONAL }
 static inline der::Result
 ResponseData(der::Input& input, Context& context,
-             const CERTSignedData& signedResponseData,
+             const SignedDataWithSignature& signedResponseData,
              /*const*/ SECItem* certs, size_t numCerts)
 {
   der::Version version;
   if (der::OptionalVersion(input, version) != der::Success) {
     return der::Failure;
   }
   if (version != der::Version::v1) {
     // TODO: more specific error code for bad version?
@@ -673,18 +674,23 @@ SingleResponse(der::Input& input, Contex
 //        issuerNameHash      OCTET STRING, -- Hash of issuer's DN
 //        issuerKeyHash       OCTET STRING, -- Hash of issuer's public key
 //        serialNumber        CertificateSerialNumber }
 static inline der::Result
 CertID(der::Input& input, const Context& context, /*out*/ bool& match)
 {
   match = false;
 
-  SECAlgorithmID hashAlgorithm;
-  if (der::AlgorithmIdentifier(input, hashAlgorithm) != der::Success) {
+  DigestAlgorithm hashAlgorithm;
+  if (der::DigestAlgorithmIdentifier(input, hashAlgorithm) != der::Success) {
+    if (PR_GetError() == SEC_ERROR_INVALID_ALGORITHM) {
+      // Skip entries that are hashed with algorithms we don't support.
+      input.SkipToEnd();
+      return der::Success;
+    }
     return der::Failure;
   }
 
   SECItem issuerNameHash;
   if (der::ExpectTagAndGetValue(input, der::OCTET_STRING, issuerNameHash)
         != der::Success) {
     return der::Failure;
   }
@@ -705,18 +711,17 @@ CertID(der::Input& input, const Context&
     // 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.
 
-  SECOidTag hashAlg = SECOID_GetAlgorithmTag(&hashAlgorithm);
-  if (hashAlg != SEC_OID_SHA1) {
+  if (hashAlgorithm != DigestAlgorithm::sha1) {
     // Again, not interested in this response. Consume input, return success.
     input.SkipToEnd();
     return der::Success;
   }
 
   if (issuerNameHash.len != SHA1_LENGTH) {
     return der::Fail(SEC_ERROR_OCSP_MALFORMED_RESPONSE);
   }
--- a/security/pkix/lib/pkixutil.h
+++ b/security/pkix/lib/pkixutil.h
@@ -98,17 +98,17 @@ public:
     , childCert(childCert)
   {
   }
 
   Result Init();
 
   const SECItem& GetDER() const { return der; }
   const der::Version GetVersion() const { return version; }
-  const CERTSignedData& GetSignedData() const { return signedData; }
+  const SignedDataWithSignature& GetSignedData() const { return signedData; }
   const SECItem& GetIssuer() const { return issuer; }
   // XXX: "validity" is a horrible name for the structure that holds
   // notBefore & notAfter, but that is the name used in RFC 5280 and we use the
   // RFC 5280 names for everything.
   const SECItem& GetValidity() const { return validity; }
   const SECItem& GetSerialNumber() const { return serialNumber; }
   const SECItem& GetSubject() const { return subject; }
   const SECItem& GetSubjectPublicKeyInfo() const
@@ -164,21 +164,18 @@ private:
   // to.
   struct NonOwningSECItem : public SECItemStr {
     NonOwningSECItem()
     {
       data = nullptr;
       len = 0;
     }
   };
-  struct NonOwningCERTSignedData : public CERTSignedDataStr {
-    NonOwningCERTSignedData() { memset(this, 0, sizeof(*this)); }
-  };
 
-  NonOwningCERTSignedData signedData;
+  SignedDataWithSignature signedData;
   NonOwningSECItem issuer;
   // XXX: "validity" is a horrible name for the structure that holds
   // notBefore & notAfter, but that is the name used in RFC 5280 and we use the
   // RFC 5280 names for everything.
   NonOwningSECItem validity;
   NonOwningSECItem serialNumber;
   NonOwningSECItem subject;
   NonOwningSECItem subjectPublicKeyInfo;
--- a/security/pkix/test/gtest/pkixbuild_tests.cpp
+++ b/security/pkix/test/gtest/pkixbuild_tests.cpp
@@ -145,17 +145,17 @@ private:
           break;
         }
       }
     }
 
     return SECSuccess;
   }
 
-  SECStatus VerifySignedData(const CERTSignedData& signedData,
+  SECStatus VerifySignedData(const SignedDataWithSignature& signedData,
                              const SECItem& subjectPublicKeyInfo)
   {
     return ::mozilla::pkix::VerifySignedData(signedData, subjectPublicKeyInfo,
                                              nullptr);
   }
 
   SECStatus CheckRevocation(EndEntityOrCA, const CertID&, PRTime,
                             /*optional*/ const SECItem*,
--- a/security/pkix/test/gtest/pkixcert_extension_tests.cpp
+++ b/security/pkix/test/gtest/pkixcert_extension_tests.cpp
@@ -85,17 +85,17 @@ private:
   SECStatus FindIssuer(const SECItem& /*encodedIssuerName*/,
                        IssuerChecker& /*checker*/, PRTime /*time*/)
   {
     PR_NOT_REACHED("FindIssuer should not be called");
     PR_SetError(SEC_ERROR_LIBRARY_FAILURE, 0);
     return SECFailure;
   }
 
-  SECStatus VerifySignedData(const CERTSignedData& signedData,
+  SECStatus VerifySignedData(const SignedDataWithSignature& signedData,
                              const SECItem& subjectPublicKeyInfo)
   {
     return ::mozilla::pkix::VerifySignedData(signedData, subjectPublicKeyInfo,
                                              nullptr);
   }
 
   SECStatus CheckRevocation(EndEntityOrCA, const CertID&, PRTime,
                             /*optional*/ const SECItem*,
--- a/security/pkix/test/gtest/pkixder_pki_types_tests.cpp
+++ b/security/pkix/test/gtest/pkixder_pki_types_tests.cpp
@@ -21,85 +21,31 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
 
 #include <functional>
 #include <vector>
 #include <gtest/gtest.h>
 
-#include "pkix/bind.h"
 #include "pkixder.h"
 
 using namespace mozilla::pkix::der;
 
 namespace {
 
 class pkixder_pki_types_tests : public ::testing::Test
 {
 protected:
   virtual void SetUp()
   {
     PR_SetError(0, 0);
   }
 };
 
-TEST_F(pkixder_pki_types_tests, AlgorithmIdentifierNoParams)
-{
-  const uint8_t DER_ALGORITHM_IDENTIFIER_NO_PARAMS[] = {
-    0x30/*SEQUENCE*/, 0x06/*LENGTH*/,
-    0x06, 0x04, 0xde, 0xad, 0xbe, 0xef   // OID
-  };
-
-  Input input;
-  ASSERT_EQ(Success, input.Init(DER_ALGORITHM_IDENTIFIER_NO_PARAMS,
-                                sizeof DER_ALGORITHM_IDENTIFIER_NO_PARAMS));
-
-  const uint8_t expectedAlgorithmID[] = {
-    0xde, 0xad, 0xbe, 0xef
-  };
-
-  SECAlgorithmID algorithmID;
-  ASSERT_EQ(Success, AlgorithmIdentifier(input, algorithmID));
-
-  ASSERT_EQ(sizeof expectedAlgorithmID, algorithmID.algorithm.len);
-  ASSERT_EQ(0, memcmp(algorithmID.algorithm.data, expectedAlgorithmID,
-                      sizeof expectedAlgorithmID));
-
-  ASSERT_EQ(0u, algorithmID.parameters.len);
-  ASSERT_FALSE(algorithmID.parameters.data);
-}
-
-TEST_F(pkixder_pki_types_tests, AlgorithmIdentifierNullParams)
-{
-  const uint8_t DER_ALGORITHM_IDENTIFIER_NULL_PARAMS[] = {
-    0x30, 0x08,                         // SEQUENCE
-    0x06, 0x04, 0xde, 0xad, 0xbe, 0xef, // OID
-    0x05, 0x00                          // NULL
-  };
-
-  Input input;
-  ASSERT_EQ(Success, input.Init(DER_ALGORITHM_IDENTIFIER_NULL_PARAMS,
-                                sizeof DER_ALGORITHM_IDENTIFIER_NULL_PARAMS));
-
-  const uint8_t expectedAlgorithmID[] = {
-    0xde, 0xad, 0xbe, 0xef
-  };
-
-  SECAlgorithmID algorithmID;
-  ASSERT_EQ(Success, AlgorithmIdentifier(input, algorithmID));
-
-  ASSERT_EQ(sizeof expectedAlgorithmID, algorithmID.algorithm.len);
-  ASSERT_TRUE(memcmp(algorithmID.algorithm.data, expectedAlgorithmID,
-                     sizeof expectedAlgorithmID) == 0);
-
-  ASSERT_EQ((size_t) 0, algorithmID.parameters.len);
-  ASSERT_EQ(NULL, algorithmID.parameters.data);
-}
-
 TEST_F(pkixder_pki_types_tests, CertificateSerialNumber)
 {
   const uint8_t DER_CERT_SERIAL[] = {
     0x02,                       // INTEGER
     8,                          // length
     0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef
   };
 
@@ -261,9 +207,10 @@ TEST_F(pkixder_pki_types_tests, Optional
   Input input;
   ASSERT_EQ(Success, input.Init(DER_OPTIONAL_VERSION_MISSING,
                                 sizeof DER_OPTIONAL_VERSION_MISSING));
 
   Version version = Version::v3;
   ASSERT_EQ(Success, OptionalVersion(input, version));
   ASSERT_EQ(Version::v1, version);
 }
+
 } // unnamed namespace
--- 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 "cert.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 {