Support encrypted RTP extensions (RFC 6904)

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 25 matching lines @@
                        std::unique_ptr<webrtc::AudioSinkInterface>* sink) {
   channel->SetRawAudioSink(ssrc, std::move(*sink));
   return true;
 }
 
 struct SendPacketMessageData : public rtc::MessageData {
   rtc::CopyOnWriteBuffer packet;
   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 {
   MSG_EARLYMEDIATIMEOUT = 1,
   MSG_SEND_RTP_PACKET,
   MSG_SEND_RTCP_PACKET,
   MSG_CHANNEL_ERROR,
   MSG_READYTOSENDDATA,
   MSG_DATARECEIVED,
@@ skipping 248 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;
+    if (srtp_filter_.IsActive()) {
+      // Only clear header information if transport changes (i.e. SRTP has been
+      // activated before), not on initial transport (which might happen during
+      // offer/answer processing).
+      rtp_encrypted_headers_ready_ = 0;

[Taylor Brandstetter, 2017/03/22 18:00:11]

This would mean that a new offer/answer is needed if the transport changes after
SRTP is already set up? May be a corner case, but that seems incorrect.

[joachim, 2017/03/23 00:04:33]

Right, I missed this. No longer applies with the change to use
"SetRtpTransportParameters" for setting up the encrypted headers.

+    }

[pthatcher1, 2017/03/21 07:07:06]

It shouldn't become writable again until we get the local and remote contents,
so I don't understand why we need to carry around this extra state.

[joachim, 2017/03/23 00:04:33]

The extra state has now been removed.

     srtp_filter_.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 647 matching lines @@
                << (was_ever_writable_ ? "" : " for the first time");
 
   if (selected_candidate_pair_)
     LOG(LS_INFO)
         << "Using "
         << selected_candidate_pair_->local_candidate().ToSensitiveString()
         << "->"
         << selected_candidate_pair_->remote_candidate().ToSensitiveString();
 
   was_ever_writable_ = true;
+  writable_ = true;
   MaybeSetupDtlsSrtp_n();

[pthatcher1, 2017/03/21 07:07:06]

Why did this have to change?  

[Taylor Brandstetter, 2017/03/22 18:00:11]

Because ShouldSetupDtlsSrtp_n now checks it. May want to mention in a comment,
if it doesn't end up changing.

[joachim, 2017/03/23 00:04:32]

Right, added a comment for now.

-  writable_ = true;
   UpdateMediaSendRecvState();
 }
 
 void BaseChannel::SignalDtlsSrtpSetupFailure_n(bool rtcp) {
   RTC_DCHECK(network_thread_->IsCurrent());
   invoker_.AsyncInvoke<void>(
       RTC_FROM_HERE, signaling_thread(),
       Bind(&BaseChannel::SignalDtlsSrtpSetupFailure_s, this, rtcp));
 }
 
@@ skipping 86 matching lines @@
     send_key = &client_write_key;
     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()));
   } else {
+    std::vector<int> send_encrypted_headers;
+    std::vector<int> recv_encrypted_headers;
+
+    // Send local extension header encrypted if the remote also uses it
+    // encrypted and vice versa.
+    for (const webrtc::RtpExtension& extension : rtp_local_encrypted_headers_) {
+      if (FindHeaderExtension(rtp_remote_encrypted_headers_, extension.uri)) {
+        send_encrypted_headers.push_back(extension.id);
+      }
+    }
+    for (const webrtc::RtpExtension& extension :
+         rtp_remote_encrypted_headers_) {
+      if (FindHeaderExtension(rtp_local_encrypted_headers_, extension.uri)) {
+        recv_encrypted_headers.push_back(extension.id);
+      }
+    }

[pthatcher1, 2017/03/21 07:07:06]

Why not just pass in all header extensions and let srtp_filter_ check the
.encrypted bit?  Seems a lot more simple.

[Taylor Brandstetter, 2017/03/22 18:00:11]

It looks like you're taking the intersection of rtp_local_encrypted_headers and
rtp_remote_encrypted_headers twice? The only thing that may change is the order,
but it doesn't seem like that should make a difference.

[joachim, 2017/03/23 00:04:33]

The lists already contain only encrypted extensions. This code does the
negotiation and determines which extensions actually will be encrypted for
sending/receiving.

Moved to "SrtpFilter::GetSendRecvEncryptedHeaderExtensions" now.

[joachim, 2017/03/23 00:04:33]

The ids of the extensions could be different for the local/remote side. That's
why both lists must be filtered separately.

[Taylor Brandstetter, 2017/03/23 20:10:56]

Ah, right. May be helpful to mention this in a comment.

[joachim, 2017/03/30 22:43:49]

Done (in "SrtpFilter::GetSendRecvEncryptedHeaderExtensions").

+
     ret = srtp_filter_.SetRtpParams(selected_crypto_suite, &(*send_key)[0],
                                     static_cast<int>(send_key->size()),
+                                    send_encrypted_headers,
                                     selected_crypto_suite, &(*recv_key)[0],
-                                    static_cast<int>(recv_key->size()));
+                                    static_cast<int>(recv_key->size()),
+                                    recv_encrypted_headers);
   }
 
   if (!ret) {
     LOG(LS_WARNING) << "DTLS-SRTP key installation failed";
   } else {
     dtls_keyed_ = true;
     UpdateTransportOverhead();
   }
   return ret;
 }
 
