From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Jeremy Apthorp Date: Wed, 8 May 2019 17:25:55 -0700 Subject: network_service_allow_remote_certificate_verification_logic.patch This adds a callback from the network service that's used to implement session.setCertificateVerifyCallback. diff --git a/services/network/network_context.cc b/services/network/network_context.cc index 0fb8b2747662373e913689a83dfe985bf0f29073..20e6a7aec3702865916d8cd339e3a2caf1793917 100644 --- a/services/network/network_context.cc +++ b/services/network/network_context.cc @@ -158,6 +158,11 @@ #include "services/network/web_transport.h" #include "url/gurl.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) // gn check does not account for BUILDFLAG(). So, for iOS builds, it will // complain about a missing dependency on the target exposing this header. Add a @@ -601,6 +606,99 @@ void RecordHSTSPreconnectUpgradeReason(HSTSRedirectUpgradeReason reason) { } // namespace +class RemoteCertVerifier : public net::CertVerifier { + public: + class Request : public net::CertVerifier::Request { + public: + Request() {} + ~Request() override = default; + void OnRemoteResponse( + const RequestParams& params, + net::CertVerifyResult* verify_result, + int error_from_upstream, + net::CompletionOnceCallback callback, + int error_from_client, + const net::CertVerifyResult& verify_result_from_client) { + if (error_from_client == net::ERR_ABORTED) { + // use the default + std::move(callback).Run(error_from_upstream); + } else { + // use the override + verify_result->Reset(); + verify_result->verified_cert = verify_result_from_client.verified_cert; + std::move(callback).Run(error_from_client); + } + } + base::WeakPtr GetWeakPtr() { return weak_factory_.GetWeakPtr(); } + private: + base::WeakPtrFactory weak_factory_{this}; + }; + + RemoteCertVerifier(std::unique_ptr upstream): upstream_(std::move(upstream)) { + } + ~RemoteCertVerifier() override = default; + + void Bind( + mojo::PendingRemote client_info) { + client_.reset(); + if (client_info.is_valid()) { + client_.Bind(std::move(client_info)); + } + } + + // CertVerifier implementation + int Verify(const RequestParams& params, + net::CertVerifyResult* verify_result, + net::CompletionOnceCallback callback, + std::unique_ptr* 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, out_req); + return upstream_->Verify(params, verify_result, std::move(callback2), out_req, net_log); + } + + + void SetConfig(const Config& config) override { + upstream_->SetConfig(config); + } + + void AddObserver(CertVerifier::Observer* observer) override { + upstream_->AddObserver(observer); + } + + void RemoveObserver(CertVerifier::Observer* observer) override { + upstream_->RemoveObserver(observer); + } + + void OnRequestFinished(const RequestParams& params, + net::CompletionOnceCallback callback, + net::CertVerifyResult* verify_result, + std::unique_ptr* out_req, + int error) { + if (client_.is_bound()) { + // We take a weak pointer to the request because deletion of the request + // is what signals cancellation. Thus if the request is cancelled, the + // callback won't be called, thus avoiding UAF, because |verify_result| + // is freed when the request is cancelled. + *out_req = std::make_unique(); + base::WeakPtr weak_req = static_cast(out_req->get())->GetWeakPtr(); + client_->Verify(error, *verify_result, params.certificate(), + params.hostname(), params.flags(), params.ocsp_response(), + base::BindOnce(&Request::OnRemoteResponse, + weak_req, params, verify_result, error, std::move(callback))); + } else { + std::move(callback).Run(error); + } + } + + private: + std::unique_ptr upstream_; + mojo::Remote client_; +}; + constexpr uint32_t NetworkContext::kMaxOutstandingRequestsPerProcess; NetworkContext::NetworkContextHttpAuthPreferences:: @@ -997,6 +1095,13 @@ void NetworkContext::SetClient( client_.Bind(std::move(client)); } +void NetworkContext::SetCertVerifierClient( + mojo::PendingRemote client) { + if (remote_cert_verifier_) { + remote_cert_verifier_->Bind(std::move(client)); + } +} + void NetworkContext::CreateURLLoaderFactory( mojo::PendingReceiver receiver, mojom::URLLoaderFactoryParamsPtr params) { @@ -2568,6 +2673,10 @@ URLRequestContextOwner NetworkContext::MakeURLRequestContext( cert_verifier = std::make_unique( std::make_unique( std::move(cert_verifier))); + + auto remote_cert_verifier = std::make_unique(std::move(cert_verifier)); + remote_cert_verifier_ = remote_cert_verifier.get(); + cert_verifier = std::make_unique(std::move(remote_cert_verifier)); } builder.SetCertVerifier(IgnoreErrorsCertVerifier::MaybeWrapCertVerifier( diff --git a/services/network/network_context.h b/services/network/network_context.h index ca3412783e1e754d1d197f61532c84c12ac97d68..e1e6f9bf5ff44bcf5f71bae1ad43fbaf071851b1 100644 --- a/services/network/network_context.h +++ b/services/network/network_context.h @@ -113,6 +113,7 @@ class URLMatcher; } namespace network { +class RemoteCertVerifier; class CookieManager; class HostResolver; class MdnsResponderManager; @@ -245,6 +246,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext void CreateURLLoaderFactory( mojo::PendingReceiver receiver, mojom::URLLoaderFactoryParamsPtr params) override; + void SetCertVerifierClient( + mojo::PendingRemote client) override; void ResetURLLoaderFactories() override; void GetViaObliviousHttp( mojom::ObliviousHttpRequestPtr request, @@ -935,6 +938,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext std::vector dismount_closures_; #endif // BUILDFLAG(IS_DIRECTORY_TRANSFER_REQUIRED) + raw_ptr remote_cert_verifier_ = nullptr; + // Created on-demand. Null if unused. std::unique_ptr internal_host_resolver_; std::set, base::UniquePtrComparator> diff --git a/services/network/public/mojom/network_context.mojom b/services/network/public/mojom/network_context.mojom index ca619101896f29cf2c2793c20968c17c11cd0bab..891a80b394671205765bddc622a12e1002c146ce 100644 --- a/services/network/public/mojom/network_context.mojom +++ b/services/network/public/mojom/network_context.mojom @@ -307,6 +307,17 @@ struct SocketBrokerRemotes { pending_remote server; }; +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 { // The user agent string. @@ -943,6 +954,9 @@ interface NetworkContext { // Sets a client for this network context. SetClient(pending_remote client); + // Sets a certificate verifier client for this network context. + SetCertVerifierClient(pending_remote? client); + // Creates a new URLLoaderFactory with the given |params|. CreateURLLoaderFactory( pending_receiver url_loader_factory, diff --git a/services/network/test/test_network_context.h b/services/network/test/test_network_context.h index 342dbfa64b8be6ad1777b7425302d5bb99e127e0..6b09b32c3a6206310baf87da0a2b4025d488fd09 100644 --- a/services/network/test/test_network_context.h +++ b/services/network/test/test_network_context.h @@ -63,6 +63,8 @@ class TestNetworkContext : public mojom::NetworkContext { void CreateURLLoaderFactory( mojo::PendingReceiver receiver, mojom::URLLoaderFactoryParamsPtr params) override {} + void SetCertVerifierClient( + mojo::PendingRemote client) override {} void GetCookieManager( mojo::PendingReceiver cookie_manager) override {} void GetRestrictedCookieManager(