diff --git a/api/rtp_parameters.cc b/api/rtp_parameters.cc index 132c888bd3..8a18f8983f 100644 --- a/api/rtp_parameters.cc +++ b/api/rtp_parameters.cc @@ -170,115 +170,63 @@ bool RtpExtension::IsSupportedForVideo(absl::string_view uri) { } bool RtpExtension::IsEncryptionSupported(absl::string_view uri) { - 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 && + 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 || #endif - uri != webrtc::RtpExtension::kEncryptHeaderExtensionsUri; -} - -// 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& extensions, - absl::string_view uri) { - for (const auto& extension : extensions) { - if (extension.uri == uri) { - return true; - } - } - return false; + 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; } const RtpExtension* RtpExtension::FindHeaderExtensionByUri( const std::vector& extensions, - absl::string_view uri, - Filter filter) { - const webrtc::RtpExtension* fallback_extension = nullptr; + absl::string_view uri) { 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& extensions, - absl::string_view uri, - bool encrypt) { - for (const auto& extension : extensions) { - if (extension.uri == uri && extension.encrypt == encrypt) { + if (extension.uri == uri) { return &extension; } } return nullptr; } -const std::vector RtpExtension::DeduplicateHeaderExtensions( - const std::vector& extensions, - Filter filter) { +std::vector RtpExtension::FilterDuplicateNonEncrypted( + const std::vector& extensions) { std::vector filtered; + for (auto extension = extensions.begin(); extension != extensions.end(); + ++extension) { + if (extension->encrypt) { + filtered.push_back(*extension); + continue; + } - // 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); - } + // 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 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 5764d9739c..7fe9f2bc83 100644 --- a/api/rtp_parameters.h +++ b/api/rtp_parameters.h @@ -246,18 +246,6 @@ struct RTC_EXPORT RtpHeaderExtensionCapability { // 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); @@ -272,23 +260,17 @@ struct RTC_EXPORT RtpExtension { // Return "true" if the given RTP header extension URI may be encrypted. static bool IsEncryptionSupported(absl::string_view uri); - // Returns the header extension with the given URI or nullptr if not found. + // Returns the named header extension if found among all extensions, + // nullptr otherwise. static const RtpExtension* FindHeaderExtensionByUri( const std::vector& extensions, - absl::string_view uri, - Filter filter); + absl::string_view uri); - // Returns the header extension with the given URI and encrypt parameter, - // if found, otherwise nullptr. - static const RtpExtension* FindHeaderExtensionByUriAndEncryption( - const std::vector& extensions, - absl::string_view uri, - bool encrypt); - - // Returns a list of extensions where any extension URI is unique. - static const std::vector DeduplicateHeaderExtensions( - const std::vector& extensions, - 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 FilterDuplicateNonEncrypted( + const std::vector& extensions); // 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 51ad426748..5928cbda63 100644 --- a/api/rtp_parameters_unittest.cc +++ b/api/rtp_parameters_unittest.cc @@ -23,249 +23,28 @@ static const RtpExtension kExtension1(kExtensionUri1, 1); static const RtpExtension kExtension1Encrypted(kExtensionUri1, 10, true); static const RtpExtension kExtension2(kExtensionUri2, 2); -TEST(RtpExtensionTest, DeduplicateHeaderExtensions) { +TEST(RtpExtensionTest, FilterDuplicateNonEncrypted) { std::vector extensions; std::vector filtered; - extensions.clear(); extensions.push_back(kExtension1); extensions.push_back(kExtension1Encrypted); - filtered = RtpExtension::DeduplicateHeaderExtensions( - extensions, RtpExtension::Filter::kDiscardEncryptedExtension); - EXPECT_EQ(1u, filtered.size()); - EXPECT_EQ(std::vector{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{kExtension1Encrypted}, filtered); - - extensions.clear(); - extensions.push_back(kExtension1); - extensions.push_back(kExtension1Encrypted); - filtered = RtpExtension::DeduplicateHeaderExtensions( - extensions, RtpExtension::Filter::kRequireEncryptedExtension); + filtered = RtpExtension::FilterDuplicateNonEncrypted(extensions); EXPECT_EQ(1u, filtered.size()); EXPECT_EQ(std::vector{kExtension1Encrypted}, filtered); extensions.clear(); extensions.push_back(kExtension1Encrypted); extensions.push_back(kExtension1); - filtered = RtpExtension::DeduplicateHeaderExtensions( - extensions, RtpExtension::Filter::kDiscardEncryptedExtension); - EXPECT_EQ(1u, filtered.size()); - EXPECT_EQ(std::vector{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{kExtension1Encrypted}, filtered); - - extensions.clear(); - extensions.push_back(kExtension1Encrypted); - extensions.push_back(kExtension1); - filtered = RtpExtension::DeduplicateHeaderExtensions( - extensions, RtpExtension::Filter::kRequireEncryptedExtension); + filtered = RtpExtension::FilterDuplicateNonEncrypted(extensions); EXPECT_EQ(1u, filtered.size()); EXPECT_EQ(std::vector{kExtension1Encrypted}, filtered); extensions.clear(); extensions.push_back(kExtension1); extensions.push_back(kExtension2); - filtered = RtpExtension::DeduplicateHeaderExtensions( - extensions, RtpExtension::Filter::kDiscardEncryptedExtension); + filtered = RtpExtension::FilterDuplicateNonEncrypted(extensions); 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{kExtension1, kExtension2}), filtered); - filtered = RtpExtension::DeduplicateHeaderExtensions( - extensions, RtpExtension::Filter::kPreferEncryptedExtension); - EXPECT_EQ(2u, filtered.size()); - EXPECT_EQ((std::vector{kExtension1Encrypted, kExtension2}), - filtered); - filtered = RtpExtension::DeduplicateHeaderExtensions( - extensions, RtpExtension::Filter::kRequireEncryptedExtension); - EXPECT_EQ(1u, filtered.size()); - EXPECT_EQ((std::vector{kExtension1Encrypted}), filtered); -} - -TEST(RtpExtensionTest, FindHeaderExtensionByUriAndEncryption) { - std::vector 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 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 diff --git a/modules/rtp_rtcp/source/rtp_packet.cc b/modules/rtp_rtcp/source/rtp_packet.cc index a8134fd124..84769d0f4b 100644 --- a/modules/rtp_rtcp/source/rtp_packet.cc +++ b/modules/rtp_rtcp/source/rtp_packet.cc @@ -27,7 +27,6 @@ constexpr size_t kFixedHeaderSize = 12; constexpr uint8_t kRtpVersion = 2; constexpr uint16_t kOneByteExtensionProfileId = 0xBEDE; constexpr uint16_t kTwoByteExtensionProfileId = 0x1000; -constexpr uint16_t kTwobyteExtensionProfileIdAppBitsFilter = 0xfff0; constexpr size_t kOneByteExtensionHeaderLength = 1; constexpr size_t kTwoByteExtensionHeaderLength = 2; constexpr size_t kDefaultPacketSize = 1500; @@ -502,8 +501,7 @@ bool RtpPacket::ParseBuffer(const uint8_t* buffer, size_t size) { return false; } if (profile != kOneByteExtensionProfileId && - (profile & kTwobyteExtensionProfileIdAppBitsFilter) != - kTwoByteExtensionProfileId) { + profile != kTwoByteExtensionProfileId) { RTC_LOG(LS_WARNING) << "Unsupported rtp extension " << profile; } else { size_t extension_header_length = profile == kOneByteExtensionProfileId diff --git a/pc/channel.cc b/pc/channel.cc index 872dc5693c..f37be6716b 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -772,12 +772,18 @@ bool BaseChannel::UpdateRemoteStreams_w( return ret; } -RtpHeaderExtensions BaseChannel::GetDeduplicatedRtpHeaderExtensions( +RtpHeaderExtensions BaseChannel::GetFilteredRtpHeaderExtensions( const RtpHeaderExtensions& extensions) { - return webrtc::RtpExtension::DeduplicateHeaderExtensions( - extensions, crypto_options_.srtp.enable_encrypted_rtp_header_extensions - ? webrtc::RtpExtension::kPreferEncryptedExtension - : webrtc::RtpExtension::kDiscardEncryptedExtension); + if (crypto_options_.srtp.enable_encrypted_rtp_header_extensions) { + RtpHeaderExtensions filtered; + absl::c_copy_if(extensions, std::back_inserter(filtered), + [](const webrtc::RtpExtension& extension) { + return !extension.encrypt; + }); + return filtered; + } + + return webrtc::RtpExtension::FilterDuplicateNonEncrypted(extensions); } void BaseChannel::OnMessage(rtc::Message* pmsg) { @@ -905,7 +911,7 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content, SetNegotiatedHeaderExtensions_w(audio->rtp_header_extensions()); RtpHeaderExtensions rtp_header_extensions = - GetDeduplicatedRtpHeaderExtensions(audio->rtp_header_extensions()); + GetFilteredRtpHeaderExtensions(audio->rtp_header_extensions()); UpdateRtpHeaderExtensionMap(rtp_header_extensions); media_channel()->SetExtmapAllowMixed(audio->extmap_allow_mixed()); @@ -972,7 +978,7 @@ bool VoiceChannel::SetRemoteContent_w(const MediaContentDescription* content, SetNegotiatedHeaderExtensions_w(audio->rtp_header_extensions()); RtpHeaderExtensions rtp_header_extensions = - GetDeduplicatedRtpHeaderExtensions(audio->rtp_header_extensions()); + GetFilteredRtpHeaderExtensions(audio->rtp_header_extensions()); AudioSendParameters send_params = last_send_params_; RtpSendParametersFromMediaDescription( @@ -1083,7 +1089,7 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, SetNegotiatedHeaderExtensions_w(video->rtp_header_extensions()); RtpHeaderExtensions rtp_header_extensions = - GetDeduplicatedRtpHeaderExtensions(video->rtp_header_extensions()); + GetFilteredRtpHeaderExtensions(video->rtp_header_extensions()); UpdateRtpHeaderExtensionMap(rtp_header_extensions); media_channel()->SetExtmapAllowMixed(video->extmap_allow_mixed()); @@ -1183,7 +1189,7 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content, SetNegotiatedHeaderExtensions_w(video->rtp_header_extensions()); RtpHeaderExtensions rtp_header_extensions = - GetDeduplicatedRtpHeaderExtensions(video->rtp_header_extensions()); + GetFilteredRtpHeaderExtensions(video->rtp_header_extensions()); VideoSendParameters send_params = last_send_params_; RtpSendParametersFromMediaDescription( @@ -1339,7 +1345,7 @@ bool RtpDataChannel::SetLocalContent_w(const MediaContentDescription* content, const RtpDataContentDescription* data = content->as_rtp_data(); RtpHeaderExtensions rtp_header_extensions = - GetDeduplicatedRtpHeaderExtensions(data->rtp_header_extensions()); + GetFilteredRtpHeaderExtensions(data->rtp_header_extensions()); DataRecvParameters recv_params = last_recv_params_; RtpParametersFromMediaDescription( @@ -1407,7 +1413,7 @@ bool RtpDataChannel::SetRemoteContent_w(const MediaContentDescription* content, } RtpHeaderExtensions rtp_header_extensions = - GetDeduplicatedRtpHeaderExtensions(data->rtp_header_extensions()); + GetFilteredRtpHeaderExtensions(data->rtp_header_extensions()); RTC_LOG(LS_INFO) << "Setting remote data description for " << ToString(); DataSendParameters send_params = last_send_params_; diff --git a/pc/channel.h b/pc/channel.h index ab8b9b8624..fe3778b6cd 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -274,11 +274,10 @@ class BaseChannel : public ChannelInterface, webrtc::SdpType type, std::string* error_desc) RTC_RUN_ON(worker_thread()) = 0; - - // Returns a list of RTP header extensions where any extension URI is unique. - // Encrypted extensions will be either preferred or discarded, depending on - // the current crypto_options_. - RtpHeaderExtensions GetDeduplicatedRtpHeaderExtensions( + // Return a list of RTP header extensions with the non-encrypted extensions + // removed depending on the current crypto_options_ and only if both the + // non-encrypted and encrypted extension is present for the same URI. + RtpHeaderExtensions GetFilteredRtpHeaderExtensions( const RtpHeaderExtensions& extensions); // From MessageHandler diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index 1632a37467..312b1280b1 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -899,10 +899,7 @@ int JsepTransportController::GetRtpAbsSendTimeHeaderExtensionId( const webrtc::RtpExtension* send_time_extension = webrtc::RtpExtension::FindHeaderExtensionByUri( content_desc->rtp_header_extensions(), - webrtc::RtpExtension::kAbsSendTimeUri, - config_.crypto_options.srtp.enable_encrypted_rtp_header_extensions - ? webrtc::RtpExtension::kPreferEncryptedExtension - : webrtc::RtpExtension::kDiscardEncryptedExtension); + webrtc::RtpExtension::kAbsSendTimeUri); return send_time_extension ? send_time_extension->id : -1; } diff --git a/pc/media_session.cc b/pc/media_session.cc index e159808ba3..6374f17be1 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -988,6 +988,68 @@ static Codecs MatchCodecPreference( return filtered_codecs; } +static bool FindByUriAndEncryption(const RtpHeaderExtensions& extensions, + const webrtc::RtpExtension& ext_to_match, + webrtc::RtpExtension* found_extension) { + auto it = absl::c_find_if( + extensions, [&ext_to_match](const webrtc::RtpExtension& extension) { + // We assume that all URIs are given in a canonical + // format. + return extension.uri == ext_to_match.uri && + extension.encrypt == ext_to_match.encrypt; + }); + if (it == extensions.end()) { + return false; + } + if (found_extension) { + *found_extension = *it; + } + return true; +} + +static bool FindByUri(const RtpHeaderExtensions& extensions, + const webrtc::RtpExtension& ext_to_match, + webrtc::RtpExtension* found_extension) { + // We assume that all URIs are given in a canonical format. + const webrtc::RtpExtension* found = + webrtc::RtpExtension::FindHeaderExtensionByUri(extensions, + ext_to_match.uri); + if (!found) { + return false; + } + if (found_extension) { + *found_extension = *found; + } + return true; +} + +static bool FindByUriWithEncryptionPreference( + const RtpHeaderExtensions& extensions, + absl::string_view uri_to_match, + bool encryption_preference, + webrtc::RtpExtension* found_extension) { + const webrtc::RtpExtension* unencrypted_extension = nullptr; + for (const webrtc::RtpExtension& extension : extensions) { + // We assume that all URIs are given in a canonical format. + if (extension.uri == uri_to_match) { + if (!encryption_preference || extension.encrypt) { + if (found_extension) { + *found_extension = extension; + } + return true; + } + unencrypted_extension = &extension; + } + } + if (unencrypted_extension) { + if (found_extension) { + *found_extension = *unencrypted_extension; + } + return true; + } + return false; +} + // Adds all extensions from |reference_extensions| to |offered_extensions| that // don't already exist in |offered_extensions| and ensure the IDs don't // collide. If an extension is added, it's also added to |regular_extensions| or @@ -1002,28 +1064,22 @@ static void MergeRtpHdrExts(const RtpHeaderExtensions& reference_extensions, RtpHeaderExtensions* encrypted_extensions, UsedRtpHeaderExtensionIds* used_ids) { for (auto reference_extension : reference_extensions) { - if (!webrtc::RtpExtension::FindHeaderExtensionByUriAndEncryption( - *offered_extensions, reference_extension.uri, - reference_extension.encrypt)) { + if (!FindByUriAndEncryption(*offered_extensions, reference_extension, + nullptr)) { + webrtc::RtpExtension existing; if (reference_extension.encrypt) { - const webrtc::RtpExtension* existing = - webrtc::RtpExtension::FindHeaderExtensionByUriAndEncryption( - *encrypted_extensions, reference_extension.uri, - reference_extension.encrypt); - if (existing) { - offered_extensions->push_back(*existing); + if (FindByUriAndEncryption(*encrypted_extensions, reference_extension, + &existing)) { + offered_extensions->push_back(existing); } else { used_ids->FindAndSetIdUsed(&reference_extension); encrypted_extensions->push_back(reference_extension); offered_extensions->push_back(reference_extension); } } else { - const webrtc::RtpExtension* existing = - webrtc::RtpExtension::FindHeaderExtensionByUriAndEncryption( - *regular_extensions, reference_extension.uri, - reference_extension.encrypt); - if (existing) { - offered_extensions->push_back(*existing); + if (FindByUriAndEncryption(*regular_extensions, reference_extension, + &existing)) { + offered_extensions->push_back(existing); } else { used_ids->FindAndSetIdUsed(&reference_extension); regular_extensions->push_back(reference_extension); @@ -1034,86 +1090,41 @@ static void MergeRtpHdrExts(const RtpHeaderExtensions& reference_extensions, } } -static 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) { +static void AddEncryptedVersionsOfHdrExts(RtpHeaderExtensions* extensions, + RtpHeaderExtensions* all_extensions, + UsedRtpHeaderExtensionIds* used_ids) { + RtpHeaderExtensions encrypted_extensions; + for (const webrtc::RtpExtension& extension : *extensions) { + webrtc::RtpExtension existing; + // Don't add encrypted extensions again that were already included in a + // previous offer or regular extensions that are also included as encrypted + // extensions. + if (extension.encrypt || + !webrtc::RtpExtension::IsEncryptionSupported(extension.uri) || + (FindByUriWithEncryptionPreference(*extensions, extension.uri, true, + &existing) && + existing.encrypt)) { continue; } - // Skip if we cannot encrypt the extension - if (!webrtc::RtpExtension::IsEncryptionSupported(extension.uri)) { - continue; + if (FindByUri(*all_extensions, extension, &existing)) { + encrypted_extensions.push_back(existing); + } else { + webrtc::RtpExtension encrypted(extension); + encrypted.encrypt = true; + used_ids->FindAndSetIdUsed(&encrypted); + all_extensions->push_back(encrypted); + encrypted_extensions.push_back(encrypted); } - - // 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. -static const webrtc::RtpExtension* FindHeaderExtensionByUriDiscardUnsupported( - const std::vector& extensions, - absl::string_view uri, - webrtc::RtpExtension::Filter filter) { - // Note: While it's technically possible to decrypt extensions that we don't - // encrypt, the symmetric API of libsrtp does not allow us to supply - // different IDs for encryption/decryption of header extensions depending on - // whether the packet is inbound or outbound. Thereby, we are limited to - // what we can send in encrypted form. - if (!webrtc::RtpExtension::IsEncryptionSupported(uri)) { - // If there's no encryption support and we only want encrypted extensions, - // there's no point in continuing the search here. - if (filter == webrtc::RtpExtension::kRequireEncryptedExtension) { - return nullptr; - } - - // Instruct to only return non-encrypted extensions - filter = webrtc::RtpExtension::Filter::kDiscardEncryptedExtension; - } - - return webrtc::RtpExtension::FindHeaderExtensionByUri(extensions, uri, - filter); + extensions->insert(extensions->end(), encrypted_extensions.begin(), + encrypted_extensions.end()); } static void NegotiateRtpHeaderExtensions( const RtpHeaderExtensions& local_extensions, const RtpHeaderExtensions& offered_extensions, - webrtc::RtpExtension::Filter filter, + bool enable_encrypted_rtp_header_extensions, RtpHeaderExtensions* negotiated_extensions) { // TransportSequenceNumberV2 is not offered by default. The special logic for // the TransportSequenceNumber extensions works as follows: @@ -1122,9 +1133,9 @@ static void NegotiateRtpHeaderExtensions( // V1 and V2 V2 regardless of local_extensions. // V2 V2 regardless of local_extensions. const webrtc::RtpExtension* transport_sequence_number_v2_offer = - FindHeaderExtensionByUriDiscardUnsupported( + webrtc::RtpExtension::FindHeaderExtensionByUri( offered_extensions, - webrtc::RtpExtension::kTransportSequenceNumberV2Uri, filter); + webrtc::RtpExtension::kTransportSequenceNumberV2Uri); bool frame_descriptor_in_local = false; bool dependency_descriptor_in_local = false; @@ -1137,10 +1148,10 @@ static void NegotiateRtpHeaderExtensions( dependency_descriptor_in_local = true; else if (ours.uri == webrtc::RtpExtension::kAbsoluteCaptureTimeUri) abs_capture_time_in_local = true; - const webrtc::RtpExtension* theirs = - FindHeaderExtensionByUriDiscardUnsupported(offered_extensions, ours.uri, - filter); - if (theirs) { + webrtc::RtpExtension theirs; + if (FindByUriWithEncryptionPreference( + offered_extensions, ours.uri, + enable_encrypted_rtp_header_extensions, &theirs)) { if (transport_sequence_number_v2_offer && ours.uri == webrtc::RtpExtension::kTransportSequenceNumberUri) { // Don't respond to @@ -1150,7 +1161,7 @@ static void NegotiateRtpHeaderExtensions( continue; } else { // We respond with their RTP header extension id. - negotiated_extensions->push_back(*theirs); + negotiated_extensions->push_back(theirs); } } } @@ -1162,35 +1173,28 @@ static void NegotiateRtpHeaderExtensions( // Frame descriptors support. If the extension is not present locally, but is // in the offer, we add it to the list. - if (!dependency_descriptor_in_local) { - const webrtc::RtpExtension* theirs = - FindHeaderExtensionByUriDiscardUnsupported( - offered_extensions, webrtc::RtpExtension::kDependencyDescriptorUri, - filter); - if (theirs) { - negotiated_extensions->push_back(*theirs); - } + webrtc::RtpExtension theirs; + if (!dependency_descriptor_in_local && + FindByUriWithEncryptionPreference( + offered_extensions, webrtc::RtpExtension::kDependencyDescriptorUri, + enable_encrypted_rtp_header_extensions, &theirs)) { + negotiated_extensions->push_back(theirs); } - if (!frame_descriptor_in_local) { - const webrtc::RtpExtension* theirs = - FindHeaderExtensionByUriDiscardUnsupported( - offered_extensions, - webrtc::RtpExtension::kGenericFrameDescriptorUri00, filter); - if (theirs) { - negotiated_extensions->push_back(*theirs); - } + if (!frame_descriptor_in_local && + FindByUriWithEncryptionPreference( + offered_extensions, + webrtc::RtpExtension::kGenericFrameDescriptorUri00, + enable_encrypted_rtp_header_extensions, &theirs)) { + negotiated_extensions->push_back(theirs); } // Absolute capture time support. If the extension is not present locally, but // is in the offer, we add it to the list. - if (!abs_capture_time_in_local) { - const webrtc::RtpExtension* theirs = - FindHeaderExtensionByUriDiscardUnsupported( - offered_extensions, webrtc::RtpExtension::kAbsoluteCaptureTimeUri, - filter); - if (theirs) { - negotiated_extensions->push_back(*theirs); - } + if (!abs_capture_time_in_local && + FindByUriWithEncryptionPreference( + offered_extensions, webrtc::RtpExtension::kAbsoluteCaptureTimeUri, + enable_encrypted_rtp_header_extensions, &theirs)) { + negotiated_extensions->push_back(theirs); } } @@ -1245,14 +1249,10 @@ static bool CreateMediaContentAnswer( bool bundle_enabled, MediaContentDescription* answer) { answer->set_extmap_allow_mixed_enum(offer->extmap_allow_mixed_enum()); - const webrtc::RtpExtension::Filter extensions_filter = - enable_encrypted_rtp_header_extensions - ? webrtc::RtpExtension::Filter::kPreferEncryptedExtension - : webrtc::RtpExtension::Filter::kDiscardEncryptedExtension; RtpHeaderExtensions negotiated_rtp_extensions; - NegotiateRtpHeaderExtensions(local_rtp_extensions, - offer->rtp_header_extensions(), - extensions_filter, &negotiated_rtp_extensions); + NegotiateRtpHeaderExtensions( + local_rtp_extensions, offer->rtp_header_extensions(), + enable_encrypted_rtp_header_extensions, &negotiated_rtp_extensions); answer->set_rtp_header_extensions(negotiated_rtp_extensions); answer->set_rtcp_mux(session_options.rtcp_mux_enabled && offer->rtcp_mux()); diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc index 745e9e495a..940d746e5f 100644 --- a/pc/media_session_unittest.cc +++ b/pc/media_session_unittest.cc @@ -151,7 +151,6 @@ static 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), }; static const RtpExtension kAudioRtpExtension2[] = { @@ -174,15 +173,7 @@ static const RtpExtension kAudioRtpExtension3ForEncryption[] = { static 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), -}; - -static 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), + RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 14, true), }; static const RtpExtension kAudioRtpExtensionAnswer[] = { @@ -201,8 +192,7 @@ static const RtpExtension kVideoRtpExtension1[] = { static 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), + RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 11, true), }; static const RtpExtension kVideoRtpExtension2[] = { @@ -227,7 +217,7 @@ static const RtpExtension kVideoRtpExtensionAnswer[] = { }; static const RtpExtension kVideoRtpExtensionEncryptedAnswer[] = { - RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 9, true), + RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 11, true), }; static const RtpExtension kRtpExtensionTransportSequenceNumber01[] = { @@ -3708,11 +3698,19 @@ TEST_F(MediaSessionDescriptionFactoryTest, RtpExtensionIdReusedEncrypted) { MAKE_VECTOR(kVideoRtpExtension3ForEncryption), &opts); std::unique_ptr offer = f1_.CreateOffer(opts, NULL); + // The extensions that are shared between audio and video should use the same + // id. + const RtpExtension kExpectedVideoRtpExtension[] = { + kVideoRtpExtension3ForEncryption[0], + kAudioRtpExtension3ForEncryptionOffer[1], + kAudioRtpExtension3ForEncryptionOffer[2], + }; + EXPECT_EQ( MAKE_VECTOR(kAudioRtpExtension3ForEncryptionOffer), GetFirstAudioContentDescription(offer.get())->rtp_header_extensions()); EXPECT_EQ( - MAKE_VECTOR(kVideoRtpExtension3ForEncryptionOffer), + MAKE_VECTOR(kExpectedVideoRtpExtension), GetFirstVideoContentDescription(offer.get())->rtp_header_extensions()); // Nothing should change when creating a new offer @@ -3722,7 +3720,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, RtpExtensionIdReusedEncrypted) { EXPECT_EQ(MAKE_VECTOR(kAudioRtpExtension3ForEncryptionOffer), GetFirstAudioContentDescription(updated_offer.get()) ->rtp_header_extensions()); - EXPECT_EQ(MAKE_VECTOR(kVideoRtpExtension3ForEncryptionOffer), + EXPECT_EQ(MAKE_VECTOR(kExpectedVideoRtpExtension), GetFirstVideoContentDescription(updated_offer.get()) ->rtp_header_extensions()); } diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index 833173a00d..53770eaa50 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -529,17 +529,13 @@ static RTCError UpdateSimulcastLayerStatusInSender( static bool SimulcastIsRejected( const ContentInfo* local_content, - const MediaContentDescription& answer_media_desc, - bool enable_encrypted_rtp_header_extensions) { + const MediaContentDescription& answer_media_desc) { bool simulcast_offered = local_content && local_content->media_description() && local_content->media_description()->HasSimulcast(); bool simulcast_answered = answer_media_desc.HasSimulcast(); bool rids_supported = RtpExtension::FindHeaderExtensionByUri( - answer_media_desc.rtp_header_extensions(), RtpExtension::kRidUri, - enable_encrypted_rtp_header_extensions - ? RtpExtension::Filter::kPreferEncryptedExtension - : RtpExtension::Filter::kDiscardEncryptedExtension); + answer_media_desc.rtp_header_extensions(), RtpExtension::kRidUri); return simulcast_offered && (!simulcast_answered || !rids_supported); } @@ -3300,9 +3296,7 @@ SdpOfferAnswerHandler::AssociateTransceiver( // Check if the offer indicated simulcast but the answer rejected it. // This can happen when simulcast is not supported on the remote party. - if (SimulcastIsRejected(old_local_content, *media_desc, - pc_->GetCryptoOptions() - .srtp.enable_encrypted_rtp_header_extensions)) { + if (SimulcastIsRejected(old_local_content, *media_desc)) { RTC_HISTOGRAM_BOOLEAN(kSimulcastDisabled, true); RTCError error = DisableSimulcastInSender(transceiver->internal()->sender_internal()); diff --git a/pc/session_description.h b/pc/session_description.h index dde404956d..dacf1a2174 100644 --- a/pc/session_description.h +++ b/pc/session_description.h @@ -150,11 +150,6 @@ class MediaContentDescription { cryptos_ = cryptos; } - // List of RTP header extensions. URIs are **NOT** guaranteed to be unique - // as they can appear twice when both encrypted and non-encrypted extensions - // are present. - // Use RtpExtension::FindHeaderExtensionByUri for finding and - // RtpExtension::DeduplicateHeaderExtensions for filtering. virtual const RtpHeaderExtensions& rtp_header_extensions() const { return rtp_header_extensions_; } diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc index 087ac3cb99..91c246c66e 100644 --- a/video/video_send_stream.cc +++ b/video/video_send_stream.cc @@ -65,10 +65,7 @@ VideoStreamEncoder::BitrateAllocationCallbackType GetBitrateAllocationCallbackType(const VideoSendStream::Config& config) { if (webrtc::RtpExtension::FindHeaderExtensionByUri( config.rtp.extensions, - webrtc::RtpExtension::kVideoLayersAllocationUri, - config.crypto_options.srtp.enable_encrypted_rtp_header_extensions - ? RtpExtension::Filter::kPreferEncryptedExtension - : RtpExtension::Filter::kDiscardEncryptedExtension)) { + webrtc::RtpExtension::kVideoLayersAllocationUri)) { return VideoStreamEncoder::BitrateAllocationCallbackType:: kVideoLayersAllocation; }