Support encrypted RTP extensions (RFC 6904)

webrtc/pc/channel.cc

Index: webrtc/pc/channel.cc
diff --git a/webrtc/pc/channel.cc b/webrtc/pc/channel.cc
index ecc1c9a633c6d4a935a146f048483256ccdc5d49..688bfadab5aa02d1cbe5660fcf976511c2defdb6 100644
--- a/webrtc/pc/channel.cc
+++ b/webrtc/pc/channel.cc
@@ -43,20 +43,6 @@ struct SendPacketMessageData : public rtc::MessageData {
   rtc::PacketOptions options;
 };
 
-#if defined(ENABLE_EXTERNAL_AUTH)
-// Returns the named header extension if found among all extensions,
-// nullptr otherwise.
-const webrtc::RtpExtension* FindHeaderExtension(
-    const std::vector<webrtc::RtpExtension>& extensions,
-    const std::string& uri) {
-  for (const auto& extension : extensions) {
-    if (extension.uri == uri)
-      return &extension;
-  }
-  return nullptr;
-}
-#endif
-
 }  // namespace
 
 enum {
@@ -133,6 +119,7 @@ static const MediaContentDescription* GetContentDescription(
 template <class Codec>
 void RtpParametersFromMediaDescription(
     const MediaContentDescriptionImpl<Codec>* desc,
+    const RtpHeaderExtensions& extensions,

[Taylor Brandstetter, 2017/04/01 00:28:59]

I'd just set params->extensions outside of this method, then there'd be no
reason to pass the extensions into it.

[joachim, 2017/04/17 10:46:09]

The method is called from different locations, so params->extensions would have
to be set in all of them including the "if ..." condition - I wanted to avoid
this code duplication.

[Taylor Brandstetter, 2017/04/19 06:48:39]

Acknowledged.

     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
@@ -143,7 +130,7 @@ void RtpParametersFromMediaDescription(
   // TODO(pthatcher): See if we really need
   // rtp_header_extensions_set() and remove it if we don't.
   if (desc->rtp_header_extensions_set()) {
-    params->extensions = desc->rtp_header_extensions();
+    params->extensions = extensions;
   }
   params->rtcp.reduced_size = desc->rtcp_reduced_size();
 }
@@ -151,8 +138,9 @@ void RtpParametersFromMediaDescription(
 template <class Codec>
 void RtpSendParametersFromMediaDescription(
     const MediaContentDescriptionImpl<Codec>* desc,
+    const RtpHeaderExtensions& extensions,

[Taylor Brandstetter, 2017/04/01 00:28:59]

Same here.

[joachim, 2017/04/17 10:46:09]

See comment above.

     RtpSendParameters<Codec>* send_params) {
-  RtpParametersFromMediaDescription(desc, send_params);
+  RtpParametersFromMediaDescription(desc, extensions, send_params);
   send_params->max_bandwidth_bps = desc->bandwidth();
 }
 
@@ -1093,16 +1081,29 @@ bool BaseChannel::SetupDtlsSrtp_n(bool rtcp) {
     recv_key = &server_write_key;
   }
 
-  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()));
+  if (!dtls_keyed_) {

[Taylor Brandstetter, 2017/04/01 00:28:59]

I don't think this works in all cases... If the transport changes, the
SrtpFilter will be reset, but dtls_keyed_ won't be. So, should just check
"srtp_filter_.IsActive()" here.

[joachim, 2017/04/17 10:46:09]

Done.

+    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()));
+    } 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()));
+    }
   } 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()));
