admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Improve SDP negotiation for mixed encrypted/unencrypted offers.

Browse Source 
According to RFC 6904 a header extension MAY be offered both encrypted and unecrypted. In the case when encryption is enabled the encrypted version SHOULD be used and vice versa. However, this is under the assumption that both peers actually offer the same extension header both encrypted and unecrypted. With this PR we tighten the negotiation rules to the encryption option SHOULD be the same both in the sender and receiver in order to not drop the extension. Especially, see test `TestOfferAnswerPreferEncryptedRtpHeaderExtensionsWhenEncryptionEnabled` and `TestOfferAnswerPreferEncryptedRtpHeaderExtensionsWhenEncryptionDisabled`.

Bug: chromium:40623740
Change-Id: I68c65a776fcf7be97aaf60a797594c4361a06800
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/363940
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Emil Vardar (xWF) <vardar@google.com>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43191}
This commit is contained in:
Emil Vardar 2024-10-08 11:24:59 +00:00 committed by WebRTC LUCI CQ
parent fec6f8d09d
commit fb4311660b
2 changed files with 153 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
pc/media_session.cc
View File

@ -399,7 +399,8 @@ RTCError CreateContentOffer(
RtpHeaderExtensions extensions;
for (auto extension_with_id : rtp_extensions) {
for (const auto& extension : media_description_options.header_extensions) {
if (extension_with_id.uri == extension.uri) {
if (extension_with_id.uri == extension.uri &&
extension_with_id.encrypt == extension.preferred_encrypt) {
// TODO(crbug.com/1051821): Configure the extension direction from
// the information in the media_description_options extension
// capability.
@ -993,7 +994,7 @@ void NegotiateRtpHeaderExtensions(const RtpHeaderExtensions& local_extensions,
const webrtc::RtpExtension* theirs =
FindHeaderExtensionByUriDiscardUnsupported(offered_extensions, ours.uri,
filter);
if (theirs) {
if (theirs && theirs->encrypt == ours.encrypt) {
// We respond with their RTP header extension id.
negotiated_extensions->push_back(*theirs);
}
@ -1080,7 +1081,8 @@ bool CreateMediaContentAnswer(
RtpHeaderExtensions local_rtp_extensions_to_reply_with;
for (auto extension_with_id : local_rtp_extensions) {
for (const auto& extension : media_description_options.header_extensions) {
if (extension_with_id.uri == extension.uri) {
if (extension_with_id.uri == extension.uri &&
extension_with_id.encrypt == extension.preferred_encrypt) {
// TODO(crbug.com/1051821): Configure the extension direction from
// the information in the media_description_options extension
// capability. For now, do not include stopped extensions.

187
pc/media_session_unittest.cc
View File

@ -143,6 +143,22 @@ const RtpExtension kAudioRtpExtension3[] = {
RtpExtension("http://google.com/testing/both_audio_and_video", 3),
};
const RtpExtension kAudioRtpExtensionMixedEncryption1[] = {
RtpExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 8),
RtpExtension("http://google.com/testing/audio_something", 9),
RtpExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 10, true),
RtpExtension("http://google.com/testing/audio_something", 11, true),
RtpExtension("http://google.com/testing/audio_something_else", 12, true),
};
const RtpExtension kAudioRtpExtensionMixedEncryption2[] = {
RtpExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 5),
RtpExtension("http://google.com/testing/audio_something", 6),
RtpExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 7, true),
RtpExtension("http://google.com/testing/audio_something", 8, true),
RtpExtension("http://google.com/testing/audio_something_else", 9),
};
const RtpExtension kAudioRtpExtensionAnswer[] = {
RtpExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 8),
};
@ -152,6 +168,18 @@ const RtpExtension kAudioRtpExtensionEncryptedAnswer[] = {
RtpExtension("http://google.com/testing/audio_something", 11, true),
};
const RtpExtension kAudioRtpExtensionMixedEncryptionAnswerEncryptionEnabled[] =
{
RtpExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 10, true),
RtpExtension("http://google.com/testing/audio_something", 11, true),
};
const RtpExtension kAudioRtpExtensionMixedEncryptionAnswerEncryptionDisabled[] =
{
RtpExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 8),
RtpExtension("http://google.com/testing/audio_something", 9),
};
const RtpExtension kVideoRtpExtension1[] = {
RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 14),
RtpExtension("http://google.com/testing/video_something", 13),
@ -179,6 +207,13 @@ const RtpExtension kVideoRtpExtension3[] = {
RtpExtension("http://google.com/testing/both_audio_and_video", 5),
};
const RtpExtension kVideoRtpExtensionMixedEncryption[] = {
RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 14),
RtpExtension("http://google.com/testing/video_something", 13),
RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 15, true),
RtpExtension("http://google.com/testing/video_something", 16, true),
};
const RtpExtension kVideoRtpExtensionAnswer[] = {
RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 14),
};
@ -188,6 +223,18 @@ const RtpExtension kVideoRtpExtensionEncryptedAnswer[] = {
RtpExtension("http://google.com/testing/video_something", 7, true),
};
const RtpExtension kVideoRtpExtensionMixedEncryptionAnswerEncryptionEnabled[] =
{
RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 15, true),
RtpExtension("http://google.com/testing/video_something", 16, true),
};
const RtpExtension kVideoRtpExtensionMixedEncryptionAnswerEncryptionDisabled[] =
{
RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 14),
RtpExtension("http://google.com/testing/video_something", 13),
};
const RtpExtension kRtpExtensionTransportSequenceNumber01[] = {
RtpExtension("http://www.ietf.org/id/"
"draft-holmer-rmcat-transport-wide-cc-extensions-01",
@ -1596,36 +1643,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, AudioOfferAnswerWithCryptoDisabled) {
EXPECT_EQ(kMediaProtocolAvpf, answer_acd->protocol());
}
// Create a video offer and answer and ensure the RTP header extensions
// matches what we expect.
TEST_F(MediaSessionDescriptionFactoryTest, TestOfferAnswerWithRtpExtensions) {
MediaSessionOptions opts;
AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &opts);
SetAudioVideoRtpHeaderExtensions(MAKE_VECTOR(kAudioRtpExtension1),
MAKE_VECTOR(kVideoRtpExtension1), &opts);
std::unique_ptr<SessionDescription> offer =
f1_.CreateOfferOrError(opts, nullptr).MoveValue();
ASSERT_TRUE(offer.get());
SetAudioVideoRtpHeaderExtensions(MAKE_VECTOR(kAudioRtpExtension2),
MAKE_VECTOR(kVideoRtpExtension2), &opts);
std::unique_ptr<SessionDescription> answer =
f2_.CreateAnswerOrError(offer.get(), opts, nullptr).MoveValue();
EXPECT_EQ(
MAKE_VECTOR(kAudioRtpExtension1),
GetFirstAudioContentDescription(offer.get())->rtp_header_extensions());
EXPECT_EQ(
MAKE_VECTOR(kVideoRtpExtension1),
GetFirstVideoContentDescription(offer.get())->rtp_header_extensions());
EXPECT_EQ(
MAKE_VECTOR(kAudioRtpExtensionAnswer),
GetFirstAudioContentDescription(answer.get())->rtp_header_extensions());
EXPECT_EQ(
MAKE_VECTOR(kVideoRtpExtensionAnswer),
GetFirstVideoContentDescription(answer.get())->rtp_header_extensions());
}
// Create a audio/video offer and answer and ensure that the
// TransportSequenceNumber RTP v1 and v2 header extensions are handled
// correctly.
@ -2041,13 +2058,42 @@ TEST_F(MediaSessionDescriptionFactoryTest,
ElementsAre(Field(&RtpExtension::uri, "uri1")))))));
}
// Create a video offer and answer and ensure the RTP header extensions
// matches what we expect.
TEST_F(MediaSessionDescriptionFactoryTest,
TestOfferAnswerWithEncryptedRtpExtensionsBoth) {
TestOfferAnswerWithRtpExtensionHeadersWithNoEncryption) {
MediaSessionOptions opts;
AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &opts);
SetAudioVideoRtpHeaderExtensions(MAKE_VECTOR(kAudioRtpExtension1),
MAKE_VECTOR(kVideoRtpExtension1), &opts);
std::unique_ptr<SessionDescription> offer =
f1_.CreateOfferOrError(opts, nullptr).MoveValue();
ASSERT_TRUE(offer.get());
SetAudioVideoRtpHeaderExtensions(MAKE_VECTOR(kAudioRtpExtension2),
MAKE_VECTOR(kVideoRtpExtension2), &opts);
std::unique_ptr<SessionDescription> answer =
f2_.CreateAnswerOrError(offer.get(), opts, nullptr).MoveValue();
EXPECT_EQ(
MAKE_VECTOR(kAudioRtpExtension1),
GetFirstAudioContentDescription(offer.get())->rtp_header_extensions());
EXPECT_EQ(
MAKE_VECTOR(kVideoRtpExtension1),
GetFirstVideoContentDescription(offer.get())->rtp_header_extensions());
EXPECT_EQ(
MAKE_VECTOR(kAudioRtpExtensionAnswer),
GetFirstAudioContentDescription(answer.get())->rtp_header_extensions());
EXPECT_EQ(
MAKE_VECTOR(kVideoRtpExtensionAnswer),
GetFirstVideoContentDescription(answer.get())->rtp_header_extensions());
}
TEST_F(MediaSessionDescriptionFactoryTest,
TestOfferAnswerWithRtpExtensionHeadersWithEncryption) {
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);
@ -2078,12 +2124,10 @@ TEST_F(MediaSessionDescriptionFactoryTest,
}
TEST_F(MediaSessionDescriptionFactoryTest,
TestOfferAnswerWithEncryptedRtpExtensionsOffer) {
TestOfferAnswerWithEncryptedRtpExtensionHeadersEnabledInOffer) {
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(kAudioRtpExtensionEncrypted1),
@ -2113,12 +2157,10 @@ TEST_F(MediaSessionDescriptionFactoryTest,
}
TEST_F(MediaSessionDescriptionFactoryTest,
TestOfferAnswerWithEncryptedRtpExtensionsAnswer) {
TestOfferAnswerWithEncryptedRtpExtensionHeadersEnabledInReceiver) {
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(kAudioRtpExtensionEncrypted1),
@ -2147,6 +2189,73 @@ TEST_F(MediaSessionDescriptionFactoryTest,
GetFirstVideoContentDescription(answer.get())->rtp_header_extensions());
}
TEST_F(MediaSessionDescriptionFactoryTest,
TestOfferAnswerPreferEncryptedRtpHeaderExtensionsWhenEncryptionEnabled) {
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(kAudioRtpExtensionMixedEncryption1),
MAKE_VECTOR(kVideoRtpExtensionMixedEncryption), &opts);
std::unique_ptr<SessionDescription> offer =
f1_.CreateOfferOrError(opts, nullptr).MoveValue();
ASSERT_TRUE(offer.get());
SetAudioVideoRtpHeaderExtensions(
MAKE_VECTOR(kAudioRtpExtensionMixedEncryption2),
MAKE_VECTOR(kVideoRtpExtensionMixedEncryption), &opts);
std::unique_ptr<SessionDescription> answer =
f2_.CreateAnswerOrError(offer.get(), opts, nullptr).MoveValue();
ASSERT_TRUE(answer.get());
EXPECT_EQ(
MAKE_VECTOR(kAudioRtpExtensionMixedEncryption1),
GetFirstAudioContentDescription(offer.get())->rtp_header_extensions());
EXPECT_EQ(
MAKE_VECTOR(kVideoRtpExtensionMixedEncryption),
GetFirstVideoContentDescription(offer.get())->rtp_header_extensions());
EXPECT_EQ(
MAKE_VECTOR(kAudioRtpExtensionMixedEncryptionAnswerEncryptionEnabled),
GetFirstAudioContentDescription(answer.get())->rtp_header_extensions());
EXPECT_EQ(
MAKE_VECTOR(kVideoRtpExtensionMixedEncryptionAnswerEncryptionEnabled),
GetFirstVideoContentDescription(answer.get())->rtp_header_extensions());
}
TEST_F(MediaSessionDescriptionFactoryTest,
TestOfferAnswerUseUnencryptedRtpHeaderExtensionsWhenEncryptionDisabled) {
MediaSessionOptions opts;
AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &opts);
SetAudioVideoRtpHeaderExtensions(
MAKE_VECTOR(kAudioRtpExtensionMixedEncryption1),
MAKE_VECTOR(kVideoRtpExtensionMixedEncryption), &opts);
std::unique_ptr<SessionDescription> offer =
f1_.CreateOfferOrError(opts, nullptr).MoveValue();
ASSERT_TRUE(offer.get());
SetAudioVideoRtpHeaderExtensions(
MAKE_VECTOR(kAudioRtpExtensionMixedEncryption2),
MAKE_VECTOR(kVideoRtpExtensionMixedEncryption), &opts);
std::unique_ptr<SessionDescription> answer =
f2_.CreateAnswerOrError(offer.get(), opts, nullptr).MoveValue();
ASSERT_TRUE(answer.get());
EXPECT_EQ(
MAKE_VECTOR(kAudioRtpExtensionMixedEncryptionAnswerEncryptionDisabled),
GetFirstAudioContentDescription(offer.get())->rtp_header_extensions());
EXPECT_EQ(
MAKE_VECTOR(kVideoRtpExtensionMixedEncryptionAnswerEncryptionDisabled),
GetFirstVideoContentDescription(offer.get())->rtp_header_extensions());
EXPECT_EQ(
MAKE_VECTOR(kAudioRtpExtensionMixedEncryptionAnswerEncryptionDisabled),
GetFirstAudioContentDescription(answer.get())->rtp_header_extensions());
EXPECT_EQ(
MAKE_VECTOR(kVideoRtpExtensionMixedEncryptionAnswerEncryptionDisabled),
GetFirstVideoContentDescription(answer.get())->rtp_header_extensions());
}
// Create an audio, video, data answer without legacy StreamParams.
TEST_F(MediaSessionDescriptionFactoryTest,
TestCreateAnswerWithoutLegacyStreams) {