From ebe66daa56c0926d004aa1769ad77e18fd044394 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 17 Nov 2015 21:36:36 +0800 Subject: [PATCH] Emit verify-certificate only when default verifier fails --- atom/browser/api/atom_api_session.cc | 5 ++- atom/browser/net/atom_cert_verifier.cc | 62 +++++++++++++++----------- atom/browser/net/atom_cert_verifier.h | 51 +++++++-------------- docs/api/session.md | 15 ++++--- 4 files changed, 65 insertions(+), 68 deletions(-) diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index 3ac36abf531..c75bf9dd0bc 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/atom/browser/api/atom_api_session.cc @@ -266,11 +266,12 @@ void Session::RequestCertVerification( const scoped_refptr& request) { bool prevent_default = Emit( "verify-certificate", - request->hostname(), - make_scoped_refptr(request->cert()), + request->args().hostname, + request->args().cert, base::Bind(&PassVerificationResult, request)); if (!prevent_default) + // Tell the request to use the result of default verifier. request->ContinueWithResult(net::ERR_IO_PENDING); } diff --git a/atom/browser/net/atom_cert_verifier.cc b/atom/browser/net/atom_cert_verifier.cc index dcfe29afdda..0f94e4fe2ad 100644 --- a/atom/browser/net/atom_cert_verifier.cc +++ b/atom/browser/net/atom_cert_verifier.cc @@ -15,6 +15,9 @@ using content::BrowserThread; namespace atom { +AtomCertVerifier::CertVerifyRequest::~CertVerifyRequest() { +} + void AtomCertVerifier::CertVerifyRequest::ContinueWithResult(int result) { DCHECK_CURRENTLY_ON(BrowserThread::UI); @@ -22,25 +25,10 @@ void AtomCertVerifier::CertVerifyRequest::ContinueWithResult(int result) { return; handled_ = true; - - if (result != net::ERR_IO_PENDING) { - BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, - base::Bind(callback_, result)); - return; - } - BrowserThread::PostTask( BrowserThread::IO, FROM_HERE, - base::Bind(&CertVerifyRequest::DelegateToDefaultVerifier, this)); -} - -void AtomCertVerifier::CertVerifyRequest::DelegateToDefaultVerifier() { - DCHECK_CURRENTLY_ON(BrowserThread::IO); - int result = cert_verifier_->default_cert_verifier_->Verify( - cert_.get(), hostname_, ocsp_response_, flags_, crl_set_.get(), - verify_result_, callback_, out_req_, net_log_); - if (result != net::ERR_IO_PENDING) - callback_.Run(result); + base::Bind(args_.callback, + result == net::ERR_IO_PENDING ? result_ : result)); } AtomCertVerifier::AtomCertVerifier() @@ -66,18 +54,42 @@ int AtomCertVerifier::Verify( if (callback.is_null() || !verify_result || hostname.empty() || !delegate_) return net::ERR_INVALID_ARGUMENT; - CertVerifyRequest* request = new CertVerifyRequest( - this, cert, hostname, ocsp_response, flags, crl_set, verify_result, - callback, out_req, net_log); - BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, - base::Bind(&Delegate::RequestCertVerification, - base::Unretained(delegate_), - make_scoped_refptr(request))); - return net::ERR_IO_PENDING; + VerifyArgs args = { cert, hostname, callback }; + int result = default_cert_verifier_->Verify( + cert, hostname, ocsp_response, flags, crl_set, verify_result, + base::Bind(&AtomCertVerifier::OnDefaultVerificationResult, + base::Unretained(this), args), + out_req, net_log); + if (result != net::OK && result != net::ERR_IO_PENDING) { + // The default verifier fails immediately. + VerifyCertificateFromDelegate(args, result); + return net::ERR_IO_PENDING; + } + + return result; } bool AtomCertVerifier::SupportsOCSPStapling() { return true; } +void AtomCertVerifier::VerifyCertificateFromDelegate( + const VerifyArgs& args, int result) { + CertVerifyRequest* request = new CertVerifyRequest(this, result, args); + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, + base::Bind(&Delegate::RequestCertVerification, + base::Unretained(delegate_), + make_scoped_refptr(request))); +} + +void AtomCertVerifier::OnDefaultVerificationResult( + const VerifyArgs& args, int result) { + if (result == net::OK) { + args.callback.Run(result); + return; + } + + VerifyCertificateFromDelegate(args, result); +} + } // namespace atom diff --git a/atom/browser/net/atom_cert_verifier.h b/atom/browser/net/atom_cert_verifier.h index 02c2444cd7a..8736db0f872 100644 --- a/atom/browser/net/atom_cert_verifier.h +++ b/atom/browser/net/atom_cert_verifier.h @@ -14,55 +14,35 @@ namespace atom { class AtomCertVerifier : public net::CertVerifier { public: + struct VerifyArgs { + scoped_refptr cert; + const std::string& hostname; + net::CompletionCallback callback; + }; + class CertVerifyRequest : public base::RefCountedThreadSafe { public: CertVerifyRequest(AtomCertVerifier* cert_verifier, - 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) + int result, + const VerifyArgs& args) : cert_verifier_(cert_verifier), - cert_(cert), - hostname_(hostname), - ocsp_response_(ocsp_response), - flags_(flags), - crl_set_(crl_set), - verify_result_(verify_result), - callback_(callback), - out_req_(out_req), - net_log_(net_log), + result_(result), + args_(args), handled_(false) { } void ContinueWithResult(int result); - const std::string& hostname() const { return hostname_; } - net::X509Certificate* cert() const { return cert_.get(); } + const VerifyArgs& args() const { return args_; } private: friend class base::RefCountedThreadSafe; - ~CertVerifyRequest() {} - - void DelegateToDefaultVerifier(); + ~CertVerifyRequest(); AtomCertVerifier* cert_verifier_; - - scoped_refptr cert_; - std::string hostname_; - std::string ocsp_response_; - int flags_; - scoped_refptr crl_set_; - net::CertVerifyResult* verify_result_; - net::CompletionCallback callback_; - scoped_ptr* out_req_; - const net::BoundNetLog& net_log_; - + int result_; + VerifyArgs args_; bool handled_; DISALLOW_COPY_AND_ASSIGN(CertVerifyRequest); @@ -100,6 +80,9 @@ class AtomCertVerifier : public net::CertVerifier { private: friend class CertVerifyRequest; + void VerifyCertificateFromDelegate(const VerifyArgs& args, int result); + void OnDefaultVerificationResult(const VerifyArgs& args, int result); + Delegate* delegate_; scoped_ptr default_cert_verifier_; diff --git a/docs/api/session.md b/docs/api/session.md index 210d5875351..cd802f34b20 100644 --- a/docs/api/session.md +++ b/docs/api/session.md @@ -21,7 +21,7 @@ var session = win.webContents.session * `item` [DownloadItem](download-item.md) * `webContents` [WebContents](web-contents.md) -Fired when Electron is about to download `item` in `webContents`. +Emitted when Electron is about to download `item` in `webContents`. Calling `event.preventDefault()` will cancel the download. @@ -43,18 +43,19 @@ session.on('will-download', function(event, item, webContents) { * `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. +Emitted when failed to verify the `certificate` for `hostname`, to trust the +certificate you should prevent the default behavior with +`event.preventDefault()` and call `callback(true)`. ```js session.on('verify-certificate', function(event, hostname, certificate, callback) { if (hostname == "github.com") { - // verification logic + // Verification logic. + event.preventDefault(); callback(true); + } else { + callback(false); } - callback(false); }); ```