From 1d4db392c71595f05a5258e16734ff3ec446c115 Mon Sep 17 00:00:00 2001 From: henrika Date: Mon, 18 Sep 2017 02:34:30 -0700 Subject: [PATCH] Revert of If SRTP sessions exist, don't create new ones when applying answer. (patchset #1 id:1 of https://codereview.webrtc.org/3019443002/ ) Reason for revert: Speculative revert since all Android bots on WebRTC FYI started to fail when this CL landed. https://build.chromium.org/p/chromium.webrtc.fyi/builders/Android%20Tests%20%28dbg%29%20%28L%20Nexus6%29 Original issue's description: > 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} > Committed: https://webrtc.googlesource.com/src/+/5ada7acf603e90e71990e9d4ff8f49389f24958c TBR=zhihuang@webrtc.org,deadbeef@webrtc.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=webrtc:8251 NOTRY=TRUE Review-Url: https://codereview.webrtc.org/3017543002 Cr-Commit-Position: refs/heads/master@{#19882} --- pc/srtptransport.cc | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/pc/srtptransport.cc b/pc/srtptransport.cc index 64889c65f6..f3b5309e37 100644 --- a/pc/srtptransport.cc +++ b/pc/srtptransport.cc @@ -174,42 +174,25 @@ bool SrtpTransport::SetRtpParams(int send_cs, int recv_cs, const uint8_t* recv_key, int recv_key_len) { - // 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; - } - + CreateSrtpSessions(); send_session_->SetEncryptedHeaderExtensionIds( send_encrypted_header_extension_ids_); if (external_auth_enabled_) { send_session_->EnableExternalAuth(); } - 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) { + if (!send_session_->SetSend(send_cs, send_key, send_key_len)) { ResetParams(); return false; } recv_session_->SetEncryptedHeaderExtensionIds( recv_encrypted_header_extension_ids_); - 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) { + if (!recv_session_->SetRecv(recv_cs, recv_key, recv_key_len)) { ResetParams(); return false; } - LOG(LS_INFO) << "SRTP " << (new_sessions ? "updated" : "activated") - << " with negotiated parameters:" + LOG(LS_INFO) << "SRTP activated with negotiated parameters:" << " send cipher_suite " << send_cs << " recv cipher_suite " << recv_cs; return true;