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

Reintroduce enable_dtls_srtp option

Browse Source 
This is a partial revert of commit f9e502d935fa9ca8d9cc7540680021b93d285f28.

Reason for revert: Functionality turns out to be needed by some partners for some months more.

Original change's description:
> Remove enable_dtls_srtp option
>
> This is part of the removal of support for SDES.
>
> Bug: webrtc:11066
> Change-Id: I448d0e0032672c04c87b00550ab4b9d792071a0b
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/234864
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#35262}

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: webrtc:11066, chromium:1271469
Change-Id: I79a90f025e53816789b391bc52a0e896b9be87e1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/238170
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35378}
This commit is contained in:
Harald Alvestrand 2021-11-18 10:01:06 +00:00 committed by WebRTC LUCI CQ
parent 1dfce5d413
commit 50b95525c7
5 changed files with 221 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
api/peer_connection_interface.h
View File

@ -432,6 +432,12 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface {
// Use new combined audio/video bandwidth estimation?
absl::optional<bool> combined_audio_video_bwe;
// TODO(bugs.webrtc.org/9891) - Move to crypto_options
// Can be used to disable DTLS-SRTP. This should never be done, but can be
// useful for testing purposes, for example in setting up a loopback call
// with a single PeerConnection.
absl::optional<bool> enable_dtls_srtp;
/////////////////////////////////////////////////
// The below fields are not part of the standard.
/////////////////////////////////////////////////

6
pc/peer_connection.cc
View File

@ -276,8 +276,8 @@ bool DtlsEnabled(const PeerConnectionInterface::RTCConfiguration& configuration,
bool default_enabled =
(dependencies.cert_generator || !configuration.certificates.empty());
RTC_DCHECK(default_enabled) << "Configuration error: No certs for DTLS";
return default_enabled;
// The `configuration` can override the default value.
return configuration.enable_dtls_srtp.value_or(default_enabled);
}
} // namespace
@ -300,6 +300,7 @@ bool PeerConnectionInterface::RTCConfiguration::operator==(
bool enable_rtp_data_channel;
absl::optional<int> screencast_min_bitrate;
absl::optional<bool> combined_audio_video_bwe;
absl::optional<bool> enable_dtls_srtp;
TcpCandidatePolicy tcp_candidate_policy;
CandidateNetworkPolicy candidate_network_policy;
int audio_jitter_buffer_max_packets;
@ -367,6 +368,7 @@ bool PeerConnectionInterface::RTCConfiguration::operator==(
disable_link_local_networks == o.disable_link_local_networks &&
screencast_min_bitrate == o.screencast_min_bitrate &&
combined_audio_video_bwe == o.combined_audio_video_bwe &&
enable_dtls_srtp == o.enable_dtls_srtp &&
ice_candidate_pool_size == o.ice_candidate_pool_size &&
prune_turn_ports == o.prune_turn_ports &&
turn_port_prune_policy == o.turn_port_prune_policy &&

180
pc/peer_connection_crypto_unittest.cc
View File

@ -181,6 +181,7 @@ SdpContentMutator RemoveDtlsFingerprint() {
// no SDES cryptos.
TEST_P(PeerConnectionCryptoTest, CorrectCryptoInOfferWhenDtlsEnabled) {
RTCConfiguration config;
config.enable_dtls_srtp.emplace(true);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto offer = caller->CreateOffer();
@ -194,6 +195,7 @@ TEST_P(PeerConnectionCryptoTest, CorrectCryptoInOfferWhenDtlsEnabled) {
}
TEST_P(PeerConnectionCryptoTest, CorrectCryptoInAnswerWhenDtlsEnabled) {
RTCConfiguration config;
config.enable_dtls_srtp.emplace(true);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto callee = CreatePeerConnectionWithAudioVideo(config);
@ -208,6 +210,39 @@ TEST_P(PeerConnectionCryptoTest, CorrectCryptoInAnswerWhenDtlsEnabled) {
answer->description()));
}
// When DTLS is disabled, the SDP offer/answer should include SDES cryptos and
// should not have a DTLS fingerprint.
TEST_P(PeerConnectionCryptoTest, CorrectCryptoInOfferWhenDtlsDisabled) {
RTCConfiguration config;
config.enable_dtls_srtp.emplace(false);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto offer = caller->CreateOffer();
ASSERT_TRUE(offer);
ASSERT_FALSE(offer->description()->contents().empty());
EXPECT_TRUE(SdpContentsAll(HaveSdesCryptos(), offer->description()));
EXPECT_TRUE(SdpContentsNone(HaveDtlsFingerprint(), offer->description()));
EXPECT_TRUE(SdpContentsAll(HaveProtocol(cricket::kMediaProtocolSavpf),
offer->description()));
}
TEST_P(PeerConnectionCryptoTest, CorrectCryptoInAnswerWhenDtlsDisabled) {
RTCConfiguration config;
config.enable_dtls_srtp.emplace(false);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto callee = CreatePeerConnectionWithAudioVideo(config);
callee->SetRemoteDescription(caller->CreateOffer());
auto answer = callee->CreateAnswer();
ASSERT_TRUE(answer);
ASSERT_FALSE(answer->description()->contents().empty());
EXPECT_TRUE(SdpContentsAll(HaveSdesCryptos(), answer->description()));
EXPECT_TRUE(SdpContentsNone(HaveDtlsFingerprint(), answer->description()));
EXPECT_TRUE(SdpContentsAll(HaveProtocol(cricket::kMediaProtocolSavpf),
answer->description()));
}
// When encryption is disabled, the SDP offer/answer should have neither a DTLS
// fingerprint nor any SDES crypto options.
TEST_P(PeerConnectionCryptoTest, CorrectCryptoInOfferWhenEncryptionDisabled) {
@ -216,6 +251,7 @@ TEST_P(PeerConnectionCryptoTest, CorrectCryptoInOfferWhenEncryptionDisabled) {
pc_factory_->SetOptions(options);
RTCConfiguration config;
config.enable_dtls_srtp.emplace(false);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto offer = caller->CreateOffer();
@ -233,6 +269,7 @@ TEST_P(PeerConnectionCryptoTest, CorrectCryptoInAnswerWhenEncryptionDisabled) {
pc_factory_->SetOptions(options);
RTCConfiguration config;
config.enable_dtls_srtp.emplace(false);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto callee = CreatePeerConnectionWithAudioVideo(config);
@ -247,12 +284,80 @@ TEST_P(PeerConnectionCryptoTest, CorrectCryptoInAnswerWhenEncryptionDisabled) {
answer->description()));
}
// CryptoOptions has been promoted to RTCConfiguration. As such if it is ever
// set in the configuration it should overrite the settings set in the factory.
TEST_P(PeerConnectionCryptoTest, RTCConfigurationCryptoOptionOverridesFactory) {
PeerConnectionFactoryInterface::Options options;
options.crypto_options.srtp.enable_gcm_crypto_suites = true;
pc_factory_->SetOptions(options);
RTCConfiguration config;
config.enable_dtls_srtp.emplace(false);
CryptoOptions crypto_options;
crypto_options.srtp.enable_gcm_crypto_suites = false;
config.crypto_options = crypto_options;
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto offer = caller->CreateOffer();
ASSERT_TRUE(offer);
ASSERT_FALSE(offer->description()->contents().empty());
// This should exist if GCM is enabled see CorrectCryptoInOfferWithSdesAndGcm
EXPECT_FALSE(SdpContentsAll(HaveSdesGcmCryptos(3), offer->description()));
}
// When DTLS is disabled and GCM cipher suites are enabled, the SDP offer/answer
// should have the correct ciphers in the SDES crypto options.
// With GCM cipher suites enabled, there will be 3 cryptos in the offer and 1
// in the answer.
TEST_P(PeerConnectionCryptoTest, CorrectCryptoInOfferWithSdesAndGcm) {
PeerConnectionFactoryInterface::Options options;
options.crypto_options.srtp.enable_gcm_crypto_suites = true;
pc_factory_->SetOptions(options);
RTCConfiguration config;
config.enable_dtls_srtp.emplace(false);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto offer = caller->CreateOffer();
ASSERT_TRUE(offer);
ASSERT_FALSE(offer->description()->contents().empty());
EXPECT_TRUE(SdpContentsAll(HaveSdesGcmCryptos(3), offer->description()));
}
TEST_P(PeerConnectionCryptoTest, CorrectCryptoInAnswerWithSdesAndGcm) {
PeerConnectionFactoryInterface::Options options;
options.crypto_options.srtp.enable_gcm_crypto_suites = true;
pc_factory_->SetOptions(options);
RTCConfiguration config;
config.enable_dtls_srtp.emplace(false);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto callee = CreatePeerConnectionWithAudioVideo(config);
auto offer = caller->CreateOffer();
for (cricket::ContentInfo& content : offer->description()->contents()) {
auto cryptos = content.media_description()->cryptos();
cryptos.erase(cryptos.begin()); // Assumes that non-GCM is the default.
content.media_description()->set_cryptos(cryptos);
}
callee->SetRemoteDescription(std::move(offer));
auto answer = callee->CreateAnswer();
ASSERT_TRUE(answer);
ASSERT_FALSE(answer->description()->contents().empty());
EXPECT_TRUE(SdpContentsAll(HaveSdesGcmCryptos(1), answer->description()));
}
TEST_P(PeerConnectionCryptoTest, CanSetSdesGcmRemoteOfferAndLocalAnswer) {
PeerConnectionFactoryInterface::Options options;
options.crypto_options.srtp.enable_gcm_crypto_suites = true;
pc_factory_->SetOptions(options);
RTCConfiguration config;
config.enable_dtls_srtp.emplace(false);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto callee = CreatePeerConnectionWithAudioVideo(config);
@ -263,8 +368,69 @@ TEST_P(PeerConnectionCryptoTest, CanSetSdesGcmRemoteOfferAndLocalAnswer) {
auto answer = callee->CreateAnswer();
ASSERT_TRUE(answer);
ASSERT_TRUE(callee->SetLocalDescription(std::move(answer)));
// Note - this test doesn't verify that Gcm is present, just that it
// does not caue a failure.
}
// The following group tests that two PeerConnections can successfully exchange
// an offer/answer when DTLS is off and that they will refuse any offer/answer
// applied locally/remotely if it does not include SDES cryptos.
TEST_P(PeerConnectionCryptoTest, ExchangeOfferAnswerWhenSdesOn) {
RTCConfiguration config;
config.enable_dtls_srtp.emplace(false);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto callee = CreatePeerConnectionWithAudioVideo(config);
auto offer = caller->CreateOfferAndSetAsLocal();
ASSERT_TRUE(offer);
ASSERT_TRUE(callee->SetRemoteDescription(std::move(offer)));
auto answer = callee->CreateAnswerAndSetAsLocal();
ASSERT_TRUE(answer);
ASSERT_TRUE(caller->SetRemoteDescription(std::move(answer)));
}
TEST_P(PeerConnectionCryptoTest, FailToSetLocalOfferWithNoCryptosWhenSdesOn) {
RTCConfiguration config;
config.enable_dtls_srtp.emplace(false);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto offer = caller->CreateOffer();
SdpContentsForEach(RemoveSdesCryptos(), offer->description());
EXPECT_FALSE(caller->SetLocalDescription(std::move(offer)));
}
TEST_P(PeerConnectionCryptoTest, FailToSetRemoteOfferWithNoCryptosWhenSdesOn) {
RTCConfiguration config;
config.enable_dtls_srtp.emplace(false);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto callee = CreatePeerConnectionWithAudioVideo(config);
auto offer = caller->CreateOffer();
SdpContentsForEach(RemoveSdesCryptos(), offer->description());
EXPECT_FALSE(callee->SetRemoteDescription(std::move(offer)));
}
TEST_P(PeerConnectionCryptoTest, FailToSetLocalAnswerWithNoCryptosWhenSdesOn) {
RTCConfiguration config;
config.enable_dtls_srtp.emplace(false);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto callee = CreatePeerConnectionWithAudioVideo(config);
callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal());
auto answer = callee->CreateAnswer();
SdpContentsForEach(RemoveSdesCryptos(), answer->description());
EXPECT_FALSE(callee->SetLocalDescription(std::move(answer)));
}
TEST_P(PeerConnectionCryptoTest, FailToSetRemoteAnswerWithNoCryptosWhenSdesOn) {
RTCConfiguration config;
config.enable_dtls_srtp.emplace(false);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto callee = CreatePeerConnectionWithAudioVideo(config);
callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal());
auto answer = callee->CreateAnswerAndSetAsLocal();
SdpContentsForEach(RemoveSdesCryptos(), answer->description());
EXPECT_FALSE(caller->SetRemoteDescription(std::move(answer)));
}
// The following group tests that two PeerConnections can successfully exchange
@ -272,6 +438,7 @@ TEST_P(PeerConnectionCryptoTest, CanSetSdesGcmRemoteOfferAndLocalAnswer) {
// applied locally/remotely if it does not include a DTLS fingerprint.
TEST_P(PeerConnectionCryptoTest, ExchangeOfferAnswerWhenDtlsOn) {
RTCConfiguration config;
config.enable_dtls_srtp.emplace(true);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto callee = CreatePeerConnectionWithAudioVideo(config);
@ -286,6 +453,7 @@ TEST_P(PeerConnectionCryptoTest, ExchangeOfferAnswerWhenDtlsOn) {
TEST_P(PeerConnectionCryptoTest,
FailToSetLocalOfferWithNoFingerprintWhenDtlsOn) {
RTCConfiguration config;
config.enable_dtls_srtp.emplace(true);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto offer = caller->CreateOffer();
@ -296,6 +464,7 @@ TEST_P(PeerConnectionCryptoTest,
TEST_P(PeerConnectionCryptoTest,
FailToSetRemoteOfferWithNoFingerprintWhenDtlsOn) {
RTCConfiguration config;
config.enable_dtls_srtp.emplace(true);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto callee = CreatePeerConnectionWithAudioVideo(config);
@ -307,6 +476,7 @@ TEST_P(PeerConnectionCryptoTest,
TEST_P(PeerConnectionCryptoTest,
FailToSetLocalAnswerWithNoFingerprintWhenDtlsOn) {
RTCConfiguration config;
config.enable_dtls_srtp.emplace(true);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto callee = CreatePeerConnectionWithAudioVideo(config);
@ -317,6 +487,7 @@ TEST_P(PeerConnectionCryptoTest,
TEST_P(PeerConnectionCryptoTest,
FailToSetRemoteAnswerWithNoFingerprintWhenDtlsOn) {
RTCConfiguration config;
config.enable_dtls_srtp.emplace(true);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto callee = CreatePeerConnectionWithAudioVideo(config);
@ -334,6 +505,7 @@ TEST_P(PeerConnectionCryptoTest, ExchangeOfferAnswerWhenNoEncryption) {
pc_factory_->SetOptions(options);
RTCConfiguration config;
config.enable_dtls_srtp.emplace(false);
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto callee = CreatePeerConnectionWithAudioVideo(config);
@ -351,11 +523,13 @@ TEST_P(PeerConnectionCryptoTest, ExchangeOfferAnswerWhenNoEncryption) {
TEST_P(PeerConnectionCryptoTest,
ExchangeOfferAnswerWhenDtlsCertificateInConfig) {
RTCConfiguration caller_config;
caller_config.enable_dtls_srtp.emplace(true);
caller_config.certificates.push_back(
FakeRTCCertificateGenerator::GenerateCertificate());
auto caller = CreatePeerConnectionWithAudioVideo(caller_config);
RTCConfiguration callee_config;
callee_config.enable_dtls_srtp.emplace(true);
callee_config.certificates.push_back(
FakeRTCCertificateGenerator::GenerateCertificate());
auto callee = CreatePeerConnectionWithAudioVideo(callee_config);
@ -426,6 +600,7 @@ class PeerConnectionCryptoDtlsCertGenTest
TEST_P(PeerConnectionCryptoDtlsCertGenTest, TestCertificateGeneration) {
RTCConfiguration config;
config.enable_dtls_srtp.emplace(true);
auto owned_fake_certificate_generator =
std::make_unique<FakeRTCCertificateGenerator>();
auto* fake_certificate_generator = owned_fake_certificate_generator.get();
@ -549,6 +724,7 @@ TEST_P(PeerConnectionCryptoTest, SessionErrorIfFingerprintInvalid) {
auto caller = CreatePeerConnectionWithAudioVideo();
RTCConfiguration callee_config;
callee_config.enable_dtls_srtp.emplace(true);
callee_config.certificates.push_back(callee_certificate);
auto callee = CreatePeerConnectionWithAudioVideo(callee_config);

9
pc/peer_connection_end_to_end_unittest.cc
View File

@ -366,6 +366,15 @@ TEST_P(PeerConnectionEndToEndTest, Call) {
WaitForCallEstablished();
}
TEST_P(PeerConnectionEndToEndTest, CallWithSdesKeyNegotiation) {
config_.enable_dtls_srtp = false;
CreatePcs(webrtc::CreateOpusAudioEncoderFactory(),
webrtc::CreateOpusAudioDecoderFactory());
GetAndAddUserMedia();
Negotiate();
WaitForCallEstablished();
}
TEST_P(PeerConnectionEndToEndTest, CallWithCustomCodec) {
class IdLoggingAudioEncoderFactory : public webrtc::AudioEncoderFactory {
public:

24
pc/peer_connection_integrationtest.cc
View File

@ -264,6 +264,30 @@ TEST_P(PeerConnectionIntegrationTest, EndToEndCallWithDtls) {
webrtc::kEnumCounterKeyProtocolSdes));
}
// Uses SDES instead of DTLS for key agreement.
TEST_P(PeerConnectionIntegrationTest, EndToEndCallWithSdes) {
PeerConnectionInterface::RTCConfiguration sdes_config;
sdes_config.enable_dtls_srtp.emplace(false);
ASSERT_TRUE(CreatePeerConnectionWrappersWithConfig(sdes_config, sdes_config));
ConnectFakeSignaling();
// Do normal offer/answer and wait for some frames to be received in each
// direction.
caller()->AddAudioVideoTracks();
callee()->AddAudioVideoTracks();
caller()->CreateAndSetAndSignalOffer();
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
MediaExpectations media_expectations;
media_expectations.ExpectBidirectionalAudioAndVideo();
ASSERT_TRUE(ExpectNewFrames(media_expectations));
EXPECT_METRIC_LE(
2, webrtc::metrics::NumEvents("WebRTC.PeerConnection.KeyProtocol",
webrtc::kEnumCounterKeyProtocolSdes));
EXPECT_METRIC_EQ(
0, webrtc::metrics::NumEvents("WebRTC.PeerConnection.KeyProtocol",
webrtc::kEnumCounterKeyProtocolDtls));
}
// Basic end-to-end test specifying the `enable_encrypted_rtp_header_extensions`
// option to offer encrypted versions of all header extensions alongside the
// unencrypted versions.