+    if (rtcp) {
+      // RTCP doesn't need to be updated.
+      ret = true;
+    } else {
+      ret = srtp_filter_.UpdateRtpParams(

[Taylor Brandstetter, 2017/04/01 00:28:59]

If UpdateRtpParams is *always* called with the same keying information (right
now), I'd suggest just removing the arguments for simplicity.

In fact: is there a reason why this couldn't just happen automatically inside
"SetEncryptedHeaderExtensions"?

[joachim, 2017/04/17 10:46:09]

"srtp_update" in the srtp filter requires the complete keying information,
that's why they are passed from here. Another option would be to save them in
the srtp filter which I wanted to avoid.

[Taylor Brandstetter, 2017/04/19 06:48:39]

Oh, I assumed they already were saved in the SRTP filter. If they're not, this
is fine.

+          selected_crypto_suite,
+          &(*send_key)[0], static_cast<int>(send_key->size()),
+          selected_crypto_suite,
+          &(*recv_key)[0], static_cast<int>(recv_key->size()));
+    }
   }
 
   if (!ret) {
@@ -1150,26 +1151,39 @@ bool BaseChannel::SetRtpTransportParameters(
     const MediaContentDescription* content,
     ContentAction action,
     ContentSource src,
+    const RtpHeaderExtensions& extensions,
     std::string* error_desc) {
   if (action == CA_UPDATE) {
     // These parameters never get changed by a CA_UDPATE.
     return true;
   }
 
+  RtpHeaderExtensions encrypted_extensions;
+  for (const webrtc::RtpExtension& extension : extensions) {
+    if (extension.encrypt) {
+      LOG(LS_INFO) << "Using " << (src == CS_LOCAL ? "local" : "remote")
+          << " encrypted extension: " << extension.ToString();
+      encrypted_extensions.push_back(extension);
+    }
+  }
+
   // Cache srtp_required_ for belt and suspenders check on SendPacket
   return network_thread_->Invoke<bool>(
       RTC_FROM_HERE, Bind(&BaseChannel::SetRtpTransportParameters_n, this,
-                          content, action, src, error_desc));
+                          content, action, src, encrypted_extensions,
+                          error_desc));
 }
 
 bool BaseChannel::SetRtpTransportParameters_n(
     const MediaContentDescription* content,
     ContentAction action,
     ContentSource src,
+    const RtpHeaderExtensions& encrypted_extensions,
     std::string* error_desc) {
   RTC_DCHECK(network_thread_->IsCurrent());
 
-  if (!SetSrtp_n(content->cryptos(), action, src, error_desc)) {
+  if (!SetSrtp_n(content->cryptos(), action, src, encrypted_extensions,
+      error_desc)) {
     return false;
   }
 
@@ -1196,6 +1210,7 @@ bool BaseChannel::CheckSrtpConfig_n(const std::vector<CryptoParams>& cryptos,
 bool BaseChannel::SetSrtp_n(const std::vector<CryptoParams>& cryptos,
                             ContentAction action,
                             ContentSource src,
+                            const RtpHeaderExtensions& encrypted_extensions,
                             std::string* error_desc) {
   TRACE_EVENT0("webrtc", "BaseChannel::SetSrtp_w");
   if (action == CA_UPDATE) {
@@ -1208,6 +1223,7 @@ bool BaseChannel::SetSrtp_n(const std::vector<CryptoParams>& cryptos,
   if (!ret) {
     return false;
   }
+  srtp_filter_.SetEncryptedHeaderExtensions(src, encrypted_extensions);
   switch (action) {
     case CA_OFFER:
       // If DTLS is already active on the channel, we could be renegotiating
@@ -1215,6 +1231,9 @@ bool BaseChannel::SetSrtp_n(const std::vector<CryptoParams>& cryptos,
       if (!dtls) {
         ret = srtp_filter_.SetOffer(cryptos, src);
       }
+      // The encrypted header extensions might have changed, update SRTP
+      // filter with new settings when the answer gets processed.
+      srtp_update_on_answer_ = true;

[Taylor Brandstetter, 2017/04/01 00:28:59]

I'm not sure this variable buys anything; an answer will always be set after an
offer, so won't it always be true when it's checked below?

[joachim, 2017/04/17 10:46:09]

You're right - removed it.

       break;
     case CA_PRANSWER:
       // If we're doing DTLS-SRTP, we don't want to update the filter
@@ -1229,6 +1248,15 @@ bool BaseChannel::SetSrtp_n(const std::vector<CryptoParams>& cryptos,
       if (!dtls) {
         ret = srtp_filter_.SetAnswer(cryptos, src);
       }
+      if (srtp_update_on_answer_) {
+        srtp_update_on_answer_ = false;
+        // Only update SRTP filter if using DTLS. SDES is handled internally
+        // by the SRTP filter.
+        if (dtls_keyed_) {
+          bool rtcp = false;
+          ret = SetupDtlsSrtp_n(rtcp);
+        }
+      }
       break;
     default:
       break;
@@ -1463,6 +1491,15 @@ bool BaseChannel::UpdateRemoteStreams_w(
   return ret;
 }
 
+RtpHeaderExtensions BaseChannel::GetFilteredRtpHeaderExtensions(
+    const RtpHeaderExtensions& extensions) {
+  if (!crypto_options_.enable_encrypted_rtp_header_extensions) {
+    return extensions;
+  }
+
+  return webrtc::RtpExtension::FilterDuplicateNonEncrypted(extensions);
+}
+
 void BaseChannel::MaybeCacheRtpAbsSendTimeHeaderExtension_w(
     const std::vector<webrtc::RtpExtension>& extensions) {
 // Absolute Send Time extension id is used only with external auth,
@@ -1470,7 +1507,8 @@ void BaseChannel::MaybeCacheRtpAbsSendTimeHeaderExtension_w(
 // something that is not used.
 #if defined(ENABLE_EXTERNAL_AUTH)
   const webrtc::RtpExtension* send_time_extension =
-      FindHeaderExtension(extensions, webrtc::RtpExtension::kAbsSendTimeUri);
+      webrtc::RtpExtension::FindHeaderExtensionByUri(
+          extensions, webrtc::RtpExtension::kAbsSendTimeUri);
   int rtp_abs_sendtime_extn_id =
       send_time_extension ? send_time_extension->id : -1;
   invoker_.AsyncInvoke<void>(
@@ -1804,12 +1842,16 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content,
     return false;
   }
 
-  if (!SetRtpTransportParameters(content, action, CS_LOCAL, error_desc)) {
+  RtpHeaderExtensions rtp_header_extensions =
+      GetFilteredRtpHeaderExtensions(audio->rtp_header_extensions());
+
+  if (!SetRtpTransportParameters(content, action, CS_LOCAL,
+      rtp_header_extensions, error_desc)) {
     return false;
   }
 
   AudioRecvParameters recv_params = last_recv_params_;
-  RtpParametersFromMediaDescription(audio, &recv_params);
+  RtpParametersFromMediaDescription(audio, rtp_header_extensions, &recv_params);
   if (!media_channel()->SetRecvParameters(recv_params)) {
     SafeSetError("Failed to set local audio description recv parameters.",
                  error_desc);
@@ -1849,12 +1891,17 @@ bool VoiceChannel::SetRemoteContent_w(const MediaContentDescription* content,
     return false;
   }
 
-  if (!SetRtpTransportParameters(content, action, CS_REMOTE, error_desc)) {
+  RtpHeaderExtensions rtp_header_extensions =
+      GetFilteredRtpHeaderExtensions(audio->rtp_header_extensions());
+
+  if (!SetRtpTransportParameters(content, action, CS_REMOTE,
+      rtp_header_extensions, error_desc)) {
     return false;
   }
 
   AudioSendParameters send_params = last_send_params_;
-  RtpSendParametersFromMediaDescription(audio, &send_params);
+  RtpSendParametersFromMediaDescription(audio, rtp_header_extensions,
+      &send_params);
   if (audio->agc_minus_10db()) {
     send_params.options.adjust_agc_delta = rtc::Optional<int>(kAgcMinus10db);
   }
@@ -1877,7 +1924,7 @@ bool VoiceChannel::SetRemoteContent_w(const MediaContentDescription* content,
   }
 
   if (audio->rtp_header_extensions_set()) {
-    MaybeCacheRtpAbsSendTimeHeaderExtension_w(audio->rtp_header_extensions());
+    MaybeCacheRtpAbsSendTimeHeaderExtension_w(rtp_header_extensions);
   }
 
   set_remote_content_direction(content->direction());
@@ -2082,12 +2129,16 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content,
     return false;
   }
 
-  if (!SetRtpTransportParameters(content, action, CS_LOCAL, error_desc)) {
+  RtpHeaderExtensions rtp_header_extensions =
+      GetFilteredRtpHeaderExtensions(video->rtp_header_extensions());
+
+  if (!SetRtpTransportParameters(content, action, CS_LOCAL,
+      rtp_header_extensions, error_desc)) {
     return false;
   }
 
   VideoRecvParameters recv_params = last_recv_params_;
-  RtpParametersFromMediaDescription(video, &recv_params);
+  RtpParametersFromMediaDescription(video, rtp_header_extensions, &recv_params);
   if (!media_channel()->SetRecvParameters(recv_params)) {
     SafeSetError("Failed to set local video description recv parameters.",
                  error_desc);
@@ -2127,12 +2178,17 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content,
     return false;
   }
 
-  if (!SetRtpTransportParameters(content, action, CS_REMOTE, error_desc)) {
+  RtpHeaderExtensions rtp_header_extensions =
+      GetFilteredRtpHeaderExtensions(video->rtp_header_extensions());
+
+  if (!SetRtpTransportParameters(content, action, CS_REMOTE,
+      rtp_header_extensions, error_desc)) {
     return false;
   }
 
   VideoSendParameters send_params = last_send_params_;
-  RtpSendParametersFromMediaDescription(video, &send_params);
+  RtpSendParametersFromMediaDescription(video, rtp_header_extensions,
+      &send_params);
   if (video->conference_mode()) {
     send_params.conference_mode = true;
   }
@@ -2156,7 +2212,7 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content,
   }
 
   if (video->rtp_header_extensions_set()) {
-    MaybeCacheRtpAbsSendTimeHeaderExtension_w(video->rtp_header_extensions());
+    MaybeCacheRtpAbsSendTimeHeaderExtension_w(rtp_header_extensions);
   }
 
   set_remote_content_direction(content->direction());
@@ -2282,12 +2338,16 @@ bool RtpDataChannel::SetLocalContent_w(const MediaContentDescription* content,
     return false;
   }
 
-  if (!SetRtpTransportParameters(content, action, CS_LOCAL, error_desc)) {
+  RtpHeaderExtensions rtp_header_extensions =
+      GetFilteredRtpHeaderExtensions(data->rtp_header_extensions());
+
+  if (!SetRtpTransportParameters(content, action, CS_LOCAL,
+      rtp_header_extensions, error_desc)) {
     return false;
   }
 
   DataRecvParameters recv_params = last_recv_params_;
-  RtpParametersFromMediaDescription(data, &recv_params);
+  RtpParametersFromMediaDescription(data, rtp_header_extensions, &recv_params);
   if (!media_channel()->SetRecvParameters(recv_params)) {
     SafeSetError("Failed to set remote data description recv parameters.",
                  error_desc);
@@ -2336,13 +2396,18 @@ bool RtpDataChannel::SetRemoteContent_w(const MediaContentDescription* content,
     return false;
   }
 
+  RtpHeaderExtensions rtp_header_extensions =
+      GetFilteredRtpHeaderExtensions(data->rtp_header_extensions());
+
   LOG(LS_INFO) << "Setting remote data description";
-  if (!SetRtpTransportParameters(content, action, CS_REMOTE, error_desc)) {
+  if (!SetRtpTransportParameters(content, action, CS_REMOTE,
+      rtp_header_extensions, error_desc)) {
     return false;
   }
 
   DataSendParameters send_params = last_send_params_;
-  RtpSendParametersFromMediaDescription<DataCodec>(data, &send_params);
+  RtpSendParametersFromMediaDescription<DataCodec>(data, rtp_header_extensions,
+      &send_params);
   if (!media_channel()->SetSendParameters(send_params)) {
     SafeSetError("Failed to set remote data description send parameters.",
                  error_desc);