Bug 1308888 - Simplify passing handle to the cert to view in the cert viewer. r=keeler draft
authorCykesiopka <cykesiopka.bmo@gmail.com>
Fri, 21 Oct 2016 00:33:36 +0800
changeset 427604 776a27111ab26cdcdc91b002890c43a3fe4f48e8
parent 427411 c0c023815311c2632a9763b696492342b0e829e1
child 534511 123be99ff898b889679ac7c3171c8599719252e3
push id33064
push usercykesiopka.bmo@gmail.com
push dateThu, 20 Oct 2016 16:36:07 +0000
reviewerskeeler
bugs1308888
milestone52.0a1
Bug 1308888 - Simplify passing handle to the cert to view in the cert viewer. r=keeler The cert viewer currently supports two ways to pass a handle to the cert: 1. Passing the nickname of the cert via window.name. 2. Via an nsIDialogParamBlock, which is itself accessed through window.arguments. Method 1 is unused and unnecessary. Method 2 is overly complex: the relevant nsIX509Cert can just be passed directly. This patch does the following: 1. Makes it so that there is only a single, straightforward way to pass a handle to the cert. 2. Makes the cert viewer title localisable while we're nearby. 3. Renames viewCertDetails.js to better reflect the current use of the file. MozReview-Commit-ID: pqtfNgvImT
security/manager/locales/en-US/chrome/pippki/pippki.properties
security/manager/pki/nsNSSDialogHelper.h
security/manager/pki/nsNSSDialogs.cpp
security/manager/pki/resources/content/certViewer.js
security/manager/pki/resources/content/certViewer.xul
security/manager/pki/resources/content/viewCertDetails.js
security/manager/pki/resources/jar.mn
security/manager/ssl/tests/mochitest/browser/browser_certViewer.js
--- a/security/manager/locales/en-US/chrome/pippki/pippki.properties
+++ b/security/manager/locales/en-US/chrome/pippki/pippki.properties
@@ -106,17 +106,19 @@ pageInfo_EncryptionWithBitsAndProtocol=C
 pageInfo_BrokenEncryption=Broken Encryption (%1$S, %2$S bit keys, %3$S)
 pageInfo_Privacy_Encrypted1=The page you are viewing was encrypted before being transmitted over the Internet.
 pageInfo_Privacy_Encrypted2=Encryption makes it difficult for unauthorized people to view information traveling between computers. It is therefore unlikely that anyone read this page as it traveled across the network.
 pageInfo_MixedContent=Connection Partially Encrypted
 pageInfo_MixedContent2=Parts of the page you are viewing were not encrypted before being transmitted over the Internet.
 pageInfo_WeakCipher=Your connection to this website uses weak encryption and is not private. Other people can view your information or modify the website’s behavior.
 
 # Cert Viewer
-certDetails=Certificate Viewer:
+# LOCALIZATION NOTE(certViewerTitle): Title used for the Certificate Viewer.
+# %1$S is a string representative of the certificate being viewed.
+certViewerTitle=Certificate Viewer: “%1$S”
 notPresent=<Not Part Of Certificate>
 
 # Token Manager
 password_not_set=(not set)
 failed_pw_change=Unable to change Master Password.
 incorrect_pw=You did not enter the correct current Master Password. Please try again.
 pw_change_ok=Master Password successfully changed.
 pw_erased_ok=Warning! You have deleted your Master Password. 
--- a/security/manager/pki/nsNSSDialogHelper.h
+++ b/security/manager/pki/nsNSSDialogHelper.h
@@ -6,22 +6,33 @@
 
 #ifndef nsNSSDialogHelper_h
 #define nsNSSDialogHelper_h
 
 class mozIDOMWindowProxy;
 class nsISupports;
 
 /**
- * Common class that uses the window watcher service to open a
- * standard dialog, with or without a parent context. The params
- * parameter can be an nsISupportsArray so any number of additional
- * arguments can be used.
+ * Helper class that uses the window watcher service to open a standard dialog,
+ * with or without a parent context.
  */
 class nsNSSDialogHelper
 {
 public:
-  // params is a nsIDialogParamBlock or a nsIKeygenThread
+  /**
+   * Opens a XUL dialog.
+   *
+   * @param window
+   *        Parent window of the dialog, or nullptr to signal no parent.
+   * @param url
+   *        URL to the XUL dialog.
+   * @param params
+   *        Parameters to pass to the dialog. Same semantics as the
+   *        nsIWindowWatcher.openWindow() |aArguments| parameter.
+   * @param modal
+   *        true if the dialog should be modal, false otherwise.
+   * @return The result of opening the dialog.
+   */
   static nsresult openDialog(mozIDOMWindowProxy* window, const char* url,
                              nsISupports* params, bool modal = true);
 };
 
-#endif
+#endif // nsNSSDialogHelper_h
--- a/security/manager/pki/nsNSSDialogs.cpp
+++ b/security/manager/pki/nsNSSDialogs.cpp
@@ -2,45 +2,38 @@
  *
  * 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/. */
 
 /*
  * Dialog services for PIP.
  */
+
+#include "nsNSSDialogs.h"
+
 #include "mozIDOMWindow.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/Casting.h"
 #include "nsArray.h"
-#include "nsDateTimeFormatCID.h"
 #include "nsEmbedCID.h"
-#include "nsIComponentManager.h"
-#include "nsIDateTimeFormat.h"
 #include "nsIDialogParamBlock.h"
 #include "nsIInterfaceRequestor.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsIKeygenThread.h"
 #include "nsIPromptService.h"
 #include "nsIProtectedAuthThread.h"
-#include "nsIServiceManager.h"
 #include "nsIWindowWatcher.h"
 #include "nsIX509CertDB.h"
 #include "nsIX509Cert.h"
-#include "nsIX509CertValidity.h"
 #include "nsNSSDialogHelper.h"
-#include "nsNSSDialogs.h"
-#include "nsPromiseFlatString.h"
-#include "nsReadableUtils.h"
 #include "nsString.h"
 
 #define PIPSTRING_BUNDLE_URL "chrome://pippki/locale/pippki.properties"
 
-/* ==== */
-
 nsNSSDialogs::nsNSSDialogs()
 {
 }
 
 nsNSSDialogs::~nsNSSDialogs()
 {
 }
 
@@ -324,43 +317,28 @@ nsNSSDialogs::GetPKCS12FilePassword(nsII
   if (*_retval) {
     _password.Assign(pwTemp);
     free(pwTemp);
   }
 
   return NS_OK;
 }
 
-NS_IMETHODIMP 
+NS_IMETHODIMP
 nsNSSDialogs::ViewCert(nsIInterfaceRequestor* ctx, nsIX509Cert* cert)
 {
-  nsCOMPtr<nsIMutableArray> dlgArray = nsArrayBase::Create();
-  if (!dlgArray) {
-    return NS_ERROR_FAILURE;
-  }
-  nsresult rv = dlgArray->AppendElement(cert, false);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-  nsCOMPtr<nsIDialogParamBlock> dlgParamBlock(
-    do_CreateInstance(NS_DIALOGPARAMBLOCK_CONTRACTID));
-  if (!dlgParamBlock) {
-    return NS_ERROR_FAILURE;
-  }
-  rv = dlgParamBlock->SetObjects(dlgArray);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
+  // |ctx| is allowed to be null.
+  NS_ENSURE_ARG(cert);
 
   // Get the parent window for the dialog
   nsCOMPtr<mozIDOMWindowProxy> parent = do_GetInterface(ctx);
   return nsNSSDialogHelper::openDialog(parent,
                                        "chrome://pippki/content/certViewer.xul",
-                                       dlgParamBlock,
-                                       false);
+                                       cert,
+                                       false /*modal*/);
 }
 
 NS_IMETHODIMP
 nsNSSDialogs::DisplayGeneratingKeypairInfo(nsIInterfaceRequestor *aCtx, nsIKeygenThread *runnable) 
 {
   nsresult rv;
 
   // Get the parent window for the dialog
rename from security/manager/pki/resources/content/viewCertDetails.js
rename to security/manager/pki/resources/content/certViewer.js
--- a/security/manager/pki/resources/content/viewCertDetails.js
+++ b/security/manager/pki/resources/content/certViewer.js
@@ -1,27 +1,34 @@
 /* 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";
 
+/**
+ * @file Implements functionality for certViewer.xul and its tabs certDump.xul
+ *       and viewCertDetails.xul: a dialog that allows various attributes of a
+ *       certificate to be viewed.
+ * @argument {nsISupports} window.arguments[0]
+ *           The cert to view, queryable to nsIX509Cert.
+ */
+
 const { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components;
 const { Services } = Cu.import("resource://gre/modules/Services.jsm", {});
 
 const nsIX509Cert = Ci.nsIX509Cert;
 const nsX509CertDB = "@mozilla.org/security/x509certdb;1";
 const nsIX509CertDB = Ci.nsIX509CertDB;
 const nsPK11TokenDB = "@mozilla.org/security/pk11tokendb;1";
 const nsIPK11TokenDB = Ci.nsIPK11TokenDB;
 const nsIASN1Object = Ci.nsIASN1Object;
 const nsIASN1Sequence = Ci.nsIASN1Sequence;
 const nsIASN1PrintableItem = Ci.nsIASN1PrintableItem;
 const nsIASN1Tree = Ci.nsIASN1Tree;
 const nsASN1Tree = "@mozilla.org/security/nsASN1Tree;1";
-const nsIDialogParamBlock = Ci.nsIDialogParamBlock;
 
 var bundle;
 
 function doPrompt(msg)
 {
   let prompts = Components.classes["@mozilla.org/embedcomp/prompt-service;1"].
     getService(Components.interfaces.nsIPromptService);
   prompts.alert(window, null, msg);
@@ -66,31 +73,21 @@ function AddUsage(usage)
   text.setAttribute("style", "margin: 2px 5px");
   text.setAttribute("readonly", "true");
   text.setAttribute("class", "scrollfield");
   verifyInfoBox.appendChild(text);
 }
 
 function setWindowName()
 {
-  //  Get the cert from the cert database
-  var certdb = Components.classes[nsX509CertDB].getService(nsIX509CertDB);
-  var myName = self.name;
   bundle = document.getElementById("pippki_bundle");
-  var cert;
 
-  var certDetails = bundle.getString('certDetails');
-  if (myName != "") {
-    document.title = certDetails + '"' + myName + '"'; // XXX l10n?
-    cert = certdb.findCertByNickname(myName);
-  } else {
-    var params = window.arguments[0].QueryInterface(nsIDialogParamBlock);
-    cert = params.objects.queryElementAt(0, nsIX509Cert);
-    document.title = certDetails + '"' + cert.windowTitle + '"'; // XXX l10n?
-  }
+  let cert = window.arguments[0].QueryInterface(Ci.nsIX509Cert);
+  document.title = bundle.getFormattedString("certViewerTitle",
+                                             [cert.windowTitle]);
 
   //
   //  Set the cert attributes for viewing
   //
 
   //  The chain of trust
   AddCertChain("treesetDump", cert.getChain());
   DisplayGeneralDataFromCert(cert);
--- a/security/manager/pki/resources/content/certViewer.xul
+++ b/security/manager/pki/resources/content/certViewer.xul
@@ -15,17 +15,18 @@
   xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
   buttons="accept"
   buttonlabelaccept="&certmgr.close.label;"
   buttonaccesskeyaccept="&certmgr.close.accesskey;"
   onload="setWindowName();">
 
 <stringbundle id="pippki_bundle" src="chrome://pippki/locale/pippki.properties"/>
 
-<script type="application/javascript" src="chrome://pippki/content/viewCertDetails.js"/>
+<script type="application/javascript"
+        src="chrome://pippki/content/certViewer.js"/>
 <script type="application/javascript" src="chrome://pippki/content/pippki.js"/>
 
   <tabbox flex="1">
     <tabs>
       <tab id="general_tab" label="&certmgr.detail.general_tab.title;"
            accesskey="&certmgr.detail.general_tab.accesskey;"/>
       <tab id="prettyprint_tab" label="&certmgr.detail.prettyprint_tab.title;"
            accesskey="&certmgr.detail.prettyprint_tab.accesskey;"/>
--- a/security/manager/pki/resources/jar.mn
+++ b/security/manager/pki/resources/jar.mn
@@ -19,22 +19,22 @@ pippki.jar:
     content/pippki/OrphanOverlay.xul         (content/OrphanOverlay.xul)
     content/pippki/viewCertDetails.xul       (content/viewCertDetails.xul)
     content/pippki/editcacert.xul            (content/editcacert.xul)
     content/pippki/editcacert.js             (content/editcacert.js)
 *   content/pippki/exceptionDialog.xul       (content/exceptionDialog.xul)
     content/pippki/exceptionDialog.js        (content/exceptionDialog.js)
     content/pippki/deletecert.xul            (content/deletecert.xul)
     content/pippki/deletecert.js             (content/deletecert.js)
-    content/pippki/viewCertDetails.js        (content/viewCertDetails.js)
     content/pippki/setp12password.xul        (content/setp12password.xul)
     content/pippki/pippki.js                 (content/pippki.js)
     content/pippki/clientauthask.xul	     (content/clientauthask.xul)
     content/pippki/clientauthask.js          (content/clientauthask.js)
     content/pippki/certViewer.xul            (content/certViewer.xul)
+    content/pippki/certViewer.js             (content/certViewer.js)
     content/pippki/certDump.xul              (content/certDump.xul)
     content/pippki/device_manager.xul        (content/device_manager.xul)
     content/pippki/device_manager.js         (content/device_manager.js)
     content/pippki/load_device.xul           (content/load_device.xul)
     content/pippki/choosetoken.xul           (content/choosetoken.xul)
     content/pippki/choosetoken.js            (content/choosetoken.js)
     content/pippki/createCertInfo.xul        (content/createCertInfo.xul)
     content/pippki/createCertInfo.js         (content/createCertInfo.js)
--- a/security/manager/ssl/tests/mochitest/browser/browser_certViewer.js
+++ b/security/manager/ssl/tests/mochitest/browser/browser_certViewer.js
@@ -5,113 +5,118 @@
 
 // Repeatedly opens the certificate viewer dialog with various certificates and
 // determines that the viewer correctly identifies either what usages those
 // certificates are valid for or what errors prevented the certificates from
 // being verified.
 
 var { OS } = Cu.import("resource://gre/modules/osfile.jsm", {});
 
-add_task(function* () {
+add_task(function* testCAandTitle() {
   let cert = yield readCertificate("ca.pem", "CTu,CTu,CTu");
   let win = yield displayCertificate(cert);
   checkUsages(win, ["SSL Certificate Authority"]);
+
+  // 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");
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testSSLEndEntity() {
   let cert = yield readCertificate("ssl-ee.pem", ",,");
   let win = yield displayCertificate(cert);
   checkUsages(win, ["SSL Server Certificate", "SSL Client Certificate"]);
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testEmailEndEntity() {
   let cert = yield readCertificate("email-ee.pem", ",,");
   let win = yield displayCertificate(cert);
   checkUsages(win, ["Email Recipient Certificate", "Email Signer Certificate"]);
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testCodeSignEndEntity() {
   let cert = yield readCertificate("code-ee.pem", ",,");
   let win = yield displayCertificate(cert);
   checkUsages(win, ["Object Signer"]);
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testExpired() {
   let cert = yield readCertificate("expired-ca.pem", ",,");
   let win = yield displayCertificate(cert);
   checkError(win, "Could not verify this certificate because it has expired.");
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testIssuerExpired() {
   let cert = yield readCertificate("ee-from-expired-ca.pem", ",,");
   let win = yield displayCertificate(cert);
   checkError(win,
              "Could not verify this certificate because the CA certificate " +
              "is invalid.");
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testUnknownIssuer() {
   let cert = yield readCertificate("unknown-issuer.pem", ",,");
   let win = yield displayCertificate(cert);
   checkError(win,
              "Could not verify this certificate because the issuer is " +
              "unknown.");
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testInsecureAlgo() {
   let cert = yield readCertificate("md5-ee.pem", ",,");
   let win = yield 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.");
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testUntrusted() {
   let cert = yield readCertificate("untrusted-ca.pem", "p,p,p");
   let win = yield displayCertificate(cert);
   checkError(win,
              "Could not verify this certificate because it is not trusted.");
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testUntrustedIssuer() {
   let cert = yield readCertificate("ee-from-untrusted-ca.pem", ",,");
   let win = yield displayCertificate(cert);
   checkError(win,
              "Could not verify this certificate because the issuer is not " +
              "trusted.");
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(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"]
                         .getService(Ci.nsICertBlocklist);
   certBlocklist.revokeCertBySubjectAndPubKey(
     "MBIxEDAOBgNVBAMMB3Jldm9rZWQ=", // CN=revoked
     "VCIlmPM9NkgFQtrs4Oa5TeFcDu6MWRTKSNdePEhOgD8="); // hash of the shared key
   let cert = yield readCertificate("revoked.pem", ",,");
   let win = yield displayCertificate(cert);
   checkError(win,
              "Could not verify this certificate because it has been revoked.");
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(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 = yield readCertificate("invalid.pem", ",,");
   let win = yield displayCertificate(cert);
   checkError(win, "Could not verify this certificate for unknown reasons.");
   yield BrowserTestUtils.closeWindow(win);
@@ -124,23 +129,18 @@ add_task(function* () {
  *
  * @param {nsIX509Cert} certificate
  *        The certificate to view and determine usages for.
  * @return {Promise}
  *         A promise that will resolve with a handle on the opened certificate
  *         viewer window when the usages have been determined.
  */
 function displayCertificate(certificate) {
-  let array = Cc["@mozilla.org/array;1"].createInstance(Ci.nsIMutableArray);
-  array.appendElement(certificate, false);
-  let params = Cc["@mozilla.org/embedcomp/dialogparam;1"]
-                 .createInstance(Ci.nsIDialogParamBlock);
-  params.objects = array;
   let win = window.openDialog("chrome://pippki/content/certViewer.xul", "",
-                              "", params);
+                              "", certificate);
   return TestUtils.topicObserved("ViewCertDetails:CertUsagesDone",
                                  (subject, data) => subject == win)
   .then(([subject, data]) => subject, error => { throw error; });
 }
 
 /**
  * Given a certificate viewer window, finds the usages the certificate is valid
  * for.