bug 1239344 - remove error alert for successful PKCS12 operations r?Cykesiopka draft
authorDavid Keeler <dkeeler@mozilla.com>
Fri, 03 Mar 2017 11:12:54 -0800
changeset 493429 7de804a63bea3cbd20c73d82c5a0ebd3447712fe
parent 493428 86e1748ab478e23f7a73e2c4af11a310d26fbda9
child 547855 66c6b1f866738af5822305f5ae37a1d103427f18
push id47755
push userbmo:dkeeler@mozilla.com
push dateFri, 03 Mar 2017 22:32:10 +0000
reviewersCykesiopka
bugs1239344
milestone54.0a1
bug 1239344 - remove error alert for successful PKCS12 operations r?Cykesiopka MozReview-Commit-ID: Hr6s2v2GmZQ
security/manager/locales/en-US/chrome/pipnss/pipnss.properties
security/manager/ssl/nsNSSCertHelper.cpp
security/manager/ssl/nsNSSCertHelper.h
security/manager/ssl/nsNSSCertificateDB.cpp
security/manager/ssl/nsNSSComponent.cpp
security/manager/ssl/nsNSSComponent.h
security/manager/ssl/nsPKCS12Blob.cpp
--- a/security/manager/locales/en-US/chrome/pipnss/pipnss.properties
+++ b/security/manager/locales/en-US/chrome/pipnss/pipnss.properties
@@ -231,18 +231,16 @@ CertDumpECsect239k1=SECG elliptic curve 
 CertDumpECsect283k1=SECG elliptic curve sect283k1 (aka NIST K-283)
 CertDumpECsect283r1=SECG elliptic curve sect283r1 (aka NIST B-283)
 CertDumpECsect409k1=SECG elliptic curve sect409k1 (aka NIST K-409)
 CertDumpECsect409r1=SECG elliptic curve sect409r1 (aka NIST B-409)
 CertDumpECsect571k1=SECG elliptic curve sect571k1 (aka NIST K-571)
 CertDumpECsect571r1=SECG elliptic curve sect571r1 (aka NIST B-571)
 CertDumpRawBytesHeader=Size: %S Bytes / %S Bits
 PK11BadPassword=The password entered was incorrect.
-SuccessfulP12Backup=Successfully backed up your security certificate(s) and private key(s).
-SuccessfulP12Restore=Successfully restored your security certificate(s) and private key(s).
 PKCS12DecodeErr=Failed to decode the file.  Either it is not in PKCS #12 format, has been corrupted, or the password you entered was incorrect.
 PKCS12UnknownErrRestore=Failed to restore the PKCS #12 file for unknown reasons.
 PKCS12UnknownErrBackup=Failed to create the PKCS #12 backup file for unknown reasons.
 PKCS12UnknownErr=The PKCS #12 operation failed for unknown reasons.
 PKCS12InfoNoSmartcardBackup=It is not possible to back up certificates from a hardware security device such as a smart card.
 PKCS12DupData=The certificate and private key already exist on the security device.
 AddModuleFailure=Unable to add module
 AddModuleDup=Security Module already exists
--- a/security/manager/ssl/nsNSSCertHelper.cpp
+++ b/security/manager/ssl/nsNSSCertHelper.cpp
@@ -84,19 +84,23 @@ GetPIPNSSBundle(nsIStringBundle** pipnss
     do_GetService(NS_STRINGBUNDLE_CONTRACTID));
   if (!bundleService) {
     return NS_ERROR_NOT_AVAILABLE;
   }
   return bundleService->CreateBundle("chrome://pipnss/locale/pipnss.properties",
                                      pipnssBundle);
 }
 
