From 3408651b2dcea69b2c029475bbbc5b327c58c06d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Blin?= <sebastien.blin@savoirfairelinux.com> Date: Tue, 25 Jul 2023 09:52:14 -0400 Subject: [PATCH] connectionmanager: fix certificate pinning and factory lifecycle 1. findCertificate must pin certificate in certificate store in order to re-use it afterwards. 2. Pass IceTransportFactory in config, else it will be destroyed before all ICE Transport are closed. Change-Id: I86398df9e0f26309719ef46989b422cc4b1b7732 --- include/connectionmanager.h | 3 +++ src/connectionmanager.cpp | 32 +++++++++++++++++++++++++------- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/include/connectionmanager.h b/include/connectionmanager.h index e6ad37b..e459eb3 100644 --- a/include/connectionmanager.h +++ b/include/connectionmanager.h @@ -18,6 +18,7 @@ #include "ice_options.h" #include "multiplexed_socket.h" +#include "ice_transport_factory.h" #include "turn_cache.h" #include <opendht/dhtrunner.h> @@ -248,6 +249,8 @@ struct ConnectionManager::Config tls::CertificateStore* certStore; + dhtnet::IceTransportFactory* factory; + /** * UPnP IGD controller and the mutex to access it */ diff --git a/src/connectionmanager.cpp b/src/connectionmanager.cpp index f58910f..928d209 100644 --- a/src/connectionmanager.cpp +++ b/src/connectionmanager.cpp @@ -16,7 +16,6 @@ */ #include "connectionmanager.h" #include "peer_connection.h" -#include "ice_transport_factory.h" #include "upnp/upnp_control.h" #include "certstore.h" #include "fileutils.h" @@ -246,6 +245,7 @@ public: bool findCertificate(const dht::PkId& id, std::function<void(const std::shared_ptr<dht::crypto::Certificate>&)>&& cb); + bool findCertificate(const dht::InfoHash& h, std::function<void(const std::shared_ptr<dht::crypto::Certificate>&)>&& cb); /** * returns whether or not UPnP is enabled and active @@ -267,8 +267,6 @@ public: std::shared_ptr<ConnectionManager::Config> config_; - IceTransportFactory iceFactory_ {}; - mutable std::mt19937_64 rand; iOSConnectedCallback iOSConnectedCb_ {}; @@ -758,7 +756,7 @@ ConnectionManager::Impl::connectDevice(const std::shared_ptr<dht::crypto::Certif ice_config.master = false; ice_config.streamsCount = 1; ice_config.compCountPerStream = 1; - info->ice_ = sthis->iceFactory_.createUTransport(""); + info->ice_ = sthis->config_->factory->createUTransport(""); if (!info->ice_) { if (sthis->config_->logger) sthis->config_->logger->error("[device {}] Cannot initialize ICE session.", deviceId); @@ -868,7 +866,7 @@ ConnectionManager::Impl::onDhtConnected(const dht::crypto::PublicKey& devicePk) shared->onPeerResponse(req); } else { // Async certificate checking - shared->dht()->findCertificate( + shared->findCertificate( req.from, [w, req = std::move(req)]( const std::shared_ptr<dht::crypto::Certificate>& cert) mutable { @@ -1157,7 +1155,7 @@ ConnectionManager::Impl::onDhtPeerRequest(const PeerConnectionRequest& req, ice_config.streamsCount = 1; ice_config.compCountPerStream = 1; // TCP ice_config.master = true; - info->ice_ = shared->iceFactory_.createUTransport(""); + info->ice_ = shared->config_->factory->createUTransport(""); if (not info->ice_) { if (shared->config_->logger) shared->config_->logger->error("[device {}] Cannot initialize ICE session", deviceId); @@ -1403,7 +1401,7 @@ IceTransportOptions ConnectionManager::Impl::getIceOptions() const noexcept { IceTransportOptions opts; - opts.factory = (IceTransportFactory*)&iceFactory_; + opts.factory = config_->factory; opts.upnpEnable = getUPnPActive(); if (config_->stunEnabled) @@ -1495,6 +1493,26 @@ ConnectionManager::Impl::findCertificate( return true; } +bool +ConnectionManager::Impl::findCertificate(const dht::InfoHash& h, + std::function<void(const std::shared_ptr<dht::crypto::Certificate>&)>&& cb) +{ + if (auto cert = certStore().getCertificate(h.toString())) { + if (cb) + cb(cert); + } else { + dht()->findCertificate(h, + [cb = std::move(cb), this]( + const std::shared_ptr<dht::crypto::Certificate>& crt) { + if (crt) + certStore().pinCertificate(crt); + if (cb) + cb(crt); + }); + } + return true; +} + ConnectionManager::ConnectionManager(std::shared_ptr<ConnectionManager::Config> config_) : pimpl_ {std::make_shared<Impl>(config_)} {} -- GitLab