Removing x-x509-user-cert mime handler.

chrome/browser/ui/webui/options/certificate_manager_handler.cc

Index: chrome/browser/ui/webui/options/certificate_manager_handler.cc
diff --git a/chrome/browser/ui/webui/options/certificate_manager_handler.cc b/chrome/browser/ui/webui/options/certificate_manager_handler.cc
index cf03f83b97bb1c5e7a01363601e52618cea1bb7f..3f451e5bacb84261c1487769596412b7d47fd257 100644
--- a/chrome/browser/ui/webui/options/certificate_manager_handler.cc
+++ b/chrome/browser/ui/webui/options/certificate_manager_handler.cc
@@ -29,6 +29,7 @@
 #include "content/public/browser/web_contents.h"
 #include "net/base/crypto_module.h"
 #include "net/base/net_errors.h"
+#include "net/cert/cert_database.h"
 #include "net/cert/x509_certificate.h"
 #include "ui/base/l10n/l10n_util.h"
 
@@ -709,8 +710,9 @@ void CertificateManagerHandler::StartImportPersonal(
   }
   file_type_info.extensions.resize(1);
   file_type_info.extensions[0].push_back(FILE_PATH_LITERAL("p12"));
+  file_type_info.extensions[0].push_back(FILE_PATH_LITERAL("crt"));
   file_type_info.extension_description_overrides.push_back(
-      l10n_util::GetStringUTF16(IDS_CERT_MANAGER_PKCS12_FILES));
+      l10n_util::GetStringUTF16(IDS_CERT_USAGE_SSL_CLIENT));
   file_type_info.include_all_files = true;
   select_file_dialog_ = ui::SelectFileDialog::Create(
       this, new ChromeSelectFilePolicy(web_ui()->GetWebContents()));
@@ -724,8 +726,17 @@ void CertificateManagerHandler::StartImportPersonal(
 void CertificateManagerHandler::ImportPersonalFileSelected(
     const base::FilePath& path) {
   file_path_ = path;
-  web_ui()->CallJavascriptFunction(
-      "CertificateManager.importPersonalAskPassword");
+  if (file_path_.MatchesExtension(FILE_PATH_LITERAL(".p12"))) {
+    web_ui()->CallJavascriptFunction(
+        "CertificateManager.importPersonalAskPassword");

[Ryan Sleevi, 2015/10/30 23:33:46]

Blergh; this is bugged even for .p12; there's no guarantee that a .p12 will be
password protected.

[svaldez, 2015/11/02 16:27:38]

It looked like it worked correctly if you just entered a blank password
previously.

+  } else {
+    password_.clear();
+    file_access_provider_->StartRead(
+        file_path_,
+        base::Bind(&CertificateManagerHandler::ImportPersonalFileRead,
+                   base::Unretained(this)),
+        &tracker_);
+  }
 }
 
 void CertificateManagerHandler::ImportPersonalPasswordSelected(
@@ -757,21 +768,39 @@ void CertificateManagerHandler::ImportPersonalFileRead(
 
   file_data_ = *data;
 
-  if (use_hardware_backed_) {
-    module_ = certificate_manager_model_->cert_db()->GetPrivateModule();
+  if (file_path_.MatchesExtension(FILE_PATH_LITERAL(".p12"))) {

[Ryan Sleevi, 2015/10/30 23:33:47]

Again, no guarantee that .p12 == private

[svaldez, 2015/11/02 16:27:38]

Can't we assume as much since this is coming in from the user cert import
codepath, and if we fail to import the cert, then we just error out?

+    if (use_hardware_backed_) {
+      module_ = certificate_manager_model_->cert_db()->GetPrivateModule();
+    } else {
+      module_ = certificate_manager_model_->cert_db()->GetPublicModule();
+    }
+
+    net::CryptoModuleList modules;
+    modules.push_back(module_);
+    chrome::UnlockSlotsIfNecessary(
+        modules,
+        chrome::kCryptoModulePasswordCertImport,
+        net::HostPortPair(),  // unused.
+        GetParentWindow(),
+        base::Bind(&CertificateManagerHandler::ImportPersonalSlotUnlocked,
+                   base::Unretained(this)));
   } else {
-    module_ = certificate_manager_model_->cert_db()->GetPublicModule();
-  }
+    scoped_refptr<net::X509Certificate> cert =
+        net::X509Certificate::CreateFromBytes(data->c_str(), data->size());

[Ryan Sleevi, 2015/10/30 23:33:47]

BUG: Part of the goal of this change was to support multiple certificate types,
much like .p12 can. There's no guarantee that |cert| will be a singular
certificate (it could be a chain in a variety of formats)

[svaldez, 2015/11/02 16:27:37]

Done.

 
-  net::CryptoModuleList modules;
-  modules.push_back(module_);
-  chrome::UnlockSlotsIfNecessary(
-      modules,
-      chrome::kCryptoModulePasswordCertImport,
-      net::HostPortPair(),  // unused.
-      GetParentWindow(),
-      base::Bind(&CertificateManagerHandler::ImportPersonalSlotUnlocked,
-                 base::Unretained(this)));
+    int cert_error =
+        net::CertDatabase::GetInstance()->CheckUserCert(cert.get());

[Ryan Sleevi, 2015/10/30 23:33:47]

net::CertDatabase is pretty awful; ideally, we'll be able to move away from it
as much as possible.

We already have a net::NSSCertDatabase (cert_db_), via the
certificate_manager_model_, ideally we should be importing certificates against
that. Even if we don't, we want the model to be responsible for this sort of
logic, I think...

There's also no (strict) need to require an associated private key.

[svaldez, 2015/11/02 16:27:38]

We'd need to modify the mozilla_security_manager to support this case, since
there currently doesn't exist an API to import a bare certificate, without also
providing the private key. Also, by spec, it looks like we are required to throw
an error on a missing private key.

+    if (cert_error == net::OK)
+      cert_error = net::CertDatabase::GetInstance()->AddUserCert(cert.get());
+
+    ImportExportCleanup();
+    web_ui()->CallJavascriptFunction("CertificateRestoreOverlay.dismiss");
+    if (cert_error != net::OK) {

[Ryan Sleevi, 2015/10/30 23:33:47]

BUG: You shouldn't assume that *any* error indicates a missing private key.
net::ERR_NO_PRIVATE_KEY_FOR_CERT is that error, but you can have other errors.

[svaldez, 2015/11/02 16:27:37]

Done.

+      ShowError(
+          l10n_util::GetStringUTF8(IDS_CERT_MANAGER_PKCS12_IMPORT_ERROR_TITLE),
+          l10n_util::GetStringUTF8(IDS_CERT_MANAGER_IMPORT_MISSING_KEY));
+    }
+  }
 }
 
 void CertificateManagerHandler::ImportPersonalSlotUnlocked() {