Commit fdb64d11 authored by Dana Keeler's avatar Dana Keeler
Browse files

Bug 1645492 - only show certificates with corresponding error overrides in the... 

Bug 1645492 - only show certificates with corresponding error overrides in the "Servers" tab of the certificate manager. r=kjacobs, a=jcristau

Before this patch, the "Servers" tab of the certificate manager would show
built-in distrust records that had corresponding certificates (lately, this has
only consisted of two DigiNotar look-alike roots that were added many years ago
to block the real DigiNotar roots and potential cross-signs).
This patch changes the implementation to only show certificates that actually
have a corresponding error override in the "Servers" tab.

Differential Revision: https://phabricator.services.mozilla.com/D83190
parent 4c1c8a4a

security/manager/ssl/nsCertTree.cpp

+4 −29
Original line number Diff line number Diff line
@@ -403,34 +403,9 @@ nsresult nsCertTree::GetCertsByTypeFromCertList( 
      // overrides, we are storing certs without any trust flags associated.

      // So we must check whether the cert really belongs to the

      // server, email or unknown tab. We will lookup the cert in the override

      // list to come to the decision. Unfortunately, the lookup in the

      // override list is quite expensive. Therefore we are using this

      // lengthy if/else statement to minimize

      // the number of override-list-lookups.



      if (aWantedType == nsIX509Cert::SERVER_CERT &&

          thisCertType == nsIX509Cert::UNKNOWN_CERT) {

        // This unknown cert was stored without trust

        // Are there host:port based overrides stored?

        // If yes, display them.

        addOverrides = true;

      } else if (aWantedType == nsIX509Cert::SERVER_CERT &&

                 thisCertType == nsIX509Cert::SERVER_CERT) {

        // This server cert is explicitly marked as a web site peer,

        // with or without trust, but editable, so show it

        wantThisCert = true;

        // Are there host:port based overrides stored?

        // If yes, display them.

        addOverrides = true;

      } else if (aWantedType == nsIX509Cert::SERVER_CERT &&

                 thisCertType == nsIX509Cert::EMAIL_CERT) {

        // This cert might have been categorized as an email cert

        // because it carries an email address. But is it really one?

        // Our cert categorization is uncertain when it comes to

        // distinguish between email certs and web site certs.

        // So, let's see if we have an override for that cert

        // and if there is, conclude it's really a web site cert.

        addOverrides = true;

      // list to come to the decision.

      if (aWantedType == nsIX509Cert::SERVER_CERT) {

        wantThisCertIfHaveOverrides = true;

      } else if (aWantedType == nsIX509Cert::EMAIL_CERT &&

                 thisCertType == nsIX509Cert::EMAIL_CERT) {

        // This cert might have been categorized as an email cert
@@ -462,7 +437,7 @@ nsresult nsCertTree::GetCertsByTypeFromCertList( 
      if (wantThisCertIfHaveOverrides) {

        if (NS_SUCCEEDED(rv) && ocount > 0) {

          // there are overrides for this cert

          wantThisCert = true;

          addOverrides = true;

        }

      }

    }

security/manager/ssl/tests/mochitest/browser/browser.ini

+1 −1
Original line number Diff line number Diff line
@@ -5,7 +5,7 @@ support-files = 
  *.pem



[browser_bug627234_perwindowpb.js]

[browser_certificateManagerLeak.js]

[browser_certificateManager.js]

[browser_certViewer.js]

skip-if = verify

[browser_clientAuth_connection.js]

security/manager/ssl/tests/mochitest/browser/browser_certificateManager.js

0 → 100644
+47 −0
Original line number Diff line number Diff line 
/* 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/. */

"use strict";



async function checkServerCertificates(expectedNumberOfServerCertificates) {

  let win = await openCertManager();



  let serverCertsTreeView = win.document.getElementById("server-tree").view;

  let rowCount = serverCertsTreeView.rowCount;

  let nonContainerCount = 0;

  for (let i = 0; i < rowCount; i++) {

    if (!serverCertsTreeView.isContainer(i)) {

      nonContainerCount++;

    }

  }

  Assert.equal(

    nonContainerCount,

    expectedNumberOfServerCertificates,

    "should have the expected number of server certificates"

  );



  win.document.getElementById("certmanager").acceptDialog();

  await BrowserTestUtils.windowClosed(win);

}



add_task(async function test_no_spurrious_server_certificates() {

  await checkServerCertificates(0);



  let cert = await readCertificate("md5-ee.pem", ",,");

  let certOverrideService = Cc[

    "@mozilla.org/security/certoverride;1"

  ].getService(Ci.nsICertOverrideService);

  certOverrideService.rememberValidityOverride(

    "example.com",

    443,

    cert,

    Ci.nsICertOverrideService.ERROR_UNTRUSTED,

    false

  );



  await checkServerCertificates(1);



  certOverrideService.clearValidityOverride("example.com", 443);



  await checkServerCertificates(0);

});

security/manager/ssl/tests/mochitest/browser/browser_certificateManagerLeak.js

deleted100644 → 0
+0 −35
Original line number Diff line number Diff line 
/* 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/. */

"use strict";



var gBugWindow;



function onLoad() {

  gBugWindow.removeEventListener("load", onLoad);

  gBugWindow.addEventListener("unload", onUnload);

  gBugWindow.close();

}



function onUnload() {

  gBugWindow.removeEventListener("unload", onUnload);

  window.focus();

  finish();

}



// This test opens and then closes the certificate manager to test that it

// does not leak. The test harness keeps track of and reports leaks, so

// there are no actual checks here.

function test() {

  waitForExplicitFinish();



  // This test relies on the test timing out in order to indicate failure so

  // let's add a dummy pass.

  ok(

    true,

    "Each test requires at least one pass, fail or todo so here is a pass."

  );



  gBugWindow = window.openDialog("chrome://pippki/content/certManager.xhtml");

  gBugWindow.addEventListener("load", onLoad);

}

security/manager/ssl/tests/mochitest/browser/browser_clientAuthRememberService.js

+1 −14
Original line number Diff line number Diff line
@@ -21,19 +21,6 @@ const { MockRegistrar } = ChromeUtils.import( 
  "resource://testing-common/MockRegistrar.jsm"

);



async function openCertmanager() {

  let win = window.openDialog("chrome://pippki/content/certManager.xhtml");

  return new Promise((resolve, reject) => {

    win.addEventListener(

      "load",

      function() {

        executeSoon(() => resolve(win));

      },

      { once: true }

    );

  });

}



function findCertByCommonName(commonName) {

  for (let cert of certDB.getCerts()) {

    if (cert.commonName == commonName) {
@@ -94,7 +81,7 @@ add_task(async function testRememberedDecisionsUI() { 
    MockRegistrar.unregister(clientAuthRememberServiceCID);

  });



  let win = await openCertmanager();

  let win = await openCertManager();



  let listItems = win.document

    .getElementById("rememberedList")