Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add proper synchronization to remote fingerprint #1212

Merged
merged 1 commit into from
Jun 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions src/impl/peerconnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -1308,6 +1312,7 @@ void PeerConnection::resetCallbacks() {
}

CertificateFingerprint PeerConnection::remoteFingerprint() {
std::lock_guard lock(mRemoteDescriptionMutex);
if (mRemoteFingerprint)
return {CertificateFingerprint{mRemoteFingerprintAlgorithm, *mRemoteFingerprint}};
else
Expand Down
14 changes: 8 additions & 6 deletions src/impl/peerconnection.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down
Loading