Support encrypted RTP extensions (RFC 6904)

webrtc/config.cc

Index: webrtc/config.cc
diff --git a/webrtc/config.cc b/webrtc/config.cc
index e0c490d1ecd8038d546b7298fc9a885ab02718df..5de94be17fb74b7e84b7306ad9c8c81dddad1854 100644
--- a/webrtc/config.cc
+++ b/webrtc/config.cc
@@ -41,6 +41,9 @@ std::string RtpExtension::ToString() const {
   std::stringstream ss;
   ss << "{uri: " << uri;
   ss << ", id: " << id;
+  if (encrypt) {
+    ss << ", encrypt";
+  }
   ss << '}';
   return ss.str();
 }
@@ -72,6 +75,9 @@ const char* RtpExtension::kPlayoutDelayUri =
     "http://www.webrtc.org/experiments/rtp-hdrext/playout-delay";
 const int RtpExtension::kPlayoutDelayDefaultId = 6;
 
+const char* RtpExtension::kEncryptHeaderExtensionsUri =
+    "urn:ietf:params:rtp-hdrext:encrypt";
+
 const int RtpExtension::kMinId = 1;
 const int RtpExtension::kMaxId = 14;
 
@@ -88,6 +94,66 @@ bool RtpExtension::IsSupportedForVideo(const std::string& uri) {
          uri == webrtc::RtpExtension::kPlayoutDelayUri;
 }
 
+bool RtpExtension::IsEncryptionSupported(const std::string& uri) {
+  return uri == webrtc::RtpExtension::kAudioLevelUri ||
+         uri == webrtc::RtpExtension::kTimestampOffsetUri ||
+#if !defined(ENABLE_EXTERNAL_AUTH)
+         // TODO(jbauch): Figure out a way to always allow "kAbsSendTimeUri"
+         // here and filter out later if external auth is really used in
+         // srtpfilter.

[pthatcher1, 2017/05/05 21:26:38]

Can you make it more clear why we can't have this?  I know it's something to do
with Chrome and re-encrypting the SRTP packet right before hitting the network. 
But having a little more explanation here would be nice.

[joachim, 2017/05/06 14:52:32]

Updated the comment, I hope it's clearer now.

+         uri == webrtc::RtpExtension::kAbsSendTimeUri ||
+#endif
+         uri == webrtc::RtpExtension::kVideoRotationUri ||
+         uri == webrtc::RtpExtension::kTransportSequenceNumberUri ||
+         uri == webrtc::RtpExtension::kPlayoutDelayUri;
+}
+
+const RtpExtension* RtpExtension::FindHeaderExtensionByUri(
+    const std::vector<RtpExtension>& extensions,
+    const std::string& uri) {
+  for (const auto& extension : extensions) {
+    if (extension.uri == uri) {
+      return &extension;
+    }
+  }
+  return nullptr;
+}
+
+std::vector<RtpExtension> RtpExtension::FilterDuplicateNonEncrypted(
+    const std::vector<RtpExtension>& extensions) {
+  std::vector<RtpExtension> filtered;
+  for (auto it = extensions.begin(); it != extensions.end(); ++it) {
+    if (it->encrypt) {
+      filtered.push_back(*it);

[pthatcher1, 2017/05/05 21:26:38]

Can we fight moving right by doing an early continue?

if (it->encrypt) {
  filtered.push_back(*it);
  continue;
}
bool found = false;
...

[joachim, 2017/05/06 14:52:32]

Done.

+    } else {
+      // Only add non-encrypted extension if no encrypted with the same URI
+      // is also present...
+      bool found = false;
+      for (auto it2 = it + 1; it2 != extensions.end(); ++it2) {
+        if (it->uri == it2->uri) {
+          found = true;
+          break;
+        }
+      }

[pthatcher1, 2017/05/05 21:26:38]

The list of header extensions is so short, I don't think this is worth the
optimization here.  If you didn't have it and took a very tiny perf hit, you'd
be able to use regular for loops with nicer variable names (not "it").  In fact,
you could just use the existing FindHeaderExtensionByUri: 
it->FindHeaderExtensionsByUri(existing);

[joachim, 2017/05/06 14:52:32]

It's not really an optimization but checking the remaining extensions in the
list. However I changed it to use "std::find_if" which should also make it more
readable now.

+
+      if (!found) {

[pthatcher1, 2017/05/05 21:26:38]

Please use early continue again:

if (found) {
  continue;
}
// ...and has not been added before.

[joachim, 2017/05/06 14:52:32]

Done.

+        // ...and has not been added before.
+        for (const RtpExtension& existing : filtered) {
+          if (it->uri == existing.uri) {
+            found = true;
+            break;
+          }
+        }

[pthatcher1, 2017/05/05 21:26:38]

Why not just use existing.FindHeaderExtensionByUri(filtered)?

[joachim, 2017/05/06 14:52:32]

Done.

+
+        if (!found) {
+          filtered.push_back(*it);
+        }
+      }
+    }
+  }
+  return filtered;
+}
+
 VideoStream::VideoStream()
     : width(0),
       height(0),