bug 1418135 - asynchronously determine the chain to display in the details pane of the certificate viewer r?mgoodwin
The current certificate viewer uses "getChain" to determine what chain to show
in the details pane. This is problematic for a number of reasons including a)
it's synchronous (and potentially slow) and b) getChain may return something
almost entirely quite unlike any actual trusted path (see
bug 1004580 comment
0).
This won't fix the whole problem (whatever's opening the certificate viewer
should really be passing in the chain itself), but that's hard, so this would at
least change the determination to be asynchronous and at least won't result in
something completely bogus.
MozReview-Commit-ID: J9uqRgxL52j
--- a/security/manager/pki/resources/content/certViewer.js
+++ b/security/manager/pki/resources/content/certViewer.js
@@ -33,23 +33,31 @@ function doPrompt(msg) {
prompts.alert(window, null, msg);
}
/**
* Fills out the "Certificate Hierarchy" tree of the cert viewer "Details" tab.
*
* @param {tree} node
* Parent tree node to append to.
- * @param {nsIArray<nsIX509Cert>} chain
- * Chain where cert element n is issued by cert element n + 1.
+ * @param {Array} chain
+ * An array of nsIX509Cert where cert n is issued by cert n + 1.
*/
function AddCertChain(node, chain) {
+ if (!chain || chain.length < 1) {
+ return;
+ }
let child = document.getElementById(node);
+ // Clear any previous state.
+ let preexistingChildren = child.querySelectorAll("treechildren");
+ for (let preexistingChild of preexistingChildren) {
+ child.removeChild(preexistingChild);
+ }
for (let i = chain.length - 1; i >= 0; i--) {
- let currCert = chain.queryElementAt(i, nsIX509Cert);
+ let currCert = chain[i];
let displayValue = currCert.displayName;
let addTwistie = i != 0;
child = addChildrenToTree(child, displayValue, currCert.dbKey, addTwistie);
}
}
/**
* Adds a "verified usage" of a cert to the "General" tab of the cert viewer.
@@ -73,18 +81,19 @@ function setWindowName() {
let cert = window.arguments[0].QueryInterface(Ci.nsIX509Cert);
document.title = bundle.getFormattedString("certViewerTitle",
[cert.displayName]);
//
// Set the cert attributes for viewing
//
- // The chain of trust
- AddCertChain("treesetDump", cert.getChain());
+ // Set initial dummy chain of just the cert itself. A more complete chain (if
+ // one can be found), will be set when asyncDetermineUsages finishes.
+ AddCertChain("treesetDump", [cert]);
DisplayGeneralDataFromCert(cert);
BuildPrettyPrint(cert);
asyncDetermineUsages(cert);
}
// Certificate usages we care about in the certificate viewer.
const certificateUsageSSLClient = 0x0001;
@@ -137,31 +146,90 @@ function asyncDetermineUsages(cert) {
let now = Date.now() / 1000;
let certdb = Cc["@mozilla.org/security/x509certdb;1"]
.getService(Ci.nsIX509CertDB);
Object.keys(certificateUsages).forEach(usageString => {
promises.push(new Promise((resolve, reject) => {
let usage = certificateUsages[usageString];
certdb.asyncVerifyCertAtTime(cert, usage, 0, null, now,
(aPRErrorCode, aVerifiedChain, aHasEVPolicy) => {
- resolve({ usageString, errorCode: aPRErrorCode });
+ resolve({ usageString,
+ errorCode: aPRErrorCode,
+ chain: aVerifiedChain });
});
}));
});
Promise.all(promises).then(displayUsages);
}
/**
+ * Given a results array (see displayUsages), returns the chain corresponding to
+ * the desired usage, if verifying for that usage succeeded. Returns null
+ * otherwise.
+ *
+ * @param {Array} results
+ * An array of results from `asyncDetermineUsages`. See `displayUsages`.
+ * @param {Number} usage
+ * A numerical value corresponding to a usage. See `certificateUsages`.
+ * @returns {Array} An array of `nsIX509Cert` representing the verified
+ * certificate chain for the given usage, or null if there is none.
+ */
+function getChainForUsage(results, usage) {
+ for (let result of results) {
+ if (certificateUsages[result.usageString] == usage &&
+ result.errorCode == PRErrorCodeSuccess) {
+ let array = [];
+ let enumerator = result.chain.getEnumerator();
+ while (enumerator.hasMoreElements()) {
+ let cert = enumerator.getNext().QueryInterface(Ci.nsIX509Cert);
+ array.push(cert);
+ }
+ return array;
+ }
+ }
+ return null;
+}
+
+/**
+ * Given a results array (see displayUsages), returns the "best" verified
+ * certificate chain. Since the primary use case is for TLS server certificates
+ * in Firefox, such a verified chain will be returned if present. Otherwise, the
+ * priority is: TLS client certificate, email signer, email recipient, CA.
+ * Returns null if no usage verified successfully.
+ *
+ * @param {Array} results
+ * An array of results from `asyncDetermineUsages`. See `displayUsages`.
+ * @param {Number} usage
+ * A numerical value corresponding to a usage. See `certificateUsages`.
+ * @returns {Array} An array of `nsIX509Cert` representing the verified
+ * certificate chain for the given usage, or null if there is none.
+ */
+function getBestChain(results) {
+ let usages = [ certificateUsageSSLServer, certificateUsageSSLClient,
+ certificateUsageEmailSigner, certificateUsageEmailRecipient,
+ certificateUsageSSLCA ];
+ for (let usage of usages) {
+ let chain = getChainForUsage(results, usage);
+ if (chain) {
+ return chain;
+ }
+ }
+ return null;
+}
+
+/**
* Updates the usage display area given the results from asyncDetermineUsages.
*
* @param {Array} results
- * An array of objects with the properties "usageString" and "errorCode".
+ * An array of objects with the properties "usageString", "errorCode",
+ * and "chain".
* usageString is a string that is a key in the certificateUsages map.
* errorCode is either an NSPR error code or PRErrorCodeSuccess (which is
* a pseudo-NSPR error code with the value 0 that indicates success).
+ * chain is the built trust path, if one was found
*/
function displayUsages(results) {
document.getElementById("verify_pending").setAttribute("hidden", "true");
let verified = document.getElementById("verified");
let someSuccess = results.some(result =>
result.errorCode == PRErrorCodeSuccess
);
if (someSuccess) {
@@ -172,16 +240,17 @@ function displayUsages(results) {
results.forEach(result => {
if (result.errorCode != PRErrorCodeSuccess) {
return;
}
let bundleName = certificateUsageToStringBundleName[result.usageString];
let usage = pipnssBundle.GetStringFromName(bundleName);
AddUsage(usage);
});
+ AddCertChain("treesetDump", getBestChain(results));
} else {
const errorRankings = [
{ error: SEC_ERROR_REVOKED_CERTIFICATE,
bundleString: "certNotVerified_CertRevoked" },
{ error: SEC_ERROR_UNTRUSTED_CERT,
bundleString: "certNotVerified_CertNotTrusted" },
{ error: SEC_ERROR_UNTRUSTED_ISSUER,
bundleString: "certNotVerified_IssuerNotTrusted" },
--- a/security/manager/ssl/tests/mochitest/browser/browser_certViewer.js
+++ b/security/manager/ssl/tests/mochitest/browser/browser_certViewer.js
@@ -9,94 +9,104 @@
// being verified.
var { OS } = Cu.import("resource://gre/modules/osfile.jsm", {});
add_task(async function testCAandTitle() {
let cert = await readCertificate("ca.pem", "CTu,CTu,CTu");
let win = await displayCertificate(cert);
checkUsages(win, ["SSL Certificate Authority"]);
+ checkDetailsPane(win, ["ca"]);
// There's no real need to test the title for every cert, so we just test it
// once here.
Assert.equal(win.document.title, "Certificate Viewer: \u201Cca\u201D",
"Actual and expected title should match");
await BrowserTestUtils.closeWindow(win);
});
add_task(async function testSSLEndEntity() {
let cert = await readCertificate("ssl-ee.pem", ",,");
let win = await displayCertificate(cert);
checkUsages(win, ["SSL Server Certificate", "SSL Client Certificate"]);
+ checkDetailsPane(win, ["ca", "ssl-ee"]);
await BrowserTestUtils.closeWindow(win);
});
add_task(async function testEmailEndEntity() {
let cert = await readCertificate("email-ee.pem", ",,");
let win = await displayCertificate(cert);
checkUsages(win, ["Email Recipient Certificate", "Email Signer Certificate"]);
+ checkDetailsPane(win, ["ca", "email-ee"]);
await BrowserTestUtils.closeWindow(win);
});
add_task(async function testCodeSignEndEntity() {
let cert = await readCertificate("code-ee.pem", ",,");
let win = await displayCertificate(cert);
checkError(win, "Could not verify this certificate for unknown reasons.");
+ checkDetailsPane(win, ["code-ee"]);
await BrowserTestUtils.closeWindow(win);
});
add_task(async function testExpired() {
let cert = await readCertificate("expired-ca.pem", ",,");
let win = await displayCertificate(cert);
checkError(win, "Could not verify this certificate because it has expired.");
+ checkDetailsPane(win, ["expired-ca"]);
await BrowserTestUtils.closeWindow(win);
// These tasks may run in any order, so we run this additional testcase in the
// same task.
let eeCert = await readCertificate("ee-from-expired-ca.pem", ",,");
let eeWin = await displayCertificate(eeCert);
checkError(eeWin,
"Could not verify this certificate because the CA certificate " +
"is invalid.");
+ checkDetailsPane(eeWin, ["ee-from-expired-ca"]);
await BrowserTestUtils.closeWindow(eeWin);
});
add_task(async function testUnknownIssuer() {
let cert = await readCertificate("unknown-issuer.pem", ",,");
let win = await displayCertificate(cert);
checkError(win,
"Could not verify this certificate because the issuer is " +
"unknown.");
+ checkDetailsPane(win, ["unknown-issuer"]);
await BrowserTestUtils.closeWindow(win);
});
add_task(async function testInsecureAlgo() {
let cert = await readCertificate("md5-ee.pem", ",,");
let win = await displayCertificate(cert);
checkError(win,
"Could not verify this certificate because it was signed using " +
"a signature algorithm that was disabled because that algorithm " +
"is not secure.");
+ checkDetailsPane(win, ["md5-ee"]);
await BrowserTestUtils.closeWindow(win);
});
add_task(async function testUntrusted() {
let cert = await readCertificate("untrusted-ca.pem", "p,p,p");
let win = await displayCertificate(cert);
checkError(win,
"Could not verify this certificate because it is not trusted.");
+ checkDetailsPane(win, ["untrusted-ca"]);
await BrowserTestUtils.closeWindow(win);
// These tasks may run in any order, so we run this additional testcase in the
// same task.
let eeCert = await readCertificate("ee-from-untrusted-ca.pem", ",,");
let eeWin = await displayCertificate(eeCert);
checkError(eeWin,
"Could not verify this certificate because the issuer is not " +
"trusted.");
+ checkDetailsPane(eeWin, ["ee-from-untrusted-ca"]);
await BrowserTestUtils.closeWindow(eeWin);
});
add_task(async function testRevoked() {
// Note that there's currently no way to un-do this. This should only be a
// problem if another test re-uses a certificate with this same key (perhaps
// likely) and subject (less likely).
let certBlocklist = Cc["@mozilla.org/security/certblocklist;1"]
@@ -106,35 +116,38 @@ add_task(async function testRevoked() {
"VCIlmPM9NkgFQtrs4Oa5TeFcDu6MWRTKSNdePEhOgD8="); // hash of the shared key
let cert = await readCertificate("revoked.pem", ",,");
let win = await displayCertificate(cert);
// As of bug 1312827, OneCRL only applies to TLS web server certificates, so
// this certificate will actually verify successfully for every end-entity
// usage except TLS web server.
checkUsages(win, ["Email Recipient Certificate", "Email Signer Certificate",
"SSL Client Certificate"]);
+ checkDetailsPane(win, ["ca", "revoked"]);
await BrowserTestUtils.closeWindow(win);
});
add_task(async function testInvalid() {
// This certificate has a keyUsage extension asserting cRLSign and
// keyCertSign, but it doesn't have a basicConstraints extension. This
// shouldn't be valid for any usage. Sadly, we give a pretty lame error
// message in this case.
let cert = await readCertificate("invalid.pem", ",,");
let win = await displayCertificate(cert);
checkError(win, "Could not verify this certificate for unknown reasons.");
+ checkDetailsPane(win, ["invalid"]);
await BrowserTestUtils.closeWindow(win);
});
add_task(async function testLongOID() {
// This certificate has a certificatePolicies extension with a policy with a
// very long OID. This tests that we don't crash when looking at it.
let cert = await readCertificate("longOID.pem", ",,");
let win = await displayCertificate(cert);
+ checkDetailsPane(win, ["Long OID"]);
await BrowserTestUtils.closeWindow(win);
});
/**
* Given a certificate, returns a promise that will resolve when the certificate
* viewer has opened is displaying that certificate, and has finished
* determining its valid usages.
*
@@ -225,8 +238,29 @@ function checkUsages(win, usages) {
*/
function checkError(win, error) {
let determinedUsages = getUsages(win);
Assert.equal(determinedUsages.length, 0,
"should not have any successful usages in error case");
Assert.equal(getError(win), error,
"determined error should be the same as expected error");
}
+
+/**
+ * Given a certificate viewer window and an expected list of certificate names,
+ * verifies that the certificate details pane of the viewer shows the expected
+ * certificates in the expected order.
+ *
+ * @param {window} win
+ * The certificate viewer window.
+ * @param {String[]} names
+ * An array of expected certificate names.
+ */
+function checkDetailsPane(win, names) {
+ let tree = win.document.getElementById("treesetDump");
+ let nodes = tree.querySelectorAll("treecell");
+ Assert.equal(nodes.length, names.length,
+ "details pane: should have the expected number of cert names");
+ for (let i = 0; i < names.length; i++) {
+ Assert.equal(nodes[i].getAttribute("label"), names[i],
+ "details pain: should have expected cert name");
+ }
+}