Support encrypted RTP extensions (RFC 6904)

webrtc/pc/mediasession.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 927 matching lines @@
     if (it->uri == ext_to_match.uri) {
       if (found_extension != NULL) {
         *found_extension = *it;
       }
       return true;
     }
   }
   return false;
 }
 
+static bool FindByUriPreferEncrypted(const RtpHeaderExtensions& extensions,

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

nit: This name sounds like it always prefers encrypted extensions. Something
like "FindByUriWithEncryptionPreference" may be better.

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

Done.

+                                     const webrtc::RtpExtension& ext_to_match,
+                                     bool prefer_encrypted,
+                                     webrtc::RtpExtension* found_extension) {
+  const webrtc::RtpExtension* regular_extension = nullptr;
+  for (RtpHeaderExtensions::const_iterator it = extensions.begin();
+       it  != extensions.end(); ++it) {
+    // We assume that all URIs are given in a canonical format.
+    if (it->uri == ext_to_match.uri) {
+      if (!found_extension) {
+        return true;
+      }
+      if (!prefer_encrypted || it->encrypted) {
+        *found_extension = *it;
+        return true;
+      }
+      regular_extension = &(*it);
+    }
+  }
+  if (regular_extension) {
+    *found_extension = *regular_extension;
+    return true;
+  }
+  return false;
+}
+
 // Iterates through |offered_extensions|, adding each one to |all_extensions|
 // and |used_ids|, and resolving ID conflicts. If an offered extension has the
 // same URI as one in |all_extensions|, it will re-use the same ID and won't be
 // treated as a conflict.
 static void FindAndSetRtpHdrExtUsed(RtpHeaderExtensions* offered_extensions,
                                     RtpHeaderExtensions* all_extensions,
                                     UsedRtpHeaderExtensionIds* used_ids) {
   for (auto& extension : *offered_extensions) {
     webrtc::RtpExtension existing;
     if (FindByUri(*all_extensions, extension, &existing)) {
@@ skipping 19 matching lines @@
         offered_extensions->push_back(existing);
       } else {
         used_ids->FindAndSetIdUsed(&reference_extension);
         all_extensions->push_back(reference_extension);
         offered_extensions->push_back(reference_extension);
       }
     }
   }
 }
 
