From 5cbbd6efe600daafc2a46539b82642316870b6af Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Mon, 17 Oct 2016 16:03:24 +0530 Subject: [PATCH 1/3] session: exclude hosts from CT verification if they are handled by custom cert verifiers --- atom/browser/atom_browser_context.cc | 15 +++++++++--- atom/browser/atom_browser_context.h | 4 +++ atom/browser/net/atom_cert_verifier.cc | 19 +++++++++----- atom/browser/net/atom_cert_verifier.h | 5 +++- atom/browser/net/atom_ct_delegate.cc | 34 ++++++++++++++++++++++++++ atom/browser/net/atom_ct_delegate.h | 33 +++++++++++++++++++++++++ filenames.gypi | 2 ++ 7 files changed, 101 insertions(+), 11 deletions(-) create mode 100644 atom/browser/net/atom_ct_delegate.cc create mode 100644 atom/browser/net/atom_ct_delegate.h diff --git a/atom/browser/atom_browser_context.cc b/atom/browser/atom_browser_context.cc index 63c8c81fb4df..e076fe85c176 100644 --- a/atom/browser/atom_browser_context.cc +++ b/atom/browser/atom_browser_context.cc @@ -12,6 +12,7 @@ #include "atom/browser/browser.h" #include "atom/browser/net/asar/asar_protocol_handler.h" #include "atom/browser/net/atom_cert_verifier.h" +#include "atom/browser/net/atom_ct_delegate.h" #include "atom/browser/net/atom_network_delegate.h" #include "atom/browser/net/atom_ssl_config_service.h" #include "atom/browser/net/atom_url_request_job_factory.h" @@ -67,10 +68,11 @@ std::string RemoveWhitespace(const std::string& str) { } // namespace -AtomBrowserContext::AtomBrowserContext( - const std::string& partition, bool in_memory, - const base::DictionaryValue& options) +AtomBrowserContext::AtomBrowserContext(const std::string& partition, + bool in_memory, + const base::DictionaryValue& options) : brightray::BrowserContext(partition, in_memory), + ct_delegate_(new AtomCTDelegate), network_delegate_(new AtomNetworkDelegate), cookie_delegate_(new AtomCookieDelegate) { // Construct user agent string. @@ -190,7 +192,7 @@ content::PermissionManager* AtomBrowserContext::GetPermissionManager() { } std::unique_ptr AtomBrowserContext::CreateCertVerifier() { - return base::WrapUnique(new AtomCertVerifier); + return base::WrapUnique(new AtomCertVerifier(ct_delegate_.get())); } net::SSLConfigService* AtomBrowserContext::CreateSSLConfigService() { @@ -205,6 +207,11 @@ std::vector AtomBrowserContext::GetCookieableSchemes() { return default_schemes; } +net::TransportSecurityState::RequireCTDelegate* +AtomBrowserContext::GetRequireCTDelegate() { + return ct_delegate_.get(); +} + void AtomBrowserContext::RegisterPrefs(PrefRegistrySimple* pref_registry) { pref_registry->RegisterFilePathPref(prefs::kSelectFileLastDirectory, base::FilePath()); diff --git a/atom/browser/atom_browser_context.h b/atom/browser/atom_browser_context.h index 0ff1cc6321ad..f149e62cb279 100644 --- a/atom/browser/atom_browser_context.h +++ b/atom/browser/atom_browser_context.h @@ -15,6 +15,7 @@ namespace atom { class AtomBlobReader; +class AtomCTDelegate; class AtomDownloadManagerDelegate; class AtomNetworkDelegate; class AtomPermissionManager; @@ -42,6 +43,8 @@ class AtomBrowserContext : public brightray::BrowserContext { std::unique_ptr CreateCertVerifier() override; net::SSLConfigService* CreateSSLConfigService() override; std::vector GetCookieableSchemes() override; + net::TransportSecurityState::RequireCTDelegate* GetRequireCTDelegate() + override; // content::BrowserContext: content::DownloadManagerDelegate* GetDownloadManagerDelegate() override; @@ -67,6 +70,7 @@ class AtomBrowserContext : public brightray::BrowserContext { std::unique_ptr guest_manager_; std::unique_ptr permission_manager_; std::unique_ptr blob_reader_; + std::unique_ptr ct_delegate_; std::string user_agent_; bool use_cache_; diff --git a/atom/browser/net/atom_cert_verifier.cc b/atom/browser/net/atom_cert_verifier.cc index 7fb2ce72297c..9892845cc83f 100644 --- a/atom/browser/net/atom_cert_verifier.cc +++ b/atom/browser/net/atom_cert_verifier.cc @@ -5,9 +5,11 @@ #include "atom/browser/net/atom_cert_verifier.h" #include "atom/browser/browser.h" +#include "atom/browser/net/atom_ct_delegate.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/cert_verify_result.h" #include "net/cert/crl_set.h" #include "net/cert/x509_certificate.h" @@ -28,12 +30,11 @@ void OnResult( } // namespace -AtomCertVerifier::AtomCertVerifier() - : default_cert_verifier_(net::CertVerifier::CreateDefault()) { -} +AtomCertVerifier::AtomCertVerifier(AtomCTDelegate* ct_delegate) + : default_cert_verifier_(net::CertVerifier::CreateDefault()), + ct_delegate_(ct_delegate) {} -AtomCertVerifier::~AtomCertVerifier() { -} +AtomCertVerifier::~AtomCertVerifier() {} void AtomCertVerifier::SetVerifyProc(const VerifyProc& proc) { verify_proc_ = proc; @@ -48,9 +49,15 @@ int AtomCertVerifier::Verify( const net::BoundNetLog& net_log) { DCHECK_CURRENTLY_ON(BrowserThread::IO); - if (verify_proc_.is_null()) + if (verify_proc_.is_null()) { + ct_delegate_->ClearCTExcludedHostsList(); return default_cert_verifier_->Verify( params, crl_set, verify_result, callback, out_req, net_log); + } + + verify_result->Reset(); + verify_result->verified_cert = params.certificate(); + ct_delegate_->AddCTExcludedHost(params.hostname()); BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, diff --git a/atom/browser/net/atom_cert_verifier.h b/atom/browser/net/atom_cert_verifier.h index 74b8f2784c16..7582fae74f61 100644 --- a/atom/browser/net/atom_cert_verifier.h +++ b/atom/browser/net/atom_cert_verifier.h @@ -12,9 +12,11 @@ namespace atom { +class AtomCTDelegate; + class AtomCertVerifier : public net::CertVerifier { public: - AtomCertVerifier(); + explicit AtomCertVerifier(AtomCTDelegate* ct_delegate); virtual ~AtomCertVerifier(); using VerifyProc = @@ -37,6 +39,7 @@ class AtomCertVerifier : public net::CertVerifier { private: VerifyProc verify_proc_; std::unique_ptr default_cert_verifier_; + AtomCTDelegate* ct_delegate_; DISALLOW_COPY_AND_ASSIGN(AtomCertVerifier); }; diff --git a/atom/browser/net/atom_ct_delegate.cc b/atom/browser/net/atom_ct_delegate.cc new file mode 100644 index 000000000000..66d6c93adb35 --- /dev/null +++ b/atom/browser/net/atom_ct_delegate.cc @@ -0,0 +1,34 @@ +// Copyright (c) 2016 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "atom/browser/net/atom_ct_delegate.h" + +#include "content/public/browser/browser_thread.h" + +namespace atom { + +AtomCTDelegate::AtomCTDelegate() {} + +AtomCTDelegate::~AtomCTDelegate() {} + +void AtomCTDelegate::AddCTExcludedHost(const std::string& host) { + DCHECK_CURRENTLY_ON(content::BrowserThread::IO); + ct_excluded_hosts_.insert(host); +} + +void AtomCTDelegate::ClearCTExcludedHostsList() { + DCHECK_CURRENTLY_ON(content::BrowserThread::IO); + ct_excluded_hosts_.clear(); +} + +AtomCTDelegate::CTRequirementLevel AtomCTDelegate::IsCTRequiredForHost( + const std::string& host) { + DCHECK_CURRENTLY_ON(content::BrowserThread::IO); + if (!ct_excluded_hosts_.empty() && + (ct_excluded_hosts_.find(host) != ct_excluded_hosts_.end())) + return CTRequirementLevel::NOT_REQUIRED; + return CTRequirementLevel::DEFAULT; +} + +} // namespace atom diff --git a/atom/browser/net/atom_ct_delegate.h b/atom/browser/net/atom_ct_delegate.h new file mode 100644 index 000000000000..680071eaa06c --- /dev/null +++ b/atom/browser/net/atom_ct_delegate.h @@ -0,0 +1,33 @@ +// Copyright (c) 2016 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef ATOM_BROWSER_NET_ATOM_CT_DELEGATE_H_ +#define ATOM_BROWSER_NET_ATOM_CT_DELEGATE_H_ + +#include +#include + +#include "net/http/transport_security_state.h" + +namespace atom { + +class AtomCTDelegate : public net::TransportSecurityState::RequireCTDelegate { + public: + AtomCTDelegate(); + ~AtomCTDelegate() override; + + void AddCTExcludedHost(const std::string& host); + void ClearCTExcludedHostsList(); + + // net::TransportSecurityState::RequireCTDelegate: + CTRequirementLevel IsCTRequiredForHost(const std::string& host) override; + + private: + std::set ct_excluded_hosts_; + DISALLOW_COPY_AND_ASSIGN(AtomCTDelegate); +}; + +} // namespace atom + +#endif // ATOM_BROWSER_NET_ATOM_CT_DELEGATE_H_ diff --git a/filenames.gypi b/filenames.gypi index 9c4b42f622d5..f1ee8d069c34 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -227,6 +227,8 @@ 'atom/browser/net/asar/url_request_asar_job.h', 'atom/browser/net/atom_cert_verifier.cc', 'atom/browser/net/atom_cert_verifier.h', + 'atom/browser/net/atom_ct_delegate.cc', + 'atom/browser/net/atom_ct_delegate.h', 'atom/browser/net/atom_cookie_delegate.cc', 'atom/browser/net/atom_cookie_delegate.h', 'atom/browser/net/atom_network_delegate.cc', From 83f47bc9804343f1aa69d9fe2255c4e27db5b51b Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 24 Oct 2016 15:44:21 +0900 Subject: [PATCH 2/3] Upgrade brighray for RequireCTDelegate --- vendor/brightray | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vendor/brightray b/vendor/brightray index 8b074eca1217..a55f26ec2f53 160000 --- a/vendor/brightray +++ b/vendor/brightray @@ -1 +1 @@ -Subproject commit 8b074eca1217db6621553a3f7764c907ef617547 +Subproject commit a55f26ec2f53ccd961381234c941564b4fd4403f From 271733fc53692d3b40dfed10675c499fbdaba4f5 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 24 Oct 2016 16:12:49 +0900 Subject: [PATCH 3/3] Add tests for ses.setCertificateVerifyProc --- spec/api-session-spec.js | 59 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/spec/api-session-spec.js b/spec/api-session-spec.js index c01abcc6edd3..8451fc0cc937 100644 --- a/spec/api-session-spec.js +++ b/spec/api-session-spec.js @@ -1,5 +1,6 @@ const assert = require('assert') const http = require('http') +const https = require('https') const path = require('path') const fs = require('fs') const {closeWindow} = require('./window-helpers') @@ -457,7 +458,7 @@ describe('session module', function () { }) }) - describe('ses.getblobData(identifier, callback)', function () { + describe('ses.getBlobData(identifier, callback)', function () { it('returns blob data for uuid', function (done) { const scheme = 'temp' const protocol = session.defaultSession.protocol @@ -507,4 +508,60 @@ describe('session module', function () { }) }) }) + + describe('ses.setCertificateVerifyProc(callback)', function () { + var server = null + + beforeEach(function (done) { + var certPath = path.join(__dirname, 'fixtures', 'certificates') + var options = { + key: fs.readFileSync(path.join(certPath, 'server.key')), + cert: fs.readFileSync(path.join(certPath, 'server.pem')), + ca: [ + fs.readFileSync(path.join(certPath, 'rootCA.pem')), + fs.readFileSync(path.join(certPath, 'intermediateCA.pem')) + ], + requestCert: true, + rejectUnauthorized: false + } + + server = https.createServer(options, function (req, res) { + res.writeHead(200) + res.end('hello') + }) + server.listen(0, '127.0.0.1', done) + }) + + afterEach(function () { + session.defaultSession.setCertificateVerifyProc(null) + server.close() + }) + + it('accepts the request when the callback is called with true', function (done) { + session.defaultSession.setCertificateVerifyProc(function (hostname, certificate, callback) { + callback(true) + }) + + w.webContents.once('did-finish-load', function () { + assert.equal(w.webContents.getTitle(), 'hello') + done() + }) + w.loadURL(`https://127.0.0.1:${server.address().port}`) + }) + + it('rejects the request when the callback is called with false', function (done) { + session.defaultSession.setCertificateVerifyProc(function (hostname, certificate, callback) { + assert.equal(hostname, '127.0.0.1') + assert.equal(certificate.issuerName, 'Intermediate CA') + callback(false) + }) + + var url = `https://127.0.0.1:${server.address().port}` + w.webContents.once('did-finish-load', function () { + assert.equal(w.webContents.getTitle(), url) + done() + }) + w.loadURL(url) + }) + }) })