Merge pull request #7651 from deepak1556/session_cert_verifier_patch

session: exclude hosts from CT verification if they are handled by custom cert verifiers

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(
+AtomBrowserContext::AtomBrowserContext(const std::string& partition,
-    const std::string& partition, bool in_memory,
+                                       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<net::CertVerifier> 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<std::string> 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());

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<net::CertVerifier> CreateCertVerifier() override;
   net::SSLConfigService* CreateSSLConfigService() override;
   std::vector<std::string> 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<WebViewManager> guest_manager_;
   std::unique_ptr<AtomPermissionManager> permission_manager_;
   std::unique_ptr<AtomBlobReader> blob_reader_;
+  std::unique_ptr<AtomCTDelegate> ct_delegate_;
   std::string user_agent_;
   bool use_cache_;
 

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()
+AtomCertVerifier::AtomCertVerifier(AtomCTDelegate* ct_delegate)
-    : default_cert_verifier_(net::CertVerifier::CreateDefault()) {
+    : 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,

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<net::CertVerifier> default_cert_verifier_;
+  AtomCTDelegate* ct_delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(AtomCertVerifier);
 };

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

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 <set>
+#include <string>
+
+#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<std::string> ct_excluded_hosts_;
+  DISALLOW_COPY_AND_ASSIGN(AtomCTDelegate);
+};
+
+}  // namespace atom
+
+#endif  // ATOM_BROWSER_NET_ATOM_CT_DELEGATE_H_

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',

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('<title>hello</title>')
+      })
+      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)
+    })
+  })
 })

vendor/brightray

@@ -1 +1 @@
-Subproject commit 8b074eca1217db6621553a3f7764c907ef617547
+Subproject commit a55f26ec2f53ccd961381234c941564b4fd4403f