Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2722)

Unified Diff: chrome/browser/ui/views/ssl_client_certificate_selector.cc

Issue 2898573002: Refactor client cert private key handling. (Closed)
Patch Set: removed no longer needed forward declaration Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/ui/views/ssl_client_certificate_selector.cc
diff --git a/chrome/browser/ui/views/ssl_client_certificate_selector.cc b/chrome/browser/ui/views/ssl_client_certificate_selector.cc
index 0176e6528f177ad45408e4c6c9bbc9f6c31db040..421a622654b50e0a87aca2389edd236dbe7af610 100644
--- a/chrome/browser/ui/views/ssl_client_certificate_selector.cc
+++ b/chrome/browser/ui/views/ssl_client_certificate_selector.cc
@@ -10,31 +10,84 @@
#include "base/bind_helpers.h"
#include "base/strings/utf_string_conversions.h"
#include "build/build_config.h"
+#include "chrome/browser/ssl/ssl_client_auth_observer.h"
#include "chrome/browser/ui/browser_dialogs.h"
#include "chrome/grit/generated_resources.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/client_certificate_delegate.h"
#include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_observer.h"
#include "net/cert/x509_certificate.h"
#include "net/ssl/ssl_cert_request_info.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/views/controls/label.h"
#include "ui/views/widget/widget.h"
-#if defined(USE_NSS_CERTS) && !defined(OS_CHROMEOS)
-#include "chrome/browser/ui/crypto_module_password_dialog_nss.h"
-#endif
+class SSLClientCertificateSelector::SSLClientAuthObserverImpl
+ : public SSLClientAuthObserver,
+ public content::WebContentsObserver {
+ public:
+ SSLClientAuthObserverImpl(
+ content::WebContents* web_contents,
+ const scoped_refptr<net::SSLCertRequestInfo>& cert_request_info,
+ std::unique_ptr<content::ClientCertificateDelegate> delegate)
+ : SSLClientAuthObserver(web_contents->GetBrowserContext(),
+ cert_request_info,
+ std::move(delegate)),
+ content::WebContentsObserver(web_contents) {}
+
+ void Init(base::OnceClosure close_dialog_callback) {
+ close_dialog_callback_ = std::move(close_dialog_callback);
+ StartObserving();
+ }
+
+ static void AcceptCertificate(
+ std::unique_ptr<SSLClientAuthObserverImpl> self,
+ std::unique_ptr<net::ClientCertIdentity> identity) {
+ // Remove the observer before we try acquiring private key, otherwise we
+ // might act on a notification while waiting for the callback, causing us
+ // to delete ourself before the callback gets called, or to try to run
+ // |close_dialog_callback_| on a dialog which is already closed.
+ self->StopObserving();
+ net::X509Certificate* cert = identity->certificate();
+ net::ClientCertIdentity::SelfOwningAcquirePrivateKey(
+ std::move(identity),
+ base::Bind(&SSLClientAuthObserverImpl::GotPrivateKey,
+ base::Passed(&self), base::Unretained(cert)));
+ }
+
+ void GotPrivateKey(net::X509Certificate* cert,
+ scoped_refptr<net::SSLPrivateKey> private_key) {
+ CertificateSelected(cert, private_key.get());
+ }
+
+ // SSLClientAuthObserver:
+ void OnCertSelectedByNotification() override {
+ std::move(close_dialog_callback_).Run();
+ }
+
+ // content::WebContentsObserver:
+ void WebContentsDestroyed() override {
+ // If the tab is closed (either while the selector dialog is still showing,
+ // or after the dialog has closed but the AcquirePrivateKey callback is
+ // still pending), abort the request.
+ CancelCertificateSelection();
+ }
+
+ private:
+ base::OnceClosure close_dialog_callback_;
+};
SSLClientCertificateSelector::SSLClientCertificateSelector(
content::WebContents* web_contents,
const scoped_refptr<net::SSLCertRequestInfo>& cert_request_info,
- net::CertificateList client_certs,
+ net::ClientCertIdentityList client_certs,
std::unique_ptr<content::ClientCertificateDelegate> delegate)
: CertificateSelector(std::move(client_certs), web_contents),
- SSLClientAuthObserver(web_contents->GetBrowserContext(),
- cert_request_info,
- std::move(delegate)),
- WebContentsObserver(web_contents) {
+ auth_observer_impl_(
+ base::MakeUnique<SSLClientAuthObserverImpl>(web_contents,
+ cert_request_info,
+ std::move(delegate))) {
chrome::RecordDialogCreation(
chrome::DialogIdentifier::SSL_CLIENT_CERTIFICATE_SELECTOR);
}
@@ -42,11 +95,13 @@ SSLClientCertificateSelector::SSLClientCertificateSelector(
SSLClientCertificateSelector::~SSLClientCertificateSelector() {}
void SSLClientCertificateSelector::Init() {
- StartObserving();
+ auth_observer_impl_->Init(base::BindOnce(
+ &SSLClientCertificateSelector::CloseDialog, base::Unretained(this)));
std::unique_ptr<views::Label> text_label(
new views::Label(l10n_util::GetStringFUTF16(
IDS_CLIENT_CERT_DIALOG_TEXT,
- base::ASCIIToUTF16(cert_request_info()->host_and_port.ToString()))));
+ base::ASCIIToUTF16(auth_observer_impl_->cert_request_info()
+ ->host_and_port.ToString()))));
text_label->SetMultiLine(true);
text_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
text_label->SetAllowCharacterBreak(true);
@@ -54,7 +109,7 @@ void SSLClientCertificateSelector::Init() {
InitWithText(std::move(text_label));
}
-void SSLClientCertificateSelector::OnCertSelectedByNotification() {
+void SSLClientCertificateSelector::CloseDialog() {
GetWidget()->Close();
}
@@ -63,40 +118,18 @@ void SSLClientCertificateSelector::DeleteDelegate() {
// to abort instead of proceeding with a null certificate. (This will be
// ignored if there was a previous call to CertificateSelected or
// CancelCertificateSelection.)
- CertificateSelected(nullptr);
+ if (auth_observer_impl_)
+ auth_observer_impl_->CertificateSelected(nullptr, nullptr);
chrome::CertificateSelector::DeleteDelegate();
}
-bool SSLClientCertificateSelector::Accept() {
- scoped_refptr<net::X509Certificate> cert = GetSelectedCert();
- if (cert.get()) {
- // Remove the observer before we try unlocking, otherwise we might act on a
- // notification while waiting for the unlock dialog, causing us to delete
- // ourself before the Unlocked callback gets called.
- StopObserving();
-#if defined(USE_NSS_CERTS) && !defined(OS_CHROMEOS)
- chrome::UnlockCertSlotIfNecessary(
- cert.get(), chrome::kCryptoModulePasswordClientAuth,
- cert_request_info()->host_and_port, GetWidget()->GetNativeView(),
- base::Bind(&SSLClientCertificateSelector::Unlocked,
- base::Unretained(this), base::RetainedRef(cert)));
-#else
- Unlocked(cert.get());
-#endif
- return false; // Unlocked() will close the dialog.
- }
-
- return false;
-}
-
-void SSLClientCertificateSelector::WebContentsDestroyed() {
- // If the dialog is closed by closing the containing tab, abort the request.
- CancelCertificateSelection();
-}
-
-void SSLClientCertificateSelector::Unlocked(net::X509Certificate* cert) {
- CertificateSelected(cert);
- GetWidget()->Close();
+void SSLClientCertificateSelector::AcceptCertificate(
+ std::unique_ptr<net::ClientCertIdentity> identity) {
+ // The SSLClientCertificateSelector will be destroyed after this method
+ // returns, so the SSLClientAuthObserverImpl manages its own lifetime while
+ // acquiring the private key from |identity|.
+ SSLClientAuthObserverImpl::AcceptCertificate(std::move(auth_observer_impl_),
+ std::move(identity));
}
namespace chrome {
@@ -104,7 +137,7 @@ namespace chrome {
void ShowSSLClientCertificateSelector(
content::WebContents* contents,
net::SSLCertRequestInfo* cert_request_info,
- net::CertificateList client_certs,
+ net::ClientCertIdentityList client_certs,
std::unique_ptr<content::ClientCertificateDelegate> delegate) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

Powered by Google App Engine
This is Rietveld 408576698