Bug 1038828: Replace mozilla::pkix::der::Result with uses of mozilla::pkix::Result, r=mmc
authorBrian Smith <brian@briansmith.org>
Tue, 15 Jul 2014 10:33:49 -0700
changeset 216210 f1c0bd6d13aad721c32b12b02dd3ff605f9eb375
parent 216209 9a3585421dd4a1a34c1b899b533dd6a100537c8f
child 216211 58fe2785ca6a354227e30858a578ab03d262c5dd
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmmc
bugs1038828
milestone33.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1038828: Replace mozilla::pkix::der::Result with uses of mozilla::pkix::Result, r=mmc
security/pkix/include/pkix/Result.h
security/pkix/lib/pkixcert.cpp
security/pkix/lib/pkixcheck.cpp
security/pkix/lib/pkixder.cpp
security/pkix/lib/pkixder.h
security/pkix/lib/pkixocsp.cpp
security/pkix/lib/pkixutil.h
security/pkix/test/gtest/pkixder_input_tests.cpp
security/pkix/test/gtest/pkixder_pki_types_tests.cpp
security/pkix/test/gtest/pkixder_universal_types_tests.cpp
security/pkix/test/lib/pkixtestutil.cpp
copy from security/pkix/lib/pkixutil.h
copy to security/pkix/include/pkix/Result.h
--- a/security/pkix/lib/pkixutil.h
+++ b/security/pkix/include/pkix/Result.h
@@ -17,22 +17,19 @@
  *
  * 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.
  */
 
-#ifndef mozilla_pkix__pkixutil_h
-#define mozilla_pkix__pkixutil_h
+#ifndef mozilla_pkix__Result_h
+#define mozilla_pkix__Result_h
 
-#include "pkix/enumclass.h"
-#include "pkix/pkixtypes.h"
-#include "pkixder.h"
 #include "prerror.h"
 #include "seccomon.h"
 #include "secerr.h"
 
 namespace mozilla { namespace pkix {
 
 enum Result
 {
@@ -64,172 +61,21 @@ MapSECStatus(SECStatus srv)
   }
 
   PRErrorCode error = PORT_GetError();
   switch (error) {
     case SEC_ERROR_EXTENSION_NOT_FOUND:
       return RecoverableError;
 
     case PR_INVALID_STATE_ERROR:
+    case SEC_ERROR_INVALID_ARGS:
     case SEC_ERROR_LIBRARY_FAILURE:
     case SEC_ERROR_NO_MEMORY:
       return FatalError;
   }
 
   // TODO: PORT_Assert(false); // we haven't classified the error yet
   return RecoverableError;
 }
 
-// During path building and verification, we build a linked list of BackCerts
-// from the current cert toward the end-entity certificate. The linked list
-// is used to verify properties that aren't local to the current certificate
-// and/or the direct link between the current certificate and its issuer,
-// such as name constraints.
-//
-// Each BackCert contains pointers to all the given certificate's extensions
-// so that we can parse the extension block once and then process the
-// extensions in an order that may be different than they appear in the cert.
-class BackCert
-{
-public:
-  // certDER and childCert must be valid for the lifetime of BackCert.
-  BackCert(const SECItem& certDER, EndEntityOrCA endEntityOrCA,
-           const BackCert* childCert)
-    : der(certDER)
-    , endEntityOrCA(endEntityOrCA)
-    , childCert(childCert)
-  {
-  }
-
-  Result Init();
-
-  const SECItem& GetDER() const { return der; }
-  const der::Version GetVersion() const { return version; }
-  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
-  {
-    return subjectPublicKeyInfo;
-  }
-  const SECItem* GetAuthorityInfoAccess() const
-  {
-    return MaybeSECItem(authorityInfoAccess);
-  }
-  const SECItem* GetBasicConstraints() const
-  {
-    return MaybeSECItem(basicConstraints);
-  }
-  const SECItem* GetCertificatePolicies() const
-  {
-    return MaybeSECItem(certificatePolicies);
-  }
-  const SECItem* GetExtKeyUsage() const { return MaybeSECItem(extKeyUsage); }
-  const SECItem* GetKeyUsage() const { return MaybeSECItem(keyUsage); }
-  const SECItem* GetInhibitAnyPolicy() const
-  {
-    return MaybeSECItem(inhibitAnyPolicy);
-  }
-  const SECItem* GetNameConstraints() const
-  {
-    return MaybeSECItem(nameConstraints);
-  }
-
-private:
-  const SECItem& der;
-
-public:
-  const EndEntityOrCA endEntityOrCA;
-  BackCert const* const childCert;
-
-private:
-  der::Version version;
-
-  // When parsing certificates in BackCert::Init, we don't accept empty
-  // extensions. Consequently, we don't have to store a distinction between
-  // empty extensions and extensions that weren't included. However, when
-  // *processing* extensions, we distinguish between whether an extension was
-  // included or not based on whetehr the GetXXX function for the extension
-  // returns nullptr.
-  static inline const SECItem* MaybeSECItem(const SECItem& item)
-  {
-    return item.len > 0 ? &item : nullptr;
-  }
-
-  // Helper classes to zero-initialize these fields on construction and to
-  // document that they contain non-owning pointers to the data they point
-  // to.
-  struct NonOwningSECItem : public SECItemStr {
-    NonOwningSECItem()
-    {
-      data = nullptr;
-      len = 0;
-    }
-  };
-
-  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;
-
-  NonOwningSECItem authorityInfoAccess;
-  NonOwningSECItem basicConstraints;
-  NonOwningSECItem certificatePolicies;
-  NonOwningSECItem extKeyUsage;
-  NonOwningSECItem inhibitAnyPolicy;
-  NonOwningSECItem keyUsage;
-  NonOwningSECItem nameConstraints;
-  NonOwningSECItem subjectAltName;
-
-  der::Result RememberExtension(der::Input& extnID, const SECItem& extnValue,
-                                /*out*/ bool& understood);
-
-  BackCert(const BackCert&) /* = delete */;
-  void operator=(const BackCert&); /* = delete */;
-};
-
-class NonOwningDERArray : public DERArray
-{
-public:
-  NonOwningDERArray()
-    : numItems(0)
-  {
-    // we don't need to initialize the items array because we always check
-    // numItems before accessing i.
-  }
-
-  virtual size_t GetLength() const { return numItems; }
-
-  virtual const SECItem* GetDER(size_t i) const
-  {
-    return i < numItems ? items[i] : nullptr;
-  }
-
-  Result Append(const SECItem& der)
-  {
-    if (numItems >= MAX_LENGTH) {
-      return Fail(RecoverableError, SEC_ERROR_INVALID_ARGS);
-    }
-    items[numItems] = &der;
-    ++numItems;
-    return Success;
-  }
-
-  // Public so we can static_assert on this. Keep in sync with MAX_SUBCA_COUNT.
-  static const size_t MAX_LENGTH = 8;
-private:
-  const SECItem* items[MAX_LENGTH]; // avoids any heap allocations
-  size_t numItems;
-};
-
 } } // namespace mozilla::pkix
 
-#endif // mozilla_pkix__pkixutil_h
+#endif // mozilla_pkix__Result_h
--- a/security/pkix/lib/pkixcert.cpp
+++ b/security/pkix/lib/pkixcert.cpp
@@ -25,44 +25,49 @@
 #include "pkix/bind.h"
 #include "pkixutil.h"
 
 namespace mozilla { namespace pkix {
 
 Result
 BackCert::Init()
 {
+  Result rv;
+
   // Certificate  ::=  SEQUENCE  {
   //         tbsCertificate       TBSCertificate,
   //         signatureAlgorithm   AlgorithmIdentifier,
   //         signatureValue       BIT STRING  }
 
   der::Input tbsCertificate;
 
   // The scope of |input| and |certificate| are limited to this block so we
   // don't accidentally confuse them for tbsCertificate later.
   {
     der::Input input;
-    if (input.Init(der.data, der.len) != der::Success) {
-      return MapSECStatus(SECFailure);
+    rv = input.Init(der.data, der.len);
+    if (rv != Success) {
+      return rv;
     }
     der::Input certificate;
-    if (der::ExpectTagAndGetValue(input, der::SEQUENCE, certificate)
-          != der::Success) {
-      return MapSECStatus(SECFailure);
+    rv = der::ExpectTagAndGetValue(input, der::SEQUENCE, certificate);
+    if (rv != Success) {
+      return rv;
     }
-    if (der::End(input) != der::Success) {
-      return MapSECStatus(SECFailure);
+    rv = der::End(input);
+    if (rv != Success) {
+      return rv;
     }
-    if (der::SignedData(certificate, tbsCertificate, signedData)
-          != der::Success) {
-      return MapSECStatus(SECFailure);
+    rv = der::SignedData(certificate, tbsCertificate, signedData);
+    if (rv != Success) {
+      return rv;
     }
-    if (der::End(certificate) != der::Success) {
-      return MapSECStatus(SECFailure);
+    rv = der::End(certificate);
+    if (rv != Success) {
+      return rv;
     }
   }
 
   // TBSCertificate  ::=  SEQUENCE  {
   //      version         [0]  EXPLICIT Version DEFAULT v1,
   //      serialNumber         CertificateSerialNumber,
   //      signature            AlgorithmIdentifier,
   //      issuer               Name,
@@ -71,97 +76,98 @@ BackCert::Init()
   //      subjectPublicKeyInfo SubjectPublicKeyInfo,
   //      issuerUniqueID  [1]  IMPLICIT UniqueIdentifier OPTIONAL,
   //                           -- If present, version MUST be v2 or v3
   //      subjectUniqueID [2]  IMPLICIT UniqueIdentifier OPTIONAL,
   //                           -- If present, version MUST be v2 or v3
   //      extensions      [3]  EXPLICIT Extensions OPTIONAL
   //                           -- If present, version MUST be v3
   //      }
-  if (der::OptionalVersion(tbsCertificate, version) != der::Success) {
-    return MapSECStatus(SECFailure);
+  rv = der::OptionalVersion(tbsCertificate, version);
+  if (rv != Success) {
+    return rv;
   }
-  if (der::CertificateSerialNumber(tbsCertificate, serialNumber)
-        != der::Success) {
-    return MapSECStatus(SECFailure);
+  rv = der::CertificateSerialNumber(tbsCertificate, serialNumber);
+  if (rv != Success) {
+    return rv;
   }
   // 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!
   SignatureAlgorithm signature;
-  if (der::SignatureAlgorithmIdentifier(tbsCertificate, signature)
-        != der::Success) {
-    return MapSECStatus(SECFailure);
+  rv = der::SignatureAlgorithmIdentifier(tbsCertificate, signature);
+  if (rv != Success) {
+    return rv;
   }
-  if (der::ExpectTagAndGetTLV(tbsCertificate, der::SEQUENCE, issuer)
-        != der::Success) {
-    return MapSECStatus(SECFailure);
+  rv = der::ExpectTagAndGetTLV(tbsCertificate, der::SEQUENCE, issuer);
+  if (rv != Success) {
+    return rv;
   }
-  if (der::ExpectTagAndGetValue(tbsCertificate, der::SEQUENCE, validity)
-        != der::Success) {
-    return MapSECStatus(SECFailure);
+  rv = der::ExpectTagAndGetValue(tbsCertificate, der::SEQUENCE, validity);
+  if (rv != Success) {
+    return rv;
   }
   // 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.
-  if (der::ExpectTagAndGetTLV(tbsCertificate, der::SEQUENCE, subject)
-        != der::Success) {
-    return MapSECStatus(SECFailure);
+  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.
-  if (der::ExpectTagAndGetTLV(tbsCertificate, der::SEQUENCE,
-                              subjectPublicKeyInfo) != der::Success) {
-    return MapSECStatus(SECFailure);
+  rv = der::ExpectTagAndGetTLV(tbsCertificate, der::SEQUENCE,
+                               subjectPublicKeyInfo);
+  if (rv != Success) {
+    return rv;
   }
 
   static const uint8_t CSC = der::CONTEXT_SPECIFIC | der::CONSTRUCTED;
 
   // RFC 5280 says: "These fields MUST only appear if the version is 2 or 3
   // (Section 4.1.2.1). These fields MUST NOT appear if the version is 1."
   if (version != der::Version::v1) {
 
     // Ignore issuerUniqueID if present.
     if (tbsCertificate.Peek(CSC | 1)) {
-      if (der::ExpectTagAndSkipValue(tbsCertificate, CSC | 1) != der::Success) {
-        return MapSECStatus(SECFailure);
+      rv = der::ExpectTagAndSkipValue(tbsCertificate, CSC | 1);
+      if (rv != Success) {
+        return rv;
       }
     }
 
     // Ignore subjectUniqueID if present.
     if (tbsCertificate.Peek(CSC | 2)) {
-      if (der::ExpectTagAndSkipValue(tbsCertificate, CSC | 2) != der::Success) {
-        return MapSECStatus(SECFailure);
+      rv = der::ExpectTagAndSkipValue(tbsCertificate, CSC | 2);
+      if (rv != Success) {
+        return rv;
       }
     }
   }
 
   // Extensions were added in v3, so only accept extensions in v3 certificates.
   if (version == der::Version::v3) {
-    if (der::OptionalExtensions(tbsCertificate, CSC | 3,
-                                bind(&BackCert::RememberExtension, this, _1, _2,
-                                     _3)) != der::Success) {
-      return MapSECStatus(SECFailure);
+    rv = der::OptionalExtensions(tbsCertificate, CSC | 3,
+                                 bind(&BackCert::RememberExtension, this, _1,
+                                      _2, _3));
+    if (rv != Success) {
+      return rv;
     }
   }
 
-  if (der::End(tbsCertificate) != der::Success) {
-    return MapSECStatus(SECFailure);
-  }
-
-  return Success;
+  return der::End(tbsCertificate);
 }
 
-der::Result
+Result
 BackCert::RememberExtension(der::Input& extnID, const SECItem& extnValue,
                             /*out*/ bool& understood)
 {
   understood = false;
 
   // python DottedOIDToCode.py id-ce-keyUsage 2.5.29.15
   static const uint8_t id_ce_keyUsage[] = {
     0x55, 0x1d, 0x0f
@@ -242,12 +248,12 @@ BackCert::RememberExtension(der::Input& 
     if (out->len != 0) {
       // Duplicate extension
       return der::Fail(SEC_ERROR_EXTENSION_VALUE_INVALID);
     }
     *out = extnValue;
     understood = true;
   }
 
-  return der::Success;
+  return Success;
 }
 
 } } // namespace mozilla::pkix
--- a/security/pkix/lib/pkixcheck.cpp
+++ b/security/pkix/lib/pkixcheck.cpp
@@ -31,41 +31,36 @@
 #include "pkixutil.h"
 
 namespace mozilla { namespace pkix {
 
 Result
 CheckValidity(const SECItem& encodedValidity, PRTime time)
 {
   der::Input validity;
-  if (validity.Init(encodedValidity.data, encodedValidity.len)
-        != der::Success) {
+  if (validity.Init(encodedValidity.data, encodedValidity.len) != Success) {
     return Fail(RecoverableError, SEC_ERROR_EXPIRED_CERTIFICATE);
   }
   PRTime notBefore;
-  if (der::TimeChoice(validity, notBefore) != der::Success) {
+  if (der::TimeChoice(validity, notBefore) != Success) {
     return Fail(RecoverableError, SEC_ERROR_EXPIRED_CERTIFICATE);
   }
   if (time < notBefore) {
     return Fail(RecoverableError, SEC_ERROR_EXPIRED_CERTIFICATE);
   }
 
   PRTime notAfter;
-  if (der::TimeChoice(validity, notAfter) != der::Success) {
+  if (der::TimeChoice(validity, notAfter) != Success) {
     return Fail(RecoverableError, SEC_ERROR_EXPIRED_CERTIFICATE);
   }
   if (time > notAfter) {
     return Fail(RecoverableError, SEC_ERROR_EXPIRED_CERTIFICATE);
   }
 
-  if (der::End(validity) != der::Success) {
-    return MapSECStatus(SECFailure);
-  }
-
-  return Success;
+  return der::End(validity);
 }
 
 // 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)
 {
@@ -88,34 +83,34 @@ CheckKeyUsage(EndEntityOrCA endEntityOrC
     //
     // TODO: Users may configure arbitrary certificates as trust anchors, not
     // just roots. We should only allow a certificate without a key usage to be
     // used as a CA when it is self-issued and self-signed.
     return Success;
   }
 
   der::Input input;
-  if (input.Init(encodedKeyUsage->data, encodedKeyUsage->len) != der::Success) {
+  if (input.Init(encodedKeyUsage->data, encodedKeyUsage->len) != Success) {
     return Fail(RecoverableError, SEC_ERROR_INADEQUATE_KEY_USAGE);
   }
   der::Input value;
-  if (der::ExpectTagAndGetValue(input, der::BIT_STRING, value) != der::Success) {
+  if (der::ExpectTagAndGetValue(input, der::BIT_STRING, value) != Success) {
     return Fail(RecoverableError, SEC_ERROR_INADEQUATE_KEY_USAGE);
   }
 
   uint8_t numberOfPaddingBits;
-  if (value.Read(numberOfPaddingBits) != der::Success) {
+  if (value.Read(numberOfPaddingBits) != Success) {
     return Fail(RecoverableError, SEC_ERROR_INADEQUATE_KEY_USAGE);
   }
   if (numberOfPaddingBits > 7) {
     return Fail(RecoverableError, SEC_ERROR_INADEQUATE_KEY_USAGE);
   }
 
   uint8_t bits;
-  if (value.Read(bits) != der::Success) {
+  if (value.Read(bits) != Success) {
     // Reject empty bit masks.
     return Fail(RecoverableError, SEC_ERROR_INADEQUATE_KEY_USAGE);
   }
 
   // The most significant bit is numbered 0 (digitalSignature) and the least
   // significant bit is numbered 7 (encipherOnly), and the padding is in the
   // least significant bits of the last byte. The numbering of bits in a byte
   // is backwards from how we usually interpret them.
@@ -164,17 +159,17 @@ CheckKeyUsage(EndEntityOrCA endEntityOrC
     // extension (Section 4.2.1.9) MUST also be asserted."
     if ((bits & KeyUsageToBitMask(KeyUsage::keyCertSign)) != 0) {
       return Fail(RecoverableError, SEC_ERROR_INADEQUATE_KEY_USAGE);
     }
   }
 
   // The padding applies to the last byte, so skip to the last byte.
   while (!value.AtEnd()) {
-    if (value.Read(bits) != der::Success) {
+    if (value.Read(bits) != Success) {
       return Fail(RecoverableError, SEC_ERROR_INADEQUATE_KEY_USAGE);
     }
   }
 
   // All of the padding bits must be zero, according to DER rules.
   uint8_t paddingMask = static_cast<uint8_t>((1 << numberOfPaddingBits) - 1);
   if ((bits & paddingMask) != 0) {
     return Fail(RecoverableError, SEC_ERROR_INADEQUATE_KEY_USAGE);
@@ -202,17 +197,17 @@ bool CertPolicyId::IsAnyPolicy() const
          (numBytes == anyPolicy.numBytes &&
           !memcmp(bytes, anyPolicy.bytes, anyPolicy.numBytes));
 }
 
 // PolicyInformation ::= SEQUENCE {
 //         policyIdentifier   CertPolicyId,
 //         policyQualifiers   SEQUENCE SIZE (1..MAX) OF
 //                                 PolicyQualifierInfo OPTIONAL }
-inline der::Result
+inline Result
 CheckPolicyInformation(der::Input& input, EndEntityOrCA endEntityOrCA,
                        const CertPolicyId& requiredPolicy,
                        /*in/out*/ bool& found)
 {
   if (input.MatchTLV(der::OIDTag, requiredPolicy.numBytes,
                      requiredPolicy.bytes)) {
     found = true;
   } else if (endEntityOrCA == EndEntityOrCA::MustBeCA &&
@@ -226,17 +221,17 @@ CheckPolicyInformation(der::Input& input
   // that Section 6, which defines validation, does not require any matching of
   // qualifiers. Thus, doing anything with the policy qualifiers would be a
   // waste of time and a source of potential incompatibilities, so we just
   // ignore them.
 
   // Skip unmatched OID and/or policyQualifiers
   input.SkipToEnd();
 
-  return der::Success;
+  return Success;
 }
 
 // certificatePolicies ::= SEQUENCE SIZE (1..MAX) OF PolicyInformation
 Result
 CheckCertificatePolicies(EndEntityOrCA endEntityOrCA,
                          const SECItem* encodedCertificatePolicies,
                          const SECItem* encodedInhibitAnyPolicy,
                          TrustLevel trustLevel,
@@ -273,86 +268,86 @@ CheckCertificatePolicies(EndEntityOrCA e
   if (!encodedCertificatePolicies) {
     return Fail(RecoverableError, SEC_ERROR_POLICY_VALIDATION_FAILED);
   }
 
   bool found = false;
 
   der::Input input;
   if (input.Init(encodedCertificatePolicies->data,
-                 encodedCertificatePolicies->len) != der::Success) {
+                 encodedCertificatePolicies->len) != Success) {
     return Fail(RecoverableError, SEC_ERROR_POLICY_VALIDATION_FAILED);
   }
   if (der::NestedOf(input, der::SEQUENCE, der::SEQUENCE, der::EmptyAllowed::No,
                     bind(CheckPolicyInformation, _1, endEntityOrCA,
-                         requiredPolicy, ref(found))) != der::Success) {
+                         requiredPolicy, ref(found))) != Success) {
     return Fail(RecoverableError, SEC_ERROR_POLICY_VALIDATION_FAILED);
   }
-  if (der::End(input) != der::Success) {
+  if (der::End(input) != Success) {
     return Fail(RecoverableError, SEC_ERROR_POLICY_VALIDATION_FAILED);
   }
   if (!found) {
     return Fail(RecoverableError, SEC_ERROR_POLICY_VALIDATION_FAILED);
   }
 
   return Success;
 }
 
 static const long UNLIMITED_PATH_LEN = -1; // must be less than zero
 
 //  BasicConstraints ::= SEQUENCE {
 //          cA                      BOOLEAN DEFAULT FALSE,
 //          pathLenConstraint       INTEGER (0..MAX) OPTIONAL }
-static der::Result
+static Result
 DecodeBasicConstraints(der::Input& input, /*out*/ bool& isCA,
                        /*out*/ long& pathLenConstraint)
 {
   // TODO(bug 989518): cA is by default false. According to DER, default
   // values must not be explicitly encoded in a SEQUENCE. So, if this
   // value is present and false, it is an encoding error. However, Go Daddy
   // has issued many certificates with this improper encoding, so we can't
   // enforce this yet (hence passing true for allowInvalidExplicitEncoding
   // to der::OptionalBoolean).
-  if (der::OptionalBoolean(input, true, isCA) != der::Success) {
+  if (der::OptionalBoolean(input, true, isCA) != Success) {
     return der::Fail(SEC_ERROR_EXTENSION_VALUE_INVALID);
   }
 
   // TODO(bug 985025): If isCA is false, pathLenConstraint MUST NOT
   // be included (as per RFC 5280 section 4.2.1.9), but for compatibility
   // reasons, we don't check this for now.
   if (OptionalInteger(input, UNLIMITED_PATH_LEN, pathLenConstraint)
-        != der::Success) {
+        != Success) {
     return der::Fail(SEC_ERROR_EXTENSION_VALUE_INVALID);
   }
 
-  return der::Success;
+  return Success;
 }
 
 // RFC5280 4.2.1.9. Basic Constraints (id-ce-basicConstraints)
 Result
 CheckBasicConstraints(EndEntityOrCA endEntityOrCA,
                       const SECItem* encodedBasicConstraints,
                       const der::Version version, TrustLevel trustLevel,
                       unsigned int subCACount)
 {
   bool isCA = false;
   long pathLenConstraint = UNLIMITED_PATH_LEN;
 
   if (encodedBasicConstraints) {
     der::Input input;
     if (input.Init(encodedBasicConstraints->data,
-                   encodedBasicConstraints->len) != der::Success) {
+                   encodedBasicConstraints->len) != Success) {
       return Fail(RecoverableError, SEC_ERROR_EXTENSION_VALUE_INVALID);
     }
     if (der::Nested(input, der::SEQUENCE,
                     bind(DecodeBasicConstraints, _1, ref(isCA),
-                         ref(pathLenConstraint))) != der::Success) {
+                         ref(pathLenConstraint))) != Success) {
       return Fail(RecoverableError, SEC_ERROR_EXTENSION_VALUE_INVALID);
     }
-    if (der::End(input) != der::Success) {
+    if (der::End(input) != Success) {
       return Fail(RecoverableError, SEC_ERROR_EXTENSION_VALUE_INVALID);
     }
   } else {
     // "If the basic constraints extension is not present in a version 3
     //  certificate, or the extension is present but the cA boolean is not
     //  asserted, then the certified public key MUST NOT be used to verify
     //  certificate signatures."
     //
@@ -461,17 +456,17 @@ CheckNameConstraints(const SECItem& enco
     } while (currentName != names);
   }
 
   return Success;
 }
 
 // 4.2.1.12. Extended Key Usage (id-ce-extKeyUsage)
 
-static der::Result
+static Result
 MatchEKU(der::Input& value, KeyPurposeId requiredEKU,
          EndEntityOrCA endEntityOrCA, /*in/out*/ bool& found,
          /*in/out*/ bool& foundOCSPSigning)
 {
   // See Section 5.9 of "A Layman's Guide to a Subset of ASN.1, BER, and DER"
   // for a description of ASN.1 DER encoding of OIDs.
 
   // id-pkix  OBJECT IDENTIFIER  ::=
@@ -541,17 +536,17 @@ MatchEKU(der::Input& value, KeyPurposeId
       foundOCSPSigning = true;
     }
   } else if (value.MatchRest(ocsp)) {
     foundOCSPSigning = true;
   }
 
   value.SkipToEnd(); // ignore unmatched OIDs.
 
-  return der::Success;
+  return Success;
 }
 
 Result
 CheckExtendedKeyUsage(EndEntityOrCA endEntityOrCA,
                       const SECItem* encodedExtendedKeyUsage,
                       KeyPurposeId requiredEKU)
 {
   // XXX: We're using SEC_ERROR_INADEQUATE_CERT_TYPE here so that callers can
@@ -561,26 +556,26 @@ CheckExtendedKeyUsage(EndEntityOrCA endE
 
   bool foundOCSPSigning = false;
 
   if (encodedExtendedKeyUsage) {
     bool found = requiredEKU == KeyPurposeId::anyExtendedKeyUsage;
 
     der::Input input;
     if (input.Init(encodedExtendedKeyUsage->data,
-                   encodedExtendedKeyUsage->len) != der::Success) {
+                   encodedExtendedKeyUsage->len) != Success) {
       return Fail(RecoverableError, SEC_ERROR_INADEQUATE_CERT_TYPE);
     }
     if (der::NestedOf(input, der::SEQUENCE, der::OIDTag, der::EmptyAllowed::No,
                       bind(MatchEKU, _1, requiredEKU, endEntityOrCA,
                            ref(found), ref(foundOCSPSigning)))
-          != der::Success) {
+          != Success) {
       return Fail(RecoverableError, SEC_ERROR_INADEQUATE_CERT_TYPE);
     }
-    if (der::End(input) != der::Success) {
+    if (der::End(input) != Success) {
       return Fail(RecoverableError, SEC_ERROR_INADEQUATE_CERT_TYPE);
     }
 
     // If the EKU extension was included, then the required EKU must be in the
     // list.
     if (!found) {
       return Fail(RecoverableError, SEC_ERROR_INADEQUATE_CERT_TYPE);
     }
--- a/security/pkix/lib/pkixder.cpp
+++ b/security/pkix/lib/pkixder.cpp
@@ -28,59 +28,64 @@
 
 namespace mozilla { namespace pkix { namespace der {
 
 // not inline
 Result
 Fail(PRErrorCode errorCode)
 {
   PR_SetError(errorCode, 0);
-  return Failure;
+  return MapSECStatus(SECFailure);
 }
 
 namespace internal {
 
 // Too complicated to be inline
 Result
 ExpectTagAndGetLength(Input& input, uint8_t expectedTag, uint16_t& length)
 {
   PR_ASSERT((expectedTag & 0x1F) != 0x1F); // high tag number form not allowed
 
   uint8_t tag;
-  if (input.Read(tag) != Success) {
-    return Failure;
+  Result rv;
+  rv = input.Read(tag);
+  if (rv != Success) {
+    return rv;
   }
 
   if (tag != expectedTag) {
     return Fail(SEC_ERROR_BAD_DER);
   }
 
   // The short form of length is a single byte with the high order bit set
   // to zero. The long form of length is one byte with the high order bit
   // set, followed by N bytes, where N is encoded in the lowest 7 bits of
   // the first byte.
   uint8_t length1;
-  if (input.Read(length1) != Success) {
-    return Failure;
+  rv = input.Read(length1);
+  if (rv != Success) {
+    return rv;
   }
   if (!(length1 & 0x80)) {
     length = length1;
   } else if (length1 == 0x81) {
     uint8_t length2;
-    if (input.Read(length2) != Success) {
-      return Failure;
+    rv = input.Read(length2);
+    if (rv != Success) {
+      return rv;
     }
     if (length2 < 128) {
       // Not shortest possible encoding
       return Fail(SEC_ERROR_BAD_DER);
     }
     length = length2;
   } else if (length1 == 0x82) {
-    if (input.Read(length) != Success) {
-      return Failure;
+    rv = input.Read(length);
+    if (rv != Success) {
+      return rv;
     }
     if (length < 256) {
       // Not shortest possible encoding
       return Fail(SEC_ERROR_BAD_DER);
     }
   } else {
     // We don't support lengths larger than 2^16 - 1.
     return Fail(SEC_ERROR_BAD_DER);
@@ -237,30 +242,34 @@ SignatureAlgorithmOIDValue(Input& algori
 }
 
 template <typename OidValueParser, typename Algorithm>
 Result
 AlgorithmIdentifier(OidValueParser oidValueParser, Input& input,
                     /*out*/ Algorithm& algorithm)
 {
   Input value;
-  if (ExpectTagAndGetValue(input, SEQUENCE, value) != Success) {
-    return Failure;
+  Result rv = ExpectTagAndGetValue(input, SEQUENCE, value);
+  if (rv != Success) {
+    return rv;
   }
 
   Input algorithmID;
-  if (ExpectTagAndGetValue(value, der::OIDTag, algorithmID) != Success) {
-    return Failure;
+  rv = ExpectTagAndGetValue(value, der::OIDTag, algorithmID);
+  if (rv != Success) {
+    return rv;
   }
-  if (oidValueParser(algorithmID, algorithm) != Success) {
-    return Failure;
+  rv = oidValueParser(algorithmID, algorithm);
+  if (rv != Success) {
+    return rv;
   }
 
-  if (OptionalNull(value) != Success) {
-    return Failure;
+  rv = OptionalNull(value);
+  if (rv != Success) {
+    return rv;
   }
 
   return End(value);
 }
 
 } // unnamed namespace
 
 Result
@@ -277,31 +286,35 @@ DigestAlgorithmIdentifier(Input& input, 
 }
 
 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;
+  Result rv;
+  rv = ExpectTagAndGetValue(input, SEQUENCE, tbs);
+  if (rv != Success) {
+    return rv;
   }
 
-  if (SignatureAlgorithmIdentifier(input, signedData.algorithm) != Success) {
-    return Failure;
+  rv = input.GetSECItem(siBuffer, mark, signedData.data);
+  if (rv != Success) {
+    return rv;
   }
 
-  if (ExpectTagAndGetValue(input, BIT_STRING, signedData.signature)
-        != Success) {
-    return Failure;
+  rv = SignatureAlgorithmIdentifier(input, signedData.algorithm);
+  if (rv != Success) {
+    return rv;
+  }
+
+  rv = ExpectTagAndGetValue(input, BIT_STRING, signedData.signature);
+  if (rv != Success) {
+    return rv;
   }
   if (signedData.signature.len == 0) {
     return Fail(SEC_ERROR_BAD_SIGNATURE);
   }
   unsigned int unusedBitsAtEnd = signedData.signature.data[0];
   // XXX: Really the constraint should be that unusedBitsAtEnd must be less
   // than 7. But, we suspect there are no real-world OCSP responses or X.509
   // certificates with non-zero unused bits. It seems like NSS assumes this in
@@ -330,22 +343,24 @@ ReadDigit(Input& input, /*out*/ int& val
   value = b - '0';
   return Success;
 }
 
 static inline Result
 ReadTwoDigits(Input& input, int minValue, int maxValue, /*out*/ int& value)
 {
   int hi;
-  if (ReadDigit(input, hi) != Success) {
-    return Failure;
+  Result rv = ReadDigit(input, hi);
+  if (rv != Success) {
+    return rv;
   }
   int lo;
-  if (ReadDigit(input, lo) != Success) {
-    return Failure;
+  rv = ReadDigit(input, lo);
+  if (rv != Success) {
+    return rv;
   }
   value = (hi * 10) + lo;
   if (value < minValue || value > maxValue) {
     return Fail(SEC_ERROR_INVALID_TIME);
   }
   return Success;
 }
 
@@ -366,32 +381,36 @@ namespace internal {
 // must always be in the format YYMMDDHHMMSSZ. Timezone formats of the form
 // +HH:MM or -HH:MM or NOT accepted.
 Result
 TimeChoice(Input& tagged, uint8_t expectedTag, /*out*/ PRTime& time)
 {
   int days;
 
   Input input;
-  if (ExpectTagAndGetValue(tagged, expectedTag, input) != Success) {
-    return Failure;
+  Result rv = ExpectTagAndGetValue(tagged, expectedTag, input);
+  if (rv != Success) {
+    return rv;
   }
 
   int yearHi;
   int yearLo;
   if (expectedTag == GENERALIZED_TIME) {
-    if (ReadTwoDigits(input, 0, 99, yearHi) != Success) {
-      return Failure;
+    rv = ReadTwoDigits(input, 0, 99, yearHi);
+    if (rv != Success) {
+      return rv;
     }
-    if (ReadTwoDigits(input, 0, 99, yearLo) != Success) {
-      return Failure;
+    rv = ReadTwoDigits(input, 0, 99, yearLo);
+    if (rv != Success) {
+      return rv;
     }
   } else if (expectedTag == UTCTime) {
-    if (ReadTwoDigits(input, 0, 99, yearLo) != Success) {
-      return Failure;
+    rv = ReadTwoDigits(input, 0, 99, yearLo);
+    if (rv != Success) {
+      return rv;
     }
     yearHi = yearLo >= 50 ? 19 : 20;
   } else {
     PR_NOT_REACHED("invalid tag given to TimeChoice");
     return Fail(SEC_ERROR_INVALID_TIME);
   }
   int year = (yearHi * 100) + yearLo;
   if (year < 1970) {
@@ -404,18 +423,19 @@ TimeChoice(Input& tagged, uint8_t expect
     days = daysBeforeYear(year) - daysBeforeYear(1970);
     // We subtract 1 because we're interested in knowing how many days there
     // were *before* the given year, relative to 1970.
   } else {
     days = 0;
   }
 
   int month;
-  if (ReadTwoDigits(input, 1, 12, month) != Success) {
-    return Failure;
+  rv = ReadTwoDigits(input, 1, 12, month);
+  if (rv != Success) {
+    return rv;
   }
   int daysInMonth;
   static const int jan = 31;
   const int feb = ((year % 4 == 0) &&
                    ((year % 100 != 0) || (year % 400 == 0)))
                 ? 29
                 : 28;
   static const int mar = 31;
@@ -453,32 +473,36 @@ TimeChoice(Input& tagged, uint8_t expect
                                         jul + aug + sep + oct + nov;
              break;
     default:
       PR_NOT_REACHED("month already bounds-checked by ReadTwoDigits");
       return Fail(PR_INVALID_STATE_ERROR);
   }
 
   int dayOfMonth;
-  if (ReadTwoDigits(input, 1, daysInMonth, dayOfMonth) != Success) {
-    return Failure;
+  rv = ReadTwoDigits(input, 1, daysInMonth, dayOfMonth);
+  if (rv != Success) {
+    return rv;
   }
   days += dayOfMonth - 1;
 
   int hours;
-  if (ReadTwoDigits(input, 0, 23, hours) != Success) {
-    return Failure;
+  rv = ReadTwoDigits(input, 0, 23, hours);
+  if (rv != Success) {
+    return rv;
   }
   int minutes;
-  if (ReadTwoDigits(input, 0, 59, minutes) != Success) {
-    return Failure;
+  rv = ReadTwoDigits(input, 0, 59, minutes);
+  if (rv != Success) {
+    return rv;
   }
   int seconds;
-  if (ReadTwoDigits(input, 0, 59, seconds) != Success) {
-    return Failure;
+  rv = ReadTwoDigits(input, 0, 59, seconds);
+  if (rv != Success) {
+    return rv;
   }
 
   uint8_t b;
   if (input.Read(b) != Success) {
     return Fail(SEC_ERROR_INVALID_TIME);
   }
   if (b != 'Z') {
     return Fail(SEC_ERROR_INVALID_TIME); // TODO: better error code?
--- a/security/pkix/lib/pkixder.h
+++ b/security/pkix/lib/pkixder.h
@@ -21,31 +21,30 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
 
 #ifndef mozilla_pkix__pkixder_h
 #define mozilla_pkix__pkixder_h
 
 // Expect* functions advance the input mark and return Success if the input
-// matches the given criteria; they return Failure with the input mark in an
-// undefined state if the input does not match the criteria.
+// matches the given criteria; they fail with the input mark in an undefined
+// state if the input does not match the criteria.
 //
 // Match* functions advance the input mark and return true if the input matches
 // the given criteria; they return false without changing the input mark if the
 // 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.
+// they are able to do so; otherwise they fail with the input mark in an
+// undefined state.
 
 #include "pkix/enumclass.h"
-#include "pkix/nullptr.h"
 #include "pkix/pkixtypes.h"
-#include "prerror.h"
+#include "pkix/Result.h"
 #include "prtime.h"
 #include "secerr.h"
 #include "secoidt.h"
 #include "stdint.h"
 
 typedef struct CERTSignedDataStr CERTSignedData;
 
 namespace mozilla { namespace pkix { namespace der {
@@ -72,22 +71,16 @@ enum Tag
   NULLTag = UNIVERSAL | 0x05,
   OIDTag = UNIVERSAL | 0x06,
   ENUMERATED = UNIVERSAL | 0x0a,
   SEQUENCE = UNIVERSAL | CONSTRUCTED | 0x10, // 0x30
   UTCTime = UNIVERSAL | 0x17,
   GENERALIZED_TIME = UNIVERSAL | 0x18,
 };
 
-enum Result
-{
-  Failure = -1,
-  Success = 0
-};
-
 MOZILLA_PKIX_ENUM_CLASS EmptyAllowed { No = 0, Yes = 1 };
 
 Result Fail(PRErrorCode errorCode);
 
 class Input
 {
 public:
   Input()
@@ -112,44 +105,47 @@ public:
     this->input = data;
     this->end = data + len;
 
     return Success;
   }
 
   Result Expect(const uint8_t* expected, uint16_t expectedLen)
   {
-    if (EnsureLength(expectedLen) != Success) {
-      return Failure;
+    Result rv = EnsureLength(expectedLen);
+    if (rv != Success) {
+      return rv;
     }
     if (memcmp(input, expected, expectedLen)) {
       return Fail(SEC_ERROR_BAD_DER);
     }
     input += expectedLen;
     return Success;
   }
 
   bool Peek(uint8_t expectedByte) const
   {
     return input < end && *input == expectedByte;
   }
 
   Result Read(uint8_t& out)
   {
-    if (EnsureLength(1) != Success) {
-      return Failure;
+    Result rv = EnsureLength(1);
+    if (rv != Success) {
+      return rv;
     }
     out = *input++;
     return Success;
   }
 
   Result Read(uint16_t& out)
   {
-    if (EnsureLength(2) != Success) {
-      return Failure;
+    Result rv = EnsureLength(2);
+    if (rv != Success) {
+      return rv;
     }
     out = *input++;
     out <<= 8u;
     out |= *input++;
     return Success;
   }
 
   template <uint16_t N>
@@ -190,39 +186,43 @@ public:
       return false;
     }
     input += totalLen;
     return true;
   }
 
   Result Skip(uint16_t len)
   {
-    if (EnsureLength(len) != Success) {
-      return Failure;
+    Result rv = EnsureLength(len);
+    if (rv != Success) {
+      return rv;
     }
     input += len;
     return Success;
   }
 
   Result Skip(uint16_t len, Input& skippedInput)
   {
-    if (EnsureLength(len) != Success) {
-      return Failure;
+    Result rv = EnsureLength(len);
+    if (rv != Success) {
+      return rv;
     }
-    if (skippedInput.Init(input, len) != Success) {
-      return Failure;
+    rv = skippedInput.Init(input, len);
+    if (rv != Success) {
+      return rv;
     }
     input += len;
     return Success;
   }
 
   Result Skip(uint16_t len, SECItem& skippedItem)
   {
-    if (EnsureLength(len) != Success) {
-      return Failure;
+    Result rv = EnsureLength(len);
+    if (rv != Success) {
+      return rv;
     }
     skippedItem.type = siBuffer;
     skippedItem.data = const_cast<uint8_t*>(input);
     skippedItem.len = len;
     input += len;
     return Success;
   }
 
@@ -275,18 +275,19 @@ private:
 
 inline Result
 ExpectTagAndLength(Input& input, uint8_t expectedTag, uint8_t expectedLength)
 {
   PR_ASSERT((expectedTag & 0x1F) != 0x1F); // high tag number form not allowed
   PR_ASSERT(expectedLength < 128); // must be a single-byte length
 
   uint16_t tagAndLength;
-  if (input.Read(tagAndLength) != Success) {
-    return Failure;
+  Result rv = input.Read(tagAndLength);
+  if (rv != Success) {
+    return rv;
   }
 
   uint16_t expectedTagAndLength = static_cast<uint16_t>(expectedTag << 8);
   expectedTagAndLength |= expectedLength;
 
   if (tagAndLength != expectedTagAndLength) {
     return Fail(SEC_ERROR_BAD_DER);
   }
@@ -307,54 +308,59 @@ ExpectTagAndSkipLength(Input& input, uin
   uint16_t ignored;
   return internal::ExpectTagAndGetLength(input, expectedTag, ignored);
 }
 
 inline Result
 ExpectTagAndSkipValue(Input& input, uint8_t tag)
 {
   uint16_t length;
-  if (internal::ExpectTagAndGetLength(input, tag, length) != Success) {
-    return Failure;
+  Result rv = internal::ExpectTagAndGetLength(input, tag, length);
+  if (rv != Success) {
+    return rv;
   }
   return input.Skip(length);
 }
 
 inline Result
 ExpectTagAndGetValue(Input& input, uint8_t tag, /*out*/ SECItem& value)
 {
   uint16_t length;
-  if (internal::ExpectTagAndGetLength(input, tag, length) != Success) {
-    return Failure;
+  Result rv = internal::ExpectTagAndGetLength(input, tag, length);
+  if (rv != Success) {
+    return rv;
   }
   return input.Skip(length, value);
 }
 
 inline Result
 ExpectTagAndGetValue(Input& input, uint8_t tag, /*out*/ Input& value)
 {
   uint16_t length;
-  if (internal::ExpectTagAndGetLength(input, tag, length) != Success) {
-    return Failure;
+  Result rv = internal::ExpectTagAndGetLength(input, tag, length);
+  if (rv != Success) {
+    return rv;
   }
   return input.Skip(length, value);
 }
 
 // Like ExpectTagAndGetValue, except the output SECItem will contain the
 // encoded tag and length along with the value.
 inline Result
 ExpectTagAndGetTLV(Input& input, uint8_t tag, /*out*/ SECItem& tlv)
 {
   Input::Mark mark(input.GetMark());
   uint16_t length;
-  if (internal::ExpectTagAndGetLength(input, tag, length) != Success) {
-    return Failure;
+  Result rv = internal::ExpectTagAndGetLength(input, tag, length);
+  if (rv != Success) {
+    return rv;
   }
-  if (input.Skip(length) != Success) {
-    return Failure;
+  rv = input.Skip(length);
+  if (rv != Success) {
+    return rv;
   }
   return input.GetSECItem(siBuffer, mark, tlv);
 }
 
 inline Result
 End(Input& input)
 {
   if (!input.AtEnd()) {
@@ -364,38 +370,42 @@ End(Input& input)
   return Success;
 }
 
 template <typename Decoder>
 inline Result
 Nested(Input& input, uint8_t tag, Decoder decoder)
 {
   Input nested;
-  if (ExpectTagAndGetValue(input, tag, nested) != Success) {
-    return Failure;
+  Result rv = ExpectTagAndGetValue(input, tag, nested);
+  if (rv != Success) {
+    return rv;
   }
-  if (decoder(nested) != Success) {
-    return Failure;
+  rv = decoder(nested);
+  if (rv != Success) {
+    return rv;
   }
   return End(nested);
 }
 
 template <typename Decoder>
 inline Result
 Nested(Input& input, uint8_t outerTag, uint8_t innerTag, Decoder decoder)
 {
   // XXX: This doesn't work (in VS2010):
   // return Nested(input, outerTag, bind(Nested, _1, innerTag, decoder));
 
   Input nestedInput;
-  if (ExpectTagAndGetValue(input, outerTag, nestedInput) != Success) {
-    return Failure;
+  Result rv = ExpectTagAndGetValue(input, outerTag, nestedInput);
+  if (rv != Success) {
+    return rv;
   }
-  if (Nested(nestedInput, innerTag, decoder) != Success) {
-    return Failure;
+  rv = Nested(nestedInput, innerTag, decoder);
+  if (rv != Success) {
+    return rv;
   }
   return End(nestedInput);
 }
 
 // This can be used to decode constructs like this:
 //
 //     ...
 //     foos SEQUENCE OF Foo,
@@ -413,30 +423,32 @@ Nested(Input& input, uint8_t outerTag, u
 // In this example, Foo will get called once for each element of foos.
 //
 template <typename Decoder>
 inline Result
 NestedOf(Input& input, uint8_t outerTag, uint8_t innerTag,
          EmptyAllowed mayBeEmpty, Decoder decoder)
 {
   Input inner;
-  if (ExpectTagAndGetValue(input, outerTag, inner) != Success) {
-    return Failure;
+  Result rv = ExpectTagAndGetValue(input, outerTag, inner);
+  if (rv != Success) {
+    return rv;
   }
 
   if (inner.AtEnd()) {
     if (mayBeEmpty != EmptyAllowed::Yes) {
       return Fail(SEC_ERROR_BAD_DER);
     }
     return Success;
   }
 
   do {
-    if (Nested(inner, innerTag, decoder) != Success) {
-      return Failure;
+    rv = Nested(inner, innerTag, decoder);
+    if (rv != Success) {
+      return rv;
     }
   } while (!inner.AtEnd());
 
   return Success;
 }
 
 // Universal types
 
@@ -445,42 +457,46 @@ namespace internal {
 // This parser will only parse values between 0..127. If this range is
 // increased then callers will need to be changed.
 template <typename T> inline Result
 IntegralValue(Input& input, uint8_t tag, T& value)
 {
   // Conveniently, all the Integers that we actually have to be able to parse
   // are positive and very small. Consequently, this parser is *much* simpler
   // than a general Integer parser would need to be.
-  if (ExpectTagAndLength(input, tag, 1) != Success) {
-    return Failure;
+  Result rv = ExpectTagAndLength(input, tag, 1);
+  if (rv != Success) {
+    return rv;
   }
   uint8_t valueByte;
-  if (input.Read(valueByte) != Success) {
-    return Failure;
+  rv = input.Read(valueByte);
+  if (rv != Success) {
+    return rv;
   }
   if (valueByte & 0x80) { // negative
     return Fail(SEC_ERROR_BAD_DER);
   }
   value = valueByte;
   return Success;
 }
 
 } // namespace internal
 
 inline Result
 Boolean(Input& input, /*out*/ bool& value)
 {
-  if (ExpectTagAndLength(input, BOOLEAN, 1) != Success) {
-    return Failure;
+  Result rv = ExpectTagAndLength(input, BOOLEAN, 1);
+  if (rv != Success) {
+    return rv;
   }
 
   uint8_t intValue;
-  if (input.Read(intValue) != Success) {
-    return Failure;
+  rv = input.Read(intValue);
+  if (rv != Success) {
+    return rv;
   }
   switch (intValue) {
     case 0: value = false; return Success;
     case 0xFF: value = true; return Success;
     default:
       return Fail(SEC_ERROR_BAD_DER);
   }
 }
@@ -490,18 +506,19 @@ Boolean(Input& input, /*out*/ bool& valu
 // TODO(bug 989518): For compatibility reasons, in some places we allow
 // invalid encodings with the explicit default value.
 inline Result
 OptionalBoolean(Input& input, bool allowInvalidExplicitEncoding,
                 /*out*/ bool& value)
 {
   value = false;
   if (input.Peek(BOOLEAN)) {
-    if (Boolean(input, value) != Success) {
-      return Failure;
+    Result rv = Boolean(input, value);
+    if (rv != Success) {
+      return rv;
     }
     if (!allowInvalidExplicitEncoding && !value) {
       return Fail(SEC_ERROR_BAD_DER);
     }
   }
   return Success;
 }
 
@@ -544,20 +561,17 @@ TimeChoice(Input& input, /*out*/ PRTime&
   return internal::TimeChoice(input, expectedTag, time);
 }
 
 // This parser will only parse values between 0..127. If this range is
 // increased then callers will need to be changed.
 inline Result
 Integer(Input& input, /*out*/ uint8_t& value)
 {
-  if (internal::IntegralValue(input, INTEGER, value) != Success) {
-    return Failure;
-  }
-  return Success;
+  return internal::IntegralValue(input, INTEGER, value);
 }
 
 // This parser will only parse values between 0..127. If this range is
 // increased then callers will need to be changed. The default value must be
 // -1; defaultValue is only a parameter to make it clear in the calling code
 // what the default value is.
 inline Result
 OptionalInteger(Input& input, long defaultValue, /*out*/ long& value)
@@ -569,35 +583,37 @@ OptionalInteger(Input& input, long defau
   }
 
   if (!input.Peek(INTEGER)) {
     value = defaultValue;
     return Success;
   }
 
   uint8_t parsedValue;
-  if (Integer(input, parsedValue) != Success) {
-    return Failure;
+  Result rv = Integer(input, parsedValue);
+  if (rv != Success) {
+    return rv;
   }
   value = parsedValue;
   return Success;
 }
 
 inline Result
 Null(Input& input)
 {
   return ExpectTagAndLength(input, NULLTag, 0);
 }
 
 template <uint8_t Len>
 Result
 OID(Input& input, const uint8_t (&expectedOid)[Len])
 {
-  if (ExpectTagAndLength(input, OIDTag, Len) != Success) {
-    return Failure;
+  Result rv = ExpectTagAndLength(input, OIDTag, Len);
+  if (rv != Success) {
+    return rv;
   }
 
   return input.Expect(expectedOid, Len);
 }
 
 // PKI-specific types
 
 inline Result
@@ -609,18 +625,19 @@ CertificateSerialNumber(Input& input, /*
   //   each certificate."
   // * "Certificate users MUST be able to handle serialNumber values up to 20
   //   octets. Conforming CAs MUST NOT use serialNumber values longer than 20
   //   octets."
   // * "Note: Non-conforming CAs may issue certificates with serial numbers
   //   that are negative or zero.  Certificate users SHOULD be prepared to
   //   gracefully handle such certificates."
 
-  if (ExpectTagAndGetValue(input, INTEGER, value) != Success) {
-    return Failure;
+  Result rv = ExpectTagAndGetValue(input, INTEGER, value);
+  if (rv != Success) {
+    return rv;
   }
 
   if (value.len == 0) {
     return Fail(SEC_ERROR_BAD_DER);
   }
 
   // Check for overly-long encodings. If the first byte is 0x00 then the high
   // bit on the second byte must be 1; otherwise the same *positive* value
@@ -648,25 +665,28 @@ inline Result
 OptionalVersion(Input& input, /*out*/ Version& version)
 {
   static const uint8_t TAG = CONTEXT_SPECIFIC | CONSTRUCTED | 0;
   if (!input.Peek(TAG)) {
     version = Version::v1;
     return Success;
   }
   Input value;
-  if (ExpectTagAndGetValue(input, TAG, value) != Success) {
-    return Failure;
+  Result rv = ExpectTagAndGetValue(input, TAG, value);
+  if (rv != Success) {
+    return rv;
   }
   uint8_t integerValue;
-  if (Integer(value, integerValue) != Success) {
-    return Failure;
+  rv = Integer(value, integerValue);
+  if (rv != Success) {
+    return rv;
   }
-  if (End(value) != Success) {
-    return Failure;
+  rv = End(value);
+  if (rv != Success) {
+    return rv;
   }
   switch (integerValue) {
     case static_cast<uint8_t>(Version::v3): version = Version::v3; break;
     case static_cast<uint8_t>(Version::v2): version = Version::v2; break;
     // XXX(bug 1031093): We shouldn't accept an explicit encoding of v1, but we
     // do here for compatibility reasons.
     case static_cast<uint8_t>(Version::v1): version = Version::v1; break;
     default:
@@ -678,67 +698,76 @@ OptionalVersion(Input& input, /*out*/ Ve
 template <typename ExtensionHandler>
 inline Result
 OptionalExtensions(Input& input, uint8_t tag, ExtensionHandler extensionHandler)
 {
   if (!input.Peek(tag)) {
     return Success;
   }
 
+  Result rv;
+
   Input extensions;
   {
     Input tagged;
-    if (ExpectTagAndGetValue(input, tag, tagged) != Success) {
-      return Failure;
+    rv = ExpectTagAndGetValue(input, tag, tagged);
+    if (rv != Success) {
+      return rv;
     }
-    if (ExpectTagAndGetValue(tagged, SEQUENCE, extensions) != Success) {
-      return Failure;
+    rv = ExpectTagAndGetValue(tagged, SEQUENCE, extensions);
+    if (rv != Success) {
+      return rv;
     }
-    if (End(tagged) != Success) {
-      return Failure;
+    rv = End(tagged);
+    if (rv != Success) {
+      return rv;
     }
   }
 
   // Extensions ::= SEQUENCE SIZE (1..MAX) OF Extension
   //
   // TODO(bug 997994): According to the specification, there should never be
   // an empty sequence of extensions but we've found OCSP responses that have
   // that (see bug 991898).
   while (!extensions.AtEnd()) {
     Input extension;
-    if (ExpectTagAndGetValue(extensions, SEQUENCE, extension)
-          != Success) {
-      return Failure;
+    rv = ExpectTagAndGetValue(extensions, SEQUENCE, extension);
+    if (rv != Success) {
+      return rv;
     }
 
     // Extension  ::=  SEQUENCE  {
     //      extnID      OBJECT IDENTIFIER,
     //      critical    BOOLEAN DEFAULT FALSE,
     //      extnValue   OCTET STRING
     //      }
     Input extnID;
-    if (ExpectTagAndGetValue(extension, OIDTag, extnID) != Success) {
-      return Failure;
+    rv = ExpectTagAndGetValue(extension, OIDTag, extnID);
+    if (rv != Success) {
+      return rv;
     }
     bool critical;
-    if (OptionalBoolean(extension, false, critical) != Success) {
-      return Failure;
+    rv = OptionalBoolean(extension, false, critical);
+    if (rv != Success) {
+      return rv;
     }
     SECItem extnValue;
-    if (ExpectTagAndGetValue(extension, OCTET_STRING, extnValue)
-          != Success) {
-      return Failure;
+    rv = ExpectTagAndGetValue(extension, OCTET_STRING, extnValue);
+    if (rv != Success) {
+      return rv;
     }
-    if (End(extension) != Success) {
-      return Failure;
+    rv = End(extension);
+    if (rv != Success) {
+      return rv;
     }
 
     bool understood = false;
-    if (extensionHandler(extnID, extnValue, understood) != Success) {
-      return Failure;
+    rv = extensionHandler(extnID, extnValue, understood);
+    if (rv != Success) {
+      return rv;
     }
     if (critical && !understood) {
       return Fail(SEC_ERROR_UNKNOWN_CRITICAL_EXTENSION);
     }
   }
 
   return Success;
 }
--- a/security/pkix/lib/pkixocsp.cpp
+++ b/security/pkix/lib/pkixocsp.cpp
@@ -148,40 +148,39 @@ CheckOCSPResponseSignerCert(TrustDomain&
 }
 
 MOZILLA_PKIX_ENUM_CLASS ResponderIDType : uint8_t
 {
   byName = der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 1,
   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 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,
-                                  /*out*/ bool& match);
+static inline Result OCSPResponse(der::Input&, Context&);
+static inline Result ResponseBytes(der::Input&, Context&);
+static inline Result BasicResponse(der::Input&, Context&);
+static inline Result ResponseData(
+                       der::Input& tbsResponseData,
+                       Context& context,
+                       const SignedDataWithSignature& signedResponseData,
+                       /*const*/ SECItem* certs, size_t numCerts);
+static inline Result SingleResponse(der::Input& input, Context& context);
+static Result ExtensionNotUnderstood(der::Input& extnID,
+                                     const SECItem& extnValue,
+                                     /*out*/ bool& understood);
+static inline Result CertID(der::Input& input,
+                            const Context& context,
+                            /*out*/ bool& match);
 static Result MatchKeyHash(TrustDomain& trustDomain,
                            const SECItem& issuerKeyHash,
                            const SECItem& issuerSubjectPublicKeyInfo,
                            /*out*/ bool& match);
 static Result KeyHash(TrustDomain& trustDomain,
                       const SECItem& subjectPublicKeyInfo,
                       /*out*/ uint8_t* hashBuf, size_t hashBufSize);
 
-
 static Result
 MatchResponderID(TrustDomain& trustDomain,
                  ResponderIDType responderIDType,
                  const SECItem& responderIDItem,
                  const SECItem& potentialSignerSubject,
                  const SECItem& potentialSignerSubjectPublicKeyInfo,
                  /*out*/ bool& match)
 {
@@ -192,24 +191,24 @@ MatchResponderID(TrustDomain& trustDomai
       // XXX(bug 926270) XXX(bug 1008133) XXX(bug 980163): Improve name
       // comparison.
       match = SECITEM_ItemsAreEqual(&responderIDItem, &potentialSignerSubject);
       return Success;
 
     case ResponderIDType::byKey:
     {
       der::Input responderID;
-      if (responderID.Init(responderIDItem.data, responderIDItem.len)
-            != der::Success) {
-        return RecoverableError;
+      Result rv = responderID.Init(responderIDItem.data, responderIDItem.len);
+      if (rv != Success) {
+        return rv;
       }
       SECItem keyHash;
-      if (der::ExpectTagAndGetValue(responderID, der::OCTET_STRING, keyHash)
-            != der::Success) {
-        return RecoverableError;
+      rv = der::ExpectTagAndGetValue(responderID, der::OCTET_STRING, keyHash);
+      if (rv != Success) {
+        return rv;
       }
       return MatchKeyHash(trustDomain, keyHash,
                           potentialSignerSubjectPublicKeyInfo, match);
     }
 
     default:
       return Fail(RecoverableError, SEC_ERROR_OCSP_MALFORMED_RESPONSE);
   }
@@ -305,30 +304,30 @@ VerifyEncodedOCSPResponse(TrustDomain& t
                           /*out*/ bool& expired,
                           /*optional out*/ PRTime* thisUpdate,
                           /*optional out*/ PRTime* validThrough)
 {
   // Always initialize this to something reasonable.
   expired = false;
 
   der::Input input;
-  if (input.Init(encodedResponse.data, encodedResponse.len) != der::Success) {
+  if (input.Init(encodedResponse.data, encodedResponse.len) != Success) {
     SetErrorToMalformedResponseOnBadDERError();
     return SECFailure;
   }
   Context context(trustDomain, certID, time, maxOCSPLifetimeInDays,
                   thisUpdate, validThrough);
 
   if (der::Nested(input, der::SEQUENCE,
-                  bind(OCSPResponse, _1, ref(context))) != der::Success) {
+                  bind(OCSPResponse, _1, ref(context))) != Success) {
     SetErrorToMalformedResponseOnBadDERError();
     return SECFailure;
   }
 
-  if (der::End(input) != der::Success) {
+  if (der::End(input) != Success) {
     SetErrorToMalformedResponseOnBadDERError();
     return SECFailure;
   }
 
   expired = context.expired;
 
   switch (context.certStatus) {
     case CertStatus::Good:
@@ -349,32 +348,33 @@ VerifyEncodedOCSPResponse(TrustDomain& t
   PR_SetError(SEC_ERROR_OCSP_UNKNOWN_CERT, 0);
   return SECFailure;
 }
 
 // OCSPResponse ::= SEQUENCE {
 //       responseStatus         OCSPResponseStatus,
 //       responseBytes          [0] EXPLICIT ResponseBytes OPTIONAL }
 //
-static inline der::Result
+static inline Result
 OCSPResponse(der::Input& input, Context& context)
 {
   // OCSPResponseStatus ::= ENUMERATED {
   //     successful            (0),  -- Response has valid confirmations
   //     malformedRequest      (1),  -- Illegal confirmation request
   //     internalError         (2),  -- Internal error in issuer
   //     tryLater              (3),  -- Try again later
   //                                 -- (4) is not used
   //     sigRequired           (5),  -- Must sign the request
   //     unauthorized          (6)   -- Request unauthorized
   // }
   uint8_t responseStatus;
 
-  if (der::Enumerated(input, responseStatus) != der::Success) {
-    return der::Failure;
+  Result rv = der::Enumerated(input, responseStatus);
+  if (rv != Success) {
+    return rv;
   }
   switch (responseStatus) {
     case 0: break; // successful
     case 1: return der::Fail(SEC_ERROR_OCSP_MALFORMED_REQUEST);
     case 2: return der::Fail(SEC_ERROR_OCSP_SERVER_ERROR);
     case 3: return der::Fail(SEC_ERROR_OCSP_TRY_SERVER_LATER);
     case 5: return der::Fail(SEC_ERROR_OCSP_REQUEST_NEEDS_SIG);
     case 6: return der::Fail(SEC_ERROR_OCSP_UNAUTHORIZED_REQUEST);
@@ -383,239 +383,250 @@ OCSPResponse(der::Input& input, Context&
 
   return der::Nested(input, der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 0,
                      der::SEQUENCE, bind(ResponseBytes, _1, ref(context)));
 }
 
 // ResponseBytes ::=       SEQUENCE {
 //     responseType   OBJECT IDENTIFIER,
 //     response       OCTET STRING }
-static inline der::Result
+static inline Result
 ResponseBytes(der::Input& input, Context& context)
 {
   static const uint8_t id_pkix_ocsp_basic[] = {
     0x2B, 0x06, 0x01, 0x05, 0x05, 0x07, 0x30, 0x01, 0x01
   };
 
-  if (der::OID(input, id_pkix_ocsp_basic) != der::Success) {
-    return der::Failure;
+  Result rv = der::OID(input, id_pkix_ocsp_basic);
+  if (rv != Success) {
+    return rv;
   }
 
   return der::Nested(input, der::OCTET_STRING, der::SEQUENCE,
                      bind(BasicResponse, _1, ref(context)));
 }
 
 // BasicOCSPResponse       ::= SEQUENCE {
 //    tbsResponseData      ResponseData,
 //    signatureAlgorithm   AlgorithmIdentifier,
 //    signature            BIT STRING,
 //    certs            [0] EXPLICIT SEQUENCE OF Certificate OPTIONAL }
-der::Result
+Result
 BasicResponse(der::Input& input, Context& context)
 {
   der::Input tbsResponseData;
   SignedDataWithSignature signedData;
-  if (der::SignedData(input, tbsResponseData, signedData) != der::Success) {
+  Result rv = der::SignedData(input, tbsResponseData, signedData);
+  if (rv != Success) {
     if (PR_GetError() == SEC_ERROR_BAD_SIGNATURE) {
-      PR_SetError(SEC_ERROR_OCSP_BAD_SIGNATURE, 0);
+      return Fail(RecoverableError, SEC_ERROR_OCSP_BAD_SIGNATURE);
     }
-    return der::Failure;
+    return rv;
   }
 
   // Parse certificates, if any
 
   SECItem certs[8];
   size_t numCerts = 0;
 
   if (!input.AtEnd()) {
     // We ignore the lengths of the wrappers because we'll detect bad lengths
     // during parsing--too short and we'll run out of input for parsing a cert,
     // and too long and we'll have leftover data that won't parse as a cert.
 
     // [0] wrapper
-    if (der::ExpectTagAndSkipLength(
-          input, der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 0)
-        != der::Success) {
-      return der::Failure;
+    rv = der::ExpectTagAndSkipLength(
+          input, der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 0);
+    if (rv != Success) {
+      return rv;
     }
 
     // SEQUENCE wrapper
-    if (der::ExpectTagAndSkipLength(input, der::SEQUENCE) != der::Success) {
-      return der::Failure;
+    rv = der::ExpectTagAndSkipLength(input, der::SEQUENCE);
+    if (rv != Success) {
+      return rv;
     }
 
     // sequence of certificates
     while (!input.AtEnd()) {
       if (numCerts == PR_ARRAY_SIZE(certs)) {
         return der::Fail(SEC_ERROR_BAD_DER);
       }
 
-      if (der::ExpectTagAndGetTLV(input, der::SEQUENCE, certs[numCerts])
-            != der::Success) {
-        return der::Failure;
+      rv = der::ExpectTagAndGetTLV(input, der::SEQUENCE, certs[numCerts]);
+      if (rv != Success) {
+        return rv;
       }
       ++numCerts;
     }
   }
 
   return ResponseData(tbsResponseData, context, signedData, certs, numCerts);
 }
 
 // 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
+static inline Result
 ResponseData(der::Input& input, Context& context,
              const SignedDataWithSignature& signedResponseData,
              /*const*/ SECItem* certs, size_t numCerts)
 {
   der::Version version;
-  if (der::OptionalVersion(input, version) != der::Success) {
-    return der::Failure;
+  Result rv = der::OptionalVersion(input, version);
+  if (rv != Success) {
+    return rv;
   }
   if (version != der::Version::v1) {
     // TODO: more specific error code for bad version?
     return der::Fail(SEC_ERROR_BAD_DER);
   }
 
   // ResponderID ::= CHOICE {
   //    byName              [1] Name,
   //    byKey               [2] KeyHash }
   SECItem responderID;
   ResponderIDType responderIDType
     = input.Peek(static_cast<uint8_t>(ResponderIDType::byName))
     ? ResponderIDType::byName
     : ResponderIDType::byKey;
-  if (ExpectTagAndGetValue(input, static_cast<uint8_t>(responderIDType),
-                           responderID) != der::Success) {
-    return der::Failure;
+  rv = ExpectTagAndGetValue(input, static_cast<uint8_t>(responderIDType),
+                            responderID);
+  if (rv != Success) {
+    return rv;
   }
 
   // This is the soonest we can verify the signature. We verify the signature
   // right away to follow the principal of minimizing the processing of data
   // before verifying its signature.
-  if (VerifySignature(context, responderIDType, responderID, certs, numCerts,
-                      signedResponseData) != Success) {
-    return der::Failure;
+  rv = VerifySignature(context, responderIDType, responderID, certs, numCerts,
+                       signedResponseData);
+  if (rv != Success) {
+    return rv;
   }
 
   // TODO: Do we even need to parse this? Should we just skip it?
   PRTime producedAt;
-  if (der::GeneralizedTime(input, producedAt) != der::Success) {
-    return der::Failure;
+  rv = der::GeneralizedTime(input, producedAt);
+  if (rv != Success) {
+    return rv;
   }
 
   // We don't accept an empty sequence of responses. In practice, a legit OCSP
   // responder will never return an empty response, and handling the case of an
   // empty response makes things unnecessarily complicated.
-  if (der::NestedOf(input, der::SEQUENCE, der::SEQUENCE,
-                    der::EmptyAllowed::No,
-                    bind(SingleResponse, _1, ref(context))) != der::Success) {
-    return der::Failure;
+  rv = der::NestedOf(input, der::SEQUENCE, der::SEQUENCE,
+                     der::EmptyAllowed::No,
+                     bind(SingleResponse, _1, ref(context)));
+  if (rv != Success) {
+    return rv;
   }
 
   return der::OptionalExtensions(input,
                                  der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 1,
                                  ExtensionNotUnderstood);
 }
 
 // SingleResponse ::= SEQUENCE {
 //    certID                       CertID,
 //    certStatus                   CertStatus,
 //    thisUpdate                   GeneralizedTime,
 //    nextUpdate           [0]     EXPLICIT GeneralizedTime OPTIONAL,
 //    singleExtensions     [1]     EXPLICIT Extensions{{re-ocsp-crl |
 //                                              re-ocsp-archive-cutoff |
 //                                              CrlEntryExtensions, ...}
 //                                              } OPTIONAL }
-static inline der::Result
+static inline Result
 SingleResponse(der::Input& input, Context& context)
 {
   bool match = false;
-  if (der::Nested(input, der::SEQUENCE,
-                  bind(CertID, _1, cref(context), ref(match)))
-        != der::Success) {
-    return der::Failure;
+  Result rv = der::Nested(input, der::SEQUENCE,
+                          bind(CertID, _1, cref(context), ref(match)));
+  if (rv != Success) {
+    return rv;
   }
 
   if (!match) {
     // This response does not reference the certificate we're interested in.
     // By consuming the rest of our input and returning successfully, we can
     // continue processing and examine another response that might have what
     // we want.
     input.SkipToEnd();
-    return der::Success;
+    return Success;
   }
 
   // CertStatus ::= CHOICE {
   //     good        [0]     IMPLICIT NULL,
   //     revoked     [1]     IMPLICIT RevokedInfo,
   //     unknown     [2]     IMPLICIT UnknownInfo }
   //
   // In the event of multiple SingleResponses for a cert that have conflicting
   // statuses, we use the following precedence rules:
   //
   // * revoked overrides good and unknown
   // * good overrides unknown
   if (input.Peek(static_cast<uint8_t>(CertStatus::Good))) {
-    if (ExpectTagAndLength(input, static_cast<uint8_t>(CertStatus::Good), 0)
-          != der::Success) {
-      return der::Failure;
+    rv = ExpectTagAndLength(input, static_cast<uint8_t>(CertStatus::Good), 0);
+    if (rv != Success) {
+      return rv;
     }
     if (context.certStatus != CertStatus::Revoked) {
       context.certStatus = CertStatus::Good;
     }
   } else if (input.Peek(static_cast<uint8_t>(CertStatus::Revoked))) {
     // We don't need any info from the RevokedInfo structure, so we don't even
     // parse it. TODO: We should mention issues like this in the explanation of
     // why we treat invalid OCSP responses equivalently to revoked for OCSP
     // stapling.
-    if (der::ExpectTagAndSkipValue(input,
-                                   static_cast<uint8_t>(CertStatus::Revoked))
-          != der::Success) {
-      return der::Failure;
+    rv = der::ExpectTagAndSkipValue(input,
+                                    static_cast<uint8_t>(CertStatus::Revoked));
+    if (rv != Success) {
+      return rv;
     }
     context.certStatus = CertStatus::Revoked;
-  } else if (ExpectTagAndLength(input,
-                                static_cast<uint8_t>(CertStatus::Unknown),
-                                0) != der::Success) {
-    return der::Failure;
+  } else {
+    rv = ExpectTagAndLength(input, static_cast<uint8_t>(CertStatus::Unknown),
+                            0);
+    if (rv != Success) {
+      return rv;
+    }
   }
 
   // http://tools.ietf.org/html/rfc6960#section-3.2
   // 5. The time at which the status being indicated is known to be
   //    correct (thisUpdate) is sufficiently recent;
   // 6. When available, the time at or before which newer information will
   //    be available about the status of the certificate (nextUpdate) is
   //    greater than the current time.
 
   const PRTime maxLifetime =
     context.maxLifetimeInDays * ONE_DAY;
 
   PRTime thisUpdate;
-  if (der::GeneralizedTime(input, thisUpdate) != der::Success) {
-    return der::Failure;
+  rv = der::GeneralizedTime(input, thisUpdate);
+  if (rv != Success) {
+    return rv;
   }
 
   if (thisUpdate > context.time + SLOP) {
     return der::Fail(SEC_ERROR_OCSP_FUTURE_RESPONSE);
   }
 
   PRTime notAfter;
   static const uint8_t NEXT_UPDATE_TAG =
     der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 0;
   if (input.Peek(NEXT_UPDATE_TAG)) {
     PRTime nextUpdate;
-    if (der::Nested(input, NEXT_UPDATE_TAG,
-                    bind(der::GeneralizedTime, _1, ref(nextUpdate)))
-          != der::Success) {
-      return der::Failure;
+    rv = der::Nested(input, NEXT_UPDATE_TAG,
+                    bind(der::GeneralizedTime, _1, ref(nextUpdate)));
+    if (rv != Success) {
+      return rv;
     }
 
     if (nextUpdate < thisUpdate) {
       return der::Fail(SEC_ERROR_OCSP_MALFORMED_RESPONSE);
     }
     if (nextUpdate - thisUpdate <= maxLifetime) {
       notAfter = nextUpdate;
     } else {
@@ -630,111 +641,108 @@ SingleResponse(der::Input& input, Contex
   if (context.time < SLOP) { // prevent underflow
     return der::Fail(SEC_ERROR_INVALID_ARGS);
   }
 
   if (context.time - SLOP > notAfter) {
     context.expired = true;
   }
 
-  if (der::OptionalExtensions(input,
-                              der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 1,
-                              ExtensionNotUnderstood)
-        != der::Success) {
-    return der::Failure;
+  rv = der::OptionalExtensions(input,
+                               der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 1,
+                               ExtensionNotUnderstood);
+  if (rv != Success) {
+    return rv;
   }
 
   if (context.thisUpdate) {
     *context.thisUpdate = thisUpdate;
   }
   if (context.validThrough) {
     *context.validThrough = notAfter;
   }
 
-  return der::Success;
+  return Success;
 }
 
 // CertID          ::=     SEQUENCE {
 //        hashAlgorithm       AlgorithmIdentifier,
 //        issuerNameHash      OCTET STRING, -- Hash of issuer's DN
 //        issuerKeyHash       OCTET STRING, -- Hash of issuer's public key
 //        serialNumber        CertificateSerialNumber }
-static inline der::Result
+static inline Result
 CertID(der::Input& input, const Context& context, /*out*/ bool& match)
 {
   match = false;
 
   DigestAlgorithm hashAlgorithm;
-  if (der::DigestAlgorithmIdentifier(input, hashAlgorithm) != der::Success) {
+  Result rv = der::DigestAlgorithmIdentifier(input, hashAlgorithm);
+  if (rv != 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 Success;
     }
-    return der::Failure;
+    return rv;
   }
 
   SECItem issuerNameHash;
-  if (der::ExpectTagAndGetValue(input, der::OCTET_STRING, issuerNameHash)
-        != der::Success) {
-    return der::Failure;
+  rv = der::ExpectTagAndGetValue(input, der::OCTET_STRING, issuerNameHash);
+  if (rv != Success) {
+    return rv;
   }
 
   SECItem issuerKeyHash;
-  if (der::ExpectTagAndGetValue(input, der::OCTET_STRING, issuerKeyHash)
-        != der::Success) {
-    return der::Failure;
+  rv = der::ExpectTagAndGetValue(input, der::OCTET_STRING, issuerKeyHash);
+  if (rv != Success) {
+    return rv;
   }
 
   SECItem serialNumber;
-  if (der::CertificateSerialNumber(input, serialNumber) != der::Success) {
-    return der::Failure;
+  rv = der::CertificateSerialNumber(input, serialNumber);
+  if (rv != Success) {
+    return rv;
   }
 
   if (!SECITEM_ItemsAreEqual(&serialNumber, &context.certID.serialNumber)) {
     // This does not reference the certificate we're interested in.
     // Consume the rest of the input and return successfully to
     // potentially continue processing other responses.
     input.SkipToEnd();
-    return der::Success;
+    return Success;
   }
 
   // TODO: support SHA-2 hashes.
 
   if (hashAlgorithm != DigestAlgorithm::sha1) {
     // Again, not interested in this response. Consume input, return success.
     input.SkipToEnd();
-    return der::Success;
+    return Success;
   }
 
   if (issuerNameHash.len != TrustDomain::DIGEST_LENGTH) {
     return der::Fail(SEC_ERROR_OCSP_MALFORMED_RESPONSE);
   }
 
   // From http://tools.ietf.org/html/rfc6960#section-4.1.1:
   // "The hash shall be calculated over the DER encoding of the
   // issuer's name field in the certificate being checked."
   uint8_t hashBuf[TrustDomain::DIGEST_LENGTH];
   if (context.trustDomain.DigestBuf(context.certID.issuer, hashBuf,
                                     sizeof(hashBuf)) != SECSuccess) {
-    return der::Failure;
+    return MapSECStatus(SECFailure);
   }
   if (memcmp(hashBuf, issuerNameHash.data, issuerNameHash.len)) {
     // Again, not interested in this response. Consume input, return success.
     input.SkipToEnd();
-    return der::Success;
+    return Success;
   }
 
-  if (MatchKeyHash(context.trustDomain, issuerKeyHash,
-                   context.certID.issuerSubjectPublicKeyInfo, match)
-        != Success) {
-    return der::Failure;
-  }
-
-  return der::Success;
+  return MatchKeyHash(context.trustDomain, issuerKeyHash,
+                      context.certID.issuerSubjectPublicKeyInfo, match);
 }
 
 // From http://tools.ietf.org/html/rfc6960#section-4.1.1:
 // "The hash shall be calculated over the value (excluding tag and length) of
 // the subject public key field in the issuer's certificate."
 //
 // From http://tools.ietf.org/html/rfc6960#appendix-B.1:
 // KeyHash ::= OCTET STRING -- SHA-1 hash of responder's public key
@@ -776,40 +784,40 @@ KeyHash(TrustDomain& trustDomain, const 
 
   der::Input spki;
 
   {
     // The scope of input is limited to reduce the possibility of confusing it
     // with spki in places we need to be using spki below.
     der::Input input;
     if (input.Init(subjectPublicKeyInfo.data, subjectPublicKeyInfo.len)
-          != der::Success) {
+          != Success) {
       return MapSECStatus(SECFailure);
     }
 
-    if (der::ExpectTagAndGetValue(input, der::SEQUENCE, spki) != der::Success) {
+    if (der::ExpectTagAndGetValue(input, der::SEQUENCE, spki) != Success) {
       return MapSECStatus(SECFailure);
     }
-    if (der::End(input) != der::Success) {
+    if (der::End(input) != Success) {
       return MapSECStatus(SECFailure);
     }
   }
 
   // Skip AlgorithmIdentifier
-  if (der::ExpectTagAndSkipValue(spki, der::SEQUENCE) != der::Success) {
+  if (der::ExpectTagAndSkipValue(spki, der::SEQUENCE) != Success) {
     return MapSECStatus(SECFailure);
   }
 
   SECItem subjectPublicKey;
   if (der::ExpectTagAndGetValue(spki, der::BIT_STRING, subjectPublicKey)
-        != der::Success) {
+        != Success) {
     return MapSECStatus(SECFailure);
   }
 
-  if (der::End(spki) != der::Success) {
+  if (der::End(spki) != Success) {
     return MapSECStatus(SECFailure);
   }
 
   // Assume/require that the number of unused bits in the public key is zero.
   if (subjectPublicKey.len == 0 || subjectPublicKey.data[0] != 0) {
     return Fail(RecoverableError, SEC_ERROR_BAD_DER);
   }
   ++subjectPublicKey.data;
@@ -817,22 +825,22 @@ KeyHash(TrustDomain& trustDomain, const 
 
   if (trustDomain.DigestBuf(subjectPublicKey, hashBuf, hashBufSize)
         != SECSuccess) {
     return MapSECStatus(SECFailure);
   }
   return Success;
 }
 
-der::Result
+Result
 ExtensionNotUnderstood(der::Input& /*extnID*/, const SECItem& /*extnValue*/,
                        /*out*/ bool& understood)
 {
   understood = false;
-  return der::Success;
+  return Success;
 }
 
 //   1. The certificate identified in a received response corresponds to
 //      the certificate that was identified in the corresponding request;
 //   2. The signature on the response is valid;
 //   3. The identity of the signer matches the intended recipient of the
 //      request;
 //   4. The signer is currently authorized to provide a response for the
--- a/security/pkix/lib/pkixutil.h
+++ b/security/pkix/lib/pkixutil.h
@@ -20,69 +20,24 @@
  * 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__pkixutil_h
 #define mozilla_pkix__pkixutil_h
 
-#include "pkix/enumclass.h"
-#include "pkix/pkixtypes.h"
+#include "pkix/Result.h"
 #include "pkixder.h"
 #include "prerror.h"
 #include "seccomon.h"
 #include "secerr.h"
 
 namespace mozilla { namespace pkix {
 
-enum Result
-{
-  Success = 0,
-  FatalError = -1,      // An error was encountered that caused path building
-                        // to stop immediately. example: out-of-memory.
-  RecoverableError = -2 // an error that will cause path building to continue
-                        // searching for alternative paths. example: expired
-                        // certificate.
-};
-
-// When returning errors, use this function instead of calling PR_SetError
-// directly. This helps ensure that we always call PR_SetError when we return
-// an error code. This is a useful place to set a breakpoint when a debugging
-// a certificate verification failure.
-inline Result
-Fail(Result result, PRErrorCode errorCode)
-{
-  PR_ASSERT(result != Success);
-  PR_SetError(errorCode, 0);
-  return result;
-}
-
-inline Result
-MapSECStatus(SECStatus srv)
-{
-  if (srv == SECSuccess) {
-    return Success;
-  }
-
-  PRErrorCode error = PORT_GetError();
-  switch (error) {
-    case SEC_ERROR_EXTENSION_NOT_FOUND:
-      return RecoverableError;
-
-    case PR_INVALID_STATE_ERROR:
-    case SEC_ERROR_LIBRARY_FAILURE:
-    case SEC_ERROR_NO_MEMORY:
-      return FatalError;
-  }
-
-  // TODO: PORT_Assert(false); // we haven't classified the error yet
-  return RecoverableError;
-}
-
 // During path building and verification, we build a linked list of BackCerts
 // from the current cert toward the end-entity certificate. The linked list
 // is used to verify properties that aren't local to the current certificate
 // and/or the direct link between the current certificate and its issuer,
 // such as name constraints.
 //
 // Each BackCert contains pointers to all the given certificate's extensions
 // so that we can parse the extension block once and then process the
@@ -184,17 +139,17 @@ private:
   NonOwningSECItem basicConstraints;
   NonOwningSECItem certificatePolicies;
   NonOwningSECItem extKeyUsage;
   NonOwningSECItem inhibitAnyPolicy;
   NonOwningSECItem keyUsage;
   NonOwningSECItem nameConstraints;
   NonOwningSECItem subjectAltName;
 
-  der::Result RememberExtension(der::Input& extnID, const SECItem& extnValue,
+  Result RememberExtension(der::Input& extnID, const SECItem& extnValue,
                                 /*out*/ bool& understood);
 
   BackCert(const BackCert&) /* = delete */;
   void operator=(const BackCert&); /* = delete */;
 };
 
 class NonOwningDERArray : public DERArray
 {
--- a/security/pkix/test/gtest/pkixder_input_tests.cpp
+++ b/security/pkix/test/gtest/pkixder_input_tests.cpp
@@ -21,20 +21,23 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
 
 #include <functional>
 #include <vector>
 #include <gtest/gtest.h>
 
+#include "pkixgtest.h"
 #include "pkix/bind.h"
 #include "pkixder.h"
 
+using namespace mozilla::pkix;
 using namespace mozilla::pkix::der;
+using namespace mozilla::pkix::test;
 
 namespace {
 
 class pkixder_input_tests : public ::testing::Test
 {
 protected:
   virtual void SetUp()
   {
@@ -89,68 +92,63 @@ const uint8_t DER_OVERRUN_SEQUENCE_OF_IN
 const uint8_t DER_INT16[] = {
   0x02,                       // INTEGER
   0x02,                       // length
   0x12, 0x34                  // 0x1234
 };
 
 TEST_F(pkixder_input_tests, FailWithError)
 {
-  ASSERT_EQ(Failure, Fail(SEC_ERROR_BAD_DER));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
-
-  ASSERT_EQ(Failure, Fail(SEC_ERROR_INVALID_ARGS));
-  ASSERT_EQ(SEC_ERROR_INVALID_ARGS, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, Fail(SEC_ERROR_BAD_DER));
+  ASSERT_FatalError(SEC_ERROR_INVALID_ARGS, Fail(SEC_ERROR_INVALID_ARGS));
 }
 
 TEST_F(pkixder_input_tests, InputInit)
 {
   Input input;
   ASSERT_EQ(Success,
             input.Init(DER_SEQUENCE_OF_INT8, sizeof DER_SEQUENCE_OF_INT8));
 }
 
 TEST_F(pkixder_input_tests, InputInitWithNullPointerOrZeroLength)
 {
   Input input;
-  ASSERT_EQ(Failure, input.Init(nullptr, 0));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, input.Init(nullptr, 0));
 
-  ASSERT_EQ(Failure, input.Init(nullptr, 100));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, input.Init(nullptr, 100));
 
   // Though it seems odd to initialize with zero-length and non-null ptr, this
   // is working as intended. The Input class was intended to protect against
   // buffer overflows, and there's no risk with the current behavior. See bug
   // 1000354.
   ASSERT_EQ(Success, input.Init((const uint8_t*) "hello", 0));
   ASSERT_TRUE(input.AtEnd());
 }
 
 TEST_F(pkixder_input_tests, InputInitWithLargeData)
 {
   Input input;
   // Data argument length does not matter, it is not touched, just
   // needs to be non-null
-  ASSERT_EQ(Failure, input.Init((const uint8_t*) "", 0xffff+1));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          input.Init((const uint8_t*) "", 0xffff+1));
 
   ASSERT_EQ(Success, input.Init((const uint8_t*) "", 0xffff));
 }
 
 TEST_F(pkixder_input_tests, InputInitMultipleTimes)
 {
   Input input;
 
   ASSERT_EQ(Success,
             input.Init(DER_SEQUENCE_OF_INT8, sizeof DER_SEQUENCE_OF_INT8));
 
-  ASSERT_EQ(Failure,
-            input.Init(DER_SEQUENCE_OF_INT8, sizeof DER_SEQUENCE_OF_INT8));
-  ASSERT_EQ(SEC_ERROR_INVALID_ARGS, PR_GetError());
+  ASSERT_FatalError(SEC_ERROR_INVALID_ARGS,
+                    input.Init(DER_SEQUENCE_OF_INT8,
+                               sizeof DER_SEQUENCE_OF_INT8));
 }
 
 TEST_F(pkixder_input_tests, ExpectSuccess)
 {
   Input input;
 
   ASSERT_EQ(Success,
             input.Init(DER_SEQUENCE_OF_INT8, sizeof DER_SEQUENCE_OF_INT8));
@@ -162,30 +160,30 @@ TEST_F(pkixder_input_tests, ExpectSucces
 TEST_F(pkixder_input_tests, ExpectMismatch)
 {
   Input input;
 
   ASSERT_EQ(Success,
             input.Init(DER_SEQUENCE_OF_INT8, sizeof DER_SEQUENCE_OF_INT8));
 
   const uint8_t expected[] = { 0x11, 0x22 };
-  ASSERT_EQ(Failure, input.Expect(expected, sizeof expected));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          input.Expect(expected, sizeof expected));
 }
 
 TEST_F(pkixder_input_tests, ExpectTooMuch)
 {
   Input input;
 
   const uint8_t der[] = { 0x11, 0x22 };
   ASSERT_EQ(Success, input.Init(der, sizeof der));
 
   const uint8_t expected[] = { 0x11, 0x22, 0x33 };
-  ASSERT_EQ(Failure, input.Expect(expected, sizeof expected));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          input.Expect(expected, sizeof expected));
 }
 
 TEST_F(pkixder_input_tests, PeekWithinBounds)
 {
   Input input;
   const uint8_t der[] = { 0x11, 0x11 };
   ASSERT_EQ(Success, input.Init(der, sizeof der));
   ASSERT_TRUE(input.Peek(0x11));
@@ -226,36 +224,34 @@ TEST_F(pkixder_input_tests, ReadBytePast
   // Initialize with too-short length
   ASSERT_EQ(Success, input.Init(der, 1));
 
   uint8_t readByte1 = 0;
   ASSERT_EQ(Success, input.Read(readByte1));
   ASSERT_EQ(0x11, readByte1);
 
   uint8_t readByte2 = 0;
-  ASSERT_EQ(Failure, input.Read(readByte2));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, input.Read(readByte2));
   ASSERT_NE(0x22, readByte2);
 }
 
 TEST_F(pkixder_input_tests, ReadByteWrapAroundPointer)
 {
   // The original implementation of our buffer read overflow checks was
   // susceptible to integer overflows which could make the checks ineffective.
   // This attempts to verify that we've fixed that. Unfortunately, decrementing
   // a null pointer is undefined behavior according to the C++ language spec.,
   // but this should catch the problem on at least some compilers, if not all of
   // them.
   const uint8_t* der = nullptr;
   --der;
   Input input;
   ASSERT_EQ(Success, input.Init(der, 0));
   uint8_t b;
-  ASSERT_EQ(Failure, input.Read(b));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, input.Read(b));
 }
 
 TEST_F(pkixder_input_tests, ReadWord)
 {
   Input input;
   const uint8_t der[] = { 0x11, 0x22, 0x33, 0x44 };
   ASSERT_EQ(Success, input.Init(der, sizeof der));
 
@@ -275,47 +271,45 @@ TEST_F(pkixder_input_tests, ReadWordPast
   // Initialize with too-short length
   ASSERT_EQ(Success, input.Init(der, 2));
 
   uint16_t readWord1 = 0;
   ASSERT_EQ(Success, input.Read(readWord1));
   ASSERT_EQ(0x1122, readWord1);
 
   uint16_t readWord2 = 0;
-  ASSERT_EQ(Failure, input.Read(readWord2));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, input.Read(readWord2));
   ASSERT_NE(0x3344, readWord2);
 }
 
 TEST_F(pkixder_input_tests, ReadWordWithInsufficentData)
 {
   Input input;
   const uint8_t der[] = { 0x11, 0x22 };
   ASSERT_EQ(Success, input.Init(der, 1));
 
   uint16_t readWord1 = 0;
-  ASSERT_EQ(Failure, input.Read(readWord1));
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, input.Read(readWord1));
   ASSERT_NE(0x1122, readWord1);
 }
 
 TEST_F(pkixder_input_tests, ReadWordWrapAroundPointer)
 {
   // The original implementation of our buffer read overflow checks was
   // susceptible to integer overflows which could make the checks ineffective.
   // This attempts to verify that we've fixed that. Unfortunately, decrementing
   // a null pointer is undefined behavior according to the C++ language spec.,
   // but this should catch the problem on at least some compilers, if not all of
   // them.
   const uint8_t* der = nullptr;
   --der;
   Input input;
   ASSERT_EQ(Success, input.Init(der, 0));
   uint16_t b;
-  ASSERT_EQ(Failure, input.Read(b));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, input.Read(b));
 }
 
 TEST_F(pkixder_input_tests, InputSkip)
 {
   Input input;
   const uint8_t der[] = { 0x11, 0x22, 0x33, 0x44 };
   ASSERT_EQ(Success, input.Init(der, sizeof der));
 
@@ -342,18 +336,17 @@ TEST_F(pkixder_input_tests, InputSkipToE
 }
 
 TEST_F(pkixder_input_tests, InputSkipPastEnd)
 {
   Input input;
   const uint8_t der[] = { 0x11, 0x22, 0x33, 0x44 };
   ASSERT_EQ(Success, input.Init(der, sizeof der));
 
-  ASSERT_EQ(Failure, input.Skip(sizeof der + 1));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, input.Skip(sizeof der + 1));
 }
 
 TEST_F(pkixder_input_tests, InputSkipToNewInput)
 {
   Input input;
   const uint8_t der[] = { 0x01, 0x02, 0x03, 0x04 };
   ASSERT_EQ(Success, input.Init(der, sizeof der));
 
@@ -380,18 +373,18 @@ TEST_F(pkixder_input_tests, InputSkipToN
 
 TEST_F(pkixder_input_tests, InputSkipToNewInputPastEnd)
 {
   Input input;
   const uint8_t der[] = { 0x11, 0x22, 0x33, 0x44 };
   ASSERT_EQ(Success, input.Init(der, sizeof der));
 
   Input skippedInput;
-  ASSERT_EQ(Failure, input.Skip(sizeof der * 2, skippedInput));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          input.Skip(sizeof der * 2, skippedInput));
 }
 
 TEST_F(pkixder_input_tests, InputSkipToSECItem)
 {
   Input input;
   const uint8_t der[] = { 0x11, 0x22, 0x33, 0x44 };
   ASSERT_EQ(Success, input.Init(der, sizeof der));
 
@@ -412,29 +405,28 @@ TEST_F(pkixder_input_tests, SkipWrapArou
   // This attempts to verify that we've fixed that. Unfortunately, decrementing
   // a null pointer is undefined behavior according to the C++ language spec.,
   // but this should catch the problem on at least some compilers, if not all of
   // them.
   const uint8_t* der = nullptr;
   --der;
   Input input;
   ASSERT_EQ(Success, input.Init(der, 0));
-  ASSERT_EQ(Failure, input.Skip(1));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, input.Skip(1));
 }
 
 TEST_F(pkixder_input_tests, SkipToSECItemPastEnd)
 {
   Input input;
   const uint8_t der[] = { 0x11, 0x22, 0x33, 0x44 };
   ASSERT_EQ(Success, input.Init(der, sizeof der));
 
   SECItem skippedSECItem;
-  ASSERT_EQ(Failure, input.Skip(sizeof der + 1, skippedSECItem));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          input.Skip(sizeof der + 1, skippedSECItem));
 }
 
 TEST_F(pkixder_input_tests, ExpectTagAndSkipValue)
 {
   Input input;
   ASSERT_EQ(Success,
             input.Init(DER_SEQUENCE_OF_INT8, sizeof DER_SEQUENCE_OF_INT8));
 
@@ -443,26 +435,27 @@ TEST_F(pkixder_input_tests, ExpectTagAnd
 }
 
 TEST_F(pkixder_input_tests, ExpectTagAndSkipValueWithTruncatedData)
 {
   Input input;
   ASSERT_EQ(Success, input.Init(DER_TRUNCATED_SEQUENCE_OF_INT8,
                                 sizeof DER_TRUNCATED_SEQUENCE_OF_INT8));
 
-  ASSERT_EQ(Failure, ExpectTagAndSkipValue(input, SEQUENCE));
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          ExpectTagAndSkipValue(input, SEQUENCE));
 }
 
 TEST_F(pkixder_input_tests, ExpectTagAndSkipValueWithOverrunData)
 {
   Input input;
   ASSERT_EQ(Success, input.Init(DER_OVERRUN_SEQUENCE_OF_INT8,
                                 sizeof DER_OVERRUN_SEQUENCE_OF_INT8));
   ASSERT_EQ(Success, ExpectTagAndSkipValue(input, SEQUENCE));
-  ASSERT_EQ(Failure, End(input));
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, End(input));
 }
 
 TEST_F(pkixder_input_tests, AtEndOnUnInitializedInput)
 {
   Input input;
   ASSERT_TRUE(input.AtEnd());
 }
 
@@ -514,18 +507,18 @@ TEST_F(pkixder_input_tests, MarkAndGetSE
   ASSERT_EQ(Success, input.Init(der, sizeof der));
 
   Input another;
   Input::Mark mark = another.GetMark();
 
   ASSERT_EQ(Success, input.Skip(3));
 
   SECItem item;
-  ASSERT_EQ(Failure, input.GetSECItem(siBuffer, mark, item));
-  ASSERT_EQ(SEC_ERROR_INVALID_ARGS, PR_GetError());
+  ASSERT_FatalError(SEC_ERROR_INVALID_ARGS,
+                    input.GetSECItem(siBuffer, mark, item));
 }
 #endif
 
 TEST_F(pkixder_input_tests, ExpectTagAndLength)
 {
   Input input;
   ASSERT_EQ(Success,
             input.Init(DER_SEQUENCE_OF_INT8, sizeof DER_SEQUENCE_OF_INT8));
@@ -535,63 +528,66 @@ TEST_F(pkixder_input_tests, ExpectTagAnd
 }
 
 TEST_F(pkixder_input_tests, ExpectTagAndLengthWithWrongLength)
 {
   Input input;
   ASSERT_EQ(Success, input.Init(DER_INT16, sizeof DER_INT16));
 
   // Wrong length
-  ASSERT_EQ(Failure, ExpectTagAndLength(input, INTEGER, 4));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          ExpectTagAndLength(input, INTEGER, 4));
 }
 
 TEST_F(pkixder_input_tests, ExpectTagAndLengthWithWrongTag)
 {
   Input input;
   ASSERT_EQ(Success, input.Init(DER_INT16, sizeof DER_INT16));
 
   // Wrong type
-  ASSERT_EQ(Failure, ExpectTagAndLength(input, OCTET_STRING, 2));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          ExpectTagAndLength(input, OCTET_STRING, 2));
 }
 
 TEST_F(pkixder_input_tests, ExpectTagAndGetLength)
 {
   Input input;
   ASSERT_EQ(Success,
             input.Init(DER_SEQUENCE_OF_INT8, sizeof DER_SEQUENCE_OF_INT8));
 
   uint16_t length = 0;
-  ASSERT_EQ(Success, internal::ExpectTagAndGetLength(input, SEQUENCE, length));
+  ASSERT_EQ(Success,
+            der::internal::ExpectTagAndGetLength(input, SEQUENCE, length));
   ASSERT_EQ(sizeof DER_SEQUENCE_OF_INT8 - 2, length);
   ASSERT_EQ(Success, input.Skip(length));
   ASSERT_TRUE(input.AtEnd());
 }
 
 TEST_F(pkixder_input_tests, ExpectTagAndGetLengthWithWrongTag)
 {
   Input input;
   ASSERT_EQ(Success,
             input.Init(DER_SEQUENCE_OF_INT8, sizeof DER_SEQUENCE_OF_INT8));
 
   uint16_t length = 0;
-  ASSERT_EQ(Failure, internal::ExpectTagAndGetLength(input, INTEGER, length));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          der::internal::ExpectTagAndGetLength(input, INTEGER,
+                                                               length));
 }
 
 TEST_F(pkixder_input_tests, ExpectTagAndGetLengthWithWrongLength)
 {
   Input input;
   ASSERT_EQ(Success, input.Init(DER_TRUNCATED_SEQUENCE_OF_INT8,
                                 sizeof DER_TRUNCATED_SEQUENCE_OF_INT8));
 
   uint16_t length = 0;
-  ASSERT_EQ(Failure, internal::ExpectTagAndGetLength(input, SEQUENCE, length));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          der::internal::ExpectTagAndGetLength(input, SEQUENCE,
+                                                               length));
 }
 
 TEST_F(pkixder_input_tests, ExpectTagAndGetValue_Input_ValidEmpty)
 {
   Input input;
   ASSERT_EQ(Success,
             input.Init(DER_SEQUENCE_EMPTY, sizeof DER_SEQUENCE_EMPTY));
   Input value;
@@ -614,38 +610,38 @@ TEST_F(pkixder_input_tests, ExpectTagAnd
 TEST_F(pkixder_input_tests,
        ExpectTagAndGetValue_Input_InvalidNotEmptyValueTruncated)
 {
   Input input;
   ASSERT_EQ(Success,
             input.Init(DER_SEQUENCE_NOT_EMPTY_VALUE_TRUNCATED,
                        sizeof DER_SEQUENCE_NOT_EMPTY_VALUE_TRUNCATED));
   Input value;
-  ASSERT_EQ(Failure, ExpectTagAndGetValue(input, SEQUENCE, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          ExpectTagAndGetValue(input, SEQUENCE, value));
 }
 
 TEST_F(pkixder_input_tests, ExpectTagAndGetValue_Input_InvalidWrongLength)
 {
   Input input;
   ASSERT_EQ(Success, input.Init(DER_TRUNCATED_SEQUENCE_OF_INT8,
                                 sizeof DER_TRUNCATED_SEQUENCE_OF_INT8));
   Input value;
-  ASSERT_EQ(Failure, ExpectTagAndGetValue(input, SEQUENCE, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          ExpectTagAndGetValue(input, SEQUENCE, value));
 }
 
 TEST_F(pkixder_input_tests, ExpectTagAndGetLength_Input_InvalidWrongTag)
 {
   Input input;
   ASSERT_EQ(Success,
             input.Init(DER_SEQUENCE_NOT_EMPTY, sizeof DER_SEQUENCE_NOT_EMPTY));
   Input value;
-  ASSERT_EQ(Failure, ExpectTagAndGetValue(input, INTEGER, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          ExpectTagAndGetValue(input, INTEGER, value));
 }
 
 TEST_F(pkixder_input_tests, ExpectTagAndGetValue_SECItem_ValidEmpty)
 {
   Input input;
   ASSERT_EQ(Success,
             input.Init(DER_SEQUENCE_EMPTY, sizeof DER_SEQUENCE_EMPTY));
   SECItem value = { siBuffer, nullptr, 5 };
@@ -671,38 +667,38 @@ TEST_F(pkixder_input_tests, ExpectTagAnd
 TEST_F(pkixder_input_tests,
        ExpectTagAndGetValue_SECItem_InvalidNotEmptyValueTruncated)
 {
   Input input;
   ASSERT_EQ(Success,
             input.Init(DER_SEQUENCE_NOT_EMPTY_VALUE_TRUNCATED,
                        sizeof DER_SEQUENCE_NOT_EMPTY_VALUE_TRUNCATED));
   SECItem value;
-  ASSERT_EQ(Failure, ExpectTagAndGetValue(input, SEQUENCE, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          ExpectTagAndGetValue(input, SEQUENCE, value));
 }
 
 TEST_F(pkixder_input_tests, ExpectTagAndGetValue_SECItem_InvalidWrongLength)
 {
   Input input;
   ASSERT_EQ(Success, input.Init(DER_TRUNCATED_SEQUENCE_OF_INT8,
                                 sizeof DER_TRUNCATED_SEQUENCE_OF_INT8));
   SECItem value;
-  ASSERT_EQ(Failure, ExpectTagAndGetValue(input, SEQUENCE, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          ExpectTagAndGetValue(input, SEQUENCE, value));
 }
 
 TEST_F(pkixder_input_tests, ExpectTagAndGetLength_SECItem_InvalidWrongTag)
 {
   Input input;
   ASSERT_EQ(Success,
             input.Init(DER_SEQUENCE_NOT_EMPTY, sizeof DER_SEQUENCE_NOT_EMPTY));
   SECItem value;
-  ASSERT_EQ(Failure, ExpectTagAndGetValue(input, INTEGER, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          ExpectTagAndGetValue(input, INTEGER, value));
 }
 
 TEST_F(pkixder_input_tests, ExpectTagAndGetTLV_SECItem_ValidEmpty)
 {
   Input input;
   ASSERT_EQ(Success,
             input.Init(DER_SEQUENCE_EMPTY, sizeof DER_SEQUENCE_EMPTY));
   SECItem tlv = { siBuffer, nullptr, 5 };
@@ -731,88 +727,88 @@ TEST_F(pkixder_input_tests, ExpectTagAnd
 TEST_F(pkixder_input_tests,
        ExpectTagAndGetTLV_SECItem_InvalidNotEmptyValueTruncated)
 {
   Input input;
   ASSERT_EQ(Success,
             input.Init(DER_SEQUENCE_NOT_EMPTY_VALUE_TRUNCATED,
                        sizeof DER_SEQUENCE_NOT_EMPTY_VALUE_TRUNCATED));
   SECItem tlv;
-  ASSERT_EQ(Failure, ExpectTagAndGetTLV(input, SEQUENCE, tlv));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          ExpectTagAndGetTLV(input, SEQUENCE, tlv));
 }
 
 TEST_F(pkixder_input_tests, ExpectTagAndGetTLV_SECItem_InvalidWrongLength)
 {
   Input input;
   ASSERT_EQ(Success, input.Init(DER_TRUNCATED_SEQUENCE_OF_INT8,
                                 sizeof DER_TRUNCATED_SEQUENCE_OF_INT8));
   SECItem tlv;
-  ASSERT_EQ(Failure, ExpectTagAndGetTLV(input, SEQUENCE, tlv));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          ExpectTagAndGetTLV(input, SEQUENCE, tlv));
 }
 
 TEST_F(pkixder_input_tests, ExpectTagAndGetTLV_SECItem_InvalidWrongTag)
 {
   Input input;
   ASSERT_EQ(Success,
             input.Init(DER_SEQUENCE_NOT_EMPTY, sizeof DER_SEQUENCE_NOT_EMPTY));
   SECItem tlv;
-  ASSERT_EQ(Failure, ExpectTagAndGetTLV(input, INTEGER, tlv));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          ExpectTagAndGetTLV(input, INTEGER, tlv));
 }
 
 TEST_F(pkixder_input_tests, ExpectTagAndSkipLength)
 {
   Input input;
   ASSERT_EQ(Success, input.Init(DER_INT16, sizeof DER_INT16));
   ASSERT_EQ(Success, ExpectTagAndSkipLength(input, INTEGER));
 }
 
 TEST_F(pkixder_input_tests, ExpectTagAndSkipLengthWithWrongTag)
 {
   Input input;
   ASSERT_EQ(Success, input.Init(DER_INT16, sizeof DER_INT16));
 
-  ASSERT_EQ(Failure, ExpectTagAndSkipLength(input, OCTET_STRING));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          ExpectTagAndSkipLength(input, OCTET_STRING));
 }
 
 TEST_F(pkixder_input_tests, EndAtEnd)
 {
   Input input;
   ASSERT_EQ(Success, input.Init(DER_INT16, sizeof DER_INT16));
   ASSERT_EQ(Success, input.Skip(4));
   ASSERT_EQ(Success, End(input));
 }
 
 TEST_F(pkixder_input_tests, EndBeforeEnd)
 {
   Input input;
   ASSERT_EQ(Success, input.Init(DER_INT16, sizeof DER_INT16));
   ASSERT_EQ(Success, input.Skip(2));
-  ASSERT_EQ(Failure, End(input));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, End(input));
 }
 
 TEST_F(pkixder_input_tests, EndAtBeginning)
 {
   Input input;
   ASSERT_EQ(Success, input.Init(DER_INT16, sizeof DER_INT16));
-  ASSERT_EQ(Failure, End(input));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, End(input));
 }
 
 // TODO: Need tests for Nested too?
 
 Result NestedOfHelper(Input& input, std::vector<uint8_t>& readValues)
 {
   uint8_t value = 0;
-  if (input.Read(value) != Success) {
-    return Failure;
+  Result rv = input.Read(value);
+  EXPECT_EQ(Success, rv);
+  if (rv != Success) {
+    return rv;
   }
   readValues.push_back(value);
   return Success;
 }
 
 TEST_F(pkixder_input_tests, NestedOf)
 {
   Input input;
@@ -833,21 +829,21 @@ TEST_F(pkixder_input_tests, NestedOf)
 
 TEST_F(pkixder_input_tests, NestedOfWithTruncatedData)
 {
   Input input;
   ASSERT_EQ(Success, input.Init(DER_TRUNCATED_SEQUENCE_OF_INT8,
                                 sizeof DER_TRUNCATED_SEQUENCE_OF_INT8));
 
   std::vector<uint8_t> readValues;
-  ASSERT_EQ(Failure,
+  ASSERT_RecoverableError(
+    SEC_ERROR_BAD_DER,
     NestedOf(input, SEQUENCE, INTEGER, EmptyAllowed::No,
              mozilla::pkix::bind(NestedOfHelper, mozilla::pkix::_1,
                                  mozilla::pkix::ref(readValues))));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
   ASSERT_EQ((size_t) 0, readValues.size());
 }
 
 TEST_F(pkixder_input_tests, MatchRestAtEnd)
 {
   Input input;
   static const uint8_t der[1] = { };
   ASSERT_EQ(Success, input.Init(der, 0));
--- a/security/pkix/test/gtest/pkixder_pki_types_tests.cpp
+++ b/security/pkix/test/gtest/pkixder_pki_types_tests.cpp
@@ -23,16 +23,17 @@
  */
 
 #include <functional>
 #include <vector>
 
 #include "nssgtest.h"
 #include "pkix/pkixtypes.h"
 #include "pkixder.h"
+#include "pkixgtest.h"
 
 using namespace mozilla::pkix;
 using namespace mozilla::pkix::der;
 using namespace mozilla::pkix::test;
 
 namespace {
 
 class pkixder_pki_types_tests : public ::testing::Test
@@ -107,36 +108,36 @@ TEST_F(pkixder_pki_types_tests, Certific
     0x00                        // length
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER_CERT_SERIAL_ZERO_LENGTH,
                                 sizeof DER_CERT_SERIAL_ZERO_LENGTH));
 
   SECItem item;
-  ASSERT_EQ(Failure, CertificateSerialNumber(input, item));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          CertificateSerialNumber(input, item));
 }
 
 TEST_F(pkixder_pki_types_tests, OptionalVersionV1ExplicitEncodingAllowed)
 {
   const uint8_t DER_OPTIONAL_VERSION_V1[] = {
     0xa0, 0x03,                   // context specific 0
     0x02, 0x01, 0x00              // INTEGER(0)
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER_OPTIONAL_VERSION_V1,
                                 sizeof DER_OPTIONAL_VERSION_V1));
 
   // XXX(bug 1031093): We shouldn't accept an explicit encoding of v1, but we
   // do here for compatibility reasons.
   // Version version;
-  // ASSERT_EQ(Failure, OptionalVersion(input, version));
-  // ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  // ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+  //                         OptionalVersion(input, version));
   der::Version version = der::Version::v3;
   ASSERT_EQ(Success, OptionalVersion(input, version));
   ASSERT_EQ(der::Version::v1, version);
 }
 
 TEST_F(pkixder_pki_types_tests, OptionalVersionV2)
 {
   const uint8_t DER_OPTIONAL_VERSION_V2[] = {
@@ -176,34 +177,32 @@ TEST_F(pkixder_pki_types_tests, Optional
     0x02, 0x01, 0x42              // INTEGER(0x42)
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER_OPTIONAL_VERSION_INVALID,
                                 sizeof DER_OPTIONAL_VERSION_INVALID));
 
   der::Version version = der::Version::v1;
-  ASSERT_EQ(Failure, OptionalVersion(input, version));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, OptionalVersion(input, version));
 }
 
 TEST_F(pkixder_pki_types_tests, OptionalVersionInvalidTooLong)
 {
   const uint8_t DER_OPTIONAL_VERSION_INVALID_TOO_LONG[] = {
     0xa0, 0x03,                   // context specific 0
     0x02, 0x02, 0x12, 0x34        // INTEGER(0x1234)
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER_OPTIONAL_VERSION_INVALID_TOO_LONG,
                                 sizeof DER_OPTIONAL_VERSION_INVALID_TOO_LONG));
 
   der::Version version;
-  ASSERT_EQ(Failure, OptionalVersion(input, version));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, OptionalVersion(input, version));
 }
 
 TEST_F(pkixder_pki_types_tests, OptionalVersionMissing)
 {
   const uint8_t DER_OPTIONAL_VERSION_MISSING[] = {
     0x02, 0x11, 0x22              // INTEGER
   };
 
@@ -297,34 +296,34 @@ TEST_F(pkixder_DigestAlgorithmIdentifier
   static const uint8_t DER[] = {
     0x30, 0x0a, 0x06, 0x08,
     0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x02, 0x05
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER, sizeof(DER)));
   DigestAlgorithm alg;
-  ASSERT_EQ(Failure, DigestAlgorithmIdentifier(input, alg));
-  ASSERT_EQ(SEC_ERROR_INVALID_ALGORITHM, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_INVALID_ALGORITHM,
+                          DigestAlgorithmIdentifier(input, alg));
 }
 
 TEST_F(pkixder_DigestAlgorithmIdentifier, Invalid_Digest_ECDSA_WITH_SHA256)
 {
   // The OID identifies ecdsa-with-SHA256 (1.2.840.10045.4.3.2). It is invalid
   // because ECDSA-with-SHA256 is not a hash algorithm.
   static const uint8_t DER[] = {
     0x30, 0x0a, 0x06, 0x08,
     0x2a, 0x86, 0x48, 0xce, 0x3d, 0x04, 0x03, 0x02, //
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER, sizeof(DER)));
   DigestAlgorithm alg;
-  ASSERT_EQ(Failure, DigestAlgorithmIdentifier(input, alg));
-  ASSERT_EQ(SEC_ERROR_INVALID_ALGORITHM, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_INVALID_ALGORITHM,
+                          DigestAlgorithmIdentifier(input, alg));
 }
 
 static const AlgorithmIdentifierTestInfo<SignatureAlgorithm>
   SIGNATURE_ALGORITHM_TEST_INFO[] =
 {
   { SignatureAlgorithm::ecdsa_with_sha512,
     { 0x30, 0x0a, 0x06, 0x08,
       0x2a, 0x86, 0x48, 0xce, 0x3d, 0x04, 0x03, 0x04 },
@@ -428,29 +427,29 @@ TEST_F(pkixder_SignatureAlgorithmIdentif
   static const uint8_t DER[] = {
     0x30, 0x0b, 0x06, 0x09,
     0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x04
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER, sizeof(DER)));
   SignatureAlgorithm alg;
-  ASSERT_EQ(Failure, SignatureAlgorithmIdentifier(input, alg));
-  ASSERT_EQ(SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED,
+                          SignatureAlgorithmIdentifier(input, alg));
 }
 
 TEST_F(pkixder_SignatureAlgorithmIdentifier, Invalid_SignatureAlgorithm_SHA256)
 {
   // The OID identifies id-sha256 (2.16.840.1.101.3.4.2.1). It is invalid
   // because SHA-256 is not a signature algorithm.
   static const uint8_t DER[] = {
     0x30, 0x0b, 0x06, 0x09,
     0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER, sizeof(DER)));
   SignatureAlgorithm alg;
-  ASSERT_EQ(Failure, SignatureAlgorithmIdentifier(input, alg));
-  ASSERT_EQ(SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED,
+                          SignatureAlgorithmIdentifier(input, alg));
 }
 
 } // unnamed namespace
--- a/security/pkix/test/gtest/pkixder_universal_types_tests.cpp
+++ b/security/pkix/test/gtest/pkixder_universal_types_tests.cpp
@@ -23,19 +23,21 @@
  */
 
 #include <limits>
 #include <vector>
 #include <gtest/gtest.h>
 
 #include "pkix/bind.h"
 #include "pkixder.h"
+#include "pkixgtest.h"
 #include "pkixtestutil.h"
 #include "stdint.h"
 
+using namespace mozilla::pkix;
 using namespace mozilla::pkix::der;
 using namespace mozilla::pkix::test;
 using namespace std;
 
 namespace {
 
 class pkixder_universal_types_tests : public ::testing::Test
 {
@@ -54,35 +56,33 @@ TEST_F(pkixder_universal_types_tests, Bo
     0x01                        // invalid
   };
 
   Input input;
   ASSERT_EQ(Success,
             input.Init(DER_BOOLEAN_TRUE_01, sizeof DER_BOOLEAN_TRUE_01));
 
   bool value = false;
-  ASSERT_EQ(Failure, Boolean(input, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, Boolean(input, value));
 }
 
 TEST_F(pkixder_universal_types_tests, BooleanTrue42)
 {
   const uint8_t DER_BOOLEAN_TRUE_42[] = {
     0x01,                       // BOOLEAN
     0x01,                       // length
     0x42                        // invalid
   };
 
   Input input;
   ASSERT_EQ(Success,
             input.Init(DER_BOOLEAN_TRUE_42, sizeof DER_BOOLEAN_TRUE_42));
 
   bool value = false;
-  ASSERT_EQ(Failure, Boolean(input, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, Boolean(input, value));
 }
 
 static const uint8_t DER_BOOLEAN_TRUE[] = {
   0x01,                       // BOOLEAN
   0x01,                       // length
   0xff                        // true
 };
 
@@ -121,34 +121,32 @@ TEST_F(pkixder_universal_types_tests, Bo
     0x42, 0x42                  // invalid
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER_BOOLEAN_INVALID_LENGTH,
                                 sizeof DER_BOOLEAN_INVALID_LENGTH));
 
   bool value = true;
-  ASSERT_EQ(Failure, Boolean(input, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, Boolean(input, value));
 }
 
 TEST_F(pkixder_universal_types_tests, BooleanInvalidZeroLength)
 {
   const uint8_t DER_BOOLEAN_INVALID_ZERO_LENGTH[] = {
     0x01,                       // BOOLEAN
     0x00                        // length
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER_BOOLEAN_INVALID_ZERO_LENGTH,
                                 sizeof DER_BOOLEAN_INVALID_ZERO_LENGTH));
 
   bool value = true;
-  ASSERT_EQ(Failure, Boolean(input, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, Boolean(input, value));
 }
 
 // OptionalBoolean implements decoding of OPTIONAL BOOLEAN DEFAULT FALSE.
 // If the field is present, it must be a valid encoding of a BOOLEAN with
 // value TRUE. If the field is not present, it defaults to FALSE. For
 // compatibility reasons, OptionalBoolean can be told to accept an encoding
 // where the field is present with value FALSE (this is technically not a
 // valid DER encoding).
@@ -203,19 +201,20 @@ TEST_F(pkixder_universal_types_tests, Op
 
   Input input1;
   ASSERT_EQ(Success, input1.Init(DER_OPTIONAL_BOOLEAN_PRESENT_FALSE,
                                  sizeof DER_OPTIONAL_BOOLEAN_PRESENT_FALSE));
   bool value;
   // If the second parameter to OptionalBoolean is false, invalid encodings
   // that include the field even when it is the DEFAULT FALSE are rejected.
   bool allowInvalidEncodings = false;
-  ASSERT_EQ(Failure, OptionalBoolean(input1, allowInvalidEncodings, value)) <<
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          OptionalBoolean(input1, allowInvalidEncodings,
+                                          value)) <<
     "Should reject an invalid encoding of present OPTIONAL BOOLEAN";
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
 
   Input input2;
   ASSERT_EQ(Success, input2.Init(DER_OPTIONAL_BOOLEAN_PRESENT_FALSE,
                                  sizeof DER_OPTIONAL_BOOLEAN_PRESENT_FALSE));
   value = true;
   // If the second parameter to OptionalBoolean is true, invalid encodings
   // that include the field even when it is the DEFAULT FALSE are accepted.
   allowInvalidEncodings = true;
@@ -230,19 +229,20 @@ TEST_F(pkixder_universal_types_tests, Op
     0x42                        // (invalid value for a BOOLEAN)
   };
 
   Input input3;
   ASSERT_EQ(Success, input3.Init(DER_OPTIONAL_BOOLEAN_PRESENT_42,
                                  sizeof DER_OPTIONAL_BOOLEAN_PRESENT_42));
   // Even with the second parameter to OptionalBoolean as true, encodings
   // of BOOLEAN that are invalid altogether are rejected.
-  ASSERT_EQ(Failure, OptionalBoolean(input3, allowInvalidEncodings, value)) <<
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER,
+                          OptionalBoolean(input3, allowInvalidEncodings,
+                                          value)) <<
     "Should reject another invalid encoding of present OPTIONAL BOOLEAN";
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
 }
 
 TEST_F(pkixder_universal_types_tests, Enumerated)
 {
   const uint8_t DER_ENUMERATED[] = {
     0x0a,                       // ENUMERATED
     0x01,                       // length
     0x42                        // value
@@ -262,17 +262,17 @@ TEST_F(pkixder_universal_types_tests, En
     0x0a,                       // ENUMERATED
     0x02,                       // length
     0x00, 0x01                  // value
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER_ENUMERATED, sizeof DER_ENUMERATED));
   uint8_t value = 0;
-  ASSERT_EQ(Failure, Enumerated(input, value));
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, Enumerated(input, value));
 }
 
 TEST_F(pkixder_universal_types_tests, EnumeratedOutOfAcceptedRange)
 {
   // Although this is a valid ENUMERATED value according to ASN.1, we
   // intentionally don't support these large values because there are no
   // ENUMERATED values in X.509 certs or OCSP this large, and we're trying to
   // keep the parser simple and fast.
@@ -282,34 +282,32 @@ TEST_F(pkixder_universal_types_tests, En
     0x12, 0x34                  // value
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER_ENUMERATED_INVALID_LENGTH,
                                 sizeof DER_ENUMERATED_INVALID_LENGTH));
 
   uint8_t value = 0;
-  ASSERT_EQ(Failure, Enumerated(input, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, Enumerated(input, value));
 }
 
 TEST_F(pkixder_universal_types_tests, EnumeratedInvalidZeroLength)
 {
   const uint8_t DER_ENUMERATED_INVALID_ZERO_LENGTH[] = {
     0x0a,                       // ENUMERATED
     0x00                        // length
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER_ENUMERATED_INVALID_ZERO_LENGTH,
                                 sizeof DER_ENUMERATED_INVALID_ZERO_LENGTH));
 
   uint8_t value = 0;
-  ASSERT_EQ(Failure, Enumerated(input, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, Enumerated(input, value));
 }
 
 ////////////////////////////////////////
 // GeneralizedTime and TimeChoice
 //
 // From RFC 5280 section 4.1.2.5.2
 //
 //   For the purposes of this profile, GeneralizedTime values MUST be
@@ -344,18 +342,20 @@ TimeChoiceForEquivalentUTCTime(const uin
   utcTimeDER[0] = 0x17; // tag UTCTime
   utcTimeDER[1] = LENGTH - 1/*tag*/ - 1/*value*/ - 2/*century*/;
   // Copy the value except for the first two digits of the year
   for (size_t i = 2; i < LENGTH - 2; ++i) {
     utcTimeDER[i] = generalizedTimeDER[i + 2];
   }
 
   Input input;
-  if (input.Init(utcTimeDER, sizeof utcTimeDER) != Success) {
-    return Failure;
+  Result rv = input.Init(utcTimeDER, sizeof utcTimeDER);
+  EXPECT_EQ(Success, rv);
+  if (rv != Success) {
+    return rv;
   }
   return TimeChoice(input, value);
 }
 
 template <uint16_t LENGTH>
 void
 ExpectGoodTime(PRTime expectedValue,
                const uint8_t (&generalizedTimeDER)[LENGTH])
@@ -391,34 +391,34 @@ template <uint16_t LENGTH>
 void
 ExpectBadTime(const uint8_t (&generalizedTimeDER)[LENGTH])
 {
   // GeneralizedTime
   {
     Input input;
     ASSERT_EQ(Success, input.Init(generalizedTimeDER, LENGTH));
     PRTime value;
-    ASSERT_EQ(Failure, GeneralizedTime(input, value));
-    EXPECT_EQ(SEC_ERROR_INVALID_TIME, PR_GetError());
+    ASSERT_RecoverableError(SEC_ERROR_INVALID_TIME,
+                            GeneralizedTime(input, value));
   }
 
   // TimeChoice: GeneralizedTime
   {
     Input input;
     ASSERT_EQ(Success, input.Init(generalizedTimeDER, LENGTH));
     PRTime value;
-    ASSERT_EQ(Failure, TimeChoice(input, value));
-    EXPECT_EQ(SEC_ERROR_INVALID_TIME, PR_GetError());
+    ASSERT_RecoverableError(SEC_ERROR_INVALID_TIME, TimeChoice(input, value));
   }
 
   // TimeChoice: UTCTime
   {
     PRTime value;
-    ASSERT_EQ(Failure,
-              TimeChoiceForEquivalentUTCTime(generalizedTimeDER, value));
+    ASSERT_RecoverableError(
+      SEC_ERROR_INVALID_TIME,
+      TimeChoiceForEquivalentUTCTime(generalizedTimeDER, value));
   }
 }
 
 // Control value: a valid time
 TEST_F(pkixder_universal_types_tests, ValidControl)
 {
   const uint8_t GT_DER[] = {
     0x18,                           // Generalized Time
@@ -448,37 +448,34 @@ TEST_F(pkixder_universal_types_tests, Ti
 
   PRTime value;
 
   // GeneralizedTime
   Input gt;
   ASSERT_EQ(Success,
             gt.Init(DER_GENERALIZED_TIME_INVALID_ZERO_LENGTH,
                     sizeof DER_GENERALIZED_TIME_INVALID_ZERO_LENGTH));
-  ASSERT_EQ(Failure, GeneralizedTime(gt, value));
-  ASSERT_EQ(SEC_ERROR_INVALID_TIME, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_INVALID_TIME, GeneralizedTime(gt, value));
 
   // TimeChoice: GeneralizedTime
   Input tc_gt;
   ASSERT_EQ(Success,
             tc_gt.Init(DER_GENERALIZED_TIME_INVALID_ZERO_LENGTH,
                        sizeof DER_GENERALIZED_TIME_INVALID_ZERO_LENGTH));
-  ASSERT_EQ(Failure, TimeChoice(tc_gt, value));
-  ASSERT_EQ(SEC_ERROR_INVALID_TIME, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_INVALID_TIME, TimeChoice(tc_gt, value));
 
   // TimeChoice: UTCTime
   const uint8_t DER_UTCTIME_INVALID_ZERO_LENGTH[] = {
     0x17, // UTCTime
     0x00  // Length = 0
   };
   Input tc_utc;
   ASSERT_EQ(Success, tc_utc.Init(DER_UTCTIME_INVALID_ZERO_LENGTH,
                                  sizeof DER_UTCTIME_INVALID_ZERO_LENGTH));
-  ASSERT_EQ(Failure, TimeChoice(tc_utc, value));
-  ASSERT_EQ(SEC_ERROR_INVALID_TIME, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_INVALID_TIME, TimeChoice(tc_utc, value));
 }
 
 // A non zulu time should fail
 TEST_F(pkixder_universal_types_tests, TimeInvalidLocal)
 {
   const uint8_t DER_GENERALIZED_TIME_INVALID_LOCAL[] = {
     0x18,                           // Generalized Time
     14,                             // Length = 14
@@ -748,27 +745,26 @@ TEST_F(pkixder_universal_types_tests, Ti
 
   // We don't use ExpectBadTime here because UTCTime can't represent 2100.
 
   // GeneralizedTime
   {
     Input input;
     ASSERT_EQ(Success, input.Init(DER, sizeof(DER)));
     PRTime value;
-    ASSERT_EQ(Failure, GeneralizedTime(input, value));
-    EXPECT_EQ(SEC_ERROR_INVALID_TIME, PR_GetError());
+    ASSERT_RecoverableError(SEC_ERROR_INVALID_TIME,
+                            GeneralizedTime(input, value));
   }
 
   // TimeChoice: GeneralizedTime
   {
     Input input;
     ASSERT_EQ(Success, input.Init(DER, sizeof(DER)));
     PRTime value;
-    ASSERT_EQ(Failure, TimeChoice(input, value));
-    EXPECT_EQ(SEC_ERROR_INVALID_TIME, PR_GetError());
+    ASSERT_RecoverableError(SEC_ERROR_INVALID_TIME, TimeChoice(input, value));
   }
 }
 
 TEST_F(pkixder_universal_types_tests, TimeHoursValidRange)
 {
   for (uint8_t i = 0; i <= 23; ++i) {
     const uint8_t DER[] = {
       0x18,                           // Generalized Time
@@ -890,29 +886,28 @@ TEST_F(pkixder_universal_types_tests, Ti
 
   // GeneralizedTime
   {
     Input input;
     ASSERT_EQ(Success,
               input.Init(DER_GENERALIZED_TIME_INVALID_CENTURY_CHAR,
                          sizeof DER_GENERALIZED_TIME_INVALID_CENTURY_CHAR));
     PRTime value = 0;
-    ASSERT_EQ(Failure, GeneralizedTime(input, value));
-    EXPECT_EQ(SEC_ERROR_INVALID_TIME, PR_GetError());
+    ASSERT_RecoverableError(SEC_ERROR_INVALID_TIME,
+                            GeneralizedTime(input, value));
   }
 
   // TimeChoice: GeneralizedTime
   {
     Input input;
     ASSERT_EQ(Success,
               input.Init(DER_GENERALIZED_TIME_INVALID_CENTURY_CHAR,
                          sizeof DER_GENERALIZED_TIME_INVALID_CENTURY_CHAR));
     PRTime value = 0;
-    ASSERT_EQ(Failure, TimeChoice(input, value));
-    EXPECT_EQ(SEC_ERROR_INVALID_TIME, PR_GetError());
+    ASSERT_RecoverableError(SEC_ERROR_INVALID_TIME, TimeChoice(input, value));
   }
 
   // This test is not applicable to TimeChoice: UTCTime
 }
 
 TEST_F(pkixder_universal_types_tests, TimeInvalidYearChar)
 {
   const uint8_t DER_GENERALIZED_TIME_INVALID_YEAR_CHAR[] = {
@@ -985,18 +980,17 @@ TEST_F(pkixder_universal_types_tests, In
     0x01, // length
     0xff, // -1 (two's complement)
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER, sizeof DER));
 
   uint8_t value;
-  ASSERT_EQ(Failure, Integer(input, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, Integer(input, value));
 }
 
 TEST_F(pkixder_universal_types_tests, Integer_Negative128)
 {
   // This is a valid integer value but our integer parser cannot parse
   // negative values.
 
   static const uint8_t DER[] = {
@@ -1004,18 +998,17 @@ TEST_F(pkixder_universal_types_tests, In
     0x01, // length
     0x80, // -128 (two's complement)
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER, sizeof DER));
 
   uint8_t value;
-  ASSERT_EQ(Failure, Integer(input, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, Integer(input, value));
 }
 
 TEST_F(pkixder_universal_types_tests, Integer_128)
 {
   // This is a valid integer value but our integer parser cannot parse
   // values that require more than one byte to encode.
 
   static const uint8_t DER[] = {
@@ -1023,18 +1016,17 @@ TEST_F(pkixder_universal_types_tests, In
     0x02, // length
     0x00, 0x80 // 128
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER, sizeof DER));
 
   uint8_t value;
-  ASSERT_EQ(Failure, Integer(input, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, Integer(input, value));
 }
 
 TEST_F(pkixder_universal_types_tests, Integer11223344)
 {
   // This is a valid integer value but our integer parser cannot parse
   // values that require more than one byte to be encoded.
 
   static const uint8_t DER[] = {
@@ -1042,100 +1034,94 @@ TEST_F(pkixder_universal_types_tests, In
     0x04,                       // length
     0x11, 0x22, 0x33, 0x44      // 0x11223344
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER, sizeof DER));
 
   uint8_t value;
-  ASSERT_EQ(Failure, Integer(input, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, Integer(input, value));
 }
 
 TEST_F(pkixder_universal_types_tests, IntegerTruncatedOneByte)
 {
   const uint8_t DER_INTEGER_TRUNCATED[] = {
     0x02,                       // INTEGER
     0x01,                       // length
     // MISSING DATA HERE
   };
 
   Input input;
   ASSERT_EQ(Success,
             input.Init(DER_INTEGER_TRUNCATED, sizeof DER_INTEGER_TRUNCATED));
 
   uint8_t value;
-  ASSERT_EQ(Failure, Integer(input, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, Integer(input, value));
 }
 
 TEST_F(pkixder_universal_types_tests, IntegerTruncatedLarge)
 {
   const uint8_t DER_INTEGER_TRUNCATED[] = {
     0x02,                       // INTEGER
     0x04,                       // length
     0x11, 0x22                  // 0x1122
     // MISSING DATA HERE
   };
 
   Input input;
   ASSERT_EQ(Success,
             input.Init(DER_INTEGER_TRUNCATED, sizeof DER_INTEGER_TRUNCATED));
 
   uint8_t value;
-  ASSERT_EQ(Failure, Integer(input, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, Integer(input, value));
 }
 
 TEST_F(pkixder_universal_types_tests, IntegerZeroLength)
 {
   const uint8_t DER_INTEGER_ZERO_LENGTH[] = {
     0x02,                       // INTEGER
     0x00                        // length
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER_INTEGER_ZERO_LENGTH,
                                 sizeof DER_INTEGER_ZERO_LENGTH));
   uint8_t value;
-  ASSERT_EQ(Failure, Integer(input, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, Integer(input, value));
 }
 
 TEST_F(pkixder_universal_types_tests, IntegerOverlyLong1)
 {
   const uint8_t DER_INTEGER_OVERLY_LONG1[] = {
     0x02,                       // INTEGER
     0x02,                       // length
     0x00, 0x01                  //
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER_INTEGER_OVERLY_LONG1,
                                 sizeof DER_INTEGER_OVERLY_LONG1));
   uint8_t value;
-  ASSERT_EQ(Failure, Integer(input, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, Integer(input, value));
 }
 
 TEST_F(pkixder_universal_types_tests, IntegerOverlyLong2)
 {
   const uint8_t DER_INTEGER_OVERLY_LONG2[] = {
     0x02,                       // INTEGER
     0x02,                       // length
     0xff, 0x80                  //
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER_INTEGER_OVERLY_LONG2,
                                 sizeof DER_INTEGER_OVERLY_LONG2));
   uint8_t value;
-  ASSERT_EQ(Failure, Integer(input, value));
-  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, Integer(input, value));
 }
 
 TEST_F(pkixder_universal_types_tests, OptionalIntegerSupportedDefault)
 {
   // The input is a BOOLEAN and not INTEGER for the input so we'll not parse
   // anything and instead use the default value.
   Input input;
   ASSERT_EQ(Success, input.Init(DER_BOOLEAN_TRUE, sizeof DER_BOOLEAN_TRUE));
@@ -1148,18 +1134,17 @@ TEST_F(pkixder_universal_types_tests, Op
 
 TEST_F(pkixder_universal_types_tests, OptionalIntegerUnsupportedDefault)
 {
   // The same as the previous test, except with an unsupported default value
   // passed in.
   Input input;
   ASSERT_EQ(Success, input.Init(DER_BOOLEAN_TRUE, sizeof DER_BOOLEAN_TRUE));
   long value;
-  ASSERT_EQ(Failure, OptionalInteger(input, 0, value));
-  ASSERT_EQ(SEC_ERROR_INVALID_ARGS, PR_GetError());
+  ASSERT_FatalError(SEC_ERROR_INVALID_ARGS, OptionalInteger(input, 0, value));
 }
 
 TEST_F(pkixder_universal_types_tests, OptionalIntegerSupportedDefaultAtEnd)
 {
   static const uint8_t dummy = 1;
 
   Input input;
   ASSERT_EQ(Success, input.Init(&dummy, 0));
@@ -1203,17 +1188,17 @@ TEST_F(pkixder_universal_types_tests, Nu
     0x01,
     0x00
   };
 
   Input input;
   ASSERT_EQ(Success,
             input.Init(DER_NULL_BAD_LENGTH, sizeof DER_NULL_BAD_LENGTH));
 
-  ASSERT_EQ(Failure, Null(input));
+  ASSERT_RecoverableError(SEC_ERROR_BAD_DER, Null(input));
 }
 
 TEST_F(pkixder_universal_types_tests, OID)
 {
   const uint8_t DER_VALID_OID[] = {
     0x06,
     0x09,
     0x2B, 0x06, 0x01, 0x05, 0x05, 0x07, 0x30, 0x01, 0x01
--- a/security/pkix/test/lib/pkixtestutil.cpp
+++ b/security/pkix/test/lib/pkixtestutil.cpp
@@ -152,32 +152,32 @@ public:
   Output()
     : numItems(0)
     , length(0)
   {
   }
 
   // Makes a shallow copy of the input item. All input items must have a
   // lifetime that extends at least to where Squash is called.
-  der::Result Add(const SECItem* item)
+  Result Add(const SECItem* item)
   {
     PR_ASSERT(item);
     PR_ASSERT(item->data);
 
     if (numItems >= MaxSequenceItems) {
       return der::Fail(SEC_ERROR_INVALID_ARGS);
     }
     if (length + item->len > 65535) {
       return der::Fail(SEC_ERROR_INVALID_ARGS);
     }
 
     contents[numItems] = item;
     numItems++;
     length += item->len;
-    return der::Success;
+    return Success;
   }
 
   SECItem* Squash(PLArenaPool* arena, uint8_t tag)
   {
     PR_ASSERT(arena);
 
     size_t lengthLength = length < 128 ? 1
                         : length < 256 ? 2
@@ -260,17 +260,17 @@ static SECItem* KeyHash(OCSPResponseCont
 static SECItem* SingleResponse(OCSPResponseContext& context);
 static SECItem* CertID(OCSPResponseContext& context);
 static SECItem* CertStatus(OCSPResponseContext& context);
 
 static SECItem*
 EncodeNested(PLArenaPool* arena, uint8_t tag, const SECItem* inner)
 {
   Output output;
-  if (output.Add(inner) != der::Success) {
+  if (output.Add(inner) != Success) {
     return nullptr;
   }
   return output.Squash(arena, tag);
 }
 
 // A return value of 0 is an error, but this should never happen in practice
 // because this function aborts in that case.
 static size_t
@@ -552,27 +552,27 @@ SignedData(PLArenaPool* arena, const SEC
                                der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 0,
                                certsSequence);
     if (!certsNested) {
       return nullptr;
     }
   }
 
   Output output;
-  if (output.Add(tbsData) != der::Success) {
+  if (output.Add(tbsData) != Success) {
     return nullptr;
   }
-  if (output.Add(signatureAlgorithm) != der::Success) {
+  if (output.Add(signatureAlgorithm) != Success) {
     return nullptr;
   }
-  if (output.Add(signatureNested) != der::Success) {
+  if (output.Add(signatureNested) != Success) {
     return nullptr;
   }
   if (certsNested) {
-    if (output.Add(certsNested) != der::Success) {
+    if (output.Add(certsNested) != Success) {
       return nullptr;
     }
   }
   return output.Squash(arena, der::SEQUENCE);
 }
 
 // Extension  ::=  SEQUENCE  {
 //      extnID      OBJECT IDENTIFIER,
@@ -593,36 +593,36 @@ Extension(PLArenaPool* arena, SECOidTag 
   }
 
   Output output;
 
   const SECItem* extnID(OID(arena, extnIDTag));
   if (!extnID) {
     return nullptr;
   }
-  if (output.Add(extnID) != der::Success) {
+  if (output.Add(extnID) != Success) {
     return nullptr;
   }
 
   if (criticality == ExtensionCriticality::Critical) {
     SECItem* critical(Boolean(arena, true));
-    if (output.Add(critical) != der::Success) {
+    if (output.Add(critical) != Success) {
       return nullptr;
     }
   }
 
   SECItem* extnValueBytes(value.Squash(arena, der::SEQUENCE));
   if (!extnValueBytes) {
     return nullptr;
   }
   SECItem* extnValue(EncodeNested(arena, der::OCTET_STRING, extnValueBytes));
   if (!extnValue) {
     return nullptr;
   }
-  if (output.Add(extnValue) != der::Success) {
+  if (output.Add(extnValue) != Success) {
     return nullptr;
   }
 
   return output.Squash(arena, der::SEQUENCE);
 }
 
 SECItem*
 MaybeLogOutput(SECItem* result, const char* suffix)
@@ -798,34 +798,34 @@ TBSCertificate(PLArenaPool* arena, long 
       return nullptr;
     }
     SECItem* version(EncodeNested(arena,
                                   der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 0,
                                   versionInteger));
     if (!version) {
       return nullptr;
     }
-    if (output.Add(version) != der::Success) {
+    if (output.Add(version) != Success) {
       return nullptr;
     }
   }
 
-  if (output.Add(serialNumber) != der::Success) {
+  if (output.Add(serialNumber) != Success) {
     return nullptr;
   }
 
   SECItem* signature(AlgorithmIdentifier(arena, signatureOidTag));
   if (!signature) {
     return nullptr;
   }
-  if (output.Add(signature) != der::Success) {
+  if (output.Add(signature) != Success) {
     return nullptr;
   }
 
-  if (output.Add(issuer) != der::Success) {
+  if (output.Add(issuer) != Success) {
     return nullptr;
   }
 
   // Validity ::= SEQUENCE {
   //       notBefore      Time,
   //       notAfter       Time }
   SECItem* validity;
   {
@@ -833,66 +833,66 @@ TBSCertificate(PLArenaPool* arena, long 
     if (!notBefore) {
       return nullptr;
     }
     SECItem* notAfter(PRTimeToTimeChoice(arena, notAfterTime));
     if (!notAfter) {
       return nullptr;
     }
     Output validityOutput;
-    if (validityOutput.Add(notBefore) != der::Success) {
+    if (validityOutput.Add(notBefore) != Success) {
       return nullptr;
     }
-    if (validityOutput.Add(notAfter) != der::Success) {
+    if (validityOutput.Add(notAfter) != Success) {
       return nullptr;
     }
     validity = validityOutput.Squash(arena, der::SEQUENCE);
     if (!validity) {
       return nullptr;
     }
   }
-  if (output.Add(validity) != der::Success) {
+  if (output.Add(validity) != Success) {
     return nullptr;
   }
 
-  if (output.Add(subject) != der::Success) {
+  if (output.Add(subject) != Success) {
     return nullptr;
   }
 
   // SubjectPublicKeyInfo  ::=  SEQUENCE  {
   //       algorithm            AlgorithmIdentifier,
   //       subjectPublicKey     BIT STRING  }
   ScopedSECItem subjectPublicKeyInfo(
     SECKEY_EncodeDERSubjectPublicKeyInfo(subjectPublicKey));
   if (!subjectPublicKeyInfo) {
     return nullptr;
   }
-  if (output.Add(subjectPublicKeyInfo.get()) != der::Success) {
+  if (output.Add(subjectPublicKeyInfo.get()) != Success) {
     return nullptr;
   }
 
   if (extensions) {
     Output extensionsOutput;
     while (*extensions) {
-      if (extensionsOutput.Add(*extensions) != der::Success) {
+      if (extensionsOutput.Add(*extensions) != Success) {
         return nullptr;
       }
       ++extensions;
     }
     SECItem* allExtensions(extensionsOutput.Squash(arena, der::SEQUENCE));
     if (!allExtensions) {
       return nullptr;
     }
     SECItem* extensionsWrapped(
       EncodeNested(arena, der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 3,
                    allExtensions));
     if (!extensions) {
       return nullptr;
     }
-    if (output.Add(extensionsWrapped) != der::Success) {
+    if (output.Add(extensionsWrapped) != Success) {
       return nullptr;
     }
   }
 
   return output.Squash(arena, der::SEQUENCE);
 }
 
 const SECItem*
@@ -924,27 +924,27 @@ CreateEncodedBasicConstraints(PLArenaPoo
   if (!arena) {
     PR_SetError(SEC_ERROR_INVALID_ARGS, 0);
     return nullptr;
   }
 
   Output value;
 
   if (isCA) {
-    if (value.Add(Boolean(arena, true)) != der::Success) {
+    if (value.Add(Boolean(arena, true)) != Success) {
       return nullptr;
     }
   }
 
   if (pathLenConstraintValue) {
     SECItem* pathLenConstraint(Integer(arena, *pathLenConstraintValue));
     if (!pathLenConstraint) {
       return nullptr;
     }
-    if (value.Add(pathLenConstraint) != der::Success) {
+    if (value.Add(pathLenConstraint) != Success) {
       return nullptr;
     }
   }
 
   return Extension(arena, SEC_OID_X509_BASIC_CONSTRAINTS, criticality, value);
 }
 
 // ExtKeyUsageSyntax ::= SEQUENCE SIZE (1..MAX) OF KeyPurposeId
@@ -961,17 +961,17 @@ CreateEncodedEKUExtension(PLArenaPool* a
   }
 
   Output value;
   for (size_t i = 0; i < ekusCount; ++i) {
     SECItem* encodedEKUOID = OID(arena, ekus[i]);
     if (!encodedEKUOID) {
       return nullptr;
     }
-    if (value.Add(encodedEKUOID) != der::Success) {
+    if (value.Add(encodedEKUOID) != Success) {
       return nullptr;
     }
   }
 
   return Extension(arena, SEC_OID_X509_EXT_KEY_USAGE, criticality, value);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -1025,21 +1025,21 @@ CreateEncodedOCSPResponse(OCSPResponseCo
                                        der::CONTEXT_SPECIFIC,
                                        responseBytes);
     if (!responseBytesNested) {
       return nullptr;
     }
   }
 
   Output output;
-  if (output.Add(responseStatus) != der::Success) {
+  if (output.Add(responseStatus) != Success) {
     return nullptr;
   }
   if (responseBytesNested) {
-    if (output.Add(responseBytesNested) != der::Success) {
+    if (output.Add(responseBytesNested) != Success) {
       return nullptr;
     }
   }
   return MaybeLogOutput(output.Squash(context.arena, der::SEQUENCE), "ocsp");
 }
 
 // ResponseBytes ::= SEQUENCE {
 //    responseType            OBJECT IDENTIFIER,
@@ -1062,20 +1062,20 @@ ResponseBytes(OCSPResponseContext& conte
   }
   SECItem* responseNested = EncodeNested(context.arena, der::OCTET_STRING,
                                          response);
   if (!responseNested) {
     return nullptr;
   }
 
   Output output;
-  if (output.Add(&id_pkix_ocsp_basic) != der::Success) {
+  if (output.Add(&id_pkix_ocsp_basic) != Success) {
     return nullptr;
   }
-  if (output.Add(responseNested) != der::Success) {
+  if (output.Add(responseNested) != Success) {
     return nullptr;
   }
   return output.Squash(context.arena, der::SEQUENCE);
 }
 
 // BasicOCSPResponse ::= SEQUENCE {
 //   tbsResponseData          ResponseData,
 //   signatureAlgorithm       AlgorithmIdentifier,
@@ -1099,36 +1099,36 @@ BasicOCSPResponse(OCSPResponseContext& c
 //   id               OBJECT IDENTIFIER,
 //   critical         BOOLEAN DEFAULT FALSE
 //   value            OCTET STRING
 // }
 static SECItem*
 OCSPExtension(OCSPResponseContext& context, OCSPResponseExtension* extension)
 {
   Output output;
-  if (output.Add(&extension->id) != der::Success) {
+  if (output.Add(&extension->id) != Success) {
     return nullptr;
   }
   if (extension->critical) {
     static const uint8_t trueEncoded[3] = { 0x01, 0x01, 0xFF };
     SECItem critical = {
       siBuffer,
       const_cast<uint8_t*>(trueEncoded),
       PR_ARRAY_SIZE(trueEncoded)
     };
-    if (output.Add(&critical) != der::Success) {
+    if (output.Add(&critical) != Success) {
       return nullptr;
     }
   }
   SECItem* value = EncodeNested(context.arena, der::OCTET_STRING,
                                 &extension->value);
   if (!value) {
     return nullptr;
   }
-  if (output.Add(value) != der::Success) {
+  if (output.Add(value) != Success) {
     return nullptr;
   }
   return output.Squash(context.arena, der::SEQUENCE);
 }
 
 // Extensions ::= [1] {
 //   SEQUENCE OF Extension
 // }
@@ -1137,17 +1137,17 @@ Extensions(OCSPResponseContext& context)
 {
   Output output;
   for (OCSPResponseExtension* extension = context.extensions;
        extension; extension = extension->next) {
     SECItem* extensionEncoded = OCSPExtension(context, extension);
     if (!extensionEncoded) {
       return nullptr;
     }
-    if (output.Add(extensionEncoded) != der::Success) {
+    if (output.Add(extensionEncoded) != Success) {
       return nullptr;
     }
   }
   SECItem* extensionsEncoded = output.Squash(context.arena, der::SEQUENCE);
   if (!extensionsEncoded) {
     return nullptr;
   }
   return EncodeNested(context.arena,
@@ -1185,27 +1185,27 @@ ResponseData(OCSPResponseContext& contex
     return nullptr;
   }
   SECItem* responseExtensions = nullptr;
   if (context.extensions || context.includeEmptyExtensions) {
     responseExtensions = Extensions(context);
   }
 
   Output output;
-  if (output.Add(responderID) != der::Success) {
+  if (output.Add(responderID) != Success) {
     return nullptr;
   }
-  if (output.Add(producedAtEncoded) != der::Success) {
+  if (output.Add(producedAtEncoded) != Success) {
     return nullptr;
   }
-  if (output.Add(responsesNested) != der::Success) {
+  if (output.Add(responsesNested) != Success) {
     return nullptr;
   }
   if (responseExtensions) {
-    if (output.Add(responseExtensions) != der::Success) {
+    if (output.Add(responseExtensions) != Success) {
       return nullptr;
     }
   }
   return output.Squash(context.arena, der::SEQUENCE);
 }
 
 // ResponderID ::= CHOICE {
 //    byName              [1] Name,
@@ -1290,27 +1290,27 @@ SingleResponse(OCSPResponseContext& cont
                                            0,
                                            nextUpdateEncoded);
     if (!nextUpdateEncodedNested) {
       return nullptr;
     }
   }
 
   Output output;
-  if (output.Add(certID) != der::Success) {
+  if (output.Add(certID) != Success) {
     return nullptr;
   }
-  if (output.Add(certStatus) != der::Success) {
+  if (output.Add(certStatus) != Success) {
     return nullptr;
   }
-  if (output.Add(thisUpdateEncoded) != der::Success) {
+  if (output.Add(thisUpdateEncoded) != Success) {
     return nullptr;
   }
   if (nextUpdateEncodedNested) {
-    if (output.Add(nextUpdateEncodedNested) != der::Success) {
+    if (output.Add(nextUpdateEncodedNested) != Success) {
       return nullptr;
     }
   }
   return output.Squash(context.arena, der::SEQUENCE);
 }
 
 // CertID          ::=     SEQUENCE {
 //        hashAlgorithm       AlgorithmIdentifier,
@@ -1350,26 +1350,26 @@ CertID(OCSPResponseContext& context)
   SECItem* serialNumber = SEC_ASN1EncodeItem(context.arena, nullptr,
                                              &context.certID.serialNumber,
                                              serialTemplate);
   if (!serialNumber) {
     return nullptr;
   }
 
   Output output;
-  if (output.Add(hashAlgorithm) != der::Success) {
+  if (output.Add(hashAlgorithm) != Success) {
     return nullptr;
   }
-  if (output.Add(issuerNameHash) != der::Success) {
+  if (output.Add(issuerNameHash) != Success) {
     return nullptr;
   }
-  if (output.Add(issuerKeyHash) != der::Success) {
+  if (output.Add(issuerKeyHash) != Success) {
     return nullptr;
   }
-  if (output.Add(serialNumber) != der::Success) {
+  if (output.Add(serialNumber) != Success) {
     return nullptr;
   }
   return output.Squash(context.arena, der::SEQUENCE);
 }
 
 // CertStatus ::= CHOICE {
 //    good                [0] IMPLICIT NULL,
 //    revoked             [1] IMPLICIT RevokedInfo,