Fix RTP header extension encryption
Previously, RTP header extensions with encryption had been filtered
if the encryption had been activated (not the other way around) which
was likely an unintended logic inversion.
In addition, it ensures that encrypted RTP header extensions are only
negotiated if RTP header extension encryption is turned on. Formerly,
which extensions had been negotiated depended on the order in which
they were inserted, regardless of whether or not header encryption was
actually enabled, leading to no extensions being sent on the wire.
Further changes:
- If RTP header encryption enabled, prefer encrypted extensions over
non-encrypted extensions
- Add most extensions to list of extensions supported for encryption
- Discard encrypted extensions in a session description in case encryption
is not supported for that extension
Note that this depends on https://github.com/cisco/libsrtp/pull/491 to get
into libwebrtc (cherry-pick or bump libsrtp version). Otherwise, two-byte
header extensions will prevent any RTP packets being sent/received.
Bug: webrtc:11713
Change-Id: Ia0779453d342fa11e06996d9bc2d3c826f3466d3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177980
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Taylor <deadbeef@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33723}
diff --git a/api/rtp_parameters.cc b/api/rtp_parameters.cc
index 8a18f89..132c888 100644
--- a/api/rtp_parameters.cc
+++ b/api/rtp_parameters.cc
@@ -170,63 +170,115 @@
}
bool RtpExtension::IsEncryptionSupported(absl::string_view 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. External auth is used by Chromium and replaces the
- // extension header value of "kAbsSendTimeUri", so it must not be
- // encrypted (which can't be done by Chromium).
- uri == webrtc::RtpExtension::kAbsSendTimeUri ||
+ return
+#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. External auth is used by Chromium and replaces the
+ // extension header value of "kAbsSendTimeUri", so it must not be
+ // encrypted (which can't be done by Chromium).
+ uri != webrtc::RtpExtension::kAbsSendTimeUri &&
#endif
- uri == webrtc::RtpExtension::kAbsoluteCaptureTimeUri ||
- uri == webrtc::RtpExtension::kVideoRotationUri ||
- uri == webrtc::RtpExtension::kTransportSequenceNumberUri ||
- uri == webrtc::RtpExtension::kTransportSequenceNumberV2Uri ||
- uri == webrtc::RtpExtension::kPlayoutDelayUri ||
- uri == webrtc::RtpExtension::kVideoContentTypeUri ||
- uri == webrtc::RtpExtension::kMidUri ||
- uri == webrtc::RtpExtension::kRidUri ||
- uri == webrtc::RtpExtension::kRepairedRidUri ||
- uri == webrtc::RtpExtension::kVideoLayersAllocationUri;
+ uri != webrtc::RtpExtension::kEncryptHeaderExtensionsUri;
}
-const RtpExtension* RtpExtension::FindHeaderExtensionByUri(
+// Returns whether a header extension with the given URI exists.
+// Note: This does not differentiate between encrypted and non-encrypted
+// extensions, so use with care!
+static bool HeaderExtensionWithUriExists(
const std::vector<RtpExtension>& extensions,
absl::string_view uri) {
for (const auto& extension : extensions) {
if (extension.uri == uri) {
+ return true;
+ }
+ }
+ return false;
+}
+
+const RtpExtension* RtpExtension::FindHeaderExtensionByUri(
+ const std::vector<RtpExtension>& extensions,
+ absl::string_view uri,
+ Filter filter) {
+ const webrtc::RtpExtension* fallback_extension = nullptr;
+ for (const auto& extension : extensions) {
+ if (extension.uri != uri) {
+ continue;
+ }
+
+ switch (filter) {
+ case kDiscardEncryptedExtension:
+ // We only accept an unencrypted extension.
+ if (!extension.encrypt) {
+ return &extension;
+ }
+ break;
+
+ case kPreferEncryptedExtension:
+ // We prefer an encrypted extension but we can fall back to an
+ // unencrypted extension.
+ if (extension.encrypt) {
+ return &extension;
+ } else {
+ fallback_extension = &extension;
+ }
+ break;
+
+ case kRequireEncryptedExtension:
+ // We only accept an encrypted extension.
+ if (extension.encrypt) {
+ return &extension;
+ }
+ break;
+ }
+ }
+
+ // Returning fallback extension (if any)
+ return fallback_extension;
+}
+
+const RtpExtension* RtpExtension::FindHeaderExtensionByUriAndEncryption(
+ const std::vector<RtpExtension>& extensions,
+ absl::string_view uri,
+ bool encrypt) {
+ for (const auto& extension : extensions) {
+ if (extension.uri == uri && extension.encrypt == encrypt) {
return &extension;
}
}
return nullptr;
}
-std::vector<RtpExtension> RtpExtension::FilterDuplicateNonEncrypted(
- const std::vector<RtpExtension>& extensions) {
+const std::vector<RtpExtension> RtpExtension::DeduplicateHeaderExtensions(
+ const std::vector<RtpExtension>& extensions,
+ Filter filter) {
std::vector<RtpExtension> filtered;
- for (auto extension = extensions.begin(); extension != extensions.end();
- ++extension) {
- if (extension->encrypt) {
- filtered.push_back(*extension);
- continue;
- }
- // Only add non-encrypted extension if no encrypted with the same URI
- // is also present...
- if (std::any_of(extension + 1, extensions.end(),
- [&](const RtpExtension& check) {
- return extension->uri == check.uri;
- })) {
- continue;
- }
-
- // ...and has not been added before.
- if (!FindHeaderExtensionByUri(filtered, extension->uri)) {
- filtered.push_back(*extension);
+ // If we do not discard encrypted extensions, add them first
+ if (filter != kDiscardEncryptedExtension) {
+ for (const auto& extension : extensions) {
+ if (!extension.encrypt) {
+ continue;
+ }
+ if (!HeaderExtensionWithUriExists(filtered, extension.uri)) {
+ filtered.push_back(extension);
+ }
}
}
+
+ // If we do not require encrypted extensions, add missing, non-encrypted
+ // extensions.
+ if (filter != kRequireEncryptedExtension) {
+ for (const auto& extension : extensions) {
+ if (extension.encrypt) {
+ continue;
+ }
+ if (!HeaderExtensionWithUriExists(filtered, extension.uri)) {
+ filtered.push_back(extension);
+ }
+ }
+ }
+
return filtered;
}
} // namespace webrtc
diff --git a/api/rtp_parameters.h b/api/rtp_parameters.h
index 7fe9f2b..5764d97 100644
--- a/api/rtp_parameters.h
+++ b/api/rtp_parameters.h
@@ -246,6 +246,18 @@
// RTP header extension, see RFC8285.
struct RTC_EXPORT RtpExtension {
+ enum Filter {
+ // Encrypted extensions will be ignored and only non-encrypted extensions
+ // will be considered.
+ kDiscardEncryptedExtension,
+ // Encrypted extensions will be preferred but will fall back to
+ // non-encrypted extensions if necessary.
+ kPreferEncryptedExtension,
+ // Encrypted extensions will be required, so any non-encrypted extensions
+ // will be discarded.
+ kRequireEncryptedExtension,
+ };
+
RtpExtension();
RtpExtension(absl::string_view uri, int id);
RtpExtension(absl::string_view uri, int id, bool encrypt);
@@ -260,17 +272,23 @@
// Return "true" if the given RTP header extension URI may be encrypted.
static bool IsEncryptionSupported(absl::string_view uri);
- // Returns the named header extension if found among all extensions,
- // nullptr otherwise.
+ // Returns the header extension with the given URI or nullptr if not found.
static const RtpExtension* FindHeaderExtensionByUri(
const std::vector<RtpExtension>& extensions,
- absl::string_view uri);
+ absl::string_view uri,
+ Filter filter);
- // Return a list of RTP header extensions with the non-encrypted extensions
- // removed if both the encrypted and non-encrypted extension is present for
- // the same URI.
- static std::vector<RtpExtension> FilterDuplicateNonEncrypted(
- const std::vector<RtpExtension>& extensions);
+ // Returns the header extension with the given URI and encrypt parameter,
+ // if found, otherwise nullptr.
+ static const RtpExtension* FindHeaderExtensionByUriAndEncryption(
+ const std::vector<RtpExtension>& extensions,
+ absl::string_view uri,
+ bool encrypt);
+
+ // Returns a list of extensions where any extension URI is unique.
+ static const std::vector<RtpExtension> DeduplicateHeaderExtensions(
+ const std::vector<RtpExtension>& extensions,
+ Filter filter);
// Encryption of Header Extensions, see RFC 6904 for details:
// https://tools.ietf.org/html/rfc6904
diff --git a/api/rtp_parameters_unittest.cc b/api/rtp_parameters_unittest.cc
index 5928cbd..51ad426 100644
--- a/api/rtp_parameters_unittest.cc
+++ b/api/rtp_parameters_unittest.cc
@@ -23,28 +23,249 @@
static const RtpExtension kExtension1Encrypted(kExtensionUri1, 10, true);
static const RtpExtension kExtension2(kExtensionUri2, 2);
-TEST(RtpExtensionTest, FilterDuplicateNonEncrypted) {
+TEST(RtpExtensionTest, DeduplicateHeaderExtensions) {
std::vector<RtpExtension> extensions;
std::vector<RtpExtension> filtered;
+ extensions.clear();
extensions.push_back(kExtension1);
extensions.push_back(kExtension1Encrypted);
- filtered = RtpExtension::FilterDuplicateNonEncrypted(extensions);
+ filtered = RtpExtension::DeduplicateHeaderExtensions(
+ extensions, RtpExtension::Filter::kDiscardEncryptedExtension);
+ EXPECT_EQ(1u, filtered.size());
+ EXPECT_EQ(std::vector<RtpExtension>{kExtension1}, filtered);
+
+ extensions.clear();
+ extensions.push_back(kExtension1);
+ extensions.push_back(kExtension1Encrypted);
+ filtered = RtpExtension::DeduplicateHeaderExtensions(
+ extensions, RtpExtension::Filter::kPreferEncryptedExtension);
+ EXPECT_EQ(1u, filtered.size());
+ EXPECT_EQ(std::vector<RtpExtension>{kExtension1Encrypted}, filtered);
+
+ extensions.clear();
+ extensions.push_back(kExtension1);
+ extensions.push_back(kExtension1Encrypted);
+ filtered = RtpExtension::DeduplicateHeaderExtensions(
+ extensions, RtpExtension::Filter::kRequireEncryptedExtension);
EXPECT_EQ(1u, filtered.size());
EXPECT_EQ(std::vector<RtpExtension>{kExtension1Encrypted}, filtered);
extensions.clear();
extensions.push_back(kExtension1Encrypted);
extensions.push_back(kExtension1);
- filtered = RtpExtension::FilterDuplicateNonEncrypted(extensions);
+ filtered = RtpExtension::DeduplicateHeaderExtensions(
+ extensions, RtpExtension::Filter::kDiscardEncryptedExtension);
+ EXPECT_EQ(1u, filtered.size());
+ EXPECT_EQ(std::vector<RtpExtension>{kExtension1}, filtered);
+
+ extensions.clear();
+ extensions.push_back(kExtension1Encrypted);
+ extensions.push_back(kExtension1);
+ filtered = RtpExtension::DeduplicateHeaderExtensions(
+ extensions, RtpExtension::Filter::kPreferEncryptedExtension);
+ EXPECT_EQ(1u, filtered.size());
+ EXPECT_EQ(std::vector<RtpExtension>{kExtension1Encrypted}, filtered);
+
+ extensions.clear();
+ extensions.push_back(kExtension1Encrypted);
+ extensions.push_back(kExtension1);
+ filtered = RtpExtension::DeduplicateHeaderExtensions(
+ extensions, RtpExtension::Filter::kRequireEncryptedExtension);
EXPECT_EQ(1u, filtered.size());
EXPECT_EQ(std::vector<RtpExtension>{kExtension1Encrypted}, filtered);
extensions.clear();
extensions.push_back(kExtension1);
extensions.push_back(kExtension2);
- filtered = RtpExtension::FilterDuplicateNonEncrypted(extensions);
+ filtered = RtpExtension::DeduplicateHeaderExtensions(
+ extensions, RtpExtension::Filter::kDiscardEncryptedExtension);
EXPECT_EQ(2u, filtered.size());
EXPECT_EQ(extensions, filtered);
+ filtered = RtpExtension::DeduplicateHeaderExtensions(
+ extensions, RtpExtension::Filter::kPreferEncryptedExtension);
+ EXPECT_EQ(2u, filtered.size());
+ EXPECT_EQ(extensions, filtered);
+ filtered = RtpExtension::DeduplicateHeaderExtensions(
+ extensions, RtpExtension::Filter::kRequireEncryptedExtension);
+ EXPECT_EQ(0u, filtered.size());
+
+ extensions.clear();
+ extensions.push_back(kExtension1);
+ extensions.push_back(kExtension2);
+ extensions.push_back(kExtension1Encrypted);
+ filtered = RtpExtension::DeduplicateHeaderExtensions(
+ extensions, RtpExtension::Filter::kDiscardEncryptedExtension);
+ EXPECT_EQ(2u, filtered.size());
+ EXPECT_EQ((std::vector<RtpExtension>{kExtension1, kExtension2}), filtered);
+ filtered = RtpExtension::DeduplicateHeaderExtensions(
+ extensions, RtpExtension::Filter::kPreferEncryptedExtension);
+ EXPECT_EQ(2u, filtered.size());
+ EXPECT_EQ((std::vector<RtpExtension>{kExtension1Encrypted, kExtension2}),
+ filtered);
+ filtered = RtpExtension::DeduplicateHeaderExtensions(
+ extensions, RtpExtension::Filter::kRequireEncryptedExtension);
+ EXPECT_EQ(1u, filtered.size());
+ EXPECT_EQ((std::vector<RtpExtension>{kExtension1Encrypted}), filtered);
+}
+
+TEST(RtpExtensionTest, FindHeaderExtensionByUriAndEncryption) {
+ std::vector<RtpExtension> extensions;
+
+ extensions.clear();
+ EXPECT_EQ(nullptr, RtpExtension::FindHeaderExtensionByUriAndEncryption(
+ extensions, kExtensionUri1, false));
+
+ extensions.clear();
+ extensions.push_back(kExtension1);
+ EXPECT_EQ(kExtension1, *RtpExtension::FindHeaderExtensionByUriAndEncryption(
+ extensions, kExtensionUri1, false));
+ EXPECT_EQ(nullptr, RtpExtension::FindHeaderExtensionByUriAndEncryption(
+ extensions, kExtensionUri1, true));
+ EXPECT_EQ(nullptr, RtpExtension::FindHeaderExtensionByUriAndEncryption(
+ extensions, kExtensionUri2, false));
+
+ extensions.clear();
+ extensions.push_back(kExtension1);
+ extensions.push_back(kExtension2);
+ extensions.push_back(kExtension1Encrypted);
+ EXPECT_EQ(kExtension1, *RtpExtension::FindHeaderExtensionByUriAndEncryption(
+ extensions, kExtensionUri1, false));
+ EXPECT_EQ(kExtension2, *RtpExtension::FindHeaderExtensionByUriAndEncryption(
+ extensions, kExtensionUri2, false));
+ EXPECT_EQ(kExtension1Encrypted,
+ *RtpExtension::FindHeaderExtensionByUriAndEncryption(
+ extensions, kExtensionUri1, true));
+ EXPECT_EQ(nullptr, RtpExtension::FindHeaderExtensionByUriAndEncryption(
+ extensions, kExtensionUri2, true));
+}
+
+TEST(RtpExtensionTest, FindHeaderExtensionByUri) {
+ std::vector<RtpExtension> extensions;
+
+ extensions.clear();
+ EXPECT_EQ(nullptr, RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri1,
+ RtpExtension::Filter::kDiscardEncryptedExtension));
+ EXPECT_EQ(nullptr, RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri1,
+ RtpExtension::Filter::kPreferEncryptedExtension));
+ EXPECT_EQ(nullptr, RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri1,
+ RtpExtension::Filter::kRequireEncryptedExtension));
+
+ extensions.clear();
+ extensions.push_back(kExtension1);
+ EXPECT_EQ(kExtension1, *RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri1,
+ RtpExtension::Filter::kDiscardEncryptedExtension));
+ EXPECT_EQ(kExtension1, *RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri1,
+ RtpExtension::Filter::kPreferEncryptedExtension));
+ EXPECT_EQ(nullptr, RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri1,
+ RtpExtension::Filter::kRequireEncryptedExtension));
+ EXPECT_EQ(nullptr, RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri2,
+ RtpExtension::Filter::kDiscardEncryptedExtension));
+ EXPECT_EQ(nullptr, RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri2,
+ RtpExtension::Filter::kPreferEncryptedExtension));
+ EXPECT_EQ(nullptr, RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri2,
+ RtpExtension::Filter::kRequireEncryptedExtension));
+
+ extensions.clear();
+ extensions.push_back(kExtension1);
+ extensions.push_back(kExtension1Encrypted);
+ EXPECT_EQ(kExtension1, *RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri1,
+ RtpExtension::Filter::kDiscardEncryptedExtension));
+
+ extensions.clear();
+ extensions.push_back(kExtension1);
+ extensions.push_back(kExtension1Encrypted);
+ EXPECT_EQ(kExtension1Encrypted,
+ *RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri1,
+ RtpExtension::Filter::kPreferEncryptedExtension));
+
+ extensions.clear();
+ extensions.push_back(kExtension1);
+ extensions.push_back(kExtension1Encrypted);
+ EXPECT_EQ(kExtension1Encrypted,
+ *RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri1,
+ RtpExtension::Filter::kRequireEncryptedExtension));
+
+ extensions.clear();
+ extensions.push_back(kExtension1Encrypted);
+ extensions.push_back(kExtension1);
+ EXPECT_EQ(kExtension1, *RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri1,
+ RtpExtension::Filter::kDiscardEncryptedExtension));
+
+ extensions.clear();
+ extensions.push_back(kExtension1Encrypted);
+ extensions.push_back(kExtension1);
+ EXPECT_EQ(kExtension1Encrypted,
+ *RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri1,
+ RtpExtension::Filter::kPreferEncryptedExtension));
+
+ extensions.clear();
+ extensions.push_back(kExtension1Encrypted);
+ extensions.push_back(kExtension1);
+ EXPECT_EQ(kExtension1Encrypted,
+ *RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri1,
+ RtpExtension::Filter::kRequireEncryptedExtension));
+
+ extensions.clear();
+ extensions.push_back(kExtension1);
+ extensions.push_back(kExtension2);
+ EXPECT_EQ(kExtension1, *RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri1,
+ RtpExtension::Filter::kDiscardEncryptedExtension));
+ EXPECT_EQ(kExtension1, *RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri1,
+ RtpExtension::Filter::kPreferEncryptedExtension));
+ EXPECT_EQ(nullptr, RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri1,
+ RtpExtension::Filter::kRequireEncryptedExtension));
+ EXPECT_EQ(kExtension2, *RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri2,
+ RtpExtension::Filter::kDiscardEncryptedExtension));
+ EXPECT_EQ(kExtension2, *RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri2,
+ RtpExtension::Filter::kPreferEncryptedExtension));
+ EXPECT_EQ(nullptr, RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri2,
+ RtpExtension::Filter::kRequireEncryptedExtension));
+
+ extensions.clear();
+ extensions.push_back(kExtension1);
+ extensions.push_back(kExtension2);
+ extensions.push_back(kExtension1Encrypted);
+ EXPECT_EQ(kExtension1, *RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri1,
+ RtpExtension::Filter::kDiscardEncryptedExtension));
+ EXPECT_EQ(kExtension1Encrypted,
+ *RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri1,
+ RtpExtension::Filter::kPreferEncryptedExtension));
+ EXPECT_EQ(kExtension1Encrypted,
+ *RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri1,
+ RtpExtension::Filter::kRequireEncryptedExtension));
+ EXPECT_EQ(kExtension2, *RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri2,
+ RtpExtension::Filter::kDiscardEncryptedExtension));
+ EXPECT_EQ(kExtension2, *RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri2,
+ RtpExtension::Filter::kPreferEncryptedExtension));
+ EXPECT_EQ(nullptr, RtpExtension::FindHeaderExtensionByUri(
+ extensions, kExtensionUri2,
+ RtpExtension::Filter::kRequireEncryptedExtension));
}
} // namespace webrtc