From 4c4f7cdb9dfef25e00a3f03f2311f22bacfaef39 Mon Sep 17 00:00:00 2001 From: "mattm@chromium.org" Date: Sat, 5 Mar 2011 02:20:44 +0000 Subject: NSS: Unlock crypto devices when populating cert manager. BUG=42073 TEST=try to use cert manager with "unfriendly" device. Review URL: http://codereview.chromium.org/6580058 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@77024 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/app/generated_resources.grd | 3 + chrome/browser/ui/crypto_module_password_dialog.h | 18 +++--- .../ui/crypto_module_password_dialog_nss.cc | 70 ++++++++++++++-------- .../ui/crypto_module_password_dialog_openssl.cc | 8 +-- .../ui/gtk/crypto_module_password_dialog.cc | 4 ++ .../webui/options/certificate_manager_handler.cc | 9 ++- content/browser/certificate_manager_model.cc | 15 +++++ content/browser/certificate_manager_model.h | 11 +++- net/base/cert_database.h | 5 ++ net/base/cert_database_nss.cc | 24 ++++++++ net/base/cert_database_openssl.cc | 6 ++ 11 files changed, 130 insertions(+), 43 deletions(-) diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index f272f03..2991709 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -6436,6 +6436,9 @@ Keep your key file in a safe place. You will need it to create new versions of y Please sign in to $1Software Security Device to authenticate to $2www.google.com with your certificate. + + Please sign in to $1Software Security Device. + Please sign in to $1Software Security Device to import the client certificate. diff --git a/chrome/browser/ui/crypto_module_password_dialog.h b/chrome/browser/ui/crypto_module_password_dialog.h index a6d9052..199120e 100644 --- a/chrome/browser/ui/crypto_module_password_dialog.h +++ b/chrome/browser/ui/crypto_module_password_dialog.h @@ -7,8 +7,10 @@ #pragma once #include +#include #include "base/callback.h" +#include "base/ref_counted.h" namespace base { class CryptoModuleBlockingPasswordDelegate; @@ -16,6 +18,7 @@ class CryptoModuleBlockingPasswordDelegate; namespace net { class CryptoModule; +typedef std::vector > CryptoModuleList; class X509Certificate; } @@ -26,6 +29,7 @@ enum CryptoModulePasswordReason { kCryptoModulePasswordKeygen, kCryptoModulePasswordCertEnrollment, kCryptoModulePasswordClientAuth, + kCryptoModulePasswordListCerts, kCryptoModulePasswordCertImport, kCryptoModulePasswordCertExport, }; @@ -49,13 +53,13 @@ base::CryptoModuleBlockingPasswordDelegate* CryptoModulePasswordReason reason, const std::string& server); -// Asynchronously unlock |module|, if necessary. |callback| is called when done -// (regardless if module was successfully unlocked or not). Should only be -// called on UI thread. -void UnlockSlotIfNecessary(net::CryptoModule* module, - browser::CryptoModulePasswordReason reason, - const std::string& server, - Callback0::Type* callback); +// Asynchronously unlock |modules|, if necessary. |callback| is called when +// done (regardless if any modules were successfully unlocked or not). Should +// only be called on UI thread. +void UnlockSlotsIfNecessary(const net::CryptoModuleList& modules, + browser::CryptoModulePasswordReason reason, + const std::string& server, + Callback0::Type* callback); // Asynchronously unlock the |cert|'s module, if necessary. |callback| is // called when done (regardless if module was successfully unlocked or not). diff --git a/chrome/browser/ui/crypto_module_password_dialog_nss.cc b/chrome/browser/ui/crypto_module_password_dialog_nss.cc index e2b6bb3..44bf47b 100644 --- a/chrome/browser/ui/crypto_module_password_dialog_nss.cc +++ b/chrome/browser/ui/crypto_module_password_dialog_nss.cc @@ -13,12 +13,18 @@ namespace { +bool ShouldShowDialog(const net::CryptoModule* module) { + // The wincx arg is unused since we don't call PK11_SetIsLoggedInFunc. + return (PK11_NeedLogin(module->os_module_handle()) && + !PK11_IsLoggedIn(module->os_module_handle(), NULL /* wincx */)); +} + // Basically an asynchronous implementation of NSS's PK11_DoPassword. // Note: This currently handles only the simple case. See the TODOs in // GotPassword for what is yet unimplemented. class SlotUnlocker { public: - SlotUnlocker(net::CryptoModule* module, + SlotUnlocker(const net::CryptoModuleList& modules, browser::CryptoModulePasswordReason reason, const std::string& host, Callback0::Type* callback); @@ -29,18 +35,20 @@ class SlotUnlocker { void GotPassword(const char* password); void Done(); - scoped_refptr module_; + size_t current_; + net::CryptoModuleList modules_; browser::CryptoModulePasswordReason reason_; std::string host_; Callback0::Type* callback_; PRBool retry_; }; -SlotUnlocker::SlotUnlocker(net::CryptoModule* module, +SlotUnlocker::SlotUnlocker(const net::CryptoModuleList& modules, browser::CryptoModulePasswordReason reason, const std::string& host, Callback0::Type* callback) - : module_(module), + : current_(0), + modules_(modules), reason_(reason), host_(host), callback_(callback), @@ -51,12 +59,18 @@ SlotUnlocker::SlotUnlocker(net::CryptoModule* module, void SlotUnlocker::Start() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - ShowCryptoModulePasswordDialog( - module_->GetTokenName(), - retry_, - reason_, - host_, - NewCallback(this, &SlotUnlocker::GotPassword)); + for (; current_ < modules_.size(); ++current_) { + if (ShouldShowDialog(modules_[current_].get())) { + ShowCryptoModulePasswordDialog( + modules_[current_]->GetTokenName(), + retry_, + reason_, + host_, + NewCallback(this, &SlotUnlocker::GotPassword)); + return; + } + } + Done(); } void SlotUnlocker::GotPassword(const char* password) { @@ -66,12 +80,13 @@ void SlotUnlocker::GotPassword(const char* password) { if (!password) { // User cancelled entering password. Oh well. - Done(); + ++current_; + Start(); return; } // TODO(mattm): handle protectedAuthPath - SECStatus rv = PK11_CheckUserPassword(module_->os_module_handle(), + SECStatus rv = PK11_CheckUserPassword(modules_[current_]->os_module_handle(), password); if (rv == SECWouldBlock) { // Incorrect password. Try again. @@ -84,11 +99,13 @@ void SlotUnlocker::GotPassword(const char* password) { // non-friendly slots. How important is that? // Correct password (SECSuccess) or too many attempts/other failure - // (SECFailure). Either way we're done. - Done(); + // (SECFailure). Either way we're done with this slot. + ++current_; + Start(); } void SlotUnlocker::Done() { + DCHECK_EQ(current_, modules_.size()); callback_->Run(); delete this; } @@ -97,27 +114,28 @@ void SlotUnlocker::Done() { namespace browser { -void UnlockSlotIfNecessary(net::CryptoModule* module, - browser::CryptoModulePasswordReason reason, - const std::string& host, - Callback0::Type* callback) { +void UnlockSlotsIfNecessary(const net::CryptoModuleList& modules, + browser::CryptoModulePasswordReason reason, + const std::string& host, + Callback0::Type* callback) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - // The wincx arg is unused since we don't call PK11_SetIsLoggedInFunc. - if (PK11_NeedLogin(module->os_module_handle()) && - !PK11_IsLoggedIn(module->os_module_handle(), NULL /* wincx */)) { - (new SlotUnlocker(module, reason, host, callback))->Start(); - } else { - callback->Run(); + for (size_t i = 0; i < modules.size(); ++i) { + if (ShouldShowDialog(modules[i].get())) { + (new SlotUnlocker(modules, reason, host, callback))->Start(); + return; + } } + callback->Run(); } void UnlockCertSlotIfNecessary(net::X509Certificate* cert, browser::CryptoModulePasswordReason reason, const std::string& host, Callback0::Type* callback) { - scoped_refptr module(net::CryptoModule::CreateFromHandle( + net::CryptoModuleList modules; + modules.push_back(net::CryptoModule::CreateFromHandle( cert->os_cert_handle()->slot)); - UnlockSlotIfNecessary(module.get(), reason, host, callback); + UnlockSlotsIfNecessary(modules, reason, host, callback); } } // namespace browser diff --git a/chrome/browser/ui/crypto_module_password_dialog_openssl.cc b/chrome/browser/ui/crypto_module_password_dialog_openssl.cc index b5c40c5..5de2d24 100644 --- a/chrome/browser/ui/crypto_module_password_dialog_openssl.cc +++ b/chrome/browser/ui/crypto_module_password_dialog_openssl.cc @@ -8,10 +8,10 @@ namespace browser { -void UnlockSlotIfNecessary(net::CryptoModule* module, - browser::CryptoModulePasswordReason reason, - const std::string& host, - Callback0::Type* callback) { +void UnlockSlotsIfNecessary(const net::CryptoModuleList& modules, + browser::CryptoModulePasswordReason reason, + const std::string& host, + Callback0::Type* callback) { // TODO(bulach): implement me. NOTREACHED(); } diff --git a/chrome/browser/ui/gtk/crypto_module_password_dialog.cc b/chrome/browser/ui/gtk/crypto_module_password_dialog.cc index 9c7c1d0..5b7f51b 100644 --- a/chrome/browser/ui/gtk/crypto_module_password_dialog.cc +++ b/chrome/browser/ui/gtk/crypto_module_password_dialog.cc @@ -140,6 +140,10 @@ CryptoModulePasswordDialog::CryptoModulePasswordDialog( text = l10n_util::GetStringFUTF8( IDS_CRYPTO_MODULE_AUTH_DIALOG_TEXT_CLIENT_AUTH, slot16, server16); break; + case browser::kCryptoModulePasswordListCerts: + text = l10n_util::GetStringFUTF8( + IDS_CRYPTO_MODULE_AUTH_DIALOG_TEXT_LIST_CERTS, slot16); + break; case browser::kCryptoModulePasswordCertImport: text = l10n_util::GetStringFUTF8( IDS_CRYPTO_MODULE_AUTH_DIALOG_TEXT_CERT_IMPORT, slot16); diff --git a/chrome/browser/ui/webui/options/certificate_manager_handler.cc b/chrome/browser/ui/webui/options/certificate_manager_handler.cc index bfc730d..f641d34 100644 --- a/chrome/browser/ui/webui/options/certificate_manager_handler.cc +++ b/chrome/browser/ui/webui/options/certificate_manager_handler.cc @@ -540,7 +540,8 @@ void CertificateManagerHandler::ExportPersonalPasswordSelected( } // Currently, we don't support exporting more than one at a time. If we do, - // this would need some cleanup to handle unlocking multiple slots. + // this would need to either change this to use UnlockSlotsIfNecessary or + // change UnlockCertSlotIfNecessary to take a CertificateList. DCHECK_EQ(selected_cert_list_.size(), 1U); // TODO(mattm): do something smarter about non-extractable keys @@ -637,8 +638,10 @@ void CertificateManagerHandler::ImportPersonalFileRead( // TODO(mattm): allow user to choose a slot to import to. module_ = certificate_manager_model_->cert_db().GetDefaultModule(); - browser::UnlockSlotIfNecessary( - module_.get(), + net::CryptoModuleList modules; + modules.push_back(module_); + browser::UnlockSlotsIfNecessary( + modules, browser::kCryptoModulePasswordCertImport, "", // unused. NewCallback(this, diff --git a/content/browser/certificate_manager_model.cc b/content/browser/certificate_manager_model.cc index c1660b6..e7b1daf 100644 --- a/content/browser/certificate_manager_model.cc +++ b/content/browser/certificate_manager_model.cc @@ -7,7 +7,9 @@ #include "base/i18n/time_formatting.h" #include "base/logging.h" #include "base/utf_string_conversions.h" +#include "chrome/browser/ui/crypto_module_password_dialog.h" #include "chrome/common/net/x509_certificate_model.h" +#include "net/base/crypto_module.h" #include "net/base/net_errors.h" #include "net/base/x509_certificate.h" @@ -20,6 +22,19 @@ CertificateManagerModel::~CertificateManagerModel() { void CertificateManagerModel::Refresh() { VLOG(1) << "refresh started"; + net::CryptoModuleList modules; + cert_db_.ListModules(&modules, false); + VLOG(1) << "refresh waiting for unlocking..."; + browser::UnlockSlotsIfNecessary( + modules, + browser::kCryptoModulePasswordListCerts, + "", // unused. + NewCallback(this, + &CertificateManagerModel::RefreshSlotsUnlocked)); +} + +void CertificateManagerModel::RefreshSlotsUnlocked() { + VLOG(1) << "refresh listing certs..."; cert_db_.ListCerts(&cert_list_); observer_->CertificatesRefreshed(); VLOG(1) << "refresh finished"; diff --git a/content/browser/certificate_manager_model.h b/content/browser/certificate_manager_model.h index f134a68..38be1e4 100644 --- a/content/browser/certificate_manager_model.h +++ b/content/browser/certificate_manager_model.h @@ -43,9 +43,10 @@ class CertificateManagerModel { // Accessor for read-only access to the underlying CertDatabase. const net::CertDatabase& cert_db() const { return cert_db_; } - // Refresh the list of certs. Following this call, the observer - // CertificatesRefreshed method will be called so the view can call - // FilterAndBuildOrgGroupingMap as necessary to refresh its tree views. + // Trigger a refresh of the list of certs, unlock any slots if necessary. + // Following this call, the observer CertificatesRefreshed method will be + // called so the view can call FilterAndBuildOrgGroupingMap as necessary to + // refresh its tree views. void Refresh(); // Fill |map| with the certificates matching |filter_type|. @@ -98,6 +99,10 @@ class CertificateManagerModel { bool Delete(net::X509Certificate* cert); private: + // Callback used by Refresh() for when the cert slots have been unlocked. + // This method does the actual refreshing. + void RefreshSlotsUnlocked(); + net::CertDatabase cert_db_; net::CertificateList cert_list_; diff --git a/net/base/cert_database.h b/net/base/cert_database.h index 69290ab..0e7bdbd 100644 --- a/net/base/cert_database.h +++ b/net/base/cert_database.h @@ -17,6 +17,7 @@ namespace net { class CryptoModule; +typedef std::vector > CryptoModuleList; class X509Certificate; typedef std::vector > CertificateList; @@ -77,6 +78,10 @@ class CertDatabase { // The returned pointer must be stored in a scoped_refptr. CryptoModule* GetDefaultModule() const; + // Get all modules. + // If |need_rw| is true, only writable modules will be returned. + void ListModules(CryptoModuleList* modules, bool need_rw) const; + // Import certificates and private keys from PKCS #12 blob into the module. // Returns OK or a network error code such as ERR_PKCS12_IMPORT_BAD_PASSWORD // or ERR_PKCS12_IMPORT_ERROR. diff --git a/net/base/cert_database_nss.cc b/net/base/cert_database_nss.cc index db2c47a..7e1abaa9 100644 --- a/net/base/cert_database_nss.cc +++ b/net/base/cert_database_nss.cc @@ -118,6 +118,30 @@ CryptoModule* CertDatabase::GetDefaultModule() const { return module; } +void CertDatabase::ListModules(CryptoModuleList* modules, bool need_rw) const { + modules->clear(); + + PK11SlotList* slot_list = NULL; + // The wincx arg is unused since we don't call PK11_SetIsLoggedInFunc. + slot_list = PK11_GetAllTokens(CKM_INVALID_MECHANISM, + need_rw ? PR_TRUE : PR_FALSE, // needRW + PR_TRUE, // loadCerts (unused) + NULL); // wincx + if (!slot_list) { + LOG(ERROR) << "PK11_GetAllTokens failed: " << PORT_GetError(); + return; + } + + PK11SlotListElement* slot_element = PK11_GetFirstSafe(slot_list); + while (slot_element) { + modules->push_back(CryptoModule::CreateFromHandle(slot_element->slot)); + slot_element = PK11_GetNextSafe(slot_list, slot_element, + PR_FALSE); // restart + } + + PK11_FreeSlotList(slot_list); +} + int CertDatabase::ImportFromPKCS12( net::CryptoModule* module, const std::string& data, diff --git a/net/base/cert_database_openssl.cc b/net/base/cert_database_openssl.cc index 51a2308..cc287b5 100644 --- a/net/base/cert_database_openssl.cc +++ b/net/base/cert_database_openssl.cc @@ -46,6 +46,12 @@ CryptoModule* CertDatabase::GetDefaultModule() const { return NULL; } +void CertDatabase::ListModules(CryptoModuleList* modules, bool need_rw) const { + // TODO(bulach): implement me. + NOTIMPLEMENTED(); + modules->clear(); +} + int CertDatabase::ImportFromPKCS12(net::CryptoModule* module, const std::string& data, const string16& password) { -- cgit v1.1