From e3517b701e0f1d18a965cb8f67888baddb8645ce Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 17 Nov 2015 19:44:55 +0800 Subject: [PATCH] Create a new CertVerifierRequest for each request It greatly simplifies the code. --- atom/browser/api/atom_api_session.cc | 3 +- atom/browser/net/atom_cert_verifier.cc | 129 ++++--------------------- atom/browser/net/atom_cert_verifier.h | 105 ++++++-------------- 3 files changed, 48 insertions(+), 189 deletions(-) diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index 579b64bf4a7d..3ac36abf5318 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/atom/browser/api/atom_api_session.cc @@ -267,7 +267,7 @@ void Session::RequestCertVerification( bool prevent_default = Emit( "verify-certificate", request->hostname(), - request->certificate(), + make_scoped_refptr(request->cert()), base::Bind(&PassVerificationResult, request)); if (!prevent_default) @@ -294,6 +294,7 @@ bool Session::IsDestroyed() const { } void Session::Destroy() { + browser_context_->cert_verifier()->SetDelegate(nullptr); browser_context_ = nullptr; } diff --git a/atom/browser/net/atom_cert_verifier.cc b/atom/browser/net/atom_cert_verifier.cc index 65e558280822..dcfe29afddad 100644 --- a/atom/browser/net/atom_cert_verifier.cc +++ b/atom/browser/net/atom_cert_verifier.cc @@ -6,74 +6,15 @@ #include "atom/browser/browser.h" #include "atom/common/native_mate_converters/net_converter.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/crl_set.h" +#include "net/cert/x509_certificate.h" using content::BrowserThread; namespace atom { -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); -} - -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); -} - -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, this), - out_req_, - net_log_); - - if (rv != net::ERR_IO_PENDING) - RunResult(rv); -} - void AtomCertVerifier::CertVerifyRequest::ContinueWithResult(int result) { DCHECK_CURRENTLY_ON(BrowserThread::UI); @@ -84,9 +25,7 @@ void AtomCertVerifier::CertVerifyRequest::ContinueWithResult(int result) { if (result != net::ERR_IO_PENDING) { BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, - base::Bind(&CertVerifyRequest::RunResult, - this, - result)); + base::Bind(callback_, result)); return; } @@ -95,6 +34,15 @@ void AtomCertVerifier::CertVerifyRequest::ContinueWithResult(int result) { 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); +} + AtomCertVerifier::AtomCertVerifier() : delegate_(nullptr) { default_cert_verifier_.reset(net::CertVerifier::CreateDefault()); @@ -118,32 +66,13 @@ int AtomCertVerifier::Verify( if (callback.is_null() || !verify_result || hostname.empty() || !delegate_) return net::ERR_INVALID_ARGUMENT; - 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(&Delegate::RequestCertVerification, - base::Unretained(delegate_), - make_scoped_refptr(request))); - } - - request->AddCompletionCallback(callback); - + 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; } @@ -151,24 +80,4 @@ bool AtomCertVerifier::SupportsOCSPStapling() { return true; } -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::RemoveRequest(CertVerifyRequest* request) { - DCHECK_CURRENTLY_ON(BrowserThread::IO); - - bool erased = requests_.erase(request) == 1; - DCHECK(erased); -} - } // namespace atom diff --git a/atom/browser/net/atom_cert_verifier.h b/atom/browser/net/atom_cert_verifier.h index 52b6fb5303e6..02c2444cd7a4 100644 --- a/atom/browser/net/atom_cert_verifier.h +++ b/atom/browser/net/atom_cert_verifier.h @@ -5,89 +5,64 @@ #ifndef ATOM_BROWSER_NET_ATOM_CERT_VERIFIER_H_ #define ATOM_BROWSER_NET_ATOM_CERT_VERIFIER_H_ -#include #include -#include #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: - 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 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) + 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) : cert_verifier_(cert_verifier), - key_(key), - certificate_(cert), + 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), handled_(false) { } - 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_; - } + const std::string& hostname() const { return hostname_; } + net::X509Certificate* cert() const { return cert_.get(); } private: friend class base::RefCountedThreadSafe; ~CertVerifyRequest() {} - AtomCertVerifier* cert_verifier_; - const RequestParams key_; + void DelegateToDefaultVerifier(); - scoped_refptr certificate_; + 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_; + const net::BoundNetLog& net_log_; - std::vector callbacks_; bool handled_; DISALLOW_COPY_AND_ASSIGN(CertVerifyRequest); @@ -95,7 +70,6 @@ class AtomCertVerifier : public net::CertVerifier { class Delegate { public: - Delegate() {} virtual ~Delegate() {} // Called on UI thread. @@ -123,35 +97,10 @@ class AtomCertVerifier : public net::CertVerifier { const net::BoundNetLog& net_log) override; bool SupportsOCSPStapling() override; - net::CertVerifier* default_cert_verifier() const { - return default_cert_verifier_.get(); - } - private: - CertVerifyRequest* FindRequest(const RequestParams& key); - void RemoveRequest(CertVerifyRequest* request); - - 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_; + friend class CertVerifyRequest; Delegate* delegate_; - scoped_ptr default_cert_verifier_; DISALLOW_COPY_AND_ASSIGN(AtomCertVerifier);