From d072e612823bdf342b519c866e09c55269b0cadf Mon Sep 17 00:00:00 2001 From: Robo Date: Thu, 5 Nov 2015 19:36:36 +0530 Subject: [PATCH 1/3] session: api to allow handling certificate verification --- atom/browser/api/atom_api_app.cc | 15 ---- atom/browser/api/atom_api_session.cc | 7 ++ atom/browser/atom_browser_context.cc | 5 ++ atom/browser/atom_browser_context.h | 1 + atom/browser/atom_cert_verifier.cc | 84 +++++++++++++++++++ atom/browser/atom_cert_verifier.h | 47 +++++++++++ atom/browser/browser.cc | 10 +++ atom/browser/browser.h | 4 + atom/browser/browser_observer.h | 11 +++ .../native_mate_converters/net_converter.cc | 19 +++++ .../native_mate_converters/net_converter.h | 8 ++ docs/api/app.md | 4 +- docs/api/session.md | 31 +++++++ filenames.gypi | 2 + 14 files changed, 231 insertions(+), 17 deletions(-) create mode 100644 atom/browser/atom_cert_verifier.cc create mode 100644 atom/browser/atom_cert_verifier.h diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index 1c5c2c04f145..e4d9c9ab6460 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -61,21 +61,6 @@ struct Converter { }; #endif -template<> -struct Converter> { - static v8::Local ToV8( - v8::Isolate* isolate, - const scoped_refptr& val) { - mate::Dictionary dict(isolate, v8::Object::New(isolate)); - std::string encoded_data; - net::X509Certificate::GetPEMEncoded( - val->os_cert_handle(), &encoded_data); - dict.Set("data", encoded_data); - dict.Set("issuerName", val->issuer().GetDisplayName()); - return dict.GetHandle(); - } -}; - } // namespace mate diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index 0ec9c05ed84e..f94d879fcc15 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/atom/browser/api/atom_api_session.cc @@ -13,9 +13,11 @@ #include "atom/browser/api/save_page_handler.h" #include "atom/browser/atom_browser_context.h" #include "atom/browser/atom_browser_main_parts.h" +#include "atom/browser/browser.h" #include "atom/common/native_mate_converters/callback.h" #include "atom/common/native_mate_converters/gurl_converter.h" #include "atom/common/native_mate_converters/file_path_converter.h" +#include "atom/common/native_mate_converters/net_converter.h" #include "atom/common/node_includes.h" #include "base/files/file_path.h" #include "base/prefs/pref_service.h" @@ -365,6 +367,7 @@ v8::Local Session::Cookies(v8::Isolate* isolate) { mate::ObjectTemplateBuilder Session::GetObjectTemplateBuilder( v8::Isolate* isolate) { + auto browser = base::Unretained(Browser::Get()); return mate::ObjectTemplateBuilder(isolate) .SetMethod("resolveProxy", &Session::ResolveProxy) .SetMethod("clearCache", &Session::ClearCache) @@ -373,6 +376,10 @@ mate::ObjectTemplateBuilder Session::GetObjectTemplateBuilder( .SetMethod("setDownloadPath", &Session::SetDownloadPath) .SetMethod("enableNetworkEmulation", &Session::EnableNetworkEmulation) .SetMethod("disableNetworkEmulation", &Session::DisableNetworkEmulation) + .SetMethod("setCertificateVerifier", + base::Bind(&Browser::SetCertificateVerifier, browser)) + .SetMethod("removeCertificateVerifier", + base::Bind(&Browser::RemoveCertificateVerifier, browser)) .SetProperty("cookies", &Session::Cookies); } diff --git a/atom/browser/atom_browser_context.cc b/atom/browser/atom_browser_context.cc index 6cfb160489fc..bfb506e8e246 100644 --- a/atom/browser/atom_browser_context.cc +++ b/atom/browser/atom_browser_context.cc @@ -5,6 +5,7 @@ #include "atom/browser/atom_browser_context.h" #include "atom/browser/atom_browser_main_parts.h" +#include "atom/browser/atom_cert_verifier.h" #include "atom/browser/atom_download_manager_delegate.h" #include "atom/browser/atom_ssl_config_service.h" #include "atom/browser/browser.h" @@ -158,6 +159,10 @@ content::BrowserPluginGuestManager* AtomBrowserContext::GetGuestManager() { return guest_manager_.get(); } +net::CertVerifier* AtomBrowserContext::CreateCertVerifier() { + return new AtomCertVerifier; +} + net::SSLConfigService* AtomBrowserContext::CreateSSLConfigService() { return new AtomSSLConfigService; } diff --git a/atom/browser/atom_browser_context.h b/atom/browser/atom_browser_context.h index aafa092442bc..81f9533c9c5e 100644 --- a/atom/browser/atom_browser_context.h +++ b/atom/browser/atom_browser_context.h @@ -27,6 +27,7 @@ class AtomBrowserContext : public brightray::BrowserContext { content::URLRequestInterceptorScopedVector* interceptors) override; net::HttpCache::BackendFactory* CreateHttpCacheBackendFactory( const base::FilePath& base_path) override; + net::CertVerifier* CreateCertVerifier() override; net::SSLConfigService* CreateSSLConfigService() override; bool AllowNTLMCredentialsForDomain(const GURL& auth_origin) override; diff --git a/atom/browser/atom_cert_verifier.cc b/atom/browser/atom_cert_verifier.cc new file mode 100644 index 000000000000..d8d1cb112dc2 --- /dev/null +++ b/atom/browser/atom_cert_verifier.cc @@ -0,0 +1,84 @@ +// Copyright (c) 2015 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "atom/browser/atom_cert_verifier.h" + +#include "atom/browser/browser.h" +#include "atom/common/native_mate_converters/net_converter.h" +#include "content/public/browser/browser_thread.h" +#include "net/base/net_errors.h" +#include "net/cert/x509_certificate.h" + +using content::BrowserThread; + +namespace atom { + +namespace { + +void RunResult(const net::CompletionCallback& callback, bool success) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + + int result = net::OK; + if (!success) + result = net::ERR_FAILED; + + BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, + base::Bind(callback, result)); +} + +} // namespace + +AtomCertVerifier::AtomCertVerifier() { + Browser::Get()->AddObserver(this); + default_cert_verifier_.reset(net::CertVerifier::CreateDefault()); +} + +AtomCertVerifier::~AtomCertVerifier() { + Browser::Get()->RemoveObserver(this); +} + +int AtomCertVerifier::Verify( + net::X509Certificate* cert, + const std::string& hostname, + const std::string& ocsp_response, + int flags, + net::CRLSet* crl_set, + net::CertVerifyResult* verify_result, + const net::CompletionCallback& callback, + scoped_ptr* out_req, + const net::BoundNetLog& net_log) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + + if (callback.is_null() || !verify_result || hostname.empty()) + return net::ERR_INVALID_ARGUMENT; + + if (!handler_.is_null()) { + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, + base::Bind(handler_, hostname, + make_scoped_refptr(cert), + base::Bind(&RunResult, callback))); + return net::ERR_IO_PENDING; + } + + return default_cert_verifier_->Verify(cert, hostname, ocsp_response, + flags, crl_set, verify_result, + callback, out_req, net_log); +} + +bool AtomCertVerifier::SupportsOCSPStapling() { + if (handler_.is_null()) + return default_cert_verifier_->SupportsOCSPStapling(); + return false; +} + +void AtomCertVerifier::OnSetCertificateVerifier( + const CertificateVerifier& handler) { + handler_ = handler; +} + +void AtomCertVerifier::OnRemoveCertificateVerifier() { + handler_.Reset(); +} + +} // namespace atom diff --git a/atom/browser/atom_cert_verifier.h b/atom/browser/atom_cert_verifier.h new file mode 100644 index 000000000000..a9e16e268837 --- /dev/null +++ b/atom/browser/atom_cert_verifier.h @@ -0,0 +1,47 @@ +// Copyright (c) 2015 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef ATOM_BROWSER_ATOM_CERT_VERIFIER_H_ +#define ATOM_BROWSER_ATOM_CERT_VERIFIER_H_ + +#include + +#include "atom/browser/browser_observer.h" +#include "net/cert/cert_verifier.h" + +namespace atom { + +class AtomCertVerifier : public net::CertVerifier, + public BrowserObserver { + public: + AtomCertVerifier(); + ~AtomCertVerifier() override; + + // net::CertVerifier: + int Verify(net::X509Certificate* cert, + const std::string& hostname, + const std::string& ocsp_response, + int flags, + net::CRLSet* crl_set, + net::CertVerifyResult* verify_result, + const net::CompletionCallback& callback, + scoped_ptr* out_req, + const net::BoundNetLog& net_log) override; + bool SupportsOCSPStapling() override; + + protected: + void OnSetCertificateVerifier(const CertificateVerifier& handler) override; + void OnRemoveCertificateVerifier() override; + + private: + scoped_ptr default_cert_verifier_; + + CertificateVerifier handler_; + + DISALLOW_COPY_AND_ASSIGN(AtomCertVerifier); +}; + +} // namespace atom + +#endif // ATOM_BROWSER_ATOM_CERT_VERIFIER_H_ diff --git a/atom/browser/browser.cc b/atom/browser/browser.cc index 57741786520d..2e743ec7535a 100644 --- a/atom/browser/browser.cc +++ b/atom/browser/browser.cc @@ -156,6 +156,16 @@ void Browser::RequestLogin(LoginHandler* login_handler) { FOR_EACH_OBSERVER(BrowserObserver, observers_, OnLogin(login_handler)); } +void Browser::SetCertificateVerifier(const CertificateVerifier& handler) { + FOR_EACH_OBSERVER(BrowserObserver, + observers_, + OnSetCertificateVerifier(handler)); +} + +void Browser::RemoveCertificateVerifier() { + FOR_EACH_OBSERVER(BrowserObserver, observers_, OnRemoveCertificateVerifier()); +} + void Browser::NotifyAndShutdown() { if (is_shutdown_) return; diff --git a/atom/browser/browser.h b/atom/browser/browser.h index e20db080b67a..04278f1b2711 100644 --- a/atom/browser/browser.h +++ b/atom/browser/browser.h @@ -135,6 +135,10 @@ class Browser : public WindowListObserver { // Request basic auth login. void RequestLogin(LoginHandler* login_handler); + // Set.remove the ceritificate verifier provided by the user. + void SetCertificateVerifier(const CertificateVerifier& handler); + void RemoveCertificateVerifier(); + void AddObserver(BrowserObserver* obs) { observers_.AddObserver(obs); } diff --git a/atom/browser/browser_observer.h b/atom/browser/browser_observer.h index 7dccbfbac3c5..679a29746324 100644 --- a/atom/browser/browser_observer.h +++ b/atom/browser/browser_observer.h @@ -7,6 +7,7 @@ #include +#include "base/callback.h" #include "base/memory/scoped_ptr.h" #include "content/public/browser/client_certificate_delegate.h" @@ -16,12 +17,19 @@ class WebContents; namespace net { class SSLCertRequestInfo; +class X509Certificate; } namespace atom { class LoginHandler; +// A callback specialisation used by AtomCertVerifier during verification. +using CertificateVerifier = + base::Callback, + const base::Callback&)>; + class BrowserObserver { public: // The browser is about to close all windows. @@ -62,6 +70,9 @@ class BrowserObserver { // The browser requests HTTP login. virtual void OnLogin(LoginHandler* login_handler) {} + virtual void OnSetCertificateVerifier(const CertificateVerifier& handler) {} + virtual void OnRemoveCertificateVerifier() {} + protected: virtual ~BrowserObserver() {} }; diff --git a/atom/common/native_mate_converters/net_converter.cc b/atom/common/native_mate_converters/net_converter.cc index 4796d962660a..4749a4fedfc2 100644 --- a/atom/common/native_mate_converters/net_converter.cc +++ b/atom/common/native_mate_converters/net_converter.cc @@ -4,7 +4,11 @@ #include "atom/common/native_mate_converters/net_converter.h" +#include + +#include "atom/common/node_includes.h" #include "native_mate/dictionary.h" +#include "net/cert/x509_certificate.h" #include "net/url_request/url_request.h" namespace mate { @@ -31,4 +35,19 @@ v8::Local Converter::ToV8( return mate::ConvertToV8(isolate, dict); } +// static +v8::Local Converter>::ToV8( + v8::Isolate* isolate, const scoped_refptr& val) { + mate::Dictionary dict(isolate, v8::Object::New(isolate)); + std::string encoded_data; + net::X509Certificate::GetPEMEncoded( + val->os_cert_handle(), &encoded_data); + auto buffer = node::Buffer::Copy(isolate, + encoded_data.data(), + encoded_data.size()).ToLocalChecked(); + dict.Set("data", buffer); + dict.Set("issuerName", val->issuer().GetDisplayName()); + return dict.GetHandle(); +} + } // namespace mate diff --git a/atom/common/native_mate_converters/net_converter.h b/atom/common/native_mate_converters/net_converter.h index 352c613eaabb..b11c55929b98 100644 --- a/atom/common/native_mate_converters/net_converter.h +++ b/atom/common/native_mate_converters/net_converter.h @@ -5,11 +5,13 @@ #ifndef ATOM_COMMON_NATIVE_MATE_CONVERTERS_NET_CONVERTER_H_ #define ATOM_COMMON_NATIVE_MATE_CONVERTERS_NET_CONVERTER_H_ +#include "base/memory/ref_counted.h" #include "native_mate/converter.h" namespace net { class AuthChallengeInfo; class URLRequest; +class X509Certificate; } namespace mate { @@ -26,6 +28,12 @@ struct Converter { const net::AuthChallengeInfo* val); }; +template<> +struct Converter> { + static v8::Local ToV8(v8::Isolate* isolate, + const scoped_refptr& val); +}; + } // namespace mate #endif // ATOM_COMMON_NATIVE_MATE_CONVERTERS_NET_CONVERTER_H_ diff --git a/docs/api/app.md b/docs/api/app.md index fdb9f9980592..ce377d915dac 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -139,8 +139,8 @@ Returns: * `webContents` [WebContents](web-contents.md) * `url` URL * `certificateList` [Objects] - * `data` PEM encoded data - * `issuerName` Issuer's Common Name + * `data` Buffer - PEM encoded data + * `issuerName` String - Issuer's Common Name * `callback` Function Emitted when a client certificate is requested. diff --git a/docs/api/session.md b/docs/api/session.md index 25db92b73b25..c0f1e8fa7955 100644 --- a/docs/api/session.md +++ b/docs/api/session.md @@ -220,3 +220,34 @@ window.webContents.session.enableNetworkEmulation({offline: true}); Disables any network emulation already active for the `session`. Resets to the original network configuration. + +### `session.setCertificateVerifier(handler)` + +* `handler` Function + * `hostname` String + * `certificate` Object + * `data` Buffer - PEM encoded data + * `issuerName` String + * `callback` Function + +Sets the certificate verifier for the `session`, will be called +whenever a server certificate verification is requested by the +network layer with `hostname`, `certificate` and `callback`. +`callback` should be called with a boolean response to +indicate continuation or cancellation of the request. + +```js +var handler = function(hostname, certificate, callback) { + if (hostname == "github.com") { + // verification logic + callback(true) + } + callback(false) +} + +window.webContents.session.setCertificateVerifier(handler) +``` + +### `session.removeCertificateVerifier()` + +Removes the certificate verifier provided for the `session`. diff --git a/filenames.gypi b/filenames.gypi index 4dc709c5ec57..9618a962cd97 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -128,6 +128,8 @@ 'atom/browser/atom_download_manager_delegate.h', 'atom/browser/atom_browser_main_parts.cc', 'atom/browser/atom_browser_main_parts.h', + 'atom/browser/atom_cert_verifier.cc', + 'atom/browser/atom_cert_verifier.h', 'atom/browser/atom_browser_main_parts_mac.mm', 'atom/browser/atom_browser_main_parts_posix.cc', 'atom/browser/atom_javascript_dialog_manager.cc', From 37e6e6fab78ed0fdb1885301359350cfcb8ffe07 Mon Sep 17 00:00:00 2001 From: Robo Date: Fri, 13 Nov 2015 01:25:23 +0530 Subject: [PATCH 2/3] emit verify-certificate event for handling verification --- atom/browser/api/atom_api_session.cc | 30 ++++- atom/browser/api/atom_api_session.h | 7 ++ atom/browser/atom_cert_verifier.cc | 159 +++++++++++++++++++++------ atom/browser/atom_cert_verifier.h | 125 +++++++++++++++++++-- atom/browser/browser.cc | 10 +- atom/browser/browser.h | 7 +- atom/browser/browser_observer.h | 13 +-- docs/api/session.md | 55 ++++----- 8 files changed, 313 insertions(+), 93 deletions(-) diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index f94d879fcc15..ecb8bba3a3c2 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/atom/browser/api/atom_api_session.cc @@ -239,12 +239,24 @@ void SetProxyInIO(net::URLRequestContextGetter* getter, RunCallbackInUI(callback); } +void PassVerificationResult( + scoped_refptr request, + bool success) { + int result = net::OK; + if (!success) + result = net::ERR_FAILED; + request->ContinueWithResult(result); +} + } // namespace Session::Session(AtomBrowserContext* browser_context) : browser_context_(browser_context) { AttachAsUserData(browser_context); + // Observe Browser to get certificate verification notification. + Browser::Get()->AddObserver(this); + // Observe DownloadManger to get download notifications. content::BrowserContext::GetDownloadManager(browser_context)-> AddObserver(this); @@ -253,9 +265,22 @@ Session::Session(AtomBrowserContext* browser_context) Session::~Session() { content::BrowserContext::GetDownloadManager(browser_context())-> RemoveObserver(this); + Browser::Get()->RemoveObserver(this); Destroy(); } +void Session::OnCertVerification( + const scoped_refptr& request) { + bool prevent_default = Emit( + "verify-certificate", + request->hostname(), + request->certificate(), + base::Bind(&PassVerificationResult, request)); + + if (!prevent_default) + request->ContinueWithResult(net::ERR_IO_PENDING); +} + void Session::OnDownloadCreated(content::DownloadManager* manager, content::DownloadItem* item) { auto web_contents = item->GetWebContents(); @@ -367,7 +392,6 @@ v8::Local Session::Cookies(v8::Isolate* isolate) { mate::ObjectTemplateBuilder Session::GetObjectTemplateBuilder( v8::Isolate* isolate) { - auto browser = base::Unretained(Browser::Get()); return mate::ObjectTemplateBuilder(isolate) .SetMethod("resolveProxy", &Session::ResolveProxy) .SetMethod("clearCache", &Session::ClearCache) @@ -376,10 +400,6 @@ mate::ObjectTemplateBuilder Session::GetObjectTemplateBuilder( .SetMethod("setDownloadPath", &Session::SetDownloadPath) .SetMethod("enableNetworkEmulation", &Session::EnableNetworkEmulation) .SetMethod("disableNetworkEmulation", &Session::DisableNetworkEmulation) - .SetMethod("setCertificateVerifier", - base::Bind(&Browser::SetCertificateVerifier, browser)) - .SetMethod("removeCertificateVerifier", - base::Bind(&Browser::RemoveCertificateVerifier, browser)) .SetProperty("cookies", &Session::Cookies); } diff --git a/atom/browser/api/atom_api_session.h b/atom/browser/api/atom_api_session.h index 39712f6c8486..e6ce5a5842db 100644 --- a/atom/browser/api/atom_api_session.h +++ b/atom/browser/api/atom_api_session.h @@ -8,6 +8,8 @@ #include #include "atom/browser/api/trackable_object.h" +#include "atom/browser/atom_cert_verifier.h" +#include "atom/browser/browser_observer.h" #include "content/public/browser/download_manager.h" #include "native_mate/handle.h" #include "net/base/completion_callback.h" @@ -34,6 +36,7 @@ class AtomBrowserContext; namespace api { class Session: public mate::TrackableObject, + public BrowserObserver, public content::DownloadManager::Observer { public: using ResolveProxyCallback = base::Callback; @@ -52,6 +55,10 @@ class Session: public mate::TrackableObject, explicit Session(AtomBrowserContext* browser_context); ~Session(); + // BrowserObserver: + void OnCertVerification( + const scoped_refptr&) override; + // content::DownloadManager::Observer: void OnDownloadCreated(content::DownloadManager* manager, content::DownloadItem* item) override; diff --git a/atom/browser/atom_cert_verifier.cc b/atom/browser/atom_cert_verifier.cc index d8d1cb112dc2..f5afdd35719b 100644 --- a/atom/browser/atom_cert_verifier.cc +++ b/atom/browser/atom_cert_verifier.cc @@ -6,36 +6,108 @@ #include "atom/browser/browser.h" #include "atom/common/native_mate_converters/net_converter.h" +#include "base/callback_helpers.h" +#include "base/sha1.h" +#include "base/stl_util.h" #include "content/public/browser/browser_thread.h" #include "net/base/net_errors.h" -#include "net/cert/x509_certificate.h" using content::BrowserThread; namespace atom { -namespace { - -void RunResult(const net::CompletionCallback& callback, bool success) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); - - int result = net::OK; - if (!success) - result = net::ERR_FAILED; - - BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, - base::Bind(callback, result)); +AtomCertVerifier::RequestParams::RequestParams( + const net::SHA1HashValue cert_fingerprint, + const net::SHA1HashValue ca_fingerprint, + const std::string& hostname_arg, + const std::string& ocsp_response_arg, + int flags_arg) + : hostname(hostname_arg), + ocsp_response(ocsp_response_arg), + flags(flags_arg) { + hash_values.reserve(3); + net::SHA1HashValue ocsp_hash; + base::SHA1HashBytes( + reinterpret_cast(ocsp_response.data()), + ocsp_response.size(), ocsp_hash.data); + hash_values.push_back(ocsp_hash); + hash_values.push_back(cert_fingerprint); + hash_values.push_back(ca_fingerprint); } -} // namespace +bool AtomCertVerifier::RequestParams::operator<( + const RequestParams& other) const { + if (flags != other.flags) + return flags < other.flags; + if (hostname != other.hostname) + return hostname < other.hostname; + return std::lexicographical_compare( + hash_values.begin(), + hash_values.end(), + other.hash_values.begin(), + other.hash_values.end(), + net::SHA1HashValueLessThan()); +} + +void AtomCertVerifier::CertVerifyRequest::RunResult(int result) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + + for (auto& callback : callbacks_) + callback.Run(result); + cert_verifier_->RemoveRequest(this); + Release(); +} + +void AtomCertVerifier::CertVerifyRequest::DelegateToDefaultVerifier() { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + + int rv = cert_verifier_->default_cert_verifier()->Verify( + certificate_.get(), + key_.hostname, + key_.ocsp_response, + key_.flags, + crl_set_.get(), + verify_result_, + base::Bind(&CertVerifyRequest::RunResult, + weak_ptr_factory_.GetWeakPtr()), + &new_out_req_, + net_log_); + + if (rv != net::ERR_IO_PENDING && !callbacks_.empty()) { + for (auto& callback : callbacks_) + callback.Run(rv); + cert_verifier_->RemoveRequest(this); + Release(); + } +} + +void AtomCertVerifier::CertVerifyRequest::ContinueWithResult(int result) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + + if (handled_) + return; + + handled_ = true; + + if (result != net::ERR_IO_PENDING) { + BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, + base::Bind(&CertVerifyRequest::RunResult, + weak_ptr_factory_.GetWeakPtr(), + result)); + return; + } + + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&CertVerifyRequest::DelegateToDefaultVerifier, + weak_ptr_factory_.GetWeakPtr())); +} AtomCertVerifier::AtomCertVerifier() { - Browser::Get()->AddObserver(this); default_cert_verifier_.reset(net::CertVerifier::CreateDefault()); } AtomCertVerifier::~AtomCertVerifier() { - Browser::Get()->RemoveObserver(this); } int AtomCertVerifier::Verify( @@ -53,32 +125,57 @@ int AtomCertVerifier::Verify( if (callback.is_null() || !verify_result || hostname.empty()) return net::ERR_INVALID_ARGUMENT; - if (!handler_.is_null()) { + const RequestParams key(cert->fingerprint(), + cert->ca_fingerprint(), + hostname, + ocsp_response, + flags); + + CertVerifyRequest* request = FindRequest(key); + + if (!request) { + request = new CertVerifyRequest(this, + key, + cert, + crl_set, + verify_result, + out_req, + net_log); + requests_.insert(make_scoped_refptr(request)); + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, - base::Bind(handler_, hostname, - make_scoped_refptr(cert), - base::Bind(&RunResult, callback))); - return net::ERR_IO_PENDING; + base::Bind(&Browser::RequestCertVerification, + base::Unretained(Browser::Get()), + make_scoped_refptr(request))); } - return default_cert_verifier_->Verify(cert, hostname, ocsp_response, - flags, crl_set, verify_result, - callback, out_req, net_log); + request->AddCompletionCallback(callback); + + return net::ERR_IO_PENDING; } bool AtomCertVerifier::SupportsOCSPStapling() { - if (handler_.is_null()) - return default_cert_verifier_->SupportsOCSPStapling(); - return false; + return true; } -void AtomCertVerifier::OnSetCertificateVerifier( - const CertificateVerifier& handler) { - handler_ = handler; +AtomCertVerifier::CertVerifyRequest* AtomCertVerifier::FindRequest( + const RequestParams& key) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + + auto it = std::lower_bound(requests_.begin(), + requests_.end(), + key, + CertVerifyRequestToRequestParamsComparator()); + if (it != requests_.end() && !(key < (*it)->key())) + return (*it).get(); + return nullptr; } -void AtomCertVerifier::OnRemoveCertificateVerifier() { - handler_.Reset(); +void AtomCertVerifier::RemoveRequest(CertVerifyRequest* request) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + + bool erased = requests_.erase(request) == 1; + DCHECK(erased); } } // namespace atom diff --git a/atom/browser/atom_cert_verifier.h b/atom/browser/atom_cert_verifier.h index a9e16e268837..27e530074d02 100644 --- a/atom/browser/atom_cert_verifier.h +++ b/atom/browser/atom_cert_verifier.h @@ -5,19 +5,108 @@ #ifndef ATOM_BROWSER_ATOM_CERT_VERIFIER_H_ #define ATOM_BROWSER_ATOM_CERT_VERIFIER_H_ +#include #include +#include -#include "atom/browser/browser_observer.h" +#include "base/memory/ref_counted.h" +#include "net/base/hash_value.h" #include "net/cert/cert_verifier.h" +#include "net/cert/crl_set.h" +#include "net/cert/x509_certificate.h" +#include "net/log/net_log.h" namespace atom { -class AtomCertVerifier : public net::CertVerifier, - public BrowserObserver { +class AtomCertVerifier : public net::CertVerifier { public: + struct RequestParams { + RequestParams( + const net::SHA1HashValue cert_fingerprint, + const net::SHA1HashValue ca_fingerprint, + const std::string& hostname_arg, + const std::string& ocsp_response, + int flags); + ~RequestParams() {} + + bool operator<(const RequestParams& other) const; + + std::string hostname; + std::string ocsp_response; + int flags; + std::vector hash_values; + }; + + class CertVerifyRequest + : public net::CertVerifier::Request, + public base::RefCountedThreadSafe { + public: + CertVerifyRequest( + AtomCertVerifier* cert_verifier, + const RequestParams& key, + scoped_refptr cert, + scoped_refptr crl_set, + net::CertVerifyResult* verify_result, + scoped_ptr* out_req, + const net::BoundNetLog& net_log) + : cert_verifier_(cert_verifier), + key_(key), + certificate_(cert), + crl_set_(crl_set), + verify_result_(verify_result), + out_req_(out_req), + net_log_(net_log), + handled_(false), + weak_ptr_factory_(this) { + out_req_->reset(this); + new_out_req_.reset(new net::CertVerifier::Request()); + } + + ~CertVerifyRequest() { + out_req_->reset(); + } + + void RunResult(int result); + void DelegateToDefaultVerifier(); + void ContinueWithResult(int result); + + void AddCompletionCallback(net::CompletionCallback callback) { + callbacks_.push_back(callback); + } + + const RequestParams key() const { return key_; } + + std::string hostname() const { return key_.hostname; } + + scoped_refptr certificate() const { + return certificate_; + } + + private: + friend class base::RefCountedThreadSafe; + + AtomCertVerifier* cert_verifier_; + const RequestParams key_; + + scoped_refptr certificate_; + scoped_refptr crl_set_; + net::CertVerifyResult* verify_result_; + scoped_ptr* out_req_; + scoped_ptr new_out_req_; + const net::BoundNetLog net_log_; + + std::vector callbacks_; + bool handled_; + + base::WeakPtrFactory weak_ptr_factory_; + + DISALLOW_COPY_AND_ASSIGN(CertVerifyRequest); + }; + AtomCertVerifier(); ~AtomCertVerifier() override; + protected: // net::CertVerifier: int Verify(net::X509Certificate* cert, const std::string& hostname, @@ -30,14 +119,34 @@ class AtomCertVerifier : public net::CertVerifier, const net::BoundNetLog& net_log) override; bool SupportsOCSPStapling() override; - protected: - void OnSetCertificateVerifier(const CertificateVerifier& handler) override; - void OnRemoveCertificateVerifier() override; + net::CertVerifier* default_cert_verifier() const { + return default_cert_verifier_.get(); + } private: - scoped_ptr default_cert_verifier_; + CertVerifyRequest* FindRequest(const RequestParams& key); + void RemoveRequest(CertVerifyRequest* request); - CertificateVerifier handler_; + struct CertVerifyRequestToRequestParamsComparator { + bool operator()(const scoped_refptr request, + const RequestParams& key) const { + return request->key() < key; + } + }; + + struct CertVerifyRequestComparator { + bool operator()(const scoped_refptr req1, + const scoped_refptr req2) const { + return req1->key() < req2->key(); + } + }; + + using ActiveRequestSet = + std::set, + CertVerifyRequestComparator>; + ActiveRequestSet requests_; + + scoped_ptr default_cert_verifier_; DISALLOW_COPY_AND_ASSIGN(AtomCertVerifier); }; diff --git a/atom/browser/browser.cc b/atom/browser/browser.cc index 2e743ec7535a..a3e1f0247cff 100644 --- a/atom/browser/browser.cc +++ b/atom/browser/browser.cc @@ -7,6 +7,7 @@ #include #include "atom/browser/atom_browser_main_parts.h" +#include "atom/browser/atom_cert_verifier.h" #include "atom/browser/native_window.h" #include "atom/browser/window_list.h" #include "base/message_loop/message_loop.h" @@ -156,14 +157,11 @@ void Browser::RequestLogin(LoginHandler* login_handler) { FOR_EACH_OBSERVER(BrowserObserver, observers_, OnLogin(login_handler)); } -void Browser::SetCertificateVerifier(const CertificateVerifier& handler) { +void Browser::RequestCertVerification( + const scoped_refptr& request) { FOR_EACH_OBSERVER(BrowserObserver, observers_, - OnSetCertificateVerifier(handler)); -} - -void Browser::RemoveCertificateVerifier() { - FOR_EACH_OBSERVER(BrowserObserver, observers_, OnRemoveCertificateVerifier()); + OnCertVerification(request)); } void Browser::NotifyAndShutdown() { diff --git a/atom/browser/browser.h b/atom/browser/browser.h index 04278f1b2711..b0ac7d272130 100644 --- a/atom/browser/browser.h +++ b/atom/browser/browser.h @@ -29,6 +29,7 @@ class MenuModel; namespace atom { +class AtomCertVerifier; class LoginHandler; // This class is used for control application-wide operations. @@ -135,9 +136,9 @@ class Browser : public WindowListObserver { // Request basic auth login. void RequestLogin(LoginHandler* login_handler); - // Set.remove the ceritificate verifier provided by the user. - void SetCertificateVerifier(const CertificateVerifier& handler); - void RemoveCertificateVerifier(); + // Request Server Certificate Verification. + void RequestCertVerification( + const scoped_refptr& request); void AddObserver(BrowserObserver* obs) { observers_.AddObserver(obs); diff --git a/atom/browser/browser_observer.h b/atom/browser/browser_observer.h index 679a29746324..75f63d85e2b0 100644 --- a/atom/browser/browser_observer.h +++ b/atom/browser/browser_observer.h @@ -7,7 +7,7 @@ #include -#include "base/callback.h" +#include "atom/browser/atom_cert_verifier.h" #include "base/memory/scoped_ptr.h" #include "content/public/browser/client_certificate_delegate.h" @@ -24,12 +24,6 @@ namespace atom { class LoginHandler; -// A callback specialisation used by AtomCertVerifier during verification. -using CertificateVerifier = - base::Callback, - const base::Callback&)>; - class BrowserObserver { public: // The browser is about to close all windows. @@ -70,8 +64,9 @@ class BrowserObserver { // The browser requests HTTP login. virtual void OnLogin(LoginHandler* login_handler) {} - virtual void OnSetCertificateVerifier(const CertificateVerifier& handler) {} - virtual void OnRemoveCertificateVerifier() {} + // The browser requests Server Certificate Verification. + virtual void OnCertVerification( + const scoped_refptr& request) {} protected: virtual ~BrowserObserver() {} diff --git a/docs/api/session.md b/docs/api/session.md index c0f1e8fa7955..928128cca32c 100644 --- a/docs/api/session.md +++ b/docs/api/session.md @@ -34,6 +34,30 @@ session.on('will-download', function(event, item, webContents) { }); ``` +### Event: 'verify-certificate' + +* `event` Event +* `hostname` String +* `certificate` Object + * `data` Buffer - PEM encoded data + * `issuerName` String +* `callback` Function + +Fired whenever a server certificate verification is requested by the +network layer with `hostname`, `certificate` and `callback`. +`callback` should be called with a boolean response to +indicate continuation or cancellation of the request. + +```js +session.on('verify-certificate', function(event, hostname, certificate, callback) { + if (hostname == "github.com") { + // verification logic + callback(true); + } + callback(false); +}); +``` + ## Methods The `session` object has the following methods: @@ -220,34 +244,3 @@ window.webContents.session.enableNetworkEmulation({offline: true}); Disables any network emulation already active for the `session`. Resets to the original network configuration. - -### `session.setCertificateVerifier(handler)` - -* `handler` Function - * `hostname` String - * `certificate` Object - * `data` Buffer - PEM encoded data - * `issuerName` String - * `callback` Function - -Sets the certificate verifier for the `session`, will be called -whenever a server certificate verification is requested by the -network layer with `hostname`, `certificate` and `callback`. -`callback` should be called with a boolean response to -indicate continuation or cancellation of the request. - -```js -var handler = function(hostname, certificate, callback) { - if (hostname == "github.com") { - // verification logic - callback(true) - } - callback(false) -} - -window.webContents.session.setCertificateVerifier(handler) -``` - -### `session.removeCertificateVerifier()` - -Removes the certificate verifier provided for the `session`. From 92c3ee8e163babb208a5ec8fba372f14ea313214 Mon Sep 17 00:00:00 2001 From: Robo Date: Mon, 16 Nov 2015 18:58:37 +0530 Subject: [PATCH 3/3] use delegate to notify verification requests --- atom/browser/api/atom_api_session.cc | 8 ++------ atom/browser/api/atom_api_session.h | 7 +++---- atom/browser/atom_browser_context.cc | 3 ++- atom/browser/atom_browser_context.h | 4 ++++ atom/browser/atom_cert_verifier.cc | 21 ++++++++------------ atom/browser/atom_cert_verifier.h | 29 ++++++++++++++++++---------- atom/browser/browser.cc | 8 -------- atom/browser/browser.h | 5 ----- atom/browser/browser_observer.h | 6 ------ 9 files changed, 38 insertions(+), 53 deletions(-) diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index ecb8bba3a3c2..07b9b68a94d7 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/atom/browser/api/atom_api_session.cc @@ -13,7 +13,6 @@ #include "atom/browser/api/save_page_handler.h" #include "atom/browser/atom_browser_context.h" #include "atom/browser/atom_browser_main_parts.h" -#include "atom/browser/browser.h" #include "atom/common/native_mate_converters/callback.h" #include "atom/common/native_mate_converters/gurl_converter.h" #include "atom/common/native_mate_converters/file_path_converter.h" @@ -253,9 +252,7 @@ void PassVerificationResult( Session::Session(AtomBrowserContext* browser_context) : browser_context_(browser_context) { AttachAsUserData(browser_context); - - // Observe Browser to get certificate verification notification. - Browser::Get()->AddObserver(this); + browser_context->cert_verifier()->SetDelegate(this); // Observe DownloadManger to get download notifications. content::BrowserContext::GetDownloadManager(browser_context)-> @@ -265,11 +262,10 @@ Session::Session(AtomBrowserContext* browser_context) Session::~Session() { content::BrowserContext::GetDownloadManager(browser_context())-> RemoveObserver(this); - Browser::Get()->RemoveObserver(this); Destroy(); } -void Session::OnCertVerification( +void Session::RequestCertVerification( const scoped_refptr& request) { bool prevent_default = Emit( "verify-certificate", diff --git a/atom/browser/api/atom_api_session.h b/atom/browser/api/atom_api_session.h index e6ce5a5842db..05d67b8842ef 100644 --- a/atom/browser/api/atom_api_session.h +++ b/atom/browser/api/atom_api_session.h @@ -9,7 +9,6 @@ #include "atom/browser/api/trackable_object.h" #include "atom/browser/atom_cert_verifier.h" -#include "atom/browser/browser_observer.h" #include "content/public/browser/download_manager.h" #include "native_mate/handle.h" #include "net/base/completion_callback.h" @@ -36,7 +35,7 @@ class AtomBrowserContext; namespace api { class Session: public mate::TrackableObject, - public BrowserObserver, + public AtomCertVerifier::Delegate, public content::DownloadManager::Observer { public: using ResolveProxyCallback = base::Callback; @@ -55,8 +54,8 @@ class Session: public mate::TrackableObject, explicit Session(AtomBrowserContext* browser_context); ~Session(); - // BrowserObserver: - void OnCertVerification( + // AtomCertVerifier::Delegate: + void RequestCertVerification( const scoped_refptr&) override; // content::DownloadManager::Observer: diff --git a/atom/browser/atom_browser_context.cc b/atom/browser/atom_browser_context.cc index bfb506e8e246..b1092757ae48 100644 --- a/atom/browser/atom_browser_context.cc +++ b/atom/browser/atom_browser_context.cc @@ -61,6 +61,7 @@ std::string RemoveWhitespace(const std::string& str) { AtomBrowserContext::AtomBrowserContext(const std::string& partition, bool in_memory) : brightray::BrowserContext(partition, in_memory), + cert_verifier_(new AtomCertVerifier), job_factory_(new AtomURLRequestJobFactory), allow_ntlm_everywhere_(false) { } @@ -160,7 +161,7 @@ content::BrowserPluginGuestManager* AtomBrowserContext::GetGuestManager() { } net::CertVerifier* AtomBrowserContext::CreateCertVerifier() { - return new AtomCertVerifier; + return cert_verifier_; } net::SSLConfigService* AtomBrowserContext::CreateSSLConfigService() { diff --git a/atom/browser/atom_browser_context.h b/atom/browser/atom_browser_context.h index 81f9533c9c5e..d3d7735c810d 100644 --- a/atom/browser/atom_browser_context.h +++ b/atom/browser/atom_browser_context.h @@ -12,6 +12,7 @@ namespace atom { class AtomDownloadManagerDelegate; +class AtomCertVerifier; class AtomURLRequestJobFactory; class WebViewManager; @@ -40,6 +41,8 @@ class AtomBrowserContext : public brightray::BrowserContext { void AllowNTLMCredentialsForAllDomains(bool should_allow); + AtomCertVerifier* cert_verifier() const { return cert_verifier_; } + AtomURLRequestJobFactory* job_factory() const { return job_factory_; } private: @@ -47,6 +50,7 @@ class AtomBrowserContext : public brightray::BrowserContext { scoped_ptr guest_manager_; // Managed by brightray::BrowserContext. + AtomCertVerifier* cert_verifier_; AtomURLRequestJobFactory* job_factory_; bool allow_ntlm_everywhere_; diff --git a/atom/browser/atom_cert_verifier.cc b/atom/browser/atom_cert_verifier.cc index f5afdd35719b..e56e611faa81 100644 --- a/atom/browser/atom_cert_verifier.cc +++ b/atom/browser/atom_cert_verifier.cc @@ -6,7 +6,6 @@ #include "atom/browser/browser.h" #include "atom/common/native_mate_converters/net_converter.h" -#include "base/callback_helpers.h" #include "base/sha1.h" #include "base/stl_util.h" #include "content/public/browser/browser_thread.h" @@ -55,7 +54,6 @@ void AtomCertVerifier::CertVerifyRequest::RunResult(int result) { for (auto& callback : callbacks_) callback.Run(result); cert_verifier_->RemoveRequest(this); - Release(); } void AtomCertVerifier::CertVerifyRequest::DelegateToDefaultVerifier() { @@ -70,15 +68,11 @@ void AtomCertVerifier::CertVerifyRequest::DelegateToDefaultVerifier() { verify_result_, base::Bind(&CertVerifyRequest::RunResult, weak_ptr_factory_.GetWeakPtr()), - &new_out_req_, + out_req_, net_log_); - if (rv != net::ERR_IO_PENDING && !callbacks_.empty()) { - for (auto& callback : callbacks_) - callback.Run(rv); - cert_verifier_->RemoveRequest(this); - Release(); - } + if (rv != net::ERR_IO_PENDING) + RunResult(rv); } void AtomCertVerifier::CertVerifyRequest::ContinueWithResult(int result) { @@ -103,7 +97,8 @@ void AtomCertVerifier::CertVerifyRequest::ContinueWithResult(int result) { weak_ptr_factory_.GetWeakPtr())); } -AtomCertVerifier::AtomCertVerifier() { +AtomCertVerifier::AtomCertVerifier() + : delegate_(nullptr) { default_cert_verifier_.reset(net::CertVerifier::CreateDefault()); } @@ -122,7 +117,7 @@ int AtomCertVerifier::Verify( const net::BoundNetLog& net_log) { DCHECK_CURRENTLY_ON(BrowserThread::IO); - if (callback.is_null() || !verify_result || hostname.empty()) + if (callback.is_null() || !verify_result || hostname.empty() || !delegate_) return net::ERR_INVALID_ARGUMENT; const RequestParams key(cert->fingerprint(), @@ -144,8 +139,8 @@ int AtomCertVerifier::Verify( requests_.insert(make_scoped_refptr(request)); BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, - base::Bind(&Browser::RequestCertVerification, - base::Unretained(Browser::Get()), + base::Bind(&Delegate::RequestCertVerification, + base::Unretained(delegate_), make_scoped_refptr(request))); } diff --git a/atom/browser/atom_cert_verifier.h b/atom/browser/atom_cert_verifier.h index 27e530074d02..e5560ff82fe4 100644 --- a/atom/browser/atom_cert_verifier.h +++ b/atom/browser/atom_cert_verifier.h @@ -38,8 +38,7 @@ class AtomCertVerifier : public net::CertVerifier { }; class CertVerifyRequest - : public net::CertVerifier::Request, - public base::RefCountedThreadSafe { + : public base::RefCountedThreadSafe { public: CertVerifyRequest( AtomCertVerifier* cert_verifier, @@ -58,12 +57,6 @@ class AtomCertVerifier : public net::CertVerifier { net_log_(net_log), handled_(false), weak_ptr_factory_(this) { - out_req_->reset(this); - new_out_req_.reset(new net::CertVerifier::Request()); - } - - ~CertVerifyRequest() { - out_req_->reset(); } void RunResult(int result); @@ -84,6 +77,7 @@ class AtomCertVerifier : public net::CertVerifier { private: friend class base::RefCountedThreadSafe; + ~CertVerifyRequest() {} AtomCertVerifier* cert_verifier_; const RequestParams key_; @@ -92,7 +86,6 @@ class AtomCertVerifier : public net::CertVerifier { scoped_refptr crl_set_; net::CertVerifyResult* verify_result_; scoped_ptr* out_req_; - scoped_ptr new_out_req_; const net::BoundNetLog net_log_; std::vector callbacks_; @@ -103,8 +96,22 @@ class AtomCertVerifier : public net::CertVerifier { DISALLOW_COPY_AND_ASSIGN(CertVerifyRequest); }; + class Delegate { + public: + Delegate() {} + virtual ~Delegate() {} + + // Called on UI thread. + virtual void RequestCertVerification( + const scoped_refptr& request) {} + }; + AtomCertVerifier(); - ~AtomCertVerifier() override; + virtual ~AtomCertVerifier(); + + void SetDelegate(Delegate* delegate) { + delegate_ = delegate; + } protected: // net::CertVerifier: @@ -146,6 +153,8 @@ class AtomCertVerifier : public net::CertVerifier { CertVerifyRequestComparator>; ActiveRequestSet requests_; + Delegate* delegate_; + scoped_ptr default_cert_verifier_; DISALLOW_COPY_AND_ASSIGN(AtomCertVerifier); diff --git a/atom/browser/browser.cc b/atom/browser/browser.cc index a3e1f0247cff..57741786520d 100644 --- a/atom/browser/browser.cc +++ b/atom/browser/browser.cc @@ -7,7 +7,6 @@ #include #include "atom/browser/atom_browser_main_parts.h" -#include "atom/browser/atom_cert_verifier.h" #include "atom/browser/native_window.h" #include "atom/browser/window_list.h" #include "base/message_loop/message_loop.h" @@ -157,13 +156,6 @@ void Browser::RequestLogin(LoginHandler* login_handler) { FOR_EACH_OBSERVER(BrowserObserver, observers_, OnLogin(login_handler)); } -void Browser::RequestCertVerification( - const scoped_refptr& request) { - FOR_EACH_OBSERVER(BrowserObserver, - observers_, - OnCertVerification(request)); -} - void Browser::NotifyAndShutdown() { if (is_shutdown_) return; diff --git a/atom/browser/browser.h b/atom/browser/browser.h index b0ac7d272130..e20db080b67a 100644 --- a/atom/browser/browser.h +++ b/atom/browser/browser.h @@ -29,7 +29,6 @@ class MenuModel; namespace atom { -class AtomCertVerifier; class LoginHandler; // This class is used for control application-wide operations. @@ -136,10 +135,6 @@ class Browser : public WindowListObserver { // Request basic auth login. void RequestLogin(LoginHandler* login_handler); - // Request Server Certificate Verification. - void RequestCertVerification( - const scoped_refptr& request); - void AddObserver(BrowserObserver* obs) { observers_.AddObserver(obs); } diff --git a/atom/browser/browser_observer.h b/atom/browser/browser_observer.h index 75f63d85e2b0..7dccbfbac3c5 100644 --- a/atom/browser/browser_observer.h +++ b/atom/browser/browser_observer.h @@ -7,7 +7,6 @@ #include -#include "atom/browser/atom_cert_verifier.h" #include "base/memory/scoped_ptr.h" #include "content/public/browser/client_certificate_delegate.h" @@ -17,7 +16,6 @@ class WebContents; namespace net { class SSLCertRequestInfo; -class X509Certificate; } namespace atom { @@ -64,10 +62,6 @@ class BrowserObserver { // The browser requests HTTP login. virtual void OnLogin(LoginHandler* login_handler) {} - // The browser requests Server Certificate Verification. - virtual void OnCertVerification( - const scoped_refptr& request) {} - protected: virtual ~BrowserObserver() {} };