-void BaseChannel::MaybeSetupDtlsSrtp_n() {
+bool BaseChannel::MaybeSetupDtlsSrtp_n() {
   if (srtp_filter_.IsActive()) {
-    return;
+    return false;
+  }
+
+  if (!writable_) {
+    return false;
+  }
+
+  if (rtp_encrypted_headers_ready_ != kAllEncryptedHeadersReady) {
+    return false;

[pthatcher1, 2017/03/21 07:07:06]

It seems like it would be more clear to put this into ShouldSetupDtlsSrtp_n()
then to put it directly here, especially since that also affects
IsReadyToSendMedia_n.

[joachim, 2017/03/23 00:04:33]

Code has been removed in latest update.

   }
 
   if (!ShouldSetupDtlsSrtp_n()) {
-    return;
+    return false;
   }
 
   if (!SetupDtlsSrtp_n(false)) {
     SignalDtlsSrtpSetupFailure_n(false);
-    return;
+    return false;
   }
 
   if (rtcp_dtls_transport_) {
     if (!SetupDtlsSrtp_n(true)) {
       SignalDtlsSrtpSetupFailure_n(true);
-      return;
+      return false;
     }
   }
+
+  return true;
 }
 
 void BaseChannel::ChannelNotWritable_n() {
   RTC_DCHECK(network_thread_->IsCurrent());
   if (!writable_)
     return;
 
   LOG(LS_INFO) << "Channel not writable (" << content_name_ << ")";
   writable_ = false;
   UpdateMediaSendRecvState();
@@ skipping 326 matching lines @@
       FindHeaderExtension(extensions, webrtc::RtpExtension::kAbsSendTimeUri);
   int rtp_abs_sendtime_extn_id =
       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::PrepareHeaderExtensions_w(ContentSource source,
+    const std::vector<webrtc::RtpExtension>& extensions) {
+  std::vector<webrtc::RtpExtension> encrypted_headers;
+  for (const webrtc::RtpExtension& extension : extensions) {
+    if (extension.encrypted) {
+      LOG(LS_INFO) << "Using " << source << " encrypted extension: "
+          << extension.ToString();
+      encrypted_headers.push_back(extension);
+    }
+  }
+
+  invoker_.AsyncInvoke<void>(
+      RTC_FROM_HERE, network_thread_,
+      Bind(&BaseChannel::PrepareHeaderExtensions_n, this, source,
+           encrypted_headers));

[Taylor Brandstetter, 2017/03/22 18:00:10]

Is there a reason why PrepareHeaderExtensions_w is called with all extensions,
but PrepareHeaderExtensions_n is only called with encrypted extensions?

[joachim, 2017/03/23 00:04:32]

I wanted to do the actual "work" (i.e. filtering the extensions) on the worker
thread and just store the value on the network thread.

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

In this case the work is pretty trivial, so I wouldn't worry about it. But as
long as the variables are named clearly, I guess it doesn't hurt.

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

Ok, the whole separated "preparing" code has been moved to
SetRtpTransportParameters" with clearly named variables ("extensions" and
"encrypted_extension_ids").

+}
+
 void BaseChannel::CacheRtpAbsSendTimeHeaderExtension_n(
     int rtp_abs_sendtime_extn_id) {
   rtp_abs_sendtime_extn_id_ = rtp_abs_sendtime_extn_id;
 }
 
+void BaseChannel::PrepareHeaderExtensions_n(ContentSource source,

[Taylor Brandstetter, 2017/03/22 18:00:10]

Can this be done in SetRtpTransportParameters_n, to avoid the extra thread hop?

[joachim, 2017/03/23 00:04:33]

Done.

+    const std::vector<webrtc::RtpExtension> encrypted_headers) {
+  if (source == CS_LOCAL) {
+    rtp_local_encrypted_headers_ = encrypted_headers;
+    rtp_encrypted_headers_ready_ |= kLocalEncryptedHeadersReady;

[pthatcher1, 2017/03/21 07:07:06]

I don't understand why we need this extra state.    This is basically just
tracking if we've received a local and remote description, right?  But we won't
become writable and get SRTP keys to extract before we have a local and remote
description.  So why do we need yet more state to track that?

[joachim, 2017/03/23 00:04:32]

Right, and no longer needed with the latest changes.

+  } else {
+    rtp_remote_encrypted_headers_ = encrypted_headers;
+    rtp_encrypted_headers_ready_ |= kRemoteEncryptedHeadersReady;
+  }
+  if (MaybeSetupDtlsSrtp_n()) {

[Taylor Brandstetter, 2017/03/22 18:00:11]

If the encrypted headers are only set when SRTP is first set up, that means if
new ones are added in a subsequent offer/answer, they're ignored. May be a
corner case, but that seems incorrect.

Would it be possible to use srtp_update to update the header extensions whenever
they change? That would also simplify some things in BaseChannel I think (no
longer need to wait for extensions to be set before setting up SRTP).

[joachim, 2017/03/23 00:04:33]

With the change to set the encrypted headers through
"SetRtpTransportParameters", this probably no longer applies. Or am I missing
something?

[Taylor Brandstetter, 2017/03/23 20:10:56]

Even though they're set through SetRtpTransportParameters, they're only used in
SrtpFilter::SetRtpParams. So, it's corner case territory, but suppose a
subsequent offer adds a new encrypted extension. How does the SRTP session get
updated?

There's also the situation where an initial offer/answer negotiates an
audio-only m= section, and then a video m= section is negotiated later. I
believe this would result in SetRtpParams being called as soon as the offer is
applied, before the information from the header extensions in the answer is
available.

[joachim, 2017/03/30 22:43:49]

Adding/changing encrypted extensions is now supported with the latest upload.

+    UpdateMediaSendRecvState();

[pthatcher1, 2017/03/21 07:07:06]

This should only be necessary if IsReadyToSendMedia_n() changes.  For audio, I
believe it will be a no-op if it doesn't:

https://cs.chromium.org/chromium/src/third_party/webrtc/media/engine/webrtcvo...

Video seems to have an effect even if it doesn't change, but that's probably
incorrect and shouldn't be relied on:

https://cs.chromium.org/chromium/src/third_party/webrtc/media/engine/webrtcvi...

[joachim, 2017/03/23 00:04:32]

This is no longer needed with the latest changes.

+  }
+}
+
 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);
       bool rtcp = pmsg->message_id == MSG_SEND_RTCP_PACKET;
       SendPacket(rtcp, &data->packet, data->options);