+static void AddEncryptedHdrExts(RtpHeaderExtensions* extensions,
+                                UsedRtpHeaderExtensionIds* used_ids) {

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

This should probably be called something like AddEncryptedVersionsOfHdrExts

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

Done.

+  RtpHeaderExtensions encrypted_extensions;
+  for (const webrtc::RtpExtension& extension : *extensions) {
+    if (extension.encrypted ||
+        !webrtc::RtpExtension::AllowEncrypt(extension.uri)) {
+      continue;
+    }
+
+    webrtc::RtpExtension encrypted(extension);
+    encrypted.encrypted = true;
+    used_ids->FindAndSetIdUsed(&encrypted);
+    encrypted_extensions.push_back(encrypted);

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

Is there a reason the separate "encrypted_extensions" list is used, rather than
calling push_back in "extensions" directly?

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

That could invalidate the iterator for "extensions" if the underlying vector is
getting reallocated.

+  }
+  extensions->insert(extensions->end(), encrypted_extensions.begin(),
+      encrypted_extensions.end());
+}
+
 static void NegotiateRtpHeaderExtensions(
     const RtpHeaderExtensions& local_extensions,
     const RtpHeaderExtensions& offered_extensions,
+    bool enable_rtp_header_encryption,
     RtpHeaderExtensions* negotiated_extenstions) {
   RtpHeaderExtensions::const_iterator ours;
   for (ours = local_extensions.begin();
        ours != local_extensions.end(); ++ours) {
     webrtc::RtpExtension theirs;
-    if (FindByUri(offered_extensions, *ours, &theirs)) {
+    if (FindByUriPreferEncrypted(offered_extensions, *ours,
+        enable_rtp_header_encryption, &theirs)) {
       // We respond with their RTP header extension id.
       negotiated_extenstions->push_back(theirs);
     }
   }
 }
 
 static void StripCNCodecs(AudioCodecs* audio_codecs) {
   AudioCodecs::iterator iter = audio_codecs->begin();
   while (iter != audio_codecs->end()) {
     if (STR_CASE_CMP(iter->name.c_str(), kComfortNoiseCodecName) == 0) {
@@ skipping 14 matching lines @@
 // from the incoming session-initiate.  If the negotiation fails, this
 // method returns false.  The created content is added to the offer.
 template <class C>
 static bool CreateMediaContentAnswer(
     const MediaContentDescriptionImpl<C>* offer,
     const MediaSessionOptions& options,
     const std::vector<C>& local_codecs,
     const SecurePolicy& sdes_policy,
     const CryptoParamsVec* current_cryptos,
     const RtpHeaderExtensions& local_rtp_extenstions,
+    bool enable_rtp_header_encryption,
     StreamParamsVec* current_streams,
     bool add_legacy_stream,
     bool bundle_enabled,
     MediaContentDescriptionImpl<C>* answer) {
   std::vector<C> negotiated_codecs;
   NegotiateCodecs(local_codecs, offer->codecs(), &negotiated_codecs);
   answer->AddCodecs(negotiated_codecs);
   answer->set_protocol(offer->protocol());
   RtpHeaderExtensions negotiated_rtp_extensions;
   NegotiateRtpHeaderExtensions(local_rtp_extenstions,
                                offer->rtp_header_extensions(),
+                               enable_rtp_header_encryption,
                                &negotiated_rtp_extensions);
   answer->set_rtp_header_extensions(negotiated_rtp_extensions);
 
   answer->set_rtcp_mux(options.rtcp_mux_enabled && offer->rtcp_mux());
   if (answer->type() == cricket::MEDIA_TYPE_VIDEO) {
     answer->set_rtcp_reduced_size(offer->rtcp_reduced_size());
   }
 
   if (sdes_policy != SEC_DISABLED) {
     CryptoParams crypto;
@@ skipping 547 matching lines @@
       FindAndSetRtpHdrExtUsed(video_extensions, &all_extensions, &used_ids);
     }
   }
 
   // Add our default RTP header extensions that are not in
   // |current_description|.
   FindRtpHdrExtsToOffer(audio_rtp_header_extensions(), audio_extensions,
                         &all_extensions, &used_ids);
   FindRtpHdrExtsToOffer(video_rtp_header_extensions(), video_extensions,
                         &all_extensions, &used_ids);
+  if (enable_rtp_header_encryption_) {
+    AddEncryptedHdrExts(audio_extensions, &used_ids);
+    AddEncryptedHdrExts(video_extensions, &used_ids);

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

Why are we storing two copies of the header extension?  Why not instead simply
have the one copy with a field that is tri-state for encryption:  enabled (like
both), disabled (like only the first), and required (like only the second)?  It
seems like it would be more simple.

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

I don't have a problem with storing two copies. That maps 1:1 with everything in
the API and SDP, and doesn't require defining a new structure.

But I think it would be simpler if the encrypted versions were added at
construction time, right after "GetSupportedAudioRtpHeaderExtensions" is called.
Then the extra method "AddEncryptedHdrExts" shouldn't be necessary;
FindRtpHdrExtsToOffer can simply handle both.

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

I think this would just move the complexity to "FindRtpHdrExtsToOffer" which
currently only searches for extensions by URI ("FindByUri"). The URI is the same
for the regular and the encrypted extension, so it would need special handling
to deal with that.

IMHO the code is more cleaner as-is, but if you want, I will look into
refactoring "FindRtpHdrExtsToOffer" to support this use case.

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

The encrypted extensions are using a different id, so that value would also need
to be stored in the RtpExtension class. I don't think that would make it simpler
than having the encrypted extension as a second entry in the list. 

+  }
 }
 
 bool MediaSessionDescriptionFactory::AddTransportOffer(
   const std::string& content_name,
   const TransportOptions& transport_options,
   const SessionDescription* current_desc,
   SessionDescription* offer_desc) const {
   if (!transport_desc_factory_)
      return false;
   const TransportDescription* current_tdesc =
@@ skipping 253 matching lines @@
   // Do not require or create SDES cryptos if DTLS is used.
   cricket::SecurePolicy sdes_policy =
       audio_transport->secure() ? cricket::SEC_DISABLED : secure();
   if (!CreateMediaContentAnswer(
           offer_audio,
           options,
           audio_codecs,
           sdes_policy,
           GetCryptos(GetFirstAudioContentDescription(current_description)),
           audio_rtp_extensions_,
+          enable_rtp_header_encryption_,
           current_streams,
           add_legacy_,
           bundle_enabled,
           audio_answer.get())) {
     return false;  // Fails the session setup.
   }
 
   bool secure = bundle_transport ? bundle_transport->description.secure()
                                  : audio_transport->secure();
   bool rejected = !options.has_audio() || audio_content->rejected ||
@@ skipping 36 matching lines @@
   bool bundle_enabled =
       offer->HasGroup(GROUP_TYPE_BUNDLE) && options.bundle_enabled;
   if (!CreateMediaContentAnswer(
           static_cast<const VideoContentDescription*>(
               video_content->description),
           options,
           video_codecs_,
           sdes_policy,
           GetCryptos(GetFirstVideoContentDescription(current_description)),
           video_rtp_extensions_,
+          enable_rtp_header_encryption_,
           current_streams,
           add_legacy_,
           bundle_enabled,
           video_answer.get())) {
     return false;
   }
   bool secure = bundle_transport ? bundle_transport->description.secure()
                                  : video_transport->secure();
   bool rejected = !options.has_video() || video_content->rejected ||
                   !IsMediaProtocolSupported(MEDIA_TYPE_VIDEO,
@@ skipping 41 matching lines @@
   bool bundle_enabled =
       offer->HasGroup(GROUP_TYPE_BUNDLE) && options.bundle_enabled;
   if (!CreateMediaContentAnswer(
           static_cast<const DataContentDescription*>(
               data_content->description),
           options,
           data_codecs_,
           sdes_policy,
           GetCryptos(GetFirstDataContentDescription(current_description)),
           RtpHeaderExtensions(),
+          enable_rtp_header_encryption_,
           current_streams,
           add_legacy_,
           bundle_enabled,
           data_answer.get())) {
     return false;  // Fails the session setup.
   }
 
   // Respond with sctpmap if the offer uses sctpmap.
   const DataContentDescription* offer_data_description =
       static_cast<const DataContentDescription*>(data_content->description);
@@ skipping 175 matching lines @@
       GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_VIDEO));
 }
 
 DataContentDescription* GetFirstDataContentDescription(
     SessionDescription* sdesc) {
   return static_cast<DataContentDescription*>(
       GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_DATA));
 }
 
 }  // namespace cricket