Cancel client auth requests when not promptable.
Currently they hang (holding the cache lock) or crash. This plumbs
through a dedicated delegate interface. If the delegate is destroyed with
no notification, the request is aborted. This is distinct from
affirmatively continuing with no certificat (what you get from pressing
cancel). This is extremely bizarre UI, but this CL does not attempt to
address the existing UI being odd.
This fixes the following:
- Closing a tab with a client auth prompt acts as if you affirmatively selected
to continue without a cert.
- A SharedWorker requesting client auth hangs the request.
- Hitting client auth in an extension background page crashes.
BUG=417092,410967
Review URL: https://codereview.chromium.org/859213006
Cr-Commit-Position: refs/heads/master@{#320117}
diff --git a/android_webview/browser/aw_content_browser_client.cc b/android_webview/browser/aw_content_browser_client.cc
index fb89fc6f..a4db9971 100644
--- a/android_webview/browser/aw_content_browser_client.cc
+++ b/android_webview/browser/aw_content_browser_client.cc
@@ -28,6 +28,7 @@
#include "content/public/browser/browser_message_filter.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/child_process_security_policy.h"
+#include "content/public/browser/client_certificate_delegate.h"
#include "content/public/browser/permission_type.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_view_host.h"
@@ -368,17 +369,13 @@
}
void AwContentBrowserClient::SelectClientCertificate(
- int render_process_id,
- int render_frame_id,
- net::SSLCertRequestInfo* cert_request_info,
- const base::Callback<void(net::X509Certificate*)>& callback) {
+ content::WebContents* web_contents,
+ net::SSLCertRequestInfo* cert_request_info,
+ scoped_ptr<content::ClientCertificateDelegate> delegate) {
AwContentsClientBridgeBase* client =
- AwContentsClientBridgeBase::FromID(render_process_id, render_frame_id);
- if (client) {
- client->SelectClientCertificate(cert_request_info, callback);
- } else {
- callback.Run(NULL);
- }
+ AwContentsClientBridgeBase::FromWebContents(web_contents);
+ if (client)
+ client->SelectClientCertificate(cert_request_info, delegate.Pass());
}
void AwContentBrowserClient::RequestPermission(
diff --git a/android_webview/browser/aw_content_browser_client.h b/android_webview/browser/aw_content_browser_client.h
index cae1bf3..c61c2c48 100644
--- a/android_webview/browser/aw_content_browser_client.h
+++ b/android_webview/browser/aw_content_browser_client.h
@@ -102,10 +102,9 @@
const base::Callback<void(bool)>& callback,
content::CertificateRequestResultType* result) override;
void SelectClientCertificate(
- int render_process_id,
- int render_frame_id,
+ content::WebContents* web_contents,
net::SSLCertRequestInfo* cert_request_info,
- const base::Callback<void(net::X509Certificate*)>& callback) override;
+ scoped_ptr<content::ClientCertificateDelegate> delegate) override;
void RequestPermission(
content::PermissionType permission,
content::WebContents* web_contents,
diff --git a/android_webview/browser/aw_contents_client_bridge_base.h b/android_webview/browser/aw_contents_client_bridge_base.h
index a24aa4b..265004eb 100644
--- a/android_webview/browser/aw_contents_client_bridge_base.h
+++ b/android_webview/browser/aw_contents_client_bridge_base.h
@@ -5,13 +5,14 @@
#ifndef ANDROID_WEBVIEW_BROWSER_AW_CONTENTS_CLIENT_BRIDGE_BASE_H_
#define ANDROID_WEBVIEW_BROWSER_AW_CONTENTS_CLIENT_BRIDGE_BASE_H_
-#include "base/callback_forward.h"
+#include "base/memory/scoped_ptr.h"
#include "base/supports_user_data.h"
#include "content/public/browser/javascript_dialog_manager.h"
class GURL;
namespace content {
+class ClientCertificateDelegate;
class WebContents;
}
@@ -29,8 +30,6 @@
// native/ from browser/ layer.
class AwContentsClientBridgeBase {
public:
- typedef base::Callback<void(net::X509Certificate*)> SelectCertificateCallback;
-
// Adds the handler to the UserData registry.
static void Associate(content::WebContents* web_contents,
AwContentsClientBridgeBase* handler);
@@ -48,7 +47,7 @@
bool* cancel_request) = 0;
virtual void SelectClientCertificate(
net::SSLCertRequestInfo* cert_request_info,
- const SelectCertificateCallback& callback) = 0;
+ scoped_ptr<content::ClientCertificateDelegate> delegate) = 0;
virtual void RunJavaScriptDialog(
content::JavaScriptMessageType message_type,
diff --git a/android_webview/native/aw_contents_client_bridge.cc b/android_webview/native/aw_contents_client_bridge.cc
index 9d686037..c54b8a1 100644
--- a/android_webview/native/aw_contents_client_bridge.cc
+++ b/android_webview/native/aw_contents_client_bridge.cc
@@ -10,7 +10,9 @@
#include "base/android/jni_array.h"
#include "base/android/jni_string.h"
#include "base/callback_helpers.h"
+#include "base/macros.h"
#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/client_certificate_delegate.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_view_host.h"
#include "content/public/browser/web_contents.h"
@@ -58,11 +60,18 @@
JNIEnv* env = AttachCurrentThread();
ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
- if (obj.is_null())
- return;
- // Clear the weak reference from the java peer to the native object since
- // it is possible that java object lifetime can exceed the AwContens.
- Java_AwContentsClientBridge_setNativeContentsClientBridge(env, obj.obj(), 0);
+ if (!obj.is_null()) {
+ // Clear the weak reference from the java peer to the native object since
+ // it is possible that java object lifetime can exceed the AwContens.
+ Java_AwContentsClientBridge_setNativeContentsClientBridge(env, obj.obj(),
+ 0);
+ }
+
+ for (IDMap<content::ClientCertificateDelegate>::iterator iter(
+ &pending_client_cert_request_delegates_);
+ !iter.IsAtEnd(); iter.Advance()) {
+ delete iter.GetCurrentValue();
+ }
}
void AwContentsClientBridge::AllowCertificateError(
@@ -114,13 +123,13 @@
// This method is inspired by SelectClientCertificate() in
// chrome/browser/ui/android/ssl_client_certificate_request.cc
void AwContentsClientBridge::SelectClientCertificate(
- net::SSLCertRequestInfo* cert_request_info,
- const SelectCertificateCallback& callback) {
+ net::SSLCertRequestInfo* cert_request_info,
+ scoped_ptr<content::ClientCertificateDelegate> delegate) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
// Add the callback to id map.
- int request_id = pending_client_cert_request_callbacks_.Add(
- new SelectCertificateCallback(callback));
+ int request_id =
+ pending_client_cert_request_delegates_.Add(delegate.release());
// Make sure callback is run on error.
base::ScopedClosureRunner guard(base::Bind(
&AwContentsClientBridge::HandleErrorInClientCertificateResponse,
@@ -196,19 +205,24 @@
jobject private_key_ref) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
- SelectCertificateCallback* callback =
- pending_client_cert_request_callbacks_.Lookup(request_id);
- DCHECK(callback);
+ content::ClientCertificateDelegate* delegate =
+ pending_client_cert_request_delegates_.Lookup(request_id);
+ DCHECK(delegate);
+
+ if (encoded_chain_ref == NULL || private_key_ref == NULL) {
+ LOG(ERROR) << "No client certificate selected";
+ pending_client_cert_request_delegates_.Remove(request_id);
+ delegate->ContinueWithCertificate(nullptr);
+ delete delegate;
+ return;
+ }
// Make sure callback is run on error.
base::ScopedClosureRunner guard(base::Bind(
&AwContentsClientBridge::HandleErrorInClientCertificateResponse,
base::Unretained(this),
request_id));
- if (encoded_chain_ref == NULL || private_key_ref == NULL) {
- LOG(ERROR) << "Client certificate request cancelled";
- return;
- }
+
// Convert the encoded chain to a vector of strings.
std::vector<std::string> encoded_chain_strings;
if (encoded_chain_ref) {
@@ -236,20 +250,20 @@
return;
}
+ // Release the guard and |pending_client_cert_request_delegates_| references
+ // to |delegate|.
+ pending_client_cert_request_delegates_.Remove(request_id);
+ ignore_result(guard.Release());
+
// RecordClientCertificateKey() must be called on the I/O thread,
- // before the callback is called with the selected certificate on
+ // before the delegate is called with the selected certificate on
// the UI thread.
content::BrowserThread::PostTaskAndReply(
- content::BrowserThread::IO,
- FROM_HERE,
- base::Bind(&RecordClientCertificateKey,
- client_cert,
+ content::BrowserThread::IO, FROM_HERE,
+ base::Bind(&RecordClientCertificateKey, client_cert,
base::Passed(&private_key)),
- base::Bind(*callback, client_cert));
- pending_client_cert_request_callbacks_.Remove(request_id);
-
- // Release the guard.
- ignore_result(guard.Release());
+ base::Bind(&content::ClientCertificateDelegate::ContinueWithCertificate,
+ base::Owned(delegate), client_cert));
}
void AwContentsClientBridge::RunJavaScriptDialog(
@@ -377,10 +391,11 @@
// Use to cleanup if there is an error in client certificate response.
void AwContentsClientBridge::HandleErrorInClientCertificateResponse(
int request_id) {
- SelectCertificateCallback* callback =
- pending_client_cert_request_callbacks_.Lookup(request_id);
- callback->Run(nullptr);
- pending_client_cert_request_callbacks_.Remove(request_id);
+ content::ClientCertificateDelegate* delegate =
+ pending_client_cert_request_delegates_.Lookup(request_id);
+ pending_client_cert_request_delegates_.Remove(request_id);
+
+ delete delegate;
}
bool RegisterAwContentsClientBridge(JNIEnv* env) {
diff --git a/android_webview/native/aw_contents_client_bridge.h b/android_webview/native/aw_contents_client_bridge.h
index 2a5caba..a2bde4b 100644
--- a/android_webview/native/aw_contents_client_bridge.h
+++ b/android_webview/native/aw_contents_client_bridge.h
@@ -40,7 +40,7 @@
bool* cancel_request) override;
void SelectClientCertificate(
net::SSLCertRequestInfo* cert_request_info,
- const SelectCertificateCallback& callback) override;
+ scoped_ptr<content::ClientCertificateDelegate> delegate) override;
void RunJavaScriptDialog(
content::JavaScriptMessageType message_type,
@@ -73,8 +73,10 @@
IDMap<CertErrorCallback, IDMapOwnPointer> pending_cert_error_callbacks_;
IDMap<content::JavaScriptDialogManager::DialogClosedCallback, IDMapOwnPointer>
pending_js_dialog_callbacks_;
- IDMap<SelectCertificateCallback, IDMapOwnPointer>
- pending_client_cert_request_callbacks_;
+ // |pending_client_cert_request_delegates_| owns its pointers, but IDMap
+ // doesn't provide Release, so ownership is managed manually.
+ IDMap<content::ClientCertificateDelegate>
+ pending_client_cert_request_delegates_;
};
bool RegisterAwContentsClientBridge(JNIEnv* env);
diff --git a/android_webview/native/aw_contents_client_bridge_unittest.cc b/android_webview/native/aw_contents_client_bridge_unittest.cc
index b301f919..4b59144 100644
--- a/android_webview/native/aw_contents_client_bridge_unittest.cc
+++ b/android_webview/native/aw_contents_client_bridge_unittest.cc
@@ -8,8 +8,10 @@
#include "base/android/jni_array.h"
#include "base/android/scoped_java_ref.h"
#include "base/bind.h"
+#include "base/macros.h"
#include "base/memory/scoped_ptr.h"
#include "base/run_loop.h"
+#include "content/public/browser/client_certificate_delegate.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "jni/MockAwContentsClientBridge_jni.h"
#include "net/android/net_jni_registrar.h"
@@ -33,9 +35,6 @@
// Tests the android_webview contents client bridge.
class AwContentsClientBridgeTest : public Test {
public:
- typedef AwContentsClientBridge::SelectCertificateCallback
- SelectCertificateCallback;
-
AwContentsClientBridgeTest() { }
// Callback method called when a cert is selected.
@@ -53,6 +52,24 @@
JNIEnv* env_;
};
+class TestClientCertificateDelegate
+ : public content::ClientCertificateDelegate {
+ public:
+ explicit TestClientCertificateDelegate(AwContentsClientBridgeTest* test)
+ : test_(test) {}
+
+ // content::ClientCertificateDelegate.
+ void ContinueWithCertificate(net::X509Certificate* cert) override {
+ test_->CertSelected(cert);
+ test_ = nullptr;
+ }
+
+ private:
+ AwContentsClientBridgeTest* test_;
+
+ DISALLOW_COPY_AND_ASSIGN(TestClientCertificateDelegate);
+};
+
} // namespace
void AwContentsClientBridgeTest::SetUp() {
@@ -90,9 +107,7 @@
cert_request_info_->cert_key_types.push_back(type);
bridge_->SelectClientCertificate(
cert_request_info_.get(),
- base::Bind(
- &AwContentsClientBridgeTest::CertSelected,
- base::Unretained(static_cast<AwContentsClientBridgeTest*>(this))));
+ make_scoped_ptr(new TestClientCertificateDelegate(this)));
base::RunLoop().RunUntilIdle();
EXPECT_EQ(0, cert_selected_callbacks_);
ScopedJavaLocalRef<jobjectArray> key_types =
@@ -112,10 +127,8 @@
// Call SelectClientCertificate to create a callback id that mock java object
// can call on.
bridge_->SelectClientCertificate(
- cert_request_info_.get(),
- base::Bind(
- &AwContentsClientBridgeTest::CertSelected,
- base::Unretained(static_cast<AwContentsClientBridgeTest*>(this))));
+ cert_request_info_.get(),
+ make_scoped_ptr(new TestClientCertificateDelegate(this)));
bridge_->ProvideClientCertificateResponse(env_, jbridge_.obj(),
Java_MockAwContentsClientBridge_getRequestId(env_, jbridge_.obj()),
Java_MockAwContentsClientBridge_createTestCertChain(
@@ -133,10 +146,8 @@
// Call SelectClientCertificate to create a callback id that mock java object
// can call on.
bridge_->SelectClientCertificate(
- cert_request_info_.get(),
- base::Bind(
- &AwContentsClientBridgeTest::CertSelected,
- base::Unretained(static_cast<AwContentsClientBridgeTest*>(this))));
+ cert_request_info_.get(),
+ make_scoped_ptr(new TestClientCertificateDelegate(this)));
int requestId =
Java_MockAwContentsClientBridge_getRequestId(env_, jbridge_.obj());
bridge_->ProvideClientCertificateResponse(env_, jbridge_.obj(),
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index a92c059..a8c7829 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -116,6 +116,7 @@
#include "content/public/browser/browser_url_handler.h"
#include "content/public/browser/child_process_data.h"
#include "content/public/browser/child_process_security_policy.h"
+#include "content/public/browser/client_certificate_delegate.h"
#include "content/public/browser/permission_type.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_process_host.h"
@@ -1861,21 +1862,11 @@
}
void ChromeContentBrowserClient::SelectClientCertificate(
- int render_process_id,
- int render_frame_id,
+ content::WebContents* web_contents,
net::SSLCertRequestInfo* cert_request_info,
- const base::Callback<void(net::X509Certificate*)>& callback) {
- content::RenderFrameHost* rfh = content::RenderFrameHost::FromID(
- render_process_id, render_frame_id);
- WebContents* tab = WebContents::FromRenderFrameHost(rfh);
- if (!tab) {
- // TODO(davidben): This makes the request hang, but returning no certificate
- // also breaks. It should abort the request. See https://crbug.com/417092
- return;
- }
-
+ scoped_ptr<content::ClientCertificateDelegate> delegate) {
prerender::PrerenderContents* prerender_contents =
- prerender::PrerenderContents::FromWebContents(tab);
+ prerender::PrerenderContents::FromWebContents(web_contents);
if (prerender_contents) {
prerender_contents->Destroy(
prerender::FINAL_STATUS_SSL_CLIENT_CERTIFICATE_REQUESTED);
@@ -1887,7 +1878,8 @@
<< "Invalid URL string: https://"
<< cert_request_info->host_and_port.ToString();
- Profile* profile = Profile::FromBrowserContext(tab->GetBrowserContext());
+ Profile* profile =
+ Profile::FromBrowserContext(web_contents->GetBrowserContext());
scoped_ptr<base::Value> filter =
profile->GetHostContentSettingsMap()->GetWebsiteSetting(
requesting_url,
@@ -1907,7 +1899,7 @@
for (size_t i = 0; i < all_client_certs.size(); ++i) {
if (CertMatchesFilter(*all_client_certs[i].get(), *filter_dict)) {
// Use the first certificate that is matched by the filter.
- callback.Run(all_client_certs[i].get());
+ delegate->ContinueWithCertificate(all_client_certs[i].get());
return;
}
}
@@ -1916,7 +1908,8 @@
}
}
- chrome::ShowSSLClientCertificateSelector(tab, cert_request_info, callback);
+ chrome::ShowSSLClientCertificateSelector(web_contents, cert_request_info,
+ delegate.Pass());
}
void ChromeContentBrowserClient::AddCertificate(
diff --git a/chrome/browser/chrome_content_browser_client.h b/chrome/browser/chrome_content_browser_client.h
index 9166286b..8e28784 100644
--- a/chrome/browser/chrome_content_browser_client.h
+++ b/chrome/browser/chrome_content_browser_client.h
@@ -166,10 +166,9 @@
const base::Callback<void(bool)>& callback,
content::CertificateRequestResultType* request) override;
void SelectClientCertificate(
- int render_process_id,
- int render_frame_id,
+ content::WebContents* web_contents,
net::SSLCertRequestInfo* cert_request_info,
- const base::Callback<void(net::X509Certificate*)>& callback) override;
+ scoped_ptr<content::ClientCertificateDelegate> delegate) override;
void AddCertificate(net::CertificateMimeType cert_type,
const void* cert_data,
size_t cert_size,
diff --git a/chrome/browser/extensions/background_xhr_browsertest.cc b/chrome/browser/extensions/background_xhr_browsertest.cc
new file mode 100644
index 0000000..6ff6deb
--- /dev/null
+++ b/chrome/browser/extensions/background_xhr_browsertest.cc
@@ -0,0 +1,82 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/bind.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/run_loop.h"
+#include "chrome/browser/extensions/extension_browsertest.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_io_data.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/browser_thread.h"
+#include "extensions/common/extension.h"
+#include "extensions/test/result_catcher.h"
+#include "net/base/escape.h"
+#include "net/base/url_util.h"
+#include "net/ssl/client_cert_store.h"
+#include "net/test/spawned_test_server/spawned_test_server.h"
+#include "url/gurl.h"
+
+namespace {
+
+scoped_ptr<net::ClientCertStore> CreateNullCertStore() {
+ return nullptr;
+}
+
+void InstallNullCertStoreFactoryOnIOThread(
+ content::ResourceContext* resource_context) {
+ ProfileIOData::FromResourceContext(resource_context)
+ ->set_client_cert_store_factory_for_testing(
+ base::Bind(&CreateNullCertStore));
+}
+
+} // namespace
+
+class BackgroundXhrTest : public ExtensionBrowserTest {
+ protected:
+ void RunTest(const std::string& path, const GURL& url) {
+ const extensions::Extension* extension =
+ LoadExtension(test_data_dir_.AppendASCII("background_xhr"));
+ ASSERT_TRUE(extension);
+
+ extensions::ResultCatcher catcher;
+ GURL test_url = net::AppendQueryParameter(extension->GetResourceURL(path),
+ "url", url.spec());
+ ui_test_utils::NavigateToURL(browser(), test_url);
+ ASSERT_TRUE(catcher.GetNextResult());
+ }
+};
+
+// Test that fetching a URL using TLS client auth doesn't crash, hang, or
+// prompt.
+IN_PROC_BROWSER_TEST_F(BackgroundXhrTest, TlsClientAuth) {
+ // Install a null ClientCertStore so the client auth prompt isn't bypassed due
+ // to the system certificate store returning no certificates.
+ base::RunLoop loop;
+ content::BrowserThread::PostTaskAndReply(
+ content::BrowserThread::IO, FROM_HERE,
+ base::Bind(&InstallNullCertStoreFactoryOnIOThread,
+ browser()->profile()->GetResourceContext()),
+ loop.QuitClosure());
+ loop.Run();
+
+ // Launch HTTPS server.
+ net::SpawnedTestServer::SSLOptions ssl_options;
+ ssl_options.request_client_certificate = true;
+ net::SpawnedTestServer https_server(
+ net::SpawnedTestServer::TYPE_HTTPS, ssl_options,
+ base::FilePath(FILE_PATH_LITERAL("content/test/data")));
+ ASSERT_TRUE(https_server.Start());
+
+ ASSERT_NO_FATAL_FAILURE(
+ RunTest("test_tls_client_auth.html", https_server.GetURL("")));
+}
+
+// Test that fetching a URL using HTTP auth doesn't crash, hang, or prompt.
+IN_PROC_BROWSER_TEST_F(BackgroundXhrTest, HttpAuth) {
+ ASSERT_TRUE(test_server()->Start());
+ ASSERT_NO_FATAL_FAILURE(
+ RunTest("test_http_auth.html", test_server()->GetURL("auth-basic")));
+}
diff --git a/chrome/browser/ssl/ssl_client_auth_observer.cc b/chrome/browser/ssl/ssl_client_auth_observer.cc
index edc63bb..acbdddd0 100644
--- a/chrome/browser/ssl/ssl_client_auth_observer.cc
+++ b/chrome/browser/ssl/ssl_client_auth_observer.cc
@@ -10,6 +10,7 @@
#include "base/logging.h"
#include "chrome/browser/chrome_notification_types.h"
#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/client_certificate_delegate.h"
#include "content/public/browser/notification_service.h"
#include "net/cert/x509_certificate.h"
#include "net/ssl/ssl_cert_request_info.h"
@@ -21,10 +22,10 @@
SSLClientAuthObserver::SSLClientAuthObserver(
const content::BrowserContext* browser_context,
const scoped_refptr<net::SSLCertRequestInfo>& cert_request_info,
- const base::Callback<void(net::X509Certificate*)>& callback)
+ scoped_ptr<content::ClientCertificateDelegate> delegate)
: browser_context_(browser_context),
cert_request_info_(cert_request_info),
- callback_(callback) {
+ delegate_(delegate.Pass()) {
}
SSLClientAuthObserver::~SSLClientAuthObserver() {
@@ -32,10 +33,11 @@
void SSLClientAuthObserver::CertificateSelected(
net::X509Certificate* certificate) {
- if (callback_.is_null())
+ if (!delegate_)
return;
- // Stop listening right away so we don't get our own notification.
+ // Stop listening now that the delegate has been resolved. This is also to
+ // avoid getting a self-notification.
StopObserving();
CertDetails details;
@@ -47,8 +49,17 @@
content::Source<content::BrowserContext>(browser_context_),
content::Details<CertDetails>(&details));
- callback_.Run(certificate);
- callback_.Reset();
+ delegate_->ContinueWithCertificate(certificate);
+ delegate_.reset();
+}
+
+void SSLClientAuthObserver::CancelCertificateSelection() {
+ if (!delegate_)
+ return;
+
+ // Stop observing now that the delegate has been resolved.
+ StopObserving();
+ delegate_.reset();
}
void SSLClientAuthObserver::Observe(
@@ -67,8 +78,8 @@
DVLOG(1) << this << " got matching notification and selecting cert "
<< cert_details->second;
StopObserving();
- callback_.Run(cert_details->second);
- callback_.Reset();
+ delegate_->ContinueWithCertificate(cert_details->second);
+ delegate_.reset();
OnCertSelectedByNotification();
}
diff --git a/chrome/browser/ssl/ssl_client_auth_observer.h b/chrome/browser/ssl/ssl_client_auth_observer.h
index 62e3fde..ee261348 100644
--- a/chrome/browser/ssl/ssl_client_auth_observer.h
+++ b/chrome/browser/ssl/ssl_client_auth_observer.h
@@ -7,6 +7,7 @@
#include "base/callback.h"
#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
@@ -17,24 +18,32 @@
namespace content {
class BrowserContext;
+class ClientCertificateDelegate;
}
+// SSLClientAuthObserver is a base class that wraps a
+// ClientCertificateDelegate. It links client certificate selection dialogs
+// attached to the same BrowserContext. When CertificateSelected is called via
+// one of them, the rest simulate the same action.
class SSLClientAuthObserver : public content::NotificationObserver {
public:
SSLClientAuthObserver(
const content::BrowserContext* browser_context,
const scoped_refptr<net::SSLCertRequestInfo>& cert_request_info,
- const base::Callback<void(net::X509Certificate*)>& callback);
+ scoped_ptr<content::ClientCertificateDelegate> delegate);
~SSLClientAuthObserver() override;
// UI should implement this to close the dialog.
virtual void OnCertSelectedByNotification() = 0;
- // Send content the certificate. Can also call with NULL if the user
- // cancelled. Derived classes must use this instead of caching the callback
- // and calling it directly.
+ // Continues the request with a certificate. Can also call with NULL to
+ // continue with no certificate. Derived classes must use this instead of
+ // caching the delegate and calling it directly.
void CertificateSelected(net::X509Certificate* cert);
+ // Cancels the certificate selection and aborts the request.
+ void CancelCertificateSelection();
+
// Begins observing notifications from other SSLClientAuthHandler instances.
// If another instance chooses a cert for a matching SSLCertRequestInfo, we
// will also use the same cert and OnCertSelectedByNotification will be called
@@ -57,7 +66,7 @@
const content::BrowserContext* browser_context_;
scoped_refptr<net::SSLCertRequestInfo> cert_request_info_;
- base::Callback<void(net::X509Certificate*)> callback_;
+ scoped_ptr<content::ClientCertificateDelegate> delegate_;
content::NotificationRegistrar notification_registrar_;
DISALLOW_COPY_AND_ASSIGN(SSLClientAuthObserver);
diff --git a/chrome/browser/ssl/ssl_client_auth_requestor_mock.cc b/chrome/browser/ssl/ssl_client_auth_requestor_mock.cc
index 6f63c32..ef114b4 100644
--- a/chrome/browser/ssl/ssl_client_auth_requestor_mock.cc
+++ b/chrome/browser/ssl/ssl_client_auth_requestor_mock.cc
@@ -4,9 +4,38 @@
#include "chrome/browser/ssl/ssl_client_auth_requestor_mock.h"
+#include "base/macros.h"
+#include "content/public/browser/client_certificate_delegate.h"
#include "net/ssl/ssl_cert_request_info.h"
#include "net/url_request/url_request.h"
+namespace {
+
+class FakeClientCertificateDelegate
+ : public content::ClientCertificateDelegate {
+ public:
+ explicit FakeClientCertificateDelegate(SSLClientAuthRequestorMock* requestor)
+ : requestor_(requestor) {}
+
+ ~FakeClientCertificateDelegate() override {
+ if (requestor_)
+ requestor_->CancelCertificateSelection();
+ }
+
+ // content::ClientCertificateDelegate implementation:
+ void ContinueWithCertificate(net::X509Certificate* cert) override {
+ requestor_->CertificateSelected(cert);
+ requestor_ = nullptr;
+ }
+
+ private:
+ scoped_refptr<SSLClientAuthRequestorMock> requestor_;
+
+ DISALLOW_COPY_AND_ASSIGN(FakeClientCertificateDelegate);
+};
+
+} // namespace
+
SSLClientAuthRequestorMock::SSLClientAuthRequestorMock(
net::URLRequest* request,
const scoped_refptr<net::SSLCertRequestInfo>& cert_request_info)
@@ -14,3 +43,8 @@
}
SSLClientAuthRequestorMock::~SSLClientAuthRequestorMock() {}
+
+scoped_ptr<content::ClientCertificateDelegate>
+SSLClientAuthRequestorMock::CreateDelegate() {
+ return make_scoped_ptr(new FakeClientCertificateDelegate(this));
+}
diff --git a/chrome/browser/ssl/ssl_client_auth_requestor_mock.h b/chrome/browser/ssl/ssl_client_auth_requestor_mock.h
index 7d63a7c..6801d1a 100644
--- a/chrome/browser/ssl/ssl_client_auth_requestor_mock.h
+++ b/chrome/browser/ssl/ssl_client_auth_requestor_mock.h
@@ -6,8 +6,13 @@
#define CHROME_BROWSER_SSL_SSL_CLIENT_AUTH_REQUESTOR_MOCK_H_
#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
#include "testing/gmock/include/gmock/gmock.h"
+namespace content {
+class ClientCertificateDelegate;
+}
+
namespace net {
class HttpNetworkSession;
class SSLCertRequestInfo;
@@ -22,7 +27,10 @@
net::URLRequest* request,
const scoped_refptr<net::SSLCertRequestInfo>& cert_request_info);
+ scoped_ptr<content::ClientCertificateDelegate> CreateDelegate();
+
MOCK_METHOD1(CertificateSelected, void(net::X509Certificate* cert));
+ MOCK_METHOD0(CancelCertificateSelection, void());
scoped_refptr<net::SSLCertRequestInfo> cert_request_info_;
diff --git a/chrome/browser/ssl/ssl_client_certificate_selector.h b/chrome/browser/ssl/ssl_client_certificate_selector.h
index 1a33c059..7c73479d 100644
--- a/chrome/browser/ssl/ssl_client_certificate_selector.h
+++ b/chrome/browser/ssl/ssl_client_certificate_selector.h
@@ -6,8 +6,10 @@
#define CHROME_BROWSER_SSL_SSL_CLIENT_CERTIFICATE_SELECTOR_H_
#include "base/callback_forward.h"
+#include "base/memory/scoped_ptr.h"
namespace content {
+class ClientCertificateDelegate;
class WebContents;
}
@@ -18,17 +20,14 @@
namespace chrome {
-typedef base::Callback<void(net::X509Certificate*)> SelectCertificateCallback;
-
// Opens a constrained SSL client certificate selection dialog under |parent|,
// offering certificates from |cert_request_info|. When the user has made a
-// selection, the dialog will report back to |callback|. |callback| is notified
-// when the dialog closes in call cases; if the user cancels the dialog, we call
-// with a NULL certificate.
+// selection, the dialog will report back to |delegate|. If the dialog is
+// closed with no selection, |delegate| will simply be destroyed.
void ShowSSLClientCertificateSelector(
content::WebContents* contents,
net::SSLCertRequestInfo* cert_request_info,
- const SelectCertificateCallback& callback);
+ scoped_ptr<content::ClientCertificateDelegate> delegate);
} // namespace chrome
diff --git a/chrome/browser/ui/android/ssl_client_certificate_request.cc b/chrome/browser/ui/android/ssl_client_certificate_request.cc
index 1acbec9..b09f483 100644
--- a/chrome/browser/ui/android/ssl_client_certificate_request.cc
+++ b/chrome/browser/ui/android/ssl_client_certificate_request.cc
@@ -9,12 +9,12 @@
#include "base/android/scoped_java_ref.h"
#include "base/basictypes.h"
#include "base/bind.h"
-#include "base/callback_helpers.h"
#include "base/compiler_specific.h"
#include "base/logging.h"
#include "chrome/browser/ssl/ssl_client_certificate_selector.h"
#include "chrome/browser/ui/android/window_android_helper.h"
#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/client_certificate_delegate.h"
#include "crypto/scoped_openssl_types.h"
#include "jni/SSLClientCertificateRequest_jni.h"
#include "net/android/keystore_openssl.h"
@@ -43,19 +43,9 @@
void StartClientCertificateRequest(
const net::SSLCertRequestInfo* cert_request_info,
ui::WindowAndroid* window,
- const chrome::SelectCertificateCallback& callback) {
+ scoped_ptr<content::ClientCertificateDelegate> delegate) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
- // Ensure that callback(NULL) is posted as a task on the UI thread
- // in case of an error.
- base::Closure post_task_closure = base::Bind(
- base::IgnoreResult(&content::BrowserThread::PostTask),
- content::BrowserThread::UI,
- FROM_HERE,
- base::Bind(callback, scoped_refptr<net::X509Certificate>()));
-
- base::ScopedClosureRunner guard(post_task_closure);
-
// Build the |key_types| JNI parameter, as a String[]
std::vector<std::string> key_types;
for (size_t n = 0; n < cert_request_info->cert_key_types.size(); ++n) {
@@ -98,12 +88,8 @@
base::android::ConvertUTF8ToJavaString(
env, cert_request_info->host_and_port.host());
- // Create a copy of the callback on the heap so that its address
- // and ownership can be passed through and returned from Java via JNI.
- scoped_ptr<chrome::SelectCertificateCallback> request(
- new chrome::SelectCertificateCallback(callback));
-
- jlong request_id = reinterpret_cast<intptr_t>(request.get());
+ // Pass the address of the delegate through to Java.
+ jlong request_id = reinterpret_cast<intptr_t>(delegate.get());
if (!chrome::android::
Java_SSLClientCertificateRequest_selectClientCertificate(
@@ -117,10 +103,8 @@
return;
}
- ignore_result(guard.Release());
-
// Ownership was transferred to Java.
- ignore_result(request.release());
+ ignore_result(delegate.release());
}
} // namespace
@@ -146,18 +130,13 @@
jobject private_key_ref) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
- // Take back ownership of the request object.
- scoped_ptr<chrome::SelectCertificateCallback> callback(
- reinterpret_cast<chrome::SelectCertificateCallback*>(request_id));
-
- // Ensure that callback(NULL) is called in case of an error.
- base::Closure null_closure =
- base::Bind(*callback, scoped_refptr<net::X509Certificate>());
-
- base::ScopedClosureRunner guard(null_closure);
+ // Take back ownership of the delegate object.
+ scoped_ptr<content::ClientCertificateDelegate> delegate(
+ reinterpret_cast<content::ClientCertificateDelegate*>(request_id));
if (encoded_chain_ref == NULL || private_key_ref == NULL) {
- LOG(ERROR) << "Client certificate request cancelled";
+ LOG(ERROR) << "No client certificate selected";
+ delegate->ContinueWithCertificate(nullptr);
return;
}
@@ -188,18 +167,15 @@
return;
}
- ignore_result(guard.Release());
-
// RecordClientCertificateKey() must be called on the I/O thread,
// before the callback is called with the selected certificate on
// the UI thread.
content::BrowserThread::PostTaskAndReply(
- content::BrowserThread::IO,
- FROM_HERE,
- base::Bind(&RecordClientCertificateKey,
- client_cert,
+ content::BrowserThread::IO, FROM_HERE,
+ base::Bind(&RecordClientCertificateKey, client_cert,
base::Passed(&private_key)),
- base::Bind(*callback, client_cert));
+ base::Bind(&content::ClientCertificateDelegate::ContinueWithCertificate,
+ base::Owned(delegate.release()), client_cert));
}
static void NotifyClientCertificatesChanged() {
@@ -226,12 +202,12 @@
void ShowSSLClientCertificateSelector(
content::WebContents* contents,
net::SSLCertRequestInfo* cert_request_info,
- const chrome::SelectCertificateCallback& callback) {
+ scoped_ptr<content::ClientCertificateDelegate> delegate) {
ui::WindowAndroid* window =
WindowAndroidHelper::FromWebContents(contents)->GetWindowAndroid();
DCHECK(window);
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
- StartClientCertificateRequest(cert_request_info, window, callback);
+ StartClientCertificateRequest(cert_request_info, window, delegate.Pass());
}
} // namespace chrome
diff --git a/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.h b/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.h
index aacdafe..f1fe2e3 100644
--- a/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.h
+++ b/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.h
@@ -18,6 +18,7 @@
namespace content {
class BrowserContext;
+class ClientCertificateDelegate;
}
class ConstrainedWindowMac;
@@ -41,13 +42,18 @@
NSRect oldSheetFrame_;
// A copy of the sheet's |autoresizesSubviews| flag to restore on show.
BOOL oldResizesSubviews_;
+ // True if the user dismissed the dialog directly, either via the OK (continue
+ // the request with a certificate) or Cancel (continue the request with no
+ // certificate) buttons.
+ BOOL userResponded_;
}
@property (readonly, nonatomic) SFChooseIdentityPanel* panel;
- (id)initWithBrowserContext:(const content::BrowserContext*)browserContext
certRequestInfo:(net::SSLCertRequestInfo*)certRequestInfo
- callback:(const chrome::SelectCertificateCallback&)callback;
+ delegate:(scoped_ptr<content::ClientCertificateDelegate>)
+ delegate;
- (void)displayForWebContents:(content::WebContents*)webContents;
- (void)closeWebContentsModalDialog;
diff --git a/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.mm b/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.mm
index a8d7912..54ff8912 100644
--- a/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.mm
+++ b/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.mm
@@ -14,7 +14,9 @@
#include "chrome/browser/ssl/ssl_client_auth_observer.h"
#import "chrome/browser/ui/cocoa/constrained_window/constrained_window_mac.h"
#include "chrome/grit/generated_resources.h"
+#include "components/web_modal/popup_manager.h"
#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/client_certificate_delegate.h"
#include "content/public/browser/web_contents.h"
#include "net/cert/x509_certificate.h"
#include "net/cert/x509_util_mac.h"
@@ -42,11 +44,12 @@
SSLClientAuthObserverCocoaBridge(
const content::BrowserContext* browser_context,
net::SSLCertRequestInfo* cert_request_info,
- const chrome::SelectCertificateCallback& callback,
+ scoped_ptr<content::ClientCertificateDelegate> delegate,
SSLClientCertificateSelectorCocoa* controller)
- : SSLClientAuthObserver(browser_context, cert_request_info, callback),
- controller_(controller) {
- }
+ : SSLClientAuthObserver(browser_context,
+ cert_request_info,
+ delegate.Pass()),
+ controller_(controller) {}
// SSLClientAuthObserver implementation:
void OnCertSelectedByNotification() override {
@@ -72,14 +75,22 @@
void ShowSSLClientCertificateSelector(
content::WebContents* contents,
net::SSLCertRequestInfo* cert_request_info,
- const SelectCertificateCallback& callback) {
+ scoped_ptr<content::ClientCertificateDelegate> delegate) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+ // Not all WebContentses can show modal dialogs.
+ //
+ // TODO(davidben): Move this hook to the WebContentsDelegate and only try to
+ // show a dialog in Browser's implementation. https://crbug.com/456255
+ if (web_modal::PopupManager::FromWebContents(contents) == nullptr)
+ return;
+
// The dialog manages its own lifetime.
SSLClientCertificateSelectorCocoa* selector =
[[SSLClientCertificateSelectorCocoa alloc]
initWithBrowserContext:contents->GetBrowserContext()
certRequestInfo:cert_request_info
- callback:callback];
+ delegate:delegate.Pass()];
[selector displayForWebContents:contents];
}
@@ -88,13 +99,14 @@
@implementation SSLClientCertificateSelectorCocoa
- (id)initWithBrowserContext:(const content::BrowserContext*)browserContext
- certRequestInfo:(net::SSLCertRequestInfo*)certRequestInfo
- callback:(const chrome::SelectCertificateCallback&)callback {
+ certRequestInfo:(net::SSLCertRequestInfo*)certRequestInfo
+ delegate:(scoped_ptr<content::ClientCertificateDelegate>)
+ delegate {
DCHECK(browserContext);
DCHECK(certRequestInfo);
if ((self = [super init])) {
observer_.reset(new SSLClientAuthObserverCocoaBridge(
- browserContext, certRequestInfo, callback, self));
+ browserContext, certRequestInfo, delegate.Pass(), self));
}
return self;
}
@@ -113,12 +125,16 @@
NOTREACHED();
}
- // Finally, tell the backend which identity (or none) the user selected.
- observer_->StopObserving();
- observer_->CertificateSelected(cert);
+ if (!closePending_) {
+ // If |closePending_| is already set, |closeSheetWithAnimation:| was called
+ // already to cancel the selection rather than continue with no
+ // certificate. Otherwise, tell the backend which identity (or none) the
+ // user selected.
+ userResponded_ = YES;
+ observer_->CertificateSelected(cert);
- if (!closePending_)
constrainedWindow_->CloseWebContentsModalDialog();
+ }
}
- (void)displayForWebContents:(content::WebContents*)webContents {
@@ -184,6 +200,14 @@
}
- (void)closeSheetWithAnimation:(BOOL)withAnimation {
+ if (!userResponded_) {
+ // If the sheet is closed by closing the tab rather than the user explicitly
+ // hitting Cancel, |closeSheetWithAnimation:| gets called before
+ // |sheetDidEnd:|. In this case, the selection should be canceled rather
+ // than continue with no certificate. The |returnCode| parameter to
+ // |sheetDidEnd:| is the same in both cases.
+ observer_->CancelCertificateSelection();
+ }
closePending_ = YES;
overlayWindow_.reset();
// Closing the sheet using -[NSApp endSheet:] doesn't work so use the private
diff --git a/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa_browsertest.mm b/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa_browsertest.mm
index a0c71106..2684236 100644
--- a/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa_browsertest.mm
+++ b/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa_browsertest.mm
@@ -8,12 +8,14 @@
#include "base/bind.h"
#import "base/mac/mac_util.h"
+#include "base/macros.h"
#include "chrome/browser/ssl/ssl_client_certificate_selector.h"
#include "chrome/browser/ssl/ssl_client_certificate_selector_test.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "components/web_modal/web_contents_modal_dialog_manager.h"
+#include "content/public/browser/client_certificate_delegate.h"
#include "content/public/browser/web_contents.h"
#include "content/public/test/test_utils.h"
#include "ui/base/cocoa/window_size_constants.h"
@@ -22,12 +24,33 @@
namespace {
-void OnCertificateSelected(net::X509Certificate** out_cert,
- int* out_count,
- net::X509Certificate* cert) {
- *out_cert = cert;
- ++(*out_count);
-}
+class TestClientCertificateDelegate
+ : public content::ClientCertificateDelegate {
+ public:
+ // Creates a ClientCertificateDelegate that sets |*destroyed| to true on
+ // destruction.
+ explicit TestClientCertificateDelegate(bool* destroyed)
+ : destroyed_(destroyed) {}
+
+ ~TestClientCertificateDelegate() override {
+ if (destroyed_ != nullptr)
+ *destroyed_ = true;
+ }
+
+ // content::ClientCertificateDelegate.
+ void ContinueWithCertificate(net::X509Certificate* cert) override {
+ // TODO(davidben): Add a test which explicitly tests selecting a
+ // certificate, or selecting no certificate, since closing the dialog
+ // (normally by closing the tab) is not the same as explicitly selecting no
+ // certificate.
+ ADD_FAILURE() << "Certificate selected";
+ }
+
+ private:
+ bool* destroyed_;
+
+ DISALLOW_COPY_AND_ASSIGN(TestClientCertificateDelegate);
+};
} // namespace
@@ -46,15 +69,13 @@
WebContentsModalDialogManager::FromWebContents(web_contents);
EXPECT_FALSE(web_contents_modal_dialog_manager->IsDialogActive());
- net::X509Certificate* cert = NULL;
- int count = 0;
- SSLClientCertificateSelectorCocoa* selector =
- [[SSLClientCertificateSelectorCocoa alloc]
- initWithBrowserContext:web_contents->GetBrowserContext()
- certRequestInfo:auth_requestor_->cert_request_info_.get()
- callback:base::Bind(&OnCertificateSelected,
- &cert,
- &count)];
+ bool destroyed = false;
+ SSLClientCertificateSelectorCocoa* selector = [
+ [SSLClientCertificateSelectorCocoa alloc]
+ initWithBrowserContext:web_contents->GetBrowserContext()
+ certRequestInfo:auth_requestor_->cert_request_info_.get()
+ delegate:make_scoped_ptr(new TestClientCertificateDelegate(
+ &destroyed))];
[selector displayForWebContents:web_contents];
content::RunAllPendingInMessageLoop();
EXPECT_TRUE([selector panel]);
@@ -66,19 +87,19 @@
content::RunAllPendingInMessageLoop();
EXPECT_FALSE(web_contents_modal_dialog_manager->IsDialogActive());
- EXPECT_EQ(NULL, cert);
- EXPECT_EQ(1, count);
+ EXPECT_TRUE(destroyed);
}
// Test that switching to another tab correctly hides the sheet.
IN_PROC_BROWSER_TEST_F(SSLClientCertificateSelectorCocoaTest, HideShow) {
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
- SSLClientCertificateSelectorCocoa* selector =
- [[SSLClientCertificateSelectorCocoa alloc]
- initWithBrowserContext:web_contents->GetBrowserContext()
- certRequestInfo:auth_requestor_->cert_request_info_.get()
- callback:chrome::SelectCertificateCallback()];
+ SSLClientCertificateSelectorCocoa* selector = [
+ [SSLClientCertificateSelectorCocoa alloc]
+ initWithBrowserContext:web_contents->GetBrowserContext()
+ certRequestInfo:auth_requestor_->cert_request_info_.get()
+ delegate:make_scoped_ptr(
+ new TestClientCertificateDelegate(nullptr))];
[selector displayForWebContents:web_contents];
content::RunAllPendingInMessageLoop();
diff --git a/chrome/browser/ui/views/ssl_client_certificate_selector.cc b/chrome/browser/ui/views/ssl_client_certificate_selector.cc
index 097195f..72f4a498 100644
--- a/chrome/browser/ui/views/ssl_client_certificate_selector.cc
+++ b/chrome/browser/ui/views/ssl_client_certificate_selector.cc
@@ -9,7 +9,9 @@
#include "base/logging.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/grit/generated_resources.h"
+#include "components/web_modal/popup_manager.h"
#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/client_certificate_delegate.h"
#include "content/public/browser/web_contents.h"
#include "net/cert/x509_certificate.h"
#include "net/ssl/ssl_cert_request_info.h"
@@ -23,11 +25,11 @@
SSLClientCertificateSelector::SSLClientCertificateSelector(
content::WebContents* web_contents,
const scoped_refptr<net::SSLCertRequestInfo>& cert_request_info,
- const chrome::SelectCertificateCallback& callback)
+ scoped_ptr<content::ClientCertificateDelegate> delegate)
: CertificateSelector(cert_request_info->client_certs, web_contents),
SSLClientAuthObserver(web_contents->GetBrowserContext(),
cert_request_info,
- callback) {
+ delegate.Pass()) {
DVLOG(1) << __FUNCTION__;
}
@@ -76,6 +78,15 @@
return false;
}
+bool SSLClientCertificateSelector::Close() {
+ // By default, closing the dialog calls the Cancel method. However, selecting
+ // cancel in the UI currently continues the request with no certificate,
+ // remembering the selection. If the dialog is closed by closing the
+ // containing tab, the request should abort.
+ CancelCertificateSelection();
+ return true;
+}
+
void SSLClientCertificateSelector::Unlocked(net::X509Certificate* cert) {
DVLOG(1) << __FUNCTION__;
CertificateSelected(cert);
@@ -87,11 +98,19 @@
void ShowSSLClientCertificateSelector(
content::WebContents* contents,
net::SSLCertRequestInfo* cert_request_info,
- const chrome::SelectCertificateCallback& callback) {
+ scoped_ptr<content::ClientCertificateDelegate> delegate) {
DVLOG(1) << __FUNCTION__ << " " << contents;
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
- SSLClientCertificateSelector* selector =
- new SSLClientCertificateSelector(contents, cert_request_info, callback);
+
+ // Not all WebContentses can show modal dialogs.
+ //
+ // TODO(davidben): Move this hook to the WebContentsDelegate and only try to
+ // show a dialog in Browser's implementation. https://crbug.com/456255
+ if (web_modal::PopupManager::FromWebContents(contents) == nullptr)
+ return;
+
+ SSLClientCertificateSelector* selector = new SSLClientCertificateSelector(
+ contents, cert_request_info, delegate.Pass());
selector->Init();
selector->Show();
}
diff --git a/chrome/browser/ui/views/ssl_client_certificate_selector.h b/chrome/browser/ui/views/ssl_client_certificate_selector.h
index d80fc82..5f9c1e2 100644
--- a/chrome/browser/ui/views/ssl_client_certificate_selector.h
+++ b/chrome/browser/ui/views/ssl_client_certificate_selector.h
@@ -29,7 +29,7 @@
SSLClientCertificateSelector(
content::WebContents* web_contents,
const scoped_refptr<net::SSLCertRequestInfo>& cert_request_info,
- const chrome::SelectCertificateCallback& callback);
+ scoped_ptr<content::ClientCertificateDelegate> delegate);
~SSLClientCertificateSelector() override;
void Init();
@@ -40,6 +40,7 @@
// chrome::CertificateSelector:
bool Cancel() override;
bool Accept() override;
+ bool Close() override;
private:
// Callback after unlocking certificate slot.
diff --git a/chrome/browser/ui/views/ssl_client_certificate_selector_browsertest.cc b/chrome/browser/ui/views/ssl_client_certificate_selector_browsertest.cc
index 63ee691..2ad2566 100644
--- a/chrome/browser/ui/views/ssl_client_certificate_selector_browsertest.cc
+++ b/chrome/browser/ui/views/ssl_client_certificate_selector_browsertest.cc
@@ -13,6 +13,7 @@
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/interactive_test_utils.h"
#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/client_certificate_delegate.h"
#include "content/public/browser/web_contents.h"
#include "content/public/test/browser_test_utils.h"
#include "net/base/request_priority.h"
@@ -86,9 +87,7 @@
browser()->tab_strip_model()->GetActiveWebContents());
selector_ = new SSLClientCertificateSelector(
browser()->tab_strip_model()->GetActiveWebContents(),
- auth_requestor_->cert_request_info_,
- base::Bind(&SSLClientAuthRequestorMock::CertificateSelected,
- auth_requestor_));
+ auth_requestor_->cert_request_info_, auth_requestor_->CreateDelegate());
selector_->Init();
selector_->Show();
@@ -178,15 +177,13 @@
selector_1_ = new SSLClientCertificateSelector(
browser()->tab_strip_model()->GetWebContentsAt(1),
auth_requestor_1_->cert_request_info_,
- base::Bind(&SSLClientAuthRequestorMock::CertificateSelected,
- auth_requestor_1_));
+ auth_requestor_1_->CreateDelegate());
selector_1_->Init();
selector_1_->Show();
selector_2_ = new SSLClientCertificateSelector(
browser()->tab_strip_model()->GetWebContentsAt(2),
auth_requestor_2_->cert_request_info_,
- base::Bind(&SSLClientAuthRequestorMock::CertificateSelected,
- auth_requestor_2_));
+ auth_requestor_2_->CreateDelegate());
selector_2_->Init();
selector_2_->Show();
@@ -256,8 +253,7 @@
selector_1_ = new SSLClientCertificateSelector(
browser_1_->tab_strip_model()->GetActiveWebContents(),
auth_requestor_1_->cert_request_info_,
- base::Bind(&SSLClientAuthRequestorMock::CertificateSelected,
- auth_requestor_1_));
+ auth_requestor_1_->CreateDelegate());
selector_1_->Init();
selector_1_->Show();
@@ -303,7 +299,7 @@
IN_PROC_BROWSER_TEST_F(SSLClientCertificateSelectorTest, MAYBE_SelectNone) {
- EXPECT_CALL(*auth_requestor_.get(), CertificateSelected(NULL));
+ EXPECT_CALL(*auth_requestor_.get(), CancelCertificateSelection());
// Let the mock get checked on destruction.
}
@@ -343,7 +339,7 @@
// Now let the default selection for auth_requestor_ mock get checked on
// destruction.
- EXPECT_CALL(*auth_requestor_.get(), CertificateSelected(NULL));
+ EXPECT_CALL(*auth_requestor_.get(), CancelCertificateSelection());
}
IN_PROC_BROWSER_TEST_F(SSLClientCertificateSelectorMultiTabTest, SelectSecond) {
@@ -371,7 +367,7 @@
// Now let the default selection for auth_requestor_ mock get checked on
// destruction.
- EXPECT_CALL(*auth_requestor_.get(), CertificateSelected(NULL));
+ EXPECT_CALL(*auth_requestor_.get(), CancelCertificateSelection());
}
IN_PROC_BROWSER_TEST_F(SSLClientCertificateSelectorMultiProfileTest, Escape) {
@@ -385,7 +381,7 @@
// Now let the default selection for auth_requestor_ mock get checked on
// destruction.
- EXPECT_CALL(*auth_requestor_.get(), CertificateSelected(NULL));
+ EXPECT_CALL(*auth_requestor_.get(), CancelCertificateSelection());
}
IN_PROC_BROWSER_TEST_F(SSLClientCertificateSelectorMultiProfileTest,
@@ -401,5 +397,5 @@
// Now let the default selection for auth_requestor_ mock get checked on
// destruction.
- EXPECT_CALL(*auth_requestor_.get(), CertificateSelected(NULL));
+ EXPECT_CALL(*auth_requestor_.get(), CancelCertificateSelection());
}
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 50a64b7..9c7ab33 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -218,6 +218,7 @@
'browser/extensions/background_app_browsertest.cc',
'browser/extensions/background_page_apitest.cc',
'browser/extensions/background_scripts_apitest.cc',
+ 'browser/extensions/background_xhr_browsertest.cc',
'browser/extensions/browsertest_util_browsertest.cc',
'browser/extensions/chrome_app_api_browsertest.cc',
'browser/extensions/chrome_ui_overrides_browsertest.cc',
diff --git a/chrome/test/data/extensions/background_xhr/background.js b/chrome/test/data/extensions/background_xhr/background.js
new file mode 100644
index 0000000..52c8e34
--- /dev/null
+++ b/chrome/test/data/extensions/background_xhr/background.js
@@ -0,0 +1,13 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+chrome.runtime.onMessage.addListener(function(message, sender, sendResponse) {
+ if (message.type == "xhr") {
+ var xhr = new XMLHttpRequest();
+ xhr.open(message.method, message.url);
+ xhr.send();
+ } else {
+ console.error("Unknown message: " + JSON.stringify(message));
+ }
+});
diff --git a/chrome/test/data/extensions/background_xhr/manifest.json b/chrome/test/data/extensions/background_xhr/manifest.json
new file mode 100644
index 0000000..70285c6
--- /dev/null
+++ b/chrome/test/data/extensions/background_xhr/manifest.json
@@ -0,0 +1,9 @@
+{
+ "name": "Extension tests which issue XHRs from the background page.",
+ "version": "1.0",
+ "manifest_version": 2,
+ "background": {
+ "scripts": ["background.js"]
+ },
+ "permissions": ["webRequest", "<all_urls>"]
+}
diff --git a/chrome/test/data/extensions/background_xhr/test_http_auth.html b/chrome/test/data/extensions/background_xhr/test_http_auth.html
new file mode 100644
index 0000000..b496e0f
--- /dev/null
+++ b/chrome/test/data/extensions/background_xhr/test_http_auth.html
@@ -0,0 +1,6 @@
+<!--
+ * Copyright 2015 The Chromium Authors. All rights reserved. Use of this
+ * source code is governed by a BSD-style license that can be found in the
+ * LICENSE file.
+-->
+<script src="test_http_auth.js"></script>
diff --git a/chrome/test/data/extensions/background_xhr/test_http_auth.js b/chrome/test/data/extensions/background_xhr/test_http_auth.js
new file mode 100644
index 0000000..beb26e8
--- /dev/null
+++ b/chrome/test/data/extensions/background_xhr/test_http_auth.js
@@ -0,0 +1,23 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// TODO(davidben): When URLSearchParams is stable and implemented, switch this
+// (and a lot of other test code) to it. https://crbug.com/303152
+var url = decodeURIComponent(/url=([^&]*)/.exec(location.search)[1]);
+var filter = {urls: [url], types: ["xmlhttprequest"]};
+
+chrome.webRequest.onCompleted.addListener(function(details) {
+ chrome.test.assertEq(-1, details.tabId);
+ chrome.test.assertEq(url, details.url);
+ chrome.test.assertEq("xmlhttprequest", details.type);
+ chrome.test.assertEq(401, details.statusCode);
+
+ chrome.test.notifyPass();
+}, filter);
+
+chrome.webRequest.onErrorOccurred.addListener(function(details) {
+ chrome.test.notifyFail("Request failed");
+}, filter);
+
+chrome.runtime.sendMessage({type: "xhr", method: "GET", url: url});
diff --git a/chrome/test/data/extensions/background_xhr/test_tls_client_auth.html b/chrome/test/data/extensions/background_xhr/test_tls_client_auth.html
new file mode 100644
index 0000000..61edc57
--- /dev/null
+++ b/chrome/test/data/extensions/background_xhr/test_tls_client_auth.html
@@ -0,0 +1,6 @@
+<!--
+ * Copyright 2015 The Chromium Authors. All rights reserved. Use of this
+ * source code is governed by a BSD-style license that can be found in the
+ * LICENSE file.
+-->
+<script src="test_tls_client_auth.js"></script>
diff --git a/chrome/test/data/extensions/background_xhr/test_tls_client_auth.js b/chrome/test/data/extensions/background_xhr/test_tls_client_auth.js
new file mode 100644
index 0000000..4a48af7
--- /dev/null
+++ b/chrome/test/data/extensions/background_xhr/test_tls_client_auth.js
@@ -0,0 +1,23 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// TODO(davidben): When URLSearchParams is stable and implemented, switch this
+// (and a lot of other test code) to it. https://crbug.com/303152
+var url = decodeURIComponent(/url=([^&]*)/.exec(location.search)[1]);
+var filter = {urls: [url], types: ["xmlhttprequest"]};
+
+chrome.webRequest.onCompleted.addListener(function(details) {
+ chrome.test.notifyFail("Request completed unexpectedly");
+}, filter);
+
+chrome.webRequest.onErrorOccurred.addListener(function(details) {
+ chrome.test.assertEq(-1, details.tabId);
+ chrome.test.assertEq(url, details.url);
+ chrome.test.assertEq("xmlhttprequest", details.type);
+ chrome.test.assertEq("net::ERR_SSL_CLIENT_AUTH_CERT_NEEDED", details.error);
+
+ chrome.test.notifyPass();
+}, filter);
+
+chrome.runtime.sendMessage({type: "xhr", method: "GET", url: url});
diff --git a/chromecast/browser/cast_content_browser_client.cc b/chromecast/browser/cast_content_browser_client.cc
index 56a82502..9793e68 100644
--- a/chromecast/browser/cast_content_browser_client.cc
+++ b/chromecast/browser/cast_content_browser_client.cc
@@ -29,6 +29,7 @@
#include "components/network_hints/browser/network_hints_message_filter.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/certificate_request_result_type.h"
+#include "content/public/browser/client_certificate_delegate.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/resource_dispatcher_host.h"
#include "content/public/common/content_descriptors.h"
@@ -194,16 +195,15 @@
}
void CastContentBrowserClient::SelectClientCertificate(
- int render_process_id,
- int render_view_id,
+ WebContents* web_contents,
net::SSLCertRequestInfo* cert_request_info,
- const base::Callback<void(net::X509Certificate*)>& callback) {
+ scoped_ptr<content::ClientCertificateDelegate> delegate) {
GURL requesting_url("https://" + cert_request_info->host_and_port.ToString());
if (!requesting_url.is_valid()) {
LOG(ERROR) << "Invalid URL string: "
<< requesting_url.possibly_invalid_spec();
- callback.Run(NULL);
+ delegate->SelectClientCertificate(nullptr);
return;
}
@@ -214,16 +214,16 @@
// it, because CastNetworkDelegate is bound to the IO thread.
// Subsequently, the callback must then itself be performed back here
// on the UI thread.
+ //
+ // TODO(davidben): Stop using child ID to identify an app.
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
content::BrowserThread::PostTaskAndReplyWithResult(
- content::BrowserThread::IO,
- FROM_HERE,
- base::Bind(
- &CastContentBrowserClient::SelectClientCertificateOnIOThread,
- base::Unretained(this),
- requesting_url,
- render_process_id),
- callback);
+ content::BrowserThread::IO, FROM_HERE,
+ base::Bind(&CastContentBrowserClient::SelectClientCertificateOnIOThread,
+ base::Unretained(this), requesting_url,
+ web_contents->GetRenderProcessHost()->GetID()),
+ base::Bind(&content::ClientCertificateDelegate::ContinueWithCertificate,
+ delegate.Pass()));
}
net::X509Certificate*
diff --git a/chromecast/browser/cast_content_browser_client.h b/chromecast/browser/cast_content_browser_client.h
index ace72869..41846746 100644
--- a/chromecast/browser/cast_content_browser_client.h
+++ b/chromecast/browser/cast_content_browser_client.h
@@ -68,10 +68,9 @@
const base::Callback<void(bool)>& callback,
content::CertificateRequestResultType* result) override;
void SelectClientCertificate(
- int render_process_id,
- int render_frame_id,
+ content::WebContents* web_contents,
net::SSLCertRequestInfo* cert_request_info,
- const base::Callback<void(net::X509Certificate*)>& callback) override;
+ scoped_ptr<content::ClientCertificateDelegate> delegate) override;
bool CanCreateWindow(
const GURL& opener_url,
const GURL& opener_top_level_frame_url,
diff --git a/content/browser/loader/resource_loader.cc b/content/browser/loader/resource_loader.cc
index b462a68..7bb55f1 100644
--- a/content/browser/loader/resource_loader.cc
+++ b/content/browser/loader/resource_loader.cc
@@ -311,8 +311,7 @@
<< "OnCertificateRequested called with ssl_client_auth_handler pending";
ssl_client_auth_handler_.reset(new SSLClientAuthHandler(
GetRequestInfo()->GetContext()->CreateClientCertStore(), request_.get(),
- cert_info, base::Bind(&ResourceLoader::ContinueWithCertificate,
- weak_ptr_factory_.GetWeakPtr())));
+ cert_info, this));
ssl_client_auth_handler_->SelectCertificate();
}
@@ -503,6 +502,18 @@
request_->ContinueDespiteLastError();
}
+void ResourceLoader::ContinueWithCertificate(net::X509Certificate* cert) {
+ DCHECK(ssl_client_auth_handler_);
+ ssl_client_auth_handler_.reset();
+ request_->ContinueWithCertificate(cert);
+}
+
+void ResourceLoader::CancelCertificateSelection() {
+ DCHECK(ssl_client_auth_handler_);
+ ssl_client_auth_handler_.reset();
+ request_->CancelWithError(net::ERR_SSL_CLIENT_AUTH_CERT_NEEDED);
+}
+
void ResourceLoader::Resume() {
DCHECK(!is_transferring_);
@@ -851,9 +862,4 @@
}
}
-void ResourceLoader::ContinueWithCertificate(net::X509Certificate* cert) {
- ssl_client_auth_handler_.reset();
- request_->ContinueWithCertificate(cert);
-}
-
} // namespace content
diff --git a/content/browser/loader/resource_loader.h b/content/browser/loader/resource_loader.h
index 92afbd2..bb21509e 100644
--- a/content/browser/loader/resource_loader.h
+++ b/content/browser/loader/resource_loader.h
@@ -5,10 +5,10 @@
#ifndef CONTENT_BROWSER_LOADER_RESOURCE_LOADER_H_
#define CONTENT_BROWSER_LOADER_RESOURCE_LOADER_H_
-#include "base/gtest_prod_util.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
#include "content/browser/loader/resource_handler.h"
+#include "content/browser/ssl/ssl_client_auth_handler.h"
#include "content/browser/ssl/ssl_error_handler.h"
#include "content/common/content_export.h"
#include "content/public/browser/resource_controller.h"
@@ -23,13 +23,13 @@
class ResourceDispatcherHostLoginDelegate;
class ResourceLoaderDelegate;
class ResourceRequestInfoImpl;
-class SSLClientAuthHandler;
// This class is responsible for driving the URLRequest (i.e., calling Start,
// Read, and servicing events). It has a ResourceHandler, which is typically a
// chain of ResourceHandlers, and is the ResourceController for its handler.
class CONTENT_EXPORT ResourceLoader : public net::URLRequest::Delegate,
public SSLErrorHandler::Delegate,
+ public SSLClientAuthHandler::Delegate,
public ResourceController {
public:
ResourceLoader(scoped_ptr<net::URLRequest> request,
@@ -55,10 +55,6 @@
void OnUploadProgressACK();
private:
- FRIEND_TEST_ALL_PREFIXES(ResourceLoaderTest, ClientCertStoreLookup);
- FRIEND_TEST_ALL_PREFIXES(ResourceLoaderTest, ClientCertStoreNull);
- FRIEND_TEST_ALL_PREFIXES(ResourceLoaderTest, ClientCertStoreAsyncCancel);
-
// net::URLRequest::Delegate implementation:
void OnReceivedRedirect(net::URLRequest* request,
const net::RedirectInfo& redirect_info,
@@ -78,6 +74,10 @@
void CancelSSLRequest(int error, const net::SSLInfo* ssl_info) override;
void ContinueSSLRequest() override;
+ // SSLClientAuthHandler::Delegate implementation.
+ void ContinueWithCertificate(net::X509Certificate* cert) override;
+ void CancelCertificateSelection() override;
+
// ResourceController implementation:
void Resume() override;
void Cancel() override;
@@ -103,7 +103,6 @@
void ResponseCompleted();
void CallDidFinishLoading();
void RecordHistograms();
- void ContinueWithCertificate(net::X509Certificate* cert);
bool is_deferred() const { return deferred_stage_ != DEFERRED_NONE; }
diff --git a/content/browser/loader/resource_loader_unittest.cc b/content/browser/loader/resource_loader_unittest.cc
index 92241b6..89f8236 100644
--- a/content/browser/loader/resource_loader_unittest.cc
+++ b/content/browser/loader/resource_loader_unittest.cc
@@ -6,24 +6,31 @@
#include "base/files/file.h"
#include "base/files/file_util.h"
+#include "base/macros.h"
#include "base/message_loop/message_loop_proxy.h"
#include "base/run_loop.h"
#include "content/browser/browser_thread_impl.h"
#include "content/browser/loader/redirect_to_file_resource_handler.h"
#include "content/browser/loader/resource_loader_delegate.h"
+#include "content/public/browser/client_certificate_delegate.h"
#include "content/public/browser/resource_request_info.h"
#include "content/public/common/resource_response.h"
#include "content/public/test/mock_resource_context.h"
+#include "content/public/test/test_browser_context.h"
#include "content/public/test/test_browser_thread_bundle.h"
+#include "content/public/test/test_renderer_host.h"
#include "content/test/test_content_browser_client.h"
+#include "content/test/test_web_contents.h"
#include "ipc/ipc_message.h"
#include "net/base/io_buffer.h"
#include "net/base/mock_file_stream.h"
+#include "net/base/net_errors.h"
#include "net/base/request_priority.h"
#include "net/cert/x509_certificate.h"
#include "net/ssl/client_cert_store.h"
#include "net/ssl/ssl_cert_request_info.h"
#include "net/url_request/url_request.h"
+#include "net/url_request/url_request_job_factory.h"
#include "net/url_request/url_request_job_factory_impl.h"
#include "net/url_request/url_request_test_job.h"
#include "net/url_request/url_request_test_util.h"
@@ -53,7 +60,6 @@
int* request_count,
std::vector<std::string>* requested_authorities)
: response_(response),
- async_(false),
requested_authorities_(requested_authorities),
request_count_(request_count) {
requested_authorities_->clear();
@@ -62,9 +68,6 @@
~ClientCertStoreStub() override {}
- // Configures whether the certificates are returned asynchronously or not.
- void set_async(bool async) { async_ = async; }
-
// net::ClientCertStore:
void GetClientCerts(const net::SSLCertRequestInfo& cert_request_info,
net::CertificateList* selected_certs,
@@ -73,20 +76,88 @@
++(*request_count_);
*selected_certs = response_;
- if (async_) {
- base::MessageLoop::current()->PostTask(FROM_HERE, callback);
- } else {
- callback.Run();
- }
+ callback.Run();
}
private:
const net::CertificateList response_;
- bool async_;
std::vector<std::string>* requested_authorities_;
int* request_count_;
};
+// Client certificate store which destroys its resource loader before the
+// asynchronous GetClientCerts callback is called.
+class LoaderDestroyingCertStore : public net::ClientCertStore {
+ public:
+ // Creates a client certificate store which, when looked up, posts a task to
+ // reset |loader| and then call the callback. The caller is responsible for
+ // ensuring the pointers remain valid until the process is complete.
+ explicit LoaderDestroyingCertStore(scoped_ptr<ResourceLoader>* loader)
+ : loader_(loader) {}
+
+ // net::ClientCertStore:
+ void GetClientCerts(const net::SSLCertRequestInfo& cert_request_info,
+ net::CertificateList* selected_certs,
+ const base::Closure& callback) override {
+ // Don't destroy |loader_| while it's on the stack.
+ base::MessageLoop::current()->PostTask(
+ FROM_HERE, base::Bind(&LoaderDestroyingCertStore::DoCallback,
+ base::Unretained(loader_), callback));
+ }
+
+ private:
+ static void DoCallback(scoped_ptr<ResourceLoader>* loader,
+ const base::Closure& callback) {
+ loader->reset();
+ callback.Run();
+ }
+
+ scoped_ptr<ResourceLoader>* loader_;
+};
+
+// A mock URLRequestJob which simulates an SSL client auth request.
+class MockClientCertURLRequestJob : public net::URLRequestTestJob {
+ public:
+ MockClientCertURLRequestJob(net::URLRequest* request,
+ net::NetworkDelegate* network_delegate)
+ : net::URLRequestTestJob(request, network_delegate) {}
+
+ static std::vector<std::string> test_authorities() {
+ return std::vector<std::string>(1, "dummy");
+ }
+
+ // net::URLRequestTestJob:
+ void Start() override {
+ scoped_refptr<net::SSLCertRequestInfo> cert_request_info(
+ new net::SSLCertRequestInfo);
+ cert_request_info->cert_authorities = test_authorities();
+ base::MessageLoop::current()->PostTask(
+ FROM_HERE,
+ base::Bind(&MockClientCertURLRequestJob::NotifyCertificateRequested,
+ this, cert_request_info));
+ }
+
+ void ContinueWithCertificate(net::X509Certificate* cert) override {
+ net::URLRequestTestJob::Start();
+ }
+
+ private:
+ ~MockClientCertURLRequestJob() override {}
+
+ DISALLOW_COPY_AND_ASSIGN(MockClientCertURLRequestJob);
+};
+
+class MockClientCertJobProtocolHandler
+ : public net::URLRequestJobFactory::ProtocolHandler {
+ public:
+ // URLRequestJobFactory::ProtocolHandler implementation:
+ net::URLRequestJob* MaybeCreateJob(
+ net::URLRequest* request,
+ net::NetworkDelegate* network_delegate) const override {
+ return new MockClientCertURLRequestJob(request, network_delegate);
+ }
+};
+
// Arbitrary read buffer size.
const int kReadBufSize = 1024;
@@ -239,25 +310,28 @@
SelectCertificateBrowserClient() : call_count_(0) {}
void SelectClientCertificate(
- int render_process_id,
- int render_view_id,
+ WebContents* web_contents,
net::SSLCertRequestInfo* cert_request_info,
- const base::Callback<void(net::X509Certificate*)>& callback) override {
+ scoped_ptr<ClientCertificateDelegate> delegate) override {
++call_count_;
passed_certs_ = cert_request_info->client_certs;
+ delegate_ = delegate.Pass();
}
- int call_count() {
- return call_count_;
+ int call_count() { return call_count_; }
+ net::CertificateList passed_certs() { return passed_certs_; }
+
+ void ContinueWithCertificate(net::X509Certificate* cert) {
+ delegate_->ContinueWithCertificate(cert);
+ delegate_.reset();
}
- net::CertificateList passed_certs() {
- return passed_certs_;
- }
+ void CancelCertificateSelection() { delegate_.reset(); }
private:
net::CertificateList passed_certs_;
int call_count_;
+ scoped_ptr<ClientCertificateDelegate> delegate_;
};
class ResourceContextStub : public MockResourceContext {
@@ -297,19 +371,19 @@
resource_context_(&test_url_request_context_),
raw_ptr_resource_handler_(NULL),
raw_ptr_to_request_(NULL) {
- job_factory_.SetProtocolHandler(
- "test", net::URLRequestTestJob::CreateProtocolHandler());
test_url_request_context_.set_job_factory(&job_factory_);
}
- GURL test_url() const {
- return net::URLRequestTestJob::test_url_1();
- }
+ GURL test_url() const { return net::URLRequestTestJob::test_url_1(); }
std::string test_data() const {
return net::URLRequestTestJob::test_data_1();
}
+ virtual net::URLRequestJobFactory::ProtocolHandler* CreateProtocolHandler() {
+ return net::URLRequestTestJob::CreateProtocolHandler();
+ }
+
virtual scoped_ptr<ResourceHandler> WrapResourceHandler(
scoped_ptr<ResourceHandlerStub> leaf_handler,
net::URLRequest* request) {
@@ -317,8 +391,14 @@
}
void SetUp() override {
- const int kRenderProcessId = 1;
- const int kRenderViewId = 2;
+ job_factory_.SetProtocolHandler("test", CreateProtocolHandler());
+
+ browser_context_.reset(new TestBrowserContext());
+ scoped_refptr<SiteInstance> site_instance =
+ SiteInstance::Create(browser_context_.get());
+ web_contents_.reset(
+ TestWebContents::Create(browser_context_.get(), site_instance.get()));
+ RenderFrameHost* rfh = web_contents_->GetMainFrame();
scoped_ptr<net::URLRequest> request(
resource_context_.GetRequestContext()->CreateRequest(
@@ -327,16 +407,12 @@
NULL /* delegate */,
NULL /* cookie_store */));
raw_ptr_to_request_ = request.get();
- ResourceRequestInfo::AllocateForTesting(request.get(),
- RESOURCE_TYPE_MAIN_FRAME,
- &resource_context_,
- kRenderProcessId,
- kRenderViewId,
- MSG_ROUTING_NONE,
- true, // is_main_frame
- false, // parent_is_main_frame
- true, // allow_download
- false); // is_async
+ ResourceRequestInfo::AllocateForTesting(
+ request.get(), RESOURCE_TYPE_MAIN_FRAME, &resource_context_,
+ rfh->GetProcess()->GetID(), rfh->GetRenderViewHost()->GetRoutingID(),
+ rfh->GetRoutingID(), true /* is_main_frame */,
+ false /* parent_is_main_frame */, true /* allow_download */,
+ false /* is_async */);
scoped_ptr<ResourceHandlerStub> resource_handler(
new ResourceHandlerStub(request.get()));
raw_ptr_resource_handler_ = resource_handler.get();
@@ -346,6 +422,14 @@
this));
}
+ void TearDown() override {
+ // Destroy the WebContents and pump the event loop before destroying
+ // |rvh_test_enabler_| and |thread_bundle_|. This lets asynchronous cleanup
+ // tasks complete.
+ web_contents_.reset();
+ base::RunLoop().RunUntilIdle();
+ }
+
// ResourceLoaderDelegate:
ResourceDispatcherHostLoginDelegate* CreateLoginDelegate(
ResourceLoader* loader,
@@ -362,11 +446,14 @@
void DidReceiveResponse(ResourceLoader* loader) override {}
void DidFinishLoading(ResourceLoader* loader) override {}
- content::TestBrowserThreadBundle thread_bundle_;
+ TestBrowserThreadBundle thread_bundle_;
+ RenderViewHostTestEnabler rvh_test_enabler_;
net::URLRequestJobFactoryImpl job_factory_;
net::TestURLRequestContext test_url_request_context_;
ResourceContextStub resource_context_;
+ scoped_ptr<TestBrowserContext> browser_context_;
+ scoped_ptr<TestWebContents> web_contents_;
// The ResourceLoader owns the URLRequest and the ResourceHandler.
ResourceHandlerStub* raw_ptr_resource_handler_;
@@ -374,11 +461,15 @@
scoped_ptr<ResourceLoader> loader_;
};
-// Verifies if a call to net::UrlRequest::Delegate::OnCertificateRequested()
-// causes client cert store to be queried for certificates and if the returned
-// certificates are correctly passed to the content browser client for
-// selection.
-TEST_F(ResourceLoaderTest, ClientCertStoreLookup) {
+class ClientCertResourceLoaderTest : public ResourceLoaderTest {
+ protected:
+ net::URLRequestJobFactory::ProtocolHandler* CreateProtocolHandler() override {
+ return new MockClientCertJobProtocolHandler;
+ }
+};
+
+// Tests that client certificates are requested with ClientCertStore lookup.
+TEST_F(ClientCertResourceLoaderTest, WithStoreLookup) {
// Set up the test client cert store.
int store_request_count;
std::vector<std::string> store_requested_authorities;
@@ -388,91 +479,125 @@
dummy_certs, &store_request_count, &store_requested_authorities));
resource_context_.SetClientCertStore(test_store.Pass());
- // Prepare a dummy certificate request.
- scoped_refptr<net::SSLCertRequestInfo> cert_request_info(
- new net::SSLCertRequestInfo());
- std::vector<std::string> dummy_authority(1, "dummy");
- cert_request_info->cert_authorities = dummy_authority;
-
// Plug in test content browser client.
SelectCertificateBrowserClient test_client;
ContentBrowserClient* old_client = SetBrowserClientForTesting(&test_client);
- // Everything is set up. Trigger the resource loader certificate request event
- // and run the message loop.
- loader_->OnCertificateRequested(raw_ptr_to_request_, cert_request_info.get());
+ // Start the request and wait for it to pause.
+ loader_->StartRequest();
base::RunLoop().RunUntilIdle();
- // Restore the original content browser client.
- SetBrowserClientForTesting(old_client);
+ EXPECT_FALSE(raw_ptr_resource_handler_->received_response_completed());
// Check if the test store was queried against correct |cert_authorities|.
EXPECT_EQ(1, store_request_count);
- EXPECT_EQ(dummy_authority, store_requested_authorities);
+ EXPECT_EQ(MockClientCertURLRequestJob::test_authorities(),
+ store_requested_authorities);
// Check if the retrieved certificates were passed to the content browser
// client.
EXPECT_EQ(1, test_client.call_count());
EXPECT_EQ(dummy_certs, test_client.passed_certs());
+
+ // Continue the request.
+ test_client.ContinueWithCertificate(dummy_certs[0].get());
+ base::RunLoop().RunUntilIdle();
+ EXPECT_TRUE(raw_ptr_resource_handler_->received_response_completed());
+ EXPECT_EQ(net::OK, raw_ptr_resource_handler_->status().error());
+
+ // Restore the original content browser client.
+ SetBrowserClientForTesting(old_client);
}
-// Verifies if a call to net::URLRequest::Delegate::OnCertificateRequested()
-// on a platform with a NULL client cert store still calls the content browser
-// client for selection.
-TEST_F(ResourceLoaderTest, ClientCertStoreNull) {
- // Prepare a dummy certificate request.
- scoped_refptr<net::SSLCertRequestInfo> cert_request_info(
- new net::SSLCertRequestInfo());
- std::vector<std::string> dummy_authority(1, "dummy");
- cert_request_info->cert_authorities = dummy_authority;
-
+// Tests that client certificates are requested on a platform with NULL
+// ClientCertStore.
+TEST_F(ClientCertResourceLoaderTest, WithNullStore) {
// Plug in test content browser client.
SelectCertificateBrowserClient test_client;
ContentBrowserClient* old_client = SetBrowserClientForTesting(&test_client);
- // Everything is set up. Trigger the resource loader certificate request event
- // and run the message loop.
- loader_->OnCertificateRequested(raw_ptr_to_request_, cert_request_info.get());
+ // Start the request and wait for it to pause.
+ loader_->StartRequest();
base::RunLoop().RunUntilIdle();
- // Restore the original content browser client.
- SetBrowserClientForTesting(old_client);
-
// Check if the SelectClientCertificate was called on the content browser
// client.
EXPECT_EQ(1, test_client.call_count());
EXPECT_EQ(net::CertificateList(), test_client.passed_certs());
+
+ // Continue the request.
+ scoped_refptr<net::X509Certificate> cert(
+ new net::X509Certificate("test", "test", base::Time(), base::Time()));
+ test_client.ContinueWithCertificate(cert.get());
+ base::RunLoop().RunUntilIdle();
+ EXPECT_TRUE(raw_ptr_resource_handler_->received_response_completed());
+ EXPECT_EQ(net::OK, raw_ptr_resource_handler_->status().error());
+
+ // Restore the original content browser client.
+ SetBrowserClientForTesting(old_client);
}
-TEST_F(ResourceLoaderTest, ClientCertStoreAsyncCancel) {
- // Set up the test client cert store.
- int store_request_count;
- std::vector<std::string> store_requested_authorities;
- scoped_ptr<ClientCertStoreStub> test_store(
- new ClientCertStoreStub(net::CertificateList(), &store_request_count,
- &store_requested_authorities));
- test_store->set_async(true);
- EXPECT_EQ(0, store_request_count);
+// Tests that the ContentBrowserClient may cancel a certificate request.
+TEST_F(ClientCertResourceLoaderTest, CancelSelection) {
+ // Plug in test content browser client.
+ SelectCertificateBrowserClient test_client;
+ ContentBrowserClient* old_client = SetBrowserClientForTesting(&test_client);
+
+ // Start the request and wait for it to pause.
+ loader_->StartRequest();
+ base::RunLoop().RunUntilIdle();
+
+ // Check if the SelectClientCertificate was called on the content browser
+ // client.
+ EXPECT_EQ(1, test_client.call_count());
+ EXPECT_EQ(net::CertificateList(), test_client.passed_certs());
+
+ // Cancel the request.
+ test_client.CancelCertificateSelection();
+ base::RunLoop().RunUntilIdle();
+ EXPECT_TRUE(raw_ptr_resource_handler_->received_response_completed());
+ EXPECT_EQ(net::ERR_SSL_CLIENT_AUTH_CERT_NEEDED,
+ raw_ptr_resource_handler_->status().error());
+
+ // Restore the original content browser client.
+ SetBrowserClientForTesting(old_client);
+}
+
+// Verifies that requests without WebContents attached abort.
+TEST_F(ClientCertResourceLoaderTest, NoWebContents) {
+ // Destroy the WebContents before starting the request.
+ web_contents_.reset();
+
+ // Plug in test content browser client.
+ SelectCertificateBrowserClient test_client;
+ ContentBrowserClient* old_client = SetBrowserClientForTesting(&test_client);
+
+ // Start the request and wait for it to pause.
+ loader_->StartRequest();
+ base::RunLoop().RunUntilIdle();
+
+ // Check that SelectClientCertificate wasn't called and the request aborted.
+ EXPECT_EQ(0, test_client.call_count());
+ EXPECT_TRUE(raw_ptr_resource_handler_->received_response_completed());
+ EXPECT_EQ(net::ERR_SSL_CLIENT_AUTH_CERT_NEEDED,
+ raw_ptr_resource_handler_->status().error());
+
+ // Restore the original content browser client.
+ SetBrowserClientForTesting(old_client);
+}
+
+// Verifies that ClientCertStore's callback doesn't crash if called after the
+// loader is destroyed.
+TEST_F(ClientCertResourceLoaderTest, StoreAsyncCancel) {
+ scoped_ptr<LoaderDestroyingCertStore> test_store(
+ new LoaderDestroyingCertStore(&loader_));
resource_context_.SetClientCertStore(test_store.Pass());
- // Prepare a dummy certificate request.
- scoped_refptr<net::SSLCertRequestInfo> cert_request_info(
- new net::SSLCertRequestInfo());
- std::vector<std::string> dummy_authority(1, "dummy");
- cert_request_info->cert_authorities = dummy_authority;
+ loader_->StartRequest();
+ base::RunLoop().RunUntilIdle();
+ EXPECT_FALSE(loader_);
- // Everything is set up. Trigger the resource loader certificate request
- // event.
- loader_->OnCertificateRequested(raw_ptr_to_request_, cert_request_info.get());
-
- // Check if the test store was queried against correct |cert_authorities|.
- EXPECT_EQ(1, store_request_count);
- EXPECT_EQ(dummy_authority, store_requested_authorities);
-
- // Cancel the request before the store calls the callback.
- loader_.reset();
-
- // Pump the event loop. There shouldn't be a crash when the callback is run.
+ // Pump the event loop to ensure nothing asynchronous crashes either.
base::RunLoop().RunUntilIdle();
}
diff --git a/content/browser/shared_worker/worker_browsertest.cc b/content/browser/shared_worker/worker_browsertest.cc
index 14100b9..3a0593b 100644
--- a/content/browser/shared_worker/worker_browsertest.cc
+++ b/content/browser/shared_worker/worker_browsertest.cc
@@ -11,6 +11,7 @@
#include "base/sys_info.h"
#include "base/test/test_timeouts.h"
#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/client_certificate_delegate.h"
#include "content/public/common/content_paths.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/content_browser_test.h"
@@ -19,15 +20,45 @@
#include "content/shell/browser/shell.h"
#include "content/shell/browser/shell_content_browser_client.h"
#include "content/shell/browser/shell_resource_dispatcher_host_delegate.h"
+#include "net/base/escape.h"
#include "net/base/test_data_directory.h"
#include "net/test/spawned_test_server/spawned_test_server.h"
#include "url/gurl.h"
namespace content {
+namespace {
+
+bool SupportsSharedWorker() {
+#if defined(OS_ANDROID)
+ // SharedWorkers are not enabled on Android. https://crbug.com/154571
+ //
+ // TODO(davidben): Move other SharedWorker exclusions from
+ // build/android/pylib/gtest/filter/ inline.
+ return false;
+#else
+ return true;
+#endif
+}
+
+} // namespace
+
class WorkerTest : public ContentBrowserTest {
public:
- WorkerTest() {}
+ WorkerTest() : select_certificate_count_(0) {}
+
+ void SetUpOnMainThread() override {
+ ShellContentBrowserClient::Get()->set_select_client_certificate_callback(
+ base::Bind(&WorkerTest::OnSelectClientCertificate,
+ base::Unretained(this)));
+ }
+
+ void TearDownOnMainThread() override {
+ ShellContentBrowserClient::Get()->set_select_client_certificate_callback(
+ base::Closure());
+ }
+
+ int select_certificate_count() const { return select_certificate_count_; }
GURL GetTestURL(const std::string& test_case, const std::string& query) {
base::FilePath test_file_path = GetTestFilePath(
@@ -64,6 +95,11 @@
shell()->LoadURL(url);
runner->Run();
}
+
+ private:
+ void OnSelectClientCertificate() { select_certificate_count_++; }
+
+ int select_certificate_count_;
};
IN_PROC_BROWSER_TEST_F(WorkerTest, SingleWorker) {
@@ -102,14 +138,55 @@
NavigateAndWaitForAuth(url);
}
-// Make sure that auth dialog is displayed from shared worker context.
+// Make sure that HTTP auth dialog is displayed from shared worker context.
// http://crbug.com/33344
+//
+// TODO(davidben): HTTP auth dialogs are no longer displayed on shared workers,
+// but this test only tests that the delegate is called. Move handling the
+// WebContentsless case from chrome/ to content/ and adjust the test
+// accordingly.
IN_PROC_BROWSER_TEST_F(WorkerTest, SharedWorkerHttpAuth) {
ASSERT_TRUE(test_server()->Start());
GURL url = test_server()->GetURL("files/workers/shared_worker_auth.html");
NavigateAndWaitForAuth(url);
}
+// Tests that TLS client auth prompts for normal workers.
+IN_PROC_BROWSER_TEST_F(WorkerTest, WorkerTlsClientAuth) {
+ // Launch HTTPS server.
+ net::SpawnedTestServer::SSLOptions ssl_options;
+ ssl_options.request_client_certificate = true;
+ net::SpawnedTestServer https_server(
+ net::SpawnedTestServer::TYPE_HTTPS, ssl_options,
+ base::FilePath(FILE_PATH_LITERAL("content/test/data")));
+ ASSERT_TRUE(https_server.Start());
+
+ RunTest("worker_tls_client_auth.html",
+ "url=" +
+ net::EscapeQueryParamValue(https_server.GetURL("").spec(), true));
+ EXPECT_EQ(1, select_certificate_count());
+}
+
+// Tests that TLS client auth does not prompt for a shared worker; shared
+// workers are not associated with a WebContents.
+IN_PROC_BROWSER_TEST_F(WorkerTest, SharedWorkerTlsClientAuth) {
+ if (!SupportsSharedWorker())
+ return;
+
+ // Launch HTTPS server.
+ net::SpawnedTestServer::SSLOptions ssl_options;
+ ssl_options.request_client_certificate = true;
+ net::SpawnedTestServer https_server(
+ net::SpawnedTestServer::TYPE_HTTPS, ssl_options,
+ base::FilePath(FILE_PATH_LITERAL("content/test/data")));
+ ASSERT_TRUE(https_server.Start());
+
+ RunTest("worker_tls_client_auth.html",
+ "shared=true&url=" +
+ net::EscapeQueryParamValue(https_server.GetURL("").spec(), true));
+ EXPECT_EQ(0, select_certificate_count());
+}
+
IN_PROC_BROWSER_TEST_F(WorkerTest, WebSocketSharedWorker) {
// Launch WebSocket server.
net::SpawnedTestServer ws_server(net::SpawnedTestServer::TYPE_WS,
diff --git a/content/browser/ssl/ssl_client_auth_handler.cc b/content/browser/ssl/ssl_client_auth_handler.cc
index d8edc5d..e528643 100644
--- a/content/browser/ssl/ssl_client_auth_handler.cc
+++ b/content/browser/ssl/ssl_client_auth_handler.cc
@@ -7,8 +7,11 @@
#include "base/bind.h"
#include "base/logging.h"
#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/client_certificate_delegate.h"
#include "content/public/browser/content_browser_client.h"
+#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/resource_request_info.h"
+#include "content/public/browser/web_contents.h"
#include "net/cert/x509_certificate.h"
#include "net/ssl/client_cert_store.h"
#include "net/url_request/url_request.h"
@@ -17,26 +20,56 @@
namespace {
-void CertificateSelectedOnUIThread(
- const SSLClientAuthHandler::CertificateCallback& io_thread_callback,
- net::X509Certificate* cert) {
- DCHECK_CURRENTLY_ON(BrowserThread::UI);
+class ClientCertificateDelegateImpl : public ClientCertificateDelegate {
+ public:
+ explicit ClientCertificateDelegateImpl(
+ const base::WeakPtr<SSLClientAuthHandler>& handler)
+ : handler_(handler), continue_called_(false) {}
- BrowserThread::PostTask(
- BrowserThread::IO, FROM_HERE,
- base::Bind(io_thread_callback, make_scoped_refptr(cert)));
-}
+ ~ClientCertificateDelegateImpl() override {
+ if (!continue_called_) {
+ BrowserThread::PostTask(
+ BrowserThread::IO, FROM_HERE,
+ base::Bind(&SSLClientAuthHandler::CancelCertificateSelection,
+ handler_));
+ }
+ }
+
+ // ClientCertificateDelegate implementation:
+ void ContinueWithCertificate(net::X509Certificate* cert) override {
+ DCHECK(!continue_called_);
+ continue_called_ = true;
+ BrowserThread::PostTask(
+ BrowserThread::IO, FROM_HERE,
+ base::Bind(&SSLClientAuthHandler::ContinueWithCertificate, handler_,
+ make_scoped_refptr(cert)));
+ }
+
+ private:
+ base::WeakPtr<SSLClientAuthHandler> handler_;
+ bool continue_called_;
+
+ DISALLOW_COPY_AND_ASSIGN(ClientCertificateDelegateImpl);
+};
void SelectCertificateOnUIThread(
int render_process_host_id,
int render_frame_host_id,
net::SSLCertRequestInfo* cert_request_info,
- const SSLClientAuthHandler::CertificateCallback& io_thread_callback) {
+ const base::WeakPtr<SSLClientAuthHandler>& handler) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
+ scoped_ptr<ClientCertificateDelegate> delegate(
+ new ClientCertificateDelegateImpl(handler));
+
+ RenderFrameHost* rfh =
+ RenderFrameHost::FromID(render_process_host_id, render_frame_host_id);
+ WebContents* web_contents = WebContents::FromRenderFrameHost(rfh);
+ if (!web_contents)
+ return;
+
GetContentClient()->browser()->SelectClientCertificate(
- render_process_host_id, render_frame_host_id, cert_request_info,
- base::Bind(&CertificateSelectedOnUIThread, io_thread_callback));
+ web_contents, cert_request_info, delegate.Pass());
}
} // namespace
@@ -88,10 +121,10 @@
scoped_ptr<net::ClientCertStore> client_cert_store,
net::URLRequest* request,
net::SSLCertRequestInfo* cert_request_info,
- const SSLClientAuthHandler::CertificateCallback& callback)
+ SSLClientAuthHandler::Delegate* delegate)
: request_(request),
cert_request_info_(cert_request_info),
- callback_(callback),
+ delegate_(delegate),
weak_factory_(this) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -109,17 +142,41 @@
core_->GetClientCerts();
}
+// static
+void SSLClientAuthHandler::ContinueWithCertificate(
+ const base::WeakPtr<SSLClientAuthHandler>& handler,
+ net::X509Certificate* cert) {
+ if (handler)
+ handler->delegate_->ContinueWithCertificate(cert);
+}
+
+// static
+void SSLClientAuthHandler::CancelCertificateSelection(
+ const base::WeakPtr<SSLClientAuthHandler>& handler) {
+ if (handler)
+ handler->delegate_->CancelCertificateSelection();
+}
+
void SSLClientAuthHandler::DidGetClientCerts() {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
// Note that if |client_cert_store_| is NULL, we intentionally fall through to
- // DoCertificateSelected. This is for platforms where the client cert matching
- // is not performed by Chrome. Those platforms handle the cert matching before
- // showing the dialog.
+ // SelectCertificateOnUIThread. This is for platforms where the client cert
+ // matching is not performed by Chrome. Those platforms handle the cert
+ // matching before showing the dialog.
if (core_->has_client_cert_store() &&
cert_request_info_->client_certs.empty()) {
// No need to query the user if there are no certs to choose from.
- CertificateSelected(NULL);
+ //
+ // TODO(davidben): The WebContents-less check on the UI thread should come
+ // before checking ClientCertStore; ClientCertStore itself should probably
+ // be handled by the embedder (https://crbug.com/394131), especially since
+ // this doesn't work on Android (https://crbug.com/345641).
+ BrowserThread::PostTask(
+ BrowserThread::IO, FROM_HERE,
+ base::Bind(&SSLClientAuthHandler::ContinueWithCertificate,
+ weak_factory_.GetWeakPtr(),
+ scoped_refptr<net::X509Certificate>()));
return;
}
@@ -128,7 +185,10 @@
if (!ResourceRequestInfo::ForRequest(request_)->GetAssociatedRenderFrame(
&render_process_host_id, &render_frame_host_id)) {
NOTREACHED();
- CertificateSelected(NULL);
+ BrowserThread::PostTask(
+ BrowserThread::IO, FROM_HERE,
+ base::Bind(&SSLClientAuthHandler::CancelCertificateSelection,
+ weak_factory_.GetWeakPtr()));
return;
}
@@ -136,16 +196,7 @@
BrowserThread::UI, FROM_HERE,
base::Bind(&SelectCertificateOnUIThread, render_process_host_id,
render_frame_host_id, cert_request_info_,
- base::Bind(&SSLClientAuthHandler::CertificateSelected,
- weak_factory_.GetWeakPtr())));
-}
-
-void SSLClientAuthHandler::CertificateSelected(net::X509Certificate* cert) {
- DVLOG(1) << this << " DoCertificateSelected " << cert;
- DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
- callback_.Run(cert);
- // |this| may be deleted at this point.
+ weak_factory_.GetWeakPtr()));
}
} // namespace content
diff --git a/content/browser/ssl/ssl_client_auth_handler.h b/content/browser/ssl/ssl_client_auth_handler.h
index 7597687..b9c2447 100644
--- a/content/browser/ssl/ssl_client_auth_handler.h
+++ b/content/browser/ssl/ssl_client_auth_handler.h
@@ -7,8 +7,10 @@
#include "base/basictypes.h"
#include "base/callback.h"
+#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
+#include "content/common/content_export.h"
#include "content/public/browser/browser_thread.h"
#include "net/ssl/ssl_cert_request_info.h"
@@ -23,30 +25,56 @@
// This class handles the approval and selection of a certificate for SSL client
// authentication by the user. Should only be used on the IO thread. If the
// SSLClientAuthHandler is destroyed before the certificate is selected, the
-// selection is canceled and the callback never called.
+// selection is canceled and the delegate never called.
class SSLClientAuthHandler {
public:
- using CertificateCallback = base::Callback<void(net::X509Certificate*)>;
+ // Delegate interface for SSLClientAuthHandler. Method implementations may
+ // delete the handler when called.
+ class CONTENT_EXPORT Delegate {
+ public:
+ Delegate() {}
+ // Called to continue the request with |cert|. |cert| may be nullptr.
+ virtual void ContinueWithCertificate(net::X509Certificate* cert) = 0;
+
+ // Called to cancel the certificate selection and abort the request.
+ virtual void CancelCertificateSelection() = 0;
+
+ protected:
+ virtual ~Delegate() {}
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(Delegate);
+ };
+
+ // Creates a new SSLClientAuthHandler. The caller ensures that the handler
+ // does not outlive |request| or |delegate|.
SSLClientAuthHandler(scoped_ptr<net::ClientCertStore> client_cert_store,
net::URLRequest* request,
net::SSLCertRequestInfo* cert_request_info,
- const CertificateCallback& callback);
+ Delegate* delegate);
~SSLClientAuthHandler();
// Selects a certificate and resumes the URL request with that certificate.
void SelectCertificate();
+ // Called to continue the request associated with |handler| using |cert|. This
+ // is static to avoid deleting |handler| while it is on the stack.
+ static void ContinueWithCertificate(
+ const base::WeakPtr<SSLClientAuthHandler>& handler,
+ net::X509Certificate* cert);
+
+ // Called to abort the request associated with |handler|. This is static to
+ // avoid deleting |handler| while it is on the stack.
+ static void CancelCertificateSelection(
+ const base::WeakPtr<SSLClientAuthHandler>& handler);
+
private:
class Core;
// Called when |core_| is done retrieving the cert list.
void DidGetClientCerts();
- // Called when the user has selected a cert. If the user chose to continue
- // with no certificate, |cert| is NULL.
- void CertificateSelected(net::X509Certificate* cert);
-
// A reference-counted core so the ClientCertStore may outlive
// SSLClientAuthHandler if the handler is destroyed while an operation on the
// ClientCertStore is in progress.
@@ -58,8 +86,8 @@
// The certs to choose from.
scoped_refptr<net::SSLCertRequestInfo> cert_request_info_;
- // The callback to call when the certificate is selected.
- CertificateCallback callback_;
+ // The delegate to call back with the result.
+ Delegate* delegate_;
base::WeakPtrFactory<SSLClientAuthHandler> weak_factory_;
diff --git a/content/content_browser.gypi b/content/content_browser.gypi
index 06b1149..fcfc728 100644
--- a/content/content_browser.gypi
+++ b/content/content_browser.gypi
@@ -92,6 +92,7 @@
'public/browser/certificate_request_result_type.h',
'public/browser/child_process_data.h',
'public/browser/child_process_security_policy.h',
+ 'public/browser/client_certificate_delegate.h',
'public/browser/color_chooser.h',
'public/browser/content_browser_client.cc',
'public/browser/content_browser_client.h',
diff --git a/content/public/browser/client_certificate_delegate.h b/content/public/browser/client_certificate_delegate.h
new file mode 100644
index 0000000..d957fd9
--- /dev/null
+++ b/content/public/browser/client_certificate_delegate.h
@@ -0,0 +1,35 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_PUBLIC_BROWSER_CLIENT_CERTIFICATE_DELEGATE_H_
+#define CONTENT_PUBLIC_BROWSER_CLIENT_CERTIFICATE_DELEGATE_H_
+
+#include "base/macros.h"
+
+namespace net {
+class X509Certificate;
+}
+
+namespace content {
+
+// A delegate interface for selecting a client certificate for use with a
+// network request. If the delegate is destroyed without calling
+// ContinueWithCertificate, the certificate request will be aborted.
+class ClientCertificateDelegate {
+ public:
+ ClientCertificateDelegate() {}
+ virtual ~ClientCertificateDelegate() {}
+
+ // Continue the request with |cert|. |cert| may be nullptr to continue without
+ // supplying a certificate. This decision will be remembered for future
+ // requests to the domain.
+ virtual void ContinueWithCertificate(net::X509Certificate* cert) = 0;
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(ClientCertificateDelegate);
+};
+
+} // namespace content
+
+#endif // CONTENT_PUBLIC_BROWSER_CLIENT_CERTIFICATE_DELEGATE_H_
diff --git a/content/public/browser/content_browser_client.cc b/content/public/browser/content_browser_client.cc
index 96c076a..1ce7105d 100644
--- a/content/public/browser/content_browser_client.cc
+++ b/content/public/browser/content_browser_client.cc
@@ -5,6 +5,7 @@
#include "content/public/browser/content_browser_client.h"
#include "base/files/file_path.h"
+#include "content/public/browser/client_certificate_delegate.h"
#include "ui/gfx/image/image_skia.h"
#include "url/gurl.h"
@@ -177,11 +178,9 @@
}
void ContentBrowserClient::SelectClientCertificate(
- int render_process_id,
- int render_frame_id,
+ WebContents* web_contents,
net::SSLCertRequestInfo* cert_request_info,
- const base::Callback<void(net::X509Certificate*)>& callback) {
- callback.Run(nullptr);
+ scoped_ptr<ClientCertificateDelegate> delegate) {
}
net::URLRequestContext* ContentBrowserClient::OverrideRequestContextForURL(
diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h
index c5c3942..3195973 100644
--- a/content/public/browser/content_browser_client.h
+++ b/content/public/browser/content_browser_client.h
@@ -89,6 +89,7 @@
class BrowserPluginGuestDelegate;
class BrowserPpapiHost;
class BrowserURLHandler;
+class ClientCertificateDelegate;
class DevToolsManagerDelegate;
class ExternalVideoSurfaceContainer;
class LocationProvider;
@@ -400,14 +401,14 @@
const base::Callback<void(bool)>& callback,
CertificateRequestResultType* result) {}
- // Selects a SSL client certificate and returns it to the |callback|. If no
- // certificate was selected nullptr is returned to the |callback|. Note:
- // |callback| may be called synchronously or asynchronously.
+ // Selects a SSL client certificate and returns it to the |delegate|. Note:
+ // |delegate| may be called synchronously or asynchronously.
+ //
+ // TODO(davidben): Move this hook to WebContentsDelegate.
virtual void SelectClientCertificate(
- int render_process_id,
- int render_frame_id,
+ WebContents* web_contents,
net::SSLCertRequestInfo* cert_request_info,
- const base::Callback<void(net::X509Certificate*)>& callback);
+ scoped_ptr<ClientCertificateDelegate> delegate);
// Adds a new installable certificate or private key.
// Typically used to install an X.509 user certificate.
diff --git a/content/shell/browser/shell_content_browser_client.cc b/content/shell/browser/shell_content_browser_client.cc
index 927fdd7c..90cc70c 100644
--- a/content/shell/browser/shell_content_browser_client.cc
+++ b/content/shell/browser/shell_content_browser_client.cc
@@ -10,6 +10,7 @@
#include "base/files/file_util.h"
#include "base/path_service.h"
#include "base/strings/utf_string_conversions.h"
+#include "content/public/browser/client_certificate_delegate.h"
#include "content/public/browser/page_navigator.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/resource_dispatcher_host.h"
@@ -287,6 +288,14 @@
return new ShellQuotaPermissionContext();
}
+void ShellContentBrowserClient::SelectClientCertificate(
+ WebContents* web_contents,
+ net::SSLCertRequestInfo* cert_request_info,
+ scoped_ptr<ClientCertificateDelegate> delegate) {
+ if (!select_client_certificate_callback_.is_null())
+ select_client_certificate_callback_.Run();
+}
+
void ShellContentBrowserClient::RequestPermission(
PermissionType permission,
WebContents* web_contents,
diff --git a/content/shell/browser/shell_content_browser_client.h b/content/shell/browser/shell_content_browser_client.h
index e78062f..6360f2c 100644
--- a/content/shell/browser/shell_content_browser_client.h
+++ b/content/shell/browser/shell_content_browser_client.h
@@ -54,6 +54,10 @@
WebContentsViewDelegate* GetWebContentsViewDelegate(
WebContents* web_contents) override;
QuotaPermissionContext* CreateQuotaPermissionContext() override;
+ void SelectClientCertificate(
+ WebContents* web_contents,
+ net::SSLCertRequestInfo* cert_request_info,
+ scoped_ptr<ClientCertificateDelegate> delegate) override;
void RequestPermission(
PermissionType permission,
WebContents* web_contents,
@@ -94,6 +98,12 @@
return shell_browser_main_parts_;
}
+ // Used for content_browsertests.
+ void set_select_client_certificate_callback(
+ base::Closure select_client_certificate_callback) {
+ select_client_certificate_callback_ = select_client_certificate_callback;
+ }
+
private:
ShellBrowserContext* ShellBrowserContextForBrowserContext(
BrowserContext* content_browser_context);
@@ -106,6 +116,8 @@
base::ScopedFD v8_snapshot_fd_;
#endif
+ base::Closure select_client_certificate_callback_;
+
ShellBrowserMainParts* shell_browser_main_parts_;
};
diff --git a/content/test/data/workers/worker_common.js b/content/test/data/workers/worker_common.js
index 675de0d..03cc7f6c 100644
--- a/content/test/data/workers/worker_common.js
+++ b/content/test/data/workers/worker_common.js
@@ -1,3 +1,7 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
// Track the number of clients for this worker - tests can use this to ensure
// that shared workers are actually shared, not distinct.
var num_clients = 0;
@@ -29,5 +33,11 @@
} catch (ex) {
postMessage(ex);
}
+ } else if (/tls-client-auth.+/.test(evt.data)) {
+ try {
+ importScripts(evt.data.substr(16));
+ } catch (ex) {
+ }
+ postMessage("done");
}
}
diff --git a/content/test/data/workers/worker_tls_client_auth.html b/content/test/data/workers/worker_tls_client_auth.html
new file mode 100644
index 0000000..02260cc
--- /dev/null
+++ b/content/test/data/workers/worker_tls_client_auth.html
@@ -0,0 +1,25 @@
+<html>
+
+<head>
+<title>Worker TLS Client Auth Test</title>
+
+<script src="worker_utils.js"></script>
+
+<script>
+
+// TODO(davidben): When URLSearchParams is stable and implemented, switch this
+// (and a lot of other test code) to it. https://crbug.com/303152
+var url = decodeURIComponent(/url=([^&]*)/.exec(location.search)[1]);
+var worker = getWorker("worker_common.js");
+worker.onmessage = function(ev) {
+ if (ev.data == "done")
+ onSuccess();
+};
+worker.postMessage("tls-client-auth " + url);
+</script>
+</head>
+
+<body>
+<div id=statusPanel></div>
+</body>
+</html>
diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc
index 8351029..f84fca4 100644
--- a/net/url_request/url_request.cc
+++ b/net/url_request/url_request.cc
@@ -157,7 +157,7 @@
void URLRequest::Delegate::OnCertificateRequested(
URLRequest* request,
SSLCertRequestInfo* cert_request_info) {
- request->Cancel();
+ request->CancelWithError(ERR_SSL_CLIENT_AUTH_CERT_NEEDED);
}
void URLRequest::Delegate::OnSSLCertificateError(URLRequest* request,
