Make encrypted versions of RTP extension headers be stopped by default.
By this change we aim to remove the flag enable-webrtc-srtp-encrypted-headers.
Bug: chromium:40623740
Change-Id: I74692c90ff1caf2a11d7b73211c1ae4472edfb4d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/362740
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Emil Vardar (xWF) <vardar@google.com>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43105}
diff --git a/api/rtp_parameters.cc b/api/rtp_parameters.cc
index c919f7c..178aeb8 100644
--- a/api/rtp_parameters.cc
+++ b/api/rtp_parameters.cc
@@ -76,6 +76,15 @@
int preferred_id,
RtpTransceiverDirection direction)
: uri(uri), preferred_id(preferred_id), direction(direction) {}
+RtpHeaderExtensionCapability::RtpHeaderExtensionCapability(
+ absl::string_view uri,
+ int preferred_id,
+ bool preferred_encrypt,
+ RtpTransceiverDirection direction)
+ : uri(uri),
+ preferred_id(preferred_id),
+ preferred_encrypt(preferred_encrypt),
+ direction(direction) {}
RtpHeaderExtensionCapability::~RtpHeaderExtensionCapability() = default;
RtpExtension::RtpExtension() = default;
diff --git a/api/rtp_parameters.h b/api/rtp_parameters.h
index b393c3c..ec2243d 100644
--- a/api/rtp_parameters.h
+++ b/api/rtp_parameters.h
@@ -212,7 +212,6 @@
std::optional<int> preferred_id;
// If true, it's preferred that the value in the header is encrypted.
- // TODO(deadbeef): Not implemented.
bool preferred_encrypt = false;
// The direction of the extension. The kStopped value is only used with
@@ -227,6 +226,10 @@
RtpHeaderExtensionCapability(absl::string_view uri,
int preferred_id,
RtpTransceiverDirection direction);
+ RtpHeaderExtensionCapability(absl::string_view uri,
+ int preferred_id,
+ bool preferred_encrypt,
+ RtpTransceiverDirection direction);
~RtpHeaderExtensionCapability();
bool operator==(const RtpHeaderExtensionCapability& o) const {
diff --git a/pc/media_session.cc b/pc/media_session.cc
index 50c4260..0acca37 100644
--- a/pc/media_session.cc
+++ b/pc/media_session.cc
@@ -62,7 +62,8 @@
webrtc::RtpExtension RtpExtensionFromCapability(
const webrtc::RtpHeaderExtensionCapability& capability) {
return webrtc::RtpExtension(capability.uri,
- capability.preferred_id.value_or(1));
+ capability.preferred_id.value_or(1),
+ capability.preferred_encrypt);
}
cricket::RtpHeaderExtensions RtpHeaderExtensionsFromCapabilities(
@@ -866,6 +867,7 @@
// and `encrypted_extensions` are used for both audio and video. There could be
// overlap between audio extensions and video extensions.
void MergeRtpHdrExts(const RtpHeaderExtensions& reference_extensions,
+ bool enable_encrypted_rtp_header_extensions,
RtpHeaderExtensions* offered_extensions,
RtpHeaderExtensions* regular_extensions,
RtpHeaderExtensions* encrypted_extensions,
@@ -875,6 +877,9 @@
*offered_extensions, reference_extension.uri,
reference_extension.encrypt)) {
if (reference_extension.encrypt) {
+ if (!enable_encrypted_rtp_header_extensions) {
+ continue;
+ }
const webrtc::RtpExtension* existing =
webrtc::RtpExtension::FindHeaderExtensionByUriAndEncryption(
*encrypted_extensions, reference_extension.uri,
@@ -903,55 +908,6 @@
}
}
-void AddEncryptedVersionsOfHdrExts(RtpHeaderExtensions* offered_extensions,
- RtpHeaderExtensions* encrypted_extensions,
- UsedRtpHeaderExtensionIds* used_ids) {
- RtpHeaderExtensions encrypted_extensions_to_add;
- for (const auto& extension : *offered_extensions) {
- // Skip existing encrypted offered extension
- if (extension.encrypt) {
- continue;
- }
-
- // Skip if we cannot encrypt the extension
- if (!webrtc::RtpExtension::IsEncryptionSupported(extension.uri)) {
- continue;
- }
-
- // Skip if an encrypted extension with that URI already exists in the
- // offered extensions.
- const bool have_encrypted_extension =
- webrtc::RtpExtension::FindHeaderExtensionByUriAndEncryption(
- *offered_extensions, extension.uri, true);
- if (have_encrypted_extension) {
- continue;
- }
-
- // Determine if a shared encrypted extension with that URI already exists.
- const webrtc::RtpExtension* shared_encrypted_extension =
- webrtc::RtpExtension::FindHeaderExtensionByUriAndEncryption(
- *encrypted_extensions, extension.uri, true);
- if (shared_encrypted_extension) {
- // Re-use the shared encrypted extension
- encrypted_extensions_to_add.push_back(*shared_encrypted_extension);
- continue;
- }
-
- // None exists. Create a new shared encrypted extension from the
- // non-encrypted one.
- webrtc::RtpExtension new_encrypted_extension(extension);
- new_encrypted_extension.encrypt = true;
- used_ids->FindAndSetIdUsed(&new_encrypted_extension);
- encrypted_extensions->push_back(new_encrypted_extension);
- encrypted_extensions_to_add.push_back(new_encrypted_extension);
- }
-
- // Append the additional encrypted extensions to be offered
- offered_extensions->insert(offered_extensions->end(),
- encrypted_extensions_to_add.begin(),
- encrypted_extensions_to_add.end());
-}
-
// Mostly identical to RtpExtension::FindHeaderExtensionByUri but discards any
// encrypted extensions that this implementation cannot encrypt.
const webrtc::RtpExtension* FindHeaderExtensionByUriDiscardUnsupported(
@@ -1927,6 +1883,7 @@
UsedRtpHeaderExtensionIds used_ids(
extmap_allow_mixed ? UsedRtpHeaderExtensionIds::IdDomain::kTwoByteAllowed
: UsedRtpHeaderExtensionIds::IdDomain::kOneByteOnly);
+
RtpHeaderExtensions all_regular_extensions;
RtpHeaderExtensions all_encrypted_extensions;
@@ -1938,10 +1895,12 @@
for (const ContentInfo* content : current_active_contents) {
if (IsMediaContentOfType(content, MEDIA_TYPE_AUDIO)) {
MergeRtpHdrExts(content->media_description()->rtp_header_extensions(),
+ enable_encrypted_rtp_header_extensions_,
&offered_extensions.audio, &all_regular_extensions,
&all_encrypted_extensions, &used_ids);
} else if (IsMediaContentOfType(content, MEDIA_TYPE_VIDEO)) {
MergeRtpHdrExts(content->media_description()->rtp_header_extensions(),
+ enable_encrypted_rtp_header_extensions_,
&offered_extensions.video, &all_regular_extensions,
&all_encrypted_extensions, &used_ids);
}
@@ -1956,22 +1915,15 @@
entry.header_extensions, all_regular_extensions,
all_encrypted_extensions));
if (entry.type == MEDIA_TYPE_AUDIO)
- MergeRtpHdrExts(filtered_extensions, &offered_extensions.audio,
- &all_regular_extensions, &all_encrypted_extensions,
- &used_ids);
+ MergeRtpHdrExts(filtered_extensions,
+ enable_encrypted_rtp_header_extensions_,
+ &offered_extensions.audio, &all_regular_extensions,
+ &all_encrypted_extensions, &used_ids);
else if (entry.type == MEDIA_TYPE_VIDEO)
- MergeRtpHdrExts(filtered_extensions, &offered_extensions.video,
- &all_regular_extensions, &all_encrypted_extensions,
- &used_ids);
- }
- // TODO(jbauch): Support adding encrypted header extensions to existing
- // sessions.
- if (enable_encrypted_rtp_header_extensions_ &&
- current_active_contents.empty()) {
- AddEncryptedVersionsOfHdrExts(&offered_extensions.audio,
- &all_encrypted_extensions, &used_ids);
- AddEncryptedVersionsOfHdrExts(&offered_extensions.video,
- &all_encrypted_extensions, &used_ids);
+ MergeRtpHdrExts(filtered_extensions,
+ enable_encrypted_rtp_header_extensions_,
+ &offered_extensions.video, &all_regular_extensions,
+ &all_encrypted_extensions, &used_ids);
}
return offered_extensions;
}
diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc
index ce81d6c..91e4eac 100644
--- a/pc/media_session_unittest.cc
+++ b/pc/media_session_unittest.cc
@@ -124,8 +124,6 @@
const RtpExtension kAudioRtpExtensionEncrypted1[] = {
RtpExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 8),
- RtpExtension("http://google.com/testing/audio_something", 10),
- RtpExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 12, true),
RtpExtension("http://google.com/testing/audio_something", 11, true),
};
@@ -135,37 +133,24 @@
RtpExtension("http://google.com/testing/both_audio_and_video", 7),
};
+const RtpExtension kAudioRtpExtensionEncrypted2[] = {
+ RtpExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 2),
+ RtpExtension("http://google.com/testing/audio_something", 13, true),
+ RtpExtension("http://google.com/testing/audio_something_else", 5, true),
+};
+
const RtpExtension kAudioRtpExtension3[] = {
RtpExtension("http://google.com/testing/audio_something", 2),
RtpExtension("http://google.com/testing/both_audio_and_video", 3),
};
-const RtpExtension kAudioRtpExtension3ForEncryption[] = {
- RtpExtension("http://google.com/testing/audio_something", 2),
- // Use RTP extension that supports encryption.
- RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 3),
-};
-
-const RtpExtension kAudioRtpExtension3ForEncryptionOffer[] = {
- RtpExtension("http://google.com/testing/audio_something", 2),
- RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 3),
- RtpExtension("http://google.com/testing/audio_something", 14, true),
- RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 13, true),
-};
-
-const RtpExtension kVideoRtpExtension3ForEncryptionOffer[] = {
- RtpExtension("http://google.com/testing/video_something", 4),
- RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 3),
- RtpExtension("http://google.com/testing/video_something", 12, true),
- RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 13, true),
-};
-
const RtpExtension kAudioRtpExtensionAnswer[] = {
RtpExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 8),
};
const RtpExtension kAudioRtpExtensionEncryptedAnswer[] = {
- RtpExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 12, true),
+ RtpExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 8),
+ RtpExtension("http://google.com/testing/audio_something", 11, true),
};
const RtpExtension kVideoRtpExtension1[] = {
@@ -175,8 +160,6 @@
const RtpExtension kVideoRtpExtensionEncrypted1[] = {
RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 14),
- RtpExtension("http://google.com/testing/video_something", 13),
- RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 9, true),
RtpExtension("http://google.com/testing/video_something", 7, true),
};
@@ -186,23 +169,24 @@
RtpExtension("http://google.com/testing/both_audio_and_video", 7),
};
+const RtpExtension kVideoRtpExtensionEncrypted2[] = {
+ RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 8),
+ RtpExtension("http://google.com/testing/video_something", 10, true),
+ RtpExtension("http://google.com/testing/video_something_else", 4, true),
+};
+
const RtpExtension kVideoRtpExtension3[] = {
RtpExtension("http://google.com/testing/video_something", 4),
RtpExtension("http://google.com/testing/both_audio_and_video", 5),
};
-const RtpExtension kVideoRtpExtension3ForEncryption[] = {
- RtpExtension("http://google.com/testing/video_something", 4),
- // Use RTP extension that supports encryption.
- RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 5),
-};
-
const RtpExtension kVideoRtpExtensionAnswer[] = {
RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 14),
};
const RtpExtension kVideoRtpExtensionEncryptedAnswer[] = {
- RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 9, true),
+ RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 14),
+ RtpExtension("http://google.com/testing/video_something", 7, true),
};
const RtpExtension kRtpExtensionTransportSequenceNumber01[] = {
@@ -602,7 +586,7 @@
std::vector<webrtc::RtpHeaderExtensionCapability> capabilities;
for (const auto& extension : extensions) {
webrtc::RtpHeaderExtensionCapability capability(
- extension.uri, extension.id,
+ extension.uri, extension.id, extension.encrypt,
webrtc::RtpTransceiverDirection::kSendRecv);
capabilities.push_back(capability);
}
@@ -612,8 +596,10 @@
void SetAudioVideoRtpHeaderExtensions(RtpHeaderExtensions audio_exts,
RtpHeaderExtensions video_exts,
MediaSessionOptions* opts) {
- auto audio_caps = HeaderExtensionCapabilitiesFromRtpExtensions(audio_exts);
- auto video_caps = HeaderExtensionCapabilitiesFromRtpExtensions(video_exts);
+ std::vector<webrtc::RtpHeaderExtensionCapability> audio_caps =
+ HeaderExtensionCapabilitiesFromRtpExtensions(audio_exts);
+ std::vector<webrtc::RtpHeaderExtensionCapability> video_caps =
+ HeaderExtensionCapabilitiesFromRtpExtensions(video_exts);
for (auto& entry : opts->media_description_options) {
switch (entry.type) {
case MEDIA_TYPE_AUDIO:
@@ -2042,16 +2028,20 @@
MediaSessionOptions opts;
AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &opts);
+ // TODO: bugs.webrtc.org/358039777 - Reverse all the tests since the default
+ // value will be set to true in the future.
f1_.set_enable_encrypted_rtp_header_extensions(true);
f2_.set_enable_encrypted_rtp_header_extensions(true);
- SetAudioVideoRtpHeaderExtensions(MAKE_VECTOR(kAudioRtpExtension1),
- MAKE_VECTOR(kVideoRtpExtension1), &opts);
+ SetAudioVideoRtpHeaderExtensions(MAKE_VECTOR(kAudioRtpExtensionEncrypted1),
+ MAKE_VECTOR(kVideoRtpExtensionEncrypted1),
+ &opts);
std::unique_ptr<SessionDescription> offer =
f1_.CreateOfferOrError(opts, nullptr).MoveValue();
ASSERT_TRUE(offer.get());
- SetAudioVideoRtpHeaderExtensions(MAKE_VECTOR(kAudioRtpExtension2),
- MAKE_VECTOR(kVideoRtpExtension2), &opts);
+ SetAudioVideoRtpHeaderExtensions(MAKE_VECTOR(kAudioRtpExtensionEncrypted2),
+ MAKE_VECTOR(kVideoRtpExtensionEncrypted2),
+ &opts);
std::unique_ptr<SessionDescription> answer =
f2_.CreateAnswerOrError(offer.get(), opts, nullptr).MoveValue();
@@ -2074,15 +2064,19 @@
MediaSessionOptions opts;
AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &opts);
+ // TODO: bugs.webrtc.org/358039777 - Reverse all the tests since the default
+ // value will be true.
f1_.set_enable_encrypted_rtp_header_extensions(true);
- SetAudioVideoRtpHeaderExtensions(MAKE_VECTOR(kAudioRtpExtension1),
- MAKE_VECTOR(kVideoRtpExtension1), &opts);
+ SetAudioVideoRtpHeaderExtensions(MAKE_VECTOR(kAudioRtpExtensionEncrypted1),
+ MAKE_VECTOR(kVideoRtpExtensionEncrypted1),
+ &opts);
std::unique_ptr<SessionDescription> offer =
f1_.CreateOfferOrError(opts, nullptr).MoveValue();
ASSERT_TRUE(offer.get());
- SetAudioVideoRtpHeaderExtensions(MAKE_VECTOR(kAudioRtpExtension2),
- MAKE_VECTOR(kVideoRtpExtension2), &opts);
+ SetAudioVideoRtpHeaderExtensions(MAKE_VECTOR(kAudioRtpExtensionEncrypted2),
+ MAKE_VECTOR(kVideoRtpExtensionEncrypted2),
+ &opts);
std::unique_ptr<SessionDescription> answer =
f2_.CreateAnswerOrError(offer.get(), opts, nullptr).MoveValue();
@@ -2105,23 +2099,27 @@
MediaSessionOptions opts;
AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &opts);
+ // TODO: bugs.webrtc.org/358039777 - Reverse all the tests since the default
+ // value will be true.
f2_.set_enable_encrypted_rtp_header_extensions(true);
- SetAudioVideoRtpHeaderExtensions(MAKE_VECTOR(kAudioRtpExtension1),
- MAKE_VECTOR(kVideoRtpExtension1), &opts);
+ SetAudioVideoRtpHeaderExtensions(MAKE_VECTOR(kAudioRtpExtensionEncrypted1),
+ MAKE_VECTOR(kVideoRtpExtensionEncrypted1),
+ &opts);
std::unique_ptr<SessionDescription> offer =
f1_.CreateOfferOrError(opts, nullptr).MoveValue();
ASSERT_TRUE(offer.get());
- SetAudioVideoRtpHeaderExtensions(MAKE_VECTOR(kAudioRtpExtension2),
- MAKE_VECTOR(kVideoRtpExtension2), &opts);
+ SetAudioVideoRtpHeaderExtensions(MAKE_VECTOR(kAudioRtpExtensionEncrypted2),
+ MAKE_VECTOR(kVideoRtpExtensionEncrypted2),
+ &opts);
std::unique_ptr<SessionDescription> answer =
f2_.CreateAnswerOrError(offer.get(), opts, nullptr).MoveValue();
EXPECT_EQ(
- MAKE_VECTOR(kAudioRtpExtension1),
+ MAKE_VECTOR(kAudioRtpExtensionAnswer),
GetFirstAudioContentDescription(offer.get())->rtp_header_extensions());
EXPECT_EQ(
- MAKE_VECTOR(kVideoRtpExtension1),
+ MAKE_VECTOR(kVideoRtpExtensionAnswer),
GetFirstVideoContentDescription(offer.get())->rtp_header_extensions());
EXPECT_EQ(
MAKE_VECTOR(kAudioRtpExtensionAnswer),
@@ -3512,39 +3510,6 @@
->rtp_header_extensions());
}
-// Same as "RtpExtensionIdReused" above for encrypted RTP extensions.
-TEST_F(MediaSessionDescriptionFactoryTest, RtpExtensionIdReusedEncrypted) {
- MediaSessionOptions opts;
- AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &opts);
-
- f1_.set_enable_encrypted_rtp_header_extensions(true);
- f2_.set_enable_encrypted_rtp_header_extensions(true);
-
- SetAudioVideoRtpHeaderExtensions(
- MAKE_VECTOR(kAudioRtpExtension3ForEncryption),
- MAKE_VECTOR(kVideoRtpExtension3ForEncryption), &opts);
- std::unique_ptr<SessionDescription> offer =
- f1_.CreateOfferOrError(opts, nullptr).MoveValue();
-
- EXPECT_EQ(
- MAKE_VECTOR(kAudioRtpExtension3ForEncryptionOffer),
- GetFirstAudioContentDescription(offer.get())->rtp_header_extensions());
- EXPECT_EQ(
- MAKE_VECTOR(kVideoRtpExtension3ForEncryptionOffer),
- GetFirstVideoContentDescription(offer.get())->rtp_header_extensions());
-
- // Nothing should change when creating a new offer
- std::unique_ptr<SessionDescription> updated_offer(
- f1_.CreateOfferOrError(opts, offer.get()).MoveValue());
-
- EXPECT_EQ(MAKE_VECTOR(kAudioRtpExtension3ForEncryptionOffer),
- GetFirstAudioContentDescription(updated_offer.get())
- ->rtp_header_extensions());
- EXPECT_EQ(MAKE_VECTOR(kVideoRtpExtension3ForEncryptionOffer),
- GetFirstVideoContentDescription(updated_offer.get())
- ->rtp_header_extensions());
-}
-
TEST(MediaSessionDescription, CopySessionDescription) {
SessionDescription source;
ContentGroup group(CN_AUDIO);
