Allow remote fingerprint update during a call

talk/session/media/channel.cc

Index: talk/session/media/channel.cc
diff --git a/talk/session/media/channel.cc b/talk/session/media/channel.cc
index ae27e332f870e8ec7eb87c04ac60de43c8ac904c..b848bd7e5668ae645d52e054d56010f2b13740c5 100644
--- a/talk/session/media/channel.cc
+++ b/talk/session/media/channel.cc
@@ -173,6 +173,7 @@ BaseChannel::BaseChannel(rtc::Thread* thread,
       rtcp_transport_enabled_(rtcp),
       transport_channel_(nullptr),
       rtcp_transport_channel_(nullptr),
+      srtp_filter_(new SrtpFilter()),
       enabled_(false),
       writable_(false),
       rtp_ready_to_send_(false),
@@ -336,6 +337,7 @@ void BaseChannel::ConnectToTransportChannel(TransportChannel* tc) {
   tc->SignalWritableState.connect(this, &BaseChannel::OnWritableState);
   tc->SignalReadPacket.connect(this, &BaseChannel::OnChannelRead);
   tc->SignalReadyToSend.connect(this, &BaseChannel::OnReadyToSend);
+  tc->SignalDtlsSrtpSetup.connect(this, &BaseChannel::OnDtlsSrtpSetup);
 }
 
 void BaseChannel::DisconnectFromTransportChannel(TransportChannel* tc) {
@@ -416,8 +418,7 @@ bool BaseChannel::IsReadyToReceive() const {
 bool BaseChannel::IsReadyToSend() const {
   // Send outgoing data if we are enabled, have local and remote content,
   // and we have had some form of connectivity.
-  return enabled() &&
-         IsReceiveContentDirection(remote_content_direction_) &&
+  return enabled() && IsReceiveContentDirection(remote_content_direction_) &&
          IsSendContentDirection(local_content_direction_) &&
          was_ever_writable();
 }
@@ -474,6 +475,13 @@ void BaseChannel::OnReadyToSend(TransportChannel* channel) {
   SetReadyToSend(channel == rtcp_transport_channel_, true);
 }
 
+void BaseChannel::OnDtlsSrtpSetup(TransportChannel* channel) {

[pthatcher1, 2015/11/18 20:42:43]

Since this will be fired on the worker thread, we might want to put an "_w" on
the end.

+  // When the channel signals the DTLS-SRTP has been setup, set the
+  // |was_ever_writable| to false, allowing re-setup SRTP here.
+  LOG(LS_INFO) << "TransportChannel has DTLS-SRTP set up.";
+  was_ever_writable_ = false;

[pthatcher1, 2015/11/18 20:42:43]

That's not what "was ever writable" means.  It it was ever writable, you can't
go back to not ever being writable.

Why not just do "srtp_filter_.reset(new SrtpFilter());" right here and then wait
until it's writable before setting up the keys?  

In fact, it might make sense to leave it null when it's not setup, and then do
srtp_filter_.release() right here and then check for null in ChannelWritable_w()
to see if it should be setup again.

Now that I think about it, that probably makes the most sense because then all
the places where we would use srtp_filter_, we can check to see if it's null,
and if it is, that's when we can drop a packet because we can't decrypt it.

+}
+
 void BaseChannel::SetReadyToSend(bool rtcp, bool ready) {
   if (rtcp) {
     rtcp_ready_to_send_ = ready;
@@ -539,7 +547,7 @@ bool BaseChannel::SendPacket(bool rtcp,
   rtc::PacketOptions updated_options;
   updated_options = options;
   // Protect if needed.
-  if (srtp_filter_.IsActive()) {
+  if (srtp_filter_->IsActive()) {
     bool res;
     uint8_t* data = packet->data();
     int len = static_cast<int>(packet->size());
@@ -550,19 +558,19 @@ bool BaseChannel::SendPacket(bool rtcp,
     // Socket layer will update rtp sendtime extension header if present in
     // packet with current time before updating the HMAC.
 #if !defined(ENABLE_EXTERNAL_AUTH)
-      res = srtp_filter_.ProtectRtp(
+      res = srtp_filter_->ProtectRtp(
           data, len, static_cast<int>(packet->capacity()), &len);
 #else
       updated_options.packet_time_params.rtp_sendtime_extension_id =
           rtp_abs_sendtime_extn_id_;
-      res = srtp_filter_.ProtectRtp(
+      res = srtp_filter_->ProtectRtp(
           data, len, static_cast<int>(packet->capacity()), &len,
           &updated_options.packet_time_params.srtp_packet_index);
       // If protection succeeds, let's get auth params from srtp.
       if (res) {
         uint8_t* auth_key = NULL;
         int key_len;
-        res = srtp_filter_.GetRtpAuthParams(
+        res = srtp_filter_->GetRtpAuthParams(
             &auth_key, &key_len,
             &updated_options.packet_time_params.srtp_auth_tag_len);
         if (res) {
@@ -583,9 +591,8 @@ bool BaseChannel::SendPacket(bool rtcp,
         return false;
       }
     } else {
-      res = srtp_filter_.ProtectRtcp(data, len,
-                                     static_cast<int>(packet->capacity()),
-                                     &len);
+      res = srtp_filter_->ProtectRtcp(
+          data, len, static_cast<int>(packet->capacity()), &len);
       if (!res) {
         int type = -1;
         GetRtcpType(data, len, &type);
@@ -647,12 +654,12 @@ void BaseChannel::HandlePacket(bool rtcp, rtc::Buffer* packet,
   }
 
   // Unprotect the packet, if needed.
-  if (srtp_filter_.IsActive()) {
+  if (srtp_filter_->IsActive()) {
     char* data = packet->data<char>();
     int len = static_cast<int>(packet->size());
     bool res;
     if (!rtcp) {
-      res = srtp_filter_.UnprotectRtp(data, len, &len);
+      res = srtp_filter_->UnprotectRtp(data, len, &len);
       if (!res) {
         int seq_num = -1;
         uint32_t ssrc = 0;
@@ -664,7 +671,7 @@ void BaseChannel::HandlePacket(bool rtcp, rtc::Buffer* packet,
         return;
       }
     } else {
-      res = srtp_filter_.UnprotectRtcp(data, len, &len);
+      res = srtp_filter_->UnprotectRtcp(data, len, &len);
       if (!res) {
         int type = -1;
         GetRtcpType(data, len, &type);
@@ -776,7 +783,11 @@ void BaseChannel::ChannelWritable_w() {
   }
 
   // If we're doing DTLS-SRTP, now is the time.
-  if (!was_ever_writable_ && ShouldSetupDtlsSrtp()) {
+  if (!was_ever_writable_ && ShouldSetupDtlsSrtp() && IsUsingDtlsSrtp()) {
+    // If we're using DTLS-SRTP, instead of resetting the SRTP context, just
+    // recreate a SRTP context.
+    srtp_filter_.reset(new SrtpFilter());
+
     if (!SetupDtlsSrtp(false)) {
       SignalDtlsSetupFailure_w(false);
       return;
@@ -822,6 +833,11 @@ bool BaseChannel::ShouldSetupDtlsSrtp() const {
   return true;
 }
 
+bool BaseChannel::IsUsingDtlsSrtp() const {
+  return (transport_channel_ && transport_channel_->IsDtlsActive()) ||
+         (rtcp_transport_channel_ && rtcp_transport_channel_->IsDtlsActive());
+}
+
 // This function returns true if either DTLS-SRTP is not in use
 // *or* DTLS-SRTP is successfully set up.
 bool BaseChannel::SetupDtlsSrtp(bool rtcp_channel) {
@@ -893,21 +909,13 @@ bool BaseChannel::SetupDtlsSrtp(bool rtcp_channel) {
   }
 
   if (rtcp_channel) {
-    ret = srtp_filter_.SetRtcpParams(
-        selected_cipher,
-        &(*send_key)[0],
-        static_cast<int>(send_key->size()),
-        selected_cipher,
-        &(*recv_key)[0],
-        static_cast<int>(recv_key->size()));
+    ret = srtp_filter_->SetRtcpParams(
+        selected_cipher, &(*send_key)[0], static_cast<int>(send_key->size()),
+        selected_cipher, &(*recv_key)[0], static_cast<int>(recv_key->size()));
   } else {
-    ret = srtp_filter_.SetRtpParams(
-        selected_cipher,
-        &(*send_key)[0],
-        static_cast<int>(send_key->size()),
-        selected_cipher,
-        &(*recv_key)[0],
-        static_cast<int>(recv_key->size()));
+    ret = srtp_filter_->SetRtpParams(
+        selected_cipher, &(*send_key)[0], static_cast<int>(send_key->size()),
+        selected_cipher, &(*recv_key)[0], static_cast<int>(recv_key->size()));
   }
 
   if (!ret)
@@ -987,21 +995,21 @@ bool BaseChannel::SetSrtp_w(const std::vector<CryptoParams>& cryptos,
       // If DTLS is already active on the channel, we could be renegotiating
       // here. We don't update the srtp filter.
       if (!dtls) {
-        ret = srtp_filter_.SetOffer(cryptos, src);
+        ret = srtp_filter_->SetOffer(cryptos, src);
       }
       break;
     case CA_PRANSWER:
       // If we're doing DTLS-SRTP, we don't want to update the filter
       // with an answer, because we already have SRTP parameters.
       if (!dtls) {
-        ret = srtp_filter_.SetProvisionalAnswer(cryptos, src);
+        ret = srtp_filter_->SetProvisionalAnswer(cryptos, src);
       }
       break;
     case CA_ANSWER:
       // If we're doing DTLS-SRTP, we don't want to update the filter
       // with an answer, because we already have SRTP parameters.
       if (!dtls) {
-        ret = srtp_filter_.SetAnswer(cryptos, src);
+        ret = srtp_filter_->SetAnswer(cryptos, src);
       }
       break;
     default: