diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index ee947f0ad2..efe84fbe4a 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -432,6 +432,12 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface { // Use new combined audio/video bandwidth estimation? absl::optional 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 enable_dtls_srtp; + ///////////////////////////////////////////////// // The below fields are not part of the standard. ///////////////////////////////////////////////// diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 95a251ecf5..cc8dc6dc24 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -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 screencast_min_bitrate; absl::optional combined_audio_video_bwe; + absl::optional 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 && diff --git a/pc/peer_connection_crypto_unittest.cc b/pc/peer_connection_crypto_unittest.cc index c0c328161a..394203cb02 100644 --- a/pc/peer_connection_crypto_unittest.cc +++ b/pc/peer_connection_crypto_unittest.cc @@ -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(); 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); diff --git a/pc/peer_connection_end_to_end_unittest.cc b/pc/peer_connection_end_to_end_unittest.cc index 19e4be33ff..4ef4c832bb 100644 --- a/pc/peer_connection_end_to_end_unittest.cc +++ b/pc/peer_connection_end_to_end_unittest.cc @@ -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: diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc index 53aa63efd9..fc094161af 100644 --- a/pc/peer_connection_integrationtest.cc +++ b/pc/peer_connection_integrationtest.cc @@ -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.