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 54b5a281161187f887032d9445f21c541b9691e5..4c8982a8ae3ae23fbeef244cd23b43c889d3fa94 100644 --- a/services/network/network_context.cc +++ b/services/network/network_context.cc @@ -127,6 +127,11 @@ #include "third_party/abseil-cpp/absl/types/optional.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) #include "components/certificate_transparency/chrome_ct_policy_enforcer.h" #include "components/certificate_transparency/chrome_require_ct_delegate.h" @@ -434,6 +439,91 @@ bool GetFullDataFilePath( } // 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 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::PendingCertVerify::PendingCertVerify() = default; @@ -678,6 +768,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) { @@ -2242,6 +2339,9 @@ URLRequestContextOwner NetworkContext::MakeURLRequestContext( std::move(cert_verifier)); cert_verifier = base::WrapUnique(cert_verifier_with_trust_anchors_); #endif // BUILDFLAG(IS_CHROMEOS) + 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 3fd4c43562aa5de00d8698096f154522420c8f24..eed05c05d1da28284af6ae0cef45dbbb47254058 100644 --- a/services/network/network_context.h +++ b/services/network/network_context.h @@ -105,6 +105,7 @@ class URLMatcher; namespace network { class CertVerifierWithTrustAnchors; +class RemoteCertVerifier; class CookieManager; class ExpectCTReporter; class HostResolver; @@ -221,6 +222,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 GetCookieManager( mojo::PendingReceiver receiver) override; @@ -796,6 +799,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext CertVerifierWithTrustAnchors* cert_verifier_with_trust_anchors_ = nullptr; #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, or if the actual // net::CertVerifier is instantiated outside of the network service. diff --git a/services/network/public/mojom/network_context.mojom b/services/network/public/mojom/network_context.mojom index 64199b44316ab4671941c734444d207591512d0a..c929eb10af5b8856debdc0d665eac767e9730714 100644 --- a/services/network/public/mojom/network_context.mojom +++ b/services/network/public/mojom/network_context.mojom @@ -280,6 +280,17 @@ struct NetworkContextFilePaths { bool trigger_migration = false; }; +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. @@ -829,6 +840,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, URLLoaderFactoryParams params);