refactor: nws13n: setCertificateVerifyProc (#18221)

patches/chromium/.patches

@@ -78,3 +78,4 @@ cross_site_document_resource_handler.patch
 woa_compiler_workaround.patch
 crashpad_pid_check.patch
 chore_add_debounce_on_the_updatewebcontentsvisibility_method_to.patch
+network_service_allow_remote_certificate_verification_logic.patch

patches/chromium/network_service_allow_remote_certificate_verification_logic.patch

@@ -0,0 +1,196 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jeremy Apthorp <nornagon@nornagon.net>
+Date: Wed, 8 May 2019 17:25:55 -0700
+Subject: network service: allow remote certificate verification logic
+
+
+diff --git a/services/network/network_context.cc b/services/network/network_context.cc
+index 5d871fc08218b71c93141963db66465f775b1bc2..97128a6b6223e7fb977b5c9f20bf18da26c8348d 100644
+--- a/services/network/network_context.cc
++++ b/services/network/network_context.cc
+@@ -89,6 +89,11 @@
+ #include "services/network/url_loader.h"
+ #include "services/network/url_request_context_builder_mojo.h"
+ 
++// Electron
++#include "net/cert/caching_cert_verifier.h"
++#include "net/cert/cert_verify_proc.h"
++#include "net/cert/multi_threaded_cert_verifier.h"
++
+ #if BUILDFLAG(IS_CT_SUPPORTED)
+ #include "components/certificate_transparency/chrome_ct_policy_enforcer.h"
+ #include "components/certificate_transparency/chrome_require_ct_delegate.h"
+@@ -310,6 +315,75 @@ std::string HashesToBase64String(const net::HashValueVector& hashes) {
+ 
+ }  // namespace
+ 
++class RemoteCertVerifier : public net::CertVerifier {
++ public:
++  RemoteCertVerifier(std::unique_ptr<net::CertVerifier> upstream): upstream_(std::move(upstream)) {
++  }
++  ~RemoteCertVerifier() override = default;
++
++  void Bind(mojom::CertVerifierClientPtr client_info) {
++    client_ = std::move(client_info);
++  }
++
++  // CertVerifier implementation
++  int Verify(const RequestParams& params,
++             net::CertVerifyResult* verify_result,
++             net::CompletionOnceCallback callback,
++             std::unique_ptr<Request>* out_req,
++             const net::NetLogWithSource& net_log) override {
++    out_req->reset();
++
++    net::CompletionOnceCallback callback2 = base::BindOnce(
++        &RemoteCertVerifier::OnRequestFinished, base::Unretained(this),
++        params, std::move(callback), verify_result);
++    int result = upstream_->Verify(params, verify_result,
++                                   std::move(callback2), out_req, net_log);
++    if (result != net::ERR_IO_PENDING) {
++      // Synchronous completion
++    }
++
++    return result;
++  }
++
++
++  void SetConfig(const Config& config) override {
++    upstream_->SetConfig(config);
++  }
++
++  void OnRequestFinished(const RequestParams& params, net::CompletionOnceCallback callback, net::CertVerifyResult* verify_result, int error) {
++    if (client_) {
++      client_->Verify(error, *verify_result, params.certificate(),
++          params.hostname(), params.flags(), params.ocsp_response(),
++          base::BindOnce(&RemoteCertVerifier::OnRemoteResponse,
++            base::Unretained(this), params, verify_result, error,
++            std::move(callback)));
++    } else {
++      std::move(callback).Run(error);
++    }
++  }
++
++  void OnRemoteResponse(
++      const RequestParams& params,
++      net::CertVerifyResult* verify_result,
++      int error,
++      net::CompletionOnceCallback callback,
++      int error2,
++      const net::CertVerifyResult& verify_result2) {
++    if (error2 == net::ERR_ABORTED) {
++      // use the default
++      std::move(callback).Run(error);
++    } else {
++      // use the override
++      verify_result->Reset();
++      verify_result->verified_cert = verify_result2.verified_cert;
++      std::move(callback).Run(error2);
++    }
++  }
++ private:
++  std::unique_ptr<net::CertVerifier> upstream_;
++  mojom::CertVerifierClientPtr client_;
++};
++
+ constexpr uint32_t NetworkContext::kMaxOutstandingRequestsPerProcess;
+ constexpr bool NetworkContext::enable_resource_scheduler_;
+ 
+@@ -668,6 +742,12 @@ void NetworkContext::SetClient(mojom::NetworkContextClientPtr client) {
+   client_ = std::move(client);
+ }
+ 
++void NetworkContext::SetCertVerifierClient(mojom::CertVerifierClientPtr client) {
++  if (remote_cert_verifier_) {
++    remote_cert_verifier_->Bind(std::move(client));
++  }
++}
++
+ void NetworkContext::CreateURLLoaderFactory(
+     mojom::URLLoaderFactoryRequest request,
+     mojom::URLLoaderFactoryParamsPtr params) {
+@@ -2130,12 +2210,19 @@ URLRequestContextOwner NetworkContext::MakeURLRequestContext() {
+               net::CreateCertVerifyProcBuiltin(cert_net_fetcher_)));
+     }
+ #endif
+-    if (!cert_verifier)
+-      cert_verifier = net::CertVerifier::CreateDefault(cert_net_fetcher_);
++    if (!cert_verifier) {
++      auto mt_verifier = std::make_unique<net::MultiThreadedCertVerifier>(
++              net::CertVerifyProc::CreateDefault(std::move(cert_net_fetcher_)));
++      auto remote_cert_verifier = std::make_unique<RemoteCertVerifier>(std::move(mt_verifier));
++      remote_cert_verifier_ = remote_cert_verifier.get();
++      cert_verifier = std::make_unique<net::CachingCertVerifier>(std::move(remote_cert_verifier));
++    }
+   }
+ 
+-  builder.SetCertVerifier(IgnoreErrorsCertVerifier::MaybeWrapCertVerifier(
+-      *command_line, nullptr, std::move(cert_verifier)));
++  cert_verifier = IgnoreErrorsCertVerifier::MaybeWrapCertVerifier(
++      *command_line, nullptr, std::move(cert_verifier));
++
++  builder.SetCertVerifier(std::move(cert_verifier));
+ 
+   std::unique_ptr<net::NetworkDelegate> network_delegate =
+       std::make_unique<NetworkServiceNetworkDelegate>(this);
+diff --git a/services/network/network_context.h b/services/network/network_context.h
+index 0f9e0fe5922c228d96ba7d8668a88d5d31c516e9..9552cfa88d2a45aa6bff24c3e89d080c8aad98a0 100644
+--- a/services/network/network_context.h
++++ b/services/network/network_context.h
+@@ -74,6 +74,7 @@ class DomainReliabilityMonitor;
+ 
+ namespace network {
+ class CertVerifierWithTrustAnchors;
++class RemoteCertVerifier;
+ class CookieManager;
+ class ExpectCTReporter;
+ class HostResolver;
+@@ -176,6 +177,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
+ 
+   // mojom::NetworkContext implementation:
+   void SetClient(mojom::NetworkContextClientPtr client) override;
++  void SetCertVerifierClient(mojom::CertVerifierClientPtr client) override;
+   void CreateURLLoaderFactory(mojom::URLLoaderFactoryRequest request,
+                               mojom::URLLoaderFactoryParamsPtr params) override;
+   void ResetURLLoaderFactories() override;
+@@ -561,6 +563,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
+   std::unique_ptr<network::NSSTempCertsCacheChromeOS> nss_temp_certs_cache_;
+ #endif
+ 
++  RemoteCertVerifier* remote_cert_verifier_ = nullptr;
++
+   // CertNetFetcher used by the context's CertVerifier. May be nullptr if
+   // CertNetFetcher is not used by the current platform.
+   scoped_refptr<net::CertNetFetcherImpl> cert_net_fetcher_;
+diff --git a/services/network/public/mojom/network_context.mojom b/services/network/public/mojom/network_context.mojom
+index 69885a8bc0e2219ed6c10e684db0ad7d5cd6b87e..bf0750e75e357275f5a569cc8e0680b606b544f6 100644
+--- a/services/network/public/mojom/network_context.mojom
++++ b/services/network/public/mojom/network_context.mojom
+@@ -160,6 +160,17 @@ interface TrustedURLLoaderHeaderClient {
+   OnLoaderCreated(int32 request_id, TrustedHeaderClient& header_client);
+ };
+ 
++interface CertVerifierClient {
++  Verify(
++    int32 default_error,
++    CertVerifyResult default_result,
++    X509Certificate certificate,
++    string hostname,
++    int32 flags,
++    string? ocsp_response
++  ) => (int32 error_code, CertVerifyResult result);
++};
++
+ // Parameters for constructing a network context.
+ struct NetworkContextParams {
+   // Name used by memory tools to identify the context.
+@@ -541,6 +552,9 @@ interface NetworkContext {
+   // Sets a client for this network context.
+   SetClient(NetworkContextClient client);
+ 
++  // Sets a certificate verifier client for this network context.
++  SetCertVerifierClient(CertVerifierClient? client);
++
+   // Creates a new URLLoaderFactory with the given |params|.
+   CreateURLLoaderFactory(URLLoaderFactory& url_loader_factory,
+                          URLLoaderFactoryParams params);

shell/browser/api/atom_api_session.cc

@@ -28,6 +28,7 @@
 #include "content/public/browser/download_manager_delegate.h"
 #include "content/public/browser/network_service_instance.h"
 #include "content/public/browser/storage_partition.h"
+#include "mojo/public/cpp/bindings/strong_binding.h"
 #include "native_mate/dictionary.h"
 #include "native_mate/object_template_builder.h"
 #include "net/base/completion_repeating_callback.h"
@@ -39,6 +40,7 @@
 #include "net/url_request/static_http_user_agent_settings.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
+#include "services/network/network_service.h"
 #include "services/network/public/cpp/features.h"
 #include "shell/browser/api/atom_api_cookies.h"
 #include "shell/browser/api/atom_api_download_item.h"
@@ -415,6 +417,40 @@ void Session::DisableNetworkEmulation() {
       network_emulation_token_, network::mojom::NetworkConditions::New());
 }
 
+class ElectronCertVerifierClient : public network::mojom::CertVerifierClient {
+ public:
+  using CertVerifyProc =
+      base::RepeatingCallback<void(const VerifyRequestParams& request,
+                                   base::RepeatingCallback<void(int)>)>;
+  explicit ElectronCertVerifierClient(CertVerifyProc proc)
+      : cert_verify_proc_(proc) {}
+  ~ElectronCertVerifierClient() override = default;
+
+  // network::mojom::CertVerifierClient
+  void Verify(int default_error,
+              const net::CertVerifyResult& default_result,
+              const scoped_refptr<net::X509Certificate>& certificate,
+              const std::string& hostname,
+              int flags,
+              const base::Optional<std::string>& ocsp_response,
+              VerifyCallback callback) override {
+    VerifyRequestParams params;
+    params.hostname = hostname;
+    params.default_result = net::ErrorToString(default_error);
+    params.error_code = default_error;
+    params.certificate = certificate;
+    cert_verify_proc_.Run(
+        params,
+        base::AdaptCallbackForRepeating(base::BindOnce(
+            [](VerifyCallback callback, const net::CertVerifyResult& result,
+               int err) { std::move(callback).Run(err, result); },
+            std::move(callback), default_result)));
+  }
+
+ private:
+  CertVerifyProc cert_verify_proc_;
+};
+
 void WrapVerifyProc(
     base::RepeatingCallback<void(const VerifyRequestParams& request,
                                  base::RepeatingCallback<void(int)>)> proc,
@@ -425,19 +461,32 @@ void WrapVerifyProc(
 
 void Session::SetCertVerifyProc(v8::Local<v8::Value> val,
                                 mate::Arguments* args) {
-  base::RepeatingCallback<void(const VerifyRequestParams& request,
+  ElectronCertVerifierClient::CertVerifyProc proc;
-                               base::RepeatingCallback<void(int)>)>
-      proc;
   if (!(val->IsNull() || mate::ConvertFromV8(args->isolate(), val, &proc))) {
     args->ThrowError("Must pass null or function");
     return;
   }
 
-  base::PostTaskWithTraits(
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
-      FROM_HERE, {BrowserThread::IO},
+    network::mojom::CertVerifierClientPtr cert_verifier_client;
-      base::BindOnce(&SetCertVerifyProcInIO,
+    if (proc) {
-                     WrapRefCounted(browser_context_->GetRequestContext()),
+      mojo::MakeStrongBinding(
-                     base::BindRepeating(&WrapVerifyProc, proc)));
+          std::make_unique<ElectronCertVerifierClient>(proc),
+          mojo::MakeRequest(&cert_verifier_client));
+    }
+    content::BrowserContext::GetDefaultStoragePartition(browser_context_.get())
+        ->GetNetworkContext()
+        ->SetCertVerifierClient(std::move(cert_verifier_client));
+
+    // This causes the cert verifier cache to be cleared.
+    content::GetNetworkService()->OnCertDBChanged();
+  } else {
+    base::PostTaskWithTraits(
+        FROM_HERE, {BrowserThread::IO},
+        base::BindOnce(&SetCertVerifyProcInIO,
+                       WrapRefCounted(browser_context_->GetRequestContext()),
+                       base::BindRepeating(&WrapVerifyProc, proc)));
+  }
 }
 
 void Session::SetPermissionRequestHandler(v8::Local<v8::Value> val,

spec-main/api-session-spec.js

@@ -468,6 +468,26 @@ describe('session module', () => {
       await expect(w.loadURL(url)).to.eventually.be.rejectedWith(/ERR_FAILED/)
       expect(w.webContents.getTitle()).to.equal(url)
     })
+
+    it('saves cached results', async () => {
+      let numVerificationRequests = 0
+      session.defaultSession.setCertificateVerifyProc(({ hostname, certificate, verificationResult }, callback) => {
+        numVerificationRequests++
+        callback(-2)
+      })
+
+      const url = `https://127.0.0.1:${server.address().port}`
+      await expect(w.loadURL(url), 'first load').to.eventually.be.rejectedWith(/ERR_FAILED/)
+      await emittedOnce(w.webContents, 'did-stop-loading')
+      await expect(w.loadURL(url + '/test'), 'second load').to.eventually.be.rejectedWith(/ERR_FAILED/)
+      expect(w.webContents.getTitle()).to.equal(url + '/test')
+
+      // TODO(nornagon): there's no way to check if the network service is
+      // enabled from JS, so once we switch it on by default just change this
+      // test :)
+      const networkServiceEnabled = false
+      expect(numVerificationRequests).to.equal(networkServiceEnabled ? 1 : 2)
+    })
   })
 
   describe('ses.clearAuthCache(options)', () => {