Bug 1546816 - Part 1-5: Add AuthCertificateSetResults helper function r=keeler

Differential Revision: https://phabricator.services.mozilla.com/D45023

--HG--
extra : moz-landing-system : lando

security/manager/ssl/SSLServerCertVerification.cpp

@@ -1312,14 +1312,48 @@ static void CollectCertTelemetry(
   }
 }
 
+static void AuthCertificateSetResults(
+    nsNSSSocketInfo* aInfoObject, const UniqueCERTCertificate& aCert,
+    UniqueCERTCertList& aBuiltCertChain, UniqueCERTCertList& aPeerCertChain,
+    const CertificateTransparencyInfo& aCertificateTransparencyInfo,
+    SECOidTag aEvOidPolicy, bool aSucceeded) {
+  MOZ_ASSERT(aInfoObject);
+
+  if (aSucceeded) {
+    // Certificate verification succeeded. Delete any potential record of
+    // certificate error bits.
+    RememberCertErrorsTable::GetInstance().RememberCertHasError(aInfoObject,
+                                                                SECSuccess);
+
+    EVStatus evStatus;
+    if (aEvOidPolicy == SEC_OID_UNKNOWN) {
+      evStatus = EVStatus::NotEV;
+    } else {
+      evStatus = EVStatus::EV;
+    }
+
+    RefPtr<nsNSSCertificate> nsc = nsNSSCertificate::Create(aCert.get());
+    aInfoObject->SetServerCert(nsc, evStatus);
+
+    aInfoObject->SetSucceededCertChain(std::move(aBuiltCertChain));
+    MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
+            ("AuthCertificate setting NEW cert %p", nsc.get()));
+
+    aInfoObject->SetCertificateTransparencyInfo(aCertificateTransparencyInfo);
+  } else {
+    // Certificate validation failed; store the peer certificate chain on
+    // infoObject so it can be used for error reporting.
+    aInfoObject->SetFailedCertChain(std::move(aPeerCertChain));
+  }
+}
+
 // Note: Takes ownership of |peerCertChain| if SECSuccess is not returned.
-SECStatus AuthCertificate(CertVerifier& certVerifier,
-                          nsNSSSocketInfo* infoObject,
-                          const UniqueCERTCertificate& cert,
-                          UniqueCERTCertList& peerCertChain,
-                          const Maybe<nsTArray<uint8_t>>& stapledOCSPResponse,
-                          const Maybe<nsTArray<uint8_t>>& sctsFromTLSExtension,
-                          uint32_t providerFlags, Time time) {
+Result AuthCertificate(CertVerifier& certVerifier, nsNSSSocketInfo* infoObject,
+                       const UniqueCERTCertificate& cert,
+                       UniqueCERTCertList& peerCertChain,
+                       const Maybe<nsTArray<uint8_t>>& stapledOCSPResponse,
+                       const Maybe<nsTArray<uint8_t>>& sctsFromTLSExtension,
+                       uint32_t providerFlags, Time time) {
   MOZ_ASSERT(infoObject);
   MOZ_ASSERT(cert);
 
@@ -1354,37 +1388,10 @@ SECStatus AuthCertificate(CertVerifier& certVerifier,
                        sha1ModeResult, pinningTelemetryInfo, builtCertChain,
                        certificateTransparencyInfo);
 
-  if (rv == Success) {
-    // Certificate verification succeeded. Delete any potential record of
-    // certificate error bits.
-    RememberCertErrorsTable::GetInstance().RememberCertHasError(infoObject,
-                                                                SECSuccess);
-
-    EVStatus evStatus;
-    if (evOidPolicy == SEC_OID_UNKNOWN) {
-      evStatus = EVStatus::NotEV;
-    } else {
-      evStatus = EVStatus::EV;
-    }
-
-    RefPtr<nsNSSCertificate> nsc = nsNSSCertificate::Create(cert.get());
-    infoObject->SetServerCert(nsc, evStatus);
-
-    infoObject->SetSucceededCertChain(std::move(builtCertChain));
-    MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
-            ("AuthCertificate setting NEW cert %p", nsc.get()));
-
-    infoObject->SetCertificateTransparencyInfo(certificateTransparencyInfo);
-  }
-
-  if (rv != Success) {
-    // Certificate validation failed; store the peer certificate chain on
-    // infoObject so it can be used for error reporting.
-    infoObject->SetFailedCertChain(std::move(peerCertChain));
-    PR_SetError(MapResultToPRErrorCode(rv), 0);
-  }
-
-  return rv == Success ? SECSuccess : SECFailure;
+  AuthCertificateSetResults(infoObject, cert, builtCertChain, peerCertChain,
+                            certificateTransparencyInfo, evOidPolicy,
+                            rv == Success);
+  return rv;
 }
 
 /*static*/
@@ -1447,18 +1454,15 @@ SSLServerCertVerificationJob::Run() {
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
           ("[%p] SSLServerCertVerificationJob::Run\n", mInfoObject.get()));
 
-  // Reset the error code here so we can detect if AuthCertificate fails to
-  // set the error code if/when it fails.
-  PR_SetError(0, 0);
   TimeStamp jobStartTime = TimeStamp::Now();
-  SECStatus rv = AuthCertificate(*mCertVerifier, mInfoObject, mCert,
-                                 mPeerCertChain, mStapledOCSPResponse,
-                                 mSCTsFromTLSExtension, mProviderFlags, mTime);
-  MOZ_ASSERT((mPeerCertChain && rv == SECSuccess) ||
-                 (!mPeerCertChain && rv != SECSuccess),
-             "AuthCertificate() should take ownership of chain on failure");
+  Result rv = AuthCertificate(*mCertVerifier, mInfoObject, mCert,
+                              mPeerCertChain, mStapledOCSPResponse,
+                              mSCTsFromTLSExtension, mProviderFlags, mTime);
+  MOZ_ASSERT(
+      (mPeerCertChain && rv == Success) || (!mPeerCertChain && rv != Success),
+      "AuthCertificate() should take ownership of chain on failure");
 
-  if (rv == SECSuccess) {
+  if (rv == Success) {
     Telemetry::AccumulateTimeDelta(
         Telemetry::SSL_SUCCESFUL_CERT_VALIDATION_TIME_MOZILLAPKIX,
         jobStartTime, TimeStamp::Now());
@@ -1469,14 +1473,11 @@ SSLServerCertVerificationJob::Run() {
     return NS_OK;
   }
 
-  // Note: the interval is not calculated once as PR_GetError MUST be called
-  // before any other  function call
-  PRErrorCode error = PR_GetError();
-
   Telemetry::AccumulateTimeDelta(
       Telemetry::SSL_INITIAL_FAILED_CERT_VALIDATION_TIME_MOZILLAPKIX,
       jobStartTime, TimeStamp::Now());
 
+  PRErrorCode error = MapResultToPRErrorCode(rv);
   if (error != 0) {
     RefPtr<CertErrorRunnable> runnable(
         CreateCertErrorRunnable(*mCertVerifier, error, mInfoObject, mCert,