Bug 1281665 - Change nsIClientAuthDialogs.chooseCertificate() to use hostname instead of CN. r=keeler
authorCykesiopka <cykesiopka.bmo@gmail.com>
Tue, 26 Jul 2016 20:16:58 +0800
changeset 348933 2fefec564929ded72295da4433196e4e0f587f61
parent 348932 48be91b202107da09bf5e6888f6563155f10f8a3
child 348934 23acad9c31c780b4fea54fb1501fcce61d530409
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1281665
milestone50.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1281665 - Change nsIClientAuthDialogs.chooseCertificate() to use hostname instead of CN. r=keeler chooseCertificate() currently uses a concatenation of the Common Name of the server cert and the port of the server to allow the user to identify the server requesting client authentication. Unfortunately, this approach is flawed, since it doesn't take into account things like SAN entries, which might be very different from the CN. Using the hostname instead avoids this problem. MozReview-Commit-ID: 6XjGCknWNi9
mobile/android/components/NSSDialogService.js
mobile/android/locales/en-US/chrome/pippki.properties
security/manager/locales/en-US/chrome/pippki/pippki.properties
security/manager/pki/nsNSSDialogs.cpp
security/manager/pki/resources/content/clientauthask.js
security/manager/ssl/nsIClientAuthDialogs.idl
security/manager/ssl/nsNSSIOLayer.cpp
--- a/mobile/android/components/NSSDialogService.js
+++ b/mobile/android/components/NSSDialogService.js
@@ -207,23 +207,24 @@ NSSDialogs.prototype = {
   viewCertDetails: function(details) {
     let p = this.getPrompt(this.getString("clientAuthAsk.message3"),
                     '',
                     [ this.getString("nssdialogs.ok.label") ]);
     p.addLabel({ label: details });
     this.showPrompt(p);
   },
 
-  chooseCertificate: function(ctx, cnAndPort, organization, issuerOrg, certList,
-                              selectedIndex) {
+  chooseCertificate: function(ctx, hostname, port, organization, issuerOrg,
+                              certList, selectedIndex) {
     let rememberSetting =
       Services.prefs.getBoolPref("security.remember_cert_checkbox_default_setting");
 
     let serverRequestedDetails = [
-      this.escapeHTML(cnAndPort),
+      this.formatString("clientAuthAsk.hostnameAndPort",
+                        [hostname, port.toString()]),
       this.formatString("clientAuthAsk.organization", [organization]),
       this.formatString("clientAuthAsk.issuer", [issuerOrg]),
     ].join("<br/>");
 
     let certNickList = [];
     let certDetailsList = [];
     for (let i = 0; i < certList.length; i++) {
       let cert = certList.queryElementAt(i, Ci.nsIX509Cert);
--- a/mobile/android/locales/en-US/chrome/pippki.properties
+++ b/mobile/android/locales/en-US/chrome/pippki.properties
@@ -19,16 +19,20 @@ clientAuthAsk.message1=This site has req
 clientAuthAsk.message2=Choose a certificate to present as identification:
 clientAuthAsk.message3=Details of selected certificate:
 clientAuthAsk.remember.label=Remember this decision
 # LOCALIZATION NOTE(clientAuthAsk.nickAndSerial): Represents a single cert when
 # the user is choosing from a list of certificates.
 # %1$S is the nickname of the cert.
 # %2$S is the serial number of the cert in AA:BB:CC hex format.
 clientAuthAsk.nickAndSerial=%1$S [%2$S]
+# LOCALIZATION NOTE(clientAuthAsk.hostnameAndPort):
+# %1$S is the hostname of the server.
+# %2$S is the port of the server.
+clientAuthAsk.hostnameAndPort=%1$S:%2$S
 # LOCALIZATION NOTE(clientAuthAsk.organization): %S is the Organization of the
 # server cert.
 clientAuthAsk.organization=Organization: "%S"
 # LOCALIZATION NOTE(clientAuthAsk.issuer): %S is the Organization of the
 # issuer cert of the server cert.
 clientAuthAsk.issuer=Issued Under: "%S"
 # LOCALIZATION NOTE(clientAuthAsk.issuedTo): %1$S is the Distinguished Name of
 # the currently selected client cert, such as "CN=John Doe,OU=Example" (without
--- a/security/manager/locales/en-US/chrome/pippki/pippki.properties
+++ b/security/manager/locales/en-US/chrome/pippki/pippki.properties
@@ -53,16 +53,20 @@ certNotVerified_Unknown=Could not verify
 
 #Client auth
 clientAuthRemember=Remember this decision
 # LOCALIZATION NOTE(clientAuthNickAndSerial): Represents a single cert when the
 # user is choosing from a list of certificates.
 # %1$S is the nickname of the cert.
 # %2$S is the serial number of the cert in AA:BB:CC hex format.
 clientAuthNickAndSerial=%1$S [%2$S]
+# LOCALIZATION NOTE(clientAuthHostnameAndPort):
+# %1$S is the hostname of the server.
+# %2$S is the port of the server.
+clientAuthHostnameAndPort=%1$S:%2$S
 # LOCALIZATION NOTE(clientAuthMessage1): %S is the Organization of the server
 # cert.
 clientAuthMessage1=Organization: ā€œ%Sā€
 # LOCALIZATION NOTE(clientAuthMessage2): %S is the Organization of the issuer
 # cert of the server cert.
 clientAuthMessage2=Issued Under: ā€œ%Sā€
 # LOCALIZATION NOTE(clientAuthIssuedTo): %1$S is the Distinguished Name of the
 # currently selected client cert, such as "CN=John Doe,OU=Example" (without
--- a/security/manager/pki/nsNSSDialogs.cpp
+++ b/security/manager/pki/nsNSSDialogs.cpp
@@ -153,19 +153,20 @@ nsNSSDialogs::ConfirmDownloadCACert(nsII
 
   *_retval = (status != 0);
 
   return rv;
 }
 
 NS_IMETHODIMP
 nsNSSDialogs::ChooseCertificate(nsIInterfaceRequestor* ctx,
-                                const nsAString& cnAndPort,
-                                const nsAString& organization,
-                                const nsAString& issuerOrg,
+                                const nsACString& hostname,
+                                int32_t port,
+                                const nsACString& organization,
+                                const nsACString& issuerOrg,
                                 nsIArray* certList,
                         /*out*/ uint32_t* selectedIndex,
                         /*out*/ bool* certificateChosen)
 {
   NS_ENSURE_ARG_POINTER(ctx);
   NS_ENSURE_ARG_POINTER(certList);
   NS_ENSURE_ARG_POINTER(selectedIndex);
   NS_ENSURE_ARG_POINTER(certificateChosen);
@@ -191,25 +192,30 @@ nsNSSDialogs::ChooseCertificate(nsIInter
     return rv;
   }
 
   rv = block->SetNumberStrings(3);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
-  rv = block->SetString(0, PromiseFlatString(cnAndPort).get());
+  rv = block->SetString(0, NS_ConvertUTF8toUTF16(hostname).get());
   if (NS_FAILED(rv)) {
     return rv;
   }
-  rv = block->SetString(1, PromiseFlatString(organization).get());
+  rv = block->SetString(1, NS_ConvertUTF8toUTF16(organization).get());
   if (NS_FAILED(rv)) {
     return rv;
   }
-  rv = block->SetString(2, PromiseFlatString(issuerOrg).get());
+  rv = block->SetString(2, NS_ConvertUTF8toUTF16(issuerOrg).get());
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  rv = block->SetInt(0, port);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   rv = nsNSSDialogHelper::openDialog(nullptr,
                                      "chrome://pippki/content/clientauthask.xul",
                                      block);
   if (NS_FAILED(rv)) {
--- a/security/manager/pki/resources/content/clientauthask.js
+++ b/security/manager/pki/resources/content/clientauthask.js
@@ -37,23 +37,27 @@ function onLoad() {
   bundle = document.getElementById("pippki_bundle");
   let rememberSetting =
     Services.prefs.getBoolPref("security.remember_cert_checkbox_default_setting");
 
   rememberBox = document.getElementById("rememberBox");
   rememberBox.label = bundle.getString("clientAuthRemember");
   rememberBox.checked = rememberSetting;
 
-  let cnAndPort = dialogParams.GetString(0);
+  let hostname = dialogParams.GetString(0);
   let org = dialogParams.GetString(1);
   let issuerOrg = dialogParams.GetString(2);
+  let port = dialogParams.GetInt(0);
   let formattedOrg = bundle.getFormattedString("clientAuthMessage1", [org]);
   let formattedIssuerOrg = bundle.getFormattedString("clientAuthMessage2",
                                                      [issuerOrg]);
-  setText("hostname", cnAndPort);
+  let formattedHostnameAndPort =
+    bundle.getFormattedString("clientAuthHostnameAndPort",
+                              [hostname, port.toString()]);
+  setText("hostname", formattedHostnameAndPort);
   setText("organization", formattedOrg);
   setText("issuer", formattedIssuerOrg);
 
   let selectElement = document.getElementById("nicknames");
   certArray = dialogParams.objects.queryElementAt(0, Ci.nsIArray);
   for (let i = 0; i < certArray.length; i++) {
     let menuItemNode = document.createElement("menuitem");
     let cert = certArray.queryElementAt(i, Ci.nsIX509Cert);
--- a/security/manager/ssl/nsIClientAuthDialogs.idl
+++ b/security/manager/ssl/nsIClientAuthDialogs.idl
@@ -13,28 +13,30 @@ interface nsIInterfaceRequestor;
 [scriptable, uuid(fa4c7520-1433-11d5-ba24-00108303b117)]
 interface nsIClientAuthDialogs : nsISupports
 {
   /**
    * Called when a user is asked to choose a certificate for client auth.
    *
    * @param ctx Context that allows at least nsIClientAuthUserDecision to be
    *            queried.
-   * @param cnAndPort Common Name of the server cert and the port of the server.
+   * @param hostname Hostname of the server.
+   * @param port Port of the server.
    * @param organization Organization field of the server cert.
    * @param issuerOrg Organization field of the issuer cert of the server cert.
    * @param certList List of certificates the user can choose from.
    * @param selectedIndex Index of the cert in |certList| that the user chose.
    *                      Ignored if the return value is false.
    * @return true if a certificate was chosen. false if the user canceled.
    */
   boolean chooseCertificate(in nsIInterfaceRequestor ctx,
-                            in AString cnAndPort,
-                            in AString organization,
-                            in AString issuerOrg,
+                            in AUTF8String hostname,
+                            in long port,
+                            in AUTF8String organization,
+                            in AUTF8String issuerOrg,
                             in nsIArray certList,
                             out unsigned long selectedIndex);
 };
 
 [scriptable, uuid(95c4373e-bdd4-4a63-b431-f5b000367721)]
 interface nsIClientAuthUserDecision : nsISupports
 {
   attribute boolean rememberClientAuthCertificate;
--- a/security/manager/ssl/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/nsNSSIOLayer.cpp
@@ -2221,41 +2221,24 @@ ClientAuthDataRunnable::RunOnTargetThrea
         }
       }
 
       if (CERT_LIST_END(CERT_LIST_HEAD(certList), certList)) {
         // list is empty - no matching certs
         goto loser;
       }
 
-      // Get CN and O of the subject and O of the issuer
-      UniquePORTString ccn(CERT_GetCommonName(&mServerCert->subject));
-      NS_ConvertUTF8toUTF16 cn(ccn.get());
-
       int32_t port;
       mSocketInfo->GetPort(&port);
 
-      nsAutoString cn_host_port;
-      if (ccn && strcmp(ccn.get(), hostname) == 0) {
-        cn_host_port.Append(cn);
-        cn_host_port.Append(':');
-        cn_host_port.AppendInt(port);
-      } else {
-        cn_host_port.Append(cn);
-        cn_host_port.AppendLiteral(" (");
-        cn_host_port.Append(':');
-        cn_host_port.AppendInt(port);
-        cn_host_port.Append(')');
-      }
-
       UniquePORTString corg(CERT_GetOrgName(&mServerCert->subject));
-      NS_ConvertUTF8toUTF16 org(corg.get());
+      nsAutoCString org(corg.get());
 
       UniquePORTString cissuer(CERT_GetOrgName(&mServerCert->issuer));
-      NS_ConvertUTF8toUTF16 issuer(cissuer.get());
+      nsAutoCString issuer(cissuer.get());
 
       nsCOMPtr<nsIMutableArray> certArray = nsArrayBase::Create();
       if (!certArray) {
         goto loser;
       }
 
       for (CERTCertListNode* node = CERT_LIST_HEAD(certList);
            !CERT_LIST_END(node, certList);
@@ -2277,17 +2260,17 @@ ClientAuthDataRunnable::RunOnTargetThrea
                          NS_CLIENTAUTHDIALOGS_CONTRACTID);
 
       if (NS_FAILED(rv)) {
         goto loser;
       }
 
       uint32_t selectedIndex = 0;
       bool certChosen = false;
-      rv = dialogs->ChooseCertificate(mSocketInfo, cn_host_port, org, issuer,
+      rv = dialogs->ChooseCertificate(mSocketInfo, hostname, port, org, issuer,
                                       certArray, &selectedIndex, &certChosen);
       if (NS_FAILED(rv)) {
         goto loser;
       }
 
       // even if the user has canceled, we want to remember that, to avoid repeating prompts
       bool wantRemember = false;
       mSocketInfo->GetRememberClientAuthCertificate(&wantRemember);