From c21dd8f92ec711c71445e5d39e53bf537158684d Mon Sep 17 00:00:00 2001
From: Paul-Louis Ageneau <paul-louis@ageneau.org>
Date: Sat, 15 Jun 2024 16:31:17 +0200
Subject: [PATCH] Add proper synchronization to remote fingerprint

---
 src/impl/peerconnection.cpp | 25 +++++++++++++++----------
 src/impl/peerconnection.hpp | 14 ++++++++------
 2 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/src/impl/peerconnection.cpp b/src/impl/peerconnection.cpp
index c305384aa..ef0125a24 100644
--- a/src/impl/peerconnection.cpp
+++ b/src/impl/peerconnection.cpp
@@ -229,13 +229,15 @@ shared_ptr<DtlsTransport> PeerConnection::initDtlsTransport() {
 
 		PLOG_VERBOSE << "Starting DTLS transport";
 
-		auto fingerprintAlgorithm = CertificateFingerprint::Algorithm::Sha256;
-		if (auto remote = remoteDescription(); remote && remote->fingerprint()) {
-			fingerprintAlgorithm = remote->fingerprint()->algorithm;
+		CertificateFingerprint::Algorithm fingerprintAlgorithm;
+		{
+			std::lock_guard lock(mRemoteDescriptionMutex);
+			if (mRemoteDescription && mRemoteDescription->fingerprint()) {
+				mRemoteFingerprintAlgorithm = mRemoteDescription->fingerprint()->algorithm;
+			}
+			fingerprintAlgorithm = mRemoteFingerprintAlgorithm;
 		}
 
-		mRemoteFingerprintAlgorithm = fingerprintAlgorithm;
-
 		auto lower = std::atomic_load(&mIceTransport);
 		if (!lower)
 			throw std::logic_error("No underlying ICE transport for DTLS transport");
@@ -443,23 +445,25 @@ void PeerConnection::rollbackLocalDescription() {
 
 bool PeerConnection::checkFingerprint(const std::string &fingerprint) {
 	std::lock_guard lock(mRemoteDescriptionMutex);
-	if (!mRemoteDescription || !mRemoteDescription->fingerprint())
+	mRemoteFingerprint = fingerprint;
+
+	if (!mRemoteDescription || !mRemoteDescription->fingerprint()
+			|| mRemoteFingerprintAlgorithm != mRemoteDescription->fingerprint()->algorithm)
 		return false;
 
-  if (config.disableFingerprintVerification) {
+	if (config.disableFingerprintVerification) {
 		PLOG_VERBOSE << "Skipping fingerprint validation";
-		mRemoteFingerprint = fingerprint;
 		return true;
 	}
 
 	auto expectedFingerprint = mRemoteDescription->fingerprint()->value;
 	if (expectedFingerprint == fingerprint) {
 		PLOG_VERBOSE << "Valid fingerprint \"" << fingerprint << "\"";
-		mRemoteFingerprint = fingerprint;
 		return true;
 	}
 
-	PLOG_ERROR << "Invalid fingerprint \"" << fingerprint << "\", expected \"" << expectedFingerprint << "\"";
+	PLOG_ERROR << "Invalid fingerprint \"" << fingerprint << "\", expected \""
+	           << expectedFingerprint << "\"";
 	return false;
 }
 
@@ -1308,6 +1312,7 @@ void PeerConnection::resetCallbacks() {
 }
 
 CertificateFingerprint PeerConnection::remoteFingerprint() {
+	std::lock_guard lock(mRemoteDescriptionMutex);
 	if (mRemoteFingerprint)
 		return {CertificateFingerprint{mRemoteFingerprintAlgorithm, *mRemoteFingerprint}};
 	else
diff --git a/src/impl/peerconnection.hpp b/src/impl/peerconnection.hpp
index 37e07cbd7..a72f58575 100644
--- a/src/impl/peerconnection.hpp
+++ b/src/impl/peerconnection.hpp
@@ -98,6 +98,7 @@ struct PeerConnection : std::enable_shared_from_this<PeerConnection> {
 	bool changeSignalingState(SignalingState newState);
 
 	void resetCallbacks();
+
 	CertificateFingerprint remoteFingerprint();
 
 	// Helper method for asynchronous callback invocation
@@ -135,12 +136,16 @@ struct PeerConnection : std::enable_shared_from_this<PeerConnection> {
 	future_certificate_ptr mCertificate;
 
 	Processor mProcessor;
-	optional<Description> mLocalDescription, mRemoteDescription;
+	optional<Description> mLocalDescription;
 	optional<Description> mCurrentLocalDescription;
-	mutable std::mutex mLocalDescriptionMutex, mRemoteDescriptionMutex;
+	mutable std::mutex mLocalDescriptionMutex;
 
-	shared_ptr<MediaHandler> mMediaHandler;
+	optional<Description> mRemoteDescription;
+	CertificateFingerprint::Algorithm mRemoteFingerprintAlgorithm = CertificateFingerprint::Algorithm::Sha256;
+	optional<string> mRemoteFingerprint;
+	mutable std::mutex mRemoteDescriptionMutex;
 
+	shared_ptr<MediaHandler> mMediaHandler;
 	mutable std::shared_mutex mMediaHandlerMutex;
 
 	shared_ptr<IceTransport> mIceTransport;
@@ -158,9 +163,6 @@ struct PeerConnection : std::enable_shared_from_this<PeerConnection> {
 
 	Queue<shared_ptr<DataChannel>> mPendingDataChannels;
 	Queue<shared_ptr<Track>> mPendingTracks;
-
-	CertificateFingerprint::Algorithm mRemoteFingerprintAlgorithm = CertificateFingerprint::Algorithm::Sha256;
-	optional<string> mRemoteFingerprint;
 };
 
 } // namespace rtc::impl