-static nsresult
+nsresult
 GetPIPNSSBundleString(const char* stringName, nsAString& result)
 {
+  MOZ_ASSERT(NS_IsMainThread());
+  if (!NS_IsMainThread()) {
+    return NS_ERROR_NOT_SAME_THREAD;
+  }
   MOZ_ASSERT(stringName);
   if (!stringName) {
     return NS_ERROR_INVALID_ARG;
   }
   nsCOMPtr<nsIStringBundle> pipnssBundle;
   nsresult rv = GetPIPNSSBundle(getter_AddRefs(pipnssBundle));
   if (NS_FAILED(rv)) {
     return rv;
--- a/security/manager/ssl/nsNSSCertHelper.h
+++ b/security/manager/ssl/nsNSSCertHelper.h
@@ -8,16 +8,19 @@
 #ifndef INET6_ADDRSTRLEN
 #define INET6_ADDRSTRLEN 46
 #endif
 
 #include "certt.h"
 #include "nsString.h"
 
 uint32_t
-getCertType(CERTCertificate *cert);
+getCertType(CERTCertificate* cert);
 
 nsresult
-GetCertFingerprintByOidTag(CERTCertificate* nsscert,
-                           SECOidTag aOidTag, 
-                           nsCString &fp);
+GetCertFingerprintByOidTag(CERTCertificate* nsscert, SECOidTag aOidTag,
+                           nsCString& fp);
+
+// Must be used on the main thread only.
+nsresult
+GetPIPNSSBundleString(const char* stringName, nsAString& result);
 
 #endif // nsNSSCertHelper_h
--- a/security/manager/ssl/nsNSSCertificateDB.cpp
+++ b/security/manager/ssl/nsNSSCertificateDB.cpp
@@ -964,30 +964,36 @@ nsNSSCertificateDB::ImportCertsFromFile(
   }
 
   return NS_ERROR_FAILURE;
 }
 
 NS_IMETHODIMP
 nsNSSCertificateDB::ImportPKCS12File(nsIFile* aFile)
 {
+  if (!NS_IsMainThread()) {
+    return NS_ERROR_NOT_SAME_THREAD;
+  }
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   NS_ENSURE_ARG(aFile);
   nsPKCS12Blob blob;
   return blob.ImportFromFile(aFile);
 }
 
 NS_IMETHODIMP
 nsNSSCertificateDB::ExportPKCS12File(nsIFile* aFile, uint32_t count,
                                      nsIX509Cert** certs)
 {
+  if (!NS_IsMainThread()) {
+    return NS_ERROR_NOT_SAME_THREAD;
+  }
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   NS_ENSURE_ARG(aFile);
   if (count == 0) {
     return NS_OK;
--- a/security/manager/ssl/nsNSSComponent.cpp
+++ b/security/manager/ssl/nsNSSComponent.cpp
@@ -2078,42 +2078,16 @@ nsNSSComponent::GetNewPrompter(nsIPrompt
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = wwatch->GetNewPrompter(0, result);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return rv;
 }
 
-/*static*/ nsresult
-nsNSSComponent::ShowAlertWithConstructedString(const nsString& message)
-{
-  nsCOMPtr<nsIPrompt> prompter;
-  nsresult rv = GetNewPrompter(getter_AddRefs(prompter));
-  if (prompter) {
-    rv = prompter->Alert(nullptr, message.get());
-  }
-  return rv;
-}
-
-NS_IMETHODIMP
-nsNSSComponent::ShowAlertFromStringBundle(const char* messageID)
-{
-  nsString message;
-  nsresult rv;
-
-  rv = GetPIPNSSBundleString(messageID, message);
-  if (NS_FAILED(rv)) {
-    NS_ERROR("GetPIPNSSBundleString failed");
-    return rv;
-  }
-
-  return ShowAlertWithConstructedString(message);
-}
-
 nsresult nsNSSComponent::LogoutAuthenticatedPK11()
 {
   nsCOMPtr<nsICertOverrideService> icos =
     do_GetService("@mozilla.org/security/certoverride;1");
   if (icos) {
     icos->ClearValidityOverride(
             NS_LITERAL_CSTRING("all:temporary-certificates"),
             0);
--- a/security/manager/ssl/nsNSSComponent.h
+++ b/security/manager/ssl/nsNSSComponent.h
@@ -62,18 +62,16 @@ extern bool EnsureNSSInitializedChromeOr
 
 extern bool EnsureNSSInitialized(EnsureNSSOperator op);
 
 class NS_NO_VTABLE nsINSSComponent : public nsISupports
 {
 public:
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_INSSCOMPONENT_IID)
 
-  NS_IMETHOD ShowAlertFromStringBundle(const char* messageID) = 0;
-
   NS_IMETHOD GetPIPNSSBundleString(const char* name,
                                    nsAString& outString) = 0;
   NS_IMETHOD PIPBundleFormatStringFromName(const char* name,
                                            const char16_t** params,
                                            uint32_t numParams,
                                            nsAString& outString) = 0;
 
   NS_IMETHOD GetNSSBundleString(const char* name,
@@ -117,18 +115,16 @@ public:
   nsNSSComponent();
 
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIOBSERVER
 
   nsresult Init();
 
   static nsresult GetNewPrompter(nsIPrompt** result);
-  static nsresult ShowAlertWithConstructedString(const nsString& message);
-  NS_IMETHOD ShowAlertFromStringBundle(const char* messageID) override;
 
   NS_IMETHOD GetPIPNSSBundleString(const char* name,
                                    nsAString& outString) override;
   NS_IMETHOD PIPBundleFormatStringFromName(const char* name,
                                            const char16_t** params,
                                            uint32_t numParams,
                                            nsAString& outString) override;
   NS_IMETHOD GetNSSBundleString(const char* name, nsAString& outString) override;
--- a/security/manager/ssl/nsPKCS12Blob.cpp
+++ b/security/manager/ssl/nsPKCS12Blob.cpp
@@ -21,18 +21,16 @@
 #include "prmem.h"
 #include "secerr.h"
 
 using namespace mozilla;
 extern LazyLogModule gPIPNSSLog;
 
 #define PIP_PKCS12_TMPFILENAME   NS_LITERAL_CSTRING(".pip_p12tmp")
 #define PIP_PKCS12_BUFFER_SIZE   2048
-#define PIP_PKCS12_RESTORE_OK          1
-#define PIP_PKCS12_BACKUP_OK           2
 #define PIP_PKCS12_USER_CANCELED       3
 #define PIP_PKCS12_NOSMARTCARD_EXPORT  4
 #define PIP_PKCS12_RESTORE_FAILED      5
 #define PIP_PKCS12_BACKUP_FAILED       6
 #define PIP_PKCS12_NSS_ERROR           7
 
 // constructor
 nsPKCS12Blob::nsPKCS12Blob()
@@ -129,17 +127,16 @@ nsPKCS12Blob::ImportFromFileHelper(nsIFi
   if (srv) goto finish;
   // validate bags
   srv = SEC_PKCS12DecoderValidateBags(dcx, nickname_collision);
   if (srv) goto finish;
   // import cert and key
   srv = SEC_PKCS12DecoderImportBags(dcx);
   if (srv) goto finish;
   // Later - check to see if this should become default email cert
-  handleError(PIP_PKCS12_RESTORE_OK);
 finish:
   // If srv != SECSuccess, NSS probably set a specific error code.
   // We should use that error code instead of inventing a new one
   // for every error possible.
   if (srv != SECSuccess) {
     if (SEC_ERROR_BAD_PASSWORD == PORT_GetError()) {
       if (unicodePw.len == sizeof(char16_t))
       {
@@ -300,17 +297,16 @@ nsPKCS12Blob::ExportToFile(nsIFile *file
     file = localFileRef;
   }
   rv = file->OpenNSPRFileDesc(PR_RDWR|PR_CREATE_FILE|PR_TRUNCATE, 0664, 
                               &mTmpFile);
   if (NS_FAILED(rv) || !this->mTmpFile) goto finish;
   // encode and write
   srv = SEC_PKCS12Encode(ecx, write_export_file, this);
   if (srv) goto finish;
-  handleError(PIP_PKCS12_BACKUP_OK);
 finish:
   if (NS_FAILED(rv) || srv != SECSuccess) {
     handleError(PIP_PKCS12_BACKUP_FAILED);
   }
   if (ecx)
     SEC_PKCS12DestroyExportContext(ecx);
   if (this->mTmpFile) {
     PR_Close(this->mTmpFile);
@@ -424,27 +420,25 @@ nsPKCS12Blob::inputToDecoder(SEC_PKCS12D
 }
 
 // nickname_collision
 // what to do when the nickname collides with one already in the db.
 // TODO: not handled, throw a dialog allowing the nick to be changed?
 SECItem *
 nsPKCS12Blob::nickname_collision(SECItem *oldNick, PRBool *cancel, void *wincx)
 {
-  static NS_DEFINE_CID(kNSSComponentCID, NS_NSSCOMPONENT_CID);
-
   nsNSSShutDownPreventionLock locker;
   *cancel = false;
-  nsresult rv;
-  nsCOMPtr<nsINSSComponent> nssComponent(do_GetService(kNSSComponentCID, &rv));
-  if (NS_FAILED(rv)) return nullptr;
   int count = 1;
   nsCString nickname;
   nsAutoString nickFromProp;
-  nssComponent->GetPIPNSSBundleString("P12DefaultNickname", nickFromProp);
+  nsresult rv = GetPIPNSSBundleString("P12DefaultNickname", nickFromProp);
+  if (NS_FAILED(rv)) {
+    return nullptr;
+  }
   NS_ConvertUTF16toUTF8 nickFromPropC(nickFromProp);
   // The user is trying to import a PKCS#12 file that doesn't have the
   // attribute we use to set the nickname.  So in order to reduce the
   // number of interactions we require with the user, we'll build a nickname
   // for the user.  The nickname isn't prominently displayed in the UI, 
   // so it's OK if we generate one on our own here.
   //   XXX If the NSS API were smarter and actually passed a pointer to
   //       the CERTCertificate* we're importing we could actually just
@@ -511,32 +505,28 @@ pip_ucs2_ascii_conversion_fn(PRBool toUn
   *outBufLen = inBufLen;
   memcpy(outBuf, inBuf, inBufLen);
   return true;
 }
 
 void
 nsPKCS12Blob::handleError(int myerr)
 {
-  static NS_DEFINE_CID(kNSSComponentCID, NS_NSSCOMPONENT_CID);
-
+  MOZ_ASSERT(NS_IsMainThread());
   if (!NS_IsMainThread()) {
-    NS_ERROR("nsPKCS12Blob::handleError called off the mai nthread.");
     return;
   }
 
   int prerr = PORT_GetError();
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("PKCS12: NSS/NSPR error(%d)", prerr));
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("PKCS12: I called(%d)", myerr));
 
   const char * msgID = nullptr;
 
   switch (myerr) {
-  case PIP_PKCS12_RESTORE_OK:       msgID = "SuccessfulP12Restore"; break;
-  case PIP_PKCS12_BACKUP_OK:        msgID = "SuccessfulP12Backup";  break;
   case PIP_PKCS12_USER_CANCELED:
     return;  /* Just ignore it for now */
   case PIP_PKCS12_NOSMARTCARD_EXPORT: msgID = "PKCS12InfoNoSmartcardBackup"; break;
   case PIP_PKCS12_RESTORE_FAILED:   msgID = "PKCS12UnknownErrRestore"; break;
   case PIP_PKCS12_BACKUP_FAILED:    msgID = "PKCS12UnknownErrBackup"; break;
   case PIP_PKCS12_NSS_ERROR:
     switch (prerr) {
     // The following errors have the potential to be "handled", by asking
@@ -558,13 +548,23 @@ nsPKCS12Blob::handleError(int myerr)
     case SEC_ERROR_PKCS12_DUPLICATE_DATA: msgID = "PKCS12DupData"; break;
     }
     break;
   }
 
   if (!msgID)
     msgID = "PKCS12UnknownErr";
 
-  nsresult rv;
-  nsCOMPtr<nsINSSComponent> nssComponent = do_GetService(kNSSComponentCID, &rv);
-  if (NS_SUCCEEDED(rv))
-    (void) nssComponent->ShowAlertFromStringBundle(msgID);
+  nsCOMPtr<nsIWindowWatcher> wwatch(do_GetService(NS_WINDOWWATCHER_CONTRACTID));
+  if (!wwatch) {
+    return;
+  }
+  nsCOMPtr<nsIPrompt> prompter;
+  if (NS_FAILED(wwatch->GetNewPrompter(nullptr, getter_AddRefs(prompter)))) {
+    return;
+  }
+  nsAutoString message;
+  if (NS_FAILED(GetPIPNSSBundleString(msgID, message))) {
+    return;
+  }
+
+  prompter->Alert(nullptr, message.get());
 }