Linux Cert manager: improve PKCS #12 import error messages.

BUG=76275
TEST=try to import a corrupt or unsupported PKCS #12 file.

Review URL: http://codereview.chromium.org/7338011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@92306 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 89 insertions, 14 deletions

diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index cabe5fa..8ee0cc2 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -3350,6 +3350,15 @@ are declared in build/common.gypi.
       <message name="IDS_CERT_MANAGER_PKCS12_IMPORT_ERROR_TITLE" desc="The title in the error dialog for PKCS #12 file import errors.">
         PKCS #12 Import Error
       </message>
+      <message name="IDS_CERT_MANAGER_PKCS12_IMPORT_INVALID_MAC" desc="The message in the error dialog for PKCS #12 files with invalid MAC.">
+        Incorrect password or corrupt file.
+      </message>
+      <message name="IDS_CERT_MANAGER_PKCS12_IMPORT_INVALID_FILE" desc="The message in the error dialog for corrupt PKCS #12 files.">
+        Invalid or corrupt file.
+      </message>
+      <message name="IDS_CERT_MANAGER_PKCS12_IMPORT_UNSUPPORTED" desc="The message in the error dialog for unsupported PKCS #12 files.">
+        File uses unsupported features.
+      </message>
       <message name="IDS_CERT_MANAGER_READ_ERROR_FORMAT" desc="The text in the error dialog for PKCS #12 file read errors.">
         There was an error while trying to read the file: <ph name="ERROR_TEXT">$1<ex>File not found.</ex></ph>.
       </message>
diff --git a/chrome/browser/ui/webui/options/certificate_manager_handler.cc b/chrome/browser/ui/webui/options/certificate_manager_handler.cc
index 7a29861..fb4fc93 100644
--- a/chrome/browser/ui/webui/options/certificate_manager_handler.cc
+++ b/chrome/browser/ui/webui/options/certificate_manager_handler.cc
@@ -678,22 +678,31 @@ void CertificateManagerHandler::ImportPersonalSlotUnlocked() {
       module_, file_data_, password_, is_extractable);
   ImportExportCleanup();
   web_ui_->CallJavascriptFunction("CertificateRestoreOverlay.dismiss");
+  int string_id;
   switch (result) {
     case net::OK:
-      break;
+      return;
     case net::ERR_PKCS12_IMPORT_BAD_PASSWORD:
-      ShowError(
-          l10n_util::GetStringUTF8(IDS_CERT_MANAGER_PKCS12_IMPORT_ERROR_TITLE),
-          l10n_util::GetStringUTF8(IDS_CERT_MANAGER_BAD_PASSWORD));
       // TODO(mattm): if the error was a bad password, we should reshow the
       // password dialog after the user dismisses the error dialog.
+      string_id = IDS_CERT_MANAGER_BAD_PASSWORD;
+      break;
+    case net::ERR_PKCS12_IMPORT_INVALID_MAC:
+      string_id = IDS_CERT_MANAGER_PKCS12_IMPORT_INVALID_MAC;
+      break;
+    case net::ERR_PKCS12_IMPORT_INVALID_FILE:
+      string_id = IDS_CERT_MANAGER_PKCS12_IMPORT_INVALID_FILE;
+      break;
+    case net::ERR_PKCS12_IMPORT_UNSUPPORTED:
+      string_id = IDS_CERT_MANAGER_PKCS12_IMPORT_UNSUPPORTED;
       break;
     default:
-      ShowError(
-          l10n_util::GetStringUTF8(IDS_CERT_MANAGER_PKCS12_IMPORT_ERROR_TITLE),
-          l10n_util::GetStringUTF8(IDS_CERT_MANAGER_UNKNOWN_ERROR));
+      string_id = IDS_CERT_MANAGER_UNKNOWN_ERROR;
       break;
   }
+  ShowError(
+      l10n_util::GetStringUTF8(IDS_CERT_MANAGER_PKCS12_IMPORT_ERROR_TITLE),
+      l10n_util::GetStringUTF8(string_id));
 }
 
 void CertificateManagerHandler::CancelImportExportProcess(
diff --git a/net/base/cert_database_nss_unittest.cc b/net/base/cert_database_nss_unittest.cc
index 70d4224..e6cf147 100644
--- a/net/base/cert_database_nss_unittest.cc
+++ b/net/base/cert_database_nss_unittest.cc
@@ -185,6 +185,24 @@ TEST_F(CertDatabaseNSSTest, ImportFromPKCS12AsExtractableAndExportAgain) {
   // TODO(mattm): further verification of exported data?
 }
 
+TEST_F(CertDatabaseNSSTest, ImportFromPKCS12Twice) {
+  std::string pkcs12_data = ReadTestFile("client.p12");
+
+  EXPECT_EQ(OK, cert_db_.ImportFromPKCS12(slot_,
+                                          pkcs12_data,
+                                          ASCIIToUTF16("12345"),
+                                          true));  // is_extractable
+  EXPECT_EQ(1U, ListCertsInSlot(slot_->os_module_handle()).size());
+
+  // NSS has a SEC_ERROR_PKCS12_DUPLICATE_DATA error, but it doesn't look like
+  // it's ever used.  This test verifies that.
+  EXPECT_EQ(OK, cert_db_.ImportFromPKCS12(slot_,
+                                          pkcs12_data,
+                                          ASCIIToUTF16("12345"),
+                                          true));  // is_extractable
+  EXPECT_EQ(1U, ListCertsInSlot(slot_->os_module_handle()).size());
+}
+
 TEST_F(CertDatabaseNSSTest, ImportFromPKCS12AsUnextractableAndExportAgain) {
   std::string pkcs12_data = ReadTestFile("client.p12");
 
@@ -205,6 +223,19 @@ TEST_F(CertDatabaseNSSTest, ImportFromPKCS12AsUnextractableAndExportAgain) {
                                        &exported_data));
 }
 
