From 5ada7acf603e90e71990e9d4ff8f49389f24958c Mon Sep 17 00:00:00 2001 From: deadbeef Date: Fri, 15 Sep 2017 17:52:36 -0700 Subject: [PATCH] If SRTP sessions exist, don't create new ones when applying answer. Instead, call the "Update" methods of SrtpSession, which will just call srtp_update, instead of wiping out the session state completely. This was causing decryption to stop working when subsequent offers/answers are applied. We don't know enough about SRTP to understand the root cause, and I wasn't able to write an integration test that reproduces the issue... But at least this fixes the bug that can be reproduced reliably using Hangouts. BUG=webrtc:8251 Review-Url: https://codereview.webrtc.org/3019443002 Cr-Commit-Position: refs/heads/master@{#19874} --- pc/srtptransport.cc | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/pc/srtptransport.cc b/pc/srtptransport.cc index f3b5309e37..64889c65f6 100644 --- a/pc/srtptransport.cc +++ b/pc/srtptransport.cc @@ -174,25 +174,42 @@ bool SrtpTransport::SetRtpParams(int send_cs, int recv_cs, const uint8_t* recv_key, int recv_key_len) { - CreateSrtpSessions(); + // If parameters are being set for the first time, we should create new SRTP + // sessions and call "SetSend/SetRecv". Otherwise we should call + // "UpdateSend"/"UpdateRecv" on the existing sessions, which will internally + // call "srtp_update". + bool new_sessions = false; + if (!send_session_) { + RTC_DCHECK(!recv_session_); + CreateSrtpSessions(); + new_sessions = true; + } + send_session_->SetEncryptedHeaderExtensionIds( send_encrypted_header_extension_ids_); if (external_auth_enabled_) { send_session_->EnableExternalAuth(); } - if (!send_session_->SetSend(send_cs, send_key, send_key_len)) { + bool ret = new_sessions + ? send_session_->SetSend(send_cs, send_key, send_key_len) + : send_session_->UpdateSend(send_cs, send_key, send_key_len); + if (!ret) { ResetParams(); return false; } recv_session_->SetEncryptedHeaderExtensionIds( recv_encrypted_header_extension_ids_); - if (!recv_session_->SetRecv(recv_cs, recv_key, recv_key_len)) { + ret = new_sessions + ? recv_session_->SetRecv(recv_cs, recv_key, recv_key_len) + : recv_session_->UpdateRecv(recv_cs, recv_key, recv_key_len); + if (!ret) { ResetParams(); return false; } - LOG(LS_INFO) << "SRTP activated with negotiated parameters:" + LOG(LS_INFO) << "SRTP " << (new_sessions ? "updated" : "activated") + << " with negotiated parameters:" << " send cipher_suite " << send_cs << " recv cipher_suite " << recv_cs; return true;