@@ skipping 299 matching lines @@
   LOG(LS_INFO) << "Setting local voice description";
 
   const AudioContentDescription* audio =
       static_cast<const AudioContentDescription*>(content);
   RTC_DCHECK(audio != NULL);
   if (!audio) {
     SafeSetError("Can't find audio content in local description.", error_desc);
     return false;
   }
 
+  PrepareHeaderExtensions_w(CS_LOCAL, audio->rtp_header_extensions());
+
   if (!SetRtpTransportParameters(content, action, CS_LOCAL, error_desc)) {
     return false;
   }
 
   AudioRecvParameters recv_params = last_recv_params_;
   RtpParametersFromMediaDescription(audio, &recv_params);
   if (!media_channel()->SetRecvParameters(recv_params)) {
     SafeSetError("Failed to set local audio description recv parameters.",
                  error_desc);
     return false;
@@ skipping 25 matching lines @@
   LOG(LS_INFO) << "Setting remote voice description";
 
   const AudioContentDescription* audio =
       static_cast<const AudioContentDescription*>(content);
   RTC_DCHECK(audio != NULL);
   if (!audio) {
     SafeSetError("Can't find audio content in remote description.", error_desc);
     return false;
   }
 
+  PrepareHeaderExtensions_w(CS_REMOTE, audio->rtp_header_extensions());
+
   if (!SetRtpTransportParameters(content, action, CS_REMOTE, error_desc)) {
     return false;
   }
 
   AudioSendParameters send_params = last_send_params_;
   RtpSendParametersFromMediaDescription(audio, &send_params);
   if (audio->agc_minus_10db()) {
     send_params.options.adjust_agc_delta = rtc::Optional<int>(kAgcMinus10db);
   }
 
@@ skipping 213 matching lines @@
   LOG(LS_INFO) << "Setting local video description";
 
   const VideoContentDescription* video =
       static_cast<const VideoContentDescription*>(content);
   RTC_DCHECK(video != NULL);
   if (!video) {
     SafeSetError("Can't find video content in local description.", error_desc);
     return false;
   }
 
+  PrepareHeaderExtensions_w(CS_LOCAL, video->rtp_header_extensions());
+
   if (!SetRtpTransportParameters(content, action, CS_LOCAL, error_desc)) {
     return false;
   }
 
   VideoRecvParameters recv_params = last_recv_params_;
   RtpParametersFromMediaDescription(video, &recv_params);
   if (!media_channel()->SetRecvParameters(recv_params)) {
     SafeSetError("Failed to set local video description recv parameters.",
                  error_desc);
     return false;
@@ skipping 25 matching lines @@
   LOG(LS_INFO) << "Setting remote video description";
 
   const VideoContentDescription* video =
       static_cast<const VideoContentDescription*>(content);
   RTC_DCHECK(video != NULL);
   if (!video) {
     SafeSetError("Can't find video content in remote description.", error_desc);
     return false;
   }
 
+  PrepareHeaderExtensions_w(CS_REMOTE, video->rtp_header_extensions());
+
   if (!SetRtpTransportParameters(content, action, CS_REMOTE, error_desc)) {
     return false;
   }
 
   VideoSendParameters send_params = last_send_params_;
   RtpSendParametersFromMediaDescription(video, &send_params);
   if (video->conference_mode()) {
     send_params.conference_mode = true;
   }
 
@@ skipping 325 matching lines @@
   signaling_thread()->Post(RTC_FROM_HERE, this, MSG_READYTOSENDDATA,
                            new DataChannelReadyToSendMessageData(writable));
 }
 
 void RtpDataChannel::GetSrtpCryptoSuites_n(
     std::vector<int>* crypto_suites) const {
   GetSupportedDataCryptoSuites(crypto_options(), crypto_suites);
 }
 
 }  // namespace cricket