+TEST_F(CertDatabaseNSSTest, ImportFromPKCS12InvalidFile) {
+  std::string pkcs12_data = "Foobarbaz";
+
+  EXPECT_EQ(ERR_PKCS12_IMPORT_INVALID_FILE,
+            cert_db_.ImportFromPKCS12(slot_,
+                                      pkcs12_data,
+                                      ASCIIToUTF16(""),
+                                      true));  // is_extractable
+
+  // Test db should still be empty.
+  EXPECT_EQ(0U, ListCertsInSlot(slot_->os_module_handle()).size());
+}
+
 TEST_F(CertDatabaseNSSTest, ImportCACert_SSLTrust) {
   std::string cert_data = ReadTestFile("root_ca_cert.crt");
 
diff --git a/net/base/net_error_list.h b/net/base/net_error_list.h
index cfde9d8..80d770f 100644
--- a/net/base/net_error_list.h
+++ b/net/base/net_error_list.h
@@ -555,6 +555,15 @@ NET_ERROR(IMPORT_CA_CERT_FAILED, -705)
 // Server certificate import failed due to some internal error.
 NET_ERROR(IMPORT_SERVER_CERT_FAILED, -706)
 
+// PKCS #12 import failed due to invalid MAC.
+NET_ERROR(PKCS12_IMPORT_INVALID_MAC, -707)
+
+// PKCS #12 import failed due to invalid/corrupt file.
+NET_ERROR(PKCS12_IMPORT_INVALID_FILE, -708)
+
+// PKCS #12 import failed due to unsupported features.
+NET_ERROR(PKCS12_IMPORT_UNSUPPORTED, -709)
+
 // DNS error codes.
 
 // DNS resolver received a malformed response.
diff --git a/net/third_party/mozilla_security_manager/nsPKCS12Blob.cpp b/net/third_party/mozilla_security_manager/nsPKCS12Blob.cpp
index e314660..d659900 100644
--- a/net/third_party/mozilla_security_manager/nsPKCS12Blob.cpp
+++ b/net/third_party/mozilla_security_manager/nsPKCS12Blob.cpp
@@ -228,13 +228,30 @@ finish:
   // 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()) {
-      import_result = net::ERR_PKCS12_IMPORT_BAD_PASSWORD;
-    }
-    else
-    {
-      LOG(ERROR) << "PKCS#12 import failed with error " << PORT_GetError();
-      import_result = net::ERR_PKCS12_IMPORT_FAILED;
+    int error = PORT_GetError();
+    LOG(ERROR) << "PKCS#12 import failed with error " << error;
+    switch (error) {
+      case SEC_ERROR_BAD_PASSWORD:
+      case SEC_ERROR_PKCS12_PRIVACY_PASSWORD_INCORRECT:
+        import_result = net::ERR_PKCS12_IMPORT_BAD_PASSWORD;
+        break;
+      case SEC_ERROR_PKCS12_INVALID_MAC:
+        import_result = net::ERR_PKCS12_IMPORT_INVALID_MAC;
+        break;
+      case SEC_ERROR_BAD_DER:
+      case SEC_ERROR_PKCS12_DECODING_PFX:
+      case SEC_ERROR_PKCS12_CORRUPT_PFX_STRUCTURE:
+        import_result = net::ERR_PKCS12_IMPORT_INVALID_FILE;
+        break;
+      case SEC_ERROR_PKCS12_UNSUPPORTED_MAC_ALGORITHM:
+      case SEC_ERROR_PKCS12_UNSUPPORTED_TRANSPORT_MODE:
+      case SEC_ERROR_PKCS12_UNSUPPORTED_PBE_ALGORITHM:
+      case SEC_ERROR_PKCS12_UNSUPPORTED_VERSION:
+        import_result = net::ERR_PKCS12_IMPORT_UNSUPPORTED;
+        break;
+      default:
+        import_result = net::ERR_PKCS12_IMPORT_FAILED;
+        break;
     }
   }
   // Finish the decoder