bug 1454504 - use a more performant API to find a root certificate in CertUtils.checkCert r?kmaglione,Mossop draft
authorDavid Keeler <dkeeler@mozilla.com>
Mon, 16 Apr 2018 15:19:51 -0700
changeset 783838 6699a3c8fe6b3380dc973fd1ebbe7c73e69ce12d
parent 783746 5ded36cb383d3ccafd9b6c231c5120dcdae196a2
push id106802
push userbmo:dkeeler@mozilla.com
push dateTue, 17 Apr 2018 20:33:37 +0000
reviewerskmaglione, Mossop
bugs1454504
milestone61.0a1
bug 1454504 - use a more performant API to find a root certificate in CertUtils.checkCert r?kmaglione,Mossop nsIX509Cert.issuer performs synchronous certificate verification and isn't even guaranteed to return a verified result. Luckily we can replace this with nsISSLStatus.succeededCertChain, which contains the already-verified certificate chain of the connection we're interested in. MozReview-Commit-ID: I8jPDVlUOvf
toolkit/modules/CertUtils.jsm
--- a/toolkit/modules/CertUtils.jsm
+++ b/toolkit/modules/CertUtils.jsm
@@ -2,16 +2,17 @@
 /* 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/. */
 var EXPORTED_SYMBOLS = ["CertUtils"];
 
 const Ce = Components.Exception;
 
 ChromeUtils.import("resource://gre/modules/Services.jsm");
+const { XPCOMUtils } = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm", {});
 
 /**
  * Reads a set of expected certificate attributes from preferences. The returned
  * array can be passed to validateCert or checkCert to validate that a
  * certificate matches the expected attributes. The preferences should look like
  * this:
  *   prefix.1.attribute1
  *   prefix.1.attribute2
@@ -137,35 +138,39 @@ function checkCert(aChannel, aAllowNonBu
     // Require https if there are certificate values to verify
     if (aCerts) {
       throw new Ce("SSL is required and URI scheme is not https.",
                    Cr.NS_ERROR_UNEXPECTED);
     }
     return;
   }
 
-  var cert =
-      aChannel.securityInfo.QueryInterface(Ci.nsISSLStatusProvider).
-      SSLStatus.QueryInterface(Ci.nsISSLStatus).serverCert;
+  let sslStatus = aChannel.securityInfo.QueryInterface(Ci.nsISSLStatusProvider)
+                          .SSLStatus;
+  let cert = sslStatus.serverCert;
 
   validateCert(cert, aCerts);
 
-  if (aAllowNonBuiltInCerts === true)
+  if (aAllowNonBuiltInCerts === true) {
     return;
+  }
 
-  var issuerCert = cert;
-  while (issuerCert.issuer && !issuerCert.issuer.equals(issuerCert))
-    issuerCert = issuerCert.issuer;
+  let certEnumerator = sslStatus.succeededCertChain.getEnumerator();
+  let issuerCert = null;
+  for (issuerCert of XPCOMUtils.IterSimpleEnumerator(certEnumerator,
+                                                     Ci.nsIX509Cert));
 
   const certNotBuiltInErr = "Certificate issuer is not built-in.";
-  if (!issuerCert)
+  if (!issuerCert) {
     throw new Ce(certNotBuiltInErr, Cr.NS_ERROR_ABORT);
+  }
 
-  if (!issuerCert.isBuiltInRoot)
+  if (!issuerCert.isBuiltInRoot) {
     throw new Ce(certNotBuiltInErr, Cr.NS_ERROR_ABORT);
+  }
 }
 
 /**
  * This class implements nsIBadCertListener.  Its job is to prevent "bad cert"
  * security dialogs from being shown to the user.  It is better to simply fail
  * if the certificate is bad. See bug 304286.
  *
  * @param  aAllowNonBuiltInCerts (optional)