Completed the functionalities of SrtpTransport.

webrtc/pc/channel.cc

 /*
  *  Copyright 2004 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 100 matching lines @@
   return direction == MD_SENDRECV || direction == MD_SENDONLY;
 }
 
 static const MediaContentDescription* GetContentDescription(
     const ContentInfo* cinfo) {
   if (cinfo == NULL)
     return NULL;
   return static_cast<const MediaContentDescription*>(cinfo->description);
 }
 
+static bool IsInsecureRtp(const std::string& protocol) {
+  return protocol == "RTP/AVPF" || protocol == "RTP/AVP";

[Taylor Brandstetter, 2017/08/26 02:40:38]

Looking elsewhere, it looks like we typically use "cryptos.empty()" to tell if
SDES is being used, and "IsDtlsActive" to tell if DTLS is being used. Also, we
actually currently accept the "RTP/AVP" string in chromium for DTLS (I checked
using the SDP munging sample). So, sorry for leading you wrong by suggesting
using the protocol string, but I think we should aim for minimizing unintended
side effects of this CL, and do something similar to CheckSrtpConfig_n

[Zhi Huang, 2017/08/29 18:40:34]

Removed.

+}
+
 template <class Codec>
 void RtpParametersFromMediaDescription(
     const MediaContentDescriptionImpl<Codec>* desc,
     const RtpHeaderExtensions& extensions,
     RtpParameters<Codec>* params) {
   // TODO(pthatcher): Remove this once we're sure no one will give us
   // a description without codecs (currently a CA_UPDATE with just
   // streams can).
   if (desc->has_codecs()) {
     params->codecs = desc->codecs();
@@ skipping 20 matching lines @@
                          rtc::Thread* signaling_thread,
                          MediaChannel* media_channel,
                          const std::string& content_name,
                          bool rtcp_mux_required,
                          bool srtp_required)
     : worker_thread_(worker_thread),
       network_thread_(network_thread),
       signaling_thread_(signaling_thread),
       content_name_(content_name),
       rtcp_mux_required_(rtcp_mux_required),
-      rtp_transport_(
-          srtp_required
-              ? rtc::WrapUnique<webrtc::RtpTransportInternal>(
-                    new webrtc::SrtpTransport(rtcp_mux_required, content_name))
-              : rtc::MakeUnique<webrtc::RtpTransport>(rtcp_mux_required)),
       srtp_required_(srtp_required),
       media_channel_(media_channel),
       selected_candidate_pair_(nullptr) {
   RTC_DCHECK(worker_thread_ == rtc::Thread::Current());
+  // Always create an SrtpTransport for |rtp_transport_|. If insecure protocol
+  // is used, |srtp_transport_| will be set to nullptr and |rtp_transport_| will
+  // act as a plain RtpTransport.
+  // TODO(zhihuang): Move the transport creation to the TransportController.
+  auto transport =
+      rtc::MakeUnique<webrtc::SrtpTransport>(rtcp_mux_required, content_name);
+  srtp_transport_ = transport.get();
+  rtp_transport_ = std::move(transport);
 #if defined(ENABLE_EXTERNAL_AUTH)
-  srtp_filter_.EnableExternalAuth();
+  srtp_transport_->EnableExternalAuth();
 #endif
   rtp_transport_->SignalReadyToSend.connect(
       this, &BaseChannel::OnTransportReadyToSend);
   // TODO(zstein):  RtpTransport::SignalPacketReceived will probably be replaced
   // with a callback interface later so that the demuxer can select which
   // channel to signal.
   rtp_transport_->SignalPacketReceived.connect(this,
                                                &BaseChannel::OnPacketReceived);
   LOG(LS_INFO) << "Created channel for " << content_name;
 }
@@ skipping 132 matching lines @@
     return;
   }
 
   // When using DTLS-SRTP, we must reset the SrtpFilter every time the transport
   // changes and wait until the DTLS handshake is complete to set the newly
   // negotiated parameters.
   if (ShouldSetupDtlsSrtp_n()) {
     // Set |writable_| to false such that UpdateWritableState_w can set up
     // DTLS-SRTP when |writable_| becomes true again.
     writable_ = false;
-    srtp_filter_.ResetParams();
+    dtls_active_ = false;
+    if (srtp_transport_) {
+      srtp_transport_->ResetParams();
+    }
   }
 
   // If this BaseChannel doesn't require RTCP mux and we haven't fully
   // negotiated RTCP mux, we need an RTCP transport.
   if (rtcp_packet_transport) {
     LOG(LS_INFO) << "Setting RTCP Transport for " << content_name() << " on "
                  << debug_name << " transport " << rtcp_packet_transport;
     SetTransport_n(true, rtcp_dtls_transport, rtcp_packet_transport);
   }
 
@@ skipping 35 matching lines @@
     rtp_transport_->SetRtpPacketTransport(new_packet_transport);
   }
   old_dtls_transport = new_dtls_transport;
 
   // If there's no new transport, we're done after disconnecting from old one.
   if (!new_packet_transport) {
     return;
   }
 
   if (rtcp && new_dtls_transport) {
-    RTC_CHECK(!(ShouldSetupDtlsSrtp_n() && srtp_filter_.IsActive()))
-        << "Setting RTCP for DTLS/SRTP after SrtpFilter is active "
+    RTC_CHECK(!(ShouldSetupDtlsSrtp_n() && secure()))
+        << "Setting RTCP for DTLS/SRTP after the channel is secure "
         << "should never happen.";
   }
 
   if (new_dtls_transport) {
     ConnectToDtlsTransport(new_dtls_transport);
   } else {
     ConnectToPacketTransport(new_packet_transport);
   }
   auto& socket_options = rtcp ? rtcp_socket_options_ : socket_options_;
   for (const auto& pair : socket_options) {
@@ skipping 130 matching lines @@
   // Need to access some state updated on the network thread.
   return network_thread_->Invoke<bool>(
       RTC_FROM_HERE, Bind(&BaseChannel::IsReadyToSendMedia_n, this));
 }
 
 bool BaseChannel::IsReadyToSendMedia_n() 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_) &&
          IsSendContentDirection(local_content_direction_) &&
-         was_ever_writable() &&
-         (srtp_filter_.IsActive() || !ShouldSetupDtlsSrtp_n());
+         was_ever_writable() && (secure() || !ShouldSetupDtlsSrtp_n());
 }
 
 bool BaseChannel::SendPacket(rtc::CopyOnWriteBuffer* packet,
                              const rtc::PacketOptions& options) {
   return SendPacket(false, packet, options);
 }
 
 bool BaseChannel::SendRtcp(rtc::CopyOnWriteBuffer* packet,
                            const rtc::PacketOptions& options) {
   return SendPacket(true, packet, options);
@@ skipping 37 matching lines @@
   if (!ShouldSetupDtlsSrtp_n()) {
     return;
   }
 
   // Reset the srtp filter if it's not the CONNECTED state. For the CONNECTED
   // state, setting up DTLS-SRTP context is deferred to ChannelWritable_w to
   // cover other scenarios like the whole transport is writable (not just this
   // TransportChannel) or when TransportChannel is attached after DTLS is
   // negotiated.
   if (state != DTLS_TRANSPORT_CONNECTED) {
-    srtp_filter_.ResetParams();
+    dtls_active_ = false;
+    if (srtp_transport_) {
+      srtp_transport_->ResetParams();
+    }
   }
 }
 
 void BaseChannel::OnSelectedCandidatePairChanged(
     IceTransportInternal* ice_transport,
     CandidatePairInterface* selected_candidate_pair,
     int last_sent_packet_id,
     bool ready_to_send) {
   RTC_DCHECK((rtp_dtls_transport_ &&
               ice_transport == rtp_dtls_transport_->ice_transport()) ||
@@ skipping 53 matching lines @@
   }
 
   // Protect ourselves against crazy data.
   if (!ValidPacket(rtcp, packet)) {
     LOG(LS_ERROR) << "Dropping outgoing " << content_name_ << " "
                   << RtpRtcpStringLiteral(rtcp)
                   << " packet: wrong size=" << packet->size();
     return false;
   }
 
-  rtc::PacketOptions updated_options;
-  updated_options = options;
-  // Protect if needed.
-  if (srtp_filter_.IsActive()) {
-    TRACE_EVENT0("webrtc", "SRTP Encode");
-    bool res;
-    uint8_t* data = packet->data();
-    int len = static_cast<int>(packet->size());
-    if (!rtcp) {
-    // If ENABLE_EXTERNAL_AUTH flag is on then packet authentication is not done
-    // inside libsrtp for a RTP packet. A external HMAC module will be writing
-    // a fake HMAC value. This is ONLY done for a RTP packet.
-    // 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(
-          data, len, static_cast<int>(packet->capacity()), &len);
-#else
-      if (!srtp_filter_.IsExternalAuthActive()) {
-        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(
-            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(
-              &auth_key, &key_len,
-              &updated_options.packet_time_params.srtp_auth_tag_len);
-          if (res) {
-            updated_options.packet_time_params.srtp_auth_key.resize(key_len);
-            updated_options.packet_time_params.srtp_auth_key.assign(
-                auth_key, auth_key + key_len);
-          }
-        }
-      }
-#endif
-      if (!res) {
-        int seq_num = -1;
-        uint32_t ssrc = 0;
-        GetRtpSeqNum(data, len, &seq_num);
-        GetRtpSsrc(data, len, &ssrc);
-        LOG(LS_ERROR) << "Failed to protect " << content_name_
-                      << " RTP packet: size=" << len
-                      << ", seqnum=" << seq_num << ", SSRC=" << ssrc;
-        return false;
-      }
-    } else {
-      res = srtp_filter_.ProtectRtcp(data, len,
-                                     static_cast<int>(packet->capacity()),
-                                     &len);
-      if (!res) {
-        int type = -1;
-        GetRtcpType(data, len, &type);
-        LOG(LS_ERROR) << "Failed to protect " << content_name_
-                      << " RTCP packet: size=" << len << ", type=" << type;
-        return false;
-      }
-    }
-
-    // Update the length of the packet now that we've added the auth tag.
-    packet->SetSize(len);
-  } else if (srtp_required_) {
+  if (!secure() && srtp_required_) {
     // The audio/video engines may attempt to send RTCP packets as soon as the
     // streams are created, so don't treat this as an error for RTCP.
     // See: https://bugs.chromium.org/p/webrtc/issues/detail?id=6809
     if (rtcp) {
       return false;
     }
     // However, there shouldn't be any RTP packets sent before SRTP is set up
     // (and SetSend(true) is called).
     LOG(LS_ERROR) << "Can't send outgoing RTP packet when SRTP is inactive"
                   << " and crypto is required";
     RTC_NOTREACHED();
     return false;
   }
 
+  if (secure()) {

[pthatcher, 2017/08/28 21:42:56]

I think making the shorter path the early return would be nicer:

if (!srtp_active()) {
  return rtp_transport_->...;
}

...
return srtp_transport->...;

[Zhi Huang, 2017/08/29 18:40:34]

Done.

+    RTC_DCHECK(srtp_transport_);
+    RTC_DCHECK(srtp_transport_->IsActive());
+    // Bon voyage.
+    int flags = secure_dtls() ? PF_SRTP_BYPASS : PF_NORMAL;
+    return srtp_transport_->SendPacket(rtcp, packet, options, flags);

[pthatcher, 2017/08/28 21:42:56]

I don't think it should be the responsibility of the BaseChannel to set the "I'm
really SRTP" flag.  That should be the job of the SrtpTransport.  I think
SrtpTransport should have a SendPacket which doesn't take the flags and then it
calls DtlsTransport with the flags.

In fact, it would be even better to have separate SendRtpPacket and
SendRtpPacket so that you wouldn't need to rtcp bool either.  

[Zhi Huang, 2017/08/29 18:40:34]

SrtpTransport doesn't interact with DtlsTransport directly and it relies on the
wrapped RtpTransport to manage the states (ready_to_send_ etc.) The RtpTransport
needs to have the capability to set the flags explicitly so we still need 
    SendPacket(bool rtcp,
               rtc::CopyOnWriteBuffer* packet,
               const rtc::PacketOptions& options,
               int flag);
in RtpTransportInternal and the SrtpTransport still needs to override this
method.

+  }
+
   // Bon voyage.
-  int flags = (secure() && secure_dtls()) ? PF_SRTP_BYPASS : PF_NORMAL;
-  return rtp_transport_->SendPacket(rtcp, packet, updated_options, flags);
+  return rtp_transport_->SendPacket(rtcp, packet, options, PF_NORMAL);

[pthatcher, 2017/08/28 21:42:56]

And similarly here, it would make sense to call SendRtpPacket or SendRtcpPacket.
 And not pass in the flag, because it's not doing anything anyway.

[Zhi Huang, 2017/08/29 18:40:34]

Done.

 }
 
 bool BaseChannel::HandlesPayloadType(int packet_type) const {
   return rtp_transport_->HandlesPayloadType(packet_type);
 }
 
 void BaseChannel::OnPacketReceived(bool rtcp,
                                    rtc::CopyOnWriteBuffer* packet,
                                    const rtc::PacketTime& packet_time) {
   if (!has_received_packet_ && !rtcp) {
     has_received_packet_ = true;
     signaling_thread()->Post(RTC_FROM_HERE, this, MSG_FIRSTPACKETRECEIVED);
   }
 
-  // Unprotect the packet, if needed.
-  if (srtp_filter_.IsActive()) {
-    TRACE_EVENT0("webrtc", "SRTP Decode");
-    char* data = packet->data<char>();
-    int len = static_cast<int>(packet->size());
-    bool res;
-    if (!rtcp) {
-      res = srtp_filter_.UnprotectRtp(data, len, &len);
-      if (!res) {
-        int seq_num = -1;
-        uint32_t ssrc = 0;
-        GetRtpSeqNum(data, len, &seq_num);
-        GetRtpSsrc(data, len, &ssrc);
-        LOG(LS_ERROR) << "Failed to unprotect " << content_name_
-                      << " RTP packet: size=" << len << ", seqnum=" << seq_num
-                      << ", SSRC=" << ssrc;
-        return;
-      }
-    } else {
-      res = srtp_filter_.UnprotectRtcp(data, len, &len);
-      if (!res) {
-        int type = -1;
-        GetRtcpType(data, len, &type);
-        LOG(LS_ERROR) << "Failed to unprotect " << content_name_
-                      << " RTCP packet: size=" << len << ", type=" << type;
-        return;
-      }
-    }
-
-    packet->SetSize(len);
-  } else if (srtp_required_) {
+  if (!secure() && srtp_required_) {
     // Our session description indicates that SRTP is required, but we got a
     // packet before our SRTP filter is active. This means either that
     // a) we got SRTP packets before we received the SDES keys, in which case
     //    we can't decrypt it anyway, or
     // b) we got SRTP packets before DTLS completed on both the RTP and RTCP
     //    transports, so we haven't yet extracted keys, even if DTLS did
     //    complete on the transport that the packets are being sent on. It's
     //    really good practice to wait for both RTP and RTCP to be good to go
     //    before sending  media, to prevent weird failure modes, so it's fine
     //    for us to just eat packets here. This is all sidestepped if RTCP mux
@@ skipping 22 matching lines @@
     media_channel_->OnPacketReceived(&data, packet_time);
   }
 }
 
 bool BaseChannel::PushdownLocalDescription(
     const SessionDescription* local_desc, ContentAction action,
     std::string* error_desc) {
   const ContentInfo* content_info = GetFirstContent(local_desc);
   const MediaContentDescription* content_desc =
       GetContentDescription(content_info);
+  if (IsInsecureRtp(content_desc->protocol())) {
+    RTC_DCHECK(!srtp_required_);

[Taylor Brandstetter, 2017/08/26 02:40:38]

This should be an error rather than a DCHECK since it's possible to hit, right?

[Zhi Huang, 2017/08/29 18:40:34]

I'll remove these for now.

+    srtp_transport_ = nullptr;

[Taylor Brandstetter, 2017/08/26 02:40:39]

This unsets the srtp_transport_ pointer, but rtp_transport_ is still really an
SrtpTransport, correct? So how does this work? Are there any tests that exercise
using plain RTP? I guess there are the ORTC tests, so I wonder if they pass (and
how).

EDIT: Nevermind, I see now. See later comment.

[Zhi Huang, 2017/08/29 18:40:33]

Done.

+  }
   if (content_desc && content_info && !content_info->rejected &&
       !SetLocalContent(content_desc, action, error_desc)) {
     LOG(LS_ERROR) << "Failure in SetLocalContent with action " << action;
     return false;
   }
   return true;
 }
 
 bool BaseChannel::PushdownRemoteDescription(
     const SessionDescription* remote_desc, ContentAction action,
     std::string* error_desc) {
   const ContentInfo* content_info = GetFirstContent(remote_desc);
   const MediaContentDescription* content_desc =
       GetContentDescription(content_info);
+  if (IsInsecureRtp(content_desc->protocol())) {
+    RTC_DCHECK(!srtp_required_);
+    srtp_transport_ = nullptr;
+  }
   if (content_desc && content_info && !content_info->rejected &&
       !SetRemoteContent(content_desc, action, error_desc)) {
     LOG(LS_ERROR) << "Failure in SetRemoteContent with action " << action;
     return false;
   }
   return true;
 }
 
 void BaseChannel::EnableMedia_w() {
   RTC_DCHECK(worker_thread_ == rtc::Thread::Current());
@@ skipping 127 matching lines @@
   }
 
   if (role == rtc::SSL_SERVER) {
     send_key = &server_write_key;
     recv_key = &client_write_key;
   } else {
     send_key = &client_write_key;
     recv_key = &server_write_key;
   }
 
-  if (!srtp_filter_.IsActive()) {
+  if (!secure_dtls()) {
     if (rtcp) {
-      ret = srtp_filter_.SetRtcpParams(selected_crypto_suite, &(*send_key)[0],
-                                       static_cast<int>(send_key->size()),
-                                       selected_crypto_suite, &(*recv_key)[0],
-                                       static_cast<int>(recv_key->size()));
+      RTC_DCHECK(srtp_transport_);
+      ret = srtp_transport_->SetRtcpParams(
+          selected_crypto_suite, &(*send_key)[0],
+          static_cast<int>(send_key->size()), selected_crypto_suite,
+          &(*recv_key)[0], static_cast<int>(recv_key->size()));
     } else {
-      ret = srtp_filter_.SetRtpParams(selected_crypto_suite, &(*send_key)[0],
-                                      static_cast<int>(send_key->size()),
-                                      selected_crypto_suite, &(*recv_key)[0],
-                                      static_cast<int>(recv_key->size()));
+      RTC_DCHECK(srtp_transport_);
+      ret = srtp_transport_->SetRtpParams(
+          selected_crypto_suite, &(*send_key)[0],
+          static_cast<int>(send_key->size()), selected_crypto_suite,
+          &(*recv_key)[0], static_cast<int>(recv_key->size()));
+      dtls_active_ = ret;
     }
   } else {
     if (rtcp) {
       // RTCP doesn't need to be updated because UpdateRtpParams is only used
       // to update the set of encrypted RTP header extension IDs.
       ret = true;
     } else {
-      ret = srtp_filter_.UpdateRtpParams(
-          selected_crypto_suite,
-          &(*send_key)[0], static_cast<int>(send_key->size()),
-          selected_crypto_suite,
+      RTC_DCHECK(srtp_transport_);
+      ret = srtp_transport_->UpdateRtpParams(
+          selected_crypto_suite, &(*send_key)[0],
+          static_cast<int>(send_key->size()), selected_crypto_suite,
           &(*recv_key)[0], static_cast<int>(recv_key->size()));
     }
   }

[pthatcher, 2017/08/28 21:42:56]

If this is only used for changing the header extensions IDs, why not have
SetEncryptedHeaderExtensionIds do the update?

[Zhi Huang, 2017/08/29 18:40:34]

SetEncryptedHeaderExtensionIds just stores the updated ids and we need to call
UpdateRtpParams for new ids to take effect.

 
   if (!ret) {
     LOG(LS_WARNING) << "DTLS-SRTP key installation failed";
   } else {
-    dtls_keyed_ = true;
     UpdateTransportOverhead();
   }
   return ret;
 }
 
 void BaseChannel::MaybeSetupDtlsSrtp_n() {
-  if (srtp_filter_.IsActive()) {
+  if (secure_dtls()) {
     return;
   }
 
   if (!ShouldSetupDtlsSrtp_n()) {
     return;
   }
 
   if (!SetupDtlsSrtp_n(false)) {
     SignalDtlsSrtpSetupFailure_n(false);
     return;
@@ skipping 86 matching lines @@
   if (action == CA_UPDATE) {
     // no crypto params.
     return true;
   }
   bool ret = false;
   bool dtls = false;
   ret = CheckSrtpConfig_n(cryptos, &dtls, error_desc);
   if (!ret) {
     return false;
   }
-  srtp_filter_.SetEncryptedHeaderExtensionIds(src, encrypted_extension_ids);
+
+  if (!srtp_transport_) {
+    LOG(LS_WARNING) << "Setting SRTP without an SrtpTransport.";

[Taylor Brandstetter, 2017/08/26 02:40:38]

It looks like this warning message would be logged for every offer/answer if not
using SRTP. Maybe it should only be logged if "cryptos" is nonempty.

[Zhi Huang, 2017/08/29 18:40:34]

Acknowledged.

+    return true;
+  }
+
+  srtp_transport_->SetEncryptedHeaderExtensionIds(src, encrypted_extension_ids);
   switch (action) {
     case CA_OFFER:
       // 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);
       }
       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);
       }
       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);
       }
       break;
     default:
       break;
   }
+
+  // If setting an SDES answer succeeded, apply the negotiated parameters
+  // to the SRTP transport.
+  if ((action == CA_PRANSWER || action == CA_ANSWER) && !dtls && ret) {
+    if (srtp_filter_.send_cipher_suite() && srtp_filter_.recv_cipher_suite()) {
+      auto send_key = srtp_filter_.send_key();
+      auto recv_key = srtp_filter_.recv_key();
+      ret = srtp_transport_->SetRtpParams(
+          *(srtp_filter_.send_cipher_suite()), &(*send_key)[0],
+          static_cast<int>(send_key->size()),
+          *(srtp_filter_.recv_cipher_suite()), &(*recv_key)[0],
+          static_cast<int>(recv_key->size()));
+    } else {
+      LOG(LS_INFO) << "No crypto keys are provided for SDES.";

[Taylor Brandstetter, 2017/08/26 02:40:38]

Is it even possible to hit this point? Won't "srtp_transport_" already be null,
from PushdownRemoteDescription?

[Zhi Huang, 2017/08/29 18:40:34]

If we still support upgrading rtp_transport to srtp_transport for now, then I
think this could be hit if the crypto is empty in answer.

+      if (action == CA_ANSWER) {
+        // Explicitly reset the |srtp_transport_| if no crypto param is
+        // provided in the answer. No need to call |ResetParams()| for
+        // |srtp_filter_| because it resets the params inside |SetAnswer|.
+        srtp_transport_->ResetParams();
+      }
+    }
+  }
+
   // Only update SRTP filter if using DTLS. SDES is handled internally
   // by the SRTP filter.
   // TODO(jbauch): Only update if encrypted extension ids have changed.
-  if (ret && dtls_keyed_ && rtp_dtls_transport_ &&
+  if (ret && secure_dtls() && rtp_dtls_transport_ &&
       rtp_dtls_transport_->dtls_state() == DTLS_TRANSPORT_CONNECTED) {
     bool rtcp = false;
     ret = SetupDtlsSrtp_n(rtcp);
   }
   if (!ret) {
     SafeSetError("Failed to setup SRTP filter.", error_desc);
     return false;
   }
   return true;
 }
@@ skipping 23 matching lines @@
       break;
     case CA_ANSWER:
       ret = rtcp_mux_filter_.SetAnswer(enable, src);
       if (ret && rtcp_mux_filter_.IsActive()) {
         // We permanently activated RTCP muxing; signal that we no longer need
         // the RTCP transport.
         std::string debug_name =
             transport_name_.empty()
                 ? rtp_transport_->rtp_packet_transport()->debug_name()
                 : transport_name_;
-        ;
         LOG(LS_INFO) << "Enabling rtcp-mux for " << content_name()
                      << "; no longer need RTCP transport for " << debug_name;
         if (rtp_transport_->rtcp_packet_transport()) {
           SetTransport_n(true, nullptr, nullptr);
           SignalRtcpMuxFullyActive(transport_name_);
         }
         UpdateWritableState_n();
       }
       break;
     case CA_UPDATE:
@@ skipping 208 matching lines @@
       send_time_extension ? send_time_extension->id : -1;
   invoker_.AsyncInvoke<void>(
       RTC_FROM_HERE, network_thread_,
       Bind(&BaseChannel::CacheRtpAbsSendTimeHeaderExtension_n, this,
            rtp_abs_sendtime_extn_id));
 #endif
 }
 
 void BaseChannel::CacheRtpAbsSendTimeHeaderExtension_n(
     int rtp_abs_sendtime_extn_id) {
-  rtp_abs_sendtime_extn_id_ = rtp_abs_sendtime_extn_id;
+  RTC_DCHECK(srtp_transport_);
+  srtp_transport_->CacheRtpAbsSendTimeHeaderExtension(rtp_abs_sendtime_extn_id);
 }
 
 void BaseChannel::OnMessage(rtc::Message *pmsg) {
   TRACE_EVENT0("webrtc", "BaseChannel::OnMessage");
   switch (pmsg->message_id) {
     case MSG_SEND_RTP_PACKET:
     case MSG_SEND_RTCP_PACKET: {
       RTC_DCHECK(network_thread_->IsCurrent());
       SendPacketMessageData* data =
           static_cast<SendPacketMessageData*>(pmsg->pdata);
@@ skipping 263 matching lines @@
           : kIpv6Overhaed;
 
   constexpr int kUdpOverhaed = 8;
   constexpr int kTcpOverhaed = 20;
   transport_overhead_per_packet +=
       selected_candidate_pair_->local_candidate().protocol() ==
               TCP_PROTOCOL_NAME
           ? kTcpOverhaed
           : kUdpOverhaed;
 
-  if (secure()) {
+  if (secure_sdes()) {
     int srtp_overhead = 0;
-    if (srtp_filter_.GetSrtpOverhead(&srtp_overhead))
+    if (srtp_transport_->GetSrtpOverhead(&srtp_overhead))
       transport_overhead_per_packet += srtp_overhead;
   }
 
   return transport_overhead_per_packet;
 }
 
 void BaseChannel::UpdateTransportOverhead() {
   int transport_overhead_per_packet = GetTransportOverheadPerPacket();
   if (transport_overhead_per_packet)
     invoker_.AsyncInvoke<void>(
@@ skipping 714 matching lines @@
 
 void RtpDataChannel::OnDataChannelReadyToSend(bool writable) {
   // This is usded for congestion control to indicate that the stream is ready
   // to send by the MediaChannel, as opposed to OnReadyToSend, which indicates
   // that the transport channel is ready.
   signaling_thread()->Post(RTC_FROM_HERE, this, MSG_READYTOSENDDATA,
                            new DataChannelReadyToSendMessageData(writable));
 }
 
 }  // namespace cricket