Bug 1296317 - Stop calling PR_SetError() in VerifyCert() and VerifySSLServerCert(). r=keeler draft
authorCykesiopka <cykesiopka.bmo@gmail.com>
Mon, 10 Oct 2016 15:44:41 +0800
changeset 423120 f09e37c6a3a930c30cce003139df86bc84d771ee
parent 423119 7fbbd504a13053564b9fa78dbe350647e3d21e97
child 533361 58ec903be1cb0653087cb5cdeb9990e1dabd67e0
push id31800
push usercykesiopka.bmo@gmail.com
push dateMon, 10 Oct 2016 08:07:59 +0000
reviewerskeeler
bugs1296317
milestone52.0a1
Bug 1296317 - Stop calling PR_SetError() in VerifyCert() and VerifySSLServerCert(). r=keeler The PR_SetError() + PR_GetError() pattern currently used is error prone and unnecessary. The functions involved can instead return mozilla::pkix::Result, which is equally expressive and more robust. MozReview-Commit-ID: Hkd39eqTvds
security/certverifier/CertVerifier.cpp
security/certverifier/CertVerifier.h
security/manager/ssl/SSLServerCertVerification.cpp
security/manager/ssl/nsDataSignatureVerifier.cpp
security/manager/ssl/nsNSSCertificate.cpp
security/manager/ssl/nsNSSCertificateDB.cpp
security/manager/ssl/nsNSSIOLayer.cpp
security/manager/ssl/nsSiteSecurityService.cpp
--- a/security/certverifier/CertVerifier.cpp
+++ b/security/certverifier/CertVerifier.cpp
@@ -3,17 +3,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "CertVerifier.h"
 
 #include <stdint.h>
 
-#include "BRNameMatchingPolicy.h"
 #include "CTKnownLogs.h"
 #include "ExtendedValidation.h"
 #include "MultiLogCTVerifier.h"
 #include "NSSCertDBTrustDomain.h"
 #include "NSSErrorsService.h"
 #include "cert.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/Casting.h"
@@ -317,17 +316,17 @@ CertVerifier::SHA1ModeMoreRestrictiveTha
       MOZ_ASSERT(false, "unexpected SHA1Mode type");
       return true;
   }
 }
 
 static const unsigned int MIN_RSA_BITS = 2048;
 static const unsigned int MIN_RSA_BITS_WEAK = 1024;
 
-SECStatus
+Result
 CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage,
                          Time time, void* pinArg, const char* hostname,
                  /*out*/ UniqueCERTCertList& builtChain,
             /*optional*/ const Flags flags,
             /*optional*/ const SECItem* stapledOCSPResponseSECItem,
             /*optional*/ const SECItem* sctsFromTLSSECItem,
         /*optional out*/ SECOidTag* evOidPolicy,
         /*optional out*/ OCSPStaplingStatus* ocspStaplingStatus,
@@ -343,51 +342,44 @@ CertVerifier::VerifyCert(CERTCertificate
   PR_ASSERT(usage == certificateUsageSSLServer || !keySizeStatus);
   PR_ASSERT(usage == certificateUsageSSLServer || !sha1ModeResult);
 
   if (evOidPolicy) {
     *evOidPolicy = SEC_OID_UNKNOWN;
   }
   if (ocspStaplingStatus) {
     if (usage != certificateUsageSSLServer) {
-      PR_SetError(SEC_ERROR_INVALID_ARGS, 0);
-      return SECFailure;
+      return Result::FATAL_ERROR_INVALID_ARGS;
     }
     *ocspStaplingStatus = OCSP_STAPLING_NEVER_CHECKED;
   }
 
   if (keySizeStatus) {
     if (usage != certificateUsageSSLServer) {
-      PR_SetError(SEC_ERROR_INVALID_ARGS, 0);
-      return SECFailure;
+      return Result::FATAL_ERROR_INVALID_ARGS;
     }
     *keySizeStatus = KeySizeStatus::NeverChecked;
   }
 
   if (sha1ModeResult) {
     if (usage != certificateUsageSSLServer) {
-      PR_SetError(SEC_ERROR_INVALID_ARGS, 0);
-      return SECFailure;
+      return Result::FATAL_ERROR_INVALID_ARGS;
     }
     *sha1ModeResult = SHA1ModeResult::NeverChecked;
   }
 
   if (!cert ||
       (usage != certificateUsageSSLServer && (flags & FLAG_MUST_BE_EV))) {
-    PR_SetError(SEC_ERROR_INVALID_ARGS, 0);
-    return SECFailure;
+    return Result::FATAL_ERROR_INVALID_ARGS;
   }
 
-  Result rv;
-
   Input certDER;
-  rv = certDER.Init(cert->derCert.data, cert->derCert.len);
+  Result rv = certDER.Init(cert->derCert.data, cert->derCert.len);
   if (rv != Success) {
-    PR_SetError(MapResultToPRErrorCode(rv), 0);
-    return SECFailure;
+    return rv;
   }
 
   // We configure the OCSP fetching modes separately for EV and non-EV
   // verifications.
   NSSCertDBTrustDomain::OCSPFetching defaultOCSPFetching
     = (mOCSPDownloadConfig == ocspOff) ||
       (mOCSPDownloadConfig == ocspEVOnly) ||
       (flags & FLAG_LOCAL_ONLY) ? NSSCertDBTrustDomain::NeverFetchOCSP
@@ -399,18 +391,17 @@ CertVerifier::VerifyCert(CERTCertificate
 
   Input stapledOCSPResponseInput;
   const Input* stapledOCSPResponse = nullptr;
   if (stapledOCSPResponseSECItem) {
     rv = stapledOCSPResponseInput.Init(stapledOCSPResponseSECItem->data,
                                        stapledOCSPResponseSECItem->len);
     if (rv != Success) {
       // The stapled OCSP response was too big.
-      PR_SetError(SEC_ERROR_OCSP_MALFORMED_RESPONSE, 0);
-      return SECFailure;
+      return Result::ERROR_OCSP_MALFORMED_RESPONSE;
     }
     stapledOCSPResponse = &stapledOCSPResponseInput;
   }
 
   Input sctsFromTLSInput;
   if (sctsFromTLSSECItem) {
     rv = sctsFromTLSInput.Init(sctsFromTLSSECItem->data,
                                sctsFromTLSSECItem->len);
@@ -792,24 +783,23 @@ CertVerifier::VerifyCert(CERTCertificate
       break;
     }
 
     default:
       rv = Result::FATAL_ERROR_INVALID_ARGS;
   }
 
   if (rv != Success) {
-    PR_SetError(MapResultToPRErrorCode(rv), 0);
-    return SECFailure;
+    return rv;
   }
 
-  return SECSuccess;
+  return Success;
 }
 
-SECStatus
+Result
 CertVerifier::VerifySSLServerCert(const UniqueCERTCertificate& peerCert,
                      /*optional*/ const SECItem* stapledOCSPResponse,
                      /*optional*/ const SECItem* sctsFromTLS,
                                   Time time,
                      /*optional*/ void* pinarg,
                                   const char* hostname,
                           /*out*/ UniqueCERTCertList& builtChain,
                      /*optional*/ bool saveIntermediatesInPermanentDatabase,
@@ -826,89 +816,79 @@ CertVerifier::VerifySSLServerCert(const 
   PR_ASSERT(hostname);
   PR_ASSERT(hostname[0]);
 
   if (evOidPolicy) {
     *evOidPolicy = SEC_OID_UNKNOWN;
   }
 
   if (!hostname || !hostname[0]) {
-    PR_SetError(SSL_ERROR_BAD_CERT_DOMAIN, 0);
-    return SECFailure;
+    return Result::ERROR_BAD_CERT_DOMAIN;
   }
 
   // CreateCertErrorRunnable assumes that CheckCertHostname is only called
   // if VerifyCert succeeded.
-  SECStatus rv = VerifyCert(peerCert.get(), certificateUsageSSLServer, time,
-                            pinarg, hostname, builtChain, flags,
-                            stapledOCSPResponse, sctsFromTLS,
-                            evOidPolicy, ocspStaplingStatus, keySizeStatus,
-                            sha1ModeResult, pinningTelemetryInfo,
-                            ctInfo);
-  if (rv != SECSuccess) {
+  Result rv = VerifyCert(peerCert.get(), certificateUsageSSLServer, time,
+                         pinarg, hostname, builtChain, flags,
+                         stapledOCSPResponse, sctsFromTLS, evOidPolicy,
+                         ocspStaplingStatus, keySizeStatus, sha1ModeResult,
+                         pinningTelemetryInfo, ctInfo);
+  if (rv != Success) {
     return rv;
   }
 
   Input peerCertInput;
-  Result result = peerCertInput.Init(peerCert->derCert.data,
-                                     peerCert->derCert.len);
-  if (result != Success) {
-    PR_SetError(MapResultToPRErrorCode(result), 0);
-    return SECFailure;
+  rv = peerCertInput.Init(peerCert->derCert.data, peerCert->derCert.len);
+  if (rv != Success) {
+    return rv;
   }
 
   Input stapledOCSPResponseInput;
   Input* responseInputPtr = nullptr;
   if (stapledOCSPResponse) {
-    result = stapledOCSPResponseInput.Init(stapledOCSPResponse->data,
-                                           stapledOCSPResponse->len);
-    if (result != Success) {
+    rv = stapledOCSPResponseInput.Init(stapledOCSPResponse->data,
+                                       stapledOCSPResponse->len);
+    if (rv != Success) {
       // The stapled OCSP response was too big.
-      PR_SetError(SEC_ERROR_OCSP_MALFORMED_RESPONSE, 0);
-      return SECFailure;
+      return Result::ERROR_OCSP_MALFORMED_RESPONSE;
     }
     responseInputPtr = &stapledOCSPResponseInput;
   }
 
   if (!(flags & FLAG_TLS_IGNORE_STATUS_REQUEST)) {
-    result = CheckTLSFeaturesAreSatisfied(peerCertInput, responseInputPtr);
-
-    if (result != Success) {
-      PR_SetError(MapResultToPRErrorCode(result), 0);
-      return SECFailure;
+    rv = CheckTLSFeaturesAreSatisfied(peerCertInput, responseInputPtr);
+    if (rv != Success) {
+      return rv;
     }
   }
 
   Input hostnameInput;
-  result = hostnameInput.Init(BitwiseCast<const uint8_t*, const char*>(hostname),
-                              strlen(hostname));
-  if (result != Success) {
-    PR_SetError(SEC_ERROR_INVALID_ARGS, 0);
-    return SECFailure;
+  rv = hostnameInput.Init(BitwiseCast<const uint8_t*, const char*>(hostname),
+                          strlen(hostname));
+  if (rv != Success) {
+    return Result::FATAL_ERROR_INVALID_ARGS;
   }
   bool isBuiltInRoot;
-  result = IsCertChainRootBuiltInRoot(builtChain, isBuiltInRoot);
-  if (result != Success) {
-    PR_SetError(MapResultToPRErrorCode(result), 0);
-    return SECFailure;
+  rv = IsCertChainRootBuiltInRoot(builtChain, isBuiltInRoot);
+  if (rv != Success) {
+    return rv;
   }
   BRNameMatchingPolicy nameMatchingPolicy(
     isBuiltInRoot ? mNameMatchingMode
                   : BRNameMatchingPolicy::Mode::DoNotEnforce);
-  result = CheckCertHostname(peerCertInput, hostnameInput, nameMatchingPolicy);
-  if (result != Success) {
+  rv = CheckCertHostname(peerCertInput, hostnameInput, nameMatchingPolicy);
+  if (rv != Success) {
     // Treat malformed name information as a domain mismatch.
-    if (result == Result::ERROR_BAD_DER) {
-      PR_SetError(SSL_ERROR_BAD_CERT_DOMAIN, 0);
-    } else {
-      PR_SetError(MapResultToPRErrorCode(result), 0);
+    if (rv == Result::ERROR_BAD_DER) {
+      return Result::ERROR_BAD_CERT_DOMAIN;
     }
-    return SECFailure;
+
+    return rv;
   }
 
   if (saveIntermediatesInPermanentDatabase) {
     SaveIntermediateCerts(builtChain);
   }
 
-  return SECSuccess;
+  return Success;
 }
 
 } } // namespace mozilla::psm
--- a/security/certverifier/CertVerifier.h
+++ b/security/certverifier/CertVerifier.h
@@ -92,33 +92,34 @@ public:
     OCSP_STAPLING_GOOD = 1,
     OCSP_STAPLING_NONE = 2,
     OCSP_STAPLING_EXPIRED = 3,
     OCSP_STAPLING_INVALID = 4,
   };
 
   // *evOidPolicy == SEC_OID_UNKNOWN means the cert is NOT EV
   // Only one usage per verification is supported.
-  SECStatus VerifyCert(CERTCertificate* cert,
-                       SECCertificateUsage usage,
-                       mozilla::pkix::Time time,
-                       void* pinArg,
-                       const char* hostname,
-               /*out*/ UniqueCERTCertList& builtChain,
-                       Flags flags = 0,
-       /*optional in*/ const SECItem* stapledOCSPResponse = nullptr,
-       /*optional in*/ const SECItem* sctsFromTLS = nullptr,
-      /*optional out*/ SECOidTag* evOidPolicy = nullptr,
-      /*optional out*/ OCSPStaplingStatus* ocspStaplingStatus = nullptr,
-      /*optional out*/ KeySizeStatus* keySizeStatus = nullptr,
-      /*optional out*/ SHA1ModeResult* sha1ModeResult = nullptr,
-      /*optional out*/ PinningTelemetryInfo* pinningTelemetryInfo = nullptr,
-      /*optional out*/ CertificateTransparencyInfo* ctInfo = nullptr);
+  mozilla::pkix::Result VerifyCert(
+                    CERTCertificate* cert,
+                    SECCertificateUsage usage,
+                    mozilla::pkix::Time time,
+                    void* pinArg,
+                    const char* hostname,
+            /*out*/ UniqueCERTCertList& builtChain,
+                    Flags flags = 0,
+    /*optional in*/ const SECItem* stapledOCSPResponse = nullptr,
+    /*optional in*/ const SECItem* sctsFromTLS = nullptr,
+   /*optional out*/ SECOidTag* evOidPolicy = nullptr,
+   /*optional out*/ OCSPStaplingStatus* ocspStaplingStatus = nullptr,
+   /*optional out*/ KeySizeStatus* keySizeStatus = nullptr,
+   /*optional out*/ SHA1ModeResult* sha1ModeResult = nullptr,
+   /*optional out*/ PinningTelemetryInfo* pinningTelemetryInfo = nullptr,
+   /*optional out*/ CertificateTransparencyInfo* ctInfo = nullptr);
 
-  SECStatus VerifySSLServerCert(
+  mozilla::pkix::Result VerifySSLServerCert(
                     const UniqueCERTCertificate& peerCert,
        /*optional*/ const SECItem* stapledOCSPResponse,
        /*optional*/ const SECItem* sctsFromTLS,
                     mozilla::pkix::Time time,
        /*optional*/ void* pinarg,
                     const char* hostname,
             /*out*/ UniqueCERTCertList& builtChain,
        /*optional*/ bool saveIntermediatesInPermanentDatabase = false,
--- a/security/manager/ssl/SSLServerCertVerification.cpp
+++ b/security/manager/ssl/SSLServerCertVerification.cpp
@@ -1304,18 +1304,16 @@ AuthCertificate(CertVerifier& certVerifi
                 const SECItem* stapledOCSPResponse,
                 const SECItem* sctsFromTLSExtension,
                 uint32_t providerFlags,
                 Time time)
 {
   MOZ_ASSERT(infoObject);
   MOZ_ASSERT(cert);
 
-  SECStatus rv;
-
   // We want to avoid storing any intermediate cert information when browsing
   // in private, transient contexts.
   bool saveIntermediates =
     !(providerFlags & nsISocketProvider::NO_PERMANENT_STORAGE);
 
   SECOidTag evOidPolicy;
   UniqueCERTCertList certList;
   CertVerifier::OCSPStaplingStatus ocspStaplingStatus =
@@ -1326,30 +1324,28 @@ AuthCertificate(CertVerifier& certVerifi
   CertificateTransparencyInfo certificateTransparencyInfo;
 
   int flags = 0;
   if (!infoObject->SharedState().IsOCSPStaplingEnabled() ||
       !infoObject->SharedState().IsOCSPMustStapleEnabled()) {
     flags |= CertVerifier::FLAG_TLS_IGNORE_STATUS_REQUEST;
   }
 
-  rv = certVerifier.VerifySSLServerCert(cert, stapledOCSPResponse,
-                                        sctsFromTLSExtension, time, infoObject,
-                                        infoObject->GetHostNameRaw(),
-                                        certList, saveIntermediates, flags,
-                                        &evOidPolicy, &ocspStaplingStatus,
-                                        &keySizeStatus, &sha1ModeResult,
-                                        &pinningTelemetryInfo,
-                                        &certificateTransparencyInfo);
-  PRErrorCode savedErrorCode;
-  if (rv != SECSuccess) {
-    savedErrorCode = PR_GetError();
-  }
+  Result rv = certVerifier.VerifySSLServerCert(cert, stapledOCSPResponse,
+                                               sctsFromTLSExtension, time,
+                                               infoObject,
+                                               infoObject->GetHostNameRaw(),
+                                               certList, saveIntermediates,
+                                               flags, &evOidPolicy,
+                                               &ocspStaplingStatus,
+                                               &keySizeStatus, &sha1ModeResult,
+                                               &pinningTelemetryInfo,
+                                               &certificateTransparencyInfo);
 
-  uint32_t evStatus = (rv != SECSuccess) ? 0                // 0 = Failure
+  uint32_t evStatus = (rv != Success) ? 0                   // 0 = Failure
                     : (evOidPolicy == SEC_OID_UNKNOWN) ? 1  // 1 = DV
                     : 2;                                    // 2 = EV
   Telemetry::Accumulate(Telemetry::CERT_EV_STATUS, evStatus);
 
   if (ocspStaplingStatus != CertVerifier::OCSP_STAPLING_NEVER_CHECKED) {
     Telemetry::Accumulate(Telemetry::SSL_OCSP_STAPLING, ocspStaplingStatus);
   }
   if (keySizeStatus != KeySizeStatus::NeverChecked) {
@@ -1374,71 +1370,70 @@ AuthCertificate(CertVerifier& certVerifi
   // We want to remember the CA certs in the temp db, so that the application can find the
   // complete chain at any time it might need it.
   // But we keep only those CA certs in the temp db, that we didn't already know.
 
   RefPtr<nsSSLStatus> status(infoObject->SSLStatus());
   RefPtr<nsNSSCertificate> nsc;
 
   if (!status || !status->HasServerCert()) {
-    if( rv == SECSuccess ){
+    if (rv == Success) {
       nsc = nsNSSCertificate::Create(cert.get(), &evOidPolicy);
-    }
-    else {
+    } else {
       nsc = nsNSSCertificate::Create(cert.get());
     }
   }
 
-  if (rv == SECSuccess) {
+  if (rv == Success) {
     GatherSuccessfulValidationTelemetry(certList);
     GatherCertificateTransparencyTelemetry(certList,
                                            certificateTransparencyInfo);
 
     // The connection may get terminated, for example, if the server requires
     // a client cert. Let's provide a minimal SSLStatus
     // to the caller that contains at least the cert and its status.
     if (!status) {
       status = new nsSSLStatus();
       infoObject->SetSSLStatus(status);
     }
 
-    if (rv == SECSuccess) {
+    if (rv == Success) {
       // Certificate verification succeeded delete any potential record
       // of certificate error bits.
       RememberCertErrorsTable::GetInstance().RememberCertHasError(infoObject,
-                                                                  nullptr, rv);
-    }
-    else {
+                                                                  nullptr,
+                                                                  SECSuccess);
+    } else {
       // Certificate verification failed, update the status' bits.
       RememberCertErrorsTable::GetInstance().LookupCertErrorBits(
         infoObject, status);
     }
 
     if (status && !status->HasServerCert()) {
       nsNSSCertificate::EVStatus evStatus;
-      if (evOidPolicy == SEC_OID_UNKNOWN || rv != SECSuccess) {
+      if (evOidPolicy == SEC_OID_UNKNOWN || rv != Success) {
         evStatus = nsNSSCertificate::ev_status_invalid;
       } else {
         evStatus = nsNSSCertificate::ev_status_valid;
       }
 
       status->SetServerCert(nsc, evStatus);
       MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
              ("AuthCertificate setting NEW cert %p\n", nsc.get()));
     }
   }
 
-  if (rv != SECSuccess) {
+  if (rv != Success) {
     // Certificate validation failed; store the peer certificate chain on
     // infoObject so it can be used for error reporting.
     infoObject->SetFailedCertChain(Move(peerCertChain));
-    PR_SetError(savedErrorCode, 0);
+    PR_SetError(MapResultToPRErrorCode(rv), 0);
   }
 
-  return rv;
+  return rv == Success ? SECSuccess : SECFailure;
 }
 
 /*static*/ SECStatus
 SSLServerCertVerificationJob::Dispatch(
   const RefPtr<SharedCertVerifier>& certVerifier,
   const void* fdForLogging,
   nsNSSSocketInfo* infoObject,
   const UniqueCERTCertificate& serverCert,
--- a/security/manager/ssl/nsDataSignatureVerifier.cpp
+++ b/security/manager/ssl/nsDataSignatureVerifier.cpp
@@ -7,16 +7,17 @@
 #include "cms.h"
 #include "cryptohi.h"
 #include "keyhi.h"
 #include "mozilla/Casting.h"
 #include "mozilla/Unused.h"
 #include "nsCOMPtr.h"
 #include "nsNSSComponent.h"
 #include "nssb64.h"
+#include "pkix/pkixnss.h"
 #include "pkix/pkixtypes.h"
 #include "ScopedNSSTypes.h"
 #include "secerr.h"
 #include "SharedCertVerifier.h"
 
 using namespace mozilla;
 using namespace mozilla::pkix;
 using namespace mozilla::psm;
@@ -251,21 +252,26 @@ VerifyCertificate(CERTCertificate* cert,
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   context->signingCert = xpcomCert;
 
   RefPtr<SharedCertVerifier> certVerifier(GetDefaultCertVerifier());
   NS_ENSURE_TRUE(certVerifier, NS_ERROR_UNEXPECTED);
 
-  return MapSECStatus(certVerifier->VerifyCert(cert,
-                                               certificateUsageObjectSigner,
-                                               Now(), pinArg,
-                                               nullptr, // hostname
-                                               context->builtChain));
+  Result result = certVerifier->VerifyCert(cert,
+                                           certificateUsageObjectSigner,
+                                           Now(), pinArg,
+                                           nullptr, // hostname
+                                           context->builtChain);
+  if (result != Success) {
+    return GetXPCOMFromNSSError(MapResultToPRErrorCode(result));
+  }
+
+  return NS_OK;
 }
 
 } // namespace
 
 NS_IMETHODIMP
 nsDataSignatureVerifier::VerifySignature(const char* aRSABuf,
                                          uint32_t aRSABufLen,
                                          const char* aPlaintext,
--- a/security/manager/ssl/nsNSSCertificate.cpp
+++ b/security/manager/ssl/nsNSSCertificate.cpp
@@ -676,17 +676,18 @@ nsNSSCertificate::GetChain(nsIArray** _r
 
   UniqueCERTCertList nssChain;
   // We want to test all usages, but we start with server because most of the
   // time Firefox users care about server certs.
   if (certVerifier->VerifyCert(mCert.get(), certificateUsageSSLServer, now,
                                nullptr, /*XXX fixme*/
                                nullptr, /* hostname */
                                nssChain,
-                               CertVerifier::FLAG_LOCAL_ONLY) != SECSuccess) {
+                               CertVerifier::FLAG_LOCAL_ONLY)
+        != mozilla::pkix::Success) {
     nssChain = nullptr;
     // keep going
   }
 
   // This is the whitelist of all non-SSLServer usages that are supported by
   // verifycert.
   const int otherUsagesToTest = certificateUsageSSLClient |
                                 certificateUsageSSLCA |
@@ -702,17 +703,18 @@ nsNSSCertificate::GetChain(nsIArray** _r
     }
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
            ("pipnss: PKIX attempting chain(%d) for '%s'\n",
             usage, mCert->nickname));
     if (certVerifier->VerifyCert(mCert.get(), usage, now,
                                  nullptr, /*XXX fixme*/
                                  nullptr, /*hostname*/
                                  nssChain,
-                                 CertVerifier::FLAG_LOCAL_ONLY) != SECSuccess) {
+                                 CertVerifier::FLAG_LOCAL_ONLY)
+          != mozilla::pkix::Success) {
       nssChain = nullptr;
       // keep going
     }
   }
 
   if (!nssChain) {
     // There is not verified path for the chain, however we still want to
     // present to the user as much of a possible chain as possible, in the case
@@ -1148,27 +1150,27 @@ nsNSSCertificate::hasValidEVOidTag(SECOi
   NS_ENSURE_TRUE(certVerifier, NS_ERROR_UNEXPECTED);
 
   validEV = false;
   resultOidTag = SEC_OID_UNKNOWN;
 
   uint32_t flags = mozilla::psm::CertVerifier::FLAG_LOCAL_ONLY |
     mozilla::psm::CertVerifier::FLAG_MUST_BE_EV;
   UniqueCERTCertList unusedBuiltChain;
-  SECStatus rv = certVerifier->VerifyCert(mCert.get(),
+  mozilla::pkix::Result result = certVerifier->VerifyCert(mCert.get(),
     certificateUsageSSLServer, mozilla::pkix::Now(),
     nullptr /* XXX pinarg */,
     nullptr /* hostname */,
     unusedBuiltChain,
     flags,
     nullptr /* stapledOCSPResponse */,
     nullptr /* sctsFromTLSExtension */,
     &resultOidTag);
 
-  if (rv != SECSuccess) {
+  if (result != mozilla::pkix::Success) {
     resultOidTag = SEC_OID_UNKNOWN;
   }
   if (resultOidTag != SEC_OID_UNKNOWN) {
     validEV = true;
   }
   return NS_OK;
 }
 
--- a/security/manager/ssl/nsNSSCertificateDB.cpp
+++ b/security/manager/ssl/nsNSSCertificateDB.cpp
@@ -34,16 +34,17 @@
 #include "nsPK11TokenDB.h"
 #include "nsPKCS12Blob.h"
 #include "nsPromiseFlatString.h"
 #include "nsProxyRelease.h"
 #include "nsReadableUtils.h"
 #include "nsThreadUtils.h"
 #include "nspr.h"
 #include "pkix/Time.h"
+#include "pkix/pkixnss.h"
 #include "pkix/pkixtypes.h"
 #include "secasn1.h"
 #include "secder.h"
 #include "secerr.h"
 #include "ssl.h"
 
 #ifdef XP_WIN
 #include <winsock.h> // for ntohl
@@ -608,27 +609,27 @@ nsNSSCertificateDB::ImportEmailCertifica
   for (CERTCertListNode* node = CERT_LIST_HEAD(filteredCerts.get());
        !CERT_LIST_END(node, filteredCerts.get());
        node = CERT_LIST_NEXT(node)) {
     if (!node->cert) {
       continue;
     }
 
     UniqueCERTCertList certChain;
-    SECStatus srv = certVerifier->VerifyCert(node->cert,
-                                             certificateUsageEmailRecipient,
-                                             mozilla::pkix::Now(), ctx,
-                                             nullptr, certChain);
-    if (srv != SECSuccess) {
+    mozilla::pkix::Result result =
+      certVerifier->VerifyCert(node->cert, certificateUsageEmailRecipient,
+                               mozilla::pkix::Now(), ctx, nullptr, certChain);
+    if (result != mozilla::pkix::Success) {
       nsCOMPtr<nsIX509Cert> certToShow = nsNSSCertificate::Create(node->cert);
       DisplayCertificateAlert(ctx, "NotImportingUnverifiedCert", certToShow, locker);
       continue;
     }
-    srv = ImportCertsIntoPermanentStorage(certChain, certUsageEmailRecipient,
-                                          false);
+    SECStatus srv = ImportCertsIntoPermanentStorage(certChain,
+                                                    certUsageEmailRecipient,
+                                                    false);
     if (srv != SECSuccess) {
       return NS_ERROR_FAILURE;
     }
     CERT_SaveSMimeProfile(node->cert, nullptr, nullptr);
   }
 
   return NS_OK;
 }
@@ -664,28 +665,28 @@ nsNSSCertificateDB::ImportValidCACertsIn
 
   // Iterate through the filtered cert list and import verified certs into
   // permanent storage.
   // Note: We verify the certs in order to prevent DoS attacks. See Bug 249004.
   for (CERTCertListNode* node = CERT_LIST_HEAD(filteredCerts.get());
        !CERT_LIST_END(node, filteredCerts.get());
        node = CERT_LIST_NEXT(node)) {
     UniqueCERTCertList certChain;
-    SECStatus rv = certVerifier->VerifyCert(node->cert,
-                                            certificateUsageVerifyCA,
-                                            mozilla::pkix::Now(), ctx,
-                                            nullptr, certChain);
-    if (rv != SECSuccess) {
+    mozilla::pkix::Result result =
+      certVerifier->VerifyCert(node->cert, certificateUsageVerifyCA,
+                               mozilla::pkix::Now(), ctx, nullptr, certChain);
+    if (result != mozilla::pkix::Success) {
       nsCOMPtr<nsIX509Cert> certToShow = nsNSSCertificate::Create(node->cert);
       DisplayCertificateAlert(ctx, "NotImportingUnverifiedCert", certToShow, proofOfLock);
       continue;
     }
 
-    rv = ImportCertsIntoPermanentStorage(certChain, certUsageAnyCA, true);
-    if (rv != SECSuccess) {
+    SECStatus srv = ImportCertsIntoPermanentStorage(certChain, certUsageAnyCA,
+                                                    true);
+    if (srv != SECSuccess) {
       return NS_ERROR_FAILURE;
     }
   }
 
   return NS_OK;
 }
 
 void nsNSSCertificateDB::DisplayCertificateAlert(nsIInterfaceRequestor *ctx, 
@@ -1135,23 +1136,23 @@ nsNSSCertificateDB::FindCertByEmailAddre
 
   CERTCertListNode *node;
   // search for a valid certificate
   for (node = CERT_LIST_HEAD(certlist);
        !CERT_LIST_END(node, certlist);
        node = CERT_LIST_NEXT(node)) {
 
     UniqueCERTCertList unusedCertChain;
-    SECStatus srv = certVerifier->VerifyCert(node->cert,
-                                             certificateUsageEmailRecipient,
-                                             mozilla::pkix::Now(),
-                                             nullptr /*XXX pinarg*/,
-                                             nullptr /*hostname*/,
-                                             unusedCertChain);
-    if (srv == SECSuccess) {
+    mozilla::pkix::Result result =
+      certVerifier->VerifyCert(node->cert, certificateUsageEmailRecipient,
+                               mozilla::pkix::Now(),
+                               nullptr /*XXX pinarg*/,
+                               nullptr /*hostname*/,
+                               unusedCertChain);
+    if (result == mozilla::pkix::Success) {
       break;
     }
   }
 
   if (CERT_LIST_END(node, certlist)) {
     // no valid cert found
     return NS_ERROR_FAILURE;
   }
@@ -1483,55 +1484,48 @@ VerifyCertAtTime(nsIX509Cert* aCert,
     return NS_ERROR_INVALID_ARG;
   }
 
   RefPtr<SharedCertVerifier> certVerifier(GetDefaultCertVerifier());
   NS_ENSURE_TRUE(certVerifier, NS_ERROR_FAILURE);
 
   UniqueCERTCertList resultChain;
   SECOidTag evOidPolicy;
-  SECStatus srv;
+  mozilla::pkix::Result result;
 
   if (aHostname && aUsage == certificateUsageSSLServer) {
-    srv = certVerifier->VerifySSLServerCert(nssCert,
-                                            nullptr, // stapledOCSPResponse
-                                            nullptr, // sctsFromTLSExtension
-                                            aTime,
-                                            nullptr, // Assume no context
-                                            aHostname,
-                                            resultChain,
-                                            false, // don't save intermediates
-                                            aFlags,
-                                            &evOidPolicy);
+    result = certVerifier->VerifySSLServerCert(nssCert,
+                                               nullptr, // stapledOCSPResponse
+                                               nullptr, // sctsFromTLSExtension
+                                               aTime,
+                                               nullptr, // Assume no context
+                                               aHostname,
+                                               resultChain,
+                                               false, // don't save intermediates
+                                               aFlags,
+                                               &evOidPolicy);
   } else {
-    srv = certVerifier->VerifyCert(nssCert.get(), aUsage, aTime,
-                                   nullptr, // Assume no context
-                                   aHostname,
-                                   resultChain,
-                                   aFlags,
-                                   nullptr, // stapledOCSPResponse
-                                   nullptr, // sctsFromTLSExtension
-                                   &evOidPolicy);
+    result = certVerifier->VerifyCert(nssCert.get(), aUsage, aTime,
+                                      nullptr, // Assume no context
+                                      aHostname,
+                                      resultChain,
+                                      aFlags,
+                                      nullptr, // stapledOCSPResponse
+                                      nullptr, // sctsFromTLSExtension
+                                      &evOidPolicy);
   }
 
-  PRErrorCode error = PR_GetError();
-
   nsCOMPtr<nsIX509CertList> nssCertList;
   // This adopts the list
   nssCertList = new nsNSSCertList(Move(resultChain), locker);
   NS_ENSURE_TRUE(nssCertList, NS_ERROR_FAILURE);
 
-  if (srv == SECSuccess) {
-    if (evOidPolicy != SEC_OID_UNKNOWN) {
-      *aHasEVPolicy = true;
-    }
-    *_retval = 0;
-  } else {
-    NS_ENSURE_TRUE(error != 0, NS_ERROR_FAILURE);
-    *_retval = error;
+  *_retval = mozilla::pkix::MapResultToPRErrorCode(result);
+  if (result == mozilla::pkix::Success && evOidPolicy != SEC_OID_UNKNOWN) {
+    *aHasEVPolicy = true;
   }
   nssCertList.forget(aVerifiedChain);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsNSSCertificateDB::VerifyCertNow(nsIX509Cert* aCert,
--- a/security/manager/ssl/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/nsNSSIOLayer.cpp
@@ -374,16 +374,20 @@ nsNSSSocketInfo::DriveHandshake()
     return GetXPCOMFromNSSError(errorCode);
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsNSSSocketInfo::IsAcceptableForHost(const nsACString& hostname, bool* _retval)
 {
+  NS_ENSURE_ARG(_retval);
+
+  *_retval = false;
+
   // If this is the same hostname then the certicate status does not
   // need to be considered. They are joinable.
   if (hostname.Equals(GetHostName())) {
     *_retval = true;
     return NS_OK;
   }
 
   // Before checking the server certificate we need to make sure the
@@ -439,26 +443,27 @@ nsNSSSocketInfo::IsAcceptableForHost(con
   // is trying to join onto this connection.
   RefPtr<SharedCertVerifier> certVerifier(GetDefaultCertVerifier());
   if (!certVerifier) {
     return NS_OK;
   }
   nsAutoCString hostnameFlat(PromiseFlatCString(hostname));
   CertVerifier::Flags flags = CertVerifier::FLAG_LOCAL_ONLY;
   UniqueCERTCertList unusedBuiltChain;
-  SECStatus rv = certVerifier->VerifySSLServerCert(nssCert,
-                                                   nullptr, // stapledOCSPResponse
-                                                   nullptr, // sctsFromTLSExtension
-                                                   mozilla::pkix::Now(),
-                                                   nullptr, // pinarg
-                                                   hostnameFlat.get(),
-                                                   unusedBuiltChain,
-                                                   false, // save intermediates
-                                                   flags);
-  if (rv != SECSuccess) {
+  mozilla::pkix::Result result =
+    certVerifier->VerifySSLServerCert(nssCert,
+                                      nullptr, // stapledOCSPResponse
+                                      nullptr, // sctsFromTLSExtension
+                                      mozilla::pkix::Now(),
+                                      nullptr, // pinarg
+                                      hostnameFlat.get(),
+                                      unusedBuiltChain,
+                                      false, // save intermediates
+                                      flags);
+  if (result != mozilla::pkix::Success) {
     return NS_OK;
   }
 
   // All tests pass
   *_retval = true;
   return NS_OK;
 }
 
--- a/security/manager/ssl/nsSiteSecurityService.cpp
+++ b/security/manager/ssl/nsSiteSecurityService.cpp
@@ -740,17 +740,17 @@ nsSiteSecurityService::ProcessPKPHeader(
   if (certVerifier->VerifySSLServerCert(nssCert,
                                         nullptr, // stapledOCSPResponse
                                         nullptr, // sctsFromTLSExtension
                                         now, nullptr, // pinarg
                                         host.get(), // hostname
                                         certList,
                                         false, // don't store intermediates
                                         flags)
-      != SECSuccess) {
+        != mozilla::pkix::Success) {
     return NS_ERROR_FAILURE;
   }
 
   CERTCertListNode* rootNode = CERT_LIST_TAIL(certList);
   if (CERT_LIST_END(rootNode, certList)) {
     return NS_ERROR_FAILURE;
   }
   bool isBuiltIn = false;