Bug 1281665 - Change nsIClientAuthDialogs.chooseCertificate() to use hostname instead of CN. r=keeler draft
authorCykesiopka <cykesiopka.bmo@gmail.com>
Tue, 26 Jul 2016 20:16:58 +0800
changeset 392942 6b104e7be8a49bc99a2351d199d2a3c02e2b7fa5
parent 392914 4ee7de4773864485b468f6178b4634c7f06ce077
child 526436 9c250e77f840f29cb429b9fb693fd5a246620a1b
push id24151
push usercykesiopka.bmo@gmail.com
push dateTue, 26 Jul 2016 14:16:25 +0000
reviewerskeeler
bugs1281665
milestone50.0a1
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);