Bug 1712983 - memoize decoded certificates in devtools to avoid unnecessary work r=ochameau

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

devtools/server/actors/network-monitor/network-observer.js

@@ -190,6 +190,9 @@ function NetworkObserver(filters, owner) {
 
   this._throttleData = null;
   this._throttler = null;
+  // This is ultimately used by NetworkHelper.parseSecurityInfo to avoid
+  // repeatedly decoding already-seen certificates.
+  this._decodedCertificateCache = new Map();
 }
 
 exports.NetworkObserver = NetworkObserver;
@@ -300,6 +303,8 @@ NetworkObserver.prototype = {
     return this._throttler;
   },
 
+  _decodedCertificateCache: null,
+
   _serviceWorkerRequest: function(subject, topic, data) {
     const channel = subject.QueryInterface(Ci.nsIHttpChannel);
 
@@ -953,7 +958,11 @@ NetworkObserver.prototype = {
     sink.init(false, false, this.responsePipeSegmentSize, PR_UINT32_MAX, null);
 
     // Add listener for the response body.
-    const newListener = new NetworkResponseListener(this, httpActivity);
+    const newListener = new NetworkResponseListener(
+      this,
+      httpActivity,
+      this._decodedCertificateCache
+    );
 
     // Remember the input stream, so it isn't released by GC.
     newListener.inputStream = sink.inputStream;
@@ -1522,6 +1531,7 @@ NetworkObserver.prototype = {
     this.owner = null;
     this.filters = null;
     this._throttler = null;
+    this._decodedCertificateCache.clear();
   },
 };
 

devtools/server/actors/network-monitor/network-response-listener.js

@@ -45,8 +45,11 @@ loader.lazyGetter(
  * @param object httpActivity
  *        HttpActivity object associated with this request. See NetworkObserver
  *        for more information.
+ * @param Map decodedCertificateCache
+ *        A Map of certificate fingerprints to decoded certificates, to avoid
+ *        repeatedly decoding previously-seen certificates.
  */
-function NetworkResponseListener(owner, httpActivity) {
+function NetworkResponseListener(owner, httpActivity, decodedCertificateCache) {
   this.owner = owner;
   this.receivedData = "";
   this.httpActivity = httpActivity;
@@ -58,6 +61,7 @@ function NetworkResponseListener(owner, httpActivity) {
   const channel = this.httpActivity.channel;
   this._wrappedNotificationCallbacks = channel.notificationCallbacks;
   channel.notificationCallbacks = this;
+  this._decodedCertificateCache = decodedCertificateCache;
 }
 
 exports.NetworkResponseListener = NetworkResponseListener;
@@ -117,6 +121,12 @@ NetworkResponseListener.prototype = {
    */
   _wrappedNotificationCallbacks: null,
 
+  /**
+   * A Map of certificate fingerprints to decoded certificates, to avoid repeatedly
+   * decoding previously-seen certificates.
+   */
+  _decodedCertificateCache: null,
+
   /**
    * The response listener owner.
    */
@@ -330,7 +340,8 @@ NetworkResponseListener.prototype = {
     }
     const info = await NetworkHelper.parseSecurityInfo(
       secinfo,
-      this.httpActivity
+      this.httpActivity,
+      this._decodedCertificateCache
     );
     let isRacing = false;
     try {

devtools/shared/webconsole/network-helper.js

@@ -555,6 +555,9 @@ var NetworkHelper = {
    * @param object httpActivity
    *        The httpActivity object for the request with at least members
    *        { private, hostname }.
+   * @param Map decodedCertificateCache
+   *        A Map of certificate fingerprints to decoded certificates, to avoid
+   *        repeatedly decoding previously-seen certificates.
    *
    * @return object
    *         Returns an object containing following members:
@@ -578,7 +581,11 @@ var NetworkHelper = {
    *            - weaknessReasons: list of reasons that cause the request to be
    *                               considered weak. See getReasonsForWeakness.
    */
-  parseSecurityInfo: async function(securityInfo, httpActivity) {
+  parseSecurityInfo: async function(
+    securityInfo,
+    httpActivity,
+    decodedCertificateCache
+  ) {
     const info = {
       state: "insecure",
     };
@@ -672,7 +679,10 @@ var NetworkHelper = {
       );
 
       // Certificate.
-      info.cert = await this.parseCertificateInfo(securityInfo.serverCert);
+      info.cert = await this.parseCertificateInfo(
+        securityInfo.serverCert,
+        decodedCertificateCache
+      );
 
       // Certificate transparency status.
       info.certificateTransparency = securityInfo.certificateTransparencyStatus;
@@ -723,6 +733,9 @@ var NetworkHelper = {
    *
    * @param nsIX509Cert cert
    *        The certificate to extract the information from.
+   * @param Map decodedCertificateCache
+   *        A Map of certificate fingerprints to decoded certificates, to avoid
+   *        repeatedly decoding previously-seen certificates.
    * @return object
    *         An object with following format:
    *           {
@@ -732,7 +745,7 @@ var NetworkHelper = {
    *             fingerprint: { sha1, sha256 }
    *           }
    */
-  parseCertificateInfo: async function(cert) {
+  parseCertificateInfo: async function(cert, decodedCertificateCache) {
     function getDNComponent(dn, componentType) {
       for (const [type, value] of dn.entries) {
         if (type == componentType) {
@@ -744,9 +757,14 @@ var NetworkHelper = {
 
     const info = {};
     if (cert) {
-      const parsedCert = await certDecoder.parse(
-        certDecoder.pemToDER(cert.getBase64DERString())
-      );
+      const certHash = cert.sha256Fingerprint;
+      let parsedCert = decodedCertificateCache.get(certHash);
+      if (!parsedCert) {
+        parsedCert = await certDecoder.parse(
+          certDecoder.pemToDER(cert.getBase64DERString())
+        );
+        decodedCertificateCache.set(certHash, parsedCert);
+      }
       info.subject = {
         commonName: getDNComponent(parsedCert.subject, "Common Name"),
         organization: getDNComponent(parsedCert.subject, "Organization"),

devtools/shared/webconsole/test/xpcshell/test_security-info-certificate.js

@@ -30,7 +30,10 @@ const DUMMY_CERT = {
 add_task(async function run_test() {
   info("Testing NetworkHelper.parseCertificateInfo.");
 
-  const result = await NetworkHelper.parseCertificateInfo(DUMMY_CERT);
+  const result = await NetworkHelper.parseCertificateInfo(
+    DUMMY_CERT,
+    new Map()
+  );
 
   // Subject
   equal(

devtools/shared/webconsole/test/xpcshell/test_security-info-parser.js

@@ -37,7 +37,11 @@ const MockSecurityInfo = {
 };
 
 add_task(async function run_test() {
-  const result = await NetworkHelper.parseSecurityInfo(MockSecurityInfo, {});
+  const result = await NetworkHelper.parseSecurityInfo(
+    MockSecurityInfo,
+    {},
+    new Map()
+  );
 
   equal(result.state, "secure", "State is correct.");
 
@@ -51,7 +55,7 @@ add_task(async function run_test() {
 
   deepEqual(
     result.cert,
-    await NetworkHelper.parseCertificateInfo(MockCertificate),
+    await NetworkHelper.parseCertificateInfo(MockCertificate, new Map()),
     "Certificate information is correct."
   );
 

devtools/shared/webconsole/test/xpcshell/test_security-info-state.js

@@ -41,7 +41,7 @@ add_task(async function run_test() {
  * Test that undefined security information is returns "insecure".
  */
 async function test_nullSecurityInfo() {
-  const result = await NetworkHelper.parseSecurityInfo(null, {});
+  const result = await NetworkHelper.parseSecurityInfo(null, {}, new Map());
   equal(
     result.state,
     "insecure",
@@ -58,7 +58,11 @@ async function test_insecureSecurityInfoWithNSSError() {
   // Taken from security/manager/ssl/tests/unit/head_psm.js.
   MockSecurityInfo.errorCode = -8180;
 
-  const result = await NetworkHelper.parseSecurityInfo(MockSecurityInfo, {});
+  const result = await NetworkHelper.parseSecurityInfo(
+    MockSecurityInfo,
+    {},
+    new Map()
+  );
   equal(
     result.state,
     "broken",
@@ -75,7 +79,11 @@ async function test_insecureSecurityInfoWithNSSError() {
 async function test_insecureSecurityInfoWithoutNSSError() {
   MockSecurityInfo.securityState = wpl.STATE_IS_INSECURE;
 
-  const result = await NetworkHelper.parseSecurityInfo(MockSecurityInfo, {});
+  const result = await NetworkHelper.parseSecurityInfo(
+    MockSecurityInfo,
+    {},
+    new Map()
+  );
   equal(
     result.state,
     "insecure",
@@ -90,7 +98,11 @@ async function test_insecureSecurityInfoWithoutNSSError() {
 async function test_secureSecurityInfo() {
   MockSecurityInfo.securityState = wpl.STATE_IS_SECURE;
 
-  const result = await NetworkHelper.parseSecurityInfo(MockSecurityInfo, {});
+  const result = await NetworkHelper.parseSecurityInfo(
+    MockSecurityInfo,
+    {},
+    new Map()
+  );
   equal(
     result.state,
     "secure",
@@ -104,7 +116,11 @@ async function test_secureSecurityInfo() {
 async function test_brokenSecurityInfo() {
   MockSecurityInfo.securityState = wpl.STATE_IS_BROKEN;
 
-  const result = await NetworkHelper.parseSecurityInfo(MockSecurityInfo, {});
+  const result = await NetworkHelper.parseSecurityInfo(
+    MockSecurityInfo,
+    {},
+    new Map()
+  );
   equal(
     result.state,
     "weak",

devtools/shared/webconsole/test/xpcshell/test_security-info-static-hpkp.js

@@ -45,7 +45,8 @@ add_task(async function run_test() {
   Services.prefs.setIntPref("security.cert_pinning.enforcement_level", 1);
   const result = await NetworkHelper.parseSecurityInfo(
     MockSecurityInfo,
-    MockHttpInfo
+    MockHttpInfo,
+    new Map()
   );
   equal(result.hpkp, true, "Static HPKP detected